fix(cli): redact URL query secrets in the error envelope

[cliffhall]

Closes #2423

What changed

The CLI's JSON error envelope ({"error":{...}} on stderr) was built from context.url, a CliExitCodeError's own envelope.url, and the raw error.message / cause chain with no redaction. That meant an OAuth code, access_token, client_secret and so on in a server URL could be echoed verbatim to a terminal, a CI log, or a pipe.

classifyError in clients/cli/src/error-handler.ts now passes the finished envelope through core's existing redactUrlQuery, the same helper the web client's Network log and OAuthRequestTimeoutError already use:

• url: redacted directly.
• message and cause: every embedded http(s):// URL is redacted in place. Trailing sentence punctuation is split off first and put back, so …?code=abc. keeps its full stop.

Both formatErrorOutput and handleError go through classifyError, so both sinks are covered. Classification still reads the unredacted text. Redaction runs on the output rather than the input, because redaction rewrites parameter values, and a pattern test on the rewritten copy could produce a different exit code. A test pins this.

TUI (and web) on-screen errors: out of scope, tracked in #2490

The TUI does not write errors to a serialized or piped sink the way the CLI envelope does. It renders error text onto its own Ink screen, and so does the web client, which puts err.message straight into toasts unredacted. The web client applies redactUrlQuery in only two places: the Network log and OAuthRequestTimeoutError. The TUI gets the latter already scrubbed. So the TUI is at parity with the web client today, and the gap parity left open was the CLI envelope.

Redacting displayed error text in both interactive clients is new work, tracked in #2490.

Tests

clients/cli/__tests__/error-handler.test.ts gains an envelope URL redaction block covering:

• the context url and a CliExitCodeError's own url
• a URL embedded in the message, and one embedded in the cause
• classification on unredacted text
• non-sensitive URLs left untouched
• the serialized stderr line

npm run local:gate passes. error-handler.ts coverage is 100 / 95.3 / 100 / 100.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

Critical redaction gaps remain for uppercase URL schemes and TUI-displayed errors.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2

Open (2)

• URL redaction misses uppercase and mixed-case schemes · New
• TUI error display bypasses credential redaction · New

What changed in this PR

Adds URL query-secret redaction to CLI JSON error envelopes while preserving error classification behavior.

Changes:

• Redacts URLs in envelope fields, messages, causes, and stderr output.
• Adds focused redaction and serialization tests.

File	Review
clients/​cli/​src/​error-handler.ts	Implements redaction. Unresolved: uppercase schemes bypass redaction (critical, 4 votes); trailing punctuation may leak credential suffixes (moderate, 1 vote); TUI errors remain unredacted (critical, 1 vote).
clients/​cli/​__tests__/​error-handler.test.ts	Tests envelope redaction, classification, and serialization; regression coverage is still needed for uppercase schemes and credential-ending punctuation.

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[cliffhall]

Copilot round 1

• Uppercase/mixed-case schemes bypass redaction: fixed in d5de6ee (i flag plus a regression test).
• TUI error display bypasses redaction: declined as out of scope. The web client does not redact displayed error text either, so the TUI already matches it. Filed as Redact URL query secrets in error text displayed by the web and TUI clients #2490.
• Trailing punctuation may leak credential suffixes (overview table only, no thread): not changed. Splitting off trailing .,;:!?)] only ever exposes those punctuation characters, never any part of the secret before them. Folding them into the value instead would mangle the far more common case of a URL ending a sentence.

npm run local:gate is green on d5de6ee. Requesting round 2.

[Copilot]

Copilot review overview

🟡 Changes recommended

Two critical URL-matching gaps can expose query secrets.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2

Open (2)

• Adjacent comma-separated URLs are merged, leaking the second secret · New
• Apostrophes truncate URL redaction and leak query suffixes · New

Resolved since last review (2)

• TUI error display bypasses credential redaction
• URL redaction misses uppercase and mixed-case schemes

[cliffhall]

Copilot round 2

• Adjacent comma-separated URLs are merged: fixed in 82adeee. The match stops at the next http(s)://; regression test added.
• Apostrophes truncate redaction: fixed in 82adeee. The apostrophe is included in the match and only a trailing one is peeled off; tests cover a mid-value apostrophe and a single-quoted URL.

npm run local:gate is green on 82adeee. Requesting round 3.

[Copilot]

Copilot review overview

🟡 Changes recommended

OAuth state can still leak, and embedded URL redaction can corrupt closing delimiters.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

• Redact OAuth state values from callback URLs · New

Resolved since last review (2)

• Apostrophes truncate URL redaction and leak query suffixes
• Adjacent comma-separated URLs are merged, leaking the second secret

[cliffhall]

Declined, with no change. state is excluded from SENSITIVE_BODY_FIELDS on purpose, and the core test for redactUrlQuery asserts that it survives.

• state is a single-use CSRF nonce bound to the client session that started the flow. It is not a credential: holding it grants nothing, and it is dead once the callback has been handled.
• CLI/TUI error output may not apply the same URL-redaction as the web client's OAuth timeout path #2423 asks the CLI for parity with the web client, and its own suggested fix is to reuse redactUrlQuery. That is exactly what this PR does.
• Adding state would change the redaction policy of the shared helper, and with it what the web client's Network log records. That is a cross-client policy decision beyond this issue, not a CLI defect.

[cliffhall]

Copilot round 3: review loop closed

• Redact OAuth state values: declined in-thread. state is a single-use CSRF nonce, not a credential, and core's shared redactUrlQuery excludes it deliberately (a core test asserts this). CLI/TUI error output may not apply the same URL-redaction as the web client's OAuth timeout path #2423 asks for parity with that helper, and changing its policy would also change the web Network log.
• The overview headline also mentions redaction "corrupting closing delimiters", but no finding or suppressed comment backs it. Closing ), ], ' and . are peeled off and put back, and each has a test.

The loop stops here: this round held only a finding declined as out of scope (pr-flow 7c).