Prevent 304 responses for SSAT root documents - #952
Merged
Conversation
prk-Jr
requested review from
ChristianPavilonis and
aram356
and removed request for
aram356
July 23, 2026 15:45
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Reviewed PR #952 against main. The Fastly path is covered, but one supported-adapter cache-bypass gap needs follow-up; it is included inline.
…t-304 # Conflicts: # crates/trusted-server-core/src/publisher.rs
CloudflareHttpClient::execute built its worker::RequestInit without ever reading PlatformHttpRequest::bypass_cache, so an ad-stack-eligible navigation could still be served from Cloudflare's subrequest cache instead of a complete origin body. Map the flag to the Workers runtime's no-store cache mode. The mapping goes through a target-independent OutboundCacheMode enum, gated the same way as reject_multi_provider_fanout, because the worker crate is a wasm32-only dependency and the native test binary cannot name worker::CacheMode.
ChristianPavilonis
added a commit
that referenced
this pull request
Jul 28, 2026
Both parents of the updated #952 merge head are already integrated in the RC. Preserve the current tree while recording the exact latest head.
aram356
requested changes
Jul 29, 2026
aram356
left a comment
aram356
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Well-designed and thoroughly tested fix: eligible SSAT navigations now strip browser validators, bypass the platform cache, return synthesized HTML as private, no-store without validators, and fail closed on an unexpected origin 304 with distinct abandoned-auction telemetry. One blocker on the Cloudflare surface: the new CacheMode::NoStore requires a compatibility date/flag the repo's wrangler configs don't have, so the eligible-navigation path would throw at runtime on that adapter.
Blocking
🔧 wrench
- Cloudflare
CacheMode::NoStorerequirescache_option_enabled:wrangler.toml/wrangler.ci.tomlpincompatibility_date = "2024-09-23", which predates the 2024-11-11cacheRequestInit support — every eligible navigation on the Cloudflare adapter would fail its origin fetch (crates/trusted-server-adapter-cloudflare/src/platform.rs:356; see inline comment).
Non-blocking
📝 note
- Spec/plan/PR description omit the Cloudflare runtime change: the design doc still states adapters without a read-through cache need no runtime change (
docs/superpowers/specs/2026-07-22-ssat-root-document-304-prevention-design.md:53; see inline comment).
🌱 seedling
- Cloudflare CDN cache directives aren't stripped from synthesized HTML:
CDN-Cache-Control/Cloudflare-CDN-Cache-Controlpass through where Fastly's surrogate headers are removed — pre-existing gap, follow-up issue (crates/trusted-server-core/src/publisher.rs:2921; see inline comment).
CI Status
- fmt: PASS
- clippy/check jobs: PASS
- rust tests (fastly/axum/cloudflare/spin + parity + CLI): PASS
- js tests (vitest): PASS
- integration + browser tests: PASS
Also verified locally: the 4 new ssat_cache_policy_tests under Viceroy, both Fastly apply_fastly_cache_bypass tests, both Cloudflare outbound_cache_mode tests, and cargo check-cloudflare (wasm).
The Cloudflare adapter maps the platform cache-bypass option to `CacheMode::NoStore` on the outbound subrequest, but setting the `cache` field on `RequestInit` requires the `cache_option_enabled` compatibility flag, which is only on by default from compatibility date 2024-11-11. Both wrangler files pin 2024-09-23, so the Workers runtime would have thrown on every auction-eligible navigation, failing the origin fetch and abandoning the auction. Set the flag explicitly in `wrangler.toml` and `wrangler.ci.toml`, and document the coupling at the call site. Also strip `CDN-Cache-Control` and `Cloudflare-CDN-Cache-Control` from synthesized SSAT HTML alongside the existing surrogate headers. Without this, an origin emitting either field could still instruct the Cloudflare edge to store a per-navigation document, reproducing on that adapter the bug this change closes on Fastly. Update the design doc and plan file map to match the shipped behavior.
ChristianPavilonis
approved these changes
Jul 29, 2026
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
Summary
The main cache-bypass and unexpected-304 handling are well structured. I found one high-severity cache-privacy issue and one medium-severity partial-response correctness issue, noted inline. All displayed CI checks pass on the reviewed head.
aram356
approved these changes
Jul 30, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
Cache-Control: private, no-store.502 Bad Gatewayand abandon the dispatched auction if an eligible origin unexpectedly returns304 Not Modified.Changes
crates/trusted-server-core/src/platform/http.rscrates/trusted-server-core/src/platform/test_support.rscrates/trusted-server-adapter-fastly/src/platform.rsset_pass(true)for synchronous and asynchronous sends.crates/trusted-server-adapter-cloudflare/src/platform.rsCacheMode::NoStore.crates/trusted-server-adapter-cloudflare/wrangler.tomlcrates/trusted-server-adapter-cloudflare/wrangler.ci.tomlcrates/trusted-server-core/src/response_privacy.rscrates/trusted-server-core/src/publisher.rsprivate, no-store, reject unexpected eligible 304s, abandon auctions, and add regression coverage.docs/superpowers/specs/2026-07-22-ssat-root-document-304-prevention-design.mddocs/superpowers/plans/2026-07-22-ssat-root-document-304-prevention.mdCloses #953
Test plan
cargo test-fastlycargo test-axumcargo test-cloudflarecargo test-spincargo fmt --all -- --checkcargo clippy-fastlycargo clippy-axumcargo clippy-cloudflarecargo clippy-cloudflare-wasmcargo clippy-spin-nativecargo clippy-spin-wasmcd crates/trusted-server-js/lib && npx vitest run(411 tests using pinned Node 24.12.0; previous reviewed head)cd crates/trusted-server-js/lib && npm run format(previous reviewed head)cd docs && npm run format(previous reviewed head)cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute serveChecklist
unwrap()in production codeprintln!)