**Spec**: Separate creative click rewrite switch

[dhruv8sh]

Summary

• Adds the design spec and implementation plan for Make creative click rewriting a separate feature with its own switch #1234. The issue asks for the spec to be approved in its own PR before any implementation PR is opened.
• The spec makes click wrapping a separate feature. Today one switch, [auction] rewrite_creatives, controls both asset proxying and click wrapping, so an operator can't keep first-party click tracking while leaving assets direct, or the other way round.
• It settles the open decisions in the issue:
  • Switch: [auction] rewrite_clicks is an Option<bool>. When unset, it follows rewrite_creatives, so no existing config changes behavior on upgrade.
  • HTML fetched through /first-party/proxy: an explicit value applies there too, while unset keeps today's always-wrap behavior.
  • <base> removal and TSJS injection: both run when either switch is on.
  • TsCreativeConfig.clickGuard: stays client-only.
  • Coordination with Add a host allowlist for creative asset rewriting #1231: both plans contain the same shared normalizer and policy step, so whichever implementation lands first introduces it and the other skips it.

Changes

File	Change
docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md	Design spec: problem, goals, design, resolved open decisions, test plan, rollout and rollback
docs/superpowers/plans/2026-10-07-1234-creative-click-rewrite-switch.md	Step-by-step TDD implementation plan, including the shared step with #1231

Closes

Part of #1234. This PR adds only the spec and plan. The implementation PR will link the approved spec and close the issue.

Test plan

• cargo test-fastly && cargo test-axum
• cargo clippy-fastly && cargo clippy-axum
• cargo fmt --all -- --check
• JS tests: cd crates/trusted-server-js/lib && npx vitest run
• JS format: cd crates/trusted-server-js/lib && npm run format
• Docs format: cd docs && npm run format
• WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
• Manual testing via fastly compute serve
• Other: prettier --config docs/.prettierrc --check on both files. This PR changes no code, so no Rust or JS gates were run. The plan was checked by implementing it in a local branch, where fmt, all clippy aliases, test-fastly, test-fastly-reuse, test-axum, test-cloudflare, test-spin, the CLI tests and the parity tests passed. That code is not included here.

Checklist

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

[prk-Jr]

Summary

Spec and plan for #1234's [auction] rewrite_clicks. The design holds up well: the plan was applied verbatim to main (d8937e1) and every Rust block compiled, passed fmt, clippy-fastly and clippy-cli; every claimed pass count matched exactly (16 / 15 / 554 — also under Viceroy 0.17.0 — / 2 / 2); the red steps fail where predicted; both MATRIX_DEFAULT_*_OUTPUT constants match main byte for byte; and the normalizer leaves tsurl/tstoken identical for edge inputs (uppercase host, :443, spaces, é, IDN, tabs). Cited files are unchanged between 7a0ecb4c and main. Two blocking items are doc-accuracy problems that would flow into operator docs.

13 of the inline comments below carry a one-click GitHub suggestion — use Commit suggestion (or Add suggestion to batch) to apply them. The remaining comments describe the fix in prose because it spans several places. All suggestions were applied together and checked with the pinned docs prettier.

Blocking

🔧 wrench

• Doc-update list misses text that becomes false — see inline at docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md:482
• renderGuard caveat is wrong for the sandboxed renderer — see inline at docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md:344 and docs/superpowers/plans/2026-10-07-1234-creative-click-rewrite-switch.md:1712

Non-blocking

🤔 thinking

• <base> removal in single-feature modes — see inline at spec :342
• Public wrappers bypass both switches — see inline at spec :305

♻️ refactor

• Task 1 red steps can't be observed — see inline at plan :637

⛏ nitpick

• Step 5 grep can never pass — plan :651
• "Unchanged output" vs the shared step's Fixed items — spec :26, plan :22
• Task 0 has no one-match case — plan :310
• Suffix-spoof case dropped from the policy test — plan :352
• Private intra-doc link warning — plan :92
• Step 8 wording replaces the wrong span — plan :1363
• Click guard isn't fully inert — spec :357
• "Without allocating" — spec :176
• Commit instructions vs CONTRIBUTING.md — plan :35

Cross-cutting / body-level findings

