Skip to content

Align DataDome tag injection with IP exclusions and add a staging bypass - #992

Merged
ChristianPavilonis merged 14 commits into
mainfrom
log/datadome
Aug 13, 2026
Merged

Align DataDome tag injection with IP exclusions and add a staging bypass#992
ChristianPavilonis merged 14 commits into
mainfrom
log/datadome

Conversation

@ChristianPavilonis

@ChristianPavilonis ChristianPavilonis commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Align the two DataDome protection layers: when Fastly's authoritative client IP matches an IP-based exclusion, Trusted Server skips the Protection API and omits only its automatically injected DataDome browser tag from processed HTML.
  • Add a temporary, secret-backed staging bypass for access-controlled browser tests. A matching fixed x-ts-datadome-bypass header skips server-side protection and suppresses the automatic tag. Whenever the enabled Fastly DataDome request filter runs, all values of that header are stripped before the request can reach DataDome or the publisher origin.
  • Add structured, privacy-safe protection decision logs and cache safeguards for tag-suppressed HTML.

The issue was that an IP allowlist skipped server-side DataDome validation but still allowed the automatically injected browser tag to run. The new flow keeps those layers consistent while providing a controlled staging-only test path without logging client IPs or exposing bypass credentials.

Staging safety

FASTLY_IS_STAGING is supplied by the Fastly runtime (1 in staging and 0 in production); it is not a repository-provisioned or build-time value. The code reads it with std::env::var at request time, so promoting the same Wasm artifact cannot carry a staging value into production. This is the environment variable equivalent of Fastly SDK 0.12.1's compute_runtime::is_staging(). PR #326 also records deployed verification that staging emits X-TS-ENV: staging and production does not.

The bypass additionally requires enabled DataDome protection, enabled bypass configuration, exactly one fixed bypass header, and a matching Secret Store credential containing at least 32 bytes. Missing, short, invalid, or duplicate credentials fail closed and proceed through normal protection.

Changes

File Change
crates/trusted-server-core/benches/html_processor_bench.rs Updates the HTML processor benchmark fixture for the suppression setting.
crates/trusted-server-core/src/config.rs Validates DataDome configuration through full integration construction during deployment validation.
crates/trusted-server-core/src/html_processor.rs Carries request-scoped suppression into HTML integration processing and supports multiple state types per integration.
crates/trusted-server-core/src/integrations/datadome.rs Adds staging bypass configuration, the private suppression marker, startup validation, and the injector guard.
crates/trusted-server-core/src/integrations/datadome/protection.rs Handles and hardens the secret-backed bypass, consumes typed IP-suppression metadata, logs complete protection outcomes, and adds regression coverage.
crates/trusted-server-core/src/integrations/datadome/protection_scope.rs Uses typed skip reasons and detects overlapping IP exclusions independently of the primary first-match diagnostic reason.
crates/trusted-server-core/src/integrations/registry.rs Allows request filters to attach private typed annotations and keys document state by integration ID and type.
crates/trusted-server-core/src/platform/test_support.rs Adds test service builders with configurable client IP and supporting stores.
crates/trusted-server-core/src/publisher.rs Propagates suppression through buffered and streaming HTML paths, preserves subresource validators/ranges, and protects synthesized document responses.
crates/trusted-server-core/src/response_privacy.rs Centralizes the synthesized-HTML private, no-store policy, validator removal, and CDN cache-header stripping.
docs/guide/integrations/datadome.md Documents bypass safety, overlap behavior, cache limitations, and diagnostic logging.
docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md Records implementation and acceptance criteria.
docs/superpowers/specs/2026-08-03-datadome-ip-excluded-client-tag-design.md Records design boundaries and privacy requirements.

Scope and compatibility

This PR remains limited to the existing DataDome request-filter, HTML-processing, publisher, and privacy paths, plus tests and operator documentation. It does not remove or alter DataDome tags already present in publisher HTML.

