Remove fastly dependency from trusted-server-core (PR 15) by prk-Jr · Pull Request #635 · IABTechLab/trusted-server

prk-Jr · 2026-04-14T15:12:02Z

Summary

Removes all fastly crate imports from trusted-server-core, making it fully platform-agnostic — no Fastly SDK types leak into the shared library
Introduces a sync ConsentKvOps trait in core so the consent pipeline stays synchronous while abstracting away the Fastly KV implementation; FastlyConsentKvStore in the adapter implements the trait
Relocates Fastly-specific code (compat, backend, geo_from_fastly, config/secret stores) to trusted-server-adapter-fastly where it belongs

Changes

File	Change
`crates/trusted-server-core/src/compat.rs`	Deleted — conversion fns moved to adapter
`crates/trusted-server-core/src/storage/`	Deleted — `FastlyConfigStore` / `FastlySecretStore` removed (callers already on platform traits)
`crates/trusted-server-core/src/geo.rs`	Removed `geo_from_fastly`; moved to adapter `platform.rs`
`crates/trusted-server-core/src/consent/kv.rs`	Replaced `fastly::kv_store::KVStore` with new sync `ConsentKvOps` trait; fingerprint stored in body `fp` field instead of Fastly metadata
`crates/trusted-server-core/src/consent/mod.rs`	Threads `kv_ops: Option<&dyn ConsentKvOps>` through `ConsentPipelineInput`
`crates/trusted-server-core/src/integrations/mod.rs`	Added platform-abstracted `ensure_integration_backend_with_timeout` and `predict_backend_name_for_url` helpers
`crates/trusted-server-core/src/integrations/{aps,adserver_mock,prebid}.rs`	Migrated from direct `BackendConfig` to platform helpers
`crates/trusted-server-core/src/publisher.rs`	Accepts `kv_ops: Option<&dyn ConsentKvOps>` instead of touching Fastly KV directly
`crates/trusted-server-core/Cargo.toml`	Removed `fastly` dep; moved `tokio` to `dev-dependencies` (test-only)
`crates/trusted-server-adapter-fastly/src/compat.rs`	New file — inlined compat conversion fns from deleted core module
`crates/trusted-server-adapter-fastly/src/backend.rs`	Relocated from core; removed dead `backend_name_for_url`
`crates/trusted-server-adapter-fastly/src/platform.rs`	Added `geo_from_fastly`; added `FastlyConsentKvStore` implementing `ConsentKvOps`
`crates/trusted-server-adapter-fastly/src/app.rs`	Wires `FastlyConsentKvStore` into `ConsentPipelineInput`
`crates/trusted-server-adapter-fastly/src/main.rs`	Wires `FastlyConsentKvStore` for legacy path; removes `use trusted_server_core::compat`
`crates/trusted-server-adapter-fastly/Cargo.toml`	Added `url` dep (needed by relocated `backend.rs`)

Closes

Closes #496

Test plan

cargo test --workspace
cargo clippy --workspace --all-targets --all-features -- -D warnings
cargo fmt --all -- --check
JS tests: cd crates/js/lib && npx vitest run
JS format: cd crates/js/lib && npm run format
Docs format: cd docs && npm run format
WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
Manual testing via fastly compute serve

Checklist

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

BackendConfig uses fastly::backend::Backend directly, making it incompatible with the platform-portability goal. This commit: - Copies backend.rs verbatim into trusted-server-adapter-fastly, updating the one crate-internal import path - Adds url dependency to the adapter Cargo.toml - Rewires platform.rs and management_api.rs to use crate::backend - Removes pub mod backend from trusted-server-core/lib.rs - Migrates aps.rs, adserver_mock.rs, and prebid.rs off the deleted BackendConfig, using context.services.backend().ensure() for registration and a new predict_backend_name_for_url free function in integrations/mod.rs for stateless name prediction cargo check --workspace passes with zero errors.

All callers were migrated to predict_backend_name_for_url in core's integrations module. Remove the now-unused method and update the parse_origin doc comment that referenced it.

Remove crates/trusted-server-core/src/storage/ entirely (config_store.rs, secret_store.rs, mod.rs). All call sites migrated to PlatformConfigStore / PlatformSecretStore traits. Also drop the now-broken intra-doc links in trusted-server-adapter-fastly/src/platform.rs that referenced the deleted types.

Replace direct fastly::kv_store::KVStore usage in consent/kv.rs with a platform-neutral ConsentKvOps trait. The trait has four sync methods (load_entry, save_entry_with_ttl, fingerprint_unchanged, delete_entry) keeping the consent pipeline synchronous. - consent/kv.rs: add ConsentKvOps trait; replace load_consent_from_kv / save_consent_to_kv / delete_consent_from_kv with load_consent / save_consent (trait-based); remove open_store / fingerprint_unchanged private fns; drop ConsentKvMetadata / metadata_from_context (metadata API was Fastly-specific; fingerprint now lives in the body fp field); add StubKvOps + integration tests - consent/mod.rs: add kv_ops field to ConsentPipelineInput; update try_kv_fallback and try_kv_write to consume kv_ops instead of store_name; update all struct literal sites - publisher.rs: add kv_ops: Option<&dyn ConsentKvOps> parameter to handle_publisher_request; wire it into ConsentPipelineInput and use it for the SSC-revocation delete - adapter/platform.rs: add FastlyConsentKvStore wrapping fastly::kv_store::KVStore implementing ConsentKvOps via the sync API - adapter/app.rs + main.rs: open FastlyConsentKvStore from settings.consent.consent_store and pass as kv_ops to handle_publisher_request

Remove `fingerprint_unchanged` from `ConsentKvOps` trait so the common case (consent unchanged) costs one KV read instead of two. `save_consent` now loads the entry once and compares the fingerprint inline. Extract `open_consent_kv` helper in `app.rs` to deduplicate the two identical KV-open blocks. Replace bare `unwrap()` with descriptive `expect` messages in consent KV tests. Run `cargo fmt --all` to fix all whitespace issues.

Resolved five conflicts: - Deleted compat.rs (PR15 goal: remove Fastly from core) - integrations/mod.rs: kept both PR15's ensure_integration_backend_with_timeout / predict_backend_name_for_url and PR14's INTEGRATION_MAX_BODY_BYTES constant - integrations/adserver_mock.rs, aps.rs, prebid.rs: merged collect_body import (PR14) with ensure_integration_backend_with_timeout / predict_backend_name_for_url imports (PR15) into single use statements

aram356

Summary

This PR delivers its stated goal — trusted-server-core is now fully fastly-free and the consent pipeline is abstracted behind a minimal ConsentKvOps trait. Local verification passes (fmt, clippy -D warnings, cargo test --workspace, wasm32-wasip1 release build).

Three concerns merit changes before merge: a diagnostic regression in predict_backend_name_for_url, duplication of the security-adjacent SPOOFABLE_FORWARDED_HEADERS list, and a test-coverage regression in the relocated adapter compat.rs. The remaining items are non-blocking observations and follow-up candidates.

Note: this PR targets feature/edgezero-pr14-entry-point-dual-path, not main; review scope is the 31-file PR-15-specific delta (+2230/-2165).

Blocking

🔧 wrench

Diagnostic regression in predict_backend_name_for_url — inline comment at crates/trusted-server-core/src/integrations/mod.rs:135
SPOOFABLE_FORWARDED_HEADERS duplicated across core and adapter — inline comment at crates/trusted-server-adapter-fastly/src/compat.rs:18
Adapter compat.rs has zero tests after relocation (14 security-relevant tests dropped) — inline comment at crates/trusted-server-adapter-fastly/src/compat.rs:82

Non-blocking

🤔 thinking

KV errors logged at debug level as "lookup miss" — inline comment at crates/trusted-server-adapter-fastly/src/platform.rs:420
Backend-name compute logic duplicated between predict_backend_name_for_url (core) and BackendConfig::compute_name (adapter) — inline comment at crates/trusted-server-core/src/integrations/mod.rs:130

🌱 seedling

Redundant KV read on fallback+save path — inline comment at crates/trusted-server-core/src/consent/kv.rs:267
Hardcoded certificate_check: true in ensure_integration_backend (integrations/mod.rs:69) and ensure_integration_backend_with_timeout (integrations/mod.rs:114). No regression vs. pre-PR behaviour, but the sibling predict_backend_name_for_url does take certificate_check: bool — asymmetric. If any integration ever needs a self-signed upstream (test/on-prem), these helpers would need to grow that parameter.

📌 out of scope

migration_guards.rs not extended — migration_guards.rs:27-46 only covers 12 files and a narrow regex (Request|Response|http::Method|StatusCode|mime::APPLICATION_JSON). With core's fastly dep now removed, this guard should either broaden to \bfastly:: across all core files, be replaced by a Cargo-metadata dependency-presence test (more robust, no false negatives), or be deleted as structurally redundant. Follow-up issue.
FastlyConsentKvStore::open silent failure — inline comment at crates/trusted-server-adapter-fastly/src/platform.rs:409

📝 note

Stale "PR 15" forward references:
- adapter-fastly/src/app.rs:150: "will be removed when legacy_main is deleted in PR 15". This is PR 15 and legacy_main was not deleted.
- adapter-fastly/src/main.rs:113: "compat.rs will be deleted in PR 15 once this legacy path is retired". This is PR 15; compat.rs was moved (not deleted) and the legacy path was not retired.
  Update comments to reference the actual future PR number or drop the promise.

⛏ nitpick

Unnecessary u16 type suffix — inline comment at crates/trusted-server-core/src/integrations/mod.rs:140

CI Status

fmt: PASS
clippy (-D warnings): PASS
rust tests (cargo test --workspace): PASS (800+ tests)
wasm32-wasip1 release build: PASS
PR-reported checks (browser integration, integration tests, prepare integration artifacts): PASS

…mpat tests - predict_backend_name_for_url now returns Result<String, Report<TrustedServerError>> instead of Option<String>; call sites use inspect_err(...).ok() to preserve the Option<String> trait interface while logging the actual failure reason - Promote SPOOFABLE_FORWARDED_HEADERS to pub in http_util; replace inline copy in compat.rs with a use import — single source of truth for the strip list - Add sanitize_fastly_forwarded_headers_strips_spoofable test (all 4 entries + Host preserved) and to_fastly_response_with_streaming_body_produces_empty_body test (pins silent-drop behaviour) to compat.rs - Change Consent KV non-ItemNotFound error log from debug "lookup miss" to warn "lookup failed" for operational visibility - Drop stale "will be removed in PR 15" forward references from app.rs and main.rs - Drop unnecessary u16 type suffixes on port literal defaults

ChristianPavilonis

Summary

Submitted reviewer-approved findings for consent/KV handling, backend naming consistency, and Fastly wiring.
Verdict: REQUEST_CHANGES (includes a P1 finding).

Findings moved to review body

The following approved findings were included in the review body because GitHub could not resolve their requested inline locations in the current diff:

🤔 Consent bootstrap logic is now duplicated across auction and publisher paths (crates/trusted-server-core/src/auction/endpoints.rs:61)
Both handlers now repeat the same sequence: parse cookies, generate/read synthetic ID, geo lookup with identical error handling, and build ConsentPipelineInput with kv_ops. This duplication is likely to drift as the consent pipeline evolves; this PR already had to touch both call sites just to thread the new abstraction through.
Suggestion: Extract a small shared helper that prepares request-scoped consent state once and returns the ConsentContext plus any reused intermediates such as synthetic_id, geo, and cookie_jar.
⛏ Public docs/comments still lag the new kv_ops contract (crates/trusted-server-core/src/auction/endpoints.rs:21)
The new kv_ops parameter materially changes how consent persistence is enabled, but the public docs still describe these handlers mostly in their pre-PR form, and one consent comment still says fallback requires consent_store rather than kv_ops.
Suggestion: Update the affected doc comments to explain that consent KV fallback/write-through occurs only when a concrete kv_ops implementation is supplied, and adjust the stale internal comment in consent/mod.rs.

prk-Jr · 2026-04-23T09:16:02Z

Fixed the two findings GitHub moved into the review body in 3a53f33.

Extracted shared consent bootstrap into consent::prepare_request_consent_state(...), and both the auction and publisher handlers now use it instead of duplicating cookie parsing, synthetic-id setup, geo lookup, and consent-context construction.
Updated the affected handler docs plus the stale internal comment so they describe the kv_ops contract correctly: KV fallback/write-through only happens when a concrete kv_ops implementation is supplied.

Fresh verification on this commit:

cargo fmt --all -- --check
cargo clippy --workspace --all-targets --all-features -- -D warnings
cargo test --workspace

…ro-pr15-remove-fastly-core

ChristianPavilonis

Summary

Approving this PR. The main refactor looks solid: trusted-server-core is decoupled from Fastly and the adapter now owns the Fastly-specific backend, KV, and compatibility wiring.

I left 2 non-blocking follow-up comments inline for logging/test coverage.

ChristianPavilonis · 2026-04-24T18:58:57Z

-        }
-    }
+    kv.save_entry_with_ttl(key, &entry, ttl);
+    log::info!("Saved consent to KV store for '{key}' (fp={new_fp}, ttl={max_age_days}d)");


Misleading success log: save_consent() logs a successful KV write unconditionally after kv.save_entry_with_ttl(...), but the trait returns (), so core cannot tell whether the adapter actually wrote anything. On Fastly, a failed write already logs a warning, so this can emit contradictory logs for the same request.

