Enforce inline shell wrapper payload matching [AI] by pgondhi987 · Pull Request #80978 · openclaw/openclaw

pgondhi987 · 2026-05-12T10:11:43Z

Summary

Problem: shell wrapper inline-command parsing missed some valid wrapper flag forms, and bare wrapper allowlist entries could satisfy approval before the embedded payload was evaluated.
Why it matters: exec approval should evaluate the command payload that will actually run, not only the wrapper binary.
What changed: added missing cmd and PowerShell flag recognition, restricted bare wrapper allowlist matches for inline payloads, and recursively evaluates extracted payloads against existing policy.
What did NOT change (scope boundary): no config, network, plugin, UI, docs, or migration surfaces changed.
AI-assisted: Yes.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Related: N/A
This PR fixes a bug or regression

Real behavior proof (required for external PRs)

Behavior or issue addressed: shell wrapper inline payloads are parsed and evaluated before approval can be satisfied by allowlist policy.
Real environment tested: Not run in this drafting step.
Exact steps or command run after this patch: Not run in this drafting step.
Evidence after fix: Added focused regression tests covering cmd inline flags, PowerShell encoded-command alias parsing, display extraction, and allowlist evaluation behavior.
Observed result after fix: Not run in this drafting step.
What was not tested: local test execution, live gateway behavior, and cross-platform runtime execution.
Before evidence: N/A

Root Cause (if applicable)

Root cause: shell wrapper parsing did not recognize every inline-command flag form used by supported wrappers, and allowlist matching could accept a bare wrapper executable before the extracted payload was evaluated.
Missing detection / guardrail: regression coverage did not include cmd dash-form inline flags, the PowerShell encoded-command alias, or a bare wrapper allowlist entry with an inline payload.
Contributing context (if known): wrapper parsing is shared by both approval policy and command preview logic.

Regression Test Plan (if applicable)

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file: src/infra/exec-approvals-analysis.test.ts, src/infra/exec-wrapper-resolution.test.ts, src/infra/shell-inline-command.test.ts, src/infra/system-run-command.test.ts
Scenario the test should lock in: cmd dash-form inline payloads are extracted; PowerShell encoded-command alias payloads are visible; bare shell wrapper allowlist entries do not approve arbitrary inline payloads; inner executable allowlist entries can still satisfy policy.
Why this is the smallest reliable guardrail: the affected behavior is contained in parser, display, and allowlist evaluation seams.
Existing test that already covers this (if any): N/A
If no new test is added, why not: N/A

User-visible / Behavior Changes

Bare allowlist entries for shell wrapper binaries no longer approve inline shell payloads by themselves. The payload must match policy, or the wrapper invocation must be covered by an argument-scoped allowlist entry.

Diagram (if applicable)

Before:
[wrapper argv] -> [wrapper allowlist match] -> [approved]

After:
[wrapper argv] -> [extract inline payload] -> [payload policy evaluation] -> [result]

Security Impact (required)

New permissions/capabilities? (Yes/No) No
Secrets/tokens handling changed? (Yes/No) No
New/changed network calls? (Yes/No) No
Command/tool execution surface changed? (Yes/No) Yes
Data access scope changed? (Yes/No) No
If any Yes, explain risk + mitigation: exec approval matching is stricter for shell-wrapper inline payloads; regression tests cover parser and policy behavior.

Repro + Verification

Environment

OS: Not run in this drafting step
Runtime/container: Not run in this drafting step
Model/provider: N/A
Integration/channel (if any): N/A
Relevant config (redacted): shell wrapper executable present in exec allowlist

Steps

Configure an exec allowlist entry for a shell wrapper executable.
Evaluate a wrapper invocation that passes an inline payload via a command-bearing flag.
Confirm approval is not satisfied by the bare wrapper entry and is instead determined by the extracted payload.

Expected

The inline payload is extracted and evaluated.
A bare wrapper allowlist entry does not approve arbitrary inline payloads.

Actual

Not run in this drafting step.

Evidence

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Added regression coverage in the touched test files; local execution was not performed in this drafting step.

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios: static review of changed parser, allowlist, and regression test coverage from the provided branch diff.
Edge cases checked: cmd dash-form inline flags, PowerShell encoded-command alias extraction, bare wrapper allowlist matching, and inner payload matching.
What you did not verify: test execution, live gateway behavior, and cross-platform runtime execution.

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? (Yes/No) Yes
Config/env changes? (Yes/No) No
Migration needed? (Yes/No) No
If yes, exact upgrade steps: N/A

Risks and Mitigations

Risk: existing configurations that rely on a bare shell wrapper allowlist entry for inline payloads may require a more specific allowlist entry.
- Mitigation: inner executable matching and argument-scoped wrapper entries remain available, and regression tests document the intended policy boundary.

