Persist the tester cookie for 30 days so it survives browser restarts

[jevansnyc]

Fixes #1136

Problem

GET /_ts/set-tester minted the ts-tester cookie without Max-Age or Expires, making it a session cookie. Safari deletes session cookies when the application quits, so Safari testers lost the cookie on every restart and silently fell back to the baseline arm on their next visit. Chrome and Firefox mask the defect because session restore resurrects session cookies.

Change

• Add TESTER_COOKIE_MAX_AGE_SECONDS (30 days, 2592000) and append Max-Age to the tester cookie in format_tester_cookie
• Update the two tests that assert the exact Set-Cookie string (core tester_cookie test, fastly dispatch_set_tester test)

/_ts/clear-tester already expires the cookie explicitly with Max-Age=0 and is unchanged. The module documents that the cookie only affects tester routing and must not gate sensitive behavior, so the bounded persistent lifetime does not change its security posture.

Testing

• Test-first: updated both assertions, watched both fail, then implemented
• cargo test-fastly: 170 adapter + 2357 core tests pass
• cargo test-axum: pass
• cargo fmt --all -- --check, cargo clippy-fastly, cargo clippy-axum: clean

Generated with Claude Code

[prk-Jr]

Summary

Correct one-line fix for a real Safari defect: the tester cookie was session-scoped, so Safari testers silently fell back to the baseline arm after every browser restart. The Max-Age addition is right, the named const carries its rationale, and /_ts/clear-tester was correctly left alone (already Max-Age=0). One blocking gap: the two published docs pages that quote the exact Set-Cookie line were not updated, so the operator-facing docs now contradict the code.

1 of the inline comments below carries a one-click GitHub suggestion — use Commit suggestion to apply it as a commit on the PR branch. The other comments describe the change in prose because the files are outside this diff or the wording is yours to choose.

Blocking

🔧 wrench

• Published docs still print the pre-Max-Age Set-Cookie line — see the Cross-cutting section below (both files are outside this diff, so there is no inline anchor)

Non-blocking

🤔 thinking

• Safari caps this at 7 days behind a CNAME — see inline at crates/trusted-server-core/src/tester_cookie.rs:22
• 30-day self-service enrollment window — see the Cross-cutting section below

⛏ nitpick

• Public doc links a private const (new rustdoc warning) — see inline at crates/trusted-server-core/src/tester_cookie.rs:45

🌱 seedling

• Keep ts-tester off any future ignorable-cookie cache allowlist — see the Cross-cutting section below

👍 praise

• Test-first, with the why captured in the const — see the Cross-cutting section below

Cross-cutting / body-level findings

• 🔧 Published docs still print the pre-Max-Age Set-Cookie line — Two operator-facing docs quote the emitted header verbatim and were not updated, so both are now factually wrong and both ship to the docs site via deploy-docs.yml:

  • docs/guide/api-reference.md:36 — Set-Cookie: ts-tester=true; Domain=<publisher.cookie_domain>; Path=/; Secure; SameSite=Lax
  • docs/guide/configuration.md:467 — the same string

  The repo's own precedent is that these pages move with the header: docs/guide/api-reference.md was updated in the very commits that introduced these routes (563ec96d, #795 and 3dc1262c, #797).

  Proposed fix — append the attribute to both http blocks:

  Set-Cookie: ts-tester=true; Domain=<publisher.cookie_domain>; Path=/; Secure; SameSite=Lax; Max-Age=2592000
  Cache-Control: no-store, private

  Worth one sentence alongside each block noting the 30-day lifetime and that /_ts/clear-tester is the opt-out, since configuration.md already documents the clear path's Max-Age=0. Run npm run format in docs/ afterwards to satisfy the required format-docs gate. Apply manually — can't be auto-applied as a suggestion because neither file is part of this diff.

• 🤔 30-day self-service enrollment window — /_ts/set-tester is unauthenticated whenever [tester_cookie].enabled = true, and the cookie is scoped to the whole publisher.cookie_domain. Before this change a stray visit self-limited at browser close; now it pins that browser to the tester arm for a month, and /_ts/clear-tester is the only opt-out — which a non-QA visitor has no reason to know about. Shared and kiosk browsers now carry the arm for 30 days.

  That is very likely fine: the module doc already states the cookie only affects tester routing and must not gate sensitive behavior, and the feature is off by default. Worth confirming explicitly that a 30-day contamination window is acceptable for whatever consumes the arm (measurement purity in particular), and that the flag is expected to be enabled only during QA windows rather than left on.

• 🌱 Keep ts-tester off any future ignorable-cookie cache allowlist — No action for this PR; flagging for whoever lands a per-cookie template-cache policy.

  Today persistence is harmless for the shared template cache: TemplateCacheBypassReason::CookieForwarded (crates/trusted-server-core/src/publisher.rs:5706) disqualifies any cookie-bearing request, and template_cache_vary rejects Cookie outright (crates/trusted-server-core/src/creative_opportunities.rs:478).

  But TS forwards Cookie to origin unchanged — there is no Cookie strip on the publisher path — so if a future policy treats some cookies as ignorable for cache-key purposes, ts-tester must not be one of them. An origin that varies the document on the tester arm would otherwise have tester HTML stored under a reader-neutral key and served to every reader. This change widens the window in which a tester's browser carries the cookie from one session to a month, which makes that mistake more likely to be observed rather than less.

• 👍 Test-first, with the why captured in the const — Both assertions were updated and watched failing before the implementation landed; the const carries the Safari session-cookie rationale rather than a bare 2592000; the assertion messages were tightened to state the new contract instead of being left describing only the domain; and format_clear_tester_cookie was correctly left untouched. Verified independently: Set-Cookie also survives EC finalize because crates/trusted-server-core/src/ec/cookies.rs:144,167 use append rather than insert, and nothing rewrites Set-Cookie attributes on egress (response_privacy only downgrades Cache-Control).

CI Status

• cargo fmt: PASS (required)
• cargo test: PASS (required)
• format-docs: PASS (required)
• format-typescript: PASS (required)
• cargo test (axum native): PASS
• cargo test (cloudflare native): PASS — reported as cargo check (cloudflare native + wasm32-unknown-unknown)
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native): PASS
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• browser integration tests: PASS
• prepare integration artifacts: PASS
• vitest: PASS
• CodeQL: PASS
• Analyze (rust): PASS
• Analyze (javascript-typescript): PASS
• Analyze (actions): PASS

[prk-Jr]

⛏ nitpick — The public doc on handle_set_tester links a private const, which rustdoc warns about and which renders as plain code rather than a link for readers of the public docs. Verified against this PR head:

warning: public documentation for `handle_set_tester` links to private item `TESTER_COOKIE_MAX_AGE_SECONDS`
  --> crates/trusted-server-core/src/tester_cookie.rs:45:7
   = note: `#[warn(rustdoc::private_intra_doc_links)]` on by default

Not CI-blocking — no workflow runs cargo doc, and 18 other instances of this warning already exist in the crate — but CLAUDE.md asks for cargo doc --no-deps --all-features verification. Making the const pub would be the worse fix (CLAUDE.md: consider visibility carefully, avoid unnecessary pub), so stating the lifetime in prose is the cheaper option:

Suggested change

	/// scoped to `publisher.cookie_domain` and persisted for
	/// [`TESTER_COOKIE_MAX_AGE_SECONDS`].
	/// scoped to `publisher.cookie_domain` and persisted for 30 days.

Scratch-verified at this head: cargo fmt --all -- --check clean, cargo clippy-fastly clean, and cargo doc -p trusted-server-core --no-deps --all-features drops from 19 to 18 private_intra_doc_links warnings with the tester one gone.

[prk-Jr]

🤔 thinking — The rationale names Safari specifically, which is worth pairing with Safari's own cap: Safari 14's CNAME-cloaking defense caps the expiry of cookies set in HTTP responses to 7 days when the setting host is a CNAME to a third party. That is the standard Trusted Server deployment shape — a publisher-owned hostname CNAME'd to the compute platform — so the lifetime a Safari tester actually gets is plausibly 7 days rather than 30.

That doesn't undermine the fix at all: 7 days beats one browser session, and #1136 is still resolved. But the const's doc currently reads as though 30 days is what Safari will honor, and nothing in the repo documents an ITP cap today. A sentence like "Safari may shorten this to 7 days when the TS host is a CNAME to the compute platform (ITP's CNAME-cloaking cap), which is still far better than a session cookie" would keep the next person from re-opening this as "the tester lost the arm after a week."

Worth confirming against your own deployment shape before wording it — the cap only applies to CNAME-cloaked hosts, so a deployment that isn't CNAME'd gets the full 30 days. Apply manually — the wording is yours, and this is doc-only.