Suggestion: Either make ConsentKvOps::save_entry_with_ttl return a Result<(), _> / success flag so core only logs success on success, or remove this post-call success log and let the platform adapter own success/failure logging.

ChristianPavilonis · 2026-04-24T18:58:57Z

-            if let Some(store_name) = &settings.consent.consent_store {
-                crate::consent::kv::delete_consent_from_kv(store_name, cookie_synthetic_id);
+            if let Some(kv) = kv_ops {
+                kv.delete_entry(cookie_synthetic_id);


Missing regression coverage for consent revocation delete path: this injected kv_ops.delete_entry(...) call is the core behavior change for revocation, but I could not find a test that exercises the existing SSC cookie + consent withdrawn branch and asserts that the cookie SID is deleted. Without that coverage, a future refactor could break revocation by dropping kv_ops wiring or deleting the wrong ID source.

Suggestion: Add a publisher test with a stub ConsentKvOps that sends a request with an existing SSC cookie and no consent signal, then assert that the response expires the SSC cookie and that delete_entry() is called exactly once with the cookie SID.

prk-Jr added 11 commits April 14, 2026 13:18

Add PR15 implementation plan: remove fastly from core crate

6c4965b

Move compat conversion fns to adapter, delete core compat.rs

f67be31

Move geo_from_fastly from core to adapter platform

c2e0776

Delete dead backend_name_for_url from adapter backend

8303b84

All callers were migrated to predict_backend_name_for_url in core's integrations module. Remove the now-unused method and update the parse_origin doc comment that referenced it.

Move tokio to dev-dependencies in core (test-only usage)

d96dbbe

Remove fastly dependency from trusted-server-core

8f40601

Remove stale fastly:: references from core doc comments

9a4357c

prk-Jr self-assigned this Apr 14, 2026

Apply cargo fmt formatting fixes

46704c0

prk-Jr marked this pull request as draft April 14, 2026 15:20

prk-Jr added 2 commits April 15, 2026 11:00

Wire consent KV into auction path and remove tokio from core tests

60611fa

prk-Jr marked this pull request as ready for review April 16, 2026 12:47

prk-Jr requested review from ChristianPavilonis and aram356 April 16, 2026 12:48

prk-Jr linked an issue Apr 16, 2026 that may be closed by this pull request

Remove fastly from core crate #496

Open

aram356 requested changes Apr 20, 2026

View reviewed changes

prk-Jr requested a review from aram356 April 21, 2026 07:10

ChristianPavilonis requested changes Apr 21, 2026

View reviewed changes

Comment thread crates/trusted-server-adapter-fastly/src/platform.rs

Comment thread crates/trusted-server-core/src/integrations/mod.rs

Comment thread crates/trusted-server-adapter-fastly/src/app.rs Outdated

Resolve PR15 consent and backend review findings

3a53f33

Merge feature/edgezero-pr14-entry-point-dual-path into feature/edgeze…

54d191b

…ro-pr15-remove-fastly-core

prk-Jr requested a review from ChristianPavilonis April 24, 2026 08:08

ChristianPavilonis approved these changes Apr 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove fastly dependency from trusted-server-core (PR 15)#635

Remove fastly dependency from trusted-server-core (PR 15)#635
prk-Jr wants to merge 17 commits intofeature/edgezero-pr14-entry-point-dual-pathfrom
feature/edgezero-pr15-remove-fastly-core

prk-Jr commented Apr 14, 2026

Uh oh!

aram356 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ChristianPavilonis left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

prk-Jr commented Apr 23, 2026

Uh oh!

ChristianPavilonis left a comment

Uh oh!

ChristianPavilonis Apr 24, 2026

Uh oh!

ChristianPavilonis Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

prk-Jr commented Apr 14, 2026

Summary

Changes

Closes

Test plan

Checklist

Uh oh!

aram356 left a comment

Choose a reason for hiding this comment

Summary

Blocking

🔧 wrench

Non-blocking

🤔 thinking

🌱 seedling

📌 out of scope

📝 note

⛏ nitpick

CI Status

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ChristianPavilonis left a comment

Choose a reason for hiding this comment

Summary

Findings moved to review body

Uh oh!

Uh oh!

Uh oh!

Uh oh!

prk-Jr commented Apr 23, 2026

Uh oh!

ChristianPavilonis left a comment

Choose a reason for hiding this comment

Summary

Uh oh!

ChristianPavilonis Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

ChristianPavilonis Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants