fix(exec): fail closed on Windows shell wrappers in allowlist mode by plgonzalezrx8 · Pull Request #67655 · openclaw/openclaw

plgonzalezrx8 · 2026-04-16T12:10:27Z

Issue

In Windows allowlist mode, the wrapper guard only failed closed for cmd.exe-style invocations. That left a gap where other shell-wrapper transports such as powershell -Command could still be analyzed like a normal allowlisted command, which is the wrong security posture for inline shell payloads and command chaining.

Fix

block any Windows shell-wrapper transport in allowlist mode when shellWrapperInvocation is true, instead of limiting the guard to cmdInvocation
update the denial guidance so it explicitly calls out powershell -Command alongside cmd.exe /c
add regression coverage for the Windows PowerShell-wrapper path in src/node-host/exec-policy.test.ts

How to test

run: pnpm test src/node-host/exec-policy.test.ts src/node-host/invoke-system-run.test.ts -t "PowerShell|shell wrapper|cmd.exe wrappers|allowlist"
verify the new PowerShell-wrapper regression covers isWindows: true, shellWrapperInvocation: true, and cmdInvocation: false
confirm the decision is denied, windowsShellWrapperBlocked is set, and the error guidance mentions powershell -Command

greptile-apps · 2026-04-16T12:12:30Z

Greptile Summary

This PR closes an allowlist-bypass for Windows shell wrappers other than cmd.exe (e.g. PowerShell) by removing the cmdInvocation guard from windowsShellWrapperBlocked, and updates the denial message to mention powershell -Command.

The new regression test for the PowerShell path is correct, but the existing formatSystemRunAllowlistMissMessage unit test (lines 67–74 of exec-policy.test.ts) was not updated: it asserts .toContain(\"Windows shell wrappers like cmd.exe /c require approval\"), which is no longer a substring of the broadened message, so this test will fail as submitted.

Confidence Score: 4/5

Not safe to merge as-is — a stale test assertion will fail in CI.

The security fix itself is correct and the new PowerShell regression test is well written, but the existing formatSystemRunAllowlistMissMessage test was left with a substring that no longer appears in the updated message, which will cause a test failure. That P1 must be fixed before landing.

src/node-host/exec-policy.test.ts lines 67–74 — stale .toContain() assertion needs to match the new message text.

Comments Outside Diff (2)

src/node-host/exec-policy.test.ts, line 67-74 (link)

Stale assertion will fail after message change

The .toContain("Windows shell wrappers like cmd.exe /c require approval") assertion no longer matches because exec-policy.ts line 38-39 now inserts " or powershell -Command" between "cmd.exe /c" and " require approval", breaking the substring match. This test will fail as written.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/node-host/exec-policy.test.ts
Line: 67-74

Comment:
**Stale assertion will fail after message change**

The `.toContain("Windows shell wrappers like cmd.exe /c require approval")` assertion no longer matches because `exec-policy.ts` line 38-39 now inserts `" or powershell -Command"` between `"cmd.exe /c"` and `" require approval"`, breaking the substring match. This test will fail as written.



How can I resolve this? If you propose a fix, please make it concise.

src/node-host/exec-policy.ts, line 61 (link)

cmdInvocation is now an unused parameter

After dropping params.cmdInvocation from the windowsShellWrapperBlocked expression, this required parameter has no effect on the function's output. If the intent is to preserve the public API surface (callers still populate it), add a brief comment explaining that; otherwise remove it and update call sites to avoid dead code and potential lint warnings from no-unused-vars.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/node-host/exec-policy.ts
Line: 61

Comment:
**`cmdInvocation` is now an unused parameter**

After dropping `params.cmdInvocation` from the `windowsShellWrapperBlocked` expression, this required parameter has no effect on the function's output. If the intent is to preserve the public API surface (callers still populate it), add a brief comment explaining that; otherwise remove it and update call sites to avoid dead code and potential lint warnings from `no-unused-vars`.

How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/node-host/exec-policy.test.ts
Line: 67-74

Comment:
**Stale assertion will fail after message change**

The `.toContain("Windows shell wrappers like cmd.exe /c require approval")` assertion no longer matches because `exec-policy.ts` line 38-39 now inserts `" or powershell -Command"` between `"cmd.exe /c"` and `" require approval"`, breaking the substring match. This test will fail as written.

```suggestion
  it("adds Windows shell-wrapper guidance when blocked by Windows shell wrapper policy", () => {
    expect(
      formatSystemRunAllowlistMissMessage({
        shellWrapperBlocked: true,
        windowsShellWrapperBlocked: true,
      }),
    ).toContain("Windows shell wrappers like cmd.exe /c or powershell -Command require approval");
  });
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/node-host/exec-policy.ts
Line: 61

Comment:
**`cmdInvocation` is now an unused parameter**

After dropping `params.cmdInvocation` from the `windowsShellWrapperBlocked` expression, this required parameter has no effect on the function's output. If the intent is to preserve the public API surface (callers still populate it), add a brief comment explaining that; otherwise remove it and update call sites to avoid dead code and potential lint warnings from `no-unused-vars`.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(exec): block Windows shell wrappers ..." | Re-trigger Greptile}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d523e57e62

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-16T12:15:17Z

    return (
      "SYSTEM_RUN_DENIED: allowlist miss " +
-      "(Windows shell wrappers like cmd.exe /c require approval; " +
+      "(Windows shell wrappers like cmd.exe /c or powershell -Command require approval; " +


Preserve message compatibility with existing allowlist tests

The updated denial text in formatSystemRunAllowlistMissMessage inserts or powershell -Command between cmd.exe /c and require approval, which no longer matches the existing assertions that still expect the old contiguous phrase (src/node-host/exec-policy.test.ts:67-74 and src/node-host/invoke-system-run.test.ts:25-31). This commit therefore introduces deterministic test regressions unless the dependent checks are updated in the same change or the message keeps a backward-compatible substring.

Useful? React with 👍 / 👎.

clawsweeper · 2026-04-27T04:51:00Z

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: the merged payload-evaluation fix in #80978 covers the useful Windows shell-wrapper allowlist hardening on current main, while this branch's remaining blanket PowerShell fail-closed behavior would add compatibility risk.

Canonical path: Keep the merged payload-evaluation policy from #80978; open a narrow docs follow-up only if node docs need to describe the current PowerShell payload-evaluation behavior more precisely.

So I’m closing this here and keeping the remaining discussion on #80978.

Review details

Best possible solution:

Keep the merged payload-evaluation policy from #80978; open a narrow docs follow-up only if node docs need to describe the current PowerShell payload-evaluation behavior more precisely.

Do we have a high-confidence way to reproduce the issue?

Yes for source-level review: current main tests and allowlist code show the original bare PowerShell wrapper allowlist gap is covered by payload evaluation. I did not run a live Windows node exec scenario in this read-only review.

Is this the best way to solve the issue?

No. The merged payload-evaluation approach in #80978 is the better fix because it rejects bare wrapper matches without blanket-denying allowlisted inner payloads.

Security review:

Security review cleared: No concrete supply-chain regression was found; the security concern here is merge compatibility with the already-merged exec approval policy, not new dependencies or secret handling.

AGENTS.md: found and applied where relevant.

What I checked:

Current-main allowlist behavior: Current main rejects bare shell-wrapper executable allowlist matches unless the entry is argument-bound or a wildcard, then falls back to evaluating inline shell payloads against allowlist policy. (src/infra/exec-approvals-allowlist.ts:251, 4644e0c10256)
Current-main PowerShell regression coverage: Current main has regression coverage that a bare pwsh allowlist entry does not satisfy PowerShell inline-command wrapper forms. (src/infra/exec-approvals-analysis.test.ts:927, 4644e0c10256)
Merged superseding PR provenance: The merged PR Enforce inline shell wrapper payload matching [AI] #80978 changed exec allowlist, shell-inline parsing, wrapper resolution, and regression tests for inline shell-wrapper payload matching. (50f4440c9663)
Git-history provenance: Commit 50f4440 is the merged implementation commit for the superseding shell-wrapper payload matching work. (50f4440c9663)
Related PowerShell parser provenance: The earlier merged PR Recognize PowerShell -ec inline commands [AI] #80893 added PowerShell encoded-command inline parsing that overlaps this PR's PowerShell wrapper concern. (de47989731d3)
Live PR state: GitHub reports this PR is open, conflicting with current base, contributor-authored, and still missing real behavior proof beyond local test/build/check claims. (3baf42a36bcd)

Likely related people:

pgondhi987: Authored the merged current-main shell-wrapper payload matching work and the related PowerShell encoded-command parser fix that supersede this branch's useful hardening. (role: recent adjacent contributor; confidence: high; commits: 50f4440c9663, de47989731d3; files: src/infra/exec-approvals-allowlist.ts, src/infra/shell-inline-command.ts, src/infra/shell-wrapper-resolution.ts)
BradGroux: Prepared the current PR head, aligned tests/docs after early review feedback, and recorded local verification commands for the branch. (role: branch preparer and recent review context; confidence: medium; commits: 3baf42a36bcd; files: src/node-host/exec-policy.ts, src/node-host/exec-policy.test.ts, docs/nodes/index.md)

Codex review notes: model internal, reasoning high; reviewed against 4644e0c10256.

BradGroux · 2026-05-08T17:31:51Z

Maintainer update: rebased this onto current main and prepared the branch at 3baf42a36bcdef3a304f90e741fe13df09f03685.

What changed in the prepared head:

Kept the fail-closed Windows shell-wrapper allowlist behavior.
Aligned denial copy and tests for cmd.exe /c, powershell, and pwsh -Command wrapper runs.
Updated node docs/troubleshooting guidance for the broadened Windows wrapper approval behavior.
Added the required changelog entry with contributor attribution.

Verification run locally:

pnpm test -- src/node-host/exec-policy.test.ts src/node-host/invoke-system-run.test.ts -t "PowerShell|shell wrapper|cmd.exe wrappers|allowlist" (13 tests passed, 26 skipped by filter)
pnpm build
pnpm check

Waiting on fresh CI/ClawSweeper for this prepared head before merge.

clawsweeper · 2026-05-19T23:26:40Z

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?

The egg game starts only after the PR passes the real-behavior proof check.
Before that, no creature or rarity is rolled. The treat waits for real proof.
This is still just collectible flavor: proof affects review readiness, not creature quality.

openclaw-barnacle · 2026-06-06T04:48:17Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

clawsweeper · 2026-06-14T16:37:51Z

ClawSweeper applied the proposed close for this PR.

Action: closed this PR.
Close reason: duplicate or superseded.
Evidence: durable ClawSweeper review.
Coverage proof: PR B clearly covers PR A's useful hardening intent and is the merged canonical implementation; PR A's leftover docs/changelog details and blanket fail-closed behavior are either incidental or not independently desirable. Covering PR: Enforce inline shell wrapper payload matching [AI] #80978.

openclaw-barnacle Bot added the size: XS label Apr 16, 2026

plgonzalezrx8 changed the title ~~fix(exec): block Windows shell wrappers from allowlist bypass~~ fix(exec): fail closed on Windows shell wrappers in allowlist mode Apr 16, 2026

chatgpt-codex-connector Bot reviewed Apr 16, 2026

View reviewed changes

BradGroux mentioned this pull request Apr 29, 2026

WORKING: All Microsoft Issues and PRs (refresh) #74163

Draft

plgonzalezrx8 and others added 2 commits May 8, 2026 12:27

fix(exec): block Windows shell wrappers from allowlist bypass

d6edcca

Node host: document Windows shell wrapper approvals

3baf42a

BradGroux force-pushed the fix-allowlist-bypass-for-powershell-wrappers-upstream-2026-04-16 branch from d523e57 to 3baf42a Compare May 8, 2026 17:31

openclaw-barnacle Bot added docs Improvements or additions to documentation plugin: file-transfer triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 8, 2026

clawsweeper Bot mentioned this pull request May 12, 2026

Enforce inline shell wrapper payload matching [AI] #80978

Merged

25 tasks

openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 6, 2026

openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jun 7, 2026

clawsweeper Bot closed this Jun 14, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(exec): fail closed on Windows shell wrappers in allowlist mode#67655

fix(exec): fail closed on Windows shell wrappers in allowlist mode#67655
plgonzalezrx8 wants to merge 2 commits into
openclaw:mainfrom
plgonzalezrx8:fix-allowlist-bypass-for-powershell-wrappers-upstream-2026-04-16

plgonzalezrx8 commented Apr 16, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Apr 16, 2026 •

edited

Loading

Comments Outside Diff (2)

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Apr 16, 2026

Uh oh!

clawsweeper Bot commented Apr 27, 2026 •

edited

Loading

Uh oh!

BradGroux commented May 8, 2026

Uh oh!

clawsweeper Bot commented May 19, 2026 •

edited

Loading

Uh oh!

openclaw-barnacle Bot commented Jun 6, 2026

Uh oh!

clawsweeper Bot commented Jun 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

plgonzalezrx8 commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Fix

How to test

Uh oh!

greptile-apps Bot commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Comments Outside Diff (2)

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

clawsweeper Bot commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BradGroux commented May 8, 2026

Uh oh!

clawsweeper Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openclaw-barnacle Bot commented Jun 6, 2026

Uh oh!

clawsweeper Bot commented Jun 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

plgonzalezrx8 commented Apr 16, 2026 •

edited

Loading

greptile-apps Bot commented Apr 16, 2026 •

edited

Loading

clawsweeper Bot commented Apr 27, 2026 •

edited

Loading

clawsweeper Bot commented May 19, 2026 •

edited

Loading