diff --git a/.abcd/config/reading-presets.json b/.abcd/config/reading-presets.json index 2ea3254f8..40dcf2c9f 100644 --- a/.abcd/config/reading-presets.json +++ b/.abcd/config/reading-presets.json @@ -61,9 +61,9 @@ ], "window": { "tokens_est": 1340000, - "measured_tokens_est": 1322032, - "measured_bytes": 5089824, - "measured_at": "b727b969a992532a9e67adb23734914a4f331513" + "measured_tokens_est": 1325292, + "measured_bytes": 5102376, + "measured_at": "0dd22bdc043952a36800be0d41d72ac65b23055d" } }, "entailment": { @@ -133,9 +133,9 @@ ], "window": { "tokens_est": 390000, - "measured_tokens_est": 382812, - "measured_bytes": 1473830, - "measured_at": "b727b969a992532a9e67adb23734914a4f331513" + "measured_tokens_est": 383234, + "measured_bytes": 1475453, + "measured_at": "0dd22bdc043952a36800be0d41d72ac65b23055d" } }, "comparative": { @@ -217,9 +217,9 @@ ], "window": { "tokens_est": 1350000, - "measured_tokens_est": 1331068, - "measured_bytes": 5124612, - "measured_at": "b727b969a992532a9e67adb23734914a4f331513" + "measured_tokens_est": 1334328, + "measured_bytes": 5137164, + "measured_at": "0dd22bdc043952a36800be0d41d72ac65b23055d" } } } diff --git a/.abcd/development/brief/04-surfaces/02-disembark.md b/.abcd/development/brief/04-surfaces/02-disembark.md index 24f54c7b5..c44cbf6e9 100644 --- a/.abcd/development/brief/04-surfaces/02-disembark.md +++ b/.abcd/development/brief/04-surfaces/02-disembark.md @@ -86,6 +86,7 @@ INVENTORY (read-only) DESTINATION SAFETY GATE refuse unless is absent, empty, or carries a parseable _provenance.json → never overwrite a directory abcd did not produce (adr-35) + refuse a reached through a symlink at any level inside a checkout │ ▼ SECRET SCAN (before any write) diff --git a/.abcd/development/brief/04-surfaces/03-embark.md b/.abcd/development/brief/04-surfaces/03-embark.md index 80b67bed2..95d3f66d1 100644 --- a/.abcd/development/brief/04-surfaces/03-embark.md +++ b/.abcd/development/brief/04-surfaces/03-embark.md @@ -88,7 +88,10 @@ scaffolder and no model sit in the write path. specs — plus the report-only files that inform the run. The lifeboat is untrusted input: embark verifies its `manifest_sha256` against the on-disk tree, over every hashed file, and refuses a symlink or an oversize file - anywhere inside. A tampered hashed record or an added stray file is refused. + anywhere inside. Both operands, the lifeboat and the target, are refused + when they are reached through a symlink at any level inside a checkout, the + one place a commit can plant one; outside every checkout the path is the + operator's own and is taken as given. A tampered hashed record or an added stray file is refused. The post-pack synthesis layer sits outside the manifest seal deliberately, because it is written after the hash: those files carry their own per-entry integrity (cite-or-be-dropped, the registered-verdict gate) rather than the diff --git a/.abcd/development/brief/04-surfaces/04-launch.md b/.abcd/development/brief/04-surfaces/04-launch.md index eda2c0d55..be68c6e67 100644 --- a/.abcd/development/brief/04-surfaces/04-launch.md +++ b/.abcd/development/brief/04-surfaces/04-launch.md @@ -584,7 +584,7 @@ fingerprinted — not the unversioned working tree. older harnesses fail to install it, and very old ones fail to load the marketplace. The install instructions and the release notes state the floor. - **Contributors** load the plugin from their own checkout rather than through a - second catalog entry (`CONTRIBUTING.md`). + second catalog entry (`.github/CONTRIBUTING.md`). **Anti-drift.** The two manifests in the artefact describe one release, so the version at the selected location and the marketplace entry must agree. A diff --git a/.abcd/development/brief/04-surfaces/07-memory.md b/.abcd/development/brief/04-surfaces/07-memory.md index 9c6936e4f..f12af6201 100644 --- a/.abcd/development/brief/04-surfaces/07-memory.md +++ b/.abcd/development/brief/04-surfaces/07-memory.md @@ -44,11 +44,13 @@ surface contract: what the user types and what happens. **Bare `/abcd:memory`** renders the store's state and nothing else: how many pages there are by class, when the last ingest happened, the recent -contradictions, and per-source quotation-budget headroom. It never mutates and -never rebuilds an index. The JSON render carries one element the text render -drops, a `drift` list saying that the catalogue or the contradictions register -no longer hash-matches what the store's pages would render, so a reader knows -the numbers are stale rather than wrong. Headroom is read-only in the same +contradictions, per-source quotation-budget headroom, and drift. It never +mutates and never rebuilds an index. Drift is a line saying that the catalogue +or the contradictions register no longer hash-matches what the store's pages +would render, naming the ingest as the verb that rebuilds it, so a reader +knows the numbers are stale rather than wrong; the text board prints +each line in the words the JSON's `drift` list carries, and a current store +prints none. Headroom is read-only in the same spirit: a fresh index shows per-source warn and block headroom, a drifted one says to run the lint, and an absent or unreadable one says the headroom is unavailable rather than guessing at it. diff --git a/.abcd/development/brief/04-surfaces/11-history.md b/.abcd/development/brief/04-surfaces/11-history.md index 51b91b8ab..f8b9fe72a 100644 --- a/.abcd/development/brief/04-surfaces/11-history.md +++ b/.abcd/development/brief/04-surfaces/11-history.md @@ -111,7 +111,9 @@ ahoy's registry stays under `~/.abcd/history/` and holds no transcripts. - **Reconstructing** — render one session, named by its id, as **one self-contained artefact** (`.md`) and **one telemetry file** (`.telemetry.json`), written into an output directory (default the working - directory) or to stdout. The artefact is Markdown because its + directory) or to stdout. The directory must already exist, and it is refused + when it is reached through a symlink at any level inside a checkout; outside + every checkout the path is the operator's own. The artefact is Markdown because its consumer is a model being handed the session as context; it names its records by basename and carries no store path, so it reads with the store gone. diff --git a/.abcd/development/brief/04-surfaces/17-guard.md b/.abcd/development/brief/04-surfaces/17-guard.md index 20f3cf35b..e187f5fdb 100644 --- a/.abcd/development/brief/04-surfaces/17-guard.md +++ b/.abcd/development/brief/04-surfaces/17-guard.md @@ -260,7 +260,15 @@ command string handed to a shell is opened and read. A git alias declared on the command git would actually run is what gets checked. A commit or push that moves `core.hooksPath` for itself is read as skipping its hooks, which is what it does. A delete chained after `pushd` or `popd` is read as one chained after -`cd`. In a repository with more +`cd`. A `kill` handed what a process search prints, in a substitution or +piped into `xargs kill`, is read as the kill by name it is — through a group +and into one, whose every command is read as handed what is piped into it, +through a shell string that runs the search, and into a shell string `xargs` +runs or a pipe or redirect feeds, whose every command is read as handed its +input — and a `pkill` or +`killall` selecting by user, group or terminal, its value written apart or +attached, as selecting every session under the account; `pkill`'s signal name +is read as a signal first, in any case. In a repository with more than one worktree, a stash or pop that does not name its entry is warned about, because the stash stack is shared across worktrees. Where the reading is a guess, over-blocking is the direction the guard takes. @@ -273,7 +281,9 @@ table does not name; a REST path an entry names by its root segment when the host serves that API under a prefix; an IFS the shell already holds when the line starts, or gains during the line through a name the guard does not read (`declare $(echo I)FS=x`, a sourced file), -since every line is read from the default IFS; a payload inside a non-shell interpreter such as `python -c`, which is +since every line is read from the default IFS; a pid list a kill reads through a variable or a file, or from a `ps | +grep` chain; +a payload inside a non-shell interpreter such as `python -c`, which is one opaque token and today a silent allow; and any dangerous form no entry describes. Nor does an allow see through a parameter expansion that carries no substitution (`$VAR`, `${VAR:-git}`), wherever it stands — as the command's diff --git a/.abcd/development/brief/04-surfaces/20-banlist.md b/.abcd/development/brief/04-surfaces/20-banlist.md index 3493e1f74..94780361e 100644 --- a/.abcd/development/brief/04-surfaces/20-banlist.md +++ b/.abcd/development/brief/04-surfaces/20-banlist.md @@ -76,7 +76,7 @@ markdown, with `exempt_paths` excusing a historical tree as it does under blocks, which the rest of the family skips by default: a fenced example is not prose, but a fence is published as readily as prose, so an entry that means to skip fences declares `skip_code_fences: true`. This repository's `name_roots` are `.abcd`, `AGENTS.md`, -`CONTRIBUTING.md` and `scripts`, and its `exempt_paths` excuse the +`.github/CONTRIBUTING.md` and `scripts`, and its `exempt_paths` excuse the configuration itself (whose entries spell every ban), the research data and the review archive. diff --git a/.abcd/development/brief/04-surfaces/22-site.md b/.abcd/development/brief/04-surfaces/22-site.md index b479fd486..0f038e3d3 100644 --- a/.abcd/development/brief/04-surfaces/22-site.md +++ b/.abcd/development/brief/04-surfaces/22-site.md @@ -151,9 +151,10 @@ The build reads the repository and nothing else — no network at any point. Its inputs are the composition declaration and the interface-string allowlist; the record itself, read through the record-lint engine's own frontmatter scan so there is one parser rather than two; the bibliography and the glossary through their own -parsers; one pass of git history; `CHANGELOG.md`; the two root prose files whose -text the site publishes, which are the acknowledgements behind the references page -and the authorship section of the contribution guide behind the contributors page; +parsers; one pass of git history; `CHANGELOG.md`; the two prose files whose +text the site publishes, which are the acknowledgements at the root behind the +references page and the authorship section of the contribution guide in `.github/` +behind the contributors page; and `docs/` with its committed assets. It writes the landing page, the record explorer, the machine-readable record export, the install script from its committed template, the redirect and header maps, the stylesheets and scripts, every referenced raster, and its own diff --git a/.abcd/development/brief/04-surfaces/23-reading.md b/.abcd/development/brief/04-surfaces/23-reading.md index ab3de0041..1e13e67a6 100644 --- a/.abcd/development/brief/04-surfaces/23-reading.md +++ b/.abcd/development/brief/04-surfaces/23-reading.md @@ -142,6 +142,11 @@ contamination. And both artefacts are refused as input wherever an admitted path holds one, recognised by the type tag they carry, so a run committed before that refusal existed cannot ride in either. +An output directory reached through a symlink at any level inside a checkout is +refused however it is spelled (relative, absolute, or climbing out of the +repository and back in), because a committed link would carry both files +elsewhere; outside every checkout the path is the operator's own. + Run identifiers are minted per adr-45, from a mint that reads no maximum, so two checkouts assembling in the same window cannot converge on one id. diff --git a/.abcd/development/principles/adopt-contributor-commits.md b/.abcd/development/principles/adopt-contributor-commits.md index 5cb891db4..de1357bdf 100644 --- a/.abcd/development/principles/adopt-contributor-commits.md +++ b/.abcd/development/principles/adopt-contributor-commits.md @@ -12,6 +12,6 @@ the same diff — a missing entry is a follow-up debt, not a separate decision. Surfaced by the second operator in the 2026-08-27 security-advisory pilot (F-W): the issue-sweep's re-author-with-`Reported-by` default cost a contributor with a ready branch their contributor-graph authorship. The -enabling convention beneath this principle is `CONTRIBUTING.md`'s attribution +enabling convention beneath this principle is `.github/CONTRIBUTING.md`'s attribution section; the discipline rung (a gate that notices an adopted-and-rewritten external branch) is unfiled. diff --git a/.abcd/docs-lint.json b/.abcd/docs-lint.json index 825ae5cf1..ddbed5da9 100644 --- a/.abcd/docs-lint.json +++ b/.abcd/docs-lint.json @@ -6,7 +6,7 @@ "name_roots": [ ".abcd", "AGENTS.md", - "CONTRIBUTING.md", + ".github/CONTRIBUTING.md", "scripts" ], "banned_tokens": [ @@ -257,10 +257,8 @@ "severity": "blocker", "extra_roots": [ "AGENTS.md", - "CONTRIBUTING.md", "CHANGELOG.md", "ACKNOWLEDGEMENTS.md", - "SECURITY.md", "RELEASE.md", ".abcd/README.md", ".github", @@ -293,8 +291,6 @@ "AGENTS", "CHANGELOG", "RELEASE", - "CONTRIBUTING", - "SECURITY", "LICENSE", "ACKNOWLEDGEMENTS" ] diff --git a/.abcd/site.json b/.abcd/site.json index d845c6b46..5e9162441 100644 --- a/.abcd/site.json +++ b/.abcd/site.json @@ -68,7 +68,7 @@ "record_pages": { "contributors": { "policy": { - "file": "CONTRIBUTING.md", + "file": ".github/CONTRIBUTING.md", "heading": "AI assistance and authorship", "part": "first-bullet" } diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 217d28bd8..f8c4fb5df 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -2566,6 +2566,8 @@ together (the script's header says why there is no escape hatch). - 2026-09-26 — Three departures the scribe lane (itd-2609020625402599, spc-2609020626045177) made from its closed spec, which its review found recorded only in code, the chapter or the lane report, recorded here (implementer of lane fix2-scribe, autonomous run A). First, the surface chapter is `04-surfaces/31-scribe.md`, not the `24-scribe.md` the spec names: row 24 is `decide`'s, taken before the lane landed, so the chapter took the next free row. The spec is closed and keeps its text; four open lanes claim row 31 (build, lab, source ledger and this one), so the integration step renumbers three of them. Second, the transcript store's check is `SessionSeparation(repoRoot, rootSHA)`, not the `SessionSeparation(rootSHA)` the spec names, because it reads through `history.List`, which takes the repository root to find a checkout's opt-in per-repo transcript store; the report is unchanged. Third, the scribe's context is assembled from the ledger as it stands in the working tree, uncommitted records included, while the intent's scope condition (cond-2609020626046719) says committed ledger content. The working-tree read is what the code does today and the chapter says so. Whether the condition or the code should move is not decided here: it is captured as iss-2609261056373310, a ruling owed. - 2026-09-25 — itd-2609211913453478's acceptance criterion 4 ships under two readings the intent's scope line does not state. A glossary entry's `not_to_be_confused_with` passes when at least one member names a family row on the record-families page or the page itself, where the scope line says the field "may name only a family on the page"; the stricter reading would force nonsense pairs such as warm against intent, and the entries keep their real confusion pairs. The family-key rule (`record_family_key`, warn) reports a record frontmatter key only when the glossary already marks that word superseded or forbidden, so a brand-new grouping word with no row (the intent's own Mechanism case, e.g. an `initiative:` key) is not detected by construction, and the stores the page does not row (adr, rdi, dsp, rdg, adm, srp) are not reported. The six `grandfathered_at_phase` warnings on itd-20, 27, 28, 63, 69 and 72 are history and stay. Recorded for the product thinker to confirm or widen (autonomous run A, glossary lane review, orchestrator abcd-39). - 2026-09-26 — The lab store is keyed `~/.abcd/lab///`, with one `index.jsonl` registry per root-sha lane beside the lab homes (lane implementer, autonomous run A, on review-lab's third finding against spc-2609212141418943 for itd-2609212137128014). This supersedes two recorded texts: the spec's literal `~/.abcd/lab//` (scope item 1), and the 2026-08-31 lab-convention entry's hand-run keying `~/.abcd/lab/-/` with a single top-level `~/.abcd/lab/index.jsonl`, whose stated divergence from root-sha keying is withdrawn. Why: the intent's scope condition keys the store "as the other machine-scoped stores are", and the worktree and transcript stores key on the repository's root commit, because a checkout moves, is renamed and is cloned twice on one machine while its root commit does none of that; a lab's identity is still its intention, carried by its id `lab--` (the UTC mint time and the pin), so several labs share one baseline inside one lane. The hand-run labs that predate the verb stay where they are, beside the root-sha lanes, and the verb neither reads nor writes them or the top-level registry, so no real lab is moved or migrated by the change. A later text naming `~/.abcd/lab//` (the open spc-2609221011151661's `pairs.jsonl` among them) means the lab home inside its root-sha lane. +- 2026-09-27 — The kill-by-search reading follows a search into and out of shell strings, and keeps one over-block, recorded so it is not mistaken for a defect (lane drainG2, autonomous run A, on iss-2609262259360005 and the review of the first reading). Every command of a string that `xargs` runs is read as handed xargs's input, so `pgrep make | xargs sh -c 'kill 4242'` blocks as `kill-by-search` though its kill names a pid: `xargs -I{}` replaces the input into any part of the string, and telling a command that reads `"$@"` or `$1` from one that does not would be a text match on the string, which the feed mechanism does not make. The same holds for the standard input a shell passes to the commands of its string (`pgrep make | sh -c 'xargs kill'`). The accepted over-block of the first reading, that a lower-case signal name (`pkill -term -g `) blocked as `pkill-by-owner` because its letters read as the `-t` and `-u` selectors, is removed rather than recorded: `pkill`'s first `-NAME` word naming a signal is read as its signal, in any case and with or without `SIG`, as procps-ng and BSD pkill read it, which is also what lets `-U` and `-G` be read attached (`pkill -Ubob`) without taking `-HUP`, `-USR1` or `-SIGTERM` for them. Only the first such word is the signal, so `pkill -9 -term -g ` still blocks: both implementations hand the second word to their option parser as `-t erm`. `killall` is not read this way, because psmisc killall reads a signal name only when it begins with a capital letter and parses `killall -term` as `killall -t erm`. +- 2026-09-27 — A pipe into a group and a redirect into a shell string reach every command there, and the reading keeps two over-blocks, recorded so they are not mistaken for defects (lane fix3-drainG, autonomous run A, on iss-2609270028388291 from review-drainG2). A pipe into a `{ … }` or `( … )` group is the standard input of every command in it, so each command emitted inside the group reads it, not only those before the group's first separator; a shell in the group reads it as a stream too. A command in a group is read as handed its group's input even when a pipe inside the group hands it another, because the command before that pipe may pass the group's input on (`cat`), and which commands do is not modelled; so `pgrep make | { true; echo 4242 | { xargs kill; }; }` blocks. The runs of the open groups are held as one covering run, which keeps each command's reading constant however deep groups nest, at the cost of also counting a search that sits inside an outer piped group before an inner one opens. A shell passes its standard input to the commands of its string, and a here-string or a process substitution redirected into the shell is that input, as a pipe is; the `<` that redirects a process substitution is not kept by the tokenizer, so a process substitution handed to the shell as an operand is read as its input as well, which is what `sh -c 'xargs kill < "$1"' _ <(pgrep make)` does with it. - 2026-09-26 — The reading ingest's prose-citation refusal departs from "every refusal past the identity point is recorded" (the rule `refuse()` in internal/core/reading/ingest.go enforces, from iss-2608311518250688): it is returned unrecorded, because a recorded refusal gives the run an outcome and `refuseARerun` would then refuse the same run re-worded, where the run left parked is ingested again once its prose describes the record rather than citing a missing id — the stance the verdict ingest takes. It keeps the rest of the refusal contract: it rolls back this run id's own half-landed records and stage as `refuse()` does, and runs no sweep of other runs' orphans. `04-surfaces/23-reading.md` states it (implementer of lane fix2-drainL, autonomous run A, on review-drainL's design point for iss-2609261835118276). - 2026-09-26 — Three of the rules loader's security records close, and one stays owed to the product thinker (autonomous run A orchestrator's lane brief, taken by the implementer of lane drainS2). (1) The home directory is never a session's repo root (iss-2609020219198779, answering the owed question "is a home-directory git toplevel a legitimate config scope, or excluded outright?" as the brief rules it): its `.abcd/` is the user layer, so the root walk passes over the home and a toplevel that is the home resolves like a non-repo directory. The lane narrowed the brief's "or an ancestor of HOME": a toplevel that contains the home, the shape of a hermetic harness that points `HOME` inside its checkout, stays the root because git vouched for it and its own `.abcd/` is its own; only the stop at the home is removed. A session whose working directory is the home still reads a `.abcd/` there as the working directory's, the posture question recorded on 2026-09-25. (2) A bundled guardrail that an override withholds is named on every load (iss-174): for COMMITTING, LOAD and PII, each bundled recall keyword, alias or rule missing from a list an override set goes to stderr with the file whose list is in force, and the merge stays per field. The other bundled domains are left out because a repository restates them in its own words, and a note on every restatement would bury the one that matters. Still owed to the product thinker: whether security-bearing lists should union with the bundled entries or take a replace-versus-extend marker instead (itd-117's finer-grained-merging follow-up). (3) The foreign-uid refusal says what it still reads (iss-2609251522588539): the note, the configuration chapter and the install how-to now say that a `.abcd/` at the working directory is read, as AGENTS.md has since 0434d475. (4) iss-2609020219265817 is deferred past v0.11.0, not closed. Every CommonMark heading construct in a rule body (ATX on any line, the first line included, setext, and HTML h1-h6) can be closed only by a code-safe rendering that flattens legitimate structure, or by a fence-aware escaper that is complete only by enumeration and changes the raw text the model reads. So "escaped, fenced, or left to the line-start contract" is the product thinker's ruling. - 2026-09-26 — The build loop's worktree store is keyed on the FULL root sha: a lane lives at `~/.abcd/worktrees//-` with the 40-hex root commit, the form the history, transcript and voyage stores use and the one the store's draft (itd-2609091014076309) specifies. The lanes of autonomous run A made by hand under the abbreviated key (`~/.abcd/worktrees/488a0aa9//`) are the pre-verb convention, not a second form of the store: the loop never reads or adopts a lane under that key, and those worktrees are retired with `git worktree remove` like any other (implementer of fix round fix2-loop2, autonomous run A, on item 4 of the loop2 review; spc-2609202134338445 piece 6). diff --git a/.abcd/work/issues/open/iss-2608291957114882-status-outdir-and-result-outdir-carry-an-absolute-path-into.md b/.abcd/work/issues/open/iss-2608291957114882-status-outdir-and-result-outdir-carry-an-absolute-path-into.md deleted file mode 100644 index 3fd7277ee..000000000 --- a/.abcd/work/issues/open/iss-2608291957114882-status-outdir-and-result-outdir-carry-an-absolute-path-into.md +++ /dev/null @@ -1,12 +0,0 @@ ---- -schema_version: 1 -id: "iss-2608291957114882" -slug: "status-outdir-and-result-outdir-carry-an-absolute-path-into" -severity: "minor" -category: "bug" -source: "agent-finding" -found_during: "v0.6.9-security-review" -found_at: "internal/core/site/build.go" ---- - -Status.OutDir and Result.OutDir carry an absolute path into abcd site --json and site build --json when --out is absolute; the iss-81 rule is that machine output never carries a developer-identity path and fsutil.RepoRel is the canonical primitive, unused here diff --git a/.abcd/work/issues/open/iss-2609251600023777-site-renderer-a-top-level-three-backtick-fence-indented-one.md b/.abcd/work/issues/open/iss-2609251600023777-site-renderer-a-top-level-three-backtick-fence-indented-one.md deleted file mode 100644 index 1f70f3973..000000000 --- a/.abcd/work/issues/open/iss-2609251600023777-site-renderer-a-top-level-three-backtick-fence-indented-one.md +++ /dev/null @@ -1,14 +0,0 @@ ---- -schema_version: 1 -id: "iss-2609251600023777" -slug: "site-renderer-a-top-level-three-backtick-fence-indented-one" -severity: "minor" -category: "bug" -source: "impl-review" -found_during: "autonomous run A resumed 2026-09-25" -origin: researcher-authored -production_mode: hand-written -found_at: "internal/core/site/markdown.go" ---- - -site renderer: a top-level three-backtick fence indented one to three spaces still renders silently as a paragraph with inline code (site/markdown.go unrenderedFenceRe covers tildes and four or more backticks only). No page in docs/ or site-src/ has the shape today. Refuse it like the other unsupported fence forms, or render it. diff --git a/.abcd/work/issues/open/iss-2609262259360005-kill-readings-leave-variable-file-brace-group-and-port-spellings.md b/.abcd/work/issues/open/iss-2609262259360005-kill-readings-leave-variable-file-brace-group-and-port-spellings.md new file mode 100644 index 000000000..2db46dc92 --- /dev/null +++ b/.abcd/work/issues/open/iss-2609262259360005-kill-readings-leave-variable-file-brace-group-and-port-spellings.md @@ -0,0 +1,18 @@ +--- +schema_version: 1 +id: "iss-2609262259360005" +slug: "kill-readings-leave-variable-file-brace-group-and-port-spellings" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/guard/defaults/guard.json" +deferred_after: "v0.11.0" +deferral_reason: "every ordinary spelling this record named is read on fix/drain-guard (4cc129009, lane drainG2 of run A); what remains asks a ruling the lane may not take (ruling AS, owed by the product thinker): whether the guard follows a pid list across lines through a file, which source commands of a ps | grep | awk chain count as a search when ps -o pid= -p N | xargs kill is the everyday form of a recorded-pid kill, and whether a kill by the holder of a port or file (lsof -t, fuser -k) belongs to the kill-by-name family at all." +--- + +The kill-by-search and by-owner readings (iss-2609251640452031) left spellings of a kill by name or selector unseen. The ordinary ones are read on fix/drain-guard (4cc129009): a brace or paren group ({ pgrep make; } | xargs kill), a search inside a shell string (kill $(sh -c 'pgrep make')), a kill inside a shell string xargs runs or one that reads the pipe (pgrep make | xargs -I{} sh -c 'kill {}', pgrep make | xargs bash -c 'kill "$@"' _, pgrep make | sh -c 'xargs kill'), a kill behind xargs and an unknown launcher (pgrep make | xargs myrunner kill, now a Tier 2 warn), a selector value attached with a byte other than a letter or digit (pkill -tpts/3, pkill -ubob.smith), and pkill -Ubob or -Gstaff attached, read once pkill's signal word is read as a signal first. + +Three remain, and each waits on ruling AS: (1) a pid list carried through a file across commands (pgrep make > p; xargs kill < p), a data flow between lines the per-line reading does not follow; (2) a pid list taken from a ps | grep | awk chain, where reading ps or grep as a search source would also block ps -o pid= -p N | xargs kill, the recorded-pid kill; (3) a kill by the holder of a port or file, kill $(lsof -t -i :8080) and fuser -k, which stop whatever holds it, a peer session's server included, and have no entry and no statement in the brief. A pid list carried through a variable or a while-read loop (pids=$(pgrep make); kill $pids) is not this record's: it is the plain-variable half of iss-2609251824244354, ruled in DECISIONS 2026-09-25 (c) and deferred there. diff --git a/.abcd/work/issues/open/iss-2609270036253187-two-more-paths-by-which-a-process-search-s-output-reaches-a.md b/.abcd/work/issues/open/iss-2609270036253187-two-more-paths-by-which-a-process-search-s-output-reaches-a.md new file mode 100644 index 000000000..b492c02e9 --- /dev/null +++ b/.abcd/work/issues/open/iss-2609270036253187-two-more-paths-by-which-a-process-search-s-output-reaches-a.md @@ -0,0 +1,14 @@ +--- +schema_version: 1 +id: "iss-2609270036253187" +slug: "two-more-paths-by-which-a-process-search-s-output-reaches-a" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: review-drainG2" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/guard/tokenize.go" +--- + +Two more paths by which a process search's output reaches a kill are not read by kill-by-search: an unquoted here-document whose body holds a command substitution, redirected into an xargs kill (a here-string of the same substitution blocks), and a command substitution inside a command that reads a pipe, which inherits that pipe as its standard input (an xargs kill in the substitution reads the search piped into its command). A third, a search handed to a shell string as a positional parameter the string's kill reads, is not read either: it is a pid carried through a variable, the half DECISIONS 2026-09-25 (c) defers with iss-2609251824244354, unless it is read the way xargs's input to a string is (DECISIONS 2026-09-27), which is a call for the next guard lane. Found while fixing iss-2609270028388291. diff --git a/.abcd/work/issues/open/iss-2608231008315498-site-contributors-page-rethink-and-assisted-total-mismatch.md b/.abcd/work/issues/resolved/iss-2608231008315498-site-contributors-page-rethink-and-assisted-total-mismatch.md similarity index 56% rename from .abcd/work/issues/open/iss-2608231008315498-site-contributors-page-rethink-and-assisted-total-mismatch.md rename to .abcd/work/issues/resolved/iss-2608231008315498-site-contributors-page-rethink-and-assisted-total-mismatch.md index a3d0d4efc..fa73fe0a0 100644 --- a/.abcd/work/issues/open/iss-2608231008315498-site-contributors-page-rethink-and-assisted-total-mismatch.md +++ b/.abcd/work/issues/resolved/iss-2608231008315498-site-contributors-page-rethink-and-assisted-total-mismatch.md @@ -7,6 +7,14 @@ category: "bug" source: "user-observation" found_during: "user-observation" found_at: "internal/core/site/explorer.go" +resolution: "The contributors page carries only its two folded full-width panels, Authors of record and Assisted-by trailers, with no stat tiles, and the None declaration is counted beneath the chart, not in it, so the panel total equals the bar sum (bed0026c; pinned by TestAuthorshipBarSumEqualsAssistedTotal). The last piece, the disclosure rate moving to the health page, lands in 52329d9f as the last tile of the health page's row of counts." +impact: fix +resolved_by: + commit: "52329d9f" --- -Contributors page rethink: keep only the two things that matter — Authors of record, and Assisted-by trailers — stacked one under the other as expandable panels; drop the three stat tiles, with 'commits disclose AI assistance' moving to the Health page (depends on that page landing with the IA intent seed iss-2608230752354909). Also a real data inconsistency: the Assisted-by panel's note reads 1035 (a.Assisted) while its bars sum to 1036, because the tally includes the 'None' row — the positive human-only declaration — which a.Assisted deliberately excludes (contributors.go counts DeclaredNone and continues before incrementing Assisted). A chart whose total is 'assisted' must not carry a row that is by definition not assistance: either split None out as its own labelled figure or relabel the panel to the declarations it actually counts, and make the note equal what the bars sum to (report D of the 2026-08-23 second pass). \ No newline at end of file +Contributors page rethink: keep only the two things that matter — Authors of record, and Assisted-by trailers — stacked one under the other as expandable panels; drop the three stat tiles, with 'commits disclose AI assistance' moving to the Health page (depends on that page landing with the IA intent seed iss-2608230752354909). Also a real data inconsistency: the Assisted-by panel's note reads 1035 (a.Assisted) while its bars sum to 1036, because the tally includes the 'None' row — the positive human-only declaration — which a.Assisted deliberately excludes (contributors.go counts DeclaredNone and continues before incrementing Assisted). A chart whose total is 'assisted' must not carry a row that is by definition not assistance: either split None out as its own labelled figure or relabel the panel to the declarations it actually counts, and make the note equal what the bars sum to (report D of the 2026-08-23 second pass). + +## Grounds + +- pursued: the health page carries the share of authored commits disclosing assistance with its fraction and the merges excluded, and the contributors page carries two folded panels and no tile (TestHealthCarriesTheDisclosureRate); a tile on the contributors page, or no rate on the health page, would show it wrong diff --git a/.abcd/work/issues/open/iss-2608270540523859-move-contributing-md-and-security-md-from-repo-root-to-githu.md b/.abcd/work/issues/resolved/iss-2608270540523859-move-contributing-md-and-security-md-from-repo-root-to-githu.md similarity index 53% rename from .abcd/work/issues/open/iss-2608270540523859-move-contributing-md-and-security-md-from-repo-root-to-githu.md rename to .abcd/work/issues/resolved/iss-2608270540523859-move-contributing-md-and-security-md-from-repo-root-to-githu.md index f54a8eb93..9b09057bf 100644 --- a/.abcd/work/issues/open/iss-2608270540523859-move-contributing-md-and-security-md-from-repo-root-to-githu.md +++ b/.abcd/work/issues/resolved/iss-2608270540523859-move-contributing-md-and-security-md-from-repo-root-to-githu.md @@ -7,6 +7,14 @@ category: "tech-debt" source: "user-observation" found_during: "config-placement-reorg-2026-08-27" found_at: "internal/core/site/compose.go" +resolution: "CONTRIBUTING.md and SECURITY.md live in .github/ (4d4bbfb3), with every reader repointed in the same change: the site manifest's policy source, the stray_root_docs allowlist, README/ACKNOWLEDGEMENTS/AGENTS links, CI's inert-path classifier, the gate-list tests, the tooling comments, the site command page and the brief. The site and the lifeboat probe read the community-health files from .github/ as the forge does (5b56c19d): the footer resolves SECURITY.md across .github/, root and docs/, the policy source admits markdown directly in .github/, the conventions tier counts .github/CONTRIBUTING.md. The launch payload never carried either file. GitHub recognises .github/ for both per its community-health documentation; not re-checked against the live forge." +impact: additive +resolved_by: + commit: "4d4bbfb3" --- -Move CONTRIBUTING.md and SECURITY.md from repo root to .github/ to de-clutter root. NOT a plain git mv: both are load-bearing inputs to abcd's own site build. Required coupled changes: (1) internal/core/site/compose.go footer builds 'blob/main/SECURITY.md' links from a hardcoded root-relative list — update it or the security link silently drops; (2) the contributors/attribution page is config-driven from CONTRIBUTING.md via record_pages.contributors.policy.file (.abcd/config/site or equivalent) — repoint to .github/CONTRIBUTING.md; (3) remove CONTRIBUTING/SECURITY stems from the stray_root_docs allowlist (internal/core/lint/config.go); (4) fix ~6 relative links (README.md, ACKNOWLEDGEMENTS.md, AGENTS.md, CONTRIBUTING.md->SECURITY.md, and site markdown/explorer tests pinning ../../CONTRIBUTING.md and CONTRIBUTING.md#attribution); (5) internal/core/lifeboat/probe.go known-files list names CONTRIBUTING.md; (6) confirm GitHub still surfaces both community-health files from .github/ (it recognises root, docs/, and .github/). Verify with site-render + docs-lint gates. Related to the root-layout / config-placement ADR discussion and the governance/ mirror rename. \ No newline at end of file +Move CONTRIBUTING.md and SECURITY.md from repo root to .github/ to de-clutter root. NOT a plain git mv: both are load-bearing inputs to abcd's own site build. Required coupled changes: (1) internal/core/site/compose.go footer builds 'blob/main/SECURITY.md' links from a hardcoded root-relative list — update it or the security link silently drops; (2) the contributors/attribution page is config-driven from CONTRIBUTING.md via record_pages.contributors.policy.file (.abcd/config/site or equivalent) — repoint to .github/CONTRIBUTING.md; (3) remove CONTRIBUTING/SECURITY stems from the stray_root_docs allowlist (internal/core/lint/config.go); (4) fix ~6 relative links (README.md, ACKNOWLEDGEMENTS.md, AGENTS.md, CONTRIBUTING.md->SECURITY.md, and site markdown/explorer tests pinning ../../CONTRIBUTING.md and CONTRIBUTING.md#attribution); (5) internal/core/lifeboat/probe.go known-files list names CONTRIBUTING.md; (6) confirm GitHub still surfaces both community-health files from .github/ (it recognises root, docs/, and .github/). Verify with site-render + docs-lint gates. Related to the root-layout / config-placement ADR discussion and the governance/ mirror rename. + +## Grounds + +- pursued: the site still publishes the contributors policy quote and the footer security link from .github/ and passes its gates (make site-render; TestTheSiteReadsCommunityHealthFilesFromTheGithubDirectory, TestTheProvenanceGateReadsTheFooterFileItResolved), and docs-lint links_resolve passes; a dropped footer link, a policy refusal or a dangling README link would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2608291957114882-status-outdir-and-result-outdir-carry-an-absolute-path-into.md b/.abcd/work/issues/resolved/iss-2608291957114882-status-outdir-and-result-outdir-carry-an-absolute-path-into.md new file mode 100644 index 000000000..b1df16aed --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2608291957114882-status-outdir-and-result-outdir-carry-an-absolute-path-into.md @@ -0,0 +1,20 @@ +--- +schema_version: 1 +id: "iss-2608291957114882" +slug: "status-outdir-and-result-outdir-carry-an-absolute-path-into" +severity: "minor" +category: "bug" +source: "agent-finding" +found_during: "v0.6.9-security-review" +found_at: "internal/core/site/build.go" +resolution: "Status.OutDir, Result.OutDir and CheckResult.OutDir go through site.displayOutDir: repo-relative inside the repository (fsutil.RepoRel), home redacted to ~ outside it (fsutil.RedactHome), a relative --out as given. The lifeboat sweep is iss-2609261848326365; launch ship's payload.dest is captured as iss-2609261848338673." +impact: fix +resolved_by: + commit: "2a05a0e1" +--- + +Status.OutDir and Result.OutDir carry an absolute path into abcd site --json and site build --json when --out is absolute; the iss-81 rule is that machine output never carries a developer-identity path and fsutil.RepoRel is the canonical primitive, unused here + +## Grounds + +- pursued: an absolute --out inside the repo reports as its repo-relative path and one under HOME as ~/…, on the board, the build and the check (TestTheSiteVerbsReportTheOutputDirectoryWithoutTheHomePath); an absolute path in any of the three would show it wrong diff --git a/.abcd/work/issues/open/iss-2609091647582259-the-memory-board-hides-its-staleness-warning-from-the-reader.md b/.abcd/work/issues/resolved/iss-2609091647582259-the-memory-board-hides-its-staleness-warning-from-the-reader.md similarity index 71% rename from .abcd/work/issues/open/iss-2609091647582259-the-memory-board-hides-its-staleness-warning-from-the-reader.md rename to .abcd/work/issues/resolved/iss-2609091647582259-the-memory-board-hides-its-staleness-warning-from-the-reader.md index d2066965e..329ff88bd 100644 --- a/.abcd/work/issues/open/iss-2609091647582259-the-memory-board-hides-its-staleness-warning-from-the-reader.md +++ b/.abcd/work/issues/resolved/iss-2609091647582259-the-memory-board-hides-its-staleness-warning-from-the-reader.md @@ -9,6 +9,14 @@ found_during: "release-gate" origin: researcher-authored production_mode: hand-written found_at: "internal/core/memory/bare.go" +resolution: "The bare memory board prints every drift line on the text render in the words the --json drift list carries, and each line names abcd memory ingest as the verb that rebuilds the stale file; a current store prints none." +impact: fix +resolved_by: + commit: "989f69a8d" --- The memory store's bare status carries a drift field that says the index is stale and an ingest should be run. It is set on the result, it is emitted under the JSON envelope, and the human render never prints it, so the only reader who can act on the warning is the one who asked for machine output. The person who typed the bare verb to see how the store is doing is shown everything except the one line that asks them to do something. That is the loud-staging principle inverted: a degraded state that announces itself to a parser and stays quiet to a person, which is the shape the principle exists to refuse, and it is worse than silence because the board looks complete. Fix direction: print the drift line in the text render beside the counts it already shows, in the same words the JSON carries, so the two surfaces say one thing. Detector: a store whose index is stale renders the staleness on the bare human board as well as in the JSON envelope, and a store that is current renders neither. + +## Grounds + +- pursued: a store with no index.md shows the index-stale line on the text board and a freshly ingested store shows no stale line on either surface; a stale store whose text board omits a line the JSON carries would show it wrong diff --git a/.abcd/work/issues/open/iss-2609251355497247-the-lifeboat-half-of-iss-2609020539188868-is-still-open.md b/.abcd/work/issues/resolved/iss-2609251355497247-the-lifeboat-half-of-iss-2609020539188868-is-still-open.md similarity index 53% rename from .abcd/work/issues/open/iss-2609251355497247-the-lifeboat-half-of-iss-2609020539188868-is-still-open.md rename to .abcd/work/issues/resolved/iss-2609251355497247-the-lifeboat-half-of-iss-2609020539188868-is-still-open.md index 195dc30c5..c37c9d752 100644 --- a/.abcd/work/issues/open/iss-2609251355497247-the-lifeboat-half-of-iss-2609020539188868-is-still-open.md +++ b/.abcd/work/issues/resolved/iss-2609251355497247-the-lifeboat-half-of-iss-2609020539188868-is-still-open.md @@ -9,6 +9,14 @@ found_during: "autonomous run A resumed 2026-09-25" origin: researcher-authored production_mode: hand-written found_at: "internal/core/lifeboat" +resolution: "Every markdown file the lifeboat writes renders through one discipline (internal/core/lifeboat/mdrender.go): untrusted fields through termsafe.CleanProse, a delimiter only through termsafe.CodeSpan (severity, finding id, evidence refs, packed source paths), and a leading-marker escape on every value that begins a block (principle, subhead, body, quote). The review severity bracket and the press-release subhead emphasis are gone." +impact: fix +resolved_by: + commit: "b156bb1bd" --- The lifeboat half of iss-2609020539188868 is still open after the memory renderers were fixed: synthesis_review renders a finding id through termsafe.Sanitize alone, never CleanProse, so it can still carry an HTML comment opener or link syntax, and wraps a severity in its own bracket; the press-release subhead wraps a cleaned value in its own emphasis; synthesis_principles writes a cleaned principle as a bare paragraph with no leading-marker escape. The fix is the one applied to memory: every untrusted field on a markdown line through CleanProse, and no renderer adding delimiters around a cleaned value (termsafe.CodeSpan where a code span is wanted). + +## Grounds + +- pursued: review, principles, press-release and brief-section renders driven with comment openers, script tags, link syntax and every block-marker lead carry none of them live and open no line with the raw marker; a raw comment opener, script tag or live link in any of the four renders, or a line opening with an untrusted marker, would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609251600023777-site-renderer-a-top-level-three-backtick-fence-indented-one.md b/.abcd/work/issues/resolved/iss-2609251600023777-site-renderer-a-top-level-three-backtick-fence-indented-one.md new file mode 100644 index 000000000..1d863fc85 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609251600023777-site-renderer-a-top-level-three-backtick-fence-indented-one.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609251600023777" +slug: "site-renderer-a-top-level-three-backtick-fence-indented-one" +severity: "minor" +category: "bug" +source: "impl-review" +found_during: "autonomous run A resumed 2026-09-25" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/site/markdown.go" +resolution: "A top-level three-backtick fence indented one to three spaces renders as a fence, its lines losing up to the opener's indent (mdrender indentedFence); the tilde and four-backtick twins stay refused, and an indented opener under prose is refused as a fence without a blank line before it. The shape was live in record pages as the fence of a loose list item." +impact: fix +resolved_by: + commit: "b2fe9f27" +--- + +site renderer: a top-level three-backtick fence indented one to three spaces still renders silently as a paragraph with inline code (site/markdown.go unrenderedFenceRe covers tildes and four or more backticks only). No page in docs/ or site-src/ has the shape today. Refuse it like the other unsupported fence forms, or render it. + +## Grounds + +- pursued: every indent 1-3 renders a command block with dedented code and no paragraph (TestRenderBlockRendersAnIndentedThreeBacktickFence); a

