Skip to content

fix: catch C-quoted filenames in four probes, close a one-way parity test, and record the pipefail question - #3476

Merged
claude[bot] merged 1 commit into
mainfrom
claude/probe-filter-and-parity-followups
Aug 28, 2026
Merged

fix: catch C-quoted filenames in four probes, close a one-way parity test, and record the pipefail question#3476
claude[bot] merged 1 commit into
mainfrom
claude/probe-filter-and-parity-followups

Conversation

@claude

@claude claude Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

No linked issue

Summary

The last five findings from an independent security review of #3460/#3468/#3469, plus the durable fix that should stop this class recurring. #3474 and #3475 closed the probe-shape and citation halves; this closes the filter half and writes the open question into the convention.

Four probes silently under-reported any filename git status --porcelain C-quotes — a plain space is enough to defeat grep '\.md$' and awk '{print $NF}'. Two correct forms already shipped in the same two plugins, so this is drift, not a missing idea.

Fix

F1 — under-reporting filters

Executed against a repo holding seven files per extension: plain, space, single quote, double quote, semicolon, pipe, and an embedded newline.

Site Before After Caught
docs-hygiene:compress grep '\.md$' grep -E '\.md"?$' 4/7 → 7/7
docs-hygiene:audit-progressive-disclosure grep '\.md$' grep -E '\.md"?$' 4/7 → 7/7
docs-hygiene:audit-derivability awk '/\.md$/' awk '/\.md"?$/' 4/7 → 7/7
code-tidying:dissolve-comments awk '{print $NF}' -z NUL parse 4/7 → 7/7

dissolve-comments took the -z parse because the site already parses fields and its sibling audit-comment-residue ships that exact program. The .md sites took audit-noise's grep form, which keeps their leading token grep so the existing grant stays valid.

The residual is stated in the changelogs rather than hidden: under -z a newline-bearing name reaches the grep as two lines and renders as its tail; under the grep form a quoted record renders C-quoted.

F2 — a parity test that could not see an over-reporting preview

audit-comment-residue's parity loop only checked one direction: "detect.sh audited it but the preview missed it". Mutating the SKILL.md awk to drop its rename skip made the preview emit a phantom path while detect.sh audited five real ones — and the suite passed 53/53.

A reverse loop now asserts every previewed path is one detect.sh audited. The same mutation fails 1/54, naming the phantom; reverted, 54/54. The forward loop is byte-identical — no existing assertion was touched or weakened.

F3 — a failure token that collided with real data

claude-ops:observability rendered unknown both outside a repo and inside one whose toplevel directory is literally named unknown. The failure case now renders (git toplevel unavailable); a repo named unknown still renders unknown, now distinguishable.

F4 — labels asserting a state the probe did not establish

The brace group's : makes the outer || unreachable, which also masks failures inside the group — a filter-stage failure, and a guard/data double-invocation. Both render empty under a label reading empty = none.

Applied at six sites, not the three first reported: all five brace-group probes plus audit-derivability. Labels now read empty = none matched or the probe returned nothing. No plumbing was restructured — the label is the honest fix, and the plumbing change would be a worse trade.

repo-hygiene:clean and the ~25 unfiltered { git status || echo; } | head -N probes were left alone: no filter stage, no double invocation, so their empty = none is already true.

F5 — the durable fix

precompute-context.md gains a subsection recording that whether Claude Code runs ! injections under pipefail is undocumented — the skills page specifies working directory, stderr merging, timeout, output size and exit-code semantics, and names no shell options. It states that the brace group is correct under both settings while guard && pipeline || token is correct under only one, gives the mechanism for each direction, and names the brace group's own price: an internal failure renders empty, which is exactly why the label must carry it. The recheck trigger now covers shell options.

This is the part that stops the next author reintroducing the shape.

Verification

Pipefail matrix on the merged tree: seven touched probes × five states (healthy, at cap, no match, clean tree, outside a repo) × two shell settings = 70 cells, all exit 0. At-cap renders exactly the cap with no failure token; only the outside-a-repo cells render one. audit-derivability additionally re-proved at 25 dirty .md files: 20 paths under both settings, no spurious token.

scripts/affected-tests.sh --run                 real exit 0, 17/17 suites (not piped)
scripts/check-changed-skills.sh origin/main     8 skills, 0 failed
check-changelog-parity --check-order/-bump/     0 each; --check-preserved compared
  --check/--check-preserved                     306 headings across 4 changelogs
node scripts/validate-plugin-contracts.mjs      3061 files
markdownlint-cli2 (173 files, 4 plugins)        0 issues
typos                                           0; added lines carry 0 em dashes
shellcheck -S warning (both test scripts)       clean

Roster delta. Two derivations unrelated to the report's table — every git status --porcelain inside an injection (35 hits), and every injection carrying an extension-anchored filter — both return the same six filtered probes and no others. Delta: +1 (audit-derivability), 0 removed. The security report listed five and called three defective; the true set is six, four defective. A fourth pass over every injection with a text-filter stage found four more (tidy, rename-references, two detect.sh | grep '^(Summary' lines), all anchored on a line prefix rather than a filename, so none share the defect.

Stale-base recovery. This branch was cut before #3474 and #3475 merged and was rebuilt onto 02e1d8b0. One line collided: audit-derivability:15, where #3474 added the brace group and this change adds the filter. Both are now present on that line — committing either alone would have silently reverted the other. docs-hygiene and code-tidying versions were renumbered above what main now carries, since #3474 had already claimed the numbers first picked.

Grants. None widened, none broken. audit-progressive-disclosure is the only touched skill declaring allowed-tools; its leading token stays grep, so Bash(grep:*) still authorizes the rewritten command and no new binary entered the pipeline.

Related

Refs #3474, refs #3475, refs #3469, refs #3468, refs #3460. Closes the filter half of the same verification pass.


Generated by Claude Code

…rectional parity test

Four of the marketplace's six extension-filtered pre-computed probes anchored on `$`
against unquoted `git status --porcelain` output. Git C-quotes any path holding a space
or a metachar, which puts the closing `"` past the extension and off the anchor, so each
showed the model a preview missing files with no signal that anything was missing.
Proven by execution on a repository holding seven files per extension whose names carry
a space, a single quote, a double quote, a semicolon, a pipe, and a newline: the old
anchors matched 4 of 7, the new ones match 7 of 7.

- docs-hygiene `compress`, `audit-progressive-disclosure`: adopt `audit-noise`'s
  `grep -E '\.md"?$'`.
- docs-hygiene `audit-derivability`: same anchor fix inside 0.21.26's brace group, which
  is preserved. Roster addition, not a listed finding; found by re-deriving the roster
  over every `git status --porcelain` injection and again over every extension-anchored
  filter. Both derivations return the same six probes and no others.
- code-tidying `dissolve-comments`: adopt `audit-comment-residue`'s `-z` NUL parse. The
  line is a preview capped at 10 whose body already tells the model to re-enumerate at
  scope time, so the impact is bounded.

`audit-comment-residue`'s SKILL.md parity test now compares in both directions. Dropping
the rename skip from the SKILL.md awk program makes the preview emit a mangled path
naming no file, and the suite still passed 53/53. With the reverse loop that mutation
fails 1/54 and reverting it passes 54/54. No existing assertion weakened.

`claude-ops:observability`'s repo-slug probe rendered `unknown` both on a failed
`git rev-parse` and in a repository whose toplevel is literally named `unknown`. The
failure case now renders `(git toplevel unavailable)`.

Six probe labels no longer assert a bare `empty = none`. The brace group's closing `:`
makes the outer `||` unreachable, so a failure inside the group renders empty too: a
filter binary off PATH, or a second `git` invocation that fails after the guard's copy
succeeded. Not a regression, but the label was making a claim the plumbing cannot back.

`playbooks:skill-authoring`'s precompute convention now records the open `pipefail`
question: the skills docs name no shell options, the brace-group form is correct under
both settings, and a `guard && pipeline || token` shape is correct under only one.

Re-ran the state matrix over every touched probe (healthy, at cap, no match, clean tree,
outside a repo, each under `set -o pipefail` and `set +o pipefail`): all 70 cells exit 0,
at-cap cells render exactly the cap with no failure token, and only the outside-a-repo
cells render one. The brace groups from #3469 and #3474 are intact.

No grant widened. `audit-progressive-disclosure` keeps `Bash(grep:*)` and its leading
token is still `grep`; the other four skills declare no `allowed-tools`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UmrZGdp1dgbZuPCy7tcRJo
@claude
claude Bot merged commit b293df8 into main Aug 28, 2026
60 checks passed
@claude
claude Bot deleted the claude/probe-filter-and-parity-followups branch August 28, 2026 15:09
claude Bot added a commit that referenced this pull request Aug 28, 2026
…, and flag a fired security re-trigger (#3477)

Every one of #3468's ADR-0018 citation fixes wrote a CHANGELOG entry quoting
the path it removed, creating twelve new instances of the shape it was closing.
The population at main is 16 fixed and 35 kept, not 16 and 23. #3475 wrote the
governing test into ADR 0018; this applies it to those twelve so a later
re-derivation does not re-open them.

All twelve are keep-correct, established row by row and anchored on text rather
than line numbers. A dated changelog entry quoting a citation it removed is the
evidence case exactly. No CHANGELOG was edited, so no convention bump is owed.

Three sub-rulings the class needed, now written into the spec rather than left
implicit. config-cascade's three rows are the only clause-3 question in the set:
they are plugin-relative and resolve against nothing, and the entry says so in
the same sentence. Clause 3 governs addresses a doc offers, not strings it
quotes in order to report them broken, and forcing resolution would delete the
finding. The other nine resolve on disk today, verified one path at a time
across eleven distinct targets. No row carries a line or step pin, the part the
amendment says rots first.

One excluded on purpose and recorded so it is not re-opened:
plugins/review/reference/topic-docs.md, a plugin-level non-skill tree the
amendment routes to its own pass, which the entry declares a keep rather than
quoting as removed.

Separately, MIGRATION-PLAYBOOK.md records a dometrain security review performed
at 0.1.0 and states that a version bump adding a new trust surface re-triggers
it. The manifest now reads 0.2.7, eleven releases later. The note added states
the reviewed version, the shipping version, that the ACCEPT below it describes
0.1.0 only, and that the re-review is owed and deliberately not performed here.
It stops short of asserting a trust surface was added: that is the condition the
clause turns on and cannot be settled by reading the page. Two checks were run
to avoid claiming otherwise and neither settles it, which the note says.

The spec's dometrain entry pinned MIGRATION-PLAYBOOK.md:943, a line this
change's own edit would have invalidated. Re-anchored on the record's heading
text. That decay rule has been violated three times across this sweep; this is
the first time it was caught before landing.

Recorded because two earlier claims about it were wrong in opposite directions:
this worker's definition declares the Agent tool and the harness withheld it at
spawn, so nesting is gated by depth realized as pre-launch tool omission. There
is no refusal string because there is no call to refuse.

Refs #3476, refs #3475, refs #3474, refs #3469, refs #3468, refs #3460.
claude Bot added a commit that referenced this pull request Aug 28, 2026
…reopened it three times (#3478)

The L4 roster's Group 2, eight intra-plugin path-form citations, was the last
block anyone believed open. All eight were already closed, by 6c7a103 (#3380)
on 2026-08-26 -- the same commit that wrote the roster. Its own message names
what it did: eight citations written with an implied base of the plugin root
while the real base was reference/, none of which resolved for any reader.

Verified one at a time against the roster's own text test. All eight now read
${CLAUDE_PLUGIN_ROOT}/skills/<skill>/<path> and all seven distinct targets
exist on disk. Zero plugins/** files edited, so no bump and no plugin changelog
entry: nothing needed changing.

The reason four separate passes each re-derived this roster from scratch is one
sentence. #3380 fixed 32 of 34 rows and left its summary asserting that all 34
path:line citations were re-verified on 2026-08-26 and every one resolves --
standing over citations it had just deleted. A fifth-round audit caught that for
Group 1, where 22 of 32 were already closed and twelve of those by #3380 itself.
This closes the same hole for Group 2. The generalizable rule is now in the
sweep spec: a record that fixes findings and updates its own summary in the same
commit must update the summary, or the summary outranks the fix for every later
reader.

The roster's inventory rows are left verbatim, per its own decay rule that the
inventory is the part that cannot be re-derived, and per the precedent that
#3474 and #3475 both closed rows without touching the file. Only an additive
closure stamp is appended.

On whether ADR 0018 reaches this class, the honest answer is that its
encapsulation half does not. Clause 1 legalises intra-plugin citations and names
this exact citing surface. Clause 2 cannot apply, because both files ship inside
one plugin so the runtime absence motivating it cannot occur, and the 2026-08-28
amendment's fix-an-address / keep-evidence test divides clause 2 applications
only. Only clause 3 reaches Group 2, and clause 3 is a resolvability rule rather
than an encapsulation one: had these been open, the remedy would have been path
form and nothing else. The clause that earned its keep is the ADR's own
observation that proximity did not prevent them.

A second derivation making no reference to the roster resolved every citation
token in every plugin-level reference/, context/ and agents/ tree plus every
plugin README against the base its own form implies: 52 tokens, 0 clause-3
failures. Group 3's two anchors were also closed by #3380. 34 closed, 0 open.

No fresh-context verifier reviewed this diff. Nested spawning is unavailable at
this depth: the Agent tool is withheld pre-launch, so there is no call to refuse
and no refusal string. A second mechanical derivation was substituted and every
line reference re-checked against the live tree, which is weaker than a fresh
context and is recorded as such.

Refs #3477, refs #3476, refs #3475, refs #3474, refs #3469, refs #3468.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant