diff --git a/.claude/commands/check-ci.md b/.claude/commands/check-ci.md index ba785ccca..62b628140 100644 --- a/.claude/commands/check-ci.md +++ b/.claude/commands/check-ci.md @@ -1,7 +1,7 @@ Run all CI checks locally, in order. Stop and report if any step fails. 1. `cargo fmt --all -- --check` -2. `cargo clippy-fastly && cargo clippy-axum && cargo clippy-cloudflare` +2. `cargo clippy-fastly && cargo clippy-axum && cargo clippy-cloudflare && cargo clippy-cloudflare-wasm && scripts/lint-wasm-clock.sh` 3. `cargo test-fastly && cargo test-axum && cargo test-cloudflare` 4. `cargo check-cloudflare` (wasm32-unknown-unknown target check, mirrors CI) 5. `cd crates/trusted-server-js/lib && npx vitest run` diff --git a/.github/workflows/format.yml b/.github/workflows/format.yml index 6a8c7b08b..ef36c83d6 100644 --- a/.github/workflows/format.yml +++ b/.github/workflows/format.yml @@ -73,6 +73,9 @@ jobs: - name: Run cargo clippy (Cloudflare — wasm32-unknown-unknown) run: cargo clippy-cloudflare-wasm + - name: Check wasm-safe clock usage in the Cloudflare build (wasm32-unknown-unknown) + run: scripts/lint-wasm-clock.sh + - name: Run cargo clippy (Spin — native) run: cargo clippy-spin-native diff --git a/AGENTS.md b/AGENTS.md index 2685d164e..dd1dd5233 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -362,6 +362,7 @@ Every PR must pass: 7. Docs format (`cd docs && npm run format`) 8. Markdown format outside `docs/` (requires `cd docs && npm ci` first): `docs/node_modules/.bin/prettier --config docs/.prettierrc --check "*.md" ".claude/**/*.md" ".github/**/*.md" "crates/**/*.md" "scripts/**/*.md" "tinybird/**/*.md"`; fix with `--write` in place of `--check` 9. Native core build-digest test and lint (`cargo test -p trusted-server-core --test template_build_digest` and `cargo clippy -p trusted-server-core --test template_build_digest -- -D warnings`) +10. Wasm clock lint: `scripts/lint-wasm-clock.sh` (crates built into the Cloudflare Worker must use `web_time`, not the `std::time::{Instant,SystemTime}` `now`/`elapsed` methods, which panic there) --- diff --git a/crates/trusted-server-adapter-cloudflare/.gitignore b/crates/trusted-server-adapter-cloudflare/.gitignore index e3df6a859..be872c72e 100644 --- a/crates/trusted-server-adapter-cloudflare/.gitignore +++ b/crates/trusted-server-adapter-cloudflare/.gitignore @@ -5,3 +5,4 @@ build/ .worker-build/ .env.cloudflare.dev .dev.vars +wrangler.integration.generated.toml diff --git a/crates/trusted-server-core/Cargo.toml b/crates/trusted-server-core/Cargo.toml index 40cbac4d4..0a8d05280 100644 --- a/crates/trusted-server-core/Cargo.toml +++ b/crates/trusted-server-core/Cargo.toml @@ -58,6 +58,9 @@ web-time = { workspace = true } # need these — this block is intentionally scoped to `target_os = "unknown"` only. getrandom = { workspace = true, features = ["js"] } uuid = { workspace = true, features = ["js"] } +# `chrono::Utc::now()` calls `std::time::SystemTime::now()`, which panics on +# `wasm32-unknown-unknown`, unless `wasmbind` makes it read the JS clock. +chrono = { workspace = true, features = ["wasmbind"] } [features] default = [] diff --git a/crates/trusted-server-core/src/auction/telemetry.rs b/crates/trusted-server-core/src/auction/telemetry.rs index c6e37d02e..e0a819ba3 100644 --- a/crates/trusted-server-core/src/auction/telemetry.rs +++ b/crates/trusted-server-core/src/auction/telemetry.rs @@ -4,12 +4,12 @@ //! adapters provide the concrete sink implementation. use std::collections::HashSet; -use std::time::Instant; use chrono::Utc; use error_stack::Report; use serde::Serialize; use uuid::Uuid; +use web_time::Instant; use crate::auction::orchestrator::OrchestrationResult; use crate::auction::types::{AuctionRequest, AuctionResponse, Bid, BidStatus, MediaType}; @@ -211,6 +211,11 @@ impl AuctionObservationContext { } /// Return elapsed milliseconds since the observation was created. + /// + /// On Cloudflare Workers, [`Instant`] reads `performance.now()`, which only + /// advances after I/O. There this counts time spent waiting on I/O and + /// reads 0 for CPU-only spans, so it is not comparable with the wall-clock + /// value other adapters report. #[must_use] pub fn elapsed_ms(&self) -> u64 { u64::try_from(self.started_at.elapsed().as_millis()).unwrap_or(u64::MAX) diff --git a/crates/trusted-server-core/src/ec/mod.rs b/crates/trusted-server-core/src/ec/mod.rs index 31a57fece..f24c489ed 100644 --- a/crates/trusted-server-core/src/ec/mod.rs +++ b/crates/trusted-server-core/src/ec/mod.rs @@ -746,6 +746,21 @@ pub(crate) fn checked_current_timestamp() -> Option { .ok() } +/// Returns the current time as a [`std::time::SystemTime`] without calling +/// `std::time::SystemTime::now()`, which panics on `wasm32-unknown-unknown`. +/// +/// Use this for APIs that only accept the std type, such as `httpdate` or +/// chrono's `DateTime::from`. It reads the wasm-safe [`web_time::SystemTime`] +/// clock and adds the elapsed time to [`std::time::UNIX_EPOCH`], which is pure +/// arithmetic. On native and `wasm32-wasip1`, [`web_time`] delegates to the +/// std clock, so this equals `std::time::SystemTime::now()` there. +pub(crate) fn std_system_time_now() -> std::time::SystemTime { + std::time::UNIX_EPOCH + + web_time::SystemTime::now() + .duration_since(web_time::UNIX_EPOCH) + .unwrap_or_default() +} + #[cfg(test)] mod tests { use super::*; @@ -1148,4 +1163,17 @@ mod tests { "should return cookie value for revocation even when header is present" ); } + + #[test] + fn std_system_time_now_tracks_std_clock() { + let before = std::time::SystemTime::now(); + + let now = std_system_time_now(); + + let after = std::time::SystemTime::now(); + assert!( + before <= now && now <= after, + "should fall between std clock reads taken before and after" + ); + } } diff --git a/crates/trusted-server-core/src/integrations/datadome/protection_scope.rs b/crates/trusted-server-core/src/integrations/datadome/protection_scope.rs index 73bd77070..6ddc51fe5 100644 --- a/crates/trusted-server-core/src/integrations/datadome/protection_scope.rs +++ b/crates/trusted-server-core/src/integrations/datadome/protection_scope.rs @@ -2,11 +2,12 @@ use std::collections::{HashMap, HashSet}; use std::net::IpAddr; use std::str::FromStr; use std::sync::{LazyLock, Mutex}; -use std::time::{Duration, Instant}; +use std::time::Duration; use error_stack::Report; use regex::Regex; use serde::Deserialize; +use web_time::Instant; use crate::error::TrustedServerError; use crate::platform::{RuntimeServices, StoreName}; diff --git a/crates/trusted-server-core/src/proxy.rs b/crates/trusted-server-core/src/proxy.rs index 5d659cdbf..99f3e5f28 100644 --- a/crates/trusted-server-core/src/proxy.rs +++ b/crates/trusted-server-core/src/proxy.rs @@ -907,16 +907,8 @@ fn apply_asset_origin_auth( &config.region, &credentials, // s3_sigv4 converts this via chrono's `DateTime::::from`, which - // only accepts `std::time::SystemTime`. `std::time::SystemTime::now()` - // panics on `wasm32-unknown-unknown` (Cloudflare Workers), so derive an - // equivalent `std::time::SystemTime` from the wasm-safe `web_time` clock: - // `UNIX_EPOCH + elapsed` is pure arithmetic and never calls the panicking - // `now()`. On Fastly (wasm32-wasip1) and native, `web_time` delegates to - // the std clock, so behavior is unchanged there. - std::time::UNIX_EPOCH - + web_time::SystemTime::now() - .duration_since(web_time::UNIX_EPOCH) - .unwrap_or_default(), + // only accepts `std::time::SystemTime`. + crate::ec::std_system_time_now(), ) } } diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index d8c217076..682ecc0f1 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -22,7 +22,7 @@ use std::borrow::Cow; use std::io::Write; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; -use std::time::{Duration, Instant, SystemTime}; +use std::time::{Duration, SystemTime}; use brotli::Decompressor; use brotli::enc::BrotliEncoderParams; @@ -35,6 +35,7 @@ use flate2::write::{GzEncoder, ZlibEncoder}; use futures::StreamExt as _; use http::{HeaderValue, Method, Request, Response, StatusCode, Uri, header}; use sha2::Digest as _; +use web_time::Instant; use crate::auction::endpoints::{ merge_auction_eids, resolve_auction_eids, resolve_client_auction_eids, @@ -6470,7 +6471,9 @@ fn origin_shared_ttl( headers: &edgezero_core::http::HeaderMap, max_age: Duration, ) -> Result { - origin_shared_ttl_at(headers, SystemTime::now(), max_age) + // `httpdate` parses `Date`/`Expires` into `std::time::SystemTime`, so take + // the wasm-safe std value rather than calling `SystemTime::now()`. + origin_shared_ttl_at(headers, crate::ec::std_system_time_now(), max_age) } fn origin_shared_ttl_at( diff --git a/crates/trusted-server-integration-tests/tests/common/config.rs b/crates/trusted-server-integration-tests/tests/common/config.rs index 3bcbfe53e..0b793833f 100644 --- a/crates/trusted-server-integration-tests/tests/common/config.rs +++ b/crates/trusted-server-integration-tests/tests/common/config.rs @@ -9,12 +9,28 @@ const GENERATED_AT: &str = "2026-06-23T00:00:00Z"; const APP_CONFIG: &str = include_str!("../../fixtures/configs/trusted-server.integration.toml"); pub fn integration_app_config_envelope(origin_port: u16) -> TestResult { + app_config_envelope(origin_port, |_| Ok(())) +} + +/// Builds the integration app-config envelope after `customize` edits the +/// parsed fixture. +fn app_config_envelope( + origin_port: u16, + customize: impl FnOnce(&mut toml::Table) -> TestResult<()>, +) -> TestResult { let origin_url = format!("http://127.0.0.1:{origin_port}"); - let app_config: TrustedServerAppConfig = toml::from_str(APP_CONFIG).map_err(|error| { + let mut table: toml::Table = toml::from_str(APP_CONFIG).map_err(|error| { Report::new(TestError::ConfigGeneration).attach(format!( "invalid Trusted Server integration config: {error}" )) })?; + customize(&mut table)?; + let app_config: TrustedServerAppConfig = + toml::Value::Table(table).try_into().map_err(|error| { + Report::new(TestError::ConfigGeneration).attach(format!( + "invalid customized Trusted Server integration config: {error}" + )) + })?; let mut settings = app_config.into_settings(); settings.publisher.origin_url = origin_url; let app_config = TrustedServerAppConfig::new(settings).map_err(|report| { @@ -36,7 +52,37 @@ pub fn integration_app_config_envelope(origin_port: u16) -> TestResult { pub fn cloudflare_config_json(origin_port: u16) -> TestResult { let envelope = integration_app_config_envelope(origin_port)?; - serde_json::to_string(&serde_json::json!({ CONFIG_BLOB_KEY: envelope })).map_err(|error| { + cloudflare_binding_json(&serde_json::json!({ CONFIG_BLOB_KEY: envelope })) +} + +/// Builds a Cloudflare config binding with server-side auctions enabled. +/// +/// The fixture's providers are removed so the enabled auction completes +/// through the orchestrator and telemetry without any outbound provider call. +pub fn cloudflare_auction_config_json(origin_port: u16) -> TestResult { + let envelope = app_config_envelope(origin_port, |table| { + let auction = table_mut(table, "auction")?; + auction.insert("enabled".to_string(), toml::Value::Boolean(true)); + auction.remove("providers"); + auction.remove("bidders"); + Ok(()) + })?; + cloudflare_binding_json(&serde_json::json!({ CONFIG_BLOB_KEY: envelope })) +} + +fn table_mut<'a>(table: &'a mut toml::Table, key: &str) -> TestResult<&'a mut toml::Table> { + table + .get_mut(key) + .and_then(toml::Value::as_table_mut) + .ok_or_else(|| { + Report::new(TestError::ConfigGeneration).attach(format!( + "integration config should contain a `[{key}]` table" + )) + }) +} + +fn cloudflare_binding_json(binding: &serde_json::Value) -> TestResult { + serde_json::to_string(binding).map_err(|error| { Report::new(TestError::ConfigGeneration).attach(format!( "failed to serialize Cloudflare config binding: {error}" )) diff --git a/crates/trusted-server-integration-tests/tests/environments/cloudflare.rs b/crates/trusted-server-integration-tests/tests/environments/cloudflare.rs index 10d16c0a7..72328f42f 100644 --- a/crates/trusted-server-integration-tests/tests/environments/cloudflare.rs +++ b/crates/trusted-server-integration-tests/tests/environments/cloudflare.rs @@ -27,7 +27,7 @@ const CI_CONFIG_TEMPLATE: &str = "wrangler.ci.toml"; const GENERATED_CI_CONFIG: &str = "wrangler.integration.generated.toml"; const TRUSTED_SERVER_CONFIG_PLACEHOLDER: &str = "TRUSTED_SERVER_CONFIG = \"{}\""; -fn write_generated_ci_config(wrangler_dir: &Path) -> TestResult { +fn write_generated_ci_config(wrangler_dir: &Path, config_json: &str) -> TestResult { let template_path = wrangler_dir.join(CI_CONFIG_TEMPLATE); let template = std::fs::read_to_string(&template_path) .change_context(TestError::RuntimeSpawn) @@ -35,8 +35,7 @@ fn write_generated_ci_config(wrangler_dir: &Path) -> TestResult { "failed to read Cloudflare CI wrangler config at {}", template_path.display() ))?; - let config_json = cloudflare_config_json(origin_port())?; - let generated = inject_cloudflare_config(&template, &config_json)?; + let generated = inject_cloudflare_config(&template, config_json)?; let output_path = wrangler_dir.join(GENERATED_CI_CONFIG); std::fs::write(&output_path, generated) .change_context(TestError::RuntimeSpawn) @@ -69,11 +68,37 @@ impl RuntimeEnvironment for CloudflareWorkers { fn spawn(&self, _wasm_path: &Path) -> TestResult { let wrangler_dir = self.wrangler_dir(); let config = if std::env::var("CI").is_ok() { - write_generated_ci_config(&wrangler_dir)? + write_generated_ci_config(&wrangler_dir, &cloudflare_config_json(origin_port())?)? } else { "wrangler.toml".to_string() }; + self.spawn_wrangler(&wrangler_dir, &config) + } + + fn health_check_path(&self) -> &str { + "/.well-known/trusted-server.json" + } +} +impl CloudflareWorkers { + /// Start `wrangler dev` with a test-specific `TRUSTED_SERVER_CONFIG` binding. + /// + /// Unlike [`RuntimeEnvironment::spawn`], this always renders the CI + /// wrangler template, so tests can enable features the shared fixture + /// leaves off without changing the other runtimes' configuration. + /// + /// # Errors + /// + /// Returns [`TestError::RuntimeSpawn`] if the wrangler config cannot be + /// written or `wrangler dev` cannot be started, and + /// [`TestError::RuntimeNotReady`] if the health check times out. + pub fn spawn_with_config_json(&self, config_json: &str) -> TestResult { + let wrangler_dir = self.wrangler_dir(); + let config = write_generated_ci_config(&wrangler_dir, config_json)?; + self.spawn_wrangler(&wrangler_dir, &config) + } + + fn spawn_wrangler(&self, wrangler_dir: &Path, config: &str) -> TestResult { let port = super::find_available_port().unwrap_or(CLOUDFLARE_DEFAULT_PORT); #[cfg(unix)] @@ -83,13 +108,13 @@ impl RuntimeEnvironment for CloudflareWorkers { .args([ "dev", "--config", - config.as_str(), + config, "--port", &port.to_string(), "--ip", "127.0.0.1", ]) - .current_dir(&wrangler_dir) + .current_dir(wrangler_dir) .stdout(Stdio::null()) .stderr(Stdio::piped()) .process_group(0) @@ -108,13 +133,13 @@ impl RuntimeEnvironment for CloudflareWorkers { .args([ "dev", "--config", - config.as_str(), + config, "--port", &port.to_string(), "--ip", "127.0.0.1", ]) - .current_dir(&wrangler_dir) + .current_dir(wrangler_dir) .stdout(Stdio::null()) .stderr(Stdio::piped()) .spawn() @@ -149,12 +174,6 @@ impl RuntimeEnvironment for CloudflareWorkers { }) } - fn health_check_path(&self) -> &str { - "/.well-known/trusted-server.json" - } -} - -impl CloudflareWorkers { /// Resolve the Cloudflare adapter crate root. /// /// Respects `CLOUDFLARE_WRANGLER_DIR` for CI overrides; falls back to diff --git a/crates/trusted-server-integration-tests/tests/integration.rs b/crates/trusted-server-integration-tests/tests/integration.rs index ebd026410..ad8249307 100644 --- a/crates/trusted-server-integration-tests/tests/integration.rs +++ b/crates/trusted-server-integration-tests/tests/integration.rs @@ -214,6 +214,60 @@ fn test_cloudflare_dynamic_publisher_response_does_not_cross_cookie_boundaries() ); } +/// Exercises an enabled `/auction` request through auction telemetry in workerd. +/// +/// `std::time::Instant::now()` traps on Cloudflare's `wasm32-unknown-unknown` +/// target, so this guards the `web_time::Instant` timing in +/// `AuctionObservationContext`. The auction config has no providers, so the +/// enabled auction completes as a no-bid without outbound calls. +#[test] +#[ignore = "requires the `wrangler` CLI in $PATH and a prebuilt Cloudflare Workers bundle (run build.sh first); the test starts wrangler dev automatically"] +fn test_cloudflare_enabled_auction_reaches_telemetry() { + init_logger(); + // The readiness probe falls back to the proxied root page. + let _origin = common::ec::MinimalOrigin::start(origin_port()); + let config_json = common::config::cloudflare_auction_config_json(origin_port()) + .expect("should build auction-enabled Cloudflare config"); + let process = environments::cloudflare::CloudflareWorkers + .spawn_with_config_json(&config_json) + .expect("should start Cloudflare Worker"); + let request_body = serde_json::json!({ + "adUnits": [{ + "code": "integration-slot", + "mediaTypes": { "banner": { "sizes": [[300, 250]] } } + }] + }); + + // A non-regulated geo passes the server-side auction consent gate, so the + // request runs the enabled orchestration path instead of a consent skip. + let response = reqwest::blocking::Client::new() + .post(format!("{}/auction", process.base_url)) + .header("cf-ipcountry", "US") + .json(&request_body) + .send() + .expect("should send auction request"); + + let status = response.status().as_u16(); + let body = response.text().expect("should read auction response"); + assert_eq!( + status, 200, + "enabled auction should complete without a worker exception: {body}" + ); + let auction_response: serde_json::Value = + serde_json::from_str(&body).expect("should return an OpenRTB JSON response"); + assert!( + auction_response["id"] + .as_str() + .is_some_and(|id| !id.is_empty()), + "auction response should carry the auction id: {body}" + ); + assert_eq!( + auction_response["ext"]["orchestrator"]["providers"], + serde_json::json!(0), + "enabled auction without providers should complete as a no-bid: {body}" + ); +} + #[test] #[ignore = "requires Docker and pre-built trusted-server-axum binary"] fn test_wordpress_axum() { diff --git a/scripts/clippy-wasm-clock/clippy.toml b/scripts/clippy-wasm-clock/clippy.toml new file mode 100644 index 000000000..1a1070e00 --- /dev/null +++ b/scripts/clippy-wasm-clock/clippy.toml @@ -0,0 +1,8 @@ +# Dedicated clippy config for scripts/lint-wasm-clock.sh. See that script for +# why this rule cannot live in the workspace-wide clippy.toml. +disallowed-methods = [ + { path = "std::time::Instant::now", reason = "panics on wasm32-unknown-unknown; use `web_time::Instant::now`" }, + { path = "std::time::Instant::elapsed", reason = "calls `Instant::now`, which panics on wasm32-unknown-unknown; use `web_time::Instant`" }, + { path = "std::time::SystemTime::now", reason = "panics on wasm32-unknown-unknown; use `web_time::SystemTime::now`" }, + { path = "std::time::SystemTime::elapsed", reason = "calls `SystemTime::now`, which panics on wasm32-unknown-unknown; use `web_time::SystemTime`" }, +] diff --git a/scripts/lint-wasm-clock.sh b/scripts/lint-wasm-clock.sh new file mode 100755 index 000000000..00e9215e6 --- /dev/null +++ b/scripts/lint-wasm-clock.sh @@ -0,0 +1,22 @@ +#!/usr/bin/env bash +# Fail if a workspace crate built into the Cloudflare Worker calls a std clock +# method that panics with `RuntimeError: unreachable` on wasm32-unknown-unknown +# (std::time::Instant::{now,elapsed}, std::time::SystemTime::{now,elapsed}); +# use `web_time` instead. +# +# This cannot live in the workspace clippy.toml: on native and wasm32-wasip1, +# `web_time::Instant` and `web_time::SystemTime` are re-exports of the std +# types, so clippy would flag the compliant `web_time` calls too. On +# wasm32-unknown-unknown the types are distinct, so only that target can tell +# them apart. The dedicated config dir holds just the disallowed-methods rule. +# +# Linting the Cloudflare adapter covers trusted-server-core and the other +# workspace crates it builds, with the production feature set, and reuses the +# dependency artifacts from `cargo clippy-cloudflare-wasm`. +set -euo pipefail + +root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" + +CLIPPY_CONF_DIR="$root/scripts/clippy-wasm-clock" \ + cargo clippy -p trusted-server-adapter-cloudflare --target wasm32-unknown-unknown \ + --features cloudflare --lib -- -A clippy::all -D clippy::disallowed_methods