diff --git a/.abcd/work/issues/open/iss-77-launch-payload-omits-agents-hooks.md b/.abcd/work/issues/open/iss-77-launch-payload-omits-agents-hooks.md new file mode 100644 index 00000000..419caf3e --- /dev/null +++ b/.abcd/work/issues/open/iss-77-launch-payload-omits-agents-hooks.md @@ -0,0 +1,11 @@ +--- +schema_version: 1 +id: "iss-77" +slug: "launch-payload-omits-agents-hooks" +severity: "major" +category: "bug" +source: "agent-finding" +found_during: "2026-07-12 /abcd:run iss-31" +--- + +launch payload completeness: .abcd/config/launch-payload.json includes only [.claude-plugin, commands, scripts, docs, README, LICENSE, .gitignore] and omits agents/ and hooks/, both of which exist and are auto-discovered Claude-Code plugin surfaces (agents are referenced by the harness; hooks are the prompt-router injection transport). The iss-31 'omits skills/' instance is now STALE (no skills/ dir — reclassified to commands/abcd/ on 2026-07-11, already shipped via the commands include). Deciding the exact public includes set (do agents/ and hooks/ ship as-is? do hooks reference a binary path that must resolve post-install?) is design-shaped — maintainer call. Detector: a bundle-completeness test asserting every auto-discovered plugin surface dir is either included or explicitly excluded-with-reason. \ No newline at end of file diff --git a/.abcd/work/issues/open/iss-78-launch-dryrun-needs-version-location.md b/.abcd/work/issues/open/iss-78-launch-dryrun-needs-version-location.md new file mode 100644 index 00000000..957096a3 --- /dev/null +++ b/.abcd/work/issues/open/iss-78-launch-dryrun-needs-version-location.md @@ -0,0 +1,11 @@ +--- +schema_version: 1 +id: "iss-78" +slug: "launch-dryrun-needs-version-location" +severity: "minor" +category: "observation" +source: "agent-finding" +found_during: "2026-07-12 /abcd:run iss-31" +--- + +launch dry-run cannot fully pass on this repo even after iss-31's three instances are fixed: .abcd/config/version-location.json is absent, so CheckLockstep reports the contract unreadable and retention refuses on an empty (non-SemVer) version. This is the un-self-installed source-repo state, not a code defect, but it blocks the 'abcd launch --dry-run is green on its own repo' dogfood gate. Resolve by running the version-location decision setup (or documenting the repo as pre-launch). Needed before the aggregate launch dogfood gate (a CI/test that dry-run is green) can be armed. \ No newline at end of file diff --git a/.abcd/work/issues/open/iss-31-launch-dogfood-gate.md b/.abcd/work/issues/resolved/iss-31-launch-dogfood-gate.md similarity index 52% rename from .abcd/work/issues/open/iss-31-launch-dogfood-gate.md rename to .abcd/work/issues/resolved/iss-31-launch-dogfood-gate.md index e7c8e2a9..d5fc8066 100644 --- a/.abcd/work/issues/open/iss-31-launch-dogfood-gate.md +++ b/.abcd/work/issues/resolved/iss-31-launch-dogfood-gate.md @@ -7,6 +7,7 @@ category: "bug" source: "agent-finding" found_during: "2026-07-08 multi-agent review" found_at: "internal/core/launch" +resolution: "Addressed all three named acceptance-corpus instances of the launch dogfood gate. FIXED: (1) identity scanner /dev/null false positive — a machine username colliding with a system directory (HomeUser=dev) no longer hard-fails on /dev/null; suppressed only as a top-level absolute system-path segment, genuine leaks still caught (security-reviewed PASS, home_path_self backstop verified); (3) globRegexpCache data race — guarded by sync.RWMutex, -race clean. STALE: (2) 'payload omits skills/' — no skills/ dir exists; skills were reclassified to commands/abcd/ on 2026-07-11 and ship via the commands include. The aggregate 'launch --dry-run green on this repo' gate is separately blocked by version-location.json absence (filed iss-78) and a broader payload-completeness question re agents/ + hooks/ (filed iss-77). Live dry-run hard_fails: 2 -> 0." --- launch dry-run cannot pass on its own repo: the identity scanner hard-fails on /dev/null as a local-username false positive (internal/adapter/scanner/identity.go:230) so the launch gates can never pass here; the launch payload omits skills/ and every file its own fallback instructions depend on (.abcd/config/launch-payload.json); globRegexpCache in the bundle resolver is an unsynchronized package-level map — a data race if the transport-agnostic core is ever driven concurrently (internal/core/launch/bundle.go:559). Detector: a dogfood gate — CI runs abcd launch --dry-run against this repo and expects a pass, plus -race coverage of concurrent resolver use. Acceptance corpus: the three instances above; the dogfood gate fails on all of them today. \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 0dd499f1..a8c79b53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -137,6 +137,14 @@ called out in a **Breaking** section. ### Fixed +- **Launch dogfood gate — identity false positive and resolver race** (iss-31). + The secret/PII scanner no longer hard-fails on a system path such as + `/dev/null` when the machine username collides with a system directory name + (e.g. a user called `dev`): a local-username match is suppressed only when it + is the top segment of an absolute system path, so genuine username leaks + (nested under a home root, or bare) are still caught. The launch bundle's + compiled-glob cache is now guarded by a mutex, removing a data race when the + transport-agnostic core resolves bundles concurrently. - **Memory-ingest boundary — partial-failure reporting and CRLF parity** (iss-30, continued). When `abcd memory ingest --keep-original` fails to store the original *after* the pages and registry are durably written, it no longer diff --git a/internal/adapter/scanner/identity.go b/internal/adapter/scanner/identity.go index 4e59b2bf..26f79622 100644 --- a/internal/adapter/scanner/identity.go +++ b/internal/adapter/scanner/identity.go @@ -234,6 +234,12 @@ func (m identityMatchers) findings(line string, lineno int, id2sev map[string]Se if inAnySpan(loc[0], supp) { return } + // A username that equals a system directory and appears as the top + // segment of an absolute path (e.g. "/dev/null" when the machine user + // is "dev") is a system path, not an identity leak. + if isSystemPathSegment(line, loc[0], loc[1]) { + return + } add(kindLocalUser, loc[0]+1, line[loc[0]:loc[1]], "(local machine username; replace with [USERNAME] or remove)") } @@ -302,6 +308,45 @@ func isUsernameWordRune(r byte) bool { return r == '.' || (r >= 'A' && r <= 'Z') || (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') } +// systemDirNames are well-known absolute top-level system directories. A local +// username equal to one of these that appears as the first segment of an +// absolute path is a system path, not an identity leak — a genuine username +// leak is nested under a home root (/Users/, /home/), never at the +// filesystem root. Suppressing only this exact collision (iss-31: "/dev/null" +// when the machine user is "dev") keeps genuine leak detection intact. +var systemDirNames = map[string]bool{ + "dev": true, "proc": true, "sys": true, "usr": true, "bin": true, + "sbin": true, "etc": true, "var": true, "tmp": true, "opt": true, + "lib": true, "run": true, "boot": true, "mnt": true, "media": true, + "srv": true, "root": true, +} + +// isSystemPathSegment reports whether line[start:end] is the first segment of an +// absolute Unix path naming a well-known system directory (e.g. the "dev" in +// "/dev/null"). It requires a leading root '/' that is not itself nested under a +// prior path segment, and a trailing '/', so "/Users/dev/x" and a bare "dev" +// are NOT suppressed. +func isSystemPathSegment(line string, start, end int) bool { + if !systemDirNames[line[start:end]] { + return false + } + if end >= len(line) || line[end] != '/' { + return false + } + if start == 0 || line[start-1] != '/' { + return false + } + root := start - 1 + return root == 0 || !isPathSegmentByte(line[root-1]) +} + +// isPathSegmentByte reports whether b can be part of a path segment, used to +// decide whether a '/' begins an absolute path or continues a nested one. +func isPathSegmentByte(b byte) bool { + return b == '/' || b == '.' || b == '-' || b == '_' || + (b >= 'A' && b <= 'Z') || (b >= 'a' && b <= 'z') || (b >= '0' && b <= '9') +} + func boundaryBefore(line string, pos int) bool { if pos == 0 { return true diff --git a/internal/adapter/scanner/scanner_test.go b/internal/adapter/scanner/scanner_test.go index 41bc6886..3ccac214 100644 --- a/internal/adapter/scanner/scanner_test.go +++ b/internal/adapter/scanner/scanner_test.go @@ -518,3 +518,25 @@ func TestUnscannedUnreadableRecorded(t *testing.T) { t.Errorf("an unreadable bundle file must be recorded in Unscanned, not silently dropped: %+v", res.Unscanned) } } + +// TestIdentityLocalUsernameSystemPathSuppressed proves the iss-31 fix: a machine +// username that collides with a system directory (here "dev" vs "/dev/null") is +// not flagged when it is the top segment of an absolute system path, while a +// genuine username occurrence is still caught. +func TestIdentityLocalUsernameSystemPathSuppressed(t *testing.T) { + id := Identity{HomeUser: "dev"} + pats := DefaultPatterns() + sev := DefaultIdentitySeverities() + + // System path — must NOT flag (the /dev/null false positive). + if got := ScanText(`run something 2>/dev/null || true`, id, pats, sev, "scripts/x.sh"); hasKind(got, kindLocalUser) { + t.Errorf("system path /dev/null wrongly flagged as local username: %+v", got) + } + // Genuine leaks are still flagged (no false negatives from the suppression). + if got := ScanText(`backup written to /home/dev/data`, id, pats, sev, "f"); !hasKind(got, kindLocalUser) { + t.Errorf("nested username /home/dev not flagged (false negative): %+v", got) + } + if got := ScanText(`last commit authored by dev`, id, pats, sev, "f"); !hasKind(got, kindLocalUser) { + t.Errorf("bare username not flagged (false negative): %+v", got) + } +} diff --git a/internal/core/launch/bundle.go b/internal/core/launch/bundle.go index cb9c72e0..c95738e5 100644 --- a/internal/core/launch/bundle.go +++ b/internal/core/launch/bundle.go @@ -17,6 +17,7 @@ import ( "regexp" "sort" "strings" + "sync" "syscall" ) @@ -556,7 +557,14 @@ func globMatch(rel, pattern string) bool { return true } -var globRegexpCache = map[string]compiledGlob{} +// globRegexpCache memoises compiled globs. It is guarded by globRegexpMu so the +// transport-agnostic core can resolve bundles concurrently without a data race +// (iss-31). A concurrent miss may compile the same pattern twice; that is benign +// (both results are equivalent), so the fast path takes only a read lock. +var ( + globRegexpMu sync.RWMutex + globRegexpCache = map[string]compiledGlob{} +) type compiledGlob struct { re *regexp.Regexp @@ -569,7 +577,10 @@ type compiledGlob struct { // via a capturing group whose captured text is post-checked for a / — the guard // group indices are returned. func globToRegexp(pattern string) (*regexp.Regexp, []int) { - if c, ok := globRegexpCache[pattern]; ok { + globRegexpMu.RLock() + c, ok := globRegexpCache[pattern] + globRegexpMu.RUnlock() + if ok { return c.re, c.guards } var b strings.Builder @@ -625,7 +636,9 @@ func globToRegexp(pattern string) (*regexp.Regexp, []int) { } b.WriteString(`$`) re := regexp.MustCompile(b.String()) + globRegexpMu.Lock() globRegexpCache[pattern] = compiledGlob{re: re, guards: guards} + globRegexpMu.Unlock() return re, guards } diff --git a/internal/core/launch/bundle_concurrent_test.go b/internal/core/launch/bundle_concurrent_test.go new file mode 100644 index 00000000..74d17fa1 --- /dev/null +++ b/internal/core/launch/bundle_concurrent_test.go @@ -0,0 +1,25 @@ +package launch + +import ( + "fmt" + "sync" + "testing" +) + +// TestGlobToRegexpConcurrent exercises the compiled-glob cache from many +// goroutines with distinct patterns, forcing concurrent cache writes. Under +// `go test -race` (part of `make preflight`) this fails on the unsynchronised +// package-level map the fix guards with globRegexpMu (iss-31). +func TestGlobToRegexpConcurrent(t *testing.T) { + var wg sync.WaitGroup + for i := 0; i < 64; i++ { + wg.Add(1) + go func(n int) { + defer wg.Done() + pat := fmt.Sprintf("dir%d/**/*.md", n) + re, _ := globToRegexp(pat) + _ = re.MatchString(fmt.Sprintf("dir%d/a/b.md", n)) + }(i) + } + wg.Wait() +}