From 515efce99162096ff2ef7adf4cd01765e0d6425d Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Thu, 11 Jun 2026 08:53:29 +0200 Subject: [PATCH] feat: support TRACKING_LINK_SECRET_PREVIOUS rotation grace on the tracking service so rotating the click-signing key never breaks links in already-delivered emails --- AGENTS.md | 1 + deploy/config/env.example | 5 +++++ tracking/src/config.rs | 22 +++++++++++++++++++++- tracking/src/handlers.rs | 28 +++++++++++++++++++++++----- 4 files changed, 50 insertions(+), 6 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e3bdcbff..f50c67aa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -662,6 +662,7 @@ The tracking service additionally defends itself before any event reaches Kafka - prefetch/scanner filtering: `Sec-Purpose`/`Purpose`-style prefetch headers and a UA marker list (crawlers, CLI clients, chat-app link previews, email security gateways) are served but never counted. Gmail's image proxy is deliberately NOT filtered — it is the only open signal Gmail exposes - URL caps on click redirects: 4096 bytes raw / 2048 decoded - signed click links: the Go sender appends `&s=` when `TRACKING_LINK_SECRET` is set (`internal/tasks/template.go`); when the tracking service has the same secret it refuses unsigned/mis-signed redirects with `404`. This closes the open-redirector hole. Rollout order matters: set the secret on the backend first, enable enforcement on the tracking service only after unsigned in-flight emails have aged out +- key rotation: the tracking service also accepts `TRACKING_LINK_SECRET_PREVIOUS` (ignored without a current secret). Rotating = move the old value to the previous slot, sign new sends with the new current value, unset the previous slot once old emails have aged out. Never rotate by replacing the current secret alone; that 404s every link in already-delivered emails - `internal/app/advanced/service.go` - `internal/repository/pg_advanced_outreach.go` - `internal/repository/pg_subscription.go` diff --git a/deploy/config/env.example b/deploy/config/env.example index 54a8a196..8c86a142 100644 --- a/deploy/config/env.example +++ b/deploy/config/env.example @@ -86,6 +86,11 @@ TRACKING_PORT=3000 # redirects are refused, so enable it on the backend first and let unsigned # in-flight emails age out before enforcing here. Empty = legacy unsigned. TRACKING_LINK_SECRET= +# Key rotation: move the retired secret here (tracking service only) so links +# in already-delivered emails keep working; new sends sign with the current +# secret above. Unset once old emails have aged out. Ignored when +# TRACKING_LINK_SECRET is empty. +TRACKING_LINK_SECRET_PREVIOUS= # Per-source request budget for the tracking endpoints (default 300/min) TRACKING_RATE_LIMIT_PER_MIN=300 diff --git a/tracking/src/config.rs b/tracking/src/config.rs index e82d27af..09c6dbdd 100644 --- a/tracking/src/config.rs +++ b/tracking/src/config.rs @@ -34,6 +34,10 @@ pub struct Config { /// Shared secret for HMAC-signed click redirects. When set, unsigned or /// mis-signed /t/c/ requests are refused (open-redirect protection). pub link_secret: Option, + /// The retired secret during a key rotation: links signed with it keep + /// verifying until in-flight emails age out. Ignored unless link_secret + /// is also set. + pub link_secret_previous: Option, /// Per-source request budget for both tracking endpoints (default 300/min). pub rate_limit_per_min: u32, } @@ -129,8 +133,18 @@ impl Config { Self::get_secret_optional("TRACKING_LINK_SECRET", "tracking/link_secret", &secrets) .await .filter(|s| !s.is_empty()); + let link_secret_previous = Self::get_secret_optional( + "TRACKING_LINK_SECRET_PREVIOUS", + "tracking/link_secret_previous", + &secrets, + ) + .await + .filter(|s| !s.is_empty()); if link_secret.is_some() { - info!("Signed click redirects enforced"); + info!( + "Signed click redirects enforced (rotation grace: {})", + link_secret_previous.is_some() + ); } let rate_limit_per_min: u32 = env::var("TRACKING_RATE_LIMIT_PER_MIN") @@ -151,6 +165,7 @@ impl Config { schema_registry_key, schema_registry_secret, link_secret, + link_secret_previous, rate_limit_per_min, }) } @@ -205,6 +220,10 @@ impl Config { .get_optional("tracking/link_secret") .await .filter(|s| !s.is_empty()); + let link_secret_previous = secrets + .get_optional("tracking/link_secret_previous") + .await + .filter(|s| !s.is_empty()); Ok(Self { env: env.to_string(), @@ -218,6 +237,7 @@ impl Config { schema_registry_key, schema_registry_secret, link_secret, + link_secret_previous, rate_limit_per_min: 300, }) } diff --git a/tracking/src/handlers.rs b/tracking/src/handlers.rs index 7c2554b1..6fd6cf20 100644 --- a/tracking/src/handlers.rs +++ b/tracking/src/handlers.rs @@ -38,8 +38,10 @@ pub struct AppState { pub dedupe_cache: Arc, /// Per-source request budget (anti-flood) pub rate_limiter: Arc, - /// Shared secret for signed click redirects; None = legacy unsigned links - pub link_secret: Option>, + /// Accepted signing secrets for click redirects, newest first (the + /// retired key rides along during a rotation so in-flight emails keep + /// working). None = legacy unsigned links. + pub link_secrets: Option>>, } impl AppState { @@ -54,11 +56,21 @@ impl AppState { .time_to_idle(Duration::from_secs(1800)) // 30 min idle .build(); + // Enforcement is keyed on the CURRENT secret: a leftover previous + // secret with no current one means signing was turned off. + let link_secrets = config.link_secret.clone().map(|current| { + let mut secrets = vec![current]; + if let Some(previous) = config.link_secret_previous.clone() { + secrets.push(previous); + } + Arc::new(secrets) + }); + Self { kafka, dedupe_cache: Arc::new(dedupe_cache), rate_limiter: Arc::new(RateLimiter::new(config.rate_limit_per_min)), - link_secret: config.link_secret.clone().map(Arc::new), + link_secrets, } } @@ -187,8 +199,14 @@ pub async fn track_click( // Signed-link enforcement: when the shared secret is configured, only // redirects minted by our own send pipeline are honored. This is what // stops the tracking domain from being abused as an open redirector. - if let Some(secret) = &state.link_secret { - if !verify_signature(secret, &task_id, &original_url, params.get("s").map(String::as_str)) { + // Any configured key may match (current, or the previous one during a + // rotation grace window). + if let Some(secrets) = &state.link_secrets { + let sig = params.get("s").map(String::as_str); + if !secrets + .iter() + .any(|secret| verify_signature(secret, &task_id, &original_url, sig)) + { return (StatusCode::NOT_FOUND, "Unknown link").into_response(); } }