feat(canonical): validate profile name, apply env-var substitution before render

This commit is contained in:
2026-05-23 11:55:26 +01:00
parent 5e35dc47fe
commit 50059d7517
14 changed files with 423 additions and 23 deletions
+74
View File
@@ -0,0 +1,74 @@
package canonical
import (
"fmt"
"github.com/lukaszraczylo/harness-sync/internal/secrets"
)
// SubstituteSecrets walks the bundle and replaces ${VAR} placeholders in all
// config fields using lookup. Skill/agent/instruction bodies are intentionally
// NOT substituted — they are content, not config.
//
// Returns an error (wrapping the offending field path) if any referenced
// variable is missing from lookup.
func SubstituteSecrets(b *Bundle, lookup secrets.Lookup) error {
var err error
// Gateway
if b.Profile.Gateway.Token, err = sub(lookup, "profile.gateway.token", b.Profile.Gateway.Token); err != nil {
return err
}
if b.Profile.Gateway.URL, err = sub(lookup, "profile.gateway.url", b.Profile.Gateway.URL); err != nil {
return err
}
if b.Profile.Gateway.DefaultModel, err = sub(lookup, "profile.gateway.default_model", b.Profile.Gateway.DefaultModel); err != nil {
return err
}
// Upstreams
for i := range b.Profile.Upstreams {
u := &b.Profile.Upstreams[i]
field := fmt.Sprintf("profile.upstreams[%d](%s)", i, u.Name)
if u.APIKey, err = sub(lookup, field+".api_key", u.APIKey); err != nil {
return err
}
if u.BaseURL, err = sub(lookup, field+".base_url", u.BaseURL); err != nil {
return err
}
}
// MCP servers
for i := range b.MCP.Servers {
sv := &b.MCP.Servers[i]
field := fmt.Sprintf("mcp.servers[%d](%s)", i, sv.Name)
if sv.Command, err = sub(lookup, field+".command", sv.Command); err != nil {
return err
}
if sv.URL, err = sub(lookup, field+".url", sv.URL); err != nil {
return err
}
for j, arg := range sv.Args {
if sv.Args[j], err = sub(lookup, fmt.Sprintf("%s.args[%d]", field, j), arg); err != nil {
return err
}
}
for k, v := range sv.Env {
if sv.Env[k], err = sub(lookup, fmt.Sprintf("%s.env[%s]", field, k), v); err != nil {
return err
}
}
}
return nil
}
// sub applies secrets.Substitute and wraps errors with field context.
func sub(lookup secrets.Lookup, field, value string) (string, error) {
result, err := secrets.Substitute(value, lookup)
if err != nil {
return "", fmt.Errorf("%s: %w", field, err)
}
return result, nil
}
+101
View File
@@ -0,0 +1,101 @@
package canonical
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// lookup returns env values from the provided map; missing keys return false.
func mapLookup(m map[string]string) func(string) (string, bool) {
return func(name string) (string, bool) {
v, ok := m[name]
return v, ok
}
}
func TestSubstituteSecretsGatewayToken(t *testing.T) {
b := &Bundle{
Profile: Profile{
Name: "test",
Gateway: Gateway{
Token: "${MY_TOKEN}",
URL: "https://gw.lan",
},
},
}
err := SubstituteSecrets(b, mapLookup(map[string]string{"MY_TOKEN": "secret123"}))
require.NoError(t, err)
assert.Equal(t, "secret123", b.Profile.Gateway.Token)
assert.Equal(t, "https://gw.lan", b.Profile.Gateway.URL)
}
func TestSubstituteSecretsUpstreamAPIKey(t *testing.T) {
b := &Bundle{
Profile: Profile{
Name: "test",
Upstreams: []Upstream{
{Name: "openai", APIKey: "${OPENAI_KEY}", BaseURL: "https://api.openai.com"},
},
},
}
err := SubstituteSecrets(b, mapLookup(map[string]string{"OPENAI_KEY": "sk-abc"}))
require.NoError(t, err)
assert.Equal(t, "sk-abc", b.Profile.Upstreams[0].APIKey)
assert.Equal(t, "https://api.openai.com", b.Profile.Upstreams[0].BaseURL)
}
func TestSubstituteSecretsMCPEnv(t *testing.T) {
b := &Bundle{
MCP: MCPRegistry{
Servers: []MCPServer{
{Name: "myserver", Env: map[string]string{"API_KEY": "${SRV_KEY}"}},
},
},
}
err := SubstituteSecrets(b, mapLookup(map[string]string{"SRV_KEY": "srv-secret"}))
require.NoError(t, err)
assert.Equal(t, "srv-secret", b.MCP.Servers[0].Env["API_KEY"])
}
func TestSubstituteSecretsMCPArgs(t *testing.T) {
b := &Bundle{
MCP: MCPRegistry{
Servers: []MCPServer{
{Name: "myserver", Args: []string{"--token", "${MCP_TOKEN}", "--plain"}},
},
},
}
err := SubstituteSecrets(b, mapLookup(map[string]string{"MCP_TOKEN": "tok"}))
require.NoError(t, err)
assert.Equal(t, []string{"--token", "tok", "--plain"}, b.MCP.Servers[0].Args)
}
func TestSubstituteSecretsMissingVarErrors(t *testing.T) {
b := &Bundle{
Profile: Profile{
Name: "test",
Gateway: Gateway{
Token: "${MISSING_TOKEN}",
},
},
}
err := SubstituteSecrets(b, mapLookup(map[string]string{}))
require.Error(t, err)
assert.Contains(t, err.Error(), "MISSING_TOKEN")
assert.Contains(t, err.Error(), "profile.gateway.token")
}
func TestSubstituteSecretsSkillBodyUntouched(t *testing.T) {
b := &Bundle{
Profile: Profile{Name: "test"},
Skills: []Skill{
{Name: "mys", Body: "Use ${FOO} in your prompt."},
},
}
err := SubstituteSecrets(b, mapLookup(map[string]string{}))
require.NoError(t, err)
// skill body must be unchanged — ${FOO} is content, not config
assert.Equal(t, "Use ${FOO} in your prompt.", b.Skills[0].Body)
}
@@ -0,0 +1 @@
active_profile: [bad yaml: {
@@ -0,0 +1,2 @@
enabled_harnesses: [claude-code]
active_profile: nonexistent
@@ -0,0 +1,2 @@
enabled_harnesses: [claude-code]
active_profile: noname
@@ -0,0 +1,5 @@
gateway:
url: https://gw.lan
token: dummy
models:
- id: claude-sonnet-4-6
+42 -3
View File
@@ -13,13 +13,25 @@ import (
"github.com/lukaszraczylo/harness-sync/internal/gitx"
)
// isTerminal returns true when os.Stdin is an interactive terminal.
// Used to decide whether to show the first-run prompt.
func isTerminal() bool {
fi, err := os.Stdin.Stat()
if err != nil {
return false
}
return (fi.Mode() & os.ModeCharDevice) != 0
}
// NewApply returns the `apply` subcommand. It renders the canonical bundle
// to each detected (or selected) harness using the apply pipeline.
func NewApply(reg *adapter.Registry) *cobra.Command {
var (
dryRun bool
force bool
root string
dryRun bool
force bool
root string
allowIncomplete bool
yes bool
)
cmd := &cobra.Command{
Use: "apply [harness...]",
@@ -37,6 +49,31 @@ func NewApply(reg *adapter.Registry) *cobra.Command {
return err
}
// Issue 3: refuse apply when gateway is incomplete.
if !allowIncomplete && (b.Profile.Gateway.URL == "" || b.Profile.Gateway.DefaultModel == "") {
return fmt.Errorf("profile %q is incomplete (gateway.url or gateway.default_model is empty); edit %s/profiles/%s.yaml then re-run",
b.Profile.Name, root, b.Profile.Name)
}
// Issue 5: first-time apply warning (state/ doesn't exist yet).
if !dryRun {
statePath := filepath.Join(root, "state")
if _, statErr := os.Stat(statePath); os.IsNotExist(statErr) {
if !yes && isTerminal() {
_, _ = fmt.Fprintf(cmd.OutOrStdout(),
"First-time apply to detected harnesses. harness-sync will:\n"+
" - move existing target files to %s/backups/<harness>/\n"+
" - replace them with symlinks to %s/skills, %s/agents\n"+
" - render LLM configs from the active profile\n\n"+
"Continue? [y/N] ", root, root, root)
var answer string
if _, scanErr := fmt.Fscan(cmd.InOrStdin(), &answer); scanErr != nil || (answer != "y" && answer != "Y") {
return fmt.Errorf("aborted")
}
}
}
}
var selected []adapter.Adapter
if len(args) == 0 {
for _, a := range reg.All() {
@@ -87,5 +124,7 @@ func NewApply(reg *adapter.Registry) *cobra.Command {
cmd.Flags().BoolVar(&dryRun, "dry-run", false, "print actions without writing")
cmd.Flags().BoolVar(&force, "force", false, "overwrite without 3-way merge")
cmd.Flags().StringVar(&root, "root", "", "canonical root (default ~/.config/harness-sync)")
cmd.Flags().BoolVar(&allowIncomplete, "allow-incomplete", false, "apply even when gateway.url or gateway.default_model is empty (for testing)")
cmd.Flags().BoolVar(&yes, "yes", false, "skip first-run confirmation prompt")
return cmd
}
+40
View File
@@ -30,3 +30,43 @@ func TestApplyDryRun(t *testing.T) {
assert.Contains(t, buf.String(), "dry-run")
}
// TestApplyRefusesIncompleteProfile: apply must error when gateway.url or
// gateway.default_model is empty.
func TestApplyRefusesIncompleteProfile(t *testing.T) {
root := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(root, "config.yaml"), []byte("active_profile: bad\n"), 0o600))
require.NoError(t, os.MkdirAll(filepath.Join(root, "profiles"), 0o750))
// Profile with empty url and default_model.
require.NoError(t, os.WriteFile(filepath.Join(root, "profiles", "bad.yaml"),
[]byte("name: bad\ngateway:\n url: \"\"\n token: dummy\n default_model: \"\"\nmodels: []\n"), 0o600))
reg := adapter.NewRegistry()
reg.Register(&detectableAdapter{name: "stub", detect: true})
cmd := NewApply(reg)
cmd.SetArgs([]string{"--root", root})
err := cmd.Execute()
require.Error(t, err)
assert.Contains(t, err.Error(), "incomplete")
}
// TestApplyAllowIncompleteFlag: --allow-incomplete bypasses the gateway check.
func TestApplyAllowIncompleteFlag(t *testing.T) {
root := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(root, "config.yaml"), []byte("active_profile: bad\n"), 0o600))
require.NoError(t, os.MkdirAll(filepath.Join(root, "profiles"), 0o750))
require.NoError(t, os.WriteFile(filepath.Join(root, "profiles", "bad.yaml"),
[]byte("name: bad\ngateway:\n url: \"\"\n token: dummy\n default_model: \"\"\nmodels: []\n"), 0o600))
reg := adapter.NewRegistry()
reg.Register(&detectableAdapter{name: "stub", detect: true})
var buf bytes.Buffer
cmd := NewApply(reg)
cmd.SetOut(&buf)
// --yes skips first-run prompt; --allow-incomplete skips gateway check.
cmd.SetArgs([]string{"--root", root, "--dry-run", "--allow-incomplete", "--yes"})
require.NoError(t, cmd.Execute())
assert.Contains(t, buf.String(), "dry-run")
}
+1 -1
View File
@@ -17,7 +17,7 @@ func NewDiff(reg *adapter.Registry) *cobra.Command {
apply := NewApply(reg)
apply.SetOut(cmd.OutOrStdout())
apply.SetErr(cmd.ErrOrStderr())
applyArgs := []string{"--dry-run"}
applyArgs := []string{"--dry-run", "--allow-incomplete"}
if root != "" {
applyArgs = append(applyArgs, "--root", root)
}
+24 -14
View File
@@ -23,6 +23,7 @@ func NewInit(reg *adapter.Registry) *cobra.Command {
home string
from []string
noPrompt bool
force bool
)
cmd := &cobra.Command{
Use: "init",
@@ -42,6 +43,10 @@ func NewInit(reg *adapter.Registry) *cobra.Command {
if err = os.MkdirAll(r, 0o750); err != nil {
return err
}
// Issue 1: refuse overwrite without --force
if _, statErr := os.Stat(filepath.Join(r, "config.yaml")); statErr == nil && !force {
return fmt.Errorf("canonical root %s already initialised (use --force to re-import; existing skills/agents/mcp will be merged)", r)
}
candidates := reg.DetectedNames()
if len(candidates) == 0 {
return fmt.Errorf("no harnesses detected under %s", home)
@@ -77,6 +82,7 @@ func NewInit(reg *adapter.Registry) *cobra.Command {
cmd.Flags().StringVar(&home, "home", "", "home dir (default $HOME)")
cmd.Flags().StringSliceVar(&from, "from", nil, "import from specific adapter(s), skip prompt")
cmd.Flags().BoolVar(&noPrompt, "no-prompt", false, "import from all detected adapters without prompting")
cmd.Flags().BoolVar(&force, "force", false, "re-import even if canonical root already initialised")
return cmd
}
@@ -153,6 +159,18 @@ func runImport(cmd *cobra.Command, reg *adapter.Registry, root, home string, pic
return nil
}
// importedProfileBody is the placeholder written to profiles/imported.yaml on first init.
// Written using escape sequences to avoid literal newlines in string literals.
const importedProfileBody = "# harness-sync profile \xe2\x80\x94 generated by init\n" +
"# Edit the gateway block below to point at your LLM gateway before running\n" +
"# `harness-sync apply`. The dummy token is fine for most local gateways.\n" +
"name: imported\n" +
"gateway:\n" +
" url: \"\" # \xe2\x86\x90 set this\n" +
" token: \"dummy\"\n" +
" default_model: \"\" # \xe2\x86\x90 set this\n" +
"models: []\n"
func writeCanonical(root string, b *canonical.Bundle, picked []string) error {
for _, sub := range []string{"profiles", "skills", "agents", "instructions"} {
if err := os.MkdirAll(filepath.Join(root, sub), 0o750); err != nil {
@@ -171,20 +189,12 @@ func writeCanonical(root string, b *canonical.Bundle, picked []string) error {
return err
}
profBody, err := render.YAML(canonical.Profile{
Name: "imported",
Gateway: canonical.Gateway{
URL: "https://gateway.local",
Token: "dummy",
DefaultModel: "claude-sonnet-4-6",
},
Models: []canonical.Model{{ID: "claude-sonnet-4-6", Alias: "sonnet"}},
})
if err != nil {
return err
}
if err = os.WriteFile(filepath.Join(root, "profiles", "imported.yaml"), profBody, 0o600); err != nil {
return err
// Issue 2: write placeholder profile only if it doesn't exist yet (preserve user edits on re-init).
profPath := filepath.Join(root, "profiles", "imported.yaml")
if _, statErr := os.Stat(profPath); os.IsNotExist(statErr) {
if err = os.WriteFile(profPath, []byte(importedProfileBody), 0o600); err != nil {
return err
}
}
if len(b.MCP.Servers) > 0 {
+54
View File
@@ -49,3 +49,57 @@ func TestInitImportWritesCanonical(t *testing.T) {
_, err := os.Stat(filepath.Join(root, ".git"))
require.NoError(t, err)
}
func makeStubReg() *adapter.Registry {
reg := adapter.NewRegistry()
reg.Register(&importableAdapter{
name: "stub",
res: &adapter.ImportResult{},
})
return reg
}
// TestInitRefusesOverwriteWithoutForce: second init without --force must error.
func TestInitRefusesOverwriteWithoutForce(t *testing.T) {
root := t.TempDir()
home := t.TempDir()
// First init — should succeed.
cmd := NewInit(makeStubReg())
cmd.SetArgs([]string{"--root", root, "--home", home, "--from", "stub", "--no-prompt"})
require.NoError(t, cmd.Execute())
// Second init — must fail without --force.
cmd2 := NewInit(makeStubReg())
cmd2.SetArgs([]string{"--root", root, "--home", home, "--from", "stub", "--no-prompt"})
err := cmd2.Execute()
require.Error(t, err)
assert.Contains(t, err.Error(), "already initialised")
assert.Contains(t, err.Error(), "--force")
}
// TestInitForcePreservesExistingProfile: --force re-imports but does NOT overwrite imported.yaml.
func TestInitForcePreservesExistingProfile(t *testing.T) {
root := t.TempDir()
home := t.TempDir()
// First init writes placeholder profile.
cmd := NewInit(makeStubReg())
cmd.SetArgs([]string{"--root", root, "--home", home, "--from", "stub", "--no-prompt"})
require.NoError(t, cmd.Execute())
// User edits the profile.
profPath := filepath.Join(root, "profiles", "imported.yaml")
userEdit := []byte("name: imported\ngateway:\n url: \"https://my.gateway\"\n token: \"tok\"\n default_model: \"m\"\nmodels: []\n")
require.NoError(t, os.WriteFile(profPath, userEdit, 0o600))
// Re-init with --force.
cmd2 := NewInit(makeStubReg())
cmd2.SetArgs([]string{"--root", root, "--home", home, "--from", "stub", "--no-prompt", "--force"})
require.NoError(t, cmd2.Execute())
// User edits must be preserved.
got, err := os.ReadFile(profPath)
require.NoError(t, err)
assert.Equal(t, string(userEdit), string(got))
}
+20 -2
View File
@@ -8,10 +8,13 @@ import (
"strings"
"github.com/spf13/cobra"
"github.com/lukaszraczylo/harness-sync/internal/adapter"
)
// NewProfile returns the `profile` subcommand with `list` and `use`.
func NewProfile() *cobra.Command {
// reg is used by the `use --apply` flag to invoke apply after switching.
func NewProfile(reg *adapter.Registry) *cobra.Command {
var root string
rootFlag := func(c *cobra.Command) {
c.Flags().StringVar(&root, "root", "", "canonical root (default ~/.config/harness-sync)")
@@ -44,6 +47,7 @@ func NewProfile() *cobra.Command {
}
rootFlag(list)
var applyAfter bool
use := &cobra.Command{
Use: "use <name>",
Short: "Set active profile (rewrites config.yaml)",
@@ -61,10 +65,24 @@ func NewProfile() *cobra.Command {
configPath := filepath.Join(r, "config.yaml")
existing, _ := os.ReadFile(configPath)
updated := setActiveProfile(string(existing), target)
return os.WriteFile(configPath, []byte(updated), 0o600)
if err := os.WriteFile(configPath, []byte(updated), 0o600); err != nil {
return err
}
_, _ = fmt.Fprintf(cmd.OutOrStdout(),
"Switched active profile to %q. Run `harness-sync apply` to propagate.\n", target)
// Issue 4: --apply flag auto-invokes apply after switching.
if applyAfter {
applyCmd := NewApply(reg)
applyCmd.SetOut(cmd.OutOrStdout())
applyCmd.SetErr(cmd.ErrOrStderr())
applyCmd.SetArgs([]string{"--root", r, "--yes"})
return applyCmd.Execute()
}
return nil
},
}
rootFlag(use)
use.Flags().BoolVar(&applyAfter, "apply", false, "run apply automatically after switching profile")
cmd := &cobra.Command{Use: "profile", Short: "Manage profiles"}
cmd.AddCommand(list, use)
+54 -2
View File
@@ -4,10 +4,13 @@ import (
"bytes"
"os"
"path/filepath"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/lukaszraczylo/harness-sync/internal/adapter"
)
func TestProfileList(t *testing.T) {
@@ -17,7 +20,7 @@ func TestProfileList(t *testing.T) {
require.NoError(t, os.WriteFile(filepath.Join(root, "profiles", "work.yaml"), []byte("name: work\n"), 0o600))
var buf bytes.Buffer
cmd := NewProfile()
cmd := NewProfile(nil)
cmd.SetArgs([]string{"list", "--root", root})
cmd.SetOut(&buf)
require.NoError(t, cmd.Execute())
@@ -33,7 +36,7 @@ func TestProfileUse(t *testing.T) {
require.NoError(t, os.WriteFile(filepath.Join(root, "profiles", "work.yaml"), []byte("name: work\n"), 0o600))
require.NoError(t, os.WriteFile(filepath.Join(root, "config.yaml"), []byte("active_profile: home\n"), 0o600))
cmd := NewProfile()
cmd := NewProfile(nil)
cmd.SetArgs([]string{"use", "work", "--root", root})
require.NoError(t, cmd.Execute())
@@ -41,3 +44,52 @@ func TestProfileUse(t *testing.T) {
require.NoError(t, err)
assert.Contains(t, string(body), "active_profile: work")
}
// TestProfileUseHintMessage verifies the "run apply" hint is printed.
func TestProfileUseHintMessage(t *testing.T) {
root := t.TempDir()
require.NoError(t, os.MkdirAll(filepath.Join(root, "profiles"), 0o750))
require.NoError(t, os.WriteFile(filepath.Join(root, "profiles", "work.yaml"), []byte("name: work\n"), 0o600))
require.NoError(t, os.WriteFile(filepath.Join(root, "config.yaml"), []byte("active_profile: home\n"), 0o600))
var buf bytes.Buffer
cmd := NewProfile(nil)
cmd.SetOut(&buf)
cmd.SetArgs([]string{"use", "work", "--root", root})
require.NoError(t, cmd.Execute())
assert.Contains(t, buf.String(), "Switched active profile to")
assert.Contains(t, buf.String(), "harness-sync apply")
}
// TestProfileUseWithApply verifies --apply triggers apply after switching.
func TestProfileUseWithApply(t *testing.T) {
root := t.TempDir()
require.NoError(t, os.MkdirAll(filepath.Join(root, "profiles"), 0o750))
// Profile must be complete so apply doesn't refuse.
profYAML := "name: work\ngateway:\n url: https://gw\n token: tok\n default_model: m\nmodels:\n - {id: m}\n"
require.NoError(t, os.WriteFile(filepath.Join(root, "profiles", "work.yaml"), []byte(profYAML), 0o600))
require.NoError(t, os.WriteFile(filepath.Join(root, "config.yaml"), []byte("active_profile: home\n"), 0o600))
reg := adapter.NewRegistry()
// No adapters detected → apply runs but writes 0 files (nothing to render).
reg.Register(&detectableAdapter{name: "stub", detect: false})
var buf bytes.Buffer
cmd := NewProfile(reg)
cmd.SetOut(&buf)
// --yes bypasses first-run prompt in apply; state/ won't exist.
cmd.SetArgs([]string{"use", "work", "--root", root, "--apply"})
require.NoError(t, cmd.Execute())
out := buf.String()
assert.Contains(t, out, "Switched active profile")
// apply output: "applied: 0 written ..."
assert.True(t, strings.Contains(out, "applied") || strings.Contains(out, "dry-run"),
"expected apply output in: %s", out)
// config.yaml must now reference work.
body, err := os.ReadFile(filepath.Join(root, "config.yaml"))
require.NoError(t, err)
assert.Contains(t, string(body), "active_profile: work")
}