Skip to content

fix(diagnostics): keep credentials and private hosts out of diagnostic and config output - #5573

Merged
lidge-jun merged 12 commits into
devfrom
codex/260922-next-diagnostic-redaction
Sep 22, 2026
Merged

lidge-jun merged 12 commits into
devfrom
codex/260922-next-diagnostic-redaction

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 22, 2026 •

Copy link
Copy Markdown
Owner

Summary

Diagnostic and config output could reproduce values that belong only in private configuration or upstream responses. This carries the consolidated work from #5538 (itself combining #5475, #5427, #5414 and #5463) onto current dev, and adds two repairs found during security review.

Carried changes, one commit per original contribution with authorship preserved:

  • Lab diagnostic sanitization redacts socket and resolver destinations, including a sentence-final period, while ordinary prose after a marker survives.
  • pool.credentialGroups validation messages identify invalid groups and members by position and no longer print group ids or credential strings.
  • ocx config show, get, and mutation output strip userinfo from proxy URLs while keeping host and port; direct and credential-free values stay readable, a non-URL value is masked whole, and config export stays raw so a backup can restore credentials.
  • Anthropic model-scoped quota windows publish only the canonical Fable, Opus, or Sonnet label after removing terminal controls; unrecognized upstream display names are dropped.

Repairs added here:

  • Normalized path aliases bypassed proxy masking. Lookup trims and drops empty path segments, but redaction received the raw final segment, so ocx config get 'providers.openai.proxy ' or providers.openai.proxy. printed the full credentialed URL. Get and mutation output now redact by the normalized final segment.
  • Resolver destinations leaked before explanatory text. The consolidated grammar licensed a bare name only as a sole argument or after lookup, so getaddrinfo ENOTFOUND redis: no such host kept redis, which the previous sanitizer redacted. A bare first argument directly after ENOTFOUND, EAI_AGAIN, or host=/host: is redacted again; socket-state prose such as ETIMEDOUT operation timed out is unchanged.

Owner docs (structure/config.md, structure/catalog.md, structure/providers-and-adapters.md, structure/adapters/compatibility-lab.md) and the CLI reference describe the resulting output contracts. No new import reaches Compatibility Lab code from the core request path.

Supersedes #5538.

Co-authored-by: luvs01 27862058+luvs01@users.noreply.github.com

Verification

  • Local test, typecheck and build runs: NOT RUN in this lane; hosted CI on this PR is the verification.
  • Regression coverage added for normalized path aliases (trailing space, trailing dot, empty segments) in get and set, direct resolver destinations followed by prose, and the unchanged socket-prose negatives.
  • git diff --check is clean. The source commits were compared by range-diff with each original PR head; the carried patches match apart from provenance trailers.
  • Independent static security review of the final diff: PASS, no blocking findings (verdict posted as a comment).

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

  • New Features

    • Added safer proxy credential handling: configuration displays mask credentials while preserving host and port; exports retain original values for backup and restore.
    • Improved diagnostic sanitization to hide network destinations and sensitive credential identifiers in logs and validation messages.
    • Quota displays now show only recognized Anthropic labels, such as Opus or Sonnet.
  • Documentation

    • Updated CLI, configuration, compatibility, and provider documentation to explain redaction and sanitized quota behavior.

luvs01 and others added 12 commits September 22, 2026 21:05
…nect-to failures

Combines three fork PRs into one change on current dev: bare destinations after resolver/socket markers are redacted only where grammar proves the position (sole argument or after 'lookup'), connect-to failures redact the bare target when a failure term follows, and rewrites splice at the tail offset so a marker word that repeats the destination cannot take the [host] itself.

bun test tests/lab/lab-evidence-sanitization.test.ts: 31 pass

(cherry picked from commit 69990d9)

Source: #5538 commit b90139f

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
Classify the token without its trailing period and leave the period outside the mask: ECONNREFUSED redis. -> ECONNREFUSED [host]. Previously the punctuation defeated host classification and leaked the destination.

