fix(e2e): encode multiline sandbox scripts

[cv]

Summary

Fix the shared Vitest E2E sandbox client after the exact-head security-posture run proved that current OpenShell rejects newline-bearing command arguments. Multiline trusted scripts are now base64-encoded into one shell argument at the client boundary instead of requiring every scenario to invent its own minifier.

Related Issue

Contributes to #5919. Follow-up to merged #6010 and failed live run 28397462103.

Changes

• Preserve direct single-line sh -lc arguments.
• Encode multiline trusted scripts as a single newline-free eval argument and decode inside the sandbox.
• Reject NUL-bearing scripts before process construction.
• Add a round-trip contract test for the exact command argument.

Type of Change

• Code change (feature, bug fix, or refactor)
• Code change with doc updates
• Doc only (prose changes, no code sample modifications)
• Doc only (includes code sample changes)

Quality Gates

• Tests added or updated for changed behavior
• Existing tests cover changed behavior — justification:
• Tests not applicable — justification:
• Docs updated for user-facing behavior changes
• Docs not applicable — justification: internal E2E transport compatibility only; no user-facing product behavior changes.
• Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
• Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: pending independent review.
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

• PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
• Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
• Targeted tests pass for changed behavior
• Full npm test passes (broad runtime changes only)
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• npm run docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

Targeted verification:

• VITEST_MAX_WORKERS=16 npx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/e2e-clients.test.ts test/e2e-scenario/support-tests/security-posture.test.ts
• npm run typecheck:cli
• make check
• normal signed commit and push hooks

---

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of trusted sandbox shell scripts so multiline scripts are safely passed without raw line breaks.
  • Added stricter script validation to reject empty scripts and scripts containing NUL characters.
  • Updated shell execution behavior to reduce the risk of malformed command arguments when running scripts.

[coderabbitai]

📝 Walkthrough

Walkthrough

SandboxClient.execShell now encodes CR/LF-containing shell scripts as base64 before passing them to sh -lc, via a new sandboxShellArgument helper. trustedSandboxShellScript validation is updated to reject empty scripts and NUL bytes, with updated error wording, and corresponding tests are added.

Sandbox shell argument encoding

Layer / File(s)	Summary
sandboxShellArgument helper and execShell wiring test/e2e-scenario/fixtures/clients/sandbox.ts	Adds sandboxShellArgument which returns scripts unchanged when they contain no CR/LF, or wraps them in eval "$(echo ... | base64 -d)" when they do. Updates trustedSandboxShellScript to reject empty scripts and NUL bytes with revised error text. execShell now passes sandboxShellArgument(script) to sh -lc instead of the raw script.
Tests for encoding and validation test/e2e-scenario/support-tests/e2e-clients.test.ts	Asserts that execShell produces a single OpenShell-safe argument (no raw \r/\n) for multiline scripts, and updates the expected error message for empty-script validation to match the new wording.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

area: sandbox, feature

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: handling multiline sandbox scripts by encoding them for E2E execution.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/e2e-multiline-sandbox-scripts

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the codex/e2e-multiline-... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.

File	main	codex/e2e-multiline-... 813fa8e	+/-
nemoclaw/src/se...cret-scanner.ts	—	100%	—
nemoclaw/src/commands/slash.ts	—	100%	—
nemoclaw/src/li...bprocess-env.ts	—	100%	—
nemoclaw/src/bl...eprint/state.ts	—	98%	—
nemoclaw/src/onboard/config.ts	—	98%	—
nemoclaw/src/bl...int/snapshot.ts	—	97%	—
nemoclaw/src/bl...print/runner.ts	—	95%	—
nemoclaw/src/co...ration-state.ts	—	94%	—
nemoclaw/src/bl...ate-networks.ts	—	94%	—
nemoclaw/src/index.ts	—	94%	—

TypeScript / code-coverage/cli

The overall coverage in the codex/e2e-multiline-... branch is 68%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.

