Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Lockr and Permutive SDK recognition now uses parsed vendor hostnames and SDK pathnames instead of searching the whole URL (#1199). The browser Lockr guard no longer replaces unrelated scripts that mention a Lockr host in their path or query, use a lookalike host, or serve a non-SDK resource from a Lockr host. Query strings and fragments no longer prevent SDK recognition in the server matchers, the browser Permutive guard, or the browser Lockr guard on `identity.loc.kr`. Matching URLs use the existing configured-SDK routes; their original query and fragment are not forwarded upstream. Server matching trims leading and trailing C0 controls and spaces, removes tab, CR, and LF, and accepts backslashes as authority separators. It no longer recognizes attribute values that contain a NUL or that write slashes as character references such as `/`, so markup like that can load the SDK from the vendor host.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

📝 note: main now conflicts with this entry, because #1107 changed an example URL in the rewrite.exclude_domains entry just below it. GitHub reports the PR as conflicting, so it hasn't run the test or format workflows on bb8a85e0f; only CodeQL ran. The resolution is a union: keep main's file and put this entry first under ### Fixed.

I ran that merge locally (git merge origin/main, union on this file only). The merged tree differs from main in exactly this PR's 16 files. main also moved EdgeZero to v0.0.11 in #1107, which this PR's Rust changes don't call. On the merged tree, these pass:

CI runs the rest once the conflict is resolved.

- `rewrite.exclude_domains` matching is now case-insensitive and ignores surrounding whitespace. Entries written with uppercase letters previously never matched and now take effect, so those hosts stop being proxied and click-wrapped, and `/first-party/sign` now rejects them with `502`; it used to sign them, or return `403` when the host was also outside `proxy.allowed_domains`. Empty and bare `"*"` `exclude_domains` entries, which never matched a host, are dropped at load with a warning, and other entries that can never match a host are logged with a warning. Absolute `http(s)` creative URLs that cannot be parsed (for example, a host containing a space) are now left untouched; previously the trimmed value was written back re-quoted (with `https:` prepended to `//` input, so `src=" //exa mple.example/x "` became `https://exa mple.example/x`), an unparseable quoted URL in `<style>` or `style=` was re-quoted through `cssparser::serialize_string`, and a link also gained `data-tsclick`. `/first-party/sign` now answers unparseable `http(s)` input with `unsupported url` instead of `invalid url`; both map to `502`. The first `ts config push` after upgrading shows a one-time `exclude_domains` diff for configs with mixed-case, padded, empty or bare `"*"` entries. Audit `exclude_domains` for mixed-case entries before upgrading.
- Next.js and Google Tag Manager script rewrites now compose in streamed HTML instead of overwriting each other, preserving Pages Router JSON and App Router Flight payloads with recomputed decoded `T`-record lengths. GTM/GA Flight rewrites require both integrations to be enabled. Buffering is bounded and per document; an uncaptured data head (including inert text quoting a Flight push), an unsupported or binary push, or a size limit keeps the remaining Flight stream raw and restores queued originals, but cannot undo already-streamed rewrites. Complete serialized form state does not disable later rewriting, foreign property references no longer hide a later qualified Flight push, and placeholder-stage document fallback emits a reason-bearing warning without source data (#1206, #1207, #1208).
- Publisher HTML is now processed when the request carries its host only in the URI (absolute-form HTTP/1.1, or HTTP/2 without a usable `Host`), instead of being returned unmodified with an "Empty request host" warning (#1118). URI userinfo is excluded from rewritten URLs, while ports and IPv6 brackets are preserved.
Expand Down
50 changes: 46 additions & 4 deletions crates/trusted-server-core/src/integrations/lockr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use validator::Validate;

use crate::constants::INTERNAL_HEADERS;
use crate::error::TrustedServerError;
use crate::integrations::sdk_url::parse_sdk_network_url;
use crate::integrations::{
AttributeRewriteAction, INTEGRATION_MAX_BODY_BYTES, IntegrationAttributeContext,
IntegrationAttributeRewriter, IntegrationEndpoint, IntegrationProxy, IntegrationRegistration,
Expand Down Expand Up @@ -98,10 +99,15 @@ impl LockrIntegration {

/// Check if a URL is a Lockr SDK URL.
fn is_lockr_sdk_url(&self, url: &str) -> bool {
let lower = url.to_ascii_lowercase();
(lower.contains("aim.loc.kr") || lower.contains("identity.loc.kr"))
&& lower.contains("identity-lockr")
&& lower.ends_with(".js")
// Keep browser/Rust cases in crates/trusted-server-js/lib/test/fixtures/sdk-url-ownership.json aligned.
let Some(parsed) = parse_sdk_network_url(url) else {
return false;
};
let host = parsed.host_str().unwrap_or_default();
let path = parsed.path().to_ascii_lowercase();
matches!(host, "aim.loc.kr" | "identity.loc.kr")
&& path.contains("identity-lockr")
&& path.ends_with(".js")
}

/// Handle SDK serving — fetch from Lockr CDN and serve through first-party domain.
Expand Down Expand Up @@ -437,6 +443,9 @@ mod tests {
use edgezero_core::http::Method as HttpMethod;
use serde_json::json;

use crate::integrations::sdk_url_test_support::{
assert_sdk_html_attribute_rewrite, assert_sdk_url_rewrite, sdk_url_cases,
};
use crate::platform::test_support::{StubHttpClient, build_services_with_http_client};
use crate::test_support::tests::create_test_settings;

Expand All @@ -463,6 +472,29 @@ mod tests {
}
}

#[test]
fn sdk_url_ownership_matches_shared_corpus() {
let integration = LockrIntegration::new(test_config());

for case in sdk_url_cases() {
let matches = case.owner.as_deref() == Some(LOCKR_INTEGRATION_ID);
assert_eq!(
integration.is_lockr_sdk_url(&case.url),
matches,
"should match expected lockr ownership for {}: {}",
case.name,
case.url
);

assert_sdk_url_rewrite(integration.as_ref(), &case, "/integrations/lockr/sdk");
assert_sdk_html_attribute_rewrite(
integration.as_ref(),
&case,
"/integrations/lockr/sdk",
);
}
}

#[test]
fn test_lockr_sdk_url_detection() {
let integration = LockrIntegration::new(test_config());
Expand All @@ -472,6 +504,16 @@ mod tests {
assert!(integration.is_lockr_sdk_url("https://aim.loc.kr/identity-lockr-trust-server.js"));
assert!(integration.is_lockr_sdk_url("https://identity.loc.kr/identity-lockr-v2.0.js"));

// Hosts that only end with a Lockr SDK host are different hosts.
assert!(
!integration.is_lockr_sdk_url("https://notaim.loc.kr/identity-lockr.js"), // allow-domain: notaim.loc.kr
"should require an exact aim.loc.kr hostname"
);
assert!(
!integration.is_lockr_sdk_url("https://notidentity.loc.kr/identity-lockr.js"), // allow-domain: notidentity.loc.kr
"should require an exact identity.loc.kr hostname"
);

// Should not match non-SDK resources on Lockr domains
assert!(
!integration.is_lockr_sdk_url("https://aim.loc.kr/pixel.gif"),
Expand Down
3 changes: 3 additions & 0 deletions crates/trusted-server-core/src/integrations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ pub mod osano;
pub mod permutive;
pub mod prebid;
mod registry;
mod sdk_url;
#[cfg(test)]
mod sdk_url_test_support;
pub mod sourcepoint;
pub mod testlight;

Expand Down
50 changes: 47 additions & 3 deletions crates/trusted-server-core/src/integrations/permutive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use validator::Validate;

use crate::constants::INTERNAL_HEADERS;
use crate::error::TrustedServerError;
use crate::integrations::sdk_url::parse_sdk_network_url;
use crate::integrations::{
AttributeRewriteAction, INTEGRATION_MAX_BODY_BYTES, IntegrationAttributeContext,
IntegrationAttributeRewriter, IntegrationEndpoint, IntegrationProxy, IntegrationRegistration,
Expand Down Expand Up @@ -99,9 +100,17 @@ impl PermutiveIntegration {

/// Check if a URL is a Permutive SDK URL.
fn is_permutive_sdk_url(&self, url: &str) -> bool {
let lower = url.to_ascii_lowercase();
(lower.contains(".edge.permutive.app") || lower.contains("cdn.permutive.com"))
&& lower.ends_with("-web.js")
// Keep browser/Rust cases in crates/trusted-server-js/lib/test/fixtures/sdk-url-ownership.json aligned.
let Some(parsed) = parse_sdk_network_url(url) else {
return false;
};
let host = parsed.host_str().unwrap_or_default();
let path = parsed.path().to_ascii_lowercase();
(host == "cdn.permutive.com"
|| host
.strip_suffix(".edge.permutive.app")
.is_some_and(|subdomain| !subdomain.is_empty()))
&& path.ends_with("-web.js")
}

/// Handle SDK serving - fetch from Permutive CDN and serve through first-party domain.
Expand Down Expand Up @@ -474,9 +483,44 @@ mod tests {
use std::sync::Arc;

use super::*;
use crate::integrations::sdk_url_test_support::{
assert_sdk_html_attribute_rewrite, assert_sdk_url_rewrite, sdk_url_cases,
};
use crate::platform::test_support::{StubHttpClient, build_services_with_http_client};
use crate::test_support::tests::create_test_settings;

#[test]
fn sdk_url_ownership_matches_shared_corpus() {
let integration = PermutiveIntegration::new(PermutiveConfig {
enabled: true,
organization_id: "example".to_string(),
workspace_id: "workspace".to_string(),
project_id: String::new(),
api_endpoint: default_api_endpoint(),
secure_signals_endpoint: default_secure_signals_endpoint(),
cache_ttl_seconds: 3600,
rewrite_sdk: true,
});

for case in sdk_url_cases() {
let matches = case.owner.as_deref() == Some(PERMUTIVE_INTEGRATION_ID);
assert_eq!(
integration.is_permutive_sdk_url(&case.url),
matches,
"should match expected permutive ownership for {}: {}",
case.name,
case.url
);

assert_sdk_url_rewrite(integration.as_ref(), &case, "/integrations/permutive/sdk");
assert_sdk_html_attribute_rewrite(
integration.as_ref(),
&case,
"/integrations/permutive/sdk",
);
}
}

#[test]
fn test_permutive_sdk_url_generation() {
let config = PermutiveConfig {
Expand Down
36 changes: 36 additions & 0 deletions crates/trusted-server-core/src/integrations/sdk_url.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
//! Shared input policy for SDK URL ownership checks.

use std::borrow::Cow;

use url::Url;

/// Parse an HTTP(S) SDK URL with an explicit authority, or a protocol-relative URL.
///
/// Applies browser C0/space trimming, removes tab/CR/LF, and accepts backslash
/// authority separators. Literal NULs, relative paths, and base-dependent shorthand
/// stay unowned; HTML replaces NULs rather than treating them as URL whitespace.
/// Browser/Rust ownership cases live in crates/trusted-server-js/lib/test/fixtures/sdk-url-ownership.json.
pub(super) fn parse_sdk_network_url(url: &str) -> Option<Url> {
if url.contains('\0') {
Comment thread
ChristianPavilonis marked this conversation as resolved.
Resolved
return None;
}
let trimmed = url.trim_matches(|character| character <= '\u{20}');
let value = if trimmed.contains(['\t', '\n', '\r']) {
Cow::Owned(trimmed.replace(['\t', '\n', '\r'], ""))
} else {
Cow::Borrowed(trimmed)
};
let has_authority = |value: &str| matches!(value.as_bytes(), [b'/' | b'\\', b'/' | b'\\', ..]);
let parsed = if has_authority(&value) {
Url::parse(&format!("https:{value}"))
} else {
let (scheme, remainder) = value.split_once(':')?;
if !(scheme.eq_ignore_ascii_case("http") || scheme.eq_ignore_ascii_case("https"))
|| !has_authority(remainder)
{
return None;
}
Url::parse(&value)
};
parsed.ok()
}
126 changes: 126 additions & 0 deletions crates/trusted-server-core/src/integrations/sdk_url_test_support.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
//! Shared SDK URL corpus and attribute-rewrite assertions.

use lol_html::{RewriteStrSettings, element, rewrite_str};
use serde::Deserialize;

use super::{AttributeRewriteAction, IntegrationAttributeContext, IntegrationAttributeRewriter};

/// One raw URL and its expected integration owner from the shared corpus.
#[derive(Deserialize)]
#[serde(rename_all = "camelCase")]
pub(super) struct SdkUrlCase {
/// Descriptive case name for assertion failures.
pub(super) name: String,
/// Input before browser DOM URL resolution.
pub(super) url: String,
/// Expected integration ID, or none for an unowned URL.
pub(super) owner: Option<String>,
/// Owner once `lol_html` has read the attribute, when that differs from `owner`.
pub(super) server_html_owner: Option<String>,
}

/// Load the URL ownership cases used by both Rust and browser tests.
///
/// # Panics
///
/// Panics if the committed fixture is invalid.
pub(super) fn sdk_url_cases() -> Vec<SdkUrlCase> {
serde_json::from_str(include_str!(
"../../../trusted-server-js/lib/test/fixtures/sdk-url-ownership.json"
))
.expect("should parse shared SDK URL ownership corpus")
}

/// Assert SDK rewriting for both script and link attribute contexts.
///
/// # Panics
///
/// Panics if the rewrite result disagrees with the corpus owner and SDK route.
pub(super) fn assert_sdk_url_rewrite(
integration: &dyn IntegrationAttributeRewriter,
case: &SdkUrlCase,
sdk_path: &str,
) {
let expected = if case.owner.as_deref() == Some(integration.integration_id()) {
AttributeRewriteAction::Replace(sdk_path.to_string())
} else {
AttributeRewriteAction::Keep
};
for (attribute, element) in [("src", "script"), ("href", "link")] {
Comment thread
ChristianPavilonis marked this conversation as resolved.
Resolved
assert!(
integration.handles_attribute(attribute),
"should route {attribute} to the {} rewriter",
integration.integration_id()
);
let context = IntegrationAttributeContext {
attribute_name: attribute,
element_name: element,
request_host: "edge.example.com",
request_scheme: "https",
origin_host: "origin.example.com",
};
assert_eq!(
integration.rewrite(attribute, &case.url, &context),
expected,
"should apply expected {} {attribute} rewrite for {}: {}",
integration.integration_id(),
case.name,
case.url
);
}
}

/// Assert the SDK rewrite of a corpus URL after `lol_html` reads it from markup.
///
/// # Panics
///
/// Panics if HTML processing fails or the output disagrees with the server-side owner.
pub(super) fn assert_sdk_html_attribute_rewrite(
Comment thread
ChristianPavilonis marked this conversation as resolved.
Resolved
Comment thread
ChristianPavilonis marked this conversation as resolved.
Resolved
integration: &dyn IntegrationAttributeRewriter,
case: &SdkUrlCase,
sdk_path: &str,
) {
let owner = case.server_html_owner.as_ref().or(case.owner.as_ref());
let owned = owner.map(String::as_str) == Some(integration.integration_id());
for (attribute, tag) in [("src", "script"), ("href", "link")] {
let html = format!(
"<{tag} {attribute}=\"{}\"></{tag}>",
case.url.replace('"', "&quot;")
);
let context = IntegrationAttributeContext {
attribute_name: attribute,
element_name: tag,
request_host: "edge.example.com",
request_scheme: "https",
origin_host: "origin.example.com",
};
let output = rewrite_str(
&html,
RewriteStrSettings {
element_content_handlers: vec![element!(tag, |element| {
let value = element
.get_attribute(attribute)
.expect("should read the SDK URL attribute");
if let AttributeRewriteAction::Replace(next) =
integration.rewrite(attribute, &value, &context)
{
element.set_attribute(attribute, &next)?;
}
Ok(())
})],
..RewriteStrSettings::default()
},
)
.expect("should process SDK URL markup");
let expected = if owned {
format!("<{tag} {attribute}=\"{sdk_path}\"></{tag}>")
} else {
html
};
assert_eq!(
output, expected,
"should apply expected HTML {attribute} rewrite for {}",
case.name
);
}
}
31 changes: 11 additions & 20 deletions crates/trusted-server-js/lib/src/integrations/lockr/script_guard.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createScriptGuard } from '../../shared/script_guard';
import { parseSdkNetworkUrl } from '../../shared/sdk_url';

/**
* Lockr SDK Script Interception Guard
Expand All @@ -17,28 +18,18 @@ import { createScriptGuard } from '../../shared/script_guard';

/**
* Check if a URL is a Lockr SDK URL.
* Matches the logic from lockr.rs:79-86
* Browser/Rust ownership cases live in test/fixtures/sdk-url-ownership.json.
*/
function isLockrSdkUrl(url: string): boolean {
if (!url) return false;

const lower = url.toLowerCase();

// Check for aim.loc.kr domain
if (lower.includes('aim.loc.kr')) {
return true;
}

// Check for identity.loc.kr with identity-lockr and .js extension
if (
lower.includes('identity.loc.kr') &&
lower.includes('identity-lockr') &&
lower.endsWith('.js')
) {
return true;
}

return false;
const parsed = parseSdkNetworkUrl(url);
if (!parsed) return false;

const path = parsed.pathname.toLowerCase();
return (
(parsed.hostname === 'aim.loc.kr' || parsed.hostname === 'identity.loc.kr') &&
path.includes('identity-lockr') &&
path.endsWith('.js')
);
}

const guard = createScriptGuard({
Expand Down
Loading
Loading