feat(onboard): declare runtime host mount support - #8368
Open
ericksoa wants to merge 28 commits into
Open
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
… into feat/read-only-host-mounts-8274
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa
added
area: onboarding
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (31)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (27)
📝 WalkthroughWalkthroughThe change adds runtime-provider capability checks for read-only host mounts, validates them during onboarding and session preparation, simplifies sandbox state handling, removes obsolete gateway and remote-access behavior, and updates related tests and documentation. ChangesSandbox runtime and state changes
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🔵 Low · up to Portable host-mount requests may receive a generic Docker-driver error instead of the documented provider-specific diagnostic. The request is still rejected safely, so the PR is mergeable with owner awareness or follow-up to make the delegated validator the sole failure path. Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant User
participant OnboardCommand
participant SessionBootstrap
participant RuntimeProviderRegistry
participant RuntimeProvider
User->>OnboardCommand: start onboarding with host mounts
OnboardCommand->>SessionBootstrap: prepare fresh or resumed session
SessionBootstrap->>RuntimeProviderRegistry: resolve provider capability
RuntimeProviderRegistry->>RuntimeProvider: check readOnlyHostMounts and host platform
RuntimeProvider-->>SessionBootstrap: capability or selection error
SessionBootstrap-->>OnboardCommand: create or reject session
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
8 tasks
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 905b4e6 in the TypeScript / code-coverage/cliThe overall coverage in commit 905b4e6 in the Show a code coverage summary of the most impacted files.
Updated |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Preserve the provider capability guard alongside the base branch's portable-profile changes. Declare Podman unsupported and fail closed before session mutation. Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
cv
requested changes
Aug 7, 2026
cv
left a comment
cv
left a comment
Collaborator
There was a problem hiding this comment.
This stacked capability declaration cannot be approved before parent PR #8280 resolves its host-mount canonicalization defect and passes review. Restack after the parent changes, then fix the failing exact all-agent activation and advisor checks and rerun the complete required set. Confirm that persisted resume rejection still occurs before durable session or runtime mutation on the final parent head.
![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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/lib/actions/sandbox/status-text.ts (2)
207-214: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBoth status renderers now propagate a throw from the shared host-mount normalizer.
normalizePersistedSandboxHostMountsthrows when persisted state holds an invalid mount. Both call sites invoke it without a guard, so one corrupt registry row breaks the status command that the user runs to diagnose that corruption.
src/lib/actions/sandbox/status-text.ts#L207-L214:printSandboxHostMountsthrows mid-render, soprintSandboxDetailsstops after the GPU section and never prints OpenShell, policies, or agent status. Catch the error and print a repair hint in place of the mount list.src/lib/actions/sandbox/status-snapshot.ts#L54-L56: the wrapper is called at line 650 duringbuildSandboxStatusReport, so--jsonstatus produces no document. Catch the error and report it as a field, or confirm the fail-closed behavior is intended.🤖 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/status-text.ts` around lines 207 - 214, Guard both call sites of normalizePersistedSandboxHostMounts: update printSandboxHostMounts in src/lib/actions/sandbox/status-text.ts (lines 207-214) to catch invalid persisted mounts and print an in-place repair hint so printSandboxDetails continues rendering; update the wrapper in src/lib/actions/sandbox/status-snapshot.ts (lines 54-56), used by buildSandboxStatusReport, to catch the error and expose it as a status field so JSON output remains valid.
392-410: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate stale status-output documentation
docs/deployment/deploy-to-headless-server.mdx:242-243anddocs/changelog/2026-08-07.mdx:30-32still state thatstatusprints SSH dashboard guidance. Update them to referencedashboard-url; retain the valid SSH hint for that command.🤖 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/status-text.ts` around lines 392 - 410, Update the stale status-output references in the deployment documentation and changelog to describe the dashboard-url guidance instead of SSH dashboard guidance, while preserving the valid SSH hint associated with the dashboard-url command.Source: Coding guidelines
src/lib/actions/sandbox/status-snapshot.ts (1)
565-578: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRestore a bounded post-recovery inference probe retry.
waitForRecoveredSandboxGatewayproves gateway health, nothttps://inference.local/v1/modelsreadiness. A single immediate probe can mark the route unprobed and set status exit code 1 while the route starts. Existing tests still require a 2-second retry and up to three probes. Restore this retry or add an equivalent route-readiness guarantee.🤖 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/status-snapshot.ts` around lines 565 - 578, Update the inference probe flow around waitForRecoveredSandboxGateway and buildSandboxInferenceRouteHealth to restore bounded readiness retries after gateway recovery: perform up to three probes with a 2-second delay between attempts, stopping early when the inference endpoint is healthy. Preserve fail-closed behavior and report unexpected probe errors through reportInferenceProbeError.src/lib/onboard/resume-config.ts (1)
15-24: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign
src/lib/onboard/resume-config.test.tswith the resume-conflict implementation. The test passescheckpoint.sandboxIdentityfor a pending sandbox and expects a conflict, butgetResumeSandboxConflictignores checkpoint identity and requires a completed sandbox step.🤖 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/onboard/resume-config.ts` around lines 15 - 24, Update resume-config.test.ts fixtures for getResumeSandboxConflict to represent a pending sandbox through checkpoint.steps.sandbox.status with a completed status, rather than checkpoint.sandboxIdentity. Keep the assertion expecting a resume conflict and align the test input with ResumeSessionLike’s steps shape.src/lib/state/registry.ts (1)
272-283: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPreserve CUA readiness invalidation in
updateSandbox. CUA readers still consumecuaRuntimeReadiness, and persistence retains valid records when qualification is enabled.Object.assignalso accepts caller-supplied readiness and writes explicitundefinedvalues. Restrict readiness writes to the canonical recorder, invalidate readiness when inference or runtime authority changes, and discard opaque records when qualification is disabled.🤖 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/state/registry.ts` around lines 272 - 283, Update updateSandbox to prevent caller-supplied cuaRuntimeReadiness, including explicit undefined, from being persisted; only the canonical readiness recorder may write it. Invalidate the stored readiness whenever inference or runtime authority changes, and remove opaque readiness records when qualification is disabled. Preserve the existing name validation, locking, and successful update behavior.
🧹 Nitpick comments (1)
test/support/status-flow-test-harness.ts (1)
206-206: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the running gateway status branch.
createStatusFlowHarnessalways mocksisSandboxGatewayRunningForStatusto returnfalse, so the status-flow suite does not assert theOpenClaw: runningoutput. Add one test that returnstrue, or document this coverage gap as intentional.🤖 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/support/status-flow-test-harness.ts` at line 206, Update createStatusFlowHarness and its status-flow tests to cover the true result from statusProcessRecovery.isSandboxGatewayRunningForStatus, asserting the expected “OpenClaw: running” output; preserve the existing false-result coverage.
🤖 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/status.test.ts`:
- Around line 96-105: Strengthen the assertions in the getSandboxStatusReport
test so they verify each returned host mount is a distinct object from the
corresponding entry in hostMounts, not just that report.hostMounts is a new
array. Preserve the existing value-equality assertion while adding element-level
identity checks that exercise the cloning behavior in
normalizePersistedSandboxHostMounts and the status snapshot wrapper.
- Around line 96-105: Expand regression coverage for invalid persisted host
mounts: in src/lib/onboard/host-mount/host-mount.test.ts, test non-array values
and terminal-control characters in source or target, asserting
normalizePersistedSandboxHostMounts throws rather than omitting unsafe mounts.
In src/lib/actions/sandbox/status.test.ts and
src/lib/actions/sandbox/status-flow.test.ts, add status-boundary cases asserting
the intended rejection behavior. In src/lib/state/onboard-session.test.ts, add
terminal-control metadata coverage and verify hasInvalidSessionHostMounts
preserves its fail-closed marker.
In `@src/lib/onboard/command.ts`:
- Around line 200-203: Remove the earlier portable-profile-specific failure
branch so portable --host-mount requests reach
requireReadOnlyHostMountRuntimeSupport() in the shown try/catch. Preserve the
validator’s error handling through fail(), making its declared capability
diagnostic the sole failure message for unsupported portable profiles.
In `@src/lib/onboard/docker-driver-gateway-config.ts`:
- Around line 59-64: The gateway ID derivation must retain its documented
per-user state-directory contract while restoring coverage for the active
sandbox namespace behavior. In src/lib/onboard/docker-driver-gateway-config.ts
lines 59-64, document that the state directory is always per-user and explain
why the process UID is intentionally excluded from the digest; in
src/lib/onboard/docker-driver-gateway-launch.test.ts lines 233-242, restore
public-entrypoint assertions that buildDockerDriverGatewayConfigToml emits
sandbox_namespace for Docker and that direct host launches set
NEMOCLAW_OPENSHELL_SANDBOX_NAMESPACE_ENV, without restoring the deleted
hasStateScopedSandboxNamespace path.
- Around line 87-92: Restore and export hasStateScopedSandboxNamespace, then
retain its existing callers in host-gateway-process.ts and uninstall run-plan.ts
so TypeScript compilation succeeds and scoped gateway shutdown remains
fail-closed. Do not alter the Docker-only supervisor_bin handling; Podman must
continue omitting that field.
In `@src/lib/onboard/session-bootstrap.ts`:
- Line 46: Update the race test around checkpointSandboxName to use a
synchronous updateSession mutator that returns the session or void, preventing
Promise values from being stored as mock session state. Assert
checkpointSandboxName’s immediate persistence, or remove the obsolete race test
if it no longer applies.
---
Outside diff comments:
In `@src/lib/actions/sandbox/status-snapshot.ts`:
- Around line 565-578: Update the inference probe flow around
waitForRecoveredSandboxGateway and buildSandboxInferenceRouteHealth to restore
bounded readiness retries after gateway recovery: perform up to three probes
with a 2-second delay between attempts, stopping early when the inference
endpoint is healthy. Preserve fail-closed behavior and report unexpected probe
errors through reportInferenceProbeError.
In `@src/lib/actions/sandbox/status-text.ts`:
- Around line 207-214: Guard both call sites of
normalizePersistedSandboxHostMounts: update printSandboxHostMounts in
src/lib/actions/sandbox/status-text.ts (lines 207-214) to catch invalid
persisted mounts and print an in-place repair hint so printSandboxDetails
continues rendering; update the wrapper in
src/lib/actions/sandbox/status-snapshot.ts (lines 54-56), used by
buildSandboxStatusReport, to catch the error and expose it as a status field so
JSON output remains valid.
- Around line 392-410: Update the stale status-output references in the
deployment documentation and changelog to describe the dashboard-url guidance
instead of SSH dashboard guidance, while preserving the valid SSH hint
associated with the dashboard-url command.
In `@src/lib/onboard/resume-config.ts`:
- Around line 15-24: Update resume-config.test.ts fixtures for
getResumeSandboxConflict to represent a pending sandbox through
checkpoint.steps.sandbox.status with a completed status, rather than
checkpoint.sandboxIdentity. Keep the assertion expecting a resume conflict and
align the test input with ResumeSessionLike’s steps shape.
In `@src/lib/state/registry.ts`:
- Around line 272-283: Update updateSandbox to prevent caller-supplied
cuaRuntimeReadiness, including explicit undefined, from being persisted; only
the canonical readiness recorder may write it. Invalidate the stored readiness
whenever inference or runtime authority changes, and remove opaque readiness
records when qualification is disabled. Preserve the existing name validation,
locking, and successful update behavior.
---
Nitpick comments:
In `@test/support/status-flow-test-harness.ts`:
- Line 206: Update createStatusFlowHarness and its status-flow tests to cover
the true result from statusProcessRecovery.isSandboxGatewayRunningForStatus,
asserting the expected “OpenClaw: running” output; preserve the existing
false-result coverage.
🪄 Autofix
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: f83b3a59-860e-49bd-b3d8-7f6cce16a108
📒 Files selected for processing (32)
docs/manage-sandboxes/workspace-files.mdxsrc/lib/actions/sandbox/status-flow.test.tssrc/lib/actions/sandbox/status-snapshot.tssrc/lib/actions/sandbox/status-text.tssrc/lib/actions/sandbox/status.test.tssrc/lib/onboard/checkpoint-resume-guard.test.tssrc/lib/onboard/command.test.tssrc/lib/onboard/command.tssrc/lib/onboard/docker-driver-gateway-config.tssrc/lib/onboard/docker-driver-gateway-env.tssrc/lib/onboard/docker-driver-gateway-launch.test.tssrc/lib/onboard/host-mount/host-mount.test.tssrc/lib/onboard/host-mount/index.tssrc/lib/onboard/managed-workload-rebuild-transaction.test.tssrc/lib/onboard/resume-config.tssrc/lib/onboard/runtime-provider/access.tssrc/lib/onboard/runtime-provider/contract.tssrc/lib/onboard/runtime-provider/docker.tssrc/lib/onboard/runtime-provider/mxc.test.tssrc/lib/onboard/runtime-provider/mxc.tssrc/lib/onboard/runtime-provider/podman.test.tssrc/lib/onboard/runtime-provider/podman.tssrc/lib/onboard/runtime-provider/registry.tssrc/lib/onboard/runtime-provider/runtime-provider-contract.test.tssrc/lib/onboard/session-bootstrap.test.tssrc/lib/onboard/session-bootstrap.tssrc/lib/state/onboard-session-station-express.test.tssrc/lib/state/onboard-session.test.tssrc/lib/state/onboard-session.tssrc/lib/state/registry.tstest/helpers/runtime-provider-bundle.tstest/support/status-flow-test-harness.ts
💤 Files with no reviewable changes (2)
- src/lib/onboard/docker-driver-gateway-env.ts
- src/lib/state/onboard-session.ts
Comment on lines
+96
to
+105
| const hostMounts = [ | ||
| { source: "/srv/project", target: "/sandbox/project", readOnly: true as const }, | ||
| ]; | ||
| const report = await getSandboxStatusReport("alpha", { | ||
| getSandbox: () => ({ name: "alpha", hostMounts }) as never, | ||
| reconcile: async () => ({ state: "missing" as const, output: "not found" }), | ||
| }); | ||
|
|
||
| await expect(reportFor("not-an-array")).rejects.toThrow( | ||
| "Persisted host mount state must be an array", | ||
| ); | ||
| await expect( | ||
| reportFor([ | ||
| { | ||
| source: "/srv/project\u202e", | ||
| target: "/sandbox/project", | ||
| readOnly: true, | ||
| }, | ||
| ]), | ||
| ).rejects.toThrow("unsafe terminal control characters"); | ||
| expect(report.hostMounts).toEqual(hostMounts); | ||
| expect(report.hostMounts).not.toBe(hostMounts); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the element-level defensive copy.
not.toBe(hostMounts) only proves that the report holds a new array. normalizePersistedSandboxHostMounts already returns a new array, so this assertion passes even if the wrapper at src/lib/actions/sandbox/status-snapshot.ts lines 54-56 stops cloning each mount. Assert the element identity to lock in the claim.
💚 Proposed assertion
expect(report.hostMounts).toEqual(hostMounts);
expect(report.hostMounts).not.toBe(hostMounts);
+ expect(report.hostMounts[0]).not.toBe(hostMounts[0]);As per path instructions: "Flag copied production algorithms, broad mocks that bypass the behavior under test, and conditionals that make a test pass without exercising its claim."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const hostMounts = [ | |
| { source: "/srv/project", target: "/sandbox/project", readOnly: true as const }, | |
| ]; | |
| const report = await getSandboxStatusReport("alpha", { | |
| getSandbox: () => ({ name: "alpha", hostMounts }) as never, | |
| reconcile: async () => ({ state: "missing" as const, output: "not found" }), | |
| }); | |
| await expect(reportFor("not-an-array")).rejects.toThrow( | |
| "Persisted host mount state must be an array", | |
| ); | |
| await expect( | |
| reportFor([ | |
| { | |
| source: "/srv/project\u202e", | |
| target: "/sandbox/project", | |
| readOnly: true, | |
| }, | |
| ]), | |
| ).rejects.toThrow("unsafe terminal control characters"); | |
| expect(report.hostMounts).toEqual(hostMounts); | |
| expect(report.hostMounts).not.toBe(hostMounts); | |
| const hostMounts = [ | |
| { source: "/srv/project", target: "/sandbox/project", readOnly: true as const }, | |
| ]; | |
| const report = await getSandboxStatusReport("alpha", { | |
| getSandbox: () => ({ name: "alpha", hostMounts }) as never, | |
| reconcile: async () => ({ state: "missing" as const, output: "not found" }), | |
| }); | |
| expect(report.hostMounts).toEqual(hostMounts); | |
| expect(report.hostMounts).not.toBe(hostMounts); | |
| expect(report.hostMounts[0]).not.toBe(hostMounts[0]); |
🤖 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/status.test.ts` around lines 96 - 105, Strengthen the
assertions in the getSandboxStatusReport test so they verify each returned host
mount is a distinct object from the corresponding entry in hostMounts, not just
that report.hostMounts is a new array. Preserve the existing value-equality
assertion while adding element-level identity checks that exercise the cloning
behavior in normalizePersistedSandboxHostMounts and the status snapshot wrapper.
Source: Path instructions
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Locate remaining coverage for unsafe host-mount rejection.
fd 'host-mount' --type f
rg -nP --type=ts -C4 'hasUnsafeHostMountTerminalText|terminal control|\\u001b|x1b'
rg -nP --type=ts -C4 'normalizePersistedSandboxHostMounts' -g '**/*.test.ts'Repository: NVIDIA/NemoClaw
Length of output: 236
🏁 Script executed:
#!/bin/bash
set -u
echo '=== candidate files ==='
fd -t f -e ts -e tsx | rg '(^|/)(host-mount|status(-flow)?|onboard-session|registry)([^/]*)?\.test\.tsx?$|src/lib/state/registry/host-mount\.ts|src/lib/state/registry\.ts|src/lib/actions/sandbox/status-(snapshot|text)\.ts'
echo '=== AST outlines ==='
for f in \
src/lib/state/registry/host-mount.ts \
src/lib/onboard/host-mount/host-mount.test.ts \
src/lib/actions/sandbox/status-snapshot.ts \
src/lib/actions/sandbox/status-text.ts \
src/lib/actions/sandbox/status.test.ts \
src/lib/actions/sandbox/status-flow.test.ts \
src/lib/state/onboard-session.test.ts
do
if [ -f "$f" ]; then
echo "--- $f ---"
ast-grep outline "$f" 2>/dev/null | head -120
fi
done
echo '=== normalizer and validator references ==='
rg -n -C5 'normalizePersistedSandboxHostMounts|hasUnsafeHostMountTerminalText|hostMounts' \
src/lib/state/registry/host-mount.ts \
src/lib/onboard/host-mount/host-mount.test.ts \
src/lib/actions/sandbox/status-snapshot.ts \
src/lib/actions/sandbox/status-text.ts \
src/lib/actions/sandbox/status.test.ts \
src/lib/actions/sandbox/status-flow.test.ts \
src/lib/state/onboard-session.test.ts \
src/lib/state/registry.ts 2>/dev/null || true
echo '=== focused current diff ==='
git diff --stat -- \
src/lib/state/registry/host-mount.ts \
src/lib/onboard/host-mount/host-mount.test.ts \
src/lib/actions/sandbox/status-snapshot.ts \
src/lib/actions/sandbox/status-text.ts \
src/lib/actions/sandbox/status.test.ts \
src/lib/actions/sandbox/status-flow.test.ts \
src/lib/state/onboard-session.test.ts \
src/lib/state/registry.ts
git diff -- \
src/lib/state/registry/host-mount.ts \
src/lib/onboard/host-mount/host-mount.test.ts \
src/lib/actions/sandbox/status-snapshot.ts \
src/lib/actions/sandbox/status-text.ts \
src/lib/actions/sandbox/status.test.ts \
src/lib/actions/sandbox/status-flow.test.ts \
src/lib/state/onboard-session.test.ts \
src/lib/state/registry.ts | sed -n '1,1200p'Repository: NVIDIA/NemoClaw
Length of output: 32872
🏁 Script executed:
#!/bin/bash
set -u
echo '=== host-mount normalizer ==='
cat -n src/lib/state/registry/host-mount.ts | sed -n '1,190p'
echo '=== owner host-mount tests ==='
cat -n src/lib/onboard/host-mount/host-mount.test.ts | sed -n '1,180p'
echo '=== session normalization implementation and relevant tests ==='
rg -n -C8 'normalizeSession|hasInvalidSessionHostMounts|metadata\.hostMounts|hostMounts' \
src/lib/state/onboard-session.ts \
src/lib/state/onboard-session.test.ts \
src/lib/state/onboard-session-normalization.test.ts \
src/lib/state/onboard-session-redaction.test.ts 2>/dev/null || true
echo '=== status flow harness and host-mount behavior ==='
rg -n -C12 'createStatusFlowHarness|showSandboxStatus|printSandboxDetails|printSandboxHostMounts|hostMounts' \
src/lib/actions/sandbox/status-flow.test.ts \
src/lib/actions/sandbox/status-lookup-rendering.test.ts \
src/lib/actions/sandbox/status-snapshot*.test.ts \
src/lib/actions/sandbox/status-text.ts \
src/lib/actions/sandbox/status-snapshot.ts 2>/dev/null || true
echo '=== full test-tree searches ==='
rg -n -i -C3 \
'terminal control|control characters|unsafe.*host.?mount|host.?mount.*unsafe|hasUnsafeHostMountTerminalText|normalizePersistedSandboxHostMounts|hasInvalidSessionHostMounts' \
--glob '**/*.{test,spec}.{ts,tsx,js,jsx,mts,mjs,cts,cjs}' . 2>/dev/null || true
echo '=== control-character literals ==='
rg -n -P -C3 '\\\\x1b|\\\\u001b|\\\\u000[0-9a-f]|\\x1b|\\u001b' \
--glob '**/*.{test,spec}.{ts,tsx,js,jsx,mts,mjs,cts,cjs}' . 2>/dev/null || trueRepository: NVIDIA/NemoClaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -u
echo '=== normalizer source ==='
sed -n '1,132p' src/lib/state/registry/host-mount.ts
echo '=== session source: host-mount normalization ==='
rg -n -C10 'hasInvalidSessionHostMounts|hostMounts|normalize.*Session|metadata' src/lib/state/onboard-session.ts | head -260
echo '=== session tests: relevant sections ==='
sed -n '1015,1060p' src/lib/state/onboard-session.test.ts
sed -n '320,420p' src/lib/state/onboard-session.test.ts
echo '=== status text source ==='
sed -n '195,220p' src/lib/actions/sandbox/status-text.ts
sed -n '340,390p' src/lib/actions/sandbox/status-text.ts
echo '=== status-flow harness declaration and implementation references ==='
rg -n -m30 -C8 'function createStatusFlowHarness|const createStatusFlowHarness|showSandboxStatus\s*=|showSandboxStatus\(' \
src/lib/actions/sandbox/status-flow.test.ts \
src/lib/actions/sandbox/*.ts
echo '=== focused status tests ==='
sed -n '90,112p' src/lib/actions/sandbox/status.test.ts
sed -n '155,178p' src/lib/actions/sandbox/status-flow.test.ts
echo '=== owner-test assertions only ==='
rg -n -C4 'normalizePersistedSandboxHostMounts|parseReadOnlyHostMount|terminal|control|malformed|invalid' \
src/lib/onboard/host-mount/host-mount.test.tsRepository: NVIDIA/NemoClaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -u
node - <<'JS'
const unsafeTerminalText = /[\p{Cc}\p{Cf}\p{Zl}\p{Zp}]/u;
const cases = [
["/srv/project", false],
["/srv/proj\u001b[31mect", true],
["/srv/proj\nect", true],
["/srv/project\u200b", true],
];
for (const [value, expected] of cases) {
const actual = unsafeTerminalText.test(value);
console.log(JSON.stringify({ value, actual, expected, pass: actual === expected }));
}
JSRepository: NVIDIA/NemoClaw
Length of output: 436
🏁 Script executed:
#!/bin/bash
set -u
echo '=== session normalization flow ==='
sed -n '784,925p' src/lib/state/onboard-session.ts
rg -n -C12 'function loadSession|export function loadSession|normalizeSession\(' src/lib/state/onboard-session.ts
echo '=== status flow implementation ==='
sed -n '68,155p' src/lib/actions/sandbox/status.ts
echo '=== owner test full focused range ==='
sed -n '1,125p' src/lib/onboard/host-mount/host-mount.test.ts
echo '=== exact relevant assertions across tests ==='
rg -n -C3 'hasInvalidSessionHostMounts|Persisted state contains an invalid read-only host mount|terminal control|Host mounts:' \
src/lib --glob '**/*.test.ts' --glob '**/*.test.tsx' | head -240Repository: NVIDIA/NemoClaw
Length of output: 22733
🏁 Script executed:
#!/bin/bash
set -u
node - <<'JS'
const unsafe = /[\p{Cc}\p{Cf}\p{Zl}\p{Zp}]/u;
function parseDeclaration(value) {
if (unsafe.test(value)) {
throw new Error("paths must not contain terminal control characters");
}
const separator = value.lastIndexOf(":/sandbox/");
if (separator <= 0) throw new Error("invalid host-mount syntax");
return value;
}
function normalize(value) {
if (value === undefined) return [];
if (!Array.isArray(value)) throw new Error("Persisted host mount state must be an array");
const declarations = value.map((candidate) => {
if (
typeof candidate !== "object" ||
candidate === null ||
typeof candidate.source !== "string" ||
typeof candidate.target !== "string" ||
candidate.readOnly !== true
) {
throw new Error("invalid read-only host mount");
}
return `${candidate.source}:${candidate.target}`;
});
return declarations.map(parseDeclaration);
}
const cases = [
["malformed array", { source: "/srv/project", target: "/sandbox/project", readOnly: true }, false],
["unsafe source", [{ source: "/srv/\u001b[31mproject", target: "/sandbox/project", readOnly: true }], true],
["unsafe target", [{ source: "/srv/project", target: "/sandbox/\u001bproject", readOnly: true }], true],
["non-read-only", [{ source: "/srv/project", target: "/sandbox/project", readOnly: false }], true],
];
for (const [name, value, shouldThrow] of cases) {
let threw = false;
try {
normalize(value);
} catch {
threw = true;
}
console.log(JSON.stringify({ name, threw, shouldThrow, pass: threw === shouldThrow }));
}
JSRepository: NVIDIA/NemoClaw
Length of output: 433
Add regression coverage for invalid host-mount state. normalizePersistedSandboxHostMounts lacks tests for non-array values and terminal-control text in source or target. Add these cases to src/lib/onboard/host-mount/host-mount.test.ts.
- Add status-boundary tests that assert the intended rejection behavior. The normalizer throws; it does not silently omit unsafe mounts.
- Add session coverage for terminal-control metadata if
hasInvalidSessionHostMountsmust preserve its fail-closed marker. The existing test covers onlyreadOnly: false.
📍 Affects 3 files
src/lib/actions/sandbox/status.test.ts#L96-L105(this comment)src/lib/actions/sandbox/status-flow.test.ts#L162-L174src/lib/state/onboard-session.test.ts#L332-L407
🤖 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/status.test.ts` around lines 96 - 105, Expand
regression coverage for invalid persisted host mounts: in
src/lib/onboard/host-mount/host-mount.test.ts, test non-array values and
terminal-control characters in source or target, asserting
normalizePersistedSandboxHostMounts throws rather than omitting unsafe mounts.
In src/lib/actions/sandbox/status.test.ts and
src/lib/actions/sandbox/status-flow.test.ts, add status-boundary cases asserting
the intended rejection behavior. In src/lib/state/onboard-session.test.ts, add
terminal-control metadata coverage and verify hasInvalidSessionHostMounts
preserves its fail-closed marker.
Comment on lines
87
to
92
| ["network_name", gatewayEnv.OPENSHELL_DOCKER_NETWORK_NAME], | ||
| ["supervisor_image", gatewayEnv.OPENSHELL_DOCKER_SUPERVISOR_IMAGE], | ||
| // OpenShell 0.0.99 accepts supervisor_bin only for the Docker driver. | ||
| // OpenShell 0.0.85 accepts supervisor_bin only for the Docker driver. | ||
| // The Podman schema rejects the entire driver table when this Docker-only | ||
| // field is present, so portable onboarding must rely on supervisor_image. | ||
| ["supervisor_bin", driver === "docker" ? (sandboxBin ?? undefined) : undefined], |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check for remaining callers of the removed validator and the declared OpenShell version.
rg -nP --type=ts -C3 'hasStateScopedSandboxNamespace'
rg -nP -C2 'OPENSHELL_VERSION|openshell.*0\.0\.[0-9]+' -g '!**/node_modules/**' | head -40
rg -nP -C3 'socket_path' -g '!**/node_modules/**'Repository: NVIDIA/NemoClaw
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(docker-driver-gateway-config|onboard|openshell|gateway|sandbox|README)' | head -120
printf '%s\n' '--- validator and related identifiers ---'
rg -n -C3 'hasStateScopedSandboxNamespace|state.?scoped|sandbox.?namespace|supervisor_bin|supervisor_image|OPENSHELL_VERSION|0\.0\.(85|99)' \
src test tests .github README.md package.json 2>/dev/null | head -240 || true
printf '%s\n' '--- relevant source outline and file size ---'
if command -v ast-grep >/dev/null 2>&1; then
ast-grep outline src/lib/onboard/docker-driver-gateway-config.ts --view compact || true
fi
wc -l src/lib/onboard/docker-driver-gateway-config.ts
printf '%s\n' '--- gateway config source ---'
cat -n src/lib/onboard/docker-driver-gateway-config.tsRepository: NVIDIA/NemoClaw
Length of output: 31969
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- gateway config consumers and writers ---'
rg -n -C4 'OPENSHELL_GATEWAY_CONFIG|openshell-gateway\.toml|writeDockerDriverGatewayConfig|prepareDockerDriverGatewayConfigEnv|NEMOCLAW_OPENSHELL_SANDBOX_NAMESPACE_ENV' \
src scripts nemoclaw test docs .github 2>/dev/null | head -320 || true
printf '%s\n' '--- state directory and gateway environment setup ---'
rg -n -C4 'stateDir|OPENSHELL_STATE|OPENSHELL_HOME|gatewayEnv|mkdirSync|chmodSync|lstatSync|realpathSync|readlinkSync' \
src/lib/onboard src/lib/state src/commands/onboard.ts src/lib/actions/onboard.ts scripts 2>/dev/null | head -360 || true
printf '%s\n' '--- version migration records ---'
for f in docs/security/openshell-0.0.85-migration-review.md docs/security/openshell-0.0.99-migration-review.md docs/security/openshell-0.0.72-compatibility-review.mdx; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
rg -n -C4 'supervisor_bin|supervisor_image|driver|Docker|Podman|version|schema|namespace|config' "$f" | head -260 || true
fi
done
printf '%s\n' '--- focused tests ---'
rg -n -C5 'docker-driver-gateway-config|supervisor_bin|supervisor_image|gateway config|gateway-config|0o600|sandbox namespace' \
test src --glob '*test.ts' 2>/dev/null | head -320 || trueRepository: NVIDIA/NemoClaw
Length of output: 50372
🌐 Web query:
Official OpenShell documentation or source for the gateway driver configuration schema: determine the first release that accepts supervisor_bin for the Docker driver and whether the Podman driver rejects that field.
💡 Result:
In NVIDIA OpenShell, the supervisor_bin configuration field for the Docker driver was introduced as part of the transition to the TOML-based configuration schema defined in RFC 0003 [1][2]. This configuration format was implemented and merged in the OpenShell gateway, making supervisor_bin available for use in the [openshell.drivers.docker] table from release v0.0.83 onward [3]. Regarding the Podman driver, the field supervisor_bin is not a supported configuration key. The Podman driver schema is separate from the Docker driver's and does not include supervisor_bin [4][5]. Consequently, providing supervisor_bin within the [openshell.drivers.podman] table will cause the gateway to reject the configuration, as the driver's deserialization logic enforces strict field validation (e.g., deny_unknown_fields) [5][6]. Users attempting to use this field with the Podman driver will encounter preflight errors [6].
Citations:
- 1: https://docs.nvidia.com/openshell/reference/gateway-config
- 2: https://github.com/NVIDIA/OpenShell/blob/main/rfc/0003-gateway-configuration/README.md
- 3: NVIDIA/OpenShell@b61a98d
- 4: https://docs.nvidia.com/openshell/latest/reference/sandbox-compute-drivers
- 5: https://github.com/NVIDIA/OpenShell/blob/abe42fb5/crates/openshell-server/src/config_file.rs
- 6: [Ubuntu 26.04][Onboard] onboard --experimental-profile portable fails preflight with "spawnSync docker ENOENT" on Podman-only host (no docker client) #8453
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- relevant gateway launch code ---'
cat -n src/lib/onboard/docker-driver-gateway-launch.ts | sed -n '90,205p'
printf '%s\n' '--- gateway config validation symbols ---'
rg -n -C5 'function assertDockerDriverGateway|export function assertDockerDriverGateway|readFileSync|OPENSHELL_GATEWAY_CONFIG' \
src/lib/onboard --glob 'docker-driver-gateway-env-config-validation.ts' \
--glob 'docker-driver-gateway-env-config-validation.test.ts' \
--glob 'docker-driver-gateway-jwt-bundle.ts' \
--glob 'docker-driver-gateway-jwt-bundle.test.ts' 2>/dev/null || true
printf '%s\n' '--- state directory creation and gateway state paths ---'
rg -n -C5 'openshell-docker-gateway|gatewayStateDir|stateDir.*mkdir|mkdir.*stateDir|ensure.*State|chmod.*0o700|mode: 0o700|mode: 0o600|realpath|lstat|readlink' \
src/lib src/commands scripts test --glob '*.ts' --glob '*.mts' --glob '*.sh' 2>/dev/null | head -300 || true
printf '%s\n' '--- current diff for the reviewed file ---'
git status --short
git diff --unified=25 -- src/lib/onboard/docker-driver-gateway-config.ts | head -260
printf '%s\n' '--- migration-document security claims ---'
sed -n '680,755p' docs/security/openshell-0.0.85-migration-review.md
rg -n -C6 'mode-0600|ownership|symlink|link count|namespace|supervisor_bin|0\.0\.85|0\.0\.99' \
docs/security/openshell-0.0.99-migration-review.md docs/security/openshell-0.0.85-migration-review.md | head -260 || trueRepository: NVIDIA/NemoClaw
Length of output: 50371
🏁 Script executed:
#!/bin/bash
python3 - <<'PY'
from pathlib import Path
import re
root = Path(".")
config = root / "src/lib/onboard/docker-driver-gateway-config.ts"
launch = root / "src/lib/onboard/docker-driver-gateway-launch.ts"
text = config.read_text()
launch_text = launch.read_text()
print("--- static config trust-path facts ---")
print("atomic writer:", "writeRestrictedFileAtomic(" in text)
print("final mode argument:", re.search(r"writeRestrictedFileAtomic\(\s*configPath,.*?0o600", text, re.S) is not None)
print("parent mkdir mode:", re.search(r"mkdirSync\(dir, \{ recursive: true, mode: 0o700 \}\)", text) is not None)
print("rename replaces destination:", "fs.renameSync(tmpPath, filePath)" in text)
print("config path from stateDir:", 'path.join(stateDir, DOCKER_DRIVER_GATEWAY_CONFIG_NAME)' in text)
print("config exported through env:", "gatewayEnv.OPENSHELL_GATEWAY_CONFIG = writeDockerDriverGatewayConfig" in text)
print("namespace validator reference count:", len(re.findall(r"hasStateScopedSandboxNamespace", "\n".join(p.read_text(errors="ignore") for p in root.rglob("*.ts")))))
print("--- launch path assignments ---")
for i, line in enumerate(launch_text.splitlines(), 1):
if "prepareDockerDriverGatewayConfigEnv" in line or "OPENSHELL_GATEWAY_CONFIG" in line:
print(f"{launch}:{i}:{line.strip()}")
PYRepository: NVIDIA/NemoClaw
Length of output: 690
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- every namespace-validator reference ---'
rg -n -C8 'hasStateScopedSandboxNamespace' . --glob '!**/node_modules/**' --glob '!**/.git/**' || true
printf '%s\n' '--- auth validator implementation ---'
fd -i 'docker-driver-gateway-env-config-validation' src/lib/onboard --type f --exec sh -c 'for f do echo "--- $f"; cat -n "$f"; done' sh
printf '%s\n' '--- JWT bundle implementation ---'
cat -n src/lib/onboard/docker-driver-gateway-jwt-bundle.ts | sed -n '1,240p'
printf '%s\n' '--- gateway state path definitions ---'
rg -n -C8 'openshell-docker-gateway|resolve.*StateDir|gateway.*state|state.*gateway|GATEWAY_STATE' \
src/lib --glob '*.ts' --glob '*.mts' | head -260 || true
printf '%s\n' '--- exact diff/status ---'
git status --short
git diff --numstat -- src/lib/onboard/docker-driver-gateway-config.ts
git diff --unified=8 -- src/lib/onboard/docker-driver-gateway-config.ts | sed -n '1,220p'Repository: NVIDIA/NemoClaw
Length of output: 44037
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- host gateway namespace validation call sites ---'
cat -n src/lib/onboard/host-gateway-process.ts | sed -n '1,35p;245,315p'
printf '%s\n' '--- uninstall namespace validation call site ---'
cat -n src/lib/actions/uninstall/run-plan.ts | sed -n '45,72p;1005,1045p'
printf '%s\n' '--- config exports and symbol definitions ---'
rg -n '^(export )?(function|const|type|interface)|hasStateScopedSandboxNamespace|processUsesStateScopedSandboxNamespace' \
src/lib/onboard/docker-driver-gateway-config.ts src/lib/onboard/host-gateway-process.ts
printf '%s\n' '--- TypeScript configuration and scripts ---'
cat package.json | sed -n '1,180p'
fd -i 'tsconfig' . --type f --max-depth 3 --exec sh -c 'for f do echo "--- $f"; sed -n "1,140p" "$f"; done' shRepository: NVIDIA/NemoClaw
Length of output: 28291
🏁 Script executed:
#!/bin/bash
python3 - <<'PY'
from pathlib import Path
import re
config = Path("src/lib/onboard/docker-driver-gateway-config.ts").read_text()
host = Path("src/lib/onboard/host-gateway-process.ts").read_text()
uninstall = Path("src/lib/actions/uninstall/run-plan.ts").read_text()
symbol = "hasStateScopedSandboxNamespace"
print("config defines symbol:", bool(re.search(rf"\b(?:export\s+)?function\s+{symbol}\b", config)))
print("config exports symbol:", bool(re.search(rf"\bexport\s+(?:function|const)\s+{symbol}\b", config)))
print("host imports symbol:", bool(re.search(rf"\b{symbol}\b", host.split("from \"./docker-driver-gateway-config\"")[0])))
print("host re-exports symbol:", bool(re.search(rf"\bexport\s*\{{[^}}]*\b{symbol}\b", host, re.S)))
print("host call count:", len(re.findall(rf"\b{symbol}\s*\(", host)))
print("uninstall call count:", len(re.findall(rf"\b{symbol}\s*\(", uninstall)))
PYRepository: NVIDIA/NemoClaw
Length of output: 309
Restore the namespace validator or remove its dependent callers.
hasStateScopedSandboxNamespace is no longer exported, but src/lib/onboard/host-gateway-process.ts and src/lib/actions/uninstall/run-plan.ts still import and call it. This breaks TypeScript compilation and removes the fail-closed guard for scoped gateway shutdown. The 0.0.85 supervisor_bin statement is accurate; Podman must continue to omit this field.
🤖 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/onboard/docker-driver-gateway-config.ts` around lines 87 - 92,
Restore and export hasStateScopedSandboxNamespace, then retain its existing
callers in host-gateway-process.ts and uninstall run-plan.ts so TypeScript
compilation succeeds and scoped gateway shutdown remains fail-closed. Do not
alter the Docker-only supervisor_bin handling; Podman must continue omitting
that field.
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
This was referenced Aug 13, 2026
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
prekshivyas
requested changes
Aug 14, 2026
prekshivyas
left a comment
prekshivyas
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed current head 29d0ab3a3e24e306a47f20102d6a109b0d3db6c9.
This PR creates a user-visible runtime-provider host-mount capability contract and canonical support documentation across Docker, Podman, and OpenShell MXC, but it has no linked accepted issue or design decision defining ownership, lifecycle, compatibility, security, and qualification expectations. The repository product-scope gate blocks that supported surface until maintainers establish those decisions.
The effective diff is also stale against current main: it removes unrelated current sandbox-status SSH guidance and associated safety tests while introducing the host-mount changes. Refresh without reverting current behavior. Then either scope the change to an accepted current consumer or record the product decision, and rerun security, behavior, documentation, and required CI review on the resulting exact head.
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
Declare read-only host-mount support through the selected runtime-provider contract and reject unsupported providers or host platforms before onboarding mutates durable session state. Docker remains supported on Linux and WSL, while Kubernetes, Podman, and OpenShell MXC fail closed with provider-specific reasons. This is a stacked follow-up to #8280.
Related Issue
Part of #8343. Depends on #8280.
Changes
Type of Change
Quality Gates
Security Review
Documentation Writer Review
docs-updateddocs/manage-sandboxes/workspace-files.mdx; independent final review found no findings and passed 202 focused tests;npm run docspassed with 0 errors and 2 pre-existing hidden-page warnings.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every pushed commit appears asVerifiedin GitHubnpm run validate:prpassed after refreshingorigin/mainnpm run build:cliandnpm run typecheck:clipassednpm run test-conditionals:scan -- --top 25passednpm run validate:prwere used because this change is limited to the runtime-provider capability and onboarding guard pathsnpm run docsbuilds without warnings (doc changes only) — 0 errors; 2 pre-existing hidden-page warnings remainSigned-off-by: Aaron Erickson aerickson@nvidia.com
Summary by CodeRabbit
New Features
Documentation
Bug Fixes