feat(exec): deny-over-allow exec approval denylist (#6615)#101276
feat(exec): deny-over-allow exec approval denylist (#6615)#101276nicknmorty wants to merge 1 commit into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 20, 2026, 3:02 PM ET / 19:02 UTC. Summary PR surface: Source +1039, Tests +1608, Docs +73, Config +23, Generated 0, Other +815. Total +3558 across 60 files. Reproducibility: not applicable. as a feature request. The branch's focused tests and prior live receipts describe the intended behavior, but the exact current head still lacks the requested native integrated proof. Review metrics: 1 noteworthy metric.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Confirm whether an authorized Do we have a high-confidence way to reproduce the issue? Not applicable as a feature request. The branch's focused tests and prior live receipts describe the intended behavior, but the exact current head still lacks the requested native integrated proof. Is this the best way to solve the issue? Unclear. A deny-over-allow policy is a coherent extension of existing approvals, but the proposed elevated-full exception must be deliberately accepted because current docs only establish the older approval-bypass contract. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against cadad3b7bd12. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +1039, Tests +1608, Docs +73, Config +23, Generated 0, Other +815. Total +3558 across 60 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (58 earlier review cycles; latest 8 shown)
|
69973e7 to
15f2286
Compare
|
Pushed P1 blockers, each now wired and covered by focused tests on the exact path:
Two extra hardening changes surfaced while wiring the above:
Typecheck: Heads-up: one unrelated pre-existing gateway test ( @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Pushed Addressed
Verification
Still requires a human maintainer (not code fixes): the remaining P1s are acceptance decisions — maintainer/security sign-off on the persisted |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Follow-up: |
|
@clawsweeper re-review All review findings addressed on head P1 — behind main / mandatory delayed-authorization snapshot: merged current main (clean); the PR's denylist provenance tests now carry the canonical policy snapshot, and the snapshot contract shape is unchanged. P1 — hot-config revocation gap: the denylist authorization binding now carries an optional P2 — regressions: added Gateway + Node pending-approval config-tightening regressions (commit fails closed with Verification on exact head
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Merge conflict with main resolved; head is now dd441f9 (MERGEABLE).
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review P1 (companion revocation) addressed on head What changed since the last review:
Gates on exact head: Live proof (exact head, fresh build): full A–E PASS on a scratch gateway↔node pair — denylist STOP, deny round-trip, allow-once round-trip, approved run exactly-once, replay rejection. Scenario F (hot-added STOP during pending approval → revoked at dispatch): test-level PASS on the built head for both node and companion-delegation paths; no live macOS companion exists in this Linux proof lane, so the Swift compile/companion live leg is honestly deferred to macOS CI. Evidence: The remaining P1 (dual policy-surface ownership) is the maintainer decision your review already routes to @steipete — no code change claims to resolve that here. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Status update on the re-review findings (no re-review requested yet — proof work still in flight):
|
|
Follow-up on the macOS proof gap — head is now
|
|
@clawsweeper re-review Exact-head liveproof refresh for PR #101276:
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Mac-side proof refresh for current head Environment:
Native focused proof run:
Results: Scope note: I also attempted the repo @clawsweeper re-review |
|
@clawsweeper re-review Conflict with main is resolved and the branch is mergeable again at head
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Exact-head proof is now complete for
Please review the current exact head. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Refreshed onto current main at 4e56a46. The proposed policy contract is: tools.exec.denylist overrides ordinary full/off policy and durable trust; an already-authorized elevated-full invocation is the sole explicit break-glass bypass. Maintainer/exec-security owner confirmation of that boundary is still requested. @clawSean please rerun the redacted integrated macOS proof on this exact head: prompt, allow-once, exactly-once execution, replay rejection, and hot revocation. I will not request ClawSweeper re-review until that receipt and exact-head CI are available. |
|
Exact-head CI infrastructure note for 4af5c35: security-fast failed when the production advisory bulk request returned HTTP 503 Service Unavailable. No vulnerability result was produced; the other completed checks are green and macos-swift is still running. Contributor credentials cannot rerun this upstream workflow, so a maintainer rerun is requested. |
|
Exact-head macos-swift follow-up for 4af5c35: run 29766857713 reached the workflow configured 20-minute first-run timeout during the ordinary Swift test step. Release build, Swift lint, OpenClawKit opt-out, and OpenClawKit tests had passed; the integrated exec-approvals proof step was skipped before it ran, so this produced neither a Swift failure nor a proof result. The workflow grants 30 minutes on rerun attempts, but contributor credentials cannot rerun upstream CI. A maintainer rerun or the requested @clawSean current-head receipt is still required. |
|
@clawsweeper re-review Exact head b325cda now has 121/121 checks complete with 0 failures or pending checks. The dedicated read-only macOS 26/arm64 job checked out the literal contributor-fork PR SHA, asserted exact equality, and passed the native prompt, allow-once execution, absent durable grant, replay rejection, hot STOP-rule revocation, and final marker-count receipts. The PR body is refreshed with the exact-head evidence. The elevated-full break-glass boundary remains explicitly called out for exec-security owner review. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Exact head The literal-fork macOS 26.4 arm64 proof passed in job The PR body now carries the exact current-head SHA, run |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: feat(exec): deny-over-allow exec approval denylist (#6615) This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
|
@clawsweeper re-review Exact head |
|
🦞🧹 I asked ClawSweeper to review this item again. |
Fixes #6615. Supersedes #92456 (author inactive since 2026-06-12, no response to review, and conflicts with main). Full credit to @Hidetsugu55 for the STOP-list concept and dual-host enforcement direction retained here.
Latest state — canonical owner and exact-head proof
f267f56c69229c3c17bb3f951be0a01ec94792beci-gateis green.origin/main(details below).openclaw.jsontools.exec.denylistis now the single persisted owner of STOP rules. The earlier dual-persistence design was removed.What Problem This Solves
Exec approvals support allowlists, but operators cannot declare commands that must stop for explicit human review when the runtime is otherwise permissive (
security=full,ask=off, broad allowlist, or durable trust). #6615 asks for a deny-over-allow layer so patterns such asgit push --forcecannot auto-run merely because another policy says “allow.”Design and implementation
One canonical policy source
tools.exec.denylistconfiguration at the global and per-agent levels.openclaw.jsonis the only persisted owner of denylist rules.exec-approvals.jsonremains the approvals/allowlist store; it does not own or evaluate denylist policy.denylistkeys inexec-approvals.jsonare tolerated on read, warned once, and stripped on rewrite. Protocol writes reject the removed field.Shared matching and deny-over-allow semantics
security=denyremains the strongest policy. A denylist hit otherwise beatssecurity=full,ask=off, allowlist matches, and durableallow-alwaystrust.Gateway, node-host, and macOS companion enforcement
system.runevaluates the same config policy and returnsdenylist-hitwhen an unapproved match reaches the node.security=full/ask=off; model auto-review cannot clear a STOP rule.allow-alwaysdecision is downgraded to one-shot so it cannot mint durable trust.Dispatch-time revocation and TOCTOU hardening
Behavior proof
Control command:
git push --force origin main, already allowlisted and durably trusted atsecurity=full/ask=off.Focused regression coverage also proves that a config STOP rule added while an approval is pending revokes dispatch, including the macOS companion path.
Evidence
GitHub CI — exact head
f267f56c69229c3c17bb3f951be0a01ec94792bemacos-swiftlane, the dedicated literal-head macOS proof, and greenci-gate.Hosted literal-head macOS integration proof
nicknmorty/openclawat literal SHAf267f56c69229c3c17bb3f951be0a01ec94792be; read-only contents permission, credentials not persisted, tags/submodules disabled, clean SHA equality asserted before proof.macos-exec-approvals-proofin workflow run30134079508— success.Historical independent macOS/arm64 companion proof
Proof credit: @clawSean ran the exact-head Apple-silicon matrix and independently reproduced the two host-specific cases against
origin/mainto separate environmental behavior from the PR diff.Environment: macOS 26.5.1 (build 25F80), Darwin 25.5.0, Apple silicon
arm64; nodev24.16.0; repo-pinned pnpm11.2.2; Vitest4.1.9; frozen install clean.pnpm tsgo:core— exit 0pnpm tsgo:core:test— exit 0src/infra/exec-approvals-denylist.test.ts— 19 passedsrc/node-host/exec-policy.test.ts— 16 passedsrc/node-host/invoke-system-run.test.ts— 81 passedsrc/agents/bash-tools.exec-host-gateway.test.ts— 80 passed per projectsrc/config/config.schema-regressions.test.ts— 40 passedsrc/config/schema.help.quality.test.ts— 22 passedAggregate: 338 passed. The run reports four failures, but they are two unique pre-existing environment tests duplicated under
agents-coreandagents-support:/varvs/private/vartemporary-path canonicalization (macOS system symlink).cd .vs/usr/bin/cd .because macOS provides a PATH-resolvable/usr/bin/cd.Both tests exist verbatim on
origin/mainatff8a0159. Swapping the denylist source files for that baseline and rerunning the two cases reproduces them identically, proving they are host-environment behavior rather than PR regressions. Every denylist-specific assertion passes.Scope
system.run, and macOS companion enforcement.