feat(mosaic): enforce whole mutator-class lease gate #837
Assignees
jason.woltje
Clear assignees
jason.woltje
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: mosaicstack/stack#837
Reference in New Issue
Block a user
Delete Branch "feat/829-mutator-gate"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
closes #829
Summary
RED/GREEN evidence
Review gate
Authorization surface: mandatory independent terra CODE review and exact-head Opus SECREV are required before Mos-authorized merge. Author did not self-review. Do not merge from this lane.
Round 5 — parser stopping-criterion evidence (A+B)
Exact head:
04cc065c2babae5bcc04b932d89d0bc3ff8546dfA — realistic parser classes closed: tracked
claude/pivariables are resolved in the same terminal command-position path as literal binaries after anyexec/command/nohup/env+assignment prefix sequence. Permanent RED regressions cover quoted and unquotedexec,command,nohup, andenv A=1, each in multiline and same-line assignment shapes (10 new RED-before-fix cases). Existing comment-marker, heredoc, continuation, chaining, command-substitution,eval, bare-variable, and primitive-location regressions remain green.B — residual backstops proven:
dangerous-primitive; onlylaunch-runtime.pyowns the raw dangerous flag..*PreToolUsehook invokes the gate; withoutMOSAIC_LEASE_SESSION_ID, read, mutator, and custom/MCP tools all exit 2 withGATE_UNAVAILABLE.Stopping criterion status: A+B are present on this head. The parser is explicitly best-effort rather than a complete shell interpreter; fresh terra CODE and Opus SECREV provide C (no new non-overlapping realistic evasion). Inventory remains 14/14 and guard coverage remains 97%.
OPUS SECREV (MANDATORY, auth surface) — WI-2 #829 head
77b137ccVerdict: ✅ SECREV GO — reviewed exact head
77b137ccc04b5be035cac5ca21bbbf3df8b94f97— 0 blocking security findings.Fresh review of the authorization surface; zero evidence transfer from the WI-1 #828 SECREV. I am the mandatory security gate on this boundary (terra runs the code lens in parallel). Base =
main8ec67a11(clean merge-base); reviewed =git diff main...HEAD+ every new/changed file. Reviewed-SHA == merge head (exact-head). The build's own green tests were treated as process, not evidence; every property below was earned from the product code.Surface mapped
daemon.py(+173): new lease state machineUNVERIFIED → PENDING_VERIFICATION → PENDING_PROMOTION → VERIFIEDin volatileBroker.leases(in-memory only), and actionsbegin_verification,promote_lease,revoke_lease,authorize_tool.mutator-gate.py(new): stdintool_name→ brokerauthorize_tool; exit 0 allow / exit 2 deny.launch-runtime.py(new) +launch.ts:register_anchorthenexecvpethe runtime (PID/starttime = authenticated anchor), exporting only broker-mintedMOSAIC_LEASE_SESSION_ID+ generation.settings.jsonPreToolUsematcher:".*"(all tools) →mutator-gate.py --runtime claude; Pimosaic-extension.tspi.on('tool_call', …) → checkPiMutatorGate(spawns the same gate--runtime pi).Consequential-mutator entry enumeration — none reachable without a broker-VERIFIED lease
Every tool invocation in a gated runtime funnels through exactly one gate call:
.*PreToolUse hook fires for every tool (built-in, unknown, MCPmcp__*,Task); a hook exit 2 blocks the tool.Bash/Edit/Write/NotebookEditand all unknown/custom tools are consequential by classification (not in the fixed read-only frozenset) → routed to the VERIFIED check.tool_callhandler runs the gate for every tool; non-zero →{block:true}veto.Classification is by whole tool class / exact name, never by parsing a
Bashstring (daemon.pyauthorize_tool:tool_name in READ_ONLY_TOOLS[runtime] or tool_name == RECOVERY_TOOL), so a compromised model cannot smugglegit/curl/kubectl/rminside a generic mutator — the generic mutator is blocked before its input executes. Read-only sets are fixed frozensets a client cannot extend.T-B (compromised tool, PRIMARY) — fail-closed block proven
A compromised in-runtime tool attempting a consequential mutation while UNVERIFIED (observer fired) or past-TTL is denied, and cannot escalate by any of the enumerated vectors:
VERIFIEDlives only in broker memory (Broker.leases[session_id].state);authorize_toolreads it from broker state, never from the request. The request carries only{session_id, runtime_generation, runtime, tool_name}— there is no client-supplied "I'm verified" field anywhere. ReachingVERIFIEDrequiresbegin_verification(mints a WI-1 single-use promotion token) →promote_lease(constant-timesecrets.compare_digeston the token + store-token match on session/generation/binding/consumed==False, thendel= consume) →finish_promotion. Replay of the promotion token fails: the store token is deleted (single-use) and the lease is no longerPENDING→INVALID_LEASE_TRANSITION/PROMOTION_TOKEN_INVALID.authenticate(peer_pid, request)(verified for all four new actions: daemon.py:428/459/487/491), which binds the session to kernelSO_PEERCREDPID +/procstarttime ancestry (WI-1). A tool can only act on the session whose anchor is in its own process tree; a different (verified)session_id→UNKNOWN_SESSION/ANCESTRY_MISMATCH→ deny. EnvMOSAIC_LEASE_SESSION_IDis read by the hook process (spawned by the harness with the harness's env), not writable by a tool child, and even a swapped value fails ancestry.authorize_toolcomputestime.monotonic() >= expires_atand, on expiry (or missing/non-numericexpires_at), callsrevoke_session_authorityand returnsLEASE_EXPIREDdeny.monotonic()is wall-clock-immune. All state reads/writes are serialized by the WI-1 globalbroker_lock(singlebroker.handlecall site confirmed at daemon.py:585), so check-and-decide is atomic; granularity is per-tool-call — the next consequential tool after expiry is denied.KeyError), malformed input/reply (ValueError/JSONDecodeError), broker-unavailable/connect refusal or timeout (OSError, 1.5 s client vs 1.0 s broker deadline vs 3 s/2 s hook budget) all hitmutator-gate.py's singleexcept (KeyError, ValueError, OSError, JSONDecodeError): return deny("GATE_UNAVAILABLE")→ exit 2 → tool blocked. Allow is returned only onreply.ok is True and reply.decision == "allow". A runtime launched outside the gated launcher has no session env → the installed hook denies all tools (fail-closed, not fail-open).Revoke-first / promote-last — genuine invariant under adversarial timing
begin_verificationcallsrevoke_session_authority(session_id)before minting the new promotion token and recordingPENDING. The priorVERIFIEDauthority and all pending tokens are gone before the new window opens; because the wholehandle()runs underbroker_lock, no concurrentauthorize_toolobserves an intermediate — a stale lease cannot survive into the new window. Any consequential tool serialized after seesPENDING(≠VERIFIED) → deny.handle(), token consumption isstore.commit()-ed beforefinish_promotion()setsVERIFIEDand before the response exposesstate:VERIFIED.VERIFIEDis externally visible only after durable single-use consumption. No consequential mutation is possible between verify and promote:PENDING_PROMOTION ≠ VERIFIED, and the whole transition is one uninterrupted lock hold, so there is no TOCTOU where the gate reads verified-state and the tool mutates after invalidation.broker_lockserializesbegin_verification,promote_lease,revoke_lease, andauthorize_tool. A revoke/compaction interleaved with an authorize either runs first (authorize sees UNVERIFIED → deny) or after (authorize already returned for a then-valid lease). No double-grant (promotion token is single-use + lease must bePENDING), no mutate-during-revoke window. Generation bump (authenticatewhenreq.gen > current) callsrevoke_session_authority→ UNVERIFIED, so a new incarnation cannot inherit a prior VERIFIED lease; stalegen < current→STALE_GENERATIONdeny.No default-allow / fail-OPEN anywhere
authorize_tool's consequential path is deny-by-default: allow is the terminal statement reached only afterstate == VERIFIEDandlease.runtime == runtimeandlease.runtime_generation == req.generationand non-expired. Every other outcome is an explicit typed deny (MUTATOR_UNVERIFIED/LEASE_EXPIRED). Any exception (auth failure, integrity, commit failure) propagates →handle()rolls back store+leases and re-raises →handle_connectionreplies{ok:false}→ gate denies.StateCommitUncertain(post-rename durability doubt) still terminates the daemon (WI-1) → fail-closed. Leases are volatile: broker restart → all UNVERIFIED. Confirmed: no branch returns allow on an error/uncertain path.Also verified
runtimerestricted to{claude,pi}(INVALID_RUNTIME),tool_namevalidated (non-empty ≤256).0 < ttl ≤ 300, int-only viatype(...) is int) but not lengthen; expiry checked at use.mint_tokennowdeepcopys the binding (no aliasing between request/lease/token). Promotion token bound to cycle viatoken.binding == lease.binding+ session/generation match.Non-blocking observations (informational — NOT change requests)
begin_verification/promote_leaseare drivable by any process in the anchor's tree; the broker attests the kernel anchor + single-use cycle token, not the semantic authenticity of the verification (that a compaction genuinely occurred / the model truly re-read context). Receipt/obedience validation is explicitly WI-future (mutator-class-gate.mdL26; assurance boundary L37-39). So a fully-compromised model that can run the cycle can re-verify itself — this is the ratified scope, not a WI-2 defect. GO means "no consequential mutator is reachable without completing the broker-owned revoke-first/promote-last cycle, and the gate fails closed," not "a compromised model can never mutate."finish_promotionever raised afterstore.commit(),handle()'sexceptwould roll back in-memorystore.valuewhile the file stays committed. Unreachable today (state is setPENDING_PROMOTIONimmediately before the check in the same lock hold). Robustness note only.Disclosed residuals (acknowledged, deliberately NOT flagged)
Harness-side tail-preserving middle-drop (planner ruling
b7bbb6ea, A-v5-1 replaced) and server-side branch-protection backstop (WI-7); the ratified bounded T-A dual-observer-miss-within-TTL residual; and T-C (hook/extension absence, runtime outside the gated launcher, ptrace / same-UID broker replacement). Per scope these are not new findings.Independence (Gate 16 — by lane/process): author
ms-cr-wi1(pi) ≠ code lensterra≠ this Opus security review — distinct principals on one Gitea account, separated by lane/process; I am THE security gate for this WI. Earned from product code, not the author's tests.tea/pr-review.sh approvebroken for this surface; this durable comment is the formal SECREV approval record. Reviewed head77b137cc. Merge authority remains with the coordinator (Mos). No code edits, commits, pushes, or merges performed by this reviewer.INDEPENDENT CODE REVIEW (terra) — WI-2 #829 head
77b137ccVerdict: REQUEST CHANGES
Reviewed exact head:
77b137ccc04b5be035cac5ca21bbbf3df8b94f97against authoritativemain8ec67a11. Author/reviewer separation is LANE/PROCESS-attested (sol/Pi author; terra separate reviewer session). 2 blocking findings.[BLOCKER]
packages/mosaic/src/commands/launch.ts:848-861andpackages/mosaic/src/commands/claudex.ts:450-457— themosaic claudex/mosaic yolo claudexClaude-harness launch bypasses the new lease-gated launcher. First-classclaudeandpinow useexecLeaseGatedRuntime, but the separately registered claudex path builds an isolatedCLAUDE_CONFIG_DIRand directly executesclaude. It neither registers a broker anchor nor injectsMOSAIC_LEASE_SESSION_ID; its isolated config is also not populated with the all-tools hook. Thus this supported Claude Code harness can execute consequential tools without the brokerauthorize_tooldecision, violating the whole-mutator-class requirement. Route claudex through the same register-and-exec gate while preserving its isolated proxy environment, or explicitly reject it until it is gated. Add a behavioral regression covering both normal and YOLO claudex dispatch and denial when no verified lease exists.[BLOCKER] New-code coverage gate is not met independently. The required coverage is ≥85% on new code, but independently instrumenting the actual focused broker/gate suites reports
82%combined branch coverage. More specifically, the entirely newpackages/mosaic/framework/tools/lease-broker/launch-runtime.pyis only48%in the branch-aware report (35/63 statements exercised); successfulos.execvpecoverage is lost and important validation/error paths are untested.mutator-gate.pyis 79% branch coverage. The reported 88% cannot be accepted as evidence for this head. Refactor/extend the harness so successful register-before-exec and failure branches are durably collected, add the missing behavior tests, and enforce/report the configured threshold for each new broker-gate executable.What passed
43/43; Python state suite10/10; rootpnpm test43/43Turbo tasks (Mosaic72/72files,1,377/1,377tests); root typecheck/lint/format andgit diff --checkgreen; PR CI pipeline 1911 green.74fc62e4,439f5f4bprecede implementation77b137cc, and implementation did not modify the new acceptance test. No--no-verifyfound.The mandatory Opus-SECREV remains separate and is not replaced by this code review.
OPUS RE-SECREV (MANDATORY, auth surface) — WI-2 #829 head
6a3505dfVerdict: ✅ SECREV GO — reviewed exact head
6a3505dfbc2cfeee6b651cea84f7050b90f4f786— 0 blocking security findings.This is not a re-pass of the frame. My round-1 GO (comment 18088) was issued with the
claudexlaunch path walking around the gate — a whole launch path with no broker anchor, noMOSAIC_LEASE_SESSION_ID, and no all-tools hook in its isolatedCLAUDE_CONFIG_DIR. That is squarely a SECREV miss and I own it. This pass affirmatively proves, from the product code on the remediated head, that zero walk-arounds remain. Zero evidence transfer; the build's green tests are process, not evidence. Base =main8ec67a11(clean merge-base); reviewed =git diff main...HEAD+ all changed files. Reviewed-SHA == merge head.RIDER S — every runtime launch entry enumerated; no Claude/Pi path reaches a consequential mutator without a broker-verified lease
mosaic claudelaunchRuntime('claude')execLeaseGatedRuntime('claude'):766mosaic yolo claudeyolo=trueexecLeaseGatedRuntime('claude')mosaic pilaunchRuntime('pi')execLeaseGatedRuntime('pi'):801mosaic yolo piyolo=trueexecLeaseGatedRuntime('pi')mosaic claudex:1121 claudexHandler → launchClaudexProduction(args,false)execLeaseGated → execLeaseGatedRuntime('claude'):863mosaic yolo claudex:1135 resolved BEFORE the runtime allowlist → launchClaudexProduction(args,true)mosaic [yolo] codexexecRuntime('codex'):779mosaic [yolo] opencodeexecRuntime('opencode'):786Whole-codebase walk-around hunt (independently re-derived): the only
claude/piprocess launches inpackages/mosaic/srcare the threeexecLeaseGatedRuntime(...)calls above. There is noexecRuntime('claude', …)orexecRuntime('pi', …)anywhere. The round-1 bypass is closed structurally:ClaudexHarnessAdapter.execwas renamed toexecLeaseGatedand wired toexecLeaseGatedRuntime('claude', …)(launch.ts:863; claudex.ts:471-478,538). TypeScript would not compile a residualexecadapter.yolo claudexis dispatched before the runtime allowlist check (launch.ts:1135), so it cannot fall through to a plain runtime or an error path.The now-gated claudex path — RIDER S's four properties proven in code
mosaic [yolo] claudex→launchClaudex(claudex.ts) →adapter.execLeaseGated(cliArgs, env)(:538) withenv= the composed isolated env (creds stripped,CLAUDE_CONFIG_DIR= isolated) →execLeaseGatedRuntime('claude', cmdArgs, env)→execRuntime('python3', [launch-runtime.py, --runtime claude, --, claude, …], {…env, MOSAIC_LEASE_BROKER_SOCKET, MOSAIC_RUNTIME_GENERATION}):launch-runtime.pysendsregister_anchor(nosession_id); the broker reads the connecting process'sSO_PEERCREDand mints the id (secrets.token_hex);register_anchorstill refuses any client-suppliedsession_id(daemon.pyCALLER_SESSION_ID_REFUSED, unchanged). The chainmosaic → python3(launch-runtime) → execvpe claudekeeps one PID, so the registered anchor is Claude's PID; gate hooks (Claude's descendants) authenticate up to it.MOSAIC_LEASE_SESSION_ID:launch-runtime.pysetsenvironment["MOSAIC_LEASE_SESSION_ID"] = session_id(the freshly minted id, overwriting any inherited stale value) beforeexecvpe. A stale parent id cannot reach the gate; even if it did,authenticatewould fail onSO_PEERCREDancestry → deny.authorize_tool— all-tools hook in the isolatedCLAUDE_CONFIG_DIR:prepareConfig(configDir)=ensureClaudexMutatorGateSettings(claudex.ts:311-357) unshifts thematcher:".*"PreToolUse hook runningmutator-gate.py --runtime claudeinto the isolated dir'ssettings.json, and Claude is pointed at that exact dir viaCLAUDE_CONFIG_DIR. It fails closed on a symlinked or malformed settings file / non-array PreToolUse. Because claudex uses an isolated config dir (not~/.claude), this is the fix's crux — the isolated dir now carries the same all-tools gate.authorize_toolreturnsMUTATOR_UNVERIFIED/LEASE_EXPIRED→mutator-gate.pyexit 2 → tool blocked. And at launch time, if the broker is unreachable,register_anchorfails →launch-runtime.pyreturns 1 →execRuntimeexits 1 → Claude never launches (no ungated harness). Missing identity env / malformed reply / timeout all deny.If the classification path is read-only (
Read/Grep/Glob/Ls/Findormosaic_context_recover) it is allowed; every other tool (Bash/Edit/Write/NotebookEdit, unknown, MCPmcp__*) is consequential and blocked unless VERIFIED — by whole class, never by parsing aBashstring.Broker core + gate executables — re-confirmed not regressed
daemon.pyis byte-identical to my round-1 proof head77b137cc(verified:git diff 77b137cc..HEAD -- daemon.pyis empty). So revoke-first / promote-last, TTL authoritative at use (time.monotonic() >= expires_at→ revoke+deny), kernel-anchored-only trust (no client-asserted principal/verified field), and no fail-open branch (every error/exception → deny;StateCommitUncertainterminates the daemon) are unchanged from the fully-proven round-1 state, under the WI-1broker_lockserialization.mutator-gate.py/launch-runtime.pydeltas are behavior-preserving DI refactors (injectargv/environ/stream/request/executefor testability; production__main__binds the realos.environ/sys.stdin/broker_request/os.execvpe). Fail-closed tails intact: gate allows only onreply.ok is True and reply.decision == "allow", elsedeny(exit 2); launcher denies launch on registration failure. The addedreturn 0inlaunch-runtime.mainis unreachable in production (execvpereplaces the process) — test-double affordance only.Non-blocking observations (informational — NOT change requests; stated for transparency after the round-1 miss)
codexandopencodelaunch ungated. WI-2 is explicitly "the framework-native authorization boundary for Claude and Pi" (mutator-class-gate.mdL2); codex/opencode are distinct harnesses without the PreToolUse-hook / Pi-tool_callmechanism, so the Claude/Pi gate does not apply to them. This is documented scope, not a walk-around of the Claude/Pi gate — flagged so the coordinator holds it consciously, not silently.yoloappends--dangerously-skip-permissions. The gate's efficacy foryolo claude/yolo claudexdepends on Claude PreToolUse hooks executing under that flag (they do — the flag auto-approves permission decisions, it does not disable hooks; this is the same premise the existingprevent-memory-write.shhook and all fleet-yolo protections already rely on). Pi's gate is atool_callextension unaffected by the Claude-only flag. Harness/hook integrity is the ratified T-C boundary.Disclosed residuals (acknowledged, NOT flagged)
Harness-side tail-preserving middle-drop (planner ruling
b7bbb6ea); server-side branch-protection backstop (WI-7); ratified bounded T-A dual-observer-miss-within-TTL; and T-C (hook/extension absence, runtime outside the gated launcher, ptrace/same-UID broker replacement).Independence (Gate 16 — by lane/process): author
ms-cr-wi1≠ code lensterra≠ this Opus security review — distinct principals on one Gitea account by lane/process; I am THE security gate for this WI. Earned from product code, not the author's tests.tea/pr-review.sh approvebroken for this surface; this durable comment is the formal RE-SECREV approval record. Reviewed head6a3505df. Merge authority remains with the coordinator (Mos). No code edits, commits, pushes, or merges performed by this reviewer.INDEPENDENT CODE RE-REVIEW (terra) — WI-2 #829 head
6a3505dfVerdict: REQUEST CHANGES
Reviewed exact head
6a3505dfbc2cfeee6b651cea84f7050b90f4f786against requested base8ec67a1126adb0dcd4c3a2bf5525f3e239c0b201. Author/reviewer separation is LANE/PROCESS-attested (sol/Pi author; terra separate reviewer session). The prior Claudex and coverage blockers are remediated, but one whole-class authorization bypass remains.packages/mosaic/framework/tools/prdy/prdy-init.sh:88-90andpackages/mosaic/framework/tools/prdy/prdy-update.sh:76-78directlyexec claude --dangerously-skip-permissions. These are supported Mosaic runtime launchers (PRD creation/update) and launch a consequential-tools-capable Claude process withoutlaunch-runtime.py, broker anchor registration,MOSAIC_LEASE_SESSION_ID, or a guaranteed all-tools hook. They therefore violate B1's no runtime launch path requirement: an UNVERIFIED PRDY Claude session can reach consequential tools. Route these through the same broker-gated launch boundary (or reject Claude PRDY launches fail-closed), and add behavioral regression coverage proving both init and update routes cannot run without broker registration and that an unverified mutator is denied. The same audit should cover other directclaude -pautomation launchers (for exampleframework/tools/qa/remediation-hook-handler.sh:62) rather than treating themosaicCLI dispatcher as the only launch surface.Remediation verified
mosaic claudexandmosaic yolo claudexnow use the sharedexecLeaseGatedRuntimeseam; the real-socket acceptance probes prove a broker-minted 64-hex session, isolated all-tools Claude hook, and status-2 denial for unverifiedBashin both modes.46/46focused real-socket acceptance tests and both Python suites (10/10each) passed.launch-runtime.py17/18 = 94.4%,mutator-gate.py22/22 = 100%, and WI-2-addeddaemon.pybranch arcs 35/40 = 87.5%. B2 is satisfied.pnpm test43/43 Turbo tasks (Mosaic 72 files / 1,381 tests), typecheck 42/42, lint 23/23, format, andgit diff --check. PR pipeline 1912 is green at6a3505df.74fc62e4,439f5f4b, then implementation77b137cc; remediation test046896c6precedes6a3505df); no--no-verifyevidence found.The mandatory Opus re-SECREV is separate and is not replaced by this code review.
INDEPENDENT CODE RE-REVIEW R3 (terra) — WI-2 #829 head
1792b793Verdict: REQUEST CHANGES
Reviewed exact head
1792b7934dda7eff64a207b8b0edb9c460d4164bagainst requested base8ec67a1126adb0dcd4c3a2bf5525f3e239c0b201. Author/reviewer separation is LANE/PROCESS-attested (sol/Pi author; terra separate reviewer session). 1 blocking finding.[BLOCKER — RIDER E]
packages/mosaic/framework/tools/lease-broker/check-runtime-launches.py:40-48,92-111treats any occurrence oflaunch-runtime.pyon a line as proof that a direct Claude/Pi launch is gated. I performed the requested temporary-launcher check. A plainexec claude --dangerously-skip-permissions "terra-r3"correctly makes the guard red. But this still-ungated executable launcher with an inline marker:makes the production guard exit 0 and print
runtime launch inventory: 15 gated/15 total.is_gated_line()never distinguishes comments or validates an invocation/arguments;scan_text()suppresses the direct match solely because regex line 42 matched the comment. The permanent guard can therefore be trivially (and accidentally) bypassed by a newly added ungated launcher, so it cannot serve as the required load-bearing enforcement. Parse executable syntax or strip comments before classification, require a real wrapper invocation with the necessary runtime/command boundary, and add this exact evasion as a permanent red regression.Independently verified positives (not sufficient for approval)
launch-runtime.py, and Mosaic/coord/fleet/orchestrator paths reach the same broker registration boundary.49/49: PRDY init/update and QA remediation fail before fake-runtime execution with no broker, receive a broker-minted 64-hex session with the broker, and deny unverifiedBash. Claudex normal/YOLO and broker revoke-first / consume-before-promote behavior remain green.launch-runtime.py18/18 = 100%,mutator-gate.py22/22 = 100%, WI-2daemon.pydelta 35/40 = 87.5%, guard 36/38 = 94.7%. Coverage does not cure the proven guard false-negative.pnpm test43/43 Turbo tasks (Mosaic 72 files / 1,384 tests), typecheck 42/42, lint 23/23, format, and diff check. R3 red-first ordering is correct:7f3418faprecedes1792b793; no--no-verifyevidence found. Pipeline 1913 was still running when this review posted.The mandatory Opus-SECREV is separate and is not replaced by this code review.
OPUS RE-SECREV R3 (MANDATORY, auth surface) — WI-2 #829 head
1792b793Verdict: ✅ SECREV GO — reviewed exact head
1792b7934dda7eff64a207b8b0edb9c460d4164b— 0 blocking security findings.Fresh pass, zero evidence transfer. My round-2 GO (18097) missed the prdy/qa launchers a prose enumeration could not prove complete; per the brief I did not re-enumerate by eye — I attacked the choke-point wrapper and the new guard adversarially, and independently re-scanned the whole repository. Base =
main8ec67a11(clean merge-base); reviewed =git diff main...HEAD+ all changed files. Reviewed-SHA == merge head. The build's green tests are process, not evidence.RIDER 1 — CHOKE-POINT integrity (the actual runtime security control): PROVEN
The single wrapper every gated path funnels through is
launch-runtime.py:execLeaseGatedRuntime→execRuntime('python3', [launch-runtime.py, --runtime X, --, X, …]);python3 …/launch-runtime.py --runtime claude -- claude …;resolveLaunchCommand→['mosaic','claude',…]→execLeaseGatedRuntime→launch-runtime.py;start-agent-session.sh+ orchestratorsession-run.sh→mosaic [yolo] <runtime>→ same.launch-runtime.pyis byte-identical to the head I fully proved in round 2 (git diff 6a3505df..HEADdoes not touch it). It enforces, on every path through it, with no flag/env/arg that skips it:register_anchoris unconditional before exec; the broker readsSO_PEERCREDand mints the session id server-side (register_anchorrefuses any clientsession_id→CALLER_SESSION_ID_REFUSED); the session id is validated (64-hex) and injected/overwritten into the child env; and on any registration failure (missing socket envKeyError, connect refusal/timeoutOSError, malformed/ok!=true/bad-id replyValueError) it returns 1 and never execs — the runtime does not launch at all (fail-closed). A tool that reaches the wrapper without a verified lease is then denied at the all-tools gate hook (authorize_tool→MUTATOR_UNVERIFIED/LEASE_EXPIRED→ exit 2). coord additionally throws if a custom Claude command is neithermosaic claudenormosaic yolo claude("…so lease registration cannot be bypassed") — closing the operator-config indirection. There is no fail-open branch inside the wrapper.RIDER 2 — GUARD evasion (adversarial): guard is a sound REGRESSION backstop, necessarily NOT adversary-complete
I ran the guard (
check-runtime-launches.py --root .→ 14 gated / 14 total, exit 0, wired into CI viapnpm test→test:framework-shell→… --root ../..) and then independently re-scanned the entire repo myself (all top-level dirs, all file types, systemd units, indirection forms). Result: zero real ungated claude/pi agent launchers exist now — every one of the 14 sites routes through the choke-point, including the fleet path (LAUNCH_COMMAND=(mosaic yolo "$MOSAIC_AGENT_RUNTIME"); its test assertsMOSAIC_AGENT_COMMANDis diagnostic-only, not a launch override). So the whole-class AC holds on this head, verified independently, not merely by the author's guard.The guard is regex static analysis, so it is — and provably cannot be — adversary-complete (deciding whether arbitrary code launches a binary is undecidable). Concrete evasion classes I confirmed by attacking its logic (all require committing source that survives review+merge+deploy):
packages/apps/plugins/tools; the repo also hasscripts/ deploy/ docker/ infra/ profiles/ briefs/ guides/ docs/ scratchpads/. A launcher placed there is not scanned. (I checked all of them — none exists today.)mosaic-ensure-excalidraw, which carries aclaude mcp addconfig subcommand — not an agent launch, but illustrates the skip), aMakefile,Dockerfile, or.serviceunit is not scanned.BIN=claude; exec "$BIN"), a non-runtime-named var in a process API (spawn(cmd, …)),sh -c "claude -p …",env claude -p,nohup claude …— none matchDIRECT_PATTERNS.gated_continuationmarks lines after any gated line ending in\as gated; a crafted gated-looking line ending in\above a real direct launch suppresses detection.Security assessment of these evasions: they are the malicious/careless-committer threat, whose irreducible backstop is exactly the ratified WI-7 server-side branch protection + Gate-16 independent review (a committer who can add an evasive launcher can equally edit the guard or the choke-point). Critically, guard evasion is not a runtime escalation path for a compromised model/tool — a live gated session cannot add-and-deploy a new source launcher to escape its own broker gate. So the runtime security control (choke-point + broker gate) is unaffected by guard incompleteness. The guard's real, delivered value is catching the accidental regressions that actually bit rounds 1–2 (the prdy/qa/coord launchers), and it does that for all natural/literal forms. This is a GO because security rests on the runtime enforcement (proven) + verified current completeness (my independent scan) + the ratified WI-7/review backstop for deliberate future evasion — not on the guard being adversary-complete, which is impossible. The guard should be understood as a regression backstop, not an adversary-complete proof; I flag its evasion classes explicitly so the coordinator holds that consciously.
RIDER 3 — broker-verified-only / revoke-first-promote-last / TTL-at-use / no-fail-open: intact
daemon.pyis byte-identical to my round-1 proof head77b137cc(empty diff), so the broker core is not regressed: the gate trusts only the kernel-anchoredSO_PEERCRED/broker-minted signal (no client-asserted principal or "verified" field exists); revoke-first (begin_verificationrevokes authority+tokens before minting) and promote-last (token consumption commits beforefinish_promotionmakes VERIFIED visible) hold under the WI-1broker_lockserialization; TTL is authoritative at use (time.monotonic() >= expires_at→ revoke+deny); and there is no fail-open branch (every error/exception denies;StateCommitUncertainterminates the daemon; volatile leases → broker restart = all UNVERIFIED). The prdy/qa/coord routes reuse the same daemon.Non-blocking observations (informational — NOT change requests)
--dangerously-skip-permissions(documented behavior; same premise asprevent-memory-write.sh); prdy/qa use--dangerously-skip-permissionsand thus share this assumption. Harness integrity = ratified T-C.Disclosed residuals (acknowledged, NOT flagged)
Harness-side tail-preserving middle-drop (planner ruling
b7bbb6ea); server-side branch protection (WI-7); ratified bounded T-A dual-observer-miss-within-TTL; T-C (hook/extension absence, runtime outside the gated launcher, ptrace/same-UID broker replacement).Independence (Gate 16 — by lane/process): author
ms-cr-wi1≠ code lensterra≠ this Opus security review — distinct principals on one Gitea account by lane/process; I am THE security gate for this WI. Earned from product code + my own repo scan, not the author's tests.tea/pr-review.sh approvebroken for this surface; this durable comment is the formal R3 RE-SECREV approval record. Reviewed head1792b793. Merge authority remains with the coordinator (Mos). No code edits, commits, pushes, or merges performed by this reviewer.OPUS SECREV R4 (MANDATORY, auth surface) — WI-2 #829 head
1eb77c17Verdict: 🔴 SECREV NO-GO / REQUEST CHANGES — reviewed exact head
1eb77c17f3147d4fa9944f77f1826243135b9cc0— 1 blocking finding (guard false-negative in a claimed-covered evasion class).Fresh pass, zero evidence transfer. Per the R4 calibration I did not reason abstractly — I built each evasion as a real launcher file under a scanned root and ran
check-runtime-launches.pyagainst each. That method surfaced a false-negative that reasoning would have rationalized away — exactly the R3 lesson.🔴 BLOCKING FINDING — guard misses runtime-variable indirection behind a command prefix
File:
packages/mosaic/framework/tools/lease-broker/check-runtime-launches.py:249(and:263-266)Class: the brief's "indirection … var-holds-launcher-then-exec" — expected RED, observed GREEN.
Exploit (constructed + run, not reasoned):
python3 check-runtime-launches.py --root <dir>→ exit 0, GREEN (no violation, not even listed). Variants also GREEN:exec $B(unquoted),command "$B",nohup "$B",env X=1 "$B". Only the bare first-token form"$B" -pandeval "$B"are caught (RED).Root cause — two complementary blind spots that intersect:
is_shell_direct_invocation(:229-251) correctly walks pastcommand/exec/nohup/env …prefixes, but its terminal check (:249) isPath(command[index]).name in {"claude","pi"}— it only resolves a literal binary, never a$varagainst the trackedruntime_variables.executes_runtime_variable(:260-267) correctly resolvesruntime_variables, but only foreval "$var"or a bare first-token^\s*"$var"— it never walks theexec/command/nohup/envprefixes.So a runtime-holding variable executed behind any command prefix (
exec "$B") satisfies neither path. The author's own regressions confirm the gap is untested:variable-exec.shuses the bare"$launcher" -pform (caught);env/command/nohup-prefix.shuse literalclaude/pi(caught). The product of the two dimensions — prefixed + variable — is never asserted, mirroring the R3 comment-marker miss (a form the tests didn't cover). CI is green (Woodpecker #1914,testOK) precisely because the test suite has this hole.Fix (localized): thread
runtime_variablesintois_shell_direct_invocationand, at the terminal check, also treat a command-position token that is a$var/${var}reference to a tracked runtime variable as a direct launch (strip${}/quoting, match the name). Equivalently, haveexecutes_runtime_variableskip the sameexec/command/nohup/env+assignment prefixes before matching the reference. Add regressions forexec "$v",exec $v,command "$v",nohup "$v",env A=1 "$v".Bounded severity (stated honestly so this is calibrated, not overstated): this is a guard/CI-regression-backstop false-negative, NOT a runtime authorization bypass. Even a committed launcher of this shape does not breach the AC at runtime, because (a) the dangerous variant is still caught by primitive-anchoring —
F="--dangerously-skip-permissions"tripsdangerous-primitiveon the assignment line, so yolo-mode can't ride this indirection; and (b) a non-dangerous ungatedclaude -pis fail-closed at runtime by the global~/.claudeall-tools hook (noMOSAIC_LEASE_SESSION_ID→GATE_UNAVAILABLE→ every tool denied) and by headless permission mode. No launcher of this shape exists in the tree today (14/14 real sites gated). But the R4 deliverable is guard completeness for these evasion classes, the brief lists this exact class as expected-RED, and the project standard (terra REQUEST-CHANGES on the R3 head for an analogous guard false-negative) is that a demonstrable, fixable false-negative in a claimed-covered class blocks. Consistency requires REQUEST-CHANGES. This is a narrow, clearly-fixable gap — I expect a fast turnaround.What DID hold (verified by construct-and-run + code read)
Evasion matrix — 9/10 constructed launchers correctly RED; 1 control correctly GREEN; 1 evasion GREEN (the finding):
# execLeaseGatedRuntime(thenexec claude -p)claude -ptrue \⏎claude -p;/&&/|+claude -pr=claude; eval "$r -p"$(claude -p)B=claude; exec "$B" -p--dangerously-skip-permissionsoutside choke-pointdangerous-primitive)exec claude -p/abs/claude,env … claude,nohup pi(literal)F="…"vardangerous-primitiveon assignment)launch-runtime.py --dangerous --runtime claude --RIDER B — primitive-anchored vs command-position; genuine defense-in-depth? Yes, genuinely complementary, and I confirmed it empirically: primitive-anchoring caught the dangerous-flag-via-variable case (c9) that command-position parsing would miss, while command-position caught literal-binary launches with no dangerous flag. The migration is real and correct:
--dangerously-skip-permissionsnow appears only inlaunch-runtime.py(CLAUDE_DANGEROUS_FLAG), injected via--dangerous, which assertsruntime=="claude"andPath(command[0]).name=="claude"before prepending it; the literal was removed fromlaunch.ts,claudex.ts, and prdy scripts (verified by diff). Primitive-anchoring is the more evasion-resistant of the two for the dangerous class (a plain substring is far harder to obfuscate than to fool a command parser), and it is not masking a command-position hole — each covers a class the other misses. The one form neither covers is the non-dangerous prefixed-variable launch (the finding).Choke-point integrity (RIDER 3) — sound
launch-runtime.pyremains the single wrapper; registration is unconditional before exec,SO_PEERCREDkernel-anchored + broker-minted session (refuses callersession_id), session id injected/overwritten, and fail-closed (return 1, no exec) on any registration failure.daemon.pyis byte-identical to my round-1 proof (git diff 77b137cc..HEADempty) → broker-verified-only trust, revoke-first/promote-last, TTL-at-use, and no-fail-open are intact and not regressed. Every real launch entry (claude/pi/claudex, yolo variants, prdy/qa) routes through the choke-point; an ungated launch anywhere is additionally fail-closed by the global all-tools hook. The runtime gate is sound — this NO-GO is strictly about the guard's static completeness in the class the round is chartered to close.CI
Green on this exact head (Woodpecker pipeline #1914, all steps OK). It does not catch the finding because the test matrix lacks the prefixed-variable case.
Disclosed residuals (NOT flagged)
Harness middle-drop (
b7bbb6ea); WI-7 branch protection; bounded T-A dual-observer-miss; T-C (hook/extension absence, runtime outside the gated launcher, ptrace/same-UID broker replacement).Independence (Gate 16 — by lane/process): author
ms-cr-wi1≠ code lensterra≠ this Opus security review — distinct principals by lane/process; I am THE security gate for this WI. Earned from product code + my own constructed-and-run guard tests, not the author's suite.tea/pr-review.sh approvebroken for this surface; this durable comment is the formal R4 SECREV record. Reviewed head1eb77c17. REQUEST CHANGES — do not merge until the prefixed-variable indirection is guarded and regressed. No code edits, commits, pushes, or merges performed by this reviewer.INDEPENDENT CODE RE-REVIEW R4 (terra) — WI-2 #829 head
1eb77c17Verdict: REQUEST CHANGES
Reviewed exact head
1eb77c17f3147d4fa9944f77f1826243135b9cc0against realorigin/mainmerge-base8ec67a1126adb0dcd4c3a2bf5525f3e239c0b201; prospective merge-tree construction succeeded (fc89c8a4). Author/reviewer separation is LANE/PROCESS-attested (sol/Pi author; terra separate reviewer session). 1 blocking finding.[BLOCKER — RIDER A/E]
packages/mosaic/framework/tools/lease-broker/check-runtime-launches.py:81-83,260-268misses a variable-held runtime executed throughexec. I added and removed this temporary production launcher:The guard exited 0 and retained
runtime launch inventory: 14 gated/14 total. The assignment is recorded, butexecutes_runtime_variable()only recognizes a bare$launcherat command position, notexec "$launcher"; the shell-direct parser likewise sees only the variable token. This is a real consequential Claude launch with no broker registration/session/hook, and the new primitive-only Rider B does not catch it because it has no dangerous flag. The same-line formlauncher=claude; exec "$launcher" -p xalso passes. Add permanent red regressions for both forms and make the command-position parser detect runtime variables afterexec(and equivalent prefixes) before treating the guard as load-bearing.Independently verified positives (not sufficient for approval)
;/&&/|chain, eval, marker-argument/echo, and primitive-outside-wrapper probes went red. R3 marker evasion is fixed; replaying the R3 guard against marker and continuation cases produced zero violations (red-before-fix evidence).--dangerously-skip-permissionsproduction ownership is limited tolaunch-runtime.py; the primitive probe elsewhere is red. The 14 current inventory entries are gated; real-socket acceptance is 49/49, including PRDY/QA no-broker denial and unverified-mutator denial. Claudex and broker ordering remain green.launch-runtime.py22/22 = 100%,mutator-gate.py22/22 = 100%, daemon delta 35/40 = 87.5%, guard 97%. The coverage does not exercise this false-negative.pnpm test43/43 Turbo tasks (Mosaic 72 files / 1,384 tests), typecheck 42/42, lint 23/23, format, diff check. R4 red-first commits95824572,88a709a9,64efdb44precede1eb77c17; no--no-verifyevidence. CI pipeline 1914 is terminal-success on this exact head.The mandatory Opus-SECREV is separate and is not replaced by this code review.
INDEPENDENT CODE RE-REVIEW R5 (terra) — WI-2 #829 head
04cc065cVerdict: APPROVE
Reviewed exact head
04cc065c2babae5bcc04b932d89d0bc3ff8546dfagainst realorigin/mainmerge-base8ec67a1126adb0dcd4c3a2bf5525f3e239c0b201; prospective merge-tree construction succeeded (e5d1a303). Author/reviewer separation is LANE/PROCESS-attested (sol/Pi author; terra separate reviewer session). 0 blocking findings.R5 stopping criterion: A/B/C = met
exec $var,command "$v",nohup "$v", andenv A=1 "$v", in both multiline and same-line assignment forms. Every one made the production guard red. The R5 permanent matrix covers those ten prefix/assignment forms, and replaying the R4 guard againstexec "$launcher"plus same-lineenvreturned zero violations (red-before-fix); current guard returns red. The fix structurally shares command-position token resolution for literals and tracked variable references rather than point-matching the prior spelling.launch-runtime.py; an alias-based exotic dangerous launcher made the guard red by primitive anchoring. I also confirmed a parser-residual non-dangeroustime claude -p xis not scanner-detected but the actual all-tools gate with no lease exits 2 (GATE_UNAVAILABLE) and denies the tool. Thus the unbounded exotic parser surface remains backstop-caught, as required.Verification
pnpm test43/43 Turbo tasks; Mosaic 72 files / 1,385 tests. Typecheck 42/42, lint 23/23, format, and diff check passed.launch-runtime.py22/22 = 100%;mutator-gate.py22/22 = 100%; WI-2 daemon delta 35/40 = 87.5%.91a4a983precedes04cc065c; no--no-verifyevidence.04cc065c.The mandatory Opus-SECREV remains a separate gate and is not replaced by this code review.
OPUS SECREV R5 (MANDATORY, auth surface) — WI-2 #829 head
04cc065cVerdict: ✅ SECREV GO — reviewed exact head
04cc065c2babae5bcc04b932d89d0bc3ff8546df— 0 blocking findings. A + B + C met → parser-complete declared on principle.Fresh pass, zero evidence transfer. Method held from R4: every claim below was constructed as a real launcher file and RUN against the guard / the hook — no abstract reasoning. My R4 NO-GO (18114) is resolved.
1. R4 finding structurally fixed (constructed + RUN — all RED)
The R4 prefixed-variable false-negative is closed structurally (
shell_command_tokenswalksexec/command/nohup/envprefixes in any nested order and resolves the command-position token;runtime_variable_referencematches$v/${v}against tracked runtime vars). Every combo I built is now RED:B=claude; exec "$B" -pB=claude⏎exec "$B"exec $B(unquoted)command "$B"/nohup "$B"/env X=1 "$B"${B}brace /P=pi; exec "$P"exec env A=1 "$B"/nohup env X=1 exec "$B"true && exec "$B"(non-first)Fix is structural, not a point-fix (any prefix×var×quoting×line combination). The only GREENs I could produce —
exec A=1 claude,exec A=1 "$B"— are bash non-launches: I verified in a real shell thatexec A=1 <cmd>yieldsexec: A=1: not found(exit 127) because bash treatsA=1as the command name onceexecis the command word. Correctly not flagged (nothing launches).2. Stopping-criterion evaluation (A / B / C) — pushed hard, by construction
(A) Realistic matrix RED-covered for the claimed class — MET. Every realistic prefix/var/quoting/line form in the claimed prefixed-variable class is RED. I did find realistic parser residuals in other classes on the acknowledged unbounded surface (all non-dangerous, all GREEN): backtick
x=`claude -p`,sh -c 'claude -p',echo x | xargs claude,${B:-claude}. These are not the class R5 claims to fix, and — critically — are proven backstop-caught (see B2). Non-blocking hardening recommendation: add backtick command-substitution to the parser for consistency, since the$(claude)form is already covered; this is the one spelling-inconsistency worth closing cheaply.(B) Backstops independently verified by construction — MET.
alias claude='claude --dangerously-skip-permissions'→ RED (dangerous-primitive);claude(){ command claude --dangerously-skip-permissions "$@"; }→ RED;F=--dangerously-skip-permissions; exec claude "$F"→ RED (primitive on the assignment) + RED (parser onexec claude); flag split across\continuation → RED (parser still sees literalclaude). I could not defeat primitive-anchoring while keeping the literal flag anywhere in source. The only way to hide the flag from primitive-anchoring is to remove the literal entirely (echo <b64> | base64 -d | sh), which then loses primitive coverage and parser coverage.mutator-gate.py) exactly as a PreToolUse invocation from an ungated claude (noMOSAIC_LEASE_SESSION_ID): consequentialBash→ exit 2GATE_UNAVAILABLE; read-onlyRead→ exit 2 (fail-closed even for read-only when ungated); broker socket set-but-unreachable → exit 2. So any parser-missed non-dangerous launch (backtick/sh-c/xargs/etc.) that runs claude under the standard/isolated config is denied every tool.echo <base64 of "CLAUDE_CONFIG_DIR=/hookless claude --dangerously-skip-permissions -p"> | base64 -d | sh(payload decodes to a dangerous claude launch — I verified the decode). It evades the parser (opaquebase64|sh), evades primitive-anchoring (literal absent from source), and evades the hook only by pointingCLAUDE_CONFIG_DIRat an attacker-pre-established hook-less config dir. That is not a defeat of a backstop within its scope — it is precisely the disclosed T-C residual: "a runtime executing outside the gated launcher … fully-rotted behavior," whose irreducible line is WI-7 server-side branch protection + independent review (any reviewer rejectsbase64 -d | shin a launcher). It also requires committed, obfuscated source (undecidable to detect statically) plus deliberate hook-less-config setup. Not a new blocker. Every residual that stays within the in-scope model (claude under the gated launcher / standard config) is caught by ≥1 robust backstop.(C) Fail-closed enum + broker core — MET. Guard on the real repo: 14/14 gated, exit 0.
daemon.pyis byte-identical to my round-1 proof (git diff 77b137cc..HEADempty) → kernel-anchored-only trust (no caller-asserted principal/verified), revoke-first, promote-last, TTL-at-use, and no fail-open are intact and not regressed.launch-runtime.py(the single choke-point) is unchanged since R4 — registration unconditional before exec,SO_PEERCRED/broker-minted, session injected, fail-closed on any registration failure. Every launch entry (claude/pi/claudex, yolo variants, prdy/qa, coord, fleet) reaches a consequential mutator only via the broker-verified lease; an ungated launch anywhere is additionally hook-denied (B2).Verdict logic applied
CI
Green on this exact head — Woodpecker pipeline #1915,
success.Disclosed residuals (NOT flagged)
Harness middle-drop (
b7bbb6ea); WI-7 branch protection; bounded T-A dual-observer-miss; T-C (runtime outside the gated launcher incl. obfuscatedbase64|sh+ hook-less config, hook/extension absence, ptrace/same-UID broker replacement).Independence (Gate 16 — by lane/process): author
ms-cr-wi1≠ code lensterra≠ this Opus security review — distinct principals by lane/process; I am THE security gate for this WI. Earned from product code + my own constructed-and-run guard/hook tests, not the author's suite.tea/pr-review.sh approvebroken for this surface; this durable comment is the formal R5 SECREV approval record and supersedes my R4 NO-GO (the R4 finding is fixed). Reviewed head04cc065c. Merge authority remains with the coordinator (Mos). No code edits, commits, pushes, or merges performed by this reviewer.