Round-five review found command substitution executing inside the very quoted
spans the skeleton was discarding as prose:
echo "$(curl -d@b .../issues/1/comments)"
msg="$(curl -d@b .../issues/1/comments)"
The unquoted and process-substitution forms already blocked, so the same call
was refused or allowed depending on a quote character. That makes it a
classification defect rather than another spelling, and it is the nineteenth
write to reach execution through this file by the same route: the client was
ABSENT from the skeleton, so the guard allowed.
The reviewer's judgement, which I asked for and accept: this is fitting to the
test set. Answering "code or data" from shell text with sed and awk is not a
hard problem, it is the wrong problem.
It was also not portable. CI has been red at `sanitization` since round four,
and the log says why: under the image's busybox awk the octal escape in the
quote-stripping regex does not bite, every quoted span survives into the
skeleton, and the guard began refusing ordinary prose. Five allow-direction
fixtures failed in CI that pass under GNU awk. A control that reverses its
verdict with the awk on the host is not a control.
So the client detection is gone — the skeleton, the invoker list, the prefix
list, the option-value skipping, all of it. What remains asks two questions of
the text: is this a write, and does it name an endpoint a wrapper owns. It
cannot fail open by hiding the caller because it never looks for one, and it
now catches clients it was never taught: `python -c ... requests.post(...)` and
`wget --post-data` are both fixtures.
The cost is stated in the file and pinned in both directions: QUOTING one of
these calls on a Bash command line is refused as well. Ten fixtures that used
to assert "discussing a call is not making one" now assert the opposite, and
the boundary that stops this becoming block-everything is asserted just as
hard — a quoted READ, an endpoint named without a body flag, a quoted write to
an UNWRAPPED endpoint, and the wrapper's own body flag all still pass. The
18-command ordinary-work sweep blocks none.
The rule an agent can hold without a parser: do not put a raw write to a
wrapped forge endpoint on a Bash command line, not even inside quotes. Write
the example with a file-writing tool.
60/60 fixtures, verified inside the CI image (busybox) as well as locally.
Gates: sanitization, resident budget, test enumeration, tools-index (self-test
4/4, git suite 100%), issue-close, prettier.
Round-four review, three more absence-driven allows.
1. The guard read the command as TYPED. A backslash before a newline is removed
before anything else happens, so an endpoint token split across the join
(`.../iss\` + newline + `ues/1/comments`) executed the comments endpoint while
the literal token never appeared in the text. Continuations are now joined
before every check, because the joined form IS the command. This is the same
defect as the split-across-variables case, minus the excuse: there the token
genuinely does not exist until the shell expands it, here it was sitting in
the input the whole time and the guard chose the wrong reading of it.
2. Transparent prefixes take option VALUES. `sudo -u root curl` hid a live write
because `root` was a word the prefix list did not know. Enumerating option
grammars per prefix is the wrong game, so what is skipped is an option and at
most one value for it, plus a bare duration for `timeout` — never an
arbitrary word. `xargs echo curl ...` therefore stays ALLOWED, because there
the command is echo and the client is its argument.
3. `find -exec` runs the client. It opens command position the same way an
operator does, and now reads that way.
All seven reviewer repros are fixtures, each with its counter-case in the
allowed direction: a continuation inside a heredoc document stays a document,
`xargs echo curl` stays allowed, `sudo apt-get install curl` stays allowed, a
prefixed READ stays allowed. 48/48, and the 18-command ordinary sweep still
blocks none.
Gates: sanitization, resident budget, test enumeration, tools-index (self-test
4/4, git suite 100%), prettier.
Round-three review found two more absence-driven allows, both in the skeleton
introduced by round two, and fixing them exposed a third the reviewer had not
reached yet.
1. A word in front of a command does not displace the command. `env VAR=v curl`,
`command curl`, `timeout 10 curl` and `/usr/bin/curl` were all real writes at
execution position that a bare-name match could not see. The `env` form is the
one that matters: it is what an agent reaches for to keep a credential out of
the global environment, so the careful spelling was the invisible one.
2. Quoted data stops being data when a shell is about to execute it, and the
first version knew only `bash -c`, `sh <<` and `eval`. It did not know the
pipe, which is the form people actually use: `printf ... | sh`, `cat <<EOF | sh`,
`sh -s <<EOF` each made a live call vanish from the skeleton while still
running.
3. Found while testing the fix: that decision was made for the WHOLE command, so
a single unrelated `docker run ... sh -c 'echo hi'` promoted every other
quoted span on every other line to code. It blocked its own author for the
second time in a day. A shell on one line does not execute a string on another
line, and over-blocking is not the safe direction — a guard that blocks
ordinary work gets switched off, and a guard that is off permits everything.
The skeleton is now built per line, and a heredoc body is code only when the
line that opened it fed a shell.
All seven reviewer repros are pinned as fixtures, each with a counter-fixture in
the allowed direction: `echo timeout 10 curl ...` is not a call, a pipe to `wc`
is not execution, an unrelated shell on another line changes nothing. Fixtures
40/40, and a sweep of 18 ordinary commands blocks none of them.
Gates: sanitization, resident budget, test enumeration, tools-index (self-test
4/4, git suite 100%), prettier.
The position test added an hour ago blocked its own author. The message being
sent quoted one of the fixtures, so the quoted text contained an operator
followed by a client, and an operator inside a string is not an operator. That
is the reported over-blocking defect one level in, and it landed within an hour
of shipping the fix for the reported one — which is the argument for pinning
both directions as fixtures rather than reasoning about them.
Position is now judged against a SKELETON: the command with its data spans
(quoted strings, heredoc bodies) removed. Endpoint, URL and body detection keep
running against the full text, because real calls quote their URLs and a
skeleton would be blind to them.
The exception is what makes quotes data in the first place. If something is
about to EXECUTE the quoted text — `bash -c`, `sh <<EOF`, `eval` — the quotes
hold code, and the skeleton keeps them as command separators so the client
inside is still at command position, one interpreter down.
Three fixtures: an operator inside a quoted string, a heredoc body, and
`bash -c` making the same text code again. 30/30.
Round two of the same independent review. Three findings, all real, and the
first two share a root cause: the guard was reading command TEXT as though it
were a command.
1. Splitting the endpoint token itself defeats fragment matching outright —
`a=/api/v1/repos/o/r/iss; b=ues/1/comments` leaves no fragment contiguous.
Round one fixed one spelling of this and the reviewer produced the general
form immediately. It is not winnable by more fragments: the endpoint does
not exist until the shell expands it, and this hook runs first. So the guard
stops pretending to read it. A write whose URL contains an expansion, on a
visibly forge-shaped command, is now BLOCKED as unreadable — because "I
could not find an endpoint" must not mean "there is no endpoint". Opaque
URLs that are not forge-shaped (webhooks, artifact stores) still pass.
2. The broadened body detection false-blocked ordinary work: `grep -R "curl -d
https://.../issues" docs/`, `echo "curl -d ..." > note.txt`, printing an
example from python. Talking about a call is not making one, and this is the
direction that actually kills a control — an over-blocking hook gets turned
off, and an off hook permits everything. The client must now appear at
COMMAND POSITION: line start or after a shell operator, optionally behind
VAR=value. In every false positive it sat behind a quote instead. Quotes are
deliberately NOT stripped before matching; real calls quote their URLs.
3. `wt_precious()` aborted `cmd_rm` under `set -euo pipefail`: `grep -v` exits 1
when it filters everything out, which is exactly the disposable-only case,
so a SAFE worktree failed to remove with no message. Fixed, and the same
defect was latent one step upstream in `wt_dirty()`, where `head -200`
SIGPIPEs git on any worktree with 201 changed files. The cap is gone —
counting is cheap and the cap only ever truncated output that is no longer
printed.
Seven new fixtures pin all of it, in both directions. 27/27.
An independent reviewer broke all three new controls before they shipped.
Every finding is reproduced as a fixture or a repro, because the class is
recurring rather than incidental: each hole was a case where the answer was
"allow" because something was ABSENT rather than because it was CHECKED.
1. wrapper-guard read only the spellings it knew. `curl -d@body` (no space),
`--request=POST` (equals form), and a URL path assembled from shell
variables each carried a real provider write straight through. Write
detection now covers every body and method form curl accepts, and the
endpoint match no longer anchors on a literal host path that a variable
can dissolve.
2. wrapper-guard blocked only when the wrapper FILE existed. A host with a
broken or partial install therefore permitted exactly the raw writes the
guard exists to stop. Blocking is now on the endpoint; a missing wrapper
changes the remedy text, not the verdict — a broken install is not
permission to bypass gate 7.
3. mosaic-worktree read a worktree's safety from two questions, and a clean,
fully-pushed tree holding a gitignored `local.secret` answered both with
zero. `git worktree remove` then deleted the one copy in existence. A file
is gitignored precisely so nothing else holds it, so ignored-but-not-
disposable files are now a third evidence question. Build junk
(node_modules, .venv, dist, caches, *.pyc) stays disposable, so the common
case still reads SAFE.
4. check-tools-index counted a documented tool as discoverable at mode 0644.
Every caller tests `[ -x ]`, so a non-executable tool is a missing tool;
it now fails the gate with its own message.
Local gates green: sanitization, resident budget, test enumeration,
tools-index (4/4 self-test, 100% on the enforced git suite), and
wrapper-guard 20/20.
Two defects found by running the guard rather than reading it.
1. The guard resolved its sibling wrappers through a hardcoded
$HOME/.config/mosaic/tools/git. On a host with no installed mosaic home — a
CI container, a bare checkout — every wrapper lookup missed, `[ -x ]` failed,
and the guard fell through allowing the raw API write it exists to block. It
failed OPEN, silently, in exactly the environment least likely to notice.
It now resolves relative to its own path, so it names the wrappers from the
install it was launched from, with $HOME as the fallback.
2. There was no test. Adding one surfaced the guard's other sharp edge
immediately: it matches the literal text of the Bash command, so a harness
that embeds a blocked pattern inline trips the guard on itself rather than on
the fixture. That is the correct fail-closed posture and it is now recorded in
the test's own comments, because the next person will hit it too.
test-wrapper-guard.sh asserts twelve fixtures and asserts the ALLOWED cases as
hard as the blocked ones. A guard that over-blocks gets routed around and a guard
that under-blocks is decoration; only pinning both edges keeps it useful. It is
hermetic — no network, no credentials, no repository — so it joins the CI
sanitization step directly rather than the exclusions file.
An undocumented tool is, from inside an agent session, indistinguishable from a
tool that was never written. The framework shipped 26 git wrappers and named 6 of
them in its resident index docs — 23% discoverability, with pr-review.sh among the
missing. The observable consequence was an agent obeying Constitution gate 7 as
best it could see it, reaching for raw curl, sending GitHub's APPROVE to a Gitea
host, and getting HTTP 200 with the review silently filed PENDING. Three times.
That is not a discipline failure and no amount of prose fixes it.
Four changes, each converting a rule that decayed into a mechanism that cannot:
- check-tools-index.sh (new, CI-blocking): every tool in an enforced suite must be
named in a resident index doc, and every tool an index names must exist. The git
suite is enforced now; other suites report coverage without failing, so the
ratchet tightens one reviewed PR at a time instead of landing as one sweep. The
enforced list is framework-owned rather than a marker inside operator-owned
TOOLS.md — a doc marker would let an operator silence the gate on exactly the
host where it matters most. Carries --self-test, because a checker that only
ever passes is indistinguishable from one that is not running.
- TOOLS-REFERENCE.md: complete 28-entry git index, plus the APPROVED/APPROVE
dialect note that explains why pr-review.sh is not a formality.
- mosaic-worktree.sh + wrapper-guard.sh (upstreamed): the rule "big work goes on a
work filesystem" already existed in prose, and 255 GB accumulated in $HOME across
842 directories anyway, under five simultaneous placement conventions on one
host. The helper therefore exposes no placement decision — given a branch name,
every path is derived from `git worktree list --porcelain`. Worktrees rather than
clones because enumerability is the only thing that makes reclaim safe, and
reclaim is by evidence (clean tree + no unpushed commits), never by size or age.
The guard blocks three mechanically-detectable mistakes and nothing else:
a checkout into $HOME, a raw provider-API write to an endpoint that has a
wrapper, and the literal APPROVE event. Reads pass untouched.
- STANDARDS.md: model tiering as a standard, named by capability class so it
survives a model generation. Start cheapest, escalate on evidence, benchmark
before demoting a task class, and keep the class->model binding in operator
config with the DB-backed config service as the end state.
Registering the guard in runtime/claude/settings.json is the point of upstreaming
it: ~/.claude/settings.json is a framework-managed copy, so a hand-added hook there
is destroyed by the next upgrade. In the template it survives, and it reaches every
host instead of one.
issue-comment.sh and pr-review.sh verify a durable write by pinning the
provider-returned object URL's origin and full path. The origin included the
SCHEME verbatim. On a Gitea whose ROOT_URL is configured `http://` while every
client reaches it over `https://`, the provider returns `http://` object URLs,
so the comparison rejects the provider's own truthful answer about a write that
LANDED. The failure is deterministic, not intermittent: every comment, every
time, on such a deployment.
The scheme was never what the check defends. The forgeries it exists to catch —
look-alike host, decoy path prefix, wrong owner/repo/kind/number — all vary the
HOST or the PATH. Both stay strict. `http` and `https` now collapse to one
scheme class; any other scheme (file:, ftp:, javascript:) stays distinguishing,
and an EXPLICIT non-default port still distinguishes, because a different port
is a different service on the same host.
Consequences of the bug, both observed:
- The wrapper reports failure on a comment that is durably on the issue/PR, and
attributes it to #865 ("no durable comment created"). The write landed; the
citation is wrong. Reproduced here: the harness's persisted state contains the
record while the wrapper exits 1.
- pr-review.sh's comment path is worse. On a host where no seat can create a
review OBJECT, comment-form is the only gate-16 review record obtainable, and
this check refuses all of it.
Test gap this closes: every URL fixture in both harnesses was `https://`, and
every negative case varied only host or path. The one axis that fails in
production had zero coverage — the fixtures encoded the assumption that breaks.
Added, in both suites:
- scheme-downgrade (http vs https, otherwise correct) — must be ACCEPTED. Fails
against the unmodified wrappers, passes against the fixed ones; verified in
both directions, and the negative control's captured output is the #865
misattribution above.
- explicit non-default port (`:8443`) — must stay REJECTED.
- non-web scheme (`ftp://`) — must stay REJECTED.
Also fixes test-issue-comment-readback.sh hermeticity (#1007), without which the
suite cannot run on any seat that has a per-agent Gitea token: detect-platform's
step-0 identity lookup reads ~/.config/mosaic/gitea-tokens/<identity>, outside
both XDG_CONFIG_HOME and MOSAIC_CREDENTIALS_FILE, so the suite resolved a
PRODUCTION credential and died at HTTP 401 before case 1. Same two-part fix
already merged for test-pr-review-gitea-comment.sh in #1006: a sandboxed HOME
plus an empty REPO-LOCAL mosaic.gitIdentity to shadow the global. Note the
env-var route does NOT work — detect-platform.sh reads `${MOSAIC_GIT_IDENTITY:-}`
and `:-` treats set-but-empty identically to unset.
The owner-side half of #991 (setting the deployment's Gitea ROOT_URL to https)
is not in scope here and is not made unnecessary by this change; this makes the
wrappers correct against a deployment that returns either scheme.
My previous commit said four. It is five. `test-issue-comment-readback.sh` has
the same defect and is fixed the same way, and I had already looked straight at
it and filed it as an *unrelated* silent failure. Correcting that here rather
than folding it in quietly.
WHY IT WAS MISSED — the general lesson, not the excuse. `run_comment()` sends
the wrapper's stdout AND stderr to `$OUTPUT_FILE`, and the `EXIT` trap deletes
`$WORK_DIR`. The suite therefore exits 1 with ZERO bytes on stdout and stderr,
and the one line that says what went wrong —
Error: Gitea authenticated-identity read failed with HTTP 401
— lives only inside a directory that no longer exists when anyone looks. Every
oracle I had swept the family with greps for a SYMPTOM in surviving output, so
against this suite all of them returned "nothing found", which I read as "clean"
in the first sweep and as "unrelated pre-existing failure" in the second. A
suite that discards or deletes its own evidence converts a post-hoc assay into a
non-measurement, and I wrote that sentence into the previous commit while it was
already false about a file in the same directory.
HOW IT WAS ACTUALLY FOUND. Intercept the identity read at its SOURCE instead of
grepping for its consequence: a PATH shim over `git` that logs every
`mosaic.gitIdentity` read — args, rc, and resolved value — to a file OUTSIDE any
suite's work dir, then execs the real git. Deletion-proof by construction, and
it measures the defect's cause rather than one of its symptoms. Sweeping all 16
suites with it under an ordinary invocation:
resolves a REAL identity (`mos-dt-0`) before the fix:
test-issue-comment-readback 1 read rc=1 (RED on every seat)
test-pr-review-repo-host-override 6 reads rc=0
test-ci-queue-wait-branch-absent 3 reads rc=0
the four fixed in the previous commit now read empty; the rest never read at all.
The latter two are NOT affected and are deliberately left alone: under a seat
replica (identity set, no per-slot token) neither reaches `get_gitea_token`'s
fail-loud branch, and under a canary HOME neither carries the canary credential
into any surviving artifact. They read the identity and never enter a credential
path. That residual is structural and belongs to the wrapper half of #1007 —
scoping the read with `git -C "$repo"` removes it for everyone at once.
An earlier version of that sweep reported the four fixed suites as still
resolving a real identity. That was my grep, not the suites: `value=\[..*\]` is
satisfied by `value=[] args=[…]`, because `.*` runs past the empty pair and
matches the closing bracket of the NEXT one. `value=\[[^]]` is the correct test.
Recorded because the wrong pattern failed in the direction that would have sent
me re-fixing four already-correct files.
VERIFICATION of this suite, four HOME arms, all rc=0 with zero non-empty
identity reads and the pass line on stdout: real HOME, seat replica, canary
HOME, and an empty HOME with no identity at all. Full 16-suite sweep after the
change: every suite rc=0.
CONSEQUENCE FOR THE FINDING LIST IN THE PREVIOUS COMMIT: item 2 there — the
"silently red, unrelated to #1007" suite — is withdrawn. It was #1007 all along.
Item 1 (`pr-metadata.sh:89-92`, the anonymous fallback that reports an HTTP 200
carrying valid JSON as "unknown API error") stands and is still unfixed here.
Refs #1007
CENSUS CORRECTION: FOUR suites, not the three my own #1007 audit named. The
fourth (test-pr-metadata-gitea.sh) was outside the candidate set that audit
worked from and was found only by sweeping the discriminator across all 16
tools/git/test-*.sh suites. Recording that as a correction to my finding, not
as part of the original claim.
THE DEFECT. get_gitea_token() (detect-platform.sh:502-599) resolves a per-agent
identity at STEP 0, from `git config --get mosaic.gitIdentity`, BEFORE both the
Mosaic credential loader (step 1) and the GITEA_TOKEN env check (step 2). On a
provisioned agent seat that value is set GLOBALLY in ~/.gitconfig and is
inherited by any freshly-`git init`ed repo, so step 0 reads a REAL per-slot
token out of $HOME and returns it without ever consulting the suite's own
MOSAIC_CREDENTIALS_FILE / GITEA_TOKEN fixtures. The suites were running against
production credentials, and the fixture credential each one carefully
constructs was inert.
THE FIX: an empty repo-local `mosaic.gitIdentity`. An empty local value shadows
the global one and reads back empty at rc=0, so step 0 declines. The env route
does NOT work: detect-platform.sh reads "${MOSAIC_GIT_IDENTITY:-}", and `:-`
treats set-but-empty identically to unset.
OPERATIVE vs CONTAINMENT — the two mechanisms are not interchangeable and the
comment in each suite says so. The pin is operative: it prevents the resolution.
The sandboxed HOME each suite now also gets is containment: it bounds a failure
the pin should already have prevented. Conflating them is how this class stays
invisible, because a decoy HOME REMOVES the trigger (~/.gitconfig is where the
global identity lives), so any suite audited under one reads clean however
vulnerable it is. To MEASURE, replicate a seat: a decoy HOME whose .gitconfig
sets mosaic.gitIdentity with no per-slot token, so step 0 reaches its fail-loud
branch. That note is in each file for the next auditor.
SECOND, INDEPENDENT DEFECT in test-pr-metadata-gitea.sh. Applying the pin alone
turned that suite RED — and a control at baseline 826a8b3 under a plain HOME
reproduced the same failure, so it is pre-existing, not introduced. Its
`GITEA_TOKEN="stub-token"` / `GITEA_URL="https://git.example.test"` pair can
never satisfy step 2, because step 2 accepts GITEA_TOKEN only when GITEA_URL
matches the remote host and this repo's origin is git.uscllc.com. The suite had
therefore only ever passed by resolving a REAL credential — step 0 on a seat, or
step 1 from the operator's own credentials.json. A MOSAIC_CREDENTIALS_FILE
fixture is added rather than leaning on the sandboxed HOME making step 1 find
nothing: a test that passes because production configuration is ABSENT fails the
moment it is present. Shipping the pin without this would have moved the failure
rather than removed it.
NO CI ARM. .woodpecker/ci.yml does not run these suites; packages/mosaic/
package.json:28 (test:framework-shell) runs an ENUMERATED list that excludes all
four. They run only by hand — i.e. exclusively on a provisioned seat, the one
environment where the defect is live. "Passes in CI, fails on a seat" does not
apply here; there is no CI observation at all.
VERIFICATION (seat replica = decoy HOME with mosaic.gitIdentity set, no per-slot
token; canary = same plus a marked non-credential at both per-slot paths; plain
= empty HOME; real = ordinary invocation):
- bash -n clean on all four.
- Sweep of all 16 suites at baseline 826a8b3 under the seat replica:
test-gitea-login-resolution rc=1 REACHES-STEP0; test-issue-create-
interactive-auth rc=1 REACHES-STEP0; test-pr-merge-gitea-empty-uid rc=1
REACHES-STEP0; test-pr-metadata-gitea rc=1 REACHES-STEP0.
- Same sweep after: every row rc=0 with step0 absent.
- test-gitea-token-identity flags REACHES-STEP0 in BOTH arms and is NOT a
defect: it runs under `env -i HOME="$FAKE_HOME"` (line 77) and its hit is
its own deliberate assert_failloud fixtures (lines 158-171). The fail-loud
grep matches the intended behaviour as well as the defect, so it needs the
second discriminator; recorded here so the next sweep does not re-file it.
- Durable-argv assay (a PATH shim that tees argv out of each suite's own mock
curl, because test-pr-merge-gitea-empty-uid truncates its log between phases
and its EXIT trap removes the sandbox — a post-hoc read of that suite is a
non-measurement, and "no trace" there is not a clearance):
test-pr-merge-gitea-empty-uid before: canary token in argv, fixture never
used. after: fixture token in argv, canary absent. 5 curl calls both arms.
test-pr-metadata-gitea before: canary in argv. after: both calls
carry the fixture token against git.uscllc.com.
- test-pr-metadata-gitea across seat/canary/plain HOMEs after the fix: rc=0,
rc=0, rc=0.
- All four under the real HOME: rc=0. No regression to ordinary invocation.
The comment block is duplicated across the four files rather than pointing at a
shared note. Deliberate, and matching the merged #1006 precedent
(test-pr-review-gitea-comment.sh:87-95): the reader who needs it is auditing one
file.
TWO FINDINGS DELIBERATELY NOT FIXED HERE (out of this branch's scope, to be
filed):
1. pr-metadata.sh:89-92 — the anonymous curl fallback does not check ^2, so an
HTTP 200 carrying valid JSON is reported as "unknown API error" at rc=1.
2. test-issue-comment-readback.sh exits 1 with ZERO bytes on stdout AND
stderr, dying at its first seed_state python3 heredoc. Reproduces at
baseline 826a8b3 under both a seat replica and the real HOME. Silently red
at main for everyone; unrelated to #1007.
Refs #1007
Hermes Agent
jason.woltjeandMos