clawsweeper · 2026-05-12T10:15:06Z

Codex review: needs real behavior proof before merge.

Summary
The PR tightens exec allowlist handling for inline shell-wrapper payloads, adds cmd and PowerShell flag parsing, preserves script argv handling, and adds focused regression tests.

Reproducibility: yes. for source-level reproduction: current main can match the wrapper executable allowlist before payload fallback on the inspected path. No high-confidence real-runtime after-fix reproduction is present because the PR body says it was not run.

Real behavior proof
Needs real behavior proof before merge: Needs real behavior proof before merge: add redacted terminal output, logs, screenshot/recording, or linked artifact proof to the PR body, then let ClawSweeper re-review or ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, ask a maintainer to comment @clawsweeper re-review.

Next step before merge
Needs human maintainer/security review because this is a protected-label, security-sensitive exec approval policy change with missing external real behavior proof; there is no narrow automated repair task from this review.

Security
Cleared: No concrete security or supply-chain regression was found; the diff tightens local exec approval parsing and tests without adding dependencies, workflows, network calls, or secret handling.

Review details

Best possible solution:

Land a maintainer-reviewed version that keeps wrapper payloads evaluated against policy, preserves wildcard and script-argv semantics, reconciles the related Windows wrapper PR, and includes redacted terminal or log proof from a real setup.

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

Yes for source-level reproduction: current main can match the wrapper executable allowlist before payload fallback on the inspected path. No high-confidence real-runtime after-fix reproduction is present because the PR body says it was not run.

Is this the best way to solve the issue?

Yes for the static code direction: the latest head addresses the prior wildcard and PowerShell alias concerns and adds targeted regression coverage. It is not merge-ready until real behavior proof and maintainer/security review are complete.

What I checked:

Protected live PR state and proof gap: The provided live GitHub context shows this PR is open at head a382af4, contributor-authored, labeled maintainer, and the PR body explicitly says the changed behavior was not run in a real environment. (a382af46b769)
Current main source-level behavior: Current main extracts a bindable shell-wrapper inline command, then calls matchAllowlist on the wrapper executable before inline fallback; a bare wrapper allowlist match can therefore satisfy approval before the payload path is evaluated. (src/infra/exec-approvals-allowlist.ts:429, db79b1cc5ea7)
PR allowlist hardening: The latest head adds matchExecutableAllowlistForSegment, rejects non-wildcard bare wrapper matches when inline payloads or wrapper args require binding, and preserves bare wildcard semantics. (src/infra/exec-approvals-allowlist.ts:394, a382af46b769)
PR inline fallback and tests: The latest head evaluates extracted inline payloads through analyzeShellCommand/evaluateSegments and adds regression coverage for cmd inline payloads, PowerShell trailing command tokens, PowerShell file argv preservation, and bare wrapper rejection. (src/infra/exec-approvals-analysis.test.ts:717, a382af46b769)
PowerShell and cmd parsing coverage: The latest head treats PowerShell command and command-with-args prefix forms as rest payload flags, keeps file flags distinct, recognizes encoded-command aliases, and recognizes cmd -c/-k inline forms. (src/infra/shell-inline-command.ts:21, a382af46b769)
Related open work: GitHub search found the related open PR fix(exec): fail closed on Windows shell wrappers in allowlist mode #67655, which covers a similar Windows shell-wrapper allowlist posture; this PR appears broader and active rather than obsolete.

Likely related people:

pgondhi987: Prior merged commits changed shell-wrapper allowlist parsing and PowerShell encoded-command recognition in the same exec approval surface, beyond authorship of this PR. (role: recent adjacent contributor; confidence: high; commits: fc065b2693bf, de47989731d3; files: src/infra/exec-approvals-allowlist.ts, src/infra/shell-inline-command.ts, src/infra/shell-wrapper-resolution.ts)
steipete: Current-main blame and recent history show repeated work around exec approval, command analysis, and shell-wrapper parsing surfaces touched by this PR. (role: recent area contributor; confidence: high; commits: 75f5d6d9b5f1, bd0e10a2f68f; files: src/infra/exec-approvals-allowlist.ts, src/infra/exec-command-resolution.ts, src/infra/shell-inline-command.ts)

Remaining risk / open question:

No after-fix real behavior proof is present; the PR body says runtime execution and cross-platform behavior were not run.
This is a security-sensitive command approval policy change with a protected maintainer label, so it needs explicit maintainer/security handling before merge.

Codex review notes: model gpt-5.5, reasoning high; reviewed against db79b1cc5ea7.

pgondhi987 · 2026-05-12T11:13:27Z

Not applicable to this automation stage; changelog/release-note and external real behavior proof requirements are handled outside auto-pr stages.

