fix(openclaw): contextualize managed audit findings#6593
Conversation
Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds remote dashboard-bind preparation tracking, fail-closed forwarding and reuse checks, managed OpenClaw audit suppressions, lifecycle and real-audit tests, CI updates, and revised security and dashboard-binding documentation. ChangesDashboard exposure and lifecycle
Managed security audit behavior
CI and tooling
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Onboarding
participant DockerfileContract
participant SandboxRegistry
participant ForwardRecovery
participant OpenClawAudit
Onboarding->>DockerfileContract: prepare remote dashboard bind
DockerfileContract->>Onboarding: return dashboardRemoteBindPrepared
Onboarding->>SandboxRegistry: persist preparation state
ForwardRecovery->>SandboxRegistry: verify preparation before forwarding
ForwardRecovery->>ForwardRecovery: select loopback or 0.0.0.0 target
Onboarding->>OpenClawAudit: generate config with audit suppressions
OpenClawAudit->>OpenClawAudit: report suppressed and active findings
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the branch. TypeScript / code-coverage/cliThe overall coverage in the branch remains at 79%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Updated |
|
🌿 Preview your docs: https://nvidia-preview-pr-6593.docs.buildwithfern.com/nemoclaw |
E2E Advisor RecommendationRequired E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
|
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
PR Review Advisor (Nemotron Ultra) — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision. |
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings 1 warning · 0 optional suggestionsWarningsThese merit maintainer attention but do not block by themselves.
|
There was a problem hiding this comment.
🧹 Nitpick comments (3)
test/generate-openclaw-config-security-audit.test.ts (1)
20-65: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a "fully secure" case to complete the suppression matrix.
The suite covers loopback-HTTP, non-loopback-HTTPS, and non-loopback-HTTP, but there's no test asserting the case with neither condition true (e.g. loopback HTTPS) — where
config.securityshould be entirely absent per thesecurityAuditSuppressions.length > 0guard inbuildConfig. This is the scenario that directly backs the PR's "unrelated findings are not suppressed" claim and guards against a regression where suppressions leak into secure configs.✅ Suggested additional test
+ it("emits no suppressions for a fully secure loopback dashboard (`#6024`)", () => { + const config = buildSecurityAuditConfig("https://127.0.0.1:18789"); + expect(config.security).toBeUndefined(); + });🤖 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 `@test/generate-openclaw-config-security-audit.test.ts` around lines 20 - 65, Add a “fully secure” test case in generate-openclaw-config-security-audit.test.ts using buildSecurityAuditConfig with a loopback HTTPS URL so neither managed auth suppression applies; assert that config.security is entirely absent, since buildConfig only adds security when securityAuditSuppressions.length > 0. Keep the new test alongside the existing managed security audit cases and verify the suppression matrix now covers loopback-HTTP, non-loopback-HTTPS, non-loopback-HTTP, and secure loopback-HTTPS.docs/security/best-practices.mdx (1)
686-699: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider a
<Warning>callout for the safety clarification.Lines 689-690 make an important point — suppressions don't make the flags safe or change enforcement — that's easy to skim past as plain prose. The file already uses
<Warning>for a comparable risk-scoping caveat later (Auto-Pair section, lines 739-742); wrapping this clarification the same way would make it consistent and harder to miss.As per coding guidelines,
docs/**/*.mdxshould "Use Fern callout components such as<Note>,<Tip>, and<Warning>for callouts in MDX pages."📝 Suggested callout wrap
-These suppressions change audit reporting only. -They do not make either flag safe, weaken enforcement, or suppress unrelated findings. +<Warning> +These suppressions change audit reporting only. +They do not make either flag safe, weaken enforcement, or suppress unrelated findings. +</Warning>🤖 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 `@docs/security/best-practices.mdx` around lines 686 - 699, Wrap the safety clarification in the OpenClaw audit section with a <Warning> callout so the statement that suppressions do not make allowInsecureAuth or dangerouslyDisableDeviceAuth safe and do not weaken enforcement stands out. Update the prose in the best-practices MDX content where the accepted findings and suppression behavior are described, matching the existing callout style used elsewhere in the document. Keep the audit details, JSON example, and risk-removal guidance inside the warning block.Source: Coding guidelines
scripts/generate-openclaw-config.mts (1)
1195-1219: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSuppression schema and checkIds verified against OpenClaw's audit implementation.
Confirmed via OpenClaw docs/source references that
security.audit.suppressionssupports exactcheckId+detailIncludes+reason, thatconfig.insecure_or_dangerous_flagsis emitted one-per-flag (justifying thedetailIncludesdisambiguation used here), and thatgateway.control_ui.insecure_authis a real checkId. No functional defect found in this segment.Minor DRY opportunity: the two blocks are structurally identical (push a specific-check entry + a generic
config.insecure_or_dangerous_flagsentry sharing the samereason). Extracting a small helper would reduce duplication and make it easier to add more managed flags later.♻️ Optional refactor to reduce duplication
+ const pushManagedFlagSuppression = ( + checkId: string, + detailIncludes: string, + reason: string, + ): void => { + securityAuditSuppressions.push( + { checkId, reason }, + { checkId: "config.insecure_or_dangerous_flags", detailIncludes, reason }, + ); + }; + if (allowInsecure) { - const reason = - "NemoClaw derives this setting from an HTTP CHAT_UI_URL; use HTTPS for non-loopback dashboards."; - securityAuditSuppressions.push( - { checkId: "gateway.control_ui.insecure_auth", reason }, - { - checkId: "config.insecure_or_dangerous_flags", - detailIncludes: "gateway.controlUi.allowInsecureAuth=true", - reason, - }, - ); + pushManagedFlagSuppression( + "gateway.control_ui.insecure_auth", + "gateway.controlUi.allowInsecureAuth=true", + "NemoClaw derives this setting from an HTTP CHAT_UI_URL; use HTTPS for non-loopback dashboards.", + ); } if (disableDeviceAuth) { - const reason = - "NemoClaw enables this compatibility setting for non-loopback or explicitly opted-out dashboards; use loopback access to retain device authentication."; - securityAuditSuppressions.push( - { checkId: "gateway.control_ui.device_auth_disabled", reason }, - { - checkId: "config.insecure_or_dangerous_flags", - detailIncludes: "gateway.controlUi.dangerouslyDisableDeviceAuth=true", - reason, - }, - ); + pushManagedFlagSuppression( + "gateway.control_ui.device_auth_disabled", + "gateway.controlUi.dangerouslyDisableDeviceAuth=true", + "NemoClaw enables this compatibility setting for non-loopback or explicitly opted-out dashboards; use loopback access to retain device authentication.", + ); }🤖 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 `@scripts/generate-openclaw-config.mts` around lines 1195 - 1219, Refactor the duplicated suppression-building logic in generate-openclaw-config.mts by extracting a small helper around the securityAuditSuppressions.push pattern used in the allowInsecure and disableDeviceAuth branches. Keep the existing checkId, detailIncludes, and reason values unchanged, but centralize the creation of the specific check entry plus the shared config.insecure_or_dangerous_flags entry so adding future managed flags is simpler and less repetitive.
🤖 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.
Nitpick comments:
In `@docs/security/best-practices.mdx`:
- Around line 686-699: Wrap the safety clarification in the OpenClaw audit
section with a <Warning> callout so the statement that suppressions do not make
allowInsecureAuth or dangerouslyDisableDeviceAuth safe and do not weaken
enforcement stands out. Update the prose in the best-practices MDX content where
the accepted findings and suppression behavior are described, matching the
existing callout style used elsewhere in the document. Keep the audit details,
JSON example, and risk-removal guidance inside the warning block.
In `@scripts/generate-openclaw-config.mts`:
- Around line 1195-1219: Refactor the duplicated suppression-building logic in
generate-openclaw-config.mts by extracting a small helper around the
securityAuditSuppressions.push pattern used in the allowInsecure and
disableDeviceAuth branches. Keep the existing checkId, detailIncludes, and
reason values unchanged, but centralize the creation of the specific check entry
plus the shared config.insecure_or_dangerous_flags entry so adding future
managed flags is simpler and less repetitive.
In `@test/generate-openclaw-config-security-audit.test.ts`:
- Around line 20-65: Add a “fully secure” test case in
generate-openclaw-config-security-audit.test.ts using buildSecurityAuditConfig
with a loopback HTTPS URL so neither managed auth suppression applies; assert
that config.security is entirely absent, since buildConfig only adds security
when securityAuditSuppressions.length > 0. Keep the new test alongside the
existing managed security audit cases and verify the suppression matrix now
covers loopback-HTTP, non-loopback-HTTPS, non-loopback-HTTP, and secure
loopback-HTTPS.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 84912228-b75f-4674-8e4b-47fdd7b9deb9
📒 Files selected for processing (3)
docs/security/best-practices.mdxscripts/generate-openclaw-config.mtstest/generate-openclaw-config-security-audit.test.ts
Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
|
Exact-head maintainer follow-up for I exercised the PR's generated configuration through the exact pinned OpenClaw audit consumer, rather than stopping at JSON-shape assertions:
Observed matrix:
This directly exercises and resolves the runtime-consumer concern in |
Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
E2E Target Results — ✅ All selected jobs passedRun: 29045058101
|
Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
E2E Target Results —
|
| Job | Result |
|---|---|
| full-e2e | ✅ success |
| security-posture |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/openclaw-security-audit-suppressions-real.test.ts (1)
101-140: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest doesn't cover the remote-HTTPS case or the "unrelated flag stays active" contract.
The PR objective and maintainer validation describe three managed scenarios (loopback HTTP, remote HTTPS, remote HTTP), and specifically call out adding an unrelated dangerous flag (
tools.exec.applyPatch.workspaceOnly=false) to every case to prove shared findings aren't over-suppressed. This automated harness only exercises loopback HTTP and remote HTTP — the remote-HTTPS scenario (where onlydevice_auth_disabledshould be suppressed, withinsecure_authnever triggered) isn't asserted here, and the unrelated-finding check only runs for the loopback case (gateway.loopback_no_auth), not for the remote case, and doesn't use the exact unrelated flag the maintainer validated manually.Since this is the "real audit" contract test meant to lock in runtime-consumer behavior (per the linked comments summary), codifying the maintainer's manual validation here would prevent regressions that manual spot-checks won't catch in CI.
🤖 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 `@test/openclaw-security-audit-suppressions-real.test.ts` around lines 101 - 140, Add coverage in the audit contract test around runOpenClawAudit so it exercises the missing remote HTTPS scenario and asserts the exact suppression behavior there: only gateway.control_ui.device_auth_disabled should be suppressed while gateway.control_ui.insecure_auth never appears. Also extend the existing loopback/remote cases to include the unrelated dangerous flag tools.exec.applyPatch.workspaceOnly=false and assert it remains active (not suppressed) in every case, using the existing helpers like findingForFlag and the suppressedFindings/findings arrays to locate the checks.
🤖 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.
Nitpick comments:
In `@test/openclaw-security-audit-suppressions-real.test.ts`:
- Around line 101-140: Add coverage in the audit contract test around
runOpenClawAudit so it exercises the missing remote HTTPS scenario and asserts
the exact suppression behavior there: only
gateway.control_ui.device_auth_disabled should be suppressed while
gateway.control_ui.insecure_auth never appears. Also extend the existing
loopback/remote cases to include the unrelated dangerous flag
tools.exec.applyPatch.workspaceOnly=false and assert it remains active (not
suppressed) in every case, using the existing helpers like findingForFlag and
the suppressedFindings/findings arrays to locate the checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1694325e-0e94-43f2-b896-ece5f758272d
📒 Files selected for processing (1)
test/openclaw-security-audit-suppressions-real.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/openclaw-security-audit-suppressions-real.test.ts (1)
44-56: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winTemp dir setup happens before the
try, so cleanup is skipped on setup failure.
mkdirSync/writeFileSync(lines 49-54) run before thetryblock that owns thefinallycleanup (line 87-89). If either call throws,tmpis never removed, leaking the temp directory.🧹 Proposed fix: move setup inside the try/finally
function runOpenClawAudit(chatUiUrl: string): AuditResult { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-openclaw-audit-")); - const home = path.join(tmp, "home"); - const configDir = path.join(home, ".openclaw"); - const cache = process.env.NEMOCLAW_REAL_OPENCLAW_AUDIT_NPM_CACHE || path.join(tmp, "npm-cache"); - fs.mkdirSync(configDir, { recursive: true, mode: 0o700 }); - fs.writeFileSync( - path.join(configDir, "openclaw.json"), - JSON.stringify(buildConfig({ ...BASE_ENV, CHAT_UI_URL: chatUiUrl })), - { mode: 0o600 }, - ); - try { + const home = path.join(tmp, "home"); + const configDir = path.join(home, ".openclaw"); + const cache = process.env.NEMOCLAW_REAL_OPENCLAW_AUDIT_NPM_CACHE || path.join(tmp, "npm-cache"); + fs.mkdirSync(configDir, { recursive: true, mode: 0o700 }); + fs.writeFileSync( + path.join(configDir, "openclaw.json"), + JSON.stringify(buildConfig({ ...BASE_ENV, CHAT_UI_URL: chatUiUrl })), + { mode: 0o600 }, + ); const version = reviewedOpenClawVersion();🤖 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 `@test/openclaw-security-audit-suppressions-real.test.ts` around lines 44 - 56, The temp directory setup in runOpenClawAudit happens before the try/finally, so failures in fs.mkdirSync or fs.writeFileSync can skip cleanup and leak tmp. Move the temp home/config/cache setup inside the try block that owns the finally, keeping the cleanup in place so any setup error still removes the temporary directory.
🤖 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 `@test/openclaw-security-audit-suppressions-real.test.ts`:
- Around line 58-62: The `childEnv` setup in the test is too permissive because
it passes nearly all of `process.env` into `npm exec` for the
`openclaw@${version}` package. Replace the current blocklist filter in the
environment-building logic with a narrow allowlist in this test, keeping only
the variables that `npm exec` actually needs (for example `HOME`, `PATH`, and
`NPM_CONFIG_CACHE`, plus any other explicitly required values). Use the existing
`childEnv` construction near the `spawnSync` call as the place to tighten the
env before launching the package.
---
Outside diff comments:
In `@test/openclaw-security-audit-suppressions-real.test.ts`:
- Around line 44-56: The temp directory setup in runOpenClawAudit happens before
the try/finally, so failures in fs.mkdirSync or fs.writeFileSync can skip
cleanup and leak tmp. Move the temp home/config/cache setup inside the try block
that owns the finally, keeping the cleanup in place so any setup error still
removes the temporary directory.
🪄 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: 66c58f63-206c-461b-85f2-9d93103e2507
📒 Files selected for processing (1)
test/openclaw-security-audit-suppressions-real.test.ts
cv
left a comment
There was a problem hiding this comment.
Security review for exact head 0986ad06b0cbe7d0089fda4a54e509bc206e8deb found four items to address before the required live runs:
- Fail closed on credentials in the real OpenClaw audit harness. Do not spread nearly all of
process.envinto the npm/OpenClaw child; pass an explicit allowlist containing only the variables the harness needs. This is the major CodeRabbit finding in #6593 (comment). - Put temporary-directory cleanup around the whole setup path so a download/install/setup exception cannot leak the directory.
- Wire
NEMOCLAW_REAL_OPENCLAW_AUDIT_HARNESS=1into a trusted CI lane. The new consumer contract is currently skipped in every workflow, so it is not regression protection. - Do not automatically move the remote
dangerouslyDisableDeviceAuth=truecondition into accepted findings for every non-loopback dashboard. Require a distinct, explicit risk-acceptance control, or keep that high-risk unauthenticated remote-dashboard finding active.
After those source/security items are addressed, rerun the exact-head advisors and the E2E-advisor-required cloud-onboard and security-posture targets. The older manual consumer proof and cloud run were at f48b03a, so they are useful diagnostics but not exact-head gate evidence.
Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
cjagwani
left a comment
There was a problem hiding this comment.
Security Review
Security review of exact head c0f02081cea091520784383313cb445a5d99aa28 still finds changes required.
-
High: the normal onboard path still suppresses the critical remote device-auth findings.
scripts/generate-openclaw-config.mts:1192-1219now suppresses whenNEMOCLAW_DISABLE_DEVICE_AUTH=1, regardless ofisRemote. But standard onboarding unconditionally rewrites that build arg to1insrc/lib/onboard/dockerfile-patch.ts:305-310, including for remoteCHAT_UI_URLvalues. The new remote tests omit that production override, so they do not exercise the actual onboard state, anddocs/security/best-practices.mdx:686-687is not yet true. Gate device-auth suppressions to a loopback-only explicit opt-out (for example,explicitDeviceAuthOptOut && !isRemote) or introduce a separate audit-risk-acceptance control. Add remote HTTP and HTTPS cases withNEMOCLAW_DISABLE_DEVICE_AUTH=1, ideally through the Dockerfile patch path. -
High: the real-audit harness still forwards almost all ambient environment variables to registry-fetched OpenClaw code at
test/openclaw-security-audit-suppressions-real.test.ts:58-77. Replace this with a minimal literal allowlist before enabling the harness in CI. -
The cleanup and regression gates remain open. Setup at
:45-54still occurs before the cleanup-owningtry/finally, and the suite at:99-157is still skip-gated with no workflow settingNEMOCLAW_REAL_OPENCLAW_AUDIT_HARNESS=1. Wire it into a trusted lane after making the child environment safe. Prefer the committed OpenClaw tarball/SRI boundary over unconstrained registry retrieval so the contract test executes the artifact NemoClaw actually reviewed.
I verified the pinned OpenClaw suppression contract itself: the check IDs and exact flag details are correct; suppression preserves severity and remediation; and unrelated findings remain active. The bounded accepted-risk scopes are loopback HTTP and a loopback-only explicit device-auth opt-out. Please address the source/harness items before dispatching fresh live E2Es, then run the requested exact-head cloud-onboard, security-posture, and current advisor targets and request re-review.
Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
Cover the production remote HTTPS override with the reviewed OpenClaw artifact. Size the trusted-main timeout for package setup and all five audit scenarios. Co-authored-by: Chengjie Wang <chengjiew@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
E2E Target Results —
|
| Job | Result |
|---|---|
| cloud-onboard | |
| device-auth-health | |
| security-posture |
E2E Target Results —
|
| Job | Result |
|---|---|
| agent-turn-latency | |
| bedrock-runtime-compatible-anthropic | |
| brave-search | |
| channels-add-remove | |
| channels-stop-start | |
| cloud-inference | |
| cloud-onboard | |
| common-egress-agent | |
| concurrent-gateway-ports | |
| credential-migration | |
| credential-sanitization | |
| cron-preflight-inference-local | |
| device-auth-health | |
| diagnostics | |
| docs-validation | |
| double-onboard | |
| full-e2e | |
| gateway-drift-preflight | |
| gateway-guard-recovery | |
| gateway-health-honest | |
| gpu-double-onboard | |
| gpu-e2e | |
| hermes-dashboard | |
| hermes-discord | |
| hermes-e2e | |
| hermes-gpu-startup | |
| hermes-inference-switch | |
| hermes-shields-config | |
| hermes-slack | |
| inference-routing | |
| issue-2478-crash-loop-recovery | |
| issue-4434-tui-unreachable-inference | |
| issue-4462-scope-upgrade-approval | |
| jetson-nvmap-gpu | |
| kimi-inference-compat | |
| launchable-smoke | |
| live | |
| mcp-bridge | |
| mcp-bridge-dev | |
| messaging-compatible-endpoint | |
| messaging-providers | |
| model-router-provider-routed-inference | |
| network-policy | |
| ollama-auth-proxy | |
| onboard-negative-paths | |
| onboard-repair | |
| onboard-resume | |
| openclaw-discord-pairing | |
| openclaw-inference-switch | |
| openclaw-plugin-runtime-exdev | |
| openclaw-skill-cli | |
| openclaw-slack-pairing | |
| openclaw-tui-chat-correlation | |
| openshell-gateway-auth-contract | |
| openshell-gateway-upgrade | |
| openshell-version-pin | |
| overlayfs-autofix | |
| rebuild-hermes | |
| rebuild-hermes-stale-base | |
| rebuild-openclaw | |
| sandbox-operations | |
| sandbox-rebuild | |
| sandbox-rlimits-connect | |
| sandbox-survival | |
| security-posture | |
| sessions-agents-cli | |
| shields-config | |
| skill-agent | |
| snapshot-commands | |
| spark-install | |
| state-backup-restore | |
| telegram-injection | |
| token-rotation | |
| tunnel-lifecycle | |
| upgrade-stale-sandbox |
Signed-off-by: Carlos Villela <cvillela@nvidia.com> # Conflicts: # test/e2e/mock-parity.json
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
/nvskills-ci |
E2E Target Results — ❌ Some tests failedRun: 29173077708
|
|
Maintainer adjudication for the current PR Review Advisor warning: the WSL all-interface forward and managed device-auth compatibility opt-out predate this change ( |
E2E Target Results —
|
| Test | Result |
|---|---|
| cloud-onboard | ✅ success |
| onboard-repair | ✅ success |
| onboard-resume | ✅ success |
| state-backup-restore | ✅ success |
| upgrade-stale-sandbox |
|
⚪ Brev E2E (dashboard-remote-bind): CANCELLED on branch |
E2E Target Results — ✅ All requested tests passedRun: 29173330713
|
|
✅ Brev E2E (dashboard-remote-bind): PASSED on branch |
Dismissed after current-commit security re-review: custom Dockerfile remote-bind requests now fail closed before text attestation, all eight demonstrated attestation vectors have production-path regressions, reconnect health enforces the requested bind, ordinary CI and trusted advisors are green, and both controller-correlated remote-dashboard E2E paths passed.
cv
left a comment
There was a problem hiding this comment.
Current-commit re-review passed. The custom-Dockerfile attestation bypasses and remote-forward mismatch are resolved with fail-closed production paths and adversarial regressions. All 53 checks, trusted advisors, five controller-correlated E2E targets, and the dedicated dashboard remote-bind regression are green. The WSL warning describes pre-existing behavior outside this PR; this change preserves that contract while keeping the risky audit findings active.
Resolved on current PR commit 31edc93: the real audit harness now uses a minimal child environment, owns setup cleanup, runs the pinned artifact in trusted CI, and keeps remote findings active. Current CI, advisors, and required live evidence pass.
<!-- markdownlint-disable MD041 --> ## Summary Release-prep documentation for v0.0.81 now summarizes user-facing changes merged since v0.0.80. It also closes the Hermes dashboard-profile backup gap and distinguishes direct blueprint-runner actions from public host CLI commands. ## Changes - Add the `v0.0.81` section to `docs/about/release-notes.mdx` with links to the detailed user guides. - Document that Hermes rebuilds preserve `.hermes/dashboard-home/`, including Dashboard `MEMORY.md` and `USER.md`. - Update Hermes manual backup and restore examples to transfer those two profile files without copying generated configuration or the secret-bearing dashboard `.env`. - Explain the new per-item backup failure causes. - Clarify that migration snapshot retention fragments are direct-runner arguments and are not exposed by the host `nemoclaw` CLI. ### Source summary - #6445 -> `docs/about/release-notes.mdx`, `docs/manage-sandboxes/backup-restore.mdx`, and `docs/manage-sandboxes/workspace-files.mdx`: Summarize manifest-owned key-level restore and current-config authority. - #6617 -> `docs/about/release-notes.mdx` and `docs/manage-sandboxes/backup-restore.mdx`: Record the fail-closed `/proc` fallback used to verify an idle Deep Agents runtime before snapshot creation. - #6685 -> `docs/about/release-notes.mdx`, `docs/manage-sandboxes/backup-restore.mdx`, and `docs/manage-sandboxes/workspace-files.mdx`: Document Hermes Web Dashboard profile persistence and safe manual transfer. - #6649 -> `docs/about/release-notes.mdx`: Summarize host-validated loopback compatible-endpoint routing through the sandbox gateway. - #6643 -> `docs/about/release-notes.mdx`: Summarize automatic `max_completion_tokens` handling for GPT-5 and o-series models. - #6661 -> `docs/about/release-notes.mdx`: Summarize bounded connection reuse for eligible provider-validation probes. - #6704 -> `docs/about/release-notes.mdx`: Record that direct blueprint apply stops instead of persisting incomplete state after provider or inference setup fails. - #6677 -> `docs/about/release-notes.mdx`: Summarize transactional recovery for legacy Docker containers whose managed supervisor disappeared after restart. - #6625 -> `docs/about/release-notes.mdx`: Record Hermes managed-startup persistence across direct Docker restarts. - #6597 -> `docs/about/release-notes.mdx`: Record final-sandbox gateway cleanup on macOS. - #6680 -> `docs/about/release-notes.mdx`: Summarize managed Deep Agents first-run and process-tree cleanup improvements. - #6647 -> `docs/about/release-notes.mdx`: Record fail-closed validation for the managed Deep Agents fetch CA bundle. - #6645 -> `docs/about/release-notes.mdx`: Summarize WhatsApp loopback pairing and trusted npm plugin provenance. - #6673 -> `docs/about/release-notes.mdx` and `docs/manage-sandboxes/backup-restore.mdx`: Document stopped-sandbox backup remediation. - #6631 -> `docs/about/release-notes.mdx` and `docs/manage-sandboxes/backup-restore.mdx`: Document per-item backup failure causes. - #6620 -> `docs/about/release-notes.mdx`: Record the created-but-not-ready sandbox lifecycle receipt. - #6664 -> `docs/about/release-notes.mdx`: Record prompt-aware onboarding progress output. - #6598 -> `docs/about/release-notes.mdx`: Summarize stale replay-result invalidation during resumed onboarding. - #6593 -> `docs/about/release-notes.mdx`: Summarize contextual OpenClaw audit findings for managed dashboard compatibility settings. - #6650 -> `docs/about/release-notes.mdx`: Record redaction of token-shaped URL query values. - #6638 -> `docs/about/release-notes.mdx`: Record the exact-path MCP `DELETE` policy recipe for session termination. - #5453 -> `docs/reference/host-files-and-state.mdx`: Clarify that snapshot retention actions belong to direct runner integrations and are not standalone host CLI commands. ### Skipped from docs-skip - #6633 matched the `openclaw-sandbox-permissive.yaml` path in `docs/.docs-skip` and produced no documentation in this update. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: This is a documentation-only release-prep update; behavior is protected by the merged source PRs, and the documentation build validates the changed examples and routes. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] 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: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — tests are not applicable for this documentation-only change; `npm run docs` completed successfully. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: not run for this documentation-only change. - [x] 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) — 0 errors; two existing Fern warnings remain. - [x] 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) — no new pages. --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Added release notes for v0.0.81 covering state preservation, inference setup, sandbox recovery, session setup, pairing, diagnostics, and security policy updates. - Expanded backup and restore guidance to include dashboard profile files and clarify files that must not be copied. - Added dashboard profile persistence details to workspace and rebuild documentation. - Clarified snapshot retention guidance and the distinction between host CLI capabilities and direct runner actions. - Added more detailed backup failure reporting information. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Carlos Villela <cvillela@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary <!-- 1-3 sentences: what this PR does and why. --> Teach generated OpenClaw configs to explain NemoClaw-managed dashboard compatibility flags through OpenClaw's native security audit suppression contract. The findings remain visible with their original severity, remediation, and an explicit risk-acceptance reason, while unrelated audit findings remain active. ## Related Issue <!-- Fixes #NNN or Closes #NNN. Remove this section if none. --> Fixes NVIDIA#6024 ## Changes <!-- Bullet list of key changes. --> - Generate exact `security.audit.suppressions` entries only when NemoClaw enables `allowInsecureAuth` or `dangerouslyDisableDeviceAuth`. - Match duplicate generic dangerous-flag findings by exact flag detail so other findings are not suppressed. - Add focused config-generation coverage for loopback HTTP, remote HTTPS, and remote HTTP dashboards. - Document the device-auth defaults, accepted risk, safer loopback/HTTPS choices, and how to inspect `suppressedFindings`. ## 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 <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [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: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run --project integration test/generate-openclaw-config.test.ts test/generate-openclaw-config-security-audit.test.ts` passed 131/131 tests - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] 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) — 0 errors; two unrelated environment/theme warnings remain for missing Fern authentication and existing light-mode contrast - [x] 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) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified. Run: git config user.name && git config user.email --> Signed-off-by: Chengjie Wang <chengjiew@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated security guidance so device authentication and related dashboard pairing are enabled for loopback-only setups, with clearer warnings for remote/public exposure. * Clarified the `NEMOCLAW_DASHBOARD_BIND=0.0.0.0` opt-in workflow, including required sandbox recreation and connect-time behavior. * **Bug Fixes** * Tightened remote dashboard bind handling: local-only sandboxes can’t be used for remote exposure without the proper opt-in preparation. * Refined managed security-audit suppression and reporting to align with loopback vs remote exposure. * **Tests** * Added/expanded unit, real-audit, and end-to-end coverage for the new suppression and remote-bind lifecycle enforcement. * **Chores** * CI reliability improvements (tooling availability and longer real-audit timeout). <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Chengjie Wang <chengjiew@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: cjagwani <cjagwani@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Release-prep documentation for v0.0.81 now summarizes user-facing changes merged since v0.0.80. It also closes the Hermes dashboard-profile backup gap and distinguishes direct blueprint-runner actions from public host CLI commands. ## Changes - Add the `v0.0.81` section to `docs/about/release-notes.mdx` with links to the detailed user guides. - Document that Hermes rebuilds preserve `.hermes/dashboard-home/`, including Dashboard `MEMORY.md` and `USER.md`. - Update Hermes manual backup and restore examples to transfer those two profile files without copying generated configuration or the secret-bearing dashboard `.env`. - Explain the new per-item backup failure causes. - Clarify that migration snapshot retention fragments are direct-runner arguments and are not exposed by the host `nemoclaw` CLI. ### Source summary - NVIDIA#6445 -> `docs/about/release-notes.mdx`, `docs/manage-sandboxes/backup-restore.mdx`, and `docs/manage-sandboxes/workspace-files.mdx`: Summarize manifest-owned key-level restore and current-config authority. - NVIDIA#6617 -> `docs/about/release-notes.mdx` and `docs/manage-sandboxes/backup-restore.mdx`: Record the fail-closed `/proc` fallback used to verify an idle Deep Agents runtime before snapshot creation. - NVIDIA#6685 -> `docs/about/release-notes.mdx`, `docs/manage-sandboxes/backup-restore.mdx`, and `docs/manage-sandboxes/workspace-files.mdx`: Document Hermes Web Dashboard profile persistence and safe manual transfer. - NVIDIA#6649 -> `docs/about/release-notes.mdx`: Summarize host-validated loopback compatible-endpoint routing through the sandbox gateway. - NVIDIA#6643 -> `docs/about/release-notes.mdx`: Summarize automatic `max_completion_tokens` handling for GPT-5 and o-series models. - NVIDIA#6661 -> `docs/about/release-notes.mdx`: Summarize bounded connection reuse for eligible provider-validation probes. - NVIDIA#6704 -> `docs/about/release-notes.mdx`: Record that direct blueprint apply stops instead of persisting incomplete state after provider or inference setup fails. - NVIDIA#6677 -> `docs/about/release-notes.mdx`: Summarize transactional recovery for legacy Docker containers whose managed supervisor disappeared after restart. - NVIDIA#6625 -> `docs/about/release-notes.mdx`: Record Hermes managed-startup persistence across direct Docker restarts. - NVIDIA#6597 -> `docs/about/release-notes.mdx`: Record final-sandbox gateway cleanup on macOS. - NVIDIA#6680 -> `docs/about/release-notes.mdx`: Summarize managed Deep Agents first-run and process-tree cleanup improvements. - NVIDIA#6647 -> `docs/about/release-notes.mdx`: Record fail-closed validation for the managed Deep Agents fetch CA bundle. - NVIDIA#6645 -> `docs/about/release-notes.mdx`: Summarize WhatsApp loopback pairing and trusted npm plugin provenance. - NVIDIA#6673 -> `docs/about/release-notes.mdx` and `docs/manage-sandboxes/backup-restore.mdx`: Document stopped-sandbox backup remediation. - NVIDIA#6631 -> `docs/about/release-notes.mdx` and `docs/manage-sandboxes/backup-restore.mdx`: Document per-item backup failure causes. - NVIDIA#6620 -> `docs/about/release-notes.mdx`: Record the created-but-not-ready sandbox lifecycle receipt. - NVIDIA#6664 -> `docs/about/release-notes.mdx`: Record prompt-aware onboarding progress output. - NVIDIA#6598 -> `docs/about/release-notes.mdx`: Summarize stale replay-result invalidation during resumed onboarding. - NVIDIA#6593 -> `docs/about/release-notes.mdx`: Summarize contextual OpenClaw audit findings for managed dashboard compatibility settings. - NVIDIA#6650 -> `docs/about/release-notes.mdx`: Record redaction of token-shaped URL query values. - NVIDIA#6638 -> `docs/about/release-notes.mdx`: Record the exact-path MCP `DELETE` policy recipe for session termination. - NVIDIA#5453 -> `docs/reference/host-files-and-state.mdx`: Clarify that snapshot retention actions belong to direct runner integrations and are not standalone host CLI commands. ### Skipped from docs-skip - NVIDIA#6633 matched the `openclaw-sandbox-permissive.yaml` path in `docs/.docs-skip` and produced no documentation in this update. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: This is a documentation-only release-prep update; behavior is protected by the merged source PRs, and the documentation build validates the changed examples and routes. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] 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: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — tests are not applicable for this documentation-only change; `npm run docs` completed successfully. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: not run for this documentation-only change. - [x] 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) — 0 errors; two existing Fern warnings remain. - [x] 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) — no new pages. --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Added release notes for v0.0.81 covering state preservation, inference setup, sandbox recovery, session setup, pairing, diagnostics, and security policy updates. - Expanded backup and restore guidance to include dashboard profile files and clarify files that must not be copied. - Added dashboard profile persistence details to workspace and rebuild documentation. - Clarified snapshot retention guidance and the distinction between host CLI capabilities and direct runner actions. - Added more detailed backup failure reporting information. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Summary
Teach generated OpenClaw configs to explain NemoClaw-managed dashboard compatibility flags through OpenClaw's native security audit suppression contract. The findings remain visible with their original severity, remediation, and an explicit risk-acceptance reason, while unrelated audit findings remain active.
Related Issue
Fixes #6024
Changes
security.audit.suppressionsentries only when NemoClaw enablesallowInsecureAuthordangerouslyDisableDeviceAuth.suppressedFindings.Type of Change
Quality Gates
Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run --project integration test/generate-openclaw-config.test.ts test/generate-openclaw-config-security-audit.test.tspassed 131/131 testsnpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only) — 0 errors; two unrelated environment/theme warnings remain for missing Fern authentication and existing light-mode contrastSigned-off-by: Chengjie Wang chengjiew@nvidia.com
Summary by CodeRabbit
NEMOCLAW_DASHBOARD_BIND=0.0.0.0opt-in workflow, including required sandbox recreation and connect-time behavior.