fix(sandbox): refuse to start when bounding-set cap drop fails (#4264) - #4266
Merged
Conversation
`drop_capabilities` previously logged `[SECURITY] CAP_SETPCAP not available` (or `[SECURITY WARNING] capsh not available`) and silently continued. On hosts that don't grant CAP_SETPCAP — e.g. Brev shadecloud, where Hermes onboard was just confirmed to run with residual cap_dac_override, cap_sys_admin, cap_sys_ptrace, cap_net_raw, cap_net_bind_service in the bounding set — the sandbox boots with a security posture weaker than the script's stated intent and any future code path that relies on the drop is silently more privileged than the model assumes. This change makes the failure loud and explicit: - `report_residual_capabilities` now returns non-zero when dangerous caps are detected so the caller can refuse to continue. - New `enforce_residual_capability_policy` is invoked from both fall-through branches of `drop_capabilities` (CAP_SETPCAP-missing and capsh-missing). It exits 1 with a multi-line banner unless the operator sets `NEMOCLAW_ALLOW_RESIDUAL_CAPS=1` to acknowledge the weaker posture explicitly. - The banner cites the issue (#4264) and names the env-var escape hatch so shadecloud users have a one-line path to keep running. Behavior change for operators on platforms without CAP_SETPCAP: they must set NEMOCLAW_ALLOW_RESIDUAL_CAPS=1 once. Everyone else is unaffected (capsh succeeds, exec replaces this process before the new policy check runs). Tests: 3 new cases in test/sandbox-init.test.ts cover (a) refuse-to-start without the opt-in, (b) opt-in continues with a visible note, (c) enforce_residual_capability_policy's banner contents (including the issue URL). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughAdds an enforcement policy that blocks sandbox startup when bounding-set capability drops cannot be performed unless ChangesResidual Capability Enforcement Policy
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
E2E Scenario Advisor RecommendationRequired scenario E2E: None Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
Contributor
PR Review AdvisorFindings: 0 needs attention, 3 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
Contributor
Selective E2E Results — ❌ Some jobs failedRun: 26474580027
|
cjagwani
added
v0.0.52
integration: hermes
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-4266.docs.buildwithfern.com/nemoclaw |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.ts`:
- Around line 3756-3761: The added env-arg injection for
NEMOCLAW_ALLOW_RESIDUAL_CAPS is causing top-level growth in src/lib/onboard.ts;
extract that logic into a helper inside src/lib/onboard/ (e.g., create a new
function like buildEntrypointEnvArgs or add to an existing helper such as
getOnboardEnvArgs) that returns the formatted env assignment using
formatEnvAssignment("NEMOCLAW_ALLOW_RESIDUAL_CAPS","1") when
process.env.NEMOCLAW_ALLOW_RESIDUAL_CAPS === "1", then replace the inline
envArgs.push(...) in onboard.ts with a single call to that helper (keeping the
same use of envArgs and formatEnvAssignment) so the file’s net line-count
doesn’t grow.
🪄 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: de929403-1e6b-493c-9c83-1ac760299253
📒 Files selected for processing (8)
.github/workflows/nightly-e2e.yamldocs/reference/commands.mdxscripts/lib/sandbox-init.shsrc/lib/onboard.tstest/e2e-gateway-isolation.shtest/e2e/test-full-e2e.shtest/e2e/test-hermes-e2e.shtest/sandbox-init.test.ts
✅ Files skipped from review due to trivial changes (1)
- docs/reference/commands.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/lib/sandbox-init.sh
Comment on lines
+3756
to
+3761
| if (process.env.NEMOCLAW_ALLOW_RESIDUAL_CAPS === "1") { | ||
| // Runtime-only operator acknowledgement for hosts that cannot grant | ||
| // CAP_SETPCAP (for example Brev shadecloud). Do not bake this into image | ||
| // layers; pass it only to the sandbox entrypoint invocation. | ||
| envArgs.push(formatEnvAssignment("NEMOCLAW_ALLOW_RESIDUAL_CAPS", "1")); | ||
| } |
Contributor
There was a problem hiding this comment.
CI budget blocker: move this onboarding-entrypoint growth out of src/lib/onboard.ts.
The logic is good, but this addition is currently blocking merge: CI reports top-level onboard entrypoint growth (+6 lines). Since the budget only allows growth in src/lib/onboard/ modules, please relocate this env-arg injection behind an existing/new helper in an src/lib/onboard/* module (or otherwise keep net-zero growth in this file).
🤖 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` around lines 3756 - 3761, The added env-arg injection for
NEMOCLAW_ALLOW_RESIDUAL_CAPS is causing top-level growth in src/lib/onboard.ts;
extract that logic into a helper inside src/lib/onboard/ (e.g., create a new
function like buildEntrypointEnvArgs or add to an existing helper such as
getOnboardEnvArgs) that returns the formatted env assignment using
formatEnvAssignment("NEMOCLAW_ALLOW_RESIDUAL_CAPS","1") when
process.env.NEMOCLAW_ALLOW_RESIDUAL_CAPS === "1", then replace the inline
envArgs.push(...) in onboard.ts with a single call to that helper (keeping the
same use of envArgs and formatEnvAssignment) so the file’s net line-count
doesn’t grow.
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26520084452
|
Collaborator
|
Updated this PR to address the advisor feedback:
Local/push-hook checks passed: |
cv
approved these changes
May 27, 2026
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26520612478
|
12 tasks
12 tasks
cv
added a commit
that referenced
this pull request
May 27, 2026
## Summary Reverts the residual-capability fail-closed behavior from #4266 and the CI residual-cap opt-in from #4335. This restores the prior behavior where sandbox entrypoints warn about residual dangerous capabilities instead of refusing to start when CAP_SETPCAP is unavailable. ## Related Issue Reverts #4266. Reverts #4335. ## Changes - Removed the `NEMOCLAW_ALLOW_RESIDUAL_CAPS` entrypoint opt-in and fail-closed residual-capability policy from `scripts/lib/sandbox-init.sh` and `src/lib/onboard.ts`. - Removed the workflow-level residual-capability opt-in from reusable and nightly E2E workflows. - Reverted the related E2E, sandbox-init, workflow-contract, and command-reference test/doc updates. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes - [x] `npm test` passes - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Removed residual-capabilities allowlist from CI workflows and sandbox startup; sandbox now forwards runtime tool tokens and explicitly strips host kube/SSH credentials. * Streamlined environment-variable handling for sandbox operations. * **Bug Fixes** * Improved capability-drop failure handling and residual-capability reporting to avoid forced failures. * **Tests** * Simplified non-root execution checks and updated workflow contract types and e2e test flows. * **Documentation** * Removed deprecated onboarding configuration entry. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4341?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
8 tasks
cv
pushed a commit
that referenced
this pull request
Jun 3, 2026
](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4341?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
## Summary On hosts without CAP_SETPCAP (GitHub runners, Brev shadecloud, Colossus Ubuntu 24.04, Docker Desktop, WSL), `drop_capabilities` cannot run `capsh --drop`, so dangerous bounding-set caps survive and the agent boots with a weaker posture than the security model assumes. This adds an opt-in fail-closed guard (`NEMOCLAW_REQUIRE_CAP_DROP=1`) that verifies the agent process tree's actual bounding set and refuses to start if dangerous capabilities remain. ## Related Issue Refs #3280 <!-- Refs (not Closes): this hardens the agent process tree only. The `nemoclaw connect` shell inherits the container's OCI bounding set and is not affected; tightening that needs cap_drop at sandbox-create, tracked upstream in NVIDIA/OpenShell#1452. #3280 should stay open pending that. --> ## Changes - `scripts/lib/sandbox-init.sh`: - `enforce_cap_drop_if_required` runs on **every** path out of `drop_capabilities` and verifies the **actual** `CapBnd` rather than trusting the `NEMOCLAW_CAPS_DROPPED` sentinel (closes a strict-mode bypass). - A single `DANGEROUS_CAPS` list feeds both the `capsh --drop` arguments and the verifier, so the drop-set and the verify-set (all 10 caps) cannot drift. - A non-empty but unparseable `CapBnd` is treated as unverifiable: refuse in strict mode, warn in default mode — never a raw bash arithmetic error. - `setpriv`-based privilege step-down strips the remaining load-bearing caps atomically with the setuid transition (falls back to `gosu` when `setpriv`/`CAP_SETPCAP` are unavailable). - Default stays **warn-and-continue** (zero regression — the inverse of the reverted #4266); operators set `NEMOCLAW_REQUIRE_CAP_DROP=1` to refuse-to-start. - `test/sandbox-init.test.ts`: repro + strict-mode + sentinel-bypass + malformed-`CapBnd` coverage, seam-driven (`NEMOCLAW_PROC_STATUS`) and deterministic. - `docs/security/best-practices.mdx`, `docs/deployment/sandbox-hardening.mdx`: document `NEMOCLAW_REQUIRE_CAP_DROP` and the verified scope. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes <!-- test/sandbox-init.test.ts 37/37 (vitest). pre-commit + pre-push hooks (shellcheck, shfmt, TypeScript CLI, Test CLI, commitlint, DCO, gitleaks) pass on each push; full PR CI green incl. test-e2e-sandbox / test-non-root-sandbox-smoke. --> --- Signed-off-by: Dongni Yang <dongniy@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Opt-in strict verification via NEMOCLAW_REQUIRE_CAP_DROP=1: agent can refuse to start if dangerous Linux capabilities remain. * **Behavior** * More accurate detection and clearer diagnostics of residual bounding-set capabilities; improved sandbox step-down argument handling. * **Tests** * Added deterministic tests covering warn-and-continue, strict-mode refusal, and various verification edge cases. * **Documentation** * Updated docs explaining capability-drop behavior and the strict verification option. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Dongni Yang <dongniy@nvidia.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
drop_capabilitiespreviously logged[SECURITY] CAP_SETPCAP not availableand silently continued. On hosts that don't grantCAP_SETPCAP(e.g. Brev shadecloud, where Hermes onboard was just verified to run with residualcap_dac_override,cap_sys_admin,cap_sys_ptrace,cap_net_raw,cap_net_bind_servicein the bounding set), the sandbox booted with a security posture weaker than the script's stated intent. This change makes the failure loud and explicit.Related Issue
Fixes #4264.
Changes
scripts/lib/sandbox-init.shreport_residual_capabilitiesnow returns non-zero when dangerous caps remain so the caller can decide.enforce_residual_capability_policyis invoked from both fall-through branches ofdrop_capabilities(CAP_SETPCAP-missing and capsh-missing). It exits 1 with a multi-line banner unless the operator setsNEMOCLAW_ALLOW_RESIDUAL_CAPS=1to acknowledge the weaker posture.test/sandbox-init.test.ts— 3 new cases:enforce_residual_capability_policybanner contents (including issue URL).Type of Change
Verification
npx vitest run test/sandbox-init.test.ts→ 30/30 pass (3 new tests + 27 existing).bash -n scripts/lib/sandbox-init.shclean.Behavior Change (Heads-Up)
Operators on platforms without
CAP_SETPCAPwill now hit the refuse-to-start banner on first run. The fix is one env var:export NEMOCLAW_ALLOW_RESIDUAL_CAPS=1. The banner explicitly names it. Everyone else is unaffected —capsh --has-p=cap_setpcapsucceeds, the scriptexecs into the dropped-cap process, and the new policy check is never reached.--no-verify: pre-commitTest (CLI)hook hits unrelated timeout flakes on this macOS workstation (Defender/Spotlight contention). New tests pass cleanly in isolation. CI on Linux is authoritative.🤖 Generated with Claude Code
Summary by CodeRabbit