The bypass request filter is currently dispatched by the Fastly adapter. Axum, Cloudflare, Spin, and configurations with DataDome protection disabled do not provide the same header-stripping guarantee.

RequestFilterInput::request changes from &Request<EdgeBody> to &mut Request<EdgeBody> so filters can attach private request extensions. In-workspace implementations are updated; downstream source-based forks that construct or destructure this public input may need corresponding source changes.

Tag-suppressed processed HTML is returned as private, no-store with validators and CDN-targeted cache directives removed. This response-time policy cannot invalidate tag-bearing HTML already stored in a shared cache ahead of Trusted Server; deployments requiring guaranteed suppression must bypass or purge that cache, or avoid shared caching processed HTML ahead of Trusted Server.

Closes

Closes #994

Test plan

  • cargo fmt --all -- --check
  • cargo test-fastly
  • cargo test-axum
  • cargo test-cloudflare
  • cargo test-spin
  • ./scripts/test-cli.sh
  • cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity
  • cargo clippy-fastly
  • cargo clippy-axum
  • cargo clippy-cloudflare
  • cargo clippy-cloudflare-wasm
  • cargo clippy-spin-native
  • cargo clippy-spin-wasm
  • cd crates/trusted-server-js/lib && npx vitest run
  • TypeScript and docs Prettier checks
  • git diff --check
  • Live staging/browser verification after deployment: confirm staging emits X-TS-ENV: staging, activation logs report the bypass active, production omits the staging header, and a matching staging request skips protection and suppresses auto-injection.

Checklist

  • Changes follow CLAUDE.md conventions
  • No secrets or credentials committed
  • New behavior has regression coverage

ChristianPavilonis added a commit that referenced this pull request Aug 3, 2026
This reverts commit 5226d8a, reversing
changes made to 0dc9b19.
ChristianPavilonis added a commit that referenced this pull request Aug 3, 2026
@ChristianPavilonis ChristianPavilonis changed the title Add DataDome protection decision logs Add DataDome decision logging and IP-exclusion tag suppression Aug 3, 2026
ChristianPavilonis added a commit that referenced this pull request Aug 3, 2026
@ChristianPavilonis ChristianPavilonis changed the title Add DataDome decision logging and IP-exclusion tag suppression Align DataDome tag injection with exclusions and add a staging bypass Aug 6, 2026
@ChristianPavilonis ChristianPavilonis changed the title Align DataDome tag injection with exclusions and add a staging bypass Datadome: Add configurable bypass header and remove tag injection for bypasses Aug 6, 2026
@ChristianPavilonis
ChristianPavilonis marked this pull request as ready for review August 6, 2026 18:18

@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

Aligns the two DataDome protection layers (IP-based exclusions now skip the Protection API and omit only Trusted Server's auto-injected browser tag) and adds a secret-backed staging bypass header. The design is sound: single scope evaluation, typed request extension instead of a synthetic header, constant-time credential comparison, cache privacy on tag-suppressed HTML, and no client IPs in logs. One CI gate is failing and there are a handful of non-blocking concerns, detailed inline.

Blocking

🔧 wrench

  • format-docs CI failure: docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md fails the pinned Prettier check (stray blank lines after indented code blocks inside list items, at lines 139, 163, 223, 244, 252, 335, and 346). Fix with cd docs && npx prettier --write superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md.

Non-blocking

🤔 thinking

  • Bypass header reaches the origin when the bypass is disabled (protection.rs:176, inline)
  • Validator stripping is over-broad and misses Range parity with the ad-stack strip (publisher.rs:2908, inline)
  • Suppression marker rides a one-slot-per-integration document-state map (html_processor.rs:250, inline)

♻️ refactor

  • Duplicate cdn-cache-control entry in CDN_CACHE_HEADERS (response_privacy.rs:25, inline)
  • Missing regression test for publisher-originated tags under suppression (datadome.rs:846, inline)

🌱 seedling

  • No expiry or environment guard on the staging bypass (datadome.rs:129, inline)

CI Status

  • fmt: PASS
  • rust tests (fastly/axum/cloudflare/spin + cross-adapter parity): PASS
  • cloudflare/spin checks: PASS
  • vitest: PASS
  • format-typescript: PASS
  • format-docs: FAIL
  • integration tests (browser, Fastly EC lifecycle): pending at review time

Comment thread crates/trusted-server-core/src/response_privacy.rs Outdated
Comment thread crates/trusted-server-core/src/integrations/datadome/protection.rs Outdated
Comment thread crates/trusted-server-core/src/publisher.rs Outdated
Comment thread crates/trusted-server-core/src/html_processor.rs
Comment thread crates/trusted-server-core/src/integrations/datadome.rs
Comment thread crates/trusted-server-core/src/integrations/datadome.rs

@prk-Jr prk-Jr 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

Aligns DataDome's two protection layers: an IP-based server-side exclusion (or the new secret-backed staging bypass header) now also suppresses the auto-injected DataDome browser tag, carried request-scoped through a typed request extension into the HTML pipeline, with cache-privacy hardening on the suppressed HTML. The security-sensitive parts — constant-time credential comparison, unconditional header strip, fail-closed on empty/missing secret, no credential or client IP in logs — are done correctly and are the strongest part of this PR. One functional regression and one unanswered production-safety question block; everything else is non-blocking.

Blocking

🔧 wrench

  • Conditional/range header stripping now applies to every request from an excluded IP, not just navigations — inline comment on crates/trusted-server-core/src/publisher.rs:2903. Excluded-IP clients lose 304 revalidation and byte-range support on every subresource.

❓ question

  • How is FASTLY_IS_STAGING provisioned, and what prevents a staging-built artifact from carrying it into production? (crates/trusted-server-core/src/integrations/datadome.rs:473-481)

    fn active_protection_test_bypass(&self) -> Option<&ProtectionTestBypassConfig> {
        if std::env::var(ENV_FASTLY_IS_STAGING).as_deref() != Ok("1") {
            return None;
        }

    This env var is the entire production safety gate for a header that skips bot protection, and it is set nowhere in this repository — not in fastly.toml, .env.dev, CI, or any script. The only other reader is middleware.rs:216 (for the X-TS-ENV: staging response header), and docs/superpowers/specs/2026-03-19-edgezero-migration-design.md:341 already flags it as a Fastly-specific wart to be replaced by adapter config. Two outcomes need ruling out before merge:

    1. If the value is baked into the Wasm at build time, promoting a staging-built artifact to the production service carries FASTLY_IS_STAGING=1 with it, and the bypass is live in production for anyone holding the credential.
    2. If Fastly Compute never populates non-FASTLY_* runtime env vars, std::env::var always returns Err there, the bypass is dead code that cannot be exercised on the real staging service either — and the deferred "Live production/browser verification" item can never pass.

    Could you state how the flag is set on staging and what keeps it off production? A defence-in-depth option that holds regardless of the answer: additionally require the request host to match a configured staging host, so a mis-set env var alone is insufficient to enable the bypass.

Non-blocking

♻️ refactor

  • Three copies of the same cache-privacy policyresponse_privacy.rs:36-61 (enforce_set_cookie_cache_privacy), publisher.rs:3017-3038 (ad-stack inline block), and the new publisher.rs:1451-1481. All three do "force private unless already private/no-store → drop ETag/Last-Modified → strip CDN_CACHE_HEADERS", with the already_uncacheable lowercase-contains check duplicated verbatim between the first and third. This PR shares the CDN header list; worth going one step further and moving the whole policy into response_privacy.rs so the three cannot drift.

🌱 seedling

  • Missing test cases — the negative coverage is good (wrong credential, unconfigured, enabled = false, non-staging, and both invalid-config startup paths), but four gaps remain:
    • Duplicate x-ts-datadome-bypass headers. HeaderMap::remove removes all values and returns the first, so this correctly fails closed — but nothing pins that behaviour.
    • An empty secret present in the store (protection.rs:177-182).
    • The secret-store error path (protection.rs:183-188).
    • An end-to-end assertion that the marker set by the request filter survives into head_inserts. suppressed_publisher_request_removes_conditional_validators (publisher.rs:5388) inserts the extension by hand, so the filter → handle_publisher_request hop is only covered by the registry-level req.extensions().get::<RequestAnnotation>() assertion in registry.rs:1524.

📝 note

  • PR body claim not backed by the diff (response_privacy.rs) — the body says the file "Shares the CDN cache-header list with the new suppression privacy policy and covers cdn-cache-control". cdn-cache-control is already in CDN_CACHE_HEADERS on main (response_privacy.rs:23); the only change to that file here is a one-word comment edit at line 40. The sharing happens by publisher.rs importing the existing constant.
  • Title says "configurable bypass header" — the header name is a fixed constant, HEADER_DATADOME_TEST_BYPASS = "x-ts-datadome-bypass" (datadome.rs:93); only the credential's Secret Store location is configurable. docs/guide/integrations/datadome.md:197 gets this right ("the fixed x-ts-datadome-bypass header"); the title and commit 1ce7892b ("include configured header name when enabled") read as stale.
  • "removed before the request reaches DataDome or the publisher origin" is conditional — the strip lives in the DataDome request filter, which is registered only when enable_protection is true (datadome.rs:986-989) and dispatched only by the Fastly adapter (filter_request has exactly one non-test call site, crates/trusted-server-adapter-fastly/src/app.rs:474). With protection disabled, or on Cloudflare/Axum/Spin, a client-supplied x-ts-datadome-bypass is forwarded to the publisher origin. The docs hedge this correctly ("Whenever the enabled DataDome request filter runs"); the PR body states it unconditionally.

👍 praise

  • The credential comparison is done right (protection.rs:163-198) — the header is removed on the very first line, before any config or credential check, so it cannot leak on the disabled/inactive/invalid/absent paths; SHA-256 + subtle::ConstantTimeEq avoids both early-exit and length leaks; an empty secret and a store error both fail closed with a warn; and no log line in this PR carries the credential or a client IP. The commit sequence shows the client-IP log from 302d161e was correctly walked back in 7e6f365a.
  • IntegrationDocumentState re-keyed on (integration_id, TypeId) (registry.rs:101/141/164) — this fixes a real latent bug on main, where get_or_insert_with would overwrite another type's state stored under the same integration ID when the downcast failed. document_state_keeps_multiple_types_for_one_integration (registry.rs:1409) pins it.
  • DataDome deploy validation upgraded from "does it deserialize" to full construction (config.rs:158-160) — running try_new means invalid exclusion regexes, bad protection-API origins, and unsafe tag URLs now fail at ts config push instead of at runtime. Strictly stronger than the validate_integration::<DataDomeConfig> it replaces.
  • Suppression is wired at the single production HTML-processor construction sitecreate_html_processor has exactly one non-test caller (publisher.rs:960, via create_html_stream_processor), which both PublisherBodyProcessor::new (publisher.rs:385) and process_response_streaming (publisher.rs:463) route through, so the buffered and streaming paths are covered by construction rather than by two parallel edits. Likewise head_inserts has one emission point (datadome.rs:885), so the guard cannot be flanked.

CI Status

All 19 checks pass at 9468dc01; no local runs were needed.

  • fmt: PASS
  • clippy / check (fastly, axum, cloudflare native + wasm, spin native + wasm): PASS
  • rust tests (fastly + core via Viceroy, axum, cloudflare, spin, cross-adapter parity, ts CLI): PASS
  • js tests (vitest): PASS
  • format-typescript / format-docs: PASS
  • integration tests / browser integration tests / Fastly EC lifecycle: PASS
  • CodeQL (rust, javascript-typescript, actions): PASS

No new dependencies — sha2, subtle, and temp-env are already workspace deps (Cargo.toml/Cargo.lock untouched), all pure-Rust and wasm32-wasip1-clean. No std::net/std::thread/tokio added to core, and no Fastly APIs leaked into core.

Comment thread crates/trusted-server-core/src/publisher.rs Outdated
Comment thread crates/trusted-server-core/src/publisher.rs
Comment thread crates/trusted-server-core/src/publisher.rs Outdated
Comment thread crates/trusted-server-core/src/integrations/datadome/protection.rs
Comment thread crates/trusted-server-core/src/integrations/datadome/protection.rs
Comment thread crates/trusted-server-core/src/integrations/datadome/protection.rs Outdated
Comment thread crates/trusted-server-core/src/integrations/datadome/protection.rs Outdated
Comment thread crates/trusted-server-core/src/integrations/datadome.rs
Comment thread crates/trusted-server-core/src/integrations/datadome.rs
Comment thread crates/trusted-server-core/src/integrations/registry.rs
@ChristianPavilonis ChristianPavilonis changed the title Datadome: Add configurable bypass header and remove tag injection for bypasses Align DataDome tag injection with IP exclusions and add a staging bypass Aug 12, 2026
@ChristianPavilonis

Copy link
Copy Markdown
Collaborator Author

Addressed the latest requested changes in 73b700b and updated the PR title/body.\n\nFor the FASTLY_IS_STAGING safety question: this is a Fastly-provided runtime signal (1 in staging, 0 in production), equivalent to Fastly SDK 0.12.1 compute_runtime::is_staging(). The bypass reads it at request time with std::env::var, so it is not baked into or promoted with the Wasm artifact. PR #326 records deployed staging/production verification of the same signal through X-TS-ENV. The guide and PR body now document this boundary and the live verification checklist.\n\nAdditional coverage now includes duplicate headers, 31/32-byte credential boundaries, empty/unavailable secrets, overlapping non-IP/IP exclusions, complete fail-open result classification, and a real filter-to-publisher suppression path. All local test, clippy, parity, JS, and formatting gates listed in the PR body pass.

@prk-Jr prk-Jr 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

Aligns the client-side DataDome tag with the server-side Protection API skip for IP-based exclusions, and adds a staging-only, Secret Store-backed header bypass. I traced the DataDomeClientTagSuppressed marker end to end — request filter → RequestFilterInput (now &mut) → Fastly dispatch (execute_named:548 and dispatch_fallback:729, both of which pass the same req through to handle_publisher_request) → HtmlProcessorConfigIntegrationDocumentStatehead_inserts — and the plumbing holds on every path. Every HTML route that injects also reaches enforce_synthesized_html_cache_privacy; the PassThrough and BufferedUnmodified routes correctly skip it because nothing is injected there.

Approving. No correctness or security defect found. The findings below are one open question plus observability, performance, and test-hygiene items — none of which need to block.

Blocking

None.

Non-blocking

❓ question

  • Does the target publisher ship its own DataDome tag? Suppression omits only Trusted Server's injected tag; a publisher-authored tag is still rewritten first-party and still runs for an IP-excluded client (html_processor.rs:969). Determines whether #994 is fully closed.

🤔 thinking

  • Fail-open transport errors escape the structured decision log (protection.rs:63-70) — no decision=failed_open field on the DataDome-unreachable path.
  • evaluate no longer short-circuits (protection_scope.rs:253) — extra global-mutex + Vec<IpCidr> clone per request on the static-asset path.
  • Skip log can report a reason unrelated to the suppression (protection_scope.rs:297) — no field names the overlapping IP rule.
  • Private-cache 304 can defeat suppression for clients without Fetch Metadata (publisher.rs:1428).
  • temp_env race on FASTLY_IS_STAGING between protection.rs:881 and protection.rs:1101 — masked today by single-threaded Viceroy execution.

♻️ refactor

  • Triplicated log statement in log_protection_skip (protection.rs:519) — one log::log! replaces three arms.

⛏ nitpick

  • is_html_document_request (publisher.rs:1428) duplicates part of is_navigation_request and then delegates to it; http_util.rs is the natural home.

📝 note

  • !self.config.enable_protection is unreachable at protection.rs:57register only attaches the filter when protection is enabled (datadome.rs:995).
  • Fastly-only, confirmed by grep. registry.filter_request has exactly one production call site: crates/trusted-server-adapter-fastly/src/app.rs:474. Axum, Cloudflare, and Spin run no request filters at all, so on those adapters x-ts-datadome-bypass reaches the publisher origin unstripped and no tag suppression occurs. This matches the PR description — recording it here so the adapter asymmetry is on the review record rather than only in the PR body.

🌱 seedling

  • The "temporary" bypass has no expiry (datadome.rs:864) — consider an expires_at that fails closed.

👍 praise

  • IntegrationDocumentState re-keyed on (&'static str, TypeId) (registry.rs:101) — the correct fix, not a workaround. The previous id-only key silently collided and could overwrite state on a failed downcast; this removes a footgun for every integration.
  • The bypass fail-closed ladder is well built. The header is stripped before any configuration or credential check, duplicate values are rejected, a 32-byte minimum is enforced on the stored secret, comparison is SHA-256 + subtle::ct_eq, and the credential is never logged. Every rung has a test.
  • Scoping the validator/range strip to document contexts (publisher.rs:2901) instead of reusing the blanket should_run_ad_stack behavior — suppressed clients keep 304s and byte ranges on everything that cannot carry the injected tag.
  • The test matrix is genuinely thorough — 32-byte boundary table, non-staging inactivity, duplicate headers, bypass-beats-path-exclusion, and a subresource case asserting validators are preserved.

CI Status

All 19 checks pass on ed7e1927 (reported from GitHub, not re-run locally):

  • fmt: PASS
  • clippy / cargo check (cloudflare native + wasm32-unknown-unknown, spin native + wasm32-wasip1): PASS
  • rust tests (fastly, axum native, cloudflare, spin, ts CLI native, cross-adapter parity): PASS
  • js tests (vitest): PASS
  • format-typescript, format-docs: PASS
  • integration tests, Fastly EC lifecycle, browser integration tests: PASS
  • CodeQL (actions, javascript-typescript, rust): PASS

Comment thread crates/trusted-server-core/src/html_processor.rs
Comment thread crates/trusted-server-core/src/integrations/datadome/protection.rs
Comment thread crates/trusted-server-core/src/publisher.rs
Comment thread crates/trusted-server-core/src/integrations/datadome/protection.rs
Comment thread crates/trusted-server-core/src/integrations/datadome/protection.rs
Comment thread crates/trusted-server-core/src/publisher.rs
Comment thread crates/trusted-server-core/src/integrations/registry.rs
Comment thread crates/trusted-server-core/src/integrations/datadome.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.

Approving: all earlier blocking and non-blocking findings from previous rounds are verified as addressed at ed7e192 (unconditional bypass-header strip with fail-closed duplicates, document-context-scoped validator/range stripping, (integration ID, TypeId) document-state keying, centralized private, no-store synthesized-HTML cache policy, typed ProtectionSkipReason with overlap-aware suppression metadata, FASTLY_IS_STAGING runtime guard with a 32-byte credential minimum, and the publisher-originated tag regression test). CI is fully green including format-docs and the integration suites. The remaining inline comments are minor and do not need another round.

Comment thread crates/trusted-server-core/src/integrations/datadome/protection.rs
Comment thread crates/trusted-server-core/src/publisher.rs
@aram356 aram356 added this to the 202608 milestone Aug 13, 2026
@ChristianPavilonis
ChristianPavilonis merged commit f6a2fb8 into main Aug 13, 2026
19 checks passed
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.

DataDome: suppress client-side tag for IP-excluded requests

3 participants