for any of those inputs, or a site-render failure on the record, would show it wrong diff --git a/.abcd/work/issues/open/iss-2609251640452031-the-kill-by-pattern-entries-leave-four-spellings-of-a-kill.md b/.abcd/work/issues/resolved/iss-2609251640452031-the-kill-by-pattern-entries-leave-four-spellings-of-a-kill.md similarity index 60% rename from .abcd/work/issues/open/iss-2609251640452031-the-kill-by-pattern-entries-leave-four-spellings-of-a-kill.md rename to .abcd/work/issues/resolved/iss-2609251640452031-the-kill-by-pattern-entries-leave-four-spellings-of-a-kill.md index d8568f8ec..aaf2c5e9a 100644 --- a/.abcd/work/issues/open/iss-2609251640452031-the-kill-by-pattern-entries-leave-four-spellings-of-a-kill.md +++ b/.abcd/work/issues/resolved/iss-2609251640452031-the-kill-by-pattern-entries-leave-four-spellings-of-a-kill.md @@ -9,6 +9,14 @@ found_during: "autonomous run A resumed 2026-09-25" origin: researcher-authored production_mode: hand-written found_at: "internal/core/guard/defaults/guard.json" +resolution: "The guard reads a kill by the search that fed it and by user or terminal: kill-by-search (args_from over pgrep and pidof, through a substitution or a pipe into xargs), pkill-by-owner and killall-by-owner block the attached selector spellings, and the population selectors no longer consume their value in pkill-by-pattern and killall-by-name. The spellings left unseen are iss-2609262259360005." +impact: fix +resolved_by: + commit: "c3f2e66af" --- The kill-by-pattern entries leave four spellings of a kill by name or selector uncovered: kill handed the output of pgrep in a command substitution, pgrep piped into xargs kill, pkill selecting by user with -u, and pkill selecting by terminal with -t. The first two reach the pattern through a second command the entries do not read; the last two are value flags that consume the selector, so no operand remains, and a kill by user is every session of that user. The same holds when the selector is a command substitution, which fills the value flag's slot and leaves no operand: `pkill -u $(whoami)` (every session of the caller) and `pkill -g $(cat p)` are allowed by design, and review2-guard finding 8 names both. The commit that added the entries named the gap; no record did. Found by review-guard finding 5. + +## Grounds + +- pursued: each of the four spellings, and pkill -u $(whoami), now blocks while pgrep alone, kill of a literal or recorded pid, and pkill -g/-P stay allowed (killspellings_test.go); a kill spelling from the record that still allows, or a near-miss control that blocks, would show it wrong diff --git a/.abcd/work/issues/open/iss-2609251755278758-applyhookplanefailopen-resets-flagerrorfunc-and-args-on-the.md b/.abcd/work/issues/resolved/iss-2609251755278758-applyhookplanefailopen-resets-flagerrorfunc-and-args-on-the.md similarity index 57% rename from .abcd/work/issues/open/iss-2609251755278758-applyhookplanefailopen-resets-flagerrorfunc-and-args-on-the.md rename to .abcd/work/issues/resolved/iss-2609251755278758-applyhookplanefailopen-resets-flagerrorfunc-and-args-on-the.md index 32101d3a6..668962b41 100644 --- a/.abcd/work/issues/open/iss-2609251755278758-applyhookplanefailopen-resets-flagerrorfunc-and-args-on-the.md +++ b/.abcd/work/issues/resolved/iss-2609251755278758-applyhookplanefailopen-resets-flagerrorfunc-and-args-on-the.md @@ -8,6 +8,14 @@ source: "user-observation" found_during: "autonomous run A resumed 2026-09-25" origin: researcher-authored production_mode: hand-written +resolution: "The hook plane wraps the flag-group PreRunE markUsageErrorsExitTwo installs, so a flag-group violation on a hook-reachable command refuses at exit 1 with the skew note, as the flag-parse and argument refusals do." +impact: internal +resolved_by: + commit: "89e98c809" --- applyHookPlaneFailOpen resets FlagErrorFunc and Args on the hook plane but not the PreRunE that markUsageErrorsExitTwo now installs on every command, so a flag group declared on guard hook or hook * in future would refuse at exit 2, the host's BLOCK, bypassing iss-269's fail-open (internal/surface/cli/cli.go:138; hypothetical today: no hook command declares a group; review2-consolidate nit). + +## Grounds + +- pursued: a mutually exclusive pair declared on every hook-reachable runnable command refuses at exit 1 (TestHookPlaneFlagGroupRefusalFailsOpen); an exit 2 there, or guard check losing its exit 2, would show it wrong diff --git a/.abcd/work/issues/open/iss-2609260948440803-local-tier-writes-by-path-memory-lint-writes-its-run-log.md b/.abcd/work/issues/resolved/iss-2609260948440803-local-tier-writes-by-path-memory-lint-writes-its-run-log.md similarity index 65% rename from .abcd/work/issues/open/iss-2609260948440803-local-tier-writes-by-path-memory-lint-writes-its-run-log.md rename to .abcd/work/issues/resolved/iss-2609260948440803-local-tier-writes-by-path-memory-lint-writes-its-run-log.md index d96cc058f..a5fa87822 100644 --- a/.abcd/work/issues/open/iss-2609260948440803-local-tier-writes-by-path-memory-lint-writes-its-run-log.md +++ b/.abcd/work/issues/resolved/iss-2609260948440803-local-tier-writes-by-path-memory-lint-writes-its-run-log.md @@ -9,6 +9,14 @@ found_during: "autonomous run A resumed 2026-09-25: review3-history item 6" origin: researcher-authored production_mode: hand-written found_at: "internal/core/memory/lint.go" +resolution: "memory lint creates its run log through fsutil.CreateRunDir, which proves every level from the checkout root down with EnsureRealDirAll and creates the run directory exclusively, and writes both reports through fsutil.OpenRealDir with WriteFileAtomicInRoot. The sweep fixed the two siblings of the same shape (the issue-drift receipt and the reading assembler's in-repo run directory); launch's pre-flight report, intent audit's review writes, history's moveFile, ahoy's statusline step, the reading ingest stage, banlist and mode already prove or contain the tier." +impact: fix +resolved_by: + commit: "7d5785bef" --- Local-tier writes by path: memory lint writes its run-log report by path into the local tier and follows a symlinked ancestor out of the checkout. Lint (internal/core/memory/lint.go, lintReportDir and the write after it) joins .abcd/.work.local/logs/memory/lint- onto the repo root, os.MkdirAll-s it, and writes report.json and report.md with fsutil.WriteFileAtomic by path. Nothing vets .abcd/.work.local or logs/ first, and a committed symlink beats .gitignore (git add -f), so a checkout that ships .abcd/.work.local as a symlink gets the directory chain and both reports created at the link's target: probed at 35d5cf5f with .abcd/.work.local linked to a directory outside the repo, Lint returned nil and logs/memory/lint-/report.json and report.md were written in the outside directory. The contained pattern for this same tier already exists: mode.SetAt (internal/core/mode/store.go) opens an os.Root on the checkout, root.Lstat-refuses a .abcd/.work.local that is not a real directory, and writes with fsutil.WriteFileAtomicInRoot. Two sites the review named alongside were checked at 35d5cf5f and are NOT in this class: intent/audit.go's review request and dead-letter writes vet .abcd/.work.local/reviews level by level with fsutil.EnsureRealDirAll, and history/location.go's moveFile writes under a chain Resolve proved real with fsutil.EnsureRealDir, so both refuse a symlinked ancestor; each keeps only a vet-by-path-then-write-by-path swap window. + +## Grounds + +- pursued: a checkout whose .abcd/.work.local, logs/ or logs/memory/ is a symlink out of the tree makes lint refuse with ErrNotRealDir and leaves the link target empty, while a real partly present tier still receives both reports; a report or directory appearing at the link target would show it wrong diff --git a/.abcd/work/issues/open/iss-2609261232464351-operand-paths-proved-one-level-only.md b/.abcd/work/issues/resolved/iss-2609261232464351-operand-paths-proved-one-level-only.md similarity index 52% rename from .abcd/work/issues/open/iss-2609261232464351-operand-paths-proved-one-level-only.md rename to .abcd/work/issues/resolved/iss-2609261232464351-operand-paths-proved-one-level-only.md index 42544ab5a..8be9b58fc 100644 --- a/.abcd/work/issues/open/iss-2609261232464351-operand-paths-proved-one-level-only.md +++ b/.abcd/work/issues/resolved/iss-2609261232464351-operand-paths-proved-one-level-only.md @@ -9,6 +9,14 @@ found_during: "autonomous run A resumed 2026-09-25: fix3-cutfix sweep" origin: researcher-authored production_mode: hand-written found_at: "internal/core/lifeboat" +resolution: "Fixed on the drainFS lane: every lifeboat operand (embark target, pack destination, graveyard and synthesis lifeboats) is proved level by level from the checkout it sits in (862809b00), through the shared gitutil.ProveOperandDir that names no absolute path in its refusal (b794114c8); the site output directory already walks every component in resolveOutDir, so it is left as it is. Resolved at the integration, where the record and the fix first meet." +impact: fix +resolved_by: + commit: "862809b00" --- Operand paths in lifeboat (embark, pack, graveyard, synthesis) and the site output directory are proved with a single-level IsRealDir, so a symlinked ancestor of the operand is followed while the leaf check passes; the multi-level fsutil.EnsureRealDirAll / ProbeRealDirAll walk the inbox and the local tier now use is the canonical proof. Sweep each operand site: prove every level below the operand's declared base, or say why the operand is trusted as given. + +## Grounds + +- pursued: a symlinked ancestor of any lifeboat operand inside a checkout is refused before a write; a link at any level below the operand's checkout that is followed would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609261848326365-the-lifeboat-verbs-carry-absolute-directories-into-their.md b/.abcd/work/issues/resolved/iss-2609261848326365-the-lifeboat-verbs-carry-absolute-directories-into-their.md new file mode 100644 index 000000000..3823d488c --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609261848326365-the-lifeboat-verbs-carry-absolute-directories-into-their.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609261848326365" +slug: "the-lifeboat-verbs-carry-absolute-directories-into-their" +severity: "minor" +category: "bug" +source: "agent-finding" +found_during: "autonomous run A resumed 2026-09-25" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/lifeboat/pack.go" +resolution: "Every lifeboat report directory field (pack dest, embark lifeboat_dir and target_dir, graveyard lessons, principles, press-release and review lifeboat_dir) goes through fsutil.RedactHome where the result is built." +impact: fix +resolved_by: + commit: "09f7dcdf" +--- + +The lifeboat verbs carry absolute directories into their --json and text reports: PackResult.Dest (disembark pack), EmbarkPlan and EmbarkResult LifeboatDir and TargetDir (embark probe, embark from), LessonsResult.LifeboatDir (disembark graveyard), and PrinciplesResult, PressReleaseResult and ReviewResult LifeboatDir (disembark principles, press-release, review). A lifeboat or target directory under the home names the developer, and the iss-81 rule is that machine output never carries a developer-identity path; the site verbs' OutDir fields had the same shape (iss-2608291957114882). + +## Grounds + +- pursued: each of the nine fields reports a directory under HOME as ~/… (TestLifeboatReportsNameDirectoriesWithoutTheHomePath, watched failing on all nine at the base); an absolute home path in any lifeboat --json would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609261848338673-launch-render-json-and-launch-ship-s-payload-line-reports.md b/.abcd/work/issues/resolved/iss-2609261848338673-launch-render-json-and-launch-ship-s-payload-line-reports.md new file mode 100644 index 000000000..65a66ea72 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609261848338673-launch-render-json-and-launch-ship-s-payload-line-reports.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609261848338673" +slug: "launch-render-json-and-launch-ship-s-payload-line-reports" +severity: "minor" +category: "bug" +source: "agent-finding" +found_during: "autonomous run A resumed 2026-09-25" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/launch/render.go" +resolution: "PayloadRenderResult keeps the resolved staging directory as its working Dest (json:\"-\"), which the archive step packs from, and reports DisplayDest under the unchanged key dest through fsutil.DisplayPath: the home redacted to ~, a destination always being outside the repository. The text payload line prints the display field." +impact: fix +resolved_by: + commit: "ae58e9260" +--- + +launch ship --json reports the release payload's destination as an absolute path in payload.dest, and the text report prints it on its payload line: PayloadRenderResult.Dest (internal/core/launch/render.go) is the symlink-resolved destination, so a destination under the home names the developer in machine output, against the iss-81 rule the site and lifeboat verbs are held to. + +## Grounds + +- pursued: a render staged under HOME reports payload.dest as ~/staging and its JSON carries neither spelling of the home, while the payload is still written to and packed from the real directory (TestTheRenderAndTheArchiveReportTheirPathsWithoutTheHome); an absolute dest, or an archive packed from the wrong directory, would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609261950061900-memory-lint-reports-its-run-log-directory-and-the-store-it.md b/.abcd/work/issues/resolved/iss-2609261950061900-memory-lint-reports-its-run-log-directory-and-the-store-it.md new file mode 100644 index 000000000..45dd41ead --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609261950061900-memory-lint-reports-its-run-log-directory-and-the-store-it.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609261950061900" +slug: "memory-lint-reports-its-run-log-directory-and-the-store-it" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: review-drainSite" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/memory/lint.go" +resolution: "Lint names report_dir, store_path, coverage_index.path and every finding's file relative to the repository through fsutil.DisplayPath, in the --json result and in the run log's report.json and report.md; the run log is still written to the absolute directory. The finding files and the coverage index path were in the same class and are fixed with the two fields the record names." +impact: fix +resolved_by: + commit: "dc6ff0c53" +--- + +memory lint reports its run-log directory and the store it read as absolute paths: LintResult.ReportDir and LintResult.StorePath (internal/core/memory/lint.go) are joined onto the repository root, so memory lint --json carries report_dir and store_path naming the developer's home whenever the checkout sits under it, against the iss-81 rule; the text report prints ReportDir too. + +## Grounds + +- pursued: a lint of a checkout under HOME reports store_path .abcd/memory, a repository-relative report_dir under which the run log exists, and no absolute finding file, and neither the JSON nor the run log carries the home (TestLintReportsItsPathsRelativeToTheRepository, TestMemoryLintFromSubdirectoryReadsAndReportsInsideTheCheckout); an absolute path in any of them would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609261950066257-the-bare-board-reports-its-directory-as-an-absolute-path-in.md b/.abcd/work/issues/resolved/iss-2609261950066257-the-bare-board-reports-its-directory-as-an-absolute-path-in.md new file mode 100644 index 000000000..3f1c3ce9a --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609261950066257-the-bare-board-reports-its-directory-as-an-absolute-path-in.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609261950066257" +slug: "the-bare-board-reports-its-directory-as-an-absolute-path-in" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: review-drainSite" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/core.go" +resolution: "core.Status reports Dir through fsutil.RedactHome, so abcd --json and the text board name a checkout under the home as ~/...; the inspection still reads the absolute directory. The board has no repository root to be relative to, since the directory is what it reports." +impact: fix +resolved_by: + commit: "c5eab8b89" +--- + +The bare board reports its directory as an absolute path in machine output: abcd --json carries dir as filepath.Abs of the working directory (core.Status, internal/core/core.go, embedded in the board envelope by internal/surface/cli/cli.go), so a checkout under the home names the developer in --json, against the iss-81 rule the site and lifeboat verbs are held to. The text board prints the same field on its first line. + +## Grounds + +- pursued: a checkout under HOME reports dir as ~/src/repo while IsGitRepo is still read from the real directory (TestStatusNamesTheDirectoryWithoutTheHome); an absolute dir in abcd --json would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609261950077063-launch-archive-json-reports-the-archive-it-wrote-as-an.md b/.abcd/work/issues/resolved/iss-2609261950077063-launch-archive-json-reports-the-archive-it-wrote-as-an.md new file mode 100644 index 000000000..12b42fb14 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609261950077063-launch-archive-json-reports-the-archive-it-wrote-as-an.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609261950077063" +slug: "launch-archive-json-reports-the-archive-it-wrote-as-an" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: review-drainSite" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/launch/archive.go" +resolution: "PluginArchive keeps the absolute working Path (json:\"-\"), which the archive verb removes a refused archive through, and reports DisplayPath under the unchanged key path through fsutil.DisplayPath: relative to the repository for the release workflow's --out bin, the home redacted to ~ otherwise. The text written line prints the display field." +impact: fix +resolved_by: + commit: "ae58e9260" +--- + +launch archive --json reports the archive it wrote as an absolute path: PluginArchive.Path (internal/core/launch/archive.go) is the --out directory made absolute by the front door and joined with the archive name, so archive.path names the developer's home whenever --out sits under it, against the iss-81 rule; the text report's written line prints the same value. Found in the drainPaths sweep of path-bearing --json fields. + +## Grounds + +- pursued: an archive written under HOME reports archive.path as ~/dist/ and one written inside the repository as bin/, with the archive still readable where it was written (TestTheRenderAndTheArchiveReportTheirPathsWithoutTheHome, TestAnArchiveInsideTheRepositoryIsReportedRelativeToIt); an absolute archive.path would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609261954288630-launch-dry-run-json-and-launch-ship-json-name-every-payload.md b/.abcd/work/issues/resolved/iss-2609261954288630-launch-dry-run-json-and-launch-ship-json-name-every-payload.md new file mode 100644 index 000000000..c19eff925 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609261954288630-launch-dry-run-json-and-launch-ship-json-name-every-payload.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609261954288630" +slug: "launch-dry-run-json-and-launch-ship-json-name-every-payload" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: review-drainSite" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/launch/bundle.go" +resolution: "IncludedFile keeps the absolute ResolvedPath (json:\"-\") that the render, the gates, the scan and the parity diff read every payload file through, and reports DisplayResolvedPath under the unchanged key resolved_path, relative to the repository, set once where the resolver emits each included file." +impact: fix +resolved_by: + commit: "ae58e9260" +--- + +launch --dry-run --json and launch ship --json name every payload file absolutely: IncludedFile.ResolvedPath (internal/core/launch/bundle.go) is the file's absolute on-disk path and is tagged resolved_path, so the bundle's files list in the dry-run report and in a ship's payload.bundle carries the checkout's absolute path once per file, naming the developer's home whenever the checkout sits under it, against the iss-81 rule. Found in the drainPaths sweep by running the read-only --json verbs from a checkout under the home. + +## Grounds + +- pursued: every bundle file of a render from a checkout under HOME reports a repository-relative resolved_path and the render's JSON carries neither spelling of the home (TestTheRenderAndTheArchiveReportTheirPathsWithoutTheHome); an absolute resolved_path in launch --dry-run --json or in a ship's payload.bundle would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609262148072415-memory-lint-s-run-log-report-md-renderlintreportmd-internal.md b/.abcd/work/issues/resolved/iss-2609262148072415-memory-lint-s-run-log-report-md-renderlintreportmd-internal.md new file mode 100644 index 000000000..6f62bfa9c --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609262148072415-memory-lint-s-run-log-report-md-renderlintreportmd-internal.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609262148072415" +slug: "memory-lint-s-run-log-report-md-renderlintreportmd-internal" +severity: "minor" +category: "security" +source: "user-observation" +found_during: "autonomous run A resumed 2026-09-25" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/memory/lint.go" +resolution: "memory lint's report.md renders the store path, each finding's code, file, message and suggestion through termsafe.CleanProseLine, and sets the store path and each file off with termsafe.CodeSpan." +impact: fix +resolved_by: + commit: "bc51fd4ae" +--- + +memory lint's run-log report.md (renderLintReportMD, internal/core/memory/lint.go) renders a finding's file, message and suggestion through termsafe.Sanitize alone, so a page name or a pii.json pattern name carrying an HTML comment opener or link syntax reaches the markdown report live; the memory renderers fixed for iss-2609020539188868 and the lifeboat renderers fixed for iss-2609251355497247 route the same kind of field through termsafe.CleanProse and set a path off with termsafe.CodeSpan. + +## Grounds + +- pursued: a finding whose file, message and suggestion carry a comment opener, a script tag and link syntax renders none of them live in report.md and its file inside a code span; any of the three appearing live would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609262156124513-history-reconstruct-proves-its-out-directory-with-a.md b/.abcd/work/issues/resolved/iss-2609262156124513-history-reconstruct-proves-its-out-directory-with-a.md new file mode 100644 index 000000000..7f066725d --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609262156124513-history-reconstruct-proves-its-out-directory-with-a.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609262156124513" +slug: "history-reconstruct-proves-its-out-directory-with-a" +severity: "minor" +category: "security" +source: "user-observation" +found_during: "autonomous run A resumed 2026-09-25" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/surface/cli/history_reconstruct.go" +resolution: "history reconstruct proves --out with gitutil.ProveOperandDir, the operand proof the lifeboat verbs use, and writes both files through fsutil.OpenRealDir with WriteFileAtomicInRoot." +impact: fix +resolved_by: + commit: "b2bf22f49" +--- + +history reconstruct proves its --out directory with a leaf-only fsutil.IsRealDir (internal/surface/cli/history_reconstruct.go, writeReconstruction) and then writes the artefact and its telemetry by path, so an --out reached through a committed symlink above the leaf inside a checkout writes both files at the link's target; the lifeboat operands are proved against a symlinked ancestor with gitutil.ProveOperandDir (the operand-paths record captured on main after this branch was cut), and this is the same operand class outside lifeboat. + +## Grounds + +- pursued: an --out reached through a symlink inside the checkout is refused and the link target stays empty, while a plain nested --out receives the artefact and its telemetry; a file at the link target would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609262231500173-reading-assemble-proves-only-a-relative-out-against-a.md b/.abcd/work/issues/resolved/iss-2609262231500173-reading-assemble-proves-only-a-relative-out-against-a.md new file mode 100644 index 000000000..eaad6df48 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609262231500173-reading-assemble-proves-only-a-relative-out-against-a.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609262231500173" +slug: "reading-assemble-proves-only-a-relative-out-against-a" +severity: "major" +category: "security" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: review-drainFS" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/reading/assemble.go" +resolution: "writeArtefacts proves every spelling of --out with gitutil.ProveOperandDir before creating anything; the relative in-repository case keeps EnsureRealDirAll" +impact: fix +resolved_by: + commit: "6412b80d8" +--- + +reading assemble proves only a relative --out against a symlinked ancestor: writeArtefacts treats every ABSOLUTE --out as outside the repository (inRepo := !filepath.IsAbs(outDir) && ValidRelPath(rel)) and hands it to os.MkdirAll as given, so --out naming the checkout's own local tier absolutely, or climbing out and back in (..//...), is followed through a committed symlink at any level and the assembled input and manifest land at the link's target. Absolute is not outside; the sibling verb history reconstruct proves every spelling with gitutil.ProveOperandDir. + +## Grounds + +- pursued: an absolute --out and a ..// --out below a committed local-tier link are both refused with nothing written at the link's target, while an absolute directory outside every checkout is still written; TestAssembleRefusesAnOutSpelledIntoTheCheckoutThroughALink failing on either spelling, or on the outside control, would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609262235543552-gitutil-proveoperanddir-documents-an-absolute-operand-but.md b/.abcd/work/issues/resolved/iss-2609262235543552-gitutil-proveoperanddir-documents-an-absolute-operand-but.md new file mode 100644 index 000000000..ac0493004 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609262235543552-gitutil-proveoperanddir-documents-an-absolute-operand-but.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609262235543552" +slug: "gitutil-proveoperanddir-documents-an-absolute-operand-but" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: review-drainFS" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/gitutil/operand.go" +resolution: "ProveOperandDir absolutises its operand with filepath.Abs before the marker walk, so a relative operand is proved from the checkout it sits in" +impact: internal +resolved_by: + commit: "58df14495" +--- + +gitutil.ProveOperandDir documents an absolute operand but does not enforce it: handed a RELATIVE path, the .git marker walk stops at '.' and never reaches the checkout, so from a subdirectory of a checkout 'alink/x' with alink a symlink out of the checkout is ACCEPTED, and '../link/x' is refused naming the checkout '..'. Every current caller absolutises first, so the gap is latent; the function should absolutise its own input so no caller can bypass the proof. + +## Grounds + +- pursued: from a subdirectory of a checkout, a relative operand below a link out of it and one climbing to a link above the working directory are both refused, the checkout named by its base name; TestProveOperandDirAbsolutisesARelativeOperand accepting either, or naming the checkout '..', would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609262237352137-lifeboat-block-escape-leaves-a-link-reference-definition.md b/.abcd/work/issues/resolved/iss-2609262237352137-lifeboat-block-escape-leaves-a-link-reference-definition.md new file mode 100644 index 000000000..a0cb23f48 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609262237352137-lifeboat-block-escape-leaves-a-link-reference-definition.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609262237352137" +slug: "lifeboat-block-escape-leaves-a-link-reference-definition" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: review-drainFS" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/lifeboat/mdrender.go" +resolution: "escapeLeadingMarker escapes a leading bracket, so no lifeboat block value is parsed as a link reference definition" +impact: fix +resolved_by: + commit: "70fefd58c" +--- + +The lifeboat markdown renderers' escapeLeadingMarker (internal/core/lifeboat/mdrender.go) leaves a leading left square bracket alone, so a principle, press-release subhead, body or quote shaped like `[label]: http://example.com` is emitted as a CommonMark link reference definition: the value itself renders as nothing, and a `[label]` shortcut reference in any other field (the cleaner breaks only the link-text adjacencies) renders as a live link to the attacker's destination. ideate's blockText already escapes the bracket for the same reason. + +## Grounds + +- pursued: a definition-shaped principle, subhead, body or quote renders as its own text and no line of principles.md or press-release.md opens with it, so a shortcut reference elsewhere stays plain text; TestBlockValueNeverDefinesALinkReference seeing the definition consumed by the site renderer, or a line opening with it, would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609262237415400-lifeboat-block-escape-breaks-a-balanced-leading-code-span.md b/.abcd/work/issues/resolved/iss-2609262237415400-lifeboat-block-escape-breaks-a-balanced-leading-code-span.md new file mode 100644 index 000000000..f9cf86bca --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609262237415400-lifeboat-block-escape-breaks-a-balanced-leading-code-span.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609262237415400" +slug: "lifeboat-block-escape-breaks-a-balanced-leading-code-span" +severity: "minor" +category: "security" +source: "agent-finding" +found_during: "autonomous run A resumed 2026-09-25: review-drainFS" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/lifeboat/mdrender.go" +resolution: "escapeLeadingMarker escapes a leading backtick only when its run is unbalanced, so a cleaned value's balanced leading code span keeps sheltering what it quotes" +impact: fix +resolved_by: + commit: "4df17e706" +--- + +The lifeboat markdown renderers' escapeLeadingMarker (internal/core/lifeboat/mdrender.go) backslash-escapes a leading backtick unconditionally, the defect ideate's blockText was fixed for. termsafe's HTML-tag rule exempts a code span, so a value opening with a balanced span that quotes a details tag keeps the tag unbroken; escaping the opening backtick kills the span and republishes the tag as live inline HTML in principles.md or press-release.md, concealing what follows (the site renderer refuses the result as an unclosed code span). Only an unbalanced leading run opens a fence and needs the escape. + +## Grounds + +- pursued: a block value opening with a balanced span quoting a details tag renders the tag inside a code element, while an unbalanced leading run is still escaped; TestBlockValueKeepsABalancedLeadingCodeSpan seeing the site renderer refuse the value, or an unbalanced run left unescaped, would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609262241109876-ideate-block-escape-shows-a-literal-backslash-before-an-ordered-marker.md b/.abcd/work/issues/resolved/iss-2609262241109876-ideate-block-escape-shows-a-literal-backslash-before-an-ordered-marker.md new file mode 100644 index 000000000..20ca825a9 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609262241109876-ideate-block-escape-shows-a-literal-backslash-before-an-ordered-marker.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609262241109876" +slug: "ideate-block-escape-shows-a-literal-backslash-before-an-ordered-marker" +severity: "minor" +category: "bug" +source: "agent-finding" +found_during: "autonomous run A resumed 2026-09-25: review-drainFS" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/ideate/render.go" +resolution: "blockText escapes an ordered-list-shaped idea before its delimiter, so the record shows the idea's own text and opens no list" +impact: fix +resolved_by: + commit: "08efdae8a" +--- + +ideate's blockText (internal/core/ideate/render.go) escapes an ordered-list-shaped idea by putting the backslash before the digits, and CommonMark treats a backslash before a non-punctuation character as literal: an idea reading '1. first' renders as a backslash followed by '1. first', so the verdict record shows text the idea never had. The escape belongs before the '.' or ')' delimiter, as the lifeboat block escaper places it. Found while aligning the two block escapers. + +## Grounds + +- pursued: ideas reading '1. first' and '12) twelve' render through the site renderer as paragraphs carrying exactly that text; TestBlockTextEscapesAnOrderedMarkerFaithfully seeing a list or a literal backslash would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609262309556167-the-site-renderer-reads-any-block-whose-first-line-starts.md b/.abcd/work/issues/resolved/iss-2609262309556167-the-site-renderer-reads-any-block-whose-first-line-starts.md new file mode 100644 index 000000000..27528ea48 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609262309556167-the-site-renderer-reads-any-block-whose-first-line-starts.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609262309556167" +slug: "the-site-renderer-reads-any-block-whose-first-line-starts" +severity: "minor" +category: "bug" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: verify-fix2-drainFS" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/mdrender/render.go" +resolution: "The site renderer opens a fence only where mdrecord's rule opens one: mdrecord exports its single opener predicate (OpensFence, the one Read uses) and every fence test in mdrender and site compose takes it, so a backtick run whose info string holds a backtick renders as the code-span paragraph it is. Both block escapers that trust a balanced leading span (lifeboat escapeLeadingMarker, ideate blockText) are correct with no change of their own." +impact: fix +resolved_by: + commit: "7f99136a6" +--- + +The site renderer reads any block whose first line starts with three backticks as a fence, without the rule mdrecord's block walk applies: a backtick fence's info string may not contain a backtick. So a line that is a balanced code span, not a fence, is mis-rendered: '``` ```' and '```x```' render as an EMPTY command block (the value's text is lost), and '``` x ```' and '```` ``` ````' fail the whole render with an unsupported-construct error. The two block escapers that leave a balanced leading code span unescaped (termsafe.OpensBalancedCodeSpan, CommonMark-correct) inherit the defect: lifeboat's escapeLeadingMarker (introduced on this branch at 4df17e706; at f9b06e720 all four inputs rendered as paragraphs) and ideate's blockText, which has carried the same pre-existing shape since d4b825630. No injection (the info string is attribute-escaped) and no concealment of neighbouring fields. The root is mdrender's fence opener (render.go RenderBlock's first-line and per-line checks, and site compose's code-tab test) spelling its own opener instead of mdrecord's; the fix routes it through mdrecord's one opener predicate. + +## Grounds + +- pursued: the four inputs render as paragraphs carrying their text through site.Renderer, lifeboat mdBlock and ideate blockText, a real three-backtick fence still renders as a command block and a tilde fence is still refused, and a full site build is byte-identical before and after; a committed page whose rendering changed, or a line mdrecord reads as a fence that the renderer now renders as prose, would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609262322244502-the-site-renderer-s-inline-code-span-closes-on-the-first.md b/.abcd/work/issues/resolved/iss-2609262322244502-the-site-renderer-s-inline-code-span-closes-on-the-first.md new file mode 100644 index 000000000..9ca11be64 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609262322244502-the-site-renderer-s-inline-code-span-closes-on-the-first.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609262322244502" +slug: "the-site-renderer-s-inline-code-span-closes-on-the-first" +severity: "minor" +category: "bug" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: fix3-drainFS" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/mdrender/render.go" +resolution: "termsafe.PairCodeSpan is the tree's one code-span pairer (closer: the first later run of exactly the opening length) and termsafe.CodeSpanText applies CommonMark's content rules (a line ending is a space; one space off each side only when both are present and the content is not all spaces). The site renderer, the prose cleaner, OpensBalancedCodeSpan, mdrecord's OpensComment and CodeSpanRanges and the surface appendix pair through it and their private walks are gone; TestNoSecondCodeSpanPairer refuses a new one. One shared table holds the renderer, lifeboat's and ideate's block escapers to the same verdicts. The fence renderer's last-line closer is mdrecord's verdict, not a prefix test. A site build of the same tree differs only by the CommonMark content rules (117 of 2271 files) and every record still renders." +impact: fix +resolved_by: + commit: "e275afcc5" +--- + +The site renderer's inline code span closes on the first occurrence of the opening run's text, not on a run of exactly the same length as CommonMark requires, and it trims every space from a multi-backtick span rather than one from each side. So a span opened by two backticks and holding a, three backticks, b (a CommonMark code span) is refused as an unclosed code span and fails the whole page; this record cannot quote the input literally, because the site renders every ledger record and would refuse it, and a span holding only a space renders empty. termsafe.OpensBalancedCodeSpan and mdrecord's findBacktickRun both pair runs by exact length, so a value a block escaper leaves unescaped as balanced can still be refused by the renderer: loud, not silent, and no text is published wrongly. Confirmed on a scratch copy while fixing iss-2609262309556167; the tree holds three code-span pairers (termsafe closingRun, mdrecord findBacktickRun, mdrender Inline) where one canonical primitive is wanted. + +## Grounds + +- pursued: a value whose leading run termsafe calls balanced renders as that span on the site, and a span opened by two backticks around a run of three renders instead of failing the page; a committed record the site refuses to render, or an escaper and the renderer disagreeing on a row of the shared table, would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609262350446885-two-more-code-span-pairers-than-the-three.md b/.abcd/work/issues/resolved/iss-2609262350446885-two-more-code-span-pairers-than-the-three.md new file mode 100644 index 000000000..9828a8709 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609262350446885-two-more-code-span-pairers-than-the-three.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609262350446885" +slug: "two-more-code-span-pairers-than-the-three" +severity: "minor" +category: "inconsistency" +source: "agent-finding" +found_during: "autonomous run A resumed 2026-09-25: drainSpan" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/lint/lint.go" +resolution: "lint's stripInlineCode and lab's parseCorrections pair backtick runs through termsafe.PairCodeSpan: a double-backtick span is one span, so a synonym or a link quoted in one is a mention, a word no span holds stays live, and a retract literal holding a backtick is read whole. TestNoSecondCodeSpanPairer holds both to the one pairer." +impact: fix +resolved_by: + commit: "da875bc58" +--- + +Two more code-span pairers than the three iss-2609262322244502 names pair backticks one at a time rather than by run. lint's stripInlineCode (GL002 synonyms, links_resolve, citations) pairs single backticks inside longer runs, so a span opened by two backticks reads as two empty spans with live prose between them: a glossary synonym quoted in a double-backtick span is flagged as live prose and a link quoted in one is checked, while a word between a single backtick and a double run no span holds is blanked. lab's parseCorrections closes a quoted literal on the next single backtick, so a retract literal quoted in a double-backtick span because it holds a backtick is read as empty and refused as noise. Both are loud or over-strict rather than silent. Found by the one-pairer guard while fixing iss-2609262322244502. + +## Grounds + +- pursued: a glossary synonym quoted in a double-backtick span raises no GL002 finding and a retract literal quoted in one sweeps as its text; a GL002 finding on such a span, or a double-backtick literal refused as noise, would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609270028388291-the-kill-by-search-reading-loses-a-pipe-that-feeds-a-group.md b/.abcd/work/issues/resolved/iss-2609270028388291-the-kill-by-search-reading-loses-a-pipe-that-feeds-a-group.md new file mode 100644 index 000000000..7569642fe --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609270028388291-the-kill-by-search-reading-loses-a-pipe-that-feeds-a-group.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609270028388291" +slug: "the-kill-by-search-reading-loses-a-pipe-that-feeds-a-group" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: review-drainG2" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/guard/tokenize.go" +resolution: "Every command inside a group reads what is piped into the group, and a shell string's commands read its runner's here-string and redirected process-substitution feeds; TestAPipeIntoAGroupFeedsEveryCommandInIt and TestARedirectIntoAStringReachesItsCommands hold it." +impact: fix +resolved_by: + commit: "a9f16767f" +--- + +The kill-by-search reading loses a pipe that feeds a group or a command string from outside it. A pipe INTO a brace or paren group feeds every command in the group, but the tokenizer hands it only to the commands before the first separator inside, so a process search piped into a group whose later command is an xargs kill (after a sleep, a read, or an and-list) allows silently while the one-command group blocks. In the same class, payloadInput hands a command string only its running shell's pipe, so a here-string or a process-substitution redirect carrying a search into a shell string whose xargs kills allows, while the same redirect into a plain xargs kill blocks. Found by review-drainG2 (tokenize.go group open/close, payload.go payloadInput). + +## Grounds + +- pursued: every group and redirect spelling the review listed blocks as kill-by-search while every NO-LEAK probe of both reviews allows; a group- or redirect-fed kill that allows, or a NO-LEAK probe that blocks, would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609270028432249-the-xargs-wrapper-grammar-in-the-guard-lacks-the-bsd-value.md b/.abcd/work/issues/resolved/iss-2609270028432249-the-xargs-wrapper-grammar-in-the-guard-lacks-the-bsd-value.md new file mode 100644 index 000000000..3437fa1ec --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609270028432249-the-xargs-wrapper-grammar-in-the-guard-lacks-the-bsd-value.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609270028432249" +slug: "the-xargs-wrapper-grammar-in-the-guard-lacks-the-bsd-value" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: review-drainG2" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/guard/match.go" +resolution: "xargs's wrapper grammar names BSD's -J, -R and -S value flags; TestBSDXargsValueFlagsAreStepped holds it." +impact: fix +resolved_by: + commit: "88d92df6d" +--- + +The xargs wrapper grammar in the guard lacks the BSD value flags -J, -R and -S, so on the macOS xargs a search piped into an xargs that uses one of them before a kill reads the flag's value as the launched command and warns as an unrecognised launcher instead of blocking as kill-by-search. Pre-existing; found by review-drainG2 (match.go wrapperValueFlags for xargs). + +## Grounds + +- pursued: a search piped into an xargs using -J, -R or -S before a kill blocks as kill-by-search, and a literal pid through the same flags allows; a warn or allow on the search spelling would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609270036259517-a-stream-piped-into-a-brace-or-paren-group-reaches-only-the.md b/.abcd/work/issues/resolved/iss-2609270036259517-a-stream-piped-into-a-brace-or-paren-group-reaches-only-the.md new file mode 100644 index 000000000..4b9121c5b --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609270036259517-a-stream-piped-into-a-brace-or-paren-group-reaches-only-the.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609270036259517" +slug: "a-stream-piped-into-a-brace-or-paren-group-reaches-only-the" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: review-drainG2" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/guard/tokenize.go" +resolution: "A shell after a separator inside a piped group reads the pipe as a stream (stdinStream from the group input); TestAPipeIntoAGroupFeedsEveryCommandInIt holds it." +impact: fix +resolved_by: + commit: "a9f16767f" +--- + +A stream piped into a brace or paren group reaches only the group's commands before its first separator as a stream, so a shell that reads its script from standard input placed after a separator inside a piped group allows under interpreter-reads-stream, while the same shell as the group's first command blocks. Same root as iss-2609270028388291 (the group's input is not handed to every command in it), found while fixing it. + +## Grounds + +- pursued: a stream piped into a group reaches a stdin-reading shell anywhere in it as interpreter-reads-stream; a group spelling where it allows would show it wrong diff --git a/CONTRIBUTING.md b/.github/CONTRIBUTING.md similarity index 92% rename from CONTRIBUTING.md rename to .github/CONTRIBUTING.md index 1a46d4ec4..c7aba5290 100644 --- a/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -1,13 +1,13 @@ # Contributing -abcd is a public project under active development. See [`AGENTS.md`](AGENTS.md) +abcd is a public project under active development. See [`AGENTS.md`](../AGENTS.md) for build/test/checks and working conventions, and -[`.abcd/development/`](.abcd/development/) for the design record. +[`.abcd/development/`](../.abcd/development/) for the design record. ## Licence Contributions are accepted under the project's licence, inbound = outbound: by -submitting a change you agree it is licensed under the [MIT licence](LICENSE) +submitting a change you agree it is licensed under the [MIT licence](../LICENSE) like the rest of the project, and that you are entitled to submit it under that licence. There is no CLA and no `Signed-off-by:` requirement — a plain inbound = outbound statement is the whole of it. @@ -34,17 +34,17 @@ inbound = outbound statement is the whole of it. outside the queue until its branch is updated. `scripts/pr-keep-current.sh` performs that update for every armed pull request (`--watch` repeats until none is armed); run it after arming auto-merge, and after every merge that - moves `main`. A pull request confined to `docs/`, - `.abcd/development/`, `.abcd/work/` and the root prose files stands the macOS - leg, the race lane and the `zizmor`, `govulncheck` and smoke lanes down while - it is in review; the queue run is not a pull-request event, so the full set - gates the merge either way. + moves `main`. A pull request confined to `docs/`, `.abcd/development/`, + `.abcd/work/`, the root prose files, and this guide and the security policy + beside it in `.github/` stands the macOS leg, the race lane and the `zizmor`, + `govulncheck` and smoke lanes down while it is in review; the queue run is not + a pull-request event, so the full set gates the merge either way. - **Publish surface reviews.** Paths listed in - [`.github/CODEOWNERS`](.github/CODEOWNERS) ship behaviour to installed users + [`.github/CODEOWNERS`](CODEOWNERS) ship behaviour to installed users (plugin hooks and commands, agent prompts, workflows, gates and build config). Changes there additionally require a code-owner review. The applied branch rulesets are mirrored under - [`.abcd/work/rulesets/`](.abcd/work/rulesets/). + [`.abcd/work/rulesets/`](../.abcd/work/rulesets/). - **Volume cap.** At most three open pull requests per external author at a time — review attention is the scarce resource this protects. - **Local gates.** `make preflight` runs the load check first (load-check, a @@ -58,7 +58,7 @@ inbound = outbound statement is the whole of it. (`make fmt` applies it); when the declared toolchain cannot be fetched, preflight refuses and names the skew rather than falling back to the `go` on PATH. The repository - ships its hooks in [`.githooks/`](.githooks/); they are per-machine opt-in — + ships its hooks in [`.githooks/`](../.githooks/); they are per-machine opt-in — run `git config core.hooksPath .githooks` once per clone to arm the pre-commit name guard (it reads this machine's private banlist, `.abcd/.work.local/private-names.txt`, which `abcd banlist add --private` @@ -82,7 +82,7 @@ inbound = outbound statement is the whole of it. that means `abcd spec close ` on every spec still open that names it. - **Docs** are Diátaxis (one type per page, present tense); the design record lives under `.abcd/`, never in `docs/`. Prose follows the canonical - [writing style guide](docs/reference/writing-style.md). + [writing style guide](../docs/reference/writing-style.md). - **New dependencies need explicit maintainer sign-off** before they land in `go.mod`. - **Run the plugin from your checkout.** The marketplace lists one plugin, and @@ -161,7 +161,7 @@ a public issue for a security finding. ## Acknowledgements -[`ACKNOWLEDGEMENTS.md`](ACKNOWLEDGEMENTS.md) credits the ideas, tools, and writing +[`ACKNOWLEDGEMENTS.md`](../ACKNOWLEDGEMENTS.md) credits the ideas, tools, and writing behind abcd in three parts — development, inspirations, and references. Add an entry **in the same change that lands it**: the PR that adopts an external pattern, cites a source in an ADR, or integrates a tool. Adding it at the moment it lands is what diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 4371f53f0..fa030649d 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -15,6 +15,6 @@ Assisted-by: None (no AI touched this change) The gate refuses a missing or mid-sentence trailer, and refuses tool - footers ("generated with ..."). See CONTRIBUTING.md § AI assistance. --> + footers ("generated with ..."). See .github/CONTRIBUTING.md § AI assistance. --> Assisted-by: diff --git a/SECURITY.md b/.github/SECURITY.md similarity index 100% rename from SECURITY.md rename to .github/SECURITY.md diff --git a/.github/workflows/attribution.yml b/.github/workflows/attribution.yml index 541ea43d9..db03995fc 100644 --- a/.github/workflows/attribution.yml +++ b/.github/workflows/attribution.yml @@ -3,7 +3,7 @@ name: attribution # Fails a pull request whose commits or body break abcd's AI-attribution # convention: the kernel trailer `Assisted-by: Claude:`, never # `Co-Authored-By:` for an AI, never a tool's own "Generated with " footer -# (AGENTS.md § Attribution and acknowledgements, CONTRIBUTING.md). +# (AGENTS.md § Attribution and acknowledgements, .github/CONTRIBUTING.md). # # It also refuses a LIVE AGENT-SESSION URL in any commit message in the range or # in the pull-request body. That half was gated NOWHERE until it was added — not diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 091a4f398..88bd3cbcc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -136,12 +136,14 @@ jobs: # The inert allowlist: user-facing documentation, the durable record, # and the shared working tier, plus the root files that are prose or - # licence text. The harness routers (AGENTS.md, CLAUDE.md, GEMINI.md) - # are deliberately absent — they configure agent behaviour. + # licence text and the two community-health prose files in .github/. + # The harness routers (AGENTS.md, CLAUDE.md, GEMINI.md) are + # deliberately absent — they configure agent behaviour. is_inert_path() { case "$1" in docs/*|.abcd/development/*|.abcd/work/*) return 0 ;; - README.md|CHANGELOG.md|RELEASE.md|CONTRIBUTING.md|SECURITY.md|ACKNOWLEDGEMENTS.md|LICENSE|LICENSE.md) return 0 ;; + README.md|CHANGELOG.md|RELEASE.md|ACKNOWLEDGEMENTS.md|LICENSE|LICENSE.md) return 0 ;; + .github/CONTRIBUTING.md|.github/SECURITY.md) return 0 ;; *) return 1 ;; esac } diff --git a/ACKNOWLEDGEMENTS.md b/ACKNOWLEDGEMENTS.md index 63a7584e0..b0ef1d8fa 100644 --- a/ACKNOWLEDGEMENTS.md +++ b/ACKNOWLEDGEMENTS.md @@ -13,7 +13,7 @@ they live in `go.mod` and the licence notices they carry. Development of abcd has been assisted by Claude Code (Anthropic). Per-commit disclosure uses an `Assisted-by:` trailer; the human contributor is the author of record and is responsible for all AI-assisted output — its correctness, licensing, -and fit for the project. See [`CONTRIBUTING.md`](CONTRIBUTING.md). +and fit for the project. See [`CONTRIBUTING.md`](.github/CONTRIBUTING.md). External reports sharpen the record, and fix commits credit their reporters with a `Reported-by:` trailer. [Andy Woods (@andytwoods)](https://github.com/andytwoods) diff --git a/AGENTS.md b/AGENTS.md index 2026bd4d0..4af5abed7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -163,9 +163,10 @@ site-render gate on the Linux leg alone. Separate jobs run the reviews-charter c (`make smoke`). A fail-closed classifier stands the macOS leg, the race lane and the `zizmor`, `govulncheck` and smoke jobs down on a pull request confined to `docs/`, -`.abcd/development/`, `.abcd/work/` and the root prose files; the Linux unit -lane, the format gate and the record gates always run, and every other event — -the merge-queue entry that gates the merge included — runs the lot. +`.abcd/development/`, `.abcd/work/`, the root prose files and the +community-health files in `.github/`; the Linux unit lane, the format gate and +the record gates always run, and every other event — the merge-queue entry that +gates the merge included — runs the lot. ## Working-tree layout (three tiers under `.abcd/`) @@ -398,7 +399,7 @@ irreversible; guessing downward costs nothing.** `Co-Authored-By:` for AI (it asserts an authorship the tool does not hold and inflates the contributor graph). There is no DCO: contributions are inbound = outbound MIT, so no `Signed-off-by:` is required (adr-43). The human is the - author of record, responsible for all AI-assisted output. See `CONTRIBUTING.md`. + author of record, responsible for all AI-assisted output. See `.github/CONTRIBUTING.md`. - **Every commit is authored by a human, and the gate refuses a machine.** The contributor graph is built from the author and committer fields, so a machine there asserts an authorship it does not hold — and a squash merge re-appends a diff --git a/README.md b/README.md index 70a9fc4cd..7d44c5272 100644 --- a/README.md +++ b/README.md @@ -152,5 +152,5 @@ repository* button reads: ## Resources - [`LICENSE`](LICENSE): MIT. -- [`SECURITY.md`](SECURITY.md): Report a vulnerability privately. +- [`SECURITY.md`](.github/SECURITY.md): Report a vulnerability privately. - [`ACKNOWLEDGEMENTS.md`](ACKNOWLEDGEMENTS.md): The ideas, tools, and writing `abcd` stands on. diff --git a/commands/abcd.md b/commands/abcd.md index 8788d7413..bb92d7297 100644 --- a/commands/abcd.md +++ b/commands/abcd.md @@ -15,7 +15,8 @@ Run: "${CLAUDE_PLUGIN_ROOT}/abcd" --json ``` -Then summarise the JSON for the user: the directory, whether it is a git repo, +Then summarise the JSON for the user: the directory (`dir`, with the home +directory written as `~`), whether it is a git repo, whether the abcd development record is present, and which `.abcd/` work tiers exist. diff --git a/commands/disembark.md b/commands/disembark.md index 4f873e2f6..2ba89040e 100644 --- a/commands/disembark.md +++ b/commands/disembark.md @@ -89,7 +89,7 @@ Each positional argument is a probe report emitted with `probe --json`. Summarise the JSON result for the user: -- `dest` — where the lifeboat was written. +- `dest` — where the lifeboat was written, with the home directory as `~`. - `files_written` / `bytes_written` — the size of the lifeboat. - `manifest_sha256` — the pinned hash over every file (matches `/_provenance.json`). - `voyage_appended` — whether the operator-level voyage ledger recorded the pack @@ -101,8 +101,11 @@ Summarise the JSON result for the user: The **destination safety gate** protects real work. A pack refuses unless `` is absent, an empty directory, or an existing lifeboat abcd produced (it carries a -parseable `_provenance.json`). It also refuses a symlinked destination, one inside -a `.git/` directory, or one that overlaps the source tree. And it **refuses on a +parseable `_provenance.json`). It also refuses a symlinked destination, a +destination reached through a symlink at any level inside a checkout (a +committed link; outside every checkout the path is taken as given), one inside +a `.git/` directory, or one that overlaps the source tree. The lifeboat operand +of every later verb is proved the same way. And it **refuses on a hard-fail secret** in the planned bytes — a secret is fixed at source, never redacted into the artefact. Relay the refusal message so the user knows what to fix. diff --git a/commands/guard.md b/commands/guard.md index 42800c129..4cfda6835 100644 --- a/commands/guard.md +++ b/commands/guard.md @@ -273,6 +273,29 @@ the repository's hooks exactly as `--no-verify` does, and blocks under the same entries whatever the value, because the guard cannot tell a directory of real hooks from an empty one. Setting the key with `git config` is not refused. +A kill is read with where its pids come from. A `kill` handed what a process +search prints — `kill $(pgrep -f make)`, `pgrep -f make | xargs kill`, `kill +$(pidof make)` — is a **block** (`kill-by-search`), because it signals every +matching process on the machine, as `pkill` does; a kill of a pid you name, one +you recorded (`kill $(cat pidfile)`), or a search of your own group (`kill +$(pgrep -g )`) is not. The search is followed through a group (`{ pgrep +…; } | xargs kill`) and into one, to every command in it (`pgrep … | { sleep 1; +xargs kill; }`), through a shell string that runs it (`kill $(sh -c 'pgrep +…')`), and into a shell string that `xargs` runs or that reads the pipe or a +redirect (`pgrep … | xargs sh -c 'kill "$@"' _`, `pgrep … | sh -c 'xargs +kill'`, `sh -c 'xargs kill' < <(pgrep …)`); behind `xargs` and a launcher the +guard does not know, the fail-safe warns. Every command of a string `xargs` +runs, and every command in a group a pipe feeds, is read as handed that input, +so `pgrep … | xargs sh -c 'kill 4242'` is a **block** too: the guard does not +read which of the commands uses it. A `pkill` or `killall` that selects by user, group or terminal +(`-u`, `-t`, and `pkill`'s `-U` and `-G`, written apart from the value or +against it, as `pkill -tpts/3`) is a **block** too, under `pkill-by-owner`, +`killall-by-owner` or the entry for a kill by name, because every session under +the account is among what it selects. `pkill`'s signal is read as a signal +first, in any case and with or without its `SIG` prefix, as `pkill` reads it, +so `pkill -term -g ` stops a group and stays allowed. A pid list carried +through a variable or a file, or taken from a `ps | grep` chain, is not seen. + What an allow still does not see is a hazard that never reaches command position at all: one launched through a known wrapper carrying a value-taking flag the guard does not name (`sudo -u bob ` is seen; the bundled short form diff --git a/commands/history.md b/commands/history.md index 951ff35a7..a2688c9dd 100644 --- a/commands/history.md +++ b/commands/history.md @@ -320,7 +320,9 @@ artefact would not fit the context it is being read into. `--max-block-bytes` caps one rendered tool input or result; what it removes is marked where it happens and counted in the telemetry. -`--out` defaults to the working directory. In a repo that follows abcd's +`--out` defaults to the working directory. It must already exist, and it is +refused when it is reached through a symlink at any level inside a checkout +(a committed link would carry both files elsewhere). In a repo that follows abcd's three-tier layout, write into `.abcd/.work.local/scratch/` rather than the repo root — a reconstruction is a derived artefact, and the root is not where derived artefacts belong. diff --git a/commands/launch.md b/commands/launch.md index 7364ed9f5..22bd0644d 100644 --- a/commands/launch.md +++ b/commands/launch.md @@ -237,6 +237,7 @@ Then summarise the JSON for the user: that tree are excluded (`symlink`): an archive carries a link as the path it names, not as content. - `bundle.files` — the files the bundle would include (an array; report its length as the count). + Each entry names its file relative to the repository, `resolved_path` included. - `scan.hard_fails` — secret/PII findings that would block the release. `scan.findings` keeps at most 10,000 of them; `scan.findings_omitted`, when present, counts the rest, and `scan.hard_fails` counts every one. @@ -556,7 +557,8 @@ stamped into the payload's copies of `plugin.json` and `marketplace.json`. The repository's own manifests are never touched: they carry no version, and the version belongs to the artefact. The staged payload is proved consistent before the command returns, so a stamp that missed a pinned location is a refusal rather -than a published half-state. Every refusal the staging step can make is checked +than a published half-state. The report's `payload.dest` names that directory +with the home directory written as `~`. Every refusal the staging step can make is checked BEFORE the dated heading is written, and a refusal that slips past that check rolls the heading back — so a ship that exits non-zero leaves no release record behind for the next attempt to trip over. Without the flag nothing is staged; @@ -792,7 +794,9 @@ heading names, from the checked-out tree, and writes `-plugin-vX.Y.Z.zip` into an existing directory. The archive is reproducible — sorted entries, stored uncompressed, one fixed timestamp, modes normalised — so the same commit renders the same bytes on any machine. The -catalog is left out of it, because the catalog is what names its digest. +catalog is left out of it, because the catalog is what names its digest. The +report's `archive.path` names the written archive relative to the repository +when `--out` is inside it, and with the home directory as `~` otherwise. ```bash "${CLAUDE_PLUGIN_ROOT}/abcd" launch archive --out

[--tag vX.Y.Z] [--verify] [--repository ] --json diff --git a/commands/memory.md b/commands/memory.md index 413ced247..438e4f5a4 100644 --- a/commands/memory.md +++ b/commands/memory.md @@ -29,8 +29,10 @@ of the checkout's store. ``` Summarise the JSON: `pages` and `by_class` (page count per source class), -`last_ingest`, any `contradictions`, and per-source `headroom` lines. The bare -render never rebuilds or mutates the coverage index. +`last_ingest`, any `contradictions`, per-source `headroom` lines, and every +`drift` line verbatim — each says the index or the contradictions register is +stale and names the ingest that rebuilds it. The bare render never rebuilds or +mutates the coverage index. ## Ingest a source @@ -95,7 +97,8 @@ and the line, never the span, and lint never rewrites the store): ``` It rebuilds the regenerable `.coverage_index.json` and writes a report under -`.abcd/.work.local/logs/memory/lint-/`. Summarise `summary.blockers` / +`.abcd/.work.local/logs/memory/lint-/`. `report_dir`, `store_path` and each +finding's `file` are named relative to the repository. Summarise `summary.blockers` / `summary.warnings` / `summary.infos` and each finding's `code` and `message`. Blockers exit nonzero; warn-only exits 0. diff --git a/commands/reading.md b/commands/reading.md index 4fc9c1f61..29aef0c8c 100644 --- a/commands/reading.md +++ b/commands/reading.md @@ -237,6 +237,10 @@ manifest describes half of what is in it. Both files are written through a temporary name and renamed into place, so a reader never opens a half-written bundle. +An output directory reached through a symlink at any level inside a checkout is +refused, however `--out` spells it: a committed link would carry both files +elsewhere. Outside every checkout the path is the operator's own. + ### The host obligation this binary cannot discharge The assembled input carries no repository path: each item is an ordinal key, a diff --git a/commands/site.md b/commands/site.md index f46b1b9cb..2afe60db6 100644 --- a/commands/site.md +++ b/commands/site.md @@ -32,7 +32,8 @@ emits `{ "manifest": …, "ui_strings": …, "baseline": …, "out_dir": … }`: - `baseline` and `baseline_entries` — the committed unresolved-reference ratchet and its size. - `version`, `commit` — what a render would stamp the footer with. -- `out_dir`, `out_exists`, `out_files` — where a render writes, and what is +- `out_dir`, `out_exists`, `out_files` — where a render writes (relative to the + repository inside it, with the home directory as `~` outside it), and what is there now. Report the declared inputs first, then the output directory's state. It writes @@ -49,11 +50,12 @@ reads exactly this set — `.abcd/site.json`, `site-src/ui.json`, `.abcd/development/` and the opted-in issue ledger, git history, `CHANGELOG.md`, the composed pages and assets under `docs/`, the static inputs `site-src/{site.css,site.js,record.js,redirects,headers}` and the served -`site-src/install.sh.tmpl`, the credit sources `CONTRIBUTING.md` and -`ACKNOWLEDGEMENTS.md` (and the existence of `SECURITY.md` and `CITATION.cff` -for the footer), `.abcd/site-baseline.json` (the ratchet the health block -counts against), and `.claude-plugin/plugin.json` (the forge URL, licence and -author the links and footer use) — and writes the landing page, the record +`site-src/install.sh.tmpl`, the credit sources `.github/CONTRIBUTING.md` and +`ACKNOWLEDGEMENTS.md` (and the existence of `SECURITY.md` — in `.github/`, at +the root or in `docs/` — and `CITATION.cff` for the footer), +`.abcd/site-baseline.json` (the ratchet the health block counts against), and +`.claude-plugin/plugin.json` (the forge URL, licence and author the links and +footer use) — and writes the landing page, the record export, the redirect and header maps, the stylesheet, the two scripts, the `install.sh`, and every referenced raster into the output directory, and nowhere else. It reaches no network. The default output directory is `site`, diff --git a/internal/core/banlist/public_test.go b/internal/core/banlist/public_test.go index bbca53b8e..999bf449e 100644 --- a/internal/core/banlist/public_test.go +++ b/internal/core/banlist/public_test.go @@ -134,7 +134,7 @@ func TestAddPublicEntryGatesUserFacingContent(t *testing.T) { // that an unresolvable configured root fails loud (GitHub #360). write("README.md", "# readme\n") // Its name_roots must resolve too (iss-279). - for _, r := range []string{".abcd/README.md", "AGENTS.md", "CONTRIBUTING.md", "scripts/README.md"} { + for _, r := range []string{".abcd/README.md", "AGENTS.md", ".github/CONTRIBUTING.md", "scripts/README.md"} { write(r, "# t\n") } provisionDocsLintTrees(t, cfg, docs) @@ -439,7 +439,7 @@ func TestAddPublicIsCaseInsensitiveLikeTheCuratedEntries(t *testing.T) { t.Fatal(err) } // Its name_roots must resolve too (iss-279). - for _, r := range []string{".abcd/README.md", "AGENTS.md", "CONTRIBUTING.md", "scripts/README.md"} { + for _, r := range []string{".abcd/README.md", "AGENTS.md", ".github/CONTRIBUTING.md", "scripts/README.md"} { p := filepath.Join(docs, filepath.FromSlash(r)) if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { t.Fatal(err) diff --git a/internal/core/capture/drift.go b/internal/core/capture/drift.go index 013438f18..c58022a1a 100644 --- a/internal/core/capture/drift.go +++ b/internal/core/capture/drift.go @@ -3,7 +3,6 @@ package capture import ( "encoding/json" "fmt" - "os" "path/filepath" "sort" "strings" @@ -168,28 +167,26 @@ func IssueDrift(req IssueDriftRequest) (IssueDriftResult, error) { // writeDriftReceipt allocates this run's receipt directory and writes the // report into it, returning its repo-relative path. func writeDriftReceipt(repoRoot string, now time.Time, res *IssueDriftResult) (string, error) { - base := filepath.Join(repoRoot, driftReceiptRelDir) - stamp := "issue-drift-" + now.UTC().Format("20060102T150405Z") - dir := filepath.Join(base, stamp) - for n := 1; ; n++ { - if _, err := os.Lstat(dir); os.IsNotExist(err) { - break - } - if n >= 1000 { - return "", fmt.Errorf("issue drift: could not allocate a unique receipt directory for %s", stamp) - } - dir = filepath.Join(base, fmt.Sprintf("%s-%03d", stamp, n)) - } - if err := os.MkdirAll(dir, 0o755); err != nil { + // The local tier's run-log create path: every level from the checkout root + // down is proved real before the receipt directory is created under it, so a + // tier the checkout carries as a committed symlink is refused rather than + // followed out of the checkout. + dir, err := fsutil.CreateRunDir(repoRoot, filepath.ToSlash(driftReceiptRelDir), + "issue-drift-"+now.UTC().Format("20060102T150405Z"), 0o755) + if err != nil { return "", fmt.Errorf("issue drift: %w", err) } - path := filepath.Join(dir, "report.json") - res.ReceiptPath = fsutil.RepoRel(repoRoot, path) + res.ReceiptPath = fsutil.RepoRel(repoRoot, filepath.Join(dir, "report.json")) data, err := json.MarshalIndent(res, "", " ") if err != nil { return "", err } - if err := fsutil.WriteFileAtomicPreserveMode(path, append(data, '\n')); err != nil { + root, err := fsutil.OpenRealDir(dir) + if err != nil { + return "", fmt.Errorf("issue drift: %w", err) + } + defer root.Close() + if err := fsutil.WriteFileAtomicInRoot(root, "report.json", append(data, '\n'), 0o644); err != nil { return "", fmt.Errorf("issue drift: %w", err) } return res.ReceiptPath, nil diff --git a/internal/core/capture/drift_localtier_test.go b/internal/core/capture/drift_localtier_test.go new file mode 100644 index 000000000..d261ce129 --- /dev/null +++ b/internal/core/capture/drift_localtier_test.go @@ -0,0 +1,72 @@ +package capture + +import ( + "errors" + "os" + "path/filepath" + "testing" + "time" + + "github.com/intentdriven/abcd/internal/fsutil" +) + +// The issue-drift receipt is written into the checkout's local tier, and a +// checkout can carry any level of that tier as a committed symlink. A by-path +// MkdirAll followed such a link out of the checkout and wrote the receipt at its +// target — the sibling of iss-2609260948440803's memory lint write. Every level +// is proved real before the receipt directory is created under it. + +// TestIssueDriftRefusesASymlinkedLocalTierAncestor plants the link at each level +// of the receipt chain and holds that the check refuses and writes nothing at +// the link's target. +func TestIssueDriftRefusesASymlinkedLocalTierAncestor(t *testing.T) { + for _, level := range []string{".abcd/.work.local", ".abcd/.work.local/logs", ".abcd/.work.local/logs/audit"} { + t.Run(level, func(t *testing.T) { + repo, ir := driftFixture(t) + outside := t.TempDir() + link := filepath.Join(repo, filepath.FromSlash(level)) + if err := os.MkdirAll(filepath.Dir(link), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, link); err != nil { + t.Fatal(err) + } + + _, err := IssueDrift(IssueDriftRequest{RepoRoot: repo, IssuesRoot: ir, Now: time.Date(2026, 9, 23, 12, 0, 0, 0, time.UTC)}) + if err == nil { + t.Fatalf("issue drift with %s symlinked out of the checkout returned nil; it must refuse", level) + } + if !errors.Is(err, fsutil.ErrNotRealDir) { + t.Errorf("issue drift refused for the wrong reason: %v", err) + } + if entries, _ := os.ReadDir(outside); len(entries) != 0 { + t.Errorf("issue drift wrote %d entr(y|ies) at the link's target outside the checkout", len(entries)) + } + }) + } +} + +// TestIssueDriftWritesItsReceiptUnderARealNestedTier is the control: a real +// tier, partly present, still receives the receipt where ReceiptPath says, and a +// second run in the same instant gets a directory of its own. +func TestIssueDriftWritesItsReceiptUnderARealNestedTier(t *testing.T) { + repo, ir := driftFixture(t) + if err := os.MkdirAll(filepath.Join(repo, ".abcd", ".work.local", "logs"), 0o755); err != nil { + t.Fatal(err) + } + now := time.Date(2026, 9, 23, 12, 0, 0, 0, time.UTC) + seen := map[string]bool{} + for i := 0; i < 2; i++ { + res, err := IssueDrift(IssueDriftRequest{RepoRoot: repo, IssuesRoot: ir, Now: now}) + if err != nil { + t.Fatalf("issue drift under a real tier: %v", err) + } + if seen[res.ReceiptPath] { + t.Fatalf("two runs in one instant share the receipt %s", res.ReceiptPath) + } + seen[res.ReceiptPath] = true + if fi, err := os.Lstat(filepath.Join(repo, filepath.FromSlash(res.ReceiptPath))); err != nil || !fi.Mode().IsRegular() { + t.Errorf("the receipt %s is not a regular file: %v", res.ReceiptPath, err) + } + } +} diff --git a/internal/core/core.go b/internal/core/core.go index 6137f28eb..85cb03510 100644 --- a/internal/core/core.go +++ b/internal/core/core.go @@ -9,6 +9,8 @@ package core import ( "os" "path/filepath" + + "github.com/intentdriven/abcd/internal/fsutil" ) // Version is abcd's version, stamped at build time via -ldflags -X (see the @@ -29,6 +31,10 @@ func NewVersion() VersionInfo { // StatusInfo is the result of Status: a read-only "where am I" snapshot of a // directory, mirroring abcd's bare-invocation status convention (never mutates). type StatusInfo struct { + // Dir is the inspected directory with the home redacted to "~": it travels + // into --json, and machine output never carries an absolute + // developer-identity path (iss-81, iss-2609261950066257). It is display + // only; the inspection reads the directory itself. Dir string `json:"dir"` IsGitRepo bool `json:"is_git_repo"` HasRecord bool `json:"has_record"` // .abcd/development present @@ -43,7 +49,7 @@ func Status(dir string) (StatusInfo, error) { return StatusInfo{}, err } s := StatusInfo{ - Dir: abs, + Dir: fsutil.RedactHome(abs), // .git is a directory in a normal clone but a regular gitfile in a linked // worktree or submodule — both are genuine checkouts, so test existence, not // dir-ness. HasRecord/WorkTiers stay dir-only (those must be directories). diff --git a/internal/core/core_test.go b/internal/core/core_test.go index 52c600e74..e17de9c50 100644 --- a/internal/core/core_test.go +++ b/internal/core/core_test.go @@ -83,3 +83,25 @@ func contains(ss []string, want string) bool { } return false } + +// The board names its directory with the home redacted to "~": a checkout under +// the home named the developer in `abcd --json`, against the iss-81 rule +// (iss-2609261950066257). The text board prints the same field. +func TestStatusNamesTheDirectoryWithoutTheHome(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + dir := filepath.Join(home, "src", "repo") + mustMkdir(t, filepath.Join(dir, ".git")) + + s, err := Status(dir) + if err != nil { + t.Fatal(err) + } + if want := filepath.Join("~", "src", "repo"); s.Dir != want { + t.Errorf("Status.Dir = %q, want %q", s.Dir, want) + } + // The directory is still inspected where it is, not where it is displayed. + if !s.IsGitRepo { + t.Error("the redacted directory must not change what is inspected: IsGitRepo = false") + } +} diff --git a/internal/core/guard/config.go b/internal/core/guard/config.go index 8fee97821..2d8d7e204 100644 --- a/internal/core/guard/config.go +++ b/internal/core/guard/config.go @@ -270,6 +270,9 @@ func mergePattern(base, over Pattern) Pattern { if over.MinOperands != 0 { r.MinOperands = over.MinOperands } + if over.ArgsFrom != nil { + r.ArgsFrom = clonePatterns(over.ArgsFrom) + } // AfterCD is a pointer precisely so an override can set it to false — a // bool field could only ever tighten the requirement, never lift it. if over.AfterCD != nil { diff --git a/internal/core/guard/defaults/guard.json b/internal/core/guard/defaults/guard.json index 34e6c541b..f0788252b 100644 --- a/internal/core/guard/defaults/guard.json +++ b/internal/core/guard/defaults/guard.json @@ -269,17 +269,19 @@ "tier": "blocker", "pattern": { "command": "pkill", - "value_flags": ["-P", "--parent", "-g", "--pgroup", "-G", "--group", "-s", "--session", "-u", "--euid", "-U", "--uid", "-t", "--terminal", "-F", "--pidfile", "--signal", "--ns", "--nslist", "-j", "-M", "-N"], + "value_flags": ["-P", "--parent", "-g", "--pgroup", "-s", "--session", "-F", "--pidfile", "--signal", "--ns", "--nslist", "-j", "-M", "-N"], "min_operands": 1 }, - "why": "`pkill` signals every process on the machine whose name matches the pattern — with `-f`, whose whole command line does — so a pattern meant for your own run also stops every other session's run of the same command.", + "why": "`pkill` signals every process on the machine whose name matches the pattern — with `-f`, whose whole command line does, and with `-u`, `-U`, `-t` or `-G`, every process of that user, terminal or group — so a selection meant for your own run also stops every other session's run of the same command.", "successor": "Stop the process you started by the pid you recorded when you started it (`kill \"$pid\"`), or stop its own process group (`kill -- -\"$pgid\"`), so no other session's process is touched.", "fixtures": { "known_bad": [ "pkill -f 'make preflight'", "pkill make", "pkill -9 -f 'go test'", - "sudo pkill -f node" + "sudo pkill -f node", + "pkill -U 501", + "pkill -G staff" ], "known_good": [ "pkill -g 4242", @@ -287,33 +289,132 @@ "kill 4242", "kill -- -4242", "pgrep -f 'make preflight'", + "pkill -HUP -P $$", "abcd capture \"pkill -f make stopped a peer's gate\"" ] } }, + "pkill-by-owner": { + "tier": "blocker", + "pattern": { + "command": "pkill", + "flags": ["-u|-t|-U|-G|--euid|--uid|--terminal|--group"] + }, + "why": "`pkill -u` or `-U` signals every process that user owns, `pkill -G` every process of that group, and `pkill -t` every process on that terminal: every session running under the account, other agents' included, so selecting your own run by user, group or terminal stops everyone else's with it.", + "successor": "Stop the process you started by the pid you recorded when you started it (`kill \"$pid\"`), or stop its own process group (`kill -- -\"$pgid\"`), so no other session's process is touched.", + "fixtures": { + "known_bad": [ + "pkill -u bob", + "pkill -u $(whoami)", + "pkill -ubob", + "pkill --euid=bob", + "pkill -t pts/3", + "pkill -tpts/3", + "pkill -Ubob" + ], + "known_good": [ + "pkill -g 4242", + "pkill -P $$", + "pkill -USR1 -g 4242", + "pkill -term -g 4242", + "pgrep -u bob", + "abcd capture \"pkill -u stopped every session\"" + ] + } + }, + "kill-by-search": { + "tier": "blocker", + "pattern": { + "command": "kill", + "args_from": [ + { + "command": "pgrep", + "value_flags": ["-P", "--parent", "-g", "--pgroup", "-s", "--session", "-F", "--pidfile", "-d", "--delimiter", "--ns", "--nslist", "-j", "-M", "-N"], + "min_operands": 1 + }, + { + "command": "pgrep", + "flags": ["-u|-t|-U|-G|--euid|--uid|--terminal|--group"] + }, + { + "command": "pidof", + "value_flags": ["-o", "--omit-pid", "-S", "--separator", "-d"], + "min_operands": 1 + } + ] + }, + "why": "`kill` handed the pids a name search prints — `kill $(pgrep -f make)`, `pgrep -f make | xargs kill`, `kill $(pidof make)` — signals every process on the machine that matches, other sessions' included, exactly as `pkill` does.", + "successor": "Stop the process you started by the pid you recorded when you started it (`kill \"$pid\"`), or stop its own process group (`kill -- -\"$pgid\"`), so no other session's process is touched.", + "fixtures": { + "known_bad": [ + "kill $(pgrep -f 'make preflight')", + "kill -9 $(pgrep make)", + "pgrep -f node | xargs kill", + "kill $(pidof make)", + "{ pgrep make; } | xargs kill", + "kill $(sh -c 'pgrep make')", + "pgrep make | xargs sh -c 'kill \"$@\"' _" + ], + "known_good": [ + "kill 4242", + "kill -- -4242", + "kill $(cat pidfile)", + "kill $(pgrep -g 4242)", + "pgrep -l make", + "pgrep make | wc -l", + "echo 4242 | xargs sh -c 'kill \"$@\"' _", + "abcd capture 'kill $(pgrep make) stopped a peer gate'" + ] + } + }, "killall-by-name": { "tier": "blocker", "pattern": { "command": "killall", - "value_flags": ["-s", "--signal", "-u", "--user", "-o", "--older-than", "-y", "--younger-than", "-n", "--ns", "-t", "-c"], + "value_flags": ["-s", "--signal", "-o", "--older-than", "-y", "--younger-than", "-n", "--ns"], "min_operands": 1 }, - "why": "`killall` signals every process on the machine with that name, other sessions' and other people's included, so stopping your own run of a command also stops everyone else's.", + "why": "`killall` signals every process on the machine with that name — with `-u` or `-t`, every process of that user or terminal — other sessions' and other people's included, so stopping your own run of a command also stops everyone else's.", "successor": "Stop the process you started by the pid you recorded when you started it (`kill \"$pid\"`), or stop its own process group (`kill -- -\"$pgid\"`), so no other session's process is touched.", "fixtures": { "known_bad": [ "killall make", "killall -9 node", - "sudo killall -KILL go" + "sudo killall -KILL go", + "killall -u bob", + "killall -t ttys001" ], "known_good": [ "killall -l", "kill 4242", "kill -- -4242", "pgrep -x make", + "killall -V", "abcd capture \"killall make stopped every session's build\"" ] } + }, + "killall-by-owner": { + "tier": "blocker", + "pattern": { + "command": "killall", + "flags": ["-u|-t|--user"] + }, + "why": "`killall -u` signals every process that user owns, a name or not, and `killall -t` every process on that terminal: every session running under the account, other agents' included.", + "successor": "Stop the process you started by the pid you recorded when you started it (`kill \"$pid\"`), or stop its own process group (`kill -- -\"$pgid\"`), so no other session's process is touched.", + "fixtures": { + "known_bad": [ + "killall -ubob", + "killall --user=bob", + "killall -u bob" + ], + "known_good": [ + "killall -l", + "kill 4242", + "pgrep -u bob", + "abcd capture \"killall -u bob stopped every session\"" + ] + } } } } diff --git a/internal/core/guard/guard.go b/internal/core/guard/guard.go index 1c57890ad..d51405929 100644 --- a/internal/core/guard/guard.go +++ b/internal/core/guard/guard.go @@ -83,6 +83,15 @@ type Pattern struct { // `pkill make`, whose operand is the pattern — from `pkill -g 4242`, which // names a process group and carries no pattern at all. MinOperands int `json:"min_operands,omitempty"` + // ArgsFrom, when set, additionally requires that the command's arguments + // come, at least in part, from the output of a command matching one of + // these patterns: a command substitution in a word after the command, or, + // where `xargs` runs the command, the commands before it in the pipeline + // that feeds xargs. It is what reads `kill $(pgrep -f make)` and `pgrep -f + // make | xargs kill` as the kill by name they are, while `kill 4242` and + // `kill $(cat pidfile)` stay a kill of the pid named. A pattern listed here + // is a plain one: it may not itself carry args_from or after_cd. + ArgsFrom []Pattern `json:"args_from,omitempty"` // AfterCD, when true, additionally requires that some EARLIER command in the // same chain is a `cd`, `pushd` or `popd` — the cd-chain structure (`cd // scratch && rm -rf *`) whose hazard is that a failed directory change @@ -250,22 +259,6 @@ func Validate(r Registry) error { default: return fmt.Errorf("%w: entry %s has tier %q, want %q or %q", ErrUnknownTier, id, e.Tier, TierBlocker, TierWarn) } - if strings.TrimSpace(e.Pattern.Command) == "" { - return fmt.Errorf("%w: entry %s has no pattern command", ErrInvalidEntry, id) - } - // The command is compared against a token's BASENAME, and a subcommand is - // only ever a non-flag argument, so a path, a phrase, or a leading dash - // describes a pattern nothing can satisfy. Reject it here rather than - // ship an entry that looks armed and never fires. - if strings.ContainsAny(e.Pattern.Command, "/ \t") { - return fmt.Errorf("%w: entry %s pattern command %q is a path or phrase and could never match a command name", ErrInvalidEntry, id, e.Pattern.Command) - } - if strings.HasPrefix(e.Pattern.Subcommand, "-") { - return fmt.Errorf("%w: entry %s subcommand %q starts with a dash and could never match a non-flag argument", ErrInvalidEntry, id, e.Pattern.Subcommand) - } - if strings.HasPrefix(e.Pattern.Subcommand2, "-") { - return fmt.Errorf("%w: entry %s subcommand2 %q starts with a dash and could never match a non-flag argument", ErrInvalidEntry, id, e.Pattern.Subcommand2) - } // A refusal with no successor leaves its replacement in prose only, and // one with no why cannot teach — both are load-time rejections, as in // the record-lint banned_tokens family (iss-51). @@ -275,60 +268,99 @@ func Validate(r Registry) error { if strings.TrimSpace(e.Why) == "" { return fmt.Errorf("%w: entry %s has no why", ErrInvalidEntry, id) } - // An empty flag group can never be satisfied, so it would silently - // defang the entry rather than fail loudly — the one failure mode a - // guard must not have. - for i, group := range e.Pattern.Flags { - if !hasAlternative(group) { - return fmt.Errorf("%w: entry %s flag group %d is empty and could never match", ErrInvalidEntry, id, i) - } + if err := validatePattern(id, e.Pattern); err != nil { + return err } - // An empty argument prefix is carried by every operand, so the entry - // would fire on anything that reached it: the over-blocking twin of the - // empty flag group, and as invisible in the file. - for i, prefix := range e.Pattern.ArgPrefixes { - if strings.TrimSpace(prefix) == "" { - return fmt.Errorf("%w: entry %s argument prefix %d is empty and would match every argument", ErrInvalidEntry, id, i) + // A source an entry reads its arguments from is a pattern in its own + // right, held to the same checks, and a plain one: a source that named + // its own sources, or a cd chain, would describe a reading the matcher + // does not make. + for i, src := range e.Pattern.ArgsFrom { + where := fmt.Sprintf("%s args_from %d", id, i) + if len(src.ArgsFrom) > 0 || src.AfterCD != nil { + return fmt.Errorf("%w: entry %s may not carry args_from or after_cd of its own", ErrInvalidEntry, where) } - // A prefix constrains an OPERAND, and `operandIndexes` never - // returns a token that starts with a dash — it reads those as - // flags. A dashed - // prefix therefore describes an argument nothing can be: the silent - // defang again, one field along. A flag belongs in Flags. - if strings.HasPrefix(prefix, "-") { - return fmt.Errorf("%w: entry %s argument prefix %q starts with a dash and could never match a non-flag argument", ErrInvalidEntry, id, prefix) + if err := validatePattern(where, src); err != nil { + return err } } - // A flag-value constraint with no flag, or with no accepted value, can - // never be satisfied — the silent defang again, one field along. - for i, fv := range e.Pattern.FlagValues { - if !hasAlternative(fv.Flag) { - return fmt.Errorf("%w: entry %s flag-value constraint %d names no flag and could never match", ErrInvalidEntry, id, i) - } - if !hasAnyValue(fv.Values) { - return fmt.Errorf("%w: entry %s flag-value constraint %d accepts no value and could never match", ErrInvalidEntry, id, i) - } + } + return nil +} + +// validatePattern checks one pattern's own constraints: the ones that would +// otherwise describe a command nothing can be, and so an entry that looks armed +// and never fires. id names the entry, or the entry's source, in the refusal. +func validatePattern(id string, p Pattern) error { + if strings.TrimSpace(p.Command) == "" { + return fmt.Errorf("%w: entry %s has no pattern command", ErrInvalidEntry, id) + } + // The command is compared against a token's BASENAME, and a subcommand is + // only ever a non-flag argument, so a path, a phrase, or a leading dash + // describes a pattern nothing can satisfy. Reject it here rather than + // ship an entry that looks armed and never fires. + if strings.ContainsAny(p.Command, "/ \t") { + return fmt.Errorf("%w: entry %s pattern command %q is a path or phrase and could never match a command name", ErrInvalidEntry, id, p.Command) + } + if strings.HasPrefix(p.Subcommand, "-") { + return fmt.Errorf("%w: entry %s subcommand %q starts with a dash and could never match a non-flag argument", ErrInvalidEntry, id, p.Subcommand) + } + if strings.HasPrefix(p.Subcommand2, "-") { + return fmt.Errorf("%w: entry %s subcommand2 %q starts with a dash and could never match a non-flag argument", ErrInvalidEntry, id, p.Subcommand2) + } + // An empty flag group can never be satisfied, so it would silently + // defang the entry rather than fail loudly — the one failure mode a + // guard must not have. + for i, group := range p.Flags { + if !hasAlternative(group) { + return fmt.Errorf("%w: entry %s flag group %d is empty and could never match", ErrInvalidEntry, id, i) } - // A negative operand count describes nothing a command line can hold. - if e.Pattern.MinOperands < 0 { - return fmt.Errorf("%w: entry %s min_operands %d is negative", ErrInvalidEntry, id, e.Pattern.MinOperands) + } + // An empty argument prefix is carried by every operand, so the entry + // would fire on anything that reached it: the over-blocking twin of the + // empty flag group, and as invisible in the file. + for i, prefix := range p.ArgPrefixes { + if strings.TrimSpace(prefix) == "" { + return fmt.Errorf("%w: entry %s argument prefix %d is empty and would match every argument", ErrInvalidEntry, id, i) } - // A path constraint with no root would depth-limit every operand that - // happened to look like a path, and one with no depth describes no path - // at all. - for i, pa := range e.Pattern.ArgPaths { - if strings.TrimSpace(pa.Root) == "" { - return fmt.Errorf("%w: entry %s path constraint %d names no root segment", ErrInvalidEntry, id, i) - } - if pa.Segments < 1 { - return fmt.Errorf("%w: entry %s path constraint %d wants %d segments; a path has at least one", ErrInvalidEntry, id, i, pa.Segments) - } - // The root is compared against the FIRST segment of an operand split - // on "/", so a root that carries a slash is not a segment and could - // never be one. Depth is what the Segments field is for. - if strings.Contains(pa.Root, "/") { - return fmt.Errorf("%w: entry %s path root %q holds a slash and could never match a single path segment; use segments for depth", ErrInvalidEntry, id, pa.Root) - } + // A prefix constrains an OPERAND, and `operandIndexes` never + // returns a token that starts with a dash — it reads those as + // flags. A dashed + // prefix therefore describes an argument nothing can be: the silent + // defang again, one field along. A flag belongs in Flags. + if strings.HasPrefix(prefix, "-") { + return fmt.Errorf("%w: entry %s argument prefix %q starts with a dash and could never match a non-flag argument", ErrInvalidEntry, id, prefix) + } + } + // A flag-value constraint with no flag, or with no accepted value, can + // never be satisfied — the silent defang again, one field along. + for i, fv := range p.FlagValues { + if !hasAlternative(fv.Flag) { + return fmt.Errorf("%w: entry %s flag-value constraint %d names no flag and could never match", ErrInvalidEntry, id, i) + } + if !hasAnyValue(fv.Values) { + return fmt.Errorf("%w: entry %s flag-value constraint %d accepts no value and could never match", ErrInvalidEntry, id, i) + } + } + // A negative operand count describes nothing a command line can hold. + if p.MinOperands < 0 { + return fmt.Errorf("%w: entry %s min_operands %d is negative", ErrInvalidEntry, id, p.MinOperands) + } + // A path constraint with no root would depth-limit every operand that + // happened to look like a path, and one with no depth describes no path + // at all. + for i, pa := range p.ArgPaths { + if strings.TrimSpace(pa.Root) == "" { + return fmt.Errorf("%w: entry %s path constraint %d names no root segment", ErrInvalidEntry, id, i) + } + if pa.Segments < 1 { + return fmt.Errorf("%w: entry %s path constraint %d wants %d segments; a path has at least one", ErrInvalidEntry, id, i, pa.Segments) + } + // The root is compared against the FIRST segment of an operand split + // on "/", so a root that carries a slash is not a segment and could + // never be one. Depth is what the Segments field is for. + if strings.Contains(pa.Root, "/") { + return fmt.Errorf("%w: entry %s path root %q holds a slash and could never match a single path segment; use segments for depth", ErrInvalidEntry, id, pa.Root) } } return nil @@ -745,20 +777,41 @@ func cloneRegistry(r Registry) Registry { func cloneEntry(e Entry) Entry { out := e - out.Pattern.ValueFlags = append([]string(nil), e.Pattern.ValueFlags...) - out.Pattern.Flags = append([]string(nil), e.Pattern.Flags...) - out.Pattern.ArgPrefixes = append([]string(nil), e.Pattern.ArgPrefixes...) - out.Pattern.ArgPaths = append([]PathArg(nil), e.Pattern.ArgPaths...) - out.Pattern.FlagValues = cloneFlagValues(e.Pattern.FlagValues) - if e.Pattern.AfterCD != nil { - v := *e.Pattern.AfterCD - out.Pattern.AfterCD = &v - } + out.Pattern = clonePattern(e.Pattern) out.Fixtures.KnownBad = append([]string(nil), e.Fixtures.KnownBad...) out.Fixtures.KnownGood = append([]string(nil), e.Fixtures.KnownGood...) return out } +// clonePattern deep-copies a pattern, its sources included, so a registry a +// caller mutates shares no slice with the one it was cloned from. +func clonePattern(p Pattern) Pattern { + out := p + out.ValueFlags = append([]string(nil), p.ValueFlags...) + out.Flags = append([]string(nil), p.Flags...) + out.ArgPrefixes = append([]string(nil), p.ArgPrefixes...) + out.ArgPaths = append([]PathArg(nil), p.ArgPaths...) + out.FlagValues = cloneFlagValues(p.FlagValues) + out.ArgsFrom = clonePatterns(p.ArgsFrom) + if p.AfterCD != nil { + v := *p.AfterCD + out.AfterCD = &v + } + return out +} + +// clonePatterns deep-copies a list of patterns, keeping nil as nil. +func clonePatterns(in []Pattern) []Pattern { + if in == nil { + return nil + } + out := make([]Pattern, len(in)) + for i, p := range in { + out[i] = clonePattern(p) + } + return out +} + // cloneFlagValues deep-copies flag-value constraints: each one holds its own // Values slice, so the copy goes a level further than the other pattern fields. func cloneFlagValues(in []FlagValue) []FlagValue { diff --git a/internal/core/guard/killfeeds_test.go b/internal/core/guard/killfeeds_test.go new file mode 100644 index 000000000..6e2fd8499 --- /dev/null +++ b/internal/core/guard/killfeeds_test.go @@ -0,0 +1,384 @@ +package guard + +import ( + "strings" + "testing" +) + +// TestKillFedThroughAGroupIsBlocked — iss-2609262259360005, brace group. A +// group's output is everything its commands print, so `{ pgrep make; } | xargs +// kill` hands kill what pgrep printed. The separator inside the group started a +// new pipeline in the tokenizer, and the pipe after the group then held only +// the group's closing word. A group now restores, at its close, the pipeline it +// opened in; a separator outside the group still breaks the pipe. +func TestKillFedThroughAGroupIsBlocked(t *testing.T) { + runVerdictCases(t, []verdictCase{ + {`{ pgrep make; } | xargs kill`, VerdictBlock, "kill-by-search"}, + {`{ pgrep make; true; } | xargs kill`, VerdictBlock, "kill-by-search"}, + {"{ pgrep make\n} | xargs kill", VerdictBlock, "kill-by-search"}, + {`{ pgrep make; } | sort | xargs kill -9`, VerdictBlock, "kill-by-search"}, + {`(pgrep make; true) | xargs kill`, VerdictBlock, "kill-by-search"}, + {`{ { pgrep make; }; true; } | xargs kill`, VerdictBlock, "kill-by-search"}, + {`echo $({ pgrep make; } | xargs kill)`, VerdictBlock, "kill-by-search"}, + + {`{ echo 4242; } | xargs kill`, VerdictAllow, ""}, + {`{ pgrep make; } | wc -l`, VerdictAllow, ""}, + {`{ pgrep make; }; echo 4242 | xargs kill`, VerdictAllow, ""}, + {`(pgrep make; true); echo 4242 | xargs kill`, VerdictAllow, ""}, + {`{ pgrep make; echo 4242 | xargs kill; }`, VerdictAllow, ""}, + }) +} + +// TestKillFedThroughAStringXargsRunsIsBlocked — iss-2609262259360005, the +// xargs payload and the launcher window. xargs hands what it reads to the +// command it runs, and when that command is a shell the pids reach the string +// it runs: through `{}` replaced into it (`-I{}`), or as the positional +// parameters (`"$@"`). A shell also passes its own standard input to the +// commands of its string, so `pgrep make | sh -c 'xargs kill'` is the same +// kill. And a kill behind xargs and an unknown launcher is still fed by the +// pipe into xargs, so the fail-safe warns on it. Every command of a string +// xargs runs is read as handed xargs's input: the guard does not read which of +// them uses it. +func TestKillFedThroughAStringXargsRunsIsBlocked(t *testing.T) { + runVerdictCases(t, []verdictCase{ + {`pgrep make | xargs -I{} sh -c 'kill {}'`, VerdictBlock, "kill-by-search"}, + {`pgrep make | xargs bash -c 'kill "$@"' _`, VerdictBlock, "kill-by-search"}, + {`pgrep -f node | xargs -n1 sh -c 'kill -9 "$1"' _`, VerdictBlock, "kill-by-search"}, + {`pgrep make | xargs sudo sh -c 'kill "$@"' _`, VerdictBlock, "kill-by-search"}, + {`xargs -a <(pgrep make) sh -c 'kill "$@"' _`, VerdictBlock, "kill-by-search"}, + {`pgrep make | xargs sh -c "sh -c 'kill \$1' _ \$1" _`, VerdictBlock, "kill-by-search"}, + {`pgrep make | xargs -I{} eval kill {}`, VerdictBlock, "kill-by-search"}, + {`pgrep make | sh -c 'xargs kill'`, VerdictBlock, "kill-by-search"}, + {`pgrep make | bash -c 'sort | xargs kill'`, VerdictBlock, "kill-by-search"}, + {`pgrep make | xargs myrunner kill`, VerdictWarn, speculativeEntryID}, + {`pgrep make | xargs -n1 myrunner kill -9`, VerdictWarn, speculativeEntryID}, + {`xargs -a <(pgrep make) myrunner kill`, VerdictWarn, speculativeEntryID}, + + {`echo 4242 | xargs sh -c 'kill "$@"' _`, VerdictAllow, ""}, + {`xargs -I{} sh -c 'kill {}' < pidfile`, VerdictAllow, ""}, + {`pgrep make | xargs sh -c 'echo "$@"' _`, VerdictAllow, ""}, + {`pgrep make | sh -c 'wc -l'`, VerdictAllow, ""}, + {`echo 4242 | sh -c 'xargs kill'`, VerdictAllow, ""}, + {`pgrep make; sh -c 'xargs kill' < pidfile`, VerdictAllow, ""}, + {`pgrep make | xargs myrunner echo`, VerdictAllow, ""}, + {`echo 4242 | xargs myrunner kill`, VerdictAllow, ""}, + }) +} + +// TestKillFedBySearchInsideAStringIsBlocked — iss-2609262259360005, the +// search inside a shell string. `kill $(sh -c 'pgrep make')` prints what +// pgrep printed, but the string's commands are read after the line is split, +// outside the run the substitution recorded. A command's own string is now +// part of what the command ran, wherever its output is read. +func TestKillFedBySearchInsideAStringIsBlocked(t *testing.T) { + runVerdictCases(t, []verdictCase{ + {`kill $(sh -c 'pgrep make')`, VerdictBlock, "kill-by-search"}, + {`kill $(bash -c "pgrep -f node")`, VerdictBlock, "kill-by-search"}, + {`kill $(eval pgrep make)`, VerdictBlock, "kill-by-search"}, + {`kill $(sh -c "sh -c 'pgrep make'")`, VerdictBlock, "kill-by-search"}, + {`sh -c 'pgrep make' | xargs kill`, VerdictBlock, "kill-by-search"}, + {`kill -9 $(sudo sh -c 'pgrep make | head -1')`, VerdictBlock, "kill-by-search"}, + + {`kill $(sh -c 'cat pidfile')`, VerdictAllow, ""}, + {`echo $(sh -c 'pgrep make')`, VerdictAllow, ""}, + {`sh -c 'pgrep make'; echo 4242 | xargs kill`, VerdictAllow, ""}, + {`sh -c 'pgrep make' | wc -l`, VerdictAllow, ""}, + }) +} + +// TestAttachedSelectorsAreRead — iss-2609262259360005, the attached selector +// values. A selector's value glued to its letter is still that selector: the +// first letter after a single dash is always an option, and a byte that is no +// option letter (`/`, `.`) is its value, so `pkill -tpts/3` is `pkill -t +// pts/3`. The upper-case selectors (`-U` real user, `-G` real group) are read +// attached too: a signal name is recognised first, case folded and with or +// without its SIG prefix, as procps-ng and BSD pkill read it, so `-HUP`, +// `-USR1`, `-SEGV` and `-SIGTERM` stay signals, and so do the lower-case +// spellings `-term`, `-hup`, `-int` and `-stop`, which the letter reading had +// taken for a terminal or user selector. +func TestAttachedSelectorsAreRead(t *testing.T) { + runVerdictCases(t, []verdictCase{ + {`pkill -tpts/3`, VerdictBlock, "pkill-by-owner"}, + {`pkill -9 -tpts/3`, VerdictBlock, "pkill-by-owner"}, + {`pkill -ubob.smith`, VerdictBlock, "pkill-by-owner"}, + {`pkill -Ubob.smith`, VerdictBlock, "pkill-by-owner"}, + {`killall -ubob.smith`, VerdictBlock, "killall-by-owner"}, + {`kill $(pgrep -ubob.smith)`, VerdictBlock, "kill-by-search"}, + {`pkill -Ubob`, VerdictBlock, "pkill-by-owner"}, + {`pkill -Gstaff`, VerdictBlock, "pkill-by-owner"}, + {`pkill -U 501`, VerdictBlock, "pkill-by-owner"}, + {`pkill -G staff`, VerdictBlock, "pkill-by-owner"}, + {`pkill -HUP -Ubob`, VerdictBlock, "pkill-by-owner"}, + {`kill $(pgrep -Ubob)`, VerdictBlock, "kill-by-search"}, + {`pgrep -Gstaff | xargs kill`, VerdictBlock, "kill-by-search"}, + {`pkill -term -u bob`, VerdictBlock, "pkill-by-owner"}, + {`pkill -term -tpts/3`, VerdictBlock, "pkill-by-owner"}, + {`pkill -hup make`, VerdictBlock, "pkill-by-pattern"}, + {`git commit -nm"fix: the parser"`, VerdictBlock, "git-commit-no-verify"}, + + {`pkill -term -g 4242`, VerdictAllow, ""}, + {`pkill -hup -P $$`, VerdictAllow, ""}, + {`pkill -int -g 4242`, VerdictAllow, ""}, + {`pkill -stop -g 4242`, VerdictAllow, ""}, + {`pkill -sigterm -g 4242`, VerdictAllow, ""}, + {`pkill -SIGTERM -P $$`, VerdictAllow, ""}, + {`pkill -SIGUSR1 -g 4242`, VerdictAllow, ""}, + {`pkill -Usr1 -g 4242`, VerdictAllow, ""}, + {`pkill -HUP -g 4242`, VerdictAllow, ""}, + {`pkill -SEGV -P $$`, VerdictAllow, ""}, + {`pkill -kill -g 4242`, VerdictAllow, ""}, + {`pgrep -Ubob`, VerdictAllow, ""}, + {`git commit -m"new feature"`, VerdictAllow, ""}, + {`git push -oci.skip origin main`, VerdictAllow, ""}, + }) +} + +// TestKillFeedNoLeak pins the reading's other edge (review of the kill-by-search +// reading): a search and a kill that share a line but not a data path stay +// allowed, and so do a search of the caller's own group or parent. It holds +// every NO-LEAK probe of the two reviews of the reading (review-drainG, +// review-drainG2) in one table, beside the ones the group-input reading adds: +// a group's input ends where the group does. +func TestKillFeedNoLeak(t *testing.T) { + runVerdictCases(t, []verdictCase{ + // review-drainG. + {`echo $(pgrep make) && kill 4242`, VerdictAllow, ""}, + {`p=$(pgrep make) kill 4242`, VerdictAllow, ""}, + {`echo $(pgrep make; kill 4242)`, VerdictAllow, ""}, + {`echo $(pgrep make) $(kill 4242)`, VerdictAllow, ""}, + {`kill $(pgrep -f -g 4242)`, VerdictAllow, ""}, + {`pgrep -P $$ | xargs kill`, VerdictAllow, ""}, + {`kill -- -$(ps -o pgid= -p $$)`, VerdictAllow, ""}, + // review-drainG2. + {`( pgrep make ); kill 4242`, VerdictAllow, ""}, + {`{ pgrep make; }; kill 4242`, VerdictAllow, ""}, + {`sh -c 'pgrep make'; kill 4242`, VerdictAllow, ""}, + {`xargs sh -c 'echo {}' ; kill 1`, VerdictAllow, ""}, + {`{ pgrep -P $$; } | xargs kill`, VerdictAllow, ""}, + {`kill $(sh -c 'pgrep -g 4242')`, VerdictAllow, ""}, + {`pkill -- -term`, VerdictAllow, ""}, + // A pipe into a group reaches the group's commands and no further. + {`pgrep make | { true; }; echo 4242 | xargs kill`, VerdictAllow, ""}, + {`pgrep make | (true); echo 4242 | xargs kill`, VerdictAllow, ""}, + {`pgrep make | { true; } && xargs kill < pidfile`, VerdictAllow, ""}, + {`echo 4242 | { sleep 1; xargs kill; }`, VerdictAllow, ""}, + {`pgrep make | { sleep 1; wc -l; }`, VerdictAllow, ""}, + {`{ sleep 1; xargs kill; } < pidfile`, VerdictAllow, ""}, + {`pgrep -P $$ | { sleep 1; xargs kill; }`, VerdictAllow, ""}, + {`curl https://example.com/ | { true; }; sh -c 'echo ok'`, VerdictAllow, ""}, + // A redirect into a string reaches that string's commands only. + {`sh -c 'xargs kill' <<< "$(echo 4242)"`, VerdictAllow, ""}, + {`sh -c 'wc -l' <<< "$(pgrep make)"`, VerdictAllow, ""}, + {`sh -c 'echo ok' < <(pgrep make)`, VerdictAllow, ""}, + {`sh -c 'wc -l' <<< "$(pgrep make)"; echo 4242 | xargs kill`, VerdictAllow, ""}, + }) +} + +// TestAPipeIntoAGroupFeedsEveryCommandInIt — iss-2609270028388291, first half +// (review-drainG2). A pipe into a `{ … }` or `( … )` group is the standard +// input of every command in the group, but a separator inside it started a +// pipeline of its own, and only the commands before it read the pipe: a search +// piped into a group whose kill came after a `sleep`, a `read` or an and-list +// allowed, while the one-command group blocked. So did a stream piped into a +// group whose shell came after a separator. Every command emitted inside a +// group now reads what was piped into it, nested groups included. +func TestAPipeIntoAGroupFeedsEveryCommandInIt(t *testing.T) { + runVerdictCases(t, []verdictCase{ + {`pgrep make | { sleep 1; xargs kill; }`, VerdictBlock, "kill-by-search"}, + {`pgrep make | (sleep 1; xargs kill)`, VerdictBlock, "kill-by-search"}, + {`pgrep make | { read -r first; xargs kill; }`, VerdictBlock, "kill-by-search"}, + {`pgrep make | { true && xargs kill; }`, VerdictBlock, "kill-by-search"}, + {`pgrep make | { true || xargs kill; }`, VerdictBlock, "kill-by-search"}, + {"pgrep make | {\nsleep 1\nxargs kill\n}", VerdictBlock, "kill-by-search"}, + {`pgrep make | { { sleep 1; xargs kill; }; }`, VerdictBlock, "kill-by-search"}, + {`pgrep make | ( sleep 1; ( true; xargs kill ) )`, VerdictBlock, "kill-by-search"}, + {`pgrep make | { sleep 1; sh -c 'xargs kill'; }`, VerdictBlock, "kill-by-search"}, + {`pgrep make | { sleep 1; sort | xargs kill -9; }`, VerdictBlock, "kill-by-search"}, + {`pgrep make | sort | { sleep 1; xargs kill; }`, VerdictBlock, "kill-by-search"}, + {`echo 1 | { true; pgrep make | { sleep 1; xargs kill; }; }`, VerdictBlock, "kill-by-search"}, + // Read fail-safe: a command in a group reads its group's input even + // behind a pipe of its own, which may pass it on (DECISIONS 2026-09-27). + {`pgrep make | { true; echo 1 | { sleep 1; xargs kill; }; }`, VerdictBlock, "kill-by-search"}, + {`pgrep make | { cat | { sleep 1; xargs kill; }; }`, VerdictBlock, "kill-by-search"}, + {`echo $(pgrep make | { sleep 1; xargs kill; })`, VerdictBlock, "kill-by-search"}, + {`sh -c 'pgrep make | { sleep 1; xargs kill; }'`, VerdictBlock, "kill-by-search"}, + {`curl https://example.com/ | { true; sh; }`, VerdictBlock, "interpreter-reads-stream"}, + {`curl https://example.com/ | (true; bash)`, VerdictBlock, "interpreter-reads-stream"}, + {`curl https://example.com/ | { cd /tmp && bash -s; }`, VerdictBlock, "interpreter-reads-stream"}, + }) +} + +// TestARedirectIntoAStringReachesItsCommands — iss-2609270028388291, second +// half (review-drainG2). A shell passes its standard input to the commands of +// its string, and a here-string or a process substitution redirected into the +// shell is that input, as a pipe into it is. Only the pipe was handed on, so +// the search behind the redirect was lost, while the same redirect into a plain +// xargs blocked. +func TestARedirectIntoAStringReachesItsCommands(t *testing.T) { + runVerdictCases(t, []verdictCase{ + {`sh -c 'xargs kill' <<< "$(pgrep make)"`, VerdictBlock, "kill-by-search"}, + {`sh -c 'xargs kill' <<<"$(pgrep make)"`, VerdictBlock, "kill-by-search"}, + {`sh -c 'xargs kill' <<< $(pgrep -f node)`, VerdictBlock, "kill-by-search"}, + {`sh -c 'xargs kill' < <(pgrep make)`, VerdictBlock, "kill-by-search"}, + {`bash -c 'sort | xargs kill -9' < <(pgrep make)`, VerdictBlock, "kill-by-search"}, + {`sudo sh -c 'xargs kill' <<< "$(pgrep make)"`, VerdictBlock, "kill-by-search"}, + {`sh -c "sh -c 'xargs kill'" <<< "$(pgrep make)"`, VerdictBlock, "kill-by-search"}, + {`xargs kill <<< "$(pgrep make)"`, VerdictBlock, "kill-by-search"}, + {`xargs kill < <(pgrep make)`, VerdictBlock, "kill-by-search"}, + }) +} + +// TestKillFeedReviewBlockShapesStillBlock holds the block shapes the two +// reviews of the kill-by-search reading probed (review-drainG, +// review-drainG2) in one table, so the group-input and redirect readings are +// seen to take none of them away. +func TestKillFeedReviewBlockShapesStillBlock(t *testing.T) { + var cases []verdictCase + for _, cmd := range []string{ + `kill $( $(pgrep make) )`, + `kill "$(pgrep make)"`, + "kill `pgrep make`", + `kill -9 $(pgrep -f make) 4242`, + `pgrep make | tee /dev/null | xargs kill`, + `pgrep make | xargs -I{} kill {}`, + `pgrep make | xargs -0 kill`, + `pgrep make | xargs -d , kill`, + `pgrep make | xargs -n 1 kill`, + `pgrep make | xargs -P 4 kill`, + `pgrep make | xargs -- kill`, + `pgrep make | xargs -t kill`, + `pgrep make | xargs -L 1 kill`, + `pgrep make | xargs -s 1024 kill`, + `pgrep make | xargs -E end kill`, + `pgrep make | xargs -i kill {}`, + `pgrep make | xargs -I% kill %`, + `kill $(cat <(pgrep make))`, + `xargs kill <<< "$(pgrep make)"`, + `kill ${pids:-$(pgrep make)}`, + `kill $(( $(pgrep make) + 0 ))`, + `pgrep make | { xargs kill; }`, + `pgrep make | (xargs kill)`, + `pgrep make | xargs env kill`, + `pgrep make | xargs nice kill`, + `pgrep make | xargs sudo kill`, + `pgrep make | xargs timeout 5 kill`, + `pgrep make | xargs busybox kill`, + `pgrep make | xargs /bin/kill`, + `echo $({ pgrep make; } | xargs kill)`, + `pgrep make 2>&1 | xargs kill`, + `xargs -a <(pgrep make) kill`, + `sh -c 'exec kill $(pgrep make)'`, + `sh -c 'command kill $(pgrep make)'`, + `sh -c 'builtin kill $(pgrep make)'`, + `sh -c "sh -c 'kill \$(pgrep make)'"`, + `kill $(sh -c 'kill $(pgrep make)')`, + `pgrep make | sh -c 'sleep 1; xargs kill'`, + `( pgrep make ) | xargs kill`, + `{ pgrep make; } | xargs kill`, + } { + cases = append(cases, verdictCase{cmd, VerdictBlock, "kill-by-search"}) + } + runVerdictCases(t, cases) +} + +// TestBSDXargsValueFlagsAreStepped — iss-2609270028432249. The xargs of macOS +// and the BSDs takes a value after `-J` (the replacement string), `-R` (the +// most replacements) and `-S` (the replacement size). The walk did not know +// them, so it read the value as the command xargs runs and warned on an +// unrecognised launcher instead of reading the kill behind it. +func TestBSDXargsValueFlagsAreStepped(t *testing.T) { + runVerdictCases(t, []verdictCase{ + {`pgrep make | xargs -J % kill %`, VerdictBlock, "kill-by-search"}, + {`pgrep make | xargs -R 5 -I{} kill {}`, VerdictBlock, "kill-by-search"}, + {`pgrep make | xargs -S 1024 -I{} kill {}`, VerdictBlock, "kill-by-search"}, + {`pgrep make | xargs -J % -R 2 kill -9 %`, VerdictBlock, "kill-by-search"}, + + {`echo 4242 | xargs -J % kill %`, VerdictAllow, ""}, + {`ls | xargs -J % cp % /tmp/`, VerdictAllow, ""}, + }) +} + +// TestSignalTableHoldsOnlySignals — review-drainG2 (INFO). The signal names +// pkill's first `-NAME` word is read by are the ones a pkill accepts: a word no +// pkill takes for a signal is a cluster of options, so `pkill -null` is `-n -u +// ll` and BSD's `pkill -unused` is `-u nused`, both by owner. +func TestSignalTableHoldsOnlySignals(t *testing.T) { + runVerdictCases(t, []verdictCase{ + {`pkill -null -g 1`, VerdictBlock, "pkill-by-owner"}, + {`pkill -unused -g 1`, VerdictBlock, "pkill-by-owner"}, + {`pkill -term -g 1`, VerdictAllow, ""}, + {`pkill -SIGINFO -g 1`, VerdictAllow, ""}, + }) +} + +// TestKillFeedGroupsAndStringsStayLinear holds the group and string readings +// to the cost class the rest of the guard keeps (work_test.go): nested groups, +// a pipeline of groups, strings in the substitutions a kill reads, a +// launcher's windows behind one xargs, and a pipeline of strings xargs runs. +func TestKillFeedGroupsAndStringsStayLinear(t *testing.T) { + shapes := map[string]func(int) string{ + "nested groups": func(n int) string { + return strings.Repeat("{ ", n) + "pgrep make" + strings.Repeat("; }", n) + " | xargs kill" + }, + "a pipeline of groups": func(n int) string { + return "pgrep make" + strings.Repeat(" | { xargs kill; }", n) + }, + "a kill of many strings": func(n int) string { + return "kill" + strings.Repeat(" $(sh -c 'pgrep make')", n) + }, + "kills behind xargs and a launcher": func(n int) string { + return "echo 4242 | xargs myrunner" + strings.Repeat(" kill $(echo 1)", n) + }, + "a pipeline of xargs strings": func(n int) string { + return "pgrep make" + strings.Repeat(` | xargs sh -c 'kill "$@"' _`, n) + }, + "a pipe into nested groups": func(n int) string { + return "pgrep make" + strings.Repeat(" | { true; ", n) + "xargs kill" + strings.Repeat("; }", n) + }, + "pipes into disjoint nested groups": func(n int) string { + return strings.Repeat("echo 1 | { true; ", n) + "xargs kill" + strings.Repeat("; }", n) + }, + "strings under here-strings": func(n int) string { + return strings.Repeat(`sh -c 'xargs kill' <<< "$(echo 1)"; `, n) + }, + } + for name, build := range shapes { + build := build + t.Run(name, func(t *testing.T) { + assertWorkGrowth(t, build, 1<<8, "a kill's feeds are counted once per list") + }) + } +} + +// TestArgsReaderReadsEachWordOnce pins the argsReader's contract directly +// (review-drainG2, finding 5): the words after an xargs are read once however +// many places are asked about, left to right. The growth shapes above do not +// catch a reader that re-reads the words per place, because the speculation +// caps turn that re-read into a constant factor; this counts the reader alone. +func TestArgsReaderReadsEachWordOnce(t *testing.T) { + segs, err := tokenize("echo 4242 | xargs myrunner" + strings.Repeat(" kill $(echo 1)", 300)) + if err != nil { + t.Fatalf("tokenize: %v", err) + } + var s segment + found := false + for _, seg := range segs { + if len(seg.tokens) > 0 && seg.tokens[0] == "xargs" { + s, found = seg, true + break + } + } + if !found { + t.Fatal("no xargs segment in the shape") + } + n := 0 + workTally = &n + defer func() { workTally = nil }() + r := newArgsReader(s) + for i := 0; i <= len(s.tokens); i++ { + r.before(i) + } + if n > 2*len(s.tokens) { + t.Errorf("asking every place of a %d-word segment counted %d units, want at most %d: the words are read once", len(s.tokens), n, 2*len(s.tokens)) + } +} diff --git a/internal/core/guard/killspellings_test.go b/internal/core/guard/killspellings_test.go new file mode 100644 index 000000000..98c8a09b3 --- /dev/null +++ b/internal/core/guard/killspellings_test.go @@ -0,0 +1,200 @@ +package guard + +import ( + "strings" + "testing" +) + +// TestKillFedByAProcessSearchIsBlocked — iss-2609251640452031, first half. A +// `kill` handed the pids a name search prints is a kill by name: `kill $(pgrep +// -f make)` and `pgrep -f make | xargs kill` signal every process `pkill -f +// make` does. Read alone, each is a bare `kill` of an unknown pid list and a +// harmless `pgrep`, so the kill entries never saw the pattern. The registry now +// names the search a kill's arguments come from (args_from), and the tokenizer +// records where a word's substitution and a command's piped input come from. +// The search alone, a kill of a literal or recorded pid, and a search that +// names no pattern (its own group) stay allowed. +func TestKillFedByAProcessSearchIsBlocked(t *testing.T) { + runVerdictCases(t, []verdictCase{ + // A command substitution in the kill's words. + {`kill $(pgrep -f 'make preflight')`, VerdictBlock, "kill-by-search"}, + {`kill -9 $(pgrep make)`, VerdictBlock, "kill-by-search"}, + {`kill "$(pgrep -f node)"`, VerdictBlock, "kill-by-search"}, + {"kill `pgrep make`", VerdictBlock, "kill-by-search"}, + {`kill -s TERM $(pgrep -x make)`, VerdictBlock, "kill-by-search"}, + {`kill -- $(pgrep make)`, VerdictBlock, "kill-by-search"}, + {`sudo kill $(pgrep make)`, VerdictBlock, "kill-by-search"}, + {`kill $(pgrep -f make | head -1)`, VerdictBlock, "kill-by-search"}, + {`kill $(echo $(pgrep make))`, VerdictBlock, "kill-by-search"}, + {`kill $(pidof make)`, VerdictBlock, "kill-by-search"}, + {`kill $(pgrep -u bob)`, VerdictBlock, "kill-by-search"}, + {`kill $(pgrep make) 2>/dev/null`, VerdictBlock, "kill-by-search"}, + {`cd /tmp && kill $(pgrep make)`, VerdictBlock, "kill-by-search"}, + {`sh -c 'kill $(pgrep make)'`, VerdictBlock, "kill-by-search"}, + {`kill $(sudo pgrep -f make)`, VerdictBlock, "kill-by-search"}, + {`kill $(( $(pgrep make) ))`, VerdictBlock, "kill-by-search"}, + {`kill {$(pgrep make),}`, VerdictBlock, "kill-by-search"}, + {`PGREP make | XARGS KILL`, VerdictBlock, "kill-by-search"}, + {`/usr/bin/pgrep make | /usr/bin/xargs /bin/kill`, VerdictBlock, "kill-by-search"}, + {`pgre? make | xargs kill`, VerdictBlock, "kill-by-search"}, + // The search piped into xargs, which hands its output to kill. + {`pgrep -f 'make preflight' | xargs kill`, VerdictBlock, "kill-by-search"}, + {`pgrep make | xargs kill -9`, VerdictBlock, "kill-by-search"}, + {`(pgrep make) | xargs kill`, VerdictBlock, "kill-by-search"}, + {`pgrep make | tee /dev/null | xargs -I{} kill {}`, VerdictBlock, "kill-by-search"}, + {`xargs -a <(pgrep make) kill`, VerdictBlock, "kill-by-search"}, + {`pgrep -f node | "$(true)"xargs kill`, VerdictBlock, "kill-by-search"}, + // Behind a launcher the guard does not know, the fail-safe warns. + {`myrunner kill $(pgrep make)`, VerdictWarn, speculativeEntryID}, + {`pgrep make | myrunner xargs kill`, VerdictWarn, speculativeEntryID}, + {`pgrep -f node | xargs -n1 kill`, VerdictBlock, "kill-by-search"}, + {`pgrep make | sort | xargs kill`, VerdictBlock, "kill-by-search"}, + {`pgrep make | xargs sudo kill`, VerdictBlock, "kill-by-search"}, + {`pgrep make |& xargs kill`, VerdictBlock, "kill-by-search"}, + {`pidof make | xargs kill`, VerdictBlock, "kill-by-search"}, + {`pgrep -f x | xargs -r kill -TERM`, VerdictBlock, "kill-by-search"}, + {`echo $(pgrep make) | xargs kill`, VerdictBlock, "kill-by-search"}, + {"pgrep make |\nxargs kill", VerdictBlock, "kill-by-search"}, + + // Near misses: the search alone, a literal or recorded pid, a search + // with no pattern, and a pipe that does not reach kill. + {`pgrep -l make`, VerdictAllow, ""}, + {`pgrep -f 'make preflight'`, VerdictAllow, ""}, + {`pidof make`, VerdictAllow, ""}, + {`kill 4242`, VerdictAllow, ""}, + {`kill -- -4242`, VerdictAllow, ""}, + {`kill $(cat pidfile)`, VerdictAllow, ""}, + {`kill $(pgrep -g 4242)`, VerdictAllow, ""}, + {`kill $(pgrep -P $$)`, VerdictAllow, ""}, + {`echo $(pgrep make)`, VerdictAllow, ""}, + {`pgrep make | wc -l`, VerdictAllow, ""}, + {`pgrep make | xargs echo`, VerdictAllow, ""}, + {`pgrep make; kill 4242`, VerdictAllow, ""}, + {`pgrep make && kill 4242`, VerdictAllow, ""}, + {`pgrep make; echo 4242 | xargs kill`, VerdictAllow, ""}, + {"pgrep make\necho 4242 | xargs kill", VerdictAllow, ""}, + {`cat pidfile | xargs kill`, VerdictAllow, ""}, + {`xargs kill < pidfile`, VerdictAllow, ""}, + {`pgrep make | kill 4242`, VerdictAllow, ""}, + {`kill 4242; echo $(pgrep make)`, VerdictAllow, ""}, + }) +} + +// TestPkillAndKillallByOwnerAreBlocked — iss-2609251640452031, second half. A +// kill selecting by user or terminal carries no pattern operand: the selector +// was listed as a value flag, which consumed it, so the count read zero and +// `pkill -u bob` — every process that user owns, every session of theirs — +// answered allow. The population selectors (user, terminal, group) are no +// longer stepped over, so the selector counts as what the kill is by, and the +// attached spellings (`-ubob`, `--euid=bob`) that stand as one flag word are +// read by their own entries. The own-group and own-parent routes stay +// allowed, a signal name that happens to hold a selector's letter included. +func TestPkillAndKillallByOwnerAreBlocked(t *testing.T) { + runVerdictCases(t, []verdictCase{ + {`pkill -u bob`, VerdictBlock, "pkill-by-owner"}, + {`pkill -u $(whoami)`, VerdictBlock, "pkill-by-owner"}, + {`pkill -ubob`, VerdictBlock, "pkill-by-owner"}, + {`pkill --euid=bob`, VerdictBlock, "pkill-by-owner"}, + {`pkill --euid bob`, VerdictBlock, "pkill-by-owner"}, + {`pkill --uid=501`, VerdictBlock, "pkill-by-owner"}, + {`pkill -t pts/3`, VerdictBlock, "pkill-by-owner"}, + {`pkill -tttys001`, VerdictBlock, "pkill-by-owner"}, + {`pkill --terminal=ttys001`, VerdictBlock, "pkill-by-owner"}, + {`pkill -9 -t ttys001`, VerdictBlock, "pkill-by-owner"}, + {`pkill --group=staff`, VerdictBlock, "pkill-by-owner"}, + {`sudo pkill -u bob`, VerdictBlock, "pkill-by-owner"}, + {`pkill -U 501`, VerdictBlock, "pkill-by-owner"}, + {`pkill -G staff`, VerdictBlock, "pkill-by-owner"}, + {`killall -u bob`, VerdictBlock, "killall-by-name"}, + {`killall -ubob`, VerdictBlock, "killall-by-owner"}, + {`killall --user=bob`, VerdictBlock, "killall-by-owner"}, + {`killall -t ttys001`, VerdictBlock, "killall-by-name"}, + {`killall -c make`, VerdictBlock, "killall-by-name"}, + + {`pkill -g 4242`, VerdictAllow, ""}, + {`pkill -P $$`, VerdictAllow, ""}, + {`pkill -g $(cat pgid)`, VerdictAllow, ""}, + {`pkill -HUP -P $$`, VerdictAllow, ""}, + {`pkill -USR1 -g 4242`, VerdictAllow, ""}, + {`pkill -QUIT -g 4242`, VerdictAllow, ""}, + {`pkill -SEGV -g 4242`, VerdictAllow, ""}, + {`pgrep -u bob`, VerdictAllow, ""}, + {`killall -l`, VerdictAllow, ""}, + }) +} + +// TestKillFeedReadingStaysLinear holds the new reading to the cost class the +// rest of the guard keeps (work_test.go). A feed names a run of the tokenizer's +// output rather than copying it, and each list counts its matching commands +// once per entry, so a kill nested in a kill, a pipeline of kills, a kill with +// many substitutions, and a launcher's windows over them each cost what the +// line's bytes do. +func TestKillFeedReadingStaysLinear(t *testing.T) { + shapes := map[string]func(int) string{ + "nested kills": func(n int) string { + return strings.Repeat("kill $(", n) + "pgrep make" + strings.Repeat(")", n) + }, + "a pipeline of kills": func(n int) string { + return "pgrep make" + strings.Repeat(" | xargs kill", n) + }, + "a kill of many searches": func(n int) string { + return "kill" + strings.Repeat(" $(pgrep make)", n) + }, + "kills behind a launcher": func(n int) string { + return "myrunner" + strings.Repeat(" kill $(echo 1)", n) + }, + "quoted kills": func(n int) string { + return strings.Repeat(`kill "$(pgrep make)"; `, n) + }, + } + for name, build := range shapes { + build := build + t.Run(name, func(t *testing.T) { + assertWorkGrowth(t, build, 1<<8, "a kill's feeds are counted once per list") + }) + } +} + +// TestArgsFromConstraint pins the pattern field kill-by-search uses: a source +// is a plain pattern held to the same load-time checks, a repo override +// replaces the list whole, a change to a blocker's sources is a weakening that +// waits for HEAD, and a cloned registry shares no source with the bundled one. +func TestArgsFromConstraint(t *testing.T) { + entry := func(p Pattern) Registry { + return Registry{SchemaVersion: 1, Entries: map[string]Entry{"x": { + Tier: TierBlocker, Why: "w", Successor: "s", Pattern: p, + }}} + } + yes := true + for name, p := range map[string]Pattern{ + "a source with no command": {Command: "kill", ArgsFrom: []Pattern{{}}}, + "a source naming a path": {Command: "kill", ArgsFrom: []Pattern{{Command: "/usr/bin/pgrep"}}}, + "a source with sources": {Command: "kill", ArgsFrom: []Pattern{{Command: "pgrep", ArgsFrom: []Pattern{{Command: "ps"}}}}}, + "a source after a cd": {Command: "kill", ArgsFrom: []Pattern{{Command: "pgrep", AfterCD: &yes}}}, + "a source with a bad count": {Command: "kill", ArgsFrom: []Pattern{{Command: "pgrep", MinOperands: -1}}}, + } { + if err := Validate(entry(p)); err == nil { + t.Errorf("%s: Validate accepted it; a source nothing can match must be refused at load", name) + } + } + if err := Validate(entry(Pattern{Command: "kill", ArgsFrom: []Pattern{{Command: "pgrep", MinOperands: 1}}})); err != nil { + t.Errorf("a plain source was refused: %v", err) + } + + over := Registry{SchemaVersion: 1, Entries: map[string]Entry{"kill-by-search": { + Pattern: Pattern{ArgsFrom: []Pattern{{Command: "pidof", MinOperands: 1}}}, + }}} + merged := Merge(Defaults(), over) + if got := merged.Entries["kill-by-search"].Pattern.ArgsFrom; len(got) != 1 || got[0].Command != "pidof" { + t.Errorf("an override's args_from replaces the list whole; got %+v", got) + } + if what := weakening(Defaults(), merged); !strings.Contains(what, "kill-by-search") { + t.Errorf("narrowing a blocker's sources is a weakening; weakening() = %q", what) + } + + r := Defaults() + r.Entries["kill-by-search"].Pattern.ArgsFrom[0].ValueFlags[0] = "mutated" + if Defaults().Entries["kill-by-search"].Pattern.ArgsFrom[0].ValueFlags[0] == "mutated" { + t.Error("Defaults() shares a source's slices with the bundled registry") + } +} diff --git a/internal/core/guard/match.go b/internal/core/guard/match.go index 71079003b..51403877c 100644 --- a/internal/core/guard/match.go +++ b/internal/core/guard/match.go @@ -94,10 +94,12 @@ var wrapperValueFlags = map[string][]string{ // walk would otherwise consume and discard the value it needs to inspect. "env": {"-u", "--unset", "-C", "--chdir"}, "time": {"-f", "--format", "-o", "--output"}, - "xargs": {"-a", "--arg-file", "-d", "--delimiter", "-E", "-I", "-L", "-n", "--max-args", "-P", "--max-procs", "-s", "--max-chars", "--process-slot-var"}, + "xargs": {"-a", "--arg-file", "-d", "--delimiter", "-E", "-I", "-J", "-L", "-n", "--max-args", "-P", "--max-procs", "-R", "-s", "-S", "--max-chars", "--process-slot-var"}, "timeout": {"-k", "--kill-after", "-s", "--signal"}, "exec": {"-a"}, - // `command` and `nohup` take no value flags at all. + // `command` and `nohup` take no value flags at all. xargs's `-J`, `-R` and + // `-S` are BSD's (macOS xargs): the replacement string, the most + // replacements, and the replacement size. // Probed on util-linux 2.39.3 / coreutils 9.4 (wrappers_test.go). Two of these // are traps a document would have got wrong: @@ -290,7 +292,13 @@ func matchSegment(p Pattern, s segment) bool { // name is unknown, not because the line names the entry's program, and Check // reports it as the substitution's, not the entry's (review4-guard finding 4). func matchSegmentNamed(p Pattern, s segment) (hit, named bool) { - tally(len(s.tokens)) + // The places a command can sit are looked through for every entry; the + // words after them are walked only for an entry whose command stands at + // one, which is where the count below charges them. Charging every word + // to every entry counted a walk no entry without a named site makes, and + // made each entry added to the registry raise the constant the cost guards + // hold (work_test.go) on lines that never name it. + tally(len(arrivalsOf(s))) // Every place the command can sit is read (commandArrivals): an unknown word // before it is read every way it can be, and an unknown word in command // position is every program its tail allows. The command NAME is folded @@ -313,11 +321,12 @@ func matchSegmentNamed(p Pattern, s segment) (hit, named bool) { if len(group) == 0 { continue } + tally(len(s.tokens)) // glob reports, per TOKEN index, whether bash would expand that token. glob := func(i int) bool { return !noglob && s.globAt(i) } m := newEntryMatcher(p, s.tokens, glob) for _, a := range group { - if m.matchesAfter(a.idx) { + if m.matchesAfter(a.idx) && argsFed(p, s, a.idx) { hit = true if !anyProgram(s.tokens[a.idx]) { return true, true @@ -328,6 +337,185 @@ func matchSegmentNamed(p Pattern, s segment) (hit, named bool) { return hit, false } +// argsFed reports whether the arguments of the command at site come from a +// command p.ArgsFrom names, or true when the entry names none. Two places hand +// a command its arguments from another command's output: a command +// substitution in a word after it (`kill $(pgrep -f make)`), and, where xargs +// runs it, the pipeline feeding xargs (`pgrep -f make | xargs kill`) or a word +// of xargs's own (`xargs -a <(pgrep make) kill`). What a substitution or a +// pipeline ran is recorded by the tokenizer (segment.feeds, segment.piped), so +// the question is only whether any of those commands matches. +// +// A command of a command string is also handed what reaches the string from +// outside it (segment.argsIn, segment.stdinIn): the input of the xargs that +// runs the shell, and the shell's own standard input, which an xargs inside +// the string reads. +func argsFed(p Pattern, s segment, site int) bool { + if len(p.ArgsFrom) == 0 { + return true + } + if anyHits(s.argsIn, p.ArgsFrom) { + return true + } + from := site + 1 + if x, ok := xargsBefore(s, site); ok { + if s.piped.hits(p.ArgsFrom) || anyHits(s.stdinIn, p.ArgsFrom) { + return true + } + from = x + 1 + } + for i, fs := range s.feeds { + if i < from { + continue + } + for _, f := range fs { + if f.hits(p.ArgsFrom) { + return true + } + } + } + return false +} + +// xargsBefore returns the earliest place before site the walk arrives at that +// can be xargs, which runs the command at site with arguments it reads from its +// input. A name a substitution prints can be xargs too (`"$(true)"xargs kill`), +// and the walk steps it as a wrapper of unknown grammar, so it is read as one. +func xargsBefore(s segment, site int) (int, bool) { + for _, a := range arrivalsOf(s) { + if a.idx >= site { + break + } + if commandNamed(s, a, "xargs") { + return a.idx, true + } + } + return 0, false +} + +// argsReader answers, for places in one segment read left to right, what +// reaches a command there as its arguments from outside its own words: what +// reached the segment itself (segment.argsIn) and, past an xargs, the input +// xargs hands the command it runs — its standard input (the pipe into it, and +// segment.stdinIn) and the output of the substitutions in its own words +// (`xargs -a <(pgrep make)`). The words are read once however many places are +// asked about, so a launcher's windows cost what the line's words do. +// +// The words' substitutions are held as one run: every command the tokenizer +// emitted while it read a segment's words is a substitution in one of them, +// in word order, so the commands of the words between two places are one run +// of the segment's list. What a place is handed therefore stays as short as +// the nesting of strings is deep, however many words it spans. +type argsReader struct { + s segment + x int + ok bool + next int + base []feed + words feed +} + +func newArgsReader(s segment) *argsReader { + r := &argsReader{s: s} + r.x, r.ok = xargsBefore(s, len(s.tokens)) + if r.ok { + r.base = append([]feed(nil), s.argsIn...) + if s.piped.list != nil { + r.base = append(r.base, s.piped) + } + r.base = append(r.base, s.stdinIn...) + r.next = r.x + 1 + } + return r +} + +// before returns what reaches a command at site, for a site at or after every +// one asked before. +func (r *argsReader) before(site int) []feed { + if !r.ok || site <= r.x { + return r.s.argsIn + } + for ; r.next < site && r.next < len(r.s.tokens); r.next++ { + tally(1) + for _, f := range r.s.feeds[r.next] { + switch { + case r.words.list == nil: + r.words = f + case f.list == r.words.list: + r.words.lo, r.words.hi = min(r.words.lo, f.lo), max(r.words.hi, f.hi) + default: + // Not reached: a word's feeds name its own tokenize call. Kept + // whole rather than dropped, should that ever change. + r.base = append(r.base, f) + } + } + } + out := append([]feed(nil), r.base...) + if r.words.list != nil { + out = append(out, r.words) + } + return out +} + +// anyHits reports whether any command in any of the runs matches one of srcs. +func anyHits(fs []feed, srcs []Pattern) bool { + for _, f := range fs { + if f.hits(srcs) { + return true + } + } + return false +} + +// segmentHits reports whether a command matches one of srcs, or any command of +// a command string it runs does (segList.payloads), however deep they nest. +func segmentHits(s segment, srcs []Pattern) bool { + for _, src := range srcs { + if matchSegment(src, s) { + return true + } + } + if s.home == nil { + return false + } + for _, ps := range s.home.payloads[s.at] { + if segmentHits(ps, srcs) { + return true + } + } + return false +} + +// hits reports whether any command in the run matches one of srcs. The first +// question asked of a list counts, once, how many of its commands match, so +// every later question about any run in it — however many runs nest inside +// one another — is two lookups. +func (f feed) hits(srcs []Pattern) bool { + if f.list == nil || f.lo >= f.hi || len(srcs) == 0 { + return false + } + key := &srcs[0] + counts, ok := f.list.hits[key] + if !ok { + counts = make([]int, len(f.list.segs)+1) + for i, s := range f.list.segs { + counts[i+1] = counts[i] + if segmentHits(s, srcs) { + counts[i+1]++ + } + } + if f.list.hits == nil { + f.list.hits = map[*Pattern][]int{} + } + f.list.hits[key] = counts + } + hi := f.hi + if hi > len(f.list.segs) { + hi = len(f.list.segs) + } + return f.lo < hi && counts[hi] > counts[f.lo] +} + // entryMatcher answers, for any place a command can sit in one segment, whether // the arguments after it meet an entry's operand and flag constraints. It reads // the tokens once however many places there are — a line whose command an @@ -342,6 +530,13 @@ type entryMatcher struct { // nextHit holds, per flag clause (each flag group, then each flag-value // constraint), the first index at or after i whose token satisfies it. nextHit [][]int + // groups is how many of the clauses are flag groups, the ones a signal + // word is no answer to. + groups int + // nextSig[i] is the first index at or after i holding a word the command + // reads as its signal (signalWordCommands), or len(tokens); nil for a + // command that reads none. + nextSig []int } // newEntryMatcher reads the tokens for one entry. One operand walk reads every @@ -381,6 +576,10 @@ func newEntryMatcher(p Pattern, tokens []string, glob func(int) bool) entryMatch alts := strings.Split(group, "|") m.nextHit = append(m.nextHit, next(func(i int) bool { return flagGroupHit(alts, tokens, i, glob, opts) })) } + m.groups = len(p.Flags) + if signalWordCommands[strings.ToLower(p.Command)] && len(p.Flags) > 0 { + m.nextSig = next(func(i int) bool { return isSignalWord(tokens[i]) }) + } for _, fv := range p.FlagValues { fv := fv m.nextHit = append(m.nextHit, next(func(i int) bool { return flagValueHit(fv, tokens, i, glob) })) @@ -397,8 +596,13 @@ func (m entryMatcher) matchesAfter(site int) bool { return false } stop := m.nextStop[start] - for _, nh := range m.nextHit { - if nh[start] >= stop { + for c, nh := range m.nextHit { + h := nh[start] + // The command's first signal word is its signal, never a flag. + if c < m.groups && m.nextSig != nil && h < len(nh)-1 && h == m.nextSig[start] { + h = nh[h+1] + } + if h >= stop { return false } } @@ -552,6 +756,14 @@ func flagMatches(alt, arg string, glob bool) bool { if isShortCluster(arg) && strings.ContainsRune(arg[1:], rune(alt[1])) { return true } + // The first letter after a single dash is always an option, and a + // byte no option letter can be (`/`, `.`) marks what follows as its + // value — or the word as one the program refuses — so `-tpts/3` is + // `-t pts/3` and `-ubob.smith` is `-u bob.smith`. Only that first + // letter is read: the rest may be the value (`-m"new feature"`). + if len(arg) > 2 && arg[0] == '-' && arg[1] == alt[1] && !isShortCluster(arg) { + return true + } if !glob || len(arg) < 2 || arg[0] != '-' || arg[1] == '-' { return false } diff --git a/internal/core/guard/payload.go b/internal/core/guard/payload.go index 7d902f52a..ab8ef342f 100644 --- a/internal/core/guard/payload.go +++ b/internal/core/guard/payload.go @@ -111,6 +111,10 @@ func expandPayloads(segs []segment) ([]segment, []payloadSignal) { out = append(out, fs) queue = append(queue, work{segs: []segment{fs}, depth: item.depth}) } + // What reaches the commands of a string s runs is read once per + // segment, however many strings it carries (payloadInput). + var stdin, args []feed + inputRead := false for _, ref := range payloadRefsOf(s) { kind, fam, payload, trailing := ref.kind, ref.family, ref.payload, ref.trailing // Past the depth budget the guard cannot follow the nesting, so a @@ -159,12 +163,31 @@ func expandPayloads(segs []segment) ([]segment, []payloadSignal) { } // Offset the payload's chains into a fresh disjoint range and append. + // Each command of the string is handed what reaches it from the + // command that runs it (segment.stdinIn, segment.argsIn), and the + // string is filed under that command, so a run holding it holds + // the string's commands too (segList.payloads). + if !inputRead { + stdin, args = payloadInput(s) + inputRead = true + } offset := chainMax + 1 for i := range psegs { psegs[i].chain += offset if psegs[i].chain > chainMax { chainMax = psegs[i].chain } + // A command in a group of the string also reads what was + // piped into that group (tokenizeAt's groupIn). + if len(psegs[i].stdinIn) == 0 { + psegs[i].stdinIn = stdin + } else { + psegs[i].stdinIn = append(append([]feed(nil), psegs[i].stdinIn...), stdin...) + } + psegs[i].argsIn = args + } + if s.home != nil { + s.home.addPayload(s.at, psegs) } out = append(out, psegs...) queue = append(queue, work{segs: psegs, depth: item.depth + 1}) @@ -177,6 +200,69 @@ func expandPayloads(segs []segment) ([]segment, []payloadSignal) { return out, signals } +// payloadInput returns what reaches the commands of a command string s runs: +// the standard input the running shell passes on — its pipe, what its +// here-strings and redirected process substitutions print, and whatever +// reached s itself — and, as their arguments, the input of an xargs that runs +// the shell, which xargs replaces into the string (`-I{}`) or appends as its +// positional parameters. Which of the string's commands reads it is not +// modelled: every one of them is read as handed it. +func payloadInput(s segment) (stdin, args []feed) { + if s.piped.list != nil { + stdin = append(stdin, s.piped) + } + stdin = append(stdin, s.stdinIn...) + stdin = append(stdin, redirectedInput(s)...) + return stdin, newArgsReader(s).before(len(s.tokens)) +} + +// redirectedInput returns the commands whose output s reads on its standard +// input through a redirect, as one run of its list: a here-string's word (the +// tokenizer keeps the operator as a word, and its text glued to it or as the +// word after it) and a process substitution. The `<` that redirects a process +// substitution is not kept, so one handed to s as an operand reads the same; +// it hands the same output to whatever opens its path, and is read as input +// all the same. nil when no redirect carries a command's output. +// +// The runs are held as one, as argsReader holds a segment's words: every +// command the tokenizer emitted while it read s's words is a substitution in +// one of them, in word order, so the covering run stays one feed however many +// redirects s carries. +func redirectedInput(s segment) []feed { + if len(s.feeds) == 0 { + return nil + } + var run feed + var rest []feed + add := func(i int) { + for _, f := range s.feeds[i] { + switch { + case run.list == nil: + run = f + case f.list == run.list: + run.lo, run.hi = min(run.lo, f.lo), max(run.hi, f.hi) + default: + // Not reached: a word's feeds name its own tokenize call. Kept + // whole rather than dropped, should that ever change. + rest = append(rest, f) + } + } + } + for i, tok := range s.tokens { + tally(1) + switch { + case tok == "<<<": + add(i + 1) + case strings.HasPrefix(tok, "<<<"), strings.Contains(tok, procSubOperand): + add(i) + } + } + if run.list == nil { + return rest + } + return append(rest, run) +} + // splitAfterIFS reports whether a segment carrying an unquoted fixed output // shares the command line, at any payload layer, with another segment that // names IFS (review7-guard finding 2). fixedOutputSegment splits an output on diff --git a/internal/core/guard/signal.go b/internal/core/guard/signal.go new file mode 100644 index 000000000..00182499d --- /dev/null +++ b/internal/core/guard/signal.go @@ -0,0 +1,66 @@ +package guard + +import "strings" + +// signalWordCommands names the commands whose first `-NAME` word naming a +// signal is the signal to send, whatever the case of its letters, and not a +// cluster of short options. procps-ng pkill reads the first such word anywhere +// in its arguments, BSD pkill reads it as the first argument, and both compare +// the name case-insensitively, with or without its SIG prefix. So `pkill -term +// -g 4242` sends TERM to a group, and the `t` in it is no terminal selector; +// read as a cluster, the letters of `-HUP`, `-USR1` and `-SEGV` would be the +// user and group selectors `-U` and `-G`. psmisc killall is not here: it reads +// a signal name only when it begins with a capital letter, and hands a +// lower-case one to its option parser, so `killall -term` is `killall -t erm`. +var signalWordCommands = map[string]bool{"pkill": true} + +// signalNames are the signal names the pkill implementations accept, Linux +// and BSD together, without the SIG prefix. A name one platform lacks is +// refused as an option there, so reading it as a signal spares nothing that +// runs. A name no pkill accepts is not here: its letters are options, so +// `pkill -null` is `-n -u ll` and BSD's `pkill -unused` is `-u nused`. +var signalNames = map[string]bool{ + "hup": true, "int": true, "quit": true, "ill": true, "trap": true, "abrt": true, + "iot": true, "bus": true, "emt": true, "fpe": true, "kill": true, "usr1": true, + "segv": true, "usr2": true, "pipe": true, "alrm": true, "term": true, "stkflt": true, + "chld": true, "cld": true, "cont": true, "stop": true, "tstp": true, "ttin": true, + "ttou": true, "urg": true, "xcpu": true, "xfsz": true, "vtalrm": true, "prof": true, + "winch": true, "io": true, "poll": true, "pwr": true, "sys": true, "info": true, + "lost": true, "rtmin": true, "rtmax": true, +} + +// isSignalWord reports whether a word is `-` and a signal: a number, or a name +// in any case, with or without the SIG prefix, and a real-time signal offset +// from its bound (`-RTMIN+3`). +func isSignalWord(tok string) bool { + if len(tok) < 2 || tok[0] != '-' { + return false + } + name := strings.ToLower(tok[1:]) + if allDigits(name) { + return true + } + name = strings.TrimPrefix(name, "sig") + if signalNames[name] { + return true + } + for _, bound := range []string{"rtmin", "rtmax"} { + if rest, ok := strings.CutPrefix(name, bound); ok && len(rest) > 1 && + (rest[0] == '+' || rest[0] == '-') && allDigits(rest[1:]) { + return true + } + } + return false +} + +func allDigits(s string) bool { + if s == "" { + return false + } + for i := 0; i < len(s); i++ { + if s[i] < '0' || s[i] > '9' { + return false + } + } + return true +} diff --git a/internal/core/guard/speculate.go b/internal/core/guard/speculate.go index 2faf10b11..e84454bad 100644 --- a/internal/core/guard/speculate.go +++ b/internal/core/guard/speculate.go @@ -168,6 +168,7 @@ func (r Registry) speculateSegment(before []segment, s segment, ids []string, bu // glob record is withheld from every window of such a segment, or Tier 2 // would re-arm the compare Tier 1 correctly stood down. noglob := allNoglob(s) + args := newArgsReader(s) for _, start := range starts { tokens := s.tokens[start:] if len(tokens) > maxSpeculativeWindow { @@ -176,10 +177,17 @@ func (r Registry) speculateSegment(before []segment, s segment, ids []string, bu } // The glob record travels with the window: a globbed flag behind an // unrecognised launcher is still a pattern bash expands. - cand := segment{tokens: tokens, chain: s.chain} + cand := segment{tokens: tokens, chain: s.chain, piped: s.piped} if !noglob { cand.globbed = s.globSlice(start, start+len(tokens)) } + // So do the words' feeds and the segment's piped input: `myrunner kill + // $(pgrep -f make)` is a kill fed by a search behind a launcher. + cand.feeds = s.feedsSlice(start, start+len(tokens)) + // And what an xargs before the window hands the command it runs: + // `pgrep make | xargs myrunner kill` is a kill fed by the search. + cand.stdinIn = s.stdinIn + cand.argsIn = args.before(start) // Expand the suffix's own payloads, so `busybox sh -c ""` is // reached: stepping busybox leaves `sh -c …` in command position, and the diff --git a/internal/core/guard/tokenize.go b/internal/core/guard/tokenize.go index c1bd9a540..b1a571294 100644 --- a/internal/core/guard/tokenize.go +++ b/internal/core/guard/tokenize.go @@ -47,8 +47,9 @@ type segment struct { substitutionUnread bool // stdinStream records that the command's standard input is a stream of // text: a pipe from the command before it, a here-document, or a - // here-string. A shell reading its script from that stream runs text the - // guard read as data (iss-2609251640462464). + // here-string, or a pipe into a group it sits in. A shell reading its + // script from that stream runs text the guard read as data + // (iss-2609251640462464). stdinStream bool // globbed is parallel to tokens and records, per token, that it carried an // UNQUOTED, unescaped `*`, `?` or `[` — a word bash expands against the @@ -78,6 +79,67 @@ type segment struct { // walkCapped records that the walk stopped at maxUnknownSites, which // Check refuses. walkCapped bool + // feeds records, per token index, where the output a word holds comes + // from: the commands each command substitution in it ran, nested ones + // included. nil when no word holds one. It is what lets an entry read a + // command's arguments by the command that printed them (Pattern.ArgsFrom): + // `kill $(pgrep -f make)` is a kill by name. + feeds map[int][]feed + // piped records the commands before this one in its pipeline, whose output + // is its standard input, with the commands their own substitutions ran; + // the zero feed when it reads no pipe. `pgrep -f make | xargs kill` hands + // kill what pgrep printed. + piped feed + // stdinIn and argsIn are what reaches a command of a command string from + // the command that runs the string (expandPayloads): the standard input a + // shell passes on to the commands of its string (`pgrep make | sh -c + // 'xargs kill'`), and, where xargs runs the shell, the input xargs hands + // it, which reaches the string's commands through `{}` or `"$@"` (`pgrep + // make | xargs sh -c 'kill "$@"' _`). A Tier 2 window that starts after an + // xargs carries that xargs's input in argsIn the same way. stdinIn also + // holds what was piped into a group the command sits in (tokenizeAt's + // groupIn): `pgrep make | { sleep 1; xargs kill; }`. nil when nothing + // reaches the command from outside its own line. + stdinIn []feed + argsIn []feed + // home and at name the segment in the tokenize call that emitted it, + // list.segs[at]; home is nil for a segment built anywhere else. They are + // what a command's own string is filed under (segList.payloads). + home *segList + at int +} + +// feed is a run of segments one tokenize call emitted, list.segs[lo:hi]: the +// commands whose output a word holds (segment.feeds) or a command reads on its +// standard input (segment.piped). It names the run rather than copying it, so +// recording one costs the same whatever it holds, and the question an entry +// asks of it — does any of these commands match — is answered from a count the +// list keeps per question (feed.hits), so asking it of nested runs costs no more +// than asking it once of the whole list. +type feed struct { + list *segList + lo, hi int +} + +// segList is one tokenize call's output, shared by every feed that call +// recorded. segs is set when the call returns; hits caches, per list of source +// patterns an entry names, how many of the first i segments match one of them. +// +// payloads holds, per index, the segments of the command strings that +// segment runs (expandPayloads), so a run that holds `sh -c 'pgrep make'` +// holds the pgrep too: `kill $(sh -c 'pgrep make')` is a kill by name. +type segList struct { + segs []segment + hits map[*Pattern][]int + payloads map[int][]segment +} + +// addPayload files the segments of a command string segment at runs. +func (l *segList) addPayload(at int, psegs []segment) { + if l.payloads == nil { + l.payloads = map[int][]segment{} + } + l.payloads[at] = append(l.payloads[at], psegs...) } // wordLiteral is the fixed text of a word whose every fixed-output command @@ -182,6 +244,26 @@ func (s segment) globSlice(lo, hi int) []bool { return nil } +// feedsSlice returns the feeds of tokens[lo:hi], indexed from lo, or nil when +// none of those words holds one — the record a sub-segment built from a token +// window (Tier 2) carries forward. It reads the window's words, not the whole +// record, so a window costs what its words do. +func (s segment) feedsSlice(lo, hi int) map[int][]feed { + if len(s.feeds) == 0 { + return nil + } + var out map[int][]feed + for i := lo; i < hi; i++ { + if fs, ok := s.feeds[i]; ok { + if out == nil { + out = map[int][]feed{} + } + out[i-lo] = fs + } + } + return out +} + // tokenize splits a candidate command line into command-position segments, // honouring shell quoting: single quotes are literal, double quotes take the // POSIX backslash escapes, and a backslash outside quotes escapes the next @@ -316,7 +398,62 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { // command emitted reads a pipe. Both land on segment.stdinStream. curStdin bool pipeNext bool + // feeds rides with the segment and records, per token index, the + // commands whose output the word holds (segment.feeds); curFeeds holds + // them for the word being built. pipeFrom is where, in segs, the + // pipeline the command being built belongs to began: the commands from + // there on are what a pipe hands it (segment.piped). + feeds map[int][]feed + curFeeds []feed + pipeFrom int + // braceFrom holds, per `{ … }` group still open, the pipeFrom its + // opening word stood in. A group's output is everything its commands + // print, so the pipeline a `}` closes resumes where the group began: + // `{ pgrep make; } | xargs kill` hands kill what pgrep printed, though + // the separator inside the group began a pipeline of its own. A `( … )` + // group keeps the same record on its frame (parenFrame.pipeFrom). + braceFrom []groupOpen + // groupIn is what was piped into the groups open here, as one run, or + // nil when no pipe reaches one. A pipe into a group is the standard + // input of every command in it, so each command emitted inside reads + // it (segment.stdinIn, segment.stdinStream), not only the ones before + // the group's first separator: `pgrep make | { sleep 1; xargs kill; }` + // hands kill what pgrep printed. A group's close restores the value + // its opening word saved (groupOpen.in, parenFrame.groupIn). + groupIn []feed + // list is this call's output as the feeds it records name it. + list = &segList{} ) + defer func() { list.segs = segs }() + // openGroup is read where a `{ … }` or `( … )` group opens, and returns + // the groupIn its close restores. A pipe into the group widens groupIn to + // cover what it hands on as well as what the groups around it were handed: + // a command in the group reads its group's input whether or not a pipe + // inside hands it another, because the command before that pipe may pass + // the group's input on (`pgrep make | { cat | xargs kill; }`), and which + // commands do is not modelled. The runs of the open groups are one run of + // this call's list: an inner group's pipeline either began where the outer + // one's did or began inside it, so one run covers them all, and each + // command reads one run however deep the groups nest. + openGroup := func() (saved []feed) { + saved = groupIn + if pipeNext && len(segs) > pipeFrom { + in := feed{list: list, lo: pipeFrom, hi: len(segs)} + if len(groupIn) > 0 { + in.lo = min(in.lo, groupIn[0].lo) + } + groupIn = []feed{in} + } + return saved + } + // feedFrom records, for the word being built, that it holds the output of + // the commands emitted since start: a substitution's own command and every + // command nested inside it. + feedFrom := func(start int) { + if len(segs) > start { + curFeeds = append(curFeeds, feed{list: list, lo: start, hi: len(segs)}) + } + } // inArithmetic reports whether the innermost construct that can change how a // `<<` reads is an arithmetic one. A plain `(` is skipped rather than // answered on: inside `(( … ))` it is sub-expression grouping, and at the @@ -376,6 +513,17 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { } hasCur = true } + // recordFeeds files the word being built's feeds under the index it is + // about to take. + recordFeeds := func() { + if len(curFeeds) == 0 { + return + } + if feeds == nil { + feeds = map[int][]feed{} + } + feeds[len(toks)] = curFeeds + } flushToken := func() { if !hasCur { return @@ -395,11 +543,12 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { if curBrace && !(isAssignment(string(cur)) && allAssignments(toks)) { if words, ok := expandBraces(bword{b: cur, m: curMask}, &braceLim); ok { for _, w := range words { + recordFeeds() toks = append(toks, unknownFromOpenExpansion(string(w.b))) globs = append(globs, w.globbed()) } cur, curMask, hasCur, curGlob, curBrace = nil, nil, false, false, false - curPieces = nil + curPieces, curFeeds = nil, nil return } braceGroup = true @@ -415,6 +564,19 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { lits[len(toks)] = joinedLiteral(cur, curPieces) } curPieces = nil + recordFeeds() + curFeeds = nil + // An unquoted `{` or `}` in command position opens or closes a group. + if len(curMask) == 1 && curMask[0]&wordStruct != 0 && allReserved(toks) { + switch tok { + case "{": + braceFrom = append(braceFrom, groupOpen{pipeFrom: pipeFrom, in: openGroup()}) + case "}": + if n := len(braceFrom); n > 0 { + pipeFrom, groupIn, braceFrom = braceFrom[n-1].pipeFrom, braceFrom[n-1].in, braceFrom[:n-1] + } + } + } toks = append(toks, tok) globs = append(globs, curGlob) cur, curMask, hasCur, curGlob, curBrace = nil, nil, false, false, false @@ -422,13 +584,19 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { flushSegment := func() { flushToken() if len(toks) > 0 { + var piped feed + if pipeNext && len(segs) > pipeFrom { + piped = feed{list: list, lo: pipeFrom, hi: len(segs)} + } segs = append(segs, segment{ tokens: toks, chain: chain, braceGroup: braceGroup, globbed: globsOrNil(globs), - stdinStream: curStdin || pipeNext, literal: lits, + stdinStream: curStdin || pipeNext || len(groupIn) > 0, literal: lits, feeds: feeds, piped: piped, + stdinIn: groupIn, home: list, at: len(segs), }) toks = nil globs = nil lits = nil + feeds = nil braceGroup = false pipeNext = false } @@ -560,10 +728,15 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { toks: toks, globs: globs, lits: lits, cur: cur, curMask: curMask, hasCur: hasCur, curGlob: curGlob, curBrace: curBrace, braceGroup: braceGroup, chain: chain, procSub: procSub, curStdin: curStdin, pipeNext: pipeNext, pieces: curPieces, + feeds: feeds, curFeeds: curFeeds, pipeFrom: pipeFrom, segStart: len(segs), braceFrom: braceFrom, + groupIn: groupIn, } toks, globs, lits, cur, curMask, hasCur, curGlob, curBrace, braceGroup = nil, nil, nil, nil, nil, false, false, false, false curPieces = nil curStdin, pipeNext = false, false + // A substitution is a command string of its own: its pipelines begin + // inside it. Its standard input is its command's, so groupIn carries on. + feeds, curFeeds, pipeFrom, braceFrom = nil, nil, len(segs), nil parens = append(parens, parenFrame{kind: kind, pos: pos, saved: saved}) } // prePassedBacktick reads a backtick opening at line[i] whose text bash's @@ -602,8 +775,12 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { toks, globs, lits, cur, curMask, hasCur, curGlob, curBrace, braceGroup, chain = e.toks, e.globs, e.lits, e.cur, e.curMask, e.hasCur, e.curGlob, e.curBrace, e.braceGroup, e.chain curStdin, pipeNext, curPieces = e.curStdin, e.pipeNext, e.pieces + feeds, curFeeds, pipeFrom, braceFrom, groupIn = e.feeds, e.curFeeds, e.pipeFrom, e.braceFrom, e.groupIn if !f.bare { addCur([]byte(arithmeticOperand), 0) + // The number it prints is computed from what the substitutions + // inside it printed. + feedFrom(e.segStart) } lastList = false } @@ -618,6 +795,8 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { toks, globs, lits, cur, curMask, hasCur, curGlob, curBrace, braceGroup, chain = e.toks, e.globs, e.lits, e.cur, e.curMask, e.hasCur, e.curGlob, e.curBrace, e.braceGroup, e.chain curStdin, pipeNext, curPieces = e.curStdin, e.pipeNext, e.pieces + feeds, curFeeds, pipeFrom, braceFrom, groupIn = e.feeds, e.curFeeds, e.pipeFrom, e.braceFrom, e.groupIn + feedFrom(e.segStart) if e.procSub { addCur([]byte(procSubOperand), 0) } else { @@ -761,7 +940,9 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { continue } if end >= 0 { + start := len(segs) arithmetic(line[j+3 : end-1]) + feedFrom(start) addCur([]byte(arithmeticOperand), 0) j = end + 1 continue @@ -786,7 +967,9 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { if line[j] == '`' { text, _ = backtickText(text, true) } + start := len(segs) follow(text) + feedFrom(start) addCur([]byte{unknownMark}, 0) // A `$(cat <<'EOF' … EOF)` prints its document verbatim, // and so does its backtick spelling; flushToken reads the @@ -877,6 +1060,7 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { chainSeq++ chain = chainSeq pipeNext = false + pipeFrom = len(segs) } case c == '#' && !hasCur: // A comment starts only at a word boundary (POSIX): `url/#frag` is @@ -1116,7 +1300,11 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { parens[n-1].kind = parenArithmetic kind = parenArithmetic } - parens = append(parens, parenFrame{kind: kind, pos: i}) + frame := parenFrame{kind: kind, pos: i, pipeFrom: pipeFrom, groupIn: groupIn} + if kind == parenGroup { + frame.groupIn = openGroup() + } + parens = append(parens, frame) case ')', '`': // A backtick is its own closer: reaching this branch means the // innermost open frame is a backtick (an opening one was taken @@ -1127,6 +1315,12 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { if n := len(parens); n > 0 { top := parens[n-1] parens = parens[:n-1] + if top.kind == parenGroup && top.saved == nil { + pipeFrom = top.pipeFrom + } + if top.saved == nil { + groupIn = top.groupIn + } if top.saved != nil { closeSubstitution(top.saved) // An unquoted `$(cat <<'EOF' … EOF)`, or its backtick @@ -1150,6 +1344,7 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { } if (c == '&' || c == '|') && i+1 < len(line) && line[i+1] == c { pipeNext = false + pipeFrom = len(segs) lastList = true i += 2 continue @@ -1159,6 +1354,7 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { pipeNext = true case ';', '&': pipeNext = false + pipeFrom = len(segs) } // A single pipe continues the list across a newline; `;`, `&`, the // grouping parens, and a backtick boundary do not. @@ -1645,6 +1841,20 @@ type parenFrame struct { // records that it is the `(( … ))` command, which leaves no word. end int bare bool + // pipeFrom is, for a `( … )` group, where the pipeline its `(` stood in + // began, which its `)` resumes (tokenizeAt's braceFrom); groupIn is the + // input of the groups around it, which its `)` restores (tokenizeAt's + // groupIn). + pipeFrom int + groupIn []feed +} + +// groupOpen is what a `{ … }` group's opening word records for its `}` to +// resume: the pipeline it stood in, and the input of the groups around it +// (tokenizeAt's braceFrom and groupIn). +type groupOpen struct { + pipeFrom int + in []feed } // enclosing is the state of a command suspended by a substitution opening @@ -1670,6 +1880,17 @@ type enclosing struct { pipeNext bool // pieces is the enclosing word's fixed outputs so far (tokenizeAt). pieces []litPiece + // feeds, curFeeds and pipeFrom are the enclosing command's own records of + // where its words and its input come from (tokenizeAt); segStart is where, + // in the output, the substitution's own commands begin. + feeds map[int][]feed + curFeeds []feed + pipeFrom int + segStart int + // braceFrom is the enclosing command string's open `{ … }` groups, and + // groupIn what was piped into the groups open around it. + braceFrom []groupOpen + groupIn []feed } // procSubOperand is the word a process substitution leaves in the enclosing diff --git a/internal/core/guard/unknownreaders_test.go b/internal/core/guard/unknownreaders_test.go index 24d2d471f..9089c4f40 100644 --- a/internal/core/guard/unknownreaders_test.go +++ b/internal/core/guard/unknownreaders_test.go @@ -38,7 +38,7 @@ func TestDashWordBeforeCommandPositionReadsBothWays(t *testing.T) { {`git -$(echo c) core.hooksPath=/dev/null commit -m x`, VerdictBlock, "git-commit-no-verify"}, {`git -$(echo c) alias.p='push --force' p origin main`, VerdictBlock, "git-push-force"}, {`gh -$(echo R) o/r api -X DELETE repos/o/r`, VerdictBlock, "gh-api-repo-delete"}, - {`pkill -$(echo g) 4242`, VerdictBlock, "pkill-by-pattern"}, + {`pkill -$(echo g) 4242`, VerdictBlock, "pkill-by-owner"}, {`sudo -$(echo u) root ` + push, VerdictBlock, "git-push-force"}, {`env -$(echo u) X ` + push, VerdictBlock, "git-push-force"}, diff --git a/internal/core/guard/unknownsites_test.go b/internal/core/guard/unknownsites_test.go index 9412de755..ff0d9f445 100644 --- a/internal/core/guard/unknownsites_test.go +++ b/internal/core/guard/unknownsites_test.go @@ -47,7 +47,9 @@ var wordReaders = map[string]string{ "flagShaped": "exempt: reads the known text flagMatches is handed", "isShortFlag": "exempt: reads a registry alternative, never a command word", "isShortCluster": "exempt: a known word's shape; clusterCouldCarry reads the unknown word", + "isSignalWord": "exempt: spares a word only when it spells a signal name whole; a word holding a substitution's output never does, so it stays every flag it can become", "pathOf": "exempt: reads a URL scheme's characters in an operand pathArgMatches reads as the rule says", + "xargsBefore": "arrivalsOf and commandNamed: every place the walk arrives at that can be xargs", // payload.go "payloadsOf": "arrivalsOf and nameCouldBe: every env on the walk", @@ -93,7 +95,7 @@ var wordReaders = map[string]string{ "allReserved": "exempt: reserved words are grammar, which no substitution prints", "keywordAt": "exempt: reserved words are grammar, which no substitution prints", "readHeredocDelim": "exempt: the `<<-` operator is grammar", - "Validate": "exempt: reads registry entries, not command words", + "validatePattern": "exempt: reads registry patterns, not command words", "validEntryID": "exempt: reads a registry id, not a command word", } diff --git a/internal/core/ideate/render.go b/internal/core/ideate/render.go index 5446972c9..04502a404 100644 --- a/internal/core/ideate/render.go +++ b/internal/core/ideate/render.go @@ -182,12 +182,15 @@ func blockText(s string) string { return `\` + s } // An ordered-list opener ("1. ", "12) ") is the one multi-character marker. + // The escape goes before the delimiter: a backslash before a digit is a + // literal backslash in CommonMark, and would show in the record + // (iss-2609262241109876). for i := 0; i < len(s); i++ { if s[i] >= '0' && s[i] <= '9' { continue } if i > 0 && (s[i] == '.' || s[i] == ')') { - return `\` + s + return s[:i] + `\` + s[i:] } break } diff --git a/internal/core/ideate/render_codespan_agreement_test.go b/internal/core/ideate/render_codespan_agreement_test.go new file mode 100644 index 000000000..f7e978210 --- /dev/null +++ b/internal/core/ideate/render_codespan_agreement_test.go @@ -0,0 +1,51 @@ +package ideate + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/intentdriven/abcd/internal/core/mdrender" +) + +// TestBlockTextAgreesWithTheRendererOnCodeSpans runs the shared code-span +// agreement table (termsafe's testdata, read by lifeboat's escapeLeadingMarker +// and the site renderer's own test too) through blockText and then the site +// renderer. A value whose leading run is balanced is left as written and +// renders as that span; an unbalanced one is escaped and renders no leading +// span. An escaper and a renderer that pair runs differently disagree here +// (iss-2609262322244502). +func TestBlockTextAgreesWithTheRendererOnCodeSpans(t *testing.T) { + data, err := os.ReadFile(filepath.Join("..", "..", "termsafe", "testdata", "codespan_agreement.json")) + if err != nil { + t.Fatal(err) + } + var table struct { + Cases []struct { + In string `json:"in"` + Balanced bool `json:"balanced"` + Code string `json:"code"` + } + } + if err := json.Unmarshal(data, &table); err != nil || len(table.Cases) == 0 { + t.Fatalf("the code-span agreement table did not load: %v", err) + } + for _, c := range table.Cases { + got := blockText(c.In) + if c.Balanced != (got == c.In) || !c.Balanced && !strings.HasPrefix(got, `\`) { + t.Errorf("blockText(%q) = %q; the table says balanced = %v", c.In, got, c.Balanced) + continue + } + html, err := renderMarkdown(got) + opens := err == nil && strings.HasPrefix(html, "

") + if opens != c.Balanced { + t.Errorf("blockText(%q) = %q: the renderer opens a leading span = %v (err %v); the table says balanced = %v", c.In, got, opens, err, c.Balanced) + continue + } + if c.Balanced && !strings.HasPrefix(html, "

"+mdrender.EscapeText(c.Code)+"") { + t.Errorf("blockText(%q) = %q rendered %q, want a leading span holding %q", c.In, got, html, c.Code) + } + } +} diff --git a/internal/core/ideate/render_codespan_test.go b/internal/core/ideate/render_codespan_test.go index b5c46d77d..80bf1042b 100644 --- a/internal/core/ideate/render_codespan_test.go +++ b/internal/core/ideate/render_codespan_test.go @@ -65,3 +65,45 @@ func TestBlockTextStillEscapesAnUnbalancedLeadingRun(t *testing.T) { } } } + +// TestBlockTextEscapesAnOrderedMarkerFaithfully (iss-2609262241109876): an idea +// shaped like an ordered-list item must neither open a list nor gain a +// character. CommonMark keeps a backslash before a digit as a literal +// backslash, so the escape goes before the delimiter, where it is consumed. +func TestBlockTextEscapesAnOrderedMarkerFaithfully(t *testing.T) { + for _, s := range []string{"1. first", "12) twelve"} { + got := blockText(s) + html, err := renderMarkdown(got) + if err != nil { + t.Fatalf("blockText(%q) = %q does not render: %v", s, got, err) + } + if want := "

" + s + "

"; !strings.Contains(html, want) { + t.Errorf("blockText(%q) = %q rendered as %q; want %q", s, got, html, want) + } + } +} + +// TestBlockTextLeavesATripleBacktickSpanAProseLine (iss-2609262309556167): a +// balanced leading run of three or more backticks is left unescaped, which is +// sound only while the renderer reads it as the span it is. A backtick run +// whose info string holds a backtick opens no fence, so each of these renders +// as a paragraph carrying its text — never an empty command block, never a +// refused page. +func TestBlockTextLeavesATripleBacktickSpanAProseLine(t *testing.T) { + for s, text := range map[string]string{ + "``` ```": "", + "```x```": "x", + "``` x ```": "x", + "```` ``` ````": "```", + } { + got := blockText(s) + html, err := renderMarkdown(got) + if err != nil { + t.Errorf("blockText(%q) = %q does not render: %v", s, got, err) + continue + } + if strings.Contains(html, `class="cmd"`) || !strings.Contains(html, "

"+text) { + t.Errorf("blockText(%q) = %q rendered %q, want a paragraph holding %q", s, got, html, text) + } + } +} diff --git a/internal/core/lab/lab_test.go b/internal/core/lab/lab_test.go index 53940440f..facaca19a 100644 --- a/internal/core/lab/lab_test.go +++ b/internal/core/lab/lab_test.go @@ -785,3 +785,25 @@ func TestListWritesNothing(t *testing.T) { t.Errorf("List created the store: %v", err) } } + +// A literal quoted in backticks is a CommonMark code span, paired by +// termsafe's pairer: a literal holding a backtick is quoted in a longer run, +// and the one space the writer padded it with comes off again. The reader +// closed the literal on the next single backtick, so such a literal was read as +// empty and refused as noise. +func TestParseCorrectionsReadsALiteralAsItsCodeSpan(t *testing.T) { + for line, want := range map[string]string{ + "- retract: `the guard returned 500` a reason": "the guard returned 500", + "- retract: ``run `abcd lint` first`` why": "run `abcd lint` first", + "- retract: `` `quoted` claim ``": "`quoted` claim", + "- retract: ``a```b literal`` why": "a```b literal", + } { + cs := parseCorrections(line + "\n") + if len(cs) != 1 || cs[0].Pattern != want || cs[0].Invalid != "" { + t.Errorf("parseCorrections(%q) = %+v, want the literal %q", line, cs, want) + } + } + if cs := parseCorrections("- retract: `unclosed literal\n"); len(cs) != 1 || cs[0].Invalid == "" { + t.Errorf("an unclosed literal was not refused: %+v", cs) + } +} diff --git a/internal/core/lab/sweep.go b/internal/core/lab/sweep.go index 5ac0aa6e3..06ed44c9b 100644 --- a/internal/core/lab/sweep.go +++ b/internal/core/lab/sweep.go @@ -12,6 +12,7 @@ import ( "unicode/utf8" "github.com/intentdriven/abcd/internal/fsutil" + "github.com/intentdriven/abcd/internal/termsafe" ) // Correction is one retracted claim: the literal text the lab's documents must @@ -53,8 +54,10 @@ var retractRe = regexp.MustCompile("^[-*] retract:\\s*(.*)$") const minPatternRunes = 3 // parseCorrections reads the corrections log: every `- retract: ` line. -// A literal in backticks ends at the closing backtick and the rest of the line -// is its reason; otherwise the whole remainder is the literal. +// A literal in backticks is the code span they open, paired by +// termsafe.PairCodeSpan and read by CommonMark's content rules, so a literal +// holding a backtick is quoted in a longer run (iss-2609262350446885); the rest +// of the line is its reason. Otherwise the whole remainder is the literal. func parseCorrections(doc string) []Correction { var out []Correction for i, line := range strings.Split(doc, "\n") { @@ -65,8 +68,8 @@ func parseCorrections(doc string) []Correction { c := Correction{N: len(out) + 1, Line: i + 1, Instances: []Instance{}} rest := strings.TrimSpace(m[1]) if strings.HasPrefix(rest, "`") { - if end := strings.Index(rest[1:], "`"); end >= 0 { - rest = rest[1 : end+1] + if sp, ok := termsafe.PairCodeSpan(rest, 0); ok { + rest = termsafe.CodeSpanText(sp.Raw(rest)) } else { c.Invalid = "an unclosed backtick" } diff --git a/internal/core/launch/archive.go b/internal/core/launch/archive.go index af4287d4d..91578db6f 100644 --- a/internal/core/launch/archive.go +++ b/internal/core/launch/archive.go @@ -84,8 +84,14 @@ var githubRepositoryRe = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9-]*/[A-Za-z0- type PluginArchive struct { // Name is the release asset's file name, -plugin-v.zip. Name string `json:"name"` - // Path is where the archive was written. - Path string `json:"path"` + // Path is where the archive was written: the absolute working value a + // caller removes or reads the archive through. It never reaches machine + // output (iss-81); DisplayPath is what a report names. + Path string `json:"-"` + // DisplayPath is Path as a report names it (fsutil.DisplayPath): relative + // to the repository when --out is inside it, the home redacted to "~" + // otherwise (iss-2609261950077063). + DisplayPath string `json:"path"` // SHA256 is the lower-case hex digest of the archive's bytes. SHA256 string `json:"sha256"` // Version is the release version stamped into the archived plugin manifest. @@ -134,6 +140,7 @@ func RenderPluginArchive(req PayloadRenderRequest, outDir string) (PluginArchive a.Name = PluginArchiveName(name, req.Version) a.Version = req.Version a.Path = filepath.Join(outDir, a.Name) + a.DisplayPath = fsutil.DisplayPath(req.RepoRoot, a.Path) if _, err := os.Lstat(a.Path); err == nil { return a, res, fmt.Errorf("%s already exists in the output directory — refusing to replace a release archive", a.Name) } diff --git a/internal/core/launch/bundle.go b/internal/core/launch/bundle.go index 99952463e..2efa165bf 100644 --- a/internal/core/launch/bundle.go +++ b/internal/core/launch/bundle.go @@ -22,6 +22,7 @@ import ( "sync" "syscall" + "github.com/intentdriven/abcd/internal/fsutil" "github.com/intentdriven/abcd/internal/gitutil" ) @@ -61,11 +62,15 @@ const ( ) // IncludedFile is a resolved payload file. Paths are repo-relative POSIX; -// ResolvedPath is the absolute on-disk (dereferenced) path. +// ResolvedPath is the absolute on-disk (dereferenced) path every reader opens +// the file through, and it never reaches machine output (iss-81): +// DisplayResolvedPath is the same file named relative to the repository, which +// is what a report carries as resolved_path (iss-2609261954288630). type IncludedFile struct { - LogicalPath string `json:"logical_path"` - ResolvedPath string `json:"resolved_path"` - GitMode string `json:"git_mode"` // "100644" | "100755" + LogicalPath string `json:"logical_path"` + ResolvedPath string `json:"-"` + DisplayResolvedPath string `json:"resolved_path"` + GitMode string `json:"git_mode"` // "100644" | "100755" } // ExcludedFile is a benign exclusion. @@ -318,6 +323,18 @@ func (r *resolver) classifyRegular(rel, abs string, info os.FileInfo, deref bool }) } +// included is the Included entry for one surviving candidate: the working +// absolute path every reader opens, and the same file named relative to the +// repository for the report. +func (r *resolver) included(c candidate) IncludedFile { + return IncludedFile{ + LogicalPath: c.logical, + ResolvedPath: c.resolved, + DisplayResolvedPath: fsutil.DisplayPath(r.root, c.resolved), + GitMode: c.gitMode, + } +} + // handleSymlink resolves a symlink structurally (escape/cycle/deny) and, when // accepted, dereferences it: a file is classified under its logical path; a // directory is walked with its contents emitted under the symlink's prefix. A @@ -498,7 +515,7 @@ func (r *resolver) finalize() { group := byLogical[logical] if len(group) == 1 { c := group[0] - r.result.Included = append(r.result.Included, IncludedFile{LogicalPath: c.logical, ResolvedPath: c.resolved, GitMode: c.gitMode}) + r.result.Included = append(r.result.Included, r.included(c)) continue } // Same logical path from multiple sources: same inode → dedup with a @@ -513,7 +530,7 @@ func (r *resolver) finalize() { } if sameInode { r.result.Warnings = append(r.result.Warnings, "duplicate provenance for "+logical+" (same inode); kept one") - r.result.Included = append(r.result.Included, IncludedFile{LogicalPath: first.logical, ResolvedPath: first.resolved, GitMode: first.gitMode}) + r.result.Included = append(r.result.Included, r.included(first)) } else { r.result.Rejected = append(r.result.Rejected, RejectedFile{LogicalPath: logical, Reason: RejectedDuplicate}) } diff --git a/internal/core/launch/render.go b/internal/core/launch/render.go index d55cc4ae0..569825fb8 100644 --- a/internal/core/launch/render.go +++ b/internal/core/launch/render.go @@ -83,8 +83,14 @@ var renderPathDocAudit = &DocAuditPreflight{ // PayloadRenderResult is a completed render. type PayloadRenderResult struct { - // Dest is the staging directory the payload was written to. - Dest string `json:"dest"` + // Dest is the staging directory the payload was written to: the resolved, + // absolute working value the archive step packs from. It never reaches + // machine output (iss-81); DisplayDest is what a report names. + Dest string `json:"-"` + // DisplayDest is Dest as a report names it (fsutil.DisplayPath): relative + // to the repository inside it, the home redacted to "~" outside it — and a + // destination is always outside it (iss-2609261848338673). + DisplayDest string `json:"dest"` // Version is the version stamped at every pinned location. Version string `json:"version"` // Bundle is the resolution the payload was written from, so a caller can @@ -432,6 +438,7 @@ func RenderPayload(req PayloadRenderRequest) (PayloadRenderResult, error) { primaryPath, primaryPtr := pre.PrimaryPath, pre.PrimaryPointer bundle := pre.Bundle res.Dest = dest + res.DisplayDest = fsutil.DisplayPath(req.RepoRoot, dest) res.Bundle = bundle if err := os.MkdirAll(dest, 0o755); err != nil { diff --git a/internal/core/launch/reportpaths_test.go b/internal/core/launch/reportpaths_test.go new file mode 100644 index 000000000..4e8707d90 --- /dev/null +++ b/internal/core/launch/reportpaths_test.go @@ -0,0 +1,144 @@ +package launch + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// noHomeIn fails when raw names the home directory in either spelling — the +// one the environment gives and the symlink-resolved one the kernel reports — +// which is the developer-identity path machine output never carries (iss-81). +func noHomeIn(t *testing.T, what string, raw []byte, home string) { + t.Helper() + spellings := []string{home} + if real, err := filepath.EvalSymlinks(home); err == nil && real != home { + spellings = append(spellings, real) + } + for _, h := range spellings { + if strings.Contains(string(raw), h) { + t.Errorf("%s carries the home directory %q:\n%s", what, h, raw) + } + } +} + +// homeFixture is renderFixture laid down under a fresh home directory, so every +// absolute path the render and the archive work with sits under $HOME — the +// shape that named the developer in `launch ship --json` and +// `launch archive --json`. +func homeFixture(t *testing.T) (home, root string) { + t.Helper() + home = t.TempDir() + t.Setenv("HOME", home) + root = filepath.Join(home, "src", "repo") + writeFile(t, root, ".abcd/config/launch-payload.json", `{"includes": [".claude-plugin", "README.md"]}`) + writeFile(t, root, "README.md", "readme\n") + writeLockstepTree(t, root, "", "", "") + writeFile(t, root, ".claude-plugin/plugin.json", + `{"name": "abcd", "repository": "https://github.com/example/abcd"}`) + return home, root +} + +// The payload's destination is reported without the home path: payload.dest +// names the staging directory with the home redacted to "~", and every bundle +// file's resolved_path is repository-relative — while the render still writes +// to, and the archive still packs from, the real directory +// (iss-2609261848338673, iss-2609261950077063). +func TestTheRenderAndTheArchiveReportTheirPathsWithoutTheHome(t *testing.T) { + home, root := homeFixture(t) + out := filepath.Join(home, "dist") + if err := os.MkdirAll(out, 0o755); err != nil { + t.Fatal(err) + } + + a, res, err := RenderPluginArchive(PayloadRenderRequest{ + RepoRoot: root, Dest: filepath.Join(home, "staging"), Version: "1.2.3", Entry: sampleEntry(), Dirty: DirtySkip, + }, out) + if err != nil { + t.Fatalf("RenderPluginArchive: %v", err) + } + + rawRes, err := json.Marshal(res) + if err != nil { + t.Fatal(err) + } + noHomeIn(t, "the render's JSON", rawRes, home) + var gotRes struct { + Dest string `json:"dest"` + Bundle struct { + Files []struct { + LogicalPath string `json:"logical_path"` + ResolvedPath string `json:"resolved_path"` + } `json:"files"` + } `json:"bundle"` + } + if err := json.Unmarshal(rawRes, &gotRes); err != nil { + t.Fatal(err) + } + if gotRes.Dest != "~/staging" { + t.Errorf("payload.dest = %q, want ~/staging", gotRes.Dest) + } + if len(gotRes.Bundle.Files) == 0 { + t.Fatal("the render reported no bundle files") + } + for _, f := range gotRes.Bundle.Files { + if f.ResolvedPath != f.LogicalPath { + t.Errorf("bundle file %q: resolved_path = %q, want it repository-relative", f.LogicalPath, f.ResolvedPath) + } + } + + rawArchive, err := json.Marshal(a) + if err != nil { + t.Fatal(err) + } + noHomeIn(t, "the archive's JSON", rawArchive, home) + var gotArchive struct { + Path string `json:"path"` + } + if err := json.Unmarshal(rawArchive, &gotArchive); err != nil { + t.Fatal(err) + } + if want := "~/dist/" + a.Name; gotArchive.Path != want { + t.Errorf("archive.path = %q, want %q", gotArchive.Path, want) + } + + // The working values still reach the directories: the payload is on disk + // where the render put it, and the archive opens from where it was written. + if _, err := os.Stat(filepath.Join(home, "staging", ".claude-plugin", "plugin.json")); err != nil { + t.Errorf("the staged payload is not where the render was told to write it: %v", err) + } + if entries := zipEntries(t, filepath.Join(out, a.Name)); len(entries) == 0 { + t.Error("the archive written to --out is empty") + } +} + +// An archive written inside the repository — the release workflow's +// `--out bin` — is reported relative to it. +func TestAnArchiveInsideTheRepositoryIsReportedRelativeToIt(t *testing.T) { + home, root := homeFixture(t) + out := filepath.Join(root, "bin") + if err := os.MkdirAll(out, 0o755); err != nil { + t.Fatal(err) + } + a, _, err := RenderPluginArchive(PayloadRenderRequest{ + RepoRoot: root, Dest: filepath.Join(home, "staging"), Version: "1.2.3", Entry: sampleEntry(), Dirty: DirtySkip, + }, out) + if err != nil { + t.Fatalf("RenderPluginArchive: %v", err) + } + raw, err := json.Marshal(a) + if err != nil { + t.Fatal(err) + } + var got struct { + Path string `json:"path"` + } + if err := json.Unmarshal(raw, &got); err != nil { + t.Fatal(err) + } + if want := "bin/" + a.Name; got.Path != want { + t.Errorf("archive.path = %q, want %q", got.Path, want) + } +} diff --git a/internal/core/lifeboat/embark.go b/internal/core/lifeboat/embark.go index 333750f65..37f2d9375 100644 --- a/internal/core/lifeboat/embark.go +++ b/internal/core/lifeboat/embark.go @@ -53,8 +53,8 @@ func EmbarkProbe(lifeboatDir, targetDir string) (EmbarkPlan, error) { marker := embarkMarker(pr.targetAbs, true) return EmbarkPlan{ SchemaVersion: EmbarkSchemaVersion, - LifeboatDir: pr.lifeboatAbs, - TargetDir: pr.targetAbs, + LifeboatDir: fsutil.RedactHome(pr.lifeboatAbs), + TargetDir: fsutil.RedactHome(pr.targetAbs), SourceName: pr.prov.SourceName, ManifestVerified: true, ManifestSHA256: pr.prov.ManifestSHA256, @@ -81,8 +81,8 @@ func EmbarkFrom(lifeboatDir, targetDir string) (EmbarkResult, error) { } res := EmbarkResult{ SchemaVersion: EmbarkSchemaVersion, - LifeboatDir: pr.lifeboatAbs, - TargetDir: pr.targetAbs, + LifeboatDir: fsutil.RedactHome(pr.lifeboatAbs), + TargetDir: fsutil.RedactHome(pr.targetAbs), SourceName: pr.prov.SourceName, Coverage: pr.coverage, Ignored: pr.ignored, @@ -192,6 +192,9 @@ func VerifyManifest(dir string) error { if err != nil { return err } + if err := proveOperand("lifeboat", abs); err != nil { + return err + } if !fsutil.IsRealDir(abs) { return fmt.Errorf("lifeboat %s is not a directory", filepath.Base(abs)) } @@ -259,6 +262,15 @@ func runPlanner(lifeboatDir, targetDir string) (plannerResult, error) { return plannerResult{}, err } + // Both operands are proved against a symlinked ancestor before either is + // read, so a refused target is refused before the lifeboat is verified. + if err := proveOperand("lifeboat", lifeboatAbs); err != nil { + return plannerResult{}, err + } + if err := proveOperand("target", targetAbs); err != nil { + return plannerResult{}, err + } + // Gate the lifeboat: a real directory carrying a parseable _provenance.json. if !fsutil.IsRealDir(lifeboatAbs) { return plannerResult{}, fmt.Errorf("lifeboat %s is not a directory", filepath.Base(lifeboatAbs)) diff --git a/internal/core/lifeboat/graveyard_lessons.go b/internal/core/lifeboat/graveyard_lessons.go index 31c15f73b..8afd78648 100644 --- a/internal/core/lifeboat/graveyard_lessons.go +++ b/internal/core/lifeboat/graveyard_lessons.go @@ -44,7 +44,11 @@ func IngestLessons(lifeboatDir string, raw []byte) (LessonsResult, error) { if err != nil { return LessonsResult{}, err } - // 1. Gate the lifeboat: a real directory carrying a parseable _provenance.json. + // 1. Gate the lifeboat: a real directory, reached through no symlinked + // ancestor inside a checkout, carrying a parseable _provenance.json. + if err := proveOperand("lifeboat", abs); err != nil { + return LessonsResult{}, err + } if !fsutil.IsRealDir(abs) { return LessonsResult{}, fmt.Errorf("lifeboat %s is not a directory", filepath.Base(abs)) } @@ -93,7 +97,7 @@ func IngestLessons(lifeboatDir string, raw []byte) (LessonsResult, error) { } // 4. Per-entry validation, drop-not-fatal. - res := LessonsResult{LifeboatDir: abs} + res := LessonsResult{LifeboatDir: fsutil.RedactHome(abs)} seen := map[string]bool{} var mainLessons, lowLessons []Lesson for _, in := range lf.Lessons { diff --git a/internal/core/lifeboat/mdrender.go b/internal/core/lifeboat/mdrender.go new file mode 100644 index 000000000..fe621cbb8 --- /dev/null +++ b/internal/core/lifeboat/mdrender.go @@ -0,0 +1,101 @@ +package lifeboat + +import ( + "strings" + + "github.com/intentdriven/abcd/internal/termsafe" +) + +// mdrender.go — the one render discipline for every markdown file the lifeboat +// writes (principles.md, press-release.md, review/review-.md and the +// packed brief section docs). It is the discipline the memory renderers got for +// iss-2609020539188868, applied to the lifeboat half (iss-2609251355497247): +// +// - every untrusted field on a markdown line goes through the file-write +// cleaner, termsafe.CleanProse, never Sanitize alone — Sanitize defangs a +// terminal and leaves an HTML comment opener or link syntax live; +// - no renderer wraps a cleaned value in a delimiter of its own (brackets, +// emphasis, backticks): termsafe's guarantees hold over the exact string it +// returned, and a wrapper the value can close parses a different string; +// where a delimiter is wanted, termsafe.CodeSpan picks one the value cannot +// close, without altering the value's bytes; +// - a value that begins a block has its leading marker escaped, so a field +// cannot turn itself into a heading, a list, a quote, a fence, a table or +// a link reference definition. +// +// Re-cleaning a field its ingest already cleaned is a no-op (the cleaner is +// idempotent), so this costs a well-formed record nothing and holds for any +// field that reaches a renderer by another route. + +// maxMDFieldBytes is the render-time cap: the largest any lifeboat field is +// cleaned to at ingest (the press-release body), so the render never cuts a +// field its ingest kept. +const maxMDFieldBytes = maxPressReleaseBodyBytes + +// mdInline is an untrusted field placed mid-line: cleaned, nothing added. +func mdInline(s string) string { return termsafe.CleanProse(s, maxMDFieldBytes) } + +// mdCode is an untrusted field set off as a code span whose fence the value +// cannot close. An empty value renders as nothing. +func mdCode(s string) string { return termsafe.CodeSpan(mdInline(s)) } + +// mdCodeList renders refs as comma-separated code spans. +func mdCodeList(refs []string) string { + out := make([]string, 0, len(refs)) + for _, r := range refs { + if c := mdCode(r); c != "" { + out = append(out, c) + } + } + return strings.Join(out, ", ") +} + +// mdBlock is an untrusted field that begins a block (a paragraph, a quote's +// first line): cleaned, then its leading marker escaped. +func mdBlock(s string) string { return escapeLeadingMarker(mdInline(s)) } + +// escapeLeadingMarker backslash-escapes the character that would make s open a +// block construct rather than a paragraph: an ATX heading (#), a bullet (- * +), +// a block quote (>), a fence (` ~), a table row (|), a thematic break or setext +// underline (- _ * =), raw HTML (<), a link reference definition ([), or an +// ordered-list marker (digits then . or )). CommonMark renders a +// backslash-escaped ASCII punctuation character as the character itself, so the +// reader sees the value's text unchanged. +// +// The bracket is the subtle one: a paragraph shaped like `[label]: ` is +// consumed as a definition and renders as nothing, and it arms a `[label]` +// shortcut reference in any other field, which the cleaner leaves alone, as a +// live link to the attacker's destination (iss-2609262237352137). ideate's +// blockText escapes it for the same reason. +// +// A leading backtick is escaped only when its run is UNBALANCED, which is the +// only run that opens a fence: a backtick fence's info string may not contain +// backticks, so a run with a matching closer on the same line is an inline +// span. The cleaner's HTML-tag rule exempts a span, and that exemption holds +// only while the value is parsed as the string it was cleaned as; escaping a +// balanced run's opener kills the span and republishes the tag it sheltered +// as live HTML (iss-2609262237415400, the defect ideate's blockText was fixed +// for). +func escapeLeadingMarker(s string) string { + if s == "" { + return s + } + if s[0] == '`' { + if termsafe.OpensBalancedCodeSpan(s) { + return s + } + return `\` + s + } + switch s[0] { + case '#', '-', '*', '+', '>', '~', '|', '=', '_', '<', '[': + return `\` + s + } + digits := 0 + for digits < len(s) && digits < 10 && s[digits] >= '0' && s[digits] <= '9' { + digits++ + } + if digits > 0 && digits < len(s) && (s[digits] == '.' || s[digits] == ')') { + return s[:digits] + `\` + s[digits:] + } + return s +} diff --git a/internal/core/lifeboat/mdrender_codespan_agreement_test.go b/internal/core/lifeboat/mdrender_codespan_agreement_test.go new file mode 100644 index 000000000..1288615aa --- /dev/null +++ b/internal/core/lifeboat/mdrender_codespan_agreement_test.go @@ -0,0 +1,51 @@ +package lifeboat + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/intentdriven/abcd/internal/core/mdrender" +) + +// TestBlockEscaperAgreesWithTheRendererOnCodeSpans runs the shared code-span +// agreement table (termsafe's testdata, read by ideate's blockText and the site +// renderer's own test too) through escapeLeadingMarker and then the site +// renderer. A value whose leading run is balanced is left as written and +// renders as that span; an unbalanced one is escaped and renders no leading +// span. An escaper and a renderer that pair runs differently disagree here +// (iss-2609262322244502). +func TestBlockEscaperAgreesWithTheRendererOnCodeSpans(t *testing.T) { + data, err := os.ReadFile(filepath.Join("..", "..", "termsafe", "testdata", "codespan_agreement.json")) + if err != nil { + t.Fatal(err) + } + var table struct { + Cases []struct { + In string `json:"in"` + Balanced bool `json:"balanced"` + Code string `json:"code"` + } + } + if err := json.Unmarshal(data, &table); err != nil || len(table.Cases) == 0 { + t.Fatalf("the code-span agreement table did not load: %v", err) + } + for _, c := range table.Cases { + got := escapeLeadingMarker(c.In) + if c.Balanced != (got == c.In) || !c.Balanced && !strings.HasPrefix(got, `\`) { + t.Errorf("escapeLeadingMarker(%q) = %q; the table says balanced = %v", c.In, got, c.Balanced) + continue + } + html, err := siteRender(t, got) + opens := err == nil && strings.HasPrefix(html, "

