From 9e8f32356eb8696822d0936714de42b1a03ceb3a Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:36:42 +0000 Subject: [PATCH 01/11] fix: memory-store reads route through the guarded primitive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The store is an adjudicated trust boundary (ingest.go: a committed page symlink to /dev/zero would hang or OOM the CLI), and ingest's own reads go through fsutil.ReadGuarded — but every sibling reader kept a raw os.ReadFile: the bare status (readOrEmpty for index.md/contradictions.md with no type check, the headroom crawl, the coverage-index read), the lint typed-page gate and both its WalkDir sweeps (WalkDir yields symlinks as non-dir entries and the read follows them), ask's page reads (the ReadDir-to-open swap window ingest documents closing), the quotation-budget config, the stored fingerprint, the writer's read-back, and the ingest licence probes over an arbitrary source root. Reproduced as five distinct hangs via committed mode-120000 fixtures against abcd memory and abcd memory lint. Route them all through fsutil.ReadGuarded (O_NOFOLLOW + regular-file-on-fd + size cap) with the existing in-package caps, and skip non-regular entries in the three WalkDir crawls. Watched-fail tests plant symlinked store leaves and assert the content never crosses. Assisted-by: Claude:claude-fable-5 --- internal/core/memory/ask.go | 5 +- internal/core/memory/bare.go | 13 +- internal/core/memory/coverage.go | 8 +- internal/core/memory/guarded_reads_test.go | 131 +++++++++++++++++++++ internal/core/memory/lint.go | 14 ++- internal/core/memory/provenance.go | 6 +- internal/core/memory/writer.go | 4 +- 7 files changed, 166 insertions(+), 15 deletions(-) create mode 100644 internal/core/memory/guarded_reads_test.go diff --git a/internal/core/memory/ask.go b/internal/core/memory/ask.go index b122192a..5c929f82 100644 --- a/internal/core/memory/ask.go +++ b/internal/core/memory/ask.go @@ -11,6 +11,7 @@ import ( "strings" "time" + "github.com/intentdriven/abcd/internal/fsutil" "github.com/intentdriven/abcd/internal/termsafe" ) @@ -214,7 +215,9 @@ func QueryPages(repoRoot, question string, topN int) ([]MatchedPage, error) { if !e.Type().IsRegular() || !IsMemoryPageName(e.Name()) { continue } - raw, err := os.ReadFile(filepath.Join(mem, e.Name())) + // ReadGuarded re-checks regular-file on the open fd (closing the + // ReadDir→open symlink-swap TOCTOU) and caps the size. + raw, err := fsutil.ReadGuarded(filepath.Join(mem, e.Name()), maxMemoryPageBytes) if err != nil { continue } diff --git a/internal/core/memory/bare.go b/internal/core/memory/bare.go index 414eab55..0427e18e 100644 --- a/internal/core/memory/bare.go +++ b/internal/core/memory/bare.go @@ -8,6 +8,8 @@ import ( "path/filepath" "sort" "strings" + + "github.com/intentdriven/abcd/internal/fsutil" ) // bare.go — the SD001-non-mutating bare render: page count by class, @@ -170,7 +172,7 @@ func bareHeadroomLines(repoRoot, mem string) []string { const header = "Quotation-budget headroom:" indexPath := CoverageIndexPath(repoRoot) - raw, err := os.ReadFile(indexPath) + raw, err := fsutil.ReadGuarded(indexPath, maxRegistryBytes) if err != nil { return []string{header + " coverage index not built yet — run `abcd memory lint`"} } @@ -189,13 +191,13 @@ func bareHeadroomLines(repoRoot, mem string) []string { // Read-only crawl over the same typed pages the lint crawls. var pages []crawledPage _ = filepath.WalkDir(mem, func(path string, d fs.DirEntry, err error) error { - if err != nil || d.IsDir() || !strings.HasSuffix(path, ".md") { + if err != nil || !d.Type().IsRegular() || !strings.HasSuffix(path, ".md") { return nil } if !isTypedMemoryPagePath(mem, path) { return nil } - if b, err := os.ReadFile(path); err == nil { + if b, err := fsutil.ReadGuarded(path, maxMemoryPageBytes); err == nil { rel, _ := filepath.Rel(mem, path) pages = append(pages, crawledPage{rel: filepath.ToSlash(rel), text: string(b)}) } @@ -265,8 +267,11 @@ func bareHeadroomLines(repoRoot, mem string) []string { return lines } +// readOrEmpty reads one store file through the guarded primitive: the store +// sits inside the repo working tree — a trust boundary — so a committed +// symlink leaf is refused rather than followed, and the read is size-capped. func readOrEmpty(path string) (string, bool) { - raw, err := os.ReadFile(path) + raw, err := fsutil.ReadGuarded(path, maxMemoryPageBytes) if err != nil { return "", false } diff --git a/internal/core/memory/coverage.go b/internal/core/memory/coverage.go index 2eb3f552..f6d9fb04 100644 --- a/internal/core/memory/coverage.go +++ b/internal/core/memory/coverage.go @@ -10,6 +10,8 @@ import ( "sort" "strconv" "strings" + + "github.com/intentdriven/abcd/internal/fsutil" ) // coverage.go — quotation-budget math + the regenerable coverage index (fn-39): @@ -60,7 +62,9 @@ func memoryConfigPath(repoRoot string) string { func loadQuotationBudget(repoRoot string) quotationBudget { def := defaultBudget() - raw, err := os.ReadFile(memoryConfigPath(repoRoot)) + // Guarded read: config.json lives in the store, a trust boundary — a + // committed symlink is refused, not followed. + raw, err := fsutil.ReadGuarded(memoryConfigPath(repoRoot), maxRegistryBytes) if err != nil { return def } @@ -520,7 +524,7 @@ func buildCoverage(pages []crawledPage, registry map[string]any, budget quotatio // --------------------------------------------------------------------------- func readStoredFingerprint(path string) string { - raw, err := os.ReadFile(path) + raw, err := fsutil.ReadGuarded(path, maxRegistryBytes) if err != nil { return "" } diff --git a/internal/core/memory/guarded_reads_test.go b/internal/core/memory/guarded_reads_test.go new file mode 100644 index 00000000..99213232 --- /dev/null +++ b/internal/core/memory/guarded_reads_test.go @@ -0,0 +1,131 @@ +package memory + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// The memory store sits inside the repo working tree — a trust boundary +// (ingest.go: maxMemoryPageBytes) — so every store read must refuse a +// committed symlink rather than follow it. These tests plant mode-120000 +// leaves pointing at readable files OUTSIDE the store and assert the content +// never crosses: a follow would succeed and leak the target, so each test +// fails against a raw os.ReadFile and passes against the guarded primitive. + +func plantSymlink(t *testing.T, target, link string) { + t.Helper() + if err := os.Symlink(target, link); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } +} + +func TestBareRefusesSymlinkedStoreFiles(t *testing.T) { + root := t.TempDir() + mem := filepath.Join(root, ".abcd", "memory") + if err := os.MkdirAll(mem, 0o755); err != nil { + t.Fatal(err) + } + outside := filepath.Join(root, "outside.md") + if err := os.WriteFile(outside, []byte("- injected contradiction\n"), 0o644); err != nil { + t.Fatal(err) + } + plantSymlink(t, outside, filepath.Join(mem, "contradictions.md")) + + status, err := Bare(root) + if err != nil { + t.Fatalf("Bare: %v", err) + } + if len(status.Contradictions) != 0 { + t.Fatalf("a symlinked contradictions.md was followed into the status: %q", status.Contradictions) + } +} + +func TestLintSkipsSymlinkedTypedPage(t *testing.T) { + root := t.TempDir() + mem := filepath.Join(root, ".abcd", "memory") + if err := os.MkdirAll(mem, 0o755); err != nil { + t.Fatal(err) + } + outside := filepath.Join(root, "outside.md") + page := "---\nsource: sha256:0000\n---\n\nInjected body.\n" + if err := os.WriteFile(outside, []byte(page), 0o644); err != nil { + t.Fatal(err) + } + plantSymlink(t, outside, filepath.Join(mem, "fact_eng_injected.md")) + + if isTypedMemoryPagePath(mem, filepath.Join(mem, "fact_eng_injected.md")) { + t.Fatal("a symlinked page was followed and classified as a typed memory page") + } +} + +func TestLoadQuotationBudgetRefusesSymlinkedConfig(t *testing.T) { + root := t.TempDir() + mem := filepath.Join(root, ".abcd", "memory") + if err := os.MkdirAll(mem, 0o755); err != nil { + t.Fatal(err) + } + outside := filepath.Join(root, "outside.json") + cfg := `{"quotation_budget": {"per_page_pct": 0.99}}` + if err := os.WriteFile(outside, []byte(cfg), 0o644); err != nil { + t.Fatal(err) + } + plantSymlink(t, outside, memoryConfigPath(root)) + + got := loadQuotationBudget(root) + if got.PerPagePct == 0.99 { + t.Fatal("a symlinked config.json was followed into the quotation budget") + } + if def := defaultBudget(); got != def { + t.Fatalf("refused config must fall back to the default budget: got %+v want %+v", got, def) + } +} + +func TestTriStateReadRefusesSymlink(t *testing.T) { + dir := t.TempDir() + outside := filepath.Join(dir, "outside.md") + if err := os.WriteFile(outside, []byte("target"), 0o644); err != nil { + t.Fatal(err) + } + link := filepath.Join(dir, "page.md") + plantSymlink(t, outside, link) + + _, present, err := triStateRead(link) + if err == nil { + t.Fatalf("a symlinked page must refuse on the write path, got present=%v err=nil", present) + } + if !strings.Contains(err.Error(), "refusing to overwrite") { + t.Fatalf("want the writer-contract refusal, got: %v", err) + } + + // Absent stays the soft branch. + if _, present, err := triStateRead(filepath.Join(dir, "absent.md")); err != nil || present { + t.Fatalf("absent file must stay (\"\", false, nil), got present=%v err=%v", present, err) + } +} + +func TestLicenceProbesRefuseSymlinks(t *testing.T) { + dir := t.TempDir() + outside := filepath.Join(dir, "outside.txt") + if err := os.WriteFile(outside, []byte(`{"license": "MIT"}`), 0o644); err != nil { + t.Fatal(err) + } + src := filepath.Join(dir, "src") + if err := os.MkdirAll(src, 0o755); err != nil { + t.Fatal(err) + } + plantSymlink(t, outside, filepath.Join(src, "package.json")) + if got := manifestLicence(src); got != "" { + t.Fatalf("a symlinked package.json was followed: %q", got) + } + + outsideLic := filepath.Join(dir, "outside-licence.txt") + if err := os.WriteFile(outsideLic, []byte("SPDX-License-Identifier: MIT\n"), 0o644); err != nil { + t.Fatal(err) + } + plantSymlink(t, outsideLic, filepath.Join(src, "LICENSE")) + if got := licenceFileLicence(src); got != "" { + t.Fatalf("a symlinked LICENSE was followed: %q", got) + } +} diff --git a/internal/core/memory/lint.go b/internal/core/memory/lint.go index 90f84a2a..f29333d8 100644 --- a/internal/core/memory/lint.go +++ b/internal/core/memory/lint.go @@ -8,6 +8,8 @@ import ( "sort" "strings" "time" + + "github.com/intentdriven/abcd/internal/fsutil" ) // lint.go — the `abcd memory lint` verb (fn-39): a full-store curator @@ -88,7 +90,9 @@ func isTypedMemoryPagePath(mem, path string) bool { return false } } - raw, err := os.ReadFile(path) + // Guarded read: the store is a trust boundary, so a committed symlink + // page is refused here (O_NOFOLLOW) rather than followed unbounded. + raw, err := fsutil.ReadGuarded(path, maxMemoryPageBytes) if err != nil { return false } @@ -291,13 +295,13 @@ func runMemoryCoverageLint(repoRoot string) ([]Finding, map[string]any, error) { var pages []crawledPage err := filepath.WalkDir(mem, func(path string, d fs.DirEntry, err error) error { - if err != nil || d.IsDir() || !strings.HasSuffix(path, ".md") { + if err != nil || !d.Type().IsRegular() || !strings.HasSuffix(path, ".md") { return nil } if !isTypedMemoryPagePath(mem, path) { return nil } - raw, err := os.ReadFile(path) + raw, err := fsutil.ReadGuarded(path, maxMemoryPageBytes) if err != nil { return nil } @@ -402,7 +406,7 @@ func Lint(req LintRequest) (LintResult, error) { if fi, err := os.Stat(mem); err == nil && fi.IsDir() { var pagePaths []string err := filepath.WalkDir(mem, func(path string, d fs.DirEntry, err error) error { - if err != nil || d.IsDir() || !strings.HasSuffix(path, ".md") { + if err != nil || !d.Type().IsRegular() || !strings.HasSuffix(path, ".md") { return nil } if isTypedMemoryPagePath(mem, path) { @@ -415,7 +419,7 @@ func Lint(req LintRequest) (LintResult, error) { } sort.Strings(pagePaths) for _, path := range pagePaths { - raw, err := os.ReadFile(path) + raw, err := fsutil.ReadGuarded(path, maxMemoryPageBytes) if err != nil { continue } diff --git a/internal/core/memory/provenance.go b/internal/core/memory/provenance.go index b9fee6b4..a1dc3617 100644 --- a/internal/core/memory/provenance.go +++ b/internal/core/memory/provenance.go @@ -402,7 +402,9 @@ func contentSPDXHeader(text string) string { func manifestLicence(sourceRoot string) string { pkg := filepath.Join(sourceRoot, "package.json") - if raw, err := os.ReadFile(pkg); err == nil { + // Guarded: sourceRoot is an arbitrary ingest source, outside the trusted + // worktree — a symlinked manifest is refused. + if raw, err := fsutil.ReadGuarded(pkg, maxRegistryBytes); err == nil { var data map[string]any if json.Unmarshal(raw, &data) == nil { if lic, ok := data["license"].(string); ok && strings.TrimSpace(lic) != "" { @@ -418,7 +420,7 @@ var licenceFileNames = []string{"LICENSE", "LICENCE", "LICENSE.md", "LICENCE.md" func licenceFileLicence(sourceRoot string) string { for _, name := range licenceFileNames { path := filepath.Join(sourceRoot, name) - raw, err := os.ReadFile(path) + raw, err := fsutil.ReadGuarded(path, maxRegistryBytes) if err != nil { continue } diff --git a/internal/core/memory/writer.go b/internal/core/memory/writer.go index a789e06e..de035091 100644 --- a/internal/core/memory/writer.go +++ b/internal/core/memory/writer.go @@ -302,7 +302,9 @@ func writeStringAtomic(path, content string) error { // present-but-unreadable file -> WriterContractError (never overwrite what we // cannot read back); bytes -> (text, true, nil). func triStateRead(path string) (string, bool, error) { - raw, err := os.ReadFile(path) + // Guarded: a symlinked or oversize page on the write path is + // present-but-unreadable, never followed. + raw, err := fsutil.ReadGuarded(path, maxMemoryPageBytes) if err != nil { if os.IsNotExist(err) { return "", false, nil From b3d4c18c93fec8a766f8b9af1737c8d2f2a8d8f0 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:38:36 +0000 Subject: [PATCH 02/11] fix: issue-resolution gate fails closed when git itself fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three probes on the ledger arm swallowed git failures into a clean pass: cd "$(git rev-parse --show-toplevel)" collapses to a successful cd '' when the substitution fails under errexit; the ls-tree listing's || true turned a git failure into 'no ledger records — nothing to check', exit 0; and the shallow probe compared a failed substitution against 'true', so the exit-2 environment-fault arm was disarmed by the same fault it exists to report. Reproduced with git's dubious-ownership refusal — the reachable local form for containers, devcontainers and sudo — which turned 113-records-checked into OK exit 0, with RS003 the sole detector for resolution stamps a squash or rebase merge rewrites. The commits arm already failed closed (a bare assignment), so the divergence sat within one file, and the sibling gate check-reviews.sh ratified the rc-checked pattern for exactly this class. rc-check all four probes (the RS002 diff listing shared the || true shape), exit 2 with the git stderr surfaced, and keep the legitimate empty-ledger pass loud. Three new cases pin both fault shapes and the surviving clean pass; both fault cases fail against the old gate. Assisted-by: Claude:claude-fable-5 --- scripts/check-issue-resolution-cases.sh | 42 ++++++++++++++++ scripts/check-issue-resolution.sh | 64 +++++++++++++++++++++---- 2 files changed, 98 insertions(+), 8 deletions(-) diff --git a/scripts/check-issue-resolution-cases.sh b/scripts/check-issue-resolution-cases.sh index 2f3493f7..f7a1cdab 100755 --- a/scripts/check-issue-resolution-cases.sh +++ b/scripts/check-issue-resolution-cases.sh @@ -276,6 +276,48 @@ git -C "$d" commit -qm "chore: resolve" mkdir -p "$d/sub/deep" expect fail "$d/sub/deep" "RS003 still scans when run from a subdirectory" -- ledger HEAD +# A git probe that fails must exit 2 as an environment fault, never read as an +# empty ledger. Before the rc-check fix, `cd "$(git rev-parse --show-toplevel)"` +# collapsed to a successful `cd ""` and the ls-tree || true turned the failure +# into "no ledger records — nothing to check", exit 0 — a vacuous pass from the +# one gate that notices rewritten resolution stamps. Two shapes: no repository +# at all, and a repository git refuses to read (dubious ownership, the form a +# container/devcontainer uid split produces on a real checkout). +d="$tmproot/not-a-repo" +mkdir -p "$d" +out="$(cd "$d" && bash "$GATE" ledger HEAD 2>&1)" && rc=0 || rc=$? +if [ "$rc" -ne 2 ]; then + printf 'cases: FAIL a non-repository cwd must exit 2, got exit %d:\n%s\n' "$rc" "$out" >&2 + failures=$((failures + 1)) +else + printf 'cases: ok a failing git probe is an environment fault, not an empty ledger\n' +fi + +d="$(newrepo git-refused)" +resolve_record "$d" "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" +git -C "$d" add -A +git -C "$d" commit -qm "chore: resolve" +out="$(cd "$d" && GIT_TEST_ASSUME_DIFFERENT_OWNER=1 bash "$GATE" ledger HEAD 2>&1)" && rc=0 || rc=$? +if [ "$rc" -ne 2 ]; then + printf 'cases: FAIL a git-refused repository must exit 2, got exit %d:\n%s\n' "$rc" "$out" >&2 + failures=$((failures + 1)) +else + printf 'cases: ok a git-refused repository (dubious ownership) is an environment fault\n' +fi + +# The legitimate empty case survives the hardening: git succeeds, zero records — +# a loud "nothing to check" pass, not a fault. +d="$tmproot/empty-ledger" +mkdir -p "$d" +git -C "$d" init -q -b main +git -C "$d" config user.name t +git -C "$d" config user.email t@example.invalid +git -C "$d" config commit.gpgsign false +echo x >"$d/README.md" +git -C "$d" add -A +git -C "$d" commit -qm "baseline" +expect pass "$d" "an actually-empty ledger still passes loudly" -- ledger HEAD + if [ "$failures" -gt 0 ]; then printf 'cases: FAILED — %d case(s) did not behave\n' "$failures" >&2 exit 1 diff --git a/scripts/check-issue-resolution.sh b/scripts/check-issue-resolution.sh index 20163351..dc617f52 100755 --- a/scripts/check-issue-resolution.sh +++ b/scripts/check-issue-resolution.sh @@ -45,11 +45,24 @@ set -euo pipefail # Resolve every path from the repository root, like the sibling gate -# check-reviews.sh:11. ISSUES_DIR and the git pathspecs below are relative, and a +# check-reviews.sh. ISSUES_DIR and the git pathspecs below are relative, and a # git pathspec is matched against the current directory — so run from a # subdirectory the diff and ls-tree match nothing and the gate reports a clean # pass having scanned zero records. cd first, so cwd cannot disarm it. -cd "$(git rev-parse --show-toplevel)" +# +# Every git probe below is fail-closed: `cd "$(...)"` collapses to a successful +# `cd ""` when the substitution fails under errexit, and a swallowed git error +# reads exactly like an empty ledger — a gate that cannot tell them apart is a +# false green (git refuses a repo entirely on e.g. dubious ownership, so the +# fault is reachable from make preflight, not just a broken cwd). +rc=0 +toplevel="$(git rev-parse --show-toplevel 2>&1)" || rc=$? +if [ "$rc" -ne 0 ]; then + echo "check-issue-resolution: not a readable git repository (git rev-parse --show-toplevel exit $rc) — refusing rather than reporting a vacuous pass:" >&2 + echo "$toplevel" >&2 + exit 2 +fi +cd "$toplevel" ISSUES_DIR=".abcd/work/issues" TRAILER_RE='^Resolves:[[:space:]]+(iss-[0-9]+)[[:space:]]*$' @@ -154,8 +167,17 @@ check_commits() { # this range introduced or changed. Scanning the raw diff for `+ commit:` instead # would reachability-check a `commit:` example in a record's prose body — a false # violation — which is exactly the boundary RS003 already draws. - local changed - changed="$(git diff --name-only "$base".."$head" -- "$ISSUES_DIR" | grep -E '\.md$' || true)" + # rc-checked like the ledger listing: the || true belongs to grep's no-match + # exit alone, never to a git failure. + local diffout changed + local rc=0 + diffout="$(git diff --name-only "$base".."$head" -- "$ISSUES_DIR" 2>&1)" || rc=$? + if [ "$rc" -ne 0 ]; then + echo "check-issue-resolution: git diff failed for $base..$head (exit $rc) — refusing rather than reporting a vacuous pass:" >&2 + echo "$diffout" >&2 + exit 2 + fi + changed="$(printf '%s\n' "$diffout" | grep -E '\.md$' || true)" while IFS= read -r f; do [ -n "$f" ] || continue local head_sha base_sha @@ -179,8 +201,18 @@ check_commits() { check_ledger() { local ref="${1:-HEAD}" local checked=0 - local files - files="$(git ls-tree -r --name-only "$ref" -- "$ISSUES_DIR" | grep -E '\.md$' || true)" + # The listing probe is rc-checked so a git failure exits 2 as an environment + # fault; only a git success with zero matches is the legitimate empty-ledger + # pass, and it stays loud so it cannot be mistaken for a verdict. + local listing files + local rc=0 + listing="$(git ls-tree -r --name-only "$ref" -- "$ISSUES_DIR" 2>&1)" || rc=$? + if [ "$rc" -ne 0 ]; then + echo "check-issue-resolution: git ls-tree failed at $ref (exit $rc) — refusing rather than reporting a vacuous pass:" >&2 + echo "$listing" >&2 + exit 2 + fi + files="$(printf '%s\n' "$listing" | grep -E '\.md$' || true)" [ -n "$files" ] || { echo "check-issue-resolution: no ledger records at $ref — nothing to check" return 0 @@ -211,10 +243,26 @@ check_ledger() { # spec that ruled git resolution out of --commit (spc-25) names shallow states # in-envelope, so the environment fault must be reported as itself: exit 2, the # code the contract reserves for it, never a violation. -if [ "$(git rev-parse --is-shallow-repository 2>/dev/null)" = "true" ]; then - echo "check-issue-resolution: shallow checkout — RS002/RS003 cannot tell an absent commit from an unfetched one; run 'git fetch --unshallow' first (CI checks out with fetch-depth: 0)." >&2 +# The probe itself is rc-checked: comparing a failed substitution against +# "true" would let the very fault this arm exists to report disarm it. +rc=0 +shallow="$(git rev-parse --is-shallow-repository 2>&1)" || rc=$? +if [ "$rc" -ne 0 ]; then + echo "check-issue-resolution: git rev-parse --is-shallow-repository failed (exit $rc) — refusing rather than reporting a vacuous pass:" >&2 + echo "$shallow" >&2 exit 2 fi +case "$shallow" in +true) + echo "check-issue-resolution: shallow checkout — RS002/RS003 cannot tell an absent commit from an unfetched one; run 'git fetch --unshallow' first (CI checks out with fetch-depth: 0)." >&2 + exit 2 + ;; +false) ;; +*) + echo "check-issue-resolution: unexpected git rev-parse --is-shallow-repository output \"$shallow\" — refusing rather than guessing." >&2 + exit 2 + ;; +esac case "${1:-}" in commits) From ac06c817255f02ec43290ff9cc3d01f847ace11a Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:41:34 +0000 Subject: [PATCH 03/11] fix: lint config severities are validated and unknown keys refused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A rule whose severity was missing, misspelt, or off-enum emitted findings that printed but counted toward no exit code, so record-lint and docs lint exited 0 beside a non-empty findings list — the shape the sibling engines name a rule bug and fail closed on (repolint.Evaluate, guard.Validate on the committed guard.json, banlist.AddPublic, which refuses to write a severity this loader would happily read). The plain json.Unmarshal compounded it: a misspelt key silently zero-valued the field it missed, so "severty" stripped a rule's exit-code weight and "enabld" disarmed it entirely, both invisible in review. The config is a documented trust boundary in this same file. LoadConfig now decodes strictly (DisallowUnknownFields, the house pattern of baseline.go and its siblings) and refuses an enabled rule or banned token whose severity is outside blocker/warn; a disabled rule stays inert and unchecked. Both live configs load unchanged. Watched-fail tests cover the four refusals and both accepted shapes. Assisted-by: Claude:claude-fable-5 --- internal/core/lint/config.go | 40 +++++++++- internal/core/lint/config_severity_test.go | 86 ++++++++++++++++++++++ 2 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 internal/core/lint/config_severity_test.go diff --git a/internal/core/lint/config.go b/internal/core/lint/config.go index f5267a1b..f661ef76 100644 --- a/internal/core/lint/config.go +++ b/internal/core/lint/config.go @@ -1,6 +1,7 @@ package lint import ( + "bytes" "encoding/json" "errors" "fmt" @@ -344,16 +345,53 @@ func LoadConfig(path string) (Config, error) { return Config{}, err } } + // Strict decode: a misspelt key would silently zero-value the field it + // missed ("enabld" disarms a rule, "severty" strips its exit-code weight), + // and both misreads survive review because the file still looks armed. var cfg Config - if err := json.Unmarshal(data, &cfg); err != nil { + dec := json.NewDecoder(bytes.NewReader(data)) + dec.DisallowUnknownFields() + if err := dec.Decode(&cfg); err != nil { return Config{}, err } if err := cfg.validateBannedTokens(); err != nil { return Config{}, err } + if err := cfg.validateSeverities(); err != nil { + return Config{}, err + } return cfg, nil } +// validateSeverities refuses a severity outside the engine's enum on any +// enabled rule or banned token. The exit paths count Severity == "blocker" +// verbatim, so an off-enum value would emit findings that serialize yet count +// toward no exit code — a clean exit beside a non-empty findings list, which +// the sibling engines (repolint.Evaluate, guard.Validate, banlist.AddPublic) +// name a rule bug and fail closed on. A disabled rule is inert and its +// severity is not consulted, so it is not checked. +func (c Config) validateSeverities() error { + for id, rc := range c.Rules { + if !rc.Enabled { + continue + } + if rc.Severity != severityBlocker && rc.Severity != severityWarn { + return &configError{"rule " + id + " has severity " + strconv.Quote(rc.Severity) + "; an enabled rule must declare \"blocker\" or \"warn\", or its findings count toward no exit code"} + } + } + for i, t := range c.BannedTokens { + if t.Severity == severityBlocker || t.Severity == severityWarn { + continue + } + who := t.ID + if who == "" { + who = "index " + strconv.Itoa(i) + } + return &configError{"banned_tokens entry " + who + " has severity " + strconv.Quote(t.Severity) + "; want \"blocker\" or \"warn\""} + } + return nil +} + // validateBannedTokens enforces the strict banned_tokens schema (iss-51): every // entry must declare a non-empty successor (the machine-readable replacement, // not prose alone) and a non-empty allow_context (where the token is legitimately diff --git a/internal/core/lint/config_severity_test.go b/internal/core/lint/config_severity_test.go new file mode 100644 index 00000000..809bb800 --- /dev/null +++ b/internal/core/lint/config_severity_test.go @@ -0,0 +1,86 @@ +package lint + +import ( + "strings" + "testing" +) + +// A rule whose severity is off-enum would emit findings that serialize yet +// count toward no exit code — a clean exit beside a non-empty findings list, +// the shape the sibling engines (repolint.Evaluate, guard.Validate) fail +// closed on. The loader is the one place that can refuse it before a gate +// runs vacuously green. + +func TestLoadConfigRefusesOffEnumRuleSeverity(t *testing.T) { + path := writeConfig(t, `{ + "roots": ["rec"], + "rules": {"links_resolve": {"enabled": true, "severity": "blocking"}} + }`) + _, err := LoadConfig(path) + if err == nil { + t.Fatal("LoadConfig accepted an enabled rule with severity \"blocking\"; want rejection") + } + if !strings.Contains(err.Error(), "links_resolve") { + t.Fatalf("rejection must name the offending rule, got: %v", err) + } +} + +func TestLoadConfigRefusesEnabledRuleWithNoSeverity(t *testing.T) { + path := writeConfig(t, `{ + "roots": ["rec"], + "rules": {"links_resolve": {"enabled": true}} + }`) + if _, err := LoadConfig(path); err == nil { + t.Fatal("LoadConfig accepted an enabled rule with no severity; want rejection") + } +} + +func TestLoadConfigRefusesOffEnumTokenSeverity(t *testing.T) { + path := writeConfig(t, `{ + "roots": ["rec"], + "banned_tokens": [ + {"id":"t1","pattern":"foo","message":"no foo","severity":"Blocker","successor":"bar","allow_context":["ok"]} + ] + }`) + if _, err := LoadConfig(path); err == nil { + t.Fatal("LoadConfig accepted a banned token with severity \"Blocker\"; want rejection") + } +} + +func TestLoadConfigRefusesUnknownKeys(t *testing.T) { + // A misspelt key silently zero-values the field it missed: "enabld" leaves + // Enabled false (a rule disarmed), "severty" leaves Severity "" (findings + // that count toward no exit). Strict decoding turns both into a refusal. + path := writeConfig(t, `{ + "roots": ["rec"], + "rules": {"links_resolve": {"enabld": true, "severity": "blocker"}} + }`) + if _, err := LoadConfig(path); err == nil { + t.Fatal("LoadConfig accepted an unknown config key (\"enabld\"); want rejection") + } +} + +func TestLoadConfigAcceptsDisabledRuleWithoutSeverity(t *testing.T) { + // A disabled rule is inert; its severity is not consulted, so absence there + // is not a fault. + path := writeConfig(t, `{ + "roots": ["rec"], + "rules": {"links_resolve": {"enabled": false}} + }`) + if _, err := LoadConfig(path); err != nil { + t.Fatalf("LoadConfig refused a disabled rule with no severity: %v", err) + } +} + +func TestLoadConfigAcceptsBothLiveSeverities(t *testing.T) { + path := writeConfig(t, `{ + "roots": ["rec"], + "rules": { + "links_resolve": {"enabled": true, "severity": "blocker"}, + "no_brittle_line_refs": {"enabled": true, "severity": "warn"} + } + }`) + if _, err := LoadConfig(path); err != nil { + t.Fatalf("LoadConfig refused the live severity vocabulary: %v", err) + } +} From b215c8c66ddcf2ac195c95016d5245d8db980f8b Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:41:34 +0000 Subject: [PATCH 04/11] fix: docs lint renderer sanitises config-derived finding fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The findings renderer asserted Severity/RuleID are enum-constrained and printed both unsanitised — but neither was: Severity was verbatim committed- config text (now validated at load) and a banned token's RuleID remains free text from the same trust-boundary file, so a hostile clone's config put a raw terminal escape on every finding line. ToUpper does not neutralise OSC/CSI sequences, and RuleID got no transform at all. Sanitise all four fields and correct the comment; a watched-fail test plants an ESC in a token id and asserts it never reaches the terminal. Assisted-by: Claude:claude-fable-5 --- internal/surface/cli/cli.go | 9 ++-- internal/surface/cli/docs_lint_render_test.go | 47 +++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 internal/surface/cli/docs_lint_render_test.go diff --git a/internal/surface/cli/cli.go b/internal/surface/cli/cli.go index eff36ef4..5a36f247 100644 --- a/internal/surface/cli/cli.go +++ b/internal/surface/cli/cli.go @@ -487,10 +487,13 @@ func newDocsCommand(asJSON *bool) *cobra.Command { res := docsLintResult{Findings: findings, Blockers: blockers} if err := render(cmd.OutOrStdout(), *asJSON, res, func(w io.Writer) { for _, f := range findings { - // File and Message embed untrusted repo content (paths, link targets); - // Severity/RuleID are enum-constrained. + // Every non-numeric field embeds untrusted repo content: File and + // Message carry paths and link targets, and Severity/RuleID come + // verbatim from the committed config (LoadConfig validates rule + // severities, but a banned token's id is free text), so all four + // are sanitised. fmt.Fprintf(w, "%s:%d: [%s %s] %s\n", - termsafe.Sanitize(f.File), f.Line, strings.ToUpper(f.Severity), f.RuleID, termsafe.Sanitize(f.Message)) + termsafe.Sanitize(f.File), f.Line, termsafe.Sanitize(strings.ToUpper(f.Severity)), termsafe.Sanitize(f.RuleID), termsafe.Sanitize(f.Message)) } fmt.Fprintf(w, "abcd docs lint — %d finding(s), %d blocker(s)\n", len(findings), blockers) }); err != nil { diff --git a/internal/surface/cli/docs_lint_render_test.go b/internal/surface/cli/docs_lint_render_test.go new file mode 100644 index 00000000..728167cd --- /dev/null +++ b/internal/surface/cli/docs_lint_render_test.go @@ -0,0 +1,47 @@ +package cli + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +// TestDocsLintRenderSanitisesConfigFields pins that the findings renderer +// sanitises every config-derived field. A banned token's id is free text from +// the committed config — a trust boundary (LoadConfig's own contract) — so a +// hostile clone must not be able to put a raw terminal escape on the finding +// line through it. +func TestDocsLintRenderSanitisesConfigFields(t *testing.T) { + repo := t.TempDir() + t.Chdir(repo) + if err := os.MkdirAll(filepath.Join(repo, ".abcd"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(repo, "docs"), 0o755); err != nil { + t.Fatal(err) + } + cfg := `{ + "roots": ["docs"], + "banned_tokens": [ + {"id": "evil\u001b[2Jtoken", "pattern": "forbidden-word", "message": "no", "severity": "warn", "successor": "allowed-word", "allow_context": ["nowhere-real"]} + ] + }` + if err := os.WriteFile(filepath.Join(repo, ".abcd", "docs-lint.json"), []byte(cfg), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(repo, "docs", "page.md"), []byte("uses the forbidden-word here\n"), 0o644); err != nil { + t.Fatal(err) + } + + var stdout, stderr bytes.Buffer + Run([]string{"docs", "lint"}, &stdout, &stderr) + out := stdout.String() + stderr.String() + if !strings.Contains(out, "evil") { + t.Fatalf("expected the finding to render (config id present), got:\n%s", out) + } + if strings.ContainsRune(out, 0x1b) { + t.Fatalf("a raw ESC from the config's token id reached the terminal:\n%q", out) + } +} From 89603e7ce8f1612309801e81fde1e672a962ab84 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:42:50 +0000 Subject: [PATCH 05/11] fix: runBounded trims the trailing side only, keeping NUL-list bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shared git primitive returned strings.TrimSpace over the whole capture buffer, and the lifeboat scan NUL-splits that string from ls-files -z to build its not-ignored set. The -z form exists so whitespace in filenames cannot desync the list, but the trim stripped leading whitespace off the first entry, so a repo whose first-sorting path begins with a space had that one file silently classified ignored and dropped from the evidence walk — the quiet-evidence-loss shape the adapter's contract forbids. Trailing NULs survive (NUL is not IsSpace), so only the leading side was damaged. Trim only the trailing side; every RunLimited/RunCapped consumer parses per-line, per-field, or per-NUL and tolerates a leading space, and none may lose one. Watched-fail test pins a leading-space name in first position. Assisted-by: Claude:claude-fable-5 --- internal/gitutil/nulsafe_test.go | 48 ++++++++++++++++++++++++++++++++ internal/gitutil/repo.go | 8 +++++- 2 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 internal/gitutil/nulsafe_test.go diff --git a/internal/gitutil/nulsafe_test.go b/internal/gitutil/nulsafe_test.go new file mode 100644 index 00000000..4fff4188 --- /dev/null +++ b/internal/gitutil/nulsafe_test.go @@ -0,0 +1,48 @@ +package gitutil_test + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/intentdriven/abcd/internal/gitutil" +) + +// TestRunCappedPreservesLeadingWhitespaceInNulLists pins the byte fidelity a +// NUL-separated listing depends on. The -z form exists so whitespace in +// filenames cannot desync the list, but a whole-buffer TrimSpace strips +// leading whitespace off the FIRST entry — so a repo whose first-sorting path +// begins with a space loses that one file from any set keyed on the returned +// names (the lifeboat not-ignored set silently classified it ignored). +// Trailing NULs are not IsSpace, so only the leading side was damaged. +func TestRunCappedPreservesLeadingWhitespaceInNulLists(t *testing.T) { + repo := newRepo(t, "") + // A leading-space name sorts before every printable-range name, so it is + // the first ls-files entry — the position the trim corrupted. + leading := " lead.txt" + if err := os.WriteFile(filepath.Join(repo, leading), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(repo, "z.txt"), []byte("y"), 0o644); err != nil { + t.Fatal(err) + } + commitAll(t, repo) + + out, err := gitutil.RunCapped(repo, 1<<20, "ls-files", "--cached", "-z") + if err != nil { + t.Fatal(err) + } + set := map[string]bool{} + for _, f := range strings.Split(out, "\x00") { + if f != "" { + set[f] = true + } + } + if !set[leading] { + t.Fatalf("the first NUL-list entry lost its leading space: %q", out) + } + if !set["z.txt"] { + t.Fatalf("z.txt missing from the listing: %q", out) + } +} diff --git a/internal/gitutil/repo.go b/internal/gitutil/repo.go index 3136cd82..976133e0 100644 --- a/internal/gitutil/repo.go +++ b/internal/gitutil/repo.go @@ -291,5 +291,11 @@ func runBounded(root string, maxBytes int, args ...string) (string, bool, error) if err := cmd.Run(); err != nil { return "", w.overflowed, fmt.Errorf("%w (stderr: %q)", err, strings.TrimSpace(string(e.buf))) } - return strings.TrimSpace(string(w.buf)), w.overflowed, nil + // Trim the trailing side only. Leading bytes are content: a NUL-separated + // listing (-z) starts with its first entry, and a whole-buffer TrimSpace + // silently stripped leading whitespace off that entry's filename — the -z + // form exists precisely so such names survive. Every consumer parses + // per-line, per-field, or per-NUL and tolerates a leading space; none may + // lose one. + return strings.TrimRight(string(w.buf), " \t\r\n"), w.overflowed, nil } From e6f90055be4d4acb89012abe29c1fc5beb70acf5 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:45:52 +0000 Subject: [PATCH 06/11] fix: privacy-hygiene warns on a tracked file it cannot open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit readTrackedFile returned a bare not-ok on an open failure, and the caller warned only on the oversize arm — so a tracked file the scan could not read (EACCES in a shared or container checkout, an I/O fault) was silently skipped and the rule reported the repository clean, against the engine contract that a check that cannot run must not be silently reported as passing. The size branch got exactly this fix (iss-356 item 4); the open branch beside it stayed silent. Surface the open error and warn on the permission/I-O class only: an absent path (deleted in the worktree, sparse checkout) and the symlink-shaped refusals (tracked link leaf, os.Root containment) are legitimate states the scan skips by design, pinned by the existing symlink tests. A polarity table pins the classification; the end-to-end warn test stages a mode-000 tracked file and skips under euid 0, where permission bits do not bind (CI runs unprivileged, so it exercises there). Assisted-by: Claude:claude-fable-5 --- internal/core/repolint/rule_privacy.go | 44 ++++++++++++++++--- .../rule_privacy_cap_internal_test.go | 4 +- .../rule_privacy_openfail_internal_test.go | 37 ++++++++++++++++ .../repolint/rule_privacy_unreadable_test.go | 38 ++++++++++++++++ 4 files changed, 114 insertions(+), 9 deletions(-) create mode 100644 internal/core/repolint/rule_privacy_openfail_internal_test.go create mode 100644 internal/core/repolint/rule_privacy_unreadable_test.go diff --git a/internal/core/repolint/rule_privacy.go b/internal/core/repolint/rule_privacy.go index b6e95403..2d4b211a 100644 --- a/internal/core/repolint/rule_privacy.go +++ b/internal/core/repolint/rule_privacy.go @@ -2,12 +2,14 @@ package repolint import ( "bytes" + "errors" "fmt" "io" "os" "path/filepath" "regexp" "strings" + "syscall" "github.com/intentdriven/abcd/internal/adapter/scanner" "github.com/intentdriven/abcd/internal/gitutil" @@ -133,13 +135,18 @@ func (privacyHygiene) Eval(ctx Context) ([]Finding, error) { } for _, rel := range tracked { - data, ok, oversizeText := readTrackedFile(root, filepath.FromSlash(rel)) + data, ok, oversizeText, openErr := readTrackedFile(root, filepath.FromSlash(rel)) if !ok { // A textual file over the scan cap was NOT scanned, and silence // here would report "conforms" for content nobody looked at — // the didn't-scan-reported-clean shape the engine contract // forbids (iss-356 item 4). Binary blobs stay quiet: the scan // would skip them anyway, so the cap loses nothing there. + // The open branch gets the same treatment (the size branch's fix + // stopped one arm short): a tracked file the scan cannot OPEN is + // content nobody looked at, so it warns — except the absent and + // tracked-symlink shapes, which are legitimate worktree states + // the scan skips by design. if oversizeText { out = append(out, Finding{ RuleID: "privacy-hygiene", @@ -148,6 +155,13 @@ func (privacyHygiene) Eval(ctx Context) ([]Finding, error) { Message: fmt.Sprintf("not scanned: over the %d MiB privacy-scan cap; split the file or verify it by hand", maxScanBytes>>20), }) + } else if openFailureWarrantsWarn(openErr) { + out = append(out, Finding{ + RuleID: "privacy-hygiene", + Severity: SeverityWarn, + File: rel, + Message: "not scanned: the tracked file could not be opened; fix its permissions or verify it by hand", + }) } continue } @@ -331,16 +345,31 @@ func isUsersRoot(m string) bool { // a regular file, or exceeds the cap is skipped (ok=false), not a scan failure; // oversizeText additionally reports that a skipped file is over the cap yet // looks textual, so the caller can say "not scanned" instead of staying silent. -func readTrackedFile(root *os.Root, rel string) (data []byte, ok, oversizeText bool) { +// openFailureWarrantsWarn parts the open failures that are legitimate worktree +// states from the ones that mean unscanned content. TrackedFiles lists INDEX +// entries, so an absent path (deleted in the worktree but not yet committed, a +// sparse checkout) is a normal state the scan has nothing to read for, and a +// symlink-shaped refusal (a tracked link leaf under O_NOFOLLOW, an os.Root +// containment refusal) is the scan's own skip-by-design, pinned by the +// symlink tests. A permission or I/O fault is different in kind: the content +// exists, nobody looked at it, and silence would read as "conforms" — the +// same not-scanned shape the oversize arm already warns on (iss-356 item 4). +func openFailureWarrantsWarn(err error) bool { + return errors.Is(err, syscall.EACCES) || errors.Is(err, syscall.EPERM) || errors.Is(err, syscall.EIO) +} + +func readTrackedFile(root *os.Root, rel string) (data []byte, ok, oversizeText bool, openErr error) { f, err := root.OpenFile(rel, os.O_RDONLY|syscallNoFollow, 0) if err != nil { - return nil, false, false // escapes the root, missing, or unreadable + // The caller decides whether the failure is warn-worthy + // (openFailureWarrantsWarn); reporting it is not this helper's call. + return nil, false, false, err } defer f.Close() info, err := f.Stat() if err != nil || !info.Mode().IsRegular() { - return nil, false, false // FIFO, device, directory, or vanished + return nil, false, false, nil // FIFO, device, directory, or vanished } if info.Size() > maxScanBytes { // Not scanned — but say whether it LOOKS like prose, so the caller @@ -352,11 +381,12 @@ func readTrackedFile(root *os.Root, rel string) (data []byte, ok, oversizeText b // The probe itself failed, so nothing about the file is known — // warn rather than stay silent (the not-scanned shape again). _ = err - return nil, false, true + return nil, false, true, nil } - return nil, false, !isBinary(probe[:n]) + return nil, false, !isBinary(probe[:n]), nil } - return capRead(f) + data, ok, oversizeText = capRead(f) + return data, ok, oversizeText, nil } // capRead reads everything the scan may see from an already-vetted regular diff --git a/internal/core/repolint/rule_privacy_cap_internal_test.go b/internal/core/repolint/rule_privacy_cap_internal_test.go index 32c07d25..05df6ec2 100644 --- a/internal/core/repolint/rule_privacy_cap_internal_test.go +++ b/internal/core/repolint/rule_privacy_cap_internal_test.go @@ -38,7 +38,7 @@ func TestReadTrackedFileCapBoundary(t *testing.T) { if err := os.WriteFile(atCap, buf, 0o644); err != nil { t.Fatal(err) } - data, ok, oversize := readTrackedFile(root, "at-cap.txt") + data, ok, oversize, _ := readTrackedFile(root, "at-cap.txt") if !ok { t.Fatalf("a file exactly at the cap must scan whole; ok=false oversize=%v", oversize) } @@ -55,7 +55,7 @@ func TestReadTrackedFileCapBoundary(t *testing.T) { if err := os.WriteFile(overCap, append(buf, 'z'), 0o644); err != nil { t.Fatal(err) } - data, ok, oversize = readTrackedFile(root, "over-cap.txt") + data, ok, oversize, _ = readTrackedFile(root, "over-cap.txt") if ok || data != nil { t.Errorf("an over-cap file must not be scanned; ok=%v len=%d", ok, len(data)) } diff --git a/internal/core/repolint/rule_privacy_openfail_internal_test.go b/internal/core/repolint/rule_privacy_openfail_internal_test.go new file mode 100644 index 00000000..cf0ab7ff --- /dev/null +++ b/internal/core/repolint/rule_privacy_openfail_internal_test.go @@ -0,0 +1,37 @@ +package repolint + +import ( + "errors" + "io/fs" + "syscall" + "testing" +) + +// The open-failure classification is the seam between "legitimate worktree +// state, skip silently" and "content nobody looked at, warn" (the engine +// contract: a check that cannot run must not be silently reported as +// passing). The polarity table is pinned here so a future edit cannot +// silently widen either side. +func TestOpenFailureWarrantsWarnPolarity(t *testing.T) { + pe := func(errno syscall.Errno) error { + return &fs.PathError{Op: "openat", Path: "x", Err: errno} + } + warns := []error{pe(syscall.EACCES), pe(syscall.EPERM), pe(syscall.EIO)} + for _, err := range warns { + if !openFailureWarrantsWarn(err) { + t.Errorf("%v must warn: the file exists and was not scanned", err) + } + } + silents := []error{ + nil, + pe(syscall.ENOENT), // deleted in the worktree, sparse checkout + pe(syscall.ENOTDIR), // parent replaced by a file — path is absent + pe(syscall.ELOOP), // tracked symlink leaf under O_NOFOLLOW — skip by design + errors.New("openat sub: path escapes from parent"), // os.Root containment refusal + } + for _, err := range silents { + if openFailureWarrantsWarn(err) { + t.Errorf("%v must stay silent: a legitimate worktree state, not unscanned content", err) + } + } +} diff --git a/internal/core/repolint/rule_privacy_unreadable_test.go b/internal/core/repolint/rule_privacy_unreadable_test.go new file mode 100644 index 00000000..c8c0c1b4 --- /dev/null +++ b/internal/core/repolint/rule_privacy_unreadable_test.go @@ -0,0 +1,38 @@ +package repolint_test + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// A tracked file the scan cannot OPEN is content nobody looked at, so it must +// warn "not scanned" — never silently count as conforming (the engine +// contract, and the same fix the oversize arm got as iss-356 item 4). Runs +// only where permissions bind: root bypasses mode bits, so under euid 0 (a +// container) the EACCES this stages cannot occur. +func TestRule_PrivacyWarnsOnUnreadableTrackedFile(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("permission bits do not bind under euid 0; the EACCES cannot be staged") + } + b := newFixtureRepo(t).conforming() + locked := filepath.Join(b.root, "locked.md") + if err := os.WriteFile(locked, []byte("ordinary content\n"), 0o644); err != nil { + t.Fatal(err) + } + b.commit() + if err := os.Chmod(locked, 0o000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(locked, 0o644) }) + + res := b.run() + f := findingFor(res, "privacy-hygiene") + if f == nil { + t.Fatal("an unreadable tracked file was silently reported as conforming; want a not-scanned warn") + } + if f.File != "locked.md" || !strings.Contains(f.Message, "not scanned") { + t.Fatalf("want a not-scanned warn for locked.md, got: %+v", f) + } +} From 6d9a7b24168bdb6f4e16d205f9245569f011efd3 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:46:33 +0000 Subject: [PATCH 07/11] docs: the sources corpus is a script MVP, not shipped behaviour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The RAG row claimed the corpus ships the script-first version, on a page whose sibling rows use ship as a precise delivery marker. Nothing corpus-related exists in the repository or any released artefact — the consult and ingest commands refuse when the corpus is absent — and the script-first-mvp principle names this tooling as its live instance of the rule that a script MVP never ships as product behaviour. State it as the user-tier MVP it is, with core absorption cited (iss-27). Assisted-by: Claude:claude-fable-5 --- docs/reference/terminology.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/terminology.md b/docs/reference/terminology.md index 0b6a5f3c..4f23bd2d 100644 --- a/docs/reference/terminology.md +++ b/docs/reference/terminology.md @@ -62,7 +62,7 @@ deep-linked. | **Orchestration** | Coordinating which agents run, in what order, and how control and results flow between them.[^orch] | **ADAPTS** — abcd is *host-delegated* (adr-25): the deterministic core prepares work and hands a prompt to the host's own dispatch. abcd owns the prompt; the host owns models, credentials, and execution — the orchestration-substrate role is deliberately declined. | | **Policy-as-code** | Expressing enforcement policy in declarative, machine-evaluated form, decoupling policy decisions from application logic; the policy *engine* is the component that decides grant or deny.[^pac] | **USES** — abcd's policy is committed JSON configuration: per-repo rules, docs-lint, record-lint, and backend-seam selection, evaluated deterministically in the core. The record practises this without using the phrase; this page is where the mapping is made. | | **Prompt injection** | A vulnerability where user prompts or ingested content alter an LLM's behaviour in unintended ways — directly or indirectly (OWASP LLM01:2025).[^pi] | **USES** — recorded defences on both sides of the boundary: agents that read untrusted input must carry injection-canary fixtures (the itd-5 discipline), and a verb that mutates state fails closed on anything not exactly recognised. The canary lint (reserved code PQ006) and automated canary execution are recorded design targets, not yet shipped. | -| **Retrieval-augmented generation (RAG)** | Combining a generator with a retriever over an external index, so generation draws on non-parametric knowledge.[^rag] | **ADAPTS** — the sources corpus ships the script-first version: per-source folders, extracted text, grep-based consult. Retrieval is recorded as a pluggable seam (iss-26) where a RAG backend is one opt-in adapter, never the default. | +| **Retrieval-augmented generation (RAG)** | Combining a generator with a retriever over an external index, so generation draws on non-parametric knowledge.[^rag] | **ADAPTS** — the sources corpus takes the script-first shape: per-source folders, extracted text, grep-based consult. It is a user-tier script MVP, not shipped product behaviour (absorption into the core is tracked as iss-27); retrieval is recorded as a pluggable seam (iss-26) where a RAG backend is one opt-in adapter, never the default. | | **Sandboxing** | An OS-enforced boundary restricting an agent's filesystem and network access, so it can act autonomously without unrestricted host access.[^sand] | **ADAPTS** — abcd's containment is structural rather than OS-level: read-only probes proven by before-and-after tree hashes, a destination safety gate that refuses directories abcd did not produce, and refusals that write nothing. OS-level sandboxes belong to the host. | | **Tamper-evidence** | Append-only log integrity via Merkle trees, so any instance of a log can be proven a superset of any earlier instance.[^tamper] | **WATCHING** — receipts are hash-anchored and manifests verified today; a compliance-grade hash chain over conversation and edit history is a draft (itd-16), and tamper-evident receipts are tracked as iss-141. | | **Tool use** | A model emits structured, schema-conformant calls to declared functions; the application executes them and returns results ("function calling" in some vendors' vocabulary).[^tooluse] | **ADAPTS** — abcd sits on the other side of the mechanism: it *is* the tool. A single binary of verbs over a transport-agnostic core that returns structured results and knows nothing about who called it (adr-23); thin front doors render those results per surface. | From 7e2740029ff4de18cfe6d5f6961dcf99ca44c264 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:46:33 +0000 Subject: [PATCH 08/11] docs: the intake container recipe can actually build the module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit S4 pinned golang:1.25 while go.mod declares go 1.26.7 — the pin matched go.mod when written and the toolchain bump swept every workflow pin but not this file — and an older container refuses a newer go directive with --network=none blocking the GOTOOLCHAIN rescue, so the one command the protocol hands a maintainer failed at the toolchain check and S5's tri-state read the stale pin as an inconclusive contribution. The recipe also carried no module provision, so it failed on dependency fetch regardless of tag. Pin the image to the module toolchain, pre-fetch modules, and mount the module cache read-only before the network is cut. Assisted-by: Claude:claude-fable-5 --- .abcd/work/intake.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.abcd/work/intake.md b/.abcd/work/intake.md index 35cacf3d..2f607101 100644 --- a/.abcd/work/intake.md +++ b/.abcd/work/intake.md @@ -36,7 +36,13 @@ code-owner path, and S1 says so explicitly. ## S4 — review runs contained, never bare ```sh -docker run --rm --network=none -v "$PWD:/src" -w /src golang:1.25 +# Provision modules first: the module cache mount is what lets the build work +# once the network is cut, and the image tag MUST track go.mod's toolchain +# (an older container refuses a newer `go` directive, and --network=none +# blocks the GOTOOLCHAIN auto rescue). Bump the tag with go.mod. +go mod download +docker run --rm --network=none -v "$PWD:/src" \ + -v "$(go env GOMODCACHE):/go/pkg/mod:ro" -w /src golang:1.26.7 ``` `go test` executes contributor Go via `init()` and `TestMain`; a bare run on a From 0458d58674bf924eea4b938e17dd01ce6afc69b8 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:46:33 +0000 Subject: [PATCH 09/11] docs: site command description discloses check's render write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The frontmatter description enumerated write posture for the bare and build forms only; check — added later, with the argument-hint updated in the same diff but the description left behind — renders the whole site when the output directory has no index.html. The body says so; the summary now does too, matching the house style of naming every form's posture with the surprising one explicit. Assisted-by: Claude:claude-fable-5 --- commands/site.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands/site.md b/commands/site.md index 744b2f12..e7da7da4 100644 --- a/commands/site.md +++ b/commands/site.md @@ -1,6 +1,6 @@ --- name: site -description: Render this repository's website — the landing page composed from repository text under the single-source rule, and the record export derived from the record, git history and the changelog — by invoking the abcd binary. The bare form performs zero writes; build writes only inside its output directory. +description: Render this repository's website — the landing page composed from repository text under the single-source rule, and the record export derived from the record, git history and the changelog — by invoking the abcd binary. The bare form performs zero writes; build and check write only inside the output directory (check renders the site first when the directory has no index.html). argument-hint: "[build|check]" --- From 99d82b63e574a103a7b57165927b66618de62699 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:47:14 +0000 Subject: [PATCH 10/11] chore: resolve round-9 bug-hunt records and log the round Nine records move open/ -> resolved/ with resolved_by provenance naming the fixing commits; the scanIntentTree alignment observation stays open by design. The round is logged in DECISIONS.md. Resolves: iss-2608261532379188 Resolves: iss-2608261532488176 Resolves: iss-2608261533033894 Resolves: iss-2608261533033587 Resolves: iss-2608261533297309 Resolves: iss-2608261533290815 Resolves: iss-2608261533174500 Resolves: iss-2608261533173466 Resolves: iss-2608261533419396 Assisted-by: Claude:claude-fable-5 --- .abcd/work/DECISIONS.md | 25 +++++++++++++++++++ ...low-alignment-with-fail-closed-doctrine.md | 12 +++++++++ ...ers-follow-committed-symlinks-unguarded.md | 16 ++++++++++++ ...n-gate-git-probe-failures-read-as-clean.md | 16 ++++++++++++ ...r-prints-config-severity-and-ruleid-raw.md | 16 ++++++++++++ ...validated-findings-count-toward-no-exit.md | 16 ++++++++++++ ...ow-module-toolchain-and-no-module-cache.md | 16 ++++++++++++ ...rag-row-claims-the-sources-corpus-ships.md | 16 ++++++++++++ ...silently-skips-unreadable-tracked-files.md | 16 ++++++++++++ ...space-corrupts-first-entry-of-nul-lists.md | 16 ++++++++++++ ...d-description-omits-checks-render-write.md | 16 ++++++++++++ 11 files changed, 181 insertions(+) create mode 100644 .abcd/work/issues/open/iss-2608261533419897-scanintenttree-stat-swallow-alignment-with-fail-closed-doctrine.md create mode 100644 .abcd/work/issues/resolved/iss-2608261532379188-memory-store-readers-follow-committed-symlinks-unguarded.md create mode 100644 .abcd/work/issues/resolved/iss-2608261532488176-issue-resolution-gate-git-probe-failures-read-as-clean.md create mode 100644 .abcd/work/issues/resolved/iss-2608261533033587-docs-lint-renderer-prints-config-severity-and-ruleid-raw.md create mode 100644 .abcd/work/issues/resolved/iss-2608261533033894-lint-config-severity-unvalidated-findings-count-toward-no-exit.md create mode 100644 .abcd/work/issues/resolved/iss-2608261533173466-intake-s4-container-pin-below-module-toolchain-and-no-module-cache.md create mode 100644 .abcd/work/issues/resolved/iss-2608261533174500-terminology-rag-row-claims-the-sources-corpus-ships.md create mode 100644 .abcd/work/issues/resolved/iss-2608261533290815-privacy-hygiene-silently-skips-unreadable-tracked-files.md create mode 100644 .abcd/work/issues/resolved/iss-2608261533297309-runbounded-trimspace-corrupts-first-entry-of-nul-lists.md create mode 100644 .abcd/work/issues/resolved/iss-2608261533419396-site-command-description-omits-checks-render-write.md diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 74a5fea6..d2a9e931 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -2109,3 +2109,28 @@ parallel-agent merge contention bites. pronouns in two planned intents. Recorded open: the memory store-lock S_IFMT mask (rides iss-129), the quoted-enum impact split, and a deferral-currency detector seed. +- 2026-08-26 — Bug-hunt round 9 (bughunt-a): baseline green after unshallowing + the environment's clone. Five parallel hunters returned 21 candidates across + the four dimensions; per-finding adversarial refutation confirmed 5 + substantive and 4 fixable nitpicks and refuted or deferred 12. Fixed and + resolved with provenance: the memory-store guarded-read sweep (every reader + outside ingest followed a committed symlink unbounded — five reproduced + hangs), the issue-resolution gate's swallowed git probes (a dubious-ownership + refusal read as an empty ledger, exit 0), lint-config severity validation + with strict decoding (off-enum severities counted toward no exit code), + the docs-lint renderer's unsanitised config-derived fields, runBounded's + whole-buffer trim corrupting the first NUL-list entry, privacy-hygiene's + silent skip of unreadable tracked files, the terminology page's corpus + "ships" overclaim, the intake S4 container pin below the module toolchain, + and the site command description's missing check-write disclosure. Recorded + open: the scanIntentTree ENOENT-parting alignment (all claimed triggers + proved closed upstream). Refuted with prior art: the spc-28 planned/drafts + sweep (iss-94 convention), the verification-matrix capture-promote row + (iss-2608231346137587 embargo), the attribution fence guard (iss-270 + wontfix; zero verdict flips across all historical PR bodies), isHexSHA + 40-hex (prior round's refutation stands), frontmatterOpen's comment latch, + the skew-meta precedence claim, guard check's exit-2-on-disabled (specified), + the prepare-this-repo bucket gloss, the MADR label, the itd-27 pronoun + candidate (a cited real person is outside the persona rule), and the + record-lint job-name mismatch (mirror is correct by construction). + nitpicks-only: no. diff --git a/.abcd/work/issues/open/iss-2608261533419897-scanintenttree-stat-swallow-alignment-with-fail-closed-doctrine.md b/.abcd/work/issues/open/iss-2608261533419897-scanintenttree-stat-swallow-alignment-with-fail-closed-doctrine.md new file mode 100644 index 00000000..5e0443bd --- /dev/null +++ b/.abcd/work/issues/open/iss-2608261533419897-scanintenttree-stat-swallow-alignment-with-fail-closed-doctrine.md @@ -0,0 +1,12 @@ +--- +schema_version: 1 +id: "iss-2608261533419897" +slug: "scanintenttree-stat-swallow-alignment-with-fail-closed-doctrine" +severity: "nitpick" +category: "observation" +source: "agent-observation" +found_during: "bughunt-a round 9" +found_at: "internal/core/lint/lint.go" +--- + +scanIntentTree and the two spec-store stat probes in internal/core/lint swallow every stat error as tree-absent, unlike scanIssueLedger and scanRecordStores which part ENOENT from real faults — the doctrine the round-9 ScanSpecLinks fix states as a tree that is present but cannot be read IS a fault. No leg is currently reachable past markdownFiles, os.ReadDir, and the armed delivery_state floor (adjudicated: the claimed vacuous-blocker triggers are all closed one line later or upstream), so this is a consistency alignment, not a live defect: part ENOENT from other errors at the three sites and stop discarding WalkDir errors, matching the sibling scanners. Recorded for a scoped consolidation rather than fixed mid-hunt. \ No newline at end of file diff --git a/.abcd/work/issues/resolved/iss-2608261532379188-memory-store-readers-follow-committed-symlinks-unguarded.md b/.abcd/work/issues/resolved/iss-2608261532379188-memory-store-readers-follow-committed-symlinks-unguarded.md new file mode 100644 index 00000000..75b7f5f7 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2608261532379188-memory-store-readers-follow-committed-symlinks-unguarded.md @@ -0,0 +1,16 @@ +--- +schema_version: 1 +id: "iss-2608261532379188" +slug: "memory-store-readers-follow-committed-symlinks-unguarded" +severity: "major" +category: "security" +source: "agent-observation" +found_during: "bughunt-a round 9" +found_at: "internal/core/memory/bare.go" +resolution: "Every memory-store and ingest-source read routes through fsutil.ReadGuarded with in-package caps; WalkDir crawls skip non-regular entries; watched-fail symlink tests on bare, lint, budget, writer and licence paths" +impact: fix +resolved_by: + commit: "9e8f3235" +--- + +Every memory-store reader outside ingest follows a committed symlink unbounded. internal/core/memory/ingest.go adjudicates the store as a trust boundary (maxMemoryPageBytes: a committed page symlink to /dev/zero would hang or OOM the CLI) and routes its own reads through fsutil.ReadGuarded, but the sibling readers kept raw os.ReadFile: readOrEmpty (bare.go, reached with no type check for index.md/contradictions.md), bareHeadroomLines and the lint WalkDir sweeps (lint.go — WalkDir yields symlinks as non-dir entries and the read follows them), loadQuotationBudget (coverage.go reading config.json), the .coverage_index.json reads (bare.go, coverage.go — its literal sibling .sources_index.json is guarded), triStateRead (writer.go), and the ingest licence probes (provenance.go manifestLicence/licenceFileLicence under an arbitrary sourceRoot). Reproduced: five distinct hangs via committed mode-120000 fixtures against abcd memory and abcd memory lint. The ReadDir-filtered sites (ask.go QueryPages, barePageInfos) carry the ReadDir-to-open swap window and no size cap — the exact TOCTOU ingest.go documents closing. Same class as the resolved lint.LoadConfig unguarded read (major/security). Detector: watched-fail tests planting a symlinked page/index; acceptance: every memory-store and source read routes through the guarded primitive. \ No newline at end of file diff --git a/.abcd/work/issues/resolved/iss-2608261532488176-issue-resolution-gate-git-probe-failures-read-as-clean.md b/.abcd/work/issues/resolved/iss-2608261532488176-issue-resolution-gate-git-probe-failures-read-as-clean.md new file mode 100644 index 00000000..4b04b023 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2608261532488176-issue-resolution-gate-git-probe-failures-read-as-clean.md @@ -0,0 +1,16 @@ +--- +schema_version: 1 +id: "iss-2608261532488176" +slug: "issue-resolution-gate-git-probe-failures-read-as-clean" +severity: "major" +category: "bug" +source: "agent-observation" +found_during: "bughunt-a round 9" +found_at: "scripts/check-issue-resolution.sh" +resolution: "All four git probes on the gate rc-check and exit 2 with stderr surfaced; empty-ledger pass stays loud; cases pin both fault shapes and the clean pass" +impact: internal +resolved_by: + commit: "b3d4c18c" +--- + +The issue-resolution gate reports OK having scanned zero records whenever a git probe fails. scripts/check-issue-resolution.sh ledger arm: cd "$(git rev-parse --show-toplevel)" collapses to cd '' (rc 0) when the substitution fails under set -e; the ls-tree listing carries || true so a git failure reads as an empty ledger and check_ledger returns 0 with 'no ledger records — nothing to check'; and the is-shallow probe compares a failed substitution against 'true', so the exit-2 environment-fault arm is disarmed by the same fault it exists to report. Reproduced: GIT_TEST_ASSUME_DIFFERENT_OWNER=1 (git's dubious-ownership refusal, the reachable local form for containers/sudo/devcontainers) turns 113-records-checked into OK exit 0; from a non-git cwd likewise. The commits arm fails closed (bare assignment) — divergence within one file, and RS003 is the sole detector for resolved_by.commit shas rewritten by the squash/rebase merges the repo permits. Same class as the round-9 check-reviews.sh fail-closed rewrite (every git probe rc-checked, exit 2), which left this sibling untouched. Detector: cases asserting exit 2 under a failing git; acceptance: every git probe on the ledger arm is rc-checked and a git fault exits 2, with the legitimate empty-ledger pass kept loud. \ No newline at end of file diff --git a/.abcd/work/issues/resolved/iss-2608261533033587-docs-lint-renderer-prints-config-severity-and-ruleid-raw.md b/.abcd/work/issues/resolved/iss-2608261533033587-docs-lint-renderer-prints-config-severity-and-ruleid-raw.md new file mode 100644 index 00000000..f9a9bfca --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2608261533033587-docs-lint-renderer-prints-config-severity-and-ruleid-raw.md @@ -0,0 +1,16 @@ +--- +schema_version: 1 +id: "iss-2608261533033587" +slug: "docs-lint-renderer-prints-config-severity-and-ruleid-raw" +severity: "nitpick" +category: "security" +source: "agent-observation" +found_during: "bughunt-a round 9" +found_at: "internal/surface/cli/cli.go" +resolution: "The renderer sanitises Severity and RuleID alongside File and Message and the enum-constrained comment is corrected; watched-fail ESC-in-token-id test" +impact: fix +resolved_by: + commit: "b215c8c6" +--- + +docs lint's findings renderer asserts 'Severity/RuleID are enum-constrained' and prints both unsanitised, but neither is constrained: Severity is verbatim committed-config text and RuleID for the banned_tokens family is the token's configured id. The config file is adjudicated a trust boundary in internal/core/lint/config.go (cross-repo-clonable), and the same surface sanitises the enum-validated banlist severity — so the one unvalidated pair on the line is the unsanitised one. ToUpper does not neutralise OSC/CSI escapes (verified), and RuleID gets no ToUpper at all. Sibling scope: the recorded cmd/record-lint File/Message gap asserts the abcd CLI renderer sanitises — true for File/Message only. Acceptance: the renderer sanitises every config-derived field or the loader makes the comment true by validation. \ No newline at end of file diff --git a/.abcd/work/issues/resolved/iss-2608261533033894-lint-config-severity-unvalidated-findings-count-toward-no-exit.md b/.abcd/work/issues/resolved/iss-2608261533033894-lint-config-severity-unvalidated-findings-count-toward-no-exit.md new file mode 100644 index 00000000..b1193c84 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2608261533033894-lint-config-severity-unvalidated-findings-count-toward-no-exit.md @@ -0,0 +1,16 @@ +--- +schema_version: 1 +id: "iss-2608261533033894" +slug: "lint-config-severity-unvalidated-findings-count-toward-no-exit" +severity: "minor" +category: "bug" +source: "agent-observation" +found_during: "bughunt-a round 9" +found_at: "internal/core/lint/config.go" +resolution: "LoadConfig decodes strictly (DisallowUnknownFields) and refuses an enabled rule or token with an off-enum severity; both live configs load unchanged" +impact: fix +resolved_by: + commit: "ac06c817" +--- + +A record-lint or docs-lint rule whose severity is missing, misspelt, or off-enum emits findings that print but count toward no exit code, so the gate exits 0 beside a non-empty findings list. internal/core/lint/config.go decodes with a plain json.Unmarshal (a misspelt key silently zero-values the field) and validates only banned-token successors; the exit paths in cmd/record-lint and abcd docs lint count Severity == blocker verbatim. The sibling engines fail closed on exactly this vocabulary (repolint Evaluate refuses a finding severity outside error/warn as a rule bug; guard Validate rejects an unknown tier on the committed guard.json; banlist AddPublic refuses to write a severity lint will happily read). The config is a documented trust boundary in this very file. The same underlying cause silently disarms a rule via a misspelt enabled key. All 27 live rules spell both correctly today — latent, one character from silent. Detector: watched-fail test loading a config with an off-enum severity; acceptance: LoadConfig refuses an enabled rule or token whose severity is outside the engine's enum, and unknown config keys are refused. \ No newline at end of file diff --git a/.abcd/work/issues/resolved/iss-2608261533173466-intake-s4-container-pin-below-module-toolchain-and-no-module-cache.md b/.abcd/work/issues/resolved/iss-2608261533173466-intake-s4-container-pin-below-module-toolchain-and-no-module-cache.md new file mode 100644 index 00000000..bb159c5e --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2608261533173466-intake-s4-container-pin-below-module-toolchain-and-no-module-cache.md @@ -0,0 +1,16 @@ +--- +schema_version: 1 +id: "iss-2608261533173466" +slug: "intake-s4-container-pin-below-module-toolchain-and-no-module-cache" +severity: "minor" +category: "documentation" +source: "agent-observation" +found_during: "bughunt-a round 9" +found_at: ".abcd/work/intake.md" +resolution: "S4 pins the image to the module toolchain, pre-fetches modules and mounts the module cache read-only before the network is cut" +impact: internal +resolved_by: + commit: "7e274002" +--- + +The intake protocol's contained-review stage pins a container that cannot build the module. The S4 recipe pins golang:1.25 while go.mod declares a higher toolchain, and --network=none in the same line blocks the GOTOOLCHAIN auto rescue, so the one command the protocol hands a maintainer fails at the toolchain check and S5's build-or-INCONCLUSIVE tri-state misreads the stale pin as an inconclusive contribution. Drift, not born broken: the pin matched go.mod when written; the toolchain bump that advertised lockstep did not include this file, and the work tier sits outside every lint-configured tree. The recipe also lacks any module-cache provision, so even a corrected tag fails on dependency fetch under --network=none — the line has never worked as written. S4 is load-bearing: a recorded maintainer decision cites it as the mitigation that closed a review-agent exposure question. Acceptance: the pin tracks the module toolchain and the recipe provisions modules before the network is cut. \ No newline at end of file diff --git a/.abcd/work/issues/resolved/iss-2608261533174500-terminology-rag-row-claims-the-sources-corpus-ships.md b/.abcd/work/issues/resolved/iss-2608261533174500-terminology-rag-row-claims-the-sources-corpus-ships.md new file mode 100644 index 00000000..dade57c9 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2608261533174500-terminology-rag-row-claims-the-sources-corpus-ships.md @@ -0,0 +1,16 @@ +--- +schema_version: 1 +id: "iss-2608261533174500" +slug: "terminology-rag-row-claims-the-sources-corpus-ships" +severity: "minor" +category: "documentation" +source: "agent-observation" +found_during: "bughunt-a round 9" +found_at: "docs/reference/terminology.md" +resolution: "The RAG row states the corpus as a user-tier script MVP with core absorption cited, matching the script-first-mvp principle" +impact: fix +resolved_by: + commit: "6d9a7b24" +--- + +docs/reference/terminology.md's RAG row claims 'the sources corpus ships the script-first version' — a false shipped-ness claim on a page whose sibling rows use ship as a precise delivery marker (No MCP server ships today; abcd ships zero skills). Nothing corpus-related is in the repo or any released artefact; the consult and ingest commands refuse when the corpus is absent and point at a README no user has; and the committed script-first-mvp principle states a script MVP never ships as product behaviour, naming this corpus tooling as the live instance. Same defect class as the open Memory-row overstatement on the same page. Acceptance: the row states the corpus as a user-tier script MVP with core absorption tracked, not as shipped behaviour. \ No newline at end of file diff --git a/.abcd/work/issues/resolved/iss-2608261533290815-privacy-hygiene-silently-skips-unreadable-tracked-files.md b/.abcd/work/issues/resolved/iss-2608261533290815-privacy-hygiene-silently-skips-unreadable-tracked-files.md new file mode 100644 index 00000000..166c3a03 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2608261533290815-privacy-hygiene-silently-skips-unreadable-tracked-files.md @@ -0,0 +1,16 @@ +--- +schema_version: 1 +id: "iss-2608261533290815" +slug: "privacy-hygiene-silently-skips-unreadable-tracked-files" +severity: "nitpick" +category: "bug" +source: "agent-observation" +found_during: "bughunt-a round 9" +found_at: "internal/core/repolint/rule_privacy.go" +resolution: "Open failures are classified: EACCES/EPERM/EIO warn not-scanned, absent paths and symlink-shaped refusals stay silent by design; polarity table pinned" +impact: fix +resolved_by: + commit: "e6f90055" +--- + +repolint privacy-hygiene silently skips a tracked file it cannot open: readTrackedFile returns not-ok with no oversize marker on an open failure, and the caller emits a finding only for the oversize case — so an EACCES/ENOTDIR tracked file produces no finding and the rule reports the repository clean, against the engine contract that a check that cannot run must not be silently reported as passing. The oversize arm of the same helper got exactly this fix; the open arm beside it stayed silent. Constraint on the fix: TrackedFiles lists index entries, so ENOENT (deleted-in-worktree, sparse checkout) must stay silent and only genuine unreadability warns. Acceptance: a non-ENOENT open failure yields a not-scanned warn finding, watched-fail via an ENOTDIR fixture. \ No newline at end of file diff --git a/.abcd/work/issues/resolved/iss-2608261533297309-runbounded-trimspace-corrupts-first-entry-of-nul-lists.md b/.abcd/work/issues/resolved/iss-2608261533297309-runbounded-trimspace-corrupts-first-entry-of-nul-lists.md new file mode 100644 index 00000000..0cc909e7 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2608261533297309-runbounded-trimspace-corrupts-first-entry-of-nul-lists.md @@ -0,0 +1,16 @@ +--- +schema_version: 1 +id: "iss-2608261533297309" +slug: "runbounded-trimspace-corrupts-first-entry-of-nul-lists" +severity: "nitpick" +category: "bug" +source: "agent-observation" +found_during: "bughunt-a round 9" +found_at: "internal/gitutil/repo.go" +resolution: "runBounded trims the trailing side only; NUL-list first-entry fidelity pinned by a leading-space fixture" +impact: fix +resolved_by: + commit: "89603e7c" +--- + +gitutil.runBounded returns strings.TrimSpace over the whole capture buffer, and lifeboat's pathIsIgnored NUL-splits that string from ls-files -z to build the not-ignored set. The -z form exists so whitespace in filenames cannot desync the list, but the shared trim strips leading whitespace off the first entry, so a repo whose first-sorting path begins with space/tab/CR/newline gets that one file silently classified ignored and dropped from the evidence walk — the quiet-evidence-loss shape the adapter's own contract forbids. Verified: -z disables quoting, the trailing NUL survives (NUL is not IsSpace), only the leading side is damaged; TrackedFiles bypasses the trim via raw Output. One-path blast radius, rare shape. Acceptance: NUL-list consumers receive the raw buffer (or a trailing-only trim) with a watched-fail test on a leading-whitespace name. \ No newline at end of file diff --git a/.abcd/work/issues/resolved/iss-2608261533419396-site-command-description-omits-checks-render-write.md b/.abcd/work/issues/resolved/iss-2608261533419396-site-command-description-omits-checks-render-write.md new file mode 100644 index 00000000..3e233ee3 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2608261533419396-site-command-description-omits-checks-render-write.md @@ -0,0 +1,16 @@ +--- +schema_version: 1 +id: "iss-2608261533419396" +slug: "site-command-description-omits-checks-render-write" +severity: "nitpick" +category: "documentation" +source: "agent-observation" +found_during: "bughunt-a round 9" +found_at: "commands/site.md" +resolution: "The description discloses check's render-if-absent write alongside bare and build" +impact: fix +resolved_by: + commit: "0458d586" +--- + +commands/site.md's frontmatter description enumerates write posture for the bare and build forms only, while check — added later with the argument-hint updated in the same diff — renders the whole site into an output directory that has no index.html. The body documents this correctly; the description line is a stale roster, and it drops the counter-intuitive form. House style names every form's posture, surprising ones explicitly (identity, version). Acceptance: the description discloses check's render-if-absent write. \ No newline at end of file From c599c749f3af0b609b1c65dda78583cadcaa7667 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:49:46 +0000 Subject: [PATCH 11/11] fix: scope strict config decoding to the rule and token objects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The banlist editor pins that a top-level annotation key (the JSON commentary convention) still loads, and whole-document DisallowUnknownFields refused it. The misspelt-key hole lives in the rule and banned-token OBJECTS — where the armed enums sit — so each is re-decoded strictly while the top level stays lenient, with the trade-off documented and both polarities pinned. Assisted-by: Claude:claude-fable-5 --- internal/core/lint/config.go | 44 +++++++++++++++++++--- internal/core/lint/config_severity_test.go | 25 +++++++++++- 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/internal/core/lint/config.go b/internal/core/lint/config.go index f661ef76..e523f326 100644 --- a/internal/core/lint/config.go +++ b/internal/core/lint/config.go @@ -345,13 +345,11 @@ func LoadConfig(path string) (Config, error) { return Config{}, err } } - // Strict decode: a misspelt key would silently zero-value the field it - // missed ("enabld" disarms a rule, "severty" strips its exit-code weight), - // and both misreads survive review because the file still looks armed. var cfg Config - dec := json.NewDecoder(bytes.NewReader(data)) - dec.DisallowUnknownFields() - if err := dec.Decode(&cfg); err != nil { + if err := json.Unmarshal(data, &cfg); err != nil { + return Config{}, err + } + if err := strictRuleAndTokenKeys(data); err != nil { return Config{}, err } if err := cfg.validateBannedTokens(); err != nil { @@ -363,6 +361,40 @@ func LoadConfig(path string) (Config, error) { return cfg, nil } +// strictRuleAndTokenKeys re-decodes each rule and banned-token OBJECT with +// DisallowUnknownFields: a misspelt key silently zero-values the field it +// missed ("enabld" disarms a rule, "severty" strips its exit-code weight), and +// both misreads survive review because the file still looks armed. The TOP +// level stays lenient on purpose — an annotation key beside the declared ones +// is the JSON commentary convention, and the banlist editor pins that a config +// carrying one still loads. +func strictRuleAndTokenKeys(data []byte) error { + var raw struct { + BannedTokens []json.RawMessage `json:"banned_tokens"` + Rules map[string]json.RawMessage `json:"rules"` + } + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + for id, body := range raw.Rules { + dec := json.NewDecoder(bytes.NewReader(body)) + dec.DisallowUnknownFields() + var rc RuleConfig + if err := dec.Decode(&rc); err != nil { + return &configError{"rule " + id + ": " + err.Error()} + } + } + for i, body := range raw.BannedTokens { + dec := json.NewDecoder(bytes.NewReader(body)) + dec.DisallowUnknownFields() + var t BannedToken + if err := dec.Decode(&t); err != nil { + return &configError{"banned_tokens index " + strconv.Itoa(i) + ": " + err.Error()} + } + } + return nil +} + // validateSeverities refuses a severity outside the engine's enum on any // enabled rule or banned token. The exit paths count Severity == "blocker" // verbatim, so an off-enum value would emit findings that serialize yet count diff --git a/internal/core/lint/config_severity_test.go b/internal/core/lint/config_severity_test.go index 809bb800..162dd30d 100644 --- a/internal/core/lint/config_severity_test.go +++ b/internal/core/lint/config_severity_test.go @@ -50,7 +50,8 @@ func TestLoadConfigRefusesOffEnumTokenSeverity(t *testing.T) { func TestLoadConfigRefusesUnknownKeys(t *testing.T) { // A misspelt key silently zero-values the field it missed: "enabld" leaves // Enabled false (a rule disarmed), "severty" leaves Severity "" (findings - // that count toward no exit). Strict decoding turns both into a refusal. + // that count toward no exit). Strict decoding of the rule and token + // objects turns both into a refusal. path := writeConfig(t, `{ "roots": ["rec"], "rules": {"links_resolve": {"enabld": true, "severity": "blocker"}} @@ -58,6 +59,28 @@ func TestLoadConfigRefusesUnknownKeys(t *testing.T) { if _, err := LoadConfig(path); err == nil { t.Fatal("LoadConfig accepted an unknown config key (\"enabld\"); want rejection") } + tok := writeConfig(t, `{ + "roots": ["rec"], + "banned_tokens": [ + {"id":"t1","pattern":"foo","message":"no","severty":"blocker","severity":"blocker","successor":"bar","allow_context":["ok"]} + ] + }`) + if _, err := LoadConfig(tok); err == nil { + t.Fatal("LoadConfig accepted an unknown banned-token key (\"severty\"); want rejection") + } +} + +func TestLoadConfigAcceptsTopLevelAnnotationKey(t *testing.T) { + // The top level stays lenient: an annotation key is the JSON commentary + // convention, and the banlist editor pins that such a config still loads. + path := writeConfig(t, `{ + "note": "banned_tokens", + "roots": ["rec"], + "rules": {"links_resolve": {"enabled": true, "severity": "blocker"}} + }`) + if _, err := LoadConfig(path); err != nil { + t.Fatalf("LoadConfig refused a top-level annotation key: %v", err) + } } func TestLoadConfigAcceptsDisabledRuleWithoutSeverity(t *testing.T) {