[Repo Assist] test(shared): add unit tests for ExecCommandToken and ExecEnvInvocationUnwrapper

[github-actions]

🤖 This PR was created by Repo Assist, an automated AI assistant.

Summary

Adds 66 direct unit tests for two security-critical components in the exec-approval pipeline that had no dedicated test coverage:

ExecCommandTokenTests (26 tests)

Covers BasenameLower, NormalizedBasename, and IsEnv:

• Plain tokens, paths with separators, extension stripping (.exe only)
• Whitespace handling, empty/null inputs
• env-lookalike rejection (envsubst, env_helper)
• Path-qualified env tokens (/usr/bin/env, C:\tools\env.exe)

ExecEnvInvocationUnwrapperTests (40 tests)

Covers Unwrap, HasModifiers, and UnwrapForResolution:

• Bare env COMMAND → strips env prefix
• VAR=val assignment stripping
• Known flags: FlagOnly (-i, --ignore-environment, -0, --null) and WithValue (-u, --unset, -c, --chdir) with inline and separate forms
• -- (ends option parsing, not a modifier) and - (clears env, is a modifier)
• Unknown flag → fail-closed (returns null)
• HasModifiers logic: VAR=val and flags return true; bare command and -- return false
• UnwrapForResolution: single and double env-wrapping, with assignments, max-depth guard, empty input

Why these tests matter

ExecCommandToken.IsEnv and ExecEnvInvocationUnwrapper.Unwrap sit at the entry of the exec-approval security pipeline. A misclassification (e.g. treating envsubst as env, or silently swallowing an unknown flag instead of failing closed) would bypass the allowlist evaluator. Direct unit tests make regressions visible immediately.

Test Status

dotnet test tests/OpenClaw.Shared.Tests ... --filter "ExecCommandTokenTests|ExecEnvInvocationUnwrapperTests"
Passed!  - Failed: 0, Passed: 66, Skipped: 0, Total: 66

Full suite: 8 pre-existing failures (infrastructure-level, unrelated to this PR); 2080 passing.

Generated by 🌈 Repo Assist, see workflow run. Learn more.

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@97143ac59cb3a13ef2a77581f929f06719c7402a

[clawsweeper]

Codex review: needs changes before merge. Reviewed May 26, 2026, 9:37 PM ET / 01:37 UTC.

Summary
The PR adds dedicated shared unit tests for ExecCommandToken and ExecEnvInvocationUnwrapper without changing production code.

Reproducibility: yes. for the review finding: the PR head source shows an empty xUnit [Fact] at the added bare-command test, so the issue is directly source-reproducible without running tests.

Review metrics: 1 noteworthy metric.

• Diff surface: 2 test files added, 0 production files changed. This keeps merge risk low and focuses review on test correctness rather than runtime behavior changes.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🐚 platinum hermit
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

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

Rank-up moves:

• Replace or remove the empty bare-command fact, then rerun the required validation commands.

Risk before merge

• This read-only review did not run the AGENTS-required build/shared/tray validation; the PR body reports targeted shared-test output, so CI or maintainer validation still needs to cover the full required commands.

Maintainer options:

1. Decide the mitigation before merge
   Keep the dedicated test coverage, replace or remove the empty fact, and rely on full repository validation before merge.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
A narrow automated repair can fix the single empty test method on the PR branch without product or security judgment.

Security
Cleared: The diff only adds shared unit tests and does not change production code, dependencies, workflows, secrets handling, or package resolution.

Review findings

• [P3] Assert or remove the empty bare-command test — tests/OpenClaw.Shared.Tests/ExecEnvInvocationUnwrapperTests.cs:21

Review details

Best possible solution:

Keep the dedicated test coverage, replace or remove the empty fact, and rely on full repository validation before merge.

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

Yes for the review finding: the PR head source shows an empty xUnit [Fact] at the added bare-command test, so the issue is directly source-reproducible without running tests.

Is this the best way to solve the issue?

No, not quite: adding focused tests is the right direction, but the empty fact should be replaced with a real assertion or removed before this is the best merge shape.

Full review comments:

• [P3] Assert or remove the empty bare-command test — tests/OpenClaw.Shared.Tests/ExecEnvInvocationUnwrapperTests.cs:21
  This [Fact] has an empty body, so xUnit will pass it without checking the bare-command path that the test name claims to cover. Please either delete it or replace it with a real assertion against the supported non-env entry point.
  Confidence: 0.97

Overall correctness: patch is correct
Overall confidence: 0.89

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

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

Label justifications:

• P3: This is low-risk shared test coverage with one small test-quality cleanup remaining.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal output for the targeted added tests passing, which is sufficient real behavior proof for this test-only change.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal output for the targeted added tests passing, which is sufficient real behavior proof for this test-only change.

Evidence reviewed

Acceptance criteria:

• ./build.ps1
• dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore
• dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore

What I checked:

• Empty test in PR diff: The added Unwrap_BareCommand_NoEnvPrefix_NotCalled test has an empty method body, so xUnit will pass it without asserting the named behavior. (tests/OpenClaw.Shared.Tests/ExecEnvInvocationUnwrapperTests.cs:21, 8ad5beaa696e)
• Current implementation context: Unwrap strips one env wrapper starting at argument index 1 and is called after callers identify an env token, so a bare-command case should be asserted through a supported entry point or removed. (src/OpenClaw.Shared/ExecApprovals/ExecEnvInvocationUnwrapper.cs:20, 0a164fc111b2)
• Existing main coverage context: Current main already has some env unwrapper coverage in ExecApprovalV2NormalizationTests, while this PR adds narrower direct coverage in two new test files. (tests/OpenClaw.Shared.Tests/ExecApprovalV2NormalizationTests.cs:69, 0a164fc111b2)
• Diff surface: The branch adds two test files with 293 inserted lines and no production file changes. (8ad5beaa696e)
• Area history: Local blame and path history point to commit 679f21c as the source of the current exec-token and env-unwrapper implementation in this checkout. (src/OpenClaw.Shared/ExecApprovals/ExecCommandToken.cs:7, 679f21cea559)
• Repository policy: AGENTS.md was read fully; its validation policy matters for this test PR, but this review remained read-only and did not run build or test commands that create artifacts. (AGENTS.md:1, 0a164fc111b2)

Likely related people:

• Mike Harsh: Local blame and path history attribute the current ExecCommandToken, ExecEnvInvocationUnwrapper, and existing normalization-test surface to the same merged implementation commit in this checkout. (role: recent area contributor; confidence: medium; commits: 679f21cea559; files: src/OpenClaw.Shared/ExecApprovals/ExecCommandToken.cs, src/OpenClaw.Shared/ExecApprovals/ExecEnvInvocationUnwrapper.cs, tests/OpenClaw.Shared.Tests/ExecApprovalV2NormalizationTests.cs)

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.

[clawsweeper]

ClawSweeper PR egg

✨ Hatched: 🥚 common Frosted Branchling

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

• Merged PRs are hatchable.
• Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
• Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🥚 common.
Trait: collects tiny proofs.
Image traits: location workflow harbor; accessory green check lantern; palette pearl, teal, and neon green; mood curious; pose peeking out from the egg shell; shell translucent glimmer shell; lighting golden review-room light; background smooth stones and checkmarks.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Frosted Branchling in ClawSweeper.

What is this egg doing here?

• Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
• The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
• Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
• The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
• Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.