diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 800c98bd..41d98110 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -2109,6 +2109,31 @@ 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. - 2026-08-26 — Bug-hunt round 9 (bughunt-b): baseline green on the unshallowed clone. Five parallel hunters, per-finding adversarial refutation: 12 substantive and 5 nitpick findings confirmed and fixed, 4 refuted or prior 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 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 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]" --- 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. | diff --git a/internal/core/lint/config.go b/internal/core/lint/config.go index f5267a1b..e523f326 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" @@ -348,12 +349,81 @@ func LoadConfig(path string) (Config, error) { 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 { return Config{}, err } + if err := cfg.validateSeverities(); err != nil { + return Config{}, err + } 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 +// 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..162dd30d --- /dev/null +++ b/internal/core/lint/config_severity_test.go @@ -0,0 +1,109 @@ +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 of the rule and token + // objects 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") + } + 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) { + // 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) + } +} 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 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) + } +} 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 } diff --git a/internal/surface/cli/cli.go b/internal/surface/cli/cli.go index 095906fb..5b36b005 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) + } +} 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)