Skip to content

Extract the protected CloudFront Geo baseline - #1532

Open
ChristianPavilonis wants to merge 20 commits into
baseline/native-aws-20261110from
baseline/protected-geo-20261110
Open

ChristianPavilonis wants to merge 20 commits into
baseline/native-aws-20261110from
baseline/protected-geo-20261110

Conversation

@ChristianPavilonis

@ChristianPavilonis ChristianPavilonis commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Default-off CloudFront boot profile, one immutable request outcome and absent optional detail.
  • Reorganize the existing working demo using EdgeZero83d7a629, not current-main forwarding compatibility.

Draft baseline review unit. Not a current-main merge candidate, deployment approval or completed current-head CI claim.

Baseline and sequence

Head: affb0d0a4c0a1f3b9ac00d11c12de1f45080a8ce. Base: baseline/native-aws-20261110.

Historical main anchor: 2dd968aeebeb2cc0d5110115f64e0ad58e2e1d13. Original demo: TS2423a2ed plus separately preserved dirty inputs. Matching EdgeZero: draft431, exact revision 83d7a6296d285e135c4ea70021b19e6c8dca7e9f.

The user selected compatible-baseline cleanup. Current-main integration with EdgeZero462 and the authenticated-forwarder incompatibility are deferred. Preserve current-main PR1525/1526 ready and1527/1528 draft, with their existing bases. No excluded1499 or independent1506 implementation, relay/privacy experiment, new transport fallback, group/codec/phase design or operational deployment is included.

Changes

Area Change
This unit Default-off CloudFront boot profile, one immutable request outcome and absent optional detail.
Unchanged work Original checkouts, dirty Prebid inputs, recovery references and existing PRs remain preserved.

Closes

Refs #1530. This unit completes one part of the baseline extraction and does not close the overall tracker on its own.

Spec

Not a new feature. This is extraction of existing demo behavior and bounded build/caller corrections. Original native and EC storage design/review records remain in the baseline where applicable; they are not presented as approval of later combined source. No new forwarding, consent, timer or provider policy is designed here.

Test plan

Core3002 and native all-feature suites pass, including10 real startup cases. No production KV Geo, local Geo database, new consent authority or cloud acceptance is added.

  • Required current-head hosted checks complete before ready-state transition.
  • Final baseline combined-check results recorded with exact input revisions and limitations.
  • Focused failures and corrections retained rather than assertions relaxed.

The b67ffc6 baseline checkpoint has passing documented local gates: Fastly/default/reuse/native/Cloudflare/Spin suites, all adapter strict lint lanes, 17 cross-adapter parity cases, 1252 JS cases, CLI suites, formatting, clock/digest/proposal checks and changed-lines domain lint. Passing checks at121ebcc36 are reused only for unchanged relevant inputs; the final follow-up covers the CLI test-import and documentation-only corrections. These results are not a substitute for this PR's current-head hosted checks.

Checklist

  • Existing behavior extracted into a scoped review unit.
  • Public examples use fictional inputs and no licensed Geo data.
  • No deployment, main merge, old-RC closure or recovery deletion.
  • No new worker launched or paused experiment resumed.
  • Review readiness confirmed independently from later units.

Operational Terraform remains in trusted-server-deployer. Local checks and prototype tests do not qualify cloud access, fleet durability or a live publisher.

Hosted docs-build correction

The first hosted format-docs job caught an additional site-build gate that the original local formatter checkpoint had not run: a relative link from the native EC guide to a Terraform example outside the site. Native EC correction8b11dbad3 replaces it with the plain repository path; no dead-link check is disabled. That docs-only correction is carried through the existing upper PR identities. Native EC and final integration8985b111 both pass docs ESLint and VitePress build; the final docs formatter also passes. Runtime, dependency, image-build-context and test inputs are unchanged from b67ffc6, so their prior exact-input proofs are reused. Hosted current-head checks still control readiness.

Startup negative-probe correction

A hosted native EC case failed the no-listener probe while its child logged the expected startup validation failure. The old log has no socket endpoints, so the failure's exact subtype is not claimed confirmed. A separate controlled Linux reproduction demonstrates a successful TCP self-connection with no listening server. Native correction8490c691d distinguishes equal local/peer endpoints from a real listener; actual socket controls prove the self-connection is excluded and a real listener is still detected. Nonzero exit, expected diagnostic, secret-redaction and the original startup deadline remain required. This changes only the acceptance fixture, not production admission, transport or trust.

The earliest native unit and final9356c557 default/all-feature acceptance checks and strict lint pass, including both controls and the original failure case. Ignores remain explicit; earlier separately executed celld proof is not relabeled as a new ignored-case run. Correction is merged upward through existing PR identities without force-pushing or retargeting. Current-head hosted checks must pass; the old failure is retained rather than rerun-only waived. Runtime/API/dependency inputs remain unchanged. A fresh exact9356 producer build and real runtime smoke now pass: image sha256:65a735d38e9dbc0573d0f6068a8b261bd202c43aea05085c2b2d6bccc91ebdbd, linux/amd64 UID/GID10001, including startup rejection and in-flight SIGTERM drain. Prior b67 image receipts remain separate; no image was pushed. All local parent runners are complete.

Browser CI host compatibility

Preserved older browser failures show Playwright1.58.2 rejecting ubuntu-latest's new Ubuntu26.04 image before browser installation; later missing-executable failures are consequences, not passing browser tests. Nativeb1216763a pins only the browser job to ubuntu-24.04, leaving dependency versions, tests and assertions unchanged. Direct Actionlint1.7.12 passes at the owning native and final integration points; the first shim-only attempt could not run because no global version was configured, and is not counted as lint proof. No global tooling was changed. The only9356 ->ee7 delta is this workflow host/comment; runtime, Cargo, docs, tests and Docker-context inputs are unchanged. Actual browser execution on the supported hosted runner remains pending and must pass before readiness. No waiver or browser skip.

@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.

Reviewed affb0d0a4c0a1f3b9ac00d11c12de1f45080a8ce against baseline b1216763a3c2a38df1a2909285330e818823423a. No actionable findings.

Reviewed native Geo admission and startup, request isolation, response finalization, optional-field serialization, consent handling, and affected auction consumers.

Local verification: cargo test --locked -p trusted-server-adapter-axum --all-features --test startup passed all 10 tests, including CloudFront trust rejection and error-response finalization, using Rust 1.98.1 and Node 24.12.0. Head and base were rechecked and remain unchanged.

This approval covers the historical baseline unit. Current-main compatibility and live AWS qualification remain outside scope. Main-only CodeQL did not run.

CI status: all 17 reported checks PASS at the reviewed head:

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

@ChristianPavilonis
ChristianPavilonis marked this pull request as ready for review November 11, 2026 13:47
Retain the existing Geo feature head while projecting private managed outcomes into partial request-local geography. Preserve current-main trace privacy, direct outbound APIs, startup router ownership and ingress diagnostics. Do not restore backend or HTTP compatibility bridges.

@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

Reviewed 0ae96ef5f against baseline/native-aws-20261110 (merge base 65ba4f3df). The CloudFront path itself is well bounded: EdgeZero handles admission, credential stripping and assertion parsing; TS reads only the immutable RequestContext::geo(); and populated fields keep their flat wire names. Changes are needed because:

  • The jurisdiction rule change reaches every adapter, so Cloudflare US traffic loses server-side auctions and EC creation.
  • The CloudFront tests never run in CI, and no test proves geography reaches consent.
  • Native startup lost its environment diagnostic.
  • At this head, the integration, Fastly EC lifecycle and browser jobs were skipped.

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. The other comments describe the fix in prose because it touches several files or lines outside the diff.

Blocking

🔧 wrench

  • US without a region now fails closed on every adapter (Cloudflare US auctions and EC blocked): see inline at crates/trusted-server-core/src/consent/jurisdiction.rs:71-74
  • Geography-to-consent wiring is untested (mutation survives): see inline at crates/trusted-server-adapter-axum/src/platform.rs:166
  • Native startup drops EdgeZero's environment diagnostic (suggestion): see inline at crates/trusted-server-adapter-axum/src/main.rs:60
  • Use the full TRUSTED_SERVER__ env prefix, one name everywhere: see inline at crates/trusted-server-adapter-axum/src/native_geo.rs:14
  • CloudFront tests never run in hosted CI: see Cross-cutting below
  • Failed CI check skipped the integration and browser jobs at this head: see Cross-cutting below

Non-blocking

  • ♻️ One core helper for geo response headers (Axum duplicates the clearing, Fastly's None branch skips it): see inline at crates/trusted-server-adapter-axum/src/middleware.rs:183-193
  • 🤔 Credential-absence assertions check the response, where the request header can't appear: see inline at crates/trusted-server-adapter-axum/tests/startup.rs:350-351
  • ⛏ derive_more::Display and error-stack for the new errors: see inline at crates/trusted-server-adapter-axum/src/native_geo.rs:27
  • ⛏ Imports inside test function bodies: see inline at crates/trusted-server-core/src/consent/jurisdiction.rs:236
  • ⛏ set_response_headers doc overclaims for Fastly (suggestion): see inline at crates/trusted-server-core/src/geo.rs:24
  • ⛏ Assertions without messages: see Cross-cutting below

Cross-cutting / body-level findings

  • 🔧 CloudFront tests never run in hosted CI. cargo test-axum (.github/workflows/test.yml, test-axum job) runs with default features. From the new tests it runs only compiled_out_geo_selection_fails_before_store_preparation_and_listen and invalid_geo_profile_fails_before_store_preparation_and_listen. Three tests, and the geo-cloudfront branch of selected_implementation_is_explicitly_validated, are only compiled by clippy-axum --all-features, never run:

    • managed_geo_is_partial_immutable_per_request_and_finalized_on_auth_errors
    • admitted_body_error_keeps_managed_geo_without_losing_framework_status
    • nonmatching_real_socket_peer_cannot_supply_geo

    They pass locally with cargo test -p trusted-server-adapter-axum --features geo-cloudfront. Please add that as a step in the test-axum job.

  • 🔧 prepare integration artifacts failed, so the downstream jobs never ran at this head. The failure is in actions/upload-artifact: Failed to CreateArtifact ... Unexpected token '<', "<!DOCTYPE "... is not valid JSON after 5 attempts. The artifacts built fine, so this is GitHub-side. As a result, integration tests, integration tests (Fastly EC lifecycle) and browser integration tests were skipped. The previous approval covered affb0d0a4, before the two EdgeZero tag merges. Please re-run the workflow so those jobs execute at the current head. No required checks are configured on this base branch, so this is a gate AGENTS.md treats as required rather than one branch protection enforces.

  • ⛏ Assertions without messages. AGENTS.md asks for a descriptive message on every assertion. Examples among the new tests:

    • crates/trusted-server-adapter-axum/src/platform.rs:241-245
    • crates/trusted-server-core/src/platform/mod.rs:305, :311-312
    • crates/trusted-server-core/src/consent/jurisdiction.rs:221-227, :247-248
    • crates/trusted-server-core/src/geo.rs:243-244, :251
    • crates/trusted-server-core/src/integrations/prebid.rs:6667
    • crates/trusted-server-core/src/integrations/aps.rs:2437
    • crates/trusted-server-adapter-cloudflare/src/platform.rs:893-894, :934
    • crates/trusted-server-adapter-axum/tests/startup.rs:342-344

CI Status

  • browser integration tests: SKIPPED (upstream artifact job failed)
  • integration tests (Fastly EC lifecycle): SKIPPED (upstream artifact job failed)
  • integration tests: SKIPPED (upstream artifact job failed)
  • prepare integration artifacts: FAIL
  • cargo check (cloudflare native + wasm32-unknown-unknown): PASS
  • cargo test (cross-adapter parity): PASS
  • cargo test (axum native): PASS
  • vitest: PASS
  • cargo test: PASS
  • cargo test (ts CLI, native) (ubuntu-latest): PASS
  • cargo test (ts CLI, native) (macos-latest): PASS
  • cargo fmt: PASS
  • format-typescript: PASS
  • docs-proposal scripts: PASS
  • format-docs: PASS
  • CLAUDE.md symlink guard: PASS
  • cargo check/build/test (spin native + wasm32-wasip2): PASS

No checks are marked required on baseline/protected-geo-20261110.

Comment on lines +71 to +74
if country.as_str() == "US" {
let Some(region) = geo.region.as_deref() else {
return Jurisdiction::Unknown;
};

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.

🔧 wrench: US without a region now fails closed on every adapter, not just the new CloudFront profile.

detect_jurisdiction is shared. CloudflareGeo::lookup always returns region: None (crates/trusted-server-adapter-cloudflare/src/platform.rs:681), so every Cloudflare US request moves from NonRegulated to Unknown. Unknown requires TCF Purpose 1 for the server-side auction (consent/mod.rs:298-308) and blocks EC creation (consent/mod.rs:572). The CountryCode::new gate at line 54 does the same for provider codes outside EdgeZero's 249-entry table.

Probe: the real Cloudflare build_geo + detect_jurisdiction + both gates, default ConsentConfig, no consent signals:

cf-ipcountry base 65ba4f3df head 0ae96ef5f
US NonRegulated, auction allowed, EC allowed Unknown, auction denied, EC denied
XK (Kosovo) NonRegulated, allowed, allowed Unknown, denied, denied
T1 (Cloudflare Tor) NonRegulated, allowed, allowed Unknown, denied, denied
JP NonRegulated, allowed, allowed unchanged

Fastly US IPs with no region() change the same way.

Failing closed for an unknown US state is defensible: at the base, a California visitor on Cloudflare was treated as non-regulated. But the PR describes itself as default-off with no new consent policy, the module comment at lines 76-77 justifies a global rule with CloudFront behavior, and docs/guide/native-rc-demo.md:121-122 says US without a subdivision "remains" Unknown.

Please choose one explicitly:

  1. Keep the rule as a deliberate cross-adapter change. Map Cloudflare's cf-region-code into region in this PR. Add a Cloudflare test pinning US to Unknown and US + CA to UsState("CA"). Update native-rc-demo.md:44-46 and :121-122, docs/guide/edge-cookies.md (which defines Unknown as "no geo data") and the PR description to say that US without a region is now Unknown on every adapter. cf-region-code comes from Cloudflare's visitor-location headers, so document that Cloudflare US traffic stays Unknown without them.
  2. Keep this PR default-off. Drop the None => Unknown branch and the CountryCode gate here, and land them as their own reviewed consent change together with the Cloudflare mapping.
// Option 1, in build_geo (crates/trusted-server-adapter-cloudflare/src/platform.rs),
// plus a `region: Option<String>` field on CloudflareGeo returned from lookup().
let region = headers
    .get("cf-region-code")
    .and_then(|v| v.to_str().ok())
    .filter(|s| !s.is_empty())
    .map(str::to_ascii_uppercase);

Apply manually: this can't be a one-click suggestion because it spans core, the Cloudflare adapter and the docs.

context.monotonic_clock(),
)))
.geo(Arc::clone(GEO.get_or_init(|| Arc::new(AxumPlatformGeo))))
.geo(Arc::new(AxumPlatformGeo::from_context(context)))

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.

🔧 wrench: No test covers this line, and it is the line that makes CloudFront geography reach consent and auctions.

The response geo headers come from FinalizeResponseMiddleware's own geo_from_outcome(ctx.geo()), and the unit tests construct AxumPlatformGeo directly. Mutation: replacing this line with .geo(Arc::new(AxumPlatformGeo::default())) passes the whole --features geo-cloudfront suite (lib 24, startup 10, routes 28, native_http 15, native_acceptance 7). Only the compiler notices, with an unused-from_context warning.

A unit test can't inject the outcome, because EdgeZero only sets RequestContext::geo() during managed ingress. So this needs a startup-binary test in tests/startup.rs:

  1. Use an envelope that enables an auction provider pointing at a local stub listener. The current ENVELOPE has no provider, so /auction returns at the "auction is disabled" branch before the consent gate.
  2. From the admitted peer, send /auction with CloudFront-Viewer-Country: US and CloudFront-Viewer-Country-Region: CA. Assert that the stub receives a bid request whose device.geo is {"country": "US", "region": "CA", ...}.
  3. Send the same request without geo headers. Assert that the stub receives nothing and the child log contains server-side auction consent gate denied.

Apply manually: this can't be a one-click suggestion because it needs a new envelope fixture and a stub listener.

fn native_options() -> Result<AxumRunOptions, String> {
let selection = load_selection().map_err(|error| error.to_string())?;
let options = AxumRunOptions::from_env()
.map_err(|_| "native hosting environment is invalid".to_owned())?

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.

🔧 wrench: This drops EdgeZero's hosting-environment diagnostic for every native deployment, with Geo Off included.

EdgeZero's from_env errors are already redacted by design. run_options.rs failure() documents that the subject is a fixed setting name, never contents. Before this PR, main printed them. Real binary with EDGEZERO__ADAPTER__PORT=notaport:

  • base 65ba4f3df: trusted-server-adapter-axum failed: startup settings: PORT: invalid nonzero port
  • head 0ae96ef5f: Trusted Server native hosting configuration failed: native hosting environment is invalid
Suggested change
.map_err(|_| "native hosting environment is invalid".to_owned())?
.map_err(|error| error.to_string())?

Verified alone at 0ae96ef5f: fmt, all six clippy lanes, check aliases, test-fastly, test-axum, test-cloudflare, test-spin and parity pass. The macOS test-axum run skipped managed_ingress_ignores_direct_spoof_and_isolates_two_gateway_clients, which binds 127.0.0.2 and only runs on Linux. The real binary prints startup settings: PORT: invalid nonzero port again.

use serde::Deserialize;

/// Path to the separately delivered TS native Geo boot profile.
pub const PROFILE_FILE_ENV: &str = "TS_NATIVE_GEO_PROFILE_FILE";

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.

🔧 wrench: Use the project's full TRUSTED_SERVER__ environment prefix instead of a TS_ shorthand, and keep one name everywhere it appears.

The full prefix is safe here:

  • Native startup never reads TRUSTED_SERVER__*. Nothing in this adapter or EdgeZero's Axum crate does.
  • The ts CLI's app-config overlay only overrides keys that already exist in trusted-server.toml, so it ignores this variable.

Rename all three places together:

  • crates/trusted-server-adapter-axum/src/native_geo.rs:14:
    pub const PROFILE_FILE_ENV: &str = "TRUSTED_SERVER__NATIVE_GEO_PROFILE_FILE";
  • crates/trusted-server-adapter-axum/tests/startup.rs:93, so the test follows the constant:
    command.env(trusted_server_adapter_axum::native_geo::PROFILE_FILE_ENV, path);
  • docs/guide/native-rc-demo.md:90.

Apply manually: the three files must change together, so a one-click suggestion on this line alone would break the startup test.

InvalidPolicy,
}

impl fmt::Display for NativeGeoProfileError {

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.

⛏ nitpick: AGENTS.md asks for derive_more::Display on error enums instead of a hand-written Display impl, and for error-stack instead of String errors. main.rs:57's native_options() returns Result<AxumRunOptions, String>. derive_more is already a workspace dependency; this crate needs derive_more = { workspace = true }.

#[derive(Debug, Clone, Copy, PartialEq, Eq, derive_more::Display)]
pub enum NativeGeoProfileError {
    /// A selected profile file could not be read.
    #[display("native Geo profile could not be read")]
    Unavailable,
    /// The file is not the supported typed profile.
    #[display("native Geo profile is invalid")]
    InvalidProfile,
    /// `EdgeZero` rejected the protected source or credential removal policy.
    #[display("native Geo protected source policy is invalid")]
    InvalidPolicy,
}

Apply manually: this can't be a one-click suggestion because it also touches Cargo.toml.


#[test]
fn partial_geo_never_synthesizes_tcf_or_auction_permission() {
use crate::consent::{ConsentContext, consent_allows_server_side_auction};

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.

⛏ nitpick: AGENTS.md doesn't allow imports inside function bodies, test code included. Move this to the tests module's use list. The same applies to crates/trusted-server-adapter-axum/src/platform.rs:204-206 (in located) and :259 (in unavailable_outcomes_and_forged_extensions_cannot_supply_geography).

Comment on lines +350 to +351
"x-origin-test-removal",
"fixture-origin-credential",

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.

🤔 thinking: These two entries, and the matching checks at lines 414-417, assert the credential is absent from the response. A request header can never appear there, so they can't fail.

The risk the removal policy addresses is forwarding. The publisher path clones the full request head (crates/trusted-server-core/src/publisher.rs:3094), so if EdgeZero's strip_credential ever regressed, the credential would go to the publisher origin. A stronger version points publisher.origin_url at a local echo listener and asserts the forwarded request has no x-origin-test-removal header.

Comment on lines +183 to +193
for name in [
HEADER_X_GEO_CITY,
HEADER_X_GEO_COUNTRY,
HEADER_X_GEO_CONTINENT,
HEADER_X_GEO_COORDINATES,
HEADER_X_GEO_METRO_CODE,
HEADER_X_GEO_REGION,
HEADER_X_GEO_INFO_AVAILABLE,
HeaderValue::from_static("false"),
);
] {
response.headers_mut().remove(name);
}

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.

♻️ refactor: Geo header clearing now lives in two places with different coverage:

  • When geo is Some, this loop clears six headers and GeoInfo::set_response_headers clears the same six again.
  • Fastly's apply_finalize_headers (crates/trusted-server-adapter-fastly/src/middleware.rs:211-218) never clears them in its None branch, so an origin-supplied x-geo-country survives next to x-geo-info-available: false on Fastly but not on Axum.

One core helper used by both adapters would remove the duplication and the divergence:

// crates/trusted-server-core/src/geo.rs
pub fn apply_geo_response_headers(geo: Option<&GeoInfo>, response: &mut Response<EdgeBody>) {
    clear_geo_response_headers(response.headers_mut());
    match geo {
        Some(geo) => geo.insert_response_headers(response),
        None => {
            response.headers_mut().insert(
                HEADER_X_GEO_INFO_AVAILABLE,
                HeaderValue::from_static("false"),
            );
        }
    }
}

Apply manually: this can't be a one-click suggestion because it spans core and both adapters.

/// `x-geo-metro-code`, `x-geo-region` (when available), and
/// `x-geo-info-available: true` to the given response.
/// Adds available geographic fields and `x-geo-info-available: true`.
/// Missing fields never produce empty headers or default coordinates.

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.

⛏ nitpick: This claim depends on the adapter. geo_from_fastly wraps every Fastly value in Some, including city() (a plain &str) and the always-present coordinates, so on Fastly an empty city or zero coordinates are still emitted. The guarantee is really about None fields:

Suggested change
/// Missing fields never produce empty headers or default coordinates.
/// Fields set to `None` produce no header. Supplied values are emitted
/// as given, including any empty city or zero coordinates an adapter supplies.

Verified alone at 0ae96ef5f: fmt, all six clippy lanes, check aliases, test-fastly, test-axum, test-cloudflare, test-spin and parity pass. The macOS test-axum run skipped the Linux-only 127.0.0.2 loopback test.

@dhruv8sh dhruv8sh 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.

Superseded by a follow-up review with inline comments: #1532 (review)

@dhruv8sh dhruv8sh 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

Makes GeoInfo detail optional (city, continent, coordinates, metro), adds the opt-in native CloudFront Geo boot profile for Axum, and tightens jurisdiction detection. The partial-geo plumbing into OpenRTB/APS/Prebid and the response headers looks right and is well tested. The jurisdiction change, though, reaches every adapter, not just CloudFront.

1 of the inline comments below carries a one-click GitHub suggestion. Use Commit suggestion to apply it. The other comments describe the fix in prose.

Blocking

🔧 wrench

  • US geography without a region now resolves to Unknown on every adapter — see inline at crates/trusted-server-core/src/consent/jurisdiction.rs:72-74

Non-blocking

🤔 thinking

  • CountryCode::new now gates every adapter — see inline at crates/trusted-server-core/src/consent/jurisdiction.rs:54-56

⛏ nitpick

  • New assertions lack "should ..." messages — see inline at crates/trusted-server-core/src/consent/jurisdiction.rs:221-230

Cross-cutting / body-level findings

  • 🌱 Stale x-geo-* headers when geo is absent on Fastly — Axum's apply_finalize_headers (trusted-server-adapter-axum/src/middleware.rs:146-155) now strips every upstream x-geo-* header before writing, and GeoInfo::set_response_headers clears them when geo is present. Fastly's apply_finalize_headers (trusted-server-adapter-fastly/src/middleware.rs:201-208) only sets x-geo-info-available: false when geo is None, so origin-supplied x-geo-city etc. pass through next to false. This was already the case before this PR, but the PR now handles the two adapters differently. A follow-up could share the clearing step.
  • ♻️ native_geo errors bypass error-stack — load_selection / parse_selection (trusted-server-adapter-axum/src/native_geo.rs) return a bare Result<_, NativeGeoProfileError>. AGENTS.md asks for Report<E>. The bounded, redacted categories are a good design and can stay; wrap them in Report with change_context at the call site so the error stays consistent with the rest of the crate.

CI Status

All 25 reported checks PASS: worker-deadlines (spin), worker-deadlines (cloudflare), vitest, ts dev lint domains (changed lines), trace runtime and browser acceptance, release scripts, prepare integration artifacts, integration tests (Fastly EC lifecycle), integration tests, HTML EOF runtime (Fastly), format-typescript, format-docs, Fastly response diff script, docs-proposal scripts, CLAUDE.md symlink guard, CI Node pin guard, cargo test (ts CLI, native) (ubuntu-latest), cargo test (ts CLI, native) (macos-latest), cargo test (cross-adapter parity), cargo test (axum native), cargo test, cargo fmt, cargo check (cloudflare native + wasm32-unknown-unknown), cargo check/build/test (spin native + wasm32-wasip1), browser integration tests. Branch protection reports no required checks for this base branch.

Comment on lines +72 to +74
let Some(region) = geo.region.as_deref() else {
return Jurisdiction::Unknown;
};

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.

🔧 wrench — US geography without a region now resolves to Unknown on every adapter, not just the CloudFront path.

Before this PR a US request with region: None fell through to NonRegulated. It now returns Unknown, which blocks EC creation (consent/mod.rs:572) and requires TCF Purpose 1 for server-side auctions (consent/mod.rs:299-303). The Cloudflare adapter always builds GeoInfo with region: None (trusted-server-adapter-cloudflare/src/platform.rs:302), so every US request on Cloudflare loses EC and server-side auctions unless it carries a TCF string. Fastly requests whose lookup has no region are affected the same way.

The PR is scoped as a default-off CloudFront boot profile, but this rule is in shared core. Either keep the previous NonRegulated fallback for a missing region and apply the strict subdivision rule only where the native CloudFront provider supplies the assertion, or call out the cross-adapter policy change explicitly (with Cloudflare region support or a documented decision) so it is not a side effect of this unit.

Comment on lines +54 to +56
let Ok(country) = CountryCode::new(&geo.country) else {
return Jurisdiction::Unknown;
};

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.

🤔 thinking — CountryCode::new now gates every adapter.

Country values that CountryCode::new rejects (for example an empty string or a provider placeholder value) previously produced NonRegulated for non-GDPR input and now produce Unknown, so they fail closed for EC and auctions. That is defensible, but like the region change it alters Fastly/Cloudflare/Spin behaviour in a unit described as CloudFront-only. Worth confirming that is intended and noting it in the PR description.

Comment on lines +221 to +230
assert_eq!(
detect_jurisdiction(Some(&geo), &config),
Jurisdiction::Unknown
);
}
for country in ["", "XX", "ZZ", "UK", "USA"] {
assert_eq!(
detect_jurisdiction(Some(&make_geo(country, None)), &config),
Jurisdiction::Unknown
);

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.

⛏ nitpick — The new loop assertions have no message, so a failure does not say which input broke. AGENTS.md asks for descriptive "should ..." assertion messages. The same applies to the assertions in partial_geo_never_synthesizes_tcf_or_auction_permission (lines 247-248) and the new tests in geo.rs.

Suggested change
assert_eq!(
detect_jurisdiction(Some(&geo), &config),
Jurisdiction::Unknown
);
}
for country in ["", "XX", "ZZ", "UK", "USA"] {
assert_eq!(
detect_jurisdiction(Some(&make_geo(country, None)), &config),
Jurisdiction::Unknown
);
assert_eq!(
detect_jurisdiction(Some(&geo), &config),
Jurisdiction::Unknown,
"should keep unrecognized US region {region:?} unknown"
);
}
for country in ["", "XX", "ZZ", "UK", "USA"] {
assert_eq!(
detect_jurisdiction(Some(&make_geo(country, None)), &config),
Jurisdiction::Unknown,
"should keep unrecognized country {country:?} unknown"
);

aram356 added a commit that referenced this pull request Nov 12, 2026


Per the user, every feature is built once and works on every adapter in
platform order, with platform gaps sent to EdgeZero (DEC-16, SH-5). T21
moves the identity routes, EC lifecycle, providers and write-back into one
router in core that each adapter mounts; A2, A10, T8, T9, T17 and T18
shrink to turning it on per adapter and passing the conformance suite, and
T7, T14 and A11 fold into A2 and A10. EZI-2a adds EdgeZero's local store
for Axum development and EZI-4 binds Axum's geo provider from the manifest.

Restack the Axum RC units on #1506: #1531 carries A1 in place of #1528,
#1533 and #1534 close as copies of #1525 and #1526, and the image and
Terraform units sit directly on #1532 so A6 and A7 stop waiting on A2.
Record #1566, refresh the provider stack's fixes at their current heads,
and flag #1535 as unplanned.
aram356 added a commit that referenced this pull request Nov 12, 2026
Per the user, the RC line is forward-ported to main (DEC-6). Rechecking the
proposed stack against DEC-3 and DEC-16: #1536 and #1537 are mostly
Axum-only stores, store selection and wiring that EZI-2b, T6 and T21
replace, so A2 becomes a new pull request that takes their tests, and both
close with #1535 once their code moves. The image and Terraform units
restack on #1543, which publishes the image and shares .dockerignore with
#1539. #1531 goes on #1506 for A1 and #1532 on #1531 for #1529.
@aram356 aram356 mentioned this pull request Nov 13, 2026
7 tasks

This branch has not been deployed

No deployments
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.

4 participants