docs/security/process-limits.md (2)

33-34: One sentence per line.

Per docs style guide, each sentence should be on its own line in the source to make diffs more readable.

📝 Suggested fix

 The default process limit is 512 per sandbox.
-agent operation (typically < 50 processes) whilst preventing fork bombs.
+This is sufficient for normal agent operation (typically < 50 processes) whilst preventing fork bombs.

Full replacement:

The default process limit is 512 per sandbox.
This is sufficient for normal agent operation (typically < 50 processes) whilst preventing fork bombs.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/security/process-limits.md` around lines 33 - 34, Split the two
sentences in the paragraph into separate source lines so each sentence is on its
own line: replace the single-line text "The default process limit is 512 per
sandbox. This is sufficient for normal agent operation (typically < 50
processes) whilst preventing fork bombs." with two lines — "The default process
limit is 512 per sandbox." and "This is sufficient for normal agent operation
(typically < 50 processes) whilst preventing fork bombs." — so diffs are more
readable per the docs style guide.

---

64-64: Consider adding a "Next Steps" section.

Per docs style guide, pages should include a "Next Steps" section at the bottom linking to related pages (e.g., sandbox policy documentation, security overview).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/security/process-limits.md` at line 64, Add a "Next Steps" section at
the end of the process-limits documentation: create a new heading "Next Steps"
(or "Next steps") and include bullet links to related pages such as the sandbox
policy documentation and the security overview so readers can navigate to those
resources from the Process Limits page; ensure link text is descriptive and
follows the existing docs style guide conventions.

bin/lib/onboard.js (1)

1511-1512: ⚠️ Potential issue | 🟠 Major

Reject NEMOCLAW_PIDS_LIMIT=0 to avoid silently disabling PID protection.

Line 1512 uses /^\d+$/, which still allows 0. For this security control, treat 0 as invalid and fall back to 512 (or fail fast).

In current Docker documentation, what is the behavior of `docker update --pids-limit 0`? Does it mean "unlimited"?

🔧 Suggested patch

-  const pidsLimit = /^\d+$/.test(rawPidsLimit) ? rawPidsLimit : "512";
+  const pidsLimit = /^[1-9]\d*$/.test(rawPidsLimit) ? rawPidsLimit : "512";

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bin/lib/onboard.js` around lines 1511 - 1512, The current parsing of
NEMOCLAW_PIDS_LIMIT accepts "0" which disables PID protection; update the
validation for rawPidsLimit (env var NEMOCLAW_PIDS_LIMIT) so that pidsLimit only
accepts positive integers > 0 (e.g., via /^\d+$/ plus explicit check that the
parsed value !== 0 or parseInt(rawPidsLimit,10) > 0); if validation fails, fall
back to "512" (or alternatively throw/fail fast) and ensure the code sets the
pidsLimit constant accordingly.

bin/lib/onboard.js (1)

1513-1521: Use exit status, not output text, to determine docker update success.

runCapture(..., { ignoreError: true }) discards failure details and returns "", so the includes("Error") check is not a reliable signal. Prefer status-based handling so logs are deterministic and actionable.

♻️ Suggested refactor

-  const dockerUpdate = runCapture(
-    `docker update --pids-limit ${pidsLimit} "${sandboxName}" 2>&1`,
-    { ignoreError: true }
-  );
-  if (dockerUpdate && !dockerUpdate.includes("Error")) {
+  const dockerUpdate = spawnSync(
+    "bash",
+    ["-lc", `docker update --pids-limit ${pidsLimit} "${sandboxName}"`],
+    { cwd: ROOT, encoding: "utf8" }
+  );
+  if (dockerUpdate.status === 0) {
     console.log(`  ✓ Process limit set (--pids-limit ${pidsLimit})`);
   } else {
+    const detail = String(dockerUpdate.stderr || dockerUpdate.stdout || "").trim();
     console.log(`  ⓘ Could not set --pids-limit (container runtime may not support it)`);
+    if (detail) console.log(`    Detail: ${detail}`);
     console.log("    The in-sandbox ulimit fallback in nemoclaw-start.sh will apply.");
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bin/lib/onboard.js` around lines 1513 - 1521, The success check for `docker
update` is using the captured stdout string (`dockerUpdate.includes("Error")`)
which is unreliable because `runCapture(..., { ignoreError: true })` can return
an empty string on failure; update the logic in onboard.js to inspect the
command exit status (or use a non-ignored run that throws on non-zero) instead
of text-matching: call the underlying runner (or adjust `runCapture`) so you
capture an explicit success boolean or exit code for the `docker update
--pids-limit ${pidsLimit} "${sandboxName}"` invocation, then log the success
message when exit code === 0 (or success === true) and the fallback/info
messages on non-zero exit, referencing the `runCapture` call and the
`dockerUpdate` handling around that command.

bin/lib/onboard.js (1)

1897-1898: ⚠️ Potential issue | 🟡 Minor

NEMOCLAW_PIDS_LIMIT=0 silently disables fork bomb protection.

The regex /^\d+$/ allows "0", but Docker interprets --pids-limit 0 as unlimited, which would silently disable the protection instead of falling back to the default 512. This was noted in the previous review discussion.

🛡️ Proposed fix to reject zero

 const rawPidsLimit = process.env.NEMOCLAW_PIDS_LIMIT || "512";
-const pidsLimit = /^\d+$/.test(rawPidsLimit) ? rawPidsLimit : "512";
+const pidsLimit = /^[1-9][0-9]*$/.test(rawPidsLimit) ? rawPidsLimit : "512";

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bin/lib/onboard.js` around lines 1897 - 1898, The pids-limit parsing allows
"0" which Docker treats as unlimited; update the validation around
rawPidsLimit/pidsLimit so it only accepts positive integers (>0) and falls back
to "512" for any non-positive or non-numeric value (e.g., reject "0", negative
numbers, empty string); modify the /^\d+$/ check to also ensure numeric value
!== "0" or parseInt(rawPidsLimit,10) > 0 and set pidsLimit accordingly using the
existing rawPidsLimit and pidsLimit identifiers.