fix(onboard): permit Jetson GPU policy paths - #8099
Conversation
Signed-off-by: San Dang <sdang@nvidia.com>
|
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:
📝 WalkthroughWalkthroughThe PR adds persistent Jetson ChangesJetson GPU sandbox support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Setup as scripts/setup-jetson.sh
participant Onboard as Sandbox creation flow
participant Docker as docker-gpu-patch-clone.ts
participant Bootstrap as jetson-device-group-bootstrap.sh
participant Sandbox as OpenClaw sandbox
Setup->>Setup: configure persistent /dev/nvmap access
Onboard->>Docker: pass OpenClaw Jetson settings
Docker->>Bootstrap: pass device GIDs and supervisor command
Bootstrap->>Sandbox: add and verify device-group membership
Bootstrap->>Sandbox: start preserved supervisor
Sandbox->>Sandbox: validate readable and writable /dev/nvmap
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-8099.docs.buildwithfern.com/nemoclaw |
Signed-off-by: San Dang <sdang@nvidia.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/setup-jetson.test.ts (1)
368-415: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover failed post-mutation permission validation.
The successful test supplies
rwfor the secondstatcall.
A removal or inversion of the validation atconfigure_nvmap_group_accesscan still pass this suite.
Add a case where the device remains a character device but its group permissions remain read-only afterchmod.
Assert the script fails with the group read-write verification error.As per path instructions, “Prefer observable outcomes through the public boundary.”
🤖 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/setup-jetson.test.ts` around lines 368 - 415, Add a test in the “setup-jetson JetPack 6 nvmap access” suite where the post-chmod stat result remains a character device with read-only group permissions, while preserving the initial valid character-device result. Assert setup exits with status 1 and reports the group read-write verification error, and verify the failure is observable through the existing spawnSetupJetson boundary.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/setup-jetson.test.ts`:
- Around line 368-415: Add a test in the “setup-jetson JetPack 6 nvmap access”
suite where the post-chmod stat result remains a character device with read-only
group permissions, while preserving the initial valid character-device result.
Assert setup exits with status 1 and reports the group read-write verification
error, and verify the failure is observable through the existing
spawnSetupJetson boundary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0e8dcf48-d76f-47dc-952a-b950c09fed29
📒 Files selected for processing (9)
docs/reference/troubleshooting.mdxscripts/setup-jetson.shsrc/lib/onboard/docker-gpu-jetson-groups.test.tssrc/lib/onboard/docker-gpu-jetson-groups.tssrc/lib/onboard/docker-gpu-patch-recreate.tssrc/lib/onboard/sandbox-gpu-direct-proof.test.tssrc/lib/onboard/sandbox-gpu-preflight.tstest/e2e/live/jetson-nvmap-gpu.test.tstest/setup-jetson.test.ts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite for the commit under review. Recommended E2E: None Manual-only E2E: 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/onboard/docker-gpu-patch-recreate.ts (1)
266-281: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve Jetson groups for
startup-commandrecreation.Line 266 excludes
startup-commandfrom Tegra group detection. A Jetson recreation using that supported mode never setsextraGroupGidsorpreserveJetsonDeviceGroupMembership. The clone builder then does not inject the bootstrap. After OpenShell callsinitgroups(), the sandbox user can lose/dev/nvmapgroup access and CUDA can fail.Remove the mode exclusion and add a
startup-commandrecreation test.Proposed fix
- if (selection.mode.kind !== "startup-command" && options.backend === "jetson") { + if (options.backend === "jetson") {As per path instructions, trace every in-scope entrypoint and lifecycle path.
🤖 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-gpu-patch-recreate.ts` around lines 266 - 281, Update the Jetson Tegra group handling in the recreation flow to run for startup-command mode as well by removing the selection.mode.kind exclusion from the condition around detectTegraDeviceGroupGids. Preserve the existing extraGroupGids and preserveJetsonDeviceGroupMembership assignments so the clone builder injects the bootstrap, and add coverage for startup-command recreation.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.
Outside diff comments:
In `@src/lib/onboard/docker-gpu-patch-recreate.ts`:
- Around line 266-281: Update the Jetson Tegra group handling in the recreation
flow to run for startup-command mode as well by removing the selection.mode.kind
exclusion from the condition around detectTegraDeviceGroupGids. Preserve the
existing extraGroupGids and preserveJetsonDeviceGroupMembership assignments so
the clone builder injects the bootstrap, and add coverage for startup-command
recreation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b94d4187-c252-4967-9c70-e3171ded73fd
📒 Files selected for processing (22)
Dockerfiledocs/reference/troubleshooting.mdxscripts/jetson-device-group-bootstrap.shscripts/setup-jetson.shsrc/lib/onboard.tssrc/lib/onboard/docker-gpu-patch-clone.tssrc/lib/onboard/docker-gpu-patch-jetson.test.tssrc/lib/onboard/docker-gpu-patch-recreate.tssrc/lib/onboard/docker-gpu-patch-types.tssrc/lib/onboard/docker-gpu-sandbox-create.tssrc/lib/onboard/initial-policy.test.tssrc/lib/onboard/initial-policy.tssrc/lib/onboard/sandbox-create-intent-types.tssrc/lib/onboard/sandbox-create-intent.tssrc/lib/onboard/sandbox-create-plan-materialization.tssrc/lib/onboard/sandbox-create-plan.test.tssrc/lib/onboard/sandbox-gpu-create-flow.tssrc/lib/onboard/sandbox-gpu-create-run-attempt.tssrc/lib/onboard/sandbox-gpu-create.tstest/e2e/live/jetson-nvmap-gpu.test.tstest/openclaw-final-image-layout.test.tstest/setup-jetson.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- test/e2e/live/jetson-nvmap-gpu.test.ts
- scripts/setup-jetson.sh
Signed-off-by: San Dang <sdang@nvidia.com>
|
Exact-head Jetson gate status for
Do not rerun or manually dispatch |
|
Addressed the remaining bootstrap review feedback in revision f25c2b5.
Validation passed: all commit and push hooks; ShellCheck, shfmt, Biome, repository checks, and secret scan; 15/15 adjacent Jetson CLI tests; 11/11 direct bootstrap integration tests; test-title and project-membership checks; and the documentation build with 0 errors and the 2 existing warnings. GitHub reports the commit as Verified, and the commit includes DCO sign-off. The PR remains draft only for the previously documented external acceptance blocker: current evidence from an affected Jetson that the non-root sandbox user retains the required device group, can read and write /dev/nvmap, and reports cuInit(0)=0, or an explicit maintainer waiver. Newly triggered GitHub Actions checks are queued during the ongoing service incident, so I have not forced reruns. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Addressed PRA-1 on the current branch revision (
Validation: focused Vitest 16/16 passed; CLI type-check passed; Biome, test-title validation, and |
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Maintainer update for the current branch revision
All inline review threads are resolved. The PR remains draft and still requires final evidence from the affected Jetson showing non-root |
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
This reverts commit 1c723ba. Signed-off-by: San Dang <sdang@nvidia.com>
|
Fresh exact-main Jetson evidence confirms this PR owns the current failure. E2E run 31571534932, job 94034710428, on main SHA 5aaf22b reached phase 5 ( |
cjagwani
left a comment
There was a problem hiding this comment.
Exact-head review at 5e2459ad92d7f4bb988306618a15b06465adc517: the two focused NemoClaw fixes are supported by the code and tests, but this investigation branch should not become the merge candidate.
The remaining hardware acceptance still fails: the same image and non-root user return cuInit(0)=0 in direct Docker execution and cuInit(0)=801 through OpenShell. That is an upstream execution-boundary blocker, so this PR cannot close #7610.
This head also ships temporary diagnostic production paths beyond the mergeable fix. In particular, jetson-openrm-proof.ts can temporarily apply root-wide read-only and then read-write filesystem policy candidates, while the process and namespace matrices execute privileged Docker probes. A hidden NEMOCLAW_DIAGNOSE_JETSON_OPENRM_POLICY=1 convention is not a product authorization boundary, and these investigation tools should not ship in the normal CLI surface.
PR #8875 is the appropriately scoped successor: it retains the device-policy and supplementary-group fixes, removes the temporary diagnostics, adds startup-command recreation coverage, and starts from current main. Please close #8099 in favor of #8875. Keep #7610 open until an exact OpenShell/IGX Orin proof reaches cuInit(0)=0.
|
Closing in favor of #8875. The core Tegra group/device-policy findings remain valid, but this branch also contains temporary production diagnostics that are not suitable to merge. #8875 carries the focused current-main successor work; #7610 remains open because OpenShell execution still returns cuInit(0)=801 on IGX Orin. |
Summary
Two boundaries blocked Jetson CUDA. OpenShell's creation-time filesystem policy hid the injected Jetson libraries and Tegra device nodes. After that policy was corrected, OpenShell rebuilt the sandbox account's supplementary groups with
initgroups(), discarding the device GIDs supplied by Docker.OpenClaw direct-GPU onboarding now grants
/opt/nvidiaread-only and only detected, eligible Tegra character devices read-write. Before the compatibility replacement starts the fixed OpenShell supervisor, a bounded image-owned wrapper records those detected device GIDs in the sandbox account's group database so the required memberships survive startup.The branch remains draft pending final end-to-end acceptance on Jetson hardware.
Related Issue
Related to #7610
Changes
/dev/nvmapto be an existing, non-symlink character device before adding Jetson filesystem entries./opt/nvidiaread-only and each existing, non-symlink device from the curated Tegra GPU path list read-write for OpenClaw direct-GPU onboarding only./opt/openshell/bin/openshell-sandbox.finally.Type of Change
Quality Gates
7b70ce2cf7ec4635e2b5616a653fc10ae2eae2baacross authentication and authorization, input validation, command execution, filesystem and symlink handling, secrets, network exposure, cryptography, dependencies, resource safety, and privilege boundaries. No blocking finding remains. The wrapper validates the complete GID list before any account mutation, keeps the fixed supervisor handoff, verifies resulting membership, and fails closed. An incremental nine-category review at revision21f21eee75b2e8722dfffa21baca38a737dfddd2found no new production security surface: the final delta is test-only, adds no credentials, dependencies, egress, or privilege, and preserves the fixture's digest-pinned base, disabled network, read-only root filesystem, dropped capabilities, no-new-privileges setting, and bounded writable temporary storage.Documentation Writer Review
no-docs-needed5e2459ad9: the revert removes the temporary Jetson CUDA Driver API diagnostic output and restores the four onboarding files to their pre-diagnostic state. The establishedcuInit(0)verification, remediation, rollback, command, flags, defaults, configuration, and public behavior remain unchanged. Reviewed validation: 51 focused tests, Biome, and normal commit/push hooks passed.DGX Station Hardware Evidence
Verification
Signed-off-by:line; GitHub verification is checked after each pushnpm run check:diffpassed against currentmain21f21eee75b2e8722dfffa21baca38a737dfddd2; prior branch validation remains 112/112 focused CLI tests and 19/19 focused integration testsJetson Validation
The read-only diagnostic isolated both boundaries: the policy hid
/opt/nvidia, and after that was exposed the replacement still failed withNvRmMemInitNvmap ... Permission deniedandcuInit(0)=100. The failed replacement inspect showed Docker had supplied GIDs 44 and 993, while the sandbox account retained only GID 998.At validated revision
7fad34abc, a disposable A/B proof ran against the Jetson sandbox image without modifying the OpenShell sandbox:initgroups()rebuilt membership from the unchanged group database./dev/nvmapread-write open failed andcuInit(0)=100.initgroups()call. The account then held GIDs 44, 993, and 998;/dev/nvmapread-write open succeeded; andcuInit(0)=0.This proves the supplementary-group loss and its wrapper mechanism, but it does not prove the complete #7610 root cause. Exact Jetson hardware later retained the required groups, admitted
/dev/nvmap,/opt/nvidia, and/dev/nvsciipc, and still returnedcuInit(0)=801only through OpenShell while direct Docker returnedcuInit(0)=0. The unproven/dev/nvsciipccandidate was reverted in250b7b459. Full acceptance is blocked on the remaining OpenShell sandbox-execution boundary; the PR must not close #7610 without a proven upstream fix.Signed-off-by: San Dang sdang@nvidia.com