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 79%, 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 |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
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 — No blocking findingsMerge posture: No blocking advisor findings 1 warning · 0 optional suggestionsWarningsThese merit maintainer attention but do not block by themselves.
|
![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>
|
Release disposition: deferring this PR from Exit criteria for the next release target remain: (1) land and pin an upstream/runtime fix compatible with the DCode policy, (2) update this branch conflict-free onto current |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
Current upstream recheck at exact head
The required unblock remains an upstream file-type-aware Landlock rule fix, followed by a reviewed released pin. Only then should we port the Landlock-specific create-failure cleanup through current main and require a real successful DCode |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
Release sweep: deferring this PR from This remains blocked on a released, file-type-aware OpenShell Landlock implementation rather than only on branch drift. The current hard requirement still names device files that the available policy builder cannot enforce successfully, and the committed fixture proves a synthetic failure path rather than successful DCode startup. The branch also conflicts with current Re-target after the runtime fix is released and pinned, then require positive DCode startup/inference proof plus negative cleanup and registry-state proof. |
9 participants
Summary
Deep Agents Code now fails closed at OpenShell's authoritative sandbox-process Landlock boundary by using the exact
hard_requirementcompatibility mode.OpenClaw and Hermes keep their intentional
best_effortbehavior, and NemoClaw no longer treats a host kernel-version heuristic as proof of sandbox enforcement.This update preserves @chengjiew's original commit and credits Chengjie Wang as co-author of the corrective implementation.
Related Issue
Fixes #5795
Changes
compatibility: hard_requirementand constrain the policy schema to OpenShell's supportedbest_effortandhard_requirementvalues.strictvalue.Type of Change
Quality Gates
Verification
Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpasses — all non-test-clicommit hooks and all pre-push hooks passed; the all-repositorytest-clihook was attempted four times and blocked only on unrelated macOS/toolchain or timeout flakes, with each residual failing file passing on immediate isolated rerun. Exact-head Linux CI is pending.npm testpasses (broad runtime changes only) — not run separately; focused CLI/integration coverage and exact-head CI/E2E are the relevant gates for this policy change.npm run docsbuilds without warnings (doc changes only) — build passes with zero errors and two unrelated pre-existing warnings.Focused verification at exact head
de42e632d:npx vitest run src/lib/onboard/landlock-warning.test.ts src/lib/onboard/initial-policy-real-policy.test.ts test/langchain-deepagents-code-image.test.ts test/onboard-landlock-failure.test.ts test/validate-config-schemas.test.ts(161 tests)npm run typecheck:clinpm run validate:configsnpm run test:projects:checknpm run test:titles:checknpm run test-conditionals:scan -- --top 25npm run test-size:checknpm run docs(zero errors; two unrelated pre-existing warnings)git diff --checkGitHub verification:
36ee0b89c,a7e031caf,98f162ce4,d922d3a19,6fc078c23,fca7dc27b, andde42e632dare allVerifiedwith reasonvalid.Signed-off-by: Chengjie Wang chengjiew@nvidia.com
Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit