fix: harden click tracking against open redirect and URL tampering

- Include URL in HMAC-SHA256 openhash so click tracking links are
  tamper-proof; replacing the `url` parameter invalidates the hash
- Add URL scheme validation in track_click to reject non-http(s) URLs
- Set SameSite=Strict and conditional Secure flag on admin session cookie
  based on whether BASE_URL uses HTTPS

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: yoyo930021

src/newsletter.rs

@@ -119,23 +119,25 @@ pub fn personalize_email(
 
 /// Rewrite all http/https links in HTML to go through `/r/c` click tracking.
 /// Each link becomes `/r/c?ucode=...&topic=...&hash=...&url=<original>`.
-/// This is per-subscriber (each subscriber gets their own openhash).
+/// The hash is HMAC-SHA256 over (ucode, topic, url), so the URL is tamper-proof.
+/// This is per-subscriber (each subscriber gets their own hash per link).
 pub fn rewrite_links_for_tracking(
     html: &str,
     base_url: &str,
     ucode: &str,
     topic: &str,
-    openhash: &str,
+    secret_code: &str,
 ) -> String {
     let re = Regex::new(r#"href="(https?://[^"]+)""#).expect("valid regex");
     re.replace_all(html, |caps: &regex::Captures| {
         let original_url = &caps[1];
+        let hash = security::compute_openhash(secret_code, ucode, topic, original_url);
         let tracking_url = format!(
             "{}/r/c?ucode={}&topic={}&hash={}&url={}",
             base_url,
             urlencoding::encode(ucode),
             urlencoding::encode(topic),
-            urlencoding::encode(openhash),
+            urlencoding::encode(&hash),
             urlencoding::encode(original_url),
         );
         format!("href=\"{tracking_url}\"")
@@ -292,17 +294,17 @@ pub async fn send_newsletter(
             continue;
         }
 
-        // Compute per-subscriber tracking
-        let openhash = security::compute_openhash(secret_code, ucode, &slug);
+        // Compute per-subscriber open-tracking pixel hash (no URL)
+        let openhash = security::compute_openhash(secret_code, ucode, &slug, "");
         let tracking_pixel = build_tracking_pixel(&state.config.base_url, ucode, &slug, &openhash);
 
-        // Rewrite links for per-subscriber click tracking
+        // Rewrite links for per-subscriber click tracking (each link gets its own HMAC)
         let tracked_html = rewrite_links_for_tracking(
             &shortened_html,
             &state.config.base_url,
             ucode,
             &slug,
-            &openhash,
+            secret_code,
         );
         let tracked_html = replace_recipient_name(&tracked_html, name);
 
@@ -717,26 +719,39 @@ mod tests {
 
     #[test]
     fn test_rewrite_links_for_tracking() {
-        let html = r#"<a href="https://coscup.org">COSCUP</a> and <a href="https://example.com/page">Example</a>"#;
+        let secret = "mysecret";
+        let ucode = "abc123";
+        let topic = "nl-01";
+        let url1 = "https://coscup.org";
+        let url2 = "https://example.com/page";
+
+        let html = format!(r#"<a href="{url1}">COSCUP</a> and <a href="{url2}">Example</a>"#);
         let result = rewrite_links_for_tracking(
-            html,
+            &html,
             "https://newsletter.coscup.org",
-            "abc123",
-            "nl-01",
-            "myhash",
+            ucode,
+            topic,
+            secret,
         );
+
         assert!(result.contains("/r/c?"));
         assert!(result.contains("ucode=abc123"));
         assert!(result.contains("topic=nl-01"));
-        assert!(result.contains("hash=myhash"));
         assert!(result.contains("url=https%3A%2F%2Fcoscup.org"));
         assert!(result.contains("url=https%3A%2F%2Fexample.com%2Fpage"));
+
+        // Each link has its own per-URL hash
+        let hash1 = security::compute_openhash(secret, ucode, topic, url1);
+        let hash2 = security::compute_openhash(secret, ucode, topic, url2);
+        assert_ne!(hash1, hash2);
+        assert!(result.contains(&urlencoding::encode(&hash1).to_string()));
+        assert!(result.contains(&urlencoding::encode(&hash2).to_string()));
     }
 
     #[test]
     fn test_rewrite_links_skips_non_http() {
         let html = r##"<a href="mailto:hi@coscup.org">Mail</a> <a href="#top">Top</a>"##;
-        let result = rewrite_links_for_tracking(html, "https://x.com", "u", "t", "h");
+        let result = rewrite_links_for_tracking(html, "https://x.com", "u", "t", "secret");
         // Non-http links should be unchanged
         assert!(result.contains("mailto:hi@coscup.org"));
         assert!(result.contains("#top"));

src/routes/admin.rs

@@ -3,6 +3,7 @@ use std::net::SocketAddr;
 use axum::extract::{ConnectInfo, Multipart, Path, Query, State};
 use axum::http::{header, HeaderMap};
 use axum::response::{Html, IntoResponse, Redirect, Response};
+use axum_extra::extract::cookie::SameSite;
 use axum_extra::extract::CookieJar;
 use chrono::Utc;
 use serde::Deserialize;
@@ -127,9 +128,12 @@ pub async fn auth_magic_link(
     )
     .await;
 
+    let is_https = state.config.base_url.starts_with("https://");
     let cookie = axum_extra::extract::cookie::Cookie::build((SESSION_COOKIE, session_token))
         .path("/admin")
         .http_only(true)
+        .secure(is_https)
+        .same_site(SameSite::Strict)
         .max_age(time::Duration::hours(24))
         .build();
 
@@ -418,7 +422,7 @@ pub async fn export_csv(State(state): State<AppState>) -> Result<Response, AppEr
         .into_iter()
         .map(|(email, name, ucode, status, secret_code)| {
             let admin_link = security::compute_admin_link(&secret_code, &email);
-            let openhash = security::compute_openhash(&secret_code, &ucode, "");
+            let openhash = security::compute_openhash(&secret_code, &ucode, "", "");
             ExportCsvRecord {
                 email,
                 name,

src/routes/tracking.rs

@@ -38,7 +38,7 @@ pub async fn track_open(
             .await?;
 
     if let Some((secret_code,)) = subscriber {
-        if security::verify_openhash(&secret_code, &query.ucode, &query.topic, &query.hash) {
+        if security::verify_openhash(&secret_code, &query.ucode, &query.topic, "", &query.hash) {
             let user_agent = headers
                 .get(header::USER_AGENT)
                 .and_then(|v| v.to_str().ok())
@@ -75,6 +75,11 @@ pub async fn track_click(
         .as_deref()
         .ok_or_else(|| AppError::BadRequest("Missing url parameter".to_string()))?;
 
+    // Validate redirect URL to prevent open redirect attacks
+    if !redirect_url.starts_with("https://") && !redirect_url.starts_with("http://") {
+        return Err(AppError::BadRequest("Invalid redirect URL".to_string()));
+    }
+
     // Verify openhash
     let subscriber =
         sqlx::query_as::<_, (String,)>("SELECT secret_code FROM subscribers WHERE ucode = $1")
@@ -83,7 +88,13 @@ pub async fn track_click(
             .await?;
 
     if let Some((secret_code,)) = subscriber {
-        if security::verify_openhash(&secret_code, &query.ucode, &query.topic, &query.hash) {
+        if security::verify_openhash(
+            &secret_code,
+            &query.ucode,
+            &query.topic,
+            redirect_url,
+            &query.hash,
+        ) {
             let user_agent = headers
                 .get(header::USER_AGENT)
                 .and_then(|v| v.to_str().ok())

src/security.rs

@@ -34,11 +34,12 @@ pub fn compute_admin_link(secret_code: &str, email: &str) -> String {
     hex::encode(hasher.finalize())
 }
 
-/// Compute openhash = HMAC-SHA256(secret_code, "ucode:topic").
-pub fn compute_openhash(secret_code: &str, ucode: &str, topic: &str) -> String {
+/// Compute openhash = HMAC-SHA256(secret_code, "ucode:topic:url").
+/// For open-tracking (no URL), pass `url = ""`.
+pub fn compute_openhash(secret_code: &str, ucode: &str, topic: &str, url: &str) -> String {
     let mut mac =
         HmacSha256::new_from_slice(secret_code.as_bytes()).expect("HMAC accepts any key length");
-    let message = format!("{ucode}:{topic}");
+    let message = format!("{ucode}:{topic}:{url}");
     mac.update(message.as_bytes());
     hex::encode(mac.finalize().into_bytes())
 }
@@ -54,8 +55,15 @@ pub fn verify_admin_link(provided: &str, expected: &str) -> bool {
 }
 
 /// Verify openhash using constant-time comparison.
-pub fn verify_openhash(secret_code: &str, ucode: &str, topic: &str, provided: &str) -> bool {
-    let expected = compute_openhash(secret_code, ucode, topic);
+/// For open-tracking (no URL), pass `url = ""`.
+pub fn verify_openhash(
+    secret_code: &str,
+    ucode: &str,
+    topic: &str,
+    url: &str,
+    provided: &str,
+) -> bool {
+    let expected = compute_openhash(secret_code, ucode, topic, url);
     verify_admin_link(provided, &expected)
 }
 
@@ -116,27 +124,77 @@ mod tests {
 
     #[test]
     fn test_compute_openhash_deterministic() {
-        let h1 = compute_openhash("secret", "abc123", "newsletter-01");
-        let h2 = compute_openhash("secret", "abc123", "newsletter-01");
+        let h1 = compute_openhash("secret", "abc123", "newsletter-01", "");
+        let h2 = compute_openhash("secret", "abc123", "newsletter-01", "");
         assert_eq!(h1, h2);
     }
 
+    #[test]
+    fn test_compute_openhash_url_changes_hash() {
+        let h1 = compute_openhash("secret", "abc123", "newsletter-01", "");
+        let h2 = compute_openhash("secret", "abc123", "newsletter-01", "https://coscup.org");
+        assert_ne!(h1, h2);
+    }
+
     #[test]
     fn test_verify_openhash_correct() {
-        let hash = compute_openhash("secret", "abc123", "newsletter-01");
-        assert!(verify_openhash("secret", "abc123", "newsletter-01", &hash));
+        let hash = compute_openhash("secret", "abc123", "newsletter-01", "");
+        assert!(verify_openhash(
+            "secret",
+            "abc123",
+            "newsletter-01",
+            "",
+            &hash
+        ));
+    }
+
+    #[test]
+    fn test_verify_openhash_correct_with_url() {
+        let url = "https://coscup.org/2025";
+        let hash = compute_openhash("secret", "abc123", "newsletter-01", url);
+        assert!(verify_openhash(
+            "secret",
+            "abc123",
+            "newsletter-01",
+            url,
+            &hash
+        ));
+    }
+
+    #[test]
+    fn test_verify_openhash_wrong_url() {
+        let hash = compute_openhash("secret", "abc123", "newsletter-01", "https://coscup.org");
+        assert!(!verify_openhash(
+            "secret",
+            "abc123",
+            "newsletter-01",
+            "https://evil.com",
+            &hash
+        ));
     }
 
     #[test]
     fn test_verify_openhash_wrong_topic() {
-        let hash = compute_openhash("secret", "abc123", "newsletter-01");
-        assert!(!verify_openhash("secret", "abc123", "newsletter-02", &hash));
+        let hash = compute_openhash("secret", "abc123", "newsletter-01", "");
+        assert!(!verify_openhash(
+            "secret",
+            "abc123",
+            "newsletter-02",
+            "",
+            &hash
+        ));
     }
 
     #[test]
     fn test_verify_openhash_wrong_secret() {
-        let hash = compute_openhash("secret", "abc123", "newsletter-01");
-        assert!(!verify_openhash("wrong", "abc123", "newsletter-01", &hash));
+        let hash = compute_openhash("secret", "abc123", "newsletter-01", "");
+        assert!(!verify_openhash(
+            "wrong",
+            "abc123",
+            "newsletter-01",
+            "",
+            &hash
+        ));
     }
 
     #[test]