Repository navigation
Allow consented first-party relay auctions under TCF - #1498
ChristianPavilonis wants to merge 3 commits into
Conversation
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Please make softVendorExceptions be config This should on for testing TS
|
Addressed in f22a83f. Added |
|
Included the current feature delta from this PR at The baseline replay reproduced the known automatic-CMP ownership regression with the testing flag explicitly enabled: a later publisher denial plus stale CMP grant produced a stubbed identity request/cookie. Port #1545 carries the existing guarded-setter correction and normal/deferred/throwing-subscriber regressions; this extra correction is not claimed present on this source branch. The default-off/testing-only setting and Purpose2/GVL controls remain unchanged. The port's45 focused artifact/consent cases and180 Rust Prebid cases pass, as do31 broader local gates and a fresh container smoke. Hosted checks for the port/advanced RC are pending; no deployment or production privacy acceptance is claimed. |
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Summary
This round makes the relay basic-ads soft vendor exception an explicit testing opt-in (integrations.prebid.relay_soft_vendor_exception, default off), as requested last round. The config plumbing, the default, and the Prebid Purpose 2 and GVL-mapping enforcement hold up. One blocking issue remains: with the flag on and managed User IDs configured, the relay rule write breaks the shim's automatic CMP ownership handoff, and a later publisher GDPR denial can be overridden by a CMP grant.
1 of the inline comments below carries a one-click GitHub
suggestion. Use Commit suggestion to apply it as a commit on the PR branch. The other comments describe the fix in prose because the change touches multiple files or lines outside the diff and can't be auto-applied.
Blocking
🔧 wrench
- Relay rule writes go through the managed-ID ownership wrapper: see inline at
crates/trusted-server-js/lib/src/integrations/prebid/index.ts:2487 - Two CI checks failed: see Cross-cutting below
Non-blocking
🤔 thinking / ⛏ nitpick / 📝 note
- 🤔 Leave the key out of the template: see inline at
trusted-server.example.toml:533 - ⛏ Keep the measured sizes in the budget comment: see inline at
crates/trusted-server-js/lib/test/prebid-artifact-integration.test.mjs:415 - ⛏ Configuration reference and CHANGELOG: see Cross-cutting below
- 📝 Description bullet with no code change: see Cross-cutting below
Cross-cutting / body-level findings
- 🔧 Two CI checks failed:
browser integration testsandtrace runtime and browser acceptanceboth failed while installing browser packages (Temporary failure resolving 'azure.archive.ubuntu.com'), before any test ran. Neither is required by branch protection, and the cause is the runner's apt mirror, not this diff. Please re-run both once the mirror recovers. - ⛏ Configuration reference and CHANGELOG: the Prebid browser-field table in
docs/guide/configuration.md(around line 2005) is the full configuration reference but has norelay_soft_vendor_exceptionrow; only the integration guide was updated.CHANGELOG.md[Unreleased]also has noAddedentry for the key. That entry is the place to say it is testing-only, defaults to false, and is rejected by older binaries, so it must be removed before a rollback. - 📝 Description bullet with no code change: "Forward original consent signals to downstream auction endpoints" describes existing server behavior; nothing in this diff changes forwarding. The claim itself holds:
/auctionfails closed on TCF Purpose 1 for GDPR traffic and forwards the TC string to PBS.
CI Status
- Analyze (actions): PASS
- Analyze (javascript-typescript): PASS
- Analyze (python): PASS
- Analyze (rust): PASS
- browser integration tests: FAIL
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- cargo fmt: PASS (required)
- cargo test: PASS (required)
- cargo test (axum native): PASS
- cargo test (cross-adapter parity): PASS
- cargo test (ts CLI, native) (macos-latest): PASS
- cargo test (ts CLI, native) (ubuntu-latest): PASS
- CI Node pin guard: PASS
- CLAUDE.md symlink guard: PASS
- CodeQL: PASS
- docs-proposal scripts: PASS
- format-docs: PASS (required)
- format-typescript: PASS (required)
- HTML EOF runtime (Fastly): PASS
- integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- prepare integration artifacts: PASS
- release scripts: PASS
- trace runtime and browser acceptance: FAIL
- ts dev lint domains (changed lines): PASS
- vitest: PASS
- worker-deadlines (cloudflare): PASS
- worker-deadlines (spin): PASS
| const prepareRelayConsent = createRelayConsentGuard( | ||
| ADAPTER_CODE, | ||
| (key) => (pbjs as unknown as { getConfig?: PrebidGetConfig }).getConfig?.(key), | ||
| (value) => pbjs.setConfig(value as PbjsConfig) |
There was a problem hiding this comment.
🔧 wrench: Relay rule writes go through the managed-ID ownership wrapper
With managed_user_ids configured and a CMP on the page, the shim activates IAB consent itself and replaces pbjs.setConfig with a wrapper that treats any consentManagement.gdpr write as a publisher takeover. This setter resolves to that wrapper at call time, so the first non-empty auction with relay_soft_vendor_exception = true runs this sequence:
retireAutomaticTcfConsentreads the relay rules as publisher ownership. It setsacceptCmpEvents = falseand writes{ gdpr: { enabled: false } }.- The rules write turns
cmpApi: 'iab'back on. Prebid 10.26setConsentConfigcallsloadConsentData()synchronously, so Prebid immediately opens a new__tcfapiaddEventListenersubscription on the unguarded API. - A later publisher
setConfigwith a static GDPR denial finds no automatic owner left to retire. The next CMP event on the unguarded subscription overwrites the publisher's denial.
Repro on f22a83f: apply the three relay cases from the #1545 port of prebid-consent-enforcement.test.mjs unchanged (git diff <this head> <#1545 head> -- crates/trusted-server-js/lib/test/prebid-consent-enforcement.test.mjs | git apply). 4 of the 5 runs fail. After the relay auction and a static Purpose 1 / vendor 97 denial, IdentityLink still requests the LiveRamp envelope and writes idl_env. The checkpoint log shows the writes {"gdpr":{"enabled":false}} then {"gdpr":{"cmpApi":"iab","rules":[...]}}, with automatic listener 2 replaced by unguarded listener 3. Your comment on this PR notes that the correction is not on this branch; the PR description should say so too until it lands.
Scope: only relay_soft_vendor_exception = true together with managed_user_ids and a CMP. That is the testing setup this flag exists for.
Proposed fix (apply manually: it touches activateManagedUserIdTcfConsent outside this diff and adds tests in another file, so it can't be a one-click suggestion). Route internal relay writes through the automatic owner, and keep its __tcfapi guard installed while Prebid subscribes:
type ManagedTcfConsentActivation = {
acceptCmpEvents: boolean;
applyConfig: typeof pbjs.setConfig;
};
// activateManagedUserIdTcfConsent: replace the one-shot guarded setConfig.
const activation: ManagedTcfConsentActivation = {
acceptCmpEvents: true,
applyConfig: setConfig,
};
// ...
activation.applyConfig = (value) => {
const previousTcfApi = tcfWindow.__tcfapi;
tcfWindow.__tcfapi = guardedTcfApi;
try {
return setConfig(value);
} finally {
if (tcfWindow.__tcfapi === guardedTcfApi) tcfWindow.__tcfapi = previousTcfApi;
}
};
activation.applyConfig({
consentManagement: { ...effectiveConsent, gdpr: { cmpApi: 'iab' } },
} as PbjsConfig);
// installPrebidNpm, before the managed-ID block.
let applyRelayConsentConfig = (value: PbjsConfig) => pbjs.setConfig(value);
// Inside the managed-ID block, after `let automaticTcfConsentActivation`.
// Relay rule updates are internal writes, not a publisher ownership claim.
applyRelayConsentConfig = (value) => {
const activeConsent = automaticTcfConsentActivation;
return activeConsent?.acceptCmpEvents
? activeConsent.applyConfig(value)
: originalSetConfig(value);
};
// Here.
(value) => applyRelayConsentConfig(value as PbjsConfig)Then add the three #1545 relay regression tests to prebid-consent-enforcement.test.mjs. I applied this change to f22a83f in a scratch tree: all 5 relay cases pass, the full vitest run passes (81 files, 3,318 tests, no type errors), Prettier and ESLint are clean, and the shim is 53,628 characters, under the 54,000 guard.
| # Testing opt-in only. Leave false for production consent enforcement. | ||
| relay_soft_vendor_exception = false |
There was a problem hiding this comment.
🤔 thinking: Leave the key out of the template
ts config init writes this file verbatim, so every new operator config would carry relay_soft_vendor_exception = false. PrebidIntegrationConfig is deny_unknown_fields, so a binary built before this PR refuses that config on rollback, even though the line only restates the default. Commenting it out matches the optional # excluded_gam_ad_unit_path_suffixes entry below. The example block at docs/guide/integrations/prebid.md:30 and the "leave relay_soft_vendor_exception = false or omit it" sentence at :302 could steer operators toward omitting it as well.
| # Testing opt-in only. Leave false for production consent enforcement. | |
| relay_soft_vendor_exception = false | |
| # Testing-only opt-in. Omitted means false; keep it omitted in production so | |
| # older binaries can still load this config on rollback. | |
| # relay_soft_vendor_exception = true |
Scratch-verified: the native core config tests and the CLI tests that load this template pass with the suggestion applied.
| // Keep a narrow 54 KB guard including relay consent handling. | ||
| // Prebid remains a separate artifact above 200 KB, and importing | ||
| // its runtime or private rendering helpers must still fail this regression. | ||
| expect(shimCode.length).toBeLessThan(52_500); | ||
| expect(shimCode.length).toBeLessThan(54_000); |
There was a problem hiding this comment.
⛏ nitpick: Keep the measured sizes in the budget comment
The old comment recorded measured sizes so the next bump could see its headroom; this version drops them. With the same Vite options: main is 52,462 characters, f22a83f is 53,465 (+1,003), and the ownership fix from the index.ts comment brings it to 53,628. Recording the final number next to the 54,000 guard keeps that trail. I didn't post a suggestion because the number changes once that fix lands.
prk-Jr
left a comment
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
The default-off configuration plumbing and Purpose 2/vendor-mapping controls hold up. I independently reproduced the existing managed-ID consent ownership blocker at f22a83fce; no additional runtime defects found.
No one-click suggestions are included: the consent correction requires changes outside the diff and combined lifecycle tests.
Blocking
- 🔧 P1 — Relay updates can override a later publisher denial — see inline at
crates/trusted-server-js/lib/src/integrations/prebid/index.ts:2487. - 🔧 Two CI gates need successful reruns —
browser integration testsandtrace runtime and browser acceptancefailed during browser dependency installation with Ubuntu mirror DNS errors. These are infrastructure failures, but browser acceptance remains unverified. Neither check is required by branch protection; both remain applicable repository gates.
Non-blocking
- ⛏ Configuration reference and rollback guidance — document
relay_soft_vendor_exceptionin the configuration reference and explain rollback compatibility: older binaries reject even an explicitfalse. Existing configurations that omit the key remain compatible.
Verification
All seven changed files and affected consumers were reviewed. Under pinned Node 24.20.0, 45 existing focused relay/artifact/consent tests passed, with no type errors. A separate production-artifact JSDOM probe reproduced an identity request and idl_env cookie after publisher denial with the flag enabled; its flag-disabled control passed. Rust suites were assessed through hosted CI, not rerun locally. No live deployment verification was performed.
CI Status
- trace runtime and browser acceptance: FAIL
- integration tests: PASS
- browser integration tests: FAIL
- integration tests (Fastly EC lifecycle): PASS
- CodeQL: PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- CLAUDE.md symlink guard: PASS
- Analyze (actions): PASS
- worker-deadlines (spin): PASS
- ts dev lint domains (changed lines): PASS
- cargo test (ts CLI, native) (macos-latest): PASS
- cargo test (ts CLI, native) (ubuntu-latest): PASS
- docs-proposal scripts: PASS
- cargo test: PASS (required)
- cargo fmt: PASS (required)
- prepare integration artifacts: PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo test (cross-adapter parity): PASS
- Analyze (javascript-typescript): PASS
- cargo test (axum native): PASS
- format-docs: PASS (required)
- vitest: PASS
- HTML EOF runtime (Fastly): PASS
- format-typescript: PASS (required)
- release scripts: PASS
- CI Node pin guard: PASS
- worker-deadlines (cloudflare): PASS
- Analyze (rust): PASS
- Analyze (javascript-typescript): PASS
- Analyze (python): PASS
| const prepareRelayConsent = createRelayConsentGuard( | ||
| ADAPTER_CODE, | ||
| (key) => (pbjs as unknown as { getConfig?: PrebidGetConfig }).getConfig?.(key), | ||
| (value) => pbjs.setConfig(value as PbjsConfig) |
There was a problem hiding this comment.
🔧 P1 — Relay updates can override a later publisher denial
With managed User IDs, a CMP, and relay_soft_vendor_exception = true, a nonempty auction makes this internal rules update go through the publisher ownership wrapper. It retires the automatic CMP guard, then Prebid synchronously creates an unguarded CMP subscription. A later CMP grant can overwrite the publisher's static denial.
I independently reproduced this with the built Prebid artifact and shim: after a static Purpose 1 denial and a subsequent CMP grant, the page made one stubbed identity request and wrote idl_env. The same probe with the flag disabled passed. This confirms the existing unresolved finding at this revision; the new static-consent relay matrix does not exercise the combined managed-ID ownership lifecycle.
Route internal updates through the managed consent owner and guard every resulting subscription, with combined lifecycle regression tests. Simply bypassing the publisher wrapper is insufficient if the replacement subscription loses its callback guard. Apply manually: the correction touches the existing activation/ownership code outside this diff and tests in another file, so it cannot be expressed as a one-click suggestion.
There was a problem hiding this comment.
Superseded by a follow-up review with inline comments: #1498 (review)
dhruv8sh
left a comment
dhruv8sh
left a comment
There was a problem hiding this comment.
Summary
Adds an opt-in integrations.prebid.relay_soft_vendor_exception flag (default false) that lets the Prebid shim add trustedServer to the basicAds soft vendor exceptions before relay auctions. The guard is idempotent, keeps Purpose 2 and publisher GVL mappings enforced, and Prebid's per-purpose defaults keep the storage rule in place when the shim adds a basicAds rule. The code looks right; the only blocker is two browser CI jobs that failed on runner infrastructure.
1 of the inline comments below carries a one-click GitHub
suggestion— use Commit suggestion to apply it as a commit on the PR branch.
Blocking
🔧 wrench
- Two browser CI jobs failed — see cross-cutting below
Non-blocking
🤔 thinking / ⛏ nitpick
- "Closes #1497" overstates the fix — see cross-cutting below
- Keep the running size breakdown when raising the shim guard — see inline at
crates/trusted-server-js/lib/test/prebid-artifact-integration.test.mjs:412
Cross-cutting / body-level findings
- 🔧 Two browser CI jobs failed —
browser integration testsandtrace runtime and browser acceptancefailed (neither is in the required set). Both logs showTemporary failure resolving 'azure.archive.ubuntu.com'while installing Playwright browser dependencies, so this looks like a runner DNS problem, not this diff. Please re-run them. - 🤔 "Closes #1497" overstates the fix — The flag defaults to
falseand is documented as a testing-only opt-in, so a production deployment that merges this still sees relay auctions blocked under GDPR, which is what #1497 reports. Consider "Refs #1497" (and a title like "Add an opt-in relay soft vendor exception for TCF testing") so the issue stays open until there is a production consent path, such as a publisher-configured relay GVL identity.
CI Status
- cargo test: PASS (required)
- cargo fmt: PASS (required)
- format-docs: PASS (required)
- format-typescript: PASS (required)
- vitest: PASS
- Analyze (rust), Analyze (javascript-typescript), Analyze (python), Analyze (actions), CodeQL: PASS
- cargo test (axum native), cargo test (cross-adapter parity), cargo test (ts CLI, native) (ubuntu-latest / macos-latest): PASS
- cargo check (cloudflare native + wasm32-unknown-unknown), cargo check/build/test (spin native + wasm32-wasip1): PASS
- worker-deadlines (cloudflare / spin), HTML EOF runtime (Fastly): PASS
- integration tests, integration tests (Fastly EC lifecycle), prepare integration artifacts: PASS
- ts dev lint domains (changed lines), release scripts, docs-proposal scripts, CI Node pin guard, CLAUDE.md symlink guard: PASS
- browser integration tests: FAIL
- trace runtime and browser acceptance: FAIL
| // Keep a narrow 54 KB guard including relay consent handling. | ||
| // Prebid remains a separate artifact above 200 KB, and importing | ||
| // its runtime or private rendering helpers must still fail this regression. |
There was a problem hiding this comment.
⛏ nitpick — Keep the running size breakdown when raising the limit. The old comment recorded where the characters went; the new one drops it, so the next bump has no baseline. The current shim builds to 53,465 characters.
| // Keep a narrow 54 KB guard including relay consent handling. | |
| // Prebid remains a separate artifact above 200 KB, and importing | |
| // its runtime or private rendering helpers must still fail this regression. | |
| // The original shim measures 40,354 characters. Shared strict trace JSON | |
| // validation, request-ID hook association, and bounded pending lifecycle add | |
| // 11,676 characters (52,030 total); relay consent handling brings it to | |
| // 53,465. Keep a narrow 54 KB guard: Prebid remains a separate artifact above | |
| // 200 KB, and importing its runtime or private rendering helpers must still | |
| // fail this regression. |
Summary
With GDPR applicable and Purpose 2 consent granted, Prebid can block
trustedServerbefore calling/auctionbecause the first-party relay has no GVL vendor identity. This PR makes the relay's basic-ads soft vendor exception an explicit testing opt-in rather than applying it to every deployment.integrations.prebid.relay_soft_vendor_exception, defaulting tofalse, and inject it aswindow.__tsjs_prebid.relaySoftVendorException.trustedServerto thebasicAdsrule'ssoftVendorExceptionsbefore relay auctions. Keep Purpose 2 enforcement, unrelated bidders' consent checks, publisher rules, and publisher-supplied GVL identity enforcement.Deployment setting
Production deployments should leave the exception disabled:
Omitting the setting also means false. Set it to
trueonly to opt into the relay exception for testing. Without an appropriate publisher-configured relay GVL identity or exception, normal TCF vendor enforcement can still block relay auctions. This option does not set Prebid Server'stest_modeor make traffic non-billable. No production deployment configuration was changed by this PR.Changes
true.Verification
GitHub CI is still running. All listed local checks passed against commit
f22a83fce.The earlier live Chrome reproduction showed that temporarily applying the source guard restored
/auctionrequests without changing CMP consent. That was a local browser patch, not a deployment. The new configuration opt-in has not been exercised in a live production deployment.Closes #1497.