docs: improve security best practices diagram - #1210
Merged
cv merged 1 commit intoMar 31, 2026
Conversation
- Redesign Protection Layers diagram for clarity - Show agent grounded in NemoClaw ecosystem (Host > NemoClaw > Sandbox) - Use plain language for layer descriptions (what each controls) - Add NVIDIA brand styling to match other project diagrams - Note diagram shows default posture after nemoclaw onboard Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
miyoungc
approved these changes
Mar 31, 2026
cv
marked this pull request as ready for review
March 31, 2026 22:48
wscurran
added
area: docs
21 tasks
cv
added a commit
that referenced
this pull request
Jul 4, 2026
<!-- markdownlint-disable MD041 --> ## Summary Adds a shared tool-disclosure mode across Deep Agents Code, Hermes, and OpenClaw. New sandboxes default to `progressive`; `--tool-disclosure direct` or `NEMOCLAW_TOOL_DISCLOSURE=direct` restores the prior fully visible catalog. The mode persists through resume and transactional rebuilds. The design follows [LangChain progressive disclosure](https://support.langchain.com/articles/8488719552-progressive-tool-disclosure-with-deep-agents), [Hermes Tool Search](https://hermes-agent.nousresearch.com/docs/user-guide/features/tool-search), and the pinned [OpenClaw v2026.5.27 Tool Search contract](https://github.com/openclaw/openclaw/blob/v2026.5.27/docs/tools/tool-search.md). This is the v0.0.74 bundle. Exact head `e4214144c11dd47b9d3185f19c1c73ec4e690552` includes `main` at `6f5ccbcbaa87367f7b521318fbc940467045b601`. ## Related Issue #5876 is merged. This PR has no remaining stacked dependency and preserves its managed-MCP ownership contract. ## Changes - Add the shared `--tool-disclosure progressive|direct` onboarding and rebuild option, with `NEMOCLAW_TOOL_DISCLOSURE` as the environment equivalent. CLI input takes precedence and fresh sandboxes default to `progressive`. - Persist the selected mode in session and registry state. Resume, recreation, and MCP-bearing rebuilds preserve or transactionally change it in either direction. - Add bounded `search_tools` middleware to Deep Agents Code when at least one MCP tool loads successfully. Core tools remain visible, discovery state survives checkpointing, and only model requests are filtered; the complete executor registry and existing authorization controls remain intact. - Enable Hermes native Tool Search with opinionated 5/20 result limits and direct core tools. - Enable OpenClaw native structured `mode: "tools"` discovery with 8/20 limits while retaining model-specific compatibility safeguards. - Patch and validate the exact pinned `deepagents-code==0.1.30` runtime with fail-closed, idempotent anchors and separate middleware instances for the main agent and local subagents. - Add build-time/runtime validators, lifecycle and overflow coverage, transactional custom-image handling, and user-facing documentation for progressive/direct behavior. ### Managed MCP boundary This PR does not change MCP configuration, credential placeholders, bridge ownership, policy generation, MCP CLI behavior, or registry schemas from #5876. Managed OpenClaw MCP remains registered in mcporter's home registry. Pinned OpenClaw Tool Search reads native bundle and `mcp.servers` catalogs, and NemoClaw does not currently synchronize those ownership stores. The OpenClaw runtime validator therefore proves native search, describe, and call behavior for eligible hidden catalog tools, while the existing live MCP scenario separately proves mcporter execution, credential rewriting and rotation, restart/rebuild behavior, policy enforcement, DNS-rebinding protection, and secret boundaries. A fixture-only projection between those stores would not represent shipped behavior. ### Apurv security review resolution 1. **Callable namespace:** Deep Agents Code rejects every duplicate resolved callable name and every non-managed owner of a reserved core name before the original factory in progressive and direct modes. Coverage includes regular/regular, regular/MCP, cross-MCP-server, direct-mode, and schema/executor mismatch cases. 2. **Build-context seal:** fingerprints require a real directory root and encode portable mode bits, nanosecond modification times, link counts, and deterministic hardlink topology. Pre-delete and final one-shot tests cover hardlink, timestamp, permission, and root-symlink mutations. 3. **Dockerfile patch boundary:** patching anchors and revalidates the staging parent, rejects multi-link files, writes a fresh private same-directory file, and atomically replaces the staged Dockerfile without truncating an attacker-selectable inode. 4. **Chunk-safe diagnostics:** raw cloudflared child output is not emitted in failures. Only bounded per-stream carry is retained for origin discovery, with split-event credential regressions proving fragments and reconstructed secrets remain absent. ### CodeRabbit and CodeQL resolution - Every CodeRabbit finding is implemented or explicitly dispositioned with repository evidence, including URL-token punctuation, the intentional private hosted model identifier, and the final Darwin fixture fail-fast nit. GraphQL reports zero unresolved review threads; exact-head CodeRabbit status is green. - [CodeQL alert #1210](https://github.com/NVIDIA/NemoClaw/security/code-scanning/1210) is dismissed as a false positive. The flagged sink opens an existing directory read-only with `O_DIRECTORY | O_NOFOLLOW | O_NONBLOCK`; it does not use `O_CREAT`. The real replacement file remains UUID-named, `O_CREAT | O_EXCL | O_NOFOLLOW`, mode `0600`, descriptor/path identity checked, single-link validated, fsynced, and atomically renamed. Exact-head CodeQL is green with no open branch alerts. ### Validation - Local focused matrix: 19 files / 327 tests passed; one true Linux-only `memfd`/`O_TMPFILE` restart case was skipped on macOS. - Exact pinned Deep Agents Code/LangChain patch and runtime validator passed: `progressive-disclosure-runtime-ok`. - Both TypeScript typechecks, build, repository checks, source-shape/test-size guards, Biome, Ruff/Python compilation, secret scanning, push checks, and `git diff --check` passed. - Final-head standard validation is green: [PR CI](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184882), [growth guardrails](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184428), [WSL](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184879), [macOS](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184874), [security](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184917), [CodeQL](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184138), [review advisors](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184863), and the [E2E advisor](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184880). The original exact-head check set and subsequent metadata checks are all green, with only the two expected skips. - The exact-head [typed OpenClaw and Deep Agents targets](https://github.com/NVIDIA/NemoClaw/actions/runs/28700290435) passed. - The exact-head [production-image workflow](https://github.com/NVIDIA/NemoClaw/actions/runs/28700292369) passed both requested image builds and all four downstream image E2E checks. - In the exact-head [advisor-selected live union](https://github.com/NVIDIA/NemoClaw/actions/runs/28700286465), 18 of 19 concrete jobs passed. The sole Hermes security-posture failure occurred in its deliberate gateway-recovery probe after restart, secret-boundary, and disclosure assertions had passed. The isolated exact-head [security retry](https://github.com/NVIDIA/NemoClaw/actions/runs/28700509914) passed both Hermes and OpenClaw legs, so every selected live lane has passing final-head evidence. - The GPT review advisor's remaining OpenClaw MCP model-loop warning is explicitly dispositioned by the managed-MCP ownership boundary above: the native OpenClaw catalog and mcporter registry are separate shipped stores, and a test-only projection would claim behavior the production bridge does not provide. ### Remaining - Obtain Apurv's exact-head re-review to clear the formal `CHANGES_REQUESTED` state and complete sensitive-path approval. - Rebuild affected existing sandboxes after this change ships; newly created sandboxes default to progressive disclosure. ## 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) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Apurv exact-head re-review pending after all four requested fixes. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: no waiver requested; exact-head security retry is green. ## Verification - [x] PR description includes the DCO sign-off declaration and every feature commit appears as `Verified` in GitHub - [ ] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes — all applicable hooks passed; the known hanging local unsharded `test-cli` coverage hook was skipped and exact-head sharded Linux CI is authoritative. - [x] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [ ] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only; current build has two pre-existing warnings) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [x] New doc pages include SPDX header and frontmatter (new pages only; no new doc pages) --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
<!-- markdownlint-disable MD041 --> ## Summary Adds a shared tool-disclosure mode across Deep Agents Code, Hermes, and OpenClaw. New sandboxes default to `progressive`; `--tool-disclosure direct` or `NEMOCLAW_TOOL_DISCLOSURE=direct` restores the prior fully visible catalog. The mode persists through resume and transactional rebuilds. The design follows [LangChain progressive disclosure](https://support.langchain.com/articles/8488719552-progressive-tool-disclosure-with-deep-agents), [Hermes Tool Search](https://hermes-agent.nousresearch.com/docs/user-guide/features/tool-search), and the pinned [OpenClaw v2026.5.27 Tool Search contract](https://github.com/openclaw/openclaw/blob/v2026.5.27/docs/tools/tool-search.md). This is the v0.0.74 bundle. Exact head `e4214144c11dd47b9d3185f19c1c73ec4e690552` includes `main` at `6f5ccbcbaa87367f7b521318fbc940467045b601`. ## Related Issue NVIDIA#5876 is merged. This PR has no remaining stacked dependency and preserves its managed-MCP ownership contract. ## Changes - Add the shared `--tool-disclosure progressive|direct` onboarding and rebuild option, with `NEMOCLAW_TOOL_DISCLOSURE` as the environment equivalent. CLI input takes precedence and fresh sandboxes default to `progressive`. - Persist the selected mode in session and registry state. Resume, recreation, and MCP-bearing rebuilds preserve or transactionally change it in either direction. - Add bounded `search_tools` middleware to Deep Agents Code when at least one MCP tool loads successfully. Core tools remain visible, discovery state survives checkpointing, and only model requests are filtered; the complete executor registry and existing authorization controls remain intact. - Enable Hermes native Tool Search with opinionated 5/20 result limits and direct core tools. - Enable OpenClaw native structured `mode: "tools"` discovery with 8/20 limits while retaining model-specific compatibility safeguards. - Patch and validate the exact pinned `deepagents-code==0.1.30` runtime with fail-closed, idempotent anchors and separate middleware instances for the main agent and local subagents. - Add build-time/runtime validators, lifecycle and overflow coverage, transactional custom-image handling, and user-facing documentation for progressive/direct behavior. ### Managed MCP boundary This PR does not change MCP configuration, credential placeholders, bridge ownership, policy generation, MCP CLI behavior, or registry schemas from NVIDIA#5876. Managed OpenClaw MCP remains registered in mcporter's home registry. Pinned OpenClaw Tool Search reads native bundle and `mcp.servers` catalogs, and NemoClaw does not currently synchronize those ownership stores. The OpenClaw runtime validator therefore proves native search, describe, and call behavior for eligible hidden catalog tools, while the existing live MCP scenario separately proves mcporter execution, credential rewriting and rotation, restart/rebuild behavior, policy enforcement, DNS-rebinding protection, and secret boundaries. A fixture-only projection between those stores would not represent shipped behavior. ### Apurv security review resolution 1. **Callable namespace:** Deep Agents Code rejects every duplicate resolved callable name and every non-managed owner of a reserved core name before the original factory in progressive and direct modes. Coverage includes regular/regular, regular/MCP, cross-MCP-server, direct-mode, and schema/executor mismatch cases. 2. **Build-context seal:** fingerprints require a real directory root and encode portable mode bits, nanosecond modification times, link counts, and deterministic hardlink topology. Pre-delete and final one-shot tests cover hardlink, timestamp, permission, and root-symlink mutations. 3. **Dockerfile patch boundary:** patching anchors and revalidates the staging parent, rejects multi-link files, writes a fresh private same-directory file, and atomically replaces the staged Dockerfile without truncating an attacker-selectable inode. 4. **Chunk-safe diagnostics:** raw cloudflared child output is not emitted in failures. Only bounded per-stream carry is retained for origin discovery, with split-event credential regressions proving fragments and reconstructed secrets remain absent. ### CodeRabbit and CodeQL resolution - Every CodeRabbit finding is implemented or explicitly dispositioned with repository evidence, including URL-token punctuation, the intentional private hosted model identifier, and the final Darwin fixture fail-fast nit. GraphQL reports zero unresolved review threads; exact-head CodeRabbit status is green. - [CodeQL alert NVIDIA#1210](https://github.com/NVIDIA/NemoClaw/security/code-scanning/1210) is dismissed as a false positive. The flagged sink opens an existing directory read-only with `O_DIRECTORY | O_NOFOLLOW | O_NONBLOCK`; it does not use `O_CREAT`. The real replacement file remains UUID-named, `O_CREAT | O_EXCL | O_NOFOLLOW`, mode `0600`, descriptor/path identity checked, single-link validated, fsynced, and atomically renamed. Exact-head CodeQL is green with no open branch alerts. ### Validation - Local focused matrix: 19 files / 327 tests passed; one true Linux-only `memfd`/`O_TMPFILE` restart case was skipped on macOS. - Exact pinned Deep Agents Code/LangChain patch and runtime validator passed: `progressive-disclosure-runtime-ok`. - Both TypeScript typechecks, build, repository checks, source-shape/test-size guards, Biome, Ruff/Python compilation, secret scanning, push checks, and `git diff --check` passed. - Final-head standard validation is green: [PR CI](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184882), [growth guardrails](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184428), [WSL](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184879), [macOS](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184874), [security](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184917), [CodeQL](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184138), [review advisors](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184863), and the [E2E advisor](https://github.com/NVIDIA/NemoClaw/actions/runs/28700184880). The original exact-head check set and subsequent metadata checks are all green, with only the two expected skips. - The exact-head [typed OpenClaw and Deep Agents targets](https://github.com/NVIDIA/NemoClaw/actions/runs/28700290435) passed. - The exact-head [production-image workflow](https://github.com/NVIDIA/NemoClaw/actions/runs/28700292369) passed both requested image builds and all four downstream image E2E checks. - In the exact-head [advisor-selected live union](https://github.com/NVIDIA/NemoClaw/actions/runs/28700286465), 18 of 19 concrete jobs passed. The sole Hermes security-posture failure occurred in its deliberate gateway-recovery probe after restart, secret-boundary, and disclosure assertions had passed. The isolated exact-head [security retry](https://github.com/NVIDIA/NemoClaw/actions/runs/28700509914) passed both Hermes and OpenClaw legs, so every selected live lane has passing final-head evidence. - The GPT review advisor's remaining OpenClaw MCP model-loop warning is explicitly dispositioned by the managed-MCP ownership boundary above: the native OpenClaw catalog and mcporter registry are separate shipped stores, and a test-only projection would claim behavior the production bridge does not provide. ### Remaining - Obtain Apurv's exact-head re-review to clear the formal `CHANGES_REQUESTED` state and complete sensitive-path approval. - Rebuild affected existing sandboxes after this change ships; newly created sandboxes default to progressive disclosure. ## 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) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Apurv exact-head re-review pending after all four requested fixes. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: no waiver requested; exact-head security retry is green. ## Verification - [x] PR description includes the DCO sign-off declaration and every feature commit appears as `Verified` in GitHub - [ ] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes — all applicable hooks passed; the known hanging local unsharded `test-cli` coverage hook was skipped and exact-head sharded Linux CI is authoritative. - [x] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [ ] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only; current build has two pre-existing warnings) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [x] New doc pages include SPDX header and frontmatter (new pages only; no new doc pages) --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.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
4 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.
Adds improved diagram and style guide updates to the security best practices docs.
This PR targets the
docs/add-best-practicesbranch to add commits to PR #1203.