rev-974 (#1085 review 130) found three blockers. This closes the first two.
[1] SANDBOX ESCAPE. The test ran under `set -uo pipefail` with unchecked mkdir,
calls-log redirect and `cd "$REPO_DIR"`, then prepended a possibly-nonexistent
$MOCK_BIN to PATH -- while `git remote add origin` names the REAL repository.
rev-974 forced setup failure with an unwritable AGENT_WORK_ROOT and the test
continued past every error, ran `git init` in its CALLER's directory, added the
real origin, and invoked its target:
TARGET_REACHED args=-i 42 -c closing note
With the committed target that is the real, provider-mutating issue-close.sh.
ShellCheck flagged the unguarded cd as SC2164 independently.
Now: `set -euo pipefail`, every setup step checked with a legible reason, and
assert_mocked() proves BOTH `tea` and `curl` resolve inside $MOCK_BIN before any
target invocation. Control: unwritable AGENT_WORK_ROOT -> rc=1 at mkdir, target
never reached.
[2] THE API/no-login BRANCH HAD NO DISCRIMINATING COVERAGE. The mock always returned
a tea login, so `gitea_issue_comment_api || fail-closed` was never executed.
rev-974 replaced the whole fallback contract with an unconditional close --
silently dropping the comment -- and the committed test still passed rc=0.
Added three cases asserting the POSTCONDITION (which HTTP calls happened, in what
order) rather than that a command ran: comment POST fails -> no PATCH and non-zero;
comment succeeds -> strictly POST,PATCH; no comment requested -> PATCH only, never
a POST. The curl mock now records method and URL. Control: replaying rev-974's
contract destruction now fails with "API path: no comment POST attempted".
Two self-inflicted traps hit while adding `set -e`, both the same family as the #1086
defect be-coder-08 found, and both silent:
- `grep -q X "$CALLS" && fail "..."` -- the ABSENT case (grep rc=1, the PASSING case
for a must-not-appear assertion) is the last command of an && list and terminates
the script with no message. All four converted to if-blocks.
- `run_target ...; rc=$?` -- the function's non-zero RETURN trips set -e in the CALLER
before rc is read; run_target's internal `set +e` protects the target, not the
caller. All five call sites now `rc=0; run_target ... || rc=$?`.
Controls:
unchanged main -> rc=1 "used 'tea issue comment'"
fallback contract destroyed -> rc=1 "API path: no comment POST attempted"
unwritable AGENT_WORK_ROOT -> rc=1 at setup, target never invoked
fixed source -> rc=0
[3] remains open: with MOSAIC_GIT_IDENTITY=rev-974 the wrapper resolves
GITEA_LOGIN_NAME=mosaicstack-mos, so one principal holds but the operation is
attributed to Mos rather than the requested seat. That changes identity resolution
shared by every wrapper in this directory and is not folded in here.
Reported-by: rev-974
check-test-enumeration.sh failed the previous push: the new test existed on disk
but was on no CI surface and not signed in the exclusions file. That guard is
correct and caught exactly what it exists to catch -- a test that would never
have run.
Registered on the sanitization step rather than the exclusions file, because
this test is hermetic: it mocks tea and curl onto PATH and sandboxes a throwaway
git repo, so it resolves no real credentials. The tools/git tests in the
exclusions file are there precisely because they do.
Guard now: population 51, enumerated 32, excluded 19, all surfaces present.
Refs #1081
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Amf1Neca162odgcbCWMk1y
Addresses both blockers from review 127 (rev-security-02) and corrects the
severity claim in the original report.
Blocker 1 -- mixed principals. Routing the comment through the token-
authenticated gitea_issue_comment_api() attributed the comment to the token
holder while the close still used --login $GITEA_LOGIN_NAME: two principals for
one operation. `tea comment` accepts the same --repo/--login flags, so the tea
branch now uses it and both calls carry the same principal. The no-login branch
keeps the API helper for both, also a single principal.
Blocker 2 -- no regression test. Adds test-issue-close-fail-closed.sh on the
existing mocked-tea/sandboxed-git harness pattern. Asserts: a failed comment
does not close the issue and exits non-zero; a successful comment does close it;
the subcommand is top-level `tea comment`, never `tea issue comment`; and the
comment and close carry the same --login. GREEN on this branch, RED on main.
Severity correction. The original report said the issue closes anyway and the
audit trail is silently lost. It does not: set -e at line 5 aborts the script
when the comment fails, so the close is never reached. The real defect is that
issue-close.sh -c cannot succeed at all where a tea login resolves -- loud, not
silent. The explicit || guard is retained deliberately: a fail-closed property
that depends on set -e disappears the moment anyone adds `|| true` or wraps the
call in a conditional. Posted as a comment on #1081 and #1085.
Refs #1081
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Amf1Neca162odgcbCWMk1y
`tea issue comment` is not a subcommand -- tea 0.11.x exposes only
list/create/edit/reopen/close under `tea issue`, and comments are the
top-level `tea comment`. The call therefore always failed. Its result was
never checked, so the script went on to `tea issue close`, which IS valid:
the issue closed and the record of why it closed was silently lost.
Route the comment through the existing gitea_issue_comment_api() helper on
both branches. It is login-independent and was already the mechanism used by
the no-login fallback. Both call sites now fail closed: if the comment cannot
be posted, the issue is not closed.
Verified behaviourally against the live provider, both directions:
negative -- comment cannot post => "NOT closing (fail closed)", exit 1,
issue left open
positive -- comment posts and issue closes => state=closed, comments=1, exit 0
Refs #1081
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Amf1Neca162odgcbCWMk1y
Closes#953.
GATE RECORD: review CLEAR at this exact head (author != reviewer, pre-registered diff-blind checks) + terminal-green CI at this exact head + queue guard clear.
CI CAVEAT (#973): green on the wake suites is currently WEAKER THAN IT LOOKS, IN BOTH DIRECTIONS. grep error/spawn exit codes are read as absence across 257 assertion sites in six idiom forms; 36 inverted (&&-fail) sites — including 19 credential-security canaries — fail toward GREEN under load. These greens were obtained on solo reruns after load-correlated FALSE reds (2115/2116/2118; main itself was red). This merge's safety therefore rests on the CONTENT review, not on the green. Remediation charter fa551c2d0 is authored and in flight.
Co-authored-by: mos-dt-0 <[email protected]>
Closes#952.
GATE RECORD: review CLEAR at this exact head (author != reviewer, pre-registered diff-blind checks) + terminal-green CI at this exact head + queue guard clear.
CI CAVEAT (#973): green on the wake suites is currently WEAKER THAN IT LOOKS, IN BOTH DIRECTIONS. grep error/spawn exit codes are read as absence across 257 assertion sites in six idiom forms; 36 inverted (&&-fail) sites — including 19 credential-security canaries — fail toward GREEN under load. These greens were obtained on solo reruns after load-correlated FALSE reds (2115/2116/2118; main itself was red). This merge's safety therefore rests on the CONTENT review, not on the green. Remediation charter fa551c2d0 is authored and in flight.
Co-authored-by: mos-dt-0 <[email protected]>
Closes#958.
The preimage definition (source-adapter.sh) is the single most consequential file in the wake pipeline — every observed_hash is a sha256 of what it emits — and it was UNVERSIONED: no git history, no backup. When it was edited at 07:27 on 2026-07-30, attribution was recoverable only because an agent transcript happened to still be on disk. A11 gives that file durable provenance (option (2) of #958: recorded content-addressed, not in-band).
DESIGN, per the pre-registration:
- Provenance is OUT-OF-BAND (never on the adapter's stdout) — an in-band record would advance observed_hash for every source at once and manufacture the re-baseline it exists to explain (B3).
- The obligation never depends on the provenance path: a missing/corrupt store cannot halt the detector or swallow a wake (#940 advisory-fields precedent; B4).
- DESC_FMT=d1 is NOT the provenance record — the tag versions the descriptor FORMAT; a behaviour change that keeps descriptor shape re-baselines every hash and leaves the tag unchanged (B6).
CREDENTIAL HARD GATE (rebuilt after the first verdict FAILED it): byte capture is now RECORD-ONLY BY DEFAULT (extras opt in via WAKE_PREIMAGE_CAPTURE), not allow-by-default-refuse-on-shape — because a shape list can only refuse the secrets someone already enumerated, and the tool's own usage text recommended adding detector.env (where HMAC material lives). Deny is evaluated on BOTH raw and resolved path forms with resolved anchors, ordered before allow — closing the realpath-before-deny ordering defect that let a renamed symlink target through.
VERIFICATION (reviewer, mos-dt, independent of the author's claims):
- Seven decoy cases by planted-marker-then-grep-whole-state-dir: known cred path / same-name symlink / RENAMED-target symlink / prefixed secret / prefixless secret / opted-in-symlink-to-DENIED-target all REFUSED; opted-in-symlink-to-ALLOWED-target CAPTURED (positive control that the harness can capture at all, and that C was not closed by breaking every symlink).
- Polarity-completeness self-test RE-RUN with the shape list stubbed always-allow AND both deny lists stubbed — case D still safe: the flip is complete, the shape list is not load-bearing. Each stub proven live first (a stub that silently fails to apply reports the dangerous state as safe).
- B11: rm-then-change fails LOUD (rc=1), refuses to re-baseline, leaves the ledger absent; absent-with-emptied-objects still first-installs cleanly (absent-is-not-corrupt not paid for by breaking first install).
- RED-first reproduced exactly P13-P16 pre-fix; each refusal corroborated three ways (loud stderr, ledger row captured:false WITH a hash so attribution survives refusal, objects/ holding only the adapter).
KNOWN RESIDUAL (filed #969, non-gating): the deny check is both-forms but the suite needles only the resolved form — a deny reduced to resolved-only survives 17/17 green and would leak a renamed-symlink case. No reachable leak at this head (shipped code correct on all seven decoys); it constrains a FUTURE edit. Doctrine: a both-forms fix needs a needle per form; a fixture that satisfies its assertion through a DIFFERENT rule is testing the rule it did not mean to test.
Authored by pepper (sb-it-1-dt); independently reviewed by mos-dt (sb-it-1-dt) under diff-blind pre-registration (7242688b1, predating first read) — NOT CLEAR on the first verdict (B2/B11 failed by decoy), CLEAR at 8aff7d8 after the polarity rebuild. Manifest version 0.7.0.
Co-authored-by: mos-dt-0 <[email protected]>
Closes#946.
The digest omitted a quarantined entry's claim from disclosure while still advancing the ack watermark it instructed the consumer to run — converting a fail-safe HOLD into a silent DISCARD, through the documented normal path. Measured: the burial instruction was re-issued FIVE times, four fresh digests plus one system-initiated redelivery fired purely because the entry had gone unconsumed for 1826s. That redelivery is the proof of the 'indefinitely' half: the mechanism re-asserted itself with no new information.
SCOPE — this was NOT a missing check in the consume path. Measured before the fix: dead-letter occurrences were digest.sh 27, store.sh 0, ack.sh 0, detector.sh 0, reconcile.sh 0. Quarantine was owned ENTIRELY by the renderer; the store that advances the watermark had zero knowledge the ledger existed, so an entry could be quarantined by one subsystem and consumed by another with no possible interaction. The fix is therefore a deliberate cross-module decision — option (b), quarantine recorded into a store-owned file, preserving the existing direction of dependency — pre-registered by the consumer before any diff existed.
VERIFICATION
- Pipeline 2107 terminal SUCCESS at e11bc6622, read clone-inclusive from the provider API rather than through `pipeline-status.sh` (which filters `.type != "clone"` per workflow and would hide a clone failure behind an all-green table): 9/9 children success, exit 0 each, no non-success member. Its `test` step runs all nine wake harnesses via turbo -> packages/mosaic `test` -> `test:framework-shell`.
- Independent review by the consumer on the affected lane: eleven pre-registered acceptance checks, authored and delivered BEFORE the diff was read — the file list deliberately unlooked-at, because a filename alone would have disclosed which option was chosen. All eleven resolved, no blocker.
- The check that decides it: a RAW `ack.sh consumed --upto N` with no digest involved must refuse to advance past a quarantined seq — the case an agent hits when a digest is MISSED, and the one that would have sunk a disclosure-only fix. Covered at the head as a named assertion (T13 ordinary-path bypass), written independently of the reviewer's list.
- Mutation: one asserted site disabled -> TWELVE assertions die, every one BEHAVIOURAL, ZERO count assertions, including one killing across the module boundary the fix spans.
- RED control at base a6b5f6a: 34 and 10 assertions fail, matching the body exactly.
- Coordinator re-verify by a different instrument than the reviewer used: static reference counts across the base/head boundary — store.sh 0 -> 49, ack.sh 0 -> 6, `--agent` unchanged at 4 (so #949 correctly stayed out). A fix present-but-inert passes the count and fails the mutation; a fix behaviourally correct but smuggling #949 passes the mutation and fails the count. Neither result is reachable by repeating the other.
KNOWN RESIDUALS
- The `consumed-hashes` repair criterion is met only for keys that RE-EMIT. A corrupted row whose key never recurs stays false indefinitely; the sweep covers those, and the known-false row named in the acceptance criteria had already self-healed by re-emission rather than by design — safe by population, not by design.
- The audit's clean-sweep message names one unprovable class; a second exists (a surviving dead-letter row with an empty `observed_hash` cannot be convicted either). Wording, not logic. Filed separately.
- The audit's provability bound makes dead-letter RETENTION load-bearing for auditability. Nothing prunes it today, so this is latent — but any future rotation or size cap silently converts provable rows into unprovable ones with no signal at either end. This is not a defect; it is a property that BECAME load-bearing and is recorded nowhere. Filed separately.
- `test-wake-detector.sh` D4 fails at this head AND identically at base, with an empty diff over detector files — pre-existing, tracked, not introduced here.
Authored by pepper (sb-it-1-dt); reviewed independently by mos-dt (sb-it-1-dt). The mos-dt-0 commit and fork identity does not identify the author — attribution collapse tracked separately.
Co-authored-by: mos-dt-0 <[email protected]>
Closes#944.
_has_hard_locator accepted only repo+issue / 40-hex sha / file — the forge vocabulary. The detector emits path (+snapshot_sha when attested) and NEVER emits file/issue/sha, so predicate and sole producer shared ZERO keys and every class=actionable board_file entry dead-lettered. Latent since #920, whose harness pinned the detector's own emission shape as its malformed example — the suite certified the gap it was written to guard.
Fix: `path` becomes a hard-locator arm, and ONLY path. Bare path-less snapshot_sha is a deliberate NON-arm (would widen past the board_file vocabulary); Q11(d) asserts it still quarantines at 7/40/64 chars, spanning the detector's ^[0-9a-f]{7,64}$ attestation range.
VERIFICATION
- Pipeline 2105 terminal-green at fa36da8. Its `test` step reaches all nine wake harnesses via turbo -> packages/mosaic `test` -> `test:framework-shell`, which names each suite explicitly. The two-levels-down indirection matters: no search of .woodpecker/* can see it, and that is exactly why this question was got wrong earlier today and then corrected. CI therefore DOES attest the quarantine suite and the detector suite at this head.
- Independent review (mos-dt, consumer on the affected lane) PASS at fa36da8, from a detached worktree: predicate provably unmoved from fb3c3c3 (comment-stripped sha256 identical, _has_hard_locator body byte-identical), RED control at base reproduced exactly 8 failures all Q11 including "got 6".
- Third reviewer (wake-judge) ACCEPT on both judgment calls: the Q1 assertion reversal is a legitimate correction (the flip was forced, not elective — base fixtures red 19 assertions against the head predicate) and path-alone satisfies §2.1, whose operative test is "one targeted call, never a search" — two of its four named exemplars already resolve to current state. Requiring path+snapshot_sha jointly would permanently dead-letter a declared source class and conflict with #940's advisory-fields ruling.
- Judge's mutation criterion met: with the reconciled exemption disabled, the gate-level assertion ("an ORIENTATION-tier enumeration must NOT be quarantined") dies at this head and did not exist as a casualty before F1.
- D4 (detector lock re-acquisition) fails intermittently at base AND head; git diff base..head over the detector files is EMPTY, so it is out of this PR's surface on structural grounds rather than on a re-roll. Known defect, fix identified (detector.sh:516, fd 9 leaked into sleep), tracked separately.
KNOWN RESIDUALS
- ENUM-B is now the sole address-free reconciled fixture, so the exemption's gate-level guard is a population of one. Safe by population, not by design. Author follow-up: assert ENUM-B carries no hard-locator arm so the harness guards its own premise.
- The binding spec (CONVERGED-DESIGN.md §2.1, separate repo) still enumerates four forge tokens and reads narrower than the shipped gate. Tracked as #948, sequenced after the dragon-lin reseed.
- Hard-locator arms are type-loose: repo/file/path accept any non-null JSON value. Pre-existing; `sha` fails closed only by accident of test(). Tracked separately.
Authored by pepper (sb-it-1-dt); reviewed independently by mos-dt (sb-it-1-dt) and wake-judge. The mos-dt-0 commit/fork identity does not identify the author — attribution collapse tracked in #3092.
Co-authored-by: mos-dt-0 <[email protected]>
grep is line-oriented, so a multi-line knob value passed the per-line anchors and was still fatal in arithmetic. Replaced with a case pattern matching the whole string, so an embedded or leading newline rejects. Manifest bumped 0.6.12 -> 0.6.13: three materially different detectors had shipped under one version string, and version= is the component sole self-identity claim.
Authored-by: pepper
Reviewed-by: mos-dt (independent, at this head; transfer proven by blob-hash equality)
Merged-by: Mos
Co-authored-by: mos-dt-0 <[email protected]>
The slack knob was interpolated raw into $((...)) under set -u: a malformed value was FATAL to the poll, falsifying the poll-never-fails invariant, and a negative value inverted the guard to deny-all. Shape validation alone was insufficient — bash reads a leading zero as octal, so 08/09 passed the regex yet were fatal and 0300 silently meant 192. Now validated ^[0-9]{1,9}$ with a loud fallback to 300, then forced to base-10 via 10# so the knob means what the operator wrote.
Authored-by: pepper
Reviewed-by: mos-dt (independent, found both the original defect and the radix residual)
Merged-by: Mos
Co-authored-by: mos-dt-0 <[email protected]>
Adapter emits snapshot sha/ts out-of-band on fd 3 so a changing value never enters the delta-gate hash. Detector validates advisorily (sha regex, epoch sanity before arithmetic, future-skew slack); malformed metadata is dropped loudly and never gates the wake. Digest renders snapshot_sha/snapshot_ts plus a git-show re-verify hint. Adapters that never write fd 3 are byte-identical.
Reviewed-by: Mos (design, independent)
Reviewed-by: mos-dt (artifact, hardening §2)
Co-authored-by: mos-dt-0 <[email protected]>
Mos
Mos
be-coder-08andMos
coder-mos1andmos-dt-0
jason.woltjeandMos