Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fdzdev wants to merge 1 commit into
base: main
Choose a base branch
from
+27 −0
Open

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions bin/lib/onboard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2278,6 +2278,33 @@ async function createSandbox(gpu, model, provider, preferredInferenceApi = null,
run(`bash "${path.join(SCRIPTS, "setup-dns-proxy.sh")}" ${GATEWAY_NAME} "${sandboxName}" 2>&1 || true`, { ignoreError: true });

console.log(` ✓ Sandbox '${sandboxName}' created`);

try {
if (process.platform === "darwin") {
const vmKernel = runCapture("docker info --format '{{.KernelVersion}}'", { ignoreError: true }).trim();
if (vmKernel) {
const parts = vmKernel.split(".");
const major = parseInt(parts[0], 10);
const minor = parseInt(parts[1], 10);
if (!isNaN(major) && !isNaN(minor) && (major < 5 || (major === 5 && minor < 13))) {
console.warn(` ⚠ Landlock: Docker VM kernel ${vmKernel} does not support Landlock (requires ≥5.13).`);
console.warn(" Sandbox filesystem restrictions will silently degrade (best_effort mode).");
}
}
} else if (process.platform === "linux") {
Comment on lines +2283 to +2294
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 31, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

macOS Landlock warning is too narrow and misses the stated security signal.

Line [2283]–Line [2293] only warns on macOS when Docker VM kernel parses as < 5.13. The PR objective/test plan says macOS hosts should warn regardless, so this can silently skip the warning on macOS with kernel >= 5.13 (or unparsable versions).

Suggested fix 
   try {
     if (process.platform === "darwin") {
       const vmKernel = runCapture("docker info --format '{{.KernelVersion}}'", { ignoreError: true }).trim();
-      if (vmKernel) {
-        const parts = vmKernel.split(".");
-        const major = parseInt(parts[0], 10);
-        const minor = parseInt(parts[1], 10);
-        if (!isNaN(major) && !isNaN(minor) && (major < 5 || (major === 5 && minor < 13))) {
-          console.warn(`  ⚠ Landlock: Docker VM kernel ${vmKernel} does not support Landlock (requires ≥5.13).`);
-          console.warn("    Sandbox filesystem restrictions will silently degrade (best_effort mode).");
-        }
-      }
+      console.warn(
+        vmKernel
+          ? `  ⚠ Landlock: macOS host (Docker VM kernel ${vmKernel}). Landlock enforcement may silently degrade (best_effort mode).`
+          : "  ⚠ Landlock: macOS host. Landlock enforcement may silently degrade (best_effort mode)."
+      );
     } else if (process.platform === "linux") {
       const uname = runCapture("uname -r", { ignoreError: true }).trim();
       if (uname) {
         const parts = uname.split(".");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (process.platform === "darwin") {
const vmKernel = runCapture("docker info --format '{{.KernelVersion}}'", { ignoreError: true }).trim();
if (vmKernel) {
const parts = vmKernel.split(".");
const major = parseInt(parts[0], 10);
const minor = parseInt(parts[1], 10);
if (!isNaN(major) && !isNaN(minor) && (major < 5 || (major === 5 && minor < 13))) {
console.warn(` ⚠ Landlock: Docker VM kernel ${vmKernel} does not support Landlock (requires ≥5.13).`);
console.warn(" Sandbox filesystem restrictions will silently degrade (best_effort mode).");
}
}
} else if (process.platform === "linux") {
if (process.platform === "darwin") {
const vmKernel = runCapture("docker info --format '{{.KernelVersion}}'", { ignoreError: true }).trim();
console.warn(
vmKernel
? ` ⚠ Landlock: macOS host (Docker VM kernel ${vmKernel}). Landlock enforcement may silently degrade (best_effort mode).`
: " ⚠ Landlock: macOS host. Landlock enforcement may silently degrade (best_effort mode)."
);
} else if (process.platform === "linux") {
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@bin/lib/onboard.js` around lines 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.

Copy link
Copy Markdown
Contributor Author

@fdzdev fdzdev Apr 3, 2026

Choose a reason for hiding this comment

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

intentional per cv's review — the point is to check the Docker VM kernel and only warn when it doesn't support Landlock (< 5.13). If the VM kernel supports it, there's nothing to warn about. Unconditional warnings are noise.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 3, 2026

Choose a reason for hiding this comment

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

@fdzdev, understood — the conditional check is the right call. Warn only when the Docker VM kernel is verifiably < 5.13, where best_effort actually degrades; stay silent when the VM kernel supports Landlock so there's no false-alarm noise on modern Docker Desktop setups. I'll retract the suggestion.

✏️ Learnings added 
Learnt from: fdzdev
Repo: NVIDIA/NemoClaw PR: 868
File: bin/lib/onboard.js:2283-2294
Timestamp: 2026-04-03T05:28:26.197Z
Learning: In `bin/lib/onboard.js` (NemoClaw project, Node.js), the macOS Landlock warning in `createSandbox()` is intentionally version-gated: it only warns when the Docker VM kernel (`docker info --format '{{.KernelVersion}}'`) parses as < 5.13. Unconditional warnings on macOS are explicitly unwanted — if the VM kernel supports Landlock (>= 5.13), no warning should be emitted.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

const uname = runCapture("uname -r", { ignoreError: true }).trim();
if (uname) {
const parts = uname.split(".");
const major = parseInt(parts[0], 10);
const minor = parseInt(parts[1], 10);
if (!isNaN(major) && !isNaN(minor) && (major < 5 || (major === 5 && minor < 13))) {
console.warn(` ⚠ Landlock: Kernel ${uname} does not support Landlock (requires ≥5.13).`);
console.warn(" Sandbox filesystem restrictions will silently degrade (best_effort mode).");
}
}
}
} catch {}

return sandboxName;
}

Expand Down