fix(logger): redact sensitive data at log boundary

[ewen-poch]

Problem

The main-process JSONL logger retained rich error messages, stacks, provider data, and recursive
causes without a default credential-redaction boundary. Call-site diagnostics could therefore persist
authorization headers, tokens, cookies, credential-bearing URLs, or request/research bodies in
main.log and its rotated files.

Proposed change

• Enforce recursive redaction in the logger-owned serialization boundary so direct formatLine() and
  every normal logger call receive the same policy.
• Redact sensitive field-name variants, credential-shaped strings, body/payload fields, and oversized
  text while preserving error categories, status/codes, run IDs, and correlation identifiers.
• Mirror the same redacted representation to the console so it is not a less-safe alternate sink.
• Add deterministic invariant tests covering nested objects/arrays/causes, provider RequestError,
  console output, and live/rotated JSONL files.

Scope and non-goals

• Changes are limited to src/main/logger.ts and src/main/logger.test.ts.
• No dependency, architecture boundary, data model, migration, renderer contract, or UI change.
• Existing notebook/settings call-site sanitizers remain as defense in depth.
• This does not rewrite historical logs or add/change a support-bundle feature.
• Pattern detection cannot identify an arbitrary opaque secret that has neither a sensitive key nor a
  recognizable credential format.

Acceptance criteria and validation

• Recursive sensitive-field/string redaction and diagnostic preservation ->
  vitest run src/main/logger.test.ts src/main/diagnostics/startup.test.ts src/main/notebook/runtime-service-logging.integration.test.ts src/main/notebook/runtime-diagnostics.test.ts
  -> 70 passed after rebasing onto the latest origin/main.
• Node/main-process contracts -> tsc --noEmit -p tsconfig.node.json --composite false -> blocked by
  two existing argument-count errors in src/main/acp/claude-agent-acp-patch.test.ts (lines 65 and
  82); the same errors reproduce on unchanged main.
• Lint -> eslint --cache . -> 0 errors, 17 pre-existing warnings in unrelated files.
• Full portable suite outside the local-socket-restricted sandbox -> vitest run -> 11,284 passed,
  184 skipped, 3 failed. All three failures are in claude-agent-acp-patch.test.ts and reproduce on
  unchanged main.

The focused checks were rerun after the final rebase. Independent review is pending in this PR.

Review focus

• Sensitive-key matching versus observability fields such as tokenUsage and inputTokens.
• Credential-pattern coverage and false-positive behavior in error messages/stacks.
• The invariant that console output and every persisted/rotated JSONL line use the same redacted
  representation.

[ewen-poch]

Addressed the actionable review findings in 824418b:

• Credential delimiters: commas no longer terminate header or key/value credential matches, and regression cases verify complete comma-containing values are redacted.
• Nested Error diagnostics: the JSON replacer now applies the existing Error serializer recursively, preserving safe name, message, and stack fields in both JSONL and console output after redaction.

Validation: focused logger and diagnostic tests pass 70/70; ESLint reports no errors; the full npm test run passes 11,291 tests and only reproduces the three documented baseline failures in claude-agent-acp-patch.test.ts.

[github-actions]

Codex Review

Verdict: needs changes

[P1] Redact credentials in URL fragments and generic key parameters

src/main/logger.ts:191

Impact: URLs such as https://host/?key=secret or https://host/#access_token=secret pass through the logger with credentials intact, so they can reach both console and persisted logs.

Recommendation: Redact generic credential query parameters and sanitize or remove URL fragments before returning the URL.

Summary: Static review found a credential-redaction gap in URL handling.

[ewen-poch]

Addressed the URL credential finding in 2070dc0:

• A query parameter named key is now redacted only within URLs, avoiding over-redaction of ordinary structured log fields named key.
• URL fragments are removed at the logging boundary because arbitrary fragment payloads cannot be safely classified.
• Regression coverage includes both a generic key query parameter and an access-token fragment.

Validation: focused logger and diagnostic tests pass 70/70; targeted ESLint passes; the full npm test result remains 11,291 passed with only the three documented baseline failures in claude-agent-acp-patch.test.ts.

[ewen-poch]

Addressed the scheme-prefixed credential finding in 3398c6e:

• Key/value and CLI credential patterns now consume a recognized authentication scheme together with its following value, preventing the value from surviving after the scheme is replaced.
• Regression coverage includes both an Authorization assignment and an authorization CLI argument with a Bearer value.

Validation: focused logger and diagnostic tests pass 70/70; targeted ESLint passes; the full npm test result remains 11,291 passed with only the three documented baseline failures in claude-agent-acp-patch.test.ts.

[ewen-poch]

Addressed the malformed URL bypass in e91d98b:

• URL parsing now fails closed: any matched URL that cannot be parsed is replaced with the redaction marker instead of returning the raw text.
• Regression coverage uses a credential-bearing URL with an invalid port and verifies that neither userinfo component survives.

Validation: focused logger and diagnostic tests pass 70/70; targeted ESLint passes; the full npm test result remains 11,291 passed with only the three documented baseline failures in claude-agent-acp-patch.test.ts.

[github-actions]

Codex Review

Verdict: needs changes

[P1] Credential assignments with compound or lowercase names are not redacted

src/main/logger.ts:217

Impact: Strings such as providerApiKey=secret or openai_api_key=secret bypass the word-boundary pattern, and the uppercase-only fallback does not catch lowercase names, so the new file and console sinks can persist credentials.

Recommendation: Match credential assignments case-insensitively and support compound identifiers, then add regression coverage for these forms.

Summary: Static inspection found a credential-redaction bypass in log text handling.

[ewen-poch]

Addressed the compound and lowercase assignment finding in 04cf785:

• The generic assignment fallback now captures complete identifiers and reuses the existing sensitive-key classifier instead of maintaining a second case-sensitive suffix list.
• This covers camelCase, snake_case, lowercase, and uppercase credential names while preserving token metric assignments classified as non-sensitive.
• Regression coverage includes providerApiKey and openai_api_key assignments.

Validation: focused logger and diagnostic tests pass 70/70; targeted ESLint passes; the full npm test result remains 11,291 passed with only the three documented baseline failures in claude-agent-acp-patch.test.ts.

[github-actions]

Codex Review

Verdict: needs changes

[P1] Redact prefixed API-key assignments

src/main/logger.ts:158

Impact: Keys such as openai_api_key are not recognized as sensitive, so their values can remain in console and JSONL logs. The new tests explicitly cover this case.

Recommendation: Treat adjacent api and key words as sensitive regardless of provider prefixes.

Summary: Found one credential-redaction bypass in the changed logger.

[ewen-poch]

No code change is needed for this finding. The current generic assignment fallback captures the complete identifier and delegates classification to isSensitiveLogKey. For openai_api_key, the classifier normalizes the words to openaiapikey, which matches the existing apikey suffix; providerApiKey follows the same path. The committed regression cases exercise both forms and the focused suite passes, confirming that neither sentinel reaches the serialized log output. This appears to be a static-analysis false positive.

[github-actions]

Codex Review

Verdict: mergeable

No actionable findings.

Summary: No concrete merge-blocking defects found in the reviewed changes.

[ewen-poch]

Rebased the branch onto the latest main commit da77dd4 and updated it with force-with-lease. The logger diff is unchanged; focused logger and diagnostic tests still pass 70/70. This starts a clean CI workflow after the previous run twice reproduced two unrelated Linux-only failures outside the changed files.