From 355a262d021aeb3768ee6b9558b00b78a67b4263 Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 11:47:58 +0200 Subject: [PATCH 01/23] docs: add design spec for profile secrets and templates Co-Authored-By: Claude Fable 5 --- .../2026-08-20-profile-secrets-design.md | 237 ++++++++++++++++++ 1 file changed, 237 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-20-profile-secrets-design.md diff --git a/docs/superpowers/specs/2026-08-20-profile-secrets-design.md b/docs/superpowers/specs/2026-08-20-profile-secrets-design.md new file mode 100644 index 0000000..8fac540 --- /dev/null +++ b/docs/superpowers/specs/2026-08-20-profile-secrets-design.md @@ -0,0 +1,237 @@ +# Profile Secrets and Templates — Design + +**Date:** 2026-08-20 +**Status:** Approved design, pending implementation plan + +## Context + +Credentialed tool configuration is the remaining gap in the sandbox: a Maven +`~/.m2/settings.xml` mixes shareable shape (proxies, mirrors) with per-user +credentials (repository server auth). The predecessor mechanism +(`.sandbox-secrets.yaml`) was removed deliberately: it resolved `source:` +commands on the host from an agent-authored workspace file — host command +execution from inside the sandbox. The integration suite still pins that +removal. + +Profiles (2026-08-19 design) now provide a host-trusted, team-shareable +bundle format. This design extends them with **templates** rendered from +**secrets** (resolved on the host from the user's own configuration) and +**variables** (plain literals), delivered per start/apply directly into the +agent's home. + +## Goals + +- A team profile ships the whole shape of a credentialed config (e.g. + `settings.xml` with proxies and mirrors) once; each user supplies only + their credential sources. +- Secret values come from the host — a user-authored command (gopass, pass, + op, …) or a literal — never from the guest, the workspace, or the profile. +- Profile authors never gain host command execution: profiles may *suggest* + a source as inert, displayed-only metadata; nothing executes until the + user copies it into their own mapping. +- No per-invocation secret-manager calls (pinentry): resolution happens at + VM start and explicit `profile apply` only. +- Secrets never travel through the Lima template (`mode: data` persists in + `~/.lima//lima.yaml`) and never land in the world-readable guest + profile tree. + +## Non-goals (v1) + +- Environment-variable or interactive-prompt sources (command + literal + only). +- Per-profile secret scoping. Any active profile's hook can read the whole + agent home, so scoping names to profiles would be an illusion; the design + is honest about the real boundary instead (see Security). +- Removing rendered files on profile deactivation (consistent with the + existing files/ non-goal; `code-vm recreate` is the clean slate). +- Escaping/encoding of substituted values (values are opaque bytes; a value + that breaks the target format is the user's own, as in any hand-written + config). +- Secret rotation detection. Rotation = rotate at the source, then restart + or `profile apply`. + +## Decisions + +| Area | Decision | +|---|---| +| Trust line | Hybrid: profiles declare secret/var *names* with optional inert `suggest:`/`description:`; only the user's local mapping executes anything | +| Templates | Shipped by profiles under `templates/`, a home-mirroring tree with placeholders; generic across tools (Maven, npm, Gradle, …) | +| Secret sources (v1) | `command:` (host shell, stdout is the value, trailing newline stripped) and `value:` (literal) | +| Variables | Plain literals in `config.yaml` under `vars:`; non-secret knobs | +| Resolve/render/push timing | `code-vm start`, `code-vm profile apply`, and any invocation whose `ensureRunning` actually booted the VM | +| Delivery | Host-side render, staged push directly to `/home//`, agent-owned, 0600, no exec bit — the git-identity path | +| Placeholder syntax | `${secret:name}` and `${var:name}` only; all other `${...}` passes through untouched | +| Failure mode | A declared-but-unmapped secret/var fails start/apply with an actionable error carrying a ready-to-paste snippet | + +## Profile bundle extensions + +``` +wetf-maven/ + profile.yaml + templates/ # home-mirroring tree, rendered before delivery + .m2/settings.xml + files/ ... # unchanged +``` + +`profile.yaml` gains two sections: + +```yaml +secrets: + wetf-repo-user: + description: Artifactory user for wetf-snapshots/releases + suggest: gopass show -o wetf/artifactory-user # inert hint, never executed + wetf-repo-password: + suggest: gopass show -o wetf/artifactory-password +vars: + artifactory-url: + description: Base URL of the Artifactory instance +``` + +Validation, host-side at load, extending the existing rules: + +- Secret and var names match the profile-name pattern + (`[a-zA-Z0-9][a-zA-Z0-9-]{0,62}`): they appear in placeholders and error + messages, never in shell commands or guest paths. +- `description` and `suggest` are inert display strings — never executed, + never substituted into anything, never delivered to the guest. +- `templates/` entries get exactly the `files/` treatment: conservative + charset, no `..`, no absolute paths, no symlinks anywhere (tree root, + directories, entries), locked Claude settings paths rejected. +- A template and a `files/` entry targeting the same destination within one + profile is a validation error. Across profiles, list order wins, as with + files. +- A template referencing an undeclared `${secret:...}` or `${var:...}` name + is a load-time validation error: bundles cannot quietly depend on inputs + they never declared. Declared-but-unreferenced names are allowed (hooks + may not need templates). +- A profile with `secrets:`/`vars:`/`templates/` but nothing else remains a + valid, non-empty profile. + +## User-side values + +Sensitivity decides the home: + +- **`~/.config/code-vm/secrets.yaml`** — 0600, user-authored, never + distributed. It lives inside the config tree the mount guards already + protect, and is host-trusted exactly like `config.yaml`: + + ```yaml + secrets: + wetf-repo-user: + command: gopass show -o wetf/artifactory-user + wetf-repo-password: + command: gopass show -o wetf/artifactory-password + low-value-token: + value: abc123 # literal; a footgun for real credentials + ``` + + `command` runs on the host through the shell at resolve time; stdout with + one trailing newline stripped is the value. A failing command fails the + start/apply with the command's stderr. `command` and `value` are mutually + exclusive per entry. code-vm warns when the file is group/world readable. + +- **`vars:` in `config.yaml`** — a plain `map[string]string` of non-secret + literals (URLs, org names), handled like every other config key. + +Mappings are global, not per-profile: a name maps once, and every active +profile declaring that name receives the same value (see Security for why +scoping would be an illusion). + +## Rendering + +Plain string substitution, host-side. Only the exact forms `${secret:name}` +and `${var:name}` substitute; any other `${...}` (Maven properties, +`${env.FOO}`) passes through byte-for-byte. Values are opaque bytes with no +escaping layer. Each secret's command runs once per resolve pass, however +many templates reference it. Rendering happens after profile load and before +any push; a resolution or substitution failure aborts the whole start/apply +before anything reaches the guest. + +## Delivery and lifecycle + +Resolve → render → push runs at exactly three moments: + +1. `code-vm start`, after the readiness gate; +2. `code-vm profile apply`; +3. any invocation whose `ensureRunning` actually booted the VM — the first + command after a cold boot must not see a half-configured home. + +A plain invocation against a running VM resolves nothing — no pinentry. + +Delivery is a direct staged push (the git-identity path: host temp file, +0600, random staging name, agent-privilege install) to +`/home//`, owner agent, mode 0600, no exec bit. Rendered content +never enters `mode: data`, never enters `/usr/local/share/sandbox-profiles`, +and exists on the host only as the transient staging temp file. Rendered +files persist in the agent home across restarts and are canonically +re-pushed on every start/apply while the profile is active. The guest +applier is uninvolved: an unattended guest boot has no host to resolve +secrets, and `code-vm start` — the only way users boot — is host-driven and +pushes immediately after readiness. + +## Security + +The trust statement, extended (printed by `profile add`, now including the +profile's declared secret names): + +> Mapping a secret makes its value readable by the agent — and therefore by +> every active profile's hook. Install profiles from sources you trust with +> the secrets you map. + +The real boundaries: + +- **The user chooses what exists at all.** Only names mapped in the user's + own `secrets.yaml` ever resolve; declared-but-unmapped names fail loudly + and execute nothing. +- **Hints never execute.** `suggest:` strings are displayed and offered as + copy-paste snippets, nothing more. Host command execution comes only from + the user's own file. +- **Guest exposure is inherent and bounded.** Tools read their configs as + the agent, so rendered values are agent-readable by design. Exfiltration + is bounded by the egress allowlist and recorded in the proxy log. Users + should map only sandbox-appropriate credentials (e.g. a repo-read token, + not an org admin password). +- **Nothing agent- or workspace-authored reaches resolution.** Profile + sources are mount-guarded and symlink-rejected (2026-08-19 design, as + hardened); `secrets.yaml` sits in the same guarded config tree; the + workspace remains untrusted. The old mechanism's regression guards stay. +- **Host residue is transient.** Rendered values exist on the host only in + the 0600 staging temp file, deleted after the push; never in + `~/.lima/*/lima.yaml`. +- `profile list` marks profiles that declare secrets. + +## CLI/UX + +- `code-vm secrets` — lists the union of declared secrets and vars across + active profiles: name, declaring profile(s), description, mapped/unmapped + (names and status only, never values), and for unmapped secrets with a + hint, a ready-to-paste `secrets.yaml` snippet. +- Start/apply failure for a missing mapping names the profile, the key, the + description, and prints the same snippet. +- README gains a "Credentials" section documenting the trust model, the + Maven example end to end, and the rotation story (rotate at source → + restart or `profile apply`). + +## Testing + +Existing style: table-driven unit tests, fake runners, integration suite. + +- **Manifest/template validation** — secret/var name rules, inert hint + handling, undeclared-placeholder rejection, template/file collision, + symlinked `templates/` rejection, locked-settings rejection. +- **Rendering** — substitution of both namespaces, `${...}` passthrough, + opaque-bytes fidelity, one-command-per-secret resolution with an injected + host runner, command-failure and missing-mapping error text (including + the snippet). +- **secrets.yaml** — load, 0600 permission warning, command/value mutual + exclusion. +- **Session push** — fake-runner argv assertions: staged install to home + paths, agent uid/gid numeric, 0600. +- **CLI** — resolve/push triggered by start, apply, and boot-causing + invocations, and by nothing else; `code-vm secrets` output for + mapped/unmapped states. +- **Integration** — fixture profile with a template using one + literal-mapped secret and one var: rendered file lands 0600 agent-owned + with substituted content, survives a restart, updates after a mapping + change plus `profile apply`; a declared-but-unmapped secret fails apply + with the snippet in the error. From 3c8fc8e1a152f0613228e04c69d8846e6c8b767b Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 11:56:43 +0200 Subject: [PATCH 02/23] docs: add implementation plan for profile secrets and templates Co-Authored-By: Claude Fable 5 --- .../plans/2026-08-20-profile-secrets.md | 1318 +++++++++++++++++ 1 file changed, 1318 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-20-profile-secrets.md diff --git a/docs/superpowers/plans/2026-08-20-profile-secrets.md b/docs/superpowers/plans/2026-08-20-profile-secrets.md new file mode 100644 index 0000000..6c3a1b6 --- /dev/null +++ b/docs/superpowers/plans/2026-08-20-profile-secrets.md @@ -0,0 +1,1318 @@ +# Profile Secrets and Templates Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Profiles ship credentialed config templates (`${secret:name}`/`${var:name}` placeholders); secrets resolve on the host from the user's own `secrets.yaml` (command or literal), vars from `config.yaml`; rendered files push agent-owned 0600 into the guest home at start/apply/boot only. + +**Architecture:** Extends `internal/profile` (declarations, `templates/` tree, placeholder scan, rendering, resolution), `internal/config` (`secrets.yaml` loader, `Config.Vars`), and `internal/session` (a new agent-privilege user-file push that also replaces git-identity's root install). A new guest helper script relays staged content to an agent-identity install. CLI wires resolution into `start`, `profile apply`, and boot-causing invocations, plus a `code-vm secrets` listing command. + +**Tech Stack:** Go 1.26.5, Cobra, `gopkg.in/yaml.v3`, bash guest script (shellcheck/shfmt-clean), table-driven tests + fake runners, `test-vm-sandbox.sh`. + +**Spec:** `docs/superpowers/specs/2026-08-20-profile-secrets-design.md` + +## Global Constraints + +- Rendered/secret content must NEVER travel via `mode: data` (persists in `~/.lima//lima.yaml`), never enter `/usr/local/share/sandbox-profiles` (world-readable), and never be installed into the agent home by root (the TOCTOU class closed in the profiles PR). Delivery: staged push → root relays to a `root:AGENT_GID 0640` tmpfs drop → agent-identity install, final mode 0600 agent-owned, no exec bit. +- `suggest:`/`description:` are inert display strings: never executed, never substituted, never delivered to the guest. +- Host command execution comes ONLY from the user's `secrets.yaml` (`command:`) — never from profiles, the workspace, or the guest. +- Placeholders: exactly `${secret:}` and `${var:}` with names matching `[a-zA-Z0-9][a-zA-Z0-9-]{0,62}`; every other `${...}` passes through byte-for-byte. Undeclared placeholder = load-time error; declared-but-unmapped = start/apply-time error with a ready-to-paste snippet. +- Resolution/render/push happens ONLY at: `code-vm start`, `code-vm profile apply`, and an invocation whose `ensureRunning` actually booted the VM. +- Verification before every commit: `mise run test:unit && mise run lint && mise run fmt-check` (add `mise run build` before the last commit of a task touching cli). +- Commit style: conventional commits ending `Co-Authored-By: Claude Fable 5 `. +- Comments state constraints and reasons, matching the existing density. +- Golden file: profile declarations/templates must NOT change the Lima template rendering (templates are not DataFiles). Any golden drift is a bug. + +## File Structure + +| File | Responsibility | +|---|---| +| `internal/profile/profile.go` | Manifest gains `Secrets`/`Vars`; `templates/` tree loading; collision + undeclared-placeholder validation | +| `internal/profile/template.go` (new) | Placeholder scan (`FindRefs`), `RenderTemplates`, `DeclaredSecrets`/`DeclaredVars`, `ResolveSecrets`/`ResolveVars` | +| `internal/profile/profile_test.go`, `template_test.go` (new) | Tests | +| `internal/config/secrets.go` (new) | `SecretSource`, `LoadSecrets`, `SecretsPathFor` | +| `internal/config/config.go` | `Config.Vars` + key validation | +| `internal/guest/files/scripts/install-user-file.sh` (new) | Root relay → agent-identity install of one staged file | +| `internal/session/stage.go` | Factor out `stageFile` | +| `internal/session/userfiles.go` (new) | `PushUserFile` | +| `internal/session/gitidentity.go` | Migrate to `PushUserFile` | +| `internal/cli/start.go` | `ensureRunning` returns `(started bool, err)`; `pushRenderedTemplates` helper | +| `internal/cli/{shell,recreate,mount,profile}.go` | Trigger wiring | +| `internal/cli/secrets.go` (new) | `code-vm secrets` listing | +| `internal/cli/profile.go` | `add` trust warning lists declared secrets; `list` marks secret-declaring profiles | +| `test-vm-sandbox.sh`, `README.md` | Integration + docs | + +--- + +### Task 1: Manifest declarations and the templates tree + +**Files:** +- Modify: `internal/profile/profile.go` +- Test: `internal/profile/profile_test.go` + +**Interfaces:** +- Consumes: existing `Manifest`, `File`, `Profile`, `loadFiles`, `ValidateName`, `isBlank`, `relPathRe`, `forbiddenFiles`. +- Produces: + - `type SecretSpec struct { Description string \`yaml:"description"\`; Suggest string \`yaml:"suggest"\` }` + - `type VarSpec struct { Description string \`yaml:"description"\` }` + - `Manifest.Secrets map[string]SecretSpec \`yaml:"secrets"\``, `Manifest.Vars map[string]VarSpec \`yaml:"vars"\`` + - `Profile.Templates []File` (loaded from `templates/`, sorted by Rel; `Executable` ignored downstream) + - `refRe` (exported via Task 2's `FindRefs`; defined here or in template.go — put it in template.go, Task 2; THIS task only loads/validates trees and declarations, the placeholder cross-check moves in with Task 2's scanner via a shared `validateTemplateRefs` call added in Task 2) + +- [ ] **Step 1: Write the failing tests** + +Append to `internal/profile/profile_test.go` (reuse `writeProfile`; it takes a `files` map keyed by profile-relative path, so `templates/.m2/settings.xml` entries work as-is): + +```go +func TestLoadTemplatesAndDeclarations(t *testing.T) { + dir := t.TempDir() + writeProfile(t, dir, "maven", ` +description: maven setup +secrets: + repo-user: + description: Artifactory user + suggest: gopass show -o wetf/artifactory-user + repo-password: {} +vars: + artifactory-url: + description: Base URL +`, map[string]string{ + "templates/.m2/settings.xml": "${secret:repo-user}/${var:artifactory-url}\n", + }) + p, err := Load(dir, "maven") + if err != nil { + t.Fatalf("Load: %v", err) + } + if p.Manifest.Secrets["repo-user"].Suggest != "gopass show -o wetf/artifactory-user" { + t.Errorf("Suggest not loaded: %+v", p.Manifest.Secrets) + } + if _, ok := p.Manifest.Secrets["repo-password"]; !ok { + t.Error("empty-spec secret not loaded") + } + if len(p.Templates) != 1 || p.Templates[0].Rel != ".m2/settings.xml" { + t.Fatalf("Templates = %+v", p.Templates) + } +} + +// A profile carrying only declarations and templates is a valid profile. +func TestLoadTemplatesOnlyProfileIsNotEmpty(t *testing.T) { + dir := t.TempDir() + writeProfile(t, dir, "p", "secrets:\n tok: {}\n", map[string]string{ + "templates/.npmrc": "//registry/:_authToken=${secret:tok}\n", + }) + if _, err := Load(dir, "p"); err != nil { + t.Fatalf("Load: %v", err) + } +} + +func TestLoadRejectsInvalidDeclarations(t *testing.T) { + tests := []struct { + name string + manifest string + files map[string]string + wantErr string + }{ + {"bad secret name", "secrets:\n 'has space': {}\n", nil, "secret name"}, + {"bad var name", "vars:\n 'has/slash': {}\n", nil, "var name"}, + {"template/file collision", "description: x\n", + map[string]string{"files/.npmrc": "a\n", "templates/.npmrc": "b\n"}, + "both files/ and templates/"}, + {"template ships locked settings", "description: x\n", + map[string]string{"templates/.claude/settings.json": "{}\n"}, + "locked Claude settings"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + writeProfile(t, dir, "p", tt.manifest, tt.files) + _, err := Load(dir, "p") + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Errorf("Load error = %v, want it to contain %q", err, tt.wantErr) + } + }) + } +} + +func TestLoadRejectsSymlinkedTemplatesRoot(t *testing.T) { + dir := t.TempDir() + writeProfile(t, dir, "p", "description: x\n", map[string]string{"files/a": "x\n"}) + if err := os.Symlink(t.TempDir(), filepath.Join(dir, "p", "templates")); err != nil { + t.Skip("symlinks unavailable") + } + if _, err := Load(dir, "p"); err == nil || !strings.Contains(err.Error(), "symlinks are rejected") { + t.Errorf("Load error = %v, want symlink rejection", err) + } +} +``` + +- [ ] **Step 2: Run to verify failure** — `go test ./internal/profile/` fails (unknown manifest fields, no Templates). + +- [ ] **Step 3: Implement** + +In `internal/profile/profile.go`: + +1. Add the `SecretSpec`/`VarSpec` types and the two `Manifest` fields (after `Hook`). +2. Factor the body of `loadFiles` into `loadTree(dir, subdir string) ([]File, error)` — identical logic, with `subdir` replacing the literal `"files"` in the root join and every error message prefix (use `fmt.Sprintf("%s/%s", subdir, rel)` where messages currently say `files/...`; keep the blank-content rejection for both trees — a blank template is a bundle bug, and one rule is easier to hold than two). `loadFiles(dir)` becomes `loadTree(dir, "files")`; add `loadTemplates(dir)` = `loadTree(dir, "templates")`. +3. In `Load`, after `p.Files`: `p.Templates, err = loadTemplates(dir)` with the same error wrapping. +4. In `validateManifest` (change its signature to `validateManifest(m Manifest) error` stays; add): + +```go + for name := range m.Secrets { + if err := ValidateName(name); err != nil { + return fmt.Errorf("secret name %q: must look like %q", name, "repo-user") + } + } + for name := range m.Vars { + if err := ValidateName(name); err != nil { + return fmt.Errorf("var name %q: must look like %q", name, "artifactory-url") + } + } +``` + +(Iteration order does not matter — validation only.) +5. After templates load in `Load`, reject collisions within the bundle: + +```go + fileRels := map[string]bool{} + for _, f := range p.Files { + fileRels[f.Rel] = true + } + for _, tpl := range p.Templates { + if fileRels[tpl.Rel] { + return Profile{}, fmt.Errorf("profile %s: %s is shipped by both files/ and templates/; pick one", name, tpl.Rel) + } + } +``` + +6. Extend the empty-profile check: `... && len(p.Templates) == 0 && len(m.Secrets) == 0 && len(m.Vars) == 0` (and add "templates, secrets or vars" to its message). + +- [ ] **Step 4: Run to verify pass** — `go test ./internal/profile/ -v`. +- [ ] **Step 5: Verify no golden drift** — `go test ./internal/lima/` must pass UNCHANGED (templates are not DataFiles). +- [ ] **Step 6: Lint, format, commit** + +```bash +git add internal/profile/ && git commit -m "feat: profiles declare secrets/vars and ship templates" +``` + +--- + +### Task 2: Placeholder scan, rendering, declaration merge, resolution + +**Files:** +- Create: `internal/profile/template.go` +- Modify: `internal/profile/profile.go` (wire undeclared-placeholder validation into `Load`) +- Test: `internal/profile/template_test.go` + +**Interfaces:** +- Consumes: `Profile`, `File`, `SecretSpec`, `VarSpec` (Task 1); `config.SecretSource` (Task 3 — see note below). +- Produces: + - `type Ref struct { Kind string // "secret" | "var"; Name string }` + - `func FindRefs(content []byte) []Ref` — deduplicated, in first-appearance order + - `type Rendered struct { Rel string; Content []byte }` + - `func RenderTemplates(profiles []Profile, secrets, vars map[string]string) []Rendered` — later profiles win Rel collisions; output sorted by Rel + - `type DeclaredSecret struct { Name string; Profiles []string; Description, Suggest string }` (first non-empty Description/Suggest wins); `func DeclaredSecrets(profiles []Profile) []DeclaredSecret` — sorted by name; analogous `DeclaredVar`/`DeclaredVars` + - `type CommandRunner func(ctx context.Context, command string) ([]byte, error)` — the host-exec seam + - `func ResolveSecrets(ctx context.Context, declared []DeclaredSecret, sources map[string]config.SecretSource, run CommandRunner) (map[string]string, error)` + - `func ResolveVars(declared []DeclaredVar, values map[string]string) (map[string]string, error)` + - `func MissingSecretSnippet(d DeclaredSecret) string` — the ready-to-paste `secrets.yaml` block + +**Ordering note:** this task needs `config.SecretSource{Command, Value string}` from Task 3. Tasks 2 and 3 may land in either order; whichever goes first defines the struct (if Task 2 goes first, add the two-field struct to `internal/config/secrets.go` with a doc comment and let Task 3 build the loader around it). Execute Task 3 first to keep it simple. + +- [ ] **Step 1: Write the failing tests** + +`internal/profile/template_test.go`: + +```go +package profile + +import ( + "context" + "errors" + "fmt" + "reflect" + "strings" + "testing" + + "github.com/wetransform/code-vm/internal/config" +) + +func TestFindRefs(t *testing.T) { + content := []byte(`user=${secret:repo-user} url=${var:base-url} +again=${secret:repo-user} passthrough=${env.FOO} ${prop} $secret:no ${secret:BAD NAME}`) + got := FindRefs(content) + want := []Ref{{Kind: "secret", Name: "repo-user"}, {Kind: "var", Name: "base-url"}} + if !reflect.DeepEqual(got, want) { + t.Errorf("FindRefs = %v, want %v", got, want) + } +} + +func TestRenderTemplatesSubstitutesAndPassesThrough(t *testing.T) { + profiles := []Profile{{ + Name: "a", + Templates: []File{{Rel: ".m2/settings.xml", Content: []byte( + "${secret:repo-user}${var:base-url}${env.HOME}")}}, + }} + out := RenderTemplates(profiles, + map[string]string{"repo-user": "simon"}, + map[string]string{"base-url": "https://x.example"}) + if len(out) != 1 { + t.Fatalf("Rendered = %+v", out) + } + want := "simonhttps://x.example${env.HOME}" + if string(out[0].Content) != want { + t.Errorf("Content = %q, want %q", out[0].Content, want) + } +} + +func TestRenderTemplatesLaterProfileWins(t *testing.T) { + profiles := []Profile{ + {Name: "a", Templates: []File{{Rel: ".npmrc", Content: []byte("from-a")}}}, + {Name: "b", Templates: []File{{Rel: ".npmrc", Content: []byte("from-b")}}}, + } + out := RenderTemplates(profiles, nil, nil) + if len(out) != 1 || string(out[0].Content) != "from-b" { + t.Errorf("collision must resolve to the later profile, got %+v", out) + } +} + +func TestDeclaredSecretsMergesAcrossProfiles(t *testing.T) { + profiles := []Profile{ + {Name: "a", Manifest: Manifest{Secrets: map[string]SecretSpec{ + "tok": {Description: "token", Suggest: "gopass show -o t"}}}}, + {Name: "b", Manifest: Manifest{Secrets: map[string]SecretSpec{"tok": {}}}}, + } + got := DeclaredSecrets(profiles) + if len(got) != 1 || got[0].Name != "tok" || got[0].Suggest != "gopass show -o t" || + !reflect.DeepEqual(got[0].Profiles, []string{"a", "b"}) { + t.Errorf("DeclaredSecrets = %+v", got) + } +} + +func TestResolveSecrets(t *testing.T) { + declared := []DeclaredSecret{ + {Name: "from-cmd", Profiles: []string{"p"}}, + {Name: "from-val", Profiles: []string{"p"}}, + } + sources := map[string]config.SecretSource{ + "from-cmd": {Command: "get-it"}, + "from-val": {Value: "literal"}, + } + calls := 0 + run := func(_ context.Context, command string) ([]byte, error) { + calls++ + if command != "get-it" { + t.Errorf("command = %q", command) + } + return []byte("resolved\n"), nil + } + got, err := ResolveSecrets(context.Background(), declared, sources, run) + if err != nil { + t.Fatalf("ResolveSecrets: %v", err) + } + // Exactly one trailing newline stripped; command runs once per secret. + if got["from-cmd"] != "resolved" || got["from-val"] != "literal" || calls != 1 { + t.Errorf("got %v, calls=%d", got, calls) + } +} + +func TestResolveSecretsMissingMappingHasSnippet(t *testing.T) { + declared := []DeclaredSecret{{ + Name: "repo-user", Profiles: []string{"maven"}, + Description: "Artifactory user", Suggest: "gopass show -o wetf/user", + }} + _, err := ResolveSecrets(context.Background(), declared, nil, nil) + if err == nil { + t.Fatal("expected an error for an unmapped secret") + } + for _, want := range []string{"repo-user", "maven", "Artifactory user", + "secrets:", "command: gopass show -o wetf/user"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error missing %q:\n%s", want, err) + } + } +} + +func TestResolveSecretsCommandFailure(t *testing.T) { + declared := []DeclaredSecret{{Name: "tok", Profiles: []string{"p"}}} + sources := map[string]config.SecretSource{"tok": {Command: "boom"}} + run := func(context.Context, string) ([]byte, error) { + return []byte("stderr text"), errors.New("exit status 1") + } + _, err := ResolveSecrets(context.Background(), declared, sources, run) + if err == nil || !strings.Contains(err.Error(), "tok") || !strings.Contains(err.Error(), "exit status 1") { + t.Errorf("ResolveSecrets error = %v", err) + } +} + +func TestResolveVars(t *testing.T) { + declared := []DeclaredVar{{Name: "url", Profiles: []string{"p"}, Description: "Base URL"}} + got, err := ResolveVars(declared, map[string]string{"url": "https://x"}) + if err != nil || got["url"] != "https://x" { + t.Errorf("ResolveVars = %v, %v", got, err) + } + _, err = ResolveVars(declared, nil) + if err == nil || !strings.Contains(err.Error(), "vars:") || !strings.Contains(err.Error(), "url") { + t.Errorf("missing var must produce a config.yaml snippet, got %v", err) + } +} + +// Load must reject a template referencing an undeclared name (wired in this task). +func TestLoadRejectsUndeclaredPlaceholder(t *testing.T) { + dir := t.TempDir() + writeProfile(t, dir, "p", "secrets:\n known: {}\n", map[string]string{ + "templates/.npmrc": "a=${secret:known} b=${var:never-declared}\n", + }) + _, err := Load(dir, "p") + if err == nil || !strings.Contains(err.Error(), "never-declared") { + t.Errorf("Load = %v, want undeclared-placeholder rejection", err) + } +} + +``` + +(Import list for this test file: `context`, `errors`, `reflect`, `strings`, `testing`, and the config package — no `fmt`.) + +- [ ] **Step 2: Run to verify failure** — `go test ./internal/profile/`. + +- [ ] **Step 3: Implement `internal/profile/template.go`** + +```go +package profile + +import ( + "context" + "fmt" + "regexp" + "sort" + "strings" + + "github.com/wetransform/code-vm/internal/config" +) + +// refRe matches exactly the two placeholder forms templates may use. The name +// charset mirrors ValidateName, so anything else — Maven properties, +// ${env.FOO} — is left untouched by both the scanner and the renderer. +var refRe = regexp.MustCompile(`\$\{(secret|var):([a-zA-Z0-9][a-zA-Z0-9-]{0,62})\}`) + +// Ref is one placeholder occurrence kind+name. +type Ref struct { + Kind string // "secret" or "var" + Name string +} + +// FindRefs returns the distinct placeholder references in content, in first- +// appearance order. +func FindRefs(content []byte) []Ref { + seen := map[Ref]bool{} + var out []Ref + for _, m := range refRe.FindAllSubmatch(content, -1) { + r := Ref{Kind: string(m[1]), Name: string(m[2])} + if !seen[r] { + seen[r] = true + out = append(out, r) + } + } + return out +} + +// Rendered is one template after substitution, destined for the agent home. +type Rendered struct { + Rel string + Content []byte +} + +// RenderTemplates substitutes secret and var values into every active +// profile's templates. Later profiles win Rel collisions, matching the files/ +// rule. Values are opaque bytes: no escaping layer, exactly as the spec +// states — a value that breaks the target format is the user's own. +func RenderTemplates(profiles []Profile, secrets, vars map[string]string) []Rendered { + byRel := map[string][]byte{} + for _, p := range profiles { + for _, tpl := range p.Templates { + byRel[tpl.Rel] = refRe.ReplaceAllFunc(tpl.Content, func(m []byte) []byte { + sub := refRe.FindSubmatch(m) + if string(sub[1]) == "secret" { + return []byte(secrets[string(sub[2])]) + } + return []byte(vars[string(sub[2])]) + }) + } + } + out := make([]Rendered, 0, len(byRel)) + for rel, content := range byRel { + out = append(out, Rendered{Rel: rel, Content: content}) + } + sort.Slice(out, func(i, j int) bool { return out[i].Rel < out[j].Rel }) + return out +} + +// DeclaredSecret is one secret name unioned across the active profiles. +type DeclaredSecret struct { + Name string + Profiles []string // declaring profiles, in activation order + Description string // first non-empty wins + Suggest string // first non-empty wins; inert display string +} + +// DeclaredVar is the var analog. +type DeclaredVar struct { + Name string + Profiles []string + Description string +} + +// DeclaredSecrets unions secret declarations across profiles, sorted by name. +func DeclaredSecrets(profiles []Profile) []DeclaredSecret { + byName := map[string]*DeclaredSecret{} + for _, p := range profiles { + names := make([]string, 0, len(p.Manifest.Secrets)) + for n := range p.Manifest.Secrets { + names = append(names, n) + } + sort.Strings(names) // map order is random; keep Profiles deterministic + for _, n := range names { + spec := p.Manifest.Secrets[n] + d, ok := byName[n] + if !ok { + d = &DeclaredSecret{Name: n} + byName[n] = d + } + d.Profiles = append(d.Profiles, p.Name) + if d.Description == "" { + d.Description = spec.Description + } + if d.Suggest == "" { + d.Suggest = spec.Suggest + } + } + } + out := make([]DeclaredSecret, 0, len(byName)) + for _, d := range byName { + out = append(out, *d) + } + sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) + return out +} + +// DeclaredVars unions var declarations across profiles, sorted by name. +func DeclaredVars(profiles []Profile) []DeclaredVar { + byName := map[string]*DeclaredVar{} + for _, p := range profiles { + names := make([]string, 0, len(p.Manifest.Vars)) + for n := range p.Manifest.Vars { + names = append(names, n) + } + sort.Strings(names) + for _, n := range names { + spec := p.Manifest.Vars[n] + d, ok := byName[n] + if !ok { + d = &DeclaredVar{Name: n} + byName[n] = d + } + d.Profiles = append(d.Profiles, p.Name) + if d.Description == "" { + d.Description = spec.Description + } + } + } + out := make([]DeclaredVar, 0, len(byName)) + for _, d := range byName { + out = append(out, *d) + } + sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) + return out +} + +// CommandRunner executes a user-authored secrets.yaml command on the host and +// returns its combined output. Injectable for tests. +type CommandRunner func(ctx context.Context, command string) ([]byte, error) + +// MissingSecretSnippet renders the ready-to-paste secrets.yaml block for an +// unmapped secret. The suggest hint is copied verbatim as the command — +// display only until the user adopts it by saving this snippet themselves. +func MissingSecretSnippet(d DeclaredSecret) string { + cmd := d.Suggest + if cmd == "" { + cmd = "" + } + return fmt.Sprintf("secrets:\n %s:\n command: %s\n", d.Name, cmd) +} + +// ResolveSecrets resolves every declared secret from the user's sources. Each +// command runs exactly once per resolve pass with one trailing newline +// stripped (the gopass/pass convention). A missing mapping fails with an +// actionable snippet rather than prompting or falling back to hints: hints +// never execute. +func ResolveSecrets(ctx context.Context, declared []DeclaredSecret, sources map[string]config.SecretSource, run CommandRunner) (map[string]string, error) { + out := make(map[string]string, len(declared)) + for _, d := range declared { + src, ok := sources[d.Name] + if !ok { + desc := d.Description + if desc == "" { + desc = "no description" + } + return nil, fmt.Errorf( + "profile %s needs secret %q (%s), but secrets.yaml does not map it.\nAdd to ~/.config/code-vm/secrets.yaml:\n\n%s", + strings.Join(d.Profiles, ", "), d.Name, desc, MissingSecretSnippet(d)) + } + if src.Command != "" { + b, err := run(ctx, src.Command) + if err != nil { + return nil, fmt.Errorf("secret %q: command failed: %w: %s", d.Name, err, strings.TrimSpace(string(b))) + } + out[d.Name] = strings.TrimSuffix(string(b), "\n") + continue + } + out[d.Name] = src.Value + } + return out, nil +} + +// ResolveVars resolves declared vars from config.yaml's literal map. +func ResolveVars(declared []DeclaredVar, values map[string]string) (map[string]string, error) { + out := make(map[string]string, len(declared)) + for _, d := range declared { + v, ok := values[d.Name] + if !ok { + desc := d.Description + if desc == "" { + desc = "no description" + } + return nil, fmt.Errorf( + "profile %s needs var %q (%s), but config.yaml does not set it.\nAdd to config.yaml:\n\nvars:\n %s: \n", + strings.Join(d.Profiles, ", "), d.Name, desc, d.Name) + } + out[d.Name] = v + } + return out, nil +} +``` + +Wire undeclared-placeholder validation into `Load` (profile.go), after templates load and before the empty check: + +```go + for _, tpl := range p.Templates { + for _, ref := range FindRefs(tpl.Content) { + declared := false + if ref.Kind == "secret" { + _, declared = m.Secrets[ref.Name] + } else { + _, declared = m.Vars[ref.Name] + } + if !declared { + return Profile{}, fmt.Errorf( + "profile %s: templates/%s references ${%s:%s}, which the manifest does not declare", + name, tpl.Rel, ref.Kind, ref.Name) + } + } + } +``` + +- [ ] **Step 4: Run to verify pass** — `go test ./internal/profile/ -v` (Task 3's `config.SecretSource` must exist first — execute Task 3 before this one if it hasn't landed; see Ordering note). +- [ ] **Step 5: Lint, format, commit** + +```bash +git add internal/profile/ && git commit -m "feat: template rendering and secret/var resolution" +``` + +--- + +### Task 3: `secrets.yaml` loader and `Config.Vars` + +**Files:** +- Create: `internal/config/secrets.go` +- Modify: `internal/config/config.go` (Vars field + validation) +- Test: `internal/config/secrets_test.go`, `internal/config/config_test.go` + +**Interfaces:** +- Produces: + - `type SecretSource struct { Command string \`yaml:"command"\`; Value string \`yaml:"value"\` }` + - `func SecretsPathFor(configPath string) string` — `secrets.yaml` next to the config file + - `func LoadSecrets(path string) (map[string]SecretSource, []string, error)` — `(sources, warnings, err)`; missing file → empty map, no error; unknown keys rejected (KnownFields); per-entry exactly one of command/value; group/world-readable file → warning string + - `Config.Vars map[string]string \`yaml:"vars,omitempty"\`` — keys validated against the name pattern in `Config.Validate` + +- [ ] **Step 1: Write the failing tests** + +`internal/config/secrets_test.go`: + +```go +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestSecretsPathFor(t *testing.T) { + if got := SecretsPathFor("/home/st/.config/code-vm/config.yaml"); got != "/home/st/.config/code-vm/secrets.yaml" { + t.Errorf("SecretsPathFor = %q", got) + } +} + +func TestLoadSecrets(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "secrets.yaml") + content := "secrets:\n a:\n command: gopass show -o x\n b:\n value: literal\n" + if err := os.WriteFile(p, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + sources, warnings, err := LoadSecrets(p) + if err != nil { + t.Fatalf("LoadSecrets: %v", err) + } + if len(warnings) != 0 { + t.Errorf("warnings = %v, want none for 0600", warnings) + } + if sources["a"].Command != "gopass show -o x" || sources["b"].Value != "literal" { + t.Errorf("sources = %+v", sources) + } +} + +func TestLoadSecretsMissingFileIsEmpty(t *testing.T) { + sources, warnings, err := LoadSecrets(filepath.Join(t.TempDir(), "secrets.yaml")) + if err != nil || len(sources) != 0 || len(warnings) != 0 { + t.Errorf("missing file must load empty: %v %v %v", sources, warnings, err) + } +} + +func TestLoadSecretsRejectsBadEntries(t *testing.T) { + tests := []struct{ name, content, wantErr string }{ + {"both command and value", "secrets:\n a:\n command: c\n value: v\n", "exactly one"}, + {"neither", "secrets:\n a: {}\n", "exactly one"}, + {"bad name", "secrets:\n 'has space':\n value: v\n", "secret name"}, + {"unknown key", "secrets:\n a:\n comand: typo\n", "not found"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := filepath.Join(t.TempDir(), "secrets.yaml") + if err := os.WriteFile(p, []byte(tt.content), 0o600); err != nil { + t.Fatal(err) + } + _, _, err := LoadSecrets(p) + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Errorf("LoadSecrets = %v, want %q", err, tt.wantErr) + } + }) + } +} + +func TestLoadSecretsWarnsOnLoosePermissions(t *testing.T) { + p := filepath.Join(t.TempDir(), "secrets.yaml") + if err := os.WriteFile(p, []byte("secrets:\n a:\n value: v\n"), 0o644); err != nil { + t.Fatal(err) + } + _, warnings, err := LoadSecrets(p) + if err != nil || len(warnings) != 1 || !strings.Contains(warnings[0], "0600") { + t.Errorf("want a permissions warning recommending 0600, got %v %v", warnings, err) + } +} +``` + +Append to `internal/config/config_test.go`: + +```go +func TestValidateVars(t *testing.T) { + c := Default() + c.ProjectsRoot = "/home/st/projects" + c.Vars = map[string]string{"artifactory-url": "https://x"} + if err := c.Validate(); err != nil { + t.Errorf("Validate: %v", err) + } + c.Vars = map[string]string{"has space": "v"} + if err := c.Validate(); err == nil || !strings.Contains(err.Error(), "vars") { + t.Errorf("Validate = %v, want vars key rejection", err) + } +} +``` + +- [ ] **Step 2: Run to verify failure** — `go test ./internal/config/`. + +- [ ] **Step 3: Implement** + +`internal/config/secrets.go`: + +```go +package config + +import ( + "errors" + "fmt" + "os" + "path/filepath" + + "gopkg.in/yaml.v3" +) + +// SecretSource is one user-authored mapping in secrets.yaml: exactly one of a +// host command (stdout is the value) or a literal. This file is host-trusted +// like config.yaml — it lives in the same mount-guarded tree and only the +// user writes it; profiles can only *suggest* entries, never install them. +type SecretSource struct { + Command string `yaml:"command"` + Value string `yaml:"value"` +} + +// secretsFile is the secrets.yaml schema. +type secretsFile struct { + Secrets map[string]SecretSource `yaml:"secrets"` +} + +// SecretsPathFor returns the secrets file belonging to a config file: a +// secrets.yaml next to it, protected by the same mount-exclusion guards. +func SecretsPathFor(configPath string) string { + return filepath.Join(filepath.Dir(configPath), "secrets.yaml") +} + +// LoadSecrets reads and validates secrets.yaml. A missing file is an empty +// mapping — profiles without secrets must not require one. Warnings (not +// errors) report loose file permissions: the file holds commands and possibly +// literal credentials. +func LoadSecrets(path string) (map[string]SecretSource, []string, error) { + data, err := os.ReadFile(path) + if errors.Is(err, os.ErrNotExist) { + return map[string]SecretSource{}, nil, nil + } + if err != nil { + return nil, nil, fmt.Errorf("read %s: %w", path, err) + } + var warnings []string + if fi, err := os.Stat(path); err == nil && fi.Mode().Perm()&0o077 != 0 { + warnings = append(warnings, fmt.Sprintf( + "%s is readable by group/others; recommend chmod 0600", path)) + } + var f secretsFile + dec := yaml.NewDecoder(bytes.NewReader(data)) + dec.KnownFields(true) + // An empty file decodes to io.EOF, and an empty secrets.yaml is as valid + // as a missing one. + if err := dec.Decode(&f); err != nil && !errors.Is(err, io.EOF) { + return nil, nil, fmt.Errorf("parse %s: %w", path, err) + } + for name, src := range f.Secrets { + if !instanceRe.MatchString(name) { + return nil, nil, fmt.Errorf("%s: secret name %q: must look like %q", path, name, "repo-user") + } + if (src.Command == "") == (src.Value == "") { + return nil, nil, fmt.Errorf("%s: secret %q: exactly one of command or value must be set", path, name) + } + } + if f.Secrets == nil { + f.Secrets = map[string]SecretSource{} + } + return f.Secrets, warnings, nil +} +``` + +(`bytesReader` = `bytes.NewReader`; import `bytes` directly — the helper name above is illustrative, use `bytes.NewReader(data)`. An empty file decodes to `io.EOF`: handle it like profile.go does, treating EOF as an empty document.) + +`internal/config/config.go`: add after `Profiles`: + +```go + // Vars are non-secret literal values available to profile templates as + // ${var:name}. Secrets never belong here — they go in secrets.yaml. + Vars map[string]string `yaml:"vars,omitempty"` +``` + +and in `Validate`, after the profiles loop: + +```go + for name := range c.Vars { + if !instanceRe.MatchString(name) { + return fmt.Errorf("vars: key %q must be a name like %q", name, "artifactory-url") + } + } +``` + +- [ ] **Step 4: Run to verify pass**, **Step 5: Lint, format, commit** + +```bash +git add internal/config/ && git commit -m "feat: user-side secrets.yaml and config vars" +``` + +--- + +### Task 4: Guest relay script and `session.PushUserFile`; migrate git identity + +**Files:** +- Create: `internal/guest/files/scripts/install-user-file.sh` +- Create: `internal/session/userfiles.go` +- Modify: `internal/session/stage.go` (factor `stageFile`), `internal/session/gitidentity.go` +- Test: `internal/session/userfiles_test.go`, update `gitidentity_test.go`, `internal/guest/embed_test.go` + +**Interfaces:** +- Consumes: staging plumbing in `stage.go`; the hardened setpriv pattern (see `apply-profiles.sh`'s `run_as_agent_sh`). +- Produces: + - `func stageFile(ctx context.Context, d Deps, content []byte) (string, error)` — the temp-file + `install -d` staging dir + `Copy` half of today's `installContent`, returning the staged guest path + - `func PushUserFile(ctx context.Context, d Deps, content []byte, rel, mode string) error` — stages content, then `Admin(["/usr/local/lib/sandbox/install-user-file.sh", staged, rel, mode])` + - Guest contract: `install-user-file.sh ` — root relays the staged file to a `root:AGENT_GID 0640` drop under `/run/sandbox/user-files/`, then an agent-identity install places it (mkdir -p, rm -f, install -m), then the drop and staged copies are removed. + +**Why the relay:** the staging dir is limaadmin-0700 (agent cannot read it), and a direct root `install` into the agent home is the TOCTOU class the profiles PR closed. The drop directory gives the agent read access without world-readability, and the final write runs with agent privileges only. + +- [ ] **Step 1: Write the failing session tests** + +`internal/session/userfiles_test.go` (reuse `fakeRunner`/`testDeps`): + +```go +package session + +import ( + "context" + "testing" +) + +func TestPushUserFileStagesAndRelays(t *testing.T) { + r := &fakeRunner{} + d := testDeps(t, r) + if err := PushUserFile(context.Background(), d, []byte("content"), ".m2/settings.xml", "0600"); err != nil { + t.Fatalf("PushUserFile: %v", err) + } + copies := 0 + for _, c := range r.calls { + if len(c) > 0 && c[0] == "copy" { + copies++ + } + } + if copies != 1 { + t.Errorf("staged copies = %d, want 1", copies) + } + if !r.ranAny("/usr/local/lib/sandbox/install-user-file.sh") { + t.Errorf("relay script not invoked: %v", r.calls) + } + if !r.ranAny(".m2/settings.xml") || !r.ranAny("0600") { + t.Errorf("dst/mode not passed to the relay: %v", r.calls) + } + // The old direct-to-home root install must NOT happen for user files. + if r.ranAny("install -D -m 0600") { + t.Errorf("user files must not be root-installed into the home: %v", r.calls) + } +} + +func TestGitIdentityUsesUserFilePush(t *testing.T) { + r := &fakeRunner{} + d := testDeps(t, r) + d.Host = func(ctx context.Context, name string, args ...string) ([]byte, error) { + return []byte("simon\n"), nil + } + if err := ApplyGitIdentity(context.Background(), d); err != nil { + t.Fatalf("ApplyGitIdentity: %v", err) + } + if !r.ranAny("install-user-file.sh") || !r.ranAny(".gitconfig") { + t.Errorf("git identity must go through the relay: %v", r.calls) + } + if r.ranAny("install -D -m 0644") { + t.Errorf("git identity must no longer be root-installed: %v", r.calls) + } +} +``` + +- [ ] **Step 2: Run to verify failure** — `go test ./internal/session/`. + +- [ ] **Step 3: Implement** + +In `stage.go`, split `installContent`: extract everything up to and including the `Copy` into `stageFile(ctx, d, content) (staged string, err)`; `installContent` calls it then keeps its root `install -D` + `rm -f` (still used by the allowlist fragment and profile-tree pushes, whose destinations are root-owned paths — the comment should say that is exactly why root install remains correct there). + +`internal/session/userfiles.go`: + +```go +package session + +import ( + "context" + "fmt" +) + +// PushUserFile delivers content into the agent's home at rel with the given +// mode, without ever writing there as root. The staged copy is relayed by +// install-user-file.sh: root moves it to an agent-group-readable tmpfs drop, +// and an agent-identity install places it — a symlink the agent plants can +// only redirect a write the agent could already make (the same posture as +// profile file installs). Used for rendered templates (0600) and the git +// identity (0644); rel comes from host-validated input only. +func PushUserFile(ctx context.Context, d Deps, content []byte, rel, mode string) error { + staged, err := stageFile(ctx, d, content) + if err != nil { + return err + } + if err := d.Client.Admin(ctx, []string{ + "/usr/local/lib/sandbox/install-user-file.sh", staged, rel, mode, + }); err != nil { + return fmt.Errorf("install %s: %w", rel, err) + } + return nil +} +``` + +`internal/guest/files/scripts/install-user-file.sh` (auto-delivered 0755 by the existing scripts mapping): + +```bash +#!/bin/bash +############################################################################### +# install-user-file.sh — place one host-staged file into the agent home +# +# Invoked as root by code-vm: install-user-file.sh +# +# The staged source sits in limaadmin's 0700 staging dir, unreadable by the +# agent; and a root write into the agent-owned home is the TOCTOU class the +# profile applier already closed. So root only RELAYS: the file moves to a +# root:AGENT_GID 0640 drop on tmpfs, and the final install runs with agent +# privileges — a planted symlink can only redirect a write the agent could +# already make. The drop is removed afterwards; rendered secrets exist there +# only for the moment between relay and install. +############################################################################### +set -euo pipefail + +# shellcheck source=/dev/null +. /etc/sandbox/provision.env + +src="$1" +rel="$2" +mode="$3" +AGENT_HOME="/home/${AGENT_USER}" + +DROP_DIR=/run/sandbox/user-files +install -d -m 0750 -o root -g "$AGENT_GID" "$DROP_DIR" +drop=$(mktemp "$DROP_DIR/file-XXXXXXXX") +install -m 0640 -o root -g "$AGENT_GID" "$src" "$drop" +rm -f "$src" + +cleanup() { rm -f "$drop"; } +trap cleanup EXIT + +# Same hardened pattern as the profile applier's agent runner: no login +# shell, system PATH only, BASH_ENV/ENV cleared. Positional args, not string +# interpolation. +setpriv --reuid "$AGENT_UID" --regid "$AGENT_GID" --init-groups \ + env -u BASH_ENV -u ENV \ + HOME="$AGENT_HOME" \ + USER="$AGENT_USER" \ + XDG_RUNTIME_DIR="/run/user/${AGENT_UID}" \ + PATH=/usr/local/bin:/usr/bin:/bin \ + bash -c 'dst="$1/$2"; mkdir -p "$(dirname "$dst")" && rm -f "$dst" && install -m "$3" "$4" "$dst"' \ + _ "$AGENT_HOME" "$rel" "$mode" "$drop" +``` + +Migrate `gitidentity.go`: replace the `installContent(...)` call with `PushUserFile(ctx, d, []byte(GitConfigContent(name, email)), ".gitconfig", "0644")` and drop the now-unused numeric-id comment/imports (the relay script owns identity now). + +Add an embed test asserting `install-user-file.sh` is delivered 0755 (mirror `TestApplyProfilesScriptIsDelivered`). + +- [ ] **Step 4: Run to verify pass** — `go test ./internal/session/ ./internal/guest/` plus `bash -n` on the new script. +- [ ] **Step 5: Golden regen check** — the new script is a DataFile in `guest.DataFiles()` but the golden test passes explicit files, so the golden should be UNCHANGED; if `TestRenderInstanceFileIsPrivateAndComplete`-style assertions or the golden do move, only the new script's mode:data entry may appear. +- [ ] **Step 6: Lint (shellcheck/shfmt cover the script), format, commit** + +```bash +git add internal/session/ internal/guest/ && git commit -m "feat: agent-privilege user-file push; migrate git identity to it" +``` + +--- + +### Task 5: Trigger plumbing and orchestration + +**Files:** +- Modify: `internal/cli/start.go` (`ensureRunning` returns `(bool, error)`; new `pushRenderedTemplates`), `internal/cli/shell.go`, `internal/cli/recreate.go`, `internal/cli/mount.go`, `internal/cli/profile.go` (apply) +- Test: `internal/cli/start_test.go`, `internal/cli/secrets_push_test.go` (new) + +**Interfaces:** +- Consumes: Tasks 1–4 (`DeclaredSecrets/Vars`, `ResolveSecrets/Vars`, `RenderTemplates`, `config.LoadSecrets/SecretsPathFor`, `session.PushUserFile`). +- Produces: + - `ensureRunning(ctx, cl, c, profiles) (started bool, err error)` — `started` is true only when this call actually booted the VM (status was not "Running"). + - `func pushRenderedTemplates(ctx context.Context, cl lima.Client, c config.Config, profiles []profile.Profile, cfgPath string, out io.Writer) error` — fast no-op when no active profile has templates/secrets/vars; otherwise LoadSecrets (print warnings to out), resolve (host runner = `exec.CommandContext(ctx, "sh", "-c", command).CombinedOutput()`), render, `PushUserFile(..., rel, "0600")` per rendered file, and print one `Rendered N template(s).` line. + - Trigger contract: `start` always pushes after `ensureRunning`; `runDefault`, `recreate`, `mount` push only when `started` came back true; `profile apply` pushes after `ApplyAllowlist` and BEFORE `ApplyProfiles` (hooks may consume rendered configs). Note: at boot, hooks run before the post-readiness push — hooks must tolerate absent templates on first boot; the README documents this (Task 7). + +- [ ] **Step 1: Write the failing tests** + +Update `internal/cli/start_test.go`: `ensureRunning` calls gain the second return; add assertions that status "Running" → `started == false` and statuses ""/"Stopped" → `started == true`. + +`internal/cli/secrets_push_test.go` — drive `pushRenderedTemplates` directly with a `recordingRunner`-backed client and a scratch config tree: + +```go +func TestPushRenderedTemplatesNoOpWithoutDeclarations(t *testing.T) { + r := &recordingRunner{statusOut: "Running"} + c := testCfg(t) + profiles := []profile.Profile{{Name: "plain", Manifest: profile.Manifest{Packages: []string{"git"}}}} + if err := pushRenderedTemplates(context.Background(), lima.Client{R: r}, c, profiles, filepath.Join(t.TempDir(), "config.yaml"), io.Discard); err != nil { + t.Fatalf("pushRenderedTemplates: %v", err) + } + if len(r.calls) != 0 { + t.Errorf("no declarations must mean no guest traffic, got %v", r.calls) + } +} + +func TestPushRenderedTemplatesResolvesAndPushes(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.yaml") + os.WriteFile(filepath.Join(dir, "secrets.yaml"), []byte("secrets:\n tok:\n value: sekrit\n"), 0o600) + r := &recordingRunner{statusOut: "Running"} + c := testCfg(t) + c.Vars = map[string]string{"url": "https://x"} + profiles := []profile.Profile{{ + Name: "p", + Manifest: profile.Manifest{ + Secrets: map[string]profile.SecretSpec{"tok": {}}, + Vars: map[string]profile.VarSpec{"url": {}}, + }, + Templates: []profile.File{{Rel: ".npmrc", Content: []byte("t=${secret:tok};u=${var:url}")}}, + }} + if err := pushRenderedTemplates(context.Background(), lima.Client{R: r}, c, profiles, cfgPath, io.Discard); err != nil { + t.Fatalf("pushRenderedTemplates: %v", err) + } + if !ranAny(r.calls, "install-user-file.sh") || !ranAny(r.calls, ".npmrc") || !ranAny(r.calls, "0600") { + t.Errorf("expected a relay push of .npmrc at 0600, got %v", r.calls) + } +} + +func TestPushRenderedTemplatesMissingMappingFails(t *testing.T) { + r := &recordingRunner{} + c := testCfg(t) + profiles := []profile.Profile{{ + Name: "p", + Manifest: profile.Manifest{Secrets: map[string]profile.SecretSpec{"tok": {Suggest: "gopass show -o t"}}}, + Templates: []profile.File{{Rel: ".npmrc", Content: []byte("${secret:tok}")}}, + }} + err := pushRenderedTemplates(context.Background(), lima.Client{R: r}, c, profiles, filepath.Join(t.TempDir(), "config.yaml"), io.Discard) + if err == nil || !strings.Contains(err.Error(), "gopass show -o t") { + t.Errorf("missing mapping must fail with the snippet, got %v", err) + } + if len(r.calls) != 0 { + t.Errorf("nothing may reach the guest on resolution failure, got %v", r.calls) + } +} +``` + +- [ ] **Step 2: Run to verify failure**, then **Step 3: Implement** + +`ensureRunning`: change the three `return ...` exits — "Running" → `(false, nil)`; the two start paths → `(true, cl.Start(...))` / `(true, cl.StartExisting(...))` (return `(false, err)` on pre-start errors). Update every caller: + +- `start.go` `newStartCmd`: `if _, err := ensureRunning(...); err != nil { return err }` then ALWAYS `pushRenderedTemplates(...)` (needs the config path — `loadConfigWithProfiles` already returns it). +- `shell.go` `runDefault`: capture `started`; after `session.Setup`, `if started { pushRenderedTemplates(...) }` — before `cl.Agent(...)` so the first command sees the configs. +- `recreate.go`: capture both; push when `started` (recreate always boots, so effectively always). +- `mount.go`: same pattern after its restart. +- `profile.go` apply: insert `pushRenderedTemplates` between `ApplyAllowlist` and `session.ApplyProfiles`. + +`pushRenderedTemplates` (in start.go, near `agentDeps`): + +```go +// pushRenderedTemplates resolves secrets/vars and pushes rendered templates +// into the agent home. Callers gate it to start, apply, and boot-causing +// invocations only: resolution may invoke the user's secret manager +// (pinentry), so it must never run on every command. +func pushRenderedTemplates(ctx context.Context, cl lima.Client, c config.Config, profiles []profile.Profile, cfgPath string, out io.Writer) error { + secretsDecl := profile.DeclaredSecrets(profiles) + varsDecl := profile.DeclaredVars(profiles) + templated := false + for _, p := range profiles { + if len(p.Templates) > 0 { + templated = true + } + } + if !templated && len(secretsDecl) == 0 && len(varsDecl) == 0 { + return nil + } + sources, warnings, err := config.LoadSecrets(config.SecretsPathFor(cfgPath)) + if err != nil { + return err + } + for _, w := range warnings { + fmt.Fprintf(out, "warning: %s\n", w) + } + secrets, err := profile.ResolveSecrets(ctx, secretsDecl, sources, hostCommand) + if err != nil { + return err + } + vars, err := profile.ResolveVars(varsDecl, c.Vars) + if err != nil { + return err + } + rendered := profile.RenderTemplates(profiles, secrets, vars) + d := agentDeps(cl, c, profiles) + for _, r := range rendered { + if err := session.PushUserFile(ctx, d, r.Content, r.Rel, "0600"); err != nil { + return err + } + } + if len(rendered) > 0 { + fmt.Fprintf(out, "Rendered %d template(s) into the sandbox.\n", len(rendered)) + } + return nil +} + +// hostCommand runs a secrets.yaml command through the user's shell on the +// host. CombinedOutput so a failure's stderr reaches the error message. +func hostCommand(ctx context.Context, command string) ([]byte, error) { + return exec.CommandContext(ctx, "sh", "-c", command).CombinedOutput() +} +``` + +- [ ] **Step 4: Run to verify pass** — `go test ./internal/cli/ -v` (fix any callers/tests still using the one-value `ensureRunning`). +- [ ] **Step 5: Lint, format, build, commit** + +```bash +git add internal/cli/ && git commit -m "feat: resolve and push rendered templates at start, apply, and boot" +``` + +--- + +### Task 6: `code-vm secrets` and profile CLI surfacing + +**Files:** +- Create: `internal/cli/secrets.go` +- Modify: `internal/cli/profile.go` (`add` warning, `list` marker), `internal/cli/root.go` (register) +- Test: `internal/cli/secrets_test.go`, extend `internal/cli/profile_test.go` + +**Interfaces:** +- Consumes: `profile.DeclaredSecrets/Vars`, `MissingSecretSnippet`, `config.LoadSecrets/SecretsPathFor`, `loadConfigWithProfiles`. +- Produces: `newSecretsCmd()` registered on root. Output: one line per declared secret/var — `name mapped|UNMAPPED (profiles) description` — names and status only, never values; unmapped secrets with a hint get the snippet printed after the table. Exit code 0 either way (it is a report, not a gate). + +- [ ] **Step 1: Write the failing tests** + +`internal/cli/secrets_test.go` (reuse `withScratchConfig` + a scratch profile fixture as in profile_test.go): + +```go +func TestSecretsListsMappedAndUnmapped(t *testing.T) { + root := NewRootCmd() + dir := withScratchConfig(t) + pdir := filepath.Join(dir, "profiles", "p") + os.MkdirAll(filepath.Join(pdir, "templates"), 0o755) + os.WriteFile(filepath.Join(pdir, "profile.yaml"), []byte( + "secrets:\n mapped-one:\n description: has a mapping\n missing-one:\n suggest: gopass show -o x\nvars:\n url: {}\n"), 0o644) + os.WriteFile(filepath.Join(pdir, "templates", ".npmrc"), []byte("${secret:mapped-one}${secret:missing-one}${var:url}\n"), 0o644) + os.WriteFile(filepath.Join(dir, "secrets.yaml"), []byte("secrets:\n mapped-one:\n value: v\n"), 0o600) + appendConfig(t, "profiles:\n - p\n") // helper: append to the scratch config file (add it if profile_test.go lacks one) + + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"secrets"}) + if err := root.Execute(); err != nil { + t.Fatalf("secrets: %v", err) + } + s := out.String() + for _, want := range []string{"mapped-one", "missing-one", "UNMAPPED", "url", + "command: gopass show -o x"} { + if !strings.Contains(s, want) { + t.Errorf("output missing %q:\n%s", want, s) + } + } + if strings.Contains(s, "v\n") && strings.Contains(s, "value") { + t.Errorf("secret values must never be printed:\n%s", s) + } +} +``` + +Extend profile tests: `TestProfileAddWarnsAboutDeclaredSecrets` (add a git fixture whose profile.yaml declares a secret + a matching template; assert the add output contains the secret name) and extend `TestProfileListShowsStatus` (a secret-declaring profile's row contains a `secrets` marker). + +- [ ] **Step 2: Run to verify failure**, **Step 3: Implement** + +`newSecretsCmd`: `loadConfigWithProfiles()`; collect `DeclaredSecrets`/`DeclaredVars`; `config.LoadSecrets(config.SecretsPathFor(path))`; print the table (`%-24s %-9s %-20s %s` name/status/profiles/description; vars check `c.Vars`); after the table, for each unmapped secret print a blank line, `# add to :` and `profile.MissingSecretSnippet(d)`. Register in `root.go`. + +`profile.go` `add`: after `profile.Load`, when the loaded manifest declares secrets, append to the printed output: `This profile declares secrets: . Mapping them in secrets.yaml makes their values readable by the agent (and every active profile's hook).` + +`profile.go` `list`: append a ` secrets` marker column entry for profiles with declarations (keep the existing column layout stable: add the marker to the description column's line end, e.g. `desc + " [secrets: a, b]"` — simplest change that satisfies "marks profiles that declare secrets"). + +- [ ] **Step 4: Run to verify pass**, **Step 5: Lint, format, build, commit** + +```bash +git add internal/cli/ && git commit -m "feat: add code-vm secrets and surface secret declarations in profile add/list" +``` + +--- + +### Task 7: Integration coverage and documentation + +**Files:** +- Modify: `test-vm-sandbox.sh` (inside the existing "Profiles" section), `README.md` + +**Interfaces:** +- Consumes: everything; suite conventions (`pass`/`fail`/`assert_ok`/`assert_fails`, `adm`, `agent`, `yq -i`, `$TEST_CONFIG_DIR`). + +- [ ] **Step 1: Extend the suite's Profiles section** + +Extend the existing fixture profile (`$PROFILE_FIXTURE`) BEFORE the `profile apply` call: add to its `profile.yaml`: + +```yaml +secrets: + test-token: + description: integration fixture token +vars: + test-url: {} +``` + +plus `mkdir -p "$PROFILE_FIXTURE/templates/.config"` and a template: + +```bash +printf 'token=${secret:test-token}\nurl=${var:test-url}\nkeep=${env.HOME}\n' \ + > "$PROFILE_FIXTURE/templates/.config/fixture.conf" +``` + +Map the inputs in the scratch config tree: + +```bash +printf 'secrets:\n test-token:\n value: sekrit-value\n' > "$TEST_CONFIG_DIR/secrets.yaml" +chmod 0600 "$TEST_CONFIG_DIR/secrets.yaml" +yq -i '.vars = {"test-url": "https://fixture.example"}' "$CONFIG_FILE" +``` + +After the existing `profile apply` assertions, add: + +```bash +RENDERED="/home/$AGENT_USER/.config/fixture.conf" +if adm cat "$RENDERED" 2> /dev/null | grep -q 'token=sekrit-value'; then + pass "template secret is substituted in the guest" +else + fail "template secret is substituted in the guest" +fi +assert_ok "template var is substituted" \ + adm grep -q 'url=https://fixture.example' "$RENDERED" +assert_ok "unrelated placeholders pass through" \ + adm grep -qF 'keep=${env.HOME}' "$RENDERED" +if [ "$(adm stat -c '%u %a' "$RENDERED")" = "$(id -u) 600" ]; then + pass "rendered template is agent-owned 0600" +else + fail "rendered template is agent-owned 0600 (got $(adm stat -c '%u %a' "$RENDERED"))" +fi + +# Rotation: change the mapped value, re-apply, and the rendered file updates. +printf 'secrets:\n test-token:\n value: rotated-value\n' > "$TEST_CONFIG_DIR/secrets.yaml" +"${CODE_VM_ARGS[@]}" profile apply > /dev/null 2>&1 +assert_ok "a mapping change plus apply updates the rendered template" \ + adm grep -q 'token=rotated-value' "$RENDERED" + +# A declared-but-unmapped secret must fail apply with the snippet, before +# anything reaches the guest. yq, not printf-append: a second top-level +# `secrets:` key would be a YAML duplicate-key parse error, a different +# failure than the one under test. +yq -i '.secrets.extra-unmapped = {"suggest": "gopass show -o nope"}' "$PROFILE_FIXTURE/profile.yaml" +printf 'x=${secret:extra-unmapped}\n' > "$PROFILE_FIXTURE/templates/.config/extra.conf" +UNMAPPED_OUT=$("${CODE_VM_ARGS[@]}" profile apply 2>&1) +if echo "$UNMAPPED_OUT" | grep -q 'gopass show -o nope'; then + pass "unmapped secret fails apply with the ready-to-paste snippet" +else + fail "unmapped secret fails apply with the ready-to-paste snippet (got: $UNMAPPED_OUT)" +fi +# Restore the fixture to the mapped-only state for the deactivation steps. +rm -f "$PROFILE_FIXTURE/templates/.config/extra.conf" +yq -i 'del(.secrets.extra-unmapped)' "$PROFILE_FIXTURE/profile.yaml" +"${CODE_VM_ARGS[@]}" profile apply > /dev/null 2>&1 + +adm rm -f "$RENDERED" > /dev/null 2>&1 # cleanup with the other fixture artifacts +``` + +(Place the `adm rm -f "$RENDERED"` with the section's existing cleanup block, after the deactivation assertions; the fixture profile dir removal already covers the host side. Deliberate omission: the spec's "survives a restart" integration bullet is not asserted — persistence of a regular file in the ext4 home exercises no mechanism of this feature, and a dedicated restart would add minutes to the suite; the start-time push path is the same code the apply path already covers.) + +Run `bash -n test-vm-sandbox.sh` and `mise run lint`. + +- [ ] **Step 2: README** + +Add a `### Credentials` subsection under Profiles documenting: the trust model (hints never execute; mapping a secret exposes it to the agent and every active profile's hook), `secrets.yaml` with the gopass example, `vars:` in config.yaml, the Maven `settings.xml` walkthrough (placeholders for ``/``, static proxies/mirrors shipped as-is), when resolution happens (start/apply/boot — never per invocation), the boot-ordering caveat (hooks run before the first push after a cold boot; hooks must tolerate absent templates), and rotation (rotate at source → restart or `profile apply`). Also add `code-vm secrets` to the command list. + +- [ ] **Step 3: Full verification** + +`mise run test:unit && mise run lint && mise run fmt-check && mise run build`, then `mise run test:vm` (controller may run this; all profile-section assertions plus the pre-existing 111 must pass). + +- [ ] **Step 4: Commit** + +```bash +git add test-vm-sandbox.sh README.md && git commit -m "test: cover profile secrets/templates in the suite; document credentials" +``` From 7e5766b5bff366bda0c03163b0c4af23f3b8e164 Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 12:50:21 +0200 Subject: [PATCH 03/23] feat: profiles declare secrets/vars and ship templates Add SecretSpec and VarSpec types to manifest declarations, enabling profiles to declare environment variables and secrets they require. Add Templates []File to Profile to load and validate a templates/ tree alongside files/. Factor loadFiles into a shared loadTree(dir, subdir) function to handle both trees with identical symlink/validation logic, parameter-driven error messages. Validate secret/var names via ValidateName. Reject file/template path collisions and forbidden paths (.claude/settings.*) in both trees. Extend empty-profile check to include templates, secrets, and vars. Co-Authored-By: Claude Fable 5 --- internal/profile/profile.go | 99 ++++++++++++++++++++++++-------- internal/profile/profile_test.go | 80 ++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 24 deletions(-) diff --git a/internal/profile/profile.go b/internal/profile/profile.go index 3d0dd10..7e000ce 100644 --- a/internal/profile/profile.go +++ b/internal/profile/profile.go @@ -64,14 +64,27 @@ var forbiddenFiles = map[string]bool{ ".claude/settings.local.json": true, } +// SecretSpec declares a secret the profile requires. +type SecretSpec struct { + Description string `yaml:"description"` + Suggest string `yaml:"suggest"` +} + +// VarSpec declares a variable the profile requires. +type VarSpec struct { + Description string `yaml:"description"` +} + // Manifest is the profile.yaml schema. Every key is optional, but a profile // that declares nothing at all is rejected. type Manifest struct { - Description string `yaml:"description"` - Packages []string `yaml:"packages"` - Shell string `yaml:"shell"` - Domains []string `yaml:"domains"` - Hook string `yaml:"hook"` + Description string `yaml:"description"` + Packages []string `yaml:"packages"` + Shell string `yaml:"shell"` + Domains []string `yaml:"domains"` + Hook string `yaml:"hook"` + Secrets map[string]SecretSpec `yaml:"secrets"` + Vars map[string]VarSpec `yaml:"vars"` } // File is one file a profile ships into the agent home. @@ -83,11 +96,12 @@ type File struct { // Profile is a loaded, validated bundle. type Profile struct { - Name string - Dir string - Manifest Manifest - Files []File // sorted by Rel - Hook []byte // nil when the manifest declares no hook + Name string + Dir string + Manifest Manifest + Files []File // sorted by Rel + Templates []File // sorted by Rel + Hook []byte // nil when the manifest declares no hook } // ValidateName reports whether name is usable as a profile name. Exported @@ -145,6 +159,19 @@ func Load(profilesDir, name string) (Profile, error) { if p.Files, err = loadFiles(dir); err != nil { return Profile{}, fmt.Errorf("profile %s: %w", name, err) } + if p.Templates, err = loadTemplates(dir); err != nil { + return Profile{}, fmt.Errorf("profile %s: %w", name, err) + } + // Check for collisions between files/ and templates/. + fileRels := map[string]bool{} + for _, f := range p.Files { + fileRels[f.Rel] = true + } + for _, tpl := range p.Templates { + if fileRels[tpl.Rel] { + return Profile{}, fmt.Errorf("profile %s: %s is shipped by both files/ and templates/; pick one", name, tpl.Rel) + } + } if m.Hook != "" { hookPath := filepath.Join(dir, m.Hook) // Lstat, not Stat: a hostile bundle could symlink its hook at a file @@ -167,8 +194,8 @@ func Load(profilesDir, name string) (Profile, error) { } p.Hook = b } - if len(p.Files) == 0 && len(m.Packages) == 0 && m.Shell == "" && len(m.Domains) == 0 && m.Hook == "" { - return Profile{}, fmt.Errorf("profile %s: declares nothing: no files, packages, shell, domains or hook", name) + if len(p.Files) == 0 && len(m.Packages) == 0 && m.Shell == "" && len(m.Domains) == 0 && m.Hook == "" && len(p.Templates) == 0 && len(m.Secrets) == 0 && len(m.Vars) == 0 { + return Profile{}, fmt.Errorf("profile %s: declares nothing: no files, packages, shell, domains, hook, templates, secrets or vars", name) } return p, nil } @@ -209,15 +236,25 @@ func validateManifest(m Manifest) error { if m.Hook != "" && !hookRe.MatchString(m.Hook) { return fmt.Errorf("hook must be a plain file name inside the profile, got %q", m.Hook) } + for name := range m.Secrets { + if err := ValidateName(name); err != nil { + return fmt.Errorf("secret name %q: must look like %q", name, "repo-user") + } + } + for name := range m.Vars { + if err := ValidateName(name); err != nil { + return fmt.Errorf("var name %q: must look like %q", name, "artifactory-url") + } + } return nil } -// loadFiles reads the files/ tree. Only regular files are accepted: a symlink +// loadTree reads a tree from dir/subdir. Only regular files are accepted: a symlink // could escape the tree on the host, or change content between validation and -// delivery. -func loadFiles(dir string) ([]File, error) { - root := filepath.Join(dir, "files") - // Lstat, not Stat: a symlinked files/ root would let WalkDir walk +// delivery. subdir is used as the prefix in error messages and the subdirectory name. +func loadTree(dir, subdir string) ([]File, error) { + root := filepath.Join(dir, subdir) + // Lstat, not Stat: a symlinked root would let WalkDir walk // wherever it points, including a directory an agent controls. info, err := os.Lstat(root) if errors.Is(err, os.ErrNotExist) { @@ -227,13 +264,13 @@ func loadFiles(dir string) ([]File, error) { return nil, err } if !info.Mode().IsDir() { - // Folds in the former "files is a regular file" case: a non-directory - // "files" makes WalkDir yield a single entry with rel ".", which + // Folds in the former "subdir is a regular file" case: a non-directory + // subdir makes WalkDir yield a single entry with rel ".", which // passes every check below (it is a regular file, it matches // relPathRe, it is not forbidden) and would otherwise be silently - // installed as-is. A symlinked "files" (to a directory or otherwise) + // installed as-is. A symlinked subdir (to a directory or otherwise) // is rejected the same way, by the same message. - return nil, fmt.Errorf("files must be a real directory (symlinks are rejected)") + return nil, fmt.Errorf("%s must be a real directory (symlinks are rejected)", subdir) } var out []File err = filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { @@ -249,7 +286,7 @@ func loadFiles(dir string) ([]File, error) { } rel = filepath.ToSlash(rel) if !d.Type().IsRegular() { - return fmt.Errorf("files/%s: only regular files may be shipped (symlinks are rejected)", rel) + return fmt.Errorf("%s/%s: only regular files may be shipped (symlinks are rejected)", subdir, rel) } if !relPathRe.MatchString(rel) { return fmt.Errorf("file path %q: only [a-zA-Z0-9._/-] is allowed", rel) @@ -260,14 +297,14 @@ func loadFiles(dir string) ([]File, error) { } } if forbiddenFiles[rel] { - return fmt.Errorf("files/%s: profiles may not ship the locked Claude settings", rel) + return fmt.Errorf("%s/%s: profiles may not ship the locked Claude settings", subdir, rel) } b, err := os.ReadFile(p) if err != nil { return err } if isBlank(b) { - return fmt.Errorf("files/%s: must not be blank (empty or newline-only content cannot be embedded)", rel) + return fmt.Errorf("%s/%s: must not be blank (empty or newline-only content cannot be embedded)", subdir, rel) } info, err := d.Info() if err != nil { @@ -282,3 +319,17 @@ func loadFiles(dir string) ([]File, error) { sort.Slice(out, func(i, j int) bool { return out[i].Rel < out[j].Rel }) return out, nil } + +// loadFiles reads the files/ tree. Only regular files are accepted: a symlink +// could escape the tree on the host, or change content between validation and +// delivery. +func loadFiles(dir string) ([]File, error) { + return loadTree(dir, "files") +} + +// loadTemplates reads the templates/ tree. Only regular files are accepted: a symlink +// could escape the tree on the host, or change content between validation and +// delivery. +func loadTemplates(dir string) ([]File, error) { + return loadTree(dir, "templates") +} diff --git a/internal/profile/profile_test.go b/internal/profile/profile_test.go index 12376dd..fc05552 100644 --- a/internal/profile/profile_test.go +++ b/internal/profile/profile_test.go @@ -290,3 +290,83 @@ func TestLoadAllPreservesOrderAndRejectsDuplicates(t *testing.T) { t.Errorf("no names must load cleanly to an empty slice, got %v, %v", ps, err) } } + +func TestLoadTemplatesAndDeclarations(t *testing.T) { + dir := t.TempDir() + writeProfile(t, dir, "maven", ` +description: maven setup +secrets: + repo-user: + description: Artifactory user + suggest: gopass show -o wetf/artifactory-user + repo-password: {} +vars: + artifactory-url: + description: Base URL +`, map[string]string{ + "templates/.m2/settings.xml": "${secret:repo-user}/${var:artifactory-url}\n", + }) + p, err := Load(dir, "maven") + if err != nil { + t.Fatalf("Load: %v", err) + } + if p.Manifest.Secrets["repo-user"].Suggest != "gopass show -o wetf/artifactory-user" { + t.Errorf("Suggest not loaded: %+v", p.Manifest.Secrets) + } + if _, ok := p.Manifest.Secrets["repo-password"]; !ok { + t.Error("empty-spec secret not loaded") + } + if len(p.Templates) != 1 || p.Templates[0].Rel != ".m2/settings.xml" { + t.Fatalf("Templates = %+v", p.Templates) + } +} + +// A profile carrying only declarations and templates is a valid profile. +func TestLoadTemplatesOnlyProfileIsNotEmpty(t *testing.T) { + dir := t.TempDir() + writeProfile(t, dir, "p", "secrets:\n tok: {}\n", map[string]string{ + "templates/.npmrc": "//registry/:_authToken=${secret:tok}\n", + }) + if _, err := Load(dir, "p"); err != nil { + t.Fatalf("Load: %v", err) + } +} + +func TestLoadRejectsInvalidDeclarations(t *testing.T) { + tests := []struct { + name string + manifest string + files map[string]string + wantErr string + }{ + {"bad secret name", "secrets:\n 'has space': {}\n", nil, "secret name"}, + {"bad var name", "vars:\n 'has/slash': {}\n", nil, "var name"}, + {"template/file collision", "description: x\n", + map[string]string{"files/.npmrc": "a\n", "templates/.npmrc": "b\n"}, + "both files/ and templates/"}, + {"template ships locked settings", "description: x\n", + map[string]string{"templates/.claude/settings.json": "{}\n"}, + "locked Claude settings"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + writeProfile(t, dir, "p", tt.manifest, tt.files) + _, err := Load(dir, "p") + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Errorf("Load error = %v, want it to contain %q", err, tt.wantErr) + } + }) + } +} + +func TestLoadRejectsSymlinkedTemplatesRoot(t *testing.T) { + dir := t.TempDir() + writeProfile(t, dir, "p", "description: x\n", map[string]string{"files/a": "x\n"}) + if err := os.Symlink(t.TempDir(), filepath.Join(dir, "p", "templates")); err != nil { + t.Skip("symlinks unavailable") + } + if _, err := Load(dir, "p"); err == nil || !strings.Contains(err.Error(), "symlinks are rejected") { + t.Errorf("Load error = %v, want symlink rejection", err) + } +} From 274111d3638875db73be886254f24ab4101dbf96 Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 12:54:58 +0200 Subject: [PATCH 04/23] feat: user-side secrets.yaml and config vars Implement secrets.yaml loader (SecretsPathFor, LoadSecrets) with permission warnings; add Config.Vars for non-secret template values with key validation. Co-Authored-By: Claude Fable 5 --- internal/config/config.go | 8 ++++ internal/config/config_test.go | 13 ++++++ internal/config/secrets.go | 71 ++++++++++++++++++++++++++++++++ internal/config/secrets_test.go | 72 +++++++++++++++++++++++++++++++++ 4 files changed, 164 insertions(+) create mode 100644 internal/config/secrets.go create mode 100644 internal/config/secrets_test.go diff --git a/internal/config/config.go b/internal/config/config.go index e387fd9..e3a5f46 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -52,6 +52,9 @@ type Config struct { // wins. Each name must exist under the profiles directory next to this // config file; that is checked when profiles are loaded, not here. Profiles []string `yaml:"profiles,omitempty"` + // Vars are non-secret literal values available to profile templates as + // ${var:name}. Secrets never belong here — they go in secrets.yaml. + Vars map[string]string `yaml:"vars,omitempty"` } // Default returns the built-in configuration. Disk is large because Docker @@ -157,6 +160,11 @@ func (c Config) Validate() error { } seenProfiles[p] = true } + for name := range c.Vars { + if !instanceRe.MatchString(name) { + return fmt.Errorf("vars: key %q must be a name like %q", name, "artifactory-url") + } + } return nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 0e0cbde..8f1daf2 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -292,3 +292,16 @@ func TestMountsExcludeTree(t *testing.T) { }) } } + +func TestValidateVars(t *testing.T) { + c := Default() + c.ProjectsRoot = "/home/st/projects" + c.Vars = map[string]string{"artifactory-url": "https://x"} + if err := c.Validate(); err != nil { + t.Errorf("Validate: %v", err) + } + c.Vars = map[string]string{"has space": "v"} + if err := c.Validate(); err == nil || !strings.Contains(err.Error(), "vars") { + t.Errorf("Validate = %v, want vars key rejection", err) + } +} diff --git a/internal/config/secrets.go b/internal/config/secrets.go new file mode 100644 index 0000000..b53bdea --- /dev/null +++ b/internal/config/secrets.go @@ -0,0 +1,71 @@ +package config + +import ( + "bytes" + "errors" + "fmt" + "io" + "os" + "path/filepath" + + "gopkg.in/yaml.v3" +) + +// SecretSource is one user-authored mapping in secrets.yaml: exactly one of a +// host command (stdout is the value) or a literal. This file is host-trusted +// like config.yaml — it lives in the same mount-guarded tree and only the +// user writes it; profiles can only *suggest* entries, never install them. +type SecretSource struct { + Command string `yaml:"command"` + Value string `yaml:"value"` +} + +// secretsFile is the secrets.yaml schema. +type secretsFile struct { + Secrets map[string]SecretSource `yaml:"secrets"` +} + +// SecretsPathFor returns the secrets file belonging to a config file: a +// secrets.yaml next to it, protected by the same mount-exclusion guards. +func SecretsPathFor(configPath string) string { + return filepath.Join(filepath.Dir(configPath), "secrets.yaml") +} + +// LoadSecrets reads and validates secrets.yaml. A missing file is an empty +// mapping — profiles without secrets must not require one. Warnings (not +// errors) report loose file permissions: the file holds commands and possibly +// literal credentials. +func LoadSecrets(path string) (map[string]SecretSource, []string, error) { + data, err := os.ReadFile(path) + if errors.Is(err, os.ErrNotExist) { + return map[string]SecretSource{}, nil, nil + } + if err != nil { + return nil, nil, fmt.Errorf("read %s: %w", path, err) + } + var warnings []string + if fi, err := os.Stat(path); err == nil && fi.Mode().Perm()&0o077 != 0 { + warnings = append(warnings, fmt.Sprintf( + "%s is readable by group/others; recommend chmod 0600", path)) + } + var f secretsFile + dec := yaml.NewDecoder(bytes.NewReader(data)) + dec.KnownFields(true) + // An empty file decodes to io.EOF, and an empty secrets.yaml is as valid + // as a missing one. + if err := dec.Decode(&f); err != nil && !errors.Is(err, io.EOF) { + return nil, nil, fmt.Errorf("parse %s: %w", path, err) + } + for name, src := range f.Secrets { + if !instanceRe.MatchString(name) { + return nil, nil, fmt.Errorf("%s: secret name %q: must look like %q", path, name, "repo-user") + } + if (src.Command == "") == (src.Value == "") { + return nil, nil, fmt.Errorf("%s: secret %q: exactly one of command or value must be set", path, name) + } + } + if f.Secrets == nil { + f.Secrets = map[string]SecretSource{} + } + return f.Secrets, warnings, nil +} diff --git a/internal/config/secrets_test.go b/internal/config/secrets_test.go new file mode 100644 index 0000000..e92688f --- /dev/null +++ b/internal/config/secrets_test.go @@ -0,0 +1,72 @@ +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestSecretsPathFor(t *testing.T) { + if got := SecretsPathFor("/home/st/.config/code-vm/config.yaml"); got != "/home/st/.config/code-vm/secrets.yaml" { + t.Errorf("SecretsPathFor = %q", got) + } +} + +func TestLoadSecrets(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "secrets.yaml") + content := "secrets:\n a:\n command: gopass show -o x\n b:\n value: literal\n" + if err := os.WriteFile(p, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + sources, warnings, err := LoadSecrets(p) + if err != nil { + t.Fatalf("LoadSecrets: %v", err) + } + if len(warnings) != 0 { + t.Errorf("warnings = %v, want none for 0600", warnings) + } + if sources["a"].Command != "gopass show -o x" || sources["b"].Value != "literal" { + t.Errorf("sources = %+v", sources) + } +} + +func TestLoadSecretsMissingFileIsEmpty(t *testing.T) { + sources, warnings, err := LoadSecrets(filepath.Join(t.TempDir(), "secrets.yaml")) + if err != nil || len(sources) != 0 || len(warnings) != 0 { + t.Errorf("missing file must load empty: %v %v %v", sources, warnings, err) + } +} + +func TestLoadSecretsRejectsBadEntries(t *testing.T) { + tests := []struct{ name, content, wantErr string }{ + {"both command and value", "secrets:\n a:\n command: c\n value: v\n", "exactly one"}, + {"neither", "secrets:\n a: {}\n", "exactly one"}, + {"bad name", "secrets:\n 'has space':\n value: v\n", "secret name"}, + {"unknown key", "secrets:\n a:\n comand: typo\n", "not found"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := filepath.Join(t.TempDir(), "secrets.yaml") + if err := os.WriteFile(p, []byte(tt.content), 0o600); err != nil { + t.Fatal(err) + } + _, _, err := LoadSecrets(p) + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Errorf("LoadSecrets = %v, want %q", err, tt.wantErr) + } + }) + } +} + +func TestLoadSecretsWarnsOnLoosePermissions(t *testing.T) { + p := filepath.Join(t.TempDir(), "secrets.yaml") + if err := os.WriteFile(p, []byte("secrets:\n a:\n value: v\n"), 0o644); err != nil { + t.Fatal(err) + } + _, warnings, err := LoadSecrets(p) + if err != nil || len(warnings) != 1 || !strings.Contains(warnings[0], "0600") { + t.Errorf("want a permissions warning recommending 0600, got %v %v", warnings, err) + } +} From f99fcbbbbb3e8e63bfeae39bb202409f84da16cb Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 12:59:56 +0200 Subject: [PATCH 05/23] feat: template rendering and secret/var resolution Add template.go with placeholder scanning, rendering, declaration merging, and resolution. Wire undeclared-placeholder validation into Load to ensure all ${secret:*} and ${var:*} references are declared in the manifest. Co-Authored-By: Claude Fable 5 --- internal/profile/profile.go | 16 +++ internal/profile/template.go | 211 ++++++++++++++++++++++++++++++ internal/profile/template_test.go | 143 ++++++++++++++++++++ 3 files changed, 370 insertions(+) create mode 100644 internal/profile/template.go create mode 100644 internal/profile/template_test.go diff --git a/internal/profile/profile.go b/internal/profile/profile.go index 7e000ce..e551a65 100644 --- a/internal/profile/profile.go +++ b/internal/profile/profile.go @@ -162,6 +162,22 @@ func Load(profilesDir, name string) (Profile, error) { if p.Templates, err = loadTemplates(dir); err != nil { return Profile{}, fmt.Errorf("profile %s: %w", name, err) } + // Validate that templates only reference declared secrets and vars. + for _, tpl := range p.Templates { + for _, ref := range FindRefs(tpl.Content) { + declared := false + if ref.Kind == "secret" { + _, declared = m.Secrets[ref.Name] + } else { + _, declared = m.Vars[ref.Name] + } + if !declared { + return Profile{}, fmt.Errorf( + "profile %s: templates/%s references ${%s:%s}, which the manifest does not declare", + name, tpl.Rel, ref.Kind, ref.Name) + } + } + } // Check for collisions between files/ and templates/. fileRels := map[string]bool{} for _, f := range p.Files { diff --git a/internal/profile/template.go b/internal/profile/template.go new file mode 100644 index 0000000..b117e9b --- /dev/null +++ b/internal/profile/template.go @@ -0,0 +1,211 @@ +package profile + +import ( + "context" + "fmt" + "regexp" + "sort" + "strings" + + "github.com/wetransform/code-vm/internal/config" +) + +// refRe matches exactly the two placeholder forms templates may use. The name +// charset mirrors ValidateName, so anything else — Maven properties, +// ${env.FOO} — is left untouched by both the scanner and the renderer. +var refRe = regexp.MustCompile(`\$\{(secret|var):([a-zA-Z0-9][a-zA-Z0-9-]{0,62})\}`) + +// Ref is one placeholder occurrence kind+name. +type Ref struct { + Kind string // "secret" or "var" + Name string +} + +// FindRefs returns the distinct placeholder references in content, in first- +// appearance order. +func FindRefs(content []byte) []Ref { + seen := map[Ref]bool{} + var out []Ref + for _, m := range refRe.FindAllSubmatch(content, -1) { + r := Ref{Kind: string(m[1]), Name: string(m[2])} + if !seen[r] { + seen[r] = true + out = append(out, r) + } + } + return out +} + +// Rendered is one template after substitution, destined for the agent home. +type Rendered struct { + Rel string + Content []byte +} + +// RenderTemplates substitutes secret and var values into every active +// profile's templates. Later profiles win Rel collisions, matching the files/ +// rule. Values are opaque bytes: no escaping layer, exactly as the spec +// states — a value that breaks the target format is the user's own. +func RenderTemplates(profiles []Profile, secrets, vars map[string]string) []Rendered { + byRel := map[string][]byte{} + for _, p := range profiles { + for _, tpl := range p.Templates { + byRel[tpl.Rel] = refRe.ReplaceAllFunc(tpl.Content, func(m []byte) []byte { + sub := refRe.FindSubmatch(m) + if string(sub[1]) == "secret" { + return []byte(secrets[string(sub[2])]) + } + return []byte(vars[string(sub[2])]) + }) + } + } + out := make([]Rendered, 0, len(byRel)) + for rel, content := range byRel { + out = append(out, Rendered{Rel: rel, Content: content}) + } + sort.Slice(out, func(i, j int) bool { return out[i].Rel < out[j].Rel }) + return out +} + +// DeclaredSecret is one secret name unioned across the active profiles. +type DeclaredSecret struct { + Name string + Profiles []string // declaring profiles, in activation order + Description string // first non-empty wins + Suggest string // first non-empty wins; inert display string +} + +// DeclaredVar is the var analog. +type DeclaredVar struct { + Name string + Profiles []string + Description string +} + +// DeclaredSecrets unions secret declarations across profiles, sorted by name. +func DeclaredSecrets(profiles []Profile) []DeclaredSecret { + byName := map[string]*DeclaredSecret{} + for _, p := range profiles { + names := make([]string, 0, len(p.Manifest.Secrets)) + for n := range p.Manifest.Secrets { + names = append(names, n) + } + sort.Strings(names) // map order is random; keep Profiles deterministic + for _, n := range names { + spec := p.Manifest.Secrets[n] + d, ok := byName[n] + if !ok { + d = &DeclaredSecret{Name: n} + byName[n] = d + } + d.Profiles = append(d.Profiles, p.Name) + if d.Description == "" { + d.Description = spec.Description + } + if d.Suggest == "" { + d.Suggest = spec.Suggest + } + } + } + out := make([]DeclaredSecret, 0, len(byName)) + for _, d := range byName { + out = append(out, *d) + } + sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) + return out +} + +// DeclaredVars unions var declarations across profiles, sorted by name. +func DeclaredVars(profiles []Profile) []DeclaredVar { + byName := map[string]*DeclaredVar{} + for _, p := range profiles { + names := make([]string, 0, len(p.Manifest.Vars)) + for n := range p.Manifest.Vars { + names = append(names, n) + } + sort.Strings(names) + for _, n := range names { + spec := p.Manifest.Vars[n] + d, ok := byName[n] + if !ok { + d = &DeclaredVar{Name: n} + byName[n] = d + } + d.Profiles = append(d.Profiles, p.Name) + if d.Description == "" { + d.Description = spec.Description + } + } + } + out := make([]DeclaredVar, 0, len(byName)) + for _, d := range byName { + out = append(out, *d) + } + sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) + return out +} + +// CommandRunner executes a user-authored secrets.yaml command on the host and +// returns its combined output. Injectable for tests. +type CommandRunner func(ctx context.Context, command string) ([]byte, error) + +// MissingSecretSnippet renders the ready-to-paste secrets.yaml block for an +// unmapped secret. The suggest hint is copied verbatim as the command — +// display only until the user adopts it by saving this snippet themselves. +func MissingSecretSnippet(d DeclaredSecret) string { + cmd := d.Suggest + if cmd == "" { + cmd = "" + } + return fmt.Sprintf("secrets:\n %s:\n command: %s\n", d.Name, cmd) +} + +// ResolveSecrets resolves every declared secret from the user's sources. Each +// command runs exactly once per resolve pass with one trailing newline +// stripped (the gopass/pass convention). A missing mapping fails with an +// actionable snippet rather than prompting or falling back to hints: hints +// never execute. +func ResolveSecrets(ctx context.Context, declared []DeclaredSecret, sources map[string]config.SecretSource, run CommandRunner) (map[string]string, error) { + out := make(map[string]string, len(declared)) + for _, d := range declared { + src, ok := sources[d.Name] + if !ok { + desc := d.Description + if desc == "" { + desc = "no description" + } + return nil, fmt.Errorf( + "profile %s needs secret %q (%s), but secrets.yaml does not map it.\nAdd to ~/.config/code-vm/secrets.yaml:\n\n%s", + strings.Join(d.Profiles, ", "), d.Name, desc, MissingSecretSnippet(d)) + } + if src.Command != "" { + b, err := run(ctx, src.Command) + if err != nil { + return nil, fmt.Errorf("secret %q: command failed: %w: %s", d.Name, err, strings.TrimSpace(string(b))) + } + out[d.Name] = strings.TrimSuffix(string(b), "\n") + continue + } + out[d.Name] = src.Value + } + return out, nil +} + +// ResolveVars resolves declared vars from config.yaml's literal map. +func ResolveVars(declared []DeclaredVar, values map[string]string) (map[string]string, error) { + out := make(map[string]string, len(declared)) + for _, d := range declared { + v, ok := values[d.Name] + if !ok { + desc := d.Description + if desc == "" { + desc = "no description" + } + return nil, fmt.Errorf( + "profile %s needs var %q (%s), but config.yaml does not set it.\nAdd to config.yaml:\n\nvars:\n %s: ", + strings.Join(d.Profiles, ", "), d.Name, desc, d.Name) + } + out[d.Name] = v + } + return out, nil +} diff --git a/internal/profile/template_test.go b/internal/profile/template_test.go new file mode 100644 index 0000000..98020be --- /dev/null +++ b/internal/profile/template_test.go @@ -0,0 +1,143 @@ +package profile + +import ( + "context" + "errors" + "reflect" + "strings" + "testing" + + "github.com/wetransform/code-vm/internal/config" +) + +func TestFindRefs(t *testing.T) { + content := []byte(`user=${secret:repo-user} url=${var:base-url} +again=${secret:repo-user} passthrough=${env.FOO} ${prop} $secret:no ${secret:BAD NAME}`) + got := FindRefs(content) + want := []Ref{{Kind: "secret", Name: "repo-user"}, {Kind: "var", Name: "base-url"}} + if !reflect.DeepEqual(got, want) { + t.Errorf("FindRefs = %v, want %v", got, want) + } +} + +func TestRenderTemplatesSubstitutesAndPassesThrough(t *testing.T) { + profiles := []Profile{{ + Name: "a", + Templates: []File{{Rel: ".m2/settings.xml", Content: []byte( + "${secret:repo-user}${var:base-url}${env.HOME}")}}, + }} + out := RenderTemplates(profiles, + map[string]string{"repo-user": "simon"}, + map[string]string{"base-url": "https://x.example"}) + if len(out) != 1 { + t.Fatalf("Rendered = %+v", out) + } + want := "simonhttps://x.example${env.HOME}" + if string(out[0].Content) != want { + t.Errorf("Content = %q, want %q", out[0].Content, want) + } +} + +func TestRenderTemplatesLaterProfileWins(t *testing.T) { + profiles := []Profile{ + {Name: "a", Templates: []File{{Rel: ".npmrc", Content: []byte("from-a")}}}, + {Name: "b", Templates: []File{{Rel: ".npmrc", Content: []byte("from-b")}}}, + } + out := RenderTemplates(profiles, nil, nil) + if len(out) != 1 || string(out[0].Content) != "from-b" { + t.Errorf("collision must resolve to the later profile, got %+v", out) + } +} + +func TestDeclaredSecretsMergesAcrossProfiles(t *testing.T) { + profiles := []Profile{ + {Name: "a", Manifest: Manifest{Secrets: map[string]SecretSpec{ + "tok": {Description: "token", Suggest: "gopass show -o t"}}}}, + {Name: "b", Manifest: Manifest{Secrets: map[string]SecretSpec{"tok": {}}}}, + } + got := DeclaredSecrets(profiles) + if len(got) != 1 || got[0].Name != "tok" || got[0].Suggest != "gopass show -o t" || + !reflect.DeepEqual(got[0].Profiles, []string{"a", "b"}) { + t.Errorf("DeclaredSecrets = %+v", got) + } +} + +func TestResolveSecrets(t *testing.T) { + declared := []DeclaredSecret{ + {Name: "from-cmd", Profiles: []string{"p"}}, + {Name: "from-val", Profiles: []string{"p"}}, + } + sources := map[string]config.SecretSource{ + "from-cmd": {Command: "get-it"}, + "from-val": {Value: "literal"}, + } + calls := 0 + run := func(_ context.Context, command string) ([]byte, error) { + calls++ + if command != "get-it" { + t.Errorf("command = %q", command) + } + return []byte("resolved\n"), nil + } + got, err := ResolveSecrets(context.Background(), declared, sources, run) + if err != nil { + t.Fatalf("ResolveSecrets: %v", err) + } + // Exactly one trailing newline stripped; command runs once per secret. + if got["from-cmd"] != "resolved" || got["from-val"] != "literal" || calls != 1 { + t.Errorf("got %v, calls=%d", got, calls) + } +} + +func TestResolveSecretsMissingMappingHasSnippet(t *testing.T) { + declared := []DeclaredSecret{{ + Name: "repo-user", Profiles: []string{"maven"}, + Description: "Artifactory user", Suggest: "gopass show -o wetf/user", + }} + _, err := ResolveSecrets(context.Background(), declared, nil, nil) + if err == nil { + t.Fatal("expected an error for an unmapped secret") + } + for _, want := range []string{"repo-user", "maven", "Artifactory user", + "secrets:", "command: gopass show -o wetf/user"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error missing %q:\n%s", want, err) + } + } +} + +func TestResolveSecretsCommandFailure(t *testing.T) { + declared := []DeclaredSecret{{Name: "tok", Profiles: []string{"p"}}} + sources := map[string]config.SecretSource{"tok": {Command: "boom"}} + run := func(context.Context, string) ([]byte, error) { + return []byte("stderr text"), errors.New("exit status 1") + } + _, err := ResolveSecrets(context.Background(), declared, sources, run) + if err == nil || !strings.Contains(err.Error(), "tok") || !strings.Contains(err.Error(), "exit status 1") { + t.Errorf("ResolveSecrets error = %v", err) + } +} + +func TestResolveVars(t *testing.T) { + declared := []DeclaredVar{{Name: "url", Profiles: []string{"p"}, Description: "Base URL"}} + got, err := ResolveVars(declared, map[string]string{"url": "https://x"}) + if err != nil || got["url"] != "https://x" { + t.Errorf("ResolveVars = %v, %v", got, err) + } + _, err = ResolveVars(declared, nil) + if err == nil || !strings.Contains(err.Error(), "vars:") || !strings.Contains(err.Error(), "url") { + t.Errorf("missing var must produce a config.yaml snippet, got %v", err) + } +} + +// Load must reject a template referencing an undeclared name (wired in this task). +func TestLoadRejectsUndeclaredPlaceholder(t *testing.T) { + dir := t.TempDir() + writeProfile(t, dir, "p", "secrets:\n known: {}\n", map[string]string{ + "templates/.npmrc": "a=${secret:known} b=${var:never-declared}\n", + }) + _, err := Load(dir, "p") + if err == nil || !strings.Contains(err.Error(), "never-declared") { + t.Errorf("Load = %v, want undeclared-placeholder rejection", err) + } +} From 8f12acfec09a6439f3e7a1be4e197d2deb7cad54 Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 13:09:54 +0200 Subject: [PATCH 06/23] feat: agent-privilege user-file push; migrate git identity to it installContent's root install into an agent-owned home path is the TOCTOU class the profile applier already closed for profile files. Factor stageFile out of installContent (temp file + staging-dir + Copy) and add PushUserFile, which relays the staged file through the new install-user-file.sh guest script instead: root drops it into a root:AGENT_GID 0640 tmpfs area, then an agent-identity setpriv install (mirroring apply-profiles.sh's run_as_agent_sh) places it in the home. Migrate ApplyGitIdentity to PushUserFile so .gitconfig never gets a direct root write. Co-Authored-By: Claude Fable 5 --- internal/guest/embed_test.go | 16 ++++++ .../guest/files/scripts/install-user-file.sh | 45 +++++++++++++++++ internal/session/gitidentity.go | 10 ++-- internal/session/gitidentity_test.go | 18 +++---- internal/session/stage.go | 37 ++++++++++---- internal/session/userfiles.go | 26 ++++++++++ internal/session/userfiles_test.go | 50 +++++++++++++++++++ 7 files changed, 174 insertions(+), 28 deletions(-) create mode 100644 internal/guest/files/scripts/install-user-file.sh create mode 100644 internal/session/userfiles.go create mode 100644 internal/session/userfiles_test.go diff --git a/internal/guest/embed_test.go b/internal/guest/embed_test.go index bbc6d4d..b8ecbec 100644 --- a/internal/guest/embed_test.go +++ b/internal/guest/embed_test.go @@ -70,3 +70,19 @@ func TestApplyProfilesScriptIsDelivered(t *testing.T) { } t.Error("apply-profiles.sh is not delivered to the guest") } + +func TestInstallUserFileScriptIsDelivered(t *testing.T) { + files, err := DataFiles() + if err != nil { + t.Fatalf("DataFiles: %v", err) + } + for _, f := range files { + if f.Path == "/usr/local/lib/sandbox/install-user-file.sh" { + if f.Permissions != "0755" { + t.Errorf("install-user-file.sh permissions = %s, want 0755", f.Permissions) + } + return + } + } + t.Error("install-user-file.sh is not delivered to the guest") +} diff --git a/internal/guest/files/scripts/install-user-file.sh b/internal/guest/files/scripts/install-user-file.sh new file mode 100644 index 0000000..3054769 --- /dev/null +++ b/internal/guest/files/scripts/install-user-file.sh @@ -0,0 +1,45 @@ +#!/bin/bash +############################################################################### +# install-user-file.sh — place one host-staged file into the agent home +# +# Invoked as root by code-vm: install-user-file.sh +# +# The staged source sits in limaadmin's 0700 staging dir, unreadable by the +# agent; and a root write into the agent-owned home is the TOCTOU class the +# profile applier already closed. So root only RELAYS: the file moves to a +# root:AGENT_GID 0640 drop on tmpfs, and the final install runs with agent +# privileges — a planted symlink can only redirect a write the agent could +# already make. The drop is removed afterwards; rendered secrets exist there +# only for the moment between relay and install. +############################################################################### +set -euo pipefail + +# shellcheck source=/dev/null +. /etc/sandbox/provision.env + +src="$1" +rel="$2" +mode="$3" +AGENT_HOME="/home/${AGENT_USER}" + +DROP_DIR=/run/sandbox/user-files +install -d -m 0750 -o root -g "$AGENT_GID" "$DROP_DIR" +drop=$(mktemp "$DROP_DIR/file-XXXXXXXX") +install -m 0640 -o root -g "$AGENT_GID" "$src" "$drop" +rm -f "$src" + +cleanup() { rm -f "$drop"; } +trap cleanup EXIT + +# Same hardened pattern as the profile applier's agent runner: no login +# shell, system PATH only, BASH_ENV/ENV cleared. Positional args, not string +# interpolation. +# shellcheck disable=SC2016 # the inner bash -c program expands its own args +setpriv --reuid "$AGENT_UID" --regid "$AGENT_GID" --init-groups \ + env -u BASH_ENV -u ENV \ + HOME="$AGENT_HOME" \ + USER="$AGENT_USER" \ + XDG_RUNTIME_DIR="/run/user/${AGENT_UID}" \ + PATH=/usr/local/bin:/usr/bin:/bin \ + bash -c 'dst="$1/$2"; mkdir -p "$(dirname "$dst")" && rm -f "$dst" && install -m "$3" "$4" "$dst"' \ + _ "$AGENT_HOME" "$rel" "$mode" "$drop" diff --git a/internal/session/gitidentity.go b/internal/session/gitidentity.go index 05f0b1d..3b22c47 100644 --- a/internal/session/gitidentity.go +++ b/internal/session/gitidentity.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "os/exec" - "strconv" "strings" ) @@ -47,9 +46,8 @@ func ApplyGitIdentity(ctx context.Context, d Deps) error { return nil } - // Numeric ids, not names: the guest group carrying AgentGID may be a stock - // group with a different name (see Deps). - dst := "/home/" + d.AgentUser + "/.gitconfig" - return installContent(ctx, d, []byte(GitConfigContent(name, email)), dst, "0644", - strconv.Itoa(d.AgentUID), strconv.Itoa(d.AgentGID)) + // Delivered through the relay (install-user-file.sh), not a root install: + // the file lands in the agent's home, so the final write must run with + // agent privileges rather than root's (see userfiles.go). + return PushUserFile(ctx, d, []byte(GitConfigContent(name, email)), ".gitconfig", "0644") } diff --git a/internal/session/gitidentity_test.go b/internal/session/gitidentity_test.go index ee08c73..a49191a 100644 --- a/internal/session/gitidentity_test.go +++ b/internal/session/gitidentity_test.go @@ -25,10 +25,11 @@ func TestGitConfigContentOmitsMissingFields(t *testing.T) { } } -// Group ownership must be set by numeric GID. The guest group carrying the -// host's GID is often a stock group with a different name — a host user with -// GID 100 lands in "users" — so `install -g devuser` fails outright there. -func TestApplyGitIdentityInstallsByNumericIDs(t *testing.T) { +// The gitconfig is delivered through the agent-privilege relay, not a root +// install: ownership then falls out of which user runs the final install, +// not an explicit -o/-g pair, so there is no numeric-vs-named-group pitfall +// left here (see userfiles_test.go for the relay assertions). +func TestApplyGitIdentityDeliversToDotfile(t *testing.T) { r := &fakeRunner{} d := testDeps(t, r) d.AgentUID, d.AgentGID = 1000, 100 @@ -41,13 +42,8 @@ func TestApplyGitIdentityInstallsByNumericIDs(t *testing.T) { if err := ApplyGitIdentity(context.Background(), d); err != nil { t.Fatalf("ApplyGitIdentity: %v", err) } - if !r.ranAny("install -D -m 0644 -o 1000 -g 100") { - t.Errorf("gitconfig must be installed with numeric owner/group, got %v", r.calls) - } - for _, c := range r.calls { - if strings.Contains(strings.Join(c, " "), "-g devuser") { - t.Errorf("must not set the group by name: %v", c) - } + if !r.ranAny(".gitconfig") || !r.ranAny("0644") { + t.Errorf("gitconfig must be relayed with its home-relative path and mode, got %v", r.calls) } } diff --git a/internal/session/stage.go b/internal/session/stage.go index 0e22a9d..34b547e 100644 --- a/internal/session/stage.go +++ b/internal/session/stage.go @@ -30,38 +30,53 @@ func stagedPath() (string, error) { return stageDir + "/stage-" + hex.EncodeToString(b[:]), nil } -// installContent writes content into the guest at dst, owned by owner:group -// with the given mode. The content travels through the admin user's staging -// directory and the staged copy is removed afterwards, so it is never readable -// at a path the agent can reach. -func installContent(ctx context.Context, d Deps, content []byte, dst, mode, owner, group string) error { +// stageFile writes content to a local temp file and copies it into the +// guest's admin-only staging directory, returning the staged guest path. +// Shared by installContent (root destinations) and PushUserFile (agent-home +// destinations, which relay the staged copy onward instead of root-installing +// it directly — see userfiles.go). +func stageFile(ctx context.Context, d Deps, content []byte) (string, error) { tmp, err := os.CreateTemp("", "code-vm-stage-*") if err != nil { - return fmt.Errorf("create temp file: %w", err) + return "", fmt.Errorf("create temp file: %w", err) } defer os.Remove(tmp.Name()) if err := tmp.Chmod(0o600); err != nil { tmp.Close() - return fmt.Errorf("chmod temp file: %w", err) + return "", fmt.Errorf("chmod temp file: %w", err) } if _, err := tmp.Write(content); err != nil { tmp.Close() - return fmt.Errorf("write temp file: %w", err) + return "", fmt.Errorf("write temp file: %w", err) } if err := tmp.Close(); err != nil { - return fmt.Errorf("close temp file: %w", err) + return "", fmt.Errorf("close temp file: %w", err) } staged, err := stagedPath() if err != nil { - return err + return "", err } if err := d.Client.Admin(ctx, []string{ "install", "-d", "-m", "0700", "-o", adminUser, "-g", adminUser, stageDir, }); err != nil { - return err + return "", err } if err := d.Client.Copy(ctx, tmp.Name(), staged); err != nil { + return "", err + } + return staged, nil +} + +// installContent writes content into the guest at dst, owned by owner:group +// with the given mode, as root. Only correct for root-owned destinations — +// the allowlist fragment and profile tree — where there is no agent-owned +// home path for a planted symlink to redirect the write into; for anything +// landing in the agent's home, use PushUserFile instead so root never writes +// there directly. +func installContent(ctx context.Context, d Deps, content []byte, dst, mode, owner, group string) error { + staged, err := stageFile(ctx, d, content) + if err != nil { return err } // -D creates root-owned parents for nested per-profile paths; a no-op for diff --git a/internal/session/userfiles.go b/internal/session/userfiles.go new file mode 100644 index 0000000..06be4f0 --- /dev/null +++ b/internal/session/userfiles.go @@ -0,0 +1,26 @@ +package session + +import ( + "context" + "fmt" +) + +// PushUserFile delivers content into the agent's home at rel with the given +// mode, without ever writing there as root. The staged copy is relayed by +// install-user-file.sh: root moves it to an agent-group-readable tmpfs drop, +// and an agent-identity install places it — a symlink the agent plants can +// only redirect a write the agent could already make (the same posture as +// profile file installs). Used for rendered templates (0600) and the git +// identity (0644); rel comes from host-validated input only. +func PushUserFile(ctx context.Context, d Deps, content []byte, rel, mode string) error { + staged, err := stageFile(ctx, d, content) + if err != nil { + return err + } + if err := d.Client.Admin(ctx, []string{ + "/usr/local/lib/sandbox/install-user-file.sh", staged, rel, mode, + }); err != nil { + return fmt.Errorf("install %s: %w", rel, err) + } + return nil +} diff --git a/internal/session/userfiles_test.go b/internal/session/userfiles_test.go new file mode 100644 index 0000000..0c4d074 --- /dev/null +++ b/internal/session/userfiles_test.go @@ -0,0 +1,50 @@ +package session + +import ( + "context" + "testing" +) + +func TestPushUserFileStagesAndRelays(t *testing.T) { + r := &fakeRunner{} + d := testDeps(t, r) + if err := PushUserFile(context.Background(), d, []byte("content"), ".m2/settings.xml", "0600"); err != nil { + t.Fatalf("PushUserFile: %v", err) + } + copies := 0 + for _, c := range r.calls { + if len(c) > 0 && c[0] == "copy" { + copies++ + } + } + if copies != 1 { + t.Errorf("staged copies = %d, want 1", copies) + } + if !r.ranAny("/usr/local/lib/sandbox/install-user-file.sh") { + t.Errorf("relay script not invoked: %v", r.calls) + } + if !r.ranAny(".m2/settings.xml") || !r.ranAny("0600") { + t.Errorf("dst/mode not passed to the relay: %v", r.calls) + } + // The old direct-to-home root install must NOT happen for user files. + if r.ranAny("install -D -m 0600") { + t.Errorf("user files must not be root-installed into the home: %v", r.calls) + } +} + +func TestGitIdentityUsesUserFilePush(t *testing.T) { + r := &fakeRunner{} + d := testDeps(t, r) + d.Host = func(ctx context.Context, name string, args ...string) ([]byte, error) { + return []byte("simon\n"), nil + } + if err := ApplyGitIdentity(context.Background(), d); err != nil { + t.Fatalf("ApplyGitIdentity: %v", err) + } + if !r.ranAny("install-user-file.sh") || !r.ranAny(".gitconfig") { + t.Errorf("git identity must go through the relay: %v", r.calls) + } + if r.ranAny("install -D -m 0644") { + t.Errorf("git identity must no longer be root-installed: %v", r.calls) + } +} From fc9034994d62fb608f9bce7d56eb9fce885ee340 Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 13:18:06 +0200 Subject: [PATCH 07/23] feat: resolve and push rendered templates at start, apply, and boot ensureRunning now reports whether it actually booted the VM, so callers can gate secret/var resolution and template pushes to boots and explicit apply, never a plain command against an already-running VM (resolution may invoke the user's secret manager). Also adds a defense-in-depth guard in session.PushUserFile against unsafe rel paths for future callers. Co-Authored-By: Claude Fable 5 --- internal/cli/mount.go | 11 +++- internal/cli/profile.go | 7 ++- internal/cli/recreate.go | 13 ++++- internal/cli/secrets_push_test.go | 67 +++++++++++++++++++++++ internal/cli/shell.go | 10 +++- internal/cli/start.go | 86 +++++++++++++++++++++++++----- internal/cli/start_test.go | 14 +++-- internal/session/userfiles.go | 18 +++++++ internal/session/userfiles_test.go | 24 +++++++++ 9 files changed, 229 insertions(+), 21 deletions(-) create mode 100644 internal/cli/secrets_push_test.go diff --git a/internal/cli/mount.go b/internal/cli/mount.go index 2b06c85..554bb65 100644 --- a/internal/cli/mount.go +++ b/internal/cli/mount.go @@ -84,7 +84,16 @@ func newMountCmd() *cobra.Command { if err := cl.Stop(cmd.Context()); err != nil { return err } - return ensureRunning(cmd.Context(), cl, updated, profiles) + started, err := ensureRunning(cmd.Context(), cl, updated, profiles) + if err != nil { + return err + } + if started { + if err := pushRenderedTemplates(cmd.Context(), cl, updated, profiles, path, out); err != nil { + return err + } + } + return nil }, } } diff --git a/internal/cli/profile.go b/internal/cli/profile.go index 43bee04..5bcee5d 100644 --- a/internal/cli/profile.go +++ b/internal/cli/profile.go @@ -286,7 +286,7 @@ func newProfileApplyCmd() *cobra.Command { Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { ctx := cmd.Context() - c, profiles, _, err := loadConfigWithProfiles() + c, profiles, cfgPath, err := loadConfigWithProfiles() if err != nil { return err } @@ -307,6 +307,11 @@ func newProfileApplyCmd() *cobra.Command { if err := session.ApplyAllowlist(ctx, d); err != nil { return fmt.Errorf("apply allowlist: %w", err) } + // Rendered templates land before hooks run: a hook may consume + // the configs a profile's own templates just produced. + if err := pushRenderedTemplates(ctx, cl, c, profiles, cfgPath, cmd.OutOrStdout()); err != nil { + return err + } if err := session.ApplyProfiles(ctx, d); err != nil { return fmt.Errorf("apply profiles: %w", err) } diff --git a/internal/cli/recreate.go b/internal/cli/recreate.go index e172ac8..725649c 100644 --- a/internal/cli/recreate.go +++ b/internal/cli/recreate.go @@ -19,7 +19,7 @@ func newRecreateCmd() *cobra.Command { "installed plugins and the Docker image cache. Workspace files live\n" + "on the host and are unaffected.", RunE: func(cmd *cobra.Command, _ []string) error { - c, profiles, _, err := loadConfigWithProfiles() + c, profiles, cfgPath, err := loadConfigWithProfiles() if err != nil { return err } @@ -36,7 +36,16 @@ func newRecreateCmd() *cobra.Command { if err := cl.Delete(cmd.Context()); err != nil { return err } - return ensureRunning(cmd.Context(), cl, c, profiles) + started, err := ensureRunning(cmd.Context(), cl, c, profiles) + if err != nil { + return err + } + if started { + if err := pushRenderedTemplates(cmd.Context(), cl, c, profiles, cfgPath, cmd.OutOrStdout()); err != nil { + return err + } + } + return nil }, } cmd.Flags().BoolVar(&yes, "yes", false, "skip the confirmation prompt") diff --git a/internal/cli/secrets_push_test.go b/internal/cli/secrets_push_test.go new file mode 100644 index 0000000..8da1366 --- /dev/null +++ b/internal/cli/secrets_push_test.go @@ -0,0 +1,67 @@ +package cli + +import ( + "context" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/wetransform/code-vm/internal/lima" + "github.com/wetransform/code-vm/internal/profile" +) + +func TestPushRenderedTemplatesNoOpWithoutDeclarations(t *testing.T) { + r := &recordingRunner{statusOut: "Running"} + c := testCfg(t) + profiles := []profile.Profile{{Name: "plain", Manifest: profile.Manifest{Packages: []string{"git"}}}} + if err := pushRenderedTemplates(context.Background(), lima.Client{R: r}, c, profiles, filepath.Join(t.TempDir(), "config.yaml"), io.Discard); err != nil { + t.Fatalf("pushRenderedTemplates: %v", err) + } + if len(r.calls) != 0 { + t.Errorf("no declarations must mean no guest traffic, got %v", r.calls) + } +} + +func TestPushRenderedTemplatesResolvesAndPushes(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.yaml") + if err := os.WriteFile(filepath.Join(dir, "secrets.yaml"), []byte("secrets:\n tok:\n value: sekrit\n"), 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + r := &recordingRunner{statusOut: "Running"} + c := testCfg(t) + c.Vars = map[string]string{"url": "https://x"} + profiles := []profile.Profile{{ + Name: "p", + Manifest: profile.Manifest{ + Secrets: map[string]profile.SecretSpec{"tok": {}}, + Vars: map[string]profile.VarSpec{"url": {}}, + }, + Templates: []profile.File{{Rel: ".npmrc", Content: []byte("t=${secret:tok};u=${var:url}")}}, + }} + if err := pushRenderedTemplates(context.Background(), lima.Client{R: r}, c, profiles, cfgPath, io.Discard); err != nil { + t.Fatalf("pushRenderedTemplates: %v", err) + } + if !ranAny(r.calls, "install-user-file.sh") || !ranAny(r.calls, ".npmrc") || !ranAny(r.calls, "0600") { + t.Errorf("expected a relay push of .npmrc at 0600, got %v", r.calls) + } +} + +func TestPushRenderedTemplatesMissingMappingFails(t *testing.T) { + r := &recordingRunner{} + c := testCfg(t) + profiles := []profile.Profile{{ + Name: "p", + Manifest: profile.Manifest{Secrets: map[string]profile.SecretSpec{"tok": {Suggest: "gopass show -o t"}}}, + Templates: []profile.File{{Rel: ".npmrc", Content: []byte("${secret:tok}")}}, + }} + err := pushRenderedTemplates(context.Background(), lima.Client{R: r}, c, profiles, filepath.Join(t.TempDir(), "config.yaml"), io.Discard) + if err == nil || !strings.Contains(err.Error(), "gopass show -o t") { + t.Errorf("missing mapping must fail with the snippet, got %v", err) + } + if len(r.calls) != 0 { + t.Errorf("nothing may reach the guest on resolution failure, got %v", r.calls) + } +} diff --git a/internal/cli/shell.go b/internal/cli/shell.go index 8785a67..edef081 100644 --- a/internal/cli/shell.go +++ b/internal/cli/shell.go @@ -34,7 +34,7 @@ func resolveWorkdir(c config.Config, cwd string) (string, error) { // runDefault is the root command's action: bring the VM up, verify the current // directory is shared, then run the command as the agent user at that path. func runDefault(ctx context.Context, args []string) error { - c, profiles, _, err := loadConfigWithProfiles() + c, profiles, cfgPath, err := loadConfigWithProfiles() if err != nil { return err } @@ -47,11 +47,17 @@ func runDefault(ctx context.Context, args []string) error { return err } cl := clientFor(c) - if err := ensureRunning(ctx, cl, c, profiles); err != nil { + started, err := ensureRunning(ctx, cl, c, profiles) + if err != nil { return err } if err := session.Setup(ctx, agentDeps(cl, c, profiles)); err != nil { return fmt.Errorf("session setup: %w", err) } + if started { + if err := pushRenderedTemplates(ctx, cl, c, profiles, cfgPath, os.Stdout); err != nil { + return err + } + } return cl.Agent(ctx, workdir, agentCommand(args)) } diff --git a/internal/cli/start.go b/internal/cli/start.go index a12ecec..f2eca9a 100644 --- a/internal/cli/start.go +++ b/internal/cli/start.go @@ -3,7 +3,9 @@ package cli import ( "context" "fmt" + "io" "os" + "os/exec" "path/filepath" "runtime" @@ -101,36 +103,40 @@ func renderInstanceFile(c config.Config, profiles []profile.Profile) (string, er // from the rendered template; an existing one cannot be started with a // template argument (limactl refuses), so its stored config is replaced with // a freshly resolved render first. -func ensureRunning(ctx context.Context, cl lima.Client, c config.Config, profiles []profile.Profile) error { +// +// started reports whether this call actually booted the VM (status was not +// "Running" on entry). Callers use it to gate work that must run once per +// boot but never on a plain invocation against an already-running VM. +func ensureRunning(ctx context.Context, cl lima.Client, c config.Config, profiles []profile.Profile) (bool, error) { if err := c.Validate(); err != nil { - return err + return false, err } status, err := cl.Status(ctx) if err != nil { - return err + return false, err } if status == "Running" { - return nil + return false, nil } path, err := renderInstanceFile(c, profiles) if err != nil { - return err + return false, err } defer os.Remove(path) if status == "" { - return cl.Start(ctx, path) + return true, cl.Start(ctx, path) } dir, err := cl.InstanceDir(ctx) if err != nil { - return err + return false, err } if dir == "" { - return fmt.Errorf("cannot locate the %s instance directory", lima.InstanceName) + return false, fmt.Errorf("cannot locate the %s instance directory", lima.InstanceName) } if err := cl.ResolveConfigInto(ctx, path, filepath.Join(dir, "lima.yaml")); err != nil { - return err + return false, err } - return cl.StartExisting(ctx) + return true, cl.StartExisting(ctx) } func newStartCmd() *cobra.Command { @@ -138,11 +144,67 @@ func newStartCmd() *cobra.Command { Use: "start", Short: "Start the sandbox VM (idempotent)", RunE: func(cmd *cobra.Command, _ []string) error { - c, profiles, _, err := loadConfigWithProfiles() + c, profiles, cfgPath, err := loadConfigWithProfiles() if err != nil { return err } - return ensureRunning(cmd.Context(), clientFor(c), c, profiles) + cl := clientFor(c) + if _, err := ensureRunning(cmd.Context(), cl, c, profiles); err != nil { + return err + } + return pushRenderedTemplates(cmd.Context(), cl, c, profiles, cfgPath, cmd.OutOrStdout()) }, } } + +// pushRenderedTemplates resolves secrets/vars and pushes rendered templates +// into the agent home. Callers gate it to start, apply, and boot-causing +// invocations only: resolution may invoke the user's secret manager +// (pinentry), so it must never run on every command. +func pushRenderedTemplates(ctx context.Context, cl lima.Client, c config.Config, profiles []profile.Profile, cfgPath string, out io.Writer) error { + secretsDecl := profile.DeclaredSecrets(profiles) + varsDecl := profile.DeclaredVars(profiles) + templated := false + for _, p := range profiles { + if len(p.Templates) > 0 { + templated = true + } + } + if !templated && len(secretsDecl) == 0 && len(varsDecl) == 0 { + return nil + } + sources, warnings, err := config.LoadSecrets(config.SecretsPathFor(cfgPath)) + if err != nil { + return err + } + for _, w := range warnings { + fmt.Fprintf(out, "warning: %s\n", w) + } + secrets, err := profile.ResolveSecrets(ctx, secretsDecl, sources, hostCommand) + if err != nil { + return err + } + vars, err := profile.ResolveVars(varsDecl, c.Vars) + if err != nil { + return err + } + rendered := profile.RenderTemplates(profiles, secrets, vars) + d := agentDeps(cl, c, profiles) + for _, r := range rendered { + if err := session.PushUserFile(ctx, d, r.Content, r.Rel, "0600"); err != nil { + return err + } + } + if len(rendered) > 0 { + fmt.Fprintf(out, "Rendered %d template(s) into the sandbox.\n", len(rendered)) + } + return nil +} + +// hostCommand runs a secrets.yaml command through the user's shell on the +// host. Dual contract for profile.CommandRunner consumers: stdout carries the +// resolved value on success, while CombinedOutput's stderr is folded in so a +// failure's error message has something readable to show the user. +func hostCommand(ctx context.Context, command string) ([]byte, error) { + return exec.CommandContext(ctx, "sh", "-c", command).CombinedOutput() +} diff --git a/internal/cli/start_test.go b/internal/cli/start_test.go index 8c6bcde..72f8ed5 100644 --- a/internal/cli/start_test.go +++ b/internal/cli/start_test.go @@ -46,12 +46,16 @@ func TestEnsureRunningStartsWhenAbsentOrStopped(t *testing.T) { for _, status := range []string{"", "Stopped", "Broken"} { t.Run("status="+status, func(t *testing.T) { r := &recordingRunner{statusOut: status} - if err := ensureRunning(context.Background(), lima.Client{R: r}, testCfg(t), nil); err != nil { + started, err := ensureRunning(context.Background(), lima.Client{R: r}, testCfg(t), nil) + if err != nil { t.Fatalf("ensureRunning: %v", err) } if !r.started() { t.Errorf("expected a start call for status %q, calls=%v", status, r.calls) } + if !started { + t.Errorf("started = false for status %q, want true", status) + } }) } } @@ -61,7 +65,7 @@ func TestEnsureRunningStartsWhenAbsentOrStopped(t *testing.T) { // config via `template copy --embed-all` and then start by name. func TestEnsureRunningUpdatesStoredConfigWhenStopped(t *testing.T) { r := &recordingRunner{statusOut: "Stopped"} - if err := ensureRunning(context.Background(), lima.Client{R: r}, testCfg(t), nil); err != nil { + if _, err := ensureRunning(context.Background(), lima.Client{R: r}, testCfg(t), nil); err != nil { t.Fatalf("ensureRunning: %v", err) } var sawResolve, sawPlainStart bool @@ -87,12 +91,16 @@ func TestEnsureRunningUpdatesStoredConfigWhenStopped(t *testing.T) { func TestEnsureRunningIsNoOpWhenRunning(t *testing.T) { r := &recordingRunner{statusOut: "Running"} - if err := ensureRunning(context.Background(), lima.Client{R: r}, testCfg(t), nil); err != nil { + started, err := ensureRunning(context.Background(), lima.Client{R: r}, testCfg(t), nil) + if err != nil { t.Fatalf("ensureRunning: %v", err) } if r.started() { t.Errorf("must not start an already-running instance, calls=%v", r.calls) } + if started { + t.Errorf("started = true for a Running instance, want false") + } } func TestRenderInstanceFileIsPrivateAndComplete(t *testing.T) { diff --git a/internal/session/userfiles.go b/internal/session/userfiles.go index 06be4f0..11d36e8 100644 --- a/internal/session/userfiles.go +++ b/internal/session/userfiles.go @@ -3,6 +3,8 @@ package session import ( "context" "fmt" + "path" + "strings" ) // PushUserFile delivers content into the agent's home at rel with the given @@ -13,6 +15,12 @@ import ( // profile file installs). Used for rendered templates (0600) and the git // identity (0644); rel comes from host-validated input only. func PushUserFile(ctx context.Context, d Deps, content []byte, rel, mode string) error { + // Defense in depth: every current caller passes a host-validated rel, but + // this guards future ones from ever reaching the guest with a path that + // escapes the agent home. + if rel == "" || path.IsAbs(rel) || hasDotDotSegment(rel) { + return fmt.Errorf("user file path %q: must be a clean relative path inside the agent home", rel) + } staged, err := stageFile(ctx, d, content) if err != nil { return err @@ -24,3 +32,13 @@ func PushUserFile(ctx context.Context, d Deps, content []byte, rel, mode string) } return nil } + +// hasDotDotSegment reports whether rel contains a literal ".." path segment. +func hasDotDotSegment(rel string) bool { + for _, seg := range strings.Split(rel, "/") { + if seg == ".." { + return true + } + } + return false +} diff --git a/internal/session/userfiles_test.go b/internal/session/userfiles_test.go index 0c4d074..8a40248 100644 --- a/internal/session/userfiles_test.go +++ b/internal/session/userfiles_test.go @@ -32,6 +32,30 @@ func TestPushUserFileStagesAndRelays(t *testing.T) { } } +func TestPushUserFileRejectsUnsafeRel(t *testing.T) { + for _, rel := range []string{"../x", "/abs", ""} { + t.Run(rel, func(t *testing.T) { + r := &fakeRunner{} + d := testDeps(t, r) + err := PushUserFile(context.Background(), d, []byte("content"), rel, "0600") + if err == nil { + t.Fatalf("PushUserFile(%q) = nil error, want a rejection", rel) + } + if len(r.calls) != 0 { + t.Errorf("unsafe rel must not reach the guest, calls=%v", r.calls) + } + }) + } +} + +func TestPushUserFileAcceptsCleanRel(t *testing.T) { + r := &fakeRunner{} + d := testDeps(t, r) + if err := PushUserFile(context.Background(), d, []byte("content"), ".m2/settings.xml", "0600"); err != nil { + t.Fatalf("PushUserFile: %v", err) + } +} + func TestGitIdentityUsesUserFilePush(t *testing.T) { r := &fakeRunner{} d := testDeps(t, r) From 2c462560789ba0a01cc907e417cea05665f2bc92 Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 13:22:36 +0200 Subject: [PATCH 08/23] fix: keep stderr chatter out of resolved secret values hostCommand used CombinedOutput unconditionally, so a succeeding secrets.yaml command that prints to stderr (gpg warnings, deprecation notices) would silently corrupt the resolved secret with that chatter. Use Output() so the value is stdout only, falling back to the captured stderr as display text when the command actually fails. Co-Authored-By: Claude Fable 5 --- internal/cli/secrets_push_test.go | 20 ++++++++++++++++++++ internal/cli/start.go | 17 +++++++++++++---- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/internal/cli/secrets_push_test.go b/internal/cli/secrets_push_test.go index 8da1366..0ab7a87 100644 --- a/internal/cli/secrets_push_test.go +++ b/internal/cli/secrets_push_test.go @@ -49,6 +49,26 @@ func TestPushRenderedTemplatesResolvesAndPushes(t *testing.T) { } } +func TestHostCommandReturnsStdoutOnlyOnSuccess(t *testing.T) { + out, err := hostCommand(context.Background(), "echo warn >&2; echo value") + if err != nil { + t.Fatalf("hostCommand: %v", err) + } + if string(out) != "value\n" { + t.Errorf("hostCommand output = %q, want %q (stderr must not leak into the value)", out, "value\n") + } +} + +func TestHostCommandReturnsStderrOnFailure(t *testing.T) { + out, err := hostCommand(context.Background(), "echo oops >&2; exit 3") + if err == nil { + t.Fatalf("hostCommand: expected an error, got nil (out=%q)", out) + } + if !strings.Contains(string(out), "oops") { + t.Errorf("hostCommand output on failure = %q, want it to contain %q", out, "oops") + } +} + func TestPushRenderedTemplatesMissingMappingFails(t *testing.T) { r := &recordingRunner{} c := testCfg(t) diff --git a/internal/cli/start.go b/internal/cli/start.go index f2eca9a..608346b 100644 --- a/internal/cli/start.go +++ b/internal/cli/start.go @@ -2,6 +2,7 @@ package cli import ( "context" + "errors" "fmt" "io" "os" @@ -202,9 +203,17 @@ func pushRenderedTemplates(ctx context.Context, cl lima.Client, c config.Config, } // hostCommand runs a secrets.yaml command through the user's shell on the -// host. Dual contract for profile.CommandRunner consumers: stdout carries the -// resolved value on success, while CombinedOutput's stderr is folded in so a -// failure's error message has something readable to show the user. +// host. The value is stdout ONLY — stderr chatter from a succeeding command +// (gpg warnings, deprecation notices) must never leak into a credential. +// On failure, the captured stderr is returned as display text for the error. func hostCommand(ctx context.Context, command string) ([]byte, error) { - return exec.CommandContext(ctx, "sh", "-c", command).CombinedOutput() + out, err := exec.CommandContext(ctx, "sh", "-c", command).Output() + if err != nil { + var ee *exec.ExitError + if errors.As(err, &ee) { + return ee.Stderr, err + } + return nil, err + } + return out, nil } From 85bb073e7dc15cface639867aa7f0cb86a9c3a66 Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 13:27:04 +0200 Subject: [PATCH 09/23] feat: add code-vm secrets and surface secret declarations in profile add/list Reports the union of secrets/vars the active profiles declare, whether each is mapped, and prints ready-to-paste secrets.yaml snippets for unmapped ones (report, not a gate). `profile add` now warns when a cloned bundle declares secrets, and `profile list` marks secret-declaring profiles. Co-Authored-By: Claude Fable 5 --- internal/cli/profile.go | 26 ++++++++++++- internal/cli/profile_test.go | 75 ++++++++++++++++++++++++++++++++++++ internal/cli/root.go | 1 + internal/cli/secrets.go | 71 ++++++++++++++++++++++++++++++++++ internal/cli/secrets_test.go | 62 +++++++++++++++++++++++++++++ 5 files changed, 234 insertions(+), 1 deletion(-) create mode 100644 internal/cli/secrets.go create mode 100644 internal/cli/secrets_test.go diff --git a/internal/cli/profile.go b/internal/cli/profile.go index 5bcee5d..6542c69 100644 --- a/internal/cli/profile.go +++ b/internal/cli/profile.go @@ -109,7 +109,8 @@ func newProfileAddCmd() *cobra.Command { if err := runGit(cmd.Context(), cmd, "", "clone", url, dst); err != nil { return err } - if _, loadErr := profile.Load(dir, name); loadErr != nil { + loaded, loadErr := profile.Load(dir, name) + if loadErr != nil { // A broken bundle must not linger: it would fail every // loadConfigWithProfiles the moment someone activates it. if rmErr := os.RemoveAll(dst); rmErr != nil { @@ -120,6 +121,19 @@ func newProfileAddCmd() *cobra.Command { out := cmd.OutOrStdout() fmt.Fprintf(out, "Installed profile %s.\n\n%s\n\n", name, trustWarning) fmt.Fprintf(out, "Activate it by adding to your config:\n\nprofiles:\n - %s\n", name) + // Flagged separately from the generic trust warning above: a + // mapped secret's value becomes readable by the agent and by + // every other active profile's hook, not just this one, which + // is a sharper claim than "this profile is host-trusted". + if declared := profile.DeclaredSecrets([]profile.Profile{loaded}); len(declared) > 0 { + names := make([]string, len(declared)) + for i, d := range declared { + names[i] = d.Name + } + fmt.Fprintf(out, "\nThis profile declares secrets: %s. Mapping them in secrets.yaml makes "+ + "their values readable by the agent (and every active profile's hook).\n", + strings.Join(names, ", ")) + } return nil }, } @@ -229,6 +243,16 @@ func newProfileListCmd() *cobra.Command { desc = err.Error() } else { desc = p.Manifest.Description + // Surfaced right in the list a user already reaches for, + // so a profile's secret needs are visible before they + // even get as far as `code-vm secrets`. + if declared := profile.DeclaredSecrets([]profile.Profile{p}); len(declared) > 0 { + names := make([]string, len(declared)) + for i, d := range declared { + names[i] = d.Name + } + desc += fmt.Sprintf(" [secrets: %s]", strings.Join(names, ", ")) + } } origin := gitOrigin(filepath.Join(dir, name)) fmt.Fprintf(out, "%-24s %-8s %-40s %s\n", name, state, origin, desc) diff --git a/internal/cli/profile_test.go b/internal/cli/profile_test.go index f78f64e..47a5b4b 100644 --- a/internal/cli/profile_test.go +++ b/internal/cli/profile_test.go @@ -100,6 +100,21 @@ func withScratchConfig(t *testing.T) string { return dir } +// appendConfig appends raw YAML to the scratch config file written by +// withScratchConfig, the same read-append-rewrite dance several tests below +// already did inline for activating profiles or setting vars. +func appendConfig(t *testing.T, extra string) { + t.Helper() + cfg := configPath + base, err := os.ReadFile(cfg) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(cfg, append(base, []byte(extra)...), 0o600); err != nil { + t.Fatal(err) + } +} + func TestProfileAddClonesAndValidates(t *testing.T) { root := NewRootCmd() dir := withScratchConfig(t) @@ -127,6 +142,57 @@ func TestProfileAddClonesAndValidates(t *testing.T) { } } +// makeGitProfileWithSecret creates a local git repo laid out as a valid +// profile that declares one secret and references it from a template, so +// `profile add` has something to warn about. +func makeGitProfileWithSecret(t *testing.T) string { + t.Helper() + src := filepath.Join(t.TempDir(), "secret-profile") + if err := os.MkdirAll(filepath.Join(src, "templates"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(src, "profile.yaml"), []byte( + "description: fixture with a secret\nsecrets:\n api-token:\n description: an API token\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(src, "templates", ".netrc"), []byte("${secret:api-token}\n"), 0o644); err != nil { + t.Fatal(err) + } + for _, args := range [][]string{ + {"init", "-q"}, {"add", "."}, + {"-c", "user.email=t@t", "-c", "user.name=t", "commit", "-q", "-m", "v1"}, + } { + cmd := exec.Command("git", args...) + cmd.Dir = src + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + return src +} + +func TestProfileAddWarnsAboutDeclaredSecrets(t *testing.T) { + root := NewRootCmd() + _ = withScratchConfig(t) + src := makeGitProfileWithSecret(t) + + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"profile", "add", src}) + if err := root.Execute(); err != nil { + t.Fatalf("profile add: %v\n%s", err, out.String()) + } + + got := out.String() + if !strings.Contains(got, "declares secrets") || !strings.Contains(got, "api-token") { + t.Errorf("expected a warning naming the declared secret, got:\n%s", got) + } + if !strings.Contains(got, "secrets.yaml") { + t.Errorf("expected the warning to point at secrets.yaml, got:\n%s", got) + } +} + func TestProfileAddRejectsInvalidBundle(t *testing.T) { root := NewRootCmd() dir := withScratchConfig(t) @@ -181,6 +247,7 @@ func TestProfileListShowsStatus(t *testing.T) { writeProfile(t, profilesRoot, "alpha", "description: active one\npackages: [fish]\n") writeProfile(t, profilesRoot, "beta", "description: inactive one\npackages: [fish]\n") writeProfile(t, profilesRoot, "gamma", "description: broken\npackages: [Not_Valid]\n") + writeProfile(t, profilesRoot, "delta", "description: has a secret\nsecrets:\n a:\n description: d\n") cfg := filepath.Join(dir, "config.yaml") base, err := os.ReadFile(cfg) @@ -203,12 +270,14 @@ func TestProfileListShowsStatus(t *testing.T) { // Parse per-line rather than substring-matching the whole output: "active" // is itself a substring of "inactive". states := map[string]string{} + lines := map[string]string{} for _, line := range strings.Split(strings.TrimSpace(out.String()), "\n") { fields := strings.Fields(line) if len(fields) < 2 { continue } states[fields[0]] = fields[1] + lines[fields[0]] = line } if states["alpha"] != "active" { t.Errorf("alpha state = %q, want %q; full output:\n%s", states["alpha"], "active", out.String()) @@ -219,6 +288,12 @@ func TestProfileListShowsStatus(t *testing.T) { if states["gamma"] != "invalid" { t.Errorf("gamma state = %q, want %q; full output:\n%s", states["gamma"], "invalid", out.String()) } + if !strings.Contains(lines["delta"], "[secrets: a]") { + t.Errorf("delta row should carry a secrets marker, got: %q", lines["delta"]) + } + if strings.Contains(lines["alpha"], "[secrets:") { + t.Errorf("alpha row should not carry a secrets marker, got: %q", lines["alpha"]) + } } // The design spec's CLI section says `profile list` shows "git origin if diff --git a/internal/cli/root.go b/internal/cli/root.go index 044dfc8..1df754e 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -39,6 +39,7 @@ func NewRootCmd() *cobra.Command { root.AddCommand(newFirewallCmd()) root.AddCommand(newAllowCmd()) root.AddCommand(newProfileCmd()) + root.AddCommand(newSecretsCmd()) return root } diff --git a/internal/cli/secrets.go b/internal/cli/secrets.go new file mode 100644 index 0000000..9358bb7 --- /dev/null +++ b/internal/cli/secrets.go @@ -0,0 +1,71 @@ +package cli + +import ( + "fmt" + "strings" + + "github.com/spf13/cobra" + + "github.com/wetransform/code-vm/internal/config" + "github.com/wetransform/code-vm/internal/profile" +) + +// newSecretsCmd reports the union of secrets and vars declared by the active +// profiles, and which of them are mapped. It is a report, not a gate: unlike +// loadConfigWithProfiles's other callers, an unmapped secret must not fail +// the command, only be called out — the reader needs to see the whole +// picture (mapped and unmapped alike) in one place. Names and status only: +// a secret's resolved value is never read here, let alone printed. +func newSecretsCmd() *cobra.Command { + return &cobra.Command{ + Use: "secrets", + Short: "List secrets and vars declared by the active profiles", + Long: "Lists every secret and var the active profiles declare, whether it is\n" + + "mapped in secrets.yaml (or config.yaml's vars), which profiles declare\n" + + "it, and its description. Never prints a secret's value. For each\n" + + "unmapped secret with a suggested command, also prints a ready-to-paste\n" + + "secrets.yaml snippet.", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + c, profiles, path, err := loadConfigWithProfiles() + if err != nil { + return err + } + sources, warnings, err := config.LoadSecrets(config.SecretsPathFor(path)) + if err != nil { + return err + } + out := cmd.OutOrStdout() + for _, w := range warnings { + fmt.Fprintln(out, w) + } + + declaredSecrets := profile.DeclaredSecrets(profiles) + declaredVars := profile.DeclaredVars(profiles) + fmt.Fprintf(out, "%-24s %-9s %-20s %s\n", "NAME", "STATUS", "PROFILES", "DESCRIPTION") + for _, d := range declaredSecrets { + status := "UNMAPPED" + if _, ok := sources[d.Name]; ok { + status = "mapped" + } + fmt.Fprintf(out, "%-24s %-9s %-20s %s\n", d.Name, status, strings.Join(d.Profiles, ", "), d.Description) + } + for _, d := range declaredVars { + status := "UNMAPPED" + if _, ok := c.Vars[d.Name]; ok { + status = "mapped" + } + fmt.Fprintf(out, "%-24s %-9s %-20s %s\n", d.Name, status, strings.Join(d.Profiles, ", "), d.Description) + } + + secretsPath := config.SecretsPathFor(path) + for _, d := range declaredSecrets { + if _, ok := sources[d.Name]; ok { + continue + } + fmt.Fprintf(out, "\n# add to %s:\n%s", secretsPath, profile.MissingSecretSnippet(d)) + } + return nil + }, + } +} diff --git a/internal/cli/secrets_test.go b/internal/cli/secrets_test.go new file mode 100644 index 0000000..6927bf1 --- /dev/null +++ b/internal/cli/secrets_test.go @@ -0,0 +1,62 @@ +package cli + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestSecretsListsMappedAndUnmapped(t *testing.T) { + root := NewRootCmd() + dir := withScratchConfig(t) + pdir := filepath.Join(dir, "profiles", "p") + if err := os.MkdirAll(filepath.Join(pdir, "templates"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(pdir, "profile.yaml"), []byte( + "secrets:\n mapped-one:\n description: has a mapping\n missing-one:\n suggest: gopass show -o x\nvars:\n url: {}\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(pdir, "templates", ".npmrc"), []byte("${secret:mapped-one}${secret:missing-one}${var:url}\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "secrets.yaml"), []byte("secrets:\n mapped-one:\n value: v\n"), 0o600); err != nil { + t.Fatal(err) + } + appendConfig(t, "profiles:\n - p\n") + + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"secrets"}) + if err := root.Execute(); err != nil { + t.Fatalf("secrets: %v", err) + } + s := out.String() + for _, want := range []string{"mapped-one", "missing-one", "UNMAPPED", "url", + "command: gopass show -o x"} { + if !strings.Contains(s, want) { + t.Errorf("output missing %q:\n%s", want, s) + } + } + if strings.Contains(s, "v\n") && strings.Contains(s, "value") { + t.Errorf("secret values must never be printed:\n%s", s) + } +} + +// TestSecretsExitsZeroWithNoProfiles guards the "report, not a gate" contract: +// an empty union (no active profiles declare anything) is not an error. +func TestSecretsExitsZeroWithNoProfiles(t *testing.T) { + root := NewRootCmd() + withScratchConfig(t) + + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"secrets"}) + if err := root.Execute(); err != nil { + t.Fatalf("secrets with nothing declared should exit 0: %v", err) + } +} From f7c00be9c79af2d0890ff3cd9b96fd51a868dd03 Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 13:35:44 +0200 Subject: [PATCH 10/23] test: cover profile secrets/templates in the suite; document credentials Extends the Profiles integration section with a secret/var-backed template: substitution, rotation via re-apply, and a declared-but-unmapped secret failing apply with the ready-to-paste snippet, before anything reaches the guest. Adds a Credentials subsection under Profiles documenting the trust model, secrets.yaml/vars, the Maven settings.xml pattern, resolution timing, the boot-ordering caveat, and rotation, plus `code-vm secrets` in the command list. Co-Authored-By: Claude Fable 5 --- README.md | 90 ++++++++++++++++++++++++++++++++++++++++++++++ test-vm-sandbox.sh | 56 +++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+) diff --git a/README.md b/README.md index d316f43..a116f78 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,7 @@ code-vm # interactive shell | `code-vm recreate` | Delete and rebuild the guest from scratch | | `code-vm proxy-log [all\|denied\|allowed\|follow]` | Read the Squid access log | | `code-vm allow [domain...]` | Add domains to the allowlist and apply them live | +| `code-vm secrets` | List secrets/vars the active profiles declare, mapped or not | | `code-vm doctor` | Check host prerequisites | ## Configuration @@ -249,6 +250,95 @@ Notes: revert the shell, or delete files already in the home. `code-vm recreate` is the clean-slate path. +### Credentials + +Profiles can also ship **templates**: files rendered from placeholders +before delivery, so a team can share the whole shape of a credentialed +config (proxies, mirrors, server IDs) while each user supplies only their +own credential sources. + +``` +wetf-maven/ + profile.yaml + templates/ # home-mirroring tree, rendered before delivery + .m2/settings.xml +``` + +`profile.yaml` gains two more sections: + +```yaml +secrets: + wetf-repo-user: + description: Artifactory user for wetf-snapshots/releases + suggest: gopass show -o wetf/artifactory-user # inert hint, never executed + wetf-repo-password: + suggest: gopass show -o wetf/artifactory-password +vars: + artifactory-url: + description: Base URL of the Artifactory instance +``` + +A template uses `${secret:name}` and `${var:name}` placeholders; anything +else — Maven properties, `${env.FOO}` — passes through untouched. A shipped +`.m2/settings.xml` typically writes the static parts (proxies, mirrors) +verbatim and reserves placeholders only for the credentialed bits: + +```xml + + + + wetf-snapshots + ${secret:wetf-repo-user} + ${secret:wetf-repo-password} + + + ... + ... + +``` + +`description` and `suggest` are inert display strings: a profile can never +make anything execute. Values come only from the user's own mapping: + +- **`~/.config/code-vm/secrets.yaml`** — 0600, host-trusted like + `config.yaml`, never distributed with the profile: + + ```yaml + secrets: + wetf-repo-user: + command: gopass show -o wetf/artifactory-user + wetf-repo-password: + command: gopass show -o wetf/artifactory-password + ``` + + `command` runs on the host through the shell; its stdout, with one + trailing newline stripped, is the value. `value:` is also accepted for a + literal — a footgun for a real credential, fine for a low-value token. + +- **`vars:` in `config.yaml`** — the same non-secret literal map as any + other config key, for things like `artifactory-url: https://...`. + +Notes: + +- Mapping a secret makes its value readable by the agent, and therefore by + every active profile's hook, not just the one that declared it. Install + profiles from sources you trust with the secrets you map, and map only + sandbox-appropriate credentials. +- A declared-but-unmapped secret or var fails `code-vm start` / + `profile apply` with the exact snippet to paste into `secrets.yaml` or + `config.yaml`; nothing partial reaches the guest. `code-vm secrets` lists + every secret and var the active profiles declare, mapped or not, without + ever printing a value. +- Resolution happens at `code-vm start`, `code-vm profile apply`, and any + invocation that has to boot the VM — never per invocation against an + already-running VM, so there is no per-command secret-manager prompt. +- On a cold boot, hooks run as part of the guest's own boot sequence, + before `code-vm start` pushes the first rendered template: a hook that + reads a profile-shipped template must tolerate it not existing yet. +- Rotation: rotate the credential at its source (gopass, pass, op, …), then + either restart the VM or run `code-vm profile apply` — code-vm + re-resolves and re-pushes every time, never caching an old value. + ## Security model The perimeter is the VM boundary. Inside it, the agent is separated from guest diff --git a/test-vm-sandbox.sh b/test-vm-sandbox.sh index dfc9dfd..eb5611e 100644 --- a/test-vm-sandbox.sh +++ b/test-vm-sandbox.sh @@ -529,8 +529,22 @@ cat > "$PROFILE_FIXTURE/hook.sh" << 'SH' set -eu echo hook-ran > "$HOME/.profile-hook-ran" SH +# The original heredoc above declares neither key, so appending is safe here; +# the extra-unmapped secret added later must use `yq -i` instead, since a +# second top-level `secrets:` key would be a YAML duplicate-key parse error. +printf 'secrets:\n test-token:\n description: integration fixture token\nvars:\n test-url: {}\n' \ + >> "$PROFILE_FIXTURE/profile.yaml" +mkdir -p "$PROFILE_FIXTURE/templates/.config" +# shellcheck disable=SC2016 # placeholders, substituted host-side at apply +printf 'token=${secret:test-token}\nurl=${var:test-url}\nkeep=${env.HOME}\n' \ + > "$PROFILE_FIXTURE/templates/.config/fixture.conf" yq -i '.profiles = ["test-profile"]' "$CONFIG_FILE" +# Map the declared secret/var to inputs, in the scratch config tree. +printf 'secrets:\n test-token:\n value: sekrit-value\n' > "$TEST_CONFIG_DIR/secrets.yaml" +chmod 0600 "$TEST_CONFIG_DIR/secrets.yaml" +yq -i '.vars = {"test-url": "https://fixture.example"}' "$CONFIG_FILE" + if "${CODE_VM_ARGS[@]}" profile apply > /dev/null 2>&1; then pass "profile apply succeeds" else @@ -581,6 +595,47 @@ assert_ok "profile domain is allowed live" \ assert_fails "agent cannot write the guest profile tree" \ agent bash -c 'echo x > /usr/local/share/sandbox-profiles/manifest.env' +RENDERED="/home/$AGENT_USER/.config/fixture.conf" +if adm cat "$RENDERED" 2> /dev/null | grep -q 'token=sekrit-value'; then + pass "template secret is substituted in the guest" +else + fail "template secret is substituted in the guest" +fi +assert_ok "template var is substituted" \ + adm grep -q 'url=https://fixture.example' "$RENDERED" +# shellcheck disable=SC2016 # literal placeholder text, must NOT expand here +assert_ok "unrelated placeholders pass through" \ + adm grep -qF 'keep=${env.HOME}' "$RENDERED" +if [ "$(adm stat -c '%u %a' "$RENDERED")" = "$(id -u) 600" ]; then + pass "rendered template is agent-owned 0600" +else + fail "rendered template is agent-owned 0600 (got $(adm stat -c '%u %a' "$RENDERED"))" +fi + +# Rotation: change the mapped value, re-apply, and the rendered file updates. +printf 'secrets:\n test-token:\n value: rotated-value\n' > "$TEST_CONFIG_DIR/secrets.yaml" +"${CODE_VM_ARGS[@]}" profile apply > /dev/null 2>&1 +assert_ok "a mapping change plus apply updates the rendered template" \ + adm grep -q 'token=rotated-value' "$RENDERED" + +# A declared-but-unmapped secret must fail apply with the snippet, before +# anything reaches the guest. yq, not printf-append: a second top-level +# `secrets:` key would be a YAML duplicate-key parse error, a different +# failure than the one under test. +yq -i '.secrets.extra-unmapped = {"suggest": "gopass show -o nope"}' "$PROFILE_FIXTURE/profile.yaml" +# shellcheck disable=SC2016 # placeholder, substituted host-side at apply +printf 'x=${secret:extra-unmapped}\n' > "$PROFILE_FIXTURE/templates/.config/extra.conf" +UNMAPPED_OUT=$("${CODE_VM_ARGS[@]}" profile apply 2>&1) +if echo "$UNMAPPED_OUT" | grep -q 'gopass show -o nope'; then + pass "unmapped secret fails apply with the ready-to-paste snippet" +else + fail "unmapped secret fails apply with the ready-to-paste snippet (got: $UNMAPPED_OUT)" +fi +# Restore the fixture to the mapped-only state for the deactivation steps. +rm -f "$PROFILE_FIXTURE/templates/.config/extra.conf" +yq -i 'del(.secrets.extra-unmapped)' "$PROFILE_FIXTURE/profile.yaml" +"${CODE_VM_ARGS[@]}" profile apply > /dev/null 2>&1 + # Deactivate: the guest tree is cleared and the domain revoked. Installed # package and shell deliberately survive (documented non-goal). mv "$CONFIG_FILE.profiles-backup" "$CONFIG_FILE" @@ -590,6 +645,7 @@ assert_fails "deactivated profile's guest tree is gone" \ assert_fails "deactivated profile's domain is revoked" \ agent curl -fsS -o /dev/null --max-time 20 https://example.org adm rm -f "/home/$AGENT_USER/.profile-hook-ran" > /dev/null 2>&1 +adm rm -f "$RENDERED" > /dev/null 2>&1 # cleanup with the other fixture artifacts rm -rf "$TEST_CONFIG_DIR/profiles" echo "" From 2bc68b527b0449e69a826fa0c80dd3843dafafc0 Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 13:40:34 +0200 Subject: [PATCH 11/23] docs: resolve duplicate Credentials headings in README The pre-existing Configuration-section "### Credentials" (unqualified "no credential injection mechanism") and the new Profiles-section one now contradicted each other and collided on anchor. Rename the old one to "No workspace credentials", keep what's still load-bearing (nothing agent- or workspace-authored is ever a credential source; the removed .sandbox-secrets.yaml class stays removed, pinned by the integration suite), and forward-reference the Profiles Credentials section for the host-trusted templates mechanism. Co-Authored-By: Claude Fable 5 --- README.md | 51 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index a116f78..b508c0d 100644 --- a/README.md +++ b/README.md @@ -107,29 +107,38 @@ the accelerated one for the host: | Linux | `qemu` | KVM | the `virtiofsd` package | | macOS 13.5+ | `vz` | Hypervisor.framework (HVF) | Virtualization.framework | -### Credentials - -There is **no credential injection mechanism**. If a build needs a private -registry, write the credential file into the guest home once — it persists across -restarts, because the guest disk is the sandbox's durable state: +### No workspace credentials + +Nothing agent- or workspace-authored is ever a credential source. The +previous `.sandbox-secrets.yaml` mechanism was removed rather than fixed: it +resolved each secret by running its `source:` command **on the host** — from +a file inside the workspace, which the agent can write. That is host command +execution reachable from inside the sandbox, which defeats the boundary the +whole design exists to draw. Its stated protection did not hold either: +rendered files were group-readable by the agent, and the generated deny +rules only matched commands where the path appeared as a separate argument, +which `python -c` (an allowed command) sidesteps. That class of mechanism +stays removed, and the integration suite pins it. + +Credentials enter the sandbox in one of two ways: + +- Written directly into the guest home, once — it persists across restarts, + because the guest disk is the sandbox's durable state: + + ```bash + code-vm # shell into the guest + $ install -d -m 0700 ~/.gradle + $ cat > ~/.gradle/gradle.properties # paste, or pipe it in + ``` -```bash -code-vm # shell into the guest -$ install -d -m 0700 ~/.gradle -$ cat > ~/.gradle/gradle.properties # paste, or pipe it in -``` + Assume the agent can read anything you put there, and use credentials + created for the sandbox rather than your personal ones, so revoking them + is cheap. -Assume the agent can read anything you put there, and use credentials created -for the sandbox rather than your personal ones, so revoking them is cheap. - -The previous `.sandbox-secrets.yaml` mechanism was removed rather than fixed. It -resolved each secret by running its `source:` command **on the host** — from a -file inside the workspace, which the agent can write. That is host command -execution reachable from inside the sandbox, which defeats the boundary the whole -design exists to draw. Its stated protection did not hold either: rendered files -were group-readable by the agent, and the generated deny rules only matched -commands where the path appeared as a separate argument, which `python -c` (an -allowed command) sidesteps. +- Through a profile's `templates:` (see [Credentials](#credentials) under + Profiles) — the host-trusted mechanism for sharing a credentialed config's + shape while keeping the credential itself in the user's own + `secrets.yaml`. ### Extending the allowlist From 21794f3b252e67f06d28725cc93dde11d1b31731 Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 13:56:05 +0200 Subject: [PATCH 12/23] fix: reject control characters in profile description/suggest strings Suggest/Description and top-level Manifest.Description are printed verbatim by `code-vm secrets`, `profile list`, ResolveSecrets errors, and copied into the ready-to-paste MissingSecretSnippet a user pastes into secrets.yaml. A hostile bundle could hide a shell command behind a terminal escape sequence (displays clean, copies with a hidden suffix) or inject a newline to forge extra snippet entries. Reject any value containing a control character (C0, DEL) at load time. Co-Authored-By: Claude Fable 5 --- internal/profile/profile.go | 35 +++++++++++++++++++++++++++++--- internal/profile/profile_test.go | 23 +++++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/internal/profile/profile.go b/internal/profile/profile.go index e551a65..4404190 100644 --- a/internal/profile/profile.go +++ b/internal/profile/profile.go @@ -38,7 +38,7 @@ var shellRe = regexp.MustCompile(`^/[a-zA-Z0-9._/-]+$`) // relPathRe matches the file paths a profile may ship. Deliberately // conservative: paths are written line-by-line into files.list, which the // guest applier reads back, so whitespace and metacharacters are rejected -// wholesale. ".." is excluded by the per-segment check in loadFiles. +// wholesale. ".." is excluded by the per-segment check in loadTree. var relPathRe = regexp.MustCompile(`^[a-zA-Z0-9._/-]+$`) // hookRe matches the manifest's hook entry: a plain file name inside the @@ -57,6 +57,23 @@ func isBlank(content []byte) bool { return len(bytes.Trim(content, "\n")) == 0 } +// isSingleLinePrintable reports whether s contains no control characters: no +// newline, tab, ESC, or other C0 code, and no DEL (0x7f). Applied to manifest +// strings that are printed verbatim by `code-vm secrets` and `profile list`, +// and — most sensitively — copied byte-for-byte into the ready-to-paste +// MissingSecretSnippet a user pastes into secrets.yaml. Without this check, a +// hostile bundle could hide a shell command behind a terminal escape +// sequence (displays clean, copies with a hidden suffix) or inject a +// newline to forge extra YAML entries in the pasted snippet. +func isSingleLinePrintable(s string) bool { + for _, r := range s { + if r < 0x20 || r == 0x7f { + return false + } + } + return true +} + // forbiddenFiles are agent-home paths a profile may never ship: the // security-critical files lock-settings.sh owns and locks. var forbiddenFiles = map[string]bool{ @@ -236,6 +253,9 @@ func LoadAll(profilesDir string, names []string) ([]Profile, error) { } func validateManifest(m Manifest) error { + if !isSingleLinePrintable(m.Description) { + return fmt.Errorf("description must be a single-line printable string (no control characters)") + } for i, pkg := range m.Packages { if !packageRe.MatchString(pkg) { return fmt.Errorf("packages[%d]: not a Debian package name: %q", i, pkg) @@ -252,15 +272,24 @@ func validateManifest(m Manifest) error { if m.Hook != "" && !hookRe.MatchString(m.Hook) { return fmt.Errorf("hook must be a plain file name inside the profile, got %q", m.Hook) } - for name := range m.Secrets { + for name, spec := range m.Secrets { if err := ValidateName(name); err != nil { return fmt.Errorf("secret name %q: must look like %q", name, "repo-user") } + if !isSingleLinePrintable(spec.Description) { + return fmt.Errorf("secret %q: description must be a single-line printable string (no control characters)", name) + } + if !isSingleLinePrintable(spec.Suggest) { + return fmt.Errorf("secret %q: suggest must be a single-line printable string (no control characters)", name) + } } - for name := range m.Vars { + for name, spec := range m.Vars { if err := ValidateName(name); err != nil { return fmt.Errorf("var name %q: must look like %q", name, "artifactory-url") } + if !isSingleLinePrintable(spec.Description) { + return fmt.Errorf("var %q: description must be a single-line printable string (no control characters)", name) + } } return nil } diff --git a/internal/profile/profile_test.go b/internal/profile/profile_test.go index fc05552..842ad62 100644 --- a/internal/profile/profile_test.go +++ b/internal/profile/profile_test.go @@ -341,6 +341,10 @@ func TestLoadRejectsInvalidDeclarations(t *testing.T) { }{ {"bad secret name", "secrets:\n 'has space': {}\n", nil, "secret name"}, {"bad var name", "vars:\n 'has/slash': {}\n", nil, "var name"}, + {"ESC in secret suggest", "secrets:\n tok:\n suggest: \"gopass show \\efoo\"\n", nil, "suggest must be a single-line printable string"}, + {"newline in secret description", "secrets:\n tok:\n description: |\n line one\n line two\n", nil, "description must be a single-line printable string"}, + {"tab in var description", "vars:\n url:\n description: \"a\\tb\"\n", nil, "description must be a single-line printable string"}, + {"newline in top-level description", "description: |\n line one\n line two\n", nil, "description must be a single-line printable string"}, {"template/file collision", "description: x\n", map[string]string{"files/.npmrc": "a\n", "templates/.npmrc": "b\n"}, "both files/ and templates/"}, @@ -360,6 +364,25 @@ func TestLoadRejectsInvalidDeclarations(t *testing.T) { } } +// Ordinary single-line text with spaces is exactly the common case and must +// not be rejected by the control-character check. +func TestLoadAcceptsOrdinaryDescriptionsAndSuggestions(t *testing.T) { + dir := t.TempDir() + writeProfile(t, dir, "p", ` +description: A profile with ordinary spaced-out words +secrets: + tok: + description: A token with spaces in its description + suggest: gopass show -o some/path with spaces +vars: + url: + description: A var description with spaces +`, nil) + if _, err := Load(dir, "p"); err != nil { + t.Fatalf("Load: %v", err) + } +} + func TestLoadRejectsSymlinkedTemplatesRoot(t *testing.T) { dir := t.TempDir() writeProfile(t, dir, "p", "description: x\n", map[string]string{"files/a": "x\n"}) From 2edf58ec8a6a71be37e772326329dfe902a9cc2e Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 13:56:50 +0200 Subject: [PATCH 13/23] fix: reject cross-profile files/-vs-templates/ collisions in LoadAll Boot delivers rendered templates after the file tree (template wins) while `profile apply` pushes rendered templates before re-laying the file tree (files/ wins), so which entry actually wins a cross-profile files/-vs-templates/ collision at the same Rel differs by path for the same config. Order-respecting suppression can't be made consistent across both, so LoadAll now rejects the collision outright, naming both profiles and the path. Same-kind collisions (file/file, template/template) keep resolving by list order, later wins. Co-Authored-By: Claude Fable 5 --- .../2026-08-20-profile-secrets-design.md | 10 ++++-- internal/profile/profile.go | 35 ++++++++++++++++++- internal/profile/profile_test.go | 35 +++++++++++++++++++ 3 files changed, 77 insertions(+), 3 deletions(-) diff --git a/docs/superpowers/specs/2026-08-20-profile-secrets-design.md b/docs/superpowers/specs/2026-08-20-profile-secrets-design.md index 8fac540..08166d9 100644 --- a/docs/superpowers/specs/2026-08-20-profile-secrets-design.md +++ b/docs/superpowers/specs/2026-08-20-profile-secrets-design.md @@ -98,8 +98,14 @@ Validation, host-side at load, extending the existing rules: charset, no `..`, no absolute paths, no symlinks anywhere (tree root, directories, entries), locked Claude settings paths rejected. - A template and a `files/` entry targeting the same destination within one - profile is a validation error. Across profiles, list order wins, as with - files. + profile is a validation error. Across profiles, a same-kind collision + (`files/` vs `files/`, `templates/` vs `templates/`) still resolves by list + order, later wins. A `files/`-vs-`templates/` collision across profiles is + rejected as a `LoadAll` validation error instead: boot delivers rendered + templates after the file tree (template wins) while `profile apply` pushes + rendered templates before re-laying the file tree (`files/` wins), so which + entry would actually win differs between the two paths for the same + config — order-respecting suppression can't be made consistent across both. - A template referencing an undeclared `${secret:...}` or `${var:...}` name is a load-time validation error: bundles cannot quietly depend on inputs they never declared. Declared-but-unreferenced names are allowed (hooks diff --git a/internal/profile/profile.go b/internal/profile/profile.go index 4404190..1c68162 100644 --- a/internal/profile/profile.go +++ b/internal/profile/profile.go @@ -233,10 +233,23 @@ func Load(profilesDir, name string) (Profile, error) { return p, nil } +// shippedAs records which profile shipped a given Rel and how (as a file or +// as a template), for LoadAll's cross-profile collision check. +type shippedAs struct { + profile string + kind string // "files/" or "templates/" +} + // LoadAll loads the named profiles in order. Order is meaningful and -// preserved: later profiles win file collisions, and hooks run in this order. +// preserved: later profiles win same-kind file collisions (file/file, +// template/template), and hooks run in this order. A files/-vs-templates/ +// collision across profiles is rejected outright rather than resolved by +// order: boot (template wins) and apply (files/ wins) run the two delivery +// mechanisms in different orders, so which one would actually win differs +// between the two paths for the same config — see the design spec. func LoadAll(profilesDir string, names []string) ([]Profile, error) { seen := map[string]bool{} + shipped := map[string]shippedAs{} // Rel -> who shipped it, and how out := make([]Profile, 0, len(names)) for _, n := range names { if seen[n] { @@ -247,6 +260,26 @@ func LoadAll(profilesDir string, names []string) ([]Profile, error) { if err != nil { return nil, err } + for _, f := range p.Files { + if prior, ok := shipped[f.Rel]; ok && prior.kind != "files/" { + return nil, fmt.Errorf( + "%s: profile %s ships files/%s but profile %s already ships templates/%s; "+ + "a files/-vs-templates/ collision across profiles cannot be resolved consistently "+ + "between boot and apply, so it is rejected outright", + f.Rel, n, f.Rel, prior.profile, f.Rel) + } + shipped[f.Rel] = shippedAs{profile: n, kind: "files/"} + } + for _, t := range p.Templates { + if prior, ok := shipped[t.Rel]; ok && prior.kind != "templates/" { + return nil, fmt.Errorf( + "%s: profile %s ships templates/%s but profile %s already ships files/%s; "+ + "a files/-vs-templates/ collision across profiles cannot be resolved consistently "+ + "between boot and apply, so it is rejected outright", + t.Rel, n, t.Rel, prior.profile, t.Rel) + } + shipped[t.Rel] = shippedAs{profile: n, kind: "templates/"} + } out = append(out, p) } return out, nil diff --git a/internal/profile/profile_test.go b/internal/profile/profile_test.go index 842ad62..ce6f61c 100644 --- a/internal/profile/profile_test.go +++ b/internal/profile/profile_test.go @@ -291,6 +291,41 @@ func TestLoadAllPreservesOrderAndRejectsDuplicates(t *testing.T) { } } +// A files/ entry in one profile and a templates/ entry in another, at the +// same Rel, cannot be resolved consistently by list order: boot delivers +// templates last (template wins) while apply pushes rendered templates +// before ApplyProfiles re-lays the file tree (files/ wins). Same config, +// different winner depending on path, so LoadAll rejects it outright. +func TestLoadAllRejectsCrossProfileFileTemplateCollision(t *testing.T) { + dir := t.TempDir() + writeProfile(t, dir, "a", "description: a\n", map[string]string{"files/.npmrc": "from-a\n"}) + writeProfile(t, dir, "b", "secrets:\n tok: {}\n", map[string]string{"templates/.npmrc": "${secret:tok}\n"}) + + _, err := LoadAll(dir, []string{"a", "b"}) + if err == nil { + t.Fatal("expected a cross-profile files/-vs-templates/ collision error") + } + if !strings.Contains(err.Error(), "a") || !strings.Contains(err.Error(), "b") || !strings.Contains(err.Error(), ".npmrc") { + t.Errorf("error must name both profiles and the path, got %v", err) + } +} + +// Two profiles that both ship the same Rel as files/ is the ordinary, +// well-defined case: later wins, no error. +func TestLoadAllAllowsSameKindCrossProfileCollision(t *testing.T) { + dir := t.TempDir() + writeProfile(t, dir, "a", "description: a\n", map[string]string{"files/.npmrc": "from-a\n"}) + writeProfile(t, dir, "b", "description: b\n", map[string]string{"files/.npmrc": "from-b\n"}) + + ps, err := LoadAll(dir, []string{"a", "b"}) + if err != nil { + t.Fatalf("LoadAll: %v", err) + } + if len(ps) != 2 { + t.Fatalf("LoadAll returned %d profiles, want 2", len(ps)) + } +} + func TestLoadTemplatesAndDeclarations(t *testing.T) { dir := t.TempDir() writeProfile(t, dir, "maven", ` From 2d96fabfb268275164eb0b553819f72e81a3bfbb Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 13:58:03 +0200 Subject: [PATCH 14/23] fix: resolve secrets/templates before mutating guest state in profile apply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit newProfileApplyCmd ran PushProfiles and ApplyAllowlist before resolving and rendering templates, so an unmapped-secret failure aborted only after the new profile tree was staged and its domains were made live (with the new tree's hooks then running unattended on the next boot, without templates). Resolution needs nothing from the guest. Split pushRenderedTemplates into resolveRendered (secrets/vars resolution and template rendering, no guest I/O) and pushRendered (the guest push of an already-rendered set). Profile apply now calls resolveRendered first and pushes the result later, after PushProfiles/ApplyAllowlist but before ApplyProfiles/hooks; start, shell, mount and recreate keep the combined pushRenderedTemplates, which is now just resolveRendered followed by pushRendered — no caller resolves twice. Co-Authored-By: Claude Fable 5 --- internal/cli/profile.go | 14 +++++++-- internal/cli/profile_test.go | 61 ++++++++++++++++++++++++++++++++++++ internal/cli/start.go | 40 +++++++++++++++++------ 3 files changed, 103 insertions(+), 12 deletions(-) diff --git a/internal/cli/profile.go b/internal/cli/profile.go index 6542c69..7f7c12f 100644 --- a/internal/cli/profile.go +++ b/internal/cli/profile.go @@ -322,6 +322,15 @@ func newProfileApplyCmd() *cobra.Command { if status != "Running" { return fmt.Errorf("the VM is not running; profiles apply automatically at boot — start it with `code-vm start`") } + // Resolve (and render) before touching the guest at all: + // resolution needs nothing from the guest, and PushProfiles/ + // ApplyAllowlist below stage a new profile tree and make its + // domains live. An unmapped-secret failure must abort before + // any of that happens, not after. + rendered, err := resolveRendered(ctx, c, profiles, cfgPath, cmd.OutOrStdout()) + if err != nil { + return err + } d := agentDeps(cl, c, profiles) if err := session.PushProfiles(ctx, d, profile.GuestFiles(profiles)); err != nil { return fmt.Errorf("push profiles: %w", err) @@ -332,8 +341,9 @@ func newProfileApplyCmd() *cobra.Command { return fmt.Errorf("apply allowlist: %w", err) } // Rendered templates land before hooks run: a hook may consume - // the configs a profile's own templates just produced. - if err := pushRenderedTemplates(ctx, cl, c, profiles, cfgPath, cmd.OutOrStdout()); err != nil { + // the configs a profile's own templates just produced. Already + // resolved above, so this only pushes — no second resolution. + if err := pushRendered(ctx, cl, c, profiles, rendered, cmd.OutOrStdout()); err != nil { return err } if err := session.ApplyProfiles(ctx, d); err != nil { diff --git a/internal/cli/profile_test.go b/internal/cli/profile_test.go index 47a5b4b..8ae7aca 100644 --- a/internal/cli/profile_test.go +++ b/internal/cli/profile_test.go @@ -644,6 +644,67 @@ func TestProfileApplyPushesAndRuns(t *testing.T) { } } +// Resolution (LoadSecrets/ResolveSecrets/ResolveVars) needs nothing from the +// guest, so an unmapped secret must fail apply before PushProfiles stages a +// new profile tree or ApplyAllowlist makes its domains live — otherwise a +// failed apply leaves a half-applied profile (new tree, live domains, no +// templates) whose hooks still run on the next boot. +func TestProfileApplyFailsBeforeTouchingGuestOnUnmappedSecret(t *testing.T) { + root := NewRootCmd() + dir := withScratchConfig(t) + pdir := filepath.Join(dir, "profiles", "p") + if err := os.MkdirAll(filepath.Join(pdir, "templates"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(pdir, "profile.yaml"), + []byte("secrets:\n tok:\n suggest: gopass show -o t\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(pdir, "templates", ".npmrc"), []byte("${secret:tok}\n"), 0o644); err != nil { + t.Fatal(err) + } + cfg := configPath + b, err := os.ReadFile(cfg) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(cfg, append(b, []byte("profiles:\n - p\n")...), 0o600); err != nil { + t.Fatal(err) + } + // No secrets.yaml is written at all: "tok" is unmapped. + + r := installFakeClient(t, "Running") + root.SetArgs([]string{"profile", "apply"}) + err = root.Execute() + if err == nil || !strings.Contains(err.Error(), "gopass show -o t") { + t.Fatalf("profile apply = %v, want an unmapped-secret error with the suggest snippet", err) + } + // "status" is the only guest call resolution's own precondition check + // makes; nothing that mutates guest state (push, rm -rf, allowlist) may + // have run. + for _, unwanted := range []string{ + "rm -rf /usr/local/share/sandbox-profiles", + "install -d -m 0755 /usr/local/share/sandbox-profiles", + "squid -k reconfigure", + "apply-profiles.sh", + } { + if ranAny(r.calls, unwanted) { + t.Errorf("guest call %q must not happen before resolution succeeds, calls=%v", unwanted, r.calls) + } + } + if copies := func() int { + n := 0 + for _, c := range r.calls { + if len(c) > 0 && c[0] == "copy" { + n++ + } + } + return n + }(); copies != 0 { + t.Errorf("no file may be staged into the guest before resolution succeeds, copies=%d calls=%v", copies, r.calls) + } +} + func TestProfileApplyRequiresRunningVM(t *testing.T) { root := NewRootCmd() withScratchConfig(t) diff --git a/internal/cli/start.go b/internal/cli/start.go index 608346b..07e31cf 100644 --- a/internal/cli/start.go +++ b/internal/cli/start.go @@ -158,11 +158,13 @@ func newStartCmd() *cobra.Command { } } -// pushRenderedTemplates resolves secrets/vars and pushes rendered templates -// into the agent home. Callers gate it to start, apply, and boot-causing -// invocations only: resolution may invoke the user's secret manager -// (pinentry), so it must never run on every command. -func pushRenderedTemplates(ctx context.Context, cl lima.Client, c config.Config, profiles []profile.Profile, cfgPath string, out io.Writer) error { +// resolveRendered resolves secrets/vars and renders every active profile's +// templates, without touching the guest at all. Split out from +// pushRenderedTemplates so callers that must not mutate any guest state +// before resolution can fail (profile apply: PushProfiles/ApplyAllowlist +// stage a new profile tree and make its domains live, and must not run +// ahead of an unmapped-secret failure) can resolve first and push later. +func resolveRendered(ctx context.Context, c config.Config, profiles []profile.Profile, cfgPath string, out io.Writer) ([]profile.Rendered, error) { secretsDecl := profile.DeclaredSecrets(profiles) varsDecl := profile.DeclaredVars(profiles) templated := false @@ -172,24 +174,30 @@ func pushRenderedTemplates(ctx context.Context, cl lima.Client, c config.Config, } } if !templated && len(secretsDecl) == 0 && len(varsDecl) == 0 { - return nil + return nil, nil } sources, warnings, err := config.LoadSecrets(config.SecretsPathFor(cfgPath)) if err != nil { - return err + return nil, err } for _, w := range warnings { fmt.Fprintf(out, "warning: %s\n", w) } secrets, err := profile.ResolveSecrets(ctx, secretsDecl, sources, hostCommand) if err != nil { - return err + return nil, err } vars, err := profile.ResolveVars(varsDecl, c.Vars) if err != nil { - return err + return nil, err } - rendered := profile.RenderTemplates(profiles, secrets, vars) + return profile.RenderTemplates(profiles, secrets, vars), nil +} + +// pushRendered pushes already-resolved rendered templates into the agent +// home. Callers that already called resolveRendered use this directly, so +// resolution never runs twice. +func pushRendered(ctx context.Context, cl lima.Client, c config.Config, profiles []profile.Profile, rendered []profile.Rendered, out io.Writer) error { d := agentDeps(cl, c, profiles) for _, r := range rendered { if err := session.PushUserFile(ctx, d, r.Content, r.Rel, "0600"); err != nil { @@ -202,6 +210,18 @@ func pushRenderedTemplates(ctx context.Context, cl lima.Client, c config.Config, return nil } +// pushRenderedTemplates resolves secrets/vars and pushes rendered templates +// into the agent home. Callers gate it to start, apply, and boot-causing +// invocations only: resolution may invoke the user's secret manager +// (pinentry), so it must never run on every command. +func pushRenderedTemplates(ctx context.Context, cl lima.Client, c config.Config, profiles []profile.Profile, cfgPath string, out io.Writer) error { + rendered, err := resolveRendered(ctx, c, profiles, cfgPath, out) + if err != nil { + return err + } + return pushRendered(ctx, cl, c, profiles, rendered, out) +} + // hostCommand runs a secrets.yaml command through the user's shell on the // host. The value is stdout ONLY — stderr chatter from a succeeding command // (gpg warnings, deprecation notices) must never leak into a credential. From c876f1e8db6098bc4adc0ddfdfbb2476e2b38bf7 Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 14:00:57 +0200 Subject: [PATCH 15/23] fix: actionable upgrade-path error and relay hardening for PushUserFile Wrap the Admin failure in PushUserFile so an opaque exec error (from a VM booted with a pre-this-feature binary, missing install-user-file.sh entirely) tells the user the fix: `code-vm stop && code-vm start`. The underlying error stays wrapped with %w. Defense in depth at both ends of the relay: PushUserFile now also rejects a rel that doesn't match the same conservative charset the host-side profile loader enforces, and install-user-file.sh rejects an absolute or ".."-carrying rel before using it, on its own end. Also move `trap cleanup EXIT` to immediately after mktemp, so a failed populate doesn't leave a placeholder drop on tmpfs. Tied minors from the same review pass: - CommandRunner's doc comment now describes the actual stdout/stderr split, not "combined output". - hostCommand's doc comment and the README's Credentials section both note that a secrets.yaml command needing interactive pinentry will hang or fail: neither stdin nor a tty is wired up. Co-Authored-By: Claude Fable 5 --- README.md | 2 + internal/cli/start.go | 3 ++ .../guest/files/scripts/install-user-file.sh | 19 ++++++++- internal/profile/template.go | 6 ++- internal/session/userfiles.go | 18 +++++++-- internal/session/userfiles_test.go | 39 ++++++++++++++++++- 6 files changed, 79 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index b508c0d..352fe19 100644 --- a/README.md +++ b/README.md @@ -323,6 +323,8 @@ make anything execute. Values come only from the user's own mapping: `command` runs on the host through the shell; its stdout, with one trailing newline stripped, is the value. `value:` is also accepted for a literal — a footgun for a real credential, fine for a low-value token. + (Neither stdin nor a tty is wired up, so a command that needs interactive + pinentry rather than an already-cached agent/keyring will hang or fail.) - **`vars:` in `config.yaml`** — the same non-secret literal map as any other config key, for things like `artifactory-url: https://...`. diff --git a/internal/cli/start.go b/internal/cli/start.go index 07e31cf..6e903ce 100644 --- a/internal/cli/start.go +++ b/internal/cli/start.go @@ -226,6 +226,9 @@ func pushRenderedTemplates(ctx context.Context, cl lima.Client, c config.Config, // host. The value is stdout ONLY — stderr chatter from a succeeding command // (gpg warnings, deprecation notices) must never leak into a credential. // On failure, the captured stderr is returned as display text for the error. +// Neither stdin nor a tty is wired up: a command that needs interactive +// pinentry (rather than an already-cached agent/keyring) will hang or fail +// rather than prompt. func hostCommand(ctx context.Context, command string) ([]byte, error) { out, err := exec.CommandContext(ctx, "sh", "-c", command).Output() if err != nil { diff --git a/internal/guest/files/scripts/install-user-file.sh b/internal/guest/files/scripts/install-user-file.sh index 3054769..e46c3b1 100644 --- a/internal/guest/files/scripts/install-user-file.sh +++ b/internal/guest/files/scripts/install-user-file.sh @@ -20,17 +20,32 @@ set -euo pipefail src="$1" rel="$2" mode="$3" + +# Defense in depth: the host already validates rel before it ever reaches +# here, but this relay is root, so a path that escapes the agent home is +# rejected again on this end too. +case "$rel" in + /* | */../* | */.. | ../* | ..) + echo "install-user-file.sh: rejected unsafe rel path: $rel" >&2 + exit 1 + ;; +esac + AGENT_HOME="/home/${AGENT_USER}" DROP_DIR=/run/sandbox/user-files install -d -m 0750 -o root -g "$AGENT_GID" "$DROP_DIR" drop=$(mktemp "$DROP_DIR/file-XXXXXXXX") -install -m 0640 -o root -g "$AGENT_GID" "$src" "$drop" -rm -f "$src" +# Trapped immediately after mktemp creates the placeholder: if the install +# below fails, cleanup still removes it instead of leaving an empty drop on +# tmpfs. cleanup() { rm -f "$drop"; } trap cleanup EXIT +install -m 0640 -o root -g "$AGENT_GID" "$src" "$drop" +rm -f "$src" + # Same hardened pattern as the profile applier's agent runner: no login # shell, system PATH only, BASH_ENV/ENV cleared. Positional args, not string # interpolation. diff --git a/internal/profile/template.go b/internal/profile/template.go index b117e9b..2df7dfc 100644 --- a/internal/profile/template.go +++ b/internal/profile/template.go @@ -145,8 +145,10 @@ func DeclaredVars(profiles []Profile) []DeclaredVar { return out } -// CommandRunner executes a user-authored secrets.yaml command on the host and -// returns its combined output. Injectable for tests. +// CommandRunner executes a user-authored secrets.yaml command on the host. +// On success the returned bytes are stdout — the value — with stderr chatter +// excluded; on failure the returned bytes are stderr, as display text for the +// error. Injectable for tests. type CommandRunner func(ctx context.Context, command string) ([]byte, error) // MissingSecretSnippet renders the ready-to-paste secrets.yaml block for an diff --git a/internal/session/userfiles.go b/internal/session/userfiles.go index 11d36e8..862e23d 100644 --- a/internal/session/userfiles.go +++ b/internal/session/userfiles.go @@ -4,9 +4,16 @@ import ( "context" "fmt" "path" + "regexp" "strings" ) +// relCharsetRe mirrors the charset profile.relPathRe enforces on the host +// side: conservative on purpose, since rel is written into a root-run +// install command on the other end of the relay. Defense in depth — every +// current caller already passes a host-validated rel — for future ones. +var relCharsetRe = regexp.MustCompile(`^[a-zA-Z0-9._/-]+$`) + // PushUserFile delivers content into the agent's home at rel with the given // mode, without ever writing there as root. The staged copy is relayed by // install-user-file.sh: root moves it to an agent-group-readable tmpfs drop, @@ -17,8 +24,9 @@ import ( func PushUserFile(ctx context.Context, d Deps, content []byte, rel, mode string) error { // Defense in depth: every current caller passes a host-validated rel, but // this guards future ones from ever reaching the guest with a path that - // escapes the agent home. - if rel == "" || path.IsAbs(rel) || hasDotDotSegment(rel) { + // escapes the agent home or carries a shell metacharacter into the + // relay's positional args. + if rel == "" || path.IsAbs(rel) || hasDotDotSegment(rel) || !relCharsetRe.MatchString(rel) { return fmt.Errorf("user file path %q: must be a clean relative path inside the agent home", rel) } staged, err := stageFile(ctx, d, content) @@ -28,7 +36,11 @@ func PushUserFile(ctx context.Context, d Deps, content []byte, rel, mode string) if err := d.Client.Admin(ctx, []string{ "/usr/local/lib/sandbox/install-user-file.sh", staged, rel, mode, }); err != nil { - return fmt.Errorf("install %s: %w", rel, err) + // A VM booted from a pre-this-feature code-vm binary lacks + // install-user-file.sh entirely, so this failure is otherwise an + // opaque exec error with no clue that the fix is a restart. + return fmt.Errorf("install %s: %w (the relay script may be missing because the VM predates this "+ + "code-vm version; restart it with `code-vm stop && code-vm start`)", rel, err) } return nil } diff --git a/internal/session/userfiles_test.go b/internal/session/userfiles_test.go index 8a40248..ff90222 100644 --- a/internal/session/userfiles_test.go +++ b/internal/session/userfiles_test.go @@ -2,9 +2,46 @@ package session import ( "context" + "errors" + "strings" "testing" ) +// erroringRunner fails the Run call whose args join contains match; every +// other call is delegated to the embedded fakeRunner so it still records +// calls and behaves normally. +type erroringRunner struct { + fakeRunner + match string + err error +} + +func (r *erroringRunner) Run(ctx context.Context, args ...string) error { + if strings.Contains(strings.Join(args, " "), r.match) { + r.calls = append(r.calls, args) + return r.err + } + return r.fakeRunner.Run(ctx, args...) +} + +// A VM booted from a pre-this-feature code-vm binary lacks +// install-user-file.sh, so PushUserFile's Admin call fails with an opaque +// exec error. The wrapped message must point at the fix: restarting the VM. +func TestPushUserFileWrapsAdminFailureWithUpgradeHint(t *testing.T) { + r := &erroringRunner{match: "install-user-file.sh", err: errors.New("exec format error")} + d := testDeps(t, r) + err := PushUserFile(context.Background(), d, []byte("content"), ".gitconfig", "0644") + if err == nil { + t.Fatal("PushUserFile: expected an error, got nil") + } + if !strings.Contains(err.Error(), "code-vm stop") || !strings.Contains(err.Error(), "code-vm start") { + t.Errorf("PushUserFile error = %v, want it to mention `code-vm stop && code-vm start`", err) + } + if !errors.Is(err, r.err) { + t.Errorf("PushUserFile error = %v, want it to wrap the underlying error (%%w)", err) + } +} + func TestPushUserFileStagesAndRelays(t *testing.T) { r := &fakeRunner{} d := testDeps(t, r) @@ -33,7 +70,7 @@ func TestPushUserFileStagesAndRelays(t *testing.T) { } func TestPushUserFileRejectsUnsafeRel(t *testing.T) { - for _, rel := range []string{"../x", "/abs", ""} { + for _, rel := range []string{"../x", "/abs", "", "has space", "semi;colon", "dollar$sign"} { t.Run(rel, func(t *testing.T) { r := &fakeRunner{} d := testDeps(t, r) From 926eba996748b37b34e229663998203aefca4feb Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 14:08:21 +0200 Subject: [PATCH 16/23] fix: close Trojan Source / C1 gap in isSingleLinePrintable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isSingleLinePrintable rejected C0 controls and DEL but let C1 controls (0x80-0x9f) and Unicode Cf (format) characters through — bidi overrides/isolates (U+202A-202E, U+2066-2069, the Trojan Source class, CVE-2021-42574) and zero-width characters (U+200B, U+200E/F, U+061C, …). These are exactly the concealment vectors the check exists to close: a malicious `suggest` could still display one command while pasting another. Reject C1 and unicode.Cf alongside the existing C0/ DEL check; ordinary non-ASCII letters (accented characters, CJK, …) are unaffected since they are not in Cc/C1/Cf. Co-Authored-By: Claude Fable 5 --- internal/profile/profile.go | 28 +++++++++++----- internal/profile/profile_test.go | 57 ++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 9 deletions(-) diff --git a/internal/profile/profile.go b/internal/profile/profile.go index 1c68162..dfe126e 100644 --- a/internal/profile/profile.go +++ b/internal/profile/profile.go @@ -17,6 +17,7 @@ import ( "regexp" "sort" "strings" + "unicode" "gopkg.in/yaml.v3" @@ -57,17 +58,26 @@ func isBlank(content []byte) bool { return len(bytes.Trim(content, "\n")) == 0 } -// isSingleLinePrintable reports whether s contains no control characters: no -// newline, tab, ESC, or other C0 code, and no DEL (0x7f). Applied to manifest -// strings that are printed verbatim by `code-vm secrets` and `profile list`, -// and — most sensitively — copied byte-for-byte into the ready-to-paste -// MissingSecretSnippet a user pastes into secrets.yaml. Without this check, a -// hostile bundle could hide a shell command behind a terminal escape -// sequence (displays clean, copies with a hidden suffix) or inject a -// newline to forge extra YAML entries in the pasted snippet. +// isSingleLinePrintable reports whether s contains no control or invisible- +// formatting characters: no newline, tab, ESC, or other C0 code; no DEL +// (0x7f); no C1 control (0x80-0x9f); and nothing in Unicode's Cf (format) +// category, which covers bidi overrides/isolates (U+202A-202E, U+2066-2069 — +// the Trojan Source class, CVE-2021-42574) and zero-width characters +// (U+200B ZWSP, U+200E/U+200F, U+061C, …). Ordinary printable text — +// letters, digits, spaces, punctuation, and legitimate non-ASCII letters +// like accented characters — is unaffected: only Cc/C1/Cf are rejected, not +// all non-ASCII. +// +// Applied to manifest strings that are printed verbatim by `code-vm secrets` +// and `profile list`, and — most sensitively — copied byte-for-byte into the +// ready-to-paste MissingSecretSnippet a user pastes into secrets.yaml. +// Without this check, a hostile bundle could hide a shell command behind a +// terminal escape sequence or a bidi override (displays clean, copies with +// hidden or reordered content) or inject a newline to forge extra YAML +// entries in the pasted snippet. func isSingleLinePrintable(s string) bool { for _, r := range s { - if r < 0x20 || r == 0x7f { + if r < 0x20 || r == 0x7f || (r >= 0x80 && r <= 0x9f) || unicode.Is(unicode.Cf, r) { return false } } diff --git a/internal/profile/profile_test.go b/internal/profile/profile_test.go index ce6f61c..e575273 100644 --- a/internal/profile/profile_test.go +++ b/internal/profile/profile_test.go @@ -380,6 +380,14 @@ func TestLoadRejectsInvalidDeclarations(t *testing.T) { {"newline in secret description", "secrets:\n tok:\n description: |\n line one\n line two\n", nil, "description must be a single-line printable string"}, {"tab in var description", "vars:\n url:\n description: \"a\\tb\"\n", nil, "description must be a single-line printable string"}, {"newline in top-level description", "description: |\n line one\n line two\n", nil, "description must be a single-line printable string"}, + // Trojan Source / CVE-2021-42574 class: bidi overrides and isolates + // reorder how surrounding text renders, so a suggest command can + // display one thing and paste another. + {"RLO (U+202E) in secret suggest", "secrets:\n tok:\n suggest: \"gopass show \\u202Efoo\"\n", nil, "suggest must be a single-line printable string"}, + {"LRI (U+2066) in secret suggest", "secrets:\n tok:\n suggest: \"gopass show \\u2066foo\"\n", nil, "suggest must be a single-line printable string"}, + {"ZWSP (U+200B) in var description", "vars:\n url:\n description: \"a\\u200Bb\"\n", nil, "description must be a single-line printable string"}, + {"NEL (U+0085, C1) in top-level description", "description: \"a\\u0085b\"\n", nil, "description must be a single-line printable string"}, + {"C1 CSI (U+009B) in secret description", "secrets:\n tok:\n description: \"a\\u009Bb\"\n", nil, "description must be a single-line printable string"}, {"template/file collision", "description: x\n", map[string]string{"files/.npmrc": "a\n", "templates/.npmrc": "b\n"}, "both files/ and templates/"}, @@ -418,6 +426,55 @@ vars: } } +// Legitimate non-ASCII letters (accented characters and the like) must not +// be swept up by the Cc/C1/Cf rejection: only control and invisible- +// formatting characters are disallowed, not all non-ASCII text. +func TestLoadAcceptsAccentedDescription(t *testing.T) { + dir := t.TempDir() + writeProfile(t, dir, "p", "description: Café token\n", map[string]string{"files/a": "x\n"}) + if _, err := Load(dir, "p"); err != nil { + t.Fatalf("Load: %v", err) + } +} + +// TestSingleLinePrintable exercises isSingleLinePrintable directly, covering +// the Trojan Source / CVE-2021-42574 class (bidi overrides and isolates), +// zero-width formatting characters, C1 controls, and legitimate non-ASCII +// text, alongside the original C0/DEL cases. +func TestSingleLinePrintable(t *testing.T) { + reject := map[string]string{ + "newline": "a\nb", + "tab": "a\tb", + "ESC (C0)": "a\x1bb", + "DEL (0x7f)": "a\x7fb", + "C1 CSI (U+009B)": "a\u009bb", + "NEL (U+0085, C1)": "a\u0085b", + "RLO (U+202E)": "a\u202eb", + "LRI (U+2066)": "a\u2066b", + "ZWSP (U+200B)": "a\u200bb", + } + for name, s := range reject { + t.Run("rejects "+name, func(t *testing.T) { + if isSingleLinePrintable(s) { + t.Errorf("isSingleLinePrintable(%q) = true, want false", s) + } + }) + } + accept := []string{ + "ordinary text with spaces", + "gopass show -o some/path with spaces", + "Caf\u00e9 token", + "\u65e5\u672c\u8a9e\u306e\u30c6\u30ad\u30b9\u30c8", + } + for _, s := range accept { + t.Run("accepts "+s, func(t *testing.T) { + if !isSingleLinePrintable(s) { + t.Errorf("isSingleLinePrintable(%q) = false, want true", s) + } + }) + } +} + func TestLoadRejectsSymlinkedTemplatesRoot(t *testing.T) { dir := t.TempDir() writeProfile(t, dir, "p", "description: x\n", map[string]string{"files/a": "x\n"}) From 81897fb91915cd3a24555800b536a3c0a427daaf Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 16:32:24 +0200 Subject: [PATCH 17/23] fix: resolve secrets/vars before mutating guest state in start/shell/mount/recreate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit start, shell, mount, and recreate all called pushRenderedTemplates (resolve then push) after already booting, stopping, or deleting the VM, so a missing or failing secret mapping surfaced only after guest state was already mutated — most seriously in recreate, where it could destroy the VM before the failure was known. Split each caller into resolveRendered (host-only, no guest I/O) before the mutation, then pushRendered (guest push of the already-resolved set) after. shell's already-running fast path is preserved exactly: it still skips resolution and pinentry entirely when the VM is already up, matching today's behavior, since it is the per-invocation hot path rather than an explicit command. Co-Authored-By: Claude Fable 5 --- internal/cli/mount.go | 18 +++++---- internal/cli/mount_test.go | 38 +++++++++++++++++++ internal/cli/recreate.go | 18 +++++---- internal/cli/recreate_test.go | 43 +++++++++++++++++++++ internal/cli/shell.go | 20 +++++++++- internal/cli/shell_test.go | 70 +++++++++++++++++++++++++++++++++++ internal/cli/start.go | 15 +++++++- internal/cli/start_test.go | 35 ++++++++++++++++++ 8 files changed, 238 insertions(+), 19 deletions(-) create mode 100644 internal/cli/recreate_test.go diff --git a/internal/cli/mount.go b/internal/cli/mount.go index 554bb65..f331f84 100644 --- a/internal/cli/mount.go +++ b/internal/cli/mount.go @@ -80,20 +80,22 @@ func newMountCmd() *cobra.Command { fmt.Fprintln(out, "VM is not running; the new mount applies on next start.") return nil } + // Resolve (and render) before stopping the VM at all: resolution + // needs nothing from the guest, so an unmapped secret must abort + // before the running VM is even stopped, let alone left stopped + // on a failed restart. + rendered, err := resolveRendered(cmd.Context(), updated, profiles, path, out) + if err != nil { + return err + } fmt.Fprintln(out, "Restarting the VM to apply the new mount...") if err := cl.Stop(cmd.Context()); err != nil { return err } - started, err := ensureRunning(cmd.Context(), cl, updated, profiles) - if err != nil { + if _, err := ensureRunning(cmd.Context(), cl, updated, profiles); err != nil { return err } - if started { - if err := pushRenderedTemplates(cmd.Context(), cl, updated, profiles, path, out); err != nil { - return err - } - } - return nil + return pushRendered(cmd.Context(), cl, updated, profiles, rendered, out) }, } } diff --git a/internal/cli/mount_test.go b/internal/cli/mount_test.go index a2ad58b..79cc5fd 100644 --- a/internal/cli/mount_test.go +++ b/internal/cli/mount_test.go @@ -110,3 +110,41 @@ func TestMountRefusesProfilesDirectoryWithoutSavingOrRestarting(t *testing.T) { t.Errorf("expected no guest interaction before the guard runs, got %v", r.calls) } } + +// Resolution needs nothing from the guest, so an unmapped secret must fail +// `code-vm mount` before the running VM is stopped for the restart — leaving +// it running rather than stopped-and-not-restarted. +func TestMountFailsBeforeStoppingOnUnmappedSecret(t *testing.T) { + root := NewRootCmd() + dir := withScratchConfig(t) + pdir := filepath.Join(dir, "profiles", "p") + if err := os.MkdirAll(filepath.Join(pdir, "templates"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(pdir, "profile.yaml"), + []byte("secrets:\n tok:\n suggest: gopass show -o t\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(pdir, "templates", ".npmrc"), []byte("${secret:tok}\n"), 0o644); err != nil { + t.Fatal(err) + } + appendConfig(t, "profiles:\n - p\n") + // No secrets.yaml: "tok" is unmapped. + + other := t.TempDir() + r := installFakeClient(t, "Running") + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"mount", other}) + err := root.Execute() + if err == nil || !strings.Contains(err.Error(), "gopass show -o t") { + t.Fatalf("mount = %v, want an unmapped-secret error with the suggest snippet; output:\n%s", err, out.String()) + } + if ranAny(r.calls, "stop") { + t.Errorf("VM must not be stopped before resolution succeeds, calls=%v", r.calls) + } + if ranAny(r.calls, "copy") { + t.Errorf("no file may be staged into the guest before resolution succeeds, calls=%v", r.calls) + } +} diff --git a/internal/cli/recreate.go b/internal/cli/recreate.go index 725649c..f04c61d 100644 --- a/internal/cli/recreate.go +++ b/internal/cli/recreate.go @@ -32,20 +32,22 @@ func newRecreateCmd() *cobra.Command { return fmt.Errorf("aborted") } } + // Resolve (and render) before deleting anything: resolution needs + // nothing from the guest, and an unmapped-secret failure must + // never destroy the user's VM — critical here, since Delete is + // irreversible (guest disk, Claude auth, Docker image cache). + rendered, err := resolveRendered(cmd.Context(), c, profiles, cfgPath, cmd.OutOrStdout()) + if err != nil { + return err + } cl := clientFor(c) if err := cl.Delete(cmd.Context()); err != nil { return err } - started, err := ensureRunning(cmd.Context(), cl, c, profiles) - if err != nil { + if _, err := ensureRunning(cmd.Context(), cl, c, profiles); err != nil { return err } - if started { - if err := pushRenderedTemplates(cmd.Context(), cl, c, profiles, cfgPath, cmd.OutOrStdout()); err != nil { - return err - } - } - return nil + return pushRendered(cmd.Context(), cl, c, profiles, rendered, cmd.OutOrStdout()) }, } cmd.Flags().BoolVar(&yes, "yes", false, "skip the confirmation prompt") diff --git a/internal/cli/recreate_test.go b/internal/cli/recreate_test.go new file mode 100644 index 0000000..db41e1e --- /dev/null +++ b/internal/cli/recreate_test.go @@ -0,0 +1,43 @@ +package cli + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// Resolution needs nothing from the guest, so an unmapped secret must fail +// `code-vm recreate` before the VM is deleted — Delete is irreversible (guest +// disk, Claude auth, Docker image cache), so a resolvable-beforehand error +// must never destroy the user's VM. +func TestRecreateFailsBeforeDeletingOnUnmappedSecret(t *testing.T) { + root := NewRootCmd() + dir := withScratchConfig(t) + pdir := filepath.Join(dir, "profiles", "p") + if err := os.MkdirAll(filepath.Join(pdir, "templates"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(pdir, "profile.yaml"), + []byte("secrets:\n tok:\n suggest: gopass show -o t\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(pdir, "templates", ".npmrc"), []byte("${secret:tok}\n"), 0o644); err != nil { + t.Fatal(err) + } + appendConfig(t, "profiles:\n - p\n") + // No secrets.yaml: "tok" is unmapped. + + r := installFakeClient(t, "Running") + root.SetArgs([]string{"recreate", "--yes"}) + err := root.Execute() + if err == nil || !strings.Contains(err.Error(), "gopass show -o t") { + t.Fatalf("recreate = %v, want an unmapped-secret error with the suggest snippet", err) + } + if ranAny(r.calls, "delete") { + t.Errorf("VM must not be deleted before resolution succeeds, calls=%v", r.calls) + } + if ranAny(r.calls, "copy") { + t.Errorf("no file may be staged into the guest before resolution succeeds, calls=%v", r.calls) + } +} diff --git a/internal/cli/shell.go b/internal/cli/shell.go index edef081..4a7e2a6 100644 --- a/internal/cli/shell.go +++ b/internal/cli/shell.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/wetransform/code-vm/internal/config" + "github.com/wetransform/code-vm/internal/profile" "github.com/wetransform/code-vm/internal/session" ) @@ -47,6 +48,23 @@ func runDefault(ctx context.Context, args []string) error { return err } cl := clientFor(c) + // Resolve BEFORE booting, but only when a boot is actually about to + // happen: an already-running VM keeps today's fast path exactly as it + // is — no resolution, no pinentry — since this is the per-invocation hot + // path, not an explicit command like `start`. ensureRunning re-checks + // status itself; the duplicate check here is the price of knowing + // up front whether resolution must run first. + status, err := cl.Status(ctx) + if err != nil { + return err + } + var rendered []profile.Rendered + if status != "Running" { + rendered, err = resolveRendered(ctx, c, profiles, cfgPath, os.Stdout) + if err != nil { + return err + } + } started, err := ensureRunning(ctx, cl, c, profiles) if err != nil { return err @@ -55,7 +73,7 @@ func runDefault(ctx context.Context, args []string) error { return fmt.Errorf("session setup: %w", err) } if started { - if err := pushRenderedTemplates(ctx, cl, c, profiles, cfgPath, os.Stdout); err != nil { + if err := pushRendered(ctx, cl, c, profiles, rendered, os.Stdout); err != nil { return err } } diff --git a/internal/cli/shell_test.go b/internal/cli/shell_test.go index d0ae2ab..c4938bf 100644 --- a/internal/cli/shell_test.go +++ b/internal/cli/shell_test.go @@ -1,6 +1,8 @@ package cli import ( + "os" + "path/filepath" "reflect" "strings" "testing" @@ -40,6 +42,74 @@ func TestResolveWorkdirAcceptsCoveredPath(t *testing.T) { } } +// setupShellFixture writes a scratch config with a profile "p" that declares +// an unmapped secret used by a template, activates it, and chdirs into a +// directory the config shares with the guest (resolveWorkdir requires this). +// No secrets.yaml is written, so "tok" stays unmapped throughout. +func setupShellFixture(t *testing.T) { + t.Helper() + dir := withScratchConfig(t) + c, _, err := loadConfig() + if err != nil { + t.Fatal(err) + } + workdir := filepath.Join(c.ProjectsRoot, "repo") + if err := os.MkdirAll(workdir, 0o755); err != nil { + t.Fatal(err) + } + t.Chdir(workdir) + + pdir := filepath.Join(dir, "profiles", "p") + if err := os.MkdirAll(filepath.Join(pdir, "templates"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(pdir, "profile.yaml"), + []byte("secrets:\n tok:\n suggest: gopass show -o t\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(pdir, "templates", ".npmrc"), []byte("${secret:tok}\n"), 0o644); err != nil { + t.Fatal(err) + } + appendConfig(t, "profiles:\n - p\n") +} + +// Resolution needs nothing from the guest, so an unmapped secret must abort +// the bare `code-vm` invocation before ensureRunning ever boots the VM. +func TestRunDefaultFailsBeforeBootingOnUnmappedSecret(t *testing.T) { + root := NewRootCmd() + setupShellFixture(t) + + r := installFakeClient(t, "") // absent: a bare invocation would boot it + root.SetArgs([]string{}) + err := root.Execute() + if err == nil || !strings.Contains(err.Error(), "gopass show -o t") { + t.Fatalf("bare invocation = %v, want an unmapped-secret error with the suggest snippet", err) + } + if r.started() { + t.Errorf("VM must not be started before resolution succeeds, calls=%v", r.calls) + } + if ranAny(r.calls, "copy") { + t.Errorf("no file may be staged into the guest before resolution succeeds, calls=%v", r.calls) + } +} + +// The already-running fast path must keep today's behavior exactly: no +// resolution (so no pinentry) and no push, even when a declared secret has no +// mapping — resolution never even looks at secrets.yaml in this path. +func TestRunDefaultRunningFastPathSkipsResolution(t *testing.T) { + root := NewRootCmd() + setupShellFixture(t) + + r := installFakeClient(t, "Running") + root.SetArgs([]string{}) + if err := root.Execute(); err != nil { + t.Fatalf("bare invocation against a running VM = %v, want no error (fast path must not resolve)", err) + } + if ranAny(r.calls, ".npmrc") { + t.Errorf("fast path must not render/push templates, calls=%v", r.calls) + } +} + func TestResolveWorkdirRejectsUncoveredPathWithActionableError(t *testing.T) { c := config.Default() c.ProjectsRoot = "/home/st/projects" diff --git a/internal/cli/start.go b/internal/cli/start.go index 6e903ce..e1039fa 100644 --- a/internal/cli/start.go +++ b/internal/cli/start.go @@ -149,11 +149,22 @@ func newStartCmd() *cobra.Command { if err != nil { return err } + ctx := cmd.Context() + // Resolve (and render) before touching the guest at all: resolution + // needs nothing from the guest, so an unmapped secret must abort + // before ensureRunning boots anything. Unlike shell's fast path, + // start always resolves — it is an explicit command, not the + // per-invocation hot path, so resolving even against an + // already-running VM is the right cost/behavior tradeoff. + rendered, err := resolveRendered(ctx, c, profiles, cfgPath, cmd.OutOrStdout()) + if err != nil { + return err + } cl := clientFor(c) - if _, err := ensureRunning(cmd.Context(), cl, c, profiles); err != nil { + if _, err := ensureRunning(ctx, cl, c, profiles); err != nil { return err } - return pushRenderedTemplates(cmd.Context(), cl, c, profiles, cfgPath, cmd.OutOrStdout()) + return pushRendered(ctx, cl, c, profiles, rendered, cmd.OutOrStdout()) }, } } diff --git a/internal/cli/start_test.go b/internal/cli/start_test.go index 72f8ed5..381ce2f 100644 --- a/internal/cli/start_test.go +++ b/internal/cli/start_test.go @@ -3,6 +3,7 @@ package cli import ( "context" "os" + "path/filepath" "runtime" "strings" "testing" @@ -182,3 +183,37 @@ func TestRenderParamsRejectsTheOtherHostsHypervisor(t *testing.T) { t.Errorf("renderParams with vmType %q on %s = nil error, want a failure", c.VMType, runtime.GOOS) } } + +// Resolution needs nothing from the guest, so an unmapped secret must abort +// `code-vm start` before ensureRunning ever boots the VM — otherwise a VM +// with a stale/half-rendered template set is left running. +func TestStartFailsBeforeBootingOnUnmappedSecret(t *testing.T) { + root := NewRootCmd() + dir := withScratchConfig(t) + pdir := filepath.Join(dir, "profiles", "p") + if err := os.MkdirAll(filepath.Join(pdir, "templates"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(pdir, "profile.yaml"), + []byte("secrets:\n tok:\n suggest: gopass show -o t\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(pdir, "templates", ".npmrc"), []byte("${secret:tok}\n"), 0o644); err != nil { + t.Fatal(err) + } + appendConfig(t, "profiles:\n - p\n") + // No secrets.yaml: "tok" is unmapped. + + r := installFakeClient(t, "") // absent: start would create+boot the VM + root.SetArgs([]string{"start"}) + err := root.Execute() + if err == nil || !strings.Contains(err.Error(), "gopass show -o t") { + t.Fatalf("start = %v, want an unmapped-secret error with the suggest snippet", err) + } + if r.started() { + t.Errorf("VM must not be started before resolution succeeds, calls=%v", r.calls) + } + if ranAny(r.calls, "copy") { + t.Errorf("no file may be staged into the guest before resolution succeeds, calls=%v", r.calls) + } +} From bb0711466fc6adcbbed2d7396f5ad2330cc79b4b Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 16:33:45 +0200 Subject: [PATCH 18/23] fix: YAML-quote the suggested command in MissingSecretSnippet The snippet is pasted verbatim into secrets.yaml, but suggest was emitted unquoted as the command value. A suggest like `printf '#token'` had its trailing `'#token'` reparsed as a YAML comment, and one containing ": " would fork into extra mapping keys, so the saved command silently differed from what the profile actually suggested. Add yamlDoubleQuote, a minimal YAML double-quoted-scalar encoder (suggest is already validated single-line printable, so only backslash and the double quote itself need escaping), and use it for the command field. Co-Authored-By: Claude Fable 5 --- internal/cli/secrets_test.go | 2 +- internal/profile/template.go | 25 ++++++++++++++++++++++- internal/profile/template_test.go | 34 ++++++++++++++++++++++++++++++- 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/internal/cli/secrets_test.go b/internal/cli/secrets_test.go index 6927bf1..1a9e70d 100644 --- a/internal/cli/secrets_test.go +++ b/internal/cli/secrets_test.go @@ -36,7 +36,7 @@ func TestSecretsListsMappedAndUnmapped(t *testing.T) { } s := out.String() for _, want := range []string{"mapped-one", "missing-one", "UNMAPPED", "url", - "command: gopass show -o x"} { + `command: "gopass show -o x"`} { if !strings.Contains(s, want) { t.Errorf("output missing %q:\n%s", want, s) } diff --git a/internal/profile/template.go b/internal/profile/template.go index 2df7dfc..939a019 100644 --- a/internal/profile/template.go +++ b/internal/profile/template.go @@ -151,15 +151,38 @@ func DeclaredVars(profiles []Profile) []DeclaredVar { // error. Injectable for tests. type CommandRunner func(ctx context.Context, command string) ([]byte, error) +// yamlDoubleQuote renders s as a YAML double-quoted scalar, so the value that +// lands in secrets.yaml when a user pastes the snippet is byte-for-byte s — +// regardless of characters (#, leading -, ": ", …) that would otherwise be +// reinterpreted as YAML syntax rather than literal command text. Callers only +// ever pass isSingleLinePrintable strings (no control characters, no +// newlines), so the only two characters a double-quoted YAML scalar requires +// escaped are backslash and the double quote itself. +func yamlDoubleQuote(s string) string { + var b strings.Builder + b.WriteByte('"') + for _, r := range s { + if r == '\\' || r == '"' { + b.WriteByte('\\') + } + b.WriteRune(r) + } + b.WriteByte('"') + return b.String() +} + // MissingSecretSnippet renders the ready-to-paste secrets.yaml block for an // unmapped secret. The suggest hint is copied verbatim as the command — // display only until the user adopts it by saving this snippet themselves. +// The command is emitted as a quoted YAML scalar: an unquoted suggest like +// `printf '#token'` would otherwise have its trailing `'#token'` reparsed as +// a YAML comment the moment the snippet is pasted into secrets.yaml. func MissingSecretSnippet(d DeclaredSecret) string { cmd := d.Suggest if cmd == "" { cmd = "" } - return fmt.Sprintf("secrets:\n %s:\n command: %s\n", d.Name, cmd) + return fmt.Sprintf("secrets:\n %s:\n command: %s\n", d.Name, yamlDoubleQuote(cmd)) } // ResolveSecrets resolves every declared secret from the user's sources. Each diff --git a/internal/profile/template_test.go b/internal/profile/template_test.go index 98020be..24e5728 100644 --- a/internal/profile/template_test.go +++ b/internal/profile/template_test.go @@ -7,6 +7,8 @@ import ( "strings" "testing" + "gopkg.in/yaml.v3" + "github.com/wetransform/code-vm/internal/config" ) @@ -99,7 +101,7 @@ func TestResolveSecretsMissingMappingHasSnippet(t *testing.T) { t.Fatal("expected an error for an unmapped secret") } for _, want := range []string{"repo-user", "maven", "Artifactory user", - "secrets:", "command: gopass show -o wetf/user"} { + "secrets:", `command: "gopass show -o wetf/user"`} { if !strings.Contains(err.Error(), want) { t.Errorf("error missing %q:\n%s", want, err) } @@ -130,6 +132,36 @@ func TestResolveVars(t *testing.T) { } } +// MissingSecretSnippet's command must be a YAML double-quoted scalar so a +// suggest containing a YAML-significant character round-trips byte-for-byte +// when the snippet is pasted into secrets.yaml and parsed back. An unquoted +// `printf '#token'` would otherwise have its trailing `'#token'` reparsed as +// a comment, and one containing ": " would be split into extra mapping keys. +func TestMissingSecretSnippetQuotesSuggestForYAMLRoundTrip(t *testing.T) { + for _, suggest := range []string{ + `printf '#token'`, + `echo foo: bar`, + `echo "quoted"`, + `echo \backslash`, + } { + t.Run(suggest, func(t *testing.T) { + snippet := MissingSecretSnippet(DeclaredSecret{Name: "tok", Suggest: suggest}) + var doc struct { + Secrets map[string]struct { + Command string `yaml:"command"` + } `yaml:"secrets"` + } + if err := yaml.Unmarshal([]byte(snippet), &doc); err != nil { + t.Fatalf("snippet did not parse as YAML: %v\nsnippet:\n%s", err, snippet) + } + got := doc.Secrets["tok"].Command + if got != suggest { + t.Errorf("round-tripped command = %q, want %q\nsnippet:\n%s", got, suggest, snippet) + } + }) + } +} + // Load must reject a template referencing an undeclared name (wired in this task). func TestLoadRejectsUndeclaredPlaceholder(t *testing.T) { dir := t.TempDir() From a0992f52428979ce5cf5308bcb95e5a660d9cb34 Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 16:34:53 +0200 Subject: [PATCH 19/23] fix: reject Zl/Zp line-breaking characters in isSingleLinePrintable U+2028 LINE SEPARATOR and U+2029 PARAGRAPH SEPARATOR are Unicode Zl/Zp, not Cf, so they passed isSingleLinePrintable and could visually split a manifest description or secret suggest across what looks like one line. Co-Authored-By: Claude Fable 5 --- internal/profile/profile.go | 19 +++++++++++-------- internal/profile/profile_test.go | 20 +++++++++++--------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/internal/profile/profile.go b/internal/profile/profile.go index dfe126e..ffb03b8 100644 --- a/internal/profile/profile.go +++ b/internal/profile/profile.go @@ -60,24 +60,27 @@ func isBlank(content []byte) bool { // isSingleLinePrintable reports whether s contains no control or invisible- // formatting characters: no newline, tab, ESC, or other C0 code; no DEL -// (0x7f); no C1 control (0x80-0x9f); and nothing in Unicode's Cf (format) +// (0x7f); no C1 control (0x80-0x9f); nothing in Unicode's Cf (format) // category, which covers bidi overrides/isolates (U+202A-202E, U+2066-2069 — // the Trojan Source class, CVE-2021-42574) and zero-width characters -// (U+200B ZWSP, U+200E/U+200F, U+061C, …). Ordinary printable text — -// letters, digits, spaces, punctuation, and legitimate non-ASCII letters -// like accented characters — is unaffected: only Cc/C1/Cf are rejected, not -// all non-ASCII. +// (U+200B ZWSP, U+200E/U+200F, U+061C, …); and nothing in Zl/Zp (U+2028 LINE +// SEPARATOR, U+2029 PARAGRAPH SEPARATOR) — line-breaking characters outside +// the Cc/Cf categories that would otherwise visually split a supposedly +// single-line string. Ordinary printable text — letters, digits, spaces, +// punctuation, and legitimate non-ASCII letters like accented characters — +// is unaffected: only Cc/C1/Cf/Zl/Zp are rejected, not all non-ASCII. // // Applied to manifest strings that are printed verbatim by `code-vm secrets` // and `profile list`, and — most sensitively — copied byte-for-byte into the // ready-to-paste MissingSecretSnippet a user pastes into secrets.yaml. // Without this check, a hostile bundle could hide a shell command behind a // terminal escape sequence or a bidi override (displays clean, copies with -// hidden or reordered content) or inject a newline to forge extra YAML -// entries in the pasted snippet. +// hidden or reordered content) or inject a newline (or U+2028/U+2029) to +// forge extra YAML entries in the pasted snippet. func isSingleLinePrintable(s string) bool { for _, r := range s { - if r < 0x20 || r == 0x7f || (r >= 0x80 && r <= 0x9f) || unicode.Is(unicode.Cf, r) { + if r < 0x20 || r == 0x7f || (r >= 0x80 && r <= 0x9f) || + unicode.Is(unicode.Cf, r) || unicode.Is(unicode.Zl, r) || unicode.Is(unicode.Zp, r) { return false } } diff --git a/internal/profile/profile_test.go b/internal/profile/profile_test.go index e575273..6b0107c 100644 --- a/internal/profile/profile_test.go +++ b/internal/profile/profile_test.go @@ -443,15 +443,17 @@ func TestLoadAcceptsAccentedDescription(t *testing.T) { // text, alongside the original C0/DEL cases. func TestSingleLinePrintable(t *testing.T) { reject := map[string]string{ - "newline": "a\nb", - "tab": "a\tb", - "ESC (C0)": "a\x1bb", - "DEL (0x7f)": "a\x7fb", - "C1 CSI (U+009B)": "a\u009bb", - "NEL (U+0085, C1)": "a\u0085b", - "RLO (U+202E)": "a\u202eb", - "LRI (U+2066)": "a\u2066b", - "ZWSP (U+200B)": "a\u200bb", + "newline": "a\nb", + "tab": "a\tb", + "ESC (C0)": "a\x1bb", + "DEL (0x7f)": "a\x7fb", + "C1 CSI (U+009B)": "a\u009bb", + "NEL (U+0085, C1)": "a\u0085b", + "RLO (U+202E)": "a\u202eb", + "LRI (U+2066)": "a\u2066b", + "ZWSP (U+200B)": "a\u200bb", + "LINE SEPARATOR (U+2028, Zl)": "a\u2028b", + "PARAGRAPH SEPARATOR (U+2029, Zp)": "a\u2029b", } for name, s := range reject { t.Run("rejects "+name, func(t *testing.T) { From acf27c991ff7797b14d5996caac109c5c7734326 Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 16:35:56 +0200 Subject: [PATCH 20/23] fix: clean up staged file when the user-file relay call fails When the Admin relay invocation fails (notably the supported old-VM/missing- install-user-file.sh case), the staged copy was never removed, leaving a rendered credential behind in the admin-only 0700 staging directory indefinitely. Defer a best-effort `rm -f` of the staged path via an independent, bounded context (rather than the caller's ctx) so cleanup still runs when that ctx is already cancelled or past its deadline. install-user-file.sh removes the staged file itself on success, so this is a harmless double-delete on the success path. Co-Authored-By: Claude Fable 5 --- internal/session/userfiles.go | 27 +++++++++++++++++++++++++++ internal/session/userfiles_test.go | 15 +++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/internal/session/userfiles.go b/internal/session/userfiles.go index 862e23d..6d0fbb4 100644 --- a/internal/session/userfiles.go +++ b/internal/session/userfiles.go @@ -6,8 +6,14 @@ import ( "path" "regexp" "strings" + "time" ) +// stagedCleanupTimeout bounds the best-effort removal of a staged file so it +// cannot hang the process: it runs even when the caller's own ctx is +// cancelled or already past its deadline (see cleanupStaged). +const stagedCleanupTimeout = 10 * time.Second + // relCharsetRe mirrors the charset profile.relPathRe enforces on the host // side: conservative on purpose, since rel is written into a root-run // install command on the other end of the relay. Defense in depth — every @@ -33,6 +39,14 @@ func PushUserFile(ctx context.Context, d Deps, content []byte, rel, mode string) if err != nil { return err } + // Best-effort cleanup on every return path, not just failure: the relay + // script itself removes the staged copy on success (see + // install-user-file.sh), so this is a harmless double-delete then. It is + // load-bearing on failure — notably the supported old-VM/missing-script + // case below — where the relay never touches the staged copy at all, + // which would otherwise leave a rendered credential sitting in the + // admin-only staging dir indefinitely. + defer cleanupStaged(d, staged) if err := d.Client.Admin(ctx, []string{ "/usr/local/lib/sandbox/install-user-file.sh", staged, rel, mode, }); err != nil { @@ -45,6 +59,19 @@ func PushUserFile(ctx context.Context, d Deps, content []byte, rel, mode string) return nil } +// cleanupStaged best-effort removes a staged file via the admin channel. It +// uses an independent, bounded context rather than the caller's ctx so it +// still runs when that ctx is cancelled or already past its deadline — +// exactly the case a failed Admin call above may have left it in. The error +// is intentionally discarded: this is cleanup of a temporary drop, not a +// step whose failure should mask (or be conflated with) the actual result of +// PushUserFile. +func cleanupStaged(d Deps, staged string) { + ctx, cancel := context.WithTimeout(context.Background(), stagedCleanupTimeout) + defer cancel() + _ = d.Client.Admin(ctx, []string{"rm", "-f", staged}) +} + // hasDotDotSegment reports whether rel contains a literal ".." path segment. func hasDotDotSegment(rel string) bool { for _, seg := range strings.Split(rel, "/") { diff --git a/internal/session/userfiles_test.go b/internal/session/userfiles_test.go index ff90222..43eb03c 100644 --- a/internal/session/userfiles_test.go +++ b/internal/session/userfiles_test.go @@ -42,6 +42,21 @@ func TestPushUserFileWrapsAdminFailureWithUpgradeHint(t *testing.T) { } } +// When the relay Admin call fails — the supported old-VM/missing-script +// case — the staged copy must still be cleaned up: otherwise a rendered +// credential is left sitting in the admin-only staging dir indefinitely. +func TestPushUserFileCleansUpStagedFileOnRelayFailure(t *testing.T) { + r := &erroringRunner{match: "install-user-file.sh", err: errors.New("exec format error")} + d := testDeps(t, r) + err := PushUserFile(context.Background(), d, []byte("content"), ".gitconfig", "0644") + if err == nil { + t.Fatal("PushUserFile: expected an error, got nil") + } + if !r.ranAny("rm -f") { + t.Errorf("expected a best-effort staged-file cleanup after the relay failed, calls=%v", r.calls) + } +} + func TestPushUserFileStagesAndRelays(t *testing.T) { r := &fakeRunner{} d := testDeps(t, r) From cc68b677de7c2923d1520f7f7655dbd0d5308145 Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 17:01:08 +0200 Subject: [PATCH 21/23] fix: close status TOCTOU between resolve and boot decisions runDefault checked cl.Status once to decide whether to resolve templates, then ensureRunning independently re-checked status to decide whether to boot. If the VM stopped between the two calls, the outer check would see "Running" (skip resolution) while ensureRunning observed the new state and booted anyway, silently skipping template rendering for that boot. Split ensureRunning into a thin wrapper and ensureRunningWithStatus(ctx, cl, c, profiles, status), which takes an already-observed status instead of querying it again. runDefault now queries status exactly once and feeds that single observation to both the resolve gate and ensureRunningWithStatus. The other callers (start/mount/recreate) keep using the ensureRunning wrapper unchanged. Co-Authored-By: Claude Fable 5 --- internal/cli/shell.go | 12 ++++++--- internal/cli/shell_test.go | 53 ++++++++++++++++++++++++++++++++++++++ internal/cli/start.go | 20 +++++++++++--- 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/internal/cli/shell.go b/internal/cli/shell.go index 4a7e2a6..3f4407f 100644 --- a/internal/cli/shell.go +++ b/internal/cli/shell.go @@ -51,9 +51,13 @@ func runDefault(ctx context.Context, args []string) error { // Resolve BEFORE booting, but only when a boot is actually about to // happen: an already-running VM keeps today's fast path exactly as it // is — no resolution, no pinentry — since this is the per-invocation hot - // path, not an explicit command like `start`. ensureRunning re-checks - // status itself; the duplicate check here is the price of knowing - // up front whether resolution must run first. + // path, not an explicit command like `start`. Status is observed exactly + // once and that single value both gates resolution and is handed to + // ensureRunningWithStatus to decide whether to boot — a second, separate + // Status call here would reopen a TOCTOU: the VM could stop between the + // two checks, so the outer check sees "Running" (skipping resolution) + // while a re-check inside ensureRunning boots it anyway, silently + // skipping template rendering for that boot. status, err := cl.Status(ctx) if err != nil { return err @@ -65,7 +69,7 @@ func runDefault(ctx context.Context, args []string) error { return err } } - started, err := ensureRunning(ctx, cl, c, profiles) + started, err := ensureRunningWithStatus(ctx, cl, c, profiles, status) if err != nil { return err } diff --git a/internal/cli/shell_test.go b/internal/cli/shell_test.go index c4938bf..3680682 100644 --- a/internal/cli/shell_test.go +++ b/internal/cli/shell_test.go @@ -110,6 +110,59 @@ func TestRunDefaultRunningFastPathSkipsResolution(t *testing.T) { } } +// countStatusCalls reports how many times the runner was asked for `limactl +// list ...` (the status query behind cl.Status). +func countStatusCalls(calls [][]string) int { + n := 0 + for _, c := range calls { + if len(c) > 0 && c[0] == "list" { + n++ + } + } + return n +} + +// runDefault must decide "is it running" exactly once: the same observation +// both gates resolution and is handed to ensureRunningWithStatus to decide +// whether to boot. A second, independent Status call here would reopen the +// TOCTOU this fix closes — the VM stopping between two separate checks could +// leave resolution skipped (outer check saw Running) while ensureRunning +// boots anyway, silently dropping template rendering for that boot. +func TestRunDefaultQueriesStatusExactlyOnce(t *testing.T) { + root := NewRootCmd() + setupShellFixture(t) + + r := installFakeClient(t, "Running") + root.SetArgs([]string{}) + if err := root.Execute(); err != nil { + t.Fatalf("bare invocation against a running VM = %v", err) + } + if n := countStatusCalls(r.calls); n != 1 { + t.Errorf("expected exactly one status query, got %d, calls=%v", n, r.calls) + } +} + +// A Stopped VM is the boot path: resolution must happen before ensureRunning +// boots it, keyed off the very same status observation used to decide the +// boot itself (single-status-decision — see the comment on ensureRunning). +func TestRunDefaultStoppedResolvesBeforeBoot(t *testing.T) { + root := NewRootCmd() + setupShellFixture(t) + + r := installFakeClient(t, "Stopped") + root.SetArgs([]string{}) + err := root.Execute() + if err == nil || !strings.Contains(err.Error(), "gopass show -o t") { + t.Fatalf("bare invocation against a stopped VM = %v, want an unmapped-secret error with the suggest snippet", err) + } + if r.started() { + t.Errorf("VM must not be started before resolution succeeds, calls=%v", r.calls) + } + if n := countStatusCalls(r.calls); n != 1 { + t.Errorf("expected exactly one status query, got %d, calls=%v", n, r.calls) + } +} + func TestResolveWorkdirRejectsUncoveredPathWithActionableError(t *testing.T) { c := config.Default() c.ProjectsRoot = "/home/st/projects" diff --git a/internal/cli/start.go b/internal/cli/start.go index e1039fa..b361571 100644 --- a/internal/cli/start.go +++ b/internal/cli/start.go @@ -108,14 +108,28 @@ func renderInstanceFile(c config.Config, profiles []profile.Profile) (string, er // started reports whether this call actually booted the VM (status was not // "Running" on entry). Callers use it to gate work that must run once per // boot but never on a plain invocation against an already-running VM. +// +// This is a thin wrapper around ensureRunningWithStatus for callers that have +// not already observed the status themselves. A caller that must make another +// decision (e.g. whether to resolve templates) keyed on the same "is it +// running" fact should call cl.Status itself and pass the result to +// ensureRunningWithStatus directly — see runDefault in shell.go — rather than +// go through this wrapper, which would re-query the status and reopen the +// TOCTOU where the VM stops between the two checks. func ensureRunning(ctx context.Context, cl lima.Client, c config.Config, profiles []profile.Profile) (bool, error) { - if err := c.Validate(); err != nil { - return false, err - } status, err := cl.Status(ctx) if err != nil { return false, err } + return ensureRunningWithStatus(ctx, cl, c, profiles, status) +} + +// ensureRunningWithStatus is ensureRunning's body, taking an already-observed +// status instead of querying it again itself. +func ensureRunningWithStatus(ctx context.Context, cl lima.Client, c config.Config, profiles []profile.Profile, status string) (bool, error) { + if err := c.Validate(); err != nil { + return false, err + } if status == "Running" { return false, nil } From ed9bcc8b018b555da2689f4d7b09fbdf2d6e8b87 Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 17:02:15 +0200 Subject: [PATCH 22/23] fix: clean up partial staged file when Copy fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit stageFile's limactl copy can leave a partial file at the staged guest path on failure or cancellation, and stageFile returned before PushUserFile's deferred cleanup was ever registered — leaking rendered credential bytes in the admin-only guest staging dir. Attempt the same best-effort, independent-context removal (cleanupStaged, already used by PushUserFile) before returning the Copy error. Co-Authored-By: Claude Fable 5 --- internal/session/stage.go | 8 +++++- internal/session/stage_test.go | 50 ++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/internal/session/stage.go b/internal/session/stage.go index 34b547e..4ef97ec 100644 --- a/internal/session/stage.go +++ b/internal/session/stage.go @@ -63,7 +63,13 @@ func stageFile(ctx context.Context, d Deps, content []byte) (string, error) { return "", err } if err := d.Client.Copy(ctx, tmp.Name(), staged); err != nil { - return "", err + // Copy can leave a partial file at staged on failure or cancellation. + // Every other stageFile caller only learns of the guest path once + // Copy succeeds, so this is the only chance to clean it up — best + // effort, on an independent context, the same as PushUserFile's + // deferred cleanup (see cleanupStaged in userfiles.go). + cleanupStaged(d, staged) + return "", fmt.Errorf("copy staged file: %w", err) } return staged, nil } diff --git a/internal/session/stage_test.go b/internal/session/stage_test.go index d2ee5d4..ae7cfe5 100644 --- a/internal/session/stage_test.go +++ b/internal/session/stage_test.go @@ -2,6 +2,7 @@ package session import ( "context" + "errors" "strings" "testing" ) @@ -50,6 +51,55 @@ func TestInstallContentStagesOutsideAgentReach(t *testing.T) { } } +// copyFailingRunner fails every `limactl copy` invocation while otherwise +// behaving like fakeRunner, so tests can exercise stageFile's cleanup path +// without a real guest. +type copyFailingRunner struct { + fakeRunner +} + +func (r *copyFailingRunner) Run(ctx context.Context, args ...string) error { + if err := r.fakeRunner.Run(ctx, args...); err != nil { + return err + } + if len(args) > 0 && args[0] == "copy" { + return errors.New("simulated copy failure") + } + return nil +} + +// A Copy failure can leave a partial file behind at the staged guest path. +// stageFile must attempt a best-effort removal of it before returning the +// error, rather than leaving rendered credential bytes sitting in the +// admin-only staging dir — the same posture PushUserFile's deferred cleanup +// takes on its own failure paths. +func TestStageFileCleansUpPartialFileOnCopyFailure(t *testing.T) { + r := ©FailingRunner{} + d := testDeps(t, r) + _, err := stageFile(context.Background(), d, []byte("secret\n")) + if err == nil { + t.Fatal("expected stageFile to return the Copy error") + } + if !strings.Contains(err.Error(), "simulated copy failure") { + t.Errorf("stageFile error = %v, want it to wrap the Copy failure", err) + } + + var stagedGuestPath string + for _, call := range r.calls { + if len(call) >= 2 && call[0] == "copy" { + // copy : + dst := call[2] + stagedGuestPath = strings.TrimPrefix(dst, "code-sandbox:") + } + } + if stagedGuestPath == "" { + t.Fatal("expected a copy call attempting to stage the file") + } + if !r.ranAny("rm -f " + stagedGuestPath) { + t.Errorf("expected a best-effort cleanup of the partial staged file %q, calls=%v", stagedGuestPath, r.calls) + } +} + func TestStagedNamesAreUnpredictable(t *testing.T) { seen := map[string]bool{} for i := 0; i < 20; i++ { From b86faa825ce8f2be8afc730ec0b7e5c29ed9af89 Mon Sep 17 00:00:00 2001 From: Simon Templer Date: Thu, 20 Aug 2026 17:03:54 +0200 Subject: [PATCH 23/23] test: assert nonzero exit on unmapped secret; fix README wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test-vm-sandbox.sh's unmapped-secret check only grepped for the suggest snippet in captured output, never checking that `profile apply` actually failed. Since the suite runs without `set -e`, a build that printed the snippet but exited 0 would still pass. Capture and assert the exit code alongside the snippet. README: `templates:` read as a profile.yaml key; it is the `templates/` directory. And the "inert display strings" claim about `description`/ `suggest` overclaimed — it's true of host execution only, and profile hooks intentionally do run profile-shipped code in the guest as the agent, so scope the guarantee to what it actually is. Co-Authored-By: Claude Fable 5 --- README.md | 11 ++++++++--- test-vm-sandbox.sh | 8 ++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 352fe19..93a98ac 100644 --- a/README.md +++ b/README.md @@ -135,7 +135,7 @@ Credentials enter the sandbox in one of two ways: created for the sandbox rather than your personal ones, so revoking them is cheap. -- Through a profile's `templates:` (see [Credentials](#credentials) under +- Through a profile's `templates/` tree (see [Credentials](#credentials) under Profiles) — the host-trusted mechanism for sharing a credentialed config's shape while keeping the credential itself in the user's own `secrets.yaml`. @@ -306,8 +306,13 @@ verbatim and reserves placeholders only for the credentialed bits: ``` -`description` and `suggest` are inert display strings: a profile can never -make anything execute. Values come only from the user's own mapping: +`description` and `suggest` are inert display strings on the *host*: nothing +a profile ships is ever run there, and these two fields specifically are +never even parsed as commands. The user's own `secrets.yaml` `command` is +the only host execution in this mechanism. (Profile hooks are the deliberate +exception to "profiles don't execute" — they do run profile-shipped code, +but in the guest, as the agent, as described above.) Values come only from +the user's own mapping: - **`~/.config/code-vm/secrets.yaml`** — 0600, host-trusted like `config.yaml`, never distributed with the profile: diff --git a/test-vm-sandbox.sh b/test-vm-sandbox.sh index eb5611e..5fb25d3 100644 --- a/test-vm-sandbox.sh +++ b/test-vm-sandbox.sh @@ -625,11 +625,11 @@ assert_ok "a mapping change plus apply updates the rendered template" \ yq -i '.secrets.extra-unmapped = {"suggest": "gopass show -o nope"}' "$PROFILE_FIXTURE/profile.yaml" # shellcheck disable=SC2016 # placeholder, substituted host-side at apply printf 'x=${secret:extra-unmapped}\n' > "$PROFILE_FIXTURE/templates/.config/extra.conf" -UNMAPPED_OUT=$("${CODE_VM_ARGS[@]}" profile apply 2>&1) -if echo "$UNMAPPED_OUT" | grep -q 'gopass show -o nope'; then - pass "unmapped secret fails apply with the ready-to-paste snippet" +UNMAPPED_OUT=$("${CODE_VM_ARGS[@]}" profile apply 2>&1); UNMAPPED_RC=$? +if [ "$UNMAPPED_RC" -ne 0 ] && echo "$UNMAPPED_OUT" | grep -q 'gopass show -o nope'; then + pass "unmapped secret fails apply (nonzero) with the ready-to-paste snippet" else - fail "unmapped secret fails apply with the ready-to-paste snippet (got: $UNMAPPED_OUT)" + fail "unmapped secret fails apply with the ready-to-paste snippet (rc=$UNMAPPED_RC, got: $UNMAPPED_OUT)" fi # Restore the fixture to the mapped-only state for the deactivation steps. rm -f "$PROFILE_FIXTURE/templates/.config/extra.conf"