Skip to content

fix(security): warn when Landlock may silently degrade#868

Merged
ericksoa merged 3 commits intoNVIDIA:mainfrom
fdzdev:fix/landlock-degradation-warn
Apr 11, 2026
Merged

fix(security): warn when Landlock may silently degrade#868
ericksoa merged 3 commits intoNVIDIA:mainfrom
fdzdev:fix/landlock-degradation-warn

Conversation

@fdzdev
Copy link
Copy Markdown
Contributor

@fdzdev fdzdev commented Mar 25, 2026

Summary

  • The base sandbox policy uses landlock: compatibility: best_effort which silently drops filesystem restrictions on unsupported kernels (CWE-440, NVBUG 6002804)
  • Adds a post-creation check in createSandbox() that warns on macOS hosts and Linux kernels < 5.13
  • Warning only — never blocks sandbox creation (wrapped in try/catch)

Test plan

  • nemoclaw onboard on macOS → see ⚠ Landlock: macOS host warning after sandbox creation
  • nemoclaw onboard on Linux ≥ 5.13 → no warning
  • nemoclaw onboard on Linux < 5.13 → see ⚠ Landlock: Kernel X.Y does not support Landlock warning
  • If uname -r fails for any reason → no crash, no warning (try/catch)

Summary by CodeRabbit

  • New Features
    • Sandbox creation now includes kernel compatibility diagnostics. A warning is displayed if your system's kernel version lacks required filesystem restriction support, with graceful fallback to best-effort restriction mode.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 25, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: aef21de1-5033-4ba7-8332-6576e7d9b474

📥 Commits

Reviewing files that changed from the base of the PR and between 854a046 and 692cb97.

📒 Files selected for processing (1)
  • src/lib/onboard.ts

📝 Walkthrough

Walkthrough

The change adds a post-sandbox-creation diagnostic check that probes host kernel versions to detect Landlock support. On macOS it uses docker info, on Linux it uses uname -r. If the kernel version is below 5.13, warnings are emitted. The logic is wrapped in try-catch to silently ignore failures.

Changes

Cohort / File(s) Summary
Kernel Version Diagnostic Check
src/lib/onboard.ts
Added post-sandbox-creation kernel version probe to check Landlock support. Detects kernel version via docker info (macOS) or uname -r (Linux), emits warnings if version < 5.13, wrapped in try-catch for robustness.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Sandbox sealed, now we check the ground,
Does the kernel speak of Landlock's sound?
Five thirteen or higher, oh what cheer!
If not, we warn without a tear,
Safe fallback paths, forever near! 🛡️

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and specifically describes the main change: adding a warning when Landlock may silently degrade on unsupported kernels. It is concise, clear, and accurately reflects the security-focused fix.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@cv cv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The underlying issue is real — best_effort silently dropping Landlock is worth surfacing. The Linux host kernel check is straightforward and correct since Docker shares the host kernel.

The macOS path is weak though: it warns every macOS user unconditionally when it could just check the Docker VM's actual kernel version via docker info --format '{{.KernelVersion}}'. That gives you the VM kernel without even spinning up a container. If that's ≥ 5.13, there's nothing to warn about.

As-is, the macOS warning is noisy without being actionable — it tells the user "depends on the Docker VM kernel" but doesn't do the one thing that would answer the question.

@cv
Copy link
Copy Markdown
Contributor

cv commented Mar 25, 2026

FYI — OpenShell is already tracking this upstream:

Once that lands, OpenShell itself will report whether Landlock enforcement actually stuck, which makes the host-side kernel guessing here unnecessary.

@fdzdev fdzdev force-pushed the fix/landlock-degradation-warn branch from e06072c to 93db763 Compare March 26, 2026 00:13
@wscurran wscurran added bug Something isn't working Platform: MacOS Support for MacOS security Something isn't secure Platform: Ubuntu Support for Linux Ubuntu priority: high Important issue that should be resolved in the next release labels Mar 30, 2026
- Check Docker VM kernel version on macOS via docker info (actionable, not unconditional)
- Check host kernel version on Linux via uname -r
- Warn only when kernel < 5.13 (Landlock minimum)
- Warning only — never blocks sandbox creation (wrapped in try/catch)

Made-with: Cursor
@fdzdev fdzdev force-pushed the fix/landlock-degradation-warn branch from 93db763 to 3d2d2bf Compare March 31, 2026 05:23
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@bin/lib/onboard.js`:
- Around line 2283-2294: The macOS branch currently only prints the Landlock
warning when the Docker VM kernel parses as <5.13; change it so macOS always
emits the Landlock warning to match the PR/test plan: keep the existing
runCapture("docker info...") and parsing of vmKernel but always log a general
macOS Landlock warning (using process.platform === "darwin"), and if vmKernel is
present and parses to a version <5.13 add the existing specific message about
lack of Landlock support; if vmKernel is unparsable still emit the general
warning (and optionally include the raw vmKernel value) so the security signal
is never silently skipped.
🪄 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: Pro

Run ID: 598da946-0617-4fc1-8498-b067424d8b1c

📥 Commits

Reviewing files that changed from the base of the PR and between 93db763 and 3d2d2bf.

📒 Files selected for processing (1)
  • bin/lib/onboard.js

@fdzdev
Copy link
Copy Markdown
Contributor Author

fdzdev commented Mar 31, 2026

@cv Addressed — macOS path now checks the Docker VM kernel via docker info --format '{{.KernelVersion}}' and only warns when < 5.13. No more unconditional warning.

Also noting this is an interim measure until openshell#599 lands upstream.

@cv cv added the v0.0.11 Release target label Apr 9, 2026
@cv
Copy link
Copy Markdown
Contributor

cv commented Apr 9, 2026

I attempted to port this branch across the JS→TS migration and merge the latest main, but it still needs manual follow-up.

Please start with:

git fetch origin
git merge origin/main
npx tsx scripts/ts-migration-assist.ts --base origin/main --write
npm run build:cli
npm run typecheck:cli
npm run lint
npm test

@cv cv added v0.0.12 Release target v0.0.13 Release target and removed v0.0.11 Release target v0.0.12 Release target labels Apr 10, 2026
…ion-warn

Signed-off-by: Test User <test@example.com>
@ericksoa ericksoa merged commit 035144e into NVIDIA:main Apr 11, 2026
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Platform: MacOS Support for MacOS Platform: Ubuntu Support for Linux Ubuntu priority: high Important issue that should be resolved in the next release security Something isn't secure v0.0.13 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants