fix: strip developer-identity paths from the cli.Run error envelope (iss-76)

.abcd/work/issues/open/iss-81-machine-output-abspath-beyond-error-scrub.md

@@ -0,0 +1,12 @@
+---
+schema_version: 1
+id: "iss-81"
+slug: "machine-output-abspath-beyond-error-scrub"
+severity: "minor"
+category: "bug"
+source: "user-observation"
+found_during: "2026-07-12 iss-76 security re-review"
+found_at: "internal/surface/cli/cli.go"
+---
+
+Machine output surfaces absolute local paths OUTSIDE the cli.Run error-identity scrub added in iss-76. Two instances found in the iss-76 security re-review: (1) 'memory ingest <abs-path>' — materialFromLocal EvalSymlinks-resolves the user's source argument and embeds the resolved absolute path in a custom IngestError (internal/core/memory/ingest.go:655,658,663,667). It lies outside cwd/home, so scrubPaths' root-redaction and PathError base-name reduction both miss it. Low severity: it is the user's own argument and carries no developer identity for out-of-home paths (a ~/x argument still redacts to ~/x). (2) STRONGER, separate surface: the capture SUCCESS envelope emits an absolute 'path' field, e.g. {"path":"<home>/.abcd/work/issues/open/iss-N-...md"} — a full home/cwd-rooted path emitted unbidden on the SUCCESS path, which the error-only scrubPaths never sees. Detector (per unrecognized-input-never-writes / no-absolute-paths-in-machine-output): extend the per-verb --json table to assert no absolute path in BOTH the success and error envelopes for verbs echoing store/source paths; fix by rendering repo-relative or base-name paths at those sites. Acceptance corpus: the ingest error sites and the capture success 'path' field above.

.abcd/work/issues/open/iss-76-json-error-abspath-leak.md → .abcd/work/issues/resolved/iss-76-json-error-abspath-leak.md

@@ -6,6 +6,7 @@ severity: "minor"
 category: "bug"
 source: "agent-finding"
 found_during: "2026-07-12 /abcd:run iss-29 security review"
+resolution: "cli.Run boundary now redacts cwd/home identity roots + base-names PathError/LinkError; residual verb-level abspath echoes tracked in iss-81"
 ---
 
 cli.Run now routes all command errors through the --json envelope, so any verb that returns a bare *fs.PathError (os.ReadFile/os.Open failures not wrapped by core) emits the absolute local path into machine JSON output, violating no-absolute-paths-in-machine-output. iss-29 sanitised the docs-lint config-load branch specifically; the systemic fix is to sanitise PathError-bearing errors at the Run() boundary (or audit which errors reach the envelope). Detector: a table test that runs each verb's known filesystem-error path under --json and asserts the envelope carries no absolute path. Pre-existing as stderr text; newly widened to --json by cli.Run.

CHANGELOG.md

@@ -12,6 +12,17 @@ called out in a **Breaking** section.
 
 ### Fixed
 
+- **`--json` and stderr command errors no longer leak the developer's home or
+  working-directory paths.** `cli.Run` routes every command error through the
+  machine envelope, so identity-bearing paths reached it three ways: an
+  `os.PathError`/`os.LinkError` (e.g. `memory ask --page-json` on a missing file),
+  a path `fmt`-formatted into a core error (e.g. `capture` on a symlinked ledger
+  dir), and a custom error type (e.g. history's home-rooted store path). The
+  `Run()` boundary now redacts the working-directory and home roots (to `.` and
+  `~`) and reduces any remaining `PathError`/`LinkError` path to its base name.
+  Generalises the per-branch fix made in `iss-29` (iss-76). A verb echoing a
+  user-supplied absolute path outside both roots is out of scope, tracked in
+  `iss-81`.
 - The `intent_lifecycle` record-lint rule now **blocks duplicate intent ids**.
   Id allocators are branch-local — parallel agents on separate branches each
   scan for `max + 1` and mint the same id — so two intents both claimed

internal/surface/cli/cli.go

@@ -1564,7 +1564,7 @@ func Run(args []string, stdout, stderr io.Writer) int {
 	if errors.As(err, &coded) {
 		code = coded.ExitCode()
 	}
-	if msg := err.Error(); msg != "" {
+	if msg := scrubPaths(err); msg != "" {
 		// Honour --json for the error surface too: a caller that asked for
 		// machine output must get a JSON envelope, never raw Go text (iss-29).
 		if asJSON, _ := root.PersistentFlags().GetBool("json"); asJSON {
@@ -1584,6 +1584,84 @@ type errorEnvelope struct {
 	Error string `json:"error"`
 }
 
+// scrubPaths renders err for machine/stderr output with the DEVELOPER-IDENTITY
+// portion of any local path removed. cli.Run routes every command error through
+// the --json envelope and the stderr line, and an identity-bearing path reaches
+// that surface three ways: an os.PathError/os.LinkError embeds one in Error();
+// core fmt-formats one via %s (e.g. capture's ledger-path guards); a custom error
+// type renders one (e.g. history's home-rooted StorePathError). All three are
+// handled (iss-76 — the identity-scrub generalisation of the one branch iss-29
+// fixed):
+//
+//   - the two roots that carry developer identity — the working directory and the
+//     home directory — are redacted to "." and "~" wherever they appear, catching
+//     fmt-formatted and custom-error-type paths a typed walk cannot see;
+//   - any remaining absolute path embedded by os.PathError/os.LinkError (e.g. a
+//     path argument outside both roots) is reduced to its base name.
+//
+// This is NOT a universal absolute-path scrub: a verb that echoes a user-supplied
+// absolute path lying outside both roots (e.g. `memory ingest /tmp/x`) still
+// surfaces it — that path carries no developer identity, and sanitising such
+// verb-level echoes is tracked separately (iss-81). Scrubbing here rather than by
+// regex is deliberate: this error surface also carries URLs (fetch failures) that
+// an absolute-path regex would mangle.
+func scrubPaths(err error) string {
+	msg := err.Error()
+	if cwd, e := os.Getwd(); e == nil {
+		msg = redactRoot(msg, cwd, ".")
+	}
+	if home, e := os.UserHomeDir(); e == nil {
+		msg = redactRoot(msg, home, "~")
+	}
+	for _, p := range embeddedPaths(err) {
+		if filepath.IsAbs(p) {
+			msg = strings.ReplaceAll(msg, p, filepath.Base(p))
+		}
+	}
+	return msg
+}
+
+// redactRoot replaces every occurrence of the absolute directory root (followed
+// by a path separator) in s with repl. The filesystem root ("/") and empty or
+// relative roots are skipped so a message is never mangled into meaninglessness.
+func redactRoot(s, root, repl string) string {
+	if len(root) <= 1 || !filepath.IsAbs(root) {
+		return s
+	}
+	sep := string(os.PathSeparator)
+	return strings.ReplaceAll(s, root+sep, repl+sep)
+}
+
+// embeddedPaths collects the filesystem paths carried by os.PathError/os.LinkError
+// anywhere in err's Unwrap chain, including errors.Join fan-out.
+func embeddedPaths(err error) []string {
+	var paths []string
+	var walk func(error)
+	walk = func(e error) {
+		for e != nil {
+			switch t := e.(type) {
+			case *os.PathError:
+				paths = append(paths, t.Path)
+			case *os.LinkError:
+				paths = append(paths, t.Old, t.New)
+			}
+			switch u := e.(type) {
+			case interface{ Unwrap() error }:
+				e = u.Unwrap()
+			case interface{ Unwrap() []error }:
+				for _, sub := range u.Unwrap() {
+					walk(sub)
+				}
+				return
+			default:
+				return
+			}
+		}
+	}
+	walk(err)
+	return paths
+}
+
 // render writes v as indented JSON when asJSON is set, otherwise delegates to
 // the text renderer. Keeping this one helper is what makes every command's
 // --json behaviour uniform.

internal/surface/cli/error_pathleak_surface_test.go

@@ -0,0 +1,188 @@
+package cli
+
+import (
+	"bytes"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io/fs"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// TestJSONErrorEnvelopeNoAbsolutePathLeak is the iss-76 detector: cli.Run routes
+// every command error through the --json envelope (and the stderr text line), so
+// any verb whose error chain carries an *os.PathError / *os.LinkError would leak
+// its absolute local path into machine output. The sanitisation belongs at the
+// Run() boundary — iss-29 fixed only the docs-lint config-load branch. This is a
+// per-verb table: each row drives a real verb into a filesystem error and asserts
+// the envelope carries no absolute path but keeps the file's base name for context.
+func TestJSONErrorEnvelopeNoAbsolutePathLeak(t *testing.T) {
+	repo := t.TempDir()
+	t.Chdir(repo)
+
+	// An absolute path guaranteed to fail os.ReadFile with a *PathError.
+	absMissing := filepath.Join(repo, "no-such-dir", "page.json")
+
+	cases := []struct {
+		name string
+		args []string
+		base string // basename the sanitised message should retain
+	}{
+		{
+			name: "memory ask --page-json missing file",
+			args: []string{"memory", "ask", "q", "--file-back", "--page-json", absMissing, "--json"},
+			base: "page.json",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			var stdout, stderr bytes.Buffer
+			code := Run(tc.args, &stdout, &stderr)
+			if code == 0 {
+				t.Fatalf("expected a non-zero exit; stdout=%q stderr=%q", stdout.String(), stderr.String())
+			}
+			var env struct {
+				Error string `json:"error"`
+			}
+			if err := json.Unmarshal(stderr.Bytes(), &env); err != nil {
+				t.Fatalf("--json error not JSON-shaped: %v\nstderr: %q", err, stderr.String())
+			}
+			if strings.Contains(env.Error, repo) {
+				t.Fatalf("envelope leaked the absolute path %q:\n%s", repo, env.Error)
+			}
+			if !strings.Contains(env.Error, tc.base) {
+				t.Fatalf("envelope dropped all file context (want basename %q):\n%s", tc.base, env.Error)
+			}
+		})
+	}
+}
+
+// TestCaptureSymlinkErrorNoPathLeak reproduces the security-review finding that
+// the fix must also cover: capture's allocator embeds an ABSOLUTE ledger path via
+// fmt.Errorf("%w … %s") — not an os.PathError — so a typed walk misses it. A
+// symlinked issues/open dir trips the guard and its error reaches the --json
+// envelope; it must not leak the developer's absolute repo path.
+func TestCaptureSymlinkErrorNoPathLeak(t *testing.T) {
+	repo := t.TempDir()
+	t.Chdir(repo)
+	issues := filepath.Join(repo, ".abcd", "work", "issues")
+	if err := os.MkdirAll(issues, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink(t.TempDir(), filepath.Join(issues, "open")); err != nil {
+		t.Fatal(err)
+	}
+
+	var stdout, stderr bytes.Buffer
+	code := Run([]string{"capture", "a defect", "--json"}, &stdout, &stderr)
+	if code == 0 {
+		t.Fatalf("expected a non-zero exit for a symlinked issues/open; stdout=%q stderr=%q", stdout.String(), stderr.String())
+	}
+	var env struct {
+		Error string `json:"error"`
+	}
+	if err := json.Unmarshal(stderr.Bytes(), &env); err != nil {
+		t.Fatalf("--json error not JSON-shaped: %v\nstderr: %q", err, stderr.String())
+	}
+	if strings.Contains(env.Error, repo) {
+		t.Fatalf("capture envelope leaked the absolute repo path %q:\n%s", repo, env.Error)
+	}
+}
+
+// homeRootedErr is a custom error type (like history.StorePathError) that renders
+// a home-dir-rooted absolute path — the class a typed PathError walk cannot see.
+type homeRootedErr struct{ path string }
+
+func (e homeRootedErr) Error() string { return "history: store unreadable (" + e.path + ")" }
+
+// TestScrubPaths is the unit-level detector for the Run()-boundary sanitiser. It
+// removes local-identity paths reaching machine output three ways — os.PathError/
+// os.LinkError (→ base name), and cwd- or home-rooted paths embedded by fmt or a
+// custom error type (→ "." / "~") — while leaving relative and non-path errors
+// untouched.
+func TestScrubPaths(t *testing.T) {
+	abs := filepath.Join(os.TempDir(), "secret", "config.json")
+	rel := filepath.Join("rel", "config.json")
+	cwd, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+	home, err := os.UserHomeDir()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cases := []struct {
+		name        string
+		err         error
+		wantAbsent  string // must NOT appear in the scrubbed message
+		wantPresent []string
+	}{
+		{
+			name:        "fmt-embedded cwd path (capture class)",
+			err:         fmt.Errorf("path unsafe: allocator lock path is a symlink: %s", filepath.Join(cwd, ".abcd", "work", "issues", ".iss-alloc.lock")),
+			wantAbsent:  cwd,
+			wantPresent: []string{".iss-alloc.lock"},
+		},
+		{
+			name:        "custom-type home path (history class)",
+			err:         homeRootedErr{path: filepath.Join(home, ".abcd", "history", "x")},
+			wantAbsent:  home,
+			wantPresent: []string{".abcd", "history: store unreadable"},
+		},
+		{
+			name:        "bare PathError",
+			err:         &os.PathError{Op: "open", Path: abs, Err: fs.ErrNotExist},
+			wantAbsent:  abs,
+			wantPresent: []string{"open", "config.json", fs.ErrNotExist.Error()},
+		},
+		{
+			name:        "wrapped PathError keeps context",
+			err:         fmt.Errorf("cannot read --page-json: %w", &os.PathError{Op: "open", Path: abs, Err: fs.ErrPermission}),
+			wantAbsent:  abs,
+			wantPresent: []string{"cannot read --page-json", "config.json"},
+		},
+		{
+			name:        "LinkError both paths",
+			err:         &os.LinkError{Op: "rename", Old: abs, New: abs + ".tmp", Err: fs.ErrExist},
+			wantAbsent:  abs,
+			wantPresent: []string{"rename", "config.json"},
+		},
+		{
+			name:        "joined errors both scrubbed",
+			err:         errors.Join(&os.PathError{Op: "open", Path: abs, Err: fs.ErrNotExist}, fmt.Errorf("and: %w", &os.PathError{Op: "stat", Path: abs, Err: fs.ErrPermission})),
+			wantAbsent:  abs,
+			wantPresent: []string{"config.json"},
+		},
+		{
+			name:        "relative PathError left intact",
+			err:         &os.PathError{Op: "open", Path: rel, Err: fs.ErrNotExist},
+			wantAbsent:  "\x00-never-", // sentinel: nothing to strip
+			wantPresent: []string{rel},
+		},
+		{
+			name:        "non-path error untouched",
+			err:         errors.New("plain failure"),
+			wantAbsent:  "\x00-never-",
+			wantPresent: []string{"plain failure"},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := scrubPaths(tc.err)
+			if strings.Contains(got, tc.wantAbsent) {
+				t.Fatalf("scrubPaths kept the absolute path %q: %s", tc.wantAbsent, got)
+			}
+			for _, want := range tc.wantPresent {
+				if !strings.Contains(got, want) {
+					t.Fatalf("scrubPaths dropped %q: %s", want, got)
+				}
+			}
+		})
+	}
+}