[prk-Jr]

Summary

Didomi SDK requests now derive X-Geo-Country, X-Geo-Region, and CloudFront-Viewer-Country only from a normalized trusted platform country/region pair. Caller-supplied FastlyGeo-* and destination geo headers cannot select Didomi's geography; unavailable or invalid trusted geo omits the complete header group outside canonical loaders.

Canonical notice loaders retain their redirects, fail-closed geo validation, and origin caching. Other SDK GETs, including static core/UI/text files, retain normal outbound caching and all origin cache metadata. Noncanonical loaders (including root and nested /loader.js paths), SDK POSTs, and API requests bypass the outbound cache and return terminal no-store, private responses. API requests perform no geo lookup.

Closes #1151.

Review fixes

• Restore origin caching for static SDK assets, with regression coverage across geo options and trusted geo availability. Retain spoofed-header security coverage for those assets.
• Log omitted SDK geo at debug level, remove the unreachable geo match arm, and correct two stale test names.
• Leave detected Didomi disabled in audit-generated configuration and require manual review; preserve GPT/DataDome/GTM auto-enablement. A parsed-TOML regression checks the disabled state and generated note.
• Add the breaking security release note, trusted client-IP migration link, cache purge guidance, and explicit loader throttling constraints. Update the guide, design, and implementation plan together; remove inaccessible chat approval and stale worktree handoff notes.

Deployment constraints

Noncanonical loaders still contact Didomi for every request and risk automatic throttling/disablement under its reverse-proxy requirements. For production Fastly use, validate complete trusted geo coverage and enable geo_query_parameters = true on the supported notice-loader path. Cloudflare, Axum, and Spin currently have no compliant loader-caching mode through this proxy; disable the integration and restore direct Didomi SDK delivery instead. CDN-fronted deployments must configure trusted client-IP forwarding.

Purge previously cached SDK entries before rollout. Fresh browser entries require expiry or a cache-busting URL change.

Validation

• New static SDK regression fails before the cache fix and passes afterward; all 38 focused Didomi tests pass.
• All eight adapter/CLI/codegen clippy aliases, Rust formatting, native build-digest tests/lint, and WASM clock lint pass.
• Fastly (Viceroy 0.21.0), reusable-sandbox, Axum, Cloudflare, Spin, and 17 adapter parity tests pass. Host CLI tests and enabled browser fixtures also pass.
• JS build, 1,211 tests across 50 files, JS/docs/repository Markdown formatting, and documentation proposal checks pass using Node 24.12.0.
• Added-line domain lint passes; the optional repository-wide audit reports existing violations.
• Independent review of the complete PR diff found two documentation gaps, both corrected; no further confirmed runtime findings. The committed PR head merges cleanly with current main.

No live Fastly deployment or CDN smoke test was performed.

[ChristianPavilonis]

Approved. Using trusted platform geography across Didomi SDK requests and limiting caching to canonical notice loaders addresses the spoofed-header and geo-cache concerns. The compatibility and rollout changes are documented clearly.

[dhruv8sh]

Summary

Solid fix for #1151: every Didomi SDK request now derives X-Geo-Country / X-Geo-Region / CloudFront-Viewer-Country only from normalized trusted platform geo (or omits the whole group), and only canonical notice loaders keep outbound caching. The trust boundary is correct and the regression matrix is thorough. Remaining comments are non-blocking: one design question about browser-side no-store on static SDK assets, a small observability addition, and two nits.

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 change in prose because it spans several call sites or lines outside the diff and can't be auto-applied.

Non-blocking

🤔 thinking / 🏕 camp site / ⛏ nitpick

• 🤔 Browser no-store on every non-loader SDK asset goes beyond the shared-cache threat — see inline at crates/trusted-server-core/src/integrations/didomi.rs:502
• 🏕 Log when a non-canonical SDK request omits geo headers — see inline at crates/trusted-server-core/src/integrations/didomi.rs:447
• ⛏ Ok(None) arm in the canonical-loader match is unreachable — see inline at crates/trusted-server-core/src/integrations/didomi.rs:435

👍 praise

• Non-canonical SDK regression matrix — see inline at crates/trusted-server-core/src/integrations/didomi.rs:971

Cross-cutting / body-level findings

• ⛏ Two untouched test names no longer describe the behavior they cover — disabled_geo_preserves_existing_loader_behavior (didomi.rs:919) still passes because it only asserts the upstream URL, but the disabled-option loader no longer keeps its existing behavior: it now bypasses the outbound cache and is made no-store, private. sdk_responses_preserve_origin_cache_metadata (didomi.rs:1351) now only holds for canonical loaders; non-canonical SDK responses have their cache metadata stripped. Consider renaming them to disabled_geo_proxies_loader_without_geo_query and canonical_loader_responses_preserve_origin_cache_metadata. (Lines are outside the diff, so no inline suggestion.)

CI Status

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

[aram356]

Summary

The trust boundary is right: every Didomi geo header now comes from platform geo or is omitted, and caller-supplied headers never reach Didomi. The cache change goes too far. Under the default config, every SDK file bypasses the edge cache and is no-store in the browser, including static chunks Didomi marks cacheable for a year. Didomi's reverse-proxy contract says it throttles and disables proxies that do this.

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 it spans several files or lines outside the diff and can't be auto-applied.

Blocking

🔧 wrench

• Static SDK files lose all caching, which breaks Didomi's reverse-proxy contract — see inline at crates/trusted-server-core/src/integrations/didomi.rs:492
• Point front-door deployments at trusted client-IP forwarding — see inline at docs/guide/integrations/didomi.md:207
• No CHANGELOG entry for a breaking security change — see body-level findings below

❓ question

• How do geo-less loaders stay within Didomi's query-frequency rule? — see inline at docs/guide/integrations/didomi.md:219

