Skip to content

Implement configurable cache header policies#860

Open
ChristianPavilonis wants to merge 2 commits into
mainfrom
refactor/cache-headers
Open

Implement configurable cache header policies#860
ChristianPavilonis wants to merge 2 commits into
mainfrom
refactor/cache-headers

Conversation

@ChristianPavilonis

@ChristianPavilonis ChristianPavilonis commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Standardizes cache policy rendering across Fastly, Cloudflare, CDN, and s-maxage fallback headers.
  • Adds safe, configurable caching for hash-validated TSJS, publisher-origin static assets, and rehosted asset proxy responses.
  • Hardens privacy handling so private, no-store, and cookie-bearing responses strip shared edge-cache headers.

Changes

File Change
crates/trusted-server-core/src/cache_policy.rs Adds typed cache-policy rendering for browser and edge headers, including no-store/private cleanup.
crates/trusted-server-core/src/settings.rs Adds cache.asset_rules config, matchers/presets, validation, runtime prep, and path-to-policy resolution.
trusted-server.example.toml Documents disabled operator-controlled static/fingerprinted asset cache-rule examples.
crates/trusted-server-core/src/http_util.rs Routes static ETag responses through the cache-policy renderer.
crates/trusted-server-core/src/tsjs.rs Uses exact module-set hashes when available and avoids unverifiable fallback hashes.
crates/trusted-server-js/Cargo.toml Makes hashing dependencies available to the build script.
crates/trusted-server-js/build.rs Generates per-module SHA-256 metadata for bundled JS modules.
crates/trusted-server-js/src/bundle.rs Exposes per-module hashes and caches concatenated bundle hashes.
crates/trusted-server-core/src/publisher.rs Applies hash-validated immutable TSJS caching and configured publisher asset cache rules.
crates/trusted-server-core/src/proxy.rs Applies normalized cache policies to rehosted asset proxy responses and reapplies them after finalization.
crates/trusted-server-core/src/response_privacy.rs Removes shared-cache headers from private/no-store/cookie-bearing responses.
crates/trusted-server-core/src/integrations/prebid.rs Makes the neutralized Prebid shim no-store, private instead of long-lived public cache.
crates/trusted-server-core/src/integrations/testlight.rs Updates the default TSJS fallback comment/source behavior for registry-free configuration.
crates/trusted-server-core/src/lib.rs Exports the new cache_policy module.
crates/trusted-server-adapter-axum/src/app.rs Passes the portable s-maxage fallback edge header mode to TSJS and publisher handlers.
crates/trusted-server-adapter-cloudflare/src/app.rs Passes the Cloudflare-specific CDN cache header mode to TSJS and publisher handlers.
crates/trusted-server-adapter-fastly/src/app.rs Passes Fastly Surrogate-Control mode through EdgeZero fallback dispatch.
crates/trusted-server-adapter-fastly/src/main.rs Reapplies asset cache policies with Fastly Surrogate-Control in legacy and EdgeZero flows.
crates/trusted-server-adapter-fastly/src/route_tests.rs Updates route-test finalization for selected edge cache headers.
crates/trusted-server-adapter-spin/src/app.rs Passes the portable s-maxage fallback edge header mode to TSJS and publisher handlers.
docs/superpowers/specs/2026-07-06-cache-control-header-design.md Adds the cache-control design scope and deferred dynamic caching notes.
docs/superpowers/plans/2026-07-06-cache-control-header-implementation-plan.md Adds the implementation and verification plan for cache-header work.

Closes

Closes #293

Follow-up: #859 tracks deferred dynamic response/template caching.