Quoted comment from @clawsweeper:

Codex review: needs real behavior proof before merge.

Summary
The PR changes exec allowlist evaluation so inline shell-wrapper payloads are evaluated instead of approved by bare wrapper matches, adds cmd/PowerShell inline parsing cases, and adds focused regression tests.

Reproducibility: yes. for source-level reproduction: current main still calls matchAllowlist on a shell wrapper before payload fallback for non-positional inline wrapper invocations. No high-confidence real-runtime after-fix reproduction is present because the PR body says it was not run.

Real behavior proof
Needs real behavior proof before merge: Missing: the PR body says no real environment, exact after-fix command, or observed result was run; the contributor should add redacted terminal output, logs, screenshot/recording, or linked artifact proof in the PR body so ClawSweeper re-reviews. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, ask a maintainer to comment @clawsweeper re-review.

Next step before merge
Protected maintainer label, missing external real behavior proof, and security-sensitive approval-policy findings make this a maintainer-review item rather than an automated repair lane.

Security
Needs attention: The diff tightens exec approval overall, but the PowerShell alias gap can leave trailing commands outside the approval policy after rebase.

Review findings

[P1] Evaluate all PowerShell command aliases as rest payloads — src/infra/shell-wrapper-resolution.ts:223

[P2] Preserve wildcard matches for inline wrapper payloads — src/infra/exec-approvals-allowlist.ts:407-410

Review details

Best possible solution:

Rebase and revise the exec approval hardening so wrapper payload evaluation preserves wildcard policy, handles every PowerShell command alias consistently, adds regression coverage for those cases, and includes redacted real terminal or log proof.

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

Yes for source-level reproduction: current main still calls matchAllowlist on a shell wrapper before payload fallback for non-positional inline wrapper invocations. No high-confidence real-runtime after-fix reproduction is present because the PR body says it was not run.

Is this the best way to solve the issue?

No: the intended direction is reasonable, but this diff misses current-main PowerShell alias forms and wildcard allowlist semantics. A safer fix should share the existing PowerShell inline/rest classification and evaluate or preserve wildcard payload policy explicitly.

Full review comments:

[P1] Evaluate all PowerShell command aliases as rest payloads — src/infra/shell-wrapper-resolution.ts:223
Current main accepts every prefix form of PowerShell -Command, and display code already treats tokens after that matched flag as significant. This branch only joins the rest of argv for -command, -c, and --command, so after rebase an invocation like pwsh -Com allowed.exe ; unlisted.exe can be evaluated against only allowed.exe and satisfy allowlist while trailing commands remain outside the policy check.
Confidence: 0.88

[P2] Preserve wildcard matches for inline wrapper payloads — src/infra/exec-approvals-allowlist.ts:407-410
The new helper drops every inline wrapper executable match that lacks an argPattern, which also drops the documented bare * wildcard. Because the POSIX fallback still skips single-command inline payloads, allowlist: [{ pattern: "*" }] would no longer allow a parsed wrapper command such as sh -c "tool --flag", despite current allowlist semantics saying * allows any parsed executable command.
Confidence: 0.84

Overall correctness: patch is incorrect
Overall confidence: 0.88

Security concerns:

[high] PowerShell aliases can bypass payload evaluation — src/infra/shell-wrapper-resolution.ts:223
Current main recognizes abbreviated PowerShell command flags, but the PR's rest-of-argv join only covers exact -command, -c, and --command; a valid abbreviated command flag can therefore approve only the first payload token while leaving trailing command tokens unevaluated.
Confidence: 0.86

What I checked:

Live PR state: The GitHub API shows this PR is open, authored by a contributor, labeled maintainer, and its body says no real environment, exact after-fix command, observed result, local test execution, live gateway behavior, or cross-platform runtime execution was run. (30de507ec38c)

PR diff scope: The PR diff adds matchExecutableAllowlistForSegment, blocks inline wrapper matches without argPattern, adds Windows inline fallback evaluation, recognizes cmd -c/-k, adds ; as a Windows unsupported token, and adds parser/allowlist tests. (src/infra/exec-approvals-allowlist.ts:383, 30de507ec38c)

Wildcard allowlist contract: Current main documents a bare * allowlist entry as allowing any parsed executable command, and the nearby test locks in that wildcard behavior for allowlist evaluation. (src/infra/exec-command-resolution.ts:351, 4bc47a3a7538)

PowerShell alias contract: Current main expands PowerShell inline-command flags across prefix forms, while display code treats tokens after the matched PowerShell command flag as meaningful trailing argv unless the flag is a rest-command form. (src/infra/shell-inline-command.ts:14, 4bc47a3a7538)

Current allowlist path provenance: Blame on the current wrapper allowlist decision path points to recent Peter Steinberger work, which is relevant ownership context for this exec approval boundary. (src/infra/exec-approvals-allowlist.ts:429, 22667fb096a1)

Related PowerShell parser provenance: Commit de47989 merged PowerShell -ec inline-command recognition into current main and overlaps this PR's PowerShell parser portion. (src/infra/shell-inline-command.ts:14, de47989731d3)

Likely related people:

steipete: Blame and recent local history put Peter Steinberger on the central exec allowlist and shell-wrapper code paths, including current wrapper allowlist behavior and earlier PowerShell encoded-command recognition. (role: recent area contributor; confidence: high; commits: 22667fb096a1, b3ded351e3f1, ce67f759304d; files: src/infra/exec-approvals-allowlist.ts, src/infra/shell-wrapper-resolution.ts, src/infra/shell-inline-command.ts)

jacobtomlinson: Authored the merged wrapper-carrier allow-always rejection work in the same exec approval boundary. (role: adjacent feature author; confidence: medium; commits: 9ec44fad390f; files: src/infra/exec-approvals-allowlist.ts, src/infra/exec-approvals-allow-always.test.ts)

pgondhi987: Authored the merged PowerShell -ec inline-command recognition work that current main now carries and that overlaps this PR's PowerShell parser hunk. (role: recent adjacent contributor; confidence: medium; commits: de47989731d3; files: src/infra/shell-inline-command.ts, src/infra/shell-wrapper-resolution.ts, src/infra/system-run-command.ts)

Remaining risk / open question:

No contributor-supplied after-fix real behavior proof is present for the changed exec approval path.

The PR overlaps current main's merged PowerShell parser work and needs a careful rebase before merge.

This changes command execution approval policy, so compatibility for wildcard and argument-scoped wrapper allowlist entries needs explicit maintainer review.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 4bc47a3a7538.

# Conflicts: # src/infra/exec-wrapper-resolution.test.ts # src/infra/shell-inline-command.ts # src/infra/shell-wrapper-resolution.ts # src/infra/system-run-command.ts

* fix: enforce inline shell wrapper payload matching * addressing review-skill * addressing codex review * addressing codex review * addressing claude review * addressing claude review * fix: complete shell wrapper allowlist handling * addressing codex review * addressing codex review * addressing codex review * addressing codex review * addressing codex review * addressing ci * docs: add changelog entry for PR merge

fix: enforce inline shell wrapper payload matching

d936c0a

openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels May 12, 2026

pgondhi987 added 2 commits May 12, 2026 10:17

addressing review-skill

94474da

addressing codex review

df046b0

openclaw-barnacle Bot added size: M and removed size: S labels May 12, 2026

pgondhi987 added 3 commits May 12, 2026 10:34

addressing codex review

30de507

addressing claude review

5e51ec6

addressing claude review

753ef11

openclaw-barnacle Bot added size: L and removed size: M labels May 12, 2026

pgondhi987 added 10 commits May 12, 2026 11:22

fix: complete shell wrapper allowlist handling

44f054a

Merge remote-tracking branch 'origin/main' into fix/fix-619

f750435

# Conflicts: # src/infra/exec-wrapper-resolution.test.ts # src/infra/shell-inline-command.ts # src/infra/shell-wrapper-resolution.ts # src/infra/system-run-command.ts

addressing codex review

ab47cb5

addressing codex review

9c61e43

addressing codex review

c59dd74

addressing codex review

7c4e10f

addressing codex review

76b8975

addressing ci

a382af4

Merge remote-tracking branch 'origin/main' into fix/fix-619

8c72b15

docs: add changelog entry for PR merge

1f1f267

pgondhi987 merged commit 50f4440 into openclaw:main May 12, 2026
21 of 22 checks passed

This was referenced May 13, 2026

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

Closed

[Feature] Discord Slash Commands: Per-channel command visibility for multi-bot setups #69629

Closed

clawsweeper Bot mentioned this pull request Jun 30, 2026

fix(exec): bind Windows allowlist execution path #98260

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Enforce inline shell wrapper payload matching [AI]#80978

Enforce inline shell wrapper payload matching [AI]#80978
pgondhi987 merged 16 commits into
openclaw:mainfrom
pgondhi987:fix/fix-619

pgondhi987 commented May 12, 2026

Uh oh!

clawsweeper Bot commented May 12, 2026 •

edited

Loading

Uh oh!

pgondhi987 commented May 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

pgondhi987 commented May 12, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Real behavior proof (required for external PRs)

Root Cause (if applicable)

Regression Test Plan (if applicable)

User-visible / Behavior Changes

Diagram (if applicable)

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Risks and Mitigations

Uh oh!

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

Uh oh!

Uh oh!

pgondhi987 commented May 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented May 12, 2026 •

edited

Loading