fix(diagnostics): redact URL userinfo credentials in the debug bundle

[udsy19]

Summary

Fixes #10040.

nemoclaw debug support bundles and terminal output now remove credentials embedded in URL userinfo. A shared full-redaction helper owns URL parsing, fail-closed fallback, and known secret-pattern replacement. The gateway restart and policy-rendering callers now use that helper without changing their existing redaction behavior.

Root cause and detection gap

The debug sink applied known secret patterns without parsing URL tokens. Its tests covered token and environment-assignment shapes but did not pass URL userinfo through the debug redactor.

Changes

• Add shared full redaction for known secret patterns and URL credentials.
• Route debug output through the shared helper, and replace the equivalent redaction sequences in gateway restart diagnostics and policy rendering without changing their behavior.
• Keep control-character removal and terminal escaping in their existing callers.
• Cover username-and-password userinfo, userinfo-only URLs, malformed URL fallback, and the debug-bundle boundary.

Type of Change

• Code change (feature, bug fix, or refactor)
• Code change with doc updates
• Doc only (prose changes, no code sample modifications)
• Doc only (includes code sample changes)

Quality Gates

• Tests added or updated for changed behavior
• Existing tests cover changed behavior — justification:
• Tests 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: Exact-commit review covered secret handling, fail-closed malformed URL behavior, caller ordering, permissions, dependencies, error output, configuration, and focused negative tests. No security finding remains.
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

DGX Station Hardware Evidence

• Tested on DGX Station
• Tested commit:
• Station profile/scenario:
• Result:
• Supporting evidence:

Verification

• PR description includes a Signed-off-by: line and every published commit appears as Verified in GitHub
• npm run validate:pr passed for the exact pull request commit
• Focused redaction, debug, gateway-restart, and policy-rendering tests passed: 4 files, 172 tests
• CLI build, CLI type-check, repository checks, and git diff --check passed
• Applicable broad gate passed — required CI is running for the current revision
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• npm run docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

---

Signed-off-by: Udaya Tejas udayatejas2004@gmail.com
Signed-off-by: Apurv Kumaria akumaria@nvidia.com

Summary by CodeRabbit

• Security

  • Debug bundles and terminal output now remove URL-embedded usernames and passwords while credential-free URLs remain readable.
  • Existing gateway restart and policy-rendering redaction now use the same shared helper without changing behavior.
  • Malformed URLs fail closed when they cannot be safely redacted.

• Tests

  • Added coverage for URL credentials, credential-free URLs, malformed URL redaction, and the debug-bundle boundary.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5afdbe5a-8e6f-4116-929a-2b361aa07348

📥 Commits

Reviewing files that changed from the base of the PR and between 73f0ca9 and dda4f0e.

📒 Files selected for processing (2)

• test/cli-coverage-sequencer.test.ts
• test/helpers/cli-coverage-sequencer.ts

Included review availability: Your plan provides up to 12 included reviews per hour; 8 remain after this review.

---

📝 Walkthrough

Walkthrough

The redaction library adds redactFullWithUrls, which removes URL credentials before full-text secret redaction. Diagnostic, gateway restart, and terminal rendering paths now use this helper. Coverage shard assignments also reflect a new salt.

Changes

URL-aware redaction

Layer / File(s)	Summary
URL-aware redaction helper src/lib/security/redact.ts, src/lib/security/redact.test.ts	Adds and documents redactFullWithUrls. Tests cover URL credentials, userinfo-only URLs, and malformed URL fallback behavior.
Redaction caller integration src/lib/diagnostics/debug.ts, src/lib/actions/sandbox/gateway-restart.ts, src/lib/policy/preset-scope-render.ts, src/lib/diagnostics/debug.test.ts	Uses redactFullWithUrls for diagnostic, gateway restart, and terminal rendering output. Diagnostic tests cover credential removal and credential-free URL preservation.

Coverage shard update

Layer / File(s)	Summary
Integration shard assignments test/helpers/cli-coverage-sequencer.ts, test/cli-coverage-sequencer.test.ts	Changes the integration shard salt and updates expected assignments for two test keys.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🟡 Moderate · up to dda4f

This change centralizes redaction of URL credentials across diagnostic outputs and adds focused coverage. The focused tests and hooks pass, but the required broad CI gate is still rerunning, so merge should wait for that check or explicit acceptance.

Suggested reviewers: cv

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The integration shard salt and assignment snapshot changes are unrelated to issue #10040's credential-redaction objective.	Move the shard-balancing configuration and snapshot updates to a separate pull request unless they are required by an explicitly linked issue.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes satisfy issue #10040 by applying combined URL and secret redaction to debug output and related full-redaction sinks.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3 files.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: redacting URL userinfo credentials from the diagnostics debug bundle.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[github-actions]

