fix(security): redact token-shaped URL query values

[fallintoplace]

Summary

Redact token-shaped URL query values even when the parameter name looks benign. This closes the onboarding/session persistence gap tracked in #6224 and adds regression coverage around repeated params, encoded values, and structured token shapes.

Related Issue

Refs #6224

Changes

• Run every URL query value through canonical standalone-secret redaction instead of relying only on sensitive parameter names.
• Extend the canonical secret patterns with JWT-shaped tokens and keep the e2e redaction fixture in parity.
• Replace the onboarding it.todo() with an active regression test and add broader URL redaction coverage for NVIDIA, OpenAI, Anthropic, GitHub, Slack, percent-encoded values, repeated keys, mixed values, fragments, and user-info.

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:
• Docs updated for user-facing behavior changes
• Docs not applicable — justification: Redaction behavior changes only affect persisted diagnostics and internal test coverage.
• 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: Focused self-review on the redaction path plus targeted regression coverage for onboarding persistence and canonical redaction parity.
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

• PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
• Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
• Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: npx vitest run --project cli src/lib/security/redact.test.ts src/lib/security/credential-filter-secret-patterns.test.ts src/lib/state/onboard-session.test.ts and npx vitest run --project e2e-support test/e2e/support/e2e-redaction-parity.test.ts both passed after the rebase onto origin/main.
• Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
• 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: Minh Vu vuhoangminh97@gmail.com

Summary by CodeRabbit

• New Features
  • Added detection and redaction for JWT-like structured tokens (e.g., header.payload.signature) across log and persisted outputs.
• Bug Fixes
  • Improved URL redaction reliability, including query-string handling: sensitive values are consistently replaced with <REDACTED> while safe parameters are preserved.
  • Onboarding session endpoint URLs no longer retain token-shaped secrets.
• Tests
  • Expanded URL/redaction and onboarding-session coverage, including tricky wrapping, malformed inputs, repeated parameters, and token-shaped values.
• Documentation
  • Updated redaction contract wording to reflect canonical token-shaped redaction behavior.

[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]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

JWT-like structured token detection is added to shared secret patterns and redaction paths. URL parsing and query redaction are centralized, with coverage added for credential handling, token-shaped values, session persistence, and product-fixture parity.

Changes

Structured token redaction

Layer / File(s)	Summary
Structured token pattern contracts src/lib/security/secret-patterns.ts, test/e2e/fixtures/redaction.ts, test/e2e/support/e2e-redaction-parity.test.ts, src/lib/onboard/lifecycle-contracts.md	A JWT-like pattern is exported, included in aggregate patterns, applied by fixture redaction, verified for parity, and reflected in lifecycle contract coverage.
Redaction and URL handling src/lib/security/redact.ts	Full, standalone-secret, and sensitive-text redaction process structured tokens; URL parsing retries after trailing delimiters, and query values are rebuilt with sensitive and token-shaped values redacted.
Redaction behavior validation test/helpers/security-token-fixtures.ts, src/lib/security/credential-filter-secret-patterns.test.ts, src/lib/security/redact-url.test.ts, src/lib/security/redact.test.ts, src/lib/state/onboard-session-redaction.test.ts, src/lib/state/onboard-session.test.ts	JWT fixtures support detection tests; URL, credential-filter, and session tests cover redaction behavior and persisted endpoint sanitization, while the former URL tests are removed from the general redaction test file.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant redactUrl
  participant URLSearchParams
  participant redactStandaloneSecrets
  Client->>redactUrl: provide URL
  redactUrl->>URLSearchParams: rebuild query parameters
  URLSearchParams->>redactStandaloneSecrets: process non-sensitive values
  redactStandaloneSecrets-->>URLSearchParams: return redacted values
  URLSearchParams-->>redactUrl: provide reconstructed query
  redactUrl-->>Client: return redacted URL

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 23.53% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: redacting token-shaped URL query values in security-related code.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[github-actions]

E2E Advisor Recommendation

Required E2E: credential-sanitization, security-posture, cloud-onboard
Optional E2E: credential-migration

Dispatch hint: credential-sanitization,security-posture

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: target/main
Head: HEAD
Confidence: high

Required E2E

• credential-sanitization: Required by the deterministic risk plan for credential/security-boundary source changes. This live E2E validates credential stripping, session/migration sanitization, artifact secrecy, and sandbox credential checks against a real install/onboard/sandbox lifecycle.
• security-posture: Required by the deterministic risk plan for redaction and secret-pattern changes. It exercises full OpenClaw and Hermes hosted onboarding paths with real credential handling and validates the broader security boundary under live assistant flows.
• cloud-onboard (high): Changed onboard, trace timing, scorecard, or E2E workflow code can affect cloud onboard wall-clock behavior and should refresh the trusted cloud-onboard trace timing signal.

Optional E2E

• credential-migration: Useful adjacent confidence for changes to canonical secret patterns and credential redaction, since legacy credential migration must not revive or expose removed secrets.

New E2E recommendations

• None.

Dispatch hint

• Workflow: .github/workflows/e2e.yaml
• jobs input: credential-sanitization,security-posture

[github-actions]

E2E Target Recommendation

Required E2E targets: credential-sanitization, security-posture, e2e-all
Optional E2E targets: None

Dispatch required E2E targets:

• gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=credential-sanitization
• gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=security-posture
• gh workflow run e2e.yaml --ref <pr-head-ref>

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: target/main
Head: HEAD
Confidence: high

Required E2E targets

• credential-sanitization: Credential and security-boundary changes must preserve secrecy, sanitization, and fail-closed policy behavior.
  • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=credential-sanitization
• security-posture: Credential and security-boundary changes must preserve secrecy, sanitization, and fail-closed policy behavior.
  • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=security-posture
• e2e-all: Shared E2E redaction fixture/parity changes affect the common live E2E artifact and secret-sanitization machinery, so the full E2E fan-out is required.
  • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref>

Optional E2E targets

• None.

Relevant changed files

• src/lib/security/redact-url.ts
• src/lib/security/redact.ts
• src/lib/security/secret-patterns.ts
• test/e2e/fixtures/redaction.ts
• test/e2e/support/e2e-redaction-parity.test.ts

[cv]

Maintainer follow-up on exact head 3ea7e6d:

• Synced current origin/main with a signed merge commit.
• Changed the epic link from Fixes Epic: stabilize onboarding and runtime lifecycle contracts before public extensibility #6224 to Refs Epic: stabilize onboarding and runtime lifecycle contracts before public extensibility #6224 so this focused redaction PR cannot close an incomplete five-child epic.
• Closed two security gaps found during babysitting: redactUrl now applies the canonical sensitive-key classifier to opaque query values such as api_key, apiKey, password, secret, authorization, and credential; structured JWT matching now accepts valid base64url payloads such as e30 rather than requiring every payload to begin eyJ.
• Kept the E2E fixture regex in exact parity with the product source and added direct regressions for both cases.

Evidence: 114 focused CLI tests passed; 33 focused E2E redaction/parity tests passed; CLI typecheck and build passed; normal pre-commit and pre-push hooks passed. A broad test:fast attempt encountered unrelated current-main test-loader failures after its clean:cli step; the affected focused security/parity lanes are green. Both new commits are GitHub Verified and DCO-signed.

[cv]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

🧹 Nitpick comments (1)

src/lib/security/redact-url.test.ts (1)

16-174: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

LGTM — solid coverage of credential stripping, wrapper-delimiter bounding, sensitive-key query redaction, and mixed/encoded/repeated token-shaped values via the public redact/redactUrl API. Per path instructions, tests assert on observable outputs rather than internals.

One coverage gap worth adding: a case where the URL fails to parse entirely (e.g., beyond the bounded trailing-delimiter fallback) and contains a sensitive-keyed query value that isn't itself a recognized token pattern. See the corresponding comment on redactUrlPartial in redact.ts (Lines 158-166) for the underlying gap this would exercise.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/security/redact-url.test.ts` around lines 16 - 174, Add a
URL-redaction test covering an unparseable URL with more than the bounded
trailing delimiters and a sensitive-key query value that is not token-shaped.
Exercise the public redactUrl and redact APIs, and assert the sensitive value is
replaced in both persisted and log outputs, targeting the redactUrlPartial
fallback behavior.

Source: Path instructions

🤖 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 `@src/lib/security/redact-url.test.ts`:
- Around line 16-174: Add a URL-redaction test covering an unparseable URL with
more than the bounded trailing delimiters and a sensitive-key query value that
is not token-shaped. Exercise the public redactUrl and redact APIs, and assert
the sensitive value is replaced in both persisted and log outputs, targeting the
redactUrlPartial fallback behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: eed457b7-c4c9-401c-89ac-29b519ac2fe2

📥 Commits

Reviewing files that changed from the base of the PR and between 9976951 and 41662e4.

📒 Files selected for processing (3)

• src/lib/security/redact-url.test.ts
• src/lib/security/redact.test.ts
• src/lib/security/redact.ts

[prekshivyas]

Overlap audit for the redaction source of truth: #6273 also touches redact.ts and secret-patterns.ts, but is already CONFLICTING at head 33b13dd8. Safe merge order is to land this focused security fix first; #6273 must then rebase and preserve both its Firecrawl patterns and this PR's structured/contextual URL-query coverage. The current #6650 head includes both the author's latest URL parser/test extraction and the follow-up Bearer/context fix.

[cv]

Approved on exact head c35c998. All required CI passes, DCO/signatures are verified, CodeRabbit has no unresolved threads, and exact manual advisor run 29149066182 has both models at high-confidence merge_as_is with zero findings/all security categories PASS. Full exact E2E run 29149050337 passed 80 substantive jobs across its original run and failed-job retry, including credential-sanitization, credential-migration, both security-posture variants, full-e2e, and the retried hermes-shields-config. The sole remaining rebuild-hermes-stale-base failure is external Debian repository drift: the untouched old-Hermes base cannot install pinned curl 8.14.1-2+deb13u3 because that version is no longer served; it occurs before PR behavior and is unrelated to the redaction diff.