From e76c78fb6b02768be6cf0e451b972a90292c3736 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:31:54 +0100 Subject: [PATCH 01/11] fix(guard): refuse a recursive delete of the root or the home directory The bundled registry's one rm entry fired only behind a cd chain, so `rm -rf /`, `rm -rf ~` and `rm -rf $HOME` were an allow while the guard chapter's headline promised the catch of an rm -rf with an unlucky glob. A new additive Pattern field, arg_values, matches an operand by its exact word (known text; empty and dash-led values are refused at load). Two bundled entries use it: - rm-rf-root-or-home (blocker): /, /*, ~, ~/, ~/*, $HOME, ${HOME} and their / and /* forms, under the recursive flag alone. - rm-rf-working-directory (warn, graded like git clean): *, ., ./, ./*, .., ../, ../* and .*, under the recursive flag alone. A target printed whole by a substitution is read by its known text, a third recorded operand residual beside the + refspec prefix, so `rm -rf $(find ...)` stays an allow. The cost tests' per-byte bar moves from 20 to 24 because the operand walk reads every token once per entry. Tests that used a bare `rm -rf *` as their ordinary-work probe now use `rm -rf ./build`; a host workdir is still never read as a cd. The decision is appended to DECISIONS.md; the guard chapter, the plugin page and the check's help state the coverage and the residual. Refs: iss-2609282105242542 Assisted-by: Claude:claude-opus-5-5 --- .../development/brief/04-surfaces/17-guard.md | 16 ++- .abcd/work/DECISIONS.md | 1 + commands/guard.md | 11 +- docs/reference/cli/commands.md | 4 +- internal/core/guard/config.go | 3 + internal/core/guard/defaults/guard.json | 67 ++++++++++ internal/core/guard/gitconfig_test.go | 4 +- internal/core/guard/guard.go | 19 +++ internal/core/guard/guard_test.go | 2 +- internal/core/guard/match.go | 19 ++- internal/core/guard/rmtargets_test.go | 117 ++++++++++++++++++ internal/core/guard/speculate_test.go | 9 +- internal/core/guard/unknown.go | 17 ++- internal/core/guard/unknownreaders_test.go | 26 +++- internal/core/guard/work_test.go | 11 +- internal/core/guard/workdir_test.go | 11 +- internal/surface/cli/guard.go | 4 +- internal/surface/cli/guard_workdir_test.go | 13 +- 18 files changed, 327 insertions(+), 27 deletions(-) create mode 100644 internal/core/guard/rmtargets_test.go diff --git a/.abcd/development/brief/04-surfaces/17-guard.md b/.abcd/development/brief/04-surfaces/17-guard.md index e187f5fdb..ba78bd42c 100644 --- a/.abcd/development/brief/04-surfaces/17-guard.md +++ b/.abcd/development/brief/04-surfaces/17-guard.md @@ -273,10 +273,24 @@ 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. +A recursive delete is read by what it deletes. Of the filesystem root or the +home directory (`/`, `/*`, `~`, `$HOME`, `${HOME}`, each also with a trailing +`/` or `/*`) it is a block wherever it stands, with or without `-f`. Of the +directory the shell is in or the one above it (`*`, `.`, `..`, `./*`, `../*`, +`.*`) it is a warn, graded like `git clean`, because that directory is usually +the repository and emptying a build directory the same way is ordinary work. +Chained after a `cd` any recursive forced delete blocks, as above. The target is +compared as written, before the shell expands it, so `$HOME` is seen as the +word `$HOME` although no other parameter expansion is. + What an allow still does not see is a hazard that never reaches command position at all: a word that is wholly a command substitution standing where a flag would be, which is read as an operand because that is how a commit message or a -branch name is spelled every day; one behind a wrapper flag the per-wrapper +branch name is spelled every day; a delete target printed whole by a +substitution (`rm -rf $(echo /)`), which is read by its known text because that +is how an everyday delete names what it removes (`rm -rf $(find . -name +'*.pyc')`); a target spelled any other way than the words above (`rm -rf +"$DIR"/*` with `DIR` unset, `rm -rf /?*`); one behind a wrapper flag the per-wrapper 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 diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 93fe892f4..9dc522248 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -2573,3 +2573,4 @@ together (the script's header says why there is no escape hatch). - 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). - 2026-09-28 — Rulings Z, AR and the cancel policy, given by the user as technical facilitator at 15:10:37Z (autonomous run A, recorded by lane cap45 for orchestrator abcd-a8). (Z) The macOS leg of ci.yml's `check` job and the main ruleset's merge-queue `check_response_timeout_minutes` both rise from 30 to 45 minutes, because a 30-minute cap cancelled passing macOS runs (#728, #730 twice, #733; iss-2609281514435020). This amends the standing rule that never raises the check job's timeout or edits the ruleset, for exactly this one change: 45 minutes, those two settings; every other timeout, every required-check name or split and every other ruleset field stays as it is, and the ubuntu leg keeps 30. The workflow and the `.abcd/work/rulesets/main-protection.json` mirror change through a reviewed pull request; the live ruleset is changed with `gh api` by the run's orchestrator after that pull request merges, and until then the live queue still fails a group at 30 minutes. (AR) All three speed-ups of iss-2609261924541555 are built: a test-only switch that skips the disk flush in the atomic write code and the slowest -race package first in the race step (lane ciSpeed), and the scanner's per-identity git calls folded into one (lane scanFold), a trust path that is security-reviewed before it lands. (Cancel policy) The rerun-once rule stands: a check cancelled at the cap is rerun once, and a second cancellation for the same reason stops that pull request and opens a speed lane, never another raise of the timeout; and a step on the macOS leg warns, in the log and the step summary, once the check has run past 35 minutes, without ever failing the job, so a speed lane opens before any cancellation. - 2026-09-28 — Release v0.11.1 is cut by autonomous run A, and the run's agenda line is: approve the publish step. Under ruling A2 of the product thinker's run A interview (2026-09-23 07:52Z: the run approves the release environment itself once every gate is green) and the product thinker's releases ruling of 2026-09-25T08:04:52Z ("cut additional releases if that makes sense, but bundle multiple intents for it"), the run approves the `release` environment's deployment of v0.11.1 only after the merge queue, the verify job and every other gate on the tagged commit report green, and stops with a handover instead if any does not. The cut: v0.11.1, impact additive (no breaking record since v0.11.0; the run had been calling it v0.12.0 until `launch ship` derived the version), 354 records since v0.11.0: eighteen shipped intents, all additive, and 336 resolved or declined issues (228 fixes, eighteen additive, 90 internal and outside the changelog); the release guard and the findings guard passed with no waiver. Content commit 8a6c83d5, on top of 9ead1d1bf, which the docs-currency gate's findings required. Both semantic gates ran at tier full. The docs-currency-reviewer (Fable 5.1) read the first roll 7c7f5525 and found four minor findings (two stale terminology rows, a README sample status line no state renders, and the root command sentence missing three dispatched record families), all fixed in 9ead1d1bf. The brief-surface cross-check (44 pinned checkers, Opus 5.5, at most four alive) found 127; an independent classification (Fable 5.1) found 126 real at the content commit: the two user-facing and three behaviour findings are captured as four records (iss-2609282105240689, iss-2609282105242542, iss-2609282105241960, iss-2609282105240081), the one major among them, the guard registry passing a bare `rm -rf /` (iss-2609282105242542), deferred out loud past v0.11.0 because it already shipped in v0.11.0 and a registry change needs its own tests and review, all four to be fixed in the first lane after the tag; the design-record drift goes to the systematic brief pass iss-2609091956001547. Landed before the cut on the cutting session's ruling: #736 (integration branch 11); integration branches 12, 13 and 14, reviewed and ready, hold until the tag and fall into the next release. +- 2026-09-28 — guard registry, the rm targets (iss-2609282105242542, autonomous run A lane gateFix): the chapter's headline promised the catch of "an `rm -rf` with an unlucky glob" while the one rm entry fired only behind a `cd` chain, so `rm -rf /` and `rm -rf ~` were an allow. Two bundled entries close it through one new additive Pattern field, `arg_values` (some operand is exactly one of the listed words, by its known text; empty and dash-led values are load-time rejections like an empty prefix). `rm-rf-root-or-home` is a BLOCKER on `/`, `/*`, `~`, `$HOME`, `${HOME}` and their `/` and `/*` forms; `rm-rf-working-directory` is a WARN on `*`, `.`, `..`, `./`, `../`, `./*`, `../*` and `.*`, graded like `git clean` because emptying a build directory the same way is ordinary work and a blocker there would teach sessions to route around the guard. Both require the recursive flag alone, not `-f`: for an agent, whose stdin is not a terminal, rm does not prompt, so `-f` changes nothing about what is destroyed (the cd-chain entry keeps its `-f` requirement; changing a blocker's pattern is a separate act). This reverses the incidental stance that a bare `rm -rf *` is not a hazard (the lab finding iss-2609012040019014 and two tests that used it as their ordinary-work probe): those tests keep their point with `rm -rf ./build` as the probe, and a host workdir is still never read as a `cd`. A third operand residual is recorded beside the two unknown.go names: a target a substitution prints WHOLE (`rm -rf $(echo /)`) is read by its known text, as the `+` refspec prefix is, because `rm -rf $(find . -name '*.pyc')` is how an everyday delete names its targets and reading the word as every target would block it as a delete of `/`; glued text still counts (`"$(true)"/` is `/`). Parameter expansions stay unread except as the literal words the entry names (`rm -rf "$DIR"/*` with `DIR` unset is not seen). The per-byte work bar of the guard's cost tests moves from 20 to 24: the operand walk reads every token once per entry, and sixteen entries put the costliest asserted shape at about 20. diff --git a/commands/guard.md b/commands/guard.md index 4cfda6835..f13bd168f 100644 --- a/commands/guard.md +++ b/commands/guard.md @@ -296,8 +296,17 @@ 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. +A recursive delete of the filesystem root or the home directory (`/`, `/*`, +`~`, `$HOME`, `${HOME}`, each also with a trailing `/` or `/*`) is a **block** +(`rm-rf-root-or-home`), with or without `-f`; one of the directory the shell is +in or the one above it (`*`, `.`, `..`, `./*`, `../*`, `.*`) is a **warn** +(`rm-rf-working-directory`). The target is compared as written, so `$HOME` is +seen as that word. + 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 +at all: a delete target printed whole by a substitution (`rm -rf $(echo /)`), +read by its known text the way `rm -rf $(find …)` names its targets every day, +or spelled any other way than the words above; 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 `sudo -Hu bob ` reaches only the warn, not the entry that names it), one whose API path an entry names by its ROOT diff --git a/docs/reference/cli/commands.md b/docs/reference/cli/commands.md index 76d7da91a..326d0773e 100644 --- a/docs/reference/cli/commands.md +++ b/docs/reference/cli/commands.md @@ -924,7 +924,9 @@ An unquoted brace group IS expanded as bash expands it, and one past 4096 words is blocked. What an allow still does not see is a hazard that never reaches command position at all: a word that is wholly a `$(…)` standing where a flag would be (read as -an operand, the way a commit message or a branch is spelled), one launched +an operand, the way a commit message or a branch is spelled), a delete +target printed whole by one (`rm -rf $(echo /)`, read by its known text +the way `rm -rf $(find …)` names its targets every day), 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 `sudo -Hu bob ` reaches diff --git a/internal/core/guard/config.go b/internal/core/guard/config.go index 2d8d7e204..6a9c364ca 100644 --- a/internal/core/guard/config.go +++ b/internal/core/guard/config.go @@ -261,6 +261,9 @@ func mergePattern(base, over Pattern) Pattern { if over.ArgPrefixes != nil { r.ArgPrefixes = append([]string(nil), over.ArgPrefixes...) } + if over.ArgValues != nil { + r.ArgValues = append([]string(nil), over.ArgValues...) + } if over.FlagValues != nil { r.FlagValues = cloneFlagValues(over.FlagValues) } diff --git a/internal/core/guard/defaults/guard.json b/internal/core/guard/defaults/guard.json index f0788252b..8dafd44fb 100644 --- a/internal/core/guard/defaults/guard.json +++ b/internal/core/guard/defaults/guard.json @@ -39,6 +39,73 @@ ] } }, + "rm-rf-root-or-home": { + "tier": "blocker", + "pattern": { + "command": "rm", + "flags": ["-r|-R|--recursive"], + "arg_values": ["/", "/*", "~", "~/", "~/*", "$HOME", "$HOME/", "$HOME/*", "${HOME}", "${HOME}/", "${HOME}/*"] + }, + "why": "A recursive delete of `/` or of the home directory destroys the whole machine or every file the account owns — other projects, keys, settings — and nothing brings any of it back.", + "successor": "Name the one directory you mean, by its full path (`rm -rf -- /absolute/path/to/target`), and never the root or the home directory itself.", + "fixtures": { + "known_bad": [ + "rm -rf /", + "rm -rf /*", + "rm -fr ~", + "rm -r -f ~/", + "rm --recursive --force $HOME", + "rm -rf \"$HOME\"/*", + "rm -rf ${HOME}", + "rm -r /", + "sudo rm -rf --no-preserve-root /", + "env rm -rf ~/*", + "sh -c 'rm -rf ~'" + ], + "known_good": [ + "rm -rf /tmp/build", + "rm -rf ~/scratch/build", + "rm -rf \"$HOME/.cache/abcd-test\"", + "rm ~/notes.txt", + "rm -f /", + "ls -la /", + "printf '%s\\n' \"rm -rf /\"", + "abcd capture \"an agent ran rm -rf ~ and the home directory was gone\"", + "cat > NOTES.md <<'EOF'\nNever run rm -rf / or rm -rf ~.\nEOF" + ] + } + }, + "rm-rf-working-directory": { + "tier": "warn", + "pattern": { + "command": "rm", + "flags": ["-r|-R|--recursive"], + "arg_values": ["*", ".", "./", "./*", "..", "../", "../*", ".*"] + }, + "why": "A recursive delete of `*`, `.` or `..` removes everything in the directory the shell is in, or the one above it — and the directory the shell is in is usually the repository itself, uncommitted work included.", + "successor": "Name the directory you mean by its path (`rm -rf -- ./build`), or check where you are first (`pwd`) and delete from the directory's parent by name.", + "fixtures": { + "known_bad": [ + "rm -rf *", + "rm -fr .", + "rm -r -f ./*", + "rm --recursive --force ..", + "rm -rf ../*", + "rm -rf .*", + "sudo rm -rf *", + "rm -r *" + ], + "known_good": [ + "rm -rf ./build", + "rm -rf build/*", + "rm -rf node_modules", + "rm -f *.o", + "rm *", + "ls -la *", + "abcd capture \"an agent ran rm -rf * in the repository root\"" + ] + } + }, "git-push-force": { "tier": "blocker", "pattern": { diff --git a/internal/core/guard/gitconfig_test.go b/internal/core/guard/gitconfig_test.go index a4961a772..966ac4869 100644 --- a/internal/core/guard/gitconfig_test.go +++ b/internal/core/guard/gitconfig_test.go @@ -163,12 +163,12 @@ func TestEachBangAliasBodyGetsItsOwnChainRange(t *testing.T) { }{ { "a cd in one bang body does not reach an rm in another", - `git -c alias.a='!cd /tmp' a && git -c alias.b='!rm -rf .' b`, + `git -c alias.a='!cd /tmp' a && git -c alias.b='!rm -rf ./build' b`, VerdictAllow, "", }, { "the sh -c twin, which already read it that way", - `sh -c 'cd /tmp' && sh -c 'rm -rf .'`, + `sh -c 'cd /tmp' && sh -c 'rm -rf ./build'`, VerdictAllow, "", }, { diff --git a/internal/core/guard/guard.go b/internal/core/guard/guard.go index d51405929..05397a173 100644 --- a/internal/core/guard/guard.go +++ b/internal/core/guard/guard.go @@ -78,6 +78,13 @@ type Pattern struct { // leading `+` on the refspec is a force push by another name and Flags has // nothing to look at. ArgPrefixes []string `json:"arg_prefixes,omitempty"` + // ArgValues constrain an OPERAND to one exact word: some non-flag argument + // must be one of the listed words, compared by its known text. It is what + // separates `rm -rf /` and `rm -rf ~`, which destroy the machine or the + // home directory, from `rm -rf /tmp/build`, which a prefix could not tell + // apart. The words are compared as written, before the shell expands them: + // `$HOME` is the word `$HOME`, and `*` the word `*`. + ArgValues []string `json:"arg_values,omitempty"` // MinOperands, when set, requires at least that many non-flag arguments // (value_flags stepped over). It is what separates a kill BY PATTERN — // `pkill make`, whose operand is the pattern — from `pkill -g 4242`, which @@ -332,6 +339,17 @@ func validatePattern(id string, p Pattern) error { return fmt.Errorf("%w: entry %s argument prefix %q starts with a dash and could never match a non-flag argument", ErrInvalidEntry, id, prefix) } } + // An empty argument value matches an empty operand nobody meant, and a + // dashed one describes an operand nothing can be: the two defangs the + // prefix check above refuses, one field along. + for i, value := range p.ArgValues { + if strings.TrimSpace(value) == "" { + return fmt.Errorf("%w: entry %s argument value %d is empty and could never name a target", ErrInvalidEntry, id, i) + } + if strings.HasPrefix(value, "-") { + return fmt.Errorf("%w: entry %s argument value %q starts with a dash and could never match a non-flag argument", ErrInvalidEntry, id, value) + } + } // 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 { @@ -790,6 +808,7 @@ func clonePattern(p Pattern) Pattern { out.ValueFlags = append([]string(nil), p.ValueFlags...) out.Flags = append([]string(nil), p.Flags...) out.ArgPrefixes = append([]string(nil), p.ArgPrefixes...) + out.ArgValues = append([]string(nil), p.ArgValues...) out.ArgPaths = append([]PathArg(nil), p.ArgPaths...) out.FlagValues = cloneFlagValues(p.FlagValues) out.ArgsFrom = clonePatterns(p.ArgsFrom) diff --git a/internal/core/guard/guard_test.go b/internal/core/guard/guard_test.go index ef583fc42..913f7065f 100644 --- a/internal/core/guard/guard_test.go +++ b/internal/core/guard/guard_test.go @@ -280,7 +280,7 @@ func TestBlockerWinsOverWarn(t *testing.T) { if d.Verdict != VerdictBlock || d.EntryID != "rm-rf-after-cd-chain" { t.Fatalf("blocker must win over warn, got %+v", d) } - if len(d.Matches) != 2 { + if !contains(d.Matches, "rm-rf-after-cd-chain") || !contains(d.Matches, "git-reset-hard") { t.Fatalf("Matches = %v, want both the blocker and the warn entry", d.Matches) } } diff --git a/internal/core/guard/match.go b/internal/core/guard/match.go index 51403877c..fea91c0eb 100644 --- a/internal/core/guard/match.go +++ b/internal/core/guard/match.go @@ -550,7 +550,7 @@ func newEntryMatcher(p Pattern, tokens []string, glob func(int) bool) entryMatch n := len(tokens) want := operandWant{ sub: p.Subcommand, sub2: p.Subcommand2, min: p.MinOperands, - prefixes: p.ArgPrefixes, paths: p.ArgPaths, + prefixes: p.ArgPrefixes, paths: p.ArgPaths, values: p.ArgValues, } m := entryMatcher{accept: operandAcceptance(tokens, p.ValueFlags, want, glob), nextStop: make([]int, n+1)} m.nextStop[n] = n @@ -678,6 +678,23 @@ func argPrefixMatches(prefix string, ops []string) bool { return false } +// argValueMatches reports whether some operand is one of the words. Only +// operands are considered, and each by its known text, as argPrefixMatches +// reads a prefix: a word that is wholly a substitution is how an everyday +// delete names its target (`rm -rf "$(mktemp -d)"`), so reading it as every +// target would refuse them all (unknown.go's operand residual). +func argValueMatches(values []string, ops []string) bool { + for _, op := range ops { + k := knownText(op) + for _, v := range values { + if k == v { + return true + } + } + } + return false +} + // flagGroupHit reports whether the token at i is an alternative of one "a|b" // flag group. glob reports, per token index, whether bash would expand that // token. The caller reads the tokens only up to `--` (entryMatcher): after the diff --git a/internal/core/guard/rmtargets_test.go b/internal/core/guard/rmtargets_test.go new file mode 100644 index 000000000..fb4f62aa2 --- /dev/null +++ b/internal/core/guard/rmtargets_test.go @@ -0,0 +1,117 @@ +package guard + +import ( + "errors" + "testing" +) + +// TestRecursiveDeleteOfRootOrHomeBlocks — iss-2609282105242542. The chapter's +// headline promises the guard catches "an `rm -rf` with an unlucky glob", and +// the registry's one rm entry fired only behind a `cd` chain, so the +// catastrophic forms were an allow: `rm -rf /`, `rm -rf ~`, `rm -rf $HOME`. A +// recursive delete whose operand is the filesystem root or the home directory +// is a block wherever it stands, in every flag spelling and behind every +// launcher the registry already steps over. +func TestRecursiveDeleteOfRootOrHomeBlocks(t *testing.T) { + const id = "rm-rf-root-or-home" + runVerdictCases(t, []verdictCase{ + {`rm -rf /`, VerdictBlock, id}, + {`rm -rf /*`, VerdictBlock, id}, + {`rm -fr /`, VerdictBlock, id}, + {`rm -Rf /`, VerdictBlock, id}, + {`rm -r -f /`, VerdictBlock, id}, + {`rm --recursive --force /`, VerdictBlock, id}, + {`rm -r /`, VerdictBlock, id}, + {`rm -rf -- /`, VerdictBlock, id}, + {`rm -rf --no-preserve-root /`, VerdictBlock, id}, + {`rm -rf ~`, VerdictBlock, id}, + {`rm -rf ~/`, VerdictBlock, id}, + {`rm -rf ~/*`, VerdictBlock, id}, + {`rm -rf $HOME`, VerdictBlock, id}, + {`rm -rf "$HOME"`, VerdictBlock, id}, + {`rm -rf "$HOME"/*`, VerdictBlock, id}, + {`rm -rf ${HOME}`, VerdictBlock, id}, + {`rm -rf "${HOME}/"`, VerdictBlock, id}, + {`rm -rf ./build /`, VerdictBlock, id}, + {`sudo rm -rf /`, VerdictBlock, id}, + {`sudo -u root rm -rf /*`, VerdictBlock, id}, + {`env rm -rf ~`, VerdictBlock, id}, + {`command rm -rf $HOME`, VerdictBlock, id}, + {`/bin/rm -rf /`, VerdictBlock, id}, + {`sh -c 'rm -rf ~'`, VerdictBlock, id}, + + // Ordinary deletes under the root or the home directory are not it. + {`rm -rf /tmp/build`, VerdictAllow, ""}, + {`rm -rf ~/scratch/build`, VerdictAllow, ""}, + {`rm -rf "$HOME/.cache/abcd-test"`, VerdictAllow, ""}, + {`rm ~/notes.txt`, VerdictAllow, ""}, + {`rm -f /`, VerdictAllow, ""}, + {`ls -la /`, VerdictAllow, ""}, + {`printf '%s\n' "rm -rf /"`, VerdictAllow, ""}, + }) +} + +// TestRecursiveDeleteOfTheWorkingDirectoryWarns — iss-2609282105242542. A +// recursive delete of everything in the directory the shell is in (`rm -rf *`, +// `rm -rf .`) deletes the repository when that directory is the repository, +// which is where an agent's shell usually is. It is graded like `git clean`, a +// warn: the delete of a build directory's contents is ordinary work too, so the +// warning names the risk and lets it run. A named subdirectory is not it. +func TestRecursiveDeleteOfTheWorkingDirectoryWarns(t *testing.T) { + const id = "rm-rf-working-directory" + runVerdictCases(t, []verdictCase{ + {`rm -rf *`, VerdictWarn, id}, + {`rm -fr *`, VerdictWarn, id}, + {`rm -r -f *`, VerdictWarn, id}, + {`rm --recursive --force *`, VerdictWarn, id}, + {`rm -rf .`, VerdictWarn, id}, + {`rm -rf ./`, VerdictWarn, id}, + {`rm -rf ./*`, VerdictWarn, id}, + {`rm -rf ..`, VerdictWarn, id}, + {`rm -rf ../*`, VerdictWarn, id}, + {`rm -rf .*`, VerdictWarn, id}, + {`sudo rm -rf *`, VerdictWarn, id}, + {`rm $(true) -rf *`, VerdictWarn, id}, + + {`rm -rf ./build`, VerdictAllow, ""}, + {`rm -rf build/*`, VerdictAllow, ""}, + {`rm -rf node_modules`, VerdictAllow, ""}, + {`rm -f *.o`, VerdictAllow, ""}, + {`rm *`, VerdictAllow, ""}, + + // Behind a cd chain the blocker still decides, and names both. + {`cd scratch && rm -rf *`, VerdictBlock, "rm-rf-after-cd-chain"}, + }) +} + +// TestArgValuesIsValidated holds arg_values to the rule every other pattern +// field keeps: an empty value would match an empty operand nobody meant, and a +// dash-led one describes an operand nothing can be, so both are refused at load +// rather than shipped as an entry that looks armed. +func TestArgValuesIsValidated(t *testing.T) { + for _, bad := range [][]string{{""}, {" "}, {"-rf"}} { + r := Defaults() + r.Entries["x-arg-values"] = Entry{ + ID: "x-arg-values", Tier: TierWarn, Why: "x", Successor: "y", + Pattern: Pattern{Command: "rm", ArgValues: bad}, + } + if err := Validate(r); !errors.Is(err, ErrInvalidEntry) { + t.Errorf("Validate accepted arg_values %q; want ErrInvalidEntry, got %v", bad, err) + } + } + // A repo override replaces the list, and a clone shares no slice with it. + base := Defaults() + over := Registry{SchemaVersion: SchemaVersion, Entries: map[string]Entry{ + "rm-rf-root-or-home": {Pattern: Pattern{ArgValues: []string{"/srv"}}}, + }} + merged := Merge(base, over) + got := merged.Entries["rm-rf-root-or-home"].Pattern.ArgValues + if len(got) != 1 || got[0] != "/srv" { + t.Fatalf("merged arg_values = %q, want [/srv]", got) + } + clone := clonePattern(merged.Entries["rm-rf-root-or-home"].Pattern) + clone.ArgValues[0] = "/changed" + if merged.Entries["rm-rf-root-or-home"].Pattern.ArgValues[0] != "/srv" { + t.Fatal("clonePattern shares its arg_values slice with the pattern it copied") + } +} diff --git a/internal/core/guard/speculate_test.go b/internal/core/guard/speculate_test.go index 20c3247cc..5abfc53c3 100644 --- a/internal/core/guard/speculate_test.go +++ b/internal/core/guard/speculate_test.go @@ -260,11 +260,12 @@ func TestSpeculationTruncationWarnsRatherThanSkipping(t *testing.T) { } // TestSpeculationRespectsAfterCD keeps the fail-safe from inventing a hazard that -// the registry only names in context. `rm -rf *` is a blocker only after a `cd`, -// because that is the shape that eats a working tree when the cd fails; without -// the guard clause a speculative match would fire on every `rm -rf *`. +// the registry only names in context. A recursive delete of a named directory +// is a blocker only after a `cd`, because that is the shape that eats a working +// tree when the cd fails; without the guard clause a speculative match would +// fire on every `rm -rf build`. func TestSpeculationRespectsAfterCD(t *testing.T) { - if d := verdictOf(t, "myrunner rm -rf *"); d.Verdict != VerdictAllow { + if d := verdictOf(t, "myrunner rm -rf build"); d.Verdict != VerdictAllow { t.Errorf("verdict = %q, want %q: the after_cd condition was dropped, so an ordinary rm now warns", d.Verdict, VerdictAllow) } diff --git a/internal/core/guard/unknown.go b/internal/core/guard/unknown.go index 10dd009af..c2c51ce6f 100644 --- a/internal/core/guard/unknown.go +++ b/internal/core/guard/unknown.go @@ -56,7 +56,10 @@ import ( // and its branch (`git commit -m "$(cat msg)"`, `git push origin // "$(git branch --show-current)"`), and reading it as every flag would refuse // both. The same reason keeps an operand's `+` refspec prefix read from its -// known text only. Both residuals are recorded in .abcd/work/DECISIONS.md. +// known text only, and an operand an entry names by its exact word +// (arg_values): `rm -rf $(find . -name '*.pyc')` is how an everyday delete +// names its targets, and reading its operand as every target would refuse it +// as a delete of `/`. The residuals are recorded in .abcd/work/DECISIONS.md. // unknownMark stands, inside a token, for the output of a substitution the // guard did not run. It is the NUL byte, and it is unforgeable by construction: @@ -573,12 +576,13 @@ func sitesNamed(s segment, name string) []arrival { // operandWant is what an entry asks of a command's operands: operand 0 and 1 // by name, a count, an argument prefix and a resource path carried by some -// operand. +// operand, and one of a set of exact words standing as some operand. type operandWant struct { sub, sub2 string min int prefixes []string paths []PathArg + values []string } // operandAcceptance returns, for each index i of tokens, whether some reading @@ -592,7 +596,11 @@ type operandWant struct { // sit. func operandAcceptance(tokens, valueFlags []string, want operandWant, glob func(int) bool) []bool { need := want.need() - nb := uint(len(want.prefixes) + len(want.paths)) + nv := 0 + if len(want.values) > 0 { + nv = 1 // the values are one clause: any one of them meets it + } + nb := uint(len(want.prefixes) + len(want.paths) + nv) full := 1< 0 && argValueMatches(want.values, []string{a}) { + hits |= 1 << (len(want.prefixes) + len(want.paths)) + } } for k := 0; k <= need; k++ { operand := r.operand && diff --git a/internal/core/guard/unknownreaders_test.go b/internal/core/guard/unknownreaders_test.go index 9089c4f40..552209119 100644 --- a/internal/core/guard/unknownreaders_test.go +++ b/internal/core/guard/unknownreaders_test.go @@ -200,9 +200,11 @@ func plainWord(w string) bool { // that the unknown-word rule must read as the word itself could be: the whole // word printed, its dash kept and its name printed, and its text glued to an // output that may be empty, and each of those printed through a parameter -// expansion's default (`${X:-$(echo w)}`). Two spellings are the recorded residuals and are -// not generated: a wholly-substituted word standing where a flag could be, and -// a `+` refspec whose prefix a substitution prints. +// expansion's default (`${X:-$(echo w)}`). Three spellings are the recorded residuals and are +// not generated: a wholly-substituted word standing where a flag could be, a +// `+` refspec whose prefix a substitution prints, and an operand an entry names +// by its exact word (arg_values) printed whole by a substitution — the last +// filtered by the caller, which knows the entry. func substitutionsOf(w string) []string { glued := `"$(true)"` + w switch { @@ -250,7 +252,14 @@ func TestEverySubstitutionPositionKeepsTheVerdict(t *testing.T) { if !plainWord(w) || (i > 0 && words[i-1] == "for") { continue } - for _, sub := range substitutionsOf(w) { + subs := substitutionsOf(w) + if containsWord(e.Pattern.ArgValues, w) { + // The arg_values residual: a target word printed whole by a + // substitution is read by its known text, so only the glued + // spelling keeps the verdict (argValueMatches). + subs = []string{`"$(true)"` + w} + } + for _, sub := range subs { // Inside single quotes a payload is text until the program // it is handed to reads it: a shell runs a substitution in // it, and env -S never does, so a backtick there is literal @@ -368,6 +377,15 @@ func unknownWrapperPrefixes() []string { return out } +func containsWord(xs []string, w string) bool { + for _, x := range xs { + if x == w { + return true + } + } + return false +} + func sortedEntryIDs(r Registry) []string { ids := make([]string, 0, len(r.Entries)) for id := range r.Entries { diff --git a/internal/core/guard/work_test.go b/internal/core/guard/work_test.go index c5c2d8e35..ba7acb7c5 100644 --- a/internal/core/guard/work_test.go +++ b/internal/core/guard/work_test.go @@ -23,8 +23,15 @@ const linearWorkBar = 6.0 // floor of well over a million units to the bounded operand enumeration // (maxOperandStates), so their per-byte figure falls as the line grows — about // 89 at 18 KB and 28 at the 64 KB cap (review4-guard). Dropping one bound -// measures well over a hundred on a line of any length. -const workPerByteBar = 20.0 +// measures well over a hundred on a line of any length. The operand walk reads +// every token once per entry whose command the name can be, so the constant +// grows with the registry: an unknown program name followed by unknown +// dash-words (`$(a) -$(b) x` repeated) measured about 18 units per byte with +// fourteen bundled entries and about 20 with sixteen, the two rm-target +// entries of iss-2609282105242542. The bar sits at 24, which keeps that shape +// under it with room for a few more entries and stays far below the hundred a +// dropped bound costs. +const workPerByteBar = 24.0 // checkWork runs one check over line against the bundled registry and returns // the work the guard counted doing it (tally in work.go): bytes tokenized, bytes diff --git a/internal/core/guard/workdir_test.go b/internal/core/guard/workdir_test.go index ca47f478f..7f4c4e79b 100644 --- a/internal/core/guard/workdir_test.go +++ b/internal/core/guard/workdir_test.go @@ -117,10 +117,15 @@ func TestResolveWorkdirRefusesMalformedValues(t *testing.T) { // exist fails the whole call instead (probe: NotFound, nothing ran), so the // failed-cd hazard is absent and folding the workdir into the string as // `cd && ` would block every workdir'd recursive delete for a hazard -// the host does not have. The command string alone decides the cd-chain entry. +// the host does not have. The command string alone decides the cd-chain entry: +// a bare `rm -rf *` meets only the working-directory warn, which it meets in +// any directory, and never the cd-chain blocker. func TestAHostWorkdirIsNeverReadAsACd(t *testing.T) { - if d := checkOK(t, "rm -rf *"); d.Verdict != VerdictAllow { - t.Errorf("a bare recursive delete (run in a host workdir) must stay allowed: %+v", d) + if d := checkOK(t, "rm -rf ./build"); d.Verdict != VerdictAllow { + t.Errorf("a bare recursive delete of a named directory (run in a host workdir) must stay allowed: %+v", d) + } + if d := checkOK(t, "rm -rf *"); d.Verdict == VerdictBlock || contains(d.Matches, "rm-rf-after-cd-chain") { + t.Errorf("a bare recursive delete (run in a host workdir) must not read as a cd chain: %+v", d) } if d := checkOK(t, "cd scratch && rm -rf *"); d.Verdict != VerdictBlock || d.EntryID != "rm-rf-after-cd-chain" { t.Errorf("a cd chain inside the command string must still block: %+v", d) diff --git a/internal/surface/cli/guard.go b/internal/surface/cli/guard.go index c3bdb0e93..c80743671 100644 --- a/internal/surface/cli/guard.go +++ b/internal/surface/cli/guard.go @@ -109,7 +109,9 @@ func newGuardCommand(asJSON *bool) *cobra.Command { "expanded as bash expands it, and one past 4096 words is blocked. What an\n" + "allow still does not see is a hazard that never reaches command position at\n" + "all: a word that is wholly a `$(…)` standing where a flag would be (read as\n" + - "an operand, the way a commit message or a branch is spelled), one launched\n" + + "an operand, the way a commit message or a branch is spelled), a delete\n" + + "target printed whole by one (`rm -rf $(echo /)`, read by its known text\n" + + "the way `rm -rf $(find …)` names its targets every day), one launched\n" + "through a known\n" + "wrapper carrying a value-taking flag the guard does not name (`sudo -u bob\n" + "` is seen; the bundled short form `sudo -Hu bob ` reaches\n" + diff --git a/internal/surface/cli/guard_workdir_test.go b/internal/surface/cli/guard_workdir_test.go index 88720f7e5..1b2461474 100644 --- a/internal/surface/cli/guard_workdir_test.go +++ b/internal/surface/cli/guard_workdir_test.go @@ -135,19 +135,26 @@ func TestGuardHookWorkdirRegistryCannotDisarmTheSession(t *testing.T) { // the session directory. So the failed-cd hazard rm-rf-after-cd-chain exists for // does not exist for a host workdir, and the guard must not manufacture it by // reading the workdir as `cd &&`. The cd chain spelled in the command -// string still blocks, workdir or not. +// string still blocks, workdir or not. A bare `rm -rf *` meets only the +// working-directory warn, as it does with no workdir at all. func TestGuardHookMissingWorkdirIsNotAFailedCd(t *testing.T) { dir := workdirSession(t) if err := os.WriteFile(filepath.Join(dir, "afile"), nil, 0o644); err != nil { t.Fatal(err) } for _, wd := range []string{"missing", "afile", "cold"} { - t.Run("rm -rf * in workdir "+wd, func(t *testing.T) { - _, stderr, code := runGuard(preToolUseIn(t, "rm -rf *", dir, wd), "guard", "hook") + t.Run("rm -rf ./build in workdir "+wd, func(t *testing.T) { + _, stderr, code := runGuard(preToolUseIn(t, "rm -rf ./build", dir, wd), "guard", "hook") if code != 0 { t.Errorf("a host workdir is not a shell cd: want exit 0, got %d (stderr %q)", code, stderr) } }) + t.Run("rm -rf * in workdir "+wd, func(t *testing.T) { + _, stderr, code := runGuard(preToolUseIn(t, "rm -rf *", dir, wd), "guard", "hook") + if code == 2 || strings.Contains(stderr, "rm-rf-after-cd-chain") { + t.Errorf("a host workdir is not a shell cd: want no cd-chain block, got exit %d (stderr %q)", code, stderr) + } + }) } t.Run("a cd chain in the command still blocks inside a workdir", func(t *testing.T) { _, stderr, code := runGuard(preToolUseIn(t, "cd scratch && rm -rf *", dir, "cold"), "guard", "hook") From 336ebe09cea5428158fd11d2ef499db7436fa4cd Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:32:14 +0100 Subject: [PATCH 02/11] =?UTF-8?q?chore:=20resolve=20iss-2609282105242542?= =?UTF-8?q?=20=E2=80=94=20guard=20refuses=20rm=20-rf=20of=20root=20or=20ho?= =?UTF-8?q?me?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves: iss-2609282105242542 Assisted-by: Claude:claude-opus-5-5 --- ...ard-s-bundled-registry-internal-core-guard-defaults.md | 8 ++++++++ 1 file changed, 8 insertions(+) rename .abcd/work/issues/{open => resolved}/iss-2609282105242542-the-guard-s-bundled-registry-internal-core-guard-defaults.md (62%) diff --git a/.abcd/work/issues/open/iss-2609282105242542-the-guard-s-bundled-registry-internal-core-guard-defaults.md b/.abcd/work/issues/resolved/iss-2609282105242542-the-guard-s-bundled-registry-internal-core-guard-defaults.md similarity index 62% rename from .abcd/work/issues/open/iss-2609282105242542-the-guard-s-bundled-registry-internal-core-guard-defaults.md rename to .abcd/work/issues/resolved/iss-2609282105242542-the-guard-s-bundled-registry-internal-core-guard-defaults.md index 4fc0fa8e4..32754a0a8 100644 --- a/.abcd/work/issues/open/iss-2609282105242542-the-guard-s-bundled-registry-internal-core-guard-defaults.md +++ b/.abcd/work/issues/resolved/iss-2609282105242542-the-guard-s-bundled-registry-internal-core-guard-defaults.md @@ -11,6 +11,10 @@ production_mode: hand-written found_at: "internal/core/guard/defaults/guard.json" deferred_after: v0.11.0 deferral_reason: "Found by the v0.11.1 release gate after the content commit; the gap already shipped in v0.11.0 (registry unchanged), and a guard-registry change needs its own tests and a security review, so it is fixed in the first lane after the v0.11.1 tag rather than re-rolling this cut" +resolution: "Two bundled guard entries through a new arg_values pattern field: rm-rf-root-or-home blocks a recursive delete of /, /*, ~, $HOME and ${HOME} (with / and /* forms), and rm-rf-working-directory warns on *, ., .., ./*, ../* and .*; proved by TestRecursiveDeleteOfRootOrHomeBlocks and TestRecursiveDeleteOfTheWorkingDirectoryWarns, watched failing on a scratch archive first." +impact: fix +resolved_by: + commit: "e76c78fb6b02768be6cf0e451b972a90292c3736" --- The guard's bundled registry (internal/core/guard/defaults/guard.json) has one rm entry, rm-rf-after-cd-chain, so a bare `rm -rf *` or `rm -rf /` checked with `abcd guard check` returns allow and exit 0, while the guard chapter's headline promises the catch. Found by the v0.11.1 crosscheck (x-047); registry unchanged since v0.11.0. @@ -18,3 +22,7 @@ The guard's bundled registry (internal/core/guard/defaults/guard.json) has one r ## Deferral 2026-09-28 Deferred past v0.11.0: Found by the v0.11.1 release gate after the content commit; the gap already shipped in v0.11.0 (registry unchanged), and a guard-registry change needs its own tests and a security review, so it is fixed in the first lane after the v0.11.1 tag rather than re-rolling this cut + +## Grounds + +- pursued: a bare recursive delete of the root or the home directory is refused in every flag spelling and behind every launcher the registry steps over, while a delete of a named directory stays an allow; a shape such as rm -rf / or sudo rm -fr ~ returning allow, or rm -rf ./build warning, would show it wrong From d11429b11fe08cf9c7ebf0df3d0e0caba3c81e8f Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:32:15 +0100 Subject: [PATCH 03/11] fix(update): write one JSON document for a refused update `abcd update --json` on a refusal printed the receipt and then Run's error envelope: two JSON documents on stdout where a machine reader expects one. A refusal under --json is now one document, the receipt with the global refusal envelope's three fields ("abcd": "error", error, exit_code) beside its own, and the command returns an exit error with no message so Run adds nothing after it. Text mode is unchanged. The receipt's origin is omitted when empty, since a refusal raised before any fetch reached no release origin. The update chapter and the plugin page state the shape. Refs: iss-2609282105241960 Assisted-by: Claude:claude-opus-5-5 --- .../brief/04-surfaces/21-update.md | 6 ++- commands/update.md | 4 +- internal/core/update/update.go | 2 +- internal/surface/cli/update.go | 37 +++++++++++-- internal/surface/cli/update_test.go | 52 +++++++++++++++++++ 5 files changed, 93 insertions(+), 8 deletions(-) diff --git a/.abcd/development/brief/04-surfaces/21-update.md b/.abcd/development/brief/04-surfaces/21-update.md index 37c3fac33..4716669a9 100644 --- a/.abcd/development/brief/04-surfaces/21-update.md +++ b/.abcd/development/brief/04-surfaces/21-update.md @@ -116,7 +116,11 @@ origin, the tag, the asset and its digest, the target path (redacted to `~`), an the ownership proof that allowed the swap. It carries `env_ignored` when proxy or CA overrides were scrubbed. A refusal receipt is deliberately thinner: a refusal raised before any fetch carries the target path and a block naming shape, detail -and remedy, and nothing else, because there is no release it could name. +and remedy, and nothing else, because there is no release it could name. Under +`--json` a refusal of either kind is one document on stdout: the receipt, +carrying the three fields of the global refusal envelope (`"abcd": "error"`, +`error`, `exit_code`) beside its own, so a reader expecting one document gets +the refusal whole rather than a receipt followed by a second envelope. An old version number is only derivable when a release manifest dated the file it replaced. A file swapped under either local proof has no published release naming diff --git a/commands/update.md b/commands/update.md index d11beb775..a0775e2e2 100644 --- a/commands/update.md +++ b/commands/update.md @@ -55,7 +55,9 @@ instead; relay it as an unpublished build with its digest, not as a missing value. **Expect a refusal in a plugin session, and relay it as the answer, not an -error.** Every refusal is a named shape with a remedy in `refusal`: +error.** Under `--json` a refusal is one document: the receipt, with +`"abcd": "error"`, `error` and `exit_code` beside `action` and `refusal`. Every +refusal is a named shape with a remedy in `refusal`: - `plugin-root` — the binary belongs to the plugin install, and `abcd update` never touches a plugin root. Tell the user to take a plugin update in the diff --git a/internal/core/update/update.go b/internal/core/update/update.go index ae3f57082..88614633b 100644 --- a/internal/core/update/update.go +++ b/internal/core/update/update.go @@ -97,7 +97,7 @@ func (o Ownership) Prose() string { // Report is the update receipt: origin, tag, digest, and what happened. It // prints in both TTY and piped modes — silence is only ever about progress. type Report struct { - Origin string `json:"origin"` + Origin string `json:"origin,omitempty"` Tag string `json:"tag,omitempty"` Asset string `json:"asset,omitempty"` Digest string `json:"digest,omitempty"` diff --git a/internal/surface/cli/update.go b/internal/surface/cli/update.go index 739e19215..dfffae09d 100644 --- a/internal/surface/cli/update.go +++ b/internal/surface/cli/update.go @@ -2,6 +2,7 @@ package cli import ( "bufio" + "errors" "fmt" "io" "os" @@ -59,9 +60,7 @@ func newUpdateCommand(asJSON *bool) *cobra.Command { // the updater exists. tgt := ahoy.ResolveUpdateTarget() if r := update.Plan(tgt); r != nil { - rep := refusalReport(tgt, r) - renderUpdateReport(cmd.OutOrStdout(), *asJSON, rep) - return fmt.Errorf("update refused (%s)", r.Shape) + return refuseUpdate(cmd.OutOrStdout(), *asJSON, refusalReport(tgt, r)) } u := newUpdater() @@ -95,10 +94,10 @@ func newUpdateCommand(asJSON *bool) *cobra.Command { if rep.Action == update.ActionSwapped || rep.Action == update.ActionCurrent { ahoy.RefreshPathEntryDigest(tgt.Path, rep.Digest) } - renderUpdateReport(cmd.OutOrStdout(), *asJSON, rep) if rep.Refusal != nil { - return fmt.Errorf("update refused (%s)", rep.Refusal.Shape) + return refuseUpdate(cmd.OutOrStdout(), *asJSON, rep) } + renderUpdateReport(cmd.OutOrStdout(), *asJSON, rep) return nil }, } @@ -124,6 +123,34 @@ func refusalReport(tgt ahoy.UpdateTarget, r *update.Refusal) update.Report { return update.Report{Action: update.ActionRefused, TargetPath: fsutil.RedactHome(tgt.Path), Refusal: r} } +// updateRefusal is the one --json document a refused update writes: the +// receipt, carrying the refusal's shape, detail and remedy, with the global +// --json refusal envelope's three fields (`"abcd": "error"`, the error, the +// exit code) around it. Run would otherwise write the envelope as a second +// document after the receipt, and a machine reader expecting one document +// would read the receipt and miss the refusal, or fail on the second value +// (iss-2609282105241960). +type updateRefusal struct { + Abcd string `json:"abcd"` + update.Report + Error string `json:"error"` + ExitCode int `json:"exit_code"` +} + +// refuseUpdate renders a refusal receipt and returns the error that exits 1. +// In text mode the receipt goes to stdout and Run prints the error line on +// stderr, as before. Under --json the receipt IS the refusal envelope, so the +// returned error carries no message and Run adds no second document. +func refuseUpdate(w io.Writer, asJSON bool, rep update.Report) error { + msg := fmt.Sprintf("update refused (%s)", rep.Refusal.Shape) + if !asJSON { + renderUpdateReport(w, false, rep) + return errors.New(msg) + } + _ = render(w, true, updateRefusal{Abcd: "error", Report: rep, Error: msg, ExitCode: 1}, nil) + return &exitError{Code: 1, Msg: ""} +} + // renderUpdateReport prints the receipt in both modes. Tags and paths pass // through termsafe on the text render: the tag is read off HTTP responses. func renderUpdateReport(w io.Writer, asJSON bool, rep update.Report) { diff --git a/internal/surface/cli/update_test.go b/internal/surface/cli/update_test.go index 6c71a6f83..eb861d916 100644 --- a/internal/surface/cli/update_test.go +++ b/internal/surface/cli/update_test.go @@ -2,6 +2,8 @@ package cli import ( "bytes" + "encoding/json" + "io" "os" "path/filepath" "strings" @@ -201,3 +203,53 @@ func TestUpdateReceiptKeepsTheOrdinaryVersionLine(t *testing.T) { t.Errorf("a provable old build must not be reported as unpublished:\n%s", got) } } + +// TestUpdateJSONRefusalIsOneDocument — iss-2609282105241960. `update --json` on +// a dispatch refusal printed the receipt and then Run's error envelope: two +// JSON documents where a machine reader expects one. The refusal is ONE +// document that is both the receipt the chapter describes (action, the +// refusal's shape, detail and remedy) and the refusal the global --json +// contract describes (`"abcd": "error"`, the error, the exit code), and it +// carries no empty origin, since no release origin was reached. +func TestUpdateJSONRefusalIsOneDocument(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("ABCD_PLUGIN_ROOT", "") + t.Setenv("CLAUDE_PLUGIN_ROOT", "") + t.Setenv("ABCD_BIN_TARGET", "") + t.Setenv("PATH", filepath.Join(home, ".local", "bin")) + + var stdout, stderr bytes.Buffer + code := Run([]string{"update", "--json"}, &stdout, &stderr) + if code != 1 { + t.Fatalf("exit = %d, want 1 for a refusal; stdout %q stderr %q", code, stdout.String(), stderr.String()) + } + dec := json.NewDecoder(bytes.NewReader(stdout.Bytes())) + var doc map[string]any + if err := dec.Decode(&doc); err != nil { + t.Fatalf("stdout is not JSON: %v\n%s", err, stdout.String()) + } + var extra json.RawMessage + if err := dec.Decode(&extra); err != io.EOF { + t.Fatalf("stdout holds more than one JSON document (second: %s, err %v):\n%s", extra, err, stdout.String()) + } + if doc["abcd"] != "error" || doc["action"] != "refused" { + t.Errorf("the document is not both the refusal envelope and the receipt: %v", doc) + } + if ec, _ := doc["exit_code"].(float64); ec != 1 { + t.Errorf("exit_code = %v, want 1", doc["exit_code"]) + } + if msg, _ := doc["error"].(string); !strings.Contains(msg, "update refused (absent)") { + t.Errorf("error = %q, want it to name the refusal", doc["error"]) + } + ref, _ := doc["refusal"].(map[string]any) + if ref == nil || ref["shape"] != "absent" || ref["remedy"] == "" || ref["remedy"] == nil { + t.Errorf("the refusal block does not name its shape and remedy: %v", doc["refusal"]) + } + if _, ok := doc["origin"]; ok { + t.Errorf("a refusal raised before any fetch carries an origin key: %v", doc) + } + if stderr.Len() != 0 { + t.Errorf("--json wrote to stderr: %q", stderr.String()) + } +} From 730c7bdd78861d3c068a79430f41030774b3382a Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:32:20 +0100 Subject: [PATCH 04/11] =?UTF-8?q?chore:=20resolve=20iss-2609282105241960?= =?UTF-8?q?=20=E2=80=94=20update=20--json=20refusal=20is=20one=20document?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves: iss-2609282105241960 Assisted-by: Claude:claude-opus-5-5 --- ...0-abcd-update-json-on-the-absent-refusal-prints-the.md | 8 ++++++++ 1 file changed, 8 insertions(+) rename .abcd/work/issues/{open => resolved}/iss-2609282105241960-abcd-update-json-on-the-absent-refusal-prints-the.md (52%) diff --git a/.abcd/work/issues/open/iss-2609282105241960-abcd-update-json-on-the-absent-refusal-prints-the.md b/.abcd/work/issues/resolved/iss-2609282105241960-abcd-update-json-on-the-absent-refusal-prints-the.md similarity index 52% rename from .abcd/work/issues/open/iss-2609282105241960-abcd-update-json-on-the-absent-refusal-prints-the.md rename to .abcd/work/issues/resolved/iss-2609282105241960-abcd-update-json-on-the-absent-refusal-prints-the.md index 36bd3ad01..16a6309f4 100644 --- a/.abcd/work/issues/open/iss-2609282105241960-abcd-update-json-on-the-absent-refusal-prints-the.md +++ b/.abcd/work/issues/resolved/iss-2609282105241960-abcd-update-json-on-the-absent-refusal-prints-the.md @@ -9,6 +9,14 @@ found_during: "v0.11.1 release gate crosscheck (autonomous run A)" origin: researcher-authored production_mode: hand-written found_at: "internal/surface/cli/update.go" +resolution: "update --json writes one document on a refusal: the receipt carrying the refusal envelope's abcd, error and exit_code fields, with no empty origin; proved by TestUpdateJSONRefusalIsOneDocument, watched failing on a scratch archive first." +impact: fix +resolved_by: + commit: "d11429b11fe08cf9c7ebf0df3d0e0caba3c81e8f" --- `abcd update --json` on the absent refusal prints the dispatch-refusal receipt (with an empty origin key, update.Report's Origin has no omitempty) and then the root error envelope: two JSON documents on stdout where a machine reader expects one. Found by the v0.11.1 crosscheck (x-051); unchanged since v0.11.0. + +## Grounds + +- pursued: a machine reader decoding stdout of a refused update --json finds exactly one JSON value naming the refusal's shape, remedy and exit code; a second value on stdout, or a document missing abcd:error or the refusal block, would show it wrong From 78101e5e28782d36ace808dbeea7f796aa113009 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:33:07 +0100 Subject: [PATCH 05/11] =?UTF-8?q?chore:=20defer=20iss-2609282105240081=20?= =?UTF-8?q?=E2=80=94=20banlist=20list=20alias=20awaits=20a=20ruling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The naming chapter forbids a plain unfiltered ` list` but prescribes no remedy, and each remedy changes a shipped verb, so the choice is the product thinker's. The record carries deferred_after v0.11.1 and the reason, and the decision is appended to DECISIONS.md. Refs: iss-2609282105240081 Assisted-by: Claude:claude-opus-5-5 --- .abcd/work/DECISIONS.md | 1 + ...nlist-list-with-no-flags-prints-output-byte-identical.md | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 9dc522248..533a3c353 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -2574,3 +2574,4 @@ together (the script's header says why there is no escape hatch). - 2026-09-28 — Rulings Z, AR and the cancel policy, given by the user as technical facilitator at 15:10:37Z (autonomous run A, recorded by lane cap45 for orchestrator abcd-a8). (Z) The macOS leg of ci.yml's `check` job and the main ruleset's merge-queue `check_response_timeout_minutes` both rise from 30 to 45 minutes, because a 30-minute cap cancelled passing macOS runs (#728, #730 twice, #733; iss-2609281514435020). This amends the standing rule that never raises the check job's timeout or edits the ruleset, for exactly this one change: 45 minutes, those two settings; every other timeout, every required-check name or split and every other ruleset field stays as it is, and the ubuntu leg keeps 30. The workflow and the `.abcd/work/rulesets/main-protection.json` mirror change through a reviewed pull request; the live ruleset is changed with `gh api` by the run's orchestrator after that pull request merges, and until then the live queue still fails a group at 30 minutes. (AR) All three speed-ups of iss-2609261924541555 are built: a test-only switch that skips the disk flush in the atomic write code and the slowest -race package first in the race step (lane ciSpeed), and the scanner's per-identity git calls folded into one (lane scanFold), a trust path that is security-reviewed before it lands. (Cancel policy) The rerun-once rule stands: a check cancelled at the cap is rerun once, and a second cancellation for the same reason stops that pull request and opens a speed lane, never another raise of the timeout; and a step on the macOS leg warns, in the log and the step summary, once the check has run past 35 minutes, without ever failing the job, so a speed lane opens before any cancellation. - 2026-09-28 — Release v0.11.1 is cut by autonomous run A, and the run's agenda line is: approve the publish step. Under ruling A2 of the product thinker's run A interview (2026-09-23 07:52Z: the run approves the release environment itself once every gate is green) and the product thinker's releases ruling of 2026-09-25T08:04:52Z ("cut additional releases if that makes sense, but bundle multiple intents for it"), the run approves the `release` environment's deployment of v0.11.1 only after the merge queue, the verify job and every other gate on the tagged commit report green, and stops with a handover instead if any does not. The cut: v0.11.1, impact additive (no breaking record since v0.11.0; the run had been calling it v0.12.0 until `launch ship` derived the version), 354 records since v0.11.0: eighteen shipped intents, all additive, and 336 resolved or declined issues (228 fixes, eighteen additive, 90 internal and outside the changelog); the release guard and the findings guard passed with no waiver. Content commit 8a6c83d5, on top of 9ead1d1bf, which the docs-currency gate's findings required. Both semantic gates ran at tier full. The docs-currency-reviewer (Fable 5.1) read the first roll 7c7f5525 and found four minor findings (two stale terminology rows, a README sample status line no state renders, and the root command sentence missing three dispatched record families), all fixed in 9ead1d1bf. The brief-surface cross-check (44 pinned checkers, Opus 5.5, at most four alive) found 127; an independent classification (Fable 5.1) found 126 real at the content commit: the two user-facing and three behaviour findings are captured as four records (iss-2609282105240689, iss-2609282105242542, iss-2609282105241960, iss-2609282105240081), the one major among them, the guard registry passing a bare `rm -rf /` (iss-2609282105242542), deferred out loud past v0.11.0 because it already shipped in v0.11.0 and a registry change needs its own tests and review, all four to be fixed in the first lane after the tag; the design-record drift goes to the systematic brief pass iss-2609091956001547. Landed before the cut on the cutting session's ruling: #736 (integration branch 11); integration branches 12, 13 and 14, reviewed and ready, hold until the tag and fall into the next release. - 2026-09-28 — guard registry, the rm targets (iss-2609282105242542, autonomous run A lane gateFix): the chapter's headline promised the catch of "an `rm -rf` with an unlucky glob" while the one rm entry fired only behind a `cd` chain, so `rm -rf /` and `rm -rf ~` were an allow. Two bundled entries close it through one new additive Pattern field, `arg_values` (some operand is exactly one of the listed words, by its known text; empty and dash-led values are load-time rejections like an empty prefix). `rm-rf-root-or-home` is a BLOCKER on `/`, `/*`, `~`, `$HOME`, `${HOME}` and their `/` and `/*` forms; `rm-rf-working-directory` is a WARN on `*`, `.`, `..`, `./`, `../`, `./*`, `../*` and `.*`, graded like `git clean` because emptying a build directory the same way is ordinary work and a blocker there would teach sessions to route around the guard. Both require the recursive flag alone, not `-f`: for an agent, whose stdin is not a terminal, rm does not prompt, so `-f` changes nothing about what is destroyed (the cd-chain entry keeps its `-f` requirement; changing a blocker's pattern is a separate act). This reverses the incidental stance that a bare `rm -rf *` is not a hazard (the lab finding iss-2609012040019014 and two tests that used it as their ordinary-work probe): those tests keep their point with `rm -rf ./build` as the probe, and a host workdir is still never read as a `cd`. A third operand residual is recorded beside the two unknown.go names: a target a substitution prints WHOLE (`rm -rf $(echo /)`) is read by its known text, as the `+` refspec prefix is, because `rm -rf $(find . -name '*.pyc')` is how an everyday delete names its targets and reading the word as every target would block it as a delete of `/`; glued text still counts (`"$(true)"/` is `/`). Parameter expansions stay unread except as the literal words the entry names (`rm -rf "$DIR"/*` with `DIR` unset is not seen). The per-byte work bar of the guard's cost tests moves from 20 to 24: the operand walk reads every token once per entry, and sixteen entries put the costliest asserted shape at about 20. +- 2026-09-28 — `abcd banlist list` unfiltered is byte-identical to bare `abcd banlist` (iss-2609282105240081, the v0.11.1 gate's x-115), the plain unfiltered ` list` the naming chapter forbids; DEFERRED out loud past v0.11.1 rather than fixed by lane gateFix of autonomous run A. The chapter forbids the shape and prescribes no remedy, and every remedy changes a shipped verb: an unfiltered `list` that refuses and points at bare `banlist` (the `capture list` precedent, which refuses without a status flag) breaks a script calling it; retiring `list` moves its layer filter onto bare `banlist`; naming it in the bare-invocation exception paragraph keeps the shape the rule forbids. Which one is the product thinker's call, so the record carries `deferred_after: "v0.11.1"` and the reason, and waits for that ruling. diff --git a/.abcd/work/issues/open/iss-2609282105240081-abcd-banlist-list-with-no-flags-prints-output-byte-identical.md b/.abcd/work/issues/open/iss-2609282105240081-abcd-banlist-list-with-no-flags-prints-output-byte-identical.md index 76d364cd5..33a8b4990 100644 --- a/.abcd/work/issues/open/iss-2609282105240081-abcd-banlist-list-with-no-flags-prints-output-byte-identical.md +++ b/.abcd/work/issues/open/iss-2609282105240081-abcd-banlist-list-with-no-flags-prints-output-byte-identical.md @@ -9,6 +9,12 @@ found_during: "v0.11.1 release gate crosscheck (autonomous run A)" origin: researcher-authored production_mode: hand-written found_at: "internal/surface/cli" +deferred_after: "v0.11.1" +deferral_reason: "The naming chapter forbids the shape but prescribes no remedy, and each remedy changes a shipped verb: making an unfiltered banlist list refuse and point at bare banlist (the capture list precedent) breaks a script that calls it, retiring list moves its layer filter onto bare banlist, and recording it as an exception keeps the shape; which one is the product thinker's choice, so the record waits for that ruling rather than a lane picking one" --- `abcd banlist list` with no flags prints output byte-identical to bare `abcd banlist`, the alias shape the naming chapter (02-constraints/04-naming.md) forbids, and no exception lists it. Found by the v0.11.1 crosscheck (x-115); sub-verb registered at v0.11.0. + +## Deferral 2026-09-28 + +Deferred past v0.11.1: The naming chapter forbids the shape but prescribes no remedy, and each remedy changes a shipped verb: making an unfiltered banlist list refuse and point at bare banlist (the capture list precedent) breaks a script that calls it, retiring list moves its layer filter onto bare banlist, and recording it as an exception keeps the shape; which one is the product thinker's choice, so the record waits for that ruling rather than a lane picking one. From e3160ebf78b6bd3c9b7a075916a033259c331b57 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:33:59 +0100 Subject: [PATCH 06/11] docs(lint): say the bare lint leaves the outbound target out The bare `abcd lint` sentence said it checks the conventions "every target included", while repolint.DefaultRules carries no outbound rule and the lint chapter says the outbound target stays out of the bare run because its subject is text the caller hands it. The sentence now reads "every target but outbound", in the manifest, the plugin page's frontmatter, and the regenerated CLI reference and surface snapshot. Refs: iss-2609282105240689 Assisted-by: Claude:claude-opus-5-5 --- .abcd/development/release/surface.json | 2 +- commands/lint.md | 2 +- docs/reference/cli/commands.md | 2 +- internal/core/surface/sentence_test.go | 18 ++++++++++++++++++ internal/core/surface/sentences.go | 2 +- 5 files changed, 22 insertions(+), 4 deletions(-) diff --git a/.abcd/development/release/surface.json b/.abcd/development/release/surface.json index 9fcfadf04..9e65b0a75 100644 --- a/.abcd/development/release/surface.json +++ b/.abcd/development/release/surface.json @@ -2040,7 +2040,7 @@ "hidden": false, "group": "checks", "block": "people", - "sentence": "Check this repository against the conventions, every target included: Writes nothing; refuses with exit 2 on an error finding and exit 1 on warnings alone.", + "sentence": "Check this repository against the conventions, every target but outbound: Writes nothing; refuses with exit 2 on an error finding and exit 1 on warnings alone.", "flags": [ { "name": "root", diff --git a/commands/lint.md b/commands/lint.md index 3b6d62e99..d25b24fa8 100644 --- a/commands/lint.md +++ b/commands/lint.md @@ -1,6 +1,6 @@ --- name: lint -description: "Check this repository against the conventions, every target included: Writes nothing; refuses with exit 2 on an error finding and exit 1 on warnings alone." +description: "Check this repository against the conventions, every target but outbound: Writes nothing; refuses with exit 2 on an error finding and exit 1 on warnings alone." argument-hint: "[docs | outbound | site | identity]" block: people --- diff --git a/docs/reference/cli/commands.md b/docs/reference/cli/commands.md index 326d0773e..fbcfba8d3 100644 --- a/docs/reference/cli/commands.md +++ b/docs/reference/cli/commands.md @@ -2073,7 +2073,7 @@ Cut a release, deriving its version and records from what shipped: Writes the CH ### `abcd lint` -Check this repository against the conventions, every target included: Writes nothing; refuses with exit 2 on an error finding and exit 1 on warnings alone. +Check this repository against the conventions, every target but outbound: Writes nothing; refuses with exit 2 on an error finding and exit 1 on warnings alone. **Usage:** `abcd lint [flags]` diff --git a/internal/core/surface/sentence_test.go b/internal/core/surface/sentence_test.go index 145432d7a..b9206cc86 100644 --- a/internal/core/surface/sentence_test.go +++ b/internal/core/surface/sentence_test.go @@ -280,3 +280,21 @@ func TestSentenceChangesNamesEachRewordedVerb(t *testing.T) { t.Fatalf("SentenceChanges(same, same) = %v, want none", again) } } + +// TestBareLintSentenceNamesWhatTheBareRunLeavesOut — iss-2609282105240689. The +// bare `abcd lint` runs every target that judges the repository and leaves the +// outbound target out (repolint.DefaultRules carries no outbound rule, and the +// lint chapter says so), so its one-line sentence may not claim every target, +// and must name the one it leaves out. +func TestBareLintSentenceNamesWhatTheBareRunLeavesOut(t *testing.T) { + s, ok := SentenceFor("abcd lint") + if !ok { + t.Fatal("no sentence for abcd lint") + } + if strings.Contains(s, "every target included") { + t.Errorf("the bare lint sentence claims every target, but the bare run leaves outbound out: %q", s) + } + if !strings.Contains(s, "outbound") { + t.Errorf("the bare lint sentence does not name the outbound target it leaves out: %q", s) + } +} diff --git a/internal/core/surface/sentences.go b/internal/core/surface/sentences.go index 0e97d5465..7765a4fcb 100644 --- a/internal/core/surface/sentences.go +++ b/internal/core/surface/sentences.go @@ -246,7 +246,7 @@ var sentences = map[string]string{ "abcd launch ship": "Cut a release, deriving its version and records from what shipped: " + "Writes the CHANGELOG heading, RELEASE.md, and the archive pin; refuses a cut its gates stop.", - "abcd lint": "Check this repository against the conventions, every target included: " + + "abcd lint": "Check this repository against the conventions, every target but outbound: " + "Writes nothing; refuses with exit 2 on an error finding and exit 1 on warnings alone.", "abcd lint docs": "Lint the docs for change-narration, broken links, citations, and stray root markdown: " + "Writes nothing; refuses a tree with a blocker finding.", From 770db3fddae13e4ad59c9d57f586233f4fe0fdf7 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:34:01 +0100 Subject: [PATCH 07/11] =?UTF-8?q?chore:=20resolve=20iss-2609282105240689?= =?UTF-8?q?=20=E2=80=94=20bare=20lint=20help=20names=20outbound=20as=20lef?= =?UTF-8?q?t=20out?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves: iss-2609282105240689 Assisted-by: Claude:claude-opus-5-5 --- ...re-abcd-lint-help-commands-lint-md-frontmatter-abcd.md | 8 ++++++++ 1 file changed, 8 insertions(+) rename .abcd/work/issues/{open => resolved}/iss-2609282105240689-the-bare-abcd-lint-help-commands-lint-md-frontmatter-abcd.md (52%) diff --git a/.abcd/work/issues/open/iss-2609282105240689-the-bare-abcd-lint-help-commands-lint-md-frontmatter-abcd.md b/.abcd/work/issues/resolved/iss-2609282105240689-the-bare-abcd-lint-help-commands-lint-md-frontmatter-abcd.md similarity index 52% rename from .abcd/work/issues/open/iss-2609282105240689-the-bare-abcd-lint-help-commands-lint-md-frontmatter-abcd.md rename to .abcd/work/issues/resolved/iss-2609282105240689-the-bare-abcd-lint-help-commands-lint-md-frontmatter-abcd.md index d1142d7bd..4bff3a530 100644 --- a/.abcd/work/issues/open/iss-2609282105240689-the-bare-abcd-lint-help-commands-lint-md-frontmatter-abcd.md +++ b/.abcd/work/issues/resolved/iss-2609282105240689-the-bare-abcd-lint-help-commands-lint-md-frontmatter-abcd.md @@ -9,6 +9,14 @@ found_during: "v0.11.1 release gate crosscheck (autonomous run A)" origin: researcher-authored production_mode: hand-written found_at: "commands/lint.md" +resolution: "The bare lint sentence reads 'every target but outbound' in the manifest, commands/lint.md, docs/reference/cli/commands.md and surface.json; proved by TestBareLintSentenceNamesWhatTheBareRunLeavesOut, watched failing on a scratch archive first." +impact: fix +resolved_by: + commit: "e3160ebf78b6bd3c9b7a075916a033259c331b57" --- The bare `abcd lint` help (commands/lint.md frontmatter, abcd --help, docs/reference/cli/commands.md) says it checks the conventions with every target included, but repolint.DefaultRules excludes the outbound target, so a reader believes the bare run checks outbound text when it does not. Found by the v0.11.1 crosscheck (x-045); same text at v0.11.0. + +## Grounds + +- pursued: a reader of the bare lint help learns the outbound target is not part of the bare run, matching repolint.DefaultRules and the lint chapter; help text still claiming every target, or a DefaultRules that gains an outbound rule without the sentence changing, would show it wrong From 17c7e5a1da0760323d36733e8d3e04f739dff47c Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:38:04 +0100 Subject: [PATCH 08/11] docs(update): state the one-document refusal without naming the flag The surface chapter's prose may not state shape above the generated appendix, and the sentence added for the one-document refusal named the --json flag. It now says "in the JSON form". Refs: iss-2609282105241960 Assisted-by: Claude:claude-opus-5-5 --- .abcd/development/brief/04-surfaces/21-update.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.abcd/development/brief/04-surfaces/21-update.md b/.abcd/development/brief/04-surfaces/21-update.md index 4716669a9..9f97e5863 100644 --- a/.abcd/development/brief/04-surfaces/21-update.md +++ b/.abcd/development/brief/04-surfaces/21-update.md @@ -116,8 +116,8 @@ origin, the tag, the asset and its digest, the target path (redacted to `~`), an the ownership proof that allowed the swap. It carries `env_ignored` when proxy or CA overrides were scrubbed. A refusal receipt is deliberately thinner: a refusal raised before any fetch carries the target path and a block naming shape, detail -and remedy, and nothing else, because there is no release it could name. Under -`--json` a refusal of either kind is one document on stdout: the receipt, +and remedy, and nothing else, because there is no release it could name. In the +JSON form a refusal of either kind is one document on stdout: the receipt, carrying the three fields of the global refusal envelope (`"abcd": "error"`, `error`, `exit_code`) beside its own, so a reader expecting one document gets the refusal whole rather than a receipt followed by a second envelope. From c06ab09ab3b954cb5515d73f0574ac2f74b76757 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Mon, 28 Sep 2026 23:13:07 +0100 Subject: [PATCH 09/11] fix(guard): refuse a recursive delete of the home's dotfiles, and warn on the working directory by name The security review of this branch found `rm -rf ~/.*`, `$HOME/.*` and `${HOME}/.*` an allow: every dotfile and dot-directory in the home (keys, shell and tool settings, abcd's own store), in a shape people type rather than an obfuscation, while the working directory's `.*` was already a warn. The three words join rm-rf-root-or-home's operands, a blocker. It also found the working-directory warn firing on `.` and `..`, which rm refuses by itself, while `rm -rf "$PWD"`, the spelling that deletes, passed. `$PWD`, `${PWD}`, both with `/*`, and the globs `*/`, `./*/` and `./.*` join rm-rf-working-directory's operands, a warn. `$PWD` is compared as the known word, the same design as `$HOME`. The review's over-match list stays an allow and is pinned: `rm -rf .git`, `.venv`, `~/.cache/x`, `"$HOME/.cache/x"`, `"$PWD/build"`. The chapter and the command page list the added operands. rm-rf-after-cd-chain is left as it is, a deliberate separate act (recorded 2026-09-28). The cost suite's maximum stays 20.07 units per byte under the bar of 24. Refs: iss-2609282105242542 Assisted-by: Claude:claude-opus-5-5 --- .../development/brief/04-surfaces/17-guard.md | 16 ++++---- commands/guard.md | 10 +++-- internal/core/guard/defaults/guard.json | 19 ++++++--- internal/core/guard/rmtargets_test.go | 39 +++++++++++++++++++ 4 files changed, 68 insertions(+), 16 deletions(-) diff --git a/.abcd/development/brief/04-surfaces/17-guard.md b/.abcd/development/brief/04-surfaces/17-guard.md index ba78bd42c..1bf08cc08 100644 --- a/.abcd/development/brief/04-surfaces/17-guard.md +++ b/.abcd/development/brief/04-surfaces/17-guard.md @@ -275,13 +275,15 @@ guess, over-blocking is the direction the guard takes. A recursive delete is read by what it deletes. Of the filesystem root or the home directory (`/`, `/*`, `~`, `$HOME`, `${HOME}`, each also with a trailing -`/` or `/*`) it is a block wherever it stands, with or without `-f`. Of the -directory the shell is in or the one above it (`*`, `.`, `..`, `./*`, `../*`, -`.*`) it is a warn, graded like `git clean`, because that directory is usually -the repository and emptying a build directory the same way is ordinary work. -Chained after a `cd` any recursive forced delete blocks, as above. The target is -compared as written, before the shell expands it, so `$HOME` is seen as the -word `$HOME` although no other parameter expansion is. +`/` or `/*`, and the home's dotfiles `~/.*`, `$HOME/.*`, `${HOME}/.*`) it is a +block wherever it stands, with or without `-f`. Of the directory the shell is +in or the one above it (`*`, `*/`, `.`, `..`, `./*`, `./*/`, `../*`, `.*`, +`./.*`, and `$PWD` or `${PWD}`, each also with `/*`) it is a warn, graded like +`git clean`, because that directory is usually the repository and emptying a +build directory the same way is ordinary work. Chained after a `cd` any +recursive forced delete blocks, as above. The target is compared as written, +before the shell expands it, so `$HOME` and `$PWD` are seen as those words +although no other parameter expansion is. What an allow still does not see is a hazard that never reaches command position at all: a word that is wholly a command substitution standing where a flag diff --git a/commands/guard.md b/commands/guard.md index f13bd168f..423a3e140 100644 --- a/commands/guard.md +++ b/commands/guard.md @@ -297,11 +297,13 @@ 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. A recursive delete of the filesystem root or the home directory (`/`, `/*`, -`~`, `$HOME`, `${HOME}`, each also with a trailing `/` or `/*`) is a **block** +`~`, `$HOME`, `${HOME}`, each also with a trailing `/` or `/*`, and the home's +dotfiles `~/.*`, `$HOME/.*`, `${HOME}/.*`) is a **block** (`rm-rf-root-or-home`), with or without `-f`; one of the directory the shell is -in or the one above it (`*`, `.`, `..`, `./*`, `../*`, `.*`) is a **warn** -(`rm-rf-working-directory`). The target is compared as written, so `$HOME` is -seen as that word. +in or the one above it (`*`, `*/`, `.`, `..`, `./*`, `./*/`, `../*`, `.*`, +`./.*`, and `$PWD` or `${PWD}`, each also with `/*`) is a **warn** +(`rm-rf-working-directory`). The target is compared as written, so `$HOME` and +`$PWD` are seen as those words. What an allow still does not see is a hazard that never reaches command position at all: a delete target printed whole by a substitution (`rm -rf $(echo /)`), diff --git a/internal/core/guard/defaults/guard.json b/internal/core/guard/defaults/guard.json index 8dafd44fb..c79500741 100644 --- a/internal/core/guard/defaults/guard.json +++ b/internal/core/guard/defaults/guard.json @@ -44,7 +44,7 @@ "pattern": { "command": "rm", "flags": ["-r|-R|--recursive"], - "arg_values": ["/", "/*", "~", "~/", "~/*", "$HOME", "$HOME/", "$HOME/*", "${HOME}", "${HOME}/", "${HOME}/*"] + "arg_values": ["/", "/*", "~", "~/", "~/*", "~/.*", "$HOME", "$HOME/", "$HOME/*", "$HOME/.*", "${HOME}", "${HOME}/", "${HOME}/*", "${HOME}/.*"] }, "why": "A recursive delete of `/` or of the home directory destroys the whole machine or every file the account owns — other projects, keys, settings — and nothing brings any of it back.", "successor": "Name the one directory you mean, by its full path (`rm -rf -- /absolute/path/to/target`), and never the root or the home directory itself.", @@ -60,12 +60,15 @@ "rm -r /", "sudo rm -rf --no-preserve-root /", "env rm -rf ~/*", - "sh -c 'rm -rf ~'" + "sh -c 'rm -rf ~'", + "rm -rf ~/.*", + "rm -rf \"$HOME\"/.*" ], "known_good": [ "rm -rf /tmp/build", "rm -rf ~/scratch/build", "rm -rf \"$HOME/.cache/abcd-test\"", + "rm -rf ~/.cache/x", "rm ~/notes.txt", "rm -f /", "ls -la /", @@ -80,9 +83,9 @@ "pattern": { "command": "rm", "flags": ["-r|-R|--recursive"], - "arg_values": ["*", ".", "./", "./*", "..", "../", "../*", ".*"] + "arg_values": ["*", "*/", ".", "./", "./*", "./*/", "./.*", "..", "../", "../*", ".*", "$PWD", "$PWD/*", "${PWD}", "${PWD}/*"] }, - "why": "A recursive delete of `*`, `.` or `..` removes everything in the directory the shell is in, or the one above it — and the directory the shell is in is usually the repository itself, uncommitted work included.", + "why": "A recursive delete of `*`, `.`, `..` or `$PWD` removes everything in the directory the shell is in, or the one above it — and the directory the shell is in is usually the repository itself, uncommitted work included.", "successor": "Name the directory you mean by its path (`rm -rf -- ./build`), or check where you are first (`pwd`) and delete from the directory's parent by name.", "fixtures": { "known_bad": [ @@ -93,12 +96,18 @@ "rm -rf ../*", "rm -rf .*", "sudo rm -rf *", - "rm -r *" + "rm -r *", + "rm -rf \"$PWD\"", + "rm -rf ${PWD}/*", + "rm -rf */", + "rm -rf ./.*" ], "known_good": [ "rm -rf ./build", "rm -rf build/*", "rm -rf node_modules", + "rm -rf .git", + "rm -rf \"$PWD/build\"", "rm -f *.o", "rm *", "ls -la *", diff --git a/internal/core/guard/rmtargets_test.go b/internal/core/guard/rmtargets_test.go index fb4f62aa2..e3ebe4c4c 100644 --- a/internal/core/guard/rmtargets_test.go +++ b/internal/core/guard/rmtargets_test.go @@ -51,6 +51,29 @@ func TestRecursiveDeleteOfRootOrHomeBlocks(t *testing.T) { }) } +// TestRecursiveDeleteOfTheHomesDotfilesBlocks — iss-2609282105242542, the +// review's medium finding. `rm -rf ~/.*` names every dotfile and dot-directory +// in the home — the keys, the shell and tool settings, abcd's own store — and it +// is a shape people type, not an obfuscation: the working directory's `.*` was +// already a warn while the home's was an allow. It blocks with the root and the +// home themselves, in every spelling of the home the entry knows. A delete that +// names one dot-directory under the home is ordinary work and stays an allow. +func TestRecursiveDeleteOfTheHomesDotfilesBlocks(t *testing.T) { + const id = "rm-rf-root-or-home" + runVerdictCases(t, []verdictCase{ + {`rm -rf ~/.*`, VerdictBlock, id}, + {`rm -r ~/.*`, VerdictBlock, id}, + {`rm -rf $HOME/.*`, VerdictBlock, id}, + {`rm -rf "$HOME"/.*`, VerdictBlock, id}, + {`rm -rf ${HOME}/.*`, VerdictBlock, id}, + {`sudo rm -rf ~/.*`, VerdictBlock, id}, + + {`rm -rf ~/.cache/x`, VerdictAllow, ""}, + {`rm -rf "$HOME/.cache/x"`, VerdictAllow, ""}, + {`rm -f ~/.*`, VerdictAllow, ""}, + }) +} + // TestRecursiveDeleteOfTheWorkingDirectoryWarns — iss-2609282105242542. A // recursive delete of everything in the directory the shell is in (`rm -rf *`, // `rm -rf .`) deletes the repository when that directory is the repository, @@ -72,12 +95,28 @@ func TestRecursiveDeleteOfTheWorkingDirectoryWarns(t *testing.T) { {`rm -rf .*`, VerdictWarn, id}, {`sudo rm -rf *`, VerdictWarn, id}, {`rm $(true) -rf *`, VerdictWarn, id}, + // The working directory by name, and the globs that still reach + // everything in it: `rm -rf .` is refused by rm itself, while + // `rm -rf "$PWD"` is the spelling that deletes (the review's low finding). + {`rm -rf $PWD`, VerdictWarn, id}, + {`rm -rf "$PWD"`, VerdictWarn, id}, + {`rm -rf ${PWD}`, VerdictWarn, id}, + {`rm -rf $PWD/*`, VerdictWarn, id}, + {`rm -rf "$PWD"/*`, VerdictWarn, id}, + {`rm -rf ${PWD}/*`, VerdictWarn, id}, + {`rm -rf */`, VerdictWarn, id}, + {`rm -rf ./*/`, VerdictWarn, id}, + {`rm -rf ./.*`, VerdictWarn, id}, {`rm -rf ./build`, VerdictAllow, ""}, {`rm -rf build/*`, VerdictAllow, ""}, {`rm -rf node_modules`, VerdictAllow, ""}, {`rm -f *.o`, VerdictAllow, ""}, {`rm *`, VerdictAllow, ""}, + {`rm -rf .git`, VerdictAllow, ""}, + {`rm -rf .venv`, VerdictAllow, ""}, + {`rm -rf "$PWD/build"`, VerdictAllow, ""}, + {`rm -rf $PWD/build/*`, VerdictAllow, ""}, // Behind a cd chain the blocker still decides, and names both. {`cd scratch && rm -rf *`, VerdictBlock, "rm-rf-after-cd-chain"}, From 9118e670933af75eb844d92866fbad8061c809f0 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Tue, 29 Sep 2026 04:29:21 +0100 Subject: [PATCH 10/11] fix(guard): compare arg_values with a variable's written spelling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Main writes a parameter expansion as the unknown word's mark, so the arg_values compare read `rm -rf $HOME` as an empty operand (allow) and `rm -rf $OUT/` as `/` (block). The tokenizer now also records, per word holding a variable, the word as the line wrote it (segment.spelled): each variable's mark replaced by its expansion's text, filled at the three addVar sites and in parameterExpansion; a substitution's mark dropped as knownText drops it; a name the next byte would extend braced; a mark with no known name kept as the unknown mark, which names nothing. Only argValueMatches reads it (writtenOperand); every token, and so every other matcher's input, is unchanged. A string handed to a shell carries only unnamed marks (payloadView), so expandPayloads also reads the string with its variables written out (spelledView, namedPayloads) and gives each word of the mark reading the spelling of the word at the same place (spellPayload), only when both readings have the same segments and words and the known text fits (fitsWritten). `sh -c "rm -rf $HOME"` blocks again, nested strings included. Verdicts: over 12,515 inputs (every string literal of the guard tests, both corpora, every bundled fixture, each also as `sh -c "…"` and `bash -c '…'`), main's tip and this commit with the two rm-target entries removed answer identically; against the merge commit, the only changes are the 48 rm-target transitions the new tests name. workPerByteBar stays 24: the merged registry measures 20.3 units per byte on the unknown dash-word shape at the merge commit and here alike (20 fails it); strings with variables cost up to about 2 units per byte more, linear. Residuals (a target spelled any other way): a brace expansion's words and a default (`${HOME:-/}`) have no written spelling. Refs: iss-2609290321312087 Assisted-by: Claude:claude-opus-5-5 --- ...-values-operand-compare-lost-a-variable.md | 14 ++ internal/core/guard/argspelling_test.go | 98 ++++++++++++++ internal/core/guard/match.go | 36 ++--- internal/core/guard/payload.go | 125 +++++++++++++++++- internal/core/guard/tokenize.go | 69 ++++++++-- internal/core/guard/unknown.go | 70 +++++++++- 6 files changed, 382 insertions(+), 30 deletions(-) create mode 100644 .abcd/work/issues/open/iss-2609290321312087-the-shell-guard-s-arg-values-operand-compare-lost-a-variable.md create mode 100644 internal/core/guard/argspelling_test.go diff --git a/.abcd/work/issues/open/iss-2609290321312087-the-shell-guard-s-arg-values-operand-compare-lost-a-variable.md b/.abcd/work/issues/open/iss-2609290321312087-the-shell-guard-s-arg-values-operand-compare-lost-a-variable.md new file mode 100644 index 000000000..6ec08b8e0 --- /dev/null +++ b/.abcd/work/issues/open/iss-2609290321312087-the-shell-guard-s-arg-values-operand-compare-lost-a-variable.md @@ -0,0 +1,14 @@ +--- +schema_version: 1 +id: "iss-2609290321312087" +slug: "the-shell-guard-s-arg-values-operand-compare-lost-a-variable" +severity: "minor" +category: "security" +source: "agent-finding" +found_during: "autonomous run 2026-09-23" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/guard/match.go" +--- + +The shell guard's arg_values operand compare lost a variable's written spelling when main began writing a parameter expansion as the unknown-value mark: rm -rf $HOME, "$HOME", ${HOME} and $HOME/.* allowed, rm -rf $PWD stopped warning, and rm -rf "$BUILD_DIR"/* and rm -rf $OUT/ blocked as deletes of the root because the variable was read as empty text. The tokenizer needs to carry each word's written spelling of its variables to arg_values without changing what any other matcher reads. diff --git a/internal/core/guard/argspelling_test.go b/internal/core/guard/argspelling_test.go new file mode 100644 index 000000000..8d274a848 --- /dev/null +++ b/internal/core/guard/argspelling_test.go @@ -0,0 +1,98 @@ +package guard + +import ( + "strings" + "testing" +) + +// TestArgValuesReadAVariableAsWritten — iss-2609290321312087. The tokenizer +// writes a parameter expansion as the unknown word's mark (unknown.go), so an +// operand's known text no longer holds `$HOME` or `$PWD`: `rm -rf $HOME` read +// as an empty operand and allowed, and `rm -rf $OUT/` read as `/` and blocked +// as a delete of the root. An entry's arg_values are compared with the word's +// written spelling of its variables instead, and a substitution's output is +// dropped from it as before. Each line is also read as the string of `sh -c +// "…"`, whose variables the enclosing shell expands, and of `bash -c '…'`, +// whose variables the string's own shell expands. +func TestArgValuesReadAVariableAsWritten(t *testing.T) { + const home, cwd = "rm-rf-root-or-home", "rm-rf-working-directory" + cases := []struct { + cmd string + want Verdict + entry string + }{ + {`rm -rf $HOME`, VerdictBlock, home}, + {`rm -rf "$HOME"`, VerdictBlock, home}, + {`rm -rf ${HOME}`, VerdictBlock, home}, + {`rm -rf $HOME/.*`, VerdictBlock, home}, + {`rm -rf "$PWD"`, VerdictWarn, cwd}, + {`rm -rf $PWD`, VerdictWarn, cwd}, + {`rm -rf ${PWD}/*`, VerdictWarn, cwd}, + {`rm -rf "$BUILD_DIR"/*`, VerdictAllow, ""}, + {`rm -rf $OUT/`, VerdictAllow, ""}, + {`rm -rf /`, VerdictBlock, home}, + {`rm -rf ~`, VerdictBlock, home}, + {`rm -rf ~/.*`, VerdictBlock, home}, + {`rm -rf *`, VerdictWarn, cwd}, + } + for _, tc := range cases { + spellings := []string{ + tc.cmd, + `sh -c "` + strings.ReplaceAll(tc.cmd, `"`, `\"`) + `"`, + `bash -c '` + tc.cmd + `'`, + } + for n, cmd := range spellings { + t.Run(cmd, func(t *testing.T) { + d := verdictOf(t, cmd) + switch { + case tc.want == VerdictBlock: + if d.Verdict != VerdictBlock || d.EntryID != tc.entry { + t.Errorf("Check(%q) = %q via %q, want block via %q", cmd, d.Verdict, d.EntryID, tc.entry) + } + case tc.want == VerdictWarn: + // A string holding `${` is itself a warn the payload + // reader raises, so only the top-level line names the + // entry; the strings must warn and never block. + if d.Verdict != VerdictWarn || (n == 0 && d.EntryID != tc.entry) { + t.Errorf("Check(%q) = %q via %q, want warn via %q", cmd, d.Verdict, d.EntryID, tc.entry) + } + default: + // Never a delete of the root or the working directory. + if d.EntryID == home || d.EntryID == cwd || d.Verdict == VerdictBlock || (n == 0 && d.Verdict != VerdictAllow) { + t.Errorf("Check(%q) = %q via %q, want no rm-target verdict", cmd, d.Verdict, d.EntryID) + } + } + }) + } + } +} + +// TestArgValuesWrittenSpellingEdges pins how the written spelling reads the +// shapes around the table above (iss-2609290321312087): a substitution glued +// to the variable is dropped as its output is; a simple name the next byte +// would extend is braced, so `"$HOM"E` is not `$HOME`; the enclosing shell's +// variable inside the string's own single quotes is still that variable's +// value; a nested string carries the name down; and a mark whose name the +// string does not hold — a raw 0x01 byte — names nothing, where reading it as +// empty text read `\x01/` as the root. A brace expansion's words and a +// default (`${HOME:-/}`) are the recorded residual: no written spelling. +func TestArgValuesWrittenSpellingEdges(t *testing.T) { + const home = "rm-rf-root-or-home" + runVerdictCases(t, []verdictCase{ + {`rm -rf "$(true)"$HOME`, VerdictBlock, home}, + {`rm -rf $HOME"$(true)"`, VerdictBlock, home}, + {`rm -rf "$(true)"$HOME/.*`, VerdictBlock, home}, + {`sh -c "rm -rf '$HOME'"`, VerdictBlock, home}, + {`sh -c 'rm -rf "$HOME"'`, VerdictBlock, home}, + {`sh -c "sh -c \"rm -rf $HOME\""`, VerdictBlock, home}, + {`sh -c "cd /tmp && rm -r $HOME"`, VerdictBlock, home}, + {`rm -rf "$HOM"E`, VerdictAllow, ""}, + {`rm -rf "$HOME"x`, VerdictAllow, ""}, + {`rm -rf $HOMEx`, VerdictAllow, ""}, + {`sh -c "rm -rf \"$HOM\"E"`, VerdictAllow, ""}, + {"rm -rf \x01/", VerdictAllow, ""}, + {"rm -rf \"\x01\"/", VerdictAllow, ""}, + {`rm -rf ${HOME:-/}`, VerdictAllow, ""}, + {`rm -rf {$HOME,x}`, VerdictAllow, ""}, + }) +} diff --git a/internal/core/guard/match.go b/internal/core/guard/match.go index 5461bad9e..079420ff0 100644 --- a/internal/core/guard/match.go +++ b/internal/core/guard/match.go @@ -335,7 +335,7 @@ func matchSegmentNamed(p Pattern, s segment) (hit, named bool) { 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) + m := newEntryMatcher(p, s.tokens, s.spelled, glob) for _, a := range group { if m.matchesAfter(a.idx) && argsFed(p, s, a.idx) { hit = true @@ -556,14 +556,15 @@ type entryMatcher struct { // -C $(pwd) push` is a push); an unknown dash-word both stands alone and takes // a value (`git -$(x) /tmp push`); a word that may print nothing both is and is // not an operand (`git $(true) push`). The subcommands, the count, the prefix -// and the path are all met by one reading. -func newEntryMatcher(p Pattern, tokens []string, glob func(int) bool) entryMatcher { +// and the path are all met by one reading. spelled is the segment's +// segment.spelled, which only the arg_values clause reads (writtenOperand). +func newEntryMatcher(p Pattern, tokens []string, spelled map[int]string, glob func(int) bool) entryMatcher { n := len(tokens) want := operandWant{ sub: p.Subcommand, sub2: p.Subcommand2, min: p.MinOperands, prefixes: p.ArgPrefixes, paths: p.ArgPaths, values: p.ArgValues, } - m := entryMatcher{accept: operandAcceptance(tokens, p.ValueFlags, want, glob), nextStop: make([]int, n+1)} + m := entryMatcher{accept: operandAcceptance(tokens, spelled, p.ValueFlags, want, glob), nextStop: make([]int, n+1)} m.nextStop[n] = n for i := n - 1; i >= 0; i-- { m.nextStop[i] = m.nextStop[i+1] @@ -689,18 +690,21 @@ func argPrefixMatches(prefix string, ops []string) bool { return false } -// argValueMatches reports whether some operand is one of the words. Only -// operands are considered, and each by its known text, as argPrefixMatches -// reads a prefix: a word that is wholly a substitution is how an everyday -// delete names its target (`rm -rf "$(mktemp -d)"`), so reading it as every -// target would refuse them all (unknown.go's operand residual). -func argValueMatches(values []string, ops []string) bool { - for _, op := range ops { - k := knownText(op) - for _, v := range values { - if k == v { - return true - } +// argValueMatches reports whether an operand, as writtenOperand reads it, is +// one of the words. Only operands are considered, and a substitution's output +// is taken as empty, as argPrefixMatches reads a prefix: a word that is wholly +// a substitution is how an everyday delete names its target (`rm -rf +// "$(mktemp -d)"`), so reading it as every target would refuse them all +// (unknown.go's operand residual). A variable is compared as the line wrote +// it, so `$HOME` names the home and `"$OUT"/` names no root; one whose text is +// not known names nothing. +func argValueMatches(values []string, written string) bool { + if isUnknown(written) { + return false + } + for _, v := range values { + if written == v { + return true } } return false diff --git a/internal/core/guard/payload.go b/internal/core/guard/payload.go index b88ff90fe..9cb535768 100644 --- a/internal/core/guard/payload.go +++ b/internal/core/guard/payload.go @@ -115,7 +115,9 @@ func expandPayloads(segs []segment) ([]segment, []payloadSignal) { // segment, however many strings it carries (payloadInput). var stdin, args []feed inputRead := false - for _, ref := range payloadRefsOf(payloadView(s)) { + refs := payloadRefsOf(payloadView(s)) + named := namedPayloads(s, refs) + for r, ref := range refs { kind, fam, payload, trailing := ref.kind, ref.family, ref.payload, ref.trailing // Past the depth budget the guard cannot follow the nesting, so a // family member here is fail-closed regardless of family. @@ -142,6 +144,7 @@ func expandPayloads(segs []segment) ([]segment, []payloadSignal) { continue } psegs = pseg + spellPayload(psegs, named[r]) case kindShellWarn: signals = append(signals, shellUnresolvedSignal()) continue @@ -157,6 +160,7 @@ func expandPayloads(segs []segment) ([]segment, []payloadSignal) { continue } psegs = pseg + spellPayload(psegs, named[r]) case kindExecStringWarn: signals = append(signals, execStringWarnSignal(fam)) continue @@ -240,6 +244,125 @@ func payloadView(s segment) segment { return v } +// namedPayloads returns, parallel to refs, the text of each string as the +// line wrote its variables (segment.spelled), "" where it is no other text or +// cannot be paired: payloadView hands a string the mark of each value the +// enclosing shell put in it, which reads as an unknown word with no name, so +// `sh -c "rm -rf $HOME"` holds a mark where `$HOME` was written. Only the +// arg_values compare reads what spellPayload takes from it; every reading of +// the string reads the marks (iss-2609290321312087). The words are paired by +// payloadsOf's own order, and a pair whose kind or family differs is not +// paired. +func namedPayloads(s segment, refs []payloadRef) []string { + named := make([]string, len(refs)) + v, ok := spelledView(s) + if !ok { + return named + } + nrefs := payloadRefsOf(v) + if len(nrefs) != len(refs) { + return named + } + for r, ref := range refs { + if n := nrefs[r]; n.kind == ref.kind && n.family == ref.family && n.payload != ref.payload { + named[r] = n.payload + } + } + return named +} + +// spelledView is payloadView with each word the line wrote with a known +// variable spelled as the line wrote it (segment.spelled) instead of with +// varMark, where payloadView spells it; a variable whose text is not known +// stays varMark. ok is false when no word changes. +func spelledView(s segment) (segment, bool) { + if len(s.spelled) == 0 { + return s, false + } + v := payloadView(s) + var toks []string + for i, text := range s.variable { + // payloadView spelled this word with varMark (text); a word it left, + // where a command can sit, keeps its unknownMark and is left here too. + w, ok := s.spelled[i] + if !ok || v.tokens[i] != text { + continue + } + if w = strings.ReplaceAll(w, unknownText, varText); w == text { + continue + } + if toks == nil { + toks = append([]string(nil), v.tokens...) + } + toks[i] = w + } + if toks == nil { + return v, false + } + v.tokens = toks + return v, true +} + +// spellPayload reads the string named, the same string as the one psegs +// were read from with its variables written out (namedPayloads), and gives +// each word of psegs that holds a variable the spelling of the word at the +// same place of named: its own segment.spelled, or its text where the string +// quotes the name (`sh -c "rm -rf '$HOME'"`). A word is paired only when +// both readings have the same segments and words, and the word read from +// named has the mark-view word's known text in the same order around it +// (fitsWritten); an unpaired word keeps the spelling it has, which names no +// variable. Nothing else of psegs is changed. +func spellPayload(psegs []segment, named string) { + if named == "" { + return + } + nsegs, err := tokenize(named) + if err != nil || len(nsegs) != len(psegs) { + return + } + for i := range psegs { + m, n := psegs[i], nsegs[i] + if len(m.spelled) == 0 || len(m.tokens) != len(n.tokens) { + continue + } + for j := range m.spelled { + w, ok := n.spelled[j] + if !ok { + if isUnknown(n.tokens[j]) { + continue + } + w = n.tokens[j] + } + if fitsWritten(m.tokens[j], n.tokens[j]) && fitsWritten(m.tokens[j], w) { + m.spelled[j] = w + } + } + } +} + +// fitsWritten reports whether word, read from a string's named text, can be +// marked, read from its mark view, at the same place: marked's known text, +// split at its marks, stands in word in the same order, the first part +// leading and the last trailing. +func fitsWritten(marked, word string) bool { + parts := strings.Split(marked, unknownText) + if len(parts) == 1 { + return marked == word + } + if !strings.HasPrefix(word, parts[0]) { + return false + } + rest := word[len(parts[0]):] + for _, p := range parts[1 : len(parts)-1] { + k := strings.Index(rest, p) + if k < 0 { + return false + } + rest = rest[k+len(p):] + } + return strings.HasSuffix(rest, parts[len(parts)-1]) +} + // 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, here-documents and redirected process substitutions print, and diff --git a/internal/core/guard/tokenize.go b/internal/core/guard/tokenize.go index 0ed8a955f..1def3cd13 100644 --- a/internal/core/guard/tokenize.go +++ b/internal/core/guard/tokenize.go @@ -81,6 +81,16 @@ type segment struct { // unknown.go), and a string handed to a shell carries the value's mark // for the re-read to take as a variable's (payloadView). variable map[int]string + // spelled records, per token index, a word holding a parameter + // expansion's mark as the line WROTE it: each variable's mark replaced by + // its expansion's text (`$HOME`, `${PWD}`; a simple name the next byte + // would extend is braced), every substitution's mark dropped, and a mark + // whose text is not known — a varMark carried into a payload's text — + // kept as unknownMark. Only an entry's arg_values read it + // (writtenOperand): every other reading takes the token, where the + // variable is the unknown word's mark (iss-2609290321312087). nil when no + // word holds a variable. + spelled map[int]string // arrivals caches commandArrivals(tokens) once Check has its final // segments (walked records that it is set), so the walk to command position // is paid once per segment rather than once per entry. A segment built @@ -373,6 +383,11 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { // substitution left their mark in it (addCur). vars map[int]string curVar, curSub bool + // spells rides with the segment (segment.spelled); curVarAt records, + // for the word being built, where in cur each variable's mark stands + // and the expansion's text, "" where it is not known. + spells map[int]string + curVarAt []varSite // curMask is parallel to cur and records, per byte, whether it reached // the tokenizer unquoted (wordStruct) and whether it began its word // (wordRawStart) — what the brace expander needs to read a word the way @@ -548,6 +563,7 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { case varMark: c = unknownMark curVar = true + curVarAt = append(curVarAt, varSite{at: len(cur)}) case unknownMark: curSub = true } @@ -594,9 +610,31 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { } vars[len(toks)] = text } - // addVar leaves the mark of a parameter expansion where its value goes. - addVar := func() { + // addVar leaves the mark of a parameter expansion where its value goes, + // and records the expansion's text as the line wrote it (segment.spelled). + addVar := func(text string) { addCur([]byte{varMark}, 0) + curVarAt[len(curVarAt)-1].text = text + } + // recordSpelling files the word being built under segment.spelled when a + // variable's mark is in it: word is the token it becomes, and whole + // reports that the token is cur as built, so each mark's place is known. + // A word whose marks cannot be placed — one brace expansion made, or one + // unknownFromOpenExpansion rewrote — is filed as unknownText, which no + // value names. + recordSpelling := func(word string, whole bool) { + if !curVar { + return + } + if spells == nil { + spells = map[int]string{} + } + switch { + case whole: + spells[len(toks)] = spellWritten(cur, curVarAt) + case isUnknown(word): + spells[len(toks)] = unknownText + } } flushToken := func() { if !hasCur { @@ -619,11 +657,13 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { for _, w := range words { recordFeeds() recordVar(false) - toks = append(toks, unknownFromOpenExpansion(string(w.b))) + word := unknownFromOpenExpansion(string(w.b)) + recordSpelling(word, false) + toks = append(toks, word) globs = append(globs, w.globbed()) } cur, curMask, hasCur, curGlob, curBrace = nil, nil, false, false, false - curPieces, curFeeds, curVar, curSub = nil, nil, false, false + curPieces, curFeeds, curVar, curSub, curVarAt = nil, nil, false, false, nil return } braceGroup = true @@ -641,7 +681,8 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { curPieces = nil recordFeeds() recordVar(true) - curFeeds, curVar, curSub = nil, false, false + recordSpelling(tok, tok == string(cur)) + curFeeds, curVar, curSub, curVarAt = nil, false, false, nil // An unquoted `{` or `}` in command position opens or closes a group. if len(curMask) == 1 && curMask[0]&wordStruct != 0 && allReserved(toks) { switch tok { @@ -670,12 +711,13 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { segs = append(segs, segment{ tokens: toks, chain: chain, braceGroup: braceGroup, globbed: globsOrNil(globs), stdinStream: curStdin || pipeNext || len(groupIn) > 0, literal: lits, feeds: feeds, piped: piped, - stdinIn: groupIn, home: list, at: len(segs), variable: vars, + stdinIn: groupIn, home: list, at: len(segs), variable: vars, spelled: spells, }) toks = nil globs = nil lits = nil vars = nil + spells = nil feeds = nil braceGroup = false pipeNext = false @@ -815,6 +857,7 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { openSubstitution := func(kind parenKind, pos int, procSub bool) { saved := &enclosing{ toks: toks, globs: globs, lits: lits, vars: vars, curVar: curVar, curSub: curSub, + spells: spells, curVarAt: curVarAt, cur: cur, curMask: curMask, hasCur: hasCur, curGlob: curGlob, curBrace: curBrace, braceGroup: braceGroup, chain: chain, procSub: procSub, curStdin: curStdin, pipeNext: pipeNext, curDocs: curDocs, pieces: curPieces, @@ -823,6 +866,7 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { } toks, globs, lits, cur, curMask, hasCur, curGlob, curBrace, braceGroup = nil, nil, nil, nil, nil, false, false, false, false curPieces, vars, curVar, curSub = nil, nil, false, false + spells, curVarAt = nil, nil // A substitution is a command string of its own: its pipelines begin // inside it. Its standard input is its command's: what was piped into // the groups around it, and the pipe into the command it sits in @@ -872,6 +916,7 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { curStdin, pipeNext, curDocs, curPieces = e.curStdin, e.pipeNext, e.curDocs, e.pieces feeds, curFeeds, pipeFrom, braceFrom, groupIn = e.feeds, e.curFeeds, e.pipeFrom, e.braceFrom, e.groupIn vars, curVar, curSub = e.vars, e.curVar, e.curSub + spells, curVarAt = e.spells, e.curVarAt if !f.bare { addCur([]byte(arithmeticOperand), 0) // The number it prints is computed from what the substitutions @@ -893,6 +938,7 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { curStdin, pipeNext, curDocs, curPieces = e.curStdin, e.pipeNext, e.curDocs, e.pieces feeds, curFeeds, pipeFrom, braceFrom, groupIn = e.feeds, e.curFeeds, e.pipeFrom, e.braceFrom, e.groupIn vars, curVar, curSub = e.vars, e.curVar, e.curSub + spells, curVarAt = e.spells, e.curVarAt feedFrom(e.segStart) if e.procSub { addCur([]byte(procSubOperand), 0) @@ -917,7 +963,7 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { start := len(segs) expandedBody(body) feedFrom(start) - addVar() + addVar("${" + body + "}") if len(segs) > start { curSub = true } @@ -1041,7 +1087,7 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { continue } if k := simpleParamEnd(line, j+1); line[j] == '$' && k >= 0 { - addVar() + addVar(line[j:k]) j = k continue } @@ -1365,9 +1411,10 @@ func tokenizeAt(line string, depth int, budget *int) ([]segment, error) { // is not in the command line, so the word holds unknownMark where // it goes (unknown.go), and `--$X` is a flag of unknown name as // `--$(x)` is. A `$` that is quoted or escaped never reaches here. - addVar() + end := simpleParamEnd(line, i+1) + addVar(line[i:end]) lastList = false - i = simpleParamEnd(line, i+1) + i = end case c == '$' && i+1 < len(line) && line[i+1] == '{': // A `${…}` expansion, read as the double-quoted one is. One whose // `}` is missing is a syntax error bash refuses, and stays text. @@ -2049,6 +2096,8 @@ type enclosing struct { vars map[int]string curVar bool curSub bool + spells map[int]string + curVarAt []varSite cur []byte curMask []byte hasCur bool diff --git a/internal/core/guard/unknown.go b/internal/core/guard/unknown.go index 4158e591b..4f02ba634 100644 --- a/internal/core/guard/unknown.go +++ b/internal/core/guard/unknown.go @@ -102,6 +102,69 @@ const varMark = '\x01' // varText is varMark as a string, for spelling a payload's text. const varText = "\x01" +// varSite is one variable's mark in a word being built: its offset in the +// word, and the expansion's text as the line wrote it (`$HOME`, `${PWD}`), "" +// for a varMark read from a payload's text, whose name the string no longer +// holds. +type varSite struct { + at int + text string +} + +// spellWritten is a word as the line wrote its variables (segment.spelled): +// each variable's mark replaced by its expansion's text, each substitution's +// mark dropped as knownText drops it, and a variable whose text is not known +// kept as unknownMark. A simple name the next byte kept would extend is +// braced (`"$A"B` is `${A}B`, not `$AB`), so the spelling reads as the same +// expansions when it is read again (spelledView). sites is in word order. +func spellWritten(word []byte, sites []varSite) string { + var b strings.Builder + k := 0 + isVar := func(p int) bool { return k < len(sites) && sites[k].at == p } + for p := 0; p < len(word); p++ { + if !isVar(p) { + if word[p] != unknownMark { + b.WriteByte(word[p]) + } + continue + } + text := sites[k].text + k++ + if text == "" { + b.WriteByte(unknownMark) + continue + } + if text[1] != '{' { + next := p + 1 + for next < len(word) && word[next] == unknownMark && !isVar(next) { + next++ + } + if next < len(word) && word[next] != unknownMark && isNameByte(word[next]) { + text = "${" + text[1:] + "}" + } + } + b.WriteString(text) + } + return b.String() +} + +// isNameByte reports whether c can continue a shell variable's name. +func isNameByte(c byte) bool { + return c == '_' || c >= '0' && c <= '9' || c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' +} + +// writtenOperand is the text an entry's arg_values compare reads for the +// word at i: the word as the line wrote its variables where it holds one +// (segment.spelled), else its known text. It is read by nothing else, so +// `rm -rf $HOME` names `$HOME` to arg_values while every other reading takes +// the variable as the unknown word it is (iss-2609290321312087). +func writtenOperand(tokens []string, spelled map[int]string, i int) string { + if w, ok := spelled[i]; ok { + return w + } + return knownText(tokens[i]) +} + // isUnknown reports whether a word carries a substitution's output. func isUnknown(tok string) bool { return strings.IndexByte(tok, unknownMark) >= 0 } @@ -650,8 +713,9 @@ type operandWant struct { // satisfy every clause together — operand 0 and 1 are the same reading's — so // the table's state is (word, operands so far, clauses met), filled from the // end once: linear in the words, whatever the number of places a command can -// sit. -func operandAcceptance(tokens, valueFlags []string, want operandWant, glob func(int) bool) []bool { +// sit. spelled is the segment's segment.spelled, read by the arg_values +// clause alone (writtenOperand). +func operandAcceptance(tokens []string, spelled map[int]string, valueFlags []string, want operandWant, glob func(int) bool) []bool { need := want.need() nv := 0 if len(want.values) > 0 { @@ -687,7 +751,7 @@ func operandAcceptance(tokens, valueFlags []string, want operandWant, glob func( hits |= 1 << (len(want.prefixes) + j) } } - if nv > 0 && argValueMatches(want.values, []string{a}) { + if nv > 0 && argValueMatches(want.values, writtenOperand(tokens, spelled, i)) { hits |= 1 << (len(want.prefixes) + len(want.paths)) } } From 8cd7f88f45d079d8b93c2ab4254b9110f0e69a76 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Tue, 29 Sep 2026 04:29:34 +0100 Subject: [PATCH 11/11] =?UTF-8?q?chore:=20resolve=20iss-2609290321312087?= =?UTF-8?q?=20=E2=80=94=20arg=5Fvalues=20reads=20a=20variable=20as=20writt?= =?UTF-8?q?en?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fix is 9118e6709: the arg_values compare reads each operand's written spelling of its variables, and nothing else reads it. Resolves: iss-2609290321312087 Assisted-by: Claude:claude-opus-5-5 --- ...-guard-s-arg-values-operand-compare-lost-a-variable.md | 8 ++++++++ 1 file changed, 8 insertions(+) rename .abcd/work/issues/{open => resolved}/iss-2609290321312087-the-shell-guard-s-arg-values-operand-compare-lost-a-variable.md (55%) diff --git a/.abcd/work/issues/open/iss-2609290321312087-the-shell-guard-s-arg-values-operand-compare-lost-a-variable.md b/.abcd/work/issues/resolved/iss-2609290321312087-the-shell-guard-s-arg-values-operand-compare-lost-a-variable.md similarity index 55% rename from .abcd/work/issues/open/iss-2609290321312087-the-shell-guard-s-arg-values-operand-compare-lost-a-variable.md rename to .abcd/work/issues/resolved/iss-2609290321312087-the-shell-guard-s-arg-values-operand-compare-lost-a-variable.md index 6ec08b8e0..55275a9b5 100644 --- a/.abcd/work/issues/open/iss-2609290321312087-the-shell-guard-s-arg-values-operand-compare-lost-a-variable.md +++ b/.abcd/work/issues/resolved/iss-2609290321312087-the-shell-guard-s-arg-values-operand-compare-lost-a-variable.md @@ -9,6 +9,14 @@ found_during: "autonomous run 2026-09-23" origin: researcher-authored production_mode: hand-written found_at: "internal/core/guard/match.go" +resolution: "arg_values compares each operand's written spelling of its variables (segment.spelled), carried into shell strings by a paired second reading; every other matcher reads the unchanged tokens." +impact: fix +resolved_by: + commit: "9118e6709" --- The shell guard's arg_values operand compare lost a variable's written spelling when main began writing a parameter expansion as the unknown-value mark: rm -rf $HOME, "$HOME", ${HOME} and $HOME/.* allowed, rm -rf $PWD stopped warning, and rm -rf "$BUILD_DIR"/* and rm -rf $OUT/ blocked as deletes of the root because the variable was read as empty text. The tokenizer needs to carry each word's written spelling of its variables to arg_values without changing what any other matcher reads. + +## Grounds + +- pursued: rm -rf $HOME, "$HOME", ${HOME} and $HOME/.* block, $PWD forms warn, and "$BUILD_DIR"/* and $OUT/ allow, at top level and inside sh -c and bash -c strings (argspelling_test.go); a verdict diff of 12,515 inputs against main's tip with the two rm entries removed shows zero changes. A variable-bearing target that allows, or any other entry's verdict moving, would show it wrong.