") + if opens != c.Balanced { + t.Errorf("escapeLeadingMarker(%q) = %q: the renderer opens a leading span = %v (err %v); the table says balanced = %v", c.In, got, opens, err, c.Balanced) + continue + } + if c.Balanced && !strings.HasPrefix(html, "

"+mdrender.EscapeText(c.Code)+"") { + t.Errorf("escapeLeadingMarker(%q) = %q rendered %q, want a leading span holding %q", c.In, got, html, c.Code) + } + } +} diff --git a/internal/core/lifeboat/mdrender_test.go b/internal/core/lifeboat/mdrender_test.go new file mode 100644 index 000000000..2861f2b41 --- /dev/null +++ b/internal/core/lifeboat/mdrender_test.go @@ -0,0 +1,197 @@ +package lifeboat + +import ( + "strings" + "testing" + + "github.com/intentdriven/abcd/internal/core/site" +) + +// The lifeboat half of iss-2609020539188868 (iss-2609251355497247): every +// untrusted field on a line of a markdown file the lifeboat writes goes through +// the file-write cleaner (termsafe.CleanProse), never Sanitize alone; no renderer +// wraps a cleaned value in a delimiter of its own (a code span through +// termsafe.CodeSpan where one is wanted); and a value that opens a block is +// escaped so its leading marker cannot turn it into a heading, list, quote or +// fence. These tests drive each renderer with the hostile value directly, so +// the render's discipline is proved on its own and not only through whatever an +// ingest happened to clean first. + +// rawMarkdownHazards are the constructs a cleaned value must never carry live. +var rawMarkdownHazards = []string{"