• ⛏ Smaller precision items
  • Citations off by one or two: spec :38 (proxy.rs:1898-1906), :229 (1672-1703, matching the plan), :486 (endpoints.rs:84-90), :487 (formats.rs:375-378).
  • The shared CHANGELOG Fixed entry (plan :290) leaves out that unparseable quoted URLs in <style>/style= are no longer re-quoted via cssparser::serialize_string, and that /first-party/sign error text changes (invalid url / unsupported scheme → unsupported url). "Re-quoted" also understates the old fallback: // input was emitted https:-prefixed (src=" //exa mple.example/x " → https://exa mple.example/x).
  • Because settings serialize normalized, the first ts config push after upgrading shows a blob diff for configs with mixed-case/whitespace/""/* exclude entries — worth one line in Rollout.
  • Expected files (spec :491) omits auction/orchestrator.rs and auction/endpoints.rs.
  • Spec :372's provenance sentence reads as if #1231's text lacks the four tests; #1248's plan already has them.
  • Spec :107-115 vs plan :886-896 doc comments differ (plan adds the "rejected by binaries that predate this field" clause); spec :122 vs plan :902 likewise.
  • Plan :39: Viceroy 0.21.1 vs pinned 0.17.0 (re-run under 0.17.0 gives the same counts).
  • PR checklist: the tracing box should read log.
• 🌱 Staged / multi-service rollback — spec :467: a blob pushed with --staging that carries rewrite_clicks also has to be re-pushed without it before rolling back a staged binary, once per service.
• 📌 Trailing-dot hosts bypass exclude_domains (pre-existing) — https://excluded.example.com./x is still proxied and wrapped in both old and new code (reproduced). Not for this PR; same follow-up as noted on #1248.
• 👍 Praise
  • The plan is executable as written, with exact counts and byte-exact pins.
  • Gating is complete: the anchor handler is the only producer of click URLs, apart from rebuild of already-signed input.
  • Keeping rewrite_clicks commented out in the example TOML is the right call, since ts config init copies it.
  • Option<bool> with per-path resolution keeps both existing operator groups unchanged.

CI Status

• All 22 checks PASS: Analyze (actions / javascript-typescript / python / rust), CodeQL, CLAUDE.md symlink guard, browser integration tests, integration tests, integration tests (Fastly EC lifecycle), prepare integration artifacts, cargo fmt, cargo test, cargo test (axum native), cargo test (cross-adapter parity), cargo check (cloudflare native + wasm32-unknown-unknown), cargo check/build/test (spin native + wasm32-wasip1), cargo test (ts CLI, native) (macos-latest / ubuntu-latest), vitest, format-typescript, format-docs

[prk-Jr]

🔧 wrench — The doc-update list misses operator text that becomes false once rewrite_clicks exists, and plan Task 6 inherits the gap:

• auction-orchestration.md:704 (full example TOML), :837/:844 (the environment-override section that tells operators which leaves to add) and :853 (rollback note) all name rewrite_creatives alone.
• configuration.md:2171: "rewrite_creatives = false skips first-party URL rewriting and creative TSJS injection" — false with rewrite_clicks = true.
• configuration.md:1956: "Neither setting affects HTML or CSS fetched through /first-party/proxy" — an explicit rewrite_clicks now does.

This row's fix is below; the configuration.md row (:480) and plan Task 6 Steps 1/3 need the :1956 and :2171 lines added by hand (spans the spec table and the plan, so it can't be one suggestion).

Suggested change

	| `docs/guide/auction-orchestration.md` | Update the `rewrite_creatives` references (lines 164, 198, 636-660) to name both switches. |
	| `docs/guide/auction-orchestration.md` | Update the `rewrite_creatives` references (lines 164, 198, 634-663, the example TOML at 704, the environment-override section at 837-844 and the rollback note at 853) to name both switches. |

[prk-Jr]

🔧 wrench — This caveat doesn't hold for the shipped renderer, and plan :1712 would put it into creative-processing.md:

• /auction creatives render in a sandbox without allow-same-origin (crates/trusted-server-js/lib/src/core/render.ts:14-29).
• In that context signProxyUrl returns early on hasOpaqueOrigin() (integrations/creative/proxy_sign.ts:40), so render-guard assets load direct.
• Separately, /first-party/sign (proxy.rs:1625-1734) checks no auction setting, so rewrite_creatives = false has never gated it — injecting TSJS doesn't open anything new.

Suggested change

- TSJS injection on `/auction` with only clicks on also gives a creative that sets `tsCreativeConfig.renderGuard` a way to send assets inserted by its own script through `/first-party/sign` and `/first-party/proxy`, although the server left the markup's asset URLs direct. `creative-processing.md` says so.

- TSJS injection on `/auction` with only clicks on does not reopen asset proxying in practice: `/auction` creatives render in a sandbox without `allow-same-origin` (`render.ts:14-29`), so the render guard's `signProxyUrl` returns early on an opaque origin (`proxy_sign.ts:40`) and script-inserted assets load direct. Independently, `/first-party/sign` checks no auction setting, so `rewrite_creatives = false` has never gated it; `creative-processing.md` says so.

[prk-Jr]

🔧 wrench — Same correction as spec :344: under the sandboxed /auction renderer the render guard can't reach /first-party/sign (proxy_sign.ts:40), so this note would mislead operators.

Suggested change

	- a `renderGuard` note: "With `rewrite_creatives = false` and `rewrite_clicks = true`, `POST /auction` still injects TSJS. A creative that turns on `tsCreativeConfig.renderGuard` can therefore still send assets inserted by its own script through `/first-party/sign` and `/first-party/proxy`, even though the server left the markup's asset URLs direct."
	- a `renderGuard` note: "With `rewrite_creatives = false` and `rewrite_clicks = true`, `POST /auction` still injects TSJS, but this does not reopen asset proxying in practice: `/auction` creatives render in a sandbox without `allow-same-origin`, so the render guard skips `/first-party/sign` and script-inserted assets load direct. `/first-party/sign` itself is not gated by `rewrite_creatives`."

[prk-Jr]

⛏ nitpick — This grep can never come back empty: to_abs also matches auction/telemetry.rs:1036 (…_default_to_absent_…) and publisher.rs:22620 (…_to_absolute_first_party_urls). Reproduced by applying the plan to main. Word-bounded form only hits the intended code (and -w doesn't match is_excluded_host). Same fix as on #1248.

Suggested change

	Run: `git grep -n "to_abs\|is_excluded(" crates/`
	Run: `git grep -nw -e to_abs -e is_excluded crates/`

[prk-Jr]

♻️ refactor — These five runtime reds can't be observed in step order: Steps 1–3 switch the creative.rs test import to normalize_creative_url and add should_proxy_asset/should_wrap_click/normalize calls, so the whole core test binary fails to compile (this step's own Expected line). Verified: the five do fail exactly as described when added to unmodified main first. Suggest adding those five with the old import first, watching them fail, then doing Steps 1–3. Same as #1248.

[prk-Jr]

⛏ nitpick — Substituting this literally into the existing sentence yields "its only job is to its click guard…"; it has to replace the whole clause.

Suggested change

	- `rewrite_inline_creative_html`: "its click guard is unnecessary for click URLs that are already absolute here".
	- `rewrite_inline_creative_html`: replace "its only job is to safeguard click URLs, which are already absolute here" with "its click guard is unnecessary for click URLs that are already absolute here".

[prk-Jr]

⛏ nitpick — Not quite inert: installClickGuard still adds capture-phase click/auxclick listeners and a document-wide attribute MutationObserver (click.ts:484-512). Harmless, but worth stating accurately (also :312).

Suggested change

	- With `rewrite_clicks` off there is nothing to guard. The guard installs and stays inert (`click.ts:433-434`, `click.ts:467`), so plumbing the server switch into the client would add config surface with no behavior change.
	- With `rewrite_clicks` off there is nothing to guard. The guard still installs its click listeners and attribute observer (`click.ts:484-512`) but never acts, because no anchor carries `data-tsclick` (`click.ts:433-434`, `click.ts:467`), so plumbing the server switch into the client would add config surface with no behavior change.

[prk-Jr]

⛏ nitpick — Url::parse allocates; what's avoided is only the intermediate format!.

Suggested change

It trims the input, maps a protocol-relative `//host/...` to `https://host/...`, and accepts only a case-insensitive `http://` or `https://` prefix. It parses absolute input in place without allocating, and returns `None` for empty, relative, non-http(s) or unparseable input. It needs no separate hostless check, because `url` rejects an `http(s)` URL without a host. It makes no policy decision and reads no settings.

It trims the input, maps a protocol-relative `//host/...` to `https://host/...`, and accepts only a case-insensitive `http://` or `https://` prefix. It parses absolute input without an intermediate `format!`, and returns `None` for empty, relative, non-http(s) or unparseable input. It needs no separate hostless check, because `url` rejects an `http(s)` URL without a host. It makes no policy decision and reads no settings.

[prk-Jr]

⛏ nitpick — CONTRIBUTING.md:44-53 doesn't ask for --signoff/-S (and -S fails without a GPG key), asks for subjects around 50 characters (four here are 74–79) and a wrapped body (the -m bodies are single ~230-char lines).

Suggested change

	- Commits use sentence case, imperative mood, and `git commit --signoff -S` with multiple `-m` flags. No prefixes, no attribution trailers, never a heredoc.
	- Commits follow `CONTRIBUTING.md`: sentence case, imperative mood, subject near 50 characters, wrapped body. No prefixes, no attribution trailers.

[prk-Jr]

🤔 thinking — rewrite_creative_html / rewrite_inline_creative_html are pub in pub mod creative with only test callers (proxy.rs:3543 is inside #[cfg(test)], module starts at :2160). Pinning them to CreativeFeatures::ALL means any future integration calling them silently bypasses both switches. Consider pub(crate)/test-only, or a doc warning.

[aram356]

@dhruv8sh please merge into implementation PR #1252