(cherry picked from commit a74038c)

Source: #5538 commit d8042a5

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
…iers in credentialGroups diagnostics

(cherry picked from commit 0354bd9)

Source: #5538 commit fc57a55

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
…ue shape

(cherry picked from commit f8a67b3)

Source: #5538 commit 038c0fa

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
(cherry picked from commit 73946f3)

Source: #5538 commit b31732a

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
Mask only credential-bearing proxy URLs (userinfo stripped via redactUrlForLog, host/port kept); direct and credential-less values print unchanged. Clarify that config export stays raw for restore.

(cherry picked from commit acdbb1c)

Source: #5538 commit dc4d273

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
(cherry picked from commit ee716bf)

Source: #5538 commit 4a04458

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
Per review on #5463: stripping terminal control characters still left attacker-chosen residue on the quota line. parseClaudeLimit now returns null unless the name reduces to a recognized model (Fable, Opus, Sonnet); the test asserts the residue is not emitted and adds a recognized-name positive case.

(cherry picked from commit a9a6bac)

Source: #5538 commit aaffe75

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
Source: #5538 commit 62c677c

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
… prose

Use normalized config keys for get and mutation output. Preserve direct
resolver destinations when explanatory text follows, while retaining socket
prose exclusions. Add focused regressions and align output contracts.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 22, 2026 17:46
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@lidge-jun

Copy link
Copy Markdown
Owner Author

Independent security review: PASS, no blocking findings.

Reviewed commit: adfb59a. The only later changes are a merge of current dev and removal of a five-line cross-reference from structure/runtime.md; the diff of every lane-owned file against dev is byte-identical to the reviewed diff.

Scope and conclusions:

  • Config get and mutation output select redaction by the same normalized path used for lookup, so path aliases cannot bypass proxy credential masking. Credential-bearing proxy URLs lose userinfo, direct and credential-free values stay readable, malformed non-direct values are masked whole, and config export remains an explicit raw backup.
  • Lab diagnostic sanitization treats a bare first argument as a destination only after direct resolver or explicit host markers; socket-state prose is not broadly redacted. Persisted artifacts pass through the store's sanitizer before serialization.
  • Anthropic quota labels are limited to canonical model names after control-character removal; the control-character class is the C0, DEL and C1 ranges written as ASCII escapes.
  • Credential-group diagnostics report group and member positions only, with the existing secret redaction kept as a second layer.

This was a static review. Runtime evidence comes from this PR's hosted CI.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 22, 2026
@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: cb3e1061-13f6-48bf-bbd3-4aa4efa2d74a

📥 Commits

Reviewing files that changed from the base of the PR and between c43f29c and d31799a.

📒 Files selected for processing (16)
  • docs-site/src/content/docs/reference/cli/agents.md
  • scripts/test-layout/layout.json
  • src/cli/config-command.ts
  • src/config/load-degrade.ts
  • src/lab/artifacts/sanitize.ts
  • src/providers/quota/vendor-probes-oauth.ts
  • src/routing/identity-domains.ts
  • structure/adapters/compatibility-lab.md
  • structure/catalog.md
  • structure/config.md
  • structure/providers-and-adapters.md
  • tests/cli/cli-config-command.test.ts
  • tests/config/config-load-degrade.test.ts
  • tests/fixtures/test-layout-expected.json
  • tests/lab/lab-evidence-sanitization.test.ts
  • tests/providers/provider-quota-label-sanitize.test.ts
 __________________
< I see dead code. >
 ------------------
  \
   \   \
        \ /\
        ( )
      .( o ).
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@lidge-jun
lidge-jun merged commit d30d120 into dev Sep 22, 2026
31 of 32 checks passed
@lidge-jun
lidge-jun deleted the codex/260922-next-diagnostic-redaction branch September 22, 2026 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants