Record the unknown-exception surface inventory and guard new raw flows

[Chris0Jeky]

Summary

Two acceptance bullets of #2351: the reviewed surface inventory, and a focused guard that blocks
new raw unknown-exception flows without flagging log statements or known-domain catches.

docs/security/UNKNOWN_EXCEPTION_SURFACE_INVENTORY.md records one row per surface — standard HTTP
(middleware + Result mapper), multi-status/batch receipts, persisted agent/command/capture/webhook
state, housekeeping workers, every file under backend/src/Taskdeck.Api/Mcp, the standalone CLI,
SignalR, provider health, and the circuit-breaker snapshot. Each row cites file:line at
a1ed795a7, names the sanitizing mechanism, carries a classification
(safe / deliberate-domain / open residual / owned-elsewhere), and names the regression test that
pins it. Every row was derived by reading the code, not from release notes.

scripts/check-unknown-exception-boundary.mjs is the guard, wired into the existing
docs-governance CI job.

Open residuals recorded by the inventory

These are recorded, not fixed, by this PR. None is silenced by the guard: each sits outside the two
regions the guard inspects, so no allowlist entry suppresses it.

• R1 — webhook delivery failure text is redacted, not generalized.
  OutboundWebhookDeliveryWorker.cs:245-248 persists Redact($"Webhook delivery threw {ex.GetType().Name}: {ex.Message}") into OutboundWebhookDelivery.LastErrorMessage. Redact is
  pattern-based, so any exception text that matches no rule in SensitiveDataRedactor's replacement
  rules is stored verbatim (capped at 1000 chars by OutboundWebhookDelivery.cs:146-154). Every other
  reviewed persisted surface generalizes instead.
• R2 — ProposalTools.Error(Result) diverges from SanitizeLlmFailureMessage.
  ProposalTools.cs:187-196 handles the UnexpectedError code correctly but returns
  result.ErrorMessage for every other code without calling Redact, unlike ReadTools.cs:154
  and WriteTools.cs:557, which route through SanitizeLlmFailureMessage.
• R3 — legacy persisted FailureReason values are replayed unsanitized. Recorded as the standing
  policy note from the #2432 / #2436 release comments: those PRs sanitized the write path for
  new failures, but rows written before them, and the read projections at ProposalResources.cs:135,
  ProposalTools.cs:71, Services/AutomationProposalService.cs:1591, AgentRuntime.cs:278,322 and
  Services/AgentRunService.cs:107,129, still surface stored FailureReason text verbatim. Historic
  rows are not backfilled.
• R4 — SignalR detailed errors are off by omission, not by assertion. SignalRRegistration.cs:20
  calls AddSignalR() with no options delegate and nothing in backend/src sets
  EnableDetailedErrors. The behaviour is correct today only because the framework default is
  false; no configuration key or test pins it, so a future options delegate could flip it silently.
• R5 — circuit-breaker snapshots hold raw exception text.
  CircuitBreakerStateTracker.LastFailureReason is populated from outcome.Exception?.Message
  (LlmProviderRegistration.cs:425,459, AuthenticationRegistration.cs:289). In-memory only, but it
  is read back into operator-visible state snapshots.
• R6 — two divergent generic strings. SensitiveDataRedactor.GenericUnexpectedFailureMessage
  ("Unexpected processing error. Check server logs with the correlation ID.") and the three private
  GenericUnexpectedErrorMessage constants ("An unexpected error occurred.") in
  UnhandledExceptionMiddleware.cs:11, AutomationExecutorService.cs:13 and
  BatchProposalExecutionService.cs:11 present two different strings for the same condition.
• R7 — CLI top-level exceptions print a stack trace. Taskdeck.Cli/Program.cs has no top-level
  try/catch on this base; fix in flight as Sanitize standalone CLI unexpected failures #2466.

Guard rules

Allowlist-based and deliberately narrow. Statements, not lines, are the unit of judgement, so a
multi-line return JsonSerializer.Serialize(new { ... }) is evaluated as one expression.

