[ericksoa]

Summary

• ulimit hardening now best-effort: ulimit -Hu/-Su 512 fatally exited on container runtimes (e.g., brev) that reject limit modification with "Invalid argument", breaking sandbox bootstrap. Now warns and continues.
• PATH test walks script line-by-line: Replaces the broken grep extraction with a while read loop that evals each line and stops at export PATH=. No hardcoded line numbers, no grep, won't break when lines are added above PATH.

Test plan

• Verify brev bootstrap no longer crashes on ulimit
• Run e2e-gateway-isolation.sh — test 7 (PATH lock) passes
• Verify ulimit warning appears in stderr on restricted runtimes

Summary by CodeRabbit

• Refactor

  • Startup is more resilient in containerized environments: resource-limit adjustments are now best-effort. Failures to tighten limits no longer abort startup and instead emit security warnings.

• Tests

  • Startup verification updated to read and evaluate the entrypoint's exported PATH lines in order, ensuring the runtime receives the expected system-directory-only PATH.

[coderabbitai]

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: 121f4f29-5b29-46aa-9541-e4e9da77503a

📥 Commits

Reviewing files that changed from the base of the PR and between 211b7b2 and 7483d60.

📒 Files selected for processing (1)

• test/e2e-gateway-isolation.sh

🚧 Files skipped from review as they are similar to previous changes (1)

• test/e2e-gateway-isolation.sh

---

📝 Walkthrough

Walkthrough

Replaced fail-fast hard ulimit enforcement with best-effort, non-blocking ulimit calls that suppress stderr and emit warnings on failure; Test 7’s PATH verification now computes PATH by evaluating the entrypoint preamble line-by-line and printing $PATH after the export PATH= line.

Changes

Cohort / File(s)	Summary
Startup ulimit enforcement scripts/nemoclaw-start.sh	Replaced hard-fail (exit 1) on ulimit -Hu/-Su nproc failures with non-blocking best-effort calls that suppress stderr; script now emits a security-warning message when the runtime disallows the change. Other startup hardening remains unchanged.
Test PATH verification logic test/e2e-gateway-isolation.sh	Updated Test 7 to compute the observed PATH by reading and eval-ing the entrypoint preamble line-by-line and stopping after the export PATH= line, then printing the resulting $PATH. Previously sourced only the first extracted export PATH= line via process substitution.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I nudge the limits, quiet and sly,
I try, I warn, I do not cry.
I read the script, line by line,
and show the PATH that feels just fine.
Hop — the startup's calm and spry.

🚥 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 accurately summarizes both main changes: converting ulimit to best-effort (addressing container runtime failures) and fixing a PATH test regression (addressing test reliability).
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/ulimit-best-effort

---

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

[coderabbitai]

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 `@test/e2e-gateway-isolation.sh`:
- Around line 128-129: The current approach uses eval on every line of
/usr/local/bin/nemoclaw-start inside run_as_root (the OUT capture) which
executes pre-PATH control statements and can abort the shell; instead, change
the OUT assignment to extract the export PATH= line without eval and then safely
expand only that RHS. Locate the OUT assignment (the run_as_root "bash -c 'while
IFS= read -r line; do eval ...'") and replace it with logic that greps/awk/sed
for the line beginning with "export PATH=" in /usr/local/bin/nemoclaw-start,
extracts the quoted/unquoted RHS, and echoes that value (or evaluates it in a
tightly-scoped subshell) to set OUT — avoid invoking eval on arbitrary lines or
sourcing the whole file.

🪄 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: bce41f22-b7f3-4e30-ab65-504079c99cd5

📥 Commits

Reviewing files that changed from the base of the PR and between 1e1243d and 7da9aee.

📒 Files selected for processing (2)

• scripts/nemoclaw-start.sh
• test/e2e-gateway-isolation.sh

[kjw3]

Thanks for jumping on the ulimit regression quickly. The scripts/nemoclaw-start.sh change is the right direction, but I don’t think this is ready to merge yet because the PATH test fix is still broken.

The blocker is test/e2e-gateway-isolation.sh test 7:

• the new loop evals each line from /usr/local/bin/nemoclaw-start
• that ends up executing set -euo pipefail, then hitting partial control-flow lines (if ..., else, etc.)
• on the current script, that aborts the shell with exit code 2 before the PATH assertion is actually trustworthy

I reproduced that directly on Mac against the PR branch content.

What I think needs to change to merge:

• keep the best-effort ulimit change
• replace the PATH test logic with something that does not eval arbitrary entrypoint lines
• the test should verify the actual PATH contract safely, not parse-and-execute the entrypoint preamble line-by-line

A couple of safe directions:

• extract just the export PATH=... assignment and evaluate only that RHS in a tightly scoped shell
• or run a minimal real entrypoint path that prints PATH without executing the rest of the control flow

Once the PATH test is rewritten so it is actually safe and reliable, I’d be happy to re-review.