fix(security): redact token-shaped values from sandbox doctor --json output - #7356
Conversation
…output `sandbox doctor --json` lacked the return-boundary redaction that `sandbox status --json` has had since #4310: the oclif path resolved with the raw report, so programmatic consumers of the resolved value saw unredacted check details (only the logJson print boundary, #3657, redacted the CLI stdout), and the lib-level renderDoctorReport JSON branch printed via console.log with no redaction at all. Route both egress points through the centralized redactForLog source of truth, computed after exit codes are derived from the raw report. Refs #4310 Signed-off-by: Dongni-Yang <dongniy@nvidia.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughSandbox doctor JSON reports are now passed through centralized redaction before logging or returning them. Tests verify token-shaped values are replaced, hints remain unchanged, and failure exit codes are preserved. ChangesSandbox doctor redaction
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 92aa402 in the TypeScript / code-coverage/cliThe overall coverage in commit 92aa402 in the Show a code coverage summary of the most impacted files.
Updated |
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/actions/sandbox/doctor-report.test.ts`:
- Around line 37-56: Update the “redacts token-shaped values from the
machine-readable report” test to wrap its assertions and report rendering in a
try/finally block, calling logSpy.mockRestore() in the finally clause so the
console.log spy is always restored.
🪄 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: 56867ffb-9368-4380-b1ba-58948ae3f0a7
📒 Files selected for processing (4)
src/commands/sandbox/doctor.tssrc/commands/sandbox/oclif-command-adapters.test.tssrc/lib/actions/sandbox/doctor-report.test.tssrc/lib/actions/sandbox/doctor-report.ts
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
cjagwani
left a comment
There was a problem hiding this comment.
Reviewed exact head 92aa402. The token redaction fix is covered, the only CodeRabbit thread is resolved/withdrawn, both review-advisor lanes report no actionable findings, and all required CI plus selected E2E passed. Approving; not merging.
<!-- markdownlint-disable MD041 --> ## Summary This PR adds the canonical dated release entry for NemoClaw v0.0.94 before the tag is cut. The entry reconciles all 26 commits since v0.0.93 and links each user-visible change to its owning documentation. ## Changes - Add `docs/changelog/2026-07-24.mdx` with the exact `## v0.0.94` heading, parser-safe SPDX comment, release summary, and detailed bullets. - Record sandbox restore and update behavior, onboarding and inference changes, network policy behavior, security evidence, Hermes build performance, DGX Station guidance, and E2E validation changes. - Preserve `docs/` as the source of truth without changing the AI-agent documentation routing skill. - Use [E2E run 30075443016](https://github.com/NVIDIA/NemoClaw/actions/runs/30075443016) for release QA evidence at exact tested SHA `04e6dfa883071dda9df429c66e73168e1a995cba`. ### Source summary - [#7461](#7461) -> `docs/changelog/2026-07-24.mdx`: Record the ownership-preserving Hermes image layer reduction and hosted timing comparison. - [#7460](#7460) -> `docs/changelog/2026-07-24.mdx`: Record removal of candidate Hermes swap setup from E2E validation. - [#7458](#7458) -> `docs/security/fern-5.80.1-dependency-review.md`, `docs/changelog/2026-07-24.mdx`: Record the reviewed Fern CLI update. - [#7457](#7457) -> `docs/changelog/2026-07-24.mdx`: Record periodic runner-pressure telemetry. - [#7455](#7455) -> `docs/changelog/2026-07-24.mdx`: Record non-blocking absent Fern previews. - [#7450](#7450) -> `docs/changelog/2026-07-24.mdx`: Record stable cancellation handling for live-test child processes. - [#7449](#7449) -> `docs/changelog/2026-07-24.mdx`: Record parallel plugin EXDEV coverage. - [#7448](#7448) -> `docs/changelog/2026-07-24.mdx`: Record isolated long-running E2E lanes. - [#7444](#7444) -> `docs/changelog/2026-07-24.mdx`: Record exact-head Hermes swap validation. - [#7437](#7437) -> `docs/manage-sandboxes/backup-restore.mdx`, `docs/changelog/2026-07-24.mdx`: Record gateway pairing and authenticated verification after cross-sandbox restore. - [#7436](#7436) -> `docs/manage-sandboxes/backup-restore.mdx`, `docs/reference/commands.mdx`, `docs/changelog/2026-07-24.mdx`: Record selected stale-state cleanup and Hermes virtual-environment access repair. - [#7385](#7385) -> `docs/network-policy/customize-network-policy.mdx`, `docs/changelog/2026-07-24.mdx`: Record the read-only agent-variant route check. - [#7371](#7371) -> `docs/changelog/2026-07-24.mdx`: Record host-artifact verification for session exports. - [#7359](#7359) -> `docs/changelog/2026-07-24.mdx`: Record platform validation for managed vLLM model overrides. - [#7356](#7356) -> `docs/changelog/2026-07-24.mdx`: Record token-shaped value redaction for `sandbox doctor --json`. - [#7354](#7354) -> `docs/security/advisory-early-warning.md`, `docs/changelog/2026-07-24.mdx`: Record advisory correlation and retained audit provenance. - [#7352](#7352) -> `docs/network-policy/customize-network-policy.mdx`, `docs/network-policy/integration-policy-examples.mdx`, `docs/reference/commands.mdx`, `docs/changelog/2026-07-24.mdx`: Record preset reapplication and bounded `tls: skip` guidance. - [#7345](#7345) -> `docs/security/openclaw-2026.6.10-dependency-review.md`, `docs/security/openclaw-2026.7.1-dependency-review.md`, `docs/changelog/2026-07-24.mdx`: Record reviewed npm audit exception enforcement. - [#7340](#7340) -> `docs/network-policy/customize-network-policy.mdx`, `docs/changelog/2026-07-24.mdx`: Record the repaired CLI-reference route. - [#7334](#7334) -> `docs/get-started/dgx-station-preparation.mdx`, `docs/changelog/2026-07-24.mdx`: Record the qualified OTA metadata fallback and narrowed override wording. - [#7322](#7322) -> `docs/changelog/2026-07-24.mdx`: Reconcile the gateway source tag added to plugin registration banners. - [#7284](#7284) -> `docs/manage-sandboxes/update-sandboxes.mdx`, `docs/changelog/2026-07-24.mdx`: Record read-only `upgrade-sandboxes --check` behavior and recorded-gateway selection. - [#7277](#7277) -> `docs/changelog/2026-07-24.mdx`: Reconcile deterministic gateway TCP refusal coverage. - [#7234](#7234) -> `docs/reference/troubleshooting.mdx`, `docs/changelog/2026-07-24.mdx`: Record preserved DGX Spark managed vLLM Express intent on resume. - [#7185](#7185) -> `docs/reference/troubleshooting.mdx`, `docs/changelog/2026-07-24.mdx`: Record IPv4 fallback DNS selection and exact resolver probing. - [#6820](#6820) -> `docs/reference/commands.mdx`, `docs/changelog/2026-07-24.mdx`: Record the versioned, redacted `--events=jsonl` onboarding stream. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `npx vitest run test/changelog-docs.test.ts` passed 6/6 tests. - [ ] Tests not applicable — justification: - [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: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/changelog/2026-07-24.mdx`; the writing rules, documentation style, exact release range, skip terms, published routes, and product scope were reviewed; the changelog test passed 6/6; `npm run docs` passed with route checking OK, zero errors, and two existing warnings. - Agent: Codex Desktop <!-- docs-review-head-sha: 65368f9 --> <!-- docs-review-agents-blob-sha: 9c9b36d --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable - Station profile/scenario: Not applicable - Result: Not applicable - Supporting evidence: Not applicable. This PR does not change `scripts/prepare-dgx-station-host.sh`. ## Verification - [x] PR description includes a `Signed-off-by:` line 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 test/changelog-docs.test.ts` passed 6/6 tests. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not applicable to the dated changelog entry. - [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). The build passed with zero errors and two existing Fern warnings. - [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). Native dated changelog entries use the required parser-safe MDX SPDX comment and no frontmatter. --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added the v0.0.94 release changelog. * Documented improvements to sandbox snapshot and restore behavior. * Added updates for gateway selection, policy comparisons, onboarding event output, and DGX recovery workflows. * Documented enhanced diagnostics redaction, npm audit provenance, image assembly performance, and validation stability improvements. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Summary
sandbox doctor --jsonresolved with the raw report, whilesandbox status --jsonhas redacted its machine-readable report since #4310. Both doctor JSON egress points now pass through the centralizedredactForLog, so token-shaped values in check details no longer reach JSON consumers. Exit codes are still computed from the raw report before redaction.Related Issue
Refs #4310 (the
status --jsonredaction precedent this change mirrors). The asymmetry was found while verifying the redaction contract for the serving-process health design discussion on #7003 (refs only; this PR does not close either issue).Changes
src/lib/actions/sandbox/doctor-report.ts: therenderDoctorReportJSON branch prints viaconsole.log, which bypasses the ocliflogJsonredaction boundary (fix(security): harden low-risk code scanning findings #3657); the report is now routed throughredactForLogbefore printing.src/commands/sandbox/doctor.ts: the oclif--jsonpath now returnsredactForLog(report), so programmatic consumers of the resolved value — not just thelogJson-printed stdout — never see raw token-shaped detail.process.exitCodeis derived from the raw report first.runSandboxDoctor(sorenderDoctorReportnever runs on that path), and the render test callsrenderDoctorReportdirectly (the oclif path bypasses it viaquietJson). Both were red before the fix.Review notes:
status --json):redactFull's word-context patterns can consume a short word following a credential-shaped env-var name, e.g.…_API_KEY for NVIDIA managed health…renders as…_API_KEY <REDACTED> NVIDIA managed health…. A candidate follow-up is to giveFULL_REDACT_PATTERNSthe same{10,}minimum value lengthCONTEXT_PATTERNSalready uses.renderDoctorReportJSON branch currently has no production caller (the oclif command always passesquietJson: truewith--json); it is redacted anyway because thatconsole.logegress has no other redaction layer for any future direct caller.Type of Change
Quality Gates
doctor --jsonis documented only as "Emit the report as JSON" with no field-level or verbatim-detail contract (docs/reference/commands.mdx); this change enforces the existing CLI redaction contract indocs/security/best-practices.mdx.92aa402after reviewing the redaction coverage, resolved automated feedback, required checks, and selected E2E.DGX Station Hardware Evidence
scripts/prepare-dgx-station-host.sh.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailable —npm run check:diffpassed (hooks skipped locally; commitlint reports one non-blockingfooter-leading-blankwarning)npx vitest run --project cli src/lib/actions/sandbox/doctor-report.test.ts src/commands/sandbox/oclif-command-adapters.test.ts src/lib/actions/sandbox/doctor-flow.test.ts src/lib/actions/sandbox/doctor-inference.test.ts→ 42/42 passednpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: not applicable (targeted change)Signed-off-by: Dongni-Yang dongniy@nvidia.com
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests