Skip to content

Extract lockr app IDs from audit page metadata #1496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ChristianPavilonis wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Extract lockr app IDs from audit page metadata #1496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0eda2fb
Extract lockr app IDs from audit page metadata
ChristianPavilonis Nov 4, 2026
cc11942
Merge main and complete lockr audit fixtures
ChristianPavilonis Nov 4, 2026
a0a8d13
Document lockr app ID extraction during audits
ChristianPavilonis Nov 4, 2026
f1b5d4a
Merge branch 'main' into feature/extract-lockr-id-during-audit
aram356 Nov 9, 2026
41dcf1f
Address lockr audit review feedback
ChristianPavilonis Nov 10, 2026
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
68 changes: 68 additions & 0 deletions crates/trusted-server-cli/src/commands/audit/generate/analyzer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use crate::commands::audit::generate::{
};
use crate::error::{CliResult, report_error};

const LOCKR_APP_ID_SELECTOR: &str = "meta[name=\"lockr-signin-app_id\"]";

static GTM_REGEX: LazyLock<Regex> =
LazyLock::new(|| Regex::new(r"\bGTM-[A-Z0-9]{4,20}\b").expect("should compile GTM regex"));
static GPT_INLINE_REGEX: LazyLock<Regex> = LazyLock::new(|| {
Expand Down Expand Up @@ -65,6 +67,26 @@ pub(crate) fn analyze_collected_page(collected: &CollectedPage) -> CliResult<Aud
));
}

let lockr_selector =
Comment thread
ChristianPavilonis marked this conversation as resolved.
Resolved
Selector::parse(LOCKR_APP_ID_SELECTOR).expect("should parse lockr app ID selector");
let mut lockr_metadata = document.select(&lockr_selector).peekable();
if lockr_metadata.peek().is_some() {
Comment thread
ChristianPavilonis marked this conversation as resolved.
Resolved
integrations.insert("lockr".to_string(), LOCKR_APP_ID_SELECTOR.to_string());
}
let mut lockr_app_ids = lockr_metadata
.filter_map(|element| element.value().attr("content"))
.map(str::trim)
.filter(|value| !value.is_empty());
let lockr_app_id = lockr_app_ids.next().map(ToOwned::to_owned);
if let Some(first) = &lockr_app_id
&& lockr_app_ids.any(|other| other != first.as_str())
{
warnings.push(
"page carries more than one distinct lockr app ID; the draft uses the first"
.to_string(),
);
Comment on lines +84 to +87

@aram356 aram356 Nov 11, 2026

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.

🔧 wrench — This warning now leaks into ts audit ad-templates generate, which writes no lockr config. It came from my round-1 suggestion: I missed that fold_collected turns every analyzer warning into a per-page note.

Repro at 41dcf1ffd: run_update_slots over a page with two distinct lockr-signin-app_id values, with desktop and mobile profiles, prints on stderr:

note: `/` on desktop: page carries more than one distinct lockr app ID; the draft uses the first
note: `/` on mobile: page carries more than one distinct lockr app ID; the draft uses the first

"the draft uses the first" is wrong for that command, which only rewrites [creative_opportunities] slots. And because the note is scoped per page and profile, a shared <head> repeats it for every crawled page (the default budget is 17 pages per profile). fold_collected already avoids that kind of repetition for CONSENT_STUB_WARNING.

Proposed fix (apply manually; it touches analyzer.rs and mod.rs): name the warning, and skip it in fold_collected the same way the consent stub is special-cased.

analyzer.rs, next to LOCKR_APP_ID_SELECTOR:

/// Recorded when page metadata carries more than one distinct lockr app ID.
pub(super) const LOCKR_APP_ID_CONFLICT_WARNING: &str =
    "page carries more than one distinct lockr app ID; the draft uses the first";

analyzer.rs, here:

        warnings.push(LOCKR_APP_ID_CONFLICT_WARNING.to_string());

mod.rs, at the top of the warning loop in fold_collected:

    for warning in &artifact.warnings {
        // Lockr app IDs only feed the `ts audit generate` draft. Slot generation
        // writes no lockr settings, so the conflict is not actionable here.
        if warning == analyzer::LOCKR_APP_ID_CONFLICT_WARNING {
            continue;
        }

Regression test for the mod.rs tests module:

    #[test]
    fn update_slots_omits_lockr_app_id_conflicts_from_notes() {
        let temp = TempDir::new().expect("should create temp dir");
        let config_path = temp.path().join("trusted-server.toml");
        fs::write(
            &config_path,
            "[creative_opportunities]\ngam_network_id = \"123456789\"\n",
        )
        .expect("should write config");
        let mut page = collected_page_with_header_slot();
        page.html = r#"<html><head><meta name="lockr-signin-app_id" content="first-app-id"><meta name="lockr-signin-app_id" content="second-app-id"></head></html>"#.to_string();
        let collector = FakeCollector::new(page);
        let mut err = Vec::new();

        run_update_slots(
            &UpdateSlotsRequest {
                url: "https://publisher.example/",
                config_path: &config_path,
                existing_creative: None,
                page_patterns: &[],
                replace: false,
                cookies: &[],
                dry_run: true,
                yes: false,
                interactive: false,
                scroll: false,
                budget: CrawlBudget::default(),
            },
            &[("desktop", &collector)],
            &mut Vec::new(),
            &mut err,
            &mut io::empty(),
        )
        .expect("should preview generated slots");

        let notes = String::from_utf8(err).expect("should write UTF-8 notes");
        assert!(
            !notes.contains("lockr app ID"),
            "should not report a generate-only lockr conflict during slot generation: {notes}"
        );
    }

Scratch-verified at 41dcf1ffd: cargo fmt --all -- --check clean, cargo clippy-cli clean, cargo test -p trusted-server-cli --lib 769 passed / 0 failed. Removing the fold_collected filter makes the new test fail. Your analyze_collected_page_warns_only_on_distinct_lockr_app_ids test passes unchanged, and it can compare against the constant if you prefer.

}

for tag in &collected.script_tags {
if let Some(src) = &tag.src {
if let Ok(asset_url) = final_url.join(src) {
Expand Down Expand Up @@ -113,6 +135,7 @@ pub(crate) fn analyze_collected_page(collected: &CollectedPage) -> CliResult<Aud
.or(derived_title);

Ok(AuditArtifact {
lockr_app_id,
audited_url: final_url.to_string(),
page_title,
js_asset_count: assets.len(),
Expand Down Expand Up @@ -315,6 +338,46 @@ mod tests {
);
}

#[test]
fn analyze_collected_page_warns_only_on_distinct_lockr_app_ids() {
for (other_id, conflicting) in [(" first-app-id ", false), ("second-app-id", true)] {
let collected = CollectedPage {
requested_url: "https://publisher.example/page".to_string(),
final_url: "https://publisher.example/page".to_string(),
page_title: None,
html: format!(
r#"<meta name="lockr-signin-app_id"><meta name="lockr-signin-app_id" content=" "><meta name="lockr-signin-app_id" content=" first-app-id "><meta name="lockr-signin-app_id" content="{other_id}"><meta name="lockr-signin-app_id" content="{other_id}">"#
),
script_tags: Vec::new(),
network_requests: Vec::new(),
gpt_slots: Vec::new(),
links: Vec::new(),
sitemap_locs: Vec::new(),
warnings: vec!["existing warning".to_string()],
};

let artifact =
analyze_collected_page(&collected).expect("should analyze collected page");

assert_eq!(
artifact.lockr_app_id.as_deref(),
Some("first-app-id"),
"should retain the first nonempty normalized ID"
);
let mut expected_warnings = vec!["existing warning".to_string()];
if conflicting {
expected_warnings.push(
"page carries more than one distinct lockr app ID; the draft uses the first"
.to_string(),
);
}
assert_eq!(
artifact.warnings, expected_warnings,
"should preserve existing warnings and warn once only for distinct IDs without echoing them"
);
}
}

#[test]
fn analyze_collected_page_uses_html_title_when_browser_title_absent() {
let collected = CollectedPage {
Expand Down Expand Up @@ -559,6 +622,7 @@ mod tests {
"https://tags.example.com/gtm.js?id=GTM-ABC123&l=dataLayer",
] {
let artifact = AuditArtifact {
lockr_app_id: None,
audited_url: "https://example.com".to_string(),
page_title: None,
js_asset_count: 0,
Expand Down Expand Up @@ -600,6 +664,7 @@ mod tests {

let evidence = format!("https://tags.example.com/gtm.js?id={container_id}&l=dataLayer");
let mut artifact = AuditArtifact {
lockr_app_id: None,
audited_url: "https://publisher.example.com".to_string(),
page_title: None,
js_asset_count: 0,
Expand Down Expand Up @@ -648,6 +713,7 @@ mod tests {
("https://tags.example.com/gtm.js?id=GTM-ABC", "GTM-BBB222"),
] {
let artifact = AuditArtifact {
lockr_app_id: None,
audited_url: "https://publisher.example.com".to_string(),
page_title: None,
js_asset_count: 1,
Expand Down Expand Up @@ -677,6 +743,7 @@ mod tests {
#[test]
fn extract_gtm_container_id_reads_query_parameter_urls() {
let artifact = AuditArtifact {
lockr_app_id: None,
audited_url: "https://publisher.example".to_string(),
page_title: None,
js_asset_count: 1,
Expand All @@ -703,6 +770,7 @@ mod tests {
#[test]
fn artifact_serialization_uses_expected_shape() {
let artifact = AuditArtifact {
lockr_app_id: None,
audited_url: "https://publisher.example".to_string(),
page_title: None,
js_asset_count: 1,
Expand Down
Loading
Loading