fix(dcode): require Landlock enforcement#5812
Conversation
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the branch. TypeScript / code-coverage/cliThe overall coverage in the branch remains at 80%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Updated |
|
🌿 Preview your docs: https://nvidia-preview-pr-5812.docs.buildwithfern.com/nemoclaw |
|
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:
📝 WalkthroughWalkthroughLandlock onboarding now fails closed for Deep Agents Code on unsupported or unenforceable setups. Policy, schema, tests, and docs were updated to use ChangesLandlock onboarding hardening
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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
|
PR Review Advisor — Blocking findings reportedAdvisor assessment: Blockers require maintainer review Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: Blockers
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
docs/deployment/sandbox-hardening.mdx (2)
147-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse active voice here.
is required by onboardingreads passive. Say directly that onboarding requires kernel5.13+with Landlock enabled.As per path instructions, "Active voice required. Flag passive constructions."
🤖 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 `@docs/deployment/sandbox-hardening.mdx` at line 147, The sentence in the sandbox-hardening documentation uses passive voice; rewrite it in active voice so onboarding is the subject, e.g. have onboarding directly require kernel 5.13+ with Landlock enabled. Update the wording in the affected prose block to keep the same meaning while removing the passive construction.Source: Path instructions
138-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared CLI placeholder here.
This reads like shared guidance, so a concrete
nemoclawexample will render the wrong command name for other agent aliases.Suggested fix
-On such kernels, `nemoclaw onboard` aborts before creating or reusing a sandbox. +On such kernels, `$$nemoclaw onboard` aborts before creating or reusing a sandbox.As per coding guidelines, "Use
$$nemoclawfor host CLI command examples on shared OpenClaw and Hermes pages." As per path instructions, "When a docs PR adds a command-line example that works across all NemoClaw agent aliases, comment if it uses a concrete alias such asnemoclawornemohermes; ask the author to use$$nemoclawinstead."🤖 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 `@docs/deployment/sandbox-hardening.mdx` at line 138, The CLI example in the shared sandbox-hardening guidance uses a concrete agent alias and should be generalized for all aliases. Update the command example in the relevant docs section to use the shared placeholder symbol $$nemoclaw instead of a specific command name like nemoclaw, so the rendered guidance is correct across OpenClaw and Hermes variants. Locate the host CLI example in the sandbox-hardening content and replace the alias-specific invocation with the shared placeholder consistently.Sources: Coding guidelines, Path instructions
docs/reference/troubleshooting.mdx (1)
1062-1062: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse active voice here.
prevents a new sandbox from being createdis passive. Say directly that the check prevents NemoClaw from creating a sandbox with reduced filesystem isolation.As per path instructions, "Active voice required. Flag passive constructions."
🤖 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 `@docs/reference/troubleshooting.mdx` at line 1062, The sentence uses passive voice; rewrite it in active voice by naming the actor directly, using the surrounding troubleshooting text to state that the check prevents NemoClaw from creating a sandbox with reduced filesystem isolation. Keep the meaning the same, but avoid constructions like “is created” or “being created” in this section.Source: Path instructions
src/lib/onboard.ts (1)
2646-2646: 🩺 Stability & Availability | 🔵 TrivialPlease rerun the onboarding E2E slice for this gate move.
This now short-circuits the core onboard flow before reuse/delete/create paths, so
cloud-e2e,sandbox-operations-e2e,rebuild-openclaw-e2e, andchannels-stop-start-e2eare the highest-value checks before merge.As per path instructions, "
src/lib/onboard.ts: This file contains core onboarding logic. Changes here affect the full sandbox creation and configuration flow."🤖 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.ts` at line 2646, The change in onboard flow around warnIfLandlockUnsupported should be validated by rerunning the onboarding E2E slice, since it now short-circuits before reuse/delete/create paths. Re-run the highest-value checks for cloud-e2e, sandbox-operations-e2e, rebuild-openclaw-e2e, and channels-stop-start-e2e, and confirm the core flow in src/lib/onboard.ts still behaves correctly after the gate move.Source: Path instructions
docs/security/best-practices.mdx (1)
276-276: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse active voice in this row.
managed sandboxes are not createdis passive. Say that NemoClaw does not create managed sandboxes in reduced Landlock mode.As per path instructions, "Active voice required. Flag passive constructions."
🤖 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 `@docs/security/best-practices.mdx` at line 276, The “Default” table row uses passive voice in the NemoClaw onboarding description, so rewrite that sentence in active voice. Update the row text to say that NemoClaw does not create managed sandboxes in reduced Landlock mode when it detects a host or Docker VM kernel older than 5.13, keeping the same meaning while removing “are not created.”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.
Inline comments:
In `@src/lib/onboard/landlock-warning.ts`:
- Around line 12-15: The Landlock support check is currently failing open when
kernel parsing or probing cannot be verified, which allows onboarding to
continue and reuse/create sandbox state unsafely. Update
unsupportedLandlockMessage and the surrounding probe/validation flow so that
parse failures, catch paths, and any inability to confirm support are treated as
blocking conditions instead of returning null; keep the fail-closed behavior in
the Landlock warning path and ensure callers of unsupportedLandlockMessage,
parseKernelMajorMinor, and the probe logic do not proceed when verification is
inconclusive.
- Line 46: The retry hint in landlock-warning.ts hardcodes the command name
instead of using the active CLI alias. Update the message in the onboarding
warning flow to build the retry command from cliName() or cliDisplayName(),
matching the rest of the user-facing onboarding text, so alternate aliases show
the correct `onboard` command. Use the landlock-warning helper that assembles
the warning copy to locate and thread the dynamic CLI name through this string.
---
Nitpick comments:
In `@docs/deployment/sandbox-hardening.mdx`:
- Line 147: The sentence in the sandbox-hardening documentation uses passive
voice; rewrite it in active voice so onboarding is the subject, e.g. have
onboarding directly require kernel 5.13+ with Landlock enabled. Update the
wording in the affected prose block to keep the same meaning while removing the
passive construction.
- Line 138: The CLI example in the shared sandbox-hardening guidance uses a
concrete agent alias and should be generalized for all aliases. Update the
command example in the relevant docs section to use the shared placeholder
symbol $$nemoclaw instead of a specific command name like nemoclaw, so the
rendered guidance is correct across OpenClaw and Hermes variants. Locate the
host CLI example in the sandbox-hardening content and replace the alias-specific
invocation with the shared placeholder consistently.
In `@docs/reference/troubleshooting.mdx`:
- Line 1062: The sentence uses passive voice; rewrite it in active voice by
naming the actor directly, using the surrounding troubleshooting text to state
that the check prevents NemoClaw from creating a sandbox with reduced filesystem
isolation. Keep the meaning the same, but avoid constructions like “is created”
or “being created” in this section.
In `@docs/security/best-practices.mdx`:
- Line 276: The “Default” table row uses passive voice in the NemoClaw
onboarding description, so rewrite that sentence in active voice. Update the row
text to say that NemoClaw does not create managed sandboxes in reduced Landlock
mode when it detects a host or Docker VM kernel older than 5.13, keeping the
same meaning while removing “are not created.”
In `@src/lib/onboard.ts`:
- Line 2646: The change in onboard flow around warnIfLandlockUnsupported should
be validated by rerunning the onboarding E2E slice, since it now short-circuits
before reuse/delete/create paths. Re-run the highest-value checks for cloud-e2e,
sandbox-operations-e2e, rebuild-openclaw-e2e, and channels-stop-start-e2e, and
confirm the core flow in src/lib/onboard.ts still behaves correctly after the
gate move.
🪄 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: cf7d7a3e-9759-4dbd-b2c6-7da90bfbe2c6
📒 Files selected for processing (7)
docs/deployment/sandbox-hardening.mdxdocs/reference/enterprise-readiness.mdxdocs/reference/troubleshooting.mdxdocs/security/best-practices.mdxsrc/lib/onboard.tssrc/lib/onboard/landlock-warning.test.tssrc/lib/onboard/landlock-warning.ts
wscurran
added
area: onboarding
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
LGTM after addressing feedback comments
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
Changes requested — Landlock gate is not authoritative
Do not merge exact head 36ee0b89c3f269df446fcd21e106bc0b74d25b01.
-
The source contract remains wrong.
agents/langchain-deepagents-code/policy-additions.yamlusescompatibility: strict, and the NemoClaw schema permits that value. Pinned OpenShell v0.0.44 recognizes only exacthard_requirement; every other string maps toBestEffort(source). OpenShell v0.0.71 retains the same contract. Therefore #5596 is not a prerequisite for this fix and does not change the relevant policy/schema files. -
The new heuristic fails open and can inspect the wrong kernel. Empty or unparseable probe output and probe exceptions all continue onboarding. A version at or above 5.13 does not prove that Landlock is compiled in, active, syscall-accessible, successfully prepared, or enforced. On Linux, host
uname -ris also not authoritative for a remote Docker daemon, OpenShell gateway, or Kubernetes node. The check applies globally even though other agent policies intentionally usebest_effort. -
The security/lifecycle evidence is incomplete. The two unit tests cover only known 5.12 and 5.13 strings. There is no create/reuse/delete/registry boundary test. The PR Review Advisor remains Blocked, both CodeRabbit threads remain unresolved, and the required E2E advisor jobs were recommended but not run.
Minimal salvage:
- Change the dcode policy from
strictto upstreamhard_requirement, and change the schema enum tobest_effort | hard_requirement. - Add policy/schema regressions that reject
strict, plus a focused onboarding failure test proving no Ready registry entry or stale sandbox when OpenShell rejects Landlock. - Remove the global host-kernel heuristic, or keep it only as scoped, non-authoritative dcode UX; treat OpenShell sandbox-process enforcement as the security boundary.
- Rewrite the documentation as dcode-specific and resolve the two current documentation conflicts after the behavior is correct.
- Run focused config/dcode/onboarding tests, then a supported-kernel dcode Landlock E2E and the advised negative-path E2E.
Co-authored-by: Chengjie Wang <chengjiew@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
cv
added
v0.0.83
|
Maintainer takeover plan for exact head
I will keep progress comments concise. Barring a material maintainer decision or blocker, my next PR comment will link the green staging GCP/Brev launchable build. |
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
|
Status handoff for @prekshivyas:
|
9 participants
Summary
Deep Agents Code now selects OpenShell's exact
hard_requirementLandlock mode, while OpenClaw and Hermes retain their intentionalbest_effortbehavior. NemoClaw treats sandbox-process startup as the enforcement boundary and no longer uses a host kernel-version heuristic as proof for a hard-required sandbox.This PR is not ready to merge until a released OpenShell runtime provides the positive hard-requirement path and transactional failed-create cleanup described below, NemoClaw pins that release, and the required live E2E matrix passes.
This update preserves @chengjiew's original commit and credits Chengjie Wang as co-author of the corrective implementation.
Related Issue
Fixes #5795
Upstream runtime blocker: NVIDIA/OpenShell#2218
Changes
compatibility: hard_requirementand constrain the policy schema to OpenShell's supportedbest_effortandhard_requirementvalues.strictspelling. OpenShell does not recognize it as fail-closed, so accepting it as an alias would silently select best-effort behavior.Created sandbox: <name>output. When exact create-operation identity is unavailable, NemoClaw retains the resource for operator inspection rather than risking deletion of an unrelated same-name sandbox.Runtime acceptance gate
The currently pinned OpenShell 0.0.72 applies directory-only Landlock rights to non-directory policy entries such as
/dev/urandomand/dev/null; hard-requirement startup therefore cannot complete successfully. OpenShell also does not yet expose an identity-bound failed-create cleanup contract. The correct completion path is:ubuntu-repo-cloud-langchain-deepagents-code.Type of Change
Quality Gates
Verification
Verifiedin GitHubnpm testpasses (broad runtime changes only) — exact-head CI is runningnpm run docsbuilds with zero errors; two unrelated pre-existing warnings remainCurrent focused verification:
npx vitest run --project cli src/lib/onboard/sandbox-gpu-create-flow.test.ts src/lib/onboard/created-sandbox-failure.test.ts src/lib/validation.sandbox-create-landlock.test.ts(67 tests)npx vitest run --project integration test/dcode-landlock-contract.test.ts test/onboard-landlock-failure.test.ts test/onboard-sandbox-create-failure.test.ts(8 tests)npx vitest run --project e2e-support test/e2e/support/e2e-phase-onboarding.test.ts(23 tests)npm run typecheck:clinpm run test:projects:check(1,602 candidate files across 8 exact projects)npm run docsgit diff --checkSigned-off-by: Chengjie Wang chengjiew@nvidia.com
Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Signed-off-by: Julie Yaunches jyaunches@nvidia.com