Test plan

  • cargo test-fastly && cargo test-axum
  • cargo clippy-fastly && cargo clippy-axum
  • cargo fmt --all -- --check
  • JS tests: cd crates/trusted-server-js/lib && npx vitest run
  • JS format: cd crates/trusted-server-js/lib && npm run format
  • Docs format: cd docs && npm run format
  • WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
  • Manual testing via fastly compute serve
  • Other: cargo test-cloudflare && cargo test-spin
  • Other: cargo clippy-cloudflare && cargo clippy-spin-native && cargo clippy-spin-wasm
  • Other: cd crates/trusted-server-js/lib && node build-all.mjs
  • Other: git diff --check
  • Other: npx prettier --check docs/superpowers/plans/2026-07-06-cache-control-header-implementation-plan.md docs/superpowers/specs/2026-07-06-cache-control-header-design.md

Note: cd docs && npm run format failed because docs-local Prettier was not installed; touched docs were checked with npx prettier instead.

Checklist

  • Changes follow CLAUDE.md conventions
  • No unwrap() in production code — use expect("should ...")
  • Uses tracing macros (not println!)
  • New code has tests
  • No secrets or credentials committed

@ChristianPavilonis ChristianPavilonis changed the base branch from main to server-side-ad-templates-impl July 7, 2026 17:59
Base automatically changed from server-side-ad-templates-impl to main July 7, 2026 20:08
ChristianPavilonis

This comment was marked as low quality.

@ChristianPavilonis ChristianPavilonis marked this pull request as ready for review July 8, 2026 19:00

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Configurable, safe-by-default cache-header policies across all four adapters. Cache policy is expressed once as typed data (CachePolicy / EdgeCacheHeader) and rendered per-runtime; hash-gated immutability, privacy stripping, and operator-controlled asset rules are all well-tested. No blocking issues — logic is sound and correctly platform-scoped. Findings below are all non-blocking.

Verified during review:

  • Hash-gated immutability is safeserve_tsjs_static marks a response immutable (1yr) only when the request ?v= equals the hash of the content actually being served, so a stale URL after a redeploy falls back to the short TTL rather than pinning old content.
  • Injection ↔ serving hash consistency — HTML injection (html_processor.rs) and the serving path (publisher.rs) both derive the hash from js_module_ids_immediate() + concatenated_hash; deferred modules use single_module_hash on both sides.
  • Privacy hardeningprivate / no-store / cookie-bearing responses strip all four edge-cache headers, with the downgrade re-run after operator headers are applied.
  • Origin no-store not upgraded — a split later Cache-Control field carrying no-store correctly blocks the normalized upgrade.
  • Asset-proxy finalization is correctly Fastly-onlyhandle_asset_proxy_request / apply_after_route_finalization are wired only on Fastly, so there is no missing-reapplication gap on Cloudflare / Axum / Spin.

Non-blocking

🤔 thinking

  • Hex-only fingerprint heuristic: filename_contains_hash misses base62/base36 bundler hashes (false negative → silently uncached) and can match coincidental hex stems (false positive → stale). (settings.rs)
  • Normalized policy overrides origin no-cache/Vary: upgrade gate checks only private/no-store. (publisher.rs)

🌱 seedling

  • handle_publisher_request is now at the 7-argument CLAUDE.md limit; EdgeCacheHeader is threaded through several signatures — consider a request-context struct. (publisher.rs)

⛏ nitpick

  • SURROGATE_CACHE_HEADERS re-export is now a misnomer (contains CDN headers) with no in-tree consumers. (response_privacy.rs:21)

📝 note

  • #[validate(nested)] on cache is a no-op; real validation lives in prepare_runtime. (settings.rs)

👍 praise

  • Directive-exact Cache-Control matching closes the old substring-match privacy hole. (cache_policy.rs)

CI Status

  • fmt: PASS
  • clippy (fastly / axum / cloudflare / cloudflare-wasm / spin-native / spin-wasm): PASS
  • rust tests (fastly / axum / cloudflare / spin / CLI / parity): PASS
  • js tests (vitest): PASS
  • docs / typescript format: PASS
  • CodeQL + integration/browser tests: PASS

(!extension.is_empty()).then(|| extension.to_ascii_lowercase())
}

fn filename_contains_hash(path: &str) -> bool {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 thinking — The hash heuristic only accepts segments that are entirely ASCII hex (len >= 8 && all is_ascii_hexdigit). Two consequences worth documenting or tightening:

  • False negative: many bundlers (Vite, esbuild, webpack contenthash) emit base62/base36 fingerprints such as app.a1B2c3D4.js. Those contain non-hex characters, so requires_hash_in_filename = true silently declines to cache them — the operator opts in expecting their fingerprinted assets to be cached long-lived, and nothing tells them why it didn't happen.
  • False positive: a mutable, non-fingerprinted file whose stem happens to be 8+ hex chars (e.g. /assets/deadbeef.js) is treated as content-addressed and cached immutable for a year → stale-content risk.

Suggest at minimum documenting the hex-only assumption next to the requires_hash_in_filename field, or broadening the match to alphanumeric segments of length ≥ 8 containing at least one digit (accepts base62 while still rejecting plain words).

edge_header: EdgeCacheHeader,
response: &mut Response<EdgeBody>,
) -> Result<(), Report<TrustedServerError>> {
if !cache_rule_method || response_cache_control_is_private_or_no_store(response) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 thinking — The upgrade gate only bails on private/no-store. An origin asset matching a configured rule that returns Cache-Control: no-cache or Vary: Cookie still gets rewritten to public, max-age=…, immutable, dropping the origin's revalidation / variant semantics. It's mitigated by the requires_hash_in_filename gate plus explicit operator opt-in, but a shared cache serving a Vary-dependent variant to the wrong user would be a real correctness bug for a misconfigured rule. Worth a one-line caveat in the cache docs that asset rules override origin no-cache/Vary.

ec_context: &mut EcContext,
auction: AuctionDispatch<'_>,
mut req: Request<EdgeBody>,
edge_header: EdgeCacheHeader,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌱 seedling — With edge_header, this handler is now at exactly 7 parameters — the CLAUDE.md ceiling. EdgeCacheHeader is also threaded through handle_tsjs_dynamic, serve_static_with_etag, apply_after_route_finalization, and every adapter call site. The next per-request knob forces a refactor anyway; consider grouping the edge-cache selection (and related request plumbing) into a small context struct now while the change is localized.

/// cannot drift apart.
pub const SURROGATE_CACHE_HEADERS: &[&str] = &["surrogate-control", "fastly-surrogate-control"];
/// Runtime edge-cache headers stripped from private or cookie-bearing responses.
pub use crate::cache_policy::EDGE_CACHE_HEADER_NAMES as SURROGATE_CACHE_HEADERS;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick — This alias now re-exports EDGE_CACHE_HEADER_NAMES, which also contains cdn-cache-control / cloudflare-cdn-cache-control — so SURROGATE_CACHE_HEADERS is a misnomer, and grep shows it has no remaining in-tree consumers. Either drop the alias and use EDGE_CACHE_HEADER_NAMES directly, or rename it to reflect the broader edge-cache scope.

pub consent: ConsentConfig,
#[serde(default)]
#[validate(nested)]
pub cache: CacheSettings,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 note#[validate(nested)] here is effectively a no-op: CacheSettings declares no field validators and asset_rules elements aren't marked #[validate(nested)] (nor does CacheAssetRule derive Validate). The real validation lives in prepare_runtime. Not a bug — just flagging that the attribute reads as if validator enforces rule shape when it doesn't.

/// Matching is directive-name exact and case-insensitive. Pseudo-directives such
/// as `not-private` or `no-storey` do not match `private` / `no-store`.
#[must_use]
pub fn cache_control_value_has_directive(value: &str, directive: &str) -> bool {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 praise — Directive-name-exact, case-insensitive matching (splitting on =/; so no-storey and not-private don't match) closes the substring-match hole the old .contains("private") check had. The paired tests and the get_all multi-value check make the privacy hardening genuinely robust.

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.

Refactor and standardize how Trusted Server sets cache response headers

2 participants