Skip to content

Conversation

@prk-Jr
Copy link
Collaborator

@prk-Jr prk-Jr commented Feb 6, 2026

Integration proxy responses were missing the x-synthetic-id header because handle_proxy dispatched to integrations without adding it. This caused identity tracking to break for first-party re-hosted integrations like Permutive's secure-signal endpoint.

Centralizing the header logic in handle_proxy ensures all current and future integrations automatically include the synthetic ID, rather than requiring each integration to implement it manually.

Fixes #205

Integration proxy responses were missing the x-synthetic-id header
because handle_proxy dispatched to integrations without adding it.
This caused identity tracking to break for first-party re-hosted
integrations like Permutive's secure-signal endpoint.

Centralizing the header logic in handle_proxy ensures all current
and future integrations automatically include the synthetic ID,
rather than requiring each integration to implement it manually.

Fixes #205
Copy link
Collaborator

@aram356 aram356 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start but should use funcitons in cookies.rs

Please don't make the feature branch have this long name. Please stick to feature/short-name

Replaces inline cookie parsing with handle_request_cookies() and
parse_cookies_to_jar() from cookies.rs for consistency.
@prk-Jr prk-Jr requested a review from aram356 February 9, 2026 05:17
Had deleted the comment accidentally but was needed already
Copy link
Collaborator

@aram356 aram356 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 We need a more thorough fix:

publisher.rs:201-211 uses manual string splitting to check for an existing synthetic cookie:

let has_synthetic_cookie = req
    .get_header(header::COOKIE)
    .and_then(|h| h.to_str().ok())
    .map(|cookies| {
        cookies.split(';').any(|cookie| {
            cookie.trim_start().starts_with(&format!("{}=", COOKIE_SYNTHETIC_ID))
        })
    })
    .unwrap_or(false);

While registry.rs:622-626 uses handle_request_cookies with CookieJar:

let has_synthetic_cookie = handle_request_cookies(&req)
    .ok()
    .flatten()
    .and_then(|jar| jar.get(COOKIE_SYNTHETIC_ID).map(|_| true))
    .unwrap_or(false);

These should use the same approach. The CookieJar version is more robust (proper cookie parsing).

Since we already have cookie utilities in cookies.rs, suggest adding a set_synthetic_cookie(settings: &Settings, response: &mut Response, synthetic_id: &str) helper there that handles the append_header call. That way both publisher.rs and registry.rs call the same function, and the append_header fix and cookie creation logic live in one place.

We don't need has_synthetic_cookie check we was primarily for logging we should always set COOKIE_SYNTHETIC_ID

assert!(!registry.has_route(&Method::GET, "/integrations/test/users"));
assert!(!registry.has_route(&Method::POST, "/integrations/test/users"));
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 Duplicate test

These three cookie_jar_* tests are testing parse_cookies_to_jar from cookies.rs — they should live there, not in the registry module. cookies.rs already has equivalent coverage (test_parse_cookies_to_jar, test_parse_cookies_to_jar_emtpy, etc.), so these are duplicates and can be removed.

@prk-Jr prk-Jr requested a review from aram356 February 11, 2026 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Synthetic ID Header [x-synthetic-id] is not being set on /integration re-hosted into first party publisher domain

3 participants