Non-blocking

⛏ nitpick

• Session notes in committed docs — see inline at docs/superpowers/specs/2026-10-19-didomi-trusted-geo-design.md:53

Cross-cutting / body-level findings

• 🔧 No CHANGELOG entry for a breaking security change — CHANGELOG.md [Unreleased] records operator-facing compatibility changes as Breaking entries and security fixes under ### Security. This PR stops honoring FastlyGeo-* headers, requires a cache purge, and changes the Didomi SDK cache policy, but none of it reaches the release notes. Suggested ### Security entry, with the cache sentence adjusted to wherever the caching comments land (apply manually; CHANGELOG.md is outside the diff):

  - Didomi SDK requests now derive `X-Geo-Country`, `X-Geo-Region`, and `CloudFront-Viewer-Country` only from trusted platform geo (#1151). Caller-supplied `FastlyGeo-CountryCode` and `FastlyGeo-Region` headers were previously copied into them, letting a caller choose the geography Didomi used. **Breaking** for deployments that injected those headers: configure CDN-fronted client IP instead. Without a complete trusted country/region pair (always on Cloudflare, Axum, and Spin) the group is omitted. Geo-less loader requests bypass the outbound cache and return `no-store, private`; purge cached Didomi SDK entries before rollout.

CI Status

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

These checks ran before the Rust 1.98.1 upgrade (#1123) landed on main. Re-verified locally on this head merged with main at f9281917a: the merge is clean, and 36/36 Didomi tests and cargo clippy-fastly pass.

[aram356]

Summary

The round-1 findings and dhruv8sh's comments are all addressed. Static SDK GETs keep Didomi's cache policy, loaders stay bypassed with the throttling trade-off documented, and the CHANGELOG entry and front-door guidance landed. Mutation probes on the new cache gates (loader suffix, GET-only, SDK backend, privacy enforcement, canonical caching) were all killed by the tests. One gap remains outside the diff: ts audit generate still turns on Didomi with the configuration the guide now says not to run in production.

2 of the inline comments below carry a one-click GitHub suggestion. Use Commit suggestion (or Add suggestion to batch for both) to apply them as commits on the PR branch. The remaining inline comment and the body-level finding describe the fix in prose because they touch files or lines outside the diff.

Blocking

🔧 wrench

• ts audit generate writes the Didomi config this PR's guide says not to deploy — see body-level findings below

Non-blocking

⛏ nitpick

• Stray line break in the throttling paragraph — see inline at docs/guide/integrations/didomi.md:230
• Blank line turns the Security list into a loose list — see inline at CHANGELOG.md:32
• Session notes in the verification record — see inline at docs/superpowers/plans/2026-10-19-didomi-trusted-geo.md:93

Cross-cutting / body-level findings

• 🔧 ts audit generate writes the Didomi config this PR's guide says not to deploy — When the audit detects Didomi, build_draft_config_with_generator sets [integrations.didomi] enabled = true and keeps the example's geo_query_parameters = false (crates/trusted-server-cli/src/commands/audit/generate/mod.rs:466-468). It adds no manual-review note. After this PR, that draft is the geo-less loader proxy that docs/guide/integrations/didomi.md:224-230 calls "not a production caching mode", one that risks Didomi throttling and disabling the proxy. The audit can't tell which adapter the draft is for, so it can't pick the compliant setting itself.

  Route Didomi to the existing manual-review notes instead (apply manually: both files are outside the diff). I checked that this change is rustfmt-canonical and that all 306 CLI audit tests still pass, which also means no test pins this behavior today:

  -    if detected.contains("didomi") {
  -        draft = replace_key_in_section(&draft, "integrations.didomi", "enabled", "enabled = true")?;
  -    }
       if detected.contains("datadome") {
  @@
       for integration in detected {
  -        if !matches!(
  -            integration,
  -            "gpt" | "didomi" | "datadome" | "google_tag_manager"
  -        ) {
  +        if !matches!(integration, "gpt" | "datadome" | "google_tag_manager") {
               manual_review.push(integration);
           }
       }

  Then:

  • update the trusted-server.example.toml comments at lines 477-479 and 549-550, which say ts audit flips Didomi's enabled;
  • add an audit test that a detected Didomi leaves enabled = false and lists Didomi under manual review;
  • point the guide's line 51 ("disabled by default for compatibility") at the rollout caveat, since the default is now the non-compliant mode.

CI Status

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

[prk-Jr]

Addressed the audit-generation blocker in b8164abde. Detected Didomi now remains explicitly disabled and appears in the existing manual-review notes. The audit does not choose an adapter-specific geo/caching mode. Updated the example comments and guide, including the rollout link, and addressed all three inline editorial comments.

The new parsed-TOML regression fails against the original auto-enable behavior and also against the incomplete fix that disables Didomi without its manual-review note. It passes with the full change and confirms GPT/DataDome auto-enablement remains intact; existing GTM coverage is retained.

Local verification passed: Rust formatting; all eight target-matched clippy aliases; Fastly and reusable-sandbox tests using Viceroy 0.21.0; Axum, Cloudflare, Spin, parity and native build-digest tests; the full host CLI helper including Chrome fixtures; JS build, 1,211 tests across 50 files, lint and formatting; docs/repository Markdown formatting; WASM clock lint; and documentation-proposal checks. Local JS used Node 24.12.0; CI uses the repository-pinned version. Independent review of the combined PR found no further confirmed defects. No live deployment or CDN smoke test was performed.

The commit is signed, cryptographically verified locally, and GitHub reports its signature as Verified. All 23 CI checks now pass for this exact commit. All three inline conversations from this review have been replied to and resolved; the audit-generation blocker is addressed as described above. The review verdict itself remains for the reviewer to reassess.