File	main	codex/e2e-multiline-... 813fa8e	+/-
src/lib/actions...all/run-plan.ts	—	80%	—
src/lib/state/o...oard-session.ts	—	79%	—
src/lib/actions...dbox/rebuild.ts	—	74%	—
src/lib/state/sandbox.ts	—	72%	—
src/lib/shields/index.ts	—	70%	—
src/lib/onboard/preflight.ts	—	69%	—
src/lib/actions...licy-channel.ts	—	59%	—
src/lib/onboard...er-gpu-patch.ts	—	59%	—
src/lib/policy/index.ts	—	52%	—
src/lib/onboard.ts	—	20%	—

---

_{Updated June 29, 2026 20:25 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.}

[cv]

Runtime evidence and acceptance gate:

• test(e2e): prove security posture parity for both agents #6010's exact-head two-agent run 28397462103 failed both agents after OpenShell rejected the multiline sh -lc argument from the shared fixture: InvalidArgument: command argument 2 contains newline or carriage return characters.
• This PR fixes that shared transport boundary and adds a round-trip test proving the argument contains no CR/LF while decoding to the original trusted script.
• A corrected exact-head two-agent run is now queued/running: 28400170844, head 813fa8ec5.

Do not treat the unit/CI gates alone as parity proof. This PR should merge only after both OpenClaw and Hermes jobs pass and their structured artifacts are inspected for the security-posture summary, cleanup failures, and credential-shaped leakage.

[coderabbitai]

🧹 Nitpick comments (1)

test/e2e-scenario/support-tests/e2e-clients.test.ts (1)

412-416: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Exercise the generated argument through sh -lc.

The regex + Buffer round-trip proves encoding shape, but not that the emitted shell argument actually runs at the shell boundary. Add an observable execution assertion for the captured argument so this test catches quoting/decoder portability regressions.

Suggested test strengthening

     const encoded = argument.match(/'([A-Za-z0-9+/=]+)' \| base64 -d/u)?.[1];
     expect(encoded).toBeTruthy();
     expect(Buffer.from(encoded ?? "", "base64").toString("utf8")).toBe(source);
+    const { stdout } = await execFileAsync("sh", ["-lc", argument]);
+    expect(stdout).toBe("ready\n");

Define execFileAsync with Node’s child_process.execFile + util.promisify near the existing test helpers/imports.

As per path instructions, “Review tests for behavioral confidence rather than implementation lock-in” and “Preserve real shell, process, installer, platform, and full-journey boundaries by invoking them from Vitest when they are the contract.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/e2e-scenario/support-tests/e2e-clients.test.ts` around lines 412 - 416,
The generated argument is only being checked for encoding shape and round-trip,
so the test in e2e-clients.test.ts should also execute the captured shell
argument through a real shell boundary. Add a small helper using execFileAsync
(built from child_process.execFile and util.promisify) near the existing test
utilities, then in the relevant assertion block run the captured argument with
sh -lc and verify it produces the expected source output. Keep the existing
regex and Buffer checks, but make the test assert observable execution behavior
from the emitted argument.

Source: Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/e2e-scenario/support-tests/e2e-clients.test.ts`:
- Around line 412-416: The generated argument is only being checked for encoding
shape and round-trip, so the test in e2e-clients.test.ts should also execute the
captured shell argument through a real shell boundary. Add a small helper using
execFileAsync (built from child_process.execFile and util.promisify) near the
existing test utilities, then in the relevant assertion block run the captured
argument with sh -lc and verify it produces the expected source output. Keep the
existing regex and Buffer checks, but make the test assert observable execution
behavior from the emitted argument.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1210b509-6cd9-4629-b435-29af4034632c

📥 Commits

Reviewing files that changed from the base of the PR and between 8296fb4 and 813fa8e.

📒 Files selected for processing (2)

• test/e2e-scenario/fixtures/clients/sandbox.ts
• test/e2e-scenario/support-tests/e2e-clients.test.ts

[github-actions]

