Skip to content

feat(exec): deny-over-allow exec approval denylist (#6615)#101276

Open
nicknmorty wants to merge 1 commit into
openclaw:mainfrom
nicknmorty:fix/exec-approvals-denylist-6615
Open

feat(exec): deny-over-allow exec approval denylist (#6615)#101276
nicknmorty wants to merge 1 commit into
openclaw:mainfrom
nicknmorty:fix/exec-approvals-denylist-6615

Conversation

@nicknmorty

@nicknmorty nicknmorty commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

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

  • Exact PR head: f267f56c69229c3c17bb3f951be0a01ec94792be
  • GitHub CI: 121/121 checks complete on that head: 95 success, 25 skipped, 1 neutral, 0 failed, 0 pending. ci-gate is green.
  • Dedicated hosted macOS 26/arm64 proof checked out the literal contributor-fork PR SHA, asserted exact equality, and passed the real native prompt → allow-once → authenticated socket → execution/store lifecycle. It proved one execution, no durable grant, exact replay rejection, hot STOP-rule revocation, and final marker count one.
  • Earlier Apple-silicon macOS proof, run and baseline-isolated by @clawSean, remains useful historical coverage: TS-Go core + core-test clean; all denylist feature paths green; 338 focused assertions passed. Two unique host-PATH cases reproduced identically on the then-current origin/main (details below).
  • openclaw.json tools.exec.denylist is 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 as git push --force cannot auto-run merely because another policy says “allow.”

Design and implementation

One canonical policy source

  • Adds strict tools.exec.denylist configuration at the global and per-agent levels.
  • openclaw.json is the only persisted owner of denylist rules.
  • exec-approvals.json remains the approvals/allowlist store; it does not own or evaluate denylist policy.
  • Stray legacy denylist keys in exec-approvals.json are tolerated on read, warned once, and stripped on rewrite. Protocol writes reject the removed field.
  • This eliminates the mixed-version risk where an older approvals-file writer could silently erase the STOP list.

Shared matching and deny-over-allow semantics

  • Denylist entries reuse the existing exec glob matcher rather than introducing a second pattern language.
  • Global and per-agent config layers are normalized, de-duplicated, and unioned: a deny anywhere wins.
  • Exact commands, glob patterns, basename variants, resolved executable paths, and shell-wrapper inner payloads are screened.
  • If a configured denylist cannot safely analyze a command, execution fails closed into approval rather than bypassing the STOP rule.
  • security=deny remains the strongest policy. A denylist hit otherwise beats security=full, ask=off, allowlist matches, and durable allow-always trust.

Gateway, node-host, and macOS companion enforcement

  • Gateway-host exec screens the effective config denylist before allowlist/auto-run decisions.
  • Node-host system.run evaluates the same config policy and returns denylist-hit when an unapproved match reaches the node.
  • Caller-visible denylist hits enter the registered human approval path even at security=full / ask=off; model auto-review cannot clear a STOP rule.
  • Explicit human allow-once can authorize that invocation, but a denylist-approved allow-always decision is downgraded to one-shot so it cannot mint durable trust.
  • Approval-backed execution carries denylist provenance through the node/macOS companion boundary.

Dispatch-time revocation and TOCTOU hardening

  • Approval-backed commands bind a canonical execution plan rather than later mutable command/cwd/session input.
  • The runtime re-reads the current config denylist immediately before dispatch. A STOP rule added while approval is pending revokes the stale authority.
  • Revalidation covers local node execution and macOS companion delegation, including rules targeting the resolved executable path.
  • The macOS companion revalidates forwarded denylist provenance inside its locked execution commit and suppresses persistent approval for denylisted runs.

Behavior proof

Control command: git push --force origin main, already allowlisted and durably trusted at security=full / ask=off.

WITHOUT tools.exec.denylist
gateway host -> AUTO-RUN
node host    -> ALLOWED

WITH tools.exec.denylist = [{ pattern: "git push*--force*", reason: "history rewrite" }]
gateway host -> HUMAN APPROVAL REQUIRED
node host    -> DENIED (denylist-hit) unless the registered human approval is supplied

AFTER explicit allow-once
node host    -> ALLOWED ONCE; no durable trust is created

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

  • Head: f267f56c69229c3c17bb3f951be0a01ec94792be
  • 121 completed / 95 success / 25 skipped / 1 neutral / 0 pending / 0 failed
  • Includes Linux test matrices, generated-artifact checks, dependency/architecture checks, the ordinary 30-minute macos-swift lane, the dedicated literal-head macOS proof, and green ci-gate.

Hosted literal-head macOS integration proof

  • Runner: macOS 26.4, arm64.
  • Checkout: nicknmorty/openclaw at literal SHA f267f56c69229c3c17bb3f951be0a01ec94792be; read-only contents permission, credentials not persisted, tags/submodules disabled, clean SHA equality asserted before proof.
  • Native prompt observed; Allow Once accepted; harmless execution marker count reached exactly one.
  • No durable allowlist grant was created.
  • Exact authenticated-envelope replay was rejected; marker count remained one.
  • A newly effective STOP rule revoked delayed dispatch; final marker count remained one.
  • Job: macos-exec-approvals-proof in workflow run 30134079508 — 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/main to separate environmental behavior from the PR diff.

Environment: macOS 26.5.1 (build 25F80), Darwin 25.5.0, Apple silicon arm64; node v24.16.0; repo-pinned pnpm 11.2.2; Vitest 4.1.9; frozen install clean.

  • pnpm tsgo:core — exit 0
  • pnpm tsgo:core:test — exit 0
  • src/infra/exec-approvals-denylist.test.ts — 19 passed
  • src/node-host/exec-policy.test.ts — 16 passed
  • src/node-host/invoke-system-run.test.ts — 81 passed
  • src/agents/bash-tools.exec-host-gateway.test.ts — 80 passed per project
  • src/config/config.schema-regressions.test.ts — 40 passed
  • src/config/schema.help.quality.test.ts — 22 passed

Aggregate: 338 passed. The run reports four failures, but they are two unique pre-existing environment tests duplicated under agents-core and agents-support:

  1. /var vs /private/var temporary-path canonicalization (macOS system symlink).
  2. cd . vs /usr/bin/cd . because macOS provides a PATH-resolvable /usr/bin/cd.

Both tests exist verbatim on origin/main at ff8a0159. 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

  • Config schema/types/help/docs; shared denylist evaluation and authorization helpers.
  • Gateway exec host, node system.run, and macOS companion enforcement.
  • Protocol validation, generated baselines, and focused regression tests.
  • No unrelated behavior refactors and no second persisted policy owner.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: L labels Jul 7, 2026
@clawsweeper

clawsweeper Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 20, 2026, 3:02 PM ET / 19:02 UTC.

Summary
Adds tools.exec.denylist as a config-owned STOP list that requires one-shot human approval for matching exec commands across gateway, node-host, and macOS companion paths.

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.

  • Exec policy surface: 1 new persisted config field across global and per-agent scopes. The new tools.exec.denylist changes existing permissive exec behavior and must have an explicit upgrade and precedence contract.

Stored data model
Persistent data-model change detected: serialized state: apps/macos/Tests/OpenClawIPCTests/ExecApprovalsIntegratedProofTests.swift, serialized state: apps/macos/Tests/OpenClawIPCTests/ExecApprovalsStoreRefactorTests.swift, serialized state: src/infra/exec-approvals-store.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #6615
Summary: This PR is the active implementation candidate for the open denylist feature request; the earlier alternative PR overlaps but is not a safe canonical landing target.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted current-head native macOS evidence for the integrated approval flow and hot revocation.
  • Obtain an exec-security owner decision on whether elevated full bypasses an operator denylist.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: Earlier live and native evidence is substantial, but the PR body explicitly says it is historical; current head 4af5c352e806b57ce680e7dd68342ae04591cb39 still needs a redacted native macOS integrated receipt showing prompt, allow-once, exactly-once execution, replay rejection, and hot revocation. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] An operator may interpret a denylist as an unconditional STOP rule, while the branch intentionally preserves /elevated full as a bypass; that security-boundary contract needs explicit owner acceptance.
  • [P1] This adds a persisted configuration field with behavior across gateway, node-host, and macOS companion execution, but the current head lacks the requested integrated native proof after its refresh.

Maintainer options:

  1. Confirm break-glass and refresh proof (recommended)
    Have an exec-security owner explicitly accept the elevated-full precedence, then add redacted current-head macOS integrated proof for the pending approval and revocation path.
  2. Make denylist non-bypassable
    Revise the branch so a configured denylist remains approval-required even in elevated-full mode, with focused coverage for both gateway and node paths.
  3. Pause pending policy direction
    Pause this broad security-policy feature if no owner wants to define the permanent elevated-versus-denylist contract now.

Next step before merge

  • [P1] An exec-security owner must resolve the elevated-full precedence and the contributor must supply current-head native proof; neither is a safe autonomous repair.

Maintainer decision needed

  • Question: Should /elevated full remain an explicit break-glass bypass of the new operator-configured tools.exec.denylist, or should a denylist always require human approval?
  • Rationale: Current docs establish that elevated full skips exec approvals, but this PR introduces a new operator STOP-list meaning whose precedence over that bypass is a product security contract rather than a mechanical implementation choice.
  • Likely owner: steipete — The existing exec approvals and elevated-mode documentation tie this decision to the current policy contract.
  • Options:
    • Preserve documented break-glass (recommended): Keep the PR's elevated-full bypass, document it prominently, and require current-head native proof before merge.
    • Make STOP rules absolute: Change the denylist to override elevated full so every matching command requires a fresh human approval.

Security
Cleared: No new dependency, supply-chain, secret, or unbounded code-execution concern is evident in the reviewed patch; the remaining blocker is an explicit security-policy decision about the elevated-full bypass.

Review details

Best possible solution:

Confirm whether an authorized /elevated full session may override a configured denylist, then land only after a redacted current-head native macOS run demonstrates prompt, allow-once, exactly-once execution, replay rejection, and hot revocation.

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 changes

Label changes:

  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit. Replaced prior rating: 🦐 gold shrimp.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.

Label justifications:

  • P2: This is a valuable but unconfirmed security-policy feature rather than an active user-facing regression.
  • merge-risk: 🚨 compatibility: Adding a deny-over-allow config layer can alter established full/off and durable-approval behavior for existing operator configurations.
  • merge-risk: 🚨 security-boundary: The unresolved elevated-full bypass determines whether an operator-configured STOP list is advisory or non-bypassable.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit. Replaced prior rating: 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Earlier live and native evidence is substantial, but the PR body explicitly says it is historical; current head 4af5c352e806b57ce680e7dd68342ae04591cb39 still needs a redacted native macOS integrated receipt showing prompt, allow-once, exactly-once execution, replay rejection, and hot revocation. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +1039, Tests +1608, Docs +73, Config +23, Generated 0, Other +815. Total +3558 across 60 files.

View PR surface stats
Area Files Added Removed Net
Source 32 1409 370 +1039
Tests 9 1616 8 +1608
Docs 2 73 0 +73
Config 1 23 0 +23
Generated 3 7 7 0
Other 13 849 34 +815
Total 60 3977 419 +3558

What I checked:

  • Current-main gap: Current main documents only deny, allowlist, and full exec security modes and has no tools.exec.denylist policy knob; its node-host orchestration likewise contains no denylist evaluation or dispatch binding. Public docs: docs/tools/exec-approvals.md. (docs/tools/exec-approvals.md:1, cadad3b7bd12)
  • Existing elevated contract: Current documentation explicitly says /elevated full skips exec approvals, so the PR's proposed break-glass exemption is compatible with established behavior but still needs explicit confirmation for the new operator-authored STOP-list semantics. Public docs: docs/tools/exec-approvals.md. (docs/tools/exec-approvals.md:1, cadad3b7bd12)
  • Branch coverage: The branch adds focused regression coverage for deny-over-allow behavior, one-shot approval, replay rejection, and revocation across TS and macOS paths. (src/agents/bash-tools.exec.approval-id.test.ts:713, 4af5c352e806)
  • Current-head proof gap: The contributor's July 20 update states the available macOS receipt is historical and that the current-head macos-swift run timed out before its integrated proof step; the requested current-head prompt, allow-once, exactly-once, replay-rejection, and revocation proof is not yet posted. (.github/workflows/ci.yml:3070, 4af5c352e806)
  • Feature-history routing: The current exec-approvals documentation and policy-management direction are connected to the earlier shipped approvals work referenced by commit a9797214338b; current docs also preserve the elevated-full approval bypass. Public docs: docs/tools/exec-approvals.md. (docs/tools/exec-approvals.md:1, a9797214338b)

Likely related people:

  • steipete: Related shipped exec-approvals work and current documentation establish the full/off and elevated-full policy contract that this PR extends. (role: recent exec-policy contributor; confidence: medium; commits: a9797214338b; files: docs/tools/exec-approvals.md, docs/gateway/sandbox-vs-tool-policy-vs-elevated.md)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (58 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-14T00:02:10.486Z sha 9319e8a :: needs real behavior proof before merge. :: [P1] Protect STOP rules from mixed-version policy writers
  • reviewed 2026-07-15T10:03:12.315Z sha 18aac64 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-16T02:03:32.061Z sha 3a69fa8 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-16T06:35:03.373Z sha c3944c1 :: needs real behavior proof before merge. :: [P1] Move the denylist out of the approvals-file example
  • reviewed 2026-07-16T20:09:57.333Z sha 556e9bc :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-16T21:02:46.698Z sha 556e9bc :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-16T23:51:35.983Z sha 71d9e11 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-20T18:18:46.362Z sha 4af5c35 :: needs real behavior proof before merge. :: [P1] Keep elevated-full from bypassing configured STOP rules

@nicknmorty
nicknmorty force-pushed the fix/exec-approvals-denylist-6615 branch from 69973e7 to 15f2286 Compare July 7, 2026 02:59
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jul 7, 2026
@openclaw-barnacle openclaw-barnacle Bot added the scripts Repository scripts label Jul 7, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 7, 2026
@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui gateway Gateway runtime labels Jul 7, 2026
@nicknmorty

Copy link
Copy Markdown
Contributor Author

Pushed ce2b93d — this completes the deny-over-allow contract across every path flagged in the last review, plus a merge of latest main (clears the CONFLICTING/behind-main note).

P1 blockers, each now wired and covered by focused tests on the exact path:

  1. tools.exec.denylist config + gateway caller — added to the strict zod ToolExecBaseShape (non-empty-pattern refinement) and threaded as execConfigDenylist through agent-tools.tsbash-tools.exec.tsprocessGatewayAllowlist, unioned across global + per-agent layers. config.schema-regressions.test.ts (39) + bash-tools.exec-host-gateway.test.ts (54) green.
  2. Node-host system.runinvoke-system-run.ts now evaluates the effective denylist (openclaw.json global + per-agent + exec-approvals.json) and passes denylisted/denylistReason into evaluateSystemRunPolicy; auto-reviewer hard-blocked (human-only), unanalyzable commands fail closed. invoke-system-run.test.ts (36) green.
  3. Protocol write schemaExecApprovalsDenylistEntrySchema + denylist added to ExecApprovalsPolicyFields, so exec.approvals.set round-trips it. exec-approvals-validators.test.ts (8) green.

Two extra hardening changes surfaced while wiring the above:

  • Defect fix: sanitizeExecApprovalPolicy was silently dropping the defaults-layer denylist on every saveExecApprovals write — a routine policy write disarmed an operator STOP list. Now preserved.
  • Standalone gate: independent requiresDenylistApproval short-circuit in processGatewayAllowlist so the deny-over-allow guarantee can't regress if requiresExecApproval changes.

Typecheck: pnpm tsgo:core + pnpm tsgo:core:test both clean (the repo's TS-Go checker; stock tsc's TS4058 in config-io.ts is pre-existing on main). Refreshed real-behavior proof (gateway APPROVAL-REQUIRED + node-host denylist-hit, cleared by explicit allow-once) is in the PR body.

Heads-up: one unrelated pre-existing gateway test (uses a plan-backed enforced command…) fails only on hosts that symlink head to cargo-coreutils (/usr/bin/head vs /usr/lib/cargo/bin/coreutils/head); it's environmental and not touched by this diff.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 7, 2026
@openclaw-barnacle openclaw-barnacle Bot added the docs Improvements or additions to documentation label Jul 9, 2026
@nicknmorty

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Pushed da0acf1 addressing the actionable review findings and refreshing the branch against main.

Addressed

  • [P2] Document the denylist policydocs/tools/exec-approvals.md now documents tools.exec.denylist: both config layers (openclaw.json global/per-agent and the host exec-approvals.json), the glob matching semantics (*[^/]*, **.*, ?[^/], basename + bash -c "..." inner-payload screening, fail-closed on unanalyzable commands), deny-over-allow precedence on both gateway and node hosts, the YOLO/security=full+ask=off caveat, and the Control UI limitation (denylist is config/CLI-only, not editable in the card yet).
  • [P3] Unsafe optional-chain lint — the two no-unsafe-optional-chaining errors in src/node-host/invoke-system-run.test.ts are fixed by continuing the optional chain after the cast instead of dereferencing a possibly-undefined value. oxlint is clean on the file.
  • Merge conflict / CONFLICTING → MERGEABLE — merged latest origin/main and resolved the plugin-sdk-surface-report.mjs budget conflict, then set the budgets to the measured post-merge surface (plugin-sdk:surface:check passes): public exports 10474, callable 5258, deprecated 3273, and infra-runtime per-entrypoint 600. The delta is the new src/infra/exec-approvals-denylist.ts exports flowing through the existing infra-runtime barrel.

Verification

  • Focused tests green: exec-approvals-denylist (19), node-host/exec-policy (19), invoke-system-run (36), bash-tools.exec-host-gateway (54/55), exec-approvals-validators (8), config.schema-regressions (39), schema.help.quality (23), check-docs-mdx (3).
  • The single bash-tools.exec-host-gateway failure is the pre-existing environmental case already noted in the PR body (/usr/bin/head vs cargo-coreutils head on this host); it is independent of this diff.
  • plugin-sdk:surface:check and oxlint both pass.

Still requires a human maintainer (not code fixes): the remaining P1s are acceptance decisions — maintainer/security sign-off on the persisted openclaw.json/exec-approvals.json policy + protocol fields and the plugin-SDK surface exposure, and on the intentional full/off behavior change. Per the review's ownership note the likely owner is steipete.

@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@nicknmorty

Copy link
Copy Markdown
Contributor Author

Follow-up: check-test-types (pnpm tsgo:test) failed on 537c6f3 — two mock approval-context fixtures added on main lacked the denylist field this PR requires on Required<ExecApprovalsDefaults>. Fixed in 08074ed3f50 (test-only, two-line fixture change; implementation unchanged since 537c6f3). pnpm tsgo:test now exits 0 locally and the touched file passes 76/76.

@nicknmorty

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

All review findings addressed on head 42f665adb04:

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 resolveCurrentConfigDenylist hook; the locked pre-dispatch re-screen resolves the live tools.exec config denylist at commit time (captured pre-wait entries remain the fallback for runtimes without live config access). A STOP rule hot-reloaded into openclaw.json while an approval is pending now revokes the pending authority for both Gateway exec and Node system.run dispatch. Node lane fix included: the production node caller previously never injected the runtime config accessor, so node hosts now fall back to the same config module system.run loads — live re-screen works in production, not just tests.

P2 — regressions: added Gateway + Node pending-approval config-tightening regressions (commit fails closed with Exec approval changed before execution; node dispatch blocked), plus a node-dispatch gate regression.

Verification on exact head 42f665adb04:

  • Focused suites: 534 tests / 10 files pass (exec-approvals-store, exec-host-gateway, exec-host-node, invoke-system-run, invoke, system-run-approval-binding, gateway exec-approvals). tsgo core + test-types clean; oxlint/oxfmt clean.
  • Live gateway↔node proof rerun (scratch gateway + paired node, fresh build of the PR head, SHA-asserted): FULL A–E PASS — (A) unapproved denylisted system.run denied, (B) deny round trip honored, (C) allow-once round trip, (D) approved command executed exactly once with marker stdout, (E) replay rejected after allow-once consumption. Harness note: the proof driver now supplies systemRunPlan.policySnapshot, required by upstream's snapshot change — harness adaptation, not a PR behavior change.

@clawsweeper

clawsweeper Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@nicknmorty

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Merge conflict with main resolved; head is now dd441f9 (MERGEABLE).

  • Conflict: src/node-host/invoke-system-run.ts only. Upstream main introduced macApprovalDecision (approval source → null; explicit-approval with null decision → allow-once). Resolution keeps that logic and re-applies this PR's denylist guard on top: a denylisted allow-always approval is downgraded to allow-once before reaching the companion exec host. Both sides' semantics preserved; no scope change.
  • Gates on dd441f9: git diff --check clean; tsgo core exit 0; focused vitest green — src/node-host/invoke-system-run.test.ts, src/infra/exec-approvals-store.test.ts, src/agents/bash-tools.exec-host-node.test.ts (69/69). Lockfile unchanged vs main.
  • Live A–E behavior proof (gateway + node pending-approval revocation, hot-config STOP rule, denylist one-shot contract) passed in full on the immediate pre-merge head 42f665a; the only delta since is the main merge plus the one-file resolution above.

@clawsweeper

clawsweeper Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@nicknmorty

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

P1 (companion revocation) addressed on head d6adb53d28ae3f260aa6236477a13ccd2ed1bf02, with fresh exact-head live proof.

What changed since the last review:

  • Companion-path final denylist revalidation (90a83e3d804): node now re-screens the live effective denylist immediately before delegating to the macOS companion, so a STOP rule hot-added during an approval wait revokes pending authority on the companion path too. Denylist provenance rides the portable delayed-authorization contract as an additive optional field — the canonical ExecApprovalPolicySnapshot shape is unchanged and older companions are unaffected. Mirrored Swift-side check included; one-shot semantics preserved.
  • Regressions added: STOP rule tightened while node approval is pending → dispatch denied; same for the companion delegation path.
  • Merge with main (d6adb53): one conflict (generated plugin-sdk baseline), resolved by regeneration.

Gates on exact head: git diff --check, tsgo core+test, lint, import-cycles, config-schema, prompt snapshots, plugin-sdk surface check — all clean. Focused suites 471/473; the 2 failures are the pre-existing safe-bins allowlistSatisfied cases that reproduce on pristine origin/main (files untouched by this PR).

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: proofrun6-output.log md5 3aec2c69544a25be022ce0b9fcc7ce56.

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.

@clawsweeper

clawsweeper Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@nicknmorty

Copy link
Copy Markdown
Contributor Author

Status update on the re-review findings (no re-review requested yet — proof work still in flight):

  • Merged origin/main (1a8d9d5f150) to clear the drift/conflict; head is now 57f447b0800, mergeable again.
  • P1 (Swift companion skipped denylist validation before ExecApprovalsStore.commitExecution): fixed at this head. Forwarded config denylist rules absent from approvedRuleKeys now fail closed when they match, and unanalyzable bindings fail closed. Added Swift unit coverage for new-rule match, already-approved rule, and unanalyzable fail-closed paths.
  • Proof at this head (Linux controller): git diff --check clean; tsgo core + core-test exit 0; oxlint clean; import cycles 0; plugin-sdk/config-schema checks pass; focused tests 339 passed across 5 files, 0 failed.
  • Outstanding before we ask for re-review: exact-head native/live macOS companion proof (no macOS hardware on this side — relying on macOS CI for compile coverage in the meantime, and evaluating options for the live-proof demand), and the maintainer decision on single vs dual persisted denylist ownership (openclaw.json + exec-approvals.json) — happy to implement whichever ownership model maintainers prefer.

@nicknmorty

Copy link
Copy Markdown
Contributor Author

Follow-up on the macOS proof gap — head is now 0788275012b:

  • Reviewed the companion revalidation path against the TS matcher and confirmed a real divergence: the Swift side used fnmatch(..., FNM_PATHNAME), which does not treat ** as slash-crossing the way matchesExecGlob does (**.*, *[^/]*, ?[^/], anchored, trimmed).
  • Fixed at this head (co-authored with clawSean): replaced the matcher with TS-parity glob compilation, split denylist authorization + output limiter into ExecHostDenylistAuthorization.swift / ExecHostOutput.swift (socket file now 1487 non-comment lines, back under the 1500 gate), and added Swift tests for globstar slash-crossing, single-star slash bounding, and argv basename targets.
  • Native macOS proof: no local macOS hardware on this side, so the GitHub macOS runner CI on this exact head is the native execution evidence for the new Swift tests. Will request re-review once that job reports.

@nicknmorty

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Exact-head liveproof refresh for PR #101276:

  • Head proven: 37d83d803bebd7b987f8a1a926099b9e8d6cebba (fix/exec-approvals-denylist-6615, fetched directly from nicknmorty/openclaw; driver asserted the SHA before build).
  • Environment: isolated PC/WSL scratch gateway + scratch node on loopback :19850, scratch state ~/.openclaw-proof-pr101276; no live Gateway/config/approvals changes.
  • A-E live gateway->node proof: PASS. Denylist STOP rejected unapproved node system.run; pending approval deny returned decision: "deny"; pending approval allow-once returned decision: "allow-once"; approved run executed exactly once with stdout marker; replay returned approval required.
  • Scenario F revocation proof: PASS. Focused exact-head tests for hot denylist tightening passed, including resolved executable-path revocation, node pending-approval revocation, TS mac-delegation revocation, bash-tools async approval revocation, and approvals-store tightening coverage (17 passed / 293 skipped).
  • Teardown: scratch gateway/node killed; port 19850 verified clean.
  • Receipts: qa/liveproof5-20260712/RESULTS.md, driver.log md5 ef7fdb47711733e4faf32e2e854da8d4, proofrun6-output.log md5 899b51c5a22143cdd87925356d84fbf8, scenarioF-tests.log md5 fb32066b423a9972ed3a0ac02674171e.
  • Scope note: macOS companion live proof is not claimed by this PC lane; native/macOS proof remains CI / Sean lane.

@clawsweeper

clawsweeper Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawSean

Copy link
Copy Markdown
Contributor

Mac-side proof refresh for current head 37d83d803bebd7b987f8a1a926099b9e8d6cebba on real macOS hardware.

Environment:

  • Host: clawPop@airbook via ClawNode
  • macOS: 26.5.1 (25F80)
  • Arch/toolchain: arm64, Apple Swift 6.3.2 (swiftlang-6.3.2.1.108 clang-2100.1.1.101)
  • Checkout: /Users/clawPop/developer/openclaw-pr-101276-macos-proof, fetched from nicknmorty/openclaw:fix/exec-approvals-denylist-6615, asserted exact HEAD before proof

Native focused proof run:

  • Compiled exact PR source apps/macos/Sources/OpenClaw/ExecHostDenylistAuthorization.swift with a standalone Swift assertion harness via /usr/bin/swiftc.
  • Passed assertions for normalization/deduping, ** crossing slashes, * remaining slash-bounded, argv basename matching (/usr/bin/git -> git push*--force*), already-approved rule not re-prompting, newly current approvals-file rule revoking pending approval, and analysis-failure + new denylist failing closed.
  • Compiled exact PR source apps/macos/Sources/OpenClaw/ExecHostOutput.swift with a standalone Swift assertion harness via /usr/bin/swiftc.
  • Passed assertions for small-output preservation, truncation marker, valid UTF-8 tail preservation, max output byte cap, and escaped JSON response staying below the JSONL cap.

Results:

PASS: normalize trims, drops empty, dedupes
PASS: globstar crosses slash
PASS: single star stays slash bounded
PASS: argv basename target matches
PASS: already approved rule does not re-prompt
PASS: new local approvals-file rule revokes pending approval
PASS: analysis failure with new denylist fails closed
RESULT: pr101276 denylist harness passed on macOS
MAC_NATIVE_HARNESS_DONE=2026-07-13T00:51:43Z

PASS: small output preserved
PASS: large output gets marker
PASS: valid utf8 tail preserved
PASS: output stays under field byte cap
PASS: escaped json output stays under response byte cap
RESULT: pr101276 output limiter harness passed on macOS
MAC_OUTPUT_HARNESS_DONE=2026-07-13T00:52:07Z

Scope note: I also attempted the repo swift test --package-path apps/macos --parallel path on ClawNode. That local machine has Swift CLT but not the full Xcode macro plugin environment, so the build failed before these PR tests on unrelated SwiftUI macro resolution (SwiftUIMacros.EntryMacro / PreviewsMacros.SwiftUIView missing under shared Chat UI). The focused harnesses above intentionally avoid that local environment gap while still compiling and executing the exact changed Swift source on native macOS/arm64. GitHub's macos-swift and macos-node checks are green on the same head.

@clawsweeper re-review

@nicknmorty

nicknmorty commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Conflict with main is resolved and the branch is mergeable again at head 4230972e6d35c95261457b34f2987e09616c39c9.

  • Merged origin/main (non-force). Single conflict: apps/.i18n/native-source.json (generated i18n scan; line-number drift in ExecApprovalsStore.swift). Resolved by regenerating via native:i18n:sync; native:i18n:check passes with no drift.
  • Post-merge verification at this head: git diff --check clean; prompt snapshots regen clean; plugin-sdk api gen + surface check pass; config schema check pass; tsgo (core + projects) 0 errors; oxlint 0; import cycles 0.
  • Focused tests (workers=1): exec-approvals denylist/policy/store, exec-host, gateway exec-approvals, node invoke-system-run — 194/194 pass. The 2 failures in src/infra/exec-approvals.test.ts (safe-bins allowlist) reproduce identically on a plain origin/main worktree — pre-existing upstream, unrelated to this PR.
  • macOS companion proof for the Swift/native side: credit to @clawSean, who produced and posted the native/live macOS proof earlier in this thread.

@clawsweeper

clawsweeper Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@nicknmorty

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Exact-head proof is now complete for 3a69fa8311d5261ddcae5bea1101e2570de50897:

  • GitHub CI: 113/113 complete and green; zero failed or pending checks.
  • Real Apple-silicon macOS (not CI): tsgo:core and tsgo:core:test both exit 0.
  • Focused macOS matrix: 338 passed across denylist, node policy/system.run, gateway exec-host, config-schema, and schema-help coverage.
  • The two unique non-passing host-PATH cases are duplicated across two Vitest projects and reproduce identically on origin/main: /var -> /private/var canonicalization and macOS /usr/bin/cd resolution. They are independent of the PR; all denylist assertions pass.
  • PR body has been refreshed to describe the final single-owner design: openclaw.json tools.exec.denylist is canonical; exec-approvals.json no longer persists STOP rules.

Please review the current exact head.

@clawsweeper

clawsweeper Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@nicknmorty

Copy link
Copy Markdown
Contributor Author

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.

@nicknmorty

Copy link
Copy Markdown
Contributor Author

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.

@nicknmorty

Copy link
Copy Markdown
Contributor Author

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.

@nicknmorty

Copy link
Copy Markdown
Contributor Author

@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.

@clawsweeper

clawsweeper Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@nicknmorty

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Exact head 4f29cfd455936c0c48c86b90985cf7dd58036bfe is MERGEABLE with 121/121 checks complete: 95 success, 25 skipped, 1 neutral, 0 failed, 0 pending. openclaw/ci-gate, ordinary macos-swift, lint, and prod/test types are green.

The literal-fork macOS 26.4 arm64 proof passed in job 89012466645 with candidate head equal to tested SHA. Its validated receipts cover native prompt observed, allow-once accepted with marker count one, durable grant absent, authenticated replay rejected with marker count unchanged, hot STOP-rule revocation rejected, and final marker count one.

The PR body now carries the exact current-head SHA, run 29946269132, and proof receipt. The exec-security owner decision on the authorized elevated-full break-glass boundary remains explicitly pending; this re-review request does not represent that policy decision as settled.

@clawsweeper

clawsweeper Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper

clawsweeper Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

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.

@nicknmorty

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Exact head f267f56c69229c3c17bb3f951be0a01ec94792be is MERGEABLE and rebased onto upstream main 872ba27ff3633f9b8458c50920ed6c1ac7e3f15d. All 127 currently reported checks are complete: 98 success, 28 skipped, 1 neutral, 0 failed, 0 pending. The author-side max-lines baseline residue is removed and the ratchet passes. Local refresh proof passed 526 focused denylist/gateway/node/schema/workflow tests plus core/core-test and generated gates. Exact-head hosted proof job 89614475200 in run 30134079508 passed the literal-fork macOS 26.4 arm64 native prompt, allow-once, absent durable grant, replay rejection, hot STOP-rule revocation, and final marker-count lifecycle; ordinary macos-swift also passed. The PR body now carries the current exact head and hosted proof receipt. The exec-security owner decision on the elevated-full break-glass boundary remains explicitly pending and is not represented as settled.

@clawsweeper

clawsweeper Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling app: macos App: macos app: web-ui App: web-ui docs Improvements or additions to documentation gateway Gateway runtime merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. scripts Repository scripts size: XL status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Add denylist support for exec-approvals

2 participants