feat/workspace-hygiene-tool-enforcement
421
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
c09392e0c4 |
fix(guard): classify Git option grammar
ci/woodpecker/pr/ci Pipeline was successful
Replace finite clone/worktree boolean allowlists with the closed separate-value grammar, including Git's accepted long abbreviations and bundled short options. Keep placement operands distinct from HOME-valued sources, metadata, commit-ish values, and rule-generated --no-* flags while preserving separate-git-dir and later-command traps. Canonicalize shell-known HOME spellings, dot aliases, and existing symlink parents before placement comparison. Expand the hermetic suite from 242 to 292 fixtures and document the requirements and review evidence. Deliberate residuals: a future unclassified value-taking clone placement option can fail open, and a future worktree value option can shift the inferred path; defaulting it to flag grammar avoids present-day over-blocking of Git's non-enumerable boolean family. PreToolUse symlink canonicalization is non-atomic against replacement after inspection; architectural closure is tracked by #1199. |
||
|
|
91cc37bcf6 |
fix(guard): inspect checkout placement operands
ci/woodpecker/pr/ci Pipeline was successful
Classify git clone and worktree add operands so HOME-valued environment assignments, sources, references, templates, and metadata do not impersonate checkout destinations. Preserve both forms of clone --separate-git-dir as real placement targets and distinguish shell words, command boundaries, and redirections in the existing quote-aware normalized stream. Deliberate fail-closed residual: unknown future Git options with a separate following word are not adjudicated as source-only. Their value remains a possible placement, so a HOME-shaped value blocks rather than silently creating a bypass. Relative destinations whose effective path depends on cwd remain out of scope in #1197. |
||
|
|
20d86e392b |
fix(guard): end the home match at a shell word boundary, not at whitespace
ci/woodpecker/pr/ci Pipeline was successful
Rounds 8 and 9 of the same class, in the two halves of one line. The path arm required the home token to be followed by `/`. That silently made `$HOME` itself -- the exact target the rule names -- legal: `git worktree add $HOME` cleared a guard whose message is "this checks a repository out under $HOME". Reachability is not theoretical; the command succeeds against an empty home directory. Trailing `/` was then admitted, and with it every terminator that is not whitespace: `$HOME;`, `$HOME&&`, `$HOME|`, `$HOME&` and end-of-string all cleared, 25 shapes in all. The fix that did not happen is worth recording, because it was mine. The brief for this round prescribed a closed continuation class, `([^A-Za-z0-9_.-]|$)`, on the reasoning that terminator sets are open and continuation sets are closed. That is true of some axes and false of this one: `+ @ , : = %` all continue a FILENAME, so `$HOME+bak/wt` and five siblings like it would have been refused -- a new over-block traded for a closed bypass, which is not a trade. The implementer measured the six counterexamples and declined the brief rather than pick between two acceptance conditions that cannot both hold. They are now permanent fixtures; a rejected over-block that nothing pins comes back. The axis that IS closed is word termination, and it is closed by specification rather than by anyone's imagination: POSIX fixes the unquoted metacharacter set at space, tab, newline, and | & ; ( ) < >. So the path normalizer marks those as an internal word boundary, in the same state machine and by the same mechanism as the existing literal-dollar and literal-tilde markers, which is what lets a QUOTED or escaped metacharacter stay word content: `"$HOME;bak"` is one word and must be allowed. A raw marker byte arriving in the input is encoded first, so input cannot forge or suppress a boundary. The home token must now be preceded by start, `=`, or a boundary, and followed by a boundary, `/` for a descendant, or end. Verified by oracle rather than against the brief -- `bash -c "printf '%s' WORD"` performs expansion and quote removal without executing, so the expected verdict comes from the shell instead of from the reading that has now been wrong once. Fixtures 198 -> 230; the new ones are red at both prior heads (15 failing at |
||
|
|
4b8eba95a3 |
fix(guard): apply word normalization to the path arm, not just the name arm
The quote/escape handling added over rounds 1-6 was wired into the command
NAME reading only. The PATH reading one line below it still matched the raw
text, so every spelling the name arm had learned to see was invisible to the
checkout check: `"$HOME"/wt`, `${HOME}/wt`, `"${HOME}"/wt` and a quoted
literal home path all cleared a guard whose entire purpose is to refuse them.
An identifier is not one spelling, and this file has now proven that seven
times; the arms of the rule were audited one at a time, and the class survived
in the arm nobody looked at.
The two readings need the same quote and escape handling but differ in one
respect, so this is one state machine with two modes rather than a copy:
substitution flattening is correct for a name and wrong for a path, where an
expansion-capable `$HOME` must stay visible. In path mode a shell-LITERAL
dollar or tilde -- single-quoted, escaped, or a quoted tilde -- becomes an
internal nonmatching marker, so quote removal cannot manufacture a home
spelling the shell would never expand, and `"~/wt"` is no longer refused.
`home_re` treats the braces as the pair they are. `$HOME}` expands HOME and
appends a literal brace; `${HOME` is not an expansion at all. Admitting either
as `${HOME}` would invent a home path the shell never resolves.
Verified by oracle rather than by assertion: for each spelling, `printf` under
bash performs expansion and quote removal without executing, and the resulting
path decides the expected verdict. 24 spellings, 0 mismatched here; 6
mismatched at
|
||
|
|
3d0a882a63 |
wrapper-guard: model quote removal and escaping as separate operations
ci/woodpecker/pr/ci Pipeline was successful
Round six of the same class: a program NAME is not one SPELLING. Two findings, and the second is one this change's own predecessor introduced. A FOURTH name consumer never went through the shared site. Checkout detection still recognized git by a raw whole-command regex, so `g"it" clone`, `g'it' clone` and `g\it clone` into $HOME were all allowed. `/usr/bin/git` blocked only because the raw text still happened to contain contiguous `git` — the same passing presentation that established nothing during the curl rounds. It now uses CMD_NAMES and NAME_PREFIX like the other three, so all four consumers share one definition of what a name looks like. Routing it through NAME_PREFIX also repaired an over-block the arm had carried from the start: the old regex found `git` INSIDE a longer word, so `mygit clone` and `gitfoo clone` were refused at every previous head. That is the mycurl and curl-wrapper class, and refusing it is how a guard gets routed around instead of repaired. The normalization itself was creating names the shell never runs. It deleted every backslash regardless of quote context, but a backslash inside single quotes is literal, so `'cu\rl' --config` names a program called cu\rl and was refused. The same holds inside double quotes before any character other than $, `, " or backslash. Both were false positives, and both were regressions — the pre-PR head allowed them. Quote removal and escape handling are different operations that were sharing one context-blind deletion pass. They are now a small state machine that follows the actual rule: outside quotes a backslash escapes the next character; inside single quotes everything is literal; inside double quotes a backslash is special only before $, `, " or backslash. Quote characters drop without splitting the word, and the substitution flattening that makes `$(which curl)` resolve to a bare name is unchanged. An over-block is not the safe direction. A guard that refuses legitimate work gets routed around rather than fixed, which is the same outcome as a bypass and arrives faster. Unchanged and still disclosed: names absent from the literal text — assembled from braces or variables — remain invisible to text matching, and `$((curl))` is over-matched at every head including the pre-PR one. Fixtures: 173 -> 184. Every one added here discriminates against the previous head |
||
|
|
1c3e79a9ed |
wrapper-guard tests: cover the shapes I had only reasoned about
ci/woodpecker/pr/ci Pipeline was successful
Test-only. No change to the guard; every case below already behaves correctly at |
||
|
|
46f52eedfe |
wrapper-guard: remove quotes instead of splitting on them; last name consumer
ci/woodpecker/pr/ci Pipeline was canceled
Round-five remediation of both blockers gate-ultron-01 raised on |
||
|
|
df83a9eec2 |
wrapper-guard: recognize program names after quote removal, in one place
ci/woodpecker/pr/ci Pipeline was successful
Round-four remediation of both blockers gate-ultron-01 raised on |
||
|
|
d99ff57e14 |
wrapper-guard: match curl by basename, not by bare word
ci/woodpecker/pr/ci Pipeline was successful
Round-three remediation of the single blocker gate-ultron-01 raised on |
||
|
|
06046f7675 |
wrapper-guard: close three fail-opens the last round left, and enumerate the large-repo test
ci/woodpecker/pr/ci Pipeline was successful
Round-two remediation of the four blockers gate-ultron-01 raised on |
||
|
|
f8d04d1bf4 |
wrapper-guard: decide allows on the shell's reading, not on the whole command text
ci/woodpecker/pr/ci Pipeline failed
Four blockers from adversarial review, and three are one defect wearing three hats: a test over the WHOLE command text deciding an ALLOW. That is the fail-open shape this file keeps rediscovering, and it had reached the break-glass itself. - Break-glass read POSITIONALLY. `case $CMD in *MOSAIC_WRAPPER_OVERRIDE=1*)` cleared the entire command if that string appeared anywhere in it, so quoting the override in a note, naming a variable after it, or writing =10 disabled the guard for the call sitting beside it. Now only leading NAME=value assignments count, exactly where the shell would honour one. Cost, pinned as a fixture: an override after `&&` no longer arms. - $HOME resolved once, and unset / empty / "/" refused. This was filed as a checkout-arm defect and is larger: under `set -u` the old file died at line 62 on EVERY command with HOME unset, exit 1, before the API arms or the APPROVE trap ran. A seat with no HOME (systemd unit, container, env -i) had no guard at all. A checkout whose question cannot be asked now blocks; the blast radius is asserted to be that one command shape and not the session. - Subresource refinement inverted. It asked whether a subresource appears anywhere in the command, so `gh api -X PATCH .../issues/1 -f body=cf-/pulls/2/files` was cleared on the strength of text in its own body. It now clears only when EVERY numbered-object occurrence carries a subresource. - -K/--config refused. curl reads the method, body, headers and URL from that file, so none of them are in the command: every write test read 0 and the call went through. An unreadable request is not a cleared one. mosaic-worktree: never close a git pipe early resolve_repo took the first porcelain line with `awk ... exit`, which closes the read end while git is still writing. git takes SIGPIPE, pipefail returns 141, and the function aborts SILENTLY — no message, no path, exit 141. It fires as a function of REPO SIZE: fine on three worktrees, reliable on seventy. Measured at 73 worktrees (10 KB of porcelain): rc=141, no output. The file already removed a `head -200` for this exact reason; the rule is now uniform. Evidence — every new case run against |
||
|
|
029af418f0 |
wrapper-guard: name the marker every forge API shares, not one dialect's
ci/woodpecker/pr/ci Pipeline was successful
The scope gate asked for a scheme URL, a `/api/v[0-9]` path, or a provider-CLI
`api` subcommand. `/api/v[0-9]` is Gitea's spelling. GitHub's API carries no
version segment at all -- `api.github.com/repos/a/b/issues` -- so the schemeless
Gitea write was in scope and the schemeless GitHub one was not:
curl -X POST -d x api.github.com/repos/a/b/issues rc 0
curl -X POST -d x api.github.com/repos/a/b/issues/1/comments rc 0
host=api.github.com; curl -X POST -d x ${host}/repos/a/b/issues rc 0
A gate calibrated to one provider's spelling rather than to what identifies a
provider API. `/repos/` is the marker both dialects share -- every forge API
addresses a repository through it -- so the gate now names both.
This is SPAN a third time, and the third layer it has appeared on. Round 8: a
block message named a wrapper that could not make the call. Round 9: a map
claimed a span its wrapper did not cover. Round 10: a control claimed a surface
it did not measure. Here the scope gate itself claimed a class of API and
recognised one member of it. Same question each time -- does this thing SPAN
what it claims -- and it has now been the answer four rounds running, which is
the argument for asking it of every arm rather than of the reported one.
Widening a scope gate can only make the guard stricter. Downstream a block still
requires a body flag AND either a mapped endpoint or an unreadable one, so this
widens what is CONSIDERED, not what is refused. The three fixtures asserting that
-- a schemeless GitHub read, an unwrapped GitHub endpoint, and a read past `--`
-- exist to hold that claim to account rather than state it.
Also: the provider-CLI arm's option scanner required a letter after the dashes,
so `gh api -X POST -- ${p}${q} -f title=x` walked its endpoint straight past.
The end-of-options marker is the one option not spelled like one, and a scanner
that skips options had to be told so.
Because the new endpoints are READABLE, each blocked fixture asserts the wrapper
its message must name. A rc-only fixture here would have passed on the unreadable
arm and proved nothing -- which is the failure mode this suite caught in itself
last round.
Evidence: 110/110 fixtures (was 101) locally and in ci-base. Negative-controlled
per change, not in aggregate: dropping `/repos/` from the gate fails exactly the
five GitHub fixtures; dropping the `--` alternative fails exactly one; neither
disturbs a pre-existing fixture. 18-command sweep unchanged (same three round-6
flips, nothing new); 12-command ordinary-work sweep 0 blocked. shellcheck clean
at warning+. CI 2374 on the previous head was green across all nine steps.
Residual, stated rather than implied: a caller who splits `/repos/` itself in a
schemeless GitHub URL leaves no literal marker anywhere and is out of scope --
the same boundary as splitting the hostname, and no longer a mistake anyone
makes by accident.
Round 11, addressing rev0 review 154.
|
||
|
|
51746f44eb |
wrapper-guard: fail closed on an unreadable endpoint in every shape the scope gate admits
ci/woodpecker/pr/ci Pipeline was successful
Round-9 review found the fail-closed rule narrower than the block it guards.
The scope gate admits three shapes — a scheme URL, a schemeless /api/vN path,
and a provider CLI's `api` subcommand — but the unreadable-endpoint test asked
only for https?://. So a split endpoint token in the other two shapes was in
scope to be blocked, produced no readable endpoint, and fell through to ALLOW,
while the identical split behind a literal scheme blocked.
Four writes reaching the provider unexamined, one of them a review verdict:
p=repos/a/b/iss; q=ues; gh api -X POST ${p}${q} -f title=x
p=repos/a/b/issues/1/comm; q=ents; gh api -X POST ${p}${q} -f body=x
p=repos/a/b/pulls/1/rev; q=iews; gh api -X POST ${p}${q} -f event=APPROVED
p=/api/v1/repos/a/b/iss; q=ues; curl -X POST -d x host${p}${q}
This is the same defect class as the milestone arm one round earlier, one layer
up: there the map claimed a span its wrapper did not cover, here a control
claimed a surface it did not measure. A control is only as wide as its narrowest
arm, and widening the scope gate without widening the fail-closed rule left the
gap exactly where the gate had just been extended.
Three arms now, one per admitted shape: a scheme URL token carrying an
expansion; a schemeless token carrying both a forge fragment and an expansion,
in either order; and the endpoint argument of a provider-CLI api call, read
positionally. Limits are stated in the source rather than implied — a caller who
splits the hostname as well, and an endpoint pushed past an option whose value
contains whitespace, are both outside what this measures.
An expansion in a BODY is explicitly not unreadable. Passing a payload in a
variable is the safe practice and leaves the endpoint fully legible; blocking it
would have been a control punishing the behaviour it wants.
101/101 fixtures, up from 92. Each arm is negative-controlled separately:
removing the schemeless arm fails exactly the two schemeless fixtures, removing
the provider-CLI arm fails exactly the four CLI fixtures, and neither disturbs
any pre-existing fixture. One added fixture was rewritten after it passed for
the wrong reason — its endpoint was readable, so it blocked on the endpoint map
and never exercised the arm it was written for.
Evidence: 101/101 locally and in ci-base; shellcheck clean at warning+; a
12-command sweep of ordinary forge work — reads with split endpoints, bodies in
variables, unwrapped endpoints, an artifact PUT — blocks none of them; the
18-command sweep still blocks the same three round-six flips and nothing new.
|
||
|
|
1bfd0ddd71 |
wrapper-guard: an arm may only claim the span its wrapper actually covers
ci/woodpecker/pr/ci Pipeline was successful
Round eight replaced an absence-driven allow with an endpoint inventory, and
review found the inventory answered the wrong question. It recorded which
wrapper TOUCHES an endpoint, when the sound question is whether the wrapper
SPANS it. A PATCH to a numbered milestone blocked with "use milestone-close.sh".
That wrapper takes only -t <title> and sends state=closed on both the gh and tea
paths, so it cannot express a title, description or due-date edit. The block was
correct and the advice was not — the same remediation-accuracy defect as the
round-seven subresource arm, one step quieter, because a wrapper was rounded up
from owning a slice to owning the endpoint.
The treatment already existed two arms away: /pulls/{n} named pr-close.sh for
state and said in the message that a PR title/body edit is a real wrapper gap.
So this was a consistency failure rather than a missing idea, which is why the
fix is not just the reported arm. Auditing every arm for span against the flags
each script accepts found a second bad one that review had not reached:
/pulls/{n}/requested_reviewers was mapped to pr-review.sh, and pr-review.sh
takes -a <action> -c <comment> and files a verdict. Nothing in this tree adds a
requested reviewer, so that arm was advertising a wrapper that cannot make the
call. It is unowned and now flows through, like a comment edit.
Changes:
- /milestones/{n} keeps blocking, and the message states that milestone-close.sh
owns the close only while title/description/due-date is a wrapper gap.
- /pulls/{n}/requested_reviewers becomes residue, above the reviews arm so it
cannot be refused with "use pr-review.sh".
- /issues/{n} now also names issue-assign.sh, which owns the assignee field;
issue-edit.sh has no assignee flag, so the old advice was short by one wrapper
for a PATCH that sets one.
- The map comment carries a span column, so a future arm has to state what its
wrapper covers rather than imply all of it.
Fixtures assert the span language, not just the wrapper name: the milestone edit
must say it owns the close only, and a PATCH setting an assignee must name
issue-assign.sh. Negative-controlled — reverting each of the three behaviours
fails that fixture and only that fixture.
92/92 (was 89), locally and in ci-base. shellcheck clean at warning+. The
18-command ordinary sweep blocks the same three round-six flips and nothing new.
Gates: fixtures 92/92 in ci-base, shellcheck clean at warning+.
|
||
|
|
a3cacac7fb |
wrapper-guard: make subresource ownership an inventory, and check the advice
ci/woodpecker/pr/ci Pipeline was canceled
Round seven fixed "wrong wrapper advice" by letting every path under a numbered issue or PR flow through, on the stated reasoning that no wrapper owned any of them. Review checked that reasoning against the directory and it was false: gh api -X PATCH repos/a/b/issues/1 -f title=x curl -X PATCH -d @b https://host/api/v1/repos/a/b/issues/1 gh api -X PATCH repos/a/b/issues/1/labels -f labels[]=bug gh api -X POST repos/a/b/issues/1/assignees -f assignees[]=u issue-edit.sh takes --title/--body/--labels/--milestone and issue-assign.sh takes assignee/labels/milestone, so all four are wrapped calls and all four returned 0. The guard answered "allow" because wrapper ownership had been ASSUMED absent rather than looked up — the same absence-driven allow this file exists to remove, committed inside the fix for it. I withdraw the round-seven departure: the reviewer's position was right on the evidence, and my argument for it was sound reasoning applied to a fact I never checked. The endpoint map is now an inventory read off tools/git/*.sh and their flags: assignees to issue-assign.sh, labels to issue-edit.sh (naming issue-assign.sh alongside it, since both set them), a numbered issue to issue-edit.sh (naming issue-close.sh/issue-reopen.sh for state), a numbered PR to pr-close.sh (with the PR title/body gap stated in the message rather than papered over), and /milestones/{n} to milestone-close.sh instead of the create wrapper. The residue is defined by SUBTRACTION, not by listing provider API surface: everything a wrapper owns is consumed by an arm above, so a numbered path that reaches the end is owned by nothing and still flows through — times, stopwatch, reactions, a comment edit at /issues/comments/{id}. A list would rot the moment a provider adds an endpoint, and rot in the blocking direction with wrong advice. That residue test is a regex, deliberately. `case` globs cannot express a path SEGMENT, so the natural allow arm *"/issues/"[0-9]*"/"* clears gh api -X PATCH repos/a/b/issues/1 -f body="see /docs" on the strength of a slash inside the body. An allow decided by a glob over the whole command is the fail-open shape again; the regex pins the segment to the number, and that command is pinned as a fixture. Also: `-f labels[]=bug` was not read as a body at all, because the key class stopped at the bracket. The array spelling is what the provider CLIs use for repeated fields, so an implicit POST carrying only array fields was invisible. And the reason six rounds of this were invisible: the harness read the exit code and nothing else, so a block naming the WRONG wrapper passed every run. Fixtures may now state the wrapper the message must name, and the wrapped ones do. The assertion was negative-controlled — pointing one fixture at the wrong wrapper fails that fixture and only that fixture. 89/89 (was 79), locally and in ci-base. All eight sanitization commands green in-image. The 18-command ordinary sweep blocks the same three round-six flips and nothing new, so the tighter map cost nothing on ordinary work. Gates: sanitization (all eight green in ci-base), shellcheck clean at warning+. |
||
|
|
b4578dcd0a |
wrapper-guard: close the absence shape at the new boundary; stop giving wrong advice
ci/woodpecker/pr/ci Pipeline was successful
Round six deleted the code/data parser and scoped what remained on `https?://`.
Review found the failure class had not been eliminated, only relocated: a raw
provider CLI carries no scheme, so the scope gate answered "not my business"
because the URL was ABSENT — the same shape, at the new boundary.
gh api -X POST repos/a/b/issues -f title=x -f body=y
gh api -X POST repos/a/b/pulls/1/reviews -f event=APPROVE
tea api -X POST repos/a/b/issues/1/comments -f body=x
curl -X POST -d x git.example.invalid/api/v1/repos/a/b/issues
All four were real writes to endpoints a wrapper owns, and all four passed.
Constitution gate 7 names raw provider CLIs explicitly, so they are in scope
rather than something to narrow the docs around. The scope gate now also
triggers on `/api/v{n}` and on the `api` subcommand of the provider CLIs, and
`-f key=value` joins curl's `-d` as an implicit POST. Adding triggers to a scope
gate can only make it stricter — it cannot open a new hole — which is why this
is a list of shapes rather than a model of any one caller.
The boundary is stated in the file rather than left to be discovered: provider
PORCELAIN (`tea pulls create`) is NOT covered, because catching it means
modelling every CLI's verb grammar, which is the parser mistake wearing a new
costume. That is a wrapper-and-review gap, not a thing this hook can hold.
Second finding, and the one I had flagged as my own worry: the endpoint `case`
was prefix-greedy, so `/issues/1/labels` blocked with "use issue-create.sh" —
the wrong wrapper for that call. A block an agent cannot comply with is worse
than no block, because it teaches that the hook is broken and the override is
routine, and an override that is routine is a guard that is off. Issue and PR
subresources now flow through, exactly as /releases and every other endpoint no
wrapper owns already does. This hook enforces "use the wrapper"; where there is
no wrapper it has nothing to enforce, and the gap belongs in the wrapper set.
I am departing from the review on that one deliberately: the review held that
blocking is correct there and only the remediation wrong. Naming a wrapper gap
in a refusal keeps gate-7 pressure, but it makes the override the normal path
for every labels and assignees call, which spends the override's meaning on the
cases where it is least needed.
Also: the APPROVE trap now catches the provider-CLI spelling `-f event=APPROVE`
alongside the JSON body, and still never matches the correct value APPROVED.
79/79 fixtures, locally and inside the CI image, with both blockers pinned in
both directions — the four repros block and name the right wrapper, while a
provider-CLI read, an unwrapped endpoint reached through one, porcelain, and the
`rm -f`/`grep -f` collisions all still pass. The 18-command ordinary sweep
blocks the same three round-six flips and nothing new, so the broader gate cost
nothing on ordinary work. Second over-block documented rather than found: prose
carrying `.post(` near a wrapped URL is refused, which follows from judging the
payload and is now stated next to the quoted-curl cost.
Gates: sanitization (all eight commands green in ci-base), shellcheck clean.
|
||
|
|
b1254f52f3 |
wrapper-guard: judge the payload, not the caller — delete the code/data parser
ci/woodpecker/pr/ci Pipeline was canceled
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. |
||
|
|
2a2a87251a |
wrapper-guard: read the command the shell will run, and stop losing the client behind option values
ci/woodpecker/pr/ci Pipeline failed
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. |
||
|
|
e6a881a795 |
wrapper-guard: judge command position on prefixes and on what a shell will execute
ci/woodpecker/pr/ci Pipeline failed
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. |
||
|
|
7962e4302f |
guard: judge command position on the code, not on the text
ci/woodpecker/pr/ci Pipeline was canceled
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. |
||
|
|
8a901cc19a |
guard: read command position, refuse unreadable URLs, survive pipefail
ci/woodpecker/pr/ci Pipeline was canceled
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. |
||
|
|
8b7ac5b51e |
guard: close four fail-open holes found by independent review
ci/woodpecker/pr/ci Pipeline was canceled
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. |
||
|
|
96609bdade |
framework: prove the wrapper guard both ways, and resolve its wrappers relatively
ci/woodpecker/pr/ci Pipeline failed
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. |
||
|
|
e3a0ee87b3 |
framework: make tool discoverability, workspace placement and model tiering mechanical
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. |
||
|
|
b590a5c3d8 |
fix(git): accept http/https as one scheme class in comment URL verification (#991)
ci/woodpecker/pr/ci Pipeline was successful
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. |
||
|
|
540ec5b6ef | Merge pull request 'fix(git): #1007 suite hermeticity — pin repo-local mosaic.gitIdentity in five test suites' (#1024) from fix/1007-suite-hermeticity into main | ||
|
|
563d1ac053 | Merge pull request 'fix(shell): remove wake validation pipe hazards' (#1107) from fix/1099-pipefail-wake into main | ||
|
|
722163671f | feat(pi): add persistent Mosaic /goal controller (#1152) | ||
|
|
f158be8003 |
fix(shell): remove wake validation pipe hazards
ci/woodpecker/pr/ci Pipeline was successful
|
||
|
|
b0f7d26dd9 |
fix(shell): remove test harness pipe hazards (#1106)
Co-authored-by: f10-coder <[email protected]> |
||
|
|
3a1203b2f8 |
fix(shell): remove runtime early-exit pipe hazards (#1105)
Co-authored-by: f10-coder <[email protected]> |
||
|
|
df4c591ab4 | fix(fleet): make framework shell assertions SIGPIPE-safe (#1100) | ||
|
|
4fa2768962 |
fix(fleet): propagate roster git identity (#1073)
Co-authored-by: be-coder-06 <[email protected]> |
||
|
|
aa0a7b5fa2 | fix(tools/git): issue-close.sh silently dropped the closing comment (#1085) | ||
|
|
f744f32214 |
feat(tools/git): explain tea's misleading user does not exist error (stale token, not a missing account) (#1086)
|
||
|
|
8ff7aac0ca | fix(tools/git): detect-platform died silently outside a repo, taking every wrapper with it (#1089) | ||
|
|
80a45b1e1c |
feat(pr-merge): preserve linked authors in squash messages (#1066)
Co-authored-by: be-coder-08 <[email protected]> |
||
|
|
85d2108e4e |
fix(ci): remove upgrade rollback signal race (#1060)
Co-authored-by: be-coder-08 <[email protected]> |
||
|
|
16f91157a1 |
test(ci): make queue guard harness deterministic (#1062)
Co-authored-by: be-coder-08 <[email protected]> |
||
|
|
5916aeefd6 |
chore(release): @mosaicstack/mosaic 0.0.49 — ship RM-03 guard fix to release channel (#1036)
Co-authored-by: coder-mos1 <[email protected]> |
||
|
|
58b971aba3 |
fix(rm-03): make CI queue guard fail on asserted non-readiness (#1032)
Co-authored-by: coder-mos1 <[email protected]> |
||
|
|
f4fd5967fc |
RM-61: prove ci-postgres teardown discrimination (#1033)
Co-authored-by: coder-mos1 <[email protected]> |
||
|
|
06e0d40352 |
feat(quality): CI test-membership guard — enumeration can no longer silently under-run the disk (#1017) (#1018)
Co-authored-by: mos-dt-0 <[email protected]> |
||
|
|
166ee8c90f | fix(wake): close fd 9 in the detector's sleep child so a dead detector's lock dies with it (#993) | ||
|
|
2fa6bcd576 |
fix(git): #1007 — test-issue-comment-readback is a FIFTH affected suite (second census correction)
ci/woodpecker/pr/ci Pipeline was successful
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
|
||
|
|
1afe2b36dc |
fix(git): #1007 suite hermeticity — pin repo-local mosaic.gitIdentity in four test suites
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 |
||
|
|
826a8b3b26 |
fix(git): pr-review.sh — surface the provider's stated reason, drop the hardcoded #865 attribution (#1006)
Co-authored-by: mos-dt-0 <[email protected]> |
||
|
|
a4280b9c98 |
fix(wake): #984 fatal source guard + #985 absorb re-scan — #973 follow-up batch (#1001)
Co-authored-by: mos-dt-0 <[email protected]> |
||
|
|
4fb44f6345 |
fix(wake): three-valued grep verdicts — has_match/count_lines across all ten suites (closes #973) (#983)
|
||
|
|
089615f63b | feat(git): push-guard — refuse verifications satisfied by the null case (closes #975) (#974) |