diff --git a/.abcd/development/plans/2026-07-08-v0.7.3.md b/.abcd/development/plans/2026-07-08-v0.7.3.md new file mode 100644 index 0000000..3492da2 --- /dev/null +++ b/.abcd/development/plans/2026-07-08-v0.7.3.md @@ -0,0 +1,50 @@ +# ferry v0.7.3 — hardening + +**Status:** shipped in v0.7.3. +**Owner:** REPPL. **Type:** implementation plan (Diátaxis: one plan, no mixing). + +## Goal + +Three follow-up fixes flagged during the v0.7.0 config-plugin work, each +self-contained and behaviour-preserving elsewhere. + +## Scope + +- **Emacs local-only deploy.** The Emacs domain deploys the union of the shared + `emacs/` tree and the per-machine `local/emacs/` overlay, so a file present only + in the overlay (a hand-authored `init.local.el` with no shared counterpart) + deploys as a machine-only file — mirroring the iTerm2 Dynamic Profiles union + walk. The volatile-path exclude filter and symlink refusal apply to both trees; + a file present in both resolves local-wins, including its executable bit. + +- **iTerm2 `Restore` running-guard.** `PreferenceDomain.Restore` for iTerm2 honours + the same running-guard as `Apply`: it refuses the `defaults import`/`delete` + while iTerm2 is running (a live iTerm2 rewrites the domain on quit and would + discard the re-import). The backup engine recognises the skip through a + `ResourceRestoreSkipper` interface and continues restoring every other path — + it never aborts a multi-domain restore and never wedges the next `apply`'s + pre-plan rollback. Apple Terminal has no such guard and is unaffected. + +- **tmux recogniser coverage.** The tmux secret recogniser covers `setw` / + `set-window-option` values, a value followed by a trailing `#` comment, and an + unquoted value, in addition to the quoted `set`/`set-option` form. Every shape + it cannot isolate still degrades to a safe refusal on capture (no partial + redaction, no whole-line clobber, no leak); the `${ENV}` exemption is preserved. + +## Known limitation + +- A tmux trailing comment that itself ends in a same-kind quote (e.g. + `set -g @token '' # note'`) over-selects the span to the final quote, so + on capture the comment is silently dropped. The secret stays fully inside the + replaced span and the quotes stay balanced — no leak, valid syntax — and it is + strictly better than the prior behaviour (which refused the common trailing- + comment case entirely). Tightening the trailer match to preserve such a comment + is a future refinement. + +## Verification + +- `make build`, `gofmt -l .` empty, `go vet ./...`, `go test ./...` clean; the + full eval suite green with `FERRY_BIN` set (the iTerm2 restore guard was + exercised with iTerm2 actually running on the host). +- New unit + eval coverage for each fix; ruthless review SHIP, security review + PASS on the restore-path change. diff --git a/CHANGELOG.md b/CHANGELOG.md index 8287368..dc3db34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,29 @@ called out in a **Breaking** section. See ## [Unreleased] +## [0.7.3] - 2026-07-08 + +### Fixed + +- **Emacs local-only overlay files now deploy.** The Emacs domain deploys the + union of the shared `emacs/` tree and the per-machine `local/emacs/` overlay, + so a file that exists only in the overlay — a hand-authored `init.local.el` + with no shared counterpart, say — deploys to `~/.emacs.d/` as a machine-only + file, like an iTerm2 Dynamic Profile does. The volatile-path exclude filter and + symlink refusal apply to both trees. +- **`ferry restore` refuses the iTerm2 preference domain while iTerm2 is + running.** `restore` now honours the same running-guard as `apply`: because a + running iTerm2 rewrites its preferences on quit and would discard a re-import, + restore skips that one domain (with a notice to quit iTerm2 and re-run) and + restores everything else — it never aborts the wider revert. Apple Terminal is + unaffected. +- **The tmux secret recogniser covers more option shapes.** It now recognises a + credential in a `setw` / `set-window-option` value, a value followed by a + trailing `#` comment, and an unquoted value — in addition to the quoted + `set`/`set-option` form. A shape it cannot isolate still degrades to a safe + refusal on capture, never a partial redaction or a leak, and a `${ENV}` + reference is left verbatim. + ## [0.7.2] - 2026-07-08 ### Changed diff --git a/cmd/restore.go b/cmd/restore.go index b9687b8..8c7a465 100644 --- a/cmd/restore.go +++ b/cmd/restore.go @@ -123,6 +123,13 @@ func runRestore(c *cobra.Command, args []string) (retErr error) { return err } + // If iTerm2 is running, the engine SKIPS its preference domain during this + // restore (a live iTerm2 rewrites com.googlecode.iterm2 on quit, so a + // re-import would be silently lost) and restores everything else. Tell the + // user up front how to finish restoring that one domain — the engine's own + // running-guard (terminal.Restore -> ErrITerm2Running) is what enforces it. + warnIfITerm2RestoreSkipped(ctx, args, out) + if len(args) > 0 { if err := scopedRestore(ctx, args, out); err != nil { return err @@ -328,6 +335,44 @@ func terminalPrefDomainID(scopeName string) (string, bool) { } } +// warnIfITerm2RestoreSkipped prints the warn-skip notice when iTerm2 is running +// and its preference domain is part of this restore. The engine's own guard +// (terminal.Restore returns ErrITerm2Running, which backup treats as a clean skip) +// leaves that one domain untouched and restores everything else; this surfaces the +// user-facing notice, since backup holds no writer. darwin-only and best-effort: +// a probe error is ignored (the engine still fails closed on its own probe). +func warnIfITerm2RestoreSkipped(ctx *cmdContext, args []string, out io.Writer) { + if !platform.IsDarwin() { + return + } + // A scoped restore only touches the named domains, so skip the notice unless + // iTerm2 is one of them; a full restore (no args) always includes it. + if len(args) > 0 { + requested := false + for _, a := range args { + if id, ok := terminalPrefDomainID(a); ok && id == terminal.ITerm2Domain { + requested = true + break + } + } + if !requested { + return + } + } + eng, err := ctx.Engine() + if err != nil { + return + } + if !eng.HasBaseline(backup.ResourcePath(terminal.ITerm2Domain)) { + return + } + running, err := terminal.ExecProcessController{}.Running() + if err != nil || !running { + return + } + fmt.Fprintln(out, "restore: iTerm2 is running; skipping its preference domain (a running iTerm2 rewrites its preferences on quit, so the restore would be silently lost). Quit iTerm2 and re-run to restore it — all other paths were restored.") +} + // registerTerminalDomains attaches the iTerm2 + Apple Terminal preference domains // to the engine so their resource baseline entries restore through the // Resource.Restore hook (folded into both full Restore and ScopedRestore). The diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index ccc29d1..32da927 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -149,12 +149,14 @@ flags are one-shot), but if you rely on a global setting the starter allowlist omits, add it to the allowlist so it is carried and preserved. **Quit iTerm2 first.** A running iTerm2 keeps its preferences in memory and -rewrites the domain on quit, so importing while it runs is silently lost — `apply` -therefore *refuses* the global-preferences import while iTerm2 is running and tells -you to quit it and re-run. After a successful import ferry runs `killall cfprefsd` -so the preferences daemon does not serve a stale cached copy; relaunch iTerm2 for -the change to take effect. (The profiles artefact above has no such constraint — it -is a plain file copy iTerm2 live-reloads.) +rewrites the domain on quit, so mutating it while it runs is silently lost — both +`apply` and `restore` therefore *refuse* to touch the global-preferences domain +while iTerm2 is running and tell you to quit it and re-run. `restore` skips only +that one domain (reverting every other managed path) and prints how to finish, so +it never aborts the wider revert. After a successful import ferry runs +`killall cfprefsd` so the preferences daemon does not serve a stale cached copy; +relaunch iTerm2 for the change to take effect. (The profiles artefact above has no +such constraint — it is a plain file copy iTerm2 live-reloads.) The `.local` layer applies whole-domain: a committed `local/iterm2/com.googlecode.iterm2.plist` is imported instead of the shared copy @@ -181,13 +183,20 @@ it does not — the tmux analogue of the shell `[ -f … ] && source …` guard. so the committed `dotfiles/tmux.conf` never carries ferry's own boilerplate, and per-machine edits route to `local/tmux/tmux.conf.local`. -**Tokens in options.** A secret set in a quoted tmux option — for example -`set -g @token 'ghp_…'` — is caught on capture: only the quoted value is routed to -the out-of-repo secret store and replaced with a `{{ferry.secret …}}` placeholder, -leaving the `set -g @token '…'` syntax and the quotes byte-for-byte intact. `apply` -renders the placeholder back to the real value (deployed `0600`). An environment -reference such as `set -g @token '${TMUX_TOKEN}'` is **not** a literal secret (tmux -expands it at read time), so it is carried to the shared repo verbatim. +**Tokens in options.** A secret set in a tmux option — for example +`set -g @token 'ghp_…'` — is caught on capture: only the value is routed to the +out-of-repo secret store and replaced with a `{{ferry.secret …}}` placeholder, +leaving the surrounding syntax byte-for-byte intact. The recogniser covers all +four option-setting commands (`set`, `set-option`, `setw`, `set-window-option`), +single- or double-quoted **and** bare unquoted values, and tolerates a trailing +`# comment` after the value — for instance `set -g @token 'ghp_…' # CI token` keeps +both the quotes and the comment untouched. `apply` renders the placeholder back to +the real value (deployed `0600`). An environment reference such as +`set -g @token '${TMUX_TOKEN}'` (or unquoted `$TMUX_TOKEN`) is **not** a literal +secret (tmux expands it at read time), so it is carried to the shared repo +verbatim. Any option shape the recogniser cannot cleanly isolate — an unbalanced +quote, or a value trailed by something other than whitespace or a comment — is +never partially rewritten: capture blocks the whole file instead. ## git @@ -354,14 +363,16 @@ step between editing `inits/repp.org` in the repo and Emacs re-tangling it on next load. There is no capture pass: `ferry capture` does not ingest live `~/.emacs.d/` edits back into the repo. -**Per-machine overlay.** The `.local` layer applies per file: an override at -`local/emacs/` wins over the shared `emacs/` on the next -`apply`, leaving every other file shared. This is the natural home for a +**Per-machine overlay.** ferry deploys the union of the shared `emacs/` tree and +the per-machine `local/emacs/` overlay tree. The `.local` layer applies per file: +an override at `local/emacs/` wins over the shared `emacs/` on +the next `apply`, leaving every other file shared. A file present **only** under +`local/emacs/` — with no shared counterpart — deploys as a machine-only file, +exactly like iTerm2's Dynamic Profiles overlay. This is the natural home for a Customize-written `inits/custom.el` (which `init.el` loads when present) or a hand-authored `init.local.el` for machine-specific bits (fonts, `exec-path`, -GUI-versus-tty). Like every terminal overlay, the override wins per shared file; -a purely machine-local file is committed under `local/emacs/` alongside a shared -counterpart in `emacs/`. +GUI-versus-tty) that lives on one machine alone. The exclude filter and symlink +refusal apply to both trees. Emacs files participate in the secret store like dotfiles: a `{{ferry.secret …}}` placeholder is rendered on apply, a real secret is commit-gated on capture, and a diff --git a/evals/emacs_test.go b/evals/emacs_test.go index b172aa0..5d06bed 100644 --- a/evals/emacs_test.go +++ b/evals/emacs_test.go @@ -122,6 +122,38 @@ func TestEmacsLocalOverlayWins(t *testing.T) { assertFileContains(t, s.HomePath(".emacs.d", "init.el"), "SHARED_INIT") } +// TestEmacsLocalOnlyFileDeploys proves a file present ONLY under local/emacs/ +// (no shared counterpart) deploys as a machine-only file: a hand-authored +// init.local.el with no emacs/init.local.el reaches ~/.emacs.d/init.local.el. +func TestEmacsLocalOnlyFileDeploys(t *testing.T) { + t.Parallel() + s := NewSandbox(t) + s.InitGitRepo(t) + s.SeedSharedManifest(t, emacsManifest) + + // A shared file plus a machine-only overlay file with NO shared counterpart. + s.WriteRepoFile(t, filepath.Join("emacs", "init.el"), "SHARED_INIT\n") + s.WriteRepoFile(t, filepath.Join("local", "emacs", "init.local.el"), "MACHINE_ONLY\n") + gitCommitAll(t, s.Repo, "emacs with a local-only file") + + if _, errOut, code := s.Ferry("apply"); code != 0 { + t.Fatalf("apply exited %d; stderr:\n%s", code, errOut) + } + + // The machine-only file deploys as a regular-file copy alongside the shared one. + localTarget := s.HomePath(".emacs.d", "init.local.el") + assertFileContains(t, localTarget, "MACHINE_ONLY") + assertFileContains(t, s.HomePath(".emacs.d", "init.el"), "SHARED_INIT") + + fi, err := os.Lstat(localTarget) + if err != nil { + t.Fatalf("lstat local-only target: %v", err) + } + if fi.Mode()&os.ModeSymlink != 0 || !fi.Mode().IsRegular() { + t.Errorf("%s must be a regular-file copy (mode %v)", localTarget, fi.Mode()) + } +} + // TestEmacsStatusCleanThenDrift proves status reports clean right after apply // (with no spurious de-scope warning) and drift after a live edit — the // repo-authoritative reconcile (apply would skip a live edit). diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index 05378de..4787084 100644 --- a/internal/backup/backup_test.go +++ b/internal/backup/backup_test.go @@ -1003,6 +1003,67 @@ func (f *fakeResource) Restore(blob []byte, absent bool) error { return nil } +// skipResourceErr is a Resource.Restore error that reports itself as a clean +// resource-restore SKIP (mirroring terminal.ErrITerm2Running when iTerm2 is +// running), so the engine must continue the surrounding restore instead of +// aborting. It satisfies ResourceRestoreSkipper without importing terminal. +type skipResourceErr struct{} + +func (skipResourceErr) Error() string { return "resource declined restore (running)" } +func (skipResourceErr) ResourceRestoreSkipped() bool { return true } + +// decliningResource is a Resource whose Restore always DECLINES with a skip error, +// recording how many times it was attempted. +type decliningResource struct { + domain string + attempts int +} + +func (d *decliningResource) Domain() string { return d.domain } +func (d *decliningResource) Backup() ([]byte, bool, error) { return []byte("PREFS"), false, nil } +func (d *decliningResource) Restore(_ []byte, _ bool) error { d.attempts++; return skipResourceErr{} } + +// TestRestoreSkipsDecliningResourceAndContinues proves the running-guard contract +// at the engine level: a resource that DECLINES its restore (skip error) does NOT +// abort the multi-path restore — every other managed path is still reverted, the +// overall restore returns no error, and the resource restore was attempted. +func TestRestoreSkipsDecliningResourceAndContinues(t *testing.T) { + e, home := newEngine(t) + + // A registered resource that will decline restore, captured into the baseline. + res := &decliningResource{domain: "com.googlecode.iterm2"} + e.Register(res) + + // A managed file whose baseline must still be restored despite the resource skip. + target := filepath.Join(home, "f") + mustWrite(t, target, []byte("ORIGINAL"), 0o600) + + r, err := e.Begin() + if err != nil { + t.Fatal(err) + } + if err := e.BackupResource(r, res.domain); err != nil { + t.Fatal(err) + } + if err := e.BackupAndWrite(r, target, []byte("CHANGED"), 0o600); err != nil { + t.Fatal(err) + } + if err := r.Commit(); err != nil { + t.Fatal(err) + } + + // Full restore: the resource declines, but that must be a clean skip. + if _, err := e.Restore(); err != nil { + t.Fatalf("Restore aborted on a declining resource instead of skipping it: %v", err) + } + if res.attempts < 1 { + t.Errorf("resource restore was not attempted (attempts=%d)", res.attempts) + } + if got, _ := os.ReadFile(target); string(got) != "ORIGINAL" { + t.Errorf("file baseline not restored past the resource skip: live = %q, want ORIGINAL", got) + } +} + func TestBackupResourceCapturesAndRestoreReapplies(t *testing.T) { e, _ := newEngine(t) res := &fakeResource{domain: "com.googlecode.iterm2", state: []byte("ORIGINAL-PREFS")} diff --git a/internal/backup/journal.go b/internal/backup/journal.go index 7dc582f..e29a994 100644 --- a/internal/backup/journal.go +++ b/internal/backup/journal.go @@ -230,6 +230,13 @@ func (e *Engine) rollbackRun(runID string) error { } } if err := e.applyState(ch.Prior, blob); err != nil { + // A resource declined its restore (e.g. iTerm2 running): SKIP this change + // and keep rolling back the rest. A running iTerm2 rewrites its domain on + // quit anyway, so we cannot re-import it now — and letting this abort would + // WEDGE the next apply, which runs RollbackIncomplete before it plans. + if isResourceRestoreSkip(err) { + continue + } return err } } diff --git a/internal/backup/restore.go b/internal/backup/restore.go index fd40bc2..c6fd02a 100644 --- a/internal/backup/restore.go +++ b/internal/backup/restore.go @@ -97,6 +97,14 @@ func (e *Engine) restorePaths(absPaths []string) (string, error) { refused = append(refused, fmt.Errorf("restore refused for %s: %w", p, err)) continue } + // A registered resource DECLINED its restore (e.g. iTerm2 running, where + // re-importing the live domain would be silently lost). This is a clean + // SKIP, not a failure: leave the domain as-is and CONTINUE restoring the + // other paths rather than aborting the whole revert. cmd surfaces the + // user-facing notice (it holds the writer and the same running probe). + if isResourceRestoreSkip(err) { + continue + } return snapID, err } } @@ -118,6 +126,24 @@ func (e *Engine) applyState(state PathState, blob []byte) error { return restoreState(state, blob) } +// ResourceRestoreSkipper is implemented by an error a Resource.Restore returns to +// signal it DECLINED the restore (not failed) — e.g. the iTerm2 domain when iTerm2 +// is running, where importing/deleting the live domain would be silently lost. The +// engine treats such an error as a CLEAN SKIP: the resource is left as-is and the +// surrounding multi-path restore / snapshot-undo / incomplete-run rollback +// CONTINUES instead of aborting. It is a structural interface so the engine stays +// decoupled from the terminal package that raises it (no import either way). +type ResourceRestoreSkipper interface { + ResourceRestoreSkipped() bool +} + +// isResourceRestoreSkip reports whether an applyState error is a resource DECLINE +// (a clean skip) rather than a genuine failure. +func isResourceRestoreSkip(err error) bool { + var s ResourceRestoreSkipper + return errors.As(err, &s) && s.ResourceRestoreSkipped() +} + // restoreResource drives a registered preference-domain Resource's own restore. // When absent is true the baseline recorded the domain as not existing pre-ferry, // so the resource REMOVES/clears it (e.g. `defaults delete`); otherwise blob (the diff --git a/internal/backup/snapshot.go b/internal/backup/snapshot.go index ea76b0f..1c154cc 100644 --- a/internal/backup/snapshot.go +++ b/internal/backup/snapshot.go @@ -79,6 +79,12 @@ func (e *Engine) RestoreSnapshot(snapID string) error { } } if err := e.applyState(state, blob); err != nil { + // A resource declined its restore (e.g. iTerm2 running): SKIP this entry + // and continue undoing the rest rather than aborting the whole snapshot + // re-apply. A running iTerm2 would silently drop the re-import anyway. + if isResourceRestoreSkip(err) { + continue + } return err } } diff --git a/internal/emacs/doc.go b/internal/emacs/doc.go index ef44608..08a5964 100644 --- a/internal/emacs/doc.go +++ b/internal/emacs/doc.go @@ -23,10 +23,15 @@ // state (auto-save-list/, transient/, url/, network-security.data, recentf, // savehist, saveplace). See exclude.go for the exact predicate. // -// Per-machine overlay. The termcfg per-file local-wins overlay applies: a file -// at local/emacs/ in the repo overrides the shared emacs/ on -// deploy (whole-file, no Emacs-Lisp parsing), the natural home for a -// Customize-written inits/custom.el or a hand-authored init.local.el. +// Per-machine overlay. The domain deploys the UNION of the shared emacs/ tree +// and the per-machine local/emacs/ overlay tree. The termcfg per-file local-wins +// overlay applies: a file at local/emacs/ in the repo overrides the +// shared emacs/ on deploy (whole-file, no Emacs-Lisp parsing). A file +// present ONLY under local/emacs/ (no shared counterpart) deploys as a +// machine-only file, exactly like iTerm2's Dynamic Profiles overlay — the +// natural home for a Customize-written inits/custom.el or a hand-authored +// init.local.el that exists on this machine alone. The exclude filter and +// symlink refusal apply to BOTH trees. // // Deploy home is ~/.emacs.d (matching the maintainer's model). Note ~/.emacs.d // shadows the XDG location ~/.config/emacs: with ~/.emacs.d present, Emacs reads diff --git a/internal/emacs/emacs_test.go b/internal/emacs/emacs_test.go index 65c27c5..64602d5 100644 --- a/internal/emacs/emacs_test.go +++ b/internal/emacs/emacs_test.go @@ -5,6 +5,7 @@ import ( "path/filepath" "reflect" "sort" + "strings" "testing" ) @@ -160,6 +161,94 @@ func TestPlan_localOverlayWins(t *testing.T) { } } +// TestPlan_localOnlyFileDeploysAsMachineOnly proves a file present ONLY under +// local/emacs/ (no shared counterpart) is enumerated by the union walk and +// deploys as a machine-only file to ~/.emacs.d/, alongside the shared tree. +func TestPlan_localOnlyFileDeploysAsMachineOnly(t *testing.T) { + repo := t.TempDir() + home := t.TempDir() + mkfile(t, repo, "emacs/init.el", "shared-init") + // A machine-only file with NO shared counterpart. + mkfile(t, repo, "local/emacs/init.local.el", "MACHINE-only") + + items, warnings, err := Plan(PlanInput{RepoRoot: repo, Home: home}) + if err != nil { + t.Fatalf("Plan: %v", err) + } + if len(warnings) != 0 { + t.Errorf("warnings = %v, want none", warnings) + } + got := planKeys(items) + want := []string{"emacs/init.el", "emacs/init.local.el"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("keys = %v, want %v (local-only file must deploy)", got, want) + } + byKey := map[string]Item{} + for _, it := range items { + byKey[it.Key] = it + } + if h := byKey["emacs/init.local.el"].Target.Home; h != filepath.Join(home, ".emacs.d/init.local.el") { + t.Errorf("init.local.el home = %s, want ~/.emacs.d/init.local.el", h) + } + if got := string(byKey["emacs/init.local.el"].Content); got != "MACHINE-only" { + t.Errorf("init.local.el content = %q, want the local-only bytes", got) + } +} + +// TestPlan_excludesApplyToLocalOnlyFiles proves the volatile-path exclude filter +// runs over the local overlay tree too: a machine-only volatile path under +// local/emacs/ is pruned exactly like one under emacs/. +func TestPlan_excludesApplyToLocalOnlyFiles(t *testing.T) { + repo := t.TempDir() + home := t.TempDir() + mkfile(t, repo, "emacs/init.el", "keep") + // Machine-only carry file plus machine-only volatile decoys. + mkfile(t, repo, "local/emacs/init.local.el", "keep-local") + mkfile(t, repo, "local/emacs/init.local.elc", "bytecode-drop") + mkfile(t, repo, "local/emacs/elpa/pkg/foo.el", "pkg-drop") + mkfile(t, repo, "local/emacs/recentf", "session-drop") + + items, _, err := Plan(PlanInput{RepoRoot: repo, Home: home}) + if err != nil { + t.Fatalf("Plan: %v", err) + } + got := planKeys(items) + want := []string{"emacs/init.el", "emacs/init.local.el"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("keys = %v, want %v (local-only volatile paths must be pruned)", got, want) + } +} + +// TestPlan_refusesLocalOnlySymlink proves a symlinked file present ONLY in the +// local overlay tree is refused with a warning and skipped, so ferry never reads +// a machine-only config through a symlink. +func TestPlan_refusesLocalOnlySymlink(t *testing.T) { + repo := t.TempDir() + home := t.TempDir() + mkfile(t, repo, "emacs/init.el", "real") + // Real overlay dir so the tree exists, plus a symlinked file inside it. + mkfile(t, repo, "local/emacs/keep.el", "real-local") + linkPath := filepath.Join(repo, "local", "emacs", "init.local.el") + if err := os.Symlink(filepath.Join(repo, "emacs", "init.el"), linkPath); err != nil { + t.Fatal(err) + } + + items, warnings, err := Plan(PlanInput{RepoRoot: repo, Home: home}) + if err != nil { + t.Fatalf("Plan: %v", err) + } + if got := planKeys(items); !reflect.DeepEqual(got, []string{"emacs/init.el", "emacs/keep.el"}) { + t.Errorf("keys = %v, want the two real files (local symlink refused)", got) + } + if len(warnings) != 1 { + t.Fatalf("warnings = %v, want one symlink refusal", warnings) + } + if !strings.Contains(warnings[0], "symlink not allowed") || + !strings.Contains(warnings[0], "local/emacs/init.local.el") { + t.Errorf("warning = %q, want a local overlay symlink refusal", warnings[0]) + } +} + // TestPlan_absentSourceDeploysNothing proves an absent emacs/ tree deploys // nothing without warning. func TestPlan_absentSourceDeploysNothing(t *testing.T) { diff --git a/internal/emacs/plan.go b/internal/emacs/plan.go index c9f718e..63abb44 100644 --- a/internal/emacs/plan.go +++ b/internal/emacs/plan.go @@ -6,6 +6,7 @@ import ( "io/fs" "os" "path/filepath" + "sort" "github.com/REPPL/ferry/internal/dotfile" ) @@ -62,41 +63,115 @@ type PlanInput struct { } // Plan expands the Emacs domain into its 1:1 (content, target) items: one per -// carried regular file under the repo's emacs/ tree, in lexical walk order, each -// destined for the same relative path under ~/.emacs.d/ and carried like a -// dotfile. Content is the per-machine overlay (local/emacs/) when -// present — local wins, mirroring the dotfile, agents-asset, and terminal -// domains — else the shared repo source. Volatile, machine-generated paths (see -// excluded) are pruned and never deployed. It reads only the config repo, never -// $HOME. +// carried regular file in the UNION of the shared emacs/ tree and the +// per-machine local/emacs/ overlay tree, in lexical order, each destined for the +// same relative path under ~/.emacs.d/ and carried like a dotfile. For a file +// present in both trees the overlay wins (local content, mirroring the dotfile, +// agents-asset, and terminal domains); a file present ONLY under local/emacs/ +// deploys as a machine-only file (mirroring iTerm2's Dynamic Profiles overlay). +// Volatile, machine-generated paths (see excluded) are pruned from BOTH trees +// and never deployed. It reads only the config repo, never $HOME. // -// An absent emacs/ tree deploys nothing (the domain is enabled but nothing was -// committed). A symlinked source (the tree root or any entry inside it) is -// refused. Recoverable per-target problems (a refused/escaping path) become -// warnings and the item is skipped; only an unexpected read failure aborts. +// An absent emacs/ tree with an absent overlay deploys nothing (the domain is +// enabled but nothing was committed). A symlinked source (either tree root or any +// entry inside either tree) is refused. Recoverable per-target problems (a +// refused/escaping path) become warnings and the item is skipped; only an +// unexpected read failure aborts. func Plan(in PlanInput) (items []Item, warnings []string, err error) { - root := filepath.Join(in.RepoRoot, RepoSubdir) - safeRoot, gerr := guardPath(in.Guard, root) + sources, warns, err := resolveSources(in) + warnings = append(warnings, warns...) + if err != nil { + return nil, warnings, err + } + + rels := make([]string, 0, len(sources)) + for rel := range sources { + rels = append(rels, rel) + } + sort.Strings(rels) // deterministic plan order + + for _, rel := range rels { + item, warn, berr := buildItem(in, sources[rel], rel) + if berr != nil { + return nil, warnings, berr + } + if warn != "" { + warnings = append(warnings, warn) + continue + } + items = append(items, item) + } + return items, warnings, nil +} + +// resolveSources returns the union of carried relpaths across the shared emacs/ +// tree and the per-machine local/emacs/ overlay tree, each mapped to its winning +// source path: the shared emacs/ unless a local/emacs/ overlay exists +// (local wins), plus any file present ONLY under the overlay (a machine-only +// file). Volatile paths are pruned and symlinks are refused with a warning in +// BOTH trees. +func resolveSources(in PlanInput) (map[string]string, []string, error) { + var warnings []string + sources := map[string]string{} + + sharedRoot := filepath.Join(in.RepoRoot, RepoSubdir) + localRoot := filepath.Join(in.RepoRoot, LocalSubdir, RepoSubdir) + + // Shared tree: the authoritative carry set. + sw, err := walkTree(in.Guard, sharedRoot, RepoSubdir) + warnings = append(warnings, sw.warnings...) + if err != nil { + return nil, warnings, err + } + for rel := range sw.files { + sources[rel] = filepath.Join(sharedRoot, rel) + } + + // Local overlay: overrides a shared rel, or adds a machine-only file. + lw, err := walkTree(in.Guard, localRoot, filepath.Join(LocalSubdir, RepoSubdir)) + warnings = append(warnings, lw.warnings...) + if err != nil { + return nil, warnings, err + } + for rel := range lw.files { + sources[rel] = filepath.Join(localRoot, rel) + } + return sources, warnings, nil +} + +type walkResult struct { + files map[string]bool + warnings []string +} + +// walkTree enumerates the carried regular files under root (relative paths), +// pruning volatile paths (excluded) and refusing symlinks. An absent root is +// normal (nothing to deploy). label is the repo-relative directory name used in +// warnings. +func walkTree(guard func(string) (string, error), root, label string) (walkResult, error) { + res := walkResult{files: map[string]bool{}} + safeRoot, gerr := guardPath(guard, root) if gerr != nil { - return nil, []string{refusal("source", RepoSubdir, gerr)}, nil + res.warnings = append(res.warnings, refusal("source", label, gerr)) + return res, nil } fi, serr := os.Lstat(safeRoot) if serr != nil { - // An absent (or unreadable-as-absent) source deploys nothing. if errors.Is(serr, fs.ErrNotExist) { - return nil, nil, nil + return res, nil // absent tree: nothing to deploy } - return nil, nil, serr + return res, serr } if fi.Mode()&fs.ModeSymlink != 0 { - return nil, []string{fmt.Sprintf( - "emacs: refusing %s: symlink not allowed in the managed repo tree (copy the real dir in)", RepoSubdir)}, nil + res.warnings = append(res.warnings, fmt.Sprintf( + "emacs: refusing %s: symlink not allowed in the managed repo tree (copy the real dir in)", label)) + return res, nil } if !fi.IsDir() { - return nil, []string{fmt.Sprintf( - "emacs: refusing %s: the Emacs source must be a directory tree", RepoSubdir)}, nil + res.warnings = append(res.warnings, fmt.Sprintf( + "emacs: refusing %s: the Emacs source must be a directory tree", label)) + return res, nil } - walkErr := filepath.WalkDir(safeRoot, func(path string, d fs.DirEntry, werr error) error { if werr != nil { return werr @@ -118,9 +193,9 @@ func Plan(in PlanInput) (items []Item, warnings []string, err error) { return nil } if d.Type()&fs.ModeSymlink != 0 { - warnings = append(warnings, fmt.Sprintf( + res.warnings = append(res.warnings, fmt.Sprintf( "emacs: refusing %s: symlink not allowed in the managed repo tree (copy the real file in)", - filepath.Join(RepoSubdir, rel))) + filepath.ToSlash(filepath.Join(label, rel)))) if d.IsDir() { return fs.SkipDir } @@ -129,36 +204,33 @@ func Plan(in PlanInput) (items []Item, warnings []string, err error) { if d.IsDir() || !d.Type().IsRegular() { return nil } - info, ierr := d.Info() - if ierr != nil { - return ierr - } - item, warn, berr := buildItem(in, path, rel, info) - if berr != nil { - return berr - } - if warn != "" { - warnings = append(warnings, warn) - return nil - } - items = append(items, item) + res.files[rel] = true return nil }) if walkErr != nil { - return nil, nil, walkErr + return res, walkErr } - return items, warnings, nil + return res, nil } -// buildItem resolves one file's deployable content (overlay-or-shared) and its -// validated $HOME destination under ~/.emacs.d/, returning a ready Item. A -// recoverable per-file problem (a refused target path) is returned as a -// non-empty warning with a zero Item; only an unexpected read failure returns -// err. -func buildItem(in PlanInput, repoFile, rel string, info fs.FileInfo) (Item, string, error) { - content, _, _, cerr := emacsContent(in.RepoRoot, repoFile, in.Guard) - if cerr != nil { - return Item{}, "", cerr +// buildItem reads one file's deployable content from its winning source path +// (shared or overlay) and resolves its validated $HOME destination under +// ~/.emacs.d/, returning a ready Item. A recoverable per-file problem (a refused +// source or target path) is returned as a non-empty warning with a zero Item; +// only an unexpected read failure returns err. The executable bit is taken from +// the winning source so a local-only or overlay file's mode is honoured. +func buildItem(in PlanInput, repoFile, rel string) (Item, string, error) { + safe, gerr := guardPath(in.Guard, repoFile) + if gerr != nil { + return Item{}, refusal("source", "emacs:"+filepath.ToSlash(rel), gerr), nil + } + info, serr := os.Lstat(safe) + if serr != nil { + return Item{}, "", serr + } + data, rerr := os.ReadFile(safe) + if rerr != nil { + return Item{}, "", rerr } key := filepath.ToSlash(rel) t, terr := dotfile.NestedTarget(in.Home, filepath.Join(TargetHome, rel), KeyPrefix+key) @@ -169,57 +241,11 @@ func buildItem(in PlanInput, repoFile, rel string, info fs.FileInfo) (Item, stri Key: KeyPrefix + key, Label: "emacs:" + key, Target: t, - Content: content, + Content: data, Exec: info.Mode()&0o111 != 0, }, "", nil } -// emacsContent resolves the bytes apply deploys for an Emacs file: the -// gitignored per-machine overlay at local/emacs/ when it exists (local -// wins), else the shared repo file. Both reads route through guard (the caller's -// symlink-refusing repo guard), so a symlink inside the managed tree is refused, -// never read through. It returns the resolved content, the overlay path (always -// computed), and whether the overlay was used. -func emacsContent(repoRoot, repoFile string, guard func(string) (string, error)) (content []byte, overlayPath string, usedOverlay bool, err error) { - overlayPath = overlayPathFor(repoRoot, repoFile) - if overlayPath != "" { - safe, gerr := guardPath(guard, overlayPath) - if gerr == nil { - if fi, serr := os.Lstat(safe); serr == nil && fi.Mode().IsRegular() { - data, rerr := os.ReadFile(safe) - if rerr != nil { - return nil, overlayPath, false, rerr - } - return data, overlayPath, true, nil - } - } - } - safe, gerr := guardPath(guard, repoFile) - if gerr != nil { - return nil, overlayPath, false, gerr - } - data, rerr := os.ReadFile(safe) - if rerr != nil { - return nil, overlayPath, false, rerr - } - return data, overlayPath, false, nil -} - -// overlayPathFor maps an Emacs file's shared repo source -// (/emacs/) to its per-machine overlay -// (/local/emacs/) — the SAME relative shape under local/. It -// returns "" when repoFile is not under /emacs (defensive; every -// planned file is), so callers treat that as "no overlay". -func overlayPathFor(repoRoot, repoFile string) string { - emacsRoot := filepath.Join(repoRoot, RepoSubdir) - rel, err := filepath.Rel(emacsRoot, repoFile) - if err != nil || rel == ".." || filepath.IsAbs(rel) || - len(rel) >= 3 && rel[:3] == ".."+string(filepath.Separator) { - return "" - } - return filepath.Join(repoRoot, LocalSubdir, RepoSubdir, rel) -} - // guardPath routes a repo-side candidate through the caller's guard (when // provided), so every repo read in this package honours the same // symlink-refusing policy as the rest of ferry. diff --git a/internal/terminal/domain.go b/internal/terminal/domain.go index dcf6a8a..1a651b3 100644 --- a/internal/terminal/domain.go +++ b/internal/terminal/domain.go @@ -29,6 +29,12 @@ type PreferenceDomain struct { // via `defaults import` (iTerm2 additionally refuses when it is running and // flushes cfprefsd afterwards — see NewITerm2). apply func(r Runner) error + // proc is the iTerm2 lifecycle seam (is iTerm2 running / flush cfprefsd). It is + // set ONLY for the iTerm2 domain (NewITerm2); Apple Terminal leaves it nil and + // so carries no running-guard. Both Apply and Restore consult it to REFUSE + // mutating a live iTerm2 domain (see ErrITerm2Running) and flush cfprefsd after + // a successful mutation. + proc ProcessController // note is the human-readable caveat surfaced in the apply result (e.g. the // iTerm2 relaunch / cfprefsd-cache note). note string @@ -56,6 +62,7 @@ func NewITerm2(exportBlob []byte, runner Runner, proc ProcessController) *Prefer d := &PreferenceDomain{ domain: ITerm2Domain, runner: runner, + proc: proc, note: "iTerm2 preferences imported; relaunch iTerm2 for them to take effect (cfprefsd was flushed).", } d.apply = func(r Runner) error { @@ -165,10 +172,31 @@ func (d *PreferenceDomain) Backup() (blob []byte, absent bool, err error) { // return the machine to that pre-ferry (absent) state rather than importing an // empty blob. A missing-domain delete (already absent) is not an error. When // absent is false a present baseline is re-imported via `defaults import`. +// +// iTerm2 running-guard (iTerm2 ONLY — d.proc is nil for Apple Terminal, which has +// no such concern and restores unconditionally). A running iTerm2 REWRITES +// com.googlecode.iterm2 on quit and cfprefsd caches it, so ANY restore mutation +// (`defaults import` OR `defaults delete`) into a LIVE iTerm2 is silently lost — +// exactly the failure Apply already refuses. Restore therefore mirrors Apply: +// when iTerm2 is running it REFUSES with ErrITerm2Running (a clean skip; the +// restore/rollback caller surfaces a notice and CONTINUES past this domain rather +// than aborting) instead of writing into a live domain. After a successful iTerm2 +// import it flushes cfprefsd so the restored values are not masked by its cache. func (d *PreferenceDomain) Restore(blob []byte, absent bool) error { if !platform.IsDarwin() { return ErrNotDarwin } + // Only the iTerm2 domain carries a proc; skip the guard entirely for Apple + // Terminal. Fail closed on a probe error rather than mutate a possibly-live domain. + if d.proc != nil { + running, err := d.proc.Running() + if err != nil { + return fmt.Errorf("terminal: check whether iTerm2 is running: %w", err) + } + if running { + return ErrITerm2Running + } + } if absent { if _, err := d.runner.Run(nil, "delete", d.domain); err != nil { // Deleting an already-absent domain is a no-op success, not a failure. @@ -182,6 +210,12 @@ func (d *PreferenceDomain) Restore(blob []byte, absent bool) error { if _, err := d.runner.Run(blob, "import", d.domain, "-"); err != nil { return fmt.Errorf("terminal: import %s: %w", d.domain, err) } + // iTerm2 only: flush cfprefsd so the re-imported values are not masked by the + // daemon's cache (mirrors Apply). Best-effort — the bytes are already on disk, + // so a flush hiccup must not fail the restore. + if d.proc != nil { + _ = d.proc.FlushPrefsCache() + } return nil } diff --git a/internal/terminal/process.go b/internal/terminal/process.go index 1d5085e..8169f81 100644 --- a/internal/terminal/process.go +++ b/internal/terminal/process.go @@ -7,14 +7,36 @@ import ( "strings" ) -// ErrITerm2Running is returned by the iTerm2 domain's Apply when iTerm2 is -// currently running. Importing the global preference plist into a running iTerm2 -// is silently lost: iTerm2 holds its preferences in memory and REWRITES +// ErrITerm2Running is returned by the iTerm2 domain's Apply AND Restore when +// iTerm2 is currently running. Mutating the global preference plist while iTerm2 +// runs is silently lost: iTerm2 holds its preferences in memory and REWRITES // com.googlecode.iterm2 on quit, and cfprefsd caches the domain, so a -// `defaults import` while it runs is overwritten the moment the app quits. Apply -// therefore REFUSES (a clean skip, not a failure) and asks the user to quit -// iTerm2 first — mirroring ErrNotDarwin's clean-skip contract. -var ErrITerm2Running = errors.New("terminal: iTerm2 is running; quit it before applying its global preferences") +// `defaults import`/`delete` while it runs is overwritten the moment the app +// quits. Apply and Restore therefore REFUSE (a clean skip, not a failure) and ask +// the user to quit iTerm2 first — mirroring ErrNotDarwin's clean-skip contract. +// +// Its concrete type additionally satisfies backup's structural resource-skip +// contract (ResourceRestoreSkipped), so when Restore refuses because iTerm2 is +// running the restore/rollback engine treats it as a CLEAN SKIP (continue past +// this one domain) rather than a hard failure that aborts a multi-domain restore. +// errors.Is(err, ErrITerm2Running) still holds (a single comparable sentinel +// value), so every existing errors.Is check is unchanged. +var ErrITerm2Running error = iterm2RunningError{} + +// iterm2RunningError is ErrITerm2Running's concrete type. It is an empty struct so +// the sentinel stays comparable (errors.Is works by value equality), and it +// implements ResourceRestoreSkipped so backup can recognise a Restore refusal as a +// skip WITHOUT the backup package importing this one. +type iterm2RunningError struct{} + +func (iterm2RunningError) Error() string { + return "terminal: iTerm2 is running; quit it before applying or restoring its global preferences" +} + +// ResourceRestoreSkipped reports that a Restore returning this error DECLINED the +// restore (iTerm2 running) rather than failing — the signal backup's +// ResourceRestoreSkipper interface reads to skip-and-continue instead of aborting. +func (iterm2RunningError) ResourceRestoreSkipped() bool { return true } // ProcessController is the injectable seam over the iTerm2 lifecycle checks the // global-plist apply needs — is iTerm2 running, and flush cfprefsd's cache after diff --git a/internal/terminal/terminal_test.go b/internal/terminal/terminal_test.go index cbbc7bc..c1c0539 100644 --- a/internal/terminal/terminal_test.go +++ b/internal/terminal/terminal_test.go @@ -149,6 +149,108 @@ func TestRestoreAbsentBaselineDeletesDomain(t *testing.T) { } } +// TestITerm2RestoreRefusesWhenRunning: Restore must honour the SAME running-guard +// as Apply. With iTerm2 running, restoring the iTerm2 domain would `defaults +// import`/`delete` into a live domain that rewrites itself on quit — silently +// lost. Restore must REFUSE with ErrITerm2Running, shell out to `defaults` zero +// times, and NOT flush cfprefsd. It also must NOT be a hard failure: the error +// reports itself as a resource-restore skip. +func TestITerm2RestoreRefusesWhenRunning(t *testing.T) { + if !platform.IsDarwin() { + t.Skip("Restore mutation is darwin-only") + } + for _, absent := range []bool{false, true} { + fr := &fakeRunner{} + proc := &fakeProc{running: true} + d := NewITerm2(nil, fr, proc) + + err := d.Restore([]byte("x"), absent) + if !errors.Is(err, ErrITerm2Running) { + t.Fatalf("Restore(absent=%v) while running = %v, want ErrITerm2Running", absent, err) + } + var skip interface{ ResourceRestoreSkipped() bool } + if !errors.As(err, &skip) || !skip.ResourceRestoreSkipped() { + t.Fatalf("Restore(absent=%v) error is not a resource-restore skip: %v", absent, err) + } + if len(fr.calls) != 0 { + t.Fatalf("Restore(absent=%v) shelled `defaults` %d times into a running iTerm2, want 0", absent, len(fr.calls)) + } + if proc.flushed { + t.Fatalf("Restore(absent=%v) flushed cfprefsd despite refusing", absent) + } + } +} + +// TestITerm2RestoreProceedsAndFlushesWhenNotRunning: with iTerm2 NOT running, +// Restore imports the captured blob AND flushes cfprefsd (mirroring Apply) so the +// restored values are not masked by the daemon's cache. +func TestITerm2RestoreProceedsAndFlushesWhenNotRunning(t *testing.T) { + if !platform.IsDarwin() { + t.Skip("Restore mutation is darwin-only") + } + fr := &fakeRunner{} + proc := &fakeProc{running: false} + d := NewITerm2(nil, fr, proc) + blob := []byte("captured") + + if err := d.Restore(blob, false); err != nil { + t.Fatalf("Restore while not running: %v", err) + } + c := fr.last() + if !equalArgs(c.args, []string{"import", ITerm2Domain, "-"}) { + t.Fatalf("Restore args = %v, want import", c.args) + } + if string(c.stdin) != string(blob) { + t.Fatalf("Restore stdin = %q, want the captured blob", c.stdin) + } + if !proc.flushed { + t.Fatalf("cfprefsd was not flushed after a not-running iTerm2 restore") + } +} + +// TestITerm2RestoreProbeErrorFailsClosed: an inconclusive running probe must fail +// closed (surface the error, mutate nothing) rather than assume "not running" and +// import into a possibly-live iTerm2. +func TestITerm2RestoreProbeErrorFailsClosed(t *testing.T) { + if !platform.IsDarwin() { + t.Skip("Restore mutation is darwin-only") + } + fr := &fakeRunner{} + proc := &fakeProc{runErr: errors.New("pgrep exploded")} + d := NewITerm2([]byte("x"), fr, proc) + + if err := d.Restore([]byte("x"), false); err == nil { + t.Fatal("Restore with a probe error returned nil, want a failure") + } + if len(fr.calls) != 0 { + t.Fatalf("Restore shelled `defaults` despite an inconclusive running probe (%d calls)", len(fr.calls)) + } +} + +// TestAppleTerminalRestoreUnaffectedByRunningGuard: Apple Terminal carries no +// ProcessController (proc is nil) and no iTerm2 concern, so its Restore imports +// unconditionally and never probes running or flushes cfprefsd — even though a +// hypothetical iTerm2 would be "running" (there is nothing to consult here). +func TestAppleTerminalRestoreUnaffectedByRunningGuard(t *testing.T) { + if !platform.IsDarwin() { + t.Skip("Restore mutation is darwin-only") + } + fr := &fakeRunner{} + d := NewAppleTerminal(nil, fr) + blob := []byte("terminal") + + if err := d.Restore(blob, false); err != nil { + t.Fatalf("Apple Terminal Restore: %v", err) + } + c := fr.last() + if !equalArgs(c.args, []string{"import", AppleTerminalDomain, "-"}) { + t.Fatalf("Apple Terminal Restore args = %v, want import", c.args) + } + if string(c.stdin) != string(blob) { + t.Fatalf("Apple Terminal Restore stdin = %q, want the captured blob", c.stdin) + } +} + // TestRestoreAbsentAlreadyMissingIsNoError: deleting an already-absent domain // (delete reports "does not exist") is a no-op success. func TestRestoreAbsentAlreadyMissingIsNoError(t *testing.T) { diff --git a/internal/tmux/secret.go b/internal/tmux/secret.go index 1773a59..a436c88 100644 --- a/internal/tmux/secret.go +++ b/internal/tmux/secret.go @@ -1,6 +1,6 @@ // Package tmux holds ferry's tmux-format knowledge that must NOT live in the // generic secret scanner: the recogniser that finds a secret sitting inside a -// quoted `set -g @token ''` option value. It is the tmux analogue of the +// `set -g @token ''` option value. It is the tmux analogue of the // zsh plugin's assignment recogniser (internal/plugin/zsh) — format-specific // span extraction that CALLS the generic secret primitives (secret.ScanText, // secret.IsNonPlaceholderSecret, and, at the capture site, secret.SwapColumns) @@ -14,56 +14,66 @@ import ( "github.com/REPPL/ferry/internal/secret" ) -// setOptionRe matches a tmux `set` / `set-option` line that assigns a QUOTED -// value to a user option (`@name`). The value carries the potential secret, -// where zsh's whole-line fallback would clobber the `set -g @name` prefix and -// the quotes. Groups: 1 = everything up to and including the opening quote, -// 2 = the opening quote, 3 = the value, 4 = the closing quote. Group 3's byte -// offsets (FindStringSubmatchIndex) become the SecretSpan column range so only -// the value — never the surrounding `set -g @name '…'` syntax — is replaced. +// setOptionPrefixRe matches the tmux command PREFIX that assigns a value to a +// user option (`@name`), up to and including the whitespace before the value: // -// The value group is greedy (`.*`) up to the LAST quote before optional trailing -// whitespace: a tmux option takes a single value, so a greedy match cleanly spans -// a value that itself contains a quote of the OTHER kind. A mismatched -// open/close quote pair is rejected by the caller. -var setOptionRe = regexp.MustCompile( - `^(\s*set(?:-option)?(?:\s+-[A-Za-z]+)*\s+@[A-Za-z0-9_-]+\s+(["']))(.*)(["'])\s*$`) +// set / set-option / setw / set-window-option [-flags]… @name +// +// Group 1 is the whole prefix; its end offset (FindStringSubmatchIndex) is where +// the value token begins. The value itself — quoted or bare, with an optional +// trailing comment — is isolated in code (extractValueSpan) rather than in the +// regex, so the `set -g @name` prefix and any quotes/comment stay outside the +// replaced span and survive byte-for-byte. +// +// Command spellings are ordered longest-first so the intended alternative wins, +// and each is anchored by the mandatory `\s+@name` that follows: `setxyz @o` and +// `set-bogus @o` never match. Flags are the usual `-g`, `-a`, `-u`, `-ga`, … in +// any number; a flag that itself takes a separate argument (`set -t main @o v`) +// is not modelled and simply degrades to a refusal (no span). +var setOptionPrefixRe = regexp.MustCompile( + `^(\s*(?:set-window-option|set-option|setw|set)(?:\s+-[A-Za-z]+)*\s+@[A-Za-z0-9_-]+\s+)`) + +// trailerRe matches what may legally follow an isolated value: nothing but +// optional whitespace and an optional `# …` comment to end of line. A value +// whose tail is anything else cannot be cleanly isolated and is refused. +var trailerRe = regexp.MustCompile(`^\s*(?:#.*)?$`) -// SecretSpans returns the column-grained secret spans for tmux -// `set -g @token ''` option lines whose quoted VALUE is a real, -// high-confidence, literal secret. A span carries the byte range of the VALUE -// INSIDE the quotes (StartCol/EndCol), so the capture site can replace ONLY that -// range via secret.SwapColumns and leave the `set -g @token '…'` syntax and the -// quotes byte-identical. +// SecretSpans returns the column-grained secret spans for tmux option lines +// whose assigned VALUE is a real, high-confidence, literal secret. The value may +// be single- or double-quoted, or a bare unquoted token, on any of the four +// option-setting commands (set / set-option / setw / set-window-option), and may +// carry a trailing `# comment`. A span carries the byte range of the VALUE ONLY +// (StartCol/EndCol, excluding any quotes), so the capture site replaces just that +// range via secret.SwapColumns and leaves the surrounding `set -g @token '…' #…` +// syntax byte-identical. // // A value is a secret only when BOTH hold, exactly as the git/zsh sub-value // recognisers gate: -// - secret.IsNonPlaceholderSecret(value) — an env-ref like ${TMUX_TOKEN}, a -// ferry {{…}} placeholder, or an empty value is NOT a secret and is left in -// the repo verbatim; and +// - secret.IsNonPlaceholderSecret(value) — an env-ref like ${TMUX_TOKEN} or +// $TMUX_TOKEN, a ferry {{…}} placeholder, or an empty value is NOT a secret +// and is left in the repo verbatim; and // - secret.ScanText(value).HasHigh() — the value is a real high-confidence // secret, not an ordinary short option string. // -// A line without a quoted `@name` value, or one whose open/close quotes differ, -// contributes nothing. +// SAFETY: any shape the recogniser cannot cleanly isolate — an unbalanced quote, +// a value whose tail is neither whitespace nor a comment, a flag that consumes an +// argument — yields NO span, so capture degrades to a whole-file refusal rather +// than a mis-sliced or whole-line clobber. The capture site's post-patch re-gate +// (secret.IsBlockedFromRepo) is the backstop. func SecretSpans(text string) []secret.SecretSpan { var spans []secret.SecretSpan for i, line := range strings.Split(text, "\n") { - m := setOptionRe.FindStringSubmatchIndex(line) + m := setOptionPrefixRe.FindStringSubmatchIndex(line) if m == nil { - continue + continue // not a `set…@name ` line at all } - // m indexes: [0:2] whole, [2:4] group1, [4:6] group2 (open quote), - // [6:8] group3 (value), [8:10] group4 (close quote). - openQuote := line[m[4]:m[5]] - closeQuote := line[m[8]:m[9]] - if openQuote != closeQuote { - continue // an unbalanced quote pair is not a clean value + valStart, valEnd, ok := extractValueSpan(line, m[3]) + if !ok { + continue // value could not be cleanly isolated: refuse, do not clobber } - valStart, valEnd := m[6], m[7] value := line[valStart:valEnd] if !secret.IsNonPlaceholderSecret(value) { - continue // ${ENV} / {{placeholder}} / empty: left verbatim, never stored + continue // ${ENV} / $ENV / {{placeholder}} / empty: left verbatim, never stored } if !secret.ScanText(value).HasHigh() { continue // not a real high-confidence secret @@ -78,3 +88,43 @@ func SecretSpans(text string) []secret.SecretSpan { } return spans } + +// extractValueSpan isolates the option value that begins at byte offset start on +// line, returning the value's [vStart, vEnd) byte range EXCLUDING any quotes. +// ok is false — a clean refusal — whenever the value cannot be unambiguously +// bounded, so the caller contributes no span rather than risk a mis-slice. +// +// Quoted value: the token opens with `'` or `"`; the value ends at the LAST +// matching quote of the SAME kind whose tail is only whitespace and an optional +// comment. Scanning from the end keeps the original greedy behaviour (a value may +// contain the OTHER quote kind) while still rejecting a trailing comment's quotes +// and an unbalanced pair. +// +// Unquoted value: a bare run of non-whitespace bytes, ending at the first space +// (or end of line); the remainder must be only whitespace and an optional +// comment. A `#` is a comment only when whitespace-separated from the value. +func extractValueSpan(line string, start int) (vStart, vEnd int, ok bool) { + rem := line[start:] + if rem == "" { + return 0, 0, false + } + if q := rem[0]; q == '\'' || q == '"' { + for j := len(rem) - 1; j >= 1; j-- { + if rem[j] != q { + continue + } + if trailerRe.MatchString(rem[j+1:]) { + return start + 1, start + j, true // inside the quotes + } + } + return 0, 0, false // no matching close quote with a clean tail + } + end := strings.IndexAny(rem, " \t") + if end < 0 { + end = len(rem) + } + if !trailerRe.MatchString(rem[end:]) { + return 0, 0, false // trailing junk after the bare token: refuse + } + return start, start + end, true +} diff --git a/internal/tmux/secret_test.go b/internal/tmux/secret_test.go index f51b456..4e01dc8 100644 --- a/internal/tmux/secret_test.go +++ b/internal/tmux/secret_test.go @@ -1,12 +1,15 @@ package tmux -import "testing" +import ( + "testing" + + "github.com/REPPL/ferry/internal/secret" +) // TestSecretSpansOffsets pins that the recogniser isolates EXACTLY the quoted // value's byte range, leaving the `set -g @token '…'` prefix and the quotes // outside the span — the precondition for a byte-stable secret.SwapColumns swap. func TestSecretSpansOffsets(t *testing.T) { - const ghToken = "ghp_A1b2C3d4E5f6G7h8I9j0K1l2M3n4O5p6Q7r8" // GitHub token: a named-token High secret line := "set -g @token '" + ghToken + "'" spans := SecretSpans(line + "\n") if len(spans) != 1 { @@ -68,3 +71,111 @@ func TestSecretSpansMismatchedQuotes(t *testing.T) { t.Errorf("mismatched quotes flagged %d spans, want 0", len(spans)) } } + +const ghToken = "ghp_A1b2C3d4E5f6G7h8I9j0K1l2M3n4O5p6Q7r8" // GitHub token: a named-token High secret + +// wantValueSpan asserts SecretSpans found exactly one span whose byte range is +// value and whose surrounding bytes are untouched. +func wantValueSpan(t *testing.T, line, value string) secret.SecretSpan { + t.Helper() + spans := SecretSpans(line) + if len(spans) != 1 { + t.Fatalf("SecretSpans(%q) found %d spans, want 1: %+v", line, len(spans), spans) + } + sp := spans[0] + if !sp.HasColumns() { + t.Fatalf("span is not column-grained: %+v", sp) + } + if got := line[sp.StartCol:sp.EndCol]; got != value { + t.Errorf("span byte range = %q, want %q (line %q)", got, value, line) + } + if sp.Value != value { + t.Errorf("span value = %q, want %q", sp.Value, value) + } + return sp +} + +// TestSecretSpansSetw proves the `setw` command spelling is recognised and the +// span isolates exactly the quoted value. +func TestSecretSpansSetw(t *testing.T) { + line := "setw -g @token '" + ghToken + "'" + sp := wantValueSpan(t, line, ghToken) + if line[sp.StartCol-1] != '\'' || line[sp.EndCol] != '\'' { + t.Errorf("span [%d,%d) not inside the quotes: %q", sp.StartCol, sp.EndCol, line) + } +} + +// TestSecretSpansSetWindowOption proves the long `set-window-option` spelling. +func TestSecretSpansSetWindowOption(t *testing.T) { + line := `set-window-option -g @api "` + ghToken + `"` + sp := wantValueSpan(t, line, ghToken) + if line[sp.StartCol-1] != '"' || line[sp.EndCol] != '"' { + t.Errorf("span [%d,%d) not inside the quotes: %q", sp.StartCol, sp.EndCol, line) + } +} + +// TestSecretSpansTrailingComment proves a `# comment` after the quoted value is +// left outside the span — including a comment that itself contains a quote, which +// must not extend the greedy value match past the true closing quote. +func TestSecretSpansTrailingComment(t *testing.T) { + line := "set -g @token '" + ghToken + "' # it's the API note" + sp := wantValueSpan(t, line, ghToken) + // The byte at EndCol is the true closing quote; the comment (with its own + // apostrophe) stays intact after it. + if line[sp.EndCol] != '\'' { + t.Errorf("EndCol %d does not sit on the closing quote: %q", sp.EndCol, line) + } + if got := line[sp.EndCol:]; got != "' # it's the API note" { + t.Errorf("trailing bytes after span = %q, want the closing quote + comment", got) + } +} + +// TestSecretSpansUnquoted proves a bare unquoted value token is isolated, both +// alone and with a trailing comment, and that offsets cover exactly the token. +func TestSecretSpansUnquoted(t *testing.T) { + line := "set -g @token " + ghToken + sp := wantValueSpan(t, line, ghToken) + if sp.StartCol != len("set -g @token ") || sp.EndCol != len(line) { + t.Errorf("unquoted span [%d,%d), want [%d,%d)", sp.StartCol, sp.EndCol, len("set -g @token "), len(line)) + } + + withComment := "setw -g @token " + ghToken + " # trailing note" + sp = wantValueSpan(t, withComment, ghToken) + if withComment[sp.EndCol] != ' ' { + t.Errorf("EndCol %d should stop at the whitespace before the comment: %q", sp.EndCol, withComment) + } +} + +// TestSecretSpansEnvRefUnquoted keeps the env-ref exemption for BARE values: an +// unquoted ${ENV} / $ENV is expanded at read time, never a literal secret. +func TestSecretSpansEnvRefUnquoted(t *testing.T) { + for _, line := range []string{ + `set -g @token ${TMUX_TOKEN}`, + `setw -g @token $TMUX_TOKEN`, + `set-window-option -g @token ${TMUX_TOKEN} # ref`, + } { + if spans := SecretSpans(line); len(spans) != 0 { + t.Errorf("SecretSpans(%q) flagged %d spans, want 0: %+v", line, len(spans), spans) + } + } +} + +// TestSecretSpansRefusesUnisolable is the SAFETY case: shapes the recogniser +// cannot cleanly bound must yield NO span (capture then blocks whole-file via the +// re-gate) rather than a mis-slice or whole-line clobber. +func TestSecretSpansRefusesUnisolable(t *testing.T) { + for _, line := range []string{ + // A closing quote followed by a second bare token: the tail is neither + // whitespace nor a comment, so there is no clean value boundary. + "set -g @token '" + ghToken + "' extra", + // A flag that consumes its own argument is not modelled: `main` is read as + // the flag argument, the `@token` prefix never matches. + "set -t main @token '" + ghToken + "'", + // Unbalanced quote (open only): no matching close with a clean tail. + "set -g @token '" + ghToken, + } { + if spans := SecretSpans(line); len(spans) != 0 { + t.Errorf("SecretSpans(%q) flagged %d spans, want 0 (must degrade to refusal): %+v", line, len(spans), spans) + } + } +}