Skip to content

Make the SSAT debug comment configurable - #943

Merged
prk-Jr merged 29 commits into
mainfrom
feat/ssat-debug-comment-config
Aug 22, 2026
Merged

Make the SSAT debug comment configurable#943
prk-Jr merged 29 commits into
mainfrom
feat/ssat-debug-comment-config

Conversation

@prk-Jr

@prk-Jr prk-Jr commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Adds [debug.auction_html_comment_options] for controlling SSAT <!-- ts-debug: ... --> auction comments: provider/mediator/bid section toggles, a fail-closed metadata subset, sensitivity modes, and compact or pretty outer JSON formatting.
  • Provides three explicit sensitivity levels: redacted (default, schema-validated server-owned diagnostics), upstream (six named provider-controlled diagnostics), and full (raw response metadata and untruncated creatives for local investigation).
  • Preserves unconditional safeguards in every mode and format: HTML-comment terminator neutralization and a 256 KiB total dump cap.

Configuration

[debug]
auction_html_comment = true

[debug.auction_html_comment_options]
include_provider_responses = true
include_mediator_response = true
include_bids = true
metadata_keys = ["error_type", "http_status", "message"]
verbosity = "redacted" # redacted | upstream | full
format = "compact"     # compact | pretty

format = "pretty" indents the outer auction dump for easier navigation. Nested JSON-looking values such as PBS requestbody and responsebody remain strings exactly as captured.

upstream and full can expose request or identity data and must not be enabled in production.

Security model

  • In redacted, metadata_keys can only narrow the fixed Rust allowlist; it cannot expose arbitrary keys such as PBS debug.
  • error_type and http_status are schema-validated, and message is generated from fixed server-owned wording rather than copied from a provider.
  • Provider-controlled errors, warnings, timings, bid statuses, and upstream messages require upstream or full.
  • Raw PBS debug.httpcalls and resolvedrequest require full.
  • Pretty formatting changes whitespace only; it does not change the dump schema or sensitivity boundary.

Main changes

File Change
crates/trusted-server-core/src/settings.rs Adds configurable section toggles, metadata subset, AuctionDebugCommentVerbosity, and AuctionDebugCommentFormat with backward-compatible defaults
crates/trusted-server-core/src/publisher.rs Applies mode-specific metadata/creative handling and compact/pretty serialization while retaining the unconditional cap and terminator neutralization
trusted-server.example.toml Documents all options, accepted modes/formats, and sensitive-mode warnings
docs/guide/auction-orchestration.md Explains how to enable, push, inspect, and safely use the SSAT HTML debug comment
docs/superpowers/specs/ Includes reviewed designs for configuration/security and output formatting
docs/superpowers/plans/ Includes reviewed test-driven implementation plans

Closes

Closes #935

Test plan

  • cargo fmt --all -- --check
  • cargo test-fastly
  • cargo test-axum
  • cargo test-cloudflare
  • cargo test-spin
  • cargo clippy-fastly
  • cargo clippy-axum
  • cargo clippy-cloudflare
  • cargo clippy-cloudflare-wasm
  • cargo clippy-spin-native
  • cargo clippy-spin-wasm
  • Focused red/green tests cover config defaults and invalid values, compact/pretty semantic equivalence, nested-string fidelity, the total cap, and comment-terminator safety
  • Manual local validation with Full output confirmed provider HTTP calls, resolved requests, bids, prices, and currency are visible according to section toggles
  • GitHub Actions (running after the latest push)

Checklist

  • Changes follow CLAUDE.md conventions
  • Compact remains the default for backward compatibility
  • Unknown verbosity and format values fail configuration loading
  • New behavior has focused regression tests
  • No secrets or credentials committed

@prk-Jr prk-Jr self-assigned this Jul 21, 2026
@aram356 aram356 added this to the 202608 milestone Aug 13, 2026
…nt-config

# Conflicts:
#	crates/trusted-server-core/src/publisher.rs
#	crates/trusted-server-core/src/settings.rs
@aram356
aram356 marked this pull request as draft August 17, 2026 15:26
@prk-Jr
prk-Jr marked this pull request as ready for review August 18, 2026 07:16

@ChristianPavilonis ChristianPavilonis left a comment

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.

Summary

🔧 Requesting changes because the new default configuration serialization breaks rollback and mixed-version deployment compatibility. One P1 inline finding is included.

Comment thread crates/trusted-server-core/src/settings.rs

@aram356 aram356 left a comment

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.

Summary

Adds [debug.auction_html_comment_options] — section toggles, a metadata_keys subset selector, a redacted/upstream/full verbosity ladder, and compact/pretty formatting — replacing the old single hardcoded allowlist. The security design is sound: the redacted-mode schema validation matches exactly what orchestrator.rs and prebid.rs actually write, all six upstream keys have real writers, the dropped "status" allowlist key has no production writer, and terminator neutralization plus the 256 KiB cap are exercised across every verbosity × format combination. One blocking config-validation gap and three smaller items, all inline.

Blocking

🔧 wrench

  • Unknown metadata_keys entries are silently dead config: validate membership in AUCTION_DEBUG_METADATA_ALLOWLIST at config load (crates/trusted-server-core/src/settings.rs:2007)

Non-blocking

♻️ refactor

  • error_type literals duplicated from orchestrator.rs: reuse the ERROR_TYPE_* constants to prevent drift (crates/trusted-server-core/src/publisher.rs:1895)

⛏ nitpick

  • metadata_keys docs omit its Upstream-mode behavior: it still gates the three base keys in upstream mode (crates/trusted-server-core/src/settings.rs:1966, docs/guide/auction-orchestration.md:863)
  • Guide recipe pairs metadata_keys with verbosity = "full", where the field has no effect (docs/guide/auction-orchestration.md:842)

CI Status

  • GitHub checks: pending at review time (fresh push)
  • fmt (local): PASS
  • targeted rust tests, new settings + publisher tests via cargo test-fastly (local): PASS (11/11)

Comment thread crates/trusted-server-core/src/settings.rs
Comment thread crates/trusted-server-core/src/publisher.rs
Comment thread crates/trusted-server-core/src/settings.rs
Comment thread docs/guide/auction-orchestration.md Outdated
Omit the default `[debug.auction_html_comment_options]` table from serialized
config blobs so an older binary's `deny_unknown_fields` `DebugConfig` still
accepts a pushed blob during a mixed-version deployment or rollback. A
non-default table still serializes.

Reject `metadata_keys` entries outside the fixed allowlist at config load
instead of silently rendering an empty metadata object, keeping the
render-time intersection as defense-in-depth.

Validate and describe redacted error classifications through the
orchestrator's `ERROR_TYPE_*` constants, with a test that fails when a new
classification ships without safe wording.

Document that `metadata_keys` also gates the three validated keys in upstream
mode and is ignored in full mode, and drop it from the full-verbosity recipe
where it has no effect.
ChristianPavilonis

This comment was marked as low quality.

@prk-Jr
prk-Jr merged commit 8eb95f8 into main Aug 22, 2026
19 checks passed
@aram356
aram356 deleted the feat/ssat-debug-comment-config branch August 22, 2026 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

For SSAT, make debug comment configurable

3 participants