PR Review Advisor — Blocking findings reported

Advisor assessment: Blockers require maintainer review
Next action: Review the blockers below.
Findings: 1 blocker · 0 warnings · 0 suggestions
Synthesis status: Completed · high confidence · 1 blocker · 0 warnings · 0 suggestions

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite for the commit under review.

Recommended E2E: inference-routing

Manual-only E2E: security-posture, onboard-repair, onboard-resume, cloud-inference, network-policy
The manual PR workflow does not run these selectors for the commit under review. Run them from reviewed code on main.

Blockers

PRA-1 Blocker — Consolidate duplicate debug URL-redaction tests

• Location: test/secret-redaction.test.ts:85
• Category: architecture
• Problem: The URL credential matrix creates a second owner for URL-redaction behavior. The shared redactor tests already specify parsed username/password, userinfo-only, and malformed URL behavior. The debug wrapper test imports `redact` directly, so its three URL cases do not exercise diagnostic collection.
• Impact: The repository has overlapping URL fixtures and expectations in separate test owners. A redaction change requires synchronized updates without adding a distinct boundary proof.
• Fix: Remove the three-case URL credential matrix from `test/secret-redaction.test.ts`. Add one credential-bearing URL and one credential-free URL assertion to `src/lib/diagnostics/debug.test.ts`, which owns the debug wrapper. Keep parsed, userinfo-only, and malformed URL cases in the shared redactor tests.
• Verification: Compare the remaining debug-wrapper assertions with `src/lib/diagnostics/debug.ts:59-66` and confirm that only `src/lib/security/redact.test.ts` owns the URL parsing matrix.
• Test coverage: `src/lib/diagnostics/debug.test.ts` should assert that its `redact` wrapper removes userinfo from one URL and preserves one credential-free URL. `src/lib/security/redact.test.ts` should retain parsed, userinfo-only, and malformed URL coverage.
• Evidence: test/secret-redaction.test.ts:85-120 defines three URL credential cases and calls `debugRedact` directly. src/lib/security/redact.test.ts:10-29 specifies username/password, userinfo-only, and malformed URL output for `redactFullWithUrls`. src/lib/security/redact-url.test.ts:49-57 covers malformed URL userinfo redaction.

Recommended refactoring

Implementation guidance; a fix with equal or lower complexity is acceptable.

• PRA-1: Remove Remove `URL_CREDENTIAL_CASES` and its parameterized test from `test/secret-redaction.test.ts`.; use Use two direct assertions in `src/lib/diagnostics/debug.test.ts` for debug-wrapper delegation: one userinfo URL and one credential-free URL.. Net: -8 lines. Keep: Keep the parsed, userinfo-only, and malformed URL matrix in shared redactor tests. Preserve a debug-wrapper test so diagnostic output cannot bypass URL-aware redaction.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

[cv]

Reviewed the complete diff at commit 224320b. The shared full redactor removes valid and malformed URL userinfo before applying existing secret-pattern redaction, and the affected diagnostic, gateway restart, and policy-rendering callers preserve their control-character and terminal escaping boundaries. Tests cover username/password userinfo, userinfo-only URLs, malformed URL fallback, the debug-bundle boundary, existing secret assignments, and credential-free URLs. The required repository checks and aggregate CLI checks pass. CodeQL and CodeRabbit pass, all commits are GitHub Verified, the PR body includes the contributor DCO declaration, and no review thread or correctness, security, product-scope, or documentation blocker remains.

[apurvvkumaria]

Resolved the automated advisor blocker on the current revision.

• Removed the duplicate URL-credential matrix from the cross-sink integration test.
• Added one credential-bearing URL case and one credential-free URL case to the debug-wrapper test that owns this boundary.
• Kept parsed, userinfo-only, and malformed URL behavior in the shared redactor tests.
• Focused diagnostics and redaction tests pass: 104 CLI tests and 43 integration tests.
• CLI type-checking, formatting, normal pre-commit checks, commit-message checks, and pre-push checks pass.
• The new commit is signed, verified by GitHub, and includes the DCO sign-off.
• Security review remains clear: URL credentials are removed before diagnostic output, malformed inputs fail closed in the shared redactor, and credential-free URLs stay readable.
• The PR is now 56 additions and 15 deletions across 6 files, below the large-change threshold.

Udaya Tejas remains the original contributor and PR author. The original signed contribution is unchanged; this commit only addresses the maintainer review finding.

[apurvvkumaria]

Maintainer security and merge-result review remains PASS after updating the branch from current main.

• The shared redactor removes URL userinfo before known-secret replacement. Valid and malformed credential-bearing URLs fail closed, while credential-free URLs remain readable.
• Diagnostic archives and terminal output, gateway restart errors, and policy rendering use the shared helper without changing their control-character or terminal-escaping boundaries.
• No untrusted value becomes a command, path, network destination, authorization decision, or approval action. The change adds no dependency and no workflow file.
• The merge result passes 172 focused CLI tests and 43 integration tests, followed by the CLI build and type check.
• The earlier automated blocker about duplicate test ownership is resolved: the debug boundary owns one credential-bearing and one credential-free case, while URL parsing cases remain with the shared redactor.
• The change is 56 additions and 15 deletions across 6 files, below the large-change threshold.

The fork workflows were held for maintainer vetting. I approved only the runs for this reviewed revision through the normal GitHub path; required checks are now running.

Udaya Tejas remains credited as the original contributor and PR author. Carlos Villela's review and branch-update work remains visible in the PR history.

[apurvvkumaria]

Approved after the current branch update, security review, merge-result tests, and compliance review.

Udaya Tejas remains the original contributor. Merge will wait for every required check to pass.

[apurvvkumaria]

CI blocker remediation

The first CLI shard failed on both #10035 and #10041 because the current main test roster exceeded the existing combined-shard balance limit by 3,305 weighted milliseconds. A focused run against the current merge result reproduced the same assertion, so this is a deterministic repository interaction rather than a redaction or documentation defect. A blind rerun would not help.

Commit dda4f0e42 recalibrates only integration-test ownership and updates the stable-assignment snapshot. It does not change the 5% combined limit, the 10% serialized-integration limit, test timing data, worker counts, retries, production code, or security behavior.

Validation for the current revision:

• Redaction, debug, gateway-restart, and policy-rendering suites: 172 tests passed.
• Secret-redaction and shard-sequencer suites: 56 tests passed.
• CLI build, CLI type-check, repository checks, and normal commit and push hooks passed.
• Security review remains PASS; the added change affects test scheduling only.

Udaya Tejas remains the original author of the redaction fix. The shard mechanism being recalibrated was introduced in #9950 by Rebecca Sliter and Prekshi Vyas; their existing work and repository history remain intact.

[cv]

Reviewed the complete current diff and latest merge commit at 8ba445e83876c5559324eb2ff93c8dd560bed4c1. The six-file product diff correctly centralizes URL-userinfo removal before full secret redaction, preserves the existing control-character and terminal-escaping boundaries, and keeps URL parsing cases with the shared redactor while the debug boundary covers one credential-bearing and one credential-free URL. The earlier Advisor test-ownership finding is resolved. The linked accepted issue establishes this security-fix scope; no new supported integration or product surface is introduced. The PR body includes DCO declarations, the DCO check passes, every PR commit has a valid GitHub signature, CodeRabbit reports no actionable comments, and the complete review-thread snapshot has no threads. I found no correctness, security, test-design, product-scope, or writing defect that needs a code change.

I am leaving a neutral review rather than inheriting the earlier approval because complete exact-head automation evidence is not available: the current Advisor specialists failed before analysis when inference configuration was skipped, and the current CI changes job failed before classification; its downstream checks were skipped. The separate growth guard also failed while reading the PR files with GitHub HTTP 403. Local validation could not run in this isolated worktree because dependencies are absent. These are automation/evidence gaps rather than concrete defects in this diff. Approval should wait for a complete exact-head Advisor result and successful required CI, or an explicit maintainer waiver.

[prekshivyas]

Security review: PASS

I reviewed exact commit 531e27cd478800ad4b5368fe228a3732c6dbcf51 against base 7e7c8140f2c0d416c9b7392380805f30b0fa64aa. The change is safe from a security-review perspective. It closes the reported debug-artifact credential leak and preserves the existing behavior of the two callers whose equivalent redaction sequence was centralized. This verdict does not waive required CI or repository merge gates.

Findings

No security findings.

Detailed analysis

