fix(exec): reject multi-line exec args with recovery guidance (#5980)#5991
Merged
Conversation
OpenShell's `sandbox exec` rejects any argv element containing a newline or carriage return, so multi-line commands (heredocs) fail with a low-level `InvalidArgument` error and no NemoClaw-specific recovery path. Detect offending arguments before dispatch and fail with a clear, non-zero NemoClaw error that names the 1-based argument position and points to supported workarounds (semicolon-joined statements or running a script file). Normal exec argv semantics are left unchanged; we only reject what OpenShell would reject anyway, with better guidance. Fixes #5980 Signed-off-by: Yimo Jiang <yimoj@nvidia.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Document that `nemoclaw <name> exec` rejects arguments containing a newline or carriage return and exits with status 2, with the semicolon and script-file workarounds. Regenerate the nemohermes variant page from the shared source. Signed-off-by: Yimo Jiang <yimoj@nvidia.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds client-side multiline argument rejection to sandbox exec before OpenShell dispatch, with injectable execution seams for tests. Related command tests, helper tests, and reference docs cover the new behavior and workaround guidance. ChangesMulti-line exec argument guard
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-5991.docs.buildwithfern.com/nemoclaw |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the branch is 96%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the branch is 68%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
Updated |
Collaborator
Author
Real worktree-CLI E2E evidence (#5980)Ran the compiled CLI ( 1. Reporter's failing scenario — multi-line $ node ./bin/nemoclaw.js sandbox exec bug5980test -- bash -lc $'cat <<EOF\nline1\nline2\nEOF'
error: command argument 3 contains a newline or carriage return, which OpenShell exec does not accept:
cat <<EOF\nline1\nline2\nEOF
Multi-line commands (for example heredocs) cannot be passed through exec argv. Instead:
- join statements with semicolons: nemoclaw bug5980test exec -- bash -lc "cmd1; cmd2"
- or write the script to a file in the sandbox and run it: nemoclaw bug5980test exec -- bash <script-path>
---> exit code: 2Previously this surfaced the bare OpenShell 2. Reporter's confirmed workaround still passes the guard (reaches OpenShell dispatch unchanged): $ node ./bin/nemoclaw.js sandbox exec bug5980test -- bash -lc "echo line1; echo line2"
Error: × status: NotFound, message: "sandbox not found" ...
---> exit code: 1The single-line command is not intercepted by the new guard — it proceeds to dispatch (and only fails here because 3. Carriage-return-only argument is also rejected: $ node ./bin/nemoclaw.js sandbox exec bug5980test -- printf $'a\rb'
error: command argument 2 contains a newline or carriage return, which OpenShell exec does not accept:
a\rb
...
---> exit code: 2 |
Contributor
PR Review Advisor (Nemotron Ultra) — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
Contributor
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@docs/reference/commands-nemohermes.mdx`:
- Line 472: The documentation in the NemoHermes commands page should use one
sentence per line and the literal `nemohermes` CLI name instead of the shorthand
`exec -- ...` fragments. Update the affected prose so each sentence stands on
its own line, and rewrite the workaround examples to show full `nemohermes`
command invocations; use the relevant command examples in this section to keep
the wording consistent with the page’s single-agent naming convention.
In `@docs/reference/commands.mdx`:
- Line 580: The paragraph in the shared command reference should be rewritten to
match the shared-doc example style by placing one sentence per source line in
the MDX content. Update the inline host CLI examples to use $$nemoclaw instead
of bare exec -- fragments, and keep the existing guidance about semicolon-joined
commands or running a script file inside the sandbox. Use the surrounding
commands/docs wording and the command example formatting pattern in this section
to locate and revise the text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7fb91386-47f8-4773-8057-27a33e71554b
📒 Files selected for processing (4)
docs/reference/commands-nemohermes.mdxdocs/reference/commands.mdxsrc/lib/actions/sandbox/exec.test.tssrc/lib/actions/sandbox/exec.ts
Address PR review feedback on the multi-line exec guard: - Security: stop echoing the offending argument's contents in the error. A multi-line value can carry pasted secrets, env files, or key material, and printing even a truncated preview risks leaking it into terminal/CI logs. Describe it neutrally (length + line count) instead; the 1-based position is enough to locate it. - Acceptance (#5980): add the issue's pipe-into-shell workaround. exec inherits stdin, so `printf '...' | nemoclaw <sb> exec -- bash` works. - Correctness: narrow the docs from "any argument" to "any command argument (the values after `--`)" so the promise matches what is validated; flag values such as --workdir are out of scope. - Docs: one sentence per source line and literal CLI names ($$nemoclaw / nemohermes) per the docs style guide. Tests: assert the secret payload is never printed, the pipe workaround is surfaced, and a normal single-line command still reaches OpenShell dispatch with unchanged argv. Signed-off-by: Yimo Jiang <yimoj@nvidia.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Collaborator
Author
Response to PR Review AdvisorPRA-2 (security — avoid echoing argument contents): Fixed in 3594786. PRA-4 (acceptance — pipe-into-shell workaround): Fixed in 3594786. PRA-3 (correctness — "any argument" vs command argv): Resolved by narrowing the docs to "any command argument (the values after PRA-1 (architecture — source-of-truth review):
PRA-T1–T8 (runtime/test follow-ups): Covered by (a) a hermetic |
Fix two CI failures introduced by the previous commit: - detect-private-key (static-checks): the redaction test used a literal `-----BEGIN PRIVATE KEY-----` fixture, which the secret scanner flagged. Use a neutral multi-line sentinel that still proves the argument contents are never echoed. - cli-test-shards: the dispatch test mocked `node:child_process` and the OpenShell runtime via `vi.mock`, which does not intercept the dynamic `require()` calls in `execSandbox`; on CI (no openshell on PATH) `getOpenshellBinary()` called `process.exit(1)`, so the assertion saw exit 1 instead of 0. Inject a `SandboxExecRunner` (mirroring the existing `WorkdirProbeRunner` seam) so the rejection test can assert no dispatch occurs without spawning, and cover the single-line pass-through via the pure argv builder instead of a brittle runtime mock. The real dispatch path stays covered by the worktree-CLI transcript on the PR. Signed-off-by: Yimo Jiang <yimoj@nvidia.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Collaborator
Author
Follow-up on PRA-T1 / PRA-T2 (runtime validation)These are now addressed by the regression tests on the current head (78dde86):
I intentionally did not unit-test the spawn itself through |
Collaborator
Author
Re: "PR review advisor unavailable" (low-confidence fallback) + PRA-T1This comment is the degraded fallback from a model that failed to load. The companion advisor run on this same head (78dde86) completed successfully and reports "No blocking advisor findings" (0 required · 0 warnings · 0 suggestions), so a manual re-review is not required. PRA-T1 (runtime validation): the changed behavior in
The docs changes ( |
Address PR Review Advisor PRA-T1/PRA-T2: add parser/command-level tests on SandboxExecCommand for the exact scenarios the advisor named — forwarding a multi-line heredoc verbatim to the action guard, and preserving --workdir while forwarding a single-line command unchanged. The real exit-2-before-dispatch guard remains asserted at the action level (execSandbox rejection test). Signed-off-by: Yimo Jiang <yimoj@nvidia.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Collaborator
Author
PRA-T1 / PRA-T2 — command/parser-level tests added (045b3df)Added the exact command-level scenarios you named, in
Together with the action-level guard/redaction/argv tests, this covers both the negative (heredoc rejected) and positive (single-line forwarded) command paths. |
wscurran
added
area: cli
Contributor
Address non-blocking PR Review Advisor items on #5991: - PRA-2: mark findMultilineExecArg @internal (test-only export). - PRA-3: replace the inert vi.mock of the OpenShell binary lookup (the dynamic require defeats vi.mock) with an injectable resolveBinary seam on execSandbox, and add a test that dispatches the single-line semicolon workaround through execSandbox, asserting the forwarded argv and exit code. - PRA-4: thread the existing workdir probe runner through execSandbox and add a test proving the workdir probe still runs after the multi-line guard for a single-line command (surfaces the workdir error and exits 1, not the multi-line error). The 4th execSandbox parameter (added earlier in this PR, unreleased) becomes an optional ExecSandboxDeps seam object; production callers pass three args and are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Collaborator
Author
Response to PR Review Advisor (Nemotron Ultra)All items were non-blocking ( PRA-2 (findMultilineExecArg not marked internal) — fixed. Added a PRA-3 (semicolon workaround test only validated argv-build, not dispatch) — fixed. Root cause: the prior PRA-4 / PRA-T6 (workdir validation after guard not tested) — fixed. Threaded the existing workdir probe runner through PRA-1 / PRA-T1–T3, T5 (live-sandbox integration test for the three workarounds) — justified, not added. A PRA-5 (duplicate doc block) — justified, not changed. The two blocks are not identical: CLI type-check clean; |
Contributor
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/actions/sandbox/exec.test.ts (1)
222-250: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a multiline +
--workdirordering assertion.Lines 222-250 only prove that single-line argv still reaches
probeWorkdir. They do not prove the new guarantee that a multiline argv exits with code2beforeprobeWorkdirruns when--workdiris also present, so that ordering could regress without this suite noticing.Suggested test shape
+ it("rejects a multi-line command before probing --workdir", async () => { + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`exit:${code}`); + }) as never); + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const run = vi.fn(() => ({ status: 0 })); + const probeWorkdir = vi.fn(() => ({ status: 0 })); + + await expect( + execSandbox( + "alpha", + ["bash", "-lc", "printf 'a\nb'"], + { workdir: "/workspace" }, + { run, resolveBinary: () => "openshell", probeWorkdir }, + ), + ).rejects.toThrow("exit:2"); + + expect(probeWorkdir).not.toHaveBeenCalled(); + expect(run).not.toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(2); + expect(errSpy.mock.calls.map((call) => String(call[0])).join("\n")).toContain( + "contains a newline or carriage return", + ); + });As per path instructions, "Review tests for behavioral confidence rather than implementation lock-in."
🤖 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 `@src/lib/actions/sandbox/exec.test.ts` around lines 222 - 250, The current execSandbox test only covers the single-line argv path, so add a new assertion in exec.test.ts that a multiline argv with --workdir exits with code 2 before probeWorkdir is called. Extend the existing execSandbox coverage by spying on probeWorkdir and run, then invoke execSandbox with a newline-containing command and a workdir option, and assert the process exits with the multiline error, probeWorkdir is never reached, and the command is not dispatched. Use execSandbox, probeWorkdir, and the existing exitSpy/errSpy setup to keep the check focused on ordering.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 `@src/lib/actions/sandbox/exec.test.ts`:
- Around line 222-250: The current execSandbox test only covers the single-line
argv path, so add a new assertion in exec.test.ts that a multiline argv with
--workdir exits with code 2 before probeWorkdir is called. Extend the existing
execSandbox coverage by spying on probeWorkdir and run, then invoke execSandbox
with a newline-containing command and a workdir option, and assert the process
exits with the multiline error, probeWorkdir is never reached, and the command
is not dispatched. Use execSandbox, probeWorkdir, and the existing
exitSpy/errSpy setup to keep the check focused on ordering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 539fb00e-1856-41fd-b71e-408e0a925a23
📒 Files selected for processing (2)
src/lib/actions/sandbox/exec.test.tssrc/lib/actions/sandbox/exec.ts
Address the re-run PR Review Advisor and CodeRabbit items on #5991: - PRA-4: add dispatch tests for the two remaining documented workarounds — stdin-pipe (`printf ... | exec -- bash`, script over stdin so argv is just `bash`) and script-file (`exec -- bash <script-path>`) — each injecting a success runner and asserting the forwarded argv and exit code. Together with the existing semicolon dispatch test, all three documented workarounds now have hermetic unit-level runtime validation. - CodeRabbit (review 4597559898): add an ordering assertion that a multi-line argv with --workdir exits 2 before the workdir probe runs (probe and runner never called), locking the guard-before-probe ordering. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Collaborator
Author
Response to re-run PR Review Advisor (Nemotron Ultra) + CodeRabbit (head
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/lib/actions/sandbox/exec.test.ts`:
- Around line 280-303: The current execSandbox test only asserts argv forwarding
via the run stub, so it does not verify the stdin-pipe workaround behavior.
Update the coverage around execSandbox and its CLI path to assert an observable
outcome at the public boundary: either add a higher-level CLI test for the
stdin-over-pipe flow, or make the existing test verify that the runner seam
preserves inherited stdin behavior rather than just mock-call arguments. Keep
the focus on execSandbox and the run/resolveBinary wiring so the workaround
cannot regress silently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 244e6629-f50c-4ddb-8dc1-4788556eb14e
📒 Files selected for processing (1)
src/lib/actions/sandbox/exec.test.ts
…ce-of-truth Address review follow-ups on the multi-line exec guard: - Add a dispatch test that exercises the default runner and asserts spawnSync inherits stdio, so a regression dropping inherited stdin (which the documented pipe workaround relies on) fails instead of passing on argv shape alone. - Reframe the stdin-pipe test name/comment to reflect it pins argv shape only. - Rename the semicolon workaround test to match the newer workaround tests. - Document the guard's invalid state, source boundary, regression coverage, and removal condition in exec.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
The guard regex intentionally covers \r and \n only because OpenShell rejects exactly "newline or carriage return characters". Unicode line separators (U+2028/U+2029) are valid argv OpenShell accepts, so broadening the pattern would reject commands OpenShell would otherwise run. Record this assumption so the scope is an explicit decision, not an oversight. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
- Add a findMultilineExecArg case asserting U+2028/U+2029 pass the guard, locking in the documented decision that the guard mirrors OpenShell's CR/LF-only rejection rather than a general line-break notion. - Comment describeMultilineArg's split so the CRLF/CR/LF handling and the "bare CR spans two lines" behavior are explicit for future maintainers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
…olon test - Move findMultilineExecArg, multilineExecMessage, and the execSandbox multi-line guard (#5980) suites into exec.multiline-guard.test.ts so the feature's coverage is self-contained and exec.test.ts stays focused on argv construction and the workdir probe. - Add a command-layer test that mirrors the action-layer semicolon workaround test, asserting the single-line semicolon command is forwarded to the action guard unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Rename the command-layer semicolon workaround test to "forwards the semicolon workaround to dispatch" so it mirrors the action-layer test name exactly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
…count - Assert a U+2028-bearing argument passes the guard and dispatches unchanged at the execSandbox boundary, confirming the CR/LF-only assumption end-to-end on the dispatch path (not just findMultilineExecArg). - Assert a trailing newline counts as a second (empty) line in the size description, pinning the documented line-count behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Merge origin/main into fix/5980-exec-newline-guidance. Resolved: - src/lib/actions/sandbox/exec.ts: keep main's async spawn dispatch (runSandboxExecChild + signal forwarding + OpenClaw permission cleanup, #6060); re-apply the PR's pre-dispatch multiline-argv guard on top, threaded through main's deps seam. The guard exits 2 before dispatch and never echoes argument contents. - docs/reference/commands.mdx: place the PR's agent-agnostic multiline guidance after main's AgentOnly exit-code blocks. - src/lib/actions/sandbox/exec.multiline-guard.test.ts: update the default-runner stdio-inherit test to assert main's async spawn (spawn(..., { stdio: 'inherit' })) instead of the retired spawnSync default. Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…removal trigger (#5991) Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28547848169
|
cjagwani
approved these changes
Jul 1, 2026
cjagwani
left a comment
cjagwani
left a comment
Contributor
There was a problem hiding this comment.
Approving.
The exec multiline-argv guard correctly resolves #5980 via the issue's option (b): a clear NemoClaw error that names the offending 1-based argument position and the three workarounds (semicolon-join / stdin-pipe / script-file), exits 2 before any OpenShell dispatch, and never echoes the argument contents (secret-safe) — verified against the live CLI. It only rejects what OpenShell already rejects (\r/\n); single-line argv passes through unchanged.
- Security review: clean (no content leak, exit-before-dispatch, no side effects).
- build:cli / typecheck:cli / biome clean; 51 exec unit tests pass.
- Recommended live E2E passed against a real sandbox —
sessions-agents-cli+cron-preflight-inference-local(run 28547848169). - The main-merge conflicts were resolved onto main's async-
spawndispatch (#6060), with the guard re-applied on top and the default-runner stdio-inherit test updated accordingly.
Note: the currently-red checks are run cancellations from unrelated concurrent main-merge activity on the branch (build-typecheck logged "operation was canceled"), not failures in this PR. A clean re-run on a settled head should go green.
3 tasks
ericksoa
pushed a commit
that referenced
this pull request
Jul 2, 2026
## Summary - Add the `v0.0.72` release-note section with links to the deeper docs pages for installer recovery, command diagnostics, inference, policy, and sandbox repair changes. - Document the custom preset `allowed_ips` guard for user-authored policy files. ## Related Issue None. ## Source summary - #6132 -> `docs/about/release-notes.mdx`: Summarizes installer and upgrade recovery before generic onboarding, with links to quickstart and lifecycle docs. - #6087 -> `docs/network-policy/customize-network-policy.mdx`: Documents that user-authored custom presets reject `allowed_ips` for ordinary endpoints; also summarized in release notes. - #5975 -> `docs/about/release-notes.mdx`: Summarizes safer curl-based inference probes that keep API keys out of process arguments. - #6044 -> `docs/about/release-notes.mdx`: Summarizes compact `channels status` configuration reporting. - #6096 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw EC2 metadata discovery disablement and links to security guidance. - #5980 and #5991 -> `docs/about/release-notes.mdx`: Summarizes `exec` multiline argument rejection and recovery guidance. - #6023 -> `docs/about/release-notes.mdx`: Summarizes registered-provider diagnostics for `inference set` failures. - #6074 -> `docs/about/release-notes.mdx`: Summarizes the refreshed NVIDIA Endpoints featured-model selection behavior. - #5969 -> `docs/about/release-notes.mdx`: Summarizes `credentials add` provider credential registration. - #6060 -> `docs/about/release-notes.mdx`: Summarizes mutable OpenClaw config permission restoration after `exec`. - #6134 -> `docs/about/release-notes.mdx`: Summarizes restored Tavily access for managed Python workflows. - #6089 -> `docs/about/release-notes.mdx`: Summarizes Hermes runtime version-scheme comparison during upgrade checks. - #6131 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw gateway watchdog recovery behavior. - #5976 and #5990 -> `docs/about/release-notes.mdx`: Summarizes prompt stdin EOF cancellation behavior during onboarding. - #5540 -> `docs/about/release-notes.mdx`: Summarizes clarified host-level and per-sandbox status command scope. - #5978 and #6018 -> `docs/about/release-notes.mdx`: Summarizes policy-denial log breadcrumbs in connect shells. ## Testing - `npm run docs:sync-agent-variants` - `npm run docs` - Commit hooks passed during `git commit`, including commitlint and gitleaks. - Pre-push hook passed during `git push`, including TypeScript CLI and package/tag version sync. ## Checklist - [x] Documentation updated. - [x] `npm run docs` completed with 0 errors and 1 existing Fern warning. - [x] No source code or generated build artifacts committed. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.72 covering improved installer recovery, clearer CLI diagnostics, safer inference setup and provider switching, better credential handling, stronger policy boundaries, and more robust runtime repair behavior. * Updated network policy guidance to clarify when `allowed_ips` can be used, including a specific exception for the sandbox-to-host bridge endpoint. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
nemoclaw <name> execforwards user argv straight to the OpenShell exec endpoint, which rejects any argument containing a newline or carriage return. Multi-line commands (for example abashheredoc) therefore failed with a low-levelInvalidArgumenterror and no NemoClaw-specific recovery guidance. This PR validates exec command argv before dispatch and fails with a clear, actionable NemoClaw error.Related Issue
Fixes #5980
Changes
findMultilineExecArg()andmultilineExecMessage()tosrc/lib/actions/sandbox/exec.ts, and wire them intoexecSandbox()so a newline/carriage-return command argument is rejected before spawning OpenShell.2, consistent with the existing usage-error path.nemoclaw <name> execindocs/reference/commands.mdx, and regenerate thecommands-nemohermes.mdxagent variant.Type of Change
Quality Gates
execargv path; change only adds pre-dispatch validation, never rewrites or weakens the command sent to OpenShell, and avoids echoing argument contents to prevent secret leakage.Verification
Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Targeted tests:
vitest run --project cli src/lib/actions/sandbox/exec.test.ts(29 passed, including the newline-guard regression tests, the secret-not-echoed test, and a dispatch-unchanged test). Full CLI project run with coverage: 4797 passed.npm run checks,npm run docs:check-agent-variants,tsc -p tsconfig.cli.json, and Biome all pass.Real worktree-CLI E2E (reporter workflow)
Ran the compiled CLI (
node ./bin/nemoclaw.js) from the worktree. The newline guard fires at the top ofexecSandbox, before any OpenShell dispatch or sandbox lookup, so this reproduces the reporter's exact failure deterministically.Reporter's failing scenario — multi-line
bashheredoc (now a clear NemoClaw error, exit 2, with no argument contents echoed):Previously this surfaced the bare OpenShell
× status: InvalidArgument, message: "command argument 2 contains newline or carriage return characters"with no recovery guidance.Reporter's confirmed single-line workaround still passes the guard and proceeds to OpenShell dispatch unchanged (only fails here because
bug5980testis not a real sandbox in this environment):This dispatch-unchanged behavior is also pinned by unit tests:
findMultilineExecArgreturns-1for the single-line form andbuildOpenshellExecArgsbuilds the forwarded argv["sandbox","exec","--name","bug5980test","--","bash","-lc","echo line1; echo line2"]unchanged. TheexecSandboxrejection test injects aSandboxExecRunnerand asserts it is never called when a multi-line argument is rejected, proving the guard fires before any OpenShell dispatch. (execSandboxresolves the OpenShell binary through a process-exiting lookup that is not present on CI runners, so the spawn itself is validated by the real worktree-CLI transcript above rather than a brittle node-builtin mock.)Signed-off-by: Yimo Jiang yimoj@nvidia.com
Summary by CodeRabbit
\n) or carriage return (\r) before execution begins (Unicode line separators are allowed).--workdir).nemohermes <name> execand$$nemoclaw <name> execto document the restriction and the supported alternatives for multi-line behavior (semicolon one-liners, stdin-piped scripts, or executing a script file).