1. mcp-error-message — in backend/src/Taskdeck.Api/Mcp/*.cs, a statement in a return /
   throw / Error(...) / JsonSerializer.Serialize(...) position that references .ErrorMessage
   must also carry a sanitizer token (SanitizeLlmFailureMessage, GenericUnexpectedFailureMessage,
   PublicFailureMessage, SummarizeException, or any SensitiveDataRedactor. call).
2. persisted-unknown-failure — in PERSISTED_STATE_FILES (AgentRuntime.cs,
   OpsCliService.cs), a statement inside a catch (Exception <var>) block may not reference
   <var>.Message, <var>.StackTrace or <var>.ToString() unless it is a logging call or carries a
   sanitizer token. catch (DomainException ex) is never matched — the catch filter is what makes
   those messages curated — and _logger / Log* calls are never flagged.
3. Everything else is out of scope. One allowlist entry covers CaptureResources.cs
errorMessage = c.ErrorMessage (Sanitize MCP capture and board resource failures #2443), safe because the write side sanitizes
   (LlmQueueToProposalWorker.cs:698, TranscriptTriageWorker.cs:347). A test enforces that every
   allowlist entry states a path, a pattern, an issue and a reason. A new persisted-state surface
   joins the guard by being added to PERSISTED_STATE_FILES in the PR that introduces it.

Part of #2351. This PR intentionally does not close the umbrella issue.

Verification

All run in the worktree at head 3ef3f3fbf.

• node --test scripts/check-unknown-exception-boundary.test.mjs — 17 tests, 17 pass, 0 fail.
  Covers a rejected synthetic snippet per rule (raw Error(result.ErrorMessage), raw
  new { error = ... }, raw interpolation into a thrown MCP exception, ex.Message persisted,
  ex.ToString() assigned, exception text interpolated into a returned failure), the sanitized
  counterpart of each, a log statement it must not flag, a catch (DomainException ex) block it must
  not flag, a nested save-failure logging catch, the allowlist shape, and a real-tree scan.
• node scripts/check-unknown-exception-boundary.mjs — passed, 0 findings on the real tree.
• node scripts/check-docs-governance.mjs — passed.
• node scripts/check-golden-principles.mjs — passed.
• node --test scripts/ci/smart-ci/*.test.mjs — 91 tests, 91 pass, 0 fail (workflow touched).
• PyYAML parse of .github/workflows/reusable-docs-governance.yml — parses, 10 steps, the new
  Validate unknown-exception boundary invariants step present.
• git diff --check — clean.

ci/policy.v1.json already routes scripts/check-*.mjs and scripts/check-*.test.mjs to the
docs-governance lane (governance-scripts, R2), so no policy change was needed. The workflow
change adds a run: step only, so the action-pins contract test is unaffected.

Documentation

• New: docs/security/UNKNOWN_EXCEPTION_SURFACE_INVENTORY.md.
• docs/security/SECURITY_LOGGING_REDACTION.md — Verification section gains the two guard commands
  and a pointer to the inventory (append-only; no existing text changed).
• No docs/STATUS.md, masterplan or ADR change: this PR adds documentation and a CI check, and
  changes no shipped runtime behaviour.

Not verified

• No hosted CI run yet at the time of writing. Per .claude/rules/ci-control.md the CI-control
  change is R4-class and hosted-only — the local PyYAML parse and the smart-ci planner tests are
  additive, not the proving check. The docs-governance job result on this head is the real proof.
• No backend build or dotnet test run. No C# was changed; the inventory's cited line numbers and
  test names were read from source and from backend/tests, not re-executed.
• The guard is a text/statement matcher, not a Roslyn analyzer. It will not catch an unknown-exception
  message laundered through an intermediate variable across statements, or through a helper in a file
  outside the two guarded regions.
• Residuals R1-R7 are recorded only. None was fixed, reproduced at runtime, or filed as its own issue
  by this PR.

[chatgpt-codex-connector]

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.

[Chris0Jeky]

Review round 1 — dispositions

New head: f777c63e7ae7c60de670ecf98f7eb26cdba0eb9e (was 3ef3f3fbf). One fix commit, one push.

1. MEDIUM — statement-wide isSanitized excused a raw sibling member. FIXED.
Rule 1 now judges each .ErrorMessage occurrence individually. enclosingCallees() walks outward
from the occurrence collecting the callee of every enclosing call, and the occurrence is accepted
only when one of them is an actual sanitizing call (SanitizeLlmFailureMessage, Redact,
PublicFailureMessage, SummarizeException, SafeExceptionDescription) — being co-located with a
sanitized sibling no longer helps. The one named exception is the reviewed guarded ternary in
ProposalTools.cs:187-196, accepted only when the statement carries both ErrorCodes.UnexpectedError
and GenericUnexpectedFailureMessage and the occurrence sits in a ternary arm.
Tests added: the reviewer's new { error = PublicFailureMessage(result), detail = result.ErrorMessage }
is rejected; the variant with both members wrapped is accepted.

2. MEDIUM — statement-wide allowlist matching. FIXED.
Allowlist entries changed from pattern (matched against the whole statement) to line, matched
against the exact source line carrying the occurrence. A new raw member added inside the already
allowlisted 14-line Serialize statement at CaptureResources.cs:83-96 is now flagged.
Test added: the allowlisted errorMessage = c.ErrorMessage line passes while a second raw member on
its own line in the same statement is reported. The allowlist-shape test now also asserts each entry
keys on a single line, never a statement.

3. MEDIUM — header overclaimed; catch (Exception ex) { return Error(ex.Message); } fired nothing. FIXED.
Rule 2 now runs over backend/src/Taskdeck.Api/Mcp/*.cs as well as the persisted-state files, under
rule id mcp-unknown-exception-text, with the same logging exemption.
Checked on the real tree as asked: the four existing bare catches are exempt on their merits, not
by allowlist — McpOperationLogger.cs:155,202,249 contain only _logger.LogDebug(ex, ...), and
McpTelemetryMiddleware.cs:112 reports LogSanitizer.SafeExceptionDescription(ex) and
ex.GetType().Name, never ex.Message. A repo-wide grep for ex.Message / ex.ToString() /
ex.StackTrace under Mcp/ returns nothing. No new allowlist entry was added and no residual was
silenced. Header comment rewritten to describe both rules as they actually behave.

Also folded in while touching the masking code: literal masking now preserves interpolation holes, so
a raw value inside $"...{x}" is analysed rather than blanked. Tests cover a raw ex.Message
interpolated into an MCP resource throw.

LOW, declined as directed — no change. maskLiterals edge cases with a trailing escaped
backslash and verbatim @"..." strings (neither shape occurs in the guarded files today); inventory
line numbers pinned to a1ed795a7 (the document states that pinning explicitly).

Verification at f777c63e7

• node --test scripts/check-unknown-exception-boundary.test.mjs — 24 tests, 24 pass, 0 fail (was 17).
• node scripts/check-unknown-exception-boundary.mjs — passed, 0 findings on the real tree with
  the widened rule 2 and the per-occurrence rule 1.
• node scripts/check-docs-governance.mjs — passed.
• node scripts/check-golden-principles.mjs — passed.
• git diff --check — clean.

docs/security/UNKNOWN_EXCEPTION_SURFACE_INVENTORY.md's Guard section was updated to match all three
rule changes.

Not verified: no hosted CI result for this head yet; no backend build or dotnet test (no C# changed).

[Chris0Jeky]

Fresh-context independent review, round 1 at 3ef3f3f, with a scoped verification pass on the fix diff 3ef3f3f..f777c63.

Verdict after the fix round: no CRITICAL or HIGH finding. Merge-blocking: none. Round count: one review round, one fix round.

Round 1 findings and disposition (all three MEDIUM, all fixed):

• Rule 1 judged sanitization statement-wide, so one sanitized member excused a raw sibling; it now walks each .ErrorMessage occurrence outward through its enclosing callees and accepts only a real sanitizer wrapping that occurrence.
• The allowlist was statement-wide, so a new raw member inside an allowlisted statement would have been excused; entries are now keyed to path plus the exact source line.
• Bare catch (Exception ex) { return Error(ex.Message); } in an MCP tool fired nothing; rule 2 now scans backend/src/Taskdeck.Api/Mcp/*.cs with the same logging exemption, and the header describes both rules accurately. The four existing bare MCP catches are exempt on their merits (logging only, or LogSanitizer with the type name), not by allowlist.

Fix-diff verification (by trace): the four adversarial shapes behave as intended (unsanitized wrapper flagged, concatenation flagged, interpolated sanitizer call accepted, half-guarded ternary flagged); interpolation-hole preservation does not break statement splitting; the widened rule 2 flags the reviewer's snippet and exempts the telemetry catches on their merits. Worker-run proof at f777c63: guard tests 24/24, real tree 0 findings with no new allowlist entry, docs governance and golden principles pass.

LOW, tracked in a follow-up issue rather than fixed here: the guarded-ternary branch is dead against the real tree and would also excuse null-conditional or named-argument shapes if it fired; rule 2's sanitized check is still statement-level and now includes LogSanitizer, so a concatenation with a sanitized prefix inside a catch is excused; a value laundered through an intermediate variable is invisible to rule 1 (stated in the doc).

Remaining gate: exact-head hosted Docs Governance green (the workflow change is R4-class, hosted-only) plus the rest of ci-required, and the three-minute aging floor, then a merge commit. Part of #2351; the umbrella stays open.

[Chris0Jeky]

Current-base refresh at exact head 7918617: merged origin/main once (PRs #2466, #2471, #2474, #2416 landed). The only conflict was the Verification section of docs/security/SECURITY_LOGGING_REDACTION.md, where #2471's proof block and this PR's guard block were appended at the same point; both are kept with balanced fences. No logic changed. Post-refresh proof: guard tests 24/24, real-tree guard 0 findings on the new main (which now includes the generalized webhook worker and the CLI boundary), docs governance and diff check clean. Remaining gate: exact-head hosted CI and the aging floor.