You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Targeted, well-scoped fix, and the core reasoning is right: auto-redact disables the event.raw_passthrough branch (completions.rs:1677-1686) and re-serializes every chunk, so a provider TEE signature over the pre-redaction bytes can never match what the client received. Moving the gateway_signature_enabled computation below maybe_redact so it can observe auto_redact_enabled is the correct placement — nothing between the old and new assignment points reads skip_provider_chat_signature, and the field is only consumed later in create_chat_completion_stream (services/src/completions/mod.rs:1676).
Gating on auto_redact_enabled (requested && !redaction_map.is_empty()) rather than auto_redact_requested is also correct and consistent with the existing passthrough guard — a no-PII request keeps byte-exact provider signing.
Note: I couldn't load the existing inline review threads for this PR, so apologies if any of the below was already covered.
⚠️model_attestation_supported == None still stores a mismatching provider signature
The unwrap_or(false) mirrors the fallback policy in chat_stream_usage_mode, but that policy doesn't transfer. There, None disables the rewrite itself — bytes stay byte-exact, so keeping the provider signature is sound. Auto-redact rewrites the chunks unconditionally; None does not turn that off.
Failure path: get_models_with_pricing() errors (completions.rs:1451-1462 — warn and continue), so model_attestation_supported = None for a model that is attested. Then:
the service layer reads model.attestation_supported == true directly and runs the provider-signature fetch (services/src/completions/mod.rs:141-145, 1676)
the client still receives re-serialized, un-redacted bytes
Net: GET /v1/signature/{chat_id} returns a provider_tee signature whose response_hash covers bytes the client never saw. That is worse than the pre-fix state in one respect — a confidently-wrong signature rather than a missing one, with nothing in the response telling the client to distrust it. The new unit test at line 3241 currently pins this behaviour (assert!(!auto_redact_requires_gateway_signature(true, None))).
Suggested fix — fail closed when attestation status is unknown but we know we have mutated the bytes:
fnauto_redact_requires_gateway_signature(auto_redact_enabled:bool,model_attestation_supported:Option<bool>,) -> bool{// Unlike the usage-rewrite modes, `None` cannot fall back to byte-exact// passthrough: auto-redact re-serializes the stream regardless. Suppress// the provider fetch rather than store a signature over bytes the client// never received.
auto_redact_enabled && model_attestation_supported.unwrap_or(true)}
Worth confirming store_chat_signature_and_unpin is a safe no-op for a chat_id that was never pinned (non-attested model) before taking unwrap_or(true). If it isn't, the alternative is to keep the gateway-sign decision as-is but set skip_provider_chat_signature = true whenever auto_redact_enabled — no signature at all is the honest answer here.
Notes (non-blocking)
Non-streaming attested + auto-redact has the same shape.completions.rs:2113 re-serializes the un-redacted body while services/src/completions/mod.rs:1834-1841 unconditionally spawns store_chat_signature_from_provider on model.attestation_supported — skip_provider_chat_signature is not consulted on the non-streaming path at all. The comment at completions.rs:2110 documents dropping the signed payload as deliberate, so I am reading it as accepted scope; flagging only because the stored signature is still served over /v1/signature/{chat_id} and will mismatch there too, in case Auto-redacted streams store a provider signature that cannot match client-received bytes #892 was meant to cover it.
E2EE + auto-redact is unguarded on the chat plane./v1/completions rejects both combinations explicitly (completions.rs:3468-3488) and native Anthropic rejects E2EE outright (anthropic.rs:771-782), but maybe_redact never inspects the encryption headers. It stays inert in practice — the detector finds no PII in ciphertext, so redaction_map is empty — but if it ever minted a placeholder, opaque E2EE chunks would get re-serialized. This PR makes that corner better (the signature would at least match the delivered bytes), so no action needed here; it is pre-existing and worth a separate guard.
Test coverage narrowed.auto_redact_streaming_splits_placeholder_across_chunks was repurposed rather than added alongside, so the default-stream (no stream_options) content-delta split case lost its dedicated test. auto_redact_unredacts_tool_call_arguments_streaming (line 552) still exercises default-stream tail reassembly, so it is not a hole — but a second test rather than a rename would have kept both.
The switch to .content_type() + .bytes(Bytes::from(request_json.clone())) is a good call for a byte-exact hash assertion, and bytes is already a regular dependency of crates/api, so it is available to the integration test.
Signature text format matches store_gateway_signature ({request_hash}:{response_hash}), and the tail awaits the store before flushing [DONE] (completions.rs:1960-1998), so the test's fetch-after-stream cannot race.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#892 was reported for streams. The same auto-redact response rewrite exists for non-streaming completions, so this PR fixes both paths.
Testing
cargo test -p api --libcargo test -p services 'completions::' --libcargo test -p api --test e2e_all --no-runFixes #892