Skip to content

refactor: unify request errors + add audit log middleware#327

Closed
Destynova2 wants to merge 5 commits intomainfrom
refactor/unified-error-and-audit-middleware-claude
Closed

refactor: unify request errors + add audit log middleware#327
Destynova2 wants to merge 5 commits intomainfrom
refactor/unified-error-and-audit-middleware-claude

Conversation

@Destynova2
Copy link
Copy Markdown
Contributor

Summary

  • Replaces the AppError + ProviderError split with a single RequestError enum that carries the upstream HTTP status verbatim (502/503/504) instead of flattening every provider failure to a generic 502 body. New variants: BadRequest, Unauthorized, Forbidden, NotFound, ParseError, RoutingError, RateLimited{provider, retry_after_ms}, ProviderUpstream{provider, status, body}, BudgetExceeded{limit_usd, actual_usd}, DlpBlocked, AuthRevoked, Internal(anyhow::Error).
  • RequestError::is_retryable() is the canonical retry classifier — removes three duplicate 429 detectors from dispatch/retry.rs and centralises the matcher.
  • Adds an Axum audit-log middleware (audit_log_layer) that emits AuditEvent::RequestProcessed for every request — including OAuth, config, and error paths that previously bypassed audit entirely. De-dupes via the AuditedAlready response-extension marker so the dispatch pipeline (which writes a richer DLP/risk/token-aware entry) is the single source of truth on the hot path. Closes the audit gap that allowed silent model enumeration via DLP probes (EU AI Act Article 6 / PCI DSS 3.4).

Originally targeted fix/preset-mod-include-str per the task brief; that branch was merged into main on 2026-04-27 and deleted, so the PR retargets main. The branch was created off the fix branch's last commit 9feae19.

Test plan

  • cargo check (default features, all features, no-default-features)
  • cargo clippy --tests --all-targets -- -D warnings
  • cargo fmt --check
  • cargo nextest run — 1309/1309 tests pass
  • 35 unit tests for RequestError::IntoResponse and is_retryable() covering every variant
  • 9 integration tests for audit middleware (status codes, AuditedAlready de-dup, provider header, error variant tag, risk levels, anonymous fallback)
  • 9 snapshot tests for error response JSON shapes

🤖 Generated with Claude Code

Clément LIARD and others added 5 commits April 28, 2026 22:41
CI re-runs the full test suite (incl. doctests) on every PR via the
.github/workflows/ci.yml tests job, so local pre-push duplication
adds ~20 min per push without catching anything new. Pre-push hooks
should be fast-fail; expensive checks belong on the CI server.

Closes audit finding: silent productivity tax (pre-push duplication).
Documents the three-state intent (true/false/absent) of ProviderConfig.is_enabled
and the dependency on deny_unknown_fields (added in the next commit) to
reject typos like enbaled = false at parse time. Behaviour is unchanged;
this is purely contractual clarity to support the silent-typo-killer audit.

Closes audit finding: silent typo killer on provider config.
Adds #[serde(deny_unknown_fields)] to AppConfig and the major
sub-structs (ProviderConfig, ModelConfig, TierConfig, RouterConfig,
ScoringConfig, CacheConfig, BudgetConfig, DlpConfig, SecurityConfig).

Without this guard, a typo like enbaled = false in a [[providers]]
block silently parses (the unknown key is dropped) and the provider
remains enabled with the wrong intent. With the guard, parsing fails
loudly and the operator gets an actionable error pointing at the
offending key.

Tested with the full nextest suite (1268 tests) plus all doctests:
no fixture, preset or example carries a stale field, so this is a
pure tightening with no migration cost.

Closes audit finding: silent typo killer on TOML config.
Each entry in DENIED_SECTIONS / DENIED_KEYS now carries a short
justification table covering why it can not be hot-reloaded — either
because the data is sensitive (credentials, DLP rules) or because the
consumer is constructed once at process start (TLS listener, secret
backend, TEE attestation, FIPS gate).

Adds tee, fips, server.tls and secrets.backend to the deny-list so
the documented "static-init" rationale matches actual behaviour. Also
emits an INFO log on every denied attempt telling the operator to
restart instead of expecting the silent reload to apply.

Adds two unit tests covering the new deny entries (tee/fips sections
and server.tls / secrets.backend keys) and asserts that sibling keys
in the same sections remain editable.

Closes audit finding: hot-reload UX (silent ignore of denied edits).
Replaces the `AppError` + `ProviderError` split with a single
`RequestError` enum that carries the upstream HTTP status verbatim
(502/503/504) instead of flattening every provider failure to a
generic 502 body. Introduces the canonical `RequestError::is_retryable`
classifier as the single source of truth for retry/backoff decisions
and removes three duplicate 429 detectors from `dispatch/retry.rs`.

Also adds an Axum audit-log middleware (`audit_log_layer`) that emits
an `AuditEvent::RequestProcessed` entry for every request lifecycle —
including OAuth, config, and error paths that previously bypassed
audit entirely. Uses an `AuditedAlready` response-extension marker so
the dispatch path (which writes a richer DLP/risk/token-aware entry)
is not double-logged. Closes the audit gap that allowed silent model
enumeration via DLP probes (EU AI Act Article 6 / PCI DSS 3.4).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Destynova2 Destynova2 enabled auto-merge April 28, 2026 21:02
@Destynova2
Copy link
Copy Markdown
Contributor Author

Superseded by a clean rebased version targeting current main. The original branch was based on a stale fix/preset-mod-include-str ancestor; the new PR rebases cleanly onto main and removes the unintended diff regressions.

@Destynova2 Destynova2 closed this Apr 28, 2026
auto-merge was automatically disabled April 28, 2026 21:04

Pull request was closed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant