feat(onboard): preserve Jetson GPU device groups - #8910
Conversation
Carry the deferred Jetson GPU policy, bootstrap, lifecycle, and validation work in a stacked draft. The OpenShell execution boundary tracked by #7610 remains unresolved. Co-authored-by: San Dang <sdang@nvidia.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> Signed-off-by: San Dang <sdang@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-8910.docs.buildwithfern.com/nemoclaw |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit fff5a76 in the TypeScript / code-coverage/cliThe overall coverage in commit fff5a76 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
5 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
3 additional E2E selections from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
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: Manual-only E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
wscurran
added
area: onboarding
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
# Conflicts: # src/lib/onboard/managed-bootstrap/docker.ts
prekshivyas
left a comment
prekshivyas
left a comment
There was a problem hiding this comment.
Reviewed exact draft head fff5a76.
The proposed implementation is appropriately fail-closed for its draft purpose: it derives supplementary groups only from existing non-symlink Jetson character devices, validates numeric group IDs, keeps the helper root-owned, limits the root action to group setup plus the OpenShell supervisor handoff, and adds focused policy/recreation/bootstrap tests. The documentation correctly avoids claiming CUDA success.
This cannot be approved or leave draft state. The repository product-scope gate is still unmet, #7610 remains open, and the only reproduced IGX Orin result is that non-root cuInit(0) fails even with the existing groups. The PR therefore has no evidence that these broader preserved groups establish the supported sandbox-user CUDA contract. GitHub also reports the branch CONFLICTING/DIRTY against current main, and the documentation review is explicitly blocked. Resolve the product decision first, reconcile with current main, then obtain exact-head Jetson hardware evidence through the supported OpenShell path proving cuInit(0)=0, lifecycle/recreation behavior, and the least-privilege device/group set; rerun docs and sensitive-path review afterward.
Security review:
- Input validation: PASS — device paths, symlink state, character-device type, and numeric group IDs are bounded and validated.
- Authentication and authorization: WARNING — supplementary group membership expands device authority and still lacks accepted least-privilege hardware proof.
- Secrets and sensitive data: PASS — no credentials or sensitive payloads are introduced.
- Injection risks: PASS — group identifiers are numeric and the root helper does not evaluate caller-supplied shell fragments.
- Data exposure and privacy: PASS — no new data egress or logging surface.
- Cryptography: PASS — no cryptographic behavior changes.
- Dependencies and supply chain: PASS — no dependency or artifact-source changes.
- System security: FAIL for approval — the core non-root CUDA security/compatibility postcondition remains unproven and product ownership is undefined.
- Testing and verification: FAIL for approval — #7610 is unresolved, exact-head Jetson hardware evidence is absent, docs review is blocked, and the branch conflicts with current main.
Files reviewed: all 40 changed Jetson device/group policy, clone/recreate/create, startup/bootstrap, managed-runtime, Dockerfile/build-context, workflow, documentation, and test files; linked issue #7610 and automated review evidence.
prekshivyas
left a comment
prekshivyas
left a comment
There was a problem hiding this comment.
Reviewed commit fff5a76e22c1c3c57731a82c09d9a2bd7155a0eb as a security-critical Jetson policy and root-helper change.
I did not find an additional correctness or security defect in the current diff. The helper validates the complete bounded GID set before mutation, requires root, accepts only the fixed OpenShell supervisor handoff, verifies the resulting membership, and fails closed. The policy path is anchored on a real non-symlink /dev/nvmap character device and grants only the enumerated device paths.
Security review: secrets PASS; input validation PASS; authentication/authorization PASS; dependencies PASS; error handling PASS; cryptography N/A; configuration/environment PASS; security tests PASS for the implemented boundary; system security BLOCKED for merge.
This is not an approval. The PR is correctly still draft and currently conflicts with the base branch. Its stated merge gates remain material: maintainer product-scope acceptance, resolution of #7610, real supported-path Jetson evidence showing cuInit(0), and completion of the sensitive-path/broad verification gates.
3 participants
Summary
This draft preserves Jetson GPU policy, device-group, container-lifecycle, and validation changes after #8875 merged the CPU-only E2E path. Issue #7610 still blocks the non-root OpenShell CUDA acceptance test and the product-scope decision. Maintainers must keep this PR as a draft until those conditions are resolved.
Related Issue
Related to #7610. This PR does not resolve or close that issue.
Changes
Draft Exit Conditions
Maintainers must keep this PR as a draft until all of these conditions are true:
cuInit(0)through the supported OpenShell sandbox path.Type of Change
Quality Gates
Documentation Writer Review
blockeddocs/reference/troubleshooting.mdx,test/e2e/README.md, andtest/e2e/docs/README.md. The changed documentation passes writing, terminology, claim-accuracy, and agent-variant review. The managed-bootstrap conflict resolution preserves the bounded supervisor reconnect timeout and the exact Jetson helper command boundary. Maintainer product-scope acceptance and IGX Orin OpenShellcuInit(0)=0evidence remain required.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — deferred while issue [Jetson][Sandbox] Sandbox GPU passthrough proof fails for the non-root sandbox user on JetPack 6.2 IGX Orin — onboarding aborts #7610 blocks the acceptance contract.npm run docsbuilds without warnings (doc changes only) — build passed with 0 errors and 2 existing Fern warnings.Signed-off-by: Charan Jagwani cjagwani@nvidia.com
Signed-off-by: San Dang sdang@nvidia.com