PR Review Advisor (Nemotron Ultra) — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Resolve or justify PRA-3: Document exec() limitation: multiline strings as single argv elements not supported.
Open items: 0 required · 1 warning · 2 suggestions · 1 test follow-up
Since last review: 0 prior items resolved · 0 still apply · 3 new items found

Action checklist

• PRA-3 Resolve or justify: Document exec() limitation: multiline strings as single argv elements not supported in test/e2e-scenario/fixtures/clients/sandbox.ts:55
• PRA-T1 Add or justify test follow-up: Add test for shell metacharacters in multiline scripts to verify encoding neutralizes them
• PRA-1 In-scope improvement: Document why single quotes are safe for base64 payload in shell command in test/e2e-scenario/fixtures/clients/sandbox.ts:36
• PRA-2 In-scope improvement: Add test for shell metacharacters in multiline scripts to verify encoding neutralizes them in test/e2e-scenario/support-tests/e2e-clients.test.ts:378

Findings index

ID	Severity	Category	Location	Required action
PRA-1	Improvement	security	test/e2e-scenario/fixtures/clients/sandbox.ts:36	Add a comment on lines 38-40 explaining: base64 output contains no single quotes, so single-quote wrapping is injection-safe; printf '%s' prevents format string interpretation.
PRA-2	Improvement	tests	test/e2e-scenario/support-tests/e2e-clients.test.ts:378	Add a test case 'encodes shell metacharacters safely in multiline scripts' with a script containing quotes, backticks, $(), etc., verifying the decoded output matches the original.
PRA-3	Resolve/justify	correctness	test/e2e-scenario/fixtures/clients/sandbox.ts:55	Add a JSDoc comment to exec() noting that callers should not pass multiline strings as single argv elements; use execShell for scripts.

Review findings by urgency: 0 required fixes, 1 item to resolve/justify, 2 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-3 Resolve/justify — Document exec() limitation: multiline strings as single argv elements not supported

• Location: test/e2e-scenario/fixtures/clients/sandbox.ts:55
• Category: correctness
• Problem: The sandboxShellArgument function is only applied in execShell (trusted script path). The exec method passes commands directly as argv after '--', which is correct for array-style commands. However, there's no protection against a caller mistakenly passing a multiline string as a single argv element via exec(). This is a caller responsibility, but worth documenting.
• Impact: Misuse risk: a test author could pass a multiline string as a single element in the command array to exec(), causing OpenShell CLI to reject it with no clear error from the fixture.
• Recommended action: Add a JSDoc comment to exec() noting that callers should not pass multiline strings as single argv elements; use execShell for scripts.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check sandbox.ts exec() method (around line 55) for a JSDoc note about multiline handling.
• Missing regression test: Not applicable - documentation improvement only.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check sandbox.ts exec() method (around line 55) for a JSDoc note about multiline handling.
• Evidence: exec() method at sandbox.ts:55-63 accepts string[] command but has no documentation about multiline restrictions.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-1 Improvement — Document why single quotes are safe for base64 payload in shell command

• Location: test/e2e-scenario/fixtures/clients/sandbox.ts:36
• Category: security
• Problem: The sandboxShellArgument function wraps the base64-encoded script in single quotes: `eval "$(printf '%s' '${encoded}' | base64 -d)"`. While mathematically safe (base64 alphabet A-Za-z0-9+/= never contains single quotes), a comment documenting this invariant would prevent future regressions if encoding changes.
• Impact: Maintainability risk: future changes to encoding could accidentally introduce shell injection if the single-quote safety assumption is not documented.
• Suggested action: Add a comment on lines 38-40 explaining: base64 output contains no single quotes, so single-quote wrapping is injection-safe; printf '%s' prevents format string interpretation.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Read sandbox.ts lines 36-40 and verify a comment explains the base64 alphabet safety property and why printf '%s' is used.
• Missing regression test: No additional test needed; existing test 'encodes multiline shell scripts into an OpenShell-safe single argument' already verifies round-trip correctness.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: sandbox.ts:36-40 shows the encoding logic without explanatory comment for the single-quote safety invariant.

PRA-2 Improvement — Add test for shell metacharacters in multiline scripts to verify encoding neutralizes them

• Location: test/e2e-scenario/support-tests/e2e-clients.test.ts:378
• Category: tests
• Problem: The new multiline encoding test verifies the happy path but doesn't test edge cases: scripts containing single quotes, double quotes, backticks, $(), or other shell metacharacters that would be problematic without encoding. The base64 approach handles these correctly, but a test would document the threat model.
• Impact: Confidence gap: without a metacharacter test, a future change to the encoding scheme (e.g., switching to hex encoding) could silently lose the injection protection.
• Suggested action: Add a test case 'encodes shell metacharacters safely in multiline scripts' with a script containing quotes, backticks, $(), etc., verifying the decoded output matches the original.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Run the test suite and confirm the new edge-case test passes, proving metacharacters are neutralized by base64 transport.
• Missing regression test: Add test: 'encodes shell metacharacters safely in multiline scripts' with a script like `echo 'hello'; rm -rf /; $(malicious)` verifying round-trip equality.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: e2e-clients.test.ts:378-390 only tests a simple multiline script without shell metacharacters.

Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

• PRA-T1 Add test for shell metacharacters in multiline scripts to verify encoding neutralizes them — Add a test case 'encodes shell metacharacters safely in multiline scripts' with a script containing quotes, backticks, $(), etc., verifying the decoded output matches the original.

Since last review details

Current findings, using the urgency labels above:

PRA-1 Improvement — Document why single quotes are safe for base64 payload in shell command

• Location: test/e2e-scenario/fixtures/clients/sandbox.ts:36
• Category: security
• Problem: The sandboxShellArgument function wraps the base64-encoded script in single quotes: `eval "$(printf '%s' '${encoded}' | base64 -d)"`. While mathematically safe (base64 alphabet A-Za-z0-9+/= never contains single quotes), a comment documenting this invariant would prevent future regressions if encoding changes.
• Impact: Maintainability risk: future changes to encoding could accidentally introduce shell injection if the single-quote safety assumption is not documented.
• Suggested action: Add a comment on lines 38-40 explaining: base64 output contains no single quotes, so single-quote wrapping is injection-safe; printf '%s' prevents format string interpretation.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Read sandbox.ts lines 36-40 and verify a comment explains the base64 alphabet safety property and why printf '%s' is used.
• Missing regression test: No additional test needed; existing test 'encodes multiline shell scripts into an OpenShell-safe single argument' already verifies round-trip correctness.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: sandbox.ts:36-40 shows the encoding logic without explanatory comment for the single-quote safety invariant.

PRA-2 Improvement — Add test for shell metacharacters in multiline scripts to verify encoding neutralizes them

• Location: test/e2e-scenario/support-tests/e2e-clients.test.ts:378
• Category: tests
• Problem: The new multiline encoding test verifies the happy path but doesn't test edge cases: scripts containing single quotes, double quotes, backticks, $(), or other shell metacharacters that would be problematic without encoding. The base64 approach handles these correctly, but a test would document the threat model.
• Impact: Confidence gap: without a metacharacter test, a future change to the encoding scheme (e.g., switching to hex encoding) could silently lose the injection protection.
• Suggested action: Add a test case 'encodes shell metacharacters safely in multiline scripts' with a script containing quotes, backticks, $(), etc., verifying the decoded output matches the original.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Run the test suite and confirm the new edge-case test passes, proving metacharacters are neutralized by base64 transport.
• Missing regression test: Add test: 'encodes shell metacharacters safely in multiline scripts' with a script like `echo 'hello'; rm -rf /; $(malicious)` verifying round-trip equality.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: e2e-clients.test.ts:378-390 only tests a simple multiline script without shell metacharacters.

PRA-3 Resolve/justify — Document exec() limitation: multiline strings as single argv elements not supported

• Location: test/e2e-scenario/fixtures/clients/sandbox.ts:55
• Category: correctness
• Problem: The sandboxShellArgument function is only applied in execShell (trusted script path). The exec method passes commands directly as argv after '--', which is correct for array-style commands. However, there's no protection against a caller mistakenly passing a multiline string as a single argv element via exec(). This is a caller responsibility, but worth documenting.
• Impact: Misuse risk: a test author could pass a multiline string as a single element in the command array to exec(), causing OpenShell CLI to reject it with no clear error from the fixture.
• Recommended action: Add a JSDoc comment to exec() noting that callers should not pass multiline strings as single argv elements; use execShell for scripts.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check sandbox.ts exec() method (around line 55) for a JSDoc note about multiline handling.
• Missing regression test: Not applicable - documentation improvement only.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check sandbox.ts exec() method (around line 55) for a JSDoc note about multiline handling.
• Evidence: exec() method at sandbox.ts:55-63 accepts string[] command but has no documentation about multiline restrictions.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

[github-actions]

E2E Advisor Recommendation

Required E2E: None
Optional E2E: sandbox-operations-vitest

Dispatch hint: sandbox-operations-vitest

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• None. No merge-blocking E2E is required because the PR is tests-only/E2E-harness-only and cannot affect NemoClaw runtime user flows. The adjacent support test added in this PR directly verifies the command-shape regression for multiline scripts.

Optional E2E

• sandbox-operations-vitest (medium): Optional confidence check for the changed SandboxClient execShell path against a real OpenShell sandbox. This job exercises live sandbox operations and in-sandbox shell probes using the modified E2E fixture.

New E2E recommendations

• None.

Dispatch hint

• Workflow: .github/workflows/e2e-vitest-scenarios.yaml
• jobs input: sandbox-operations-vitest

[github-actions]

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: e2e-scenarios-all
Optional Vitest E2E scenarios: None

Dispatch required Vitest E2E scenarios:

• gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref>

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

• e2e-scenarios-all: The PR changes a shared Vitest E2E fixture client used by live scenarios, including sandbox shell execution argument handling. Shared fixture changes require the full Vitest scenario fan-out to validate all live-supported scenarios against the updated client behavior.
  • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref>

Optional Vitest E2E scenarios

• None.

Relevant changed files

• test/e2e-scenario/fixtures/clients/sandbox.ts
• test/e2e-scenario/support-tests/e2e-clients.test.ts

[github-actions]

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: No advisor follow-up required beyond maintainer review.
Open items: 0 required · 0 warnings · 0 suggestions · 0 test follow-ups

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

[github-actions]

Vitest E2E Scenario Results — ✅ All requested jobs passed

Run: 28400170844
Workflow ref: codex/e2e-multiline-sandbox-scripts
Requested scenarios: (default — all supported)
Requested jobs: security-posture-vitest
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job	Result
security-posture-vitest	✅ success

[cv]

Corrected live acceptance is complete for commit 813fa8ec5: run 28400170844 passed for both OpenClaw and Hermes.

Artifact inspection (not just job conclusions) confirms for both agents:

• host process runs as uid/gid 1001/1001
• /sandbox/.bashrc and /sandbox/.profile finish as mode 444, root:root
• /tmp/nemoclaw-proxy-env.sh finishes as mode 444
• in-sandbox configure commands are blocked with the expected resume guidance
• startup-log checks pass and credential-shaped scans contain only [REDACTED] values or documentation placeholders
• live inference passed and cleanup.json reports no failures

The artifacts also explicitly record the current PID 1 posture (uid=0, NoNewPrivs=1, capability masks and decoded dangerous-capability names). That is parity evidence, not a claim that PID 1 is already non-root/capability-free.

This closes the runtime regression introduced by passing multiline scripts as an OpenShell command argument. Ordinary PR CI is also green with no unresolved automated-review threads. Independent human review remains the merge gate.