Add HttpOnly flag to synthetic ID cookie

crates/common/src/cookies.rs

@@ -61,13 +61,57 @@ pub fn handle_request_cookies(
 
 /// Creates a synthetic ID cookie string.
 ///
-/// Generates a properly formatted cookie with security attributes
-/// for storing the synthetic ID.
+/// Generates a `Set-Cookie` header value with the following security attributes:
+/// - `Secure`: transmitted over HTTPS only.
+/// - `HttpOnly`: inaccessible to JavaScript (`document.cookie`), blocking XSS exfiltration.
+///   Safe to set because integrations receive the synthetic ID via the `x-synthetic-id`
+///   response header instead of reading it from the cookie directly.
+/// - `SameSite=Lax`: sent on same-site requests and top-level cross-site navigations.
+///   `Strict` is intentionally avoided — it would suppress the cookie on the first
+///   request when a user arrives from an external page, breaking first-visit attribution.
+/// - `Max-Age`: 1 year retention.
+///
+/// The `synthetic_id` is sanitized via an allowlist before embedding in the cookie value.
+/// Only ASCII alphanumeric characters and `.`, `-`, `_` are permitted — matching the
+/// known synthetic ID format (`{64-char-hex}.{6-char-alphanumeric}`). Any stripped
+/// characters are logged as a warning so header/cookie mismatches can be detected.
+///
+/// The `cookie_domain` is validated at config load time via [`validator::Validate`] on
+/// [`crate::settings::Publisher`]; bad config fails at startup, not per-request.
+///
+/// # Examples
+///
+/// ```no_run
+/// # use trusted_server_common::cookies::create_synthetic_cookie;
+/// # use trusted_server_common::settings::Settings;
+/// // `settings` is loaded at startup via `Settings::from_toml_and_env`.
+/// # fn example(settings: &Settings) {
+/// let cookie = create_synthetic_cookie(settings, "abc123.xk92ab");
+/// assert!(cookie.contains("HttpOnly"));
+/// assert!(cookie.contains("Secure"));
+/// # }
+/// ```
 #[must_use]
 pub fn create_synthetic_cookie(settings: &Settings, synthetic_id: &str) -> String {
+    // Sanitize synthetic_id at runtime using an allowlist: only ASCII alphanumeric
+    // and `.`, `-`, `_` are permitted. This is stricter than a denylist and covers
+    // NUL bytes, spaces, tabs, and other control characters that a denylist would miss.
+    // Synthetic IDs originating from the x-synthetic-id request header are untrusted.
+    let safe_id: String = synthetic_id
+        .chars()
+        .filter(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | '_'))
+        .collect();
+    if safe_id.len() != synthetic_id.len() {
+        log::warn!(
+            "Stripped disallowed characters from synthetic_id before setting cookie (len {} -> {}); \
+             the x-synthetic-id response header may differ from the cookie value",
+            synthetic_id.len(),
+            safe_id.len(),
+        );
+    }
     format!(
-        "{}={}; Domain={}; Path=/; Secure; SameSite=Lax; Max-Age={}",
-        COOKIE_SYNTHETIC_ID, synthetic_id, settings.publisher.cookie_domain, COOKIE_MAX_AGE,
+        "{}={}; Domain={}; Path=/; Secure; HttpOnly; SameSite=Lax; Max-Age={}",
+        COOKIE_SYNTHETIC_ID, safe_id, settings.publisher.cookie_domain, COOKIE_MAX_AGE,
     )
 }
 
@@ -174,12 +218,43 @@ mod tests {
         assert_eq!(
             result,
             format!(
-                "{}=12345; Domain={}; Path=/; Secure; SameSite=Lax; Max-Age={}",
+                "{}=12345; Domain={}; Path=/; Secure; HttpOnly; SameSite=Lax; Max-Age={}",
                 COOKIE_SYNTHETIC_ID, settings.publisher.cookie_domain, COOKIE_MAX_AGE,
             )
         );
     }
 
+    #[test]
+    fn test_create_synthetic_cookie_sanitizes_disallowed_chars_in_id() {
+        let settings = create_test_settings();
+        // Allowlist permits only ASCII alphanumeric, '.', '-', '_'.
+        // ';', '=', '\r', '\n', spaces, NUL bytes, and other control chars are all stripped.
+        let result = create_synthetic_cookie(&settings, "evil;injected\r\nfoo=bar\0baz");
+        // Extract the value portion anchored to the cookie name constant to
+        // avoid false positives from disallowed chars in cookie attributes.
+        let value = result
+            .strip_prefix(&format!("{}=", COOKIE_SYNTHETIC_ID))
+            .and_then(|s| s.split_once(';').map(|(v, _)| v))
+            .expect("should have cookie value portion");
+        assert_eq!(
+            value, "evilinjectedfoobarbaz",
+            "should strip disallowed characters and preserve safe chars"
+        );
+    }
+
+    #[test]
+    fn test_create_synthetic_cookie_preserves_well_formed_id() {
+        let settings = create_test_settings();
+        // A well-formed ID should pass through the allowlist unmodified.
+        let id = "abc123def0123456789abcdef0123456789abcdef0123456789abcdef01234567.xk92ab";
+        let result = create_synthetic_cookie(&settings, id);
+        let value = result
+            .strip_prefix(&format!("{}=", COOKIE_SYNTHETIC_ID))
+            .and_then(|s| s.split_once(';').map(|(v, _)| v))
+            .expect("should have cookie value portion");
+        assert_eq!(value, id, "should not modify a well-formed synthetic ID");
+    }
+
     #[test]
     fn test_set_synthetic_cookie() {
         let settings = create_test_settings();

crates/common/src/settings.rs

@@ -18,6 +18,7 @@ pub const ENVIRONMENT_VARIABLE_SEPARATOR: &str = "__";
 #[derive(Debug, Default, Clone, Deserialize, Serialize, Validate)]
 pub struct Publisher {
     pub domain: String,
+    #[validate(custom(function = validate_cookie_domain))]
     pub cookie_domain: String,
     #[validate(custom(function = validate_no_trailing_slash))]
     pub origin_url: String,
@@ -448,6 +449,18 @@ impl Settings {
     }
 }
 
+fn validate_cookie_domain(value: &str) -> Result<(), ValidationError> {
+    // `=` is excluded: it only has special meaning in the name=value pair,
+    // not within the Domain attribute value.
+    if value.contains([';', '\n', '\r']) {
+        let mut err = ValidationError::new("cookie_metacharacters");
+        err.message =
+            Some("cookie_domain must not contain cookie metacharacters (;, \\n, \\r)".into());
+        return Err(err);
+    }
+    Ok(())
+}
+
 fn validate_no_trailing_slash(value: &str) -> Result<(), ValidationError> {
     if value.ends_with('/') {
         let mut err = ValidationError::new("trailing_slash");
@@ -1242,6 +1255,32 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_publisher_rejects_cookie_domain_with_metacharacters() {
+        for bad_domain in [
+            "evil.com;\nSet-Cookie: bad=1",
+            "evil.com\r\nX-Injected: yes",
+            "evil.com;path=/",
+        ] {
+            let mut settings = create_test_settings();
+            settings.publisher.cookie_domain = bad_domain.to_string();
+            assert!(
+                settings.validate().is_err(),
+                "should reject cookie_domain containing metacharacters: {bad_domain:?}"
+            );
+        }
+    }
+
+    #[test]
+    fn test_publisher_accepts_valid_cookie_domain() {
+        let mut settings = create_test_settings();
+        settings.publisher.cookie_domain = ".example.com".to_string();
+        assert!(
+            settings.validate().is_ok(),
+            "should accept a valid cookie_domain"
+        );
+    }
+
     /// Helper that returns a settings TOML string WITHOUT any admin handler,
     /// for tests that need to verify uncovered-admin-endpoint behaviour.
     fn settings_str_without_admin_handler() -> String {