fix: detect joined inline eval flags [AI]#101353
Conversation
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Codex review: needs maintainer review before merge. Reviewed July 7, 2026, 10:14 AM ET / 14:14 UTC. Summary PR surface: Source +182, Tests +100. Total +282 across 3 files. Reproducibility: yes. from source inspection and contributor proof. Current main lacks joined/clustered matcher paths while strict allow-always persistence depends on detectInlineEvalArgv, and the PR comment shows python3 -xcprint becoming one-shot on the current head. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Land the shared detector hardening with the added regressions after an exec/security owner explicitly accepts the one-shot approval compatibility change. Do we have a high-confidence way to reproduce the issue? Yes from source inspection and contributor proof. Current main lacks joined/clustered matcher paths while strict allow-always persistence depends on detectInlineEvalArgv, and the PR comment shows python3 -xcprint becoming one-shot on the current head. Is this the best way to solve the issue? Yes, this is the best implementation layer. Gateway, node-host, command explanation, and persistence paths already share detectPolicyInlineEval/detectInlineEvalArgv, so per-host workarounds would duplicate approval-boundary logic. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c8d95da14c5b. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +182, Tests +100. Total +282 across 3 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 (8 earlier review cycles)
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
ClawSweeper proof for current head 1952488 Behavior proved: joined allowlisted interpreter inline-eval flags are detected by the OpenClaw exec authorization path and remain one-shot under Environment: local PR checkout on Linux Node, plus GitHub Actions PR CI for the same head. Remote Testbox proof was unavailable because the installed Crabbox binary is 0.20.0 and the Testbox path requires 0.22.0. Command/artifact:
Evidence:
Protected boundary/sink: reusable allow-always persistence for an allowlisted interpreter path. The fixed scenario proves the joined inline-eval form does not reach the reusable approval sink; the benign control proves ordinary allowlisted script execution still can use the reusable approval path. Compatibility/operator note: this intentionally tightens Live gaps: no live gateway UI or remote Testbox artifact was available in this environment; the proof uses the production exec authorization adapter and redacted terminal output from the current checkout. |
|
@clawsweeper re-review |
|
Verification before merge: Behavior addressed: joined and clustered interpreter inline-eval flags are now detected by the shared exec authorization path so
Regression Risk: Best fix verdict: best/appropriate. The shared detector is the right location because gateway, node-host, policy, risk, and persistence callers already consume it; duplicating checks downstream would leave sibling paths easier to miss. User behavior change: |
* fix: detect joined inline eval flags * fix: avoid joined eval flag false positives * fix: detect clustered inline eval flags * fix: avoid cluster matcher string spread * fix: cover common eval flag clusters * fix: cover ruby perl eval clusters * fix: detect python x inline eval cluster * fix: detect numeric eval clusters * fix: cover ruby perl eval clusters
* fix: detect joined inline eval flags * fix: avoid joined eval flag false positives * fix: detect clustered inline eval flags * fix: avoid cluster matcher string spread * fix: cover common eval flag clusters * fix: cover ruby perl eval clusters * fix: detect python x inline eval cluster * fix: detect numeric eval clusters * fix: cover ruby perl eval clusters
What Problem This Solves
Fixes an issue where users who allowlist interpreter or runtime binaries could have joined inline-code flags treated as ordinary interpreter invocations when strict inline-eval hardening was enabled.
Why This Change Was Made
The inline-eval detector now recognizes glued single-letter eval flags such as
-c...and equals-joined long eval flags such as--eval=..., while keeping unrelated multi-letter flags exact-only unless their spec explicitly declares a prefix form. Regression coverage exercises the parser and the strict allow-always persistence boundary.User Impact
Operators using exec allowlists get the same inline-eval hardening for joined interpreter flag forms that already existed for separated flag/value forms. Normal script execution and unrelated options remain unchanged.
Evidence
node scripts/run-vitest.mjs src/infra/command-analysis/inline-eval.test.ts src/infra/exec-authorization-allowlist.test.tspassed locally with 2 shards and 48 tests.AI-assisted: yes.