Skip to content

[Security] Make unexpected application failures safe across API, persisted runs, MCP, and CLI #2351

Description

@Chris0Jeky

Problem

Taskdeck has good top-level protection: UnhandledExceptionMiddleware and ResultExtensions already return a generic body for unhandled or ordinary HTTP UnexpectedError failures. The remaining boundary is fragmented application catches that copy unknown Exception.Message into persisted run state, multi-status/batch receipts, provider health, MCP/CLI results, or non-standard response paths before the top-level mapper can protect them.

Confirmed examples include AgentRuntime.MarkFailed, OpsCliService command-run state, AutomationExecutorService batch receipts (#2281), provider-health details (#2213), database I/O failures mapped as public InvalidOperation, and several import/planner paths. The supplied CSV also contains benign logging and deliberate DomainException/validation messages, so it is an inventory input rather than a zero-false-positive gate.

Refs #2349, #2281, and #2213.

Direction

Build on the existing request correlation and HTTP error boundary. Classify expected domain/validation/provider failures explicitly; log unknown exceptions once with the exception object and the existing bounded request/trace reference; expose or persist only a stable error code, safe retry/fallback guidance, and that reference where appropriate.

Do not add a second unrelated correlation scheme or make UI behavior depend on prose.

Acceptance

  • Record a reviewed surface inventory: standard HTTP, multi-status/batch DTOs, persisted worker/agent/command state, MCP, CLI, SignalR, and provider-health responses.
  • Unknown exceptions cannot place paths, SQL/constraint details, provider internals, configuration, tokens, or raw exception text in any user-visible or persisted surface.
  • Full diagnostic exceptions remain in protected structured logs, correlated with the existing X-Request-Id/trace context, and are not double-logged by every layer.
  • Deliberate DomainException, validation, parse, conflict, quota, and safe provider failures retain useful stable messages.
  • A focused guard blocks new raw unknown-exception flows without flagging every log statement or known-domain catch.
  • Migration is split by surface/module; security: sanitize batch-execute errors and measure timing parity #2281 remains the owner for the batch-execute timing and receipt slice.

Verification

Use synthetic exceptions containing a secret-like token, Windows path, SQLite constraint, and provider URL. Prove the response/persisted record is safe and the protected log carries the same reference and full exception. Add regressions for a deliberate domain message and a parse/validation message.

Non-goals

No wholesale Result rewrite, no blanket replacement of every ex.Message, and no suppression of exceptions from protected logs.

Supplied patch disposition

0002-centralize-public-unexpected-errors-pilot.patch applies cleanly but is not merge-ready. It duplicates current correlation/error infrastructure, proves sanitization only with a NullLogger, and does not cover the non-standard surfaces that motivate this issue. Its database wording can inform a later small pilot.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Priority IIThe active direction's next tranche: wedge capabilities, significant defects, near-term hardening.backendPrimary implementation impact in .NET API/domain/application/infrastructure.hardeningReliability, safety, operability, and production-readiness improvements.securityAuthentication, authorization, data protection, abuse prevention, and compliance-related changes.testingTest coverage, harnesses, regression prevention, and verification pipeline work.

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions