From 378131d8fd8354347cd13d6001d820f517461a20 Mon Sep 17 00:00:00 2001 From: Vasu Jain Date: Tue, 15 Sep 2026 08:09:57 -0700 Subject: [PATCH 1/7] feat(cli): implement ts dev audit headers cache-header audit Implements the origin cache-header audit from the #834 spec: fetches origin responses (explicit URLs or discovered from the origin root HTML), classifies each by content type, evaluates cache directives, and reports a per-type pass/warn/fail verdict. - rules.rs: ContentTypeGroup taxonomy (MIME param stripping), unit Verdict enum, per-group cacheability rules. no-store alone passes HTML/RTB per RFC 9111 5.2.2.5; s-maxage honored as CDN TTL; Vary checked on all cacheable groups; Surrogate-Key only on cacheable groups. - fetch.rs: AuditHeadersArgs, OriginClient trait + reqwest blocking impl, config-derived origin resolution, HTML-parse discovery (never probes /_ts/). - analyze.rs: worst-of rollup per group + summary + exit-code mapping. - output.rs: writer-injected human table + JSON (bare-string verdicts). - Adds Audit variant to the existing DevCommand enum (cross-platform, unlike the macOS-only proxy). reqwest scoped to cfg(not(target_arch = wasm32)). Follows the CLI's CliResult string-error and writer-injection conventions. 25 unit tests cover classification, rules, discovery, rollup, and rendering. Refs #834 --- Cargo.lock | 1 + crates/trusted-server-cli/Cargo.toml | 4 + .../src/commands/dev/audit/headers/analyze.rs | 282 +++++++ .../src/commands/dev/audit/headers/fetch.rs | 404 +++++++++ .../src/commands/dev/audit/headers/mod.rs | 194 +++++ .../src/commands/dev/audit/headers/output.rs | 159 ++++ .../src/commands/dev/audit/headers/rules.rs | 766 ++++++++++++++++++ .../src/commands/dev/audit/mod.rs | 40 + .../src/commands/dev/mod.rs | 24 +- 9 files changed, 1859 insertions(+), 15 deletions(-) create mode 100644 crates/trusted-server-cli/src/commands/dev/audit/headers/analyze.rs create mode 100644 crates/trusted-server-cli/src/commands/dev/audit/headers/fetch.rs create mode 100644 crates/trusted-server-cli/src/commands/dev/audit/headers/mod.rs create mode 100644 crates/trusted-server-cli/src/commands/dev/audit/headers/output.rs create mode 100644 crates/trusted-server-cli/src/commands/dev/audit/headers/rules.rs create mode 100644 crates/trusted-server-cli/src/commands/dev/audit/mod.rs diff --git a/Cargo.lock b/Cargo.lock index 4f8d87c297..f0eafc4aa8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5449,6 +5449,7 @@ dependencies = [ "rand 0.8.6", "rcgen", "regex", + "reqwest 0.12.28", "rustls", "rustls-pemfile", "scraper", diff --git a/crates/trusted-server-cli/Cargo.toml b/crates/trusted-server-cli/Cargo.toml index 0f93651c50..653c0478c0 100644 --- a/crates/trusted-server-cli/Cargo.toml +++ b/crates/trusted-server-cli/Cargo.toml @@ -26,6 +26,10 @@ http = { workspace = true } log = { workspace = true } rand = { workspace = true } regex = { workspace = true } +# `ts dev audit headers` fetches origin responses over HTTP. Scoped to host +# targets (not the repo-default wasm32-wasip1) alongside the other native-only +# deps so unsupported targets do not try to build reqwest's TLS stack. +reqwest = { workspace = true, features = ["blocking"] } scraper = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } diff --git a/crates/trusted-server-cli/src/commands/dev/audit/headers/analyze.rs b/crates/trusted-server-cli/src/commands/dev/audit/headers/analyze.rs new file mode 100644 index 0000000000..4d07fd3515 --- /dev/null +++ b/crates/trusted-server-cli/src/commands/dev/audit/headers/analyze.rs @@ -0,0 +1,282 @@ +//! Groups fetched responses by content type, evaluates each against the +//! cacheability rules, and rolls the per-header verdicts up into a per-type +//! report. + +use std::collections::BTreeMap; + +use serde::Serialize; +use url::Url; + +use super::rules::{ContentTypeGroup, HeaderVerdict, ResponseHeaders, Verdict, evaluate}; + +/// One fetched origin response, paired with the URL it came from. +#[derive(Debug, Clone)] +pub(crate) struct FetchedResponse { + /// The URL that was fetched. + pub(crate) url: Url, + /// The cache-relevant headers of the response. + pub(crate) headers: ResponseHeaders, +} + +/// The audit outcome for a single content-type group. +#[derive(Debug, Clone, Serialize)] +pub(crate) struct GroupReport { + /// The content-type family this report covers. + pub(crate) content_type: ContentTypeGroup, + /// The URLs sampled into this group. + pub(crate) urls_sampled: Vec, + /// Group-level (per-type) verdict: worst-of rollup over `verdicts`. + pub(crate) verdict: Verdict, + /// The per-header verdicts, merged worst-of across sampled responses. + pub(crate) verdicts: Vec, +} + +/// Per-content-type-group tallies. `pass + warn + fail == total_groups`. +#[derive(Debug, Clone, Serialize)] +pub(crate) struct AuditSummary { + /// Number of evaluated content-type groups. + pub(crate) total_groups: usize, + /// Groups whose rollup verdict is [`Verdict::Pass`]. + pub(crate) pass: usize, + /// Groups whose rollup verdict is [`Verdict::Warn`]. + pub(crate) warn: usize, + /// Groups whose rollup verdict is [`Verdict::Fail`]. + pub(crate) fail: usize, +} + +/// The full audit report for one origin. +#[derive(Debug, Clone, Serialize)] +pub(crate) struct AuditReport { + /// The origin that was audited. + pub(crate) origin: String, + /// One report per evaluated content-type group. + pub(crate) groups: Vec, + /// Group-level tallies. + pub(crate) summary: AuditSummary, +} + +impl AuditReport { + /// The process exit code the CLI should return: 1 if any group failed, + /// 2 if any warned (and none failed), 0 otherwise. + pub(crate) fn exit_code(&self) -> i32 { + if self.summary.fail > 0 { + 1 + } else if self.summary.warn > 0 { + 2 + } else { + 0 + } + } +} + +/// Builds an [`AuditReport`] from fetched responses. +/// +/// Responses classified as [`ContentTypeGroup::Other`] are ignored — that group +/// is informational and never evaluated. Within a group, per-header verdicts are +/// merged worst-of across every sampled response, so a single bad response +/// downgrades the group. +pub(crate) fn run_analysis(origin: &str, responses: &[FetchedResponse]) -> AuditReport { + // Preserve a stable, deterministic group ordering for output. + let mut grouped: BTreeMap = BTreeMap::new(); + + for response in responses { + let content_type = response.headers.content_type.as_deref().unwrap_or_default(); + let group = ContentTypeGroup::classify(content_type); + if matches!(group, ContentTypeGroup::Other) { + continue; + } + + let accumulator = grouped + .entry(group_order(group)) + .or_insert_with(|| GroupAccumulator::new(group)); + accumulator.add(response.url.as_str(), evaluate(group, &response.headers)); + } + + let groups: Vec = grouped + .into_values() + .map(GroupAccumulator::finish) + .collect(); + + let summary = summarize(&groups); + + AuditReport { + origin: origin.to_owned(), + groups, + summary, + } +} + +/// A stable sort key so groups always render in the same order. +fn group_order(group: ContentTypeGroup) -> usize { + match group { + ContentTypeGroup::Html => 0, + ContentTypeGroup::JavaScript => 1, + ContentTypeGroup::Image => 2, + ContentTypeGroup::StaticAsset => 3, + ContentTypeGroup::RtbJson => 4, + ContentTypeGroup::Other => 5, + } +} + +/// Accumulates sampled URLs and merges per-header verdicts for one group. +struct GroupAccumulator { + content_type: ContentTypeGroup, + urls_sampled: Vec, + // Header name -> merged worst-of verdict, preserving first-seen order. + verdicts: Vec, +} + +impl GroupAccumulator { + fn new(content_type: ContentTypeGroup) -> Self { + Self { + content_type, + urls_sampled: Vec::new(), + verdicts: Vec::new(), + } + } + + fn add(&mut self, url: &str, verdicts: Vec) { + if !self.urls_sampled.iter().any(|sampled| sampled == url) { + self.urls_sampled.push(url.to_owned()); + } + for incoming in verdicts { + match self + .verdicts + .iter_mut() + .find(|existing| existing.header == incoming.header) + { + // Keep the more severe verdict when the same header is seen on + // multiple sampled responses in this group. + Some(existing) if is_more_severe(incoming.verdict, existing.verdict) => { + *existing = incoming; + } + Some(_) => {} + None => self.verdicts.push(incoming), + } + } + } + + fn finish(self) -> GroupReport { + let verdict = Verdict::rollup(self.verdicts.iter().map(|entry| entry.verdict)); + GroupReport { + content_type: self.content_type, + urls_sampled: self.urls_sampled, + verdict, + verdicts: self.verdicts, + } + } +} + +/// Whether `candidate` is strictly more severe than `current`. +fn is_more_severe(candidate: Verdict, current: Verdict) -> bool { + severity(candidate) > severity(current) +} + +fn severity(verdict: Verdict) -> u8 { + match verdict { + Verdict::Pass => 0, + Verdict::Warn => 1, + Verdict::Fail => 2, + } +} + +fn summarize(groups: &[GroupReport]) -> AuditSummary { + let mut summary = AuditSummary { + total_groups: groups.len(), + pass: 0, + warn: 0, + fail: 0, + }; + for group in groups { + match group.verdict { + Verdict::Pass => summary.pass += 1, + Verdict::Warn => summary.warn += 1, + Verdict::Fail => summary.fail += 1, + } + } + summary +} + +#[cfg(test)] +mod tests { + use super::*; + + fn response(url: &str, content_type: &str, cache_control: &str) -> FetchedResponse { + FetchedResponse { + url: Url::parse(url).expect("should parse test url"), + headers: ResponseHeaders { + content_type: Some(content_type.to_owned()), + cache_control: Some(cache_control.to_owned()), + ..ResponseHeaders::default() + }, + } + } + + #[test] + fn ignores_other_content_types() { + let responses = vec![response( + "https://origin.example/data.bin", + "application/octet-stream", + "public, max-age=60", + )]; + let report = run_analysis("https://origin.example", &responses); + assert_eq!( + report.summary.total_groups, 0, + "Other group should not be counted" + ); + } + + #[test] + fn counts_groups_by_rollup_verdict() { + let responses = vec![ + response("https://origin.example/", "text/html", "no-store"), + response( + "https://origin.example/rtb", + "application/json", + "public, max-age=60", + ), + ]; + let report = run_analysis("https://origin.example", &responses); + assert_eq!(report.summary.total_groups, 2, "should count HTML and RTB"); + assert_eq!(report.summary.pass, 1, "HTML no-store should pass"); + assert_eq!(report.summary.fail, 1, "cacheable RTB should fail"); + assert_eq!(report.exit_code(), 1, "any fail should exit 1"); + } + + #[test] + fn worst_of_merge_within_group() { + // Two HTML responses: one safe, one cacheable. The group must fail. + let responses = vec![ + response("https://origin.example/a", "text/html", "no-store"), + response( + "https://origin.example/b", + "text/html", + "public, max-age=3600", + ), + ]; + let report = run_analysis("https://origin.example", &responses); + assert_eq!(report.groups.len(), 1, "both HTML pages share one group"); + assert_eq!( + report.groups[0].verdict, + Verdict::Fail, + "a single cacheable HTML response should fail the group" + ); + assert_eq!( + report.groups[0].urls_sampled.len(), + 2, + "both sampled URLs should be recorded" + ); + } + + #[test] + fn warn_only_exits_two() { + let responses = vec![response( + "https://origin.example/app.js", + "application/javascript", + "public, max-age=600", + )]; + let report = run_analysis("https://origin.example", &responses); + assert_eq!(report.summary.warn, 1, "non-immutable JS should warn"); + assert_eq!(report.exit_code(), 2, "warn-only should exit 2"); + } +} diff --git a/crates/trusted-server-cli/src/commands/dev/audit/headers/fetch.rs b/crates/trusted-server-cli/src/commands/dev/audit/headers/fetch.rs new file mode 100644 index 0000000000..2b8d801d2e --- /dev/null +++ b/crates/trusted-server-cli/src/commands/dev/audit/headers/fetch.rs @@ -0,0 +1,404 @@ +//! Origin fetching, config-derived URL discovery, and response classification. +//! +//! `ts dev audit headers` fetches directly from the publisher origin. When no +//! explicit URLs are given, it fetches the origin root, parses the returned HTML +//! for asset URLs, and audits those. Routes under `/_ts/` are served by the +//! Trusted Server edge — not the origin — so they are never probed here; supply +//! them explicitly if you need to audit them. + +use std::path::PathBuf; +use std::time::Duration; + +use scraper::{Html, Selector}; +use url::Url; + +use super::analyze::FetchedResponse; +use super::rules::ResponseHeaders; +use crate::error::{CliResult, cli_error}; + +/// Caps discovery so one crawl of a large page can't fan out unbounded. +const MAX_DISCOVERED_URLS: usize = 50; + +/// Per-request timeout for origin fetches. +const REQUEST_TIMEOUT: Duration = Duration::from_secs(15); + +/// Arguments for `ts dev audit headers`. +#[derive(Debug, clap::Args)] +pub struct AuditHeadersArgs { + /// Explicit URLs to audit. When omitted, URLs are discovered from the + /// origin root page. + pub urls: Vec, + /// Path to `trusted-server.toml`, used to resolve the origin when no URLs + /// and no `--origin` are given. + #[arg(long, default_value = "trusted-server.toml")] + pub config: PathBuf, + /// Override the origin URL, skipping the config lookup. + #[arg(long)] + pub origin: Option, + /// Emit machine-readable JSON instead of the human table. + #[arg(long)] + pub json: bool, +} + +/// A single origin response: the cache-relevant headers plus the body (needed +/// only to discover assets from the root HTML page). +#[derive(Debug, Clone)] +pub(crate) struct OriginResponse { + /// The cache-relevant response headers. + pub(crate) headers: ResponseHeaders, + /// The response body as text (empty when not decodable as UTF-8). + pub(crate) body: String, +} + +/// Fetches origin responses. Abstracted so the fetch/discovery logic is +/// unit-testable without real network I/O, mirroring the `audit` command's +/// `AuditCollector`. +pub(crate) trait OriginClient { + /// Fetches a single URL from the origin. + fn fetch(&self, url: &Url) -> CliResult; +} + +/// The production [`OriginClient`], backed by a blocking `reqwest` client. +pub(crate) struct ReqwestOriginClient { + client: reqwest::blocking::Client, +} + +impl ReqwestOriginClient { + /// Builds a client with a bounded per-request timeout. + pub(crate) fn new() -> CliResult { + let client = reqwest::blocking::Client::builder() + .timeout(REQUEST_TIMEOUT) + .build() + .map_err(|error| format!("failed to build HTTP client: {error}"))?; + Ok(Self { client }) + } +} + +impl OriginClient for ReqwestOriginClient { + fn fetch(&self, url: &Url) -> CliResult { + let response = self + .client + .get(url.clone()) + .send() + .map_err(|error| format!("failed to fetch {url}: {error}"))?; + + let headers = extract_headers(response.headers()); + let body = response.text().unwrap_or_default(); + + Ok(OriginResponse { headers, body }) + } +} + +/// Pulls the cache-relevant headers out of a `reqwest` header map. +fn extract_headers(headers: &reqwest::header::HeaderMap) -> ResponseHeaders { + let value = |name: &str| { + headers + .get(name) + .and_then(|value| value.to_str().ok()) + .map(str::to_owned) + }; + ResponseHeaders { + content_type: value("content-type"), + cache_control: value("cache-control"), + surrogate_control: value("surrogate-control"), + surrogate_key: value("surrogate-key"), + vary: value("vary"), + etag: value("etag"), + } +} + +/// Resolves the target URLs and fetches each, returning classified responses. +pub(crate) fn collect_responses( + args: &AuditHeadersArgs, + client: &dyn OriginClient, +) -> CliResult<(String, Vec)> { + if args.urls.is_empty() { + collect_via_discovery(args, client) + } else { + collect_explicit(&args.urls, client) + } +} + +/// Fetches an explicit list of URLs. +fn collect_explicit( + urls: &[String], + client: &dyn OriginClient, +) -> CliResult<(String, Vec)> { + let mut responses = Vec::with_capacity(urls.len()); + for raw in urls { + let url = Url::parse(raw).map_err(|error| format!("invalid URL `{raw}`: {error}"))?; + let response = client.fetch(&url)?; + responses.push(FetchedResponse { + url, + headers: response.headers, + }); + } + // With explicit URLs there is no single origin; report the first host. + let origin = responses + .first() + .map(|response| response.url.origin().ascii_serialization()) + .unwrap_or_default(); + Ok((origin, responses)) +} + +/// Resolves the origin, fetches its root, and audits the assets discovered in +/// the returned HTML alongside the root itself. +fn collect_via_discovery( + args: &AuditHeadersArgs, + client: &dyn OriginClient, +) -> CliResult<(String, Vec)> { + let origin = resolve_origin(args)?; + let root = + Url::parse(&origin).map_err(|error| format!("invalid origin `{origin}`: {error}"))?; + + let root_response = client.fetch(&root)?; + let discovered = discover_asset_urls(&root, &root_response.body); + + let mut responses = Vec::with_capacity(discovered.len() + 1); + responses.push(FetchedResponse { + url: root.clone(), + headers: root_response.headers, + }); + + for url in discovered { + // A single asset fetch failing should not abort the whole audit. + match client.fetch(&url) { + Ok(response) => responses.push(FetchedResponse { + url, + headers: response.headers, + }), + Err(error) => log::warn!("skipping {url}: {error}"), + } + } + + Ok((origin, responses)) +} + +/// Resolves the origin from `--origin`, else from the config's +/// `publisher.origin_url`. +fn resolve_origin(args: &AuditHeadersArgs) -> CliResult { + if let Some(origin) = &args.origin { + return Ok(origin.clone()); + } + + let text = std::fs::read_to_string(&args.config).map_err(|error| { + format!( + "failed to read config {}: {error}; pass --origin to audit without a config", + args.config.display() + ) + })?; + let parsed: toml::Value = toml::from_str(&text) + .map_err(|error| format!("failed to parse {}: {error}", args.config.display()))?; + + parsed + .get("publisher") + .and_then(|publisher| publisher.get("origin_url")) + .and_then(toml::Value::as_str) + .map(str::to_owned) + .ok_or(()) + .or_else(|()| { + cli_error(format!( + "publisher.origin_url not found in {}", + args.config.display() + )) + }) +} + +/// Extracts asset URLs from HTML: ` + + + + "#; + let urls = discover_asset_urls(&base, html); + assert!( + urls.contains(&Url::parse("https://origin.example/app.js").unwrap()), + "should discover script src" + ); + assert!( + urls.contains(&Url::parse("https://origin.example/style.css").unwrap()), + "should discover stylesheet href" + ); + assert!( + urls.contains(&Url::parse("https://origin.example/logo.png").unwrap()), + "should discover img src" + ); + } + + #[test] + fn discovery_fetches_root_and_assets_but_never_ts_routes() { + let html = r#""#; + let client = FakeClient::new() + .with("https://origin.example/", "text/html", html) + .with("https://origin.example/logo.png", "image/png", "") + .with("https://origin.example/favicon.ico", "image/x-icon", ""); + + let (origin, responses) = + collect_responses(&args("https://origin.example"), &client).expect("should collect"); + + assert_eq!(origin, "https://origin.example"); + let fetched = client.fetched.borrow(); + assert!( + fetched.iter().any(|url| url == "https://origin.example/"), + "should fetch the root" + ); + assert!( + fetched + .iter() + .any(|url| url == "https://origin.example/logo.png"), + "should fetch the discovered image" + ); + assert!( + !fetched.iter().any(|url| url.contains("/_ts/")), + "should never probe edge-only /_ts/ routes against the origin" + ); + assert!( + !responses.is_empty(), + "should return at least the root response" + ); + } + + #[test] + fn explicit_urls_skip_discovery() { + let client = FakeClient::new().with("https://origin.example/rtb", "application/json", ""); + let explicit = AuditHeadersArgs { + urls: vec!["https://origin.example/rtb".to_owned()], + config: PathBuf::from("trusted-server.toml"), + origin: None, + json: false, + }; + let (_, responses) = collect_responses(&explicit, &client).expect("should collect"); + assert_eq!(responses.len(), 1, "should fetch only the explicit URL"); + assert_eq!( + client.fetched.borrow().len(), + 1, + "should not fetch a root page for explicit URLs" + ); + } + + #[test] + fn invalid_explicit_url_is_rejected() { + let client = FakeClient::new(); + let explicit = AuditHeadersArgs { + urls: vec!["not a url".to_owned()], + config: PathBuf::from("trusted-server.toml"), + origin: None, + json: false, + }; + let error = collect_responses(&explicit, &client).expect_err("should reject invalid URL"); + assert!(error.contains("invalid URL"), "should explain the failure"); + } +} diff --git a/crates/trusted-server-cli/src/commands/dev/audit/headers/mod.rs b/crates/trusted-server-cli/src/commands/dev/audit/headers/mod.rs new file mode 100644 index 0000000000..c4de91d70d --- /dev/null +++ b/crates/trusted-server-cli/src/commands/dev/audit/headers/mod.rs @@ -0,0 +1,194 @@ +//! `ts dev audit headers` — audits origin cache-header posture per content type. +//! +//! Fetches origin responses (explicit URLs, or discovered from the origin root), +//! classifies them by content type, evaluates each against the cacheability +//! rules, and renders a per-type pass/warn/fail report. The command exits 1 when +//! any content-type group fails, 2 when only warnings are present, and 0 when +//! all pass. + +mod analyze; +mod fetch; +mod output; +mod rules; + +use std::io::Write; + +pub use fetch::AuditHeadersArgs; + +use crate::error::CliResult; +use analyze::AuditReport; +use fetch::{OriginClient, ReqwestOriginClient, collect_responses}; + +/// Runs the header audit against a live origin, printing to stdout and returning +/// the process exit code (0 pass, 1 fail, 2 warn-only). +pub fn run(args: &AuditHeadersArgs) -> Result { + let client = ReqwestOriginClient::new()?; + let stdout = std::io::stdout(); + let mut out = stdout.lock(); + let report = run_audit_headers(args, &client, &mut out)?; + Ok(report.exit_code()) +} + +/// Fetches, analyzes, and renders the audit, returning the report. +/// +/// Kept separate from [`run`] — with the [`OriginClient`] and output sink +/// injected — so the full pipeline is unit-testable without network I/O or +/// touching stdout. +fn run_audit_headers( + args: &AuditHeadersArgs, + client: &dyn OriginClient, + out: &mut dyn Write, +) -> CliResult { + let (origin, responses) = collect_responses(args, client)?; + let report = analyze::run_analysis(&origin, &responses); + output::render(&report, args.json, out)?; + Ok(report) +} + +#[cfg(test)] +mod tests { + use std::cell::RefCell; + use std::collections::HashMap; + use std::path::PathBuf; + + use url::Url; + + use super::fetch::OriginResponse; + use super::rules::ResponseHeaders; + use super::*; + + struct FakeClient { + responses: HashMap, + fetched: RefCell>, + } + + impl FakeClient { + fn new() -> Self { + Self { + responses: HashMap::new(), + fetched: RefCell::new(Vec::new()), + } + } + + fn insert(&mut self, url: &str, headers: ResponseHeaders, body: &str) { + self.responses.insert( + url.to_owned(), + OriginResponse { + headers, + body: body.to_owned(), + }, + ); + } + } + + impl OriginClient for FakeClient { + fn fetch(&self, url: &Url) -> CliResult { + self.fetched.borrow_mut().push(url.to_string()); + self.responses + .get(url.as_str()) + .cloned() + .ok_or_else(|| format!("no fake response for {url}")) + } + } + + fn args_for(origin: &str, json: bool) -> AuditHeadersArgs { + AuditHeadersArgs { + urls: Vec::new(), + config: PathBuf::from("trusted-server.toml"), + origin: Some(origin.to_owned()), + json, + } + } + + #[test] + fn end_to_end_discovery_produces_report() { + let html = r#" + + "#; + + let mut client = FakeClient::new(); + client.insert( + "https://origin.example/", + ResponseHeaders { + content_type: Some("text/html".to_owned()), + cache_control: Some("no-store".to_owned()), + ..ResponseHeaders::default() + }, + html, + ); + client.insert( + "https://origin.example/app.js", + ResponseHeaders { + content_type: Some("application/javascript".to_owned()), + cache_control: Some("public, max-age=31536000, immutable".to_owned()), + surrogate_key: Some("js".to_owned()), + etag: Some("\"abc\"".to_owned()), + ..ResponseHeaders::default() + }, + "", + ); + client.insert( + "https://origin.example/favicon.ico", + ResponseHeaders { + content_type: Some("image/x-icon".to_owned()), + cache_control: Some("public, max-age=86400".to_owned()), + surrogate_key: Some("icon".to_owned()), + ..ResponseHeaders::default() + }, + "", + ); + + let mut buffer = Vec::new(); + let report = run_audit_headers( + &args_for("https://origin.example", false), + &client, + &mut buffer, + ) + .expect("should run end to end"); + + assert!( + report.summary.total_groups >= 2, + "should audit at least HTML and JS groups" + ); + assert_eq!(report.exit_code(), 0, "clean posture should exit 0"); + let text = String::from_utf8(buffer).expect("should be utf-8"); + assert!(text.contains("Origin:"), "should render the report"); + } + + #[test] + fn failing_group_sets_exit_code_one() { + let mut client = FakeClient::new(); + client.insert( + "https://origin.example/", + ResponseHeaders { + content_type: Some("text/html".to_owned()), + cache_control: Some("public, max-age=3600".to_owned()), + ..ResponseHeaders::default() + }, + "", + ); + client.insert( + "https://origin.example/favicon.ico", + ResponseHeaders { + content_type: Some("image/x-icon".to_owned()), + cache_control: Some("public, max-age=86400".to_owned()), + surrogate_key: Some("icon".to_owned()), + ..ResponseHeaders::default() + }, + "", + ); + + let mut buffer = Vec::new(); + let report = run_audit_headers( + &args_for("https://origin.example", true), + &client, + &mut buffer, + ) + .expect("should run"); + assert_eq!( + report.exit_code(), + 1, + "cacheable HTML should drive a failing exit code" + ); + } +} diff --git a/crates/trusted-server-cli/src/commands/dev/audit/headers/output.rs b/crates/trusted-server-cli/src/commands/dev/audit/headers/output.rs new file mode 100644 index 0000000000..972b3ebaf6 --- /dev/null +++ b/crates/trusted-server-cli/src/commands/dev/audit/headers/output.rs @@ -0,0 +1,159 @@ +//! Renders an [`AuditReport`] as a human-readable table or as JSON. +//! +//! Output is written to an injected `Write` sink (rather than `println!`) so the +//! rendering is unit-testable, following the `config init` and `prebid bundle` +//! commands. + +use std::io::Write; + +use super::analyze::AuditReport; +use super::rules::Verdict; +use crate::error::CliResult; + +/// Writes the report to `out` in the format selected by `json`. +pub(crate) fn render(report: &AuditReport, json: bool, out: &mut dyn Write) -> CliResult<()> { + if json { + render_json(report, out) + } else { + render_human(report, out) + } +} + +fn render_json(report: &AuditReport, out: &mut dyn Write) -> CliResult<()> { + let serialized = serde_json::to_string_pretty(report) + .map_err(|error| format!("failed to serialize audit report: {error}"))?; + writeln!(out, "{serialized}").map_err(|error| write_error(&error)) +} + +fn render_human(report: &AuditReport, out: &mut dyn Write) -> CliResult<()> { + writeln!(out, "Origin: {}", report.origin).map_err(|error| write_error(&error))?; + writeln!(out).map_err(|error| write_error(&error))?; + + if report.groups.is_empty() { + writeln!(out, "No auditable content types found.").map_err(|error| write_error(&error))?; + return Ok(()); + } + + for group in &report.groups { + writeln!( + out, + "{} {}", + verdict_glyph(group.verdict), + group.content_type.label() + ) + .map_err(|error| write_error(&error))?; + + for verdict in &group.verdicts { + let recommendation = if verdict.recommendation.is_empty() { + "--" + } else { + &verdict.recommendation + }; + writeln!( + out, + " {} {:<18} {}", + verdict_glyph(verdict.verdict), + verdict.header, + recommendation + ) + .map_err(|error| write_error(&error))?; + } + writeln!(out).map_err(|error| write_error(&error))?; + } + + writeln!( + out, + "Summary (per content type): {} pass, {} warn, {} fail ({} types audited)", + report.summary.pass, report.summary.warn, report.summary.fail, report.summary.total_groups + ) + .map_err(|error| write_error(&error))?; + + Ok(()) +} + +/// A terminal glyph for a verdict. ASCII-safe fallbacks are unnecessary; the +/// glyphs match the spec's sample output. +fn verdict_glyph(verdict: Verdict) -> char { + match verdict { + Verdict::Pass => '✓', + Verdict::Warn => '⚠', + Verdict::Fail => '✗', + } +} + +fn write_error(error: &std::io::Error) -> String { + format!("failed to write command output: {error}") +} + +#[cfg(test)] +mod tests { + use url::Url; + + use super::super::analyze::{FetchedResponse, run_analysis}; + use super::super::rules::ResponseHeaders; + use super::*; + + fn sample_report() -> AuditReport { + let responses = vec![ + FetchedResponse { + url: Url::parse("https://origin.example/").expect("should parse"), + headers: ResponseHeaders { + content_type: Some("text/html".to_owned()), + cache_control: Some("public, max-age=3600".to_owned()), + ..ResponseHeaders::default() + }, + }, + FetchedResponse { + url: Url::parse("https://origin.example/app.js").expect("should parse"), + headers: ResponseHeaders { + content_type: Some("application/javascript".to_owned()), + cache_control: Some("public, max-age=31536000, immutable".to_owned()), + surrogate_key: Some("js".to_owned()), + etag: Some("\"abc\"".to_owned()), + ..ResponseHeaders::default() + }, + }, + ]; + run_analysis("https://origin.example", &responses) + } + + #[test] + fn human_output_includes_origin_and_summary() { + let report = sample_report(); + let mut buffer = Vec::new(); + render(&report, false, &mut buffer).expect("should render"); + let text = String::from_utf8(buffer).expect("should be utf-8"); + assert!( + text.contains("Origin: https://origin.example"), + "should print the origin" + ); + assert!( + text.contains("Summary (per content type):"), + "should print the summary line" + ); + assert!(text.contains("HTML"), "should list the HTML group"); + } + + #[test] + fn json_output_is_valid_and_stable() { + let report = sample_report(); + let mut buffer = Vec::new(); + render(&report, true, &mut buffer).expect("should render"); + let text = String::from_utf8(buffer).expect("should be utf-8"); + let parsed: serde_json::Value = + serde_json::from_str(&text).expect("should emit valid JSON"); + assert_eq!( + parsed["origin"], "https://origin.example", + "should serialize origin" + ); + assert_eq!( + parsed["summary"]["total_groups"], 2, + "should count both groups" + ); + // Verdicts serialize as bare strings, not payload-carrying objects. + assert_eq!( + parsed["groups"][0]["verdict"], "Fail", + "cacheable HTML group should serialize a bare Fail verdict" + ); + } +} diff --git a/crates/trusted-server-cli/src/commands/dev/audit/headers/rules.rs b/crates/trusted-server-cli/src/commands/dev/audit/headers/rules.rs new file mode 100644 index 0000000000..bc9dc9bf7d --- /dev/null +++ b/crates/trusted-server-cli/src/commands/dev/audit/headers/rules.rs @@ -0,0 +1,766 @@ +//! Content-type taxonomy and per-group cacheability rules. +//! +//! The audit classifies each origin response into a [`ContentTypeGroup`] by its +//! `Content-Type`, then evaluates the cache-related response headers against the +//! expected posture for that group. Evaluation is pure — it takes already-parsed +//! header values (see [`ResponseHeaders`]) and returns a list of per-header +//! [`HeaderVerdict`]s, so it is exercised entirely by unit tests without any I/O. + +use std::collections::BTreeSet; + +use serde::Serialize; + +/// One year in seconds — the `max-age` floor for immutable, hashed assets. +const IMMUTABLE_MAX_AGE: u64 = 31_536_000; + +/// One day in seconds — the `max-age` floor for creatives/images. +const IMAGE_MIN_MAX_AGE: u64 = 86_400; + +/// The content-type family a response belongs to. +/// +/// Classification strips `Content-Type` parameters (`text/html; charset=utf-8` +/// becomes `text/html`) and lowercases the value before matching. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +pub(crate) enum ContentTypeGroup { + /// `text/html` — personalized, consent-sensitive. + Html, + /// Hashed, immutable script bundles. + JavaScript, + /// `image/*` creatives. + Image, + /// CSS and fonts. + StaticAsset, + /// `application/json` real-time bidding responses. + RtbJson, + /// Everything else — reported for information, never evaluated. + Other, +} + +impl ContentTypeGroup { + /// Classifies a raw `Content-Type` header value into a group. + /// + /// Parameters after the first `;` are ignored and matching is + /// case-insensitive, so `text/html; charset=utf-8` maps to + /// [`ContentTypeGroup::Html`]. + pub(crate) fn classify(content_type: &str) -> Self { + let mime = content_type + .split(';') + .next() + .unwrap_or(content_type) + .trim() + .to_ascii_lowercase(); + + match mime.as_str() { + "text/html" => Self::Html, + "application/javascript" | "text/javascript" | "application/x-javascript" => { + Self::JavaScript + } + "application/json" => Self::RtbJson, + "text/css" => Self::StaticAsset, + other if other.starts_with("image/") => Self::Image, + other if other.starts_with("font/") || other.starts_with("application/font-") => { + Self::StaticAsset + } + _ => Self::Other, + } + } + + /// Whether this group is expected to be CDN-cacheable, and therefore + /// evaluated for `Surrogate-Key` purge coverage. Non-cacheable groups + /// (`Html`, `RtbJson`) never need selective purge, so the check is skipped. + fn is_cacheable(self) -> bool { + matches!(self, Self::JavaScript | Self::Image | Self::StaticAsset) + } + + /// A short, stable display label for terminal output. + pub(crate) fn label(self) -> &'static str { + match self { + Self::Html => "HTML", + Self::JavaScript => "JS Bundle", + Self::Image => "Image", + Self::StaticAsset => "Static Asset", + Self::RtbJson => "RTB/JSON", + Self::Other => "Other", + } + } +} + +/// The outcome of a single header (or group) check. +/// +/// Unit variants only — the human-readable message lives on +/// [`HeaderVerdict::recommendation`], and serializes to a bare string +/// (`"Pass"`, `"Warn"`, `"Fail"`) for the `--json` output. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +pub(crate) enum Verdict { + /// Header posture matches the expected value. + Pass, + /// Suboptimal but not incorrect — hit ratio or purge ergonomics suffer. + Warn, + /// Incorrect — risks stale or cross-user content. + Fail, +} + +impl Verdict { + /// Combines two verdicts into the more severe of the two. + fn worst(self, other: Self) -> Self { + match (self, other) { + (Self::Fail, _) | (_, Self::Fail) => Self::Fail, + (Self::Warn, _) | (_, Self::Warn) => Self::Warn, + _ => Self::Pass, + } + } + + /// Folds an iterator of verdicts into a single worst-of rollup, defaulting + /// to [`Verdict::Pass`] when empty. + pub(crate) fn rollup(verdicts: impl IntoIterator) -> Self { + verdicts.into_iter().fold(Self::Pass, Self::worst) + } +} + +/// A single header's evaluation result within a content-type group. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub(crate) struct HeaderVerdict { + /// The header this verdict is about (e.g. `Cache-Control`). + pub(crate) header: String, + /// Pass / warn / fail for this header. + pub(crate) verdict: Verdict, + /// The observed header value, if the header was present. + pub(crate) actual: Option, + /// The expected posture, described briefly. + pub(crate) expected: String, + /// What to change to satisfy the rule (empty when passing). + pub(crate) recommendation: String, +} + +impl HeaderVerdict { + fn pass(header: &str, actual: Option, expected: &str) -> Self { + Self { + header: header.to_owned(), + verdict: Verdict::Pass, + actual, + expected: expected.to_owned(), + recommendation: String::new(), + } + } + + fn flagged( + header: &str, + verdict: Verdict, + actual: Option, + expected: &str, + recommendation: &str, + ) -> Self { + Self { + header: header.to_owned(), + verdict, + actual, + expected: expected.to_owned(), + recommendation: recommendation.to_owned(), + } + } +} + +/// The parsed, cache-relevant response headers for one origin response. +/// +/// Extracted from the transport layer in `fetch.rs` so this module stays free +/// of any HTTP-client types and is unit-testable in isolation. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub(crate) struct ResponseHeaders { + /// `Content-Type` — drives group classification. + pub(crate) content_type: Option, + /// `Cache-Control`. + pub(crate) cache_control: Option, + /// `Surrogate-Control` (Fastly CDN-only directives). + pub(crate) surrogate_control: Option, + /// `Surrogate-Key` (Fastly purge tags). + pub(crate) surrogate_key: Option, + /// `Vary`. + pub(crate) vary: Option, + /// `ETag`. + pub(crate) etag: Option, +} + +/// A parsed `Cache-Control` (or `Surrogate-Control`) value. +/// +/// Directives are lowercased tokens; `max-age` and `s-maxage` are captured +/// separately because Fastly resolves the shared-cache TTL from +/// `Surrogate-Control` first, then `s-maxage`, then `max-age`. +struct CacheDirectives { + tokens: BTreeSet, + max_age: Option, + s_maxage: Option, +} + +impl CacheDirectives { + fn parse(value: &str) -> Self { + let mut tokens = BTreeSet::new(); + let mut max_age = None; + let mut s_maxage = None; + + for raw in value.split(',') { + let token = raw.trim().to_ascii_lowercase(); + if token.is_empty() { + continue; + } + if let Some((name, seconds)) = token.split_once('=') { + let seconds = seconds.trim_matches('"').parse::().ok(); + match name.trim() { + "max-age" => max_age = seconds, + "s-maxage" => s_maxage = seconds, + _ => {} + } + tokens.insert(name.trim().to_owned()); + } else { + tokens.insert(token); + } + } + + Self { + tokens, + max_age, + s_maxage, + } + } + + fn has(&self, directive: &str) -> bool { + self.tokens.contains(directive) + } +} + +/// Whether a `Vary` value contains a hit-ratio-destroying token. +fn vary_has_high_cardinality(vary: &str) -> bool { + vary.split(',').any(|token| { + let token = token.trim().to_ascii_lowercase(); + token == "user-agent" || token == "cookie" + }) +} + +/// Evaluates the cache posture of a single response for its content-type group. +/// +/// Returns one [`HeaderVerdict`] per header the group cares about. +/// [`ContentTypeGroup::Other`] is never evaluated and yields an empty list. +pub(crate) fn evaluate(group: ContentTypeGroup, headers: &ResponseHeaders) -> Vec { + match group { + ContentTypeGroup::Html => evaluate_html(headers), + ContentTypeGroup::JavaScript => evaluate_immutable(group, headers, "JS bundles"), + ContentTypeGroup::Image => evaluate_image(headers), + ContentTypeGroup::StaticAsset => evaluate_immutable(group, headers, "static assets"), + ContentTypeGroup::RtbJson => evaluate_rtb(headers), + ContentTypeGroup::Other => Vec::new(), + } +} + +/// HTML must not be shared across users: `no-store` alone is sufficient +/// (RFC 9111 §5.2.2.5), or `private` combined with `no-cache`. +fn evaluate_html(headers: &ResponseHeaders) -> Vec { + let mut verdicts = Vec::new(); + + let expected = "no-store, or private + no-cache"; + match headers.cache_control.as_deref() { + Some(value) => { + let directives = CacheDirectives::parse(value); + let safe = directives.has("no-store") + || (directives.has("private") && directives.has("no-cache")); + if safe { + verdicts.push(HeaderVerdict::pass( + "Cache-Control", + Some(value.to_owned()), + expected, + )); + } else { + verdicts.push(HeaderVerdict::flagged( + "Cache-Control", + Verdict::Fail, + Some(value.to_owned()), + expected, + "HTML served without no-store (or private + no-cache) risks sharing personalized content across users", + )); + } + } + None => verdicts.push(HeaderVerdict::flagged( + "Cache-Control", + Verdict::Fail, + None, + expected, + "HTML has no Cache-Control; set `no-store` to prevent shared caching of personalized content", + )), + } + + verdicts.extend(evaluate_vary(headers, /* flag_wildcard */ true)); + verdicts.extend(evaluate_surrogate_no_store( + headers, + "CDN may cache personalized HTML; set Surrogate-Control to `no-store` or `private`", + )); + + verdicts +} + +/// JS bundles and static assets share the immutable, long-lived posture. +fn evaluate_immutable( + group: ContentTypeGroup, + headers: &ResponseHeaders, + noun: &str, +) -> Vec { + let mut verdicts = Vec::new(); + + let expected = "public, max-age>=31536000, immutable"; + match headers.cache_control.as_deref() { + Some(value) => { + let directives = CacheDirectives::parse(value); + let immutable = directives.has("public") + && directives.has("immutable") + && directives + .max_age + .is_some_and(|age| age >= IMMUTABLE_MAX_AGE); + if immutable { + verdicts.push(HeaderVerdict::pass( + "Cache-Control", + Some(value.to_owned()), + expected, + )); + } else { + verdicts.push(HeaderVerdict::flagged( + "Cache-Control", + Verdict::Warn, + Some(value.to_owned()), + expected, + &format!("Hashed {noun} should use long-lived immutable caching"), + )); + } + } + None => verdicts.push(HeaderVerdict::flagged( + "Cache-Control", + Verdict::Warn, + None, + expected, + &format!("{noun} have no Cache-Control; set `public, max-age=31536000, immutable`"), + )), + } + + if matches!(group, ContentTypeGroup::JavaScript) { + verdicts.push(evaluate_etag(headers)); + } + verdicts.extend(evaluate_surrogate_key(group, headers)); + verdicts.extend(evaluate_vary(headers, /* flag_wildcard */ false)); + + verdicts +} + +/// Images should be publicly cacheable for at least a day, and the CDN TTL +/// should not undercut the browser TTL. +fn evaluate_image(headers: &ResponseHeaders) -> Vec { + let mut verdicts = Vec::new(); + + let expected = "public, max-age>=86400"; + let cache_control = headers.cache_control.as_deref().map(CacheDirectives::parse); + match (&headers.cache_control, &cache_control) { + (Some(value), Some(directives)) => { + let cacheable = directives.has("public") + && directives + .max_age + .is_some_and(|age| age >= IMAGE_MIN_MAX_AGE); + if cacheable { + verdicts.push(HeaderVerdict::pass( + "Cache-Control", + Some(value.clone()), + expected, + )); + } else { + verdicts.push(HeaderVerdict::flagged( + "Cache-Control", + Verdict::Warn, + Some(value.clone()), + expected, + "Images re-fetched too frequently; set `public, max-age=86400` or longer", + )); + } + } + _ => verdicts.push(HeaderVerdict::flagged( + "Cache-Control", + Verdict::Warn, + None, + expected, + "Images have no Cache-Control; set `public, max-age=86400` or longer", + )), + } + + // The CDN TTL (Surrogate-Control max-age, else s-maxage) should be at least + // the browser TTL, so the edge does not re-fetch more often than clients. + if let Some(surrogate) = headers.surrogate_control.as_deref() { + let surrogate_directives = CacheDirectives::parse(surrogate); + let cdn_ttl = surrogate_directives + .max_age + .or(surrogate_directives.s_maxage); + let browser_ttl = cache_control + .as_ref() + .and_then(|directives| directives.max_age); + if let (Some(cdn), Some(browser)) = (cdn_ttl, browser_ttl) { + if cdn < browser { + verdicts.push(HeaderVerdict::flagged( + "Surrogate-Control", + Verdict::Warn, + Some(surrogate.to_owned()), + "CDN max-age >= browser max-age", + "CDN caching shorter than browser caching; raise Surrogate-Control max-age (or s-maxage)", + )); + } else { + verdicts.push(HeaderVerdict::pass( + "Surrogate-Control", + Some(surrogate.to_owned()), + "CDN max-age >= browser max-age", + )); + } + } + } + + verdicts.extend(evaluate_surrogate_key(ContentTypeGroup::Image, headers)); + verdicts.extend(evaluate_vary(headers, /* flag_wildcard */ false)); + + verdicts +} + +/// RTB/JSON must never be cached: `no-store` is required (RFC 9111 §5.2.2.5). +fn evaluate_rtb(headers: &ResponseHeaders) -> Vec { + let mut verdicts = Vec::new(); + + let expected = "no-store"; + match headers.cache_control.as_deref() { + Some(value) if CacheDirectives::parse(value).has("no-store") => { + verdicts.push(HeaderVerdict::pass( + "Cache-Control", + Some(value.to_owned()), + expected, + )); + } + Some(value) => verdicts.push(HeaderVerdict::flagged( + "Cache-Control", + Verdict::Fail, + Some(value.to_owned()), + expected, + "RTB responses cached means stale bids served to users; set `no-store`", + )), + None => verdicts.push(HeaderVerdict::flagged( + "Cache-Control", + Verdict::Fail, + None, + expected, + "RTB responses have no Cache-Control; set `no-store`", + )), + } + + verdicts.extend(evaluate_surrogate_no_store( + headers, + "CDN may cache RTB responses; set Surrogate-Control to `no-store`", + )); + + verdicts +} + +/// `Surrogate-Control`, when present, must not permit CDN caching for +/// non-cacheable groups (HTML, RTB). +fn evaluate_surrogate_no_store( + headers: &ResponseHeaders, + recommendation: &str, +) -> Option { + let value = headers.surrogate_control.as_deref()?; + let directives = CacheDirectives::parse(value); + let safe = directives.has("no-store") || directives.has("private"); + Some(if safe { + HeaderVerdict::pass( + "Surrogate-Control", + Some(value.to_owned()), + "no-store or private", + ) + } else { + HeaderVerdict::flagged( + "Surrogate-Control", + Verdict::Fail, + Some(value.to_owned()), + "no-store or private", + recommendation, + ) + }) +} + +/// `Surrogate-Key` enables targeted CDN purge; absence is a warning on +/// cacheable groups only. +fn evaluate_surrogate_key( + group: ContentTypeGroup, + headers: &ResponseHeaders, +) -> Option { + if !group.is_cacheable() { + return None; + } + Some(match headers.surrogate_key.as_deref() { + Some(value) => HeaderVerdict::pass("Surrogate-Key", Some(value.to_owned()), "present"), + None => HeaderVerdict::flagged( + "Surrogate-Key", + Verdict::Warn, + None, + "present", + "Without Surrogate-Key, this content type can't be selectively purged from the CDN — full-cache purges required", + ), + }) +} + +/// `ETag` enables conditional revalidation; absence is a warning. +fn evaluate_etag(headers: &ResponseHeaders) -> HeaderVerdict { + match headers.etag.as_deref() { + Some(value) => HeaderVerdict::pass("ETag", Some(value.to_owned()), "present"), + None => HeaderVerdict::flagged( + "ETag", + Verdict::Warn, + None, + "present", + "Missing ETag means no conditional revalidation", + ), + } +} + +/// `Vary` checks: `*` disables all caching (HTML only), and `User-Agent` / +/// `Cookie` destroy the CDN hit ratio on any group. +fn evaluate_vary(headers: &ResponseHeaders, flag_wildcard: bool) -> Option { + let value = headers.vary.as_deref()?; + + if flag_wildcard && value.split(',').any(|token| token.trim() == "*") { + return Some(HeaderVerdict::flagged( + "Vary", + Verdict::Warn, + Some(value.to_owned()), + "no `*`", + "Vary: * disables all caching including the CDN", + )); + } + + if vary_has_high_cardinality(value) { + return Some(HeaderVerdict::flagged( + "Vary", + Verdict::Warn, + Some(value.to_owned()), + "no User-Agent or Cookie", + "Vary on User-Agent/Cookie destroys the CDN hit ratio", + )); + } + + Some(HeaderVerdict::pass( + "Vary", + Some(value.to_owned()), + "no `*`, User-Agent, or Cookie", + )) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn headers_with(cache_control: Option<&str>) -> ResponseHeaders { + ResponseHeaders { + cache_control: cache_control.map(str::to_owned), + ..ResponseHeaders::default() + } + } + + fn verdict_for(verdicts: &[HeaderVerdict], header: &str) -> Verdict { + verdicts + .iter() + .find(|verdict| verdict.header == header) + .unwrap_or_else(|| panic!("should have a verdict for {header}")) + .verdict + } + + #[test] + fn classify_strips_parameters_and_lowercases() { + assert_eq!( + ContentTypeGroup::classify("text/HTML; charset=utf-8"), + ContentTypeGroup::Html, + "should ignore charset parameter and case" + ); + assert_eq!( + ContentTypeGroup::classify("image/png"), + ContentTypeGroup::Image, + "should match image/* prefix" + ); + assert_eq!( + ContentTypeGroup::classify("font/woff2"), + ContentTypeGroup::StaticAsset, + "should treat fonts as static assets" + ); + assert_eq!( + ContentTypeGroup::classify("application/font-woff"), + ContentTypeGroup::StaticAsset, + "should treat legacy font MIME as static assets" + ); + assert_eq!( + ContentTypeGroup::classify("application/octet-stream"), + ContentTypeGroup::Other, + "should fall through to Other" + ); + } + + #[test] + fn html_passes_on_no_store_alone() { + let verdicts = evaluate(ContentTypeGroup::Html, &headers_with(Some("no-store"))); + assert_eq!( + verdict_for(&verdicts, "Cache-Control"), + Verdict::Pass, + "no-store alone should satisfy RFC 9111 for HTML" + ); + } + + #[test] + fn html_passes_on_private_no_cache() { + let verdicts = evaluate( + ContentTypeGroup::Html, + &headers_with(Some("private, no-cache")), + ); + assert_eq!( + verdict_for(&verdicts, "Cache-Control"), + Verdict::Pass, + "private + no-cache should satisfy HTML" + ); + } + + #[test] + fn html_fails_on_public_cacheable() { + let verdicts = evaluate( + ContentTypeGroup::Html, + &headers_with(Some("public, max-age=3600")), + ); + assert_eq!( + verdict_for(&verdicts, "Cache-Control"), + Verdict::Fail, + "publicly cacheable HTML should fail" + ); + } + + #[test] + fn rtb_passes_on_no_store_without_private() { + let verdicts = evaluate(ContentTypeGroup::RtbJson, &headers_with(Some("no-store"))); + assert_eq!( + verdict_for(&verdicts, "Cache-Control"), + Verdict::Pass, + "bare no-store should pass for RTB" + ); + } + + #[test] + fn javascript_warns_without_immutable() { + let verdicts = evaluate( + ContentTypeGroup::JavaScript, + &headers_with(Some("public, max-age=31536000")), + ); + assert_eq!( + verdict_for(&verdicts, "Cache-Control"), + Verdict::Warn, + "missing immutable should warn" + ); + } + + #[test] + fn javascript_passes_on_full_immutable_posture() { + let verdicts = evaluate( + ContentTypeGroup::JavaScript, + &ResponseHeaders { + cache_control: Some("public, max-age=31536000, immutable".to_owned()), + surrogate_key: Some("js".to_owned()), + etag: Some("\"abc\"".to_owned()), + ..ResponseHeaders::default() + }, + ); + assert_eq!( + Verdict::rollup(verdicts.iter().map(|verdict| verdict.verdict)), + Verdict::Pass, + "full immutable posture with ETag and Surrogate-Key should pass" + ); + } + + #[test] + fn surrogate_key_skipped_for_non_cacheable_groups() { + let verdicts = evaluate(ContentTypeGroup::Html, &headers_with(Some("no-store"))); + assert!( + !verdicts + .iter() + .any(|verdict| verdict.header == "Surrogate-Key"), + "HTML should not be checked for Surrogate-Key" + ); + } + + #[test] + fn vary_wildcard_warns_only_for_html() { + let html = evaluate( + ContentTypeGroup::Html, + &ResponseHeaders { + cache_control: Some("no-store".to_owned()), + vary: Some("*".to_owned()), + ..ResponseHeaders::default() + }, + ); + assert_eq!( + verdict_for(&html, "Vary"), + Verdict::Warn, + "Vary: * should warn on HTML" + ); + } + + #[test] + fn vary_high_cardinality_warns_on_assets() { + let verdicts = evaluate( + ContentTypeGroup::JavaScript, + &ResponseHeaders { + cache_control: Some("public, max-age=31536000, immutable".to_owned()), + surrogate_key: Some("js".to_owned()), + etag: Some("\"abc\"".to_owned()), + vary: Some("User-Agent".to_owned()), + ..ResponseHeaders::default() + }, + ); + assert_eq!( + verdict_for(&verdicts, "Vary"), + Verdict::Warn, + "Vary: User-Agent should warn on JS assets" + ); + } + + #[test] + fn image_warns_when_cdn_ttl_below_browser_ttl() { + let verdicts = evaluate( + ContentTypeGroup::Image, + &ResponseHeaders { + cache_control: Some("public, max-age=86400".to_owned()), + surrogate_control: Some("max-age=60".to_owned()), + surrogate_key: Some("img".to_owned()), + ..ResponseHeaders::default() + }, + ); + assert_eq!( + verdict_for(&verdicts, "Surrogate-Control"), + Verdict::Warn, + "CDN TTL below browser TTL should warn" + ); + } + + #[test] + fn other_group_is_never_evaluated() { + let verdicts = evaluate(ContentTypeGroup::Other, &headers_with(Some("whatever"))); + assert!(verdicts.is_empty(), "Other should yield no verdicts"); + } + + #[test] + fn rollup_is_worst_of() { + assert_eq!( + Verdict::rollup([Verdict::Pass, Verdict::Warn, Verdict::Fail]), + Verdict::Fail, + "any fail should dominate" + ); + assert_eq!( + Verdict::rollup([Verdict::Pass, Verdict::Warn]), + Verdict::Warn, + "warn should dominate pass" + ); + assert_eq!( + Verdict::rollup(std::iter::empty()), + Verdict::Pass, + "empty should default to pass" + ); + } +} diff --git a/crates/trusted-server-cli/src/commands/dev/audit/mod.rs b/crates/trusted-server-cli/src/commands/dev/audit/mod.rs new file mode 100644 index 0000000000..cdc8938180 --- /dev/null +++ b/crates/trusted-server-cli/src/commands/dev/audit/mod.rs @@ -0,0 +1,40 @@ +//! The `ts dev audit …` command group. +//! +//! Groups origin-audit subcommands under `ts dev audit`. Today the only member +//! is `headers`, which audits cache-header posture per content type. Unlike +//! `ts dev proxy` (macOS-only), audit subcommands are available on all host +//! platforms — they have no OS-specific dependencies. + +mod headers; + +use std::process; + +pub use headers::AuditHeadersArgs; + +/// Subcommands of `ts dev audit`. +#[derive(Debug, clap::Subcommand)] +pub enum DevAuditCommand { + /// Audit origin cache-related response headers per content type. + Headers(AuditHeadersArgs), +} + +/// Dispatches a `dev audit` subcommand. +/// +/// # Errors +/// +/// Returns the subcommand's failure message when fetching, config resolution, +/// or output rendering fails. +pub fn run(command: &DevAuditCommand) -> Result<(), String> { + match command { + DevAuditCommand::Headers(args) => { + let exit_code = headers::run(args)?; + // 0 means every content-type group passed; 1 (a group failed) and 2 + // (warnings only) are surfaced as process exit codes for CI gating. + if exit_code == 0 { + Ok(()) + } else { + process::exit(exit_code) + } + } + } +} diff --git a/crates/trusted-server-cli/src/commands/dev/mod.rs b/crates/trusted-server-cli/src/commands/dev/mod.rs index 7a61d769bc..3bd5ec84cb 100644 --- a/crates/trusted-server-cli/src/commands/dev/mod.rs +++ b/crates/trusted-server-cli/src/commands/dev/mod.rs @@ -1,12 +1,17 @@ // `ts dev proxy` is macOS-only; its dependencies are scoped to macOS in -// `Cargo.toml`, so the module and the `Proxy` subcommand only exist there. On -// other host targets `ts dev` parses but exposes no subcommands. +// `Cargo.toml`, so the module and the `Proxy` subcommand only exist there. The +// `ts dev audit` group has no OS-specific dependencies and is available on +// every host target. +pub mod audit; #[cfg(target_os = "macos")] pub mod proxy; /// The `ts dev …` command group. #[derive(Debug, clap::Subcommand)] pub enum DevCommand { + /// Audit origin responses (e.g. cache headers per content type). + #[command(subcommand)] + Audit(audit::DevAuditCommand), /// Run the local production-hostname dev proxy (macOS only). #[cfg(target_os = "macos")] Proxy(proxy::ProxyArgs), @@ -15,21 +20,10 @@ pub enum DevCommand { /// Dispatches a `dev` subcommand. /// /// # Errors -/// Returns the subcommand's failure rendered as a message. On non-macOS targets -/// `DevCommand` has no variants, so this never returns an error there. -// On non-macOS targets `DevCommand` is an empty enum: the by-value parameter is -// consumed by an empty `match`, which clippy reads as a needless by-value pass. -// Taking `&DevCommand` is not an option — a zero-arm `match` is not exhaustive -// over a reference type — so the owned parameter is required. -#[cfg_attr( - not(target_os = "macos"), - allow( - clippy::needless_pass_by_value, - reason = "empty enum requires owned value for exhaustive match" - ) -)] +/// Returns the subcommand's failure rendered as a message. pub fn run(command: DevCommand) -> Result<(), String> { match command { + DevCommand::Audit(command) => audit::run(&command), #[cfg(target_os = "macos")] DevCommand::Proxy(args) => proxy::run(&args).map_err(|report| format!("{report:?}")), } From 6cf2d03d0644a44935306d1335d5412e7c6a0903 Mon Sep 17 00:00:00 2001 From: Vasu Jain Date: Wed, 23 Sep 2026 08:05:02 -0700 Subject: [PATCH 2/7] fix(cli): address PR #1172 review on ts dev audit headers Blockers (aram356 + prk-Jr): - Cargo.toml: pin reqwest to rustls-tls-webpki-roots-no-provider instead of inheriting the workspace rustls-tls feature (which pulled rustls/ring alongside the proxy's aws-lc-rs and made rustls unable to pick a process CryptoProvider -> ts dev proxy TLS panics, 4 CI failures). Because the proxy (and thus its aws-lc-rs) is macOS-only, the -no-provider variant leaves the audit's own reqwest with no provider on Linux; so also add rustls (aws-lc-rs) to the all-host deps and install_default() it in ReqwestOriginClient::new(). Verified at runtime on Linux: audit runs, proxy stays single-provider. - Exit code: warn-only now exits 3, not 2 (2 is main's generic CLI-error code). - Vary: * now flagged on JS/CSS/image groups. - Discovery restricted to the audited origin (+ --include-cross-origin opt-in); redirects no longer followed (Policy::none). Non-blocking: - private alone now passes HTML (RFC 9111 5.2.2.7). - Cache-Control s-maxage honored as CDN TTL without Surrogate-Control (labeled distinctly so it does not collide with the cacheability verdict). - Surrogate-Control no-store/private/no-cache flagged on cacheable groups. - Directive parsing: trim whitespace around '='; first-valid-occurrence-wins so a trailing malformed/duplicate directive can't null a good value (RFC 4.2.1). - Bounded body reads (2 MiB); explicit-URL path tolerates per-URL failures and hard-errors only if all fail. - Added docs/guide/cli.md section with the exit-code contract. 11 new unit tests + end-to-end runtime verification against a local origin. 445 CLI tests pass, clippy -D warnings clean, fmt clean, Cargo.lock unchanged. Refs #834 --- crates/trusted-server-cli/Cargo.toml | 20 +- .../src/commands/dev/audit/headers/analyze.rs | 17 +- .../src/commands/dev/audit/headers/fetch.rs | 147 +++++++++- .../src/commands/dev/audit/headers/mod.rs | 9 +- .../src/commands/dev/audit/headers/rules.rs | 256 +++++++++++++++--- .../src/commands/dev/audit/mod.rs | 6 +- docs/guide/cli.md | 55 ++++ 7 files changed, 451 insertions(+), 59 deletions(-) diff --git a/crates/trusted-server-cli/Cargo.toml b/crates/trusted-server-cli/Cargo.toml index 653c0478c0..6c03c14aae 100644 --- a/crates/trusted-server-cli/Cargo.toml +++ b/crates/trusted-server-cli/Cargo.toml @@ -29,7 +29,25 @@ regex = { workspace = true } # `ts dev audit headers` fetches origin responses over HTTP. Scoped to host # targets (not the repo-default wasm32-wasip1) alongside the other native-only # deps so unsupported targets do not try to build reqwest's TLS stack. -reqwest = { workspace = true, features = ["blocking"] } +# +# Deliberately NOT `workspace = true`: the workspace feature list enables +# `rustls-tls`, which resolves to reqwest's `__rustls-ring` and turns on +# `rustls/ring` on top of the `rustls/aws-lc-rs` that `ts dev proxy` uses on +# macOS. With both providers compiled in, rustls cannot determine a +# process-level `CryptoProvider` and every `ts dev proxy` TLS config build +# panics. The `-no-provider` variant bakes in no provider, so it never adds +# `ring` and cannot collide with the proxy's aws-lc-rs. +reqwest = { version = "0.12", default-features = false, features = [ + "blocking", + "rustls-tls-webpki-roots-no-provider", +] } +# The `-no-provider` reqwest variant expects the process-level rustls +# `CryptoProvider` to be installed by the caller. `ts dev proxy` only exists on +# macOS, so on other hosts nothing else pulls a rustls provider; this pins +# aws-lc-rs (rustls's default) on every host target so `ts dev audit` can +# install it before its first HTTPS fetch. On macOS it deduplicates with the +# proxy's aws-lc-rs — still exactly one provider, so no ambiguity. +rustls = { workspace = true } scraper = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } diff --git a/crates/trusted-server-cli/src/commands/dev/audit/headers/analyze.rs b/crates/trusted-server-cli/src/commands/dev/audit/headers/analyze.rs index 4d07fd3515..ae0ff8d223 100644 --- a/crates/trusted-server-cli/src/commands/dev/audit/headers/analyze.rs +++ b/crates/trusted-server-cli/src/commands/dev/audit/headers/analyze.rs @@ -57,12 +57,17 @@ pub(crate) struct AuditReport { impl AuditReport { /// The process exit code the CLI should return: 1 if any group failed, - /// 2 if any warned (and none failed), 0 otherwise. + /// 3 if any warned (and none failed), 0 otherwise. + /// + /// 2 is deliberately skipped: `main` exits 2 for any CLI error, so reusing + /// it for warnings would make a warn-only audit indistinguishable from an + /// audit that never ran (unreachable origin, bad config). CI gates keying + /// on the code can then tell "cache warnings" (3) from "audit failed" (2). pub(crate) fn exit_code(&self) -> i32 { if self.summary.fail > 0 { 1 } else if self.summary.warn > 0 { - 2 + 3 } else { 0 } @@ -269,7 +274,7 @@ mod tests { } #[test] - fn warn_only_exits_two() { + fn warn_only_exits_three() { let responses = vec![response( "https://origin.example/app.js", "application/javascript", @@ -277,6 +282,10 @@ mod tests { )]; let report = run_analysis("https://origin.example", &responses); assert_eq!(report.summary.warn, 1, "non-immutable JS should warn"); - assert_eq!(report.exit_code(), 2, "warn-only should exit 2"); + assert_eq!( + report.exit_code(), + 3, + "warn-only should exit 3, not 2 (2 is the CLI error code)" + ); } } diff --git a/crates/trusted-server-cli/src/commands/dev/audit/headers/fetch.rs b/crates/trusted-server-cli/src/commands/dev/audit/headers/fetch.rs index 2b8d801d2e..50d8aa8185 100644 --- a/crates/trusted-server-cli/src/commands/dev/audit/headers/fetch.rs +++ b/crates/trusted-server-cli/src/commands/dev/audit/headers/fetch.rs @@ -6,6 +6,7 @@ //! Trusted Server edge — not the origin — so they are never probed here; supply //! them explicitly if you need to audit them. +use std::io::Read; use std::path::PathBuf; use std::time::Duration; @@ -22,6 +23,12 @@ const MAX_DISCOVERED_URLS: usize = 50; /// Per-request timeout for origin fetches. const REQUEST_TIMEOUT: Duration = Duration::from_secs(15); +/// Cap on how many body bytes are read per response. Only the root page's HTML +/// is ever parsed for discovery; every other body is discarded, so this bounds +/// a fast link from pulling an unbounded payload (a large image or video) that +/// the timeout alone would not catch. +const MAX_BODY_BYTES: u64 = 2 * 1024 * 1024; + /// Arguments for `ts dev audit headers`. #[derive(Debug, clap::Args)] pub struct AuditHeadersArgs { @@ -35,6 +42,11 @@ pub struct AuditHeadersArgs { /// Override the origin URL, skipping the config lookup. #[arg(long)] pub origin: Option, + /// Follow discovered asset URLs that resolve to a different origin. By + /// default the automatic crawl audits only same-origin assets; explicit + /// URLs are always fetched regardless of this flag. + #[arg(long)] + pub include_cross_origin: bool, /// Emit machine-readable JSON instead of the human table. #[arg(long)] pub json: bool, @@ -64,10 +76,23 @@ pub(crate) struct ReqwestOriginClient { } impl ReqwestOriginClient { - /// Builds a client with a bounded per-request timeout. + /// Builds a client with a bounded per-request timeout and no redirect + /// following. Redirects are not followed so the audited URL's own cache + /// headers are classified on their merits — a long-lived cached 3xx is a + /// real misconfiguration this tool should surface, and following would + /// silently attribute the final hop's headers to the requested URL. pub(crate) fn new() -> CliResult { + // reqwest is built with `rustls-tls-webpki-roots-no-provider`, which + // installs no rustls crypto provider. `ts dev proxy` (macOS-only) + // installs aws-lc-rs, but on other hosts nothing does, so the first + // HTTPS build would panic with "no provider set". Install it here; + // `install_default` is idempotent and returns `Err` if one is already + // installed (e.g. by the proxy on macOS), which is fine to ignore. + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + let client = reqwest::blocking::Client::builder() .timeout(REQUEST_TIMEOUT) + .redirect(reqwest::redirect::Policy::none()) .build() .map_err(|error| format!("failed to build HTTP client: {error}"))?; Ok(Self { client }) @@ -83,7 +108,12 @@ impl OriginClient for ReqwestOriginClient { .map_err(|error| format!("failed to fetch {url}: {error}"))?; let headers = extract_headers(response.headers()); - let body = response.text().unwrap_or_default(); + + // Read at most MAX_BODY_BYTES: only the root page's HTML is parsed for + // discovery, so a truncated body is harmless for every other response. + let mut buffer = Vec::new(); + let _ = response.take(MAX_BODY_BYTES).read_to_end(&mut buffer); + let body = String::from_utf8_lossy(&buffer).into_owned(); Ok(OriginResponse { headers, body }) } @@ -119,7 +149,10 @@ pub(crate) fn collect_responses( } } -/// Fetches an explicit list of URLs. +/// Fetches an explicit list of URLs. A malformed URL is a hard error (operator +/// typo), but a per-URL fetch failure is tolerated with a warning so one +/// unreachable URL does not discard the verdicts for the others; only an +/// all-fail run errors out. fn collect_explicit( urls: &[String], client: &dyn OriginClient, @@ -127,11 +160,16 @@ fn collect_explicit( let mut responses = Vec::with_capacity(urls.len()); for raw in urls { let url = Url::parse(raw).map_err(|error| format!("invalid URL `{raw}`: {error}"))?; - let response = client.fetch(&url)?; - responses.push(FetchedResponse { - url, - headers: response.headers, - }); + match client.fetch(&url) { + Ok(response) => responses.push(FetchedResponse { + url, + headers: response.headers, + }), + Err(error) => log::warn!("skipping {url}: {error}"), + } + } + if responses.is_empty() { + return cli_error("all requested URLs failed to fetch"); } // With explicit URLs there is no single origin; report the first host. let origin = responses @@ -152,7 +190,7 @@ fn collect_via_discovery( Url::parse(&origin).map_err(|error| format!("invalid origin `{origin}`: {error}"))?; let root_response = client.fetch(&root)?; - let discovered = discover_asset_urls(&root, &root_response.body); + let discovered = discover_asset_urls(&root, &root_response.body, args.include_cross_origin); let mut responses = Vec::with_capacity(discovered.len() + 1); responses.push(FetchedResponse { @@ -207,7 +245,12 @@ fn resolve_origin(args: &AuditHeadersArgs) -> CliResult { /// Extracts asset URLs from HTML: ` + + + "#; + let urls = discover_asset_urls(&base, html, false); + assert!( + urls.contains(&Url::parse("https://origin.example/local.js").unwrap()), + "should keep the same-origin asset" + ); + assert!( + !urls + .iter() + .any(|url| url.host_str() == Some("cdn.other.example")), + "should not follow a cross-origin asset by default" + ); + } + + #[test] + fn discovery_includes_cross_origin_when_opted_in() { + let base = Url::parse("https://origin.example").expect("should parse base"); + let html = r#" + + "#; + let urls = discover_asset_urls(&base, html, true); + assert!( + urls.iter() + .any(|url| url.host_str() == Some("cdn.other.example")), + "should follow a cross-origin asset when opted in" + ); + } + #[test] fn discovery_fetches_root_and_assets_but_never_ts_routes() { let html = r#""#; @@ -378,6 +459,7 @@ mod tests { urls: vec!["https://origin.example/rtb".to_owned()], config: PathBuf::from("trusted-server.toml"), origin: None, + include_cross_origin: false, json: false, }; let (_, responses) = collect_responses(&explicit, &client).expect("should collect"); @@ -396,9 +478,52 @@ mod tests { urls: vec!["not a url".to_owned()], config: PathBuf::from("trusted-server.toml"), origin: None, + include_cross_origin: false, json: false, }; let error = collect_responses(&explicit, &client).expect_err("should reject invalid URL"); assert!(error.contains("invalid URL"), "should explain the failure"); } + + #[test] + fn explicit_urls_tolerate_partial_failure() { + // One reachable URL, one unreachable: the audit should keep the good + // verdict rather than aborting on the first failure. + let client = FakeClient::new().with("https://origin.example/ok", "text/html", ""); + let explicit = AuditHeadersArgs { + urls: vec![ + "https://origin.example/ok".to_owned(), + "https://origin.example/down".to_owned(), + ], + config: PathBuf::from("trusted-server.toml"), + origin: None, + include_cross_origin: false, + json: false, + }; + let (_, responses) = + collect_responses(&explicit, &client).expect("should tolerate one failure"); + assert_eq!( + responses.len(), + 1, + "should keep the reachable URL's response" + ); + } + + #[test] + fn explicit_urls_error_when_all_fail() { + let client = FakeClient::new(); + let explicit = AuditHeadersArgs { + urls: vec!["https://origin.example/down".to_owned()], + config: PathBuf::from("trusted-server.toml"), + origin: None, + include_cross_origin: false, + json: false, + }; + let error = + collect_responses(&explicit, &client).expect_err("should error when all URLs fail"); + assert!( + error.contains("all requested URLs failed"), + "should explain the all-fail case" + ); + } } diff --git a/crates/trusted-server-cli/src/commands/dev/audit/headers/mod.rs b/crates/trusted-server-cli/src/commands/dev/audit/headers/mod.rs index c4de91d70d..2fa0c688f0 100644 --- a/crates/trusted-server-cli/src/commands/dev/audit/headers/mod.rs +++ b/crates/trusted-server-cli/src/commands/dev/audit/headers/mod.rs @@ -3,8 +3,9 @@ //! Fetches origin responses (explicit URLs, or discovered from the origin root), //! classifies them by content type, evaluates each against the cacheability //! rules, and renders a per-type pass/warn/fail report. The command exits 1 when -//! any content-type group fails, 2 when only warnings are present, and 0 when -//! all pass. +//! any content-type group fails, 3 when only warnings are present, and 0 when +//! all pass. Exit 2 is reserved for CLI-level errors (see `main.rs`), so a CI +//! gate can distinguish cache warnings from an audit that never ran. mod analyze; mod fetch; @@ -20,7 +21,8 @@ use analyze::AuditReport; use fetch::{OriginClient, ReqwestOriginClient, collect_responses}; /// Runs the header audit against a live origin, printing to stdout and returning -/// the process exit code (0 pass, 1 fail, 2 warn-only). +/// the process exit code (0 pass, 1 fail, 3 warn-only; 2 is reserved for CLI +/// errors). pub fn run(args: &AuditHeadersArgs) -> Result { let client = ReqwestOriginClient::new()?; let stdout = std::io::stdout(); @@ -96,6 +98,7 @@ mod tests { urls: Vec::new(), config: PathBuf::from("trusted-server.toml"), origin: Some(origin.to_owned()), + include_cross_origin: false, json, } } diff --git a/crates/trusted-server-cli/src/commands/dev/audit/headers/rules.rs b/crates/trusted-server-cli/src/commands/dev/audit/headers/rules.rs index bc9dc9bf7d..65b271f8f9 100644 --- a/crates/trusted-server-cli/src/commands/dev/audit/headers/rules.rs +++ b/crates/trusted-server-cli/src/commands/dev/audit/headers/rules.rs @@ -203,10 +203,16 @@ impl CacheDirectives { continue; } if let Some((name, seconds)) = token.split_once('=') { - let seconds = seconds.trim_matches('"').parse::().ok(); + // Trim whitespace on both sides of the quote-stripping so + // `max-age = 3600` and `max-age = "3600"` (legal per HTTP + // parameter syntax) still parse. + let seconds = seconds.trim().trim_matches('"').trim().parse::().ok(); match name.trim() { - "max-age" => max_age = seconds, - "s-maxage" => s_maxage = seconds, + // First valid occurrence wins: a later malformed or + // duplicate directive must not discard an already-parsed + // value (RFC 9111 §4.2.1 — treat unparseable as absent). + "max-age" => max_age = max_age.or(seconds), + "s-maxage" => s_maxage = s_maxage.or(seconds), _ => {} } tokens.insert(name.trim().to_owned()); @@ -250,17 +256,19 @@ pub(crate) fn evaluate(group: ContentTypeGroup, headers: &ResponseHeaders) -> Ve } } -/// HTML must not be shared across users: `no-store` alone is sufficient -/// (RFC 9111 §5.2.2.5), or `private` combined with `no-cache`. +/// HTML must not be shared across users. `no-store` alone is sufficient +/// (RFC 9111 §5.2.2.5), as is `private` alone (RFC 9111 §5.2.2.7 — a shared +/// cache must not store a `private` response). `no-cache` alone is NOT +/// sufficient: a shared cache may still store the response and, on a 304, +/// serve one user's body to another. fn evaluate_html(headers: &ResponseHeaders) -> Vec { let mut verdicts = Vec::new(); - let expected = "no-store, or private + no-cache"; + let expected = "no-store, or private"; match headers.cache_control.as_deref() { Some(value) => { let directives = CacheDirectives::parse(value); - let safe = directives.has("no-store") - || (directives.has("private") && directives.has("no-cache")); + let safe = directives.has("no-store") || directives.has("private"); if safe { verdicts.push(HeaderVerdict::pass( "Cache-Control", @@ -273,7 +281,7 @@ fn evaluate_html(headers: &ResponseHeaders) -> Vec { Verdict::Fail, Some(value.to_owned()), expected, - "HTML served without no-store (or private + no-cache) risks sharing personalized content across users", + "HTML served without no-store or private risks sharing personalized content across users", )); } } @@ -282,7 +290,7 @@ fn evaluate_html(headers: &ResponseHeaders) -> Vec { Verdict::Fail, None, expected, - "HTML has no Cache-Control; set `no-store` to prevent shared caching of personalized content", + "HTML has no Cache-Control; set `no-store` or `private` to prevent shared caching of personalized content", )), } @@ -341,7 +349,8 @@ fn evaluate_immutable( verdicts.push(evaluate_etag(headers)); } verdicts.extend(evaluate_surrogate_key(group, headers)); - verdicts.extend(evaluate_vary(headers, /* flag_wildcard */ false)); + verdicts.extend(evaluate_surrogate_disables_caching(headers)); + verdicts.extend(evaluate_vary(headers, /* flag_wildcard */ true)); verdicts } @@ -384,37 +393,63 @@ fn evaluate_image(headers: &ResponseHeaders) -> Vec { )), } - // The CDN TTL (Surrogate-Control max-age, else s-maxage) should be at least - // the browser TTL, so the edge does not re-fetch more often than clients. - if let Some(surrogate) = headers.surrogate_control.as_deref() { - let surrogate_directives = CacheDirectives::parse(surrogate); - let cdn_ttl = surrogate_directives - .max_age - .or(surrogate_directives.s_maxage); - let browser_ttl = cache_control - .as_ref() - .and_then(|directives| directives.max_age); - if let (Some(cdn), Some(browser)) = (cdn_ttl, browser_ttl) { - if cdn < browser { - verdicts.push(HeaderVerdict::flagged( - "Surrogate-Control", - Verdict::Warn, - Some(surrogate.to_owned()), - "CDN max-age >= browser max-age", - "CDN caching shorter than browser caching; raise Surrogate-Control max-age (or s-maxage)", - )); - } else { - verdicts.push(HeaderVerdict::pass( - "Surrogate-Control", - Some(surrogate.to_owned()), - "CDN max-age >= browser max-age", - )); + // The CDN TTL should be at least the browser TTL, so the edge does not + // re-fetch more often than clients. The CDN TTL is resolved from + // `Surrogate-Control` max-age first, then a `Cache-Control: s-maxage` + // (the standard shared-cache TTL, honored even without Surrogate-Control), + // matching the three-level resolution the module documents. + let browser_ttl = cache_control + .as_ref() + .and_then(|directives| directives.max_age); + let surrogate_directives = headers + .surrogate_control + .as_deref() + .map(CacheDirectives::parse); + let cdn_ttl = surrogate_directives + .as_ref() + .and_then(|directives| directives.max_age.or(directives.s_maxage)) + .or_else(|| { + cache_control + .as_ref() + .and_then(|directives| directives.s_maxage) + }); + if let (Some(cdn), Some(browser)) = (cdn_ttl, browser_ttl) { + // Label the verdict after whichever header actually supplied the TTL so + // it never collides with the cacheability `Cache-Control` verdict above. + let (header, actual) = match &headers.surrogate_control { + Some(surrogate) + if surrogate_directives + .as_ref() + .and_then(|directives| directives.max_age.or(directives.s_maxage)) + .is_some() => + { + ("Surrogate-Control", surrogate.clone()) } + _ => ( + "s-maxage", + headers.cache_control.clone().unwrap_or_default(), + ), + }; + if cdn < browser { + verdicts.push(HeaderVerdict::flagged( + header, + Verdict::Warn, + Some(actual), + "CDN TTL >= browser TTL", + "CDN caching shorter than browser caching; raise the shared-cache TTL (Surrogate-Control max-age or s-maxage)", + )); + } else { + verdicts.push(HeaderVerdict::pass( + header, + Some(actual), + "CDN TTL >= browser TTL", + )); } } verdicts.extend(evaluate_surrogate_key(ContentTypeGroup::Image, headers)); - verdicts.extend(evaluate_vary(headers, /* flag_wildcard */ false)); + verdicts.extend(evaluate_surrogate_disables_caching(headers)); + verdicts.extend(evaluate_vary(headers, /* flag_wildcard */ true)); verdicts } @@ -482,6 +517,29 @@ fn evaluate_surrogate_no_store( }) } +/// On a cacheable group, a `Surrogate-Control` that forbids edge caching +/// (`no-store` / `private` / `no-cache`, or a zero TTL) defeats the point of a +/// cacheable asset. Numeric-TTL cases like `max-age=0` are already caught by the +/// CDN-vs-browser comparison, so this covers the token-only directives that +/// carry no numeric value. +fn evaluate_surrogate_disables_caching(headers: &ResponseHeaders) -> Option { + let value = headers.surrogate_control.as_deref()?; + let directives = CacheDirectives::parse(value); + let disables = + directives.has("no-store") || directives.has("private") || directives.has("no-cache"); + if disables { + Some(HeaderVerdict::flagged( + "Surrogate-Control", + Verdict::Warn, + Some(value.to_owned()), + "should allow CDN caching", + "Surrogate-Control disables edge caching for a cacheable asset; the CDN will not cache it", + )) + } else { + None + } +} + /// `Surrogate-Key` enables targeted CDN purge; absence is a warning on /// cacheable groups only. fn evaluate_surrogate_key( @@ -686,7 +744,7 @@ mod tests { } #[test] - fn vary_wildcard_warns_only_for_html() { + fn vary_wildcard_warns_on_html() { let html = evaluate( ContentTypeGroup::Html, &ResponseHeaders { @@ -763,4 +821,126 @@ mod tests { "empty should default to pass" ); } + + #[test] + fn html_passes_on_private_alone() { + // RFC 9111 §5.2.2.7: `private` alone forbids a shared cache from storing + // the response, so `private, max-age=0` is a correct personalized-HTML + // posture and must not fail. + let verdicts = evaluate( + ContentTypeGroup::Html, + &headers_with(Some("private, max-age=0")), + ); + assert_eq!( + verdict_for(&verdicts, "Cache-Control"), + Verdict::Pass, + "private alone should satisfy HTML" + ); + } + + #[test] + fn vary_wildcard_warns_on_all_cacheable_groups() { + // Vary: * makes a response uncacheable by any cache, so it must be + // flagged on the immutable/cacheable groups where it is most damaging, + // not only on HTML. + for group in [ + ContentTypeGroup::JavaScript, + ContentTypeGroup::Image, + ContentTypeGroup::StaticAsset, + ] { + let verdicts = evaluate( + group, + &ResponseHeaders { + cache_control: Some("public, max-age=31536000, immutable".to_owned()), + surrogate_key: Some("k".to_owned()), + etag: Some("\"abc\"".to_owned()), + vary: Some("*".to_owned()), + ..ResponseHeaders::default() + }, + ); + assert_eq!( + verdict_for(&verdicts, "Vary"), + Verdict::Warn, + "Vary: * should warn on {}", + group.label() + ); + } + } + + #[test] + fn image_honors_cache_control_s_maxage_as_cdn_ttl() { + // A standard `Cache-Control: s-maxage` shared-cache TTL below the browser + // max-age should warn even without a Surrogate-Control header. + let verdicts = evaluate( + ContentTypeGroup::Image, + &ResponseHeaders { + cache_control: Some("public, max-age=86400, s-maxage=60".to_owned()), + surrogate_key: Some("img".to_owned()), + ..ResponseHeaders::default() + }, + ); + assert_eq!( + verdict_for(&verdicts, "s-maxage"), + Verdict::Warn, + "s-maxage below browser TTL should warn via Cache-Control" + ); + } + + #[test] + fn cacheable_group_flags_surrogate_control_no_store() { + // An image with a perfectly cacheable Cache-Control but + // Surrogate-Control: no-store is not cached by the CDN — must not pass. + let verdicts = evaluate( + ContentTypeGroup::Image, + &ResponseHeaders { + cache_control: Some("public, max-age=86400".to_owned()), + surrogate_control: Some("no-store".to_owned()), + surrogate_key: Some("img".to_owned()), + ..ResponseHeaders::default() + }, + ); + assert_eq!( + verdict_for(&verdicts, "Surrogate-Control"), + Verdict::Warn, + "Surrogate-Control: no-store should warn on a cacheable image" + ); + } + + #[test] + fn max_age_with_spaces_around_equals_parses() { + let verdicts = evaluate( + ContentTypeGroup::JavaScript, + &ResponseHeaders { + cache_control: Some("public, max-age = 31536000, immutable".to_owned()), + surrogate_key: Some("js".to_owned()), + etag: Some("\"abc\"".to_owned()), + ..ResponseHeaders::default() + }, + ); + assert_eq!( + verdict_for(&verdicts, "Cache-Control"), + Verdict::Pass, + "whitespace around `=` should not lose the max-age value" + ); + } + + #[test] + fn duplicate_or_malformed_directive_keeps_first_valid_value() { + // RFC 9111 §4.2.1: a later malformed/duplicate directive is treated as + // absent, not allowed to discard an already-parsed value. + let verdicts = evaluate( + ContentTypeGroup::JavaScript, + &ResponseHeaders { + cache_control: Some("public, immutable, max-age=31536000, max-age=foo".to_owned()), + surrogate_key: Some("js".to_owned()), + etag: Some("\"abc\"".to_owned()), + ..ResponseHeaders::default() + }, + ); + assert_eq!( + verdict_for(&verdicts, "Cache-Control"), + Verdict::Pass, + "a trailing malformed max-age must not null the valid one" + ); + } } diff --git a/crates/trusted-server-cli/src/commands/dev/audit/mod.rs b/crates/trusted-server-cli/src/commands/dev/audit/mod.rs index cdc8938180..515c99a5ca 100644 --- a/crates/trusted-server-cli/src/commands/dev/audit/mod.rs +++ b/crates/trusted-server-cli/src/commands/dev/audit/mod.rs @@ -28,8 +28,10 @@ pub fn run(command: &DevAuditCommand) -> Result<(), String> { match command { DevAuditCommand::Headers(args) => { let exit_code = headers::run(args)?; - // 0 means every content-type group passed; 1 (a group failed) and 2 - // (warnings only) are surfaced as process exit codes for CI gating. + // 0 means every content-type group passed. Non-zero audit outcomes + // (1 = a group failed, 3 = warnings only) are surfaced as process + // exit codes for CI gating. 3 is used for warnings rather than 2 + // because `main` already exits 2 for any CLI error. if exit_code == 0 { Ok(()) } else { diff --git a/docs/guide/cli.md b/docs/guide/cli.md index 8b26661763..c1a5c0f024 100644 --- a/docs/guide/cli.md +++ b/docs/guide/cli.md @@ -649,3 +649,58 @@ ts prebid bundle --config publisher-a.toml --out build/prebid `ts prebid bundle` is local-only. It has no `--adapter` option and does not upload, provision, deploy, or push config. + +## Audit origin cache headers + +`ts dev audit headers` fetches responses from the publisher origin, classifies +each by content type, and grades its cache-related response headers +(`Cache-Control`, `Surrogate-Control`, `s-maxage`, `Surrogate-Key`, `Vary`, +`ETag`) against the expected posture for that type. It reports a per-type +pass/warn/fail verdict naming the responsible header and the recommended value. + +```bash +# Audit the origin from trusted-server.toml (publisher.origin_url), discovering +# assets from the root page's HTML. +ts dev audit headers + +# Point at an origin explicitly, skipping the config lookup. +ts dev audit headers --origin https://origin.publisher.com + +# Audit specific URLs instead of crawling. +ts dev audit headers https://origin.publisher.com/app.js https://origin.publisher.com/ + +# Machine-readable output for pipelines. +ts dev audit headers --json +``` + +When no URLs are given, discovery fetches the origin root and follows the +`