1. Secrets and credentials — PASS. Debug bundle files and terminal output now pass URL tokens through full URL redaction before known secret-pattern replacement. Username-and-password userinfo, userinfo-only URLs, and malformed userinfo are covered. Credential-free URLs remain readable.
2. Input validation and data sanitization — PASS. The helper reuses the repository URL-token parser and its bounded parsing plus fail-closed malformed-URL fallback. It does not create a second URL interpretation. Gateway restart diagnostics retain control-character removal, and policy rendering retains terminal escaping after redaction.
3. Authentication and authorization — PASS. The diff does not change authentication, authorization, ownership, roles, or resource access.
4. Dependencies and third-party libraries — PASS. The diff adds no dependency, artifact, registry, install hook, or runtime loader.
5. Error handling and logging — PASS. The affected debug sink no longer emits URL userinfo to the support bundle or terminal. Malformed input is redacted without logging the rejected credential or turning a security failure into success.
6. Cryptography and data protection — PASS. The change does not introduce cryptography or claim masking as encryption. It reduces plaintext credential retention at diagnostics boundaries.
7. Configuration and security headers — PASS. The diff changes no deployed default, policy precedence, capability, port, network rule, TLS setting, browser header, or override. Policy rendering behavior remains equivalent.
8. Security testing — PASS. Exact-commit tests cover allowed credential-free URLs and forbidden credential-bearing cases, including malformed URL fallback and the debug boundary. The four affected CLI test files passed 172 of 172 tests after the required CLI build.
9. System security — PASS. The shared helper keeps the authoritative order used by existing full-redaction sinks: redact URL tokens first, then apply known full-secret patterns. The gateway and policy callers preserve their adjacent controls, and the debug caller now uses the same sequence. No alternate debug output path in the changed surface bypasses the helper.

Files reviewed

• src/lib/actions/sandbox/gateway-restart.ts
• src/lib/diagnostics/debug.test.ts
• src/lib/diagnostics/debug.ts
• src/lib/policy/preset-scope-render.ts
• src/lib/security/redact.test.ts
• src/lib/security/redact.ts

Exact-commit evidence

• CLI build passed.
• Four focused CLI test files passed: 172 of 172 tests.
• CLI type-check passed after the build.
• Repository checks passed.
• npm run validate:pr passed, including gitleaks and the exact diff's pre-commit, commit-message, and pre-push checks.
• git diff --check passed.
• GitHub reports all 17 pull request commits as Verified; Udaya Tejas's original signed implementation commit remains in the ancestry.

[prekshivyas]

Documentation Writer Review Receipt

• Result: PASS — no-docs-needed
• PR: fix(diagnostics): redact URL userinfo credentials in the debug bundle #10041
• Commit under review: 531e27cd478800ad4b5368fe228a3732c6dbcf51
• Base SHA: 7e7c8140f2c0d416c9b7392380805f30b0fa64aa
• AGENTS.md blob SHA: becb5c5864e9dcade3810bc71fbfa8ab848eae05
• Review surface: Codex Desktop

Files reviewed:

• src/lib/actions/sandbox/gateway-restart.ts
• src/lib/diagnostics/debug.test.ts
• src/lib/diagnostics/debug.ts
• src/lib/policy/preset-scope-render.ts
• src/lib/security/redact.test.ts
• src/lib/security/redact.ts

Documentation disposition: No source-page documentation change is required. Existing documentation describes the intended secret-redaction control and tells users to inspect diagnostics before sharing them. This change makes debug output conform to that existing contract. It adds no command, flag, configuration, workflow, or supported surface. The post-merge documentation workflow can record the shipped security correction.

Changed-text review: The helper documentation, debug redaction comment, case labels, and test titles are accurate and behavior-specific. redactFullWithUrls correctly names URL-token redaction followed by known-pattern replacement. Gateway restart diagnostics retain control-character removal. Policy rendering retains terminal escaping.

PR-text review: The corrected description now matches the six-file diff. It identifies the debug bundle and terminal output as the corrected behavior. It accurately describes gateway restart and policy rendering as unchanged consumers of the shared helper. The stale integration-shard claims are absent.

Product and security scope: Issue #10040 records the exact debug-bundle credential leak and reproduction. A repository member classified it as security-sensitive. The diff remediates that bounded defect without requiring a product, business, or architecture decision.

Attribution and compliance: Udaya Tejas remains the author of commit 6172ecdc9fed70a0331977b1b55ab72fae05f24b, which contains the original diagnosis and remediation with Udaya's DCO declaration. Later centralization and tests retain separate authorship. GitHub reports every PR commit through the latest PR commit as Verified.

Exact-commit validation:

• npm run validate:pr: passed.
• Four focused CLI test files: 172 of 172 passed.
• CLI build: passed.
• CLI type-check after build: passed.
• Repository checks: passed.
• git diff --check: passed.

The initial test and type-check failures resulted from absent compiled dist artifacts in the fresh worktree. Both passed after the required CLI build.

Blocking findings: None.

Nonblocking suggestion: Replace “current revision” in the pending broad-gate item with “latest PR commit 531e27cd478800ad4b5368fe228a3732c6dbcf51” when the description is next updated. GitHub's exact-commit check state remains the merge authority.