Prevent accidental Prebid Server stored requests

[ChristianPavilonis]

Summary

• Prevent generated browser auction slots from accidentally requesting Prebid Server (PBS) stored impressions. Previously, an empty bidder map could trigger a lookup using a dynamic slot code; a missing stored impression could reject the whole provider request, including valid sibling impressions.
• Add optional trustedServer.params.storedRequest intent: false disables stored fallback, true permits it, and omission in valid envelopes preserves legacy behavior. Filter demandless PBS impressions after server-side parameter overrides without suppressing eligible Amazon Publisher Services (APS) or other providers.
• Preserve publisher intent across repeated and refresh auctions, with regression tests for the serialized browser payload and actual outbound provider request bytes.

PBS stored-request context

Stored requests are an existing optional Prebid Server feature. A caller normally opts in by referencing configuration already stored in PBS by ID. This PR does not introduce that feature or require new PBS configuration.

The defect was in Trusted Server's fallback behavior: when a slot had no usable inline bidder parameters, it could automatically use the dynamic slot code as a stored-impression ID. That accidentally opted generated slots into stored lookup. This fix makes newly generated envelopes explicitly opt out with storedRequest: false, while preserving intentional true and the legacy behavior for valid callers that omit the field. One compatibility exception is intentional: a malformed envelope disables stored fallback for the entire slot, including independent direct demand left unusable after provider-local overrides. Usable direct demand is still retained.

Changes

Changed files

File	Change
crates/trusted-server-core/src/auction/routing.rs	Validates stored intent atomically, retains legacy admission facts, and carries intent into provider-local routing.
crates/trusted-server-core/src/auction/openrtb.rs	Honors intent after bidder overrides, filters paired impressions safely, and skips empty requests before signing or transport.
crates/trusted-server-core/src/auction/openrtb/tests.rs	Covers explicit, disabled, and legacy fallback, override-populated params, and retained impression identity.
crates/trusted-server-core/src/auction/orchestrator.rs	Adds transport tests for zero, one, and two active PBS providers alongside APS, including valid bid preservation.
crates/trusted-server-core/src/auction/provider.rs	Restricts response admission to emitted impressions and marks providers skipped after demand filtering.
crates/trusted-server-core/src/auction/formats.rs	Documents the wire field and tests conversion through strict intent admission.
crates/trusted-server-core/src/creative_opportunities.rs	Updates the test helper name to describe stored-fallback permission without bidder candidates.
crates/trusted-server-js/lib/src/integrations/prebid/index.ts	Defaults newly generated envelopes to false and preserves authored intent in live ad units and immutable refresh snapshots.
crates/trusted-server-js/lib/test/integrations/prebid/index.test.ts	Tests initial, repeated, and refresh serialization, including omitted and malformed intent and container-ID lookup.
crates/trusted-server-js/lib/test/core/auction.test.ts	Confirms the shared serializer retains boolean values, omission, and invalid null for server validation.
crates/trusted-server-js/lib/test/prebid-artifact-integration.test.mjs	Verifies generated and publisher-authored intent through the built adapter.
docs/guide/api-reference.md	Defines the field, validation behavior, legacy compatibility, and request examples.
docs/guide/integrations/prebid.md	Explains refresh behavior, server-first deployment, and rollback constraints.
docs/guide/auction-orchestration.md	Describes provider routing and post-override impression filtering.
CHANGELOG.md	Records the fix and deployment requirement.
docs/superpowers/plans/2026-09-10-pbs-stored-request-intent.md	Records the approved plan, scope correction, regression evidence, and verification results.

Scope

This touches 16 files because the intent must survive browser generation, refresh reconstruction, server routing, final request construction, and response admission. Most additions are regression tests and documentation; changing only the router would leave a second stored-request fallback in the request builder.

No dependencies, adapter implementations, provider configuration, browser provider selectors, or explicit stored IDs are changed. Legacy inference and intentional stored-demand fanout remain supported. PBS all_eligible configuration remains rejected by the existing compiler, so its boundary test is retained rather than enabling a new configuration mode. Routing admits only slots with positive banner dimensions that fit in i32, and build_imp preserves one impression for every routed slot before provider-specific filtering, so paired impression and slot iteration cannot misalign.

Deployment

Deploy server support everywhere before serving the new JavaScript. The old router rejects unknown envelope fields and can drop valid inline demand when it receives storedRequest.

Rust artifacts embed the JavaScript bundles. First prepare a server-support-only build retaining old JS emission, then distribute the full build. Keep compatible server admission during rollback while browsers may retain the new JS. Until upgraded JavaScript reaches a publisher, an omitted storedRequest beside routed bidder entries that all contain empty parameter objects retains legacy slot-code fallback. Existing legacy callers and intentional stored requests can still reference missing slot-code IDs; this change does not eliminate unrelated PBS validation errors.

Closes

Closes #1086

Test plan

Meaningful failing-before assertions were captured for explicit intent admission, generated JS serialization, post-override fallback, filtered response admission, and no-usable-demand observability. They pass after the fix. Review findings are covered by focused regressions and the full validation below.

• cargo test-axum -p trusted-server-core --lib, 2834 tests passed
• cargo test-fastly && cargo test-axum
• cargo test-cloudflare && cargo test-spin
• ./scripts/test-cli.sh, including opt-in browser audit fixtures
• cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity, 14 tests passed
• cargo clippy-fastly && cargo clippy-axum
• cargo clippy-cloudflare && cargo clippy-cloudflare-wasm
• cargo clippy-spin-native && cargo clippy-spin-wasm
• cargo clippy-cli && cargo clippy-codegen
• cargo clippy --manifest-path crates/trusted-server-integration-tests/Cargo.toml --all-targets -- -D warnings
• cargo fmt --all -- --check
• JS tests: cd crates/trusted-server-js/lib && npx vitest run, 1155 tests passed
• JS lint/format/build: npm run lint, npm run format, and node build-all.mjs
• Docs lint/format/build: cd docs && npm run lint && npm run format && npm run build
• Changelog format: cd docs && npx prettier --check ../CHANGELOG.md
• git diff --check
• Standalone release WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
• Manual testing via fastly compute serve

Fastly tests ran through Viceroy; Cloudflare and Spin production WASM paths passed Clippy. The full Rust suites passed with 10 pre-existing Fastly tests/doctests ignored. All six target-matched Clippy checks passed after the review fixes. The full JavaScript suite passed 1155 tests across 47 files with no type errors, and the 13-module bundle build passed. No live PBS service was contacted.

Checklist

• Changes follow the project coding conventions
• No new unwrap() in production code; use expect("should ...")
• Existing log instrumentation conventions preserved; no println! added
• New behavior has regression tests
• No secrets or credentials committed

[prk-Jr]

Summary

Reviewed commit 1e89a976a205bde3feaa9a599f725d99454afecf.

The explicit intent contract, provider-local filtering, and browser refresh preservation are well covered. One response-admission regression remains: PBS parse state still admits impressions removed from the outgoing request, allowing an unsolicited bid for an omitted slot to win.

Blocking

• 🔧 Restrict response admission to impressions actually sent — see inline at crates/trusted-server-core/src/auction/openrtb.rs:515–516.

Validation

Local checks passed: 222 JS tests, 16 routing tests, 29 OpenRTB tests, one orchestrator transport test, and the 13-module bundle build. An additional scratch transport regression returned a bid for the omitted fictional-slot; the assertion that it cannot win failed, confirming the finding. Scratch changes were restored. No live PBS service was contacted; general adapter/lint gates rely on remote CI.

CI Status

• integration tests (Fastly EC lifecycle): PASS
• browser integration tests: PASS
• integration tests: PASS
• CodeQL: PASS
• cargo test (ts CLI, native): PASS
• Analyze (rust): PASS
• Analyze (javascript-typescript): PASS
• format-docs: PASS (required)
• Analyze (actions): PASS
• cargo test (axum native): PASS
• vitest: PASS
• CLAUDE.md symlink guard: PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• Analyze (javascript-typescript): PASS
• format-typescript: PASS (required)
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo test (cross-adapter parity): PASS
• prepare integration artifacts: PASS
• cargo test: PASS (required)
• cargo fmt: PASS (required)

[aram356]

Summary

The mechanism is correct and the tests are load-bearing rather than decorative. I verified locally against the PR head: 2675 core tests and 981 JS tests pass, cargo fmt --all -- --check clean, cargo clippy-axum clean, eslint/prettier clean. All 20 remote checks pass.

