Prebid external first-party bundle loading

[ChristianPavilonis]

Summary

• Hard cut Prebid to the external first-party bundle flow: no bundle_mode, no embedded/deferred tsjs-prebid loading, and external_bundle_url is required whenever Prebid is enabled.
• Always inject the same-origin /integrations/prebid/bundle.js script and keep publisher Prebid script interception/removal to prevent duplicate instances.
• Proxy the generated bundle with static-asset behavior: no EC forwarding, no copied request headers, streaming passthrough, HTTPS-only targets/redirects, and proxy.allowed_domains enforcement.
• Keep external_bundle_sha256 optional; when present it drives versioned first-party URLs, immutable cache headers, and sha256: ETags. SRI is validated if configured but is not required.
• Move Prebid out of the embedded Cargo TSJS build and document/generate publisher-specific external bundles with build-prebid-external.mjs.

Quick how-to: generate and configure a Prebid bundle

1. Generate a publisher-specific external Prebid bundle:

cd crates/js/lib
npm run build:prebid-external -- \
  --adapters=rubicon,appnexus,openx \
  --user-id-modules=sharedIdSystem,uid2IdSystem \
  --out=dist/prebid

2. Upload the generated trusted-prebid-<sha256>.js from dist/prebid/ to an HTTPS asset host that is allowed by proxy.allowed_domains.

3. Copy values from dist/prebid/manifest.json into config:

[integrations.prebid]
enabled = true
server_url = "https://prebid-server.example.com/openrtb2/auction"
external_bundle_url = "https://assets.example.com/prebid/trusted-prebid-<sha256>.js"

# Optional but recommended for versioned first-party URL + immutable cache headers.
external_bundle_sha256 = "<manifest sha256>"

# Optional browser SRI. Validated if present, but not required.
external_bundle_sri = "<manifest sri>"

# Include any client-side bidders whose adapters were included above.
client_side_bidders = ["rubicon"]

Trusted Server injects the same-origin /integrations/prebid/bundle.js route for browsers; publishers should not reference the external asset URL directly in page markup.

Testing

• cargo fmt --all -- --check
• TSJS_SKIP_BUILD=1 cargo check -p trusted-server-core
• TSJS_SKIP_BUILD=1 cargo test -p trusted-server-core prebid --lib
• TSJS_SKIP_BUILD=1 cargo test -p trusted-server-core publisher::tests --lib
• TSJS_SKIP_BUILD=1 cargo test -p trusted-server-core registry::tests --lib
• TSJS_SKIP_BUILD=1 cargo test --workspace
• TSJS_SKIP_BUILD=1 cargo clippy --workspace --all-targets --all-features -- -D warnings
• cargo check -p trusted-server-js
• cd crates/js/lib && npm run build
• cd crates/js/lib && npm run format
• cd crates/js/lib && npx vitest run
• cd crates/js/lib && npm run build:prebid-external -- --adapters=rubicon --user-id-modules=sharedIdSystem --out=/tmp/trusted-server-prebid-external-hard-cutover
• cd docs && npm ci && npm run format

[aram356]

Summary

Reworks Prebid to load an external first-party bundle proxied through the server (hard cut: external_bundle_url required when enabled, no embedded tsjs-prebid), adds the trusted-server-cli crate, the Osano consent mirror, and a hash-verified config-store payload. Overall this is high-quality, security-conscious work — the proxy path in particular is well-built. Verdict is REQUEST_CHANGES for one validation-parity bug plus a few open questions; the blocking fixes are small.

Reviewed against base feature/ts-cli-next (this is a stacked PR — the range also carries Osano #773 and tester-cookie-clear #797). 1 of the inline comments carries a one-click, scratch-verified GitHub suggestion; the rest describe fixes in prose because they span multiple files, touch new files, or are design questions.

Blocking

🔧 wrench

• ts config validate skips external-bundle validation (CLI passes, server boot-fails) — see inline at crates/trusted-server-core/src/integrations/prebid.rs:233 (one-click suggestion, verified: 130 tests green)

❓ question

• External-bundle builder mutates git-tracked generated files; restore is finally-only, not signal-safe — see inline at crates/js/lib/build-prebid-external.mjs:263
• config push --dry-run safety rests entirely on the pinned edgezero-adapter rev — see inline at crates/trusted-server-cli/src/edgezero_delegate.rs:329
• request_body_bytes dead scaffolding with a latent stream-drop (rotate-key → default-key rotation) — see inline at crates/trusted-server-core/src/request_signing/endpoints.rs:27

Non-blocking

🤔 thinking / 🌱 seedling / 📝 note / ⛏ nitpick

• Infallible Result + unused arg churn on body helpers — see inline at crates/trusted-server-core/src/publisher.rs:35
• CLI env/secret handling diverges between manifest-command and in-process paths — see inline at crates/trusted-server-cli/src/edgezero_delegate.rs:108
• Redundant/split HTTPS redirect check — see inline at crates/trusted-server-core/src/proxy.rs:1413
• Tester set/clear endpoints unauthenticated (design carry-over) — see inline at crates/trusted-server-core/src/tester_cookie.rs:78
• OsanoConfig lacks deny_unknown_fields — see inline at crates/trusted-server-core/src/integrations/osano.rs:21
• validate_enabled_integrations hardcodes the integration list — see inline at crates/trusted-server-cli/src/config_command.rs:187
• CLI crate is edition = "2021" vs documented 2024 — see inline at crates/trusted-server-cli/Cargo.toml:5

Cross-cutting / body-level findings

• 🤔 Open-proxy fallback when proxy.allowed_domains is empty — validate_external_bundle_config (prebid.rs:245) only enforces the allowlist when non-empty, and redirect_is_permitted (proxy.rs:1211) treats an empty allowlist as "permit all". With an empty list, a redirecting origin can send the bundle fetch to any HTTPS host (≤4 hops) and it's streamed as first-party JS with immutable cache; require_https is the only backstop. The host match itself is correctly dot-boundary-safe (evil-example.com ≠ *.example.com — verified). Consider requiring a non-empty allowlist whenever external_bundle_url is set.
• 🤔 Two edgezero-core revs in the dependency graph — trusted-server-cli pins @2eeccc97 (edgezero-cli/adapter) while its dep trusted-server-core pins @38198f98, so the CLI's graph compiles two full copies of edgezero-core. Verified benign for correctness today (only String/BTreeMap cross the boundary), but it's build-cost + a latent footgun the moment an edgezero_core type crosses that seam. Align to one rev — the split looks accidental.
• 🌱 Test-coverage gaps — no end-to-end test for the proxy fetch+sanitize happy path (handle_external_bundle with a mock streaming client); no mjs-level test asserting filename sha256 == hash(bytes) and SRI derivation (the load-bearing CLI↔runtime contract); buildMirrorPlan untested with USP+GPP+TCF all present at once.
• 🤔 Build-tooling robustness (benign today) — --out resolves against cwd in the Rust CLI but __dirname in the mjs (safe only because the CLI always passes an absolute path); the mjs temp file is a fixed name with emptyOutDir:false, so concurrent/overlapping runs into one dir can hash a partial file. Align the mjs base to process.cwd() and PID-suffix the temp file.
• 📝 Insecure-certificate_check warning relocated — the old startup log::warn!("INSECURE: proxy.certificate_check is disabled") in load_settings is gone; the warning still fires per-backend at connect time (adapter-fastly/src/backend.rs:220), so it's not lost but the semantics shifted from once-at-startup to per-connection. Confirm operators still see it.

👍 Praise

• Proxy path disables both EC-ID and client-header forwarding for the static-asset fetch (without_ec_id().without_forward_headers()), rebuilds responses from a strict header allowlist, strips Set-Cookie, and forces no-store on non-200 to protect the immutable cache. Cache-mode/version logic robustly resists cache poisoning (exact-hash immutable mode, multi-v rejection).
• sha256/SRI formats line up across all three layers (mjs emitter → CLI manifest validator → runtime config validator) — the invariant this whole PR rests on.
• Osano consent mirror is genuinely fail-safe (never fabricates a permissive signal on missing/malformed input) with a real mirrorGeneration race guard and test.
• write_atomic/ensure_output_dir_writable and the argv-array npm invocation (no shell) are careful, well-tested systems code; the config payload is hash-verified with no panics on operator input; real domains/secrets were removed from source control.

CI Status

• All GitHub checks: not run — the host-target CLI clippy/test workflows are added by this PR but haven't executed on this branch. Recommend pushing to trigger CI before merge, since the two-rev edgezero graph and the new CLI crate are only exercised there.

[aram356]

Summary — Approve, conditioned on green CI ✅ (pending 🔧)

Second review pass (retargeted to main; force-pushed since pass 1). The Prebid external-bundle code is merge-ready and every finding from the first review was addressed correctly. I'm approving the code — but this approval is contingent on the two required CI gates below going green; branch protection should hold the merge until they do.

Prior review fully addressed (verified)

• CLI ↔ runtime validation parity — validate_config_for_startup now calls validate_external_bundle_config (prebid.rs:234); runs at both deploy-audit and registry-build time.
• Streaming-body guard — request_body_bytes now rejects is_stream() with BadRequest.
• Open-proxy on empty allowlist — now a hard startup error requiring proxy.allowed_domains when an external bundle is configured (prebid.rs:436), so the open-mode path is unreachable for this route.
• OsanoConfig deny_unknown_fields — added, with a rejection test and deploy-validation coverage.
• mjs hash test — added and genuine (asserts filename == trusted-prebid-${sha256(bytes)}.js and the sha384 SRI against independently-computed digests).

Security posture verified sound: EC-ID + client-header forwarding disabled for the bundle fetch, https_only on initial and redirect hops, origin Set-Cookie stripped, no-store on non-200, multi-v/off-version → 404, and the one expect() on external_bundle_sha256 is guarded by schema validation. sha256/SRI consistent across mjs → manifest → CLI validator → runtime.

Blocking on CI (must be green before merge)

• 🔧 cargo fmt --check fails (required gate). Confirmed on CI and locally with the pinned rustfmt 1.95.0 — main passes locally, this branch does not (e.g. the PR-added use super::{register, OsanoConfig}; at osano.rs:53, which rustfmt wants as {OsanoConfig, register}). The reordering pattern spans nearly every changed file, indicating the branch was formatted with a newer rustfmt (1.96/nightly) than pinned 1.95.0. Fix: run cargo fmt --all under the pinned toolchain and commit.
• 🔧 prepare integration artifacts fails — dependency drift (required gate). This PR bumped the workspace Cargo.lock (aws-lc-rs → 1.17.1, time → 0.3.53, …) but left crates/trusted-server-integration-tests/Cargo.lock on the old versions, so check-integration-dependency-versions.sh fails on 5 packages. Fix (from the CI output):

cargo update --manifest-path crates/trusted-server-integration-tests/Cargo.toml -p aws-lc-rs --precise 1.17.1
cargo update --manifest-path crates/trusted-server-integration-tests/Cargo.toml -p aws-lc-sys --precise 0.42.0
cargo update --manifest-path crates/trusted-server-integration-tests/Cargo.toml -p rustls-pki-types --precise 1.15.0
cargo update --manifest-path crates/trusted-server-integration-tests/Cargo.toml -p time --precise 0.3.53
cargo update --manifest-path crates/trusted-server-integration-tests/Cargo.toml -p time-macros --precise 0.2.31

Non-blocking (optional polish — see inline)

• 🌱 Content-addressed rebuild leaves external_bundle_url stale (patch_config_metadata rewrites hash/SRI but not the URL) — see inline.
• 🌱 Error-path bundle responses drop Content-Type / set no nosniff — see inline.
• ⛏ build-all.mjs discovery is filesystem-ordered — see inline (one-click suggestion).
• 📝 Example config omits the runtime external_bundle_* keys — see inline.
• ⛏ HashSet::with_capacity(DEPLOY_VALIDATED_INTEGRATION_IDS.len()) at config.rs:136 oversizes a set that holds ≤3 auction-provider IDs — cosmetic; HashSet::new() reads clearer.
• 🌱 The bundle generator buffers all stdout/stderr (prebid_bundle.rs:70) and shows nothing until esbuild exits — consider inherited/streamed stdio.

👍 Praise

Strong, responsive iteration — all pass-1 findings fixed correctly. The IntegrationBuilder/registered_builder_ids() refactor plus the deploy_validation_covers_registered_integration_builders test is nice defensive design; the argv-array npm invocation stays injection-free; the mjs hash test is a real known-answer test.

CI Status (against main)

• cargo fmt: FAIL (required) · prepare integration artifacts: FAIL (required)
• CodeQL, cargo test, cargo test (axum native), cargo check (cloudflare), cargo check/build/test (spin), cargo test (cross-adapter parity), vitest, format-typescript, format-docs, Analyze (rust/js/actions): PASS
• integration tests, browser integration tests, integration tests (EdgeZero entry point): SKIPPED

[ChristianPavilonis]

Follow-up commit 5904598 addresses the approval-pass follow-ups:

• Ran pinned cargo fmt --all and fixed the rustfmt drift.
• Updated crates/trusted-server-integration-tests/Cargo.lock to match workspace dependency versions.
• Applied the optional Prebid polish from the inline comments: deterministic JS integration discovery, nosniff/explicit content type for external-bundle error responses, exact generated filename in the CLI next-step hint, and commented runtime bundle keys in the example config.

Local verification:

• cargo fmt --all -- --check
• ./scripts/check-integration-dependency-versions.sh
• ./scripts/test-cli.sh
• cargo test-fastly
• cargo clippy-fastly
• host CLI clippy with -D warnings
• cd crates/trusted-server-js/lib && npm run format && npx vitest run