Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion crates/trusted-server-adapter-axum/src/platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ fn normalize_env_segment(s: &str) -> String {
s.to_uppercase().replace(['-', '.', ' '], "_")
}

fn config_env_var(store_name: &str, key: &str) -> String {
/// Returns the environment-variable name for a config store entry.
#[must_use]
pub fn config_env_var(store_name: &str, key: &str) -> String {
format!(
"TRUSTED_SERVER_CONFIG_{}_{}",
normalize_env_segment(store_name),
Expand Down Expand Up @@ -601,6 +603,15 @@ mod tests {
use std::time::Duration;
use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _};

#[test]
fn config_env_var_normalizes_store_and_key() {
assert_eq!(
config_env_var("my-store.name", "my key"),
"TRUSTED_SERVER_CONFIG_MY_STORE_NAME_MY_KEY",
"should normalize environment-variable segments"
);
}

#[test]
fn config_store_reads_from_env_var() {
temp_env::with_var(
Expand Down
173 changes: 164 additions & 9 deletions crates/trusted-server-adapter-cloudflare/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ use edgezero_core::router::RouterService;
use error_stack::Report;
use trusted_server_core::auction::endpoints::handle_auction;
use trusted_server_core::auction::{AuctionOrchestrator, build_orchestrator};
#[cfg(any(test, target_arch = "wasm32"))]
use trusted_server_core::config_payload::CONFIG_BLOB_KEY;
#[cfg(target_arch = "wasm32")]
use trusted_server_core::config_payload::settings_from_config_blob;
use trusted_server_core::ec::EcContext;
Expand Down Expand Up @@ -78,31 +80,92 @@ fn load_startup_settings() -> Result<Settings, Report<TrustedServerError>> {
Settings::from_toml(include_str!("../../../trusted-server.example.toml"))
}

/// Older Cloudflare bindings used this JSON property before config stores adopted
/// the manifest-derived default.
///
/// Remove this fallback only when support for those bindings is deliberately retired.
#[cfg(any(test, target_arch = "wasm32"))]
const LEGACY_CONFIG_BLOB_KEY: &str = "app_config";

#[cfg(any(test, target_arch = "wasm32"))]
#[derive(Debug, Eq, PartialEq)]
enum CloudflareConfigEnvelopeError {
Comment thread
ChristianPavilonis marked this conversation as resolved.

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 — This error type carries its message in an inherent method rather than through the project's error convention.

CLAUDE.md is explicit: define errors with derive_more::Display plus impl core::error::Error, not thiserror and not a hand-rolled formatter. configuration_message() is a Display implementation under another name — the sole caller does message: error.configuration_message(), which is what {error} would give for free. The #[display(...)] attributes also put each message next to the variant it belongs to, which is where a reader looks for it.

Proposed fix (apply manually — it rewrites the type, deletes the impl block, and edits the call site in settings_from_cloudflare_config_json, which spans three separate hunks):

#[cfg(any(test, target_arch = "wasm32"))]
#[derive(Debug, Eq, PartialEq, derive_more::Display)]
enum CloudflareConfigEnvelopeError {
    #[display(
        "Cloudflare TRUSTED_SERVER_CONFIG missing string values at `{primary_key}` and legacy `{legacy_key}`"
    )]
    Missing {
        primary_key: &'static str,
        legacy_key: &'static str,
    },
    #[display("Cloudflare TRUSTED_SERVER_CONFIG value at `{key}` must be a string")]
    NonString { key: &'static str },
}

#[cfg(any(test, target_arch = "wasm32"))]
impl core::error::Error for CloudflareConfigEnvelopeError {}

The call site then becomes message: error.to_string(), and the assertion in cloudflare_config_reports_malformed_legacy_value becomes error.to_string() — the message bytes are unchanged either way.

Missing {
primary_key: &'static str,
legacy_key: &'static str,
},
NonString {
key: &'static str,
},
}

#[cfg(any(test, target_arch = "wasm32"))]
impl CloudflareConfigEnvelopeError {
fn configuration_message(&self) -> String {
match self {
Self::Missing {
primary_key,
legacy_key,
} => {
format!(
"Cloudflare TRUSTED_SERVER_CONFIG missing string values at `{primary_key}` and legacy `{legacy_key}`"
)
}
Self::NonString { key } => {
format!("Cloudflare TRUSTED_SERVER_CONFIG value at `{key}` must be a string")
}
}
}
}

#[cfg(target_arch = "wasm32")]
fn settings_from_cloudflare_config_json() -> Result<Settings, Report<TrustedServerError>> {
let raw_config = CLOUDFLARE_CONFIG_JSON.get().ok_or_else(|| {
Report::new(TrustedServerError::Configuration {
message: "Cloudflare TRUSTED_SERVER_CONFIG is required".to_string(),
})
.attach("set TRUSTED_SERVER_CONFIG to JSON containing the app_config blob envelope")
.attach(format!(
"set TRUSTED_SERVER_CONFIG to JSON containing the `{CONFIG_BLOB_KEY}` blob envelope"
))
})?;
let value: serde_json::Value = serde_json::from_str(raw_config).map_err(|error| {
Report::new(TrustedServerError::Configuration {
message: "invalid Cloudflare TRUSTED_SERVER_CONFIG JSON".to_string(),
})
.attach(format!("failed to parse TRUSTED_SERVER_CONFIG: {error}"))
})?;
let envelope = value
.get("app_config")
.and_then(serde_json::Value::as_str)
.ok_or_else(|| {
Report::new(TrustedServerError::Configuration {
message: "Cloudflare TRUSTED_SERVER_CONFIG missing app_config".to_string(),
})
})?;
let envelope = cloudflare_config_envelope(&value).map_err(|error| {
Report::new(TrustedServerError::Configuration {
message: error.configuration_message(),
})
})?;
settings_from_config_blob(envelope)
}

#[cfg(any(test, target_arch = "wasm32"))]
fn cloudflare_config_envelope(
value: &serde_json::Value,
) -> Result<&str, CloudflareConfigEnvelopeError> {
match value.get(CONFIG_BLOB_KEY) {
Some(envelope) => envelope
.as_str()
.ok_or(CloudflareConfigEnvelopeError::NonString {
key: CONFIG_BLOB_KEY,
}),
None => match value.get(LEGACY_CONFIG_BLOB_KEY) {
Some(envelope) => envelope
.as_str()
.ok_or(CloudflareConfigEnvelopeError::NonString {
key: LEGACY_CONFIG_BLOB_KEY,
}),
None => Err(CloudflareConfigEnvelopeError::Missing {
primary_key: CONFIG_BLOB_KEY,
legacy_key: LEGACY_CONFIG_BLOB_KEY,
}),
},
}
}

/// Build the application state from explicit settings.
///
/// # Errors
Expand Down Expand Up @@ -614,3 +677,95 @@ fn build_router(state: &Arc<AppState>) -> RouterService {
router.build()
}
}

#[cfg(test)]
mod tests {
use super::*;

fn config_value(entries: &[(&str, &str)]) -> serde_json::Value {

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 — This helper hand-assembles a Value::Object that json! already builds, and the same file proves the macro accepts these const keys.

cloudflare_config_reports_malformed_legacy_value (line 754) writes serde_json::json!({ LEGACY_CONFIG_BLOB_KEY: false }), and cloudflare_config_reports_missing_keys uses json!({}). So three of the five tests in this module already use the macro and two go through config_value, for the same shape. CLAUDE.md's testing section asks for json! over raw construction, and the previous round removed exactly this pattern from tests/common/config.rs.

Proposed fix (apply manually — deleting the helper and rewriting its three call sites spans several non-contiguous ranges, so it cannot be one suggestion):

#[test]
fn cloudflare_config_prefers_manifest_default_key() {
    let value = serde_json::json!({
        LEGACY_CONFIG_BLOB_KEY: "legacy-envelope",
        CONFIG_BLOB_KEY: "manifest-envelope",
    });
    // …
}

#[test]
fn cloudflare_config_accepts_legacy_app_config_key() {
    let value = serde_json::json!({ LEGACY_CONFIG_BLOB_KEY: "legacy-envelope" });
    // …
}

#[test]
fn cloudflare_config_does_not_mask_malformed_manifest_value() {
    let value = serde_json::json!({
        LEGACY_CONFIG_BLOB_KEY: "legacy-envelope",
        CONFIG_BLOB_KEY: true,
    });
    // …
}

The third one also drops the let mut + index-assignment dance, which only existed to inject a non-string value into the map the helper could not express.

serde_json::Value::Object(
entries
.iter()
.map(|(key, value)| {
(
(*key).to_string(),
serde_json::Value::String((*value).to_string()),
)
})
.collect(),
)
}

#[test]
fn cloudflare_config_prefers_manifest_default_key() {
let value = config_value(&[
(LEGACY_CONFIG_BLOB_KEY, "legacy-envelope"),
(CONFIG_BLOB_KEY, "manifest-envelope"),
]);

assert_eq!(
cloudflare_config_envelope(&value),
Ok("manifest-envelope"),
"manifest-derived key should take precedence"
);
}

#[test]
fn cloudflare_config_accepts_legacy_app_config_key() {
let value = config_value(&[(LEGACY_CONFIG_BLOB_KEY, "legacy-envelope")]);

assert_eq!(
cloudflare_config_envelope(&value),
Ok("legacy-envelope"),
"legacy app_config key should remain compatible"
);
}

#[test]
fn cloudflare_config_reports_missing_keys() {
let value = serde_json::json!({});

assert_eq!(
cloudflare_config_envelope(&value),
Err(CloudflareConfigEnvelopeError::Missing {
primary_key: CONFIG_BLOB_KEY,
legacy_key: LEGACY_CONFIG_BLOB_KEY,
}),
"missing config should name both accepted keys"
);
}

#[test]
fn cloudflare_config_does_not_mask_malformed_manifest_value() {
Comment thread
ChristianPavilonis marked this conversation as resolved.
let mut value = config_value(&[(LEGACY_CONFIG_BLOB_KEY, "legacy-envelope")]);
value[CONFIG_BLOB_KEY] = serde_json::Value::Bool(true);

assert_eq!(
cloudflare_config_envelope(&value),
Err(CloudflareConfigEnvelopeError::NonString {
key: CONFIG_BLOB_KEY,
}),
"malformed manifest-derived value should not fall back"
);
}

#[test]
fn cloudflare_config_reports_malformed_legacy_value() {
let value = serde_json::json!({ LEGACY_CONFIG_BLOB_KEY: false });
let error = cloudflare_config_envelope(&value)
.expect_err("should reject a malformed legacy config value");

assert_eq!(
error,
CloudflareConfigEnvelopeError::NonString {
key: LEGACY_CONFIG_BLOB_KEY,
},
"malformed legacy value should name the legacy key"
);
assert_eq!(
error.configuration_message(),
"Cloudflare TRUSTED_SERVER_CONFIG value at `app_config` must be a string",
"configuration error should name the malformed legacy key"
);
}
}
6 changes: 3 additions & 3 deletions crates/trusted-server-adapter-cloudflare/wrangler.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ id = "REPLACE_WITH_YOUR_KV_NAMESPACE_ID"

[vars]
# TRUSTED_SERVER_CONFIG is required at startup. Replace this intentionally
# invalid placeholder with JSON containing an `app_config` blob envelope before
# deploying or running `wrangler dev` against real traffic.
TRUSTED_SERVER_CONFIG = '{"app_config":""}'
# invalid placeholder with JSON containing the manifest-default app-config blob
# envelope before deploying or running `wrangler dev` against real traffic.
TRUSTED_SERVER_CONFIG = '{"trusted_server_config":""}'
12 changes: 5 additions & 7 deletions crates/trusted-server-adapter-fastly/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use trusted_server_core::platform::PlatformGeo as _;
use trusted_server_core::platform::RuntimeServices;
use trusted_server_core::proxy::{AssetProxyCachePolicy, stream_asset_body};
use trusted_server_core::settings::Settings;
use trusted_server_core::settings_data::default_config_store_name;

mod app;
mod backend;
Expand All @@ -42,18 +43,15 @@ use crate::middleware::{HEADER_X_TS_FINALIZED, apply_finalize_headers, resolve_g
use crate::platform::{FastlyPlatformGeo, client_info_from_request};
use crate::rate_limiter::{FastlyRateLimiter, RATE_COUNTER_NAME};

const TRUSTED_SERVER_CONFIG_STORE: &str = "trusted_server_config";

/// Opens the Fastly Config Store used by the `EdgeZero` dispatcher.
/// Opens the manifest-default Fastly Config Store used by the `EdgeZero` dispatcher.
///
/// # Errors
///
/// Returns [`fastly::Error`] if the config store cannot be opened.
fn open_trusted_server_config_store() -> Result<ConfigStoreHandle, fastly::Error> {
let store = EdgeZeroFastlyConfigStore::try_open(TRUSTED_SERVER_CONFIG_STORE).map_err(|e| {
fastly::Error::msg(format!(
"failed to open config store `{TRUSTED_SERVER_CONFIG_STORE}`: {e}"
))
let store_name = default_config_store_name();
Comment thread
ChristianPavilonis marked this conversation as resolved.
let store = EdgeZeroFastlyConfigStore::try_open(store_name.as_ref()).map_err(|e| {
fastly::Error::msg(format!("failed to open config store `{store_name}`: {e}"))
})?;
Ok(ConfigStoreHandle::new(Arc::new(store)))
}
Expand Down
3 changes: 3 additions & 0 deletions crates/trusted-server-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ web-time = { workspace = true }
getrandom = { workspace = true, features = ["js"] }
uuid = { workspace = true, features = ["js"] }

[build-dependencies]
edgezero-core = { workspace = true }

[features]
default = []
# Exposes test-only constructors (e.g. `IntegrationRegistry::from_request_filters`)
Expand Down
33 changes: 33 additions & 0 deletions crates/trusted-server-core/build.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,36 @@
use std::env;
use std::path::PathBuf;

use edgezero_core::manifest::ManifestLoader;

fn main() {
println!("cargo:rerun-if-changed=build.rs");

// Keep every adapter's compiled default synchronized with the repository manifest.
let manifest_path = PathBuf::from(
env::var("CARGO_MANIFEST_DIR").expect("should receive CARGO_MANIFEST_DIR from Cargo"),
)
.join("../..")
.join("edgezero.toml");
println!("cargo:rerun-if-changed={}", manifest_path.display());

let manifest = match ManifestLoader::from_path(&manifest_path) {
Ok(manifest) => manifest,
Err(error) => {
println!(

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.

👍 praise — This build script fails closed on manifest problems and says exactly which file it could not use.

Two details make it worth calling out. The error path emits cargo::error= with manifest_path.display() interpolated, so a developer who moves or corrupts edgezero.toml gets the resolved absolute path rather than "build failed" — and the missing-[stores.config] case gets its own message instead of falling out as a parse error. Second, ManifestLoader::from_path runs the full manifest validation, not just a TOML parse: validate_store_declaration rejects empty ids, blank or control-character ids, ids outside [A-Za-z0-9_] or containing __, case-insensitive duplicates, and a default that is not one of the declared ids. So default_id() cannot hand back an empty or shell-hostile string, and DEFAULT_CONFIG_STORE_ID is safe to splice into EDGEZERO__STORES__CONFIG__<ID>__NAME without further checking.

The alternative — reading the manifest at runtime, or trusting a hand-copied constant — would have moved both failure modes to the edge. Catching them at compile time is the right trade for a value that can never change without a rebuild anyway.

"cargo::error=should load EdgeZero manifest at {}: {error}",
manifest_path.display()
);
std::process::exit(1);
}
};
let Some(config_store) = manifest.manifest().stores.config.as_ref() else {
println!(
"cargo::error=should declare [stores.config] in EdgeZero manifest at {}",
manifest_path.display()
);
std::process::exit(1);
};
let default_store_id = config_store.default_id();
println!("cargo:rustc-env=TRUSTED_SERVER_DEFAULT_CONFIG_STORE_ID={default_store_id}");
}
7 changes: 6 additions & 1 deletion crates/trusted-server-core/src/config_payload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,13 @@ use error_stack::Report;
use crate::error::TrustedServerError;
use crate::settings::Settings;

/// Default logical config-store id, from `[stores.config].default` in `edgezero.toml`.
///
Comment thread
ChristianPavilonis marked this conversation as resolved.
/// Derived at build time so every adapter uses the repository manifest's default.
pub const DEFAULT_CONFIG_STORE_ID: &str = env!("TRUSTED_SERVER_DEFAULT_CONFIG_STORE_ID");

/// Default config-store key containing the Trusted Server app-config blob.
pub const CONFIG_BLOB_KEY: &str = "trusted_server_config";
pub const CONFIG_BLOB_KEY: &str = DEFAULT_CONFIG_STORE_ID;
Comment thread
ChristianPavilonis marked this conversation as resolved.

/// Reconstruct validated [`Settings`] from a serialized config blob envelope.
///
Expand Down
Loading
Loading