[scottwofford]

Implements Trello: COE audit passthrough fallback (P1 follow-up from the 2026-03-25 COE audit of PR #204). Rebased on main after v4.0.0.

Summary

Opt-in passthrough fallback: when a policy-modified request is rejected upstream with a request-shaped 4xx, the gateway retries once with the original, pre-policy request. Design principle from the card: the proxy should never make things worse than direct API access, with the constraint that fail-open is acceptable pre-users only if it is opt-in, logged, and never undoes a redaction.

This is the narrow slice of PR #204 that survived its review: on failure it removes the policy's own modifications rather than adding new ones. No sanitization, no pattern-matched auto-fixes, no error rewriting.

Safety design: double opt-in, fail-closed by default

Adversarial review found that a global flag alone let a client undo request-side safety policies: craft input so the redaction leaves an invalid request (e.g. an empty text block after a secret is stripped), get a 400, and the fallback then sends the unredacted original upstream. A model-restriction policy could likewise be undone by a 404.

The fallback now requires both:

1. PASSTHROUGH_FALLBACK_ENABLED on (default off; runtime-settable via the admin config API by anyone with the admin key), and
2. The active policy declares passthrough_fallback_safe = True (new ClassVar on BasePolicy, default False). MultiSerialPolicy allows fallback only if every sub-policy opts in, so one redactor in a chain vetoes it. Policies that don't derive from BasePolicy can't declare safety and are treated as fail-closed.

Why a per-policy declaration rather than detecting "content was removed" at runtime: any request edit can be safety-relevant (a model swap removes nothing but restricts access), so a structural diff can't tell safety edits from cosmetic ones. Only the policy author knows, so the policy declares it, and the default is the safe answer.

What opts in today: only DogfoodSafetyPolicy, which never edits requests; it opts in so dogfood mode (which auto-composes it into every chain) doesn't silently veto an otherwise opted-in chain. No shipped request-rewriting policy opts in (StringReplacementPolicy with apply_to: request is exactly how a redactor is built, and SimplePolicy subclasses are user-defined). So in practice this PR ships the mechanism plus a fail-closed default; a policy whose request edits are harmless compatibility tweaks can opt in with one line.

Trigger condition (all must hold)

1. Flag on AND active policy opted in (above)
2. Upstream AnthropicStatusError with status in {400, 404, 413, 422}. Excluded: 401/403 (credential-scoped), 429 (retry amplifies load), 5xx/529 (SDK already retries)
3. The request sent differs from a deepcopy snapshot of the request as it entered the policy
4. Streaming only: zero backend events received yet (no mid-stream recovery)

Exactly one retry. If the original also fails, that error propagates unchanged. Response-side policy behavior (response rewrites, blocks) still runs on the fallback response; only request-side edits are discarded.

Audit record now reflects what was actually sent

Previously transaction.request_recorded named the policy-modified request as final_request even when upstream received and accepted the original. Now, while the fallback is armed (allowed + request modified), that one-per-transaction record is deferred until the first attempt's outcome is known:

• Fallback fired: final_request / final_model = the original (what upstream accepted), plus a passthrough_fallback block: {status_code, error_message, rejected_request}
• No fallback (success, or non-eligible error): final_request = the modified request, as before
• Streaming: the deferred record is written when the first event arrives, before it's handed on

Unchanged default path: when the fallback isn't armed, recording happens before the send exactly as on main. Both attempts still appear as pipeline.backend_request events, the request log's outbound body is the resent original, and the pipeline.passthrough_fallback event, WARNING log, and luthien.passthrough_fallback span attribute are all still emitted before the retry.

Interaction with #798 (known-bad request pattern regressions)

Those tests pin transparency: known-bad client bodies pass through verbatim and upstream 400s are relayed as clean invalid_request_error. Unaffected: the fallback only fires when a policy changed the request, and it's off by default. test_known_bad_request_patterns.py passes on the rebased branch.

Judgment call flagged for review

The "original" snapshot is taken after the INJECT_POLICY_CONTEXT system-prompt injection, so the fallback removes what the active policy changed, not the injection (unchanged from the first revision of this PR).

Files changed

File	Change
src/luthien_proxy/policy_core/base_policy.py	passthrough_fallback_safe ClassVar (default False) + allows_passthrough_fallback()
src/luthien_proxy/policies/multi_serial_policy.py	Chain allows fallback only if every sub-policy does
src/luthien_proxy/policies/dogfood_safety_policy.py	Opts in (never edits requests)
src/luthien_proxy/pipeline/anthropic_processor.py	Fallback in _AnthropicPolicyIO.complete()/stream(), policy gate _passthrough_fallback_allowed, deferred/truthful transaction record
src/luthien_proxy/config_fields.py, settings.py, .env.example	passthrough_fallback_enabled field (db-settable, no restart); description updated
tests/.../pipeline/test_anthropic_processor.py	28 fallback tests (16 original + 5 audit-record + 7 policy-gate, incl. the redaction attack)
tests/.../e2e_tests/test_mock_passthrough_fallback.py	Now pins fail-closed behavior end to end: flag on + request-rewriting policy + 400 (non-streaming and streaming) propagates with exactly one backend request; default-off unchanged
changelog.d/passthrough-fallback.md	Accurate wording (response-side blocks only; runtime toggle by admin-key holders)
dev/context/decisions.md	Decision record

Test plan

• Unit: 2914 passed (full tests/luthien_proxy/unit_tests), including:
  • redaction-style modification + 400 with flag ON: one upstream call, secret never sent (test_redaction_plus_400_never_resends_secret)
  • opted-in policy + 400: falls back to original (test_opted_in_policy_plus_400_falls_back)
  • gate: flag off, default policy, non-BasePolicy, mixed chain all fail-closed; all-opted-in chain allowed
  • audit: after fallback exactly one transaction.request_recorded with final_request = original + passthrough_fallback block (non-streaming and streaming); armed-without-fallback records the modified request once
• Mock e2e (./scripts/run_e2e.sh mock): 211 passed
• ./scripts/dev_checks.sh passes (format, lint, pyright, unit tests)
• Not covered end to end: the positive fallback path, since no shipped request-rewriting policy opts in (covered by unit tests)

🤖 Generated with Claude Code

[github-actions]

CHANGELOG reminder — This PR has no changelog fragment.

Add a file to changelog.d/ (see changelog.d/README.md for format).
If this is a chore/infra-only change, add the skip-changelog or chore label to suppress this reminder.

[claude]

Review Status: No Changes to Review

This draft PR currently contains only the objective-setting empty commit (chore: set objective to implement opt-in passthrough fallback for policy 4xx). There are 0 files changed, 0 additions, and 0 deletions, so there is nothing to substantively review yet.

Happy to review once implementation lands. A few things I'll be watching for when the code arrives, given the objective ("opt-in passthrough fallback when a policy modification causes an upstream 4xx"):

Correctness / semantics

• Attribution of the 4xx: how will the code decide the 4xx was caused by the policy modification vs. a legitimate client-side error the upstream would have returned regardless? Blindly retrying every 4xx with the unmodified request risks:
  • Bypassing the policy for prompts that were legitimately 400-worthy (masking real client bugs).
  • Turning the policy into an advisory layer instead of an enforcement layer — the whole point of a blocking policy is that it can refuse.
• Which 4xx codes are eligible? 400 (schema drift from policy edits) is the plausible one. 401/403 (auth) and 429 (rate limit) should never trigger passthrough. Please explicitly enumerate.
• Streaming: 4xx can arrive after event: message_start in some SSE flows. Make sure the fallback path handles both "4xx on the HTTP response" and "error event mid-stream" cases, and doesn't emit a partial event stream followed by a fresh one.
• Idempotency / side effects: if the policy already emitted observability events, ran judge calls, or logged to request_logs before the upstream 4xx, the fallback retry needs a clear story about duplication (retry a second request row? amend the first? emit a fallback_triggered event?).

Opt-in surface

• Where does the opt-in live — per-policy config, per-request header, or a gateway-wide config field? Per-policy config is the most auditable; header-based opt-in is easy to abuse. If it's a config field, it belongs in config_fields.py (see CLAUDE.md "Configuration System") and needs a matching entry in the generated settings + .env.example.
• Default should be off. Users have to explicitly acknowledge they're trading enforcement for availability.

Security

• Passthrough of the original unmodified request is exactly the failure mode a red-team would want. Please make sure this can't be triggered by an attacker who can influence the upstream 4xx (e.g. by crafting a prompt that reliably produces a 400 → policy silently drops out).
• Log every fallback event loudly (structured event, dashboard-visible). Silent fallback is a security-critical misconfiguration waiting to happen.

Tests (per CLAUDE.md "Test Requirements")

• Unit tests for the fallback decision logic (which 4xx codes, which conditions).
• Integration test in sqlite_e2e or mock_e2e tier that:
  • Confirms policy modification → 4xx → unmodified retry → success path.
  • Confirms 4xx unrelated to policy (auth, rate limit) does not trigger fallback.
  • Confirms opt-in-off → 4xx surfaces to client unchanged.
• Streaming variant of the above.

Docs / changelog

• changelog.d/ fragment.
• Note the security tradeoff prominently wherever the opt-in is documented — this is an availability-vs-enforcement knob and users need to understand what they're turning off.

Ping me once there's a diff and I'll do a real review.

[claude]

Code Review: Passthrough Fallback

Overall this is a clean, focused PR that lands a well-scoped follow-up to the PR #204 audit. The trigger predicate is conservative in the right ways, the deep-copy snapshot is the right shape for the in-place-mutation case, and the observability + test coverage are solid. Findings below are minor.

Correctness observations

1. transaction.request_recorded still describes the modified request even when fallback sent the original.
ensure_request_recorded is one-shot (guarded by self._request_recorded), so on the fallback path only the first call fires — with final_request set to the modified request. The second _record_backend_request(fallback_request) in the fallback branch only emits pipeline.backend_request and the request-log row. Any consumer reading only the transaction.request_recorded event will believe the modified request reached the backend, when in fact the fallback body was sent. The ground truth is recoverable by cross-referencing the pipeline.passthrough_fallback and second pipeline.backend_request events, but that's a load-bearing correlation. Consider either:

• adding the fallback request into the pipeline.passthrough_fallback event payload (so it's self-contained), or
• documenting the correlation contract in the fallback event's summary.

Not blocking — but worth being explicit about for downstream observability.

2. Pre-existing in-place-mutation risk on _initial_request (out of scope, flagging for context).
self._initial_request = initial_request in __init__ doesn't copy, so if a policy mutates the request dict in place, dict(self._initial_request) inside ensure_request_recorded sees the mutated payload — meaning original_request == final_request in the emitted transaction event. This PR is unaffected (the fallback deepcopy is a separate protected snapshot) but the transaction event's usefulness for detecting policy modifications is quietly weakened. Might be worth a follow-up ticket to always deepcopy _initial_request at construction, mirroring what this PR does for the fallback snapshot.

3. Streaming: except AnthropicStatusError inside the yield-loop could theoretically catch a caller-athrown exception.

async for mse in _iterate(final_request):
    events_yielded += 1
    yield mse

If a downstream consumer of _stream() calls .athrow(AnthropicStatusError(...)) at the yield point, it would enter the except block. In practice events_yielded > 0 at that point and the if events_yielded: raise short-circuits, so this is safe. But it's the sort of coincidence that's worth a one-line comment ("events_yielded > 0 check also prevents re-entering fallback if the consumer athrows this exception type") because a future edit that moves the counter increment could silently break the invariant.

Nits

• str(exc.message) in _record_passthrough_fallback is redundant — APIStatusError.message is already a str. Not worth blocking.
• if sent_request == original is deep dict equality, which is O(n) over the payload. Fine here because it's only on the error path, but noting it in case the predicate ever moves to a hot path.
• The comment above _PASSTHROUGH_FALLBACK_STATUS_CODES is excellent — reads exactly like a design doc's rationale section and future editors will thank you.

Test coverage

Comprehensive. The parametrized test_non_request_shaped_errors_never_fall_back [401, 403, 429, 500, 529] closes the door cleanly on the excluded status codes, test_in_place_policy_mutation_is_detected pins down the whole reason the snapshot is a deepcopy rather than a dict(), and test_intentional_response_block_is_untouched + test_policy_raised_error_is_not_a_fallback_trigger are exactly the right pair to nail the "structurally cannot override a block" claim from the PR body.

One suggestion: add a test asserting the pipeline.backend_request event count is 2 on fallback and 1 without it. test_modified_request_400_falls_back_to_original currently asserts client.complete.call_count == 2 (the wire-level effect) but not the event-level count — and future changes to _record_backend_request's emission behavior wouldn't get caught by the existing test.

Design principle

Falling back inside _AnthropicPolicyIO.complete()/stream() at the backend-call site is exactly the right layer — high enough to see the modified vs. original request pair, low enough that intentional policy blocks (which surface as response rewrites or hook-raised errors, not upstream AnthropicStatusError) structurally cannot trigger it. The PR body's argument for this is convincing and the tests back it up. The default-off decision with the fail-open tradeoff spelled out in the field description is the right call for an AI-control proxy.

Style / conventions

• Config field follows the established ConfigFieldMeta pattern with db_settable=True, restart_required=False for the live-toggle path via the admin dashboard — matches dogfood_mode, inject_policy_context.
• Docstrings are appropriately Google-style; the WHY-focused comments on _PASSTHROUGH_FALLBACK_STATUS_CODES and the deepcopy snapshot follow the CLAUDE.md convention nicely.
• Changelog fragment present, .env.example regenerated. Good hygiene.

LGTM with the observability note above worth considering before merge. Nothing here is blocking.

[scottwofford]

---

Post-review note (adversarial verification pass, Jul 7): this PR and its sibling both add a one-shot upstream retry to the same two methods in anthropic_processor.py (#797 passthrough-fallback, #799 retry-with-fix). git merge-tree off the shared base shows 4 real conflict hunks, and merged naively both retries can fire on the same 400. Merge ONE first, then rebase the other and reconcile the retry semantics (order: repair-then-fallback, or unify into one retry ladder) before merging the second.

[scottwofford]

Claude-generated merge-queue triage of all open Luthien PRs, requested by Scott (Jul 7, 2026). Advisory only; Scott has not yet acted on these recommendations.

Recommendation: merge, after #799 is closed (see the comment there).

Verified in the diff: PASSTHROUGH_FALLBACK_ENABLED defaults to false, the request snapshot is only taken when the flag is on (a true no-op stays a no-op, which was the objection that soft-closed the original card), the trigger set is conservative (policy-actually-modified, 400/404/413/422 only, streaming pre-first-event only, exactly one retry), and the fallback is recorded before retrying so policy failures are not masked. Default-off means zero change for existing deployments; it exists as an opt-in escape hatch for when a policy modification breaks a request. To be clear, this is new scope rather than a bug fix, but it is the defensible sibling of the #797/#799 pair.

[scottwofford]

Pushed fixes for the adversarial review findings (rebased on main after v4.0.0 first; clean rebase):

1. Fallback could undo redaction / request-side safety edits. Now double opt-in: the flag AND the active policy must declare passthrough_fallback_safe = True (BasePolicy default False, fail-closed; chains need every sub-policy to opt in). No shipped request-rewriting policy opts in, so the redaction-then-400 attack and the model-restriction-then-404 case both propagate the error instead of resending the original. New unit test reproduces the attack; e2e tests now pin fail-closed behavior.
2. Audit record was wrong. transaction.request_recorded is deferred while the fallback is armed, so final_request is always what upstream actually accepted, with a passthrough_fallback block (status, message, rejected request) when it fires.
3. Changelog now says only response-side blocks are untouched, and that admin-key holders can toggle the flag at runtime. Flag still defaults off.
4. test: regression suite for known-bad API request patterns from prior COEs #798 known-bad pattern tests pass on the rebased branch; the fallback only fires on policy-modified requests, so it doesn't affect their transparency contract.

Checks: dev_checks.sh green, 2914 unit tests, mock e2e 211 passed. PR description is updated with the full safety design. Not merging; ready for review.

🤖 Generated with Claude Code

[claude]

Review

Overall this is a carefully-scoped PR with a strong security design and excellent test coverage. The double opt-in (PASSTHROUGH_FALLBACK_ENABLED + per-policy passthrough_fallback_safe) is well-justified in the code and docstrings, and the redaction-attack test (test_redaction_plus_400_never_resends_secret) directly pins the invariant the review of PR #204 called out. The deferred transaction.request_recorded design is a nice cleanup of the pre-existing audit-record confusion. Approvable as-is; the notes below are minor.

Correctness / minor concerns

1. Pre-existing _initial_request aliasing overlaps with your audit-record fix
   src/luthien_proxy/pipeline/anthropic_processor.py:128 — self._initial_request = initial_request is aliased with self._request. If a policy mutates the request in place (which the codebase supports and your test_in_place_policy_mutation_is_detected explicitly relies on), then dict(self._initial_request) at anthropic_processor.py:181 records the mutated state as original_request in the audit. So after a fallback fires you can end up with original_request = mutated and final_request = pristine original, which reads inverted. Not introduced by this PR, but since you already deepcopy for _fallback_original_request and are actively fixing this same audit event, worth considering whether _initial_request should also be a deepcopy (or reuse the fallback snapshot) when fallback is armed.

2. Empty MultiSerialPolicy chain vacuously allows fallback
   src/luthien_proxy/policies/multi_serial_policy.py:116 — all(p.allows_passthrough_fallback() for p in self._sub_policies) returns True on an empty tuple, contradicting the docstring's "every sub-policy must opt in". In practice an empty chain leaves the request unchanged so _fallback_armed short-circuits and nothing fires — but if some future path composes an empty chain and then mutates the request downstream, this is a fail-open corner. Consider return bool(self._sub_policies) and all(...) for defense in depth, or note the vacuous case in the docstring.

3. Dead code preserved in the extracted _iterate
src/luthien_proxy/pipeline/anthropic_processor.py:340-341 — if self._buffer_raw_events: self._raw_backend_events.append(mse) is unreachable inside _iterate. _iterate is only called from _stream() (streaming path), but self._buffer_raw_events = not is_streaming (line 155), so it's always False here. Pre-existing, but since you just refactored this loop into _iterate, it's a good moment to drop the branch (or move buffering inline where it's actually reachable).

4. final_request when the fallback retry itself fails
   _record_passthrough_fallback writes transaction.request_recorded before the retry runs, so if the second attempt also 4xxs, the audit still names the original as final_request even though upstream accepted neither request. That's consistent with the PR's stated "first attempt's outcome is known" framing and is fine because pipeline.passthrough_fallback and both pipeline.backend_request events are present — but if consumers of transaction.request_recorded treat final_request as "what was successfully sent", this may surprise them. Worth a brief note somewhere, or consider recording after retry completion.

Nice touches worth calling out

• Snapshot only when armed (copy.deepcopy(initial_request) if passthrough_fallback_enabled else None) — no cost on the default no-op path. Test test_snapshot_only_taken_when_enabled pins this.
• The gate lives at the backend-call site, so response-side blocks and policy-raised errors can never surface as an AnthropicStatusError here and therefore cannot trigger fallback. test_intentional_response_block_is_untouched and test_policy_raised_error_is_not_a_fallback_trigger make that explicit.
• Streaming: events_yielded == 0 gate correctly prevents mid-stream recovery, and the fallback_fired sentinel makes the deferred-record logic in finally provably one-shot.
• _PASSTHROUGH_FALLBACK_STATUS_CODES = {400, 404, 413, 422} with an inline comment explaining why 401/403/429/5xx are deliberately excluded — future-you will appreciate that comment.

Test coverage

Very thorough. Unit tests cover the request-shape / non-request-shape split, streaming connect vs. mid-stream, in-place mutation, response blocks, policy-raised errors, and the full audit-record matrix (fired / armed-but-no-fallback / armed-success, non-streaming and streaming). The policy-gate class exercises the redaction-attack path end to end. The one gap you already flag — no e2e positive fallback path because no shipped request-rewriting policy opts in — is unavoidable given the design and is documented in the PR description.

Style / nits

• src/luthien_proxy/config_fields.py:79 — the passthrough_fallback_enabled description is a long single sentence; it renders in the admin UI and .env.example. Consider tightening or splitting.
• tests/luthien_proxy/unit_tests/pipeline/test_anthropic_processor.py — import copy as _copy inside _make_io could move to module top-level (copy isn't currently imported there).

🤖 Generated with Claude Code

[claude]

Review: opt-in passthrough fallback (PR #797)

Overall this reads as a careful, well-scoped implementation of the P1 follow-up from the PR #204 COE. The double opt-in gate, the fail-closed default, and the invariant test that enforces "opted-in policies never modify requests" together make the security posture much stronger than a global flag would give you. The audit-record change (deferred transaction.request_recorded so final_request names what actually went upstream) is also a nice correctness win independent of the feature.

Below is what I noticed on read-through. Nothing in the must-fix column — a couple of small suggestions and one clarification worth calling out.

Correctness

• Non-streaming path (complete, anthropic_processor.py:295-325). The nested try/except/finally inside the span is easy to get wrong; I walked through it and I think it's right in all four combinations of (armed × fallback-eligible-error). In particular:

  • Armed + fallback fires: _record_passthrough_fallback calls _record_backend_request(fallback_request) which flips _request_recorded=True with the passthrough_fallback block, and the outer finally's ensure_request_recorded(final_request) correctly no-ops.
  • Armed + non-eligible error (e.g. 429): the finally records final_request = modified request, no passthrough_fallback block. Matches test_armed_but_no_fallback_records_modified_request.
  • Armed + retry also fails: the first attempt's fallback record was already emitted, then the second exception propagates; the finally no-ops. Matches test_fallback_retry_failure_propagates.
  • Not armed: defer_transaction_record=False, record is emitted synchronously; finally no-ops.

• Streaming path (stream, anthropic_processor.py:327-383). The events_yielded gate against mid-stream fallback is the right call. One subtle detail worth confirming in your head: an AnthropicStatusError raised from _iterate(fallback_request) is inside the except AnthropicStatusError block, so Python won't re-catch it — it propagates unchanged, which is the documented "retry fails, client sees direct-API behavior" contract. Good.

• _fallback_armed equality check (anthropic_processor.py:194-203). sent_request != original is a dict-equality comparison against a deepcopy snapshot. That's what you want (data equality, not identity), and it survives in-place mutation because the snapshot is independent. test_in_place_policy_mutation_is_detected pins this. Nice.

Suggestions (non-blocking)

1. extra_headers are not snapshotted alongside the request (anthropic_processor.py:136). The fallback resends the pristine request body but with the current extra_headers. If a policy ever mutates extra_headers (today no shipped policy does, but the protocol doesn't forbid it), the fallback would send the mutated headers with the original body — which is not "no worse than direct API access." Consider either (a) snapshotting extra_headers too when armed, or (b) documenting on AnthropicPolicyIOProtocol that extra_headers are not part of the fallback scope. Given no policy touches them today, this is a low-priority hardening item, but the invariant test doesn't guard it.

2. _passthrough_fallback_info stores a mutable dict reference (anthropic_processor.py:288-292). rejected_request is dict(rejected_request) — a shallow copy — and the whole info dict is embedded by reference into the transaction payload at record time. If anything downstream mutates the request after recording, the audit event held in memory (or replayed for debug UIs) could drift. In practice the emitter serializes immediately, so this is theoretical, but a copy.deepcopy(rejected_request) here would make the audit trail's immutability match its intent. Minor.

3. The judgment call flagged in the PR description — the snapshot is taken after the INJECT_POLICY_CONTEXT system-prompt injection — is worth surfacing in the config field's description too, not just the PR body. Someone reading .env.example or the config dashboard will assume "original unmodified request" means byte-for-byte what the client sent. Adding a one-line "note: policy-context injection is preserved on fallback" to the field's description in config_fields.py would prevent that confusion.

4. _ALLOWLIST in the invariant test is empty and unused. Nice defensive posture, but consider adding a comment on it explaining that any future addition should require a linked ADR / review — otherwise "why is this empty set here?" will invite someone to just append a name to make a test pass.

Test coverage

Excellent. 28 unit tests covering: happy path, disabled path, unmodified-request short-circuit, non-request-shaped statuses (parametrized), retry-failure propagation, in-place mutation detection, streaming connect vs. mid-stream, intentional response blocks and policy-raised errors, snapshot-not-taken-when-disabled, and the full audit-record matrix (fallback fired / armed-but-no-fallback / armed-success / streaming variants). The test_redaction_plus_400_never_resends_secret case is exactly the attack the design is protecting against and it's good to have it pinned.

The test_passthrough_fallback_invariant.py guard is the piece I like most — it makes the safety property a code-level invariant instead of prose. test_request_modifying_shipped_policies_are_fail_closed as a self-check on the detector is a nice touch.

The one gap the PR itself calls out (no shipped opted-in request-rewriting policy, so no positive e2e path) is a reasonable limitation to ship with, given the unit-level positive path is well covered.

Performance / security

• Deepcopy cost is gated behind flag+opt-in; default path is unchanged. Good.
• Rate-limit / 5xx exclusion from the retry set is correct — the design note in the constant's comment (anthropic_processor.py:99-107) is the kind of thing that saves a future reader an hour.
• No new external calls, no logging of secret bodies (the pipeline.passthrough_fallback event carries status + error message but not the request body; the rejected_request in the transaction record is intentional and gated by the same auth as everything else in the audit log).

Style / conventions

Matches CLAUDE.md conventions: Google-style docstrings on public methods, comments focus on the why (particularly good in _passthrough_fallback_request's docstring), f-strings throughout, type annotations on all new public surfaces. The changelog fragment and decision record are both present and accurate.

---

Ready to approve modulo the small suggestions above. Great work on the adversarial-safety framing — this reads like a PR that survived several rounds of "what if a client tries to game this," which is exactly what you want from a fail-open-when-on feature.

[claude]

Review of PR #797 — opt-in passthrough fallback

Overall this reads as a careful, safety-first implementation. The double opt-in (PASSTHROUGH_FALLBACK_ENABLED and passthrough_fallback_safe = True on the active policy, with MultiSerialPolicy requiring unanimous consent) is the right shape for the attack surface described, and it's good to see the adversarial review that motivated it in the PR body — the redaction-attack case is genuinely load-bearing. A few things worth calling out.

Things I liked

• Fail-closed as the default answer. BasePolicy.passthrough_fallback_safe = False, non-BasePolicy policies can't opt in, chains veto on any single opt-out. This matches the "never make things worse than direct API access" framing and, importantly, keeps that invariant even if a future policy author forgets to think about it.
• The class-scan invariant test (test_passthrough_fallback_invariant.py) is the piece that stops this from being a code-comment-enforced rule. Nice choice to walk BasePolicy.__subclasses__() recursively and filter to shipped modules; that catches TextModifierPolicy subclasses (imported transitively via AllCapsPolicy etc.) without hardcoding a list.
• Zero cost on the default (disabled) path. The copy.deepcopy(initial_request) is only taken when fallback is armed for this request, so the no-op stays a no-op — good.
• Truthful audit trail. Deferring transaction.request_recorded until the first attempt resolves, and recording the rejected policy-modified request in a passthrough_fallback block, is exactly what an operator needs to reconstruct what actually happened. Both attempts still surface as pipeline.backend_request events.
• Streaming rule is right. Fallback only fires when events_yielded == 0; a mid-stream failure re-raises. Re-sending after a partial stream would either interleave content for the client or re-run response-side policy hooks on duplicated events; neither is recoverable.
• Test coverage is broad. 28 fallback-specific unit tests covering non-streaming/streaming, disabled, unmodified, non-eligible statuses (401/403/429/500/529), retry-also-fails, in-place mutation, response blocks vs. policy-raised errors, audit-record shapes, the redaction attack, plus the positive control with an opt-in policy. E2E tests pin the fail-closed behavior with a real request-rewriting policy.

Suggestions

1. rejected_request is a shallow copy (src/luthien_proxy/pipeline/anthropic_processor.py:291). _passthrough_fallback_info holds dict(rejected_request). If any downstream consumer of transaction.request_recorded doesn't serialize immediately (or if the request has nested mutable structures that get modified later — I don't think anything currently does, but the interface allows it), the audit record could drift. Given every other similar site uses dict(...) too, this is consistent, but for a safety-audit record I'd lean toward copy.deepcopy here — the record is meant to be a snapshot, and the perf cost is bounded to the failure path.

2. _ALLOWLIST is defined empty and unused (tests/luthien_proxy/unit_tests/policies/test_passthrough_fallback_invariant.py:22). If it's intended as a documented extension point for reviewed exceptions, a one-line comment saying so (and what the review bar is) would prevent a future editor from just deleting it as dead. Otherwise, drop it — right now it reads like leftover scaffolding.

3. _modifies_requests heuristic (same file): the SimplePolicy carve-out (owner != "SimplePolicy") is name-based, which is brittle if someone renames the class or introduces a similar delegator. Not urgent — the safety net is that any actual override of simple_on_request is still caught — but a comment naming why SimplePolicy is exempt (its on_anthropic_request is pure delegation to simple_on_request) would help. Could also check owner is SimplePolicy via identity if you import the class, avoiding the string match.

4. Changelog wording on retry-also-fails. The changelog says the fallback "records the resent original as the final request." The decisions.md update in patch 4 tightens this to "names the request sent last (the one upstream accepted, unless the retry also failed)" — worth mirroring the tighter wording in the changelog too, since operators reading the changelog are exactly the people who'll be confused if they see a final_request = original audit line paired with a 4xx to the client.

5. sent_request == original deep equality (anthropic_processor.py:203). O(n) in request size on every armed request. In practice bounded by the request body, so almost certainly fine, but for large transcripts (long conversations) it's not free. If it ever shows up in a profile, a cheap id(sent_request) is id(original) or … fast path could help — noting for future rather than as a change ask.

6. Fallback-stream errors are handled implicitly but subtly (anthropic_processor.py:374). If the second _iterate(fallback_request) throws AnthropicStatusError too, it propagates out of the outer try/except block (which is already inside the except handler, so it doesn't re-enter). fallback_fired=True correctly makes finally skip the (now redundant) record because _record_passthrough_fallback already wrote it. This is correct, but there's no test for "fallback stream also fails at connect" — worth adding a small one so the invariant "the audit record after a fallback-then-fail names the fallback request and preserves passthrough_fallback info" is pinned. The non-streaming version has test_fallback_retry_failure_propagates; a streaming twin would round out coverage.

7. Minor: set_request in test_in_place_policy_mutation_is_detected is redundant with the in-place mutation on the same dict object. Not wrong, just noise — either drop the set_request call or make the test use a fresh dict via _modified_request() to actually exercise set_request. I read the intent as "check that in-place mutation via the same reference is detected," in which case the set_request call is misleading.

Not blocking, but worth thinking about later

• _iterate shared _raw_backend_events across fallback attempts (anthropic_processor.py:341): non-streaming path only, and by construction empty on entry to fallback (streaming sets _buffer_raw_events=False, and non-streaming doesn't use _iterate), so no bug today. But if _iterate ever gets reused for non-streaming, someone will need to clear the list between attempts.
• _first_backend_response after fallback now captures the fallback response as the first response, which is correct given the fallback is what actually reached the client. Worth an inline note near line 322 so a reader doesn't wonder whether the pre-policy diff would show the (never-yielded) upstream 4xx.

Nothing here is a blocker. The safety story is solid, the tests do the enforcement work, and the failure modes I could think of are already covered or documented.

[scottwofford]

Merging on Scott's decision (Sep 27) after an adversarial review found no blockers (composition, hot-swap, streaming, retries, audit-record loss, and secrets-in-logs paths all held). Added before merge: a unit test that fails if any shipped policy opts in to fallback while overriding a request hook (mutation-checked); accurate audit wording ('request sent last'); changelog note that no shipped request-editing policy opts in yet. Known non-blocking follow-ups: no e2e proof of the runtime admin toggle; history UI doesn't surface the rejected request after a fallback.

🤖 Generated with Claude Code