From d8b267bbb4105f15a22aa4734648a83370794501 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Mon, 23 Mar 2026 14:03:16 +0530 Subject: [PATCH 1/5] Add config store read path and storage module split - Split fastly_storage.rs into storage/{config_store,secret_store,api_client,mod}.rs - Add PlatformConfigStore read path via FastlyPlatformConfigStore::get using ConfigStore::try_open/try_get - Add PlatformError::NotImplemented variant; stub write methods on FastlyPlatformConfigStore and FastlyPlatformSecretStore - Add StoreName/StoreId newtypes with From, From<&str>, AsRef - Add UnavailableKvStore to core platform module - Add RuntimeServicesBuilder replacing 7-arg constructor - Migrate get_active_jwks and handle_trusted_server_discovery to use &RuntimeServices - Update call sites in signing.rs, rotation.rs, main.rs - Add success-path test for handle_trusted_server_discovery using StubJwksConfigStore - Fix test_parse_cookies_to_jar_empty typo (was emtpy) --- .../trusted-server-adapter-fastly/src/main.rs | 2 +- .../src/platform.rs | 62 ++--- crates/trusted-server-core/src/lib.rs | 2 +- .../trusted-server-core/src/platform/error.rs | 3 + .../src/platform/traits.rs | 16 +- .../trusted-server-core/src/platform/types.rs | 4 +- .../src/request_signing/endpoints.rs | 170 ++++++++++++- .../src/request_signing/jwks.rs | 226 +++++++++++++++-- .../src/request_signing/rotation.rs | 2 +- .../src/request_signing/signing.rs | 2 +- .../api_client.rs} | 228 ++++-------------- .../src/storage/config_store.rs | 74 ++++++ crates/trusted-server-core/src/storage/mod.rs | 15 ++ .../src/storage/secret_store.rs | 107 ++++++++ 14 files changed, 655 insertions(+), 258 deletions(-) rename crates/trusted-server-core/src/{fastly_storage.rs => storage/api_client.rs} (52%) create mode 100644 crates/trusted-server-core/src/storage/config_store.rs create mode 100644 crates/trusted-server-core/src/storage/mod.rs create mode 100644 crates/trusted-server-core/src/storage/secret_store.rs diff --git a/crates/trusted-server-adapter-fastly/src/main.rs b/crates/trusted-server-adapter-fastly/src/main.rs index 226b9708..859ce117 100644 --- a/crates/trusted-server-adapter-fastly/src/main.rs +++ b/crates/trusted-server-adapter-fastly/src/main.rs @@ -129,7 +129,7 @@ async fn route_request( // Discovery endpoint for trusted-server capabilities and JWKS (Method::GET, "/.well-known/trusted-server.json") => { - handle_trusted_server_discovery(settings, req) + handle_trusted_server_discovery(settings, runtime_services, req) } // Signature verification endpoint diff --git a/crates/trusted-server-adapter-fastly/src/platform.rs b/crates/trusted-server-adapter-fastly/src/platform.rs index 5b864696..d06b7a37 100644 --- a/crates/trusted-server-adapter-fastly/src/platform.rs +++ b/crates/trusted-server-adapter-fastly/src/platform.rs @@ -15,7 +15,6 @@ use fastly::geo::geo_lookup; use fastly::{ConfigStore, Request, SecretStore}; use trusted_server_core::backend::BackendConfig; -use trusted_server_core::fastly_storage::FastlyApiClient; use trusted_server_core::geo::geo_from_fastly; use trusted_server_core::platform::{ ClientInfo, GeoInfo, PlatformBackend, PlatformBackendSpec, PlatformConfigStore, PlatformError, @@ -34,16 +33,10 @@ pub(crate) use trusted_server_core::platform::UnavailableKvStore; /// /// Stateless — the store name is supplied per call, matching the trait /// signature. This replaces the store-name-at-construction pattern of -/// [`trusted_server_core::fastly_storage::FastlyConfigStore`]. +/// [`trusted_server_core::storage::FastlyConfigStore`]. /// -/// # Write cost -/// -/// `put` and `delete` construct a [`FastlyApiClient`] on every call, which -/// opens the `"api-keys"` secret store to read the management API key. On -/// Fastly Compute, the SDK caches the open handle so repeated opens within a -/// single request are cheap. Callers that issue many writes in one request -/// should be aware that each call performs a synchronous outbound API -/// request to the Fastly management API. +/// Write methods (`put`, `delete`) are not yet implemented and return +/// [`PlatformError::NotImplemented`]. Management writes land in a follow-up PR. pub struct FastlyPlatformConfigStore; impl PlatformConfigStore for FastlyPlatformConfigStore { @@ -66,20 +59,17 @@ impl PlatformConfigStore for FastlyPlatformConfigStore { }) } - fn put(&self, store_id: &StoreId, key: &str, value: &str) -> Result<(), Report> { - FastlyApiClient::new() - .change_context(PlatformError::ConfigStore) - .attach("failed to initialize Fastly API client for config store write")? - .update_config_item(store_id.as_ref(), key, value) - .change_context(PlatformError::ConfigStore) + fn put( + &self, + _store_id: &StoreId, + _key: &str, + _value: &str, + ) -> Result<(), Report> { + Err(Report::new(PlatformError::NotImplemented)) } - fn delete(&self, store_id: &StoreId, key: &str) -> Result<(), Report> { - FastlyApiClient::new() - .change_context(PlatformError::ConfigStore) - .attach("failed to initialize Fastly API client for config store delete")? - .delete_config_item(store_id.as_ref(), key) - .change_context(PlatformError::ConfigStore) + fn delete(&self, _store_id: &StoreId, _key: &str) -> Result<(), Report> { + Err(Report::new(PlatformError::NotImplemented)) } } @@ -91,12 +81,10 @@ impl PlatformConfigStore for FastlyPlatformConfigStore { /// /// Stateless — the store name is supplied per call. This replaces the /// store-name-at-construction pattern of -/// [`trusted_server_core::fastly_storage::FastlySecretStore`]. -/// -/// # Write cost +/// [`trusted_server_core::storage::FastlySecretStore`]. /// -/// `create` and `delete` have the same per-call [`FastlyApiClient`] cost -/// described on [`FastlyPlatformConfigStore`]. +/// Write methods (`create`, `delete`) are not yet implemented and return +/// [`PlatformError::NotImplemented`]. Management writes land in a follow-up PR. pub struct FastlyPlatformSecretStore; impl PlatformSecretStore for FastlyPlatformSecretStore { @@ -132,23 +120,15 @@ impl PlatformSecretStore for FastlyPlatformSecretStore { fn create( &self, - store_id: &StoreId, - name: &str, - value: &str, + _store_id: &StoreId, + _name: &str, + _value: &str, ) -> Result<(), Report> { - FastlyApiClient::new() - .change_context(PlatformError::SecretStore) - .attach("failed to initialize Fastly API client for secret store create")? - .create_secret(store_id.as_ref(), name, value) - .change_context(PlatformError::SecretStore) + Err(Report::new(PlatformError::NotImplemented)) } - fn delete(&self, store_id: &StoreId, name: &str) -> Result<(), Report> { - FastlyApiClient::new() - .change_context(PlatformError::SecretStore) - .attach("failed to initialize Fastly API client for secret store delete")? - .delete_secret(store_id.as_ref(), name) - .change_context(PlatformError::SecretStore) + fn delete(&self, _store_id: &StoreId, _name: &str) -> Result<(), Report> { + Err(Report::new(PlatformError::NotImplemented)) } } diff --git a/crates/trusted-server-core/src/lib.rs b/crates/trusted-server-core/src/lib.rs index bc986beb..e09f7ef8 100644 --- a/crates/trusted-server-core/src/lib.rs +++ b/crates/trusted-server-core/src/lib.rs @@ -42,7 +42,6 @@ pub mod constants; pub mod cookies; pub mod creative; pub mod error; -pub mod fastly_storage; pub mod geo; pub(crate) mod host_rewrite; pub mod html_processor; @@ -58,6 +57,7 @@ pub mod request_signing; pub mod rsc_flight; pub mod settings; pub mod settings_data; +pub mod storage; pub mod streaming_processor; pub mod streaming_replacer; pub mod synthetic; diff --git a/crates/trusted-server-core/src/platform/error.rs b/crates/trusted-server-core/src/platform/error.rs index 699eb7d4..9619a562 100644 --- a/crates/trusted-server-core/src/platform/error.rs +++ b/crates/trusted-server-core/src/platform/error.rs @@ -26,6 +26,9 @@ pub enum PlatformError { /// Operation is not supported by this platform adapter. #[display("unsupported platform operation")] Unsupported, + /// Operation is defined by the trait but not yet implemented in this adapter. + #[display("not yet implemented")] + NotImplemented, } impl core::error::Error for PlatformError {} diff --git a/crates/trusted-server-core/src/platform/traits.rs b/crates/trusted-server-core/src/platform/traits.rs index ecc886c9..281b4236 100644 --- a/crates/trusted-server-core/src/platform/traits.rs +++ b/crates/trusted-server-core/src/platform/traits.rs @@ -23,7 +23,9 @@ pub trait PlatformConfigStore: Send + Sync { /// # Errors /// /// Returns [`PlatformError::ConfigStore`] when the write fails or the - /// platform management API is unreachable. + /// platform management API is unreachable. Returns + /// [`PlatformError::NotImplemented`] when the adapter has not yet + /// implemented write support. fn put(&self, store_id: &StoreId, key: &str, value: &str) -> Result<(), Report>; /// Delete a key from the management store identified by `store_id`. @@ -31,7 +33,9 @@ pub trait PlatformConfigStore: Send + Sync { /// # Errors /// /// Returns [`PlatformError::ConfigStore`] when the delete fails or the - /// platform management API is unreachable. + /// platform management API is unreachable. Returns + /// [`PlatformError::NotImplemented`] when the adapter has not yet + /// implemented write support. fn delete(&self, store_id: &StoreId, key: &str) -> Result<(), Report>; } @@ -75,7 +79,9 @@ pub trait PlatformSecretStore: Send + Sync { /// # Errors /// /// Returns [`PlatformError::SecretStore`] when the create fails or the - /// platform management API is unreachable. + /// platform management API is unreachable. Returns + /// [`PlatformError::NotImplemented`] when the adapter has not yet + /// implemented write support. fn create( &self, store_id: &StoreId, @@ -88,7 +94,9 @@ pub trait PlatformSecretStore: Send + Sync { /// # Errors /// /// Returns [`PlatformError::SecretStore`] when the delete fails or the - /// platform management API is unreachable. + /// platform management API is unreachable. Returns + /// [`PlatformError::NotImplemented`] when the adapter has not yet + /// implemented write support. fn delete(&self, store_id: &StoreId, name: &str) -> Result<(), Report>; } diff --git a/crates/trusted-server-core/src/platform/types.rs b/crates/trusted-server-core/src/platform/types.rs index e1ce4380..05b1c1b8 100644 --- a/crates/trusted-server-core/src/platform/types.rs +++ b/crates/trusted-server-core/src/platform/types.rs @@ -62,7 +62,7 @@ pub struct ClientInfo { /// accidentally passing a management API identifier where a runtime name is /// expected. #[derive(Debug, Clone, PartialEq, Eq, Hash, derive_more::Display)] -pub struct StoreName(pub String); +pub struct StoreName(String); impl From for StoreName { fn from(s: String) -> Self { @@ -89,7 +89,7 @@ impl AsRef for StoreName { /// accidentally passing a runtime store name where a management API /// identifier is expected. #[derive(Debug, Clone, PartialEq, Eq, Hash, derive_more::Display)] -pub struct StoreId(pub String); +pub struct StoreId(String); impl From for StoreId { fn from(s: String) -> Self { diff --git a/crates/trusted-server-core/src/request_signing/endpoints.rs b/crates/trusted-server-core/src/request_signing/endpoints.rs index 032a6fc5..c1974197 100644 --- a/crates/trusted-server-core/src/request_signing/endpoints.rs +++ b/crates/trusted-server-core/src/request_signing/endpoints.rs @@ -8,6 +8,7 @@ use fastly::{Request, Response}; use serde::{Deserialize, Serialize}; use crate::error::TrustedServerError; +use crate::platform::RuntimeServices; use crate::request_signing::discovery::TrustedServerDiscovery; use crate::request_signing::rotation::KeyRotationManager; use crate::request_signing::signing; @@ -24,12 +25,12 @@ use crate::settings::Settings; /// Returns an error if JWKS cannot be retrieved, parsed, or serialized. pub fn handle_trusted_server_discovery( _settings: &Settings, + services: &RuntimeServices, _req: Request, ) -> Result> { - // Get JWKS - let jwks_json = crate::request_signing::jwks::get_active_jwks().change_context( + let jwks_json = crate::request_signing::jwks::get_active_jwks(services).change_context( TrustedServerError::Configuration { - message: "Failed to retrieve JWKS".into(), + message: "failed to retrieve JWKS".into(), }, )?; @@ -282,7 +283,10 @@ pub fn handle_deactivate_key( match result { Ok(()) => { - let remaining_keys = manager.list_active_keys().unwrap_or_else(|_| vec![]); + let remaining_keys = manager.list_active_keys().unwrap_or_else(|e| { + log::warn!("failed to list active keys after deactivation: {}", e); + vec![] + }); let response = DeactivateKeyResponse { success: true, @@ -336,9 +340,133 @@ pub fn handle_deactivate_key( #[cfg(test)] mod tests { + use std::net::IpAddr; + use std::sync::Arc; + + use error_stack::Report; + + use crate::platform::{ + ClientInfo, GeoInfo, PlatformBackend, PlatformBackendSpec, PlatformConfigStore, + PlatformError, PlatformGeo, PlatformHttpClient, PlatformHttpRequest, + PlatformPendingRequest, PlatformResponse, PlatformSecretStore, PlatformSelectResult, + RuntimeServices, StoreId, StoreName, + }; + use super::*; use fastly::http::{Method, StatusCode}; + struct NoopConfigStore; + impl PlatformConfigStore for NoopConfigStore { + fn get(&self, _: &StoreName, _: &str) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + fn put(&self, _: &StoreId, _: &str, _: &str) -> Result<(), Report> { + Err(Report::new(PlatformError::Unsupported)) + } + fn delete(&self, _: &StoreId, _: &str) -> Result<(), Report> { + Err(Report::new(PlatformError::Unsupported)) + } + } + + struct NoopSecretStore; + impl PlatformSecretStore for NoopSecretStore { + fn get_bytes(&self, _: &StoreName, _: &str) -> Result, Report> { + Err(Report::new(PlatformError::Unsupported)) + } + fn create(&self, _: &StoreId, _: &str, _: &str) -> Result<(), Report> { + Err(Report::new(PlatformError::Unsupported)) + } + fn delete(&self, _: &StoreId, _: &str) -> Result<(), Report> { + Err(Report::new(PlatformError::Unsupported)) + } + } + + struct NoopBackend; + impl PlatformBackend for NoopBackend { + fn predict_name(&self, _: &PlatformBackendSpec) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + fn ensure(&self, _: &PlatformBackendSpec) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + } + + struct NoopHttpClient; + #[async_trait::async_trait(?Send)] + impl PlatformHttpClient for NoopHttpClient { + async fn send( + &self, + _: PlatformHttpRequest, + ) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + async fn send_async( + &self, + _: PlatformHttpRequest, + ) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + async fn select( + &self, + _: Vec, + ) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + } + + struct NoopGeo; + impl PlatformGeo for NoopGeo { + fn lookup(&self, _: Option) -> Result, Report> { + Ok(None) + } + } + + fn noop_services() -> RuntimeServices { + build_services_with_config(NoopConfigStore) + } + + /// Config store stub that returns a minimal JWKS with one Ed25519 key. + struct StubJwksConfigStore; + + impl PlatformConfigStore for StubJwksConfigStore { + fn get(&self, _store_name: &StoreName, key: &str) -> Result> { + match key { + "active-kids" => Ok("test-kid-1".to_string()), + "test-kid-1" => Ok( + r#"{"kty":"OKP","crv":"Ed25519","x":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","kid":"test-kid-1","alg":"EdDSA"}"# + .to_string(), + ), + _ => Err(Report::new(PlatformError::ConfigStore)), + } + } + + fn put(&self, _: &StoreId, _: &str, _: &str) -> Result<(), Report> { + Err(Report::new(PlatformError::Unsupported)) + } + + fn delete(&self, _: &StoreId, _: &str) -> Result<(), Report> { + Err(Report::new(PlatformError::Unsupported)) + } + } + + fn build_services_with_config( + config_store: impl PlatformConfigStore + 'static, + ) -> RuntimeServices { + RuntimeServices::builder() + .config_store(Arc::new(config_store)) + .secret_store(Arc::new(NoopSecretStore)) + .kv_store(Arc::new(edgezero_core::key_value_store::NoopKvStore)) + .backend(Arc::new(NoopBackend)) + .http_client(Arc::new(NoopHttpClient)) + .geo(Arc::new(NoopGeo)) + .client_info(ClientInfo { + client_ip: None, + tls_protocol: None, + tls_cipher: None, + }) + .build() + } + #[test] fn test_handle_verify_signature_valid() { let settings = crate::test_support::tests::create_test_settings(); @@ -580,7 +708,8 @@ mod tests { "https://test.com/.well-known/trusted-server.json", ); - let result = handle_trusted_server_discovery(&settings, req); + let services = noop_services(); + let result = handle_trusted_server_discovery(&settings, &services, req); match result { Ok(mut resp) => { assert_eq!(resp.get_status(), StatusCode::OK); @@ -601,4 +730,35 @@ mod tests { Err(e) => log::debug!("Expected error in test environment: {}", e), } } + + #[test] + fn test_handle_trusted_server_discovery_returns_jwks_document() { + let settings = crate::test_support::tests::create_test_settings(); + let req = Request::new( + Method::GET, + "https://test.com/.well-known/trusted-server.json", + ); + + let services = build_services_with_config(StubJwksConfigStore); + let mut resp = handle_trusted_server_discovery(&settings, &services, req) + .expect("should return discovery document when config store is populated"); + + assert_eq!(resp.get_status(), StatusCode::OK, "should return 200 OK"); + + let body = resp.take_body_str(); + let discovery: serde_json::Value = + serde_json::from_str(&body).expect("should parse discovery document as JSON"); + + assert_eq!(discovery["version"], "1.0", "should return version 1.0"); + + let keys = discovery["jwks"]["keys"] + .as_array() + .expect("should have jwks.keys array"); + assert_eq!(keys.len(), 1, "should contain exactly one key"); + assert_eq!( + keys[0]["kid"], "test-kid-1", + "should include the active key ID" + ); + assert_eq!(keys[0]["crv"], "Ed25519", "should be an Ed25519 key"); + } } diff --git a/crates/trusted-server-core/src/request_signing/jwks.rs b/crates/trusted-server-core/src/request_signing/jwks.rs index 24b86fb4..2df4ea38 100644 --- a/crates/trusted-server-core/src/request_signing/jwks.rs +++ b/crates/trusted-server-core/src/request_signing/jwks.rs @@ -12,15 +12,17 @@ use jose_jwk::{ use rand::rngs::OsRng; use crate::error::TrustedServerError; -use crate::fastly_storage::FastlyConfigStore; +use crate::platform::{RuntimeServices, StoreName}; use crate::request_signing::JWKS_CONFIG_STORE_NAME; +/// An Ed25519 keypair used for request signing. pub struct Keypair { pub signing_key: SigningKey, pub verifying_key: VerifyingKey, } impl Keypair { + /// Generate a new random Ed25519 keypair. #[must_use] pub fn generate() -> Self { let mut csprng = OsRng; @@ -34,6 +36,7 @@ impl Keypair { } } + /// Produce a public JWK from the verifying key, tagged with the given `kid`. #[must_use] pub fn get_jwk(&self, kid: String) -> Jwk { let public_key_bytes = self.verifying_key.as_bytes(); @@ -41,7 +44,7 @@ impl Keypair { let okp = Okp { crv: OkpCurves::Ed25519, x: public_key_bytes.to_vec().into(), - d: None, // No private key in JWK (public only) + d: None, }; Jwk { @@ -57,13 +60,23 @@ impl Keypair { /// Retrieves active JSON Web Keys from the config store. /// +/// Reads the `active-kids` entry from the platform config store, then fetches +/// each referenced JWK and assembles a JWKS JSON document. +/// /// # Errors /// -/// Returns an error if the config store cannot be accessed or if active keys cannot be retrieved. -pub fn get_active_jwks() -> Result> { - let store = FastlyConfigStore::new(JWKS_CONFIG_STORE_NAME); - let active_kids_str = store - .get("active-kids") +/// Returns [`TrustedServerError::Configuration`] if the config store is +/// unavailable, the `active-kids` key is missing, or any referenced JWK entry +/// cannot be read. The underlying [`crate::platform::PlatformError`] is +/// preserved as context in the error chain. +pub fn get_active_jwks(services: &RuntimeServices) -> Result> { + let store_name = StoreName::from(JWKS_CONFIG_STORE_NAME); + let active_kids_str = services + .config_store() + .get(&store_name, "active-kids") + .change_context(TrustedServerError::Configuration { + message: "failed to read active-kids from config store".into(), + }) .attach("while fetching active kids list")?; let active_kids: Vec<&str> = active_kids_str @@ -74,9 +87,12 @@ pub fn get_active_jwks() -> Result> { let mut jwks = Vec::new(); for kid in active_kids { - let jwk = store - .get(kid) - .attach(format!("Failed to get JWK for kid: {}", kid))?; + let jwk = services + .config_store() + .get(&store_name, kid) + .change_context(TrustedServerError::Configuration { + message: format!("failed to get JWK for kid: {}", kid), + })?; jwks.push(jwk); } @@ -86,41 +102,207 @@ pub fn get_active_jwks() -> Result> { #[cfg(test)] mod tests { - use super::*; + use std::net::IpAddr; + use std::sync::Arc; + use ed25519_dalek::{Signer, Verifier}; + use error_stack::Report; use jose_jwk::Key; + use crate::platform::{ + ClientInfo, GeoInfo, PlatformBackend, PlatformBackendSpec, PlatformConfigStore, + PlatformError, PlatformGeo, PlatformHttpClient, PlatformHttpRequest, + PlatformPendingRequest, PlatformResponse, PlatformSecretStore, PlatformSelectResult, + RuntimeServices, StoreId, StoreName, + }; + + use super::*; + + // --------------------------------------------------------------------------- + // Test doubles + // --------------------------------------------------------------------------- + + struct FailingConfigStore; + + impl PlatformConfigStore for FailingConfigStore { + fn get( + &self, + _store_name: &StoreName, + _key: &str, + ) -> Result> { + Err(Report::new(PlatformError::ConfigStore)) + } + + fn put( + &self, + _store_id: &StoreId, + _key: &str, + _value: &str, + ) -> Result<(), Report> { + Err(Report::new(PlatformError::Unsupported)) + } + + fn delete(&self, _store_id: &StoreId, _key: &str) -> Result<(), Report> { + Err(Report::new(PlatformError::Unsupported)) + } + } + + struct NoopSecretStore; + + impl PlatformSecretStore for NoopSecretStore { + fn get_bytes( + &self, + _store_name: &StoreName, + _key: &str, + ) -> Result, Report> { + Err(Report::new(PlatformError::Unsupported)) + } + + fn create( + &self, + _store_id: &StoreId, + _name: &str, + _value: &str, + ) -> Result<(), Report> { + Err(Report::new(PlatformError::Unsupported)) + } + + fn delete(&self, _store_id: &StoreId, _name: &str) -> Result<(), Report> { + Err(Report::new(PlatformError::Unsupported)) + } + } + + struct NoopBackend; + + impl PlatformBackend for NoopBackend { + fn predict_name( + &self, + _spec: &PlatformBackendSpec, + ) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + + fn ensure(&self, _spec: &PlatformBackendSpec) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + } + + struct NoopHttpClient; + + #[async_trait::async_trait(?Send)] + impl PlatformHttpClient for NoopHttpClient { + async fn send( + &self, + _request: PlatformHttpRequest, + ) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + + async fn send_async( + &self, + _request: PlatformHttpRequest, + ) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + + async fn select( + &self, + _pending_requests: Vec, + ) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + } + + struct NoopGeo; + + impl PlatformGeo for NoopGeo { + fn lookup( + &self, + _client_ip: Option, + ) -> Result, Report> { + Ok(None) + } + } + + fn build_services_with_config( + config_store: impl PlatformConfigStore + 'static, + ) -> RuntimeServices { + RuntimeServices::builder() + .config_store(Arc::new(config_store)) + .secret_store(Arc::new(NoopSecretStore)) + .kv_store(Arc::new(edgezero_core::key_value_store::NoopKvStore)) + .backend(Arc::new(NoopBackend)) + .http_client(Arc::new(NoopHttpClient)) + .geo(Arc::new(NoopGeo)) + .client_info(ClientInfo { + client_ip: None, + tls_protocol: None, + tls_cipher: None, + }) + .build() + } + + // --------------------------------------------------------------------------- + // Tests + // --------------------------------------------------------------------------- + #[test] - fn test_key_pair_generation() { + fn get_active_jwks_fails_with_configuration_error_when_store_unavailable() { + let services = build_services_with_config(FailingConfigStore); + let result = get_active_jwks(&services); + + assert!( + result.is_err(), + "should fail when config store is unavailable" + ); + let err = result.expect_err("should be an error"); + assert!( + err.contains::(), + "should surface as TrustedServerError" + ); + assert!( + err.contains::(), + "should preserve platform error context in the error chain" + ); + } + + #[test] + fn key_pair_generates_valid_signing_key() { let keypair = Keypair::generate(); let message = b"test message"; let signature = keypair.signing_key.sign(message); - assert!(keypair.verifying_key.verify(message, &signature).is_ok()); + assert!( + keypair.verifying_key.verify(message, &signature).is_ok(), + "should verify signature produced by generated key" + ); } #[test] - fn test_create_jwk_from_verifying_key() { + fn get_jwk_produces_correct_structure() { let jwk = Keypair::generate().get_jwk("test-kid".to_string()); - // Verify JWK structure - assert_eq!(jwk.prm.kid, Some("test-kid".to_string())); + assert_eq!( + jwk.prm.kid, + Some("test-kid".to_string()), + "should set kid parameter" + ); assert_eq!( jwk.prm.alg, Some(jose_jwk::jose_jwa::Algorithm::Signing( jose_jwk::jose_jwa::Signing::EdDsa - )) + )), + "should set EdDSA algorithm" ); - // Verify it's an OKP key with Ed25519 curve match jwk.key { Key::Okp(okp) => { - assert_eq!(okp.crv, OkpCurves::Ed25519); - assert_eq!(okp.x.len(), 32); // Ed25519 public keys are 32 bytes - assert!(okp.d.is_none()); // No private key component + assert_eq!(okp.crv, OkpCurves::Ed25519, "should use Ed25519 curve"); + assert_eq!(okp.x.len(), 32, "should be 32-byte Ed25519 public key"); + assert!(okp.d.is_none(), "should have no private key component"); } - _ => panic!("Expected OKP key type"), + _ => panic!("should be OKP key type"), } } } diff --git a/crates/trusted-server-core/src/request_signing/rotation.rs b/crates/trusted-server-core/src/request_signing/rotation.rs index da3abd9e..e2fd8bd5 100644 --- a/crates/trusted-server-core/src/request_signing/rotation.rs +++ b/crates/trusted-server-core/src/request_signing/rotation.rs @@ -9,8 +9,8 @@ use error_stack::{Report, ResultExt}; use jose_jwk::Jwk; use crate::error::TrustedServerError; -use crate::fastly_storage::{FastlyApiClient, FastlyConfigStore}; use crate::request_signing::JWKS_CONFIG_STORE_NAME; +use crate::storage::{FastlyApiClient, FastlyConfigStore}; use super::Keypair; diff --git a/crates/trusted-server-core/src/request_signing/signing.rs b/crates/trusted-server-core/src/request_signing/signing.rs index c256da02..bed25084 100644 --- a/crates/trusted-server-core/src/request_signing/signing.rs +++ b/crates/trusted-server-core/src/request_signing/signing.rs @@ -9,8 +9,8 @@ use error_stack::{Report, ResultExt}; use serde::Serialize; use crate::error::TrustedServerError; -use crate::fastly_storage::{FastlyConfigStore, FastlySecretStore}; use crate::request_signing::{JWKS_CONFIG_STORE_NAME, SIGNING_SECRET_STORE_NAME}; +use crate::storage::{FastlyConfigStore, FastlySecretStore}; /// Retrieves the current active key ID from the config store. /// diff --git a/crates/trusted-server-core/src/fastly_storage.rs b/crates/trusted-server-core/src/storage/api_client.rs similarity index 52% rename from crates/trusted-server-core/src/fastly_storage.rs rename to crates/trusted-server-core/src/storage/api_client.rs index 6af3d6b4..70f044b0 100644 --- a/crates/trusted-server-core/src/fastly_storage.rs +++ b/crates/trusted-server-core/src/storage/api_client.rs @@ -1,116 +1,33 @@ +//! Fastly management API client (legacy). +//! +//! This module holds [`FastlyApiClient`], which wraps the Fastly management +//! REST API for write operations on config and secret stores. +//! New code should use [`crate::platform::PlatformConfigStore`] and +//! [`crate::platform::PlatformSecretStore`] write methods instead. +//! This type will be removed once all call sites have migrated. + use std::io::Read; use error_stack::{Report, ResultExt}; -use fastly::{ConfigStore, Request, Response, SecretStore}; +use fastly::{Request, Response}; use http::StatusCode; use crate::backend::BackendConfig; use crate::error::TrustedServerError; +use crate::storage::secret_store::FastlySecretStore; const FASTLY_API_HOST: &str = "https://api.fastly.com"; -/// Fastly-backed config store with the store name baked in at construction. +/// HTTP client for the Fastly management API. /// -/// # Migration note -/// -/// This type predates the `platform` abstraction. New code should use -/// [`crate::platform::PlatformConfigStore`] via [`crate::platform::RuntimeServices`] -/// instead. `FastlyConfigStore` will be removed once all call sites have -/// migrated. -pub struct FastlyConfigStore { - store_name: String, -} - -impl FastlyConfigStore { - pub fn new(store_name: impl Into) -> Self { - Self { - store_name: store_name.into(), - } - } - - /// Retrieves a configuration value from the store. - /// - /// # Errors - /// - /// Returns an error if the key is not found in the config store. - pub fn get(&self, key: &str) -> Result> { - // TODO use try_open and return the error - let store = ConfigStore::open(&self.store_name); - store.get(key).ok_or_else(|| { - Report::new(TrustedServerError::Configuration { - message: format!( - "Key '{}' not found in config store '{}'", - key, self.store_name - ), - }) - }) - } -} - -/// Fastly-backed secret store with the store name baked in at construction. +/// Used to perform write operations on config and secret stores via the +/// Fastly REST API. Reads are performed directly through the edge-side SDK. /// /// # Migration note /// /// This type predates the `platform` abstraction. New code should use -/// [`crate::platform::PlatformSecretStore`] via [`crate::platform::RuntimeServices`] -/// instead. `FastlySecretStore` will be removed once all call sites have -/// migrated. -pub struct FastlySecretStore { - store_name: String, -} - -impl FastlySecretStore { - pub fn new(store_name: impl Into) -> Self { - Self { - store_name: store_name.into(), - } - } - - /// Retrieves a secret value from the store. - /// - /// # Errors - /// - /// Returns an error if the secret store cannot be opened, the key is not found, - /// or the secret plaintext cannot be retrieved. - pub fn get(&self, key: &str) -> Result, Report> { - let store = SecretStore::open(&self.store_name).map_err(|_| { - Report::new(TrustedServerError::Configuration { - message: format!("Failed to open SecretStore '{}'", self.store_name), - }) - })?; - - let secret = store.get(key).ok_or_else(|| { - Report::new(TrustedServerError::Configuration { - message: format!( - "Secret '{}' not found in secret store '{}'", - key, self.store_name - ), - }) - })?; - - secret - .try_plaintext() - .map_err(|_| { - Report::new(TrustedServerError::Configuration { - message: "Failed to get secret plaintext".into(), - }) - }) - .map(|bytes| bytes.into_iter().collect()) - } - - /// Retrieves a secret value from the store and decodes it as a UTF-8 string. - /// - /// # Errors - /// - /// Returns an error if the secret cannot be retrieved or is not valid UTF-8. - pub fn get_string(&self, key: &str) -> Result> { - let bytes = self.get(key)?; - String::from_utf8(bytes).change_context(TrustedServerError::Configuration { - message: "Failed to decode secret as UTF-8".to_string(), - }) - } -} - +/// [`crate::platform::PlatformConfigStore`] and +/// [`crate::platform::PlatformSecretStore`] write methods instead. pub struct FastlyApiClient { api_key: Vec, base_url: &'static str, @@ -122,24 +39,25 @@ impl FastlyApiClient { /// /// # Errors /// - /// Returns an error if the secret store cannot be opened or the API key cannot be retrieved. + /// Returns an error if the secret store cannot be opened or the API key + /// cannot be retrieved. pub fn new() -> Result> { Self::from_secret_store("api-keys", "api_key") } - /// Creates a new Fastly API client from a specified secret store. + /// Creates a new Fastly API client reading credentials from a specified + /// secret store entry. /// /// # Errors /// - /// Returns an error if the API backend cannot be ensured or the API key cannot be retrieved. + /// Returns an error if the API backend cannot be ensured or the API key + /// cannot be retrieved. pub fn from_secret_store( store_name: &str, key_name: &str, ) -> Result> { let backend_name = BackendConfig::from_url("https://api.fastly.com", true)?; - - let secret_store = FastlySecretStore::new(store_name); - let api_key = secret_store.get(key_name)?; + let api_key = FastlySecretStore::new(store_name).get(key_name)?; log::debug!("FastlyApiClient initialized with backend: {}", backend_name); @@ -158,7 +76,6 @@ impl FastlyApiClient { content_type: &str, ) -> Result> { let url = format!("{}{}", self.base_url, path); - let api_key_str = String::from_utf8_lossy(&self.api_key).to_string(); let mut request = match method { @@ -168,7 +85,7 @@ impl FastlyApiClient { "DELETE" => Request::delete(&url), _ => { return Err(Report::new(TrustedServerError::Configuration { - message: format!("Unsupported HTTP method: {}", method), + message: format!("unsupported HTTP method: {}", method), })) } }; @@ -185,7 +102,7 @@ impl FastlyApiClient { request.send(&self.backend_name).map_err(|e| { Report::new(TrustedServerError::Configuration { - message: format!("Failed to send API request: {}", e), + message: format!("failed to send API request: {}", e), }) }) } @@ -215,10 +132,8 @@ impl FastlyApiClient { response .get_body_mut() .read_to_string(&mut buf) - .map_err(|e| { - Report::new(TrustedServerError::Configuration { - message: format!("Failed to read API response: {}", e), - }) + .change_context(TrustedServerError::Configuration { + message: "failed to read config store API response".into(), })?; if response.get_status() == StatusCode::OK { @@ -226,7 +141,7 @@ impl FastlyApiClient { } else { Err(Report::new(TrustedServerError::Configuration { message: format!( - "Failed to update config item: HTTP {} - {}", + "failed to update config item: HTTP {} - {}", response.get_status(), buf ), @@ -246,7 +161,6 @@ impl FastlyApiClient { secret_value: &str, ) -> Result<(), Report> { let path = format!("/resources/stores/secret/{}/secrets", store_id); - let payload = serde_json::json!({ "name": secret_name, "secret": secret_value @@ -259,10 +173,8 @@ impl FastlyApiClient { response .get_body_mut() .read_to_string(&mut buf) - .map_err(|e| { - Report::new(TrustedServerError::Configuration { - message: format!("Failed to read API response: {}", e), - }) + .change_context(TrustedServerError::Configuration { + message: "failed to read secret store API response".into(), })?; if response.get_status() == StatusCode::OK { @@ -270,7 +182,7 @@ impl FastlyApiClient { } else { Err(Report::new(TrustedServerError::Configuration { message: format!( - "Failed to create secret: HTTP {} - {}", + "failed to create secret: HTTP {} - {}", response.get_status(), buf ), @@ -282,7 +194,8 @@ impl FastlyApiClient { /// /// # Errors /// - /// Returns an error if the API request fails or returns a non-OK/NO_CONTENT status. + /// Returns an error if the API request fails or returns a non-OK or + /// non-NO_CONTENT status. pub fn delete_config_item( &self, store_id: &str, @@ -296,10 +209,8 @@ impl FastlyApiClient { response .get_body_mut() .read_to_string(&mut buf) - .map_err(|e| { - Report::new(TrustedServerError::Configuration { - message: format!("Failed to read API response: {}", e), - }) + .change_context(TrustedServerError::Configuration { + message: "failed to read config store delete API response".into(), })?; if response.get_status() == StatusCode::OK @@ -309,7 +220,7 @@ impl FastlyApiClient { } else { Err(Report::new(TrustedServerError::Configuration { message: format!( - "Failed to delete config item: HTTP {} - {}", + "failed to delete config item: HTTP {} - {}", response.get_status(), buf ), @@ -321,7 +232,8 @@ impl FastlyApiClient { /// /// # Errors /// - /// Returns an error if the API request fails or returns a non-OK/NO_CONTENT status. + /// Returns an error if the API request fails or returns a non-OK or + /// non-NO_CONTENT status. pub fn delete_secret( &self, store_id: &str, @@ -338,10 +250,8 @@ impl FastlyApiClient { response .get_body_mut() .read_to_string(&mut buf) - .map_err(|e| { - Report::new(TrustedServerError::Configuration { - message: format!("Failed to read API response: {}", e), - }) + .change_context(TrustedServerError::Configuration { + message: "failed to read secret store delete API response".into(), })?; if response.get_status() == StatusCode::OK @@ -351,7 +261,7 @@ impl FastlyApiClient { } else { Err(Report::new(TrustedServerError::Configuration { message: format!( - "Failed to delete secret: HTTP {} - {}", + "failed to delete secret: HTTP {} - {}", response.get_status(), buf ), @@ -365,71 +275,29 @@ mod tests { use super::*; #[test] - fn test_config_store_new() { - let store = FastlyConfigStore::new("test_store"); - assert_eq!(store.store_name, "test_store"); - } - - #[test] - fn test_secret_store_new() { - let store = FastlySecretStore::new("test_secrets"); - assert_eq!(store.store_name, "test_secrets"); - } - - #[test] - fn test_config_store_get() { - let store = FastlyConfigStore::new("jwks_store"); - let result = store.get("current-kid"); - match result { - Ok(kid) => println!("Current KID: {}", kid), - Err(e) => println!("Expected error in test environment: {}", e), - } - } - - #[test] - fn test_secret_store_get() { - let store = FastlySecretStore::new("signing_keys"); - let config_store = FastlyConfigStore::new("jwks_store"); - - match config_store.get("current-kid") { - Ok(kid) => match store.get(&kid) { - Ok(bytes) => { - println!("Successfully loaded secret, {} bytes", bytes.len()); - assert!(!bytes.is_empty()); - } - Err(e) => println!("Error loading secret: {}", e), - }, - Err(e) => println!("Error getting current kid: {}", e), - } - } - - #[test] - fn test_api_client_creation() { + fn api_client_creation_in_test_environment() { let result = FastlyApiClient::new(); match result { - Ok(_client) => println!("Successfully created API client"), + Ok(_) => println!("Successfully created API client"), Err(e) => println!("Expected error in test environment: {}", e), } } - // Other tests logic is preserved, prints error which is now a Report #[test] - fn test_update_config_item() { - let result = FastlyApiClient::new(); - if let Ok(client) = result { + fn api_client_update_config_item_in_test_environment() { + if let Ok(client) = FastlyApiClient::new() { let result = client.update_config_item("5WNlRjznCUAGTU0QeYU8x2", "test-key", "test-value"); match result { Ok(()) => println!("Successfully updated config item"), - Err(e) => println!("Failed to update config item: {}", e), + Err(e) => println!("Expected error in test environment: {}", e), } } } #[test] - fn test_create_secret() { - let result = FastlyApiClient::new(); - if let Ok(client) = result { + fn api_client_create_secret_in_test_environment() { + if let Ok(client) = FastlyApiClient::new() { let result = client.create_secret( "Ltf3CkSGV0Yn2PIC2lDcZx", "test-secret-new", @@ -437,7 +305,7 @@ mod tests { ); match result { Ok(()) => println!("Successfully created secret"), - Err(e) => println!("Failed to create secret: {}", e), + Err(e) => println!("Expected error in test environment: {}", e), } } } diff --git a/crates/trusted-server-core/src/storage/config_store.rs b/crates/trusted-server-core/src/storage/config_store.rs new file mode 100644 index 00000000..95055128 --- /dev/null +++ b/crates/trusted-server-core/src/storage/config_store.rs @@ -0,0 +1,74 @@ +//! Fastly-backed config store (legacy). +//! +//! This module holds the pre-platform [`FastlyConfigStore`] type. +//! New code should use [`crate::platform::PlatformConfigStore`] via +//! [`crate::platform::RuntimeServices`] instead. This type will be removed +//! once all call sites have migrated. + +use error_stack::Report; +use fastly::ConfigStore; + +use crate::error::TrustedServerError; + +/// Fastly-backed config store with the store name baked in at construction. +/// +/// # Migration note +/// +/// This type predates the `platform` abstraction. New code should use +/// [`crate::platform::PlatformConfigStore`] via [`crate::platform::RuntimeServices`] +/// instead. `FastlyConfigStore` will be removed once all call sites have +/// migrated. +pub struct FastlyConfigStore { + store_name: String, +} + +impl FastlyConfigStore { + /// Create a new config store handle for the named store. + pub fn new(store_name: impl Into) -> Self { + Self { + store_name: store_name.into(), + } + } + + /// Retrieves a configuration value from the store. + /// + /// # Errors + /// + /// Returns an error if the key is not found in the config store. + pub fn get(&self, key: &str) -> Result> { + // TODO(pr3): replace ConfigStore::open with try_open when all callers migrate + let store = ConfigStore::open(&self.store_name); + store.get(key).ok_or_else(|| { + Report::new(TrustedServerError::Configuration { + message: format!( + "key '{}' not found in config store '{}'", + key, self.store_name + ), + }) + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn config_store_new_stores_name() { + let store = FastlyConfigStore::new("test_store"); + assert_eq!( + store.store_name, "test_store", + "should store the store name" + ); + } + + #[test] + fn config_store_get_in_test_environment() { + let store = FastlyConfigStore::new("jwks_store"); + let result = store.get("current-kid"); + match result { + Ok(kid) => println!("Current KID: {}", kid), + Err(e) => println!("Expected error in test environment: {}", e), + } + } +} diff --git a/crates/trusted-server-core/src/storage/mod.rs b/crates/trusted-server-core/src/storage/mod.rs new file mode 100644 index 00000000..6010ae2e --- /dev/null +++ b/crates/trusted-server-core/src/storage/mod.rs @@ -0,0 +1,15 @@ +//! Legacy Fastly-backed store types. +//! +//! These types predate the [`crate::platform`] abstraction and will be removed +//! once all call sites have migrated to the platform traits. New code should +//! use [`crate::platform::PlatformConfigStore`], +//! [`crate::platform::PlatformSecretStore`], and the management write methods +//! via [`crate::platform::RuntimeServices`]. + +pub(crate) mod api_client; +pub(crate) mod config_store; +pub(crate) mod secret_store; + +pub use api_client::FastlyApiClient; +pub use config_store::FastlyConfigStore; +pub use secret_store::FastlySecretStore; diff --git a/crates/trusted-server-core/src/storage/secret_store.rs b/crates/trusted-server-core/src/storage/secret_store.rs new file mode 100644 index 00000000..490a3351 --- /dev/null +++ b/crates/trusted-server-core/src/storage/secret_store.rs @@ -0,0 +1,107 @@ +//! Fastly-backed secret store (legacy). +//! +//! This module holds the pre-platform [`FastlySecretStore`] type. +//! New code should use [`crate::platform::PlatformSecretStore`] via +//! [`crate::platform::RuntimeServices`] instead. This type will be removed +//! once all call sites have migrated. + +use error_stack::{Report, ResultExt}; +use fastly::SecretStore; + +use crate::error::TrustedServerError; + +/// Fastly-backed secret store with the store name baked in at construction. +/// +/// # Migration note +/// +/// This type predates the `platform` abstraction. New code should use +/// [`crate::platform::PlatformSecretStore`] via [`crate::platform::RuntimeServices`] +/// instead. `FastlySecretStore` will be removed once all call sites have +/// migrated. +pub struct FastlySecretStore { + store_name: String, +} + +impl FastlySecretStore { + /// Create a new secret store handle for the named store. + pub fn new(store_name: impl Into) -> Self { + Self { + store_name: store_name.into(), + } + } + + /// Retrieves a secret value as raw bytes from the store. + /// + /// # Errors + /// + /// Returns an error if the secret store cannot be opened, the key is not + /// found, or the plaintext cannot be retrieved. + pub fn get(&self, key: &str) -> Result, Report> { + let store = SecretStore::open(&self.store_name).map_err(|_| { + Report::new(TrustedServerError::Configuration { + message: format!("failed to open secret store '{}'", self.store_name), + }) + })?; + + let secret = store.get(key).ok_or_else(|| { + Report::new(TrustedServerError::Configuration { + message: format!( + "secret '{}' not found in secret store '{}'", + key, self.store_name + ), + }) + })?; + + secret + .try_plaintext() + .map_err(|_| { + Report::new(TrustedServerError::Configuration { + message: "failed to retrieve secret plaintext".into(), + }) + }) + .map(|bytes| bytes.into_iter().collect()) + } + + /// Retrieves a secret value from the store and decodes it as a UTF-8 string. + /// + /// # Errors + /// + /// Returns an error if the secret cannot be retrieved or is not valid UTF-8. + pub fn get_string(&self, key: &str) -> Result> { + let bytes = self.get(key)?; + String::from_utf8(bytes).change_context(TrustedServerError::Configuration { + message: "failed to decode secret as UTF-8".to_string(), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::storage::FastlyConfigStore; + + #[test] + fn secret_store_new_stores_name() { + let store = FastlySecretStore::new("test_secrets"); + assert_eq!( + store.store_name, "test_secrets", + "should store the store name" + ); + } + + #[test] + fn secret_store_get_in_test_environment() { + let store = FastlySecretStore::new("signing_keys"); + let config_store = FastlyConfigStore::new("jwks_store"); + + match config_store.get("current-kid") { + Ok(kid) => match store.get(&kid) { + Ok(bytes) => { + assert!(!bytes.is_empty(), "should have non-empty secret bytes"); + } + Err(e) => println!("Expected error in test environment: {}", e), + }, + Err(e) => println!("Expected error in test environment: {}", e), + } + } +} From a8c5648a53fdaa3a32c425bf800a3dda0042ceab Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Tue, 24 Mar 2026 21:16:44 +0530 Subject: [PATCH 2/5] Harden legacy config-store reads and align Fastly adapter stubs --- .../src/platform.rs | 241 +++++++++++++++--- .../src/storage/config_store.rs | 112 +++++++- 2 files changed, 302 insertions(+), 51 deletions(-) diff --git a/crates/trusted-server-adapter-fastly/src/platform.rs b/crates/trusted-server-adapter-fastly/src/platform.rs index d06b7a37..eaf5cf8c 100644 --- a/crates/trusted-server-adapter-fastly/src/platform.rs +++ b/crates/trusted-server-adapter-fastly/src/platform.rs @@ -5,6 +5,7 @@ //! constructs a [`RuntimeServices`] instance once at the entry point from the //! incoming Fastly request. +use core::fmt::Display; use std::net::IpAddr; use std::sync::Arc; @@ -25,6 +26,114 @@ use trusted_server_core::platform::{ pub(crate) use trusted_server_core::platform::UnavailableKvStore; +trait ConfigStoreReader: Sized { + type LookupError: Display; + + fn try_get(&self, key: &str) -> Result, Self::LookupError>; +} + +impl ConfigStoreReader for ConfigStore { + type LookupError = fastly::config_store::LookupError; + + fn try_get(&self, key: &str) -> Result, Self::LookupError> { + ConfigStore::try_get(self, key) + } +} + +fn get_config_value( + store_name: &str, + key: &str, + open_store: Open, +) -> Result> +where + S: ConfigStoreReader, + Open: FnOnce() -> Result, + OpenError: Display, +{ + let store = open_store().map_err(|error| { + Report::new(PlatformError::ConfigStore).attach(format!( + "failed to open config store '{store_name}': {error}" + )) + })?; + + store + .try_get(key) + .map_err(|error| { + Report::new(PlatformError::ConfigStore).attach(format!( + "lookup for key '{key}' in config store '{store_name}' failed: {error}" + )) + })? + .ok_or_else(|| { + Report::new(PlatformError::ConfigStore).attach(format!( + "key '{key}' not found in config store '{store_name}'" + )) + }) +} + +enum SecretReadError { + Lookup(LookupError), + Decrypt(DecryptError), +} + +type SecretBytesResult = + Result>, SecretReadError>; + +trait SecretStoreReader: Sized { + type LookupError: Display; + type DecryptError: Display; + + fn try_get_bytes(&self, key: &str) -> SecretBytesResult; +} + +impl SecretStoreReader for SecretStore { + type LookupError = fastly::secret_store::LookupError; + type DecryptError = fastly::secret_store::DecryptError; + + fn try_get_bytes(&self, key: &str) -> SecretBytesResult { + let secret = self.try_get(key).map_err(SecretReadError::Lookup)?; + let Some(secret) = secret else { + return Ok(None); + }; + + secret + .try_plaintext() + .map(|bytes| Some(bytes.into_iter().collect())) + .map_err(SecretReadError::Decrypt) + } +} + +fn get_secret_bytes( + store_name: &str, + key: &str, + open_store: Open, +) -> Result, Report> +where + S: SecretStoreReader, + Open: FnOnce() -> Result, + OpenError: Display, +{ + let store = open_store().map_err(|error| { + Report::new(PlatformError::SecretStore).attach(format!( + "failed to open secret store '{store_name}': {error}" + )) + })?; + + store + .try_get_bytes(key) + .map_err(|error| match error { + SecretReadError::Lookup(error) => Report::new(PlatformError::SecretStore).attach( + format!("lookup for key '{key}' in secret store '{store_name}' failed: {error}"), + ), + SecretReadError::Decrypt(error) => Report::new(PlatformError::SecretStore) + .attach(format!("failed to decrypt secret '{key}': {error}")), + })? + .ok_or_else(|| { + Report::new(PlatformError::SecretStore).attach(format!( + "key '{key}' not found in secret store '{store_name}'" + )) + }) +} + // --------------------------------------------------------------------------- // FastlyPlatformConfigStore // --------------------------------------------------------------------------- @@ -42,21 +151,7 @@ pub struct FastlyPlatformConfigStore; impl PlatformConfigStore for FastlyPlatformConfigStore { fn get(&self, store_name: &StoreName, key: &str) -> Result> { let name = store_name.as_ref(); - let store = ConfigStore::try_open(name).map_err(|e| { - Report::new(PlatformError::ConfigStore) - .attach(format!("failed to open config store '{name}': {e}")) - })?; - store - .try_get(key) - .map_err(|e| { - Report::new(PlatformError::ConfigStore).attach(format!( - "lookup for key '{key}' in config store '{name}' failed: {e}" - )) - })? - .ok_or_else(|| { - Report::new(PlatformError::ConfigStore) - .attach(format!("key '{key}' not found in config store '{name}'")) - }) + get_config_value::(name, key, || ConfigStore::try_open(name)) } fn put( @@ -94,28 +189,7 @@ impl PlatformSecretStore for FastlyPlatformSecretStore { key: &str, ) -> Result, Report> { let name = store_name.as_ref(); - let store = SecretStore::open(name).map_err(|e| { - Report::new(PlatformError::SecretStore) - .attach(format!("failed to open secret store '{name}': {e}")) - })?; - let secret = store - .try_get(key) - .map_err(|e| { - Report::new(PlatformError::SecretStore).attach(format!( - "lookup for key '{key}' in secret store '{name}' failed: {e}" - )) - })? - .ok_or_else(|| { - Report::new(PlatformError::SecretStore) - .attach(format!("key '{key}' not found in secret store '{name}'")) - })?; - secret - .try_plaintext() - .map(|bytes| bytes.into_iter().collect()) - .map_err(|e| { - Report::new(PlatformError::SecretStore) - .attach(format!("failed to decrypt secret '{key}': {e}")) - }) + get_secret_bytes::(name, key, || SecretStore::open(name)) } fn create( @@ -171,7 +245,7 @@ impl PlatformBackend for FastlyPlatformBackend { /// /// The Fastly-backed `send` / `send_async` / `select` behavior lands in a /// follow-up PR once the orchestrator migration is complete. Until then all -/// methods return [`PlatformError::Unsupported`]. +/// methods return [`PlatformError::NotImplemented`]. /// /// Implementation lands in #487 (PR 6: Backend + HTTP client traits). pub struct FastlyPlatformHttpClient; @@ -182,7 +256,7 @@ impl PlatformHttpClient for FastlyPlatformHttpClient { &self, _request: PlatformHttpRequest, ) -> Result> { - Err(Report::new(PlatformError::Unsupported) + Err(Report::new(PlatformError::NotImplemented) .attach("FastlyPlatformHttpClient::send is not yet implemented")) } @@ -190,7 +264,7 @@ impl PlatformHttpClient for FastlyPlatformHttpClient { &self, _request: PlatformHttpRequest, ) -> Result> { - Err(Report::new(PlatformError::Unsupported) + Err(Report::new(PlatformError::NotImplemented) .attach("FastlyPlatformHttpClient::send_async is not yet implemented")) } @@ -198,7 +272,7 @@ impl PlatformHttpClient for FastlyPlatformHttpClient { &self, _pending_requests: Vec, ) -> Result> { - Err(Report::new(PlatformError::Unsupported) + Err(Report::new(PlatformError::NotImplemented) .attach("FastlyPlatformHttpClient::select is not yet implemented")) } } @@ -273,10 +347,48 @@ mod tests { use std::sync::Arc; use std::time::Duration; + use edgezero_core::body::Body; + use edgezero_core::http::request_builder; use edgezero_core::key_value_store::NoopKvStore; use super::*; + struct StubConfigStore { + value: Result, &'static str>, + } + + impl ConfigStoreReader for StubConfigStore { + type LookupError = &'static str; + + fn try_get(&self, _key: &str) -> Result, Self::LookupError> { + self.value.clone() + } + } + + enum StubSecretReadError { + Decrypt(&'static str), + } + + struct StubSecretStore { + value: Result>, StubSecretReadError>, + } + + impl SecretStoreReader for StubSecretStore { + type LookupError = &'static str; + type DecryptError = &'static str; + + fn try_get_bytes( + &self, + _key: &str, + ) -> SecretBytesResult { + match &self.value { + Ok(Some(bytes)) => Ok(Some(bytes.clone())), + Ok(None) => Ok(None), + Err(StubSecretReadError::Decrypt(error)) => Err(SecretReadError::Decrypt(*error)), + } + } + } + fn noop_kv_store() -> Arc { Arc::new(NoopKvStore) } @@ -390,4 +502,51 @@ mod tests { "should preserve client_ip through clone" ); } + + #[test] + fn get_config_value_returns_error_when_lookup_fails() { + let err = get_config_value::("jwks_store", "active-kids", || { + Ok::(StubConfigStore { + value: Err("lookup failed"), + }) + }) + .expect_err("should return an error when config lookup fails"); + + assert!( + matches!(err.current_context(), &PlatformError::ConfigStore), + "should surface as PlatformError::ConfigStore" + ); + } + + #[test] + fn get_secret_bytes_returns_error_when_decrypt_fails() { + let err = get_secret_bytes::("signing_keys", "kid", || { + Ok::(StubSecretStore { + value: Err(StubSecretReadError::Decrypt("decrypt failed")), + }) + }) + .expect_err("should return an error when secret decryption fails"); + + assert!( + matches!(err.current_context(), &PlatformError::SecretStore), + "should surface as PlatformError::SecretStore" + ); + } + + #[test] + fn fastly_platform_http_client_reports_not_implemented() { + let client = FastlyPlatformHttpClient; + let request = request_builder() + .uri("https://example.com/") + .body(Body::empty()) + .expect("should build test request"); + let err = + futures::executor::block_on(client.send(PlatformHttpRequest::new(request, "origin"))) + .expect_err("should fail until the HTTP client is implemented"); + + assert!( + matches!(err.current_context(), &PlatformError::NotImplemented), + "should report NotImplemented while the Fastly HTTP client is still a stub" + ); + } } diff --git a/crates/trusted-server-core/src/storage/config_store.rs b/crates/trusted-server-core/src/storage/config_store.rs index 95055128..8b11573e 100644 --- a/crates/trusted-server-core/src/storage/config_store.rs +++ b/crates/trusted-server-core/src/storage/config_store.rs @@ -5,11 +5,57 @@ //! [`crate::platform::RuntimeServices`] instead. This type will be removed //! once all call sites have migrated. +use core::fmt::Display; + use error_stack::Report; use fastly::ConfigStore; use crate::error::TrustedServerError; +trait ConfigStoreReader { + type LookupError: Display; + + fn try_get(&self, key: &str) -> Result, Self::LookupError>; +} + +impl ConfigStoreReader for ConfigStore { + type LookupError = fastly::config_store::LookupError; + + fn try_get(&self, key: &str) -> Result, Self::LookupError> { + ConfigStore::try_get(self, key) + } +} + +fn load_config_value( + store_name: &str, + key: &str, + open_store: Open, +) -> Result> +where + S: ConfigStoreReader, + Open: FnOnce(&str) -> Result, + OpenError: Display, +{ + let store = open_store(store_name).map_err(|error| { + Report::new(TrustedServerError::Configuration { + message: format!("failed to open config store '{store_name}': {error}"), + }) + })?; + + store + .try_get(key) + .map_err(|error| { + Report::new(TrustedServerError::Configuration { + message: format!("lookup for key '{key}' failed: {error}"), + }) + })? + .ok_or_else(|| { + Report::new(TrustedServerError::Configuration { + message: format!("key '{key}' not found in config store '{store_name}'"), + }) + }) +} + /// Fastly-backed config store with the store name baked in at construction. /// /// # Migration note @@ -36,16 +82,7 @@ impl FastlyConfigStore { /// /// Returns an error if the key is not found in the config store. pub fn get(&self, key: &str) -> Result> { - // TODO(pr3): replace ConfigStore::open with try_open when all callers migrate - let store = ConfigStore::open(&self.store_name); - store.get(key).ok_or_else(|| { - Report::new(TrustedServerError::Configuration { - message: format!( - "key '{}' not found in config store '{}'", - key, self.store_name - ), - }) - }) + load_config_value::(&self.store_name, key, ConfigStore::try_open) } } @@ -53,6 +90,18 @@ impl FastlyConfigStore { mod tests { use super::*; + struct StubConfigStore { + value: Result, &'static str>, + } + + impl ConfigStoreReader for StubConfigStore { + type LookupError = &'static str; + + fn try_get(&self, _key: &str) -> Result, Self::LookupError> { + self.value.clone() + } + } + #[test] fn config_store_new_stores_name() { let store = FastlyConfigStore::new("test_store"); @@ -62,6 +111,49 @@ mod tests { ); } + #[test] + fn load_config_value_returns_error_when_open_fails() { + let err = load_config_value::("jwks_store", "current-kid", |_| { + Err("open failed") + }) + .expect_err("should return an error when the store cannot be opened"); + + assert!( + err.to_string().contains("failed to open config store"), + "should describe the open failure" + ); + } + + #[test] + fn load_config_value_returns_error_when_lookup_fails() { + let err = load_config_value::("jwks_store", "current-kid", |_| { + Ok::(StubConfigStore { + value: Err("lookup failed"), + }) + }) + .expect_err("should return an error when lookup fails"); + + assert!( + err.to_string() + .contains("lookup for key 'current-kid' failed"), + "should describe the lookup failure" + ); + } + + #[test] + fn load_config_value_returns_error_when_key_is_missing() { + let err = load_config_value::("jwks_store", "current-kid", |_| { + Ok::(StubConfigStore { value: Ok(None) }) + }) + .expect_err("should return an error when the key is absent"); + + assert!( + err.to_string() + .contains("key 'current-kid' not found in config store 'jwks_store'"), + "should describe the missing key" + ); + } + #[test] fn config_store_get_in_test_environment() { let store = FastlyConfigStore::new("jwks_store"); From 14e54c42fe287acb973efef4fc09466d0f2e8d7b Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Wed, 25 Mar 2026 10:18:33 +0530 Subject: [PATCH 3/5] Address storage review feedback --- .../trusted-server-core/src/platform/mod.rs | 125 +-------------- .../src/platform/test_support.rs | 126 +++++++++++++++ .../src/request_signing/endpoints.rs | 128 ++------------- .../src/request_signing/jwks.rs | 114 ++------------ .../src/storage/api_client.rs | 45 ++---- .../src/storage/config_store.rs | 13 +- .../src/storage/secret_store.rs | 148 +++++++++++++----- 7 files changed, 281 insertions(+), 418 deletions(-) create mode 100644 crates/trusted-server-core/src/platform/test_support.rs diff --git a/crates/trusted-server-core/src/platform/mod.rs b/crates/trusted-server-core/src/platform/mod.rs index 218c7bac..312c9b13 100644 --- a/crates/trusted-server-core/src/platform/mod.rs +++ b/crates/trusted-server-core/src/platform/mod.rs @@ -7,6 +7,8 @@ mod error; mod http; mod kv; +#[cfg(test)] +pub(crate) mod test_support; mod traits; mod types; @@ -25,11 +27,7 @@ pub use types::{ #[cfg(test)] mod tests { - use std::net::IpAddr; - use std::sync::Arc; - - use error_stack::Report; - + use super::test_support::noop_services; use super::*; fn _assert_config_store_object_safe(_: &dyn PlatformConfigStore) {} @@ -47,123 +45,6 @@ mod tests { { } - struct NoopConfigStore; - impl PlatformConfigStore for NoopConfigStore { - fn get( - &self, - _store_name: &StoreName, - _key: &str, - ) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - - fn put( - &self, - _store_id: &StoreId, - _key: &str, - _value: &str, - ) -> Result<(), Report> { - Err(Report::new(PlatformError::Unsupported)) - } - - fn delete(&self, _store_id: &StoreId, _key: &str) -> Result<(), Report> { - Err(Report::new(PlatformError::Unsupported)) - } - } - - struct NoopSecretStore; - impl PlatformSecretStore for NoopSecretStore { - fn get_bytes( - &self, - _store_name: &StoreName, - _key: &str, - ) -> Result, Report> { - Err(Report::new(PlatformError::Unsupported)) - } - - fn create( - &self, - _store_id: &StoreId, - _name: &str, - _value: &str, - ) -> Result<(), Report> { - Err(Report::new(PlatformError::Unsupported)) - } - - fn delete(&self, _store_id: &StoreId, _name: &str) -> Result<(), Report> { - Err(Report::new(PlatformError::Unsupported)) - } - } - - struct NoopBackend; - impl PlatformBackend for NoopBackend { - fn predict_name( - &self, - _spec: &PlatformBackendSpec, - ) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - - fn ensure(&self, _spec: &PlatformBackendSpec) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - } - - struct NoopHttpClient; - // ?Send matches PlatformHttpClient — Body wraps LocalBoxStream which is !Send - // by design; see http.rs for the full rationale. - #[async_trait::async_trait(?Send)] - impl PlatformHttpClient for NoopHttpClient { - async fn send( - &self, - _request: PlatformHttpRequest, - ) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - - async fn send_async( - &self, - _request: PlatformHttpRequest, - ) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - - async fn select( - &self, - _pending_requests: Vec, - ) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - } - - struct NoopGeo; - impl PlatformGeo for NoopGeo { - fn lookup( - &self, - _client_ip: Option, - ) -> Result, Report> { - Ok(None) - } - } - - fn noop_services() -> RuntimeServices { - // edgezero_core::key_value_store::NoopKvStore is available via the - // test-utils feature enabled in dev-dependencies. - RuntimeServices::builder() - .config_store(Arc::new(NoopConfigStore)) - .secret_store(Arc::new(NoopSecretStore)) - .kv_store(Arc::new(edgezero_core::key_value_store::NoopKvStore)) - .backend(Arc::new(NoopBackend)) - .http_client(Arc::new(NoopHttpClient)) - .geo(Arc::new(NoopGeo)) - .client_info(ClientInfo { - client_ip: None, - tls_protocol: None, - tls_cipher: None, - }) - .build() - } - #[test] fn runtime_services_can_be_constructed_and_cloned() { let services = noop_services(); diff --git a/crates/trusted-server-core/src/platform/test_support.rs b/crates/trusted-server-core/src/platform/test_support.rs new file mode 100644 index 00000000..3bdb6b2b --- /dev/null +++ b/crates/trusted-server-core/src/platform/test_support.rs @@ -0,0 +1,126 @@ +use std::net::IpAddr; +use std::sync::Arc; + +use error_stack::Report; + +use super::{ + ClientInfo, GeoInfo, PlatformBackend, PlatformBackendSpec, PlatformConfigStore, PlatformError, + PlatformGeo, PlatformHttpClient, PlatformHttpRequest, PlatformPendingRequest, PlatformResponse, + PlatformSecretStore, PlatformSelectResult, RuntimeServices, StoreId, StoreName, +}; + +pub(crate) struct NoopConfigStore; + +impl PlatformConfigStore for NoopConfigStore { + fn get(&self, _store_name: &StoreName, _key: &str) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + + fn put( + &self, + _store_id: &StoreId, + _key: &str, + _value: &str, + ) -> Result<(), Report> { + Err(Report::new(PlatformError::Unsupported)) + } + + fn delete(&self, _store_id: &StoreId, _key: &str) -> Result<(), Report> { + Err(Report::new(PlatformError::Unsupported)) + } +} + +pub(crate) struct NoopSecretStore; + +impl PlatformSecretStore for NoopSecretStore { + fn get_bytes( + &self, + _store_name: &StoreName, + _key: &str, + ) -> Result, Report> { + Err(Report::new(PlatformError::Unsupported)) + } + + fn create( + &self, + _store_id: &StoreId, + _name: &str, + _value: &str, + ) -> Result<(), Report> { + Err(Report::new(PlatformError::Unsupported)) + } + + fn delete(&self, _store_id: &StoreId, _name: &str) -> Result<(), Report> { + Err(Report::new(PlatformError::Unsupported)) + } +} + +pub(crate) struct NoopBackend; + +impl PlatformBackend for NoopBackend { + fn predict_name(&self, _spec: &PlatformBackendSpec) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + + fn ensure(&self, _spec: &PlatformBackendSpec) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } +} + +pub(crate) struct NoopHttpClient; + +// ?Send matches PlatformHttpClient. Body wraps LocalBoxStream which is !Send +// by design; see http.rs for the full rationale. +#[async_trait::async_trait(?Send)] +impl PlatformHttpClient for NoopHttpClient { + async fn send( + &self, + _request: PlatformHttpRequest, + ) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + + async fn send_async( + &self, + _request: PlatformHttpRequest, + ) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + + async fn select( + &self, + _pending_requests: Vec, + ) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } +} + +pub(crate) struct NoopGeo; + +impl PlatformGeo for NoopGeo { + fn lookup(&self, _client_ip: Option) -> Result, Report> { + Ok(None) + } +} + +pub(crate) fn build_services_with_config( + config_store: impl PlatformConfigStore + 'static, +) -> RuntimeServices { + RuntimeServices::builder() + .config_store(Arc::new(config_store)) + .secret_store(Arc::new(NoopSecretStore)) + .kv_store(Arc::new(edgezero_core::key_value_store::NoopKvStore)) + .backend(Arc::new(NoopBackend)) + .http_client(Arc::new(NoopHttpClient)) + .geo(Arc::new(NoopGeo)) + .client_info(ClientInfo { + client_ip: None, + tls_protocol: None, + tls_cipher: None, + }) + .build() +} + +pub(crate) fn noop_services() -> RuntimeServices { + build_services_with_config(NoopConfigStore) +} diff --git a/crates/trusted-server-core/src/request_signing/endpoints.rs b/crates/trusted-server-core/src/request_signing/endpoints.rs index c1974197..8bee239a 100644 --- a/crates/trusted-server-core/src/request_signing/endpoints.rs +++ b/crates/trusted-server-core/src/request_signing/endpoints.rs @@ -36,14 +36,14 @@ pub fn handle_trusted_server_discovery( let jwks_value: serde_json::Value = serde_json::from_str(&jwks_json).change_context(TrustedServerError::Configuration { - message: "Failed to parse JWKS JSON".into(), + message: "failed to parse JWKS JSON".into(), })?; let discovery = TrustedServerDiscovery::new(jwks_value); let json = serde_json::to_string_pretty(&discovery).change_context( TrustedServerError::Configuration { - message: "Failed to serialize discovery document".into(), + message: "failed to serialize discovery document".into(), }, )?; @@ -81,7 +81,7 @@ pub fn handle_verify_signature( let body = req.take_body_str(); let verify_req: VerifySignatureRequest = serde_json::from_str(&body).change_context(TrustedServerError::Configuration { - message: "Invalid JSON request body".into(), + message: "invalid JSON request body".into(), })?; let verification_result = signing::verify_signature( @@ -113,7 +113,7 @@ pub fn handle_verify_signature( let response_json = serde_json::to_string(&response).map_err(|e| { Report::new(TrustedServerError::Configuration { - message: format!("Failed to serialize response: {}", e), + message: format!("failed to serialize response: {}", e), }) })?; @@ -153,7 +153,7 @@ pub fn handle_rotate_key( Some(setting) => (&setting.config_store_id, &setting.secret_store_id), None => { return Err(TrustedServerError::Configuration { - message: "Missing signing storage configuration.".to_string(), + message: "missing signing storage configuration".to_string(), } .into()); } @@ -164,13 +164,13 @@ pub fn handle_rotate_key( RotateKeyRequest { kid: None } } else { serde_json::from_str(&body).change_context(TrustedServerError::Configuration { - message: "Invalid JSON request body".into(), + message: "invalid JSON request body".into(), })? }; let manager = KeyRotationManager::new(config_store_id, secret_store_id).change_context( TrustedServerError::Configuration { - message: "Failed to create KeyRotationManager".into(), + message: "failed to create KeyRotationManager".into(), }, )?; @@ -178,7 +178,7 @@ pub fn handle_rotate_key( Ok(result) => { let jwk_value = serde_json::to_value(&result.jwk).map_err(|e| { Report::new(TrustedServerError::Configuration { - message: format!("Failed to serialize JWK: {}", e), + message: format!("failed to serialize JWK: {}", e), }) })?; @@ -194,7 +194,7 @@ pub fn handle_rotate_key( let response_json = serde_json::to_string(&response).map_err(|e| { Report::new(TrustedServerError::Configuration { - message: format!("Failed to serialize response: {}", e), + message: format!("failed to serialize response: {}", e), }) })?; @@ -215,7 +215,7 @@ pub fn handle_rotate_key( let response_json = serde_json::to_string(&response).map_err(|e| { Report::new(TrustedServerError::Configuration { - message: format!("Failed to serialize response: {}", e), + message: format!("failed to serialize response: {}", e), }) })?; @@ -257,7 +257,7 @@ pub fn handle_deactivate_key( Some(setting) => (&setting.config_store_id, &setting.secret_store_id), None => { return Err(TrustedServerError::Configuration { - message: "Missing signing storage configuration.".to_string(), + message: "missing signing storage configuration".to_string(), } .into()); } @@ -266,12 +266,12 @@ pub fn handle_deactivate_key( let body = req.take_body_str(); let deactivate_req: DeactivateKeyRequest = serde_json::from_str(&body).change_context(TrustedServerError::Configuration { - message: "Invalid JSON request body".into(), + message: "invalid JSON request body".into(), })?; let manager = KeyRotationManager::new(config_store_id, secret_store_id).change_context( TrustedServerError::Configuration { - message: "Failed to create KeyRotationManager".into(), + message: "failed to create KeyRotationManager".into(), }, )?; @@ -303,7 +303,7 @@ pub fn handle_deactivate_key( let response_json = serde_json::to_string(&response).map_err(|e| { Report::new(TrustedServerError::Configuration { - message: format!("Failed to serialize response: {}", e), + message: format!("failed to serialize response: {}", e), }) })?; @@ -327,7 +327,7 @@ pub fn handle_deactivate_key( let response_json = serde_json::to_string(&response).map_err(|e| { Report::new(TrustedServerError::Configuration { - message: format!("Failed to serialize response: {}", e), + message: format!("failed to serialize response: {}", e), }) })?; @@ -340,91 +340,16 @@ pub fn handle_deactivate_key( #[cfg(test)] mod tests { - use std::net::IpAddr; - use std::sync::Arc; - use error_stack::Report; use crate::platform::{ - ClientInfo, GeoInfo, PlatformBackend, PlatformBackendSpec, PlatformConfigStore, - PlatformError, PlatformGeo, PlatformHttpClient, PlatformHttpRequest, - PlatformPendingRequest, PlatformResponse, PlatformSecretStore, PlatformSelectResult, - RuntimeServices, StoreId, StoreName, + test_support::{build_services_with_config, noop_services}, + PlatformConfigStore, PlatformError, StoreId, StoreName, }; use super::*; use fastly::http::{Method, StatusCode}; - struct NoopConfigStore; - impl PlatformConfigStore for NoopConfigStore { - fn get(&self, _: &StoreName, _: &str) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - fn put(&self, _: &StoreId, _: &str, _: &str) -> Result<(), Report> { - Err(Report::new(PlatformError::Unsupported)) - } - fn delete(&self, _: &StoreId, _: &str) -> Result<(), Report> { - Err(Report::new(PlatformError::Unsupported)) - } - } - - struct NoopSecretStore; - impl PlatformSecretStore for NoopSecretStore { - fn get_bytes(&self, _: &StoreName, _: &str) -> Result, Report> { - Err(Report::new(PlatformError::Unsupported)) - } - fn create(&self, _: &StoreId, _: &str, _: &str) -> Result<(), Report> { - Err(Report::new(PlatformError::Unsupported)) - } - fn delete(&self, _: &StoreId, _: &str) -> Result<(), Report> { - Err(Report::new(PlatformError::Unsupported)) - } - } - - struct NoopBackend; - impl PlatformBackend for NoopBackend { - fn predict_name(&self, _: &PlatformBackendSpec) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - fn ensure(&self, _: &PlatformBackendSpec) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - } - - struct NoopHttpClient; - #[async_trait::async_trait(?Send)] - impl PlatformHttpClient for NoopHttpClient { - async fn send( - &self, - _: PlatformHttpRequest, - ) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - async fn send_async( - &self, - _: PlatformHttpRequest, - ) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - async fn select( - &self, - _: Vec, - ) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - } - - struct NoopGeo; - impl PlatformGeo for NoopGeo { - fn lookup(&self, _: Option) -> Result, Report> { - Ok(None) - } - } - - fn noop_services() -> RuntimeServices { - build_services_with_config(NoopConfigStore) - } - /// Config store stub that returns a minimal JWKS with one Ed25519 key. struct StubJwksConfigStore; @@ -448,25 +373,6 @@ mod tests { Err(Report::new(PlatformError::Unsupported)) } } - - fn build_services_with_config( - config_store: impl PlatformConfigStore + 'static, - ) -> RuntimeServices { - RuntimeServices::builder() - .config_store(Arc::new(config_store)) - .secret_store(Arc::new(NoopSecretStore)) - .kv_store(Arc::new(edgezero_core::key_value_store::NoopKvStore)) - .backend(Arc::new(NoopBackend)) - .http_client(Arc::new(NoopHttpClient)) - .geo(Arc::new(NoopGeo)) - .client_info(ClientInfo { - client_ip: None, - tls_protocol: None, - tls_cipher: None, - }) - .build() - } - #[test] fn test_handle_verify_signature_valid() { let settings = crate::test_support::tests::create_test_settings(); diff --git a/crates/trusted-server-core/src/request_signing/jwks.rs b/crates/trusted-server-core/src/request_signing/jwks.rs index 2df4ea38..5c4dda94 100644 --- a/crates/trusted-server-core/src/request_signing/jwks.rs +++ b/crates/trusted-server-core/src/request_signing/jwks.rs @@ -3,6 +3,8 @@ //! This module provides functionality for generating, storing, and retrieving //! Ed25519 keypairs in JWK format for request signing. +use std::sync::LazyLock; + use ed25519_dalek::{SigningKey, VerifyingKey}; use error_stack::{Report, ResultExt}; use jose_jwk::{ @@ -15,6 +17,9 @@ use crate::error::TrustedServerError; use crate::platform::{RuntimeServices, StoreName}; use crate::request_signing::JWKS_CONFIG_STORE_NAME; +static JWKS_STORE_NAME: LazyLock = + LazyLock::new(|| StoreName::from(JWKS_CONFIG_STORE_NAME)); + /// An Ed25519 keypair used for request signing. pub struct Keypair { pub signing_key: SigningKey, @@ -70,10 +75,9 @@ impl Keypair { /// cannot be read. The underlying [`crate::platform::PlatformError`] is /// preserved as context in the error chain. pub fn get_active_jwks(services: &RuntimeServices) -> Result> { - let store_name = StoreName::from(JWKS_CONFIG_STORE_NAME); let active_kids_str = services .config_store() - .get(&store_name, "active-kids") + .get(&JWKS_STORE_NAME, "active-kids") .change_context(TrustedServerError::Configuration { message: "failed to read active-kids from config store".into(), }) @@ -89,7 +93,7 @@ pub fn get_active_jwks(services: &RuntimeServices) -> Result Result Result, Report> { - Err(Report::new(PlatformError::Unsupported)) - } - - fn create( - &self, - _store_id: &StoreId, - _name: &str, - _value: &str, - ) -> Result<(), Report> { - Err(Report::new(PlatformError::Unsupported)) - } - - fn delete(&self, _store_id: &StoreId, _name: &str) -> Result<(), Report> { - Err(Report::new(PlatformError::Unsupported)) - } - } - - struct NoopBackend; - - impl PlatformBackend for NoopBackend { - fn predict_name( - &self, - _spec: &PlatformBackendSpec, - ) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - - fn ensure(&self, _spec: &PlatformBackendSpec) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - } - - struct NoopHttpClient; - - #[async_trait::async_trait(?Send)] - impl PlatformHttpClient for NoopHttpClient { - async fn send( - &self, - _request: PlatformHttpRequest, - ) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - - async fn send_async( - &self, - _request: PlatformHttpRequest, - ) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - - async fn select( - &self, - _pending_requests: Vec, - ) -> Result> { - Err(Report::new(PlatformError::Unsupported)) - } - } - - struct NoopGeo; - - impl PlatformGeo for NoopGeo { - fn lookup( - &self, - _client_ip: Option, - ) -> Result, Report> { - Ok(None) - } - } - - fn build_services_with_config( - config_store: impl PlatformConfigStore + 'static, - ) -> RuntimeServices { - RuntimeServices::builder() - .config_store(Arc::new(config_store)) - .secret_store(Arc::new(NoopSecretStore)) - .kv_store(Arc::new(edgezero_core::key_value_store::NoopKvStore)) - .backend(Arc::new(NoopBackend)) - .http_client(Arc::new(NoopHttpClient)) - .geo(Arc::new(NoopGeo)) - .client_info(ClientInfo { - client_ip: None, - tls_protocol: None, - tls_cipher: None, - }) - .build() - } - // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- diff --git a/crates/trusted-server-core/src/storage/api_client.rs b/crates/trusted-server-core/src/storage/api_client.rs index 70f044b0..c4f55737 100644 --- a/crates/trusted-server-core/src/storage/api_client.rs +++ b/crates/trusted-server-core/src/storage/api_client.rs @@ -18,6 +18,10 @@ use crate::storage::secret_store::FastlySecretStore; const FASTLY_API_HOST: &str = "https://api.fastly.com"; +fn build_config_item_payload(value: &str) -> String { + format!("item_value={}", urlencoding::encode(value)) +} + /// HTTP client for the Fastly management API. /// /// Used to perform write operations on config and secret stores via the @@ -119,7 +123,7 @@ impl FastlyApiClient { value: &str, ) -> Result<(), Report> { let path = format!("/resources/stores/config/{}/item/{}", store_id, key); - let payload = format!("item_value={}", value); + let payload = build_config_item_payload(value); let mut response = self.make_request( "PUT", @@ -275,38 +279,13 @@ mod tests { use super::*; #[test] - fn api_client_creation_in_test_environment() { - let result = FastlyApiClient::new(); - match result { - Ok(_) => println!("Successfully created API client"), - Err(e) => println!("Expected error in test environment: {}", e), - } - } + fn config_item_payload_url_encodes_reserved_characters() { + let payload = build_config_item_payload(r#"value with spaces + symbols &= {"kid":"a+b"}"#); - #[test] - fn api_client_update_config_item_in_test_environment() { - if let Ok(client) = FastlyApiClient::new() { - let result = - client.update_config_item("5WNlRjznCUAGTU0QeYU8x2", "test-key", "test-value"); - match result { - Ok(()) => println!("Successfully updated config item"), - Err(e) => println!("Expected error in test environment: {}", e), - } - } - } - - #[test] - fn api_client_create_secret_in_test_environment() { - if let Ok(client) = FastlyApiClient::new() { - let result = client.create_secret( - "Ltf3CkSGV0Yn2PIC2lDcZx", - "test-secret-new", - "SGVsbG8sIHdvcmxkIQ==", - ); - match result { - Ok(()) => println!("Successfully created secret"), - Err(e) => println!("Expected error in test environment: {}", e), - } - } + assert_eq!( + payload, + "item_value=value%20with%20spaces%20%2B%20symbols%20%26%3D%20%7B%22kid%22%3A%22a%2Bb%22%7D", + "should URL-encode config item values in form payloads" + ); } } diff --git a/crates/trusted-server-core/src/storage/config_store.rs b/crates/trusted-server-core/src/storage/config_store.rs index 8b11573e..cc396f73 100644 --- a/crates/trusted-server-core/src/storage/config_store.rs +++ b/crates/trusted-server-core/src/storage/config_store.rs @@ -12,6 +12,9 @@ use fastly::ConfigStore; use crate::error::TrustedServerError; +// TODO: Deduplicate this transitional helper with +// trusted-server-adapter-fastly/src/platform.rs:get_config_value once +// FastlyConfigStore is removed. trait ConfigStoreReader { type LookupError: Display; @@ -153,14 +156,4 @@ mod tests { "should describe the missing key" ); } - - #[test] - fn config_store_get_in_test_environment() { - let store = FastlyConfigStore::new("jwks_store"); - let result = store.get("current-kid"); - match result { - Ok(kid) => println!("Current KID: {}", kid), - Err(e) => println!("Expected error in test environment: {}", e), - } - } } diff --git a/crates/trusted-server-core/src/storage/secret_store.rs b/crates/trusted-server-core/src/storage/secret_store.rs index 490a3351..f2dd7b91 100644 --- a/crates/trusted-server-core/src/storage/secret_store.rs +++ b/crates/trusted-server-core/src/storage/secret_store.rs @@ -5,11 +5,81 @@ //! [`crate::platform::RuntimeServices`] instead. This type will be removed //! once all call sites have migrated. +use core::fmt::Display; + use error_stack::{Report, ResultExt}; use fastly::SecretStore; use crate::error::TrustedServerError; +#[derive(Clone)] +enum SecretReadError { + Lookup(LookupError), + Decrypt(DecryptError), +} + +type SecretBytesResult = + Result>, SecretReadError>; + +trait SecretStoreReader: Sized { + type LookupError: Display; + type DecryptError: Display; + + fn try_get_bytes(&self, key: &str) -> SecretBytesResult; +} + +impl SecretStoreReader for SecretStore { + type LookupError = fastly::secret_store::LookupError; + type DecryptError = fastly::secret_store::DecryptError; + + fn try_get_bytes(&self, key: &str) -> SecretBytesResult { + let secret = self.try_get(key).map_err(SecretReadError::Lookup)?; + let Some(secret) = secret else { + return Ok(None); + }; + + secret + .try_plaintext() + .map(|bytes| Some(bytes.into_iter().collect())) + .map_err(SecretReadError::Decrypt) + } +} + +fn get_secret_bytes( + store_name: &str, + key: &str, + open_store: Open, +) -> Result, Report> +where + S: SecretStoreReader, + Open: FnOnce() -> Result, + OpenError: Display, +{ + let store = open_store().map_err(|error| { + Report::new(TrustedServerError::Configuration { + message: format!("failed to open secret store '{store_name}': {error}"), + }) + })?; + + store + .try_get_bytes(key) + .map_err(|error| match error { + SecretReadError::Lookup(error) => Report::new(TrustedServerError::Configuration { + message: format!( + "lookup for secret '{key}' in secret store '{store_name}' failed: {error}" + ), + }), + SecretReadError::Decrypt(error) => Report::new(TrustedServerError::Configuration { + message: format!("failed to decrypt secret '{key}': {error}"), + }), + })? + .ok_or_else(|| { + Report::new(TrustedServerError::Configuration { + message: format!("secret '{key}' not found in secret store '{store_name}'"), + }) + }) +} + /// Fastly-backed secret store with the store name baked in at construction. /// /// # Migration note @@ -37,29 +107,9 @@ impl FastlySecretStore { /// Returns an error if the secret store cannot be opened, the key is not /// found, or the plaintext cannot be retrieved. pub fn get(&self, key: &str) -> Result, Report> { - let store = SecretStore::open(&self.store_name).map_err(|_| { - Report::new(TrustedServerError::Configuration { - message: format!("failed to open secret store '{}'", self.store_name), - }) - })?; - - let secret = store.get(key).ok_or_else(|| { - Report::new(TrustedServerError::Configuration { - message: format!( - "secret '{}' not found in secret store '{}'", - key, self.store_name - ), - }) - })?; - - secret - .try_plaintext() - .map_err(|_| { - Report::new(TrustedServerError::Configuration { - message: "failed to retrieve secret plaintext".into(), - }) - }) - .map(|bytes| bytes.into_iter().collect()) + get_secret_bytes::(&self.store_name, key, || { + SecretStore::open(&self.store_name) + }) } /// Retrieves a secret value from the store and decodes it as a UTF-8 string. @@ -77,8 +127,34 @@ impl FastlySecretStore { #[cfg(test)] mod tests { + use core::fmt::{self, Display}; + use super::*; - use crate::storage::FastlyConfigStore; + + struct StubSecretStore { + value: SecretBytesResult<&'static str, &'static str>, + } + + impl SecretStoreReader for StubSecretStore { + type LookupError = &'static str; + type DecryptError = &'static str; + + fn try_get_bytes( + &self, + _key: &str, + ) -> SecretBytesResult { + self.value.clone() + } + } + + #[derive(Clone)] + struct StubOpenError(&'static str); + + impl Display for StubOpenError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.0) + } + } #[test] fn secret_store_new_stores_name() { @@ -90,18 +166,16 @@ mod tests { } #[test] - fn secret_store_get_in_test_environment() { - let store = FastlySecretStore::new("signing_keys"); - let config_store = FastlyConfigStore::new("jwks_store"); - - match config_store.get("current-kid") { - Ok(kid) => match store.get(&kid) { - Ok(bytes) => { - assert!(!bytes.is_empty(), "should have non-empty secret bytes"); - } - Err(e) => println!("Expected error in test environment: {}", e), - }, - Err(e) => println!("Expected error in test environment: {}", e), - } + fn get_secret_bytes_includes_open_error_details() { + let err = get_secret_bytes::("signing_keys", "active", || { + Err(StubOpenError("permission denied")) + }) + .expect_err("should return an error when the secret store cannot be opened"); + + assert!( + err.to_string() + .contains("failed to open secret store 'signing_keys': permission denied"), + "should preserve the original open error message" + ); } } From c682c6d8618de6dd8569902152843eb12e0cdf98 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Wed, 25 Mar 2026 10:29:25 +0530 Subject: [PATCH 4/5] Resolved github-advanced-security bot problems --- crates/trusted-server-core/src/storage/api_client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/trusted-server-core/src/storage/api_client.rs b/crates/trusted-server-core/src/storage/api_client.rs index c4f55737..81a2d57b 100644 --- a/crates/trusted-server-core/src/storage/api_client.rs +++ b/crates/trusted-server-core/src/storage/api_client.rs @@ -63,7 +63,7 @@ impl FastlyApiClient { let backend_name = BackendConfig::from_url("https://api.fastly.com", true)?; let api_key = FastlySecretStore::new(store_name).get(key_name)?; - log::debug!("FastlyApiClient initialized with backend: {}", backend_name); + log::debug!("FastlyApiClient initialized"); Ok(Self { api_key, From 3adf42c8977602966e74dc50e21b0a991fde9b89 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Mon, 30 Mar 2026 17:19:33 +0530 Subject: [PATCH 5/5] Make client_info field pub(crate) and add a client_info() accessor --- crates/trusted-server-adapter-fastly/src/main.rs | 2 +- crates/trusted-server-adapter-fastly/src/platform.rs | 9 +++++---- crates/trusted-server-core/src/platform/mod.rs | 8 ++++---- crates/trusted-server-core/src/platform/types.rs | 8 +++++++- 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/crates/trusted-server-adapter-fastly/src/main.rs b/crates/trusted-server-adapter-fastly/src/main.rs index 8bbd7c6c..04043d8d 100644 --- a/crates/trusted-server-adapter-fastly/src/main.rs +++ b/crates/trusted-server-adapter-fastly/src/main.rs @@ -111,7 +111,7 @@ async fn route_request( // already captured in RuntimeServices at the entry point. let geo_info = runtime_services .geo() - .lookup(runtime_services.client_info.client_ip) + .lookup(runtime_services.client_info().client_ip) .unwrap_or_else(|e| { log::warn!("geo lookup failed: {e}"); None diff --git a/crates/trusted-server-adapter-fastly/src/platform.rs b/crates/trusted-server-adapter-fastly/src/platform.rs index b506d4c4..da32c929 100644 --- a/crates/trusted-server-adapter-fastly/src/platform.rs +++ b/crates/trusted-server-adapter-fastly/src/platform.rs @@ -305,7 +305,7 @@ impl PlatformGeo for FastlyPlatformGeo { /// Call this once at the entry point before dispatching to handlers. /// `client_info` is populated from TLS and IP metadata available on the /// request; geo lookup is deferred to handler time via -/// `services.geo.lookup(services.client_info.client_ip)`. +/// `services.geo().lookup(services.client_info().client_ip)`. /// /// `kv_store` is an [`Arc`] opened by the caller for /// the primary KV store. Use [`open_kv_store`] to construct it. @@ -483,11 +483,11 @@ mod tests { let services = build_runtime_services(&req, noop_kv_store()); assert!( - services.client_info.tls_protocol.is_none(), + services.client_info().tls_protocol.is_none(), "should have no tls_protocol on plain test request" ); assert!( - services.client_info.tls_cipher.is_none(), + services.client_info().tls_cipher.is_none(), "should have no tls_cipher on plain test request" ); } @@ -499,7 +499,8 @@ mod tests { let cloned = services.clone(); assert_eq!( - services.client_info.client_ip, cloned.client_info.client_ip, + services.client_info().client_ip, + cloned.client_info().client_ip, "should preserve client_ip through clone" ); } diff --git a/crates/trusted-server-core/src/platform/mod.rs b/crates/trusted-server-core/src/platform/mod.rs index 7e26a5f9..9b9da1a3 100644 --- a/crates/trusted-server-core/src/platform/mod.rs +++ b/crates/trusted-server-core/src/platform/mod.rs @@ -60,11 +60,11 @@ mod tests { let cloned = services.clone(); assert!( - cloned.client_info.client_ip.is_none(), + cloned.client_info().client_ip.is_none(), "should preserve client_ip through clone" ); assert!( - cloned.client_info.tls_protocol.is_none(), + cloned.client_info().tls_protocol.is_none(), "should preserve tls_protocol through clone" ); } @@ -73,8 +73,8 @@ mod tests { fn runtime_services_geo_lookup_returns_none_for_no_ip() { let services = noop_services(); let result = services - .geo - .lookup(services.client_info.client_ip) + .geo() + .lookup(services.client_info().client_ip) .expect("should not fail for noop geo with no ip"); assert!(result.is_none(), "should return None when no IP is present"); } diff --git a/crates/trusted-server-core/src/platform/types.rs b/crates/trusted-server-core/src/platform/types.rs index 05b1c1b8..0eaa3a0c 100644 --- a/crates/trusted-server-core/src/platform/types.rs +++ b/crates/trusted-server-core/src/platform/types.rs @@ -146,7 +146,7 @@ pub struct RuntimeServices { /// Geographic information lookup. pub(crate) geo: Arc, /// Per-request client metadata extracted at the entry point. - pub client_info: ClientInfo, + pub(crate) client_info: ClientInfo, } impl RuntimeServices { @@ -204,6 +204,12 @@ impl RuntimeServices { &*self.geo } + /// Returns per-request client metadata (IP address, TLS details). + #[must_use] + pub fn client_info(&self) -> &ClientInfo { + &self.client_info + } + /// Wrap the KV store in a [`super::KvHandle`] for ergonomic access to /// JSON helpers, pagination, and validation. #[must_use]