I checked the two hunks that read like regressions and both are safe. The AllEligible branch disappearing for prebid providers (routing.rs:376-382) is dead code, because plan.rs:307 rejects all_eligible + prebid-server at compile time. The zip over a filter_map-built impression list cannot misalign, because eligible_banner_slot guarantees build_imp never returns None for a routed slot. Neither is obvious from the diff, so both deserve a sentence in the PR description.

The deployment-ordering warning in the docs is accurate and load-bearing: I confirmed the base normalize_envelope rejects storedRequest as an unknown key, which drops bidderParams and zone for that slot. Old server plus new JS is a real demand-loss hazard, correctly identified.

One blocking finding: replacing a debug_assert! invariant with a routine production path that drops demand and emits no metadata, no log, and no counter.

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 remaining comments describe the fix in prose because the change spans multiple files or lines outside the diff and can't be auto-applied.

Blocking

🔧 wrench

• NoImpressions drops demand with no metadata, log, or counter — see Cross-cutting below

Non-blocking

🤔 thinking / ♻️ refactor / 📝 note / ⛏ nitpick

• The original defect is still live for un-upgraded publishers — see Cross-cutting below
• storedRequestParams({ params: snapshot }) launders a snapshot through a fake bid — see inline at crates/trusted-server-js/lib/src/integrations/prebid/index.ts:842
• undefined and null authored intent diverge silently — see inline at crates/trusted-server-js/lib/src/integrations/prebid/index.ts:707
• auction.test.ts case passes with the entire PR reverted — see inline at crates/trusted-server-js/lib/test/core/auction.test.ts:94
• allows_stored_fallback's has_candidates argument is dead in production — see inline at crates/trusted-server-core/src/auction/routing.rs:187

Cross-cutting / body-level findings

• 🔧 NoImpressions drops demand with no metadata, log, or counter — This PR deletes debug_assert!(!prebid.is_empty(), "should never route a demandless slot to prebid-server") and turns that same condition into a routine production path via the new filter in apply_prebid plus the early return at openrtb.rs:266-268.

  That path lands on an unchanged call site, crates/trusted-server-core/src/auction/provider.rs:296-301, which returns a bare AuctionResponse::no_bid(self.provider_name(), 0) with empty metadata. To an operator that is indistinguishable from PBS legitimately returning no bids. Partial drops are invisible too: if 3 of 4 impressions are filtered but 1 survives, the request goes out as Ready and nothing records that 3 were dropped.

  The reachability change is what makes this matter. On base, NoImpressions could only fire when a provider had zero eligible slots from the start, and that case is already annotated upstream by provider_skipped_response (orchestrator.rs:218-223). This PR adds a second, semantically different trigger — demand filtered out after provider-local overrides — and routes it into the same metadata-free response. Once new TSJS ships storedRequest: false on every generated envelope, a misconfigured [auction.bidders] route produces no PBS request, no metadata, no log line, and no counter.

  The repo already has both idioms to fix this, so it is a small in-pattern change rather than a redesign. Minimum fix at provider.rs:296 (apply manually — provider.rs is outside this PR's diff, so it can't be a suggestion):

  OpenRtbBuildOutcome::NoImpressions => {
      return Ok(ProviderRequestOutcome::Immediate(
          AuctionResponse::no_bid(self.provider_name(), 0).with_metadata(
              "routing",
              serde_json::json!({"skipped_no_usable_demand": true}),
          ),
      ));
  }

  Better still, count the partial drops in apply_prebid using the existing RoutingDiagnostics saturating-counter pattern (routing.rs:70-105), which would restore the signal the debug_assert used to provide. pbs_disabled_empty_candidate_does_not_become_stored_demand and the new orchestrator transport test are the natural places to pin it.

• 🤔 The original defect is still live for un-upgraded publishers — I verified this with a scratch test rather than inferring it. Routing the envelope {"bidderParams":{"exampleBidder":{}}} with no storedRequest key through build_request still produces:

  imp[0].ext.prebid = {"storedrequest":{"id":"fictional-slot"}}
  

  That is the reported defect — a stored-impression lookup keyed by the dynamic slot code — for any publisher who has not yet received the new JS.

  This is deliberate, not an oversight: base had || !slot.bidder_params().is_empty() in apply_prebid, and the PR faithfully preserves it as the has_candidates arm of StoredRequestIntent::Legacy. The pre-existing test openrtb/tests.rs:724 (pbs_empty_params_without_matching_override_fall_back_to_stored_request) pins exactly this shape, and creative_opportunities.rs:891 depends on legacy inference, so the compatibility choice is sound.

  The concern is only how it is described. The fix is effective once new JS reaches browsers, which the deployment section correctly says is gated on cached clients. "Closes #1086" reads as fully closed, and the escape hatch — any envelope with non-empty bidderParams whose entries are all empty objects — is not called out in the PR body or the api-reference table. Worth one sentence so operators know the server-side hole persists until client rollout completes.

CI Status

• cargo fmt: PASS (required)
• cargo test: PASS (required)
• format-docs: PASS (required)
• format-typescript: PASS (required)
• cargo test (axum native): PASS
• cargo test (cloudflare native + wasm32-unknown-unknown): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native): PASS
• vitest: PASS
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• browser integration tests: PASS
• prepare integration artifacts: PASS
• CodeQL: PASS
• Analyze (rust): PASS
• Analyze (javascript-typescript): PASS
• Analyze (actions): PASS
• CLAUDE.md symlink guard: PASS

[ChristianPavilonis]

Addressed the body-level review findings in 99743c09 and the PR description:

• A PBS provider whose impressions are all removed after override processing now returns routing.skipped_no_usable_demand: true. The zero, one, and two-provider transport regression pins that metadata. Per the recorded decision, partial-drop counting is deferred until its field name and consumer are defined.
• The PR description now explains the compiled rejection of PBS all_eligible, the routed-slot and build_imp pairing invariant, and the legacy empty-candidate fallback that remains until upgraded JavaScript reaches publishers.
• The API guide documents that explicit JavaScript undefined serializes as omission and receives legacy inference.

Validation passed: all four adapter test aliases, ./scripts/test-cli.sh, all six target-matched Clippy aliases, Rust formatting, 984 JavaScript tests with no type errors, JS lint/format/build, docs format/build, and git diff --check.

[prk-Jr]

Summary

Solid, well-scoped fix. The StoredRequestIntent enum preserves every pre-existing legacy admission path while adding explicit opt-in/opt-out, and the post-override impression filter closes the second stored-request fallback in the request builder that a router-only change would have left behind. No blocking findings.

What I checked rather than assumed:

• Compared the old and new admission truth tables path by path. They match on every legacy input (empty bidders map; valid envelope with missing/null/empty bidderParams; non-empty params; envelope absent with direct bidders). Only the two new explicit intents and one malformed-envelope case differ, and that difference is noted below.
• Dropping routing == AllEligible from the PBS include arm is a dead path: plan.rs:307 rejects all_eligible for the prebid-server profile at compile time.
• Server-generated opportunities are unaffected. creative_opportunities.rs:891 emits {"bidderParams":{}} with no storedRequest, which resolves to Legacy { empty_admission: true } and retains stored fallback.
• filtered_slots feeds build_bid_dimension_index, so a bid for an impression that was filtered out resolves to BidRejectionReason::UnrequestedImpression rather than being silently admitted.

All four findings are non-blocking and prose-only; none is expressed as a one-click suggestion, because each either spans multiple files, changes behaviour, or is a design question rather than a defect.

Non-blocking

🤔 thinking

• zip pairing guarded only by debug_assert, and the blast radius is now larger — see inline at crates/trusted-server-core/src/auction/openrtb.rs:491
• Shim forwards a malformed authored storedRequest, which rejects the whole envelope server-side — see inline at crates/trusted-server-js/lib/src/integrations/prebid/index.ts:704

♻️ refactor

• has_trusted_stored_request() no longer means what its name says — see inline at crates/trusted-server-core/src/auction/routing.rs:190

📝 note

• Malformed envelope now hard-disables stored fallback even with independent direct demand — see inline at crates/trusted-server-core/src/auction/routing.rs:482

Cross-cutting / body-level findings

• 👍 Test coverage is unusually strong — the matrices cover routing intent x params x direct demand, the orchestrator across zero, one, and two active PBS providers with APS participation preserved, and the fictional-slot case proves a bid for an unsent impression is rejected rather than admitted. Routing that rejection through the existing dimension index (UnrequestedImpression) instead of an ad-hoc check is the right mechanism.
• 📝 Deploy ordering is already covered — the old-server-rejects-unknown-field hazard, and the fact that it drops valid inline demand rather than merely failing to fix the stored lookup, is documented in the PR body, the CHANGELOG entry, and docs/guide/integrations/prebid.md#stored-intent-deployment. Noting only that I verified it, not raising it as a finding.
• 📝 TrustedProviderRoutes has no production caller — the new "server-owned routes do not authorize demandless PBS wire impressions" semantics, covered by pbs_filtering_keeps_slot_pairs_and_drops_demandless_trusted_routes, are exercised only by tests today. Pre-existing, not introduced here.

CI Status

• cargo fmt: PASS (required)
• cargo test: PASS (required)
• format-docs: PASS (required)
• format-typescript: PASS (required)
• cargo test (ts CLI, native): PASS
• cargo test (cross-adapter parity): PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• vitest: PASS
• CLAUDE.md symlink guard: PASS
• Analyze (actions): PASS
• Analyze (javascript-typescript): PASS
• CodeQL: SKIPPED
• cargo test (axum native): PENDING
• Analyze (rust): PENDING
• prepare integration artifacts: PENDING

No failing checks at the time of review.

[aram356]

Summary

All six findings from the previous pass are resolved, and the blocking one was fixed more thoroughly than requested. Approving.

I re-verified each fix against the code rather than from the replies:

• NoImpressions silent drop — now carries routing.skipped_no_usable_demand. I traced the chain (types.rs:331 → ProviderSummary.metadata → provider_details) to confirm an operator actually sees it in the auction response, and the key matches the existing skipped_no_eligible_slots shape.
• Response admission — f222033fd goes further than the review asked, deriving admission from the sent request.imp rather than the routed slot list. I confirmed request is not mutated between serialization at provider.rs:339 and the ID collection at provider.rs:403, so the gate is built from the same value that produced the wire bytes. I also checked the narrowing is safe for the one consumer that counts over slots(): unused_bidder_params_count early-returns 0 for PBS at openrtb.rs:895, so it never sees the shortened list.
• The regression test is a real guard. I reverted filtered_slots to input.clone() in a scratch worktree and confirmed the test fails loudly at orchestrator.rs:6031 — winning_bids.len() becomes 2 instead of 1, because the 3.0 bid for the never-sent impression wins. It proves rejection, not merely losing.
• JS snapshot read — applied as suggested. undefined coverage — added with not.toHaveProperty on parsed wire objects, which is the assertion form that can actually distinguish an absent key. Test rename and the allows_fallback comment are both in.

On the browser warning for non-boolean authored intent: the reasoning for declining it — that logging arbitrary authored values could expose publisher data to console collectors — is a stronger argument than the diagnosability concern that motivated it. Not re-raising.

Verification on this head: cargo test-axum -p trusted-server-core --lib 2675 passed, cargo test-cloudflare 22, cargo test-spin 49/37, parity 13, npx vitest run 984 passed / 45 files, cargo fmt --all -- --check and cargo clippy-axum clean. All 20 GitHub checks pass.

Two optional notes below. Neither affects behavior and neither needs to block merge — resolve them as you see fit.

Non-blocking

📝 note

• materialize_planned_response no longer matches its own doc comment — see inline at crates/trusted-server-core/src/auction/provider.rs:299
• Imp::id is Option<String>, and the invariant is now load-bearing — see inline at crates/trusted-server-core/src/auction/provider.rs:406

CI Status

• cargo fmt: PASS (required)
• cargo test: PASS (required)
• format-docs: PASS (required)
• format-typescript: PASS (required)
• cargo test (axum native): PASS
• cargo test (cloudflare native + wasm32-unknown-unknown): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native): PASS
• vitest: PASS
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• browser integration tests: PASS
• prepare integration artifacts: PASS
• CodeQL: PASS
• Analyze (rust): PASS
• Analyze (javascript-typescript): PASS
• Analyze (actions): PASS
• CLAUDE.md symlink guard: PASS