Add cache-header audit design spec for ts dev audit headers by vasujain00 · Pull Request #930 · IABTechLab/trusted-server

vasujain00 · 2026-07-17T15:58:17Z

Technical spec for epic #834: origin cache-header audit with per-content-type cacheability diagnostics. Covers content-type taxonomy, cacheability rules for all five directives (Cache-Control, Surrogate-Control, Surrogate-Key, Vary, ETag), per-type verdict rollup, CLI integration into the ts dev group, and a 7-task implementation breakdown.

Closes #835

Summary

Changes

File	Change

Closes

Closes #

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:

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

aram356

Summary

The spec is a solid starting shape for ts dev audit headers (clear taxonomy, verdict rollup, task breakdown), but several load-bearing claims about the codebase are stale or incorrect — ts dev is already a subcommand tree, there is no CliError enum, reqwest is not a CLI dependency, and the discovery section probes edge-only /_ts/ routes against the publisher origin. Two cacheability rules also fail the strictest correct configuration, and the doc itself is what's breaking the format-docs CI check.

Details are in the inline comments (8 blocking 🔧, 5 🤔, 1 ♻️, 2 🌱, 2 ⛏). Cross-cutting items below.

Blocking

🔧 wrench

format-docs CI failure: prettier rejects this file's tables — cd docs && npm run format:write (line 30)
Dependency graph must be mermaid: docs use vitepress-plugin-mermaid; ASCII art won't render (line 391)
Stale ts dev premise: DevCommand subcommand tree already exists on main; no Serve to default to (lines 254, 289)
CliError doesn't exist: CLI errors are CliResult<T> = Result<T, String> (line 271)
reqwest is not a CLI dep + target scoping unaddressed: workspace default target is wasm32-wasip1; spec must say where deps are scoped and that audit is cross-platform, unlike the macOS-only proxy (line 323)
Discovery probes edge routes against origin: /_ts/* paths 404 on publisher.origin_url (line 131)
Wrong issue link: #293 text links to /issues/834 (line 7)
HTML/RTB rules fail no-store alone: RFC 9111's strongest posture would FAIL the audit (lines 75, 106)

Non-blocking

📌 out of scope

Plan document and epic breakdown still pending: issue #835 asks for a spec and a plan under docs/superpowers/plans/ plus a task breakdown in epic #834; this PR delivers the spec only (Section 10 is a partial plan). Also, the PR body retains the unfilled template (empty Summary/Changes tables, dangling Closes #).

CI Status

format-docs: FAIL (caused by this file)
All other checks (fmt, clippy targets, Rust/JS tests, integration, parity, CodeQL): PASS

aram356 · 2026-08-14T14:42:33Z

+
+### Arguments
+
+| Argument | Description |


🔧 wrench — This file is what's failing the format-docs CI check: npx prettier --check rejects every markdown table in the doc (unpadded cells).

Fix: cd docs && npm run format:write and commit the result.

Fixed. Ran cd docs && npx prettier --write -- tables are now padded. CI should pass.

aram356 · 2026-08-14T14:42:33Z

+
+## 11. Dependency Graph
+
+```


🔧 wrench — The dependency graph should be a mermaid diagram, not ASCII box art. The docs site uses vitepress with vitepress-plugin-mermaid, and existing specs (including the ts-dev-proxy spec referenced by #835 as the template) use mermaid; this block will render as a dead code block.

Fix:

```mermaid flowchart TD T1["Task 1: CLI restructure"] --> T6["Task 6: E2E wiring"] T2["Task 2: Rules engine"] --> T3["Task 3: Origin fetch"] --> T4["Task 4: Analysis"] --> T5["Task 5: Output"] --> T6 T2 --> T4 T6 --> T7["Task 7: Docs"] ```

Replaced ASCII art with mermaid flowchart using the same vitepress-plugin-mermaid syntax as the ts-dev-proxy spec.

aram356 · 2026-08-14T14:42:33Z

+
+### 8.1 `ts dev` restructure
+
+`ts dev` is currently a flat command wrapping `fastly compute serve`. It must be restructured into a subcommand tree:


🔧 wrench — Stale premise: ts dev is not "a flat command wrapping fastly compute serve". On main, DevCommand is already a clap::Subcommand enum with a macOS-only Proxy variant (crates/trusted-server-cli/src/commands/dev/mod.rs), and local serving lives in the separate top-level ts serve (edgezero). There is no DevArgs and no Serve variant to default to — implementing this section as written would invent a Serve behavior ts dev never had.

Fix: rewrite Section 8.1 (and Task 1, line 289) against current main: add an Audit(DevAuditCommand) variant to the existing DevCommand enum and drop the Serve default / backward-compat requirement. Task 1's "modifies existing code on feature/ts-cli" is also stale — the CLI landed on main.

aram356 · 2026-08-14T14:42:33Z

+
+### 8.2 Error handling
+
+Add `CliError::HeaderAudit` variant to `error.rs`.


🔧 wrench — There is no CliError enum to add a variant to. crates/trusted-server-cli/src/error.rs defines CliResult<T> = Result<T, String> plus helper functions.

Fix: either follow the existing String-error pattern, or explicitly propose introducing a typed error enum for the CLI — which is a much larger change than this one-liner implies and should be called out as such.

aram356 · 2026-08-14T14:42:33Z

+**Type:** Net-new (`dev_audit/fetch.rs`)
+**Dependencies:** Task 2 (uses `ContentTypeGroup`)
+
+Fetch responses from origin using `reqwest` (already a dep). Support explicit URL list and config-derived discovery.


🔧 wrench — reqwest is not "already a dep" of trusted-server-cli; it's defined at the workspace level but not consumed by the CLI crate. More importantly, the workspace's default build target is wasm32-wasip1, which is exactly why the dev-proxy's tokio/rustls/ring stack is cfg-scoped to macOS in the CLI's Cargo.toml. This spec's own design decision says explicit URLs exist "for CI" (i.e. Linux) — but today DevCommand has zero variants off macOS.

Fix: state where the HTTP-client dependencies get scoped (presumably cfg(not(target_arch = "wasm32"))) and that ts dev audit, unlike ts dev proxy, must be available on all host platforms.

aram356 · 2026-08-14T14:42:33Z

+
+```rust
+pub enum ContentTypeGroup { Html, JavaScript, Image, StaticAsset, RtbJson, Other }
+pub enum Verdict { Pass, Warn(String), Fail(String) }


♻️ refactor — Verdict::Warn(String) / Fail(String) embed a message that duplicates HeaderVerdict.recommendation, and the JSON example (line 172) shows "verdict": "Fail" as a bare string — which payload-carrying variants can't serialize to without a custom serde impl.

Fix: make Verdict a unit enum (Pass, Warn, Fail) and keep messages on HeaderVerdict.

aram356 · 2026-08-14T14:42:33Z

+
+## 4. Cacheability Rules
+
+Each content-type group has an expected caching posture. Rules are evaluated against: `Cache-Control`, `Surrogate-Control`, `Surrogate-Key`, `Vary`, `ETag`.


🌱 seedling — s-maxage is ignored throughout. It's the shared-cache TTL Fastly honors between Surrogate-Control and max-age, so the CDN-vs-browser comparison in 4.3 (and the RTB/HTML shared-cache rules) should account for it.

aram356 · 2026-08-14T14:42:33Z

+
+## 3. Content-Type Taxonomy
+
+Responses are classified into groups by their `Content-Type` response header:


🌱 seedling — Two gaps worth noting: (1) Vary is listed among the five audited headers but only evaluated for HTML — Vary: User-Agent or Vary: Cookie on JS/CSS/images are classic hit-ratio killers; (2) classification must strip Content-Type parameters (text/html; charset=utf-8) before matching, which the spec doesn't state.

aram356 · 2026-08-14T14:42:33Z

+| `Html` | `text/html` |
+| `JavaScript` | `application/javascript`, `text/javascript`, `application/x-javascript` |
+| `Image` | `image/*` |
+| `StaticAsset` | `text/css`, `font/*`, `application/font-*`, `application/woff*` |


⛏ nitpick — application/woff* isn't a real MIME pattern: fonts are font/woff2 (covered by font/*) or legacy application/font-woff (covered by application/font-*). The entry is redundant.

aram356 · 2026-08-14T14:42:34Z

+# Technical Specification: Origin Cache-Header Audit (`ts dev audit headers`)
+
+**Status:** Draft
+**Author:** @vasujai


⛏ nitpick — @vasujai doesn't match the author's GitHub login (vasujain00).

Technical spec for epic IABTechLab#834: origin cache-header audit with per-content-type cacheability diagnostics. Covers content-type taxonomy, cacheability rules for all five directives (Cache-Control, Surrogate-Control, Surrogate-Key, Vary, ETag), per-type verdict rollup, CLI integration into the ts dev group, and a 7-task implementation breakdown. Closes IABTechLab#835

@aram356

Resolves all 8 blocking 🔧 items from @aram356's review: 1. format-docs CI: ran prettier (tables now padded correctly) 2. Dependency graph: replaced ASCII art with mermaid flowchart 3. Stale ts dev premise: rewrote §8.1 to add Audit variant to existing DevCommand enum (no Serve default, no DevArgs invention) 4. CliError: corrected §8.2 to use existing CliResult<T>/cli_error() pattern 5. reqwest scoping: added §8.3 explaining cfg(not(wasm32)) dep scoping and cross-platform requirement for audit (unlike macOS-only proxy) 6. Discovery probes: rewrote §5 to HTML-parse origin root page instead of probing edge-only /_ts/ routes against origin 7. Issue link: fixed [IABTechLab#293] to link to IABTechLab/issues/293 (was linking IABTechLab/issues/834) 8. HTML/RTB no-store rules: no-store alone now passes per RFC 9111 §5.2.2.5 Non-blocking items also addressed: - Verdict: changed to unit enum (messages on HeaderVerdict) - Author: corrected @Vasujai to @vasujain00 - s-maxage: added to audited headers with explanation - Vary: now evaluated on all cacheable groups (not just HTML) - application/woff*: removed (redundant with font/*) - Content-Type param stripping: documented in §3 - Module path: corrected to commands/dev/audit/headers/

vasujain00 · 2026-08-21T18:54:45Z

Thanks @aram356 -- thorough review. All 8 blocking items and non-blocking feedback addressed in e268913. Rebased on latest main.

vasujain00 · 2026-08-21T18:55:51Z

+# Technical Specification: Origin Cache-Header Audit (`ts dev audit headers`)
+
+**Status:** Draft
+**Author:** @vasujai


vasujain00 · 2026-08-21T18:56:27Z

+**Author:** @vasujai
+**Epic:** [#834](https://github.com/IABTechLab/trusted-server/issues/834)
+**Planning task:** [#835](https://github.com/IABTechLab/trusted-server/issues/835)
+**Related:** [#293](https://github.com/IABTechLab/trusted-server/issues/834) (cache header refactoring), [#428](https://github.com/IABTechLab/trusted-server/issues/428) (ETag multi-value), PR #860


Fixed: [#293] now links to /issues/293.

vasujain00 · 2026-08-21T18:58:26Z

+
+### Arguments
+
+| Argument | Description |


Fixed. Ran cd docs && npx prettier --write -- tables are now padded. CI should pass.

vasujain00 · 2026-08-21T18:59:04Z

+
+## 11. Dependency Graph
+
+```


Replaced ASCII art with mermaid flowchart using the same vitepress-plugin-mermaid syntax as the ts-dev-proxy spec.

aram356 assigned vasujain00 Jul 19, 2026

aram356 requested review from ChristianPavilonis, aram356 and prk-Jr August 13, 2026 16:22

aram356 requested changes Aug 14, 2026

View reviewed changes

This comment was marked as low quality.

Sign in to view

ChristianPavilonis requested review from ChristianPavilonis and removed request for ChristianPavilonis August 14, 2026 18:52

Vasu Jain added 2 commits August 21, 2026 11:38

vasujain00 force-pushed the spec/834-audit-headers branch from cf69725 to e268913 Compare August 21, 2026 18:53

vasujain00 commented Aug 21, 2026

View reviewed changes


		### 8.1 `ts dev` restructure

		`ts dev` is currently a flat command wrapping `fastly compute serve`. It must be restructured into a subcommand tree:


		### 8.2 Error handling

		Add `CliError::HeaderAudit` variant to `error.rs`.


		## 4. Cacheability Rules

		Each content-type group has an expected caching posture. Rules are evaluated against: `Cache-Control`, `Surrogate-Control`, `Surrogate-Key`, `Vary`, `ETag`.


		## 3. Content-Type Taxonomy

		Responses are classified into groups by their `Content-Type` response header:

Uh oh!

Conversation

vasujain00 commented Jul 17, 2026

Summary

Changes

Closes

Test plan

Checklist

Uh oh!

aram356 left a comment

Choose a reason for hiding this comment

Summary

Blocking

🔧 wrench

Non-blocking

📌 out of scope

CI Status

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as low quality.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants