Dump full SSAT prebid responses in ts-debug auction comment

[prk-Jr]

Summary

• The server-side auction (SSAT) stream path only emitted a summary line in the ts-debug HTML comment (ssp/mediator/winning/time), so an operator seeing winning=0 could not tell whether prebid returned nothing, errored, or bid below the floor.
• Dump the full provider_responses (and mediator_response) into the comment so the SSAT surfaces the same prebid server response detail an operator gets from the /auction endpoint — status, every bid, and metadata (which carries PBS ext.errors / ext.debug.httpcalls when prebid debug=true).
• Bid creative and provider metadata are attacker/partner-influenced, so the embedded JSON has its HTML-comment terminators neutralized to keep the dump inside the comment and out of the live DOM.

Changes

File	Change
crates/trusted-server-core/src/publisher.rs	prepend_auction_debug_comment now serializes provider_responses + mediator_response as JSON into the ts-debug comment; neutralizes --> / --!> in the payload; adds a unit test asserting status is surfaced and terminators are neutralized (exactly one closing -->).

Closes

Closes #889

Test plan

• cargo fmt --all -- --check
• cargo test-fastly (new test auction_debug_comment_dumps_provider_status_and_neutralises_terminators passes; cargo check-fastly clean)
• cargo test-axum — deferred to CI
• cargo clippy-fastly && cargo clippy-axum — deferred to CI
• JS tests / format — n/a (no JS change)
• Docs format — n/a (no docs change)

Notes

• The dump is gated behind the existing [debug].auction_html_comment flag — off by default.
• On a winning auction the dump includes Bid.creative (full ad markup) inline and can be large; intended for debugging, not steady-state production.

Checklist

• Changes follow CLAUDE.md conventions
• No unwrap() in production code — use expect("should ...")
• Uses log macros (not println!)
• New code has tests
• No secrets or credentials committed

[aram356]

Summary

The HTML-comment side of this PR is sound — I fuzzed the terminator neutralisation against a real HTML5 parser (11 escape vectors, including the <!--> / <!--!> nested-comment paths) and nothing escaped the comment. The problem is the other half: the new prebid error metadata is not confined to the debug comment. It flows into the public POST /auction response, which breaks an invariant the codebase documents explicitly.

Blocking

🔧 wrench

• Upstream PBS error body is echoed into the public /auction response (integrations/prebid.rs:2143): .with_metadata("body", …) lands in ProviderSummary.metadata (auction/types.rs:252) → ext.orchestrator.provider_details[] (auction/formats.rs:305), ungated by any debug flag. auction/orchestrator.rs:100 carries a // SECURITY: comment forbidding exactly this: "Providers MUST NOT interpolate upstream-controlled content (response bodies, parse errors, headers)… Use static text and log details server-side with log::warn! instead." Verified by serializing the resulting summary — the body snippet appears on the wire. See inline comment for the fix.
• error_type: "http_status" is mislabelled as a transport error in telemetry (integrations/prebid.rs:2141): it isn't one of the four ERROR_TYPE_* consts (auction/orchestrator.rs:95-98), so provider_status() (auction/telemetry.rs:795) hits its _ => "transport_error" fallback. Every PBS 4xx/5xx is now bucketed with genuine connection failures — which defeats this PR's own goal on the telemetry path.
• [debug].auction_html_comment doc no longer describes what the flag does (settings.rs:1904): it still promises "auction pipeline stats (SSP count, mediator status, winning bid count)", but the comment now embeds every bid's full raw SSP creative markup. Operators make the enable/disable call from that sentence. The sibling adm_in_bids flag already carries the right warning ("Never enable in production — injects raw HTML from SSPs") — please mirror it here, and update the prepend_auction_debug_comment doc at publisher.rs:858, which still says "summarising".

Non-blocking

🤔 thinking

• Unbounded dump size (publisher.rs:885): every bid's creative — including losers — pretty-printed into every page render. See inline.

♻️ refactor

• A single replace("--", "- -") would replace the two-terminator blacklist (publisher.rs:884) — equivalent, trivially auditable, and XHTML-safe. Inline comment has the fuzz results.
• Test coverage gap: --!> and the nested-comment paths are handled but unpinned (publisher.rs:2502).
• Local imports inside test fn bodies violate CLAUDE.md (publisher.rs:2465, integrations/prebid.rs:2355).

⛏ nitpick

• mediator_response=null is dumped when there is no mediator, duplicating mediator=none (publisher.rs:888).

🌱 seedling

• path_label only ever receives "stream" (publisher.rs:1265), though the doc says it differentiates stream from buffered. The buffered path produces no dump at all, so an operator debugging that path gets nothing from this feature. Pre-existing, not introduced here — worth a follow-up.

CI Status

All 19 checks pass on 08de9c4 (fmt, clippy, cargo test across fastly/axum/cloudflare/spin/parity/CLI, vitest, docs format, integration + browser tests, CodeQL).

[ChristianPavilonis]

Summary

Approved after reviewing the current head against main. The HTML-comment containment and public-response protections appear correct. I noted two follow-up findings inline: one telemetry aggregation gap and one edge-runtime resource concern.

[aram356]

Summary

Second, deeper pass across the surfaces the first review didn't reach (privacy/PII content of the dump, every egress channel, runtime/memory/caching, and the test surface). It turned up two problems that outrank everything in my previous review.

The short version: with prebid debug = true, this dump renders the visitor's identity graph — EC ID, EIDs, TC consent string, client IP, and geo lat/lon — into the publisher's page source, where any third-party tag on the page can read it. And because the dump is unbounded, it can push the page over the buffered-body cap and turn a working page into a 500.

Blocking

🔧 wrench

• The dump renders the visitor's identity graph into the page HTML (publisher.rs:874-890). AuctionResponse.metadata carries PBS's entire ext.debug subtree, copied verbatim at integrations/prebid.rs:1831-1836 when [integration.prebid].debug = true — that is resolvedrequest (the full OpenRTB request TS sent) and httpcalls (the outbound body sent to each bidder). That request is built at integrations/prebid.rs:1514-1600 and contains user.id (EC ID), user.ext.eids, the raw TC string, device.ip, and device.geo.lat/lon. Serializing metadata into a DOM comment node hands the consent-gated identity graph straight back to the client-side script environment that server-side EID resolution exists to keep it out of. Reproduction below. See inline.
• The unbounded dump can convert a working page into a 500 (publisher.rs:892). Output is written through a BoundedWriter capped at publisher.max_buffered_body_bytes (default 16 MiB, settings.rs:71-73); write() returns Err past the cap (publisher.rs:574-583) and that surfaces as TrustedServerError::Proxy → 5xx. The whole comment goes out in one end_tag.before() write. With UPSTREAM_RTB_MAX_RESPONSE_BYTES at 2 MiB per provider (integrations/mod.rs:163) and httpcalls carrying full per-bidder request/response bodies, multi-MB dumps are realistic — so the debug flag can break the page it was enabled to debug. Every other partner-influenced payload here is bounded (MAX_AUCTION_BODY_SIZE 256 KiB, MAX_CREATIVE_SIZE 1 MiB — and this PR's own 512-byte snippet). This dump is the one that isn't. See inline.
• The new prebid test asserts the leak as intended behaviour (integrations/prebid.rs:2383-2389). Fixing the /auction body exposure from my previous review means this assertion has to be deleted or inverted — flagging so it isn't mistaken for a regression. See inline.

Non-blocking

♻️ refactor

• The new error response omits message (integrations/prebid.rs:2140), the one key every other error path sets (auction/orchestrator.rs:123, 129, 141, 147). The prebid HTTP-error shape now diverges from every other provider error. See inline.
• --!> neutralisation is untested. Delete .replace("--!>", "-- !>") from publisher.rs:884 and the suite stays green — the fixture only plants --> in Bid.creative. Half the claimed protection is unguarded. Also worth planting a terminator in metadata (which the code comment at publisher.rs:879-882 explicitly calls out as attacker-influenced, but the test never exercises) and using a Some(_) mediator so the second neutralise call at publisher.rs:888-890 is covered at all.

⛏ nitpick

• The serialize-error fallback bypasses the neutraliser (publisher.rs:887, 890) — .map() applies only to the Ok branch. Unreachable in practice, but it's a latent seam in a security-relevant path. See inline.
• Non-deterministic output. AuctionResponse.metadata and winning_bids are HashMaps, so key order varies per render and two page sources of the same auction won't diff cleanly.

📌 out of scope

• auction_html_comment is undocumented. Zero hits across docs/, and it is absent from the [debug] block in trusted-server.example.toml (which lists only ja4_endpoint_enabled). There is no operator-facing warning anywhere for a flag that now publishes creatives and, with prebid debug on, user identifiers.

Reproduction — identity graph in page source

Drop into publisher.rs's test module and run cargo test -p trusted-server-core --target wasm32-wasip1 --lib -- probe_identity_graph --nocapture. It passes on this branch — every needle is present in the injected HTML:

#[test]
fn probe_identity_graph_lands_in_page_html() {
    // Shape of PBS `ext.debug` as prebid.rs:1831-1836 stores it verbatim.
    let prebid = crate::auction::types::AuctionResponse::error("prebid", 12).with_metadata(
        "debug",
        serde_json::json!({
            "resolvedrequest": {
                "user": {
                    "id": "EC-ID-abc123",
                    "consent": "CPtc-TCSTRING-xyz",
                    "ext": { "eids": [{ "source": "example.com",
                                        "uids": [{ "id": "EID-USER-999" }] }] }
                },
                "device": { "ip": "203.0.113.77",
                            "geo": { "lat": 37.7749, "lon": -122.4194 } }
            }
        }),
    );
    let result = crate::auction::orchestrator::OrchestrationResult {
        provider_responses: vec![prebid],
        mediator_response: None,
        winning_bids: std::collections::HashMap::new(),
        total_time_ms: 12,
        metadata: std::collections::HashMap::new(),
    };
    let state = Arc::new(Mutex::new(Some("<script>BIDS</script>".to_string())));
    prepend_auction_debug_comment("stream", &result, &state);
    let page = state.lock().expect("should lock").clone().expect("should have html");
    for needle in ["EC-ID-abc123", "EID-USER-999", "CPtc-TCSTRING-xyz", "203.0.113.77", "37.7749"] {
        assert!(page.contains(needle), "leaked into page HTML: {needle}");
    }
}

Checked and not a problem

Cross-user cache poisoning. publisher.rs:1671-1683 forces Cache-Control: private, max-age=0 and strips Surrogate-Control on any ad-stack HTML, which is exactly the condition under which this comment can exist — so one visitor's dump cannot be shared-cached and served to another. Flagging explicitly so nobody spends time on it. The exposure is same-page JS, not the CDN.

Also clean: GET /__ts/page-bids, window.tsjs.bids, the telemetry event rows, and the mediator request all read Bid fields rather than AuctionResponse.metadata, so none of them carry the new data.

Why CI's 19 green checks caught none of this

Both this and the /auction exposure from my previous review live in cross-module data-flow contracts with no test at any boundary. Every formats.rs fixture hardcodes metadata: HashMap::new() and BidStatus::Success (auction/formats.rs:418, 448), so no error response with metadata is ever pushed through convert_to_openrtb_response; the one exact-JSON wire test hardcodes provider_details: vec![] (openrtb.rs:212); and provider_status() has a _ => catch-all (auction/telemetry.rs:803) which by construction cannot fail on an unrecognised error_type. It's an assertion gap, not an execution gap — the tests do run.

Suggested minimal fix

Whitelist what goes into the dump instead of serializing the whole metadata map: keep error_type, status, responsetimemillis, errors, warnings, bidstatus; exclude debug outright (or scrub resolvedrequest + httpcalls[].requestbody). Then cap the serialized payload — 256 KiB matching MAX_AUCTION_BODY_SIZE, with a <truncated N bytes> marker, and truncate Bid.creative per bid. That closes the identity exposure, the 5xx risk, and the memory pressure in one change, and it is consistent with the 512-byte cap this same PR already applies at integrations/prebid.rs:2139.