From 794b66d7a208e4aed7d76edc31fb18660fb1e8e2 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Tue, 31 Mar 2026 18:47:56 +0530 Subject: [PATCH 01/14] Add plan for PR9: wire signing to store primitives --- ...31-pr9-wire-signing-to-store-primitives.md | 1807 +++++++++++++++++ 1 file changed, 1807 insertions(+) create mode 100644 docs/superpowers/plans/2026-03-31-pr9-wire-signing-to-store-primitives.md diff --git a/docs/superpowers/plans/2026-03-31-pr9-wire-signing-to-store-primitives.md b/docs/superpowers/plans/2026-03-31-pr9-wire-signing-to-store-primitives.md new file mode 100644 index 00000000..be478529 --- /dev/null +++ b/docs/superpowers/plans/2026-03-31-pr9-wire-signing-to-store-primitives.md @@ -0,0 +1,1807 @@ +# PR 9: Wire Request-Signing to Platform Store Primitives + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Remove `api_client.rs` from `trusted-server-core`, move Fastly management API transport to the adapter as `management_api.rs`, and replace all direct `Fastly*Store` / `FastlyApiClient` usage in `request_signing/` with `RuntimeServices` store primitives. + +**Architecture:** Core `request_signing/` code calls platform-agnostic `services.config_store()` and `services.secret_store()` for all reads and writes. The Fastly adapter's `management_api.rs` absorbs the HTTP transport (calls to `api.fastly.com`) and backs the `put`/`delete`/`create` write methods in `FastlyPlatformConfigStore` and `FastlyPlatformSecretStore`. No signing-specific trait is introduced — adapters only implement store CRUD, and core owns all signing business logic. + +**Tech Stack:** Rust 2024 edition, `error-stack`, `derive_more::Display`, `fastly`, `ed25519-dalek`, `serde_json`, `urlencoding` + +--- + +## Background: What the Current Code Does + +Before touching anything, read these files to understand the current state: + +| File | Status | Notes | +|------|--------|-------| +| `crates/trusted-server-core/src/storage/api_client.rs` | **Delete** | Contains `FastlyApiClient` — HTTP calls to `api.fastly.com`. Used only by `rotation.rs`. | +| `crates/trusted-server-core/src/request_signing/rotation.rs` | **Migrate** | Uses `FastlyConfigStore` (reads) + `FastlyApiClient` (writes). Main migration target. | +| `crates/trusted-server-core/src/request_signing/signing.rs` | **Migrate** | Uses `FastlyConfigStore` + `FastlySecretStore` in 3 places. | +| `crates/trusted-server-core/src/request_signing/endpoints.rs` | **Update** | `handle_verify_signature`, `handle_rotate_key`, `handle_deactivate_key` don't receive `&RuntimeServices`. | +| `crates/trusted-server-core/src/request_signing/jwks.rs` | Already migrated ✓ | Uses `RuntimeServices`. No changes needed. | +| `crates/trusted-server-adapter-fastly/src/platform.rs` | **Update** | `FastlyPlatformConfigStore::put/delete` and `FastlyPlatformSecretStore::create/delete` return `PlatformError::NotImplemented`. | +| `crates/trusted-server-adapter-fastly/src/main.rs` | **Update** | Three call sites pass handlers without `runtime_services`. | + +## File Map + +### Delete +- `crates/trusted-server-core/src/storage/api_client.rs` + +### Modify (core) +- `crates/trusted-server-core/src/storage/mod.rs` — remove `api_client` submodule + re-export +- `crates/trusted-server-core/src/platform/test_support.rs` — add `build_services_with_config_and_secret` +- `crates/trusted-server-core/src/request_signing/rotation.rs` — replace `FastlyConfigStore`/`FastlyApiClient` with `RuntimeServices` +- `crates/trusted-server-core/src/request_signing/signing.rs` — replace `FastlyConfigStore`/`FastlySecretStore` with `RuntimeServices` +- `crates/trusted-server-core/src/request_signing/endpoints.rs` — add `&RuntimeServices` to three handlers + +### Create (adapter) +- `crates/trusted-server-adapter-fastly/src/management_api.rs` — Fastly management API transport (absorbs `api_client.rs` logic, returns `PlatformError`) + +### Modify (adapter) +- `crates/trusted-server-adapter-fastly/src/platform.rs` — implement `put`/`delete` for config, `create`/`delete` for secrets +- `crates/trusted-server-adapter-fastly/src/main.rs` — pass `runtime_services` to three handlers + +--- + +## Tasks + +### Task 1: Add `build_services_with_config_and_secret` to `test_support.rs` + +**Why:** Tasks 4 and 5 need a `RuntimeServices` with both a custom config store AND a custom secret store. The current `build_services_with_config` only customises the config store. + +**Files:** +- Modify: `crates/trusted-server-core/src/platform/test_support.rs` + +- [ ] **Step 1: Write a failing test that calls `build_services_with_config_and_secret`** + +Add to the `#[cfg(test)]` block at the bottom of `test_support.rs`: + +```rust +#[test] +fn build_services_with_config_and_secret_uses_provided_stores() { + // Arrange: noop stores + let services = build_services_with_config_and_secret(NoopConfigStore, NoopSecretStore); + + // Act: both stores return Unsupported (confirming the injected impls are active) + let config_result = services.config_store().get(&StoreName::from("s"), "k"); + let secret_result = services.secret_store().get_bytes(&StoreName::from("s"), "k"); + + assert!(config_result.is_err(), "should delegate to injected config store"); + assert!(secret_result.is_err(), "should delegate to injected secret store"); +} +``` + +- [ ] **Step 2: Run to confirm it fails to compile** + +```bash +cargo test --package trusted-server-core platform::test_support 2>&1 | head -20 +``` + +Expected: compile error — `build_services_with_config_and_secret` not found. + +- [ ] **Step 3: Add the function above the existing `build_services_with_config`** + +```rust +/// Build a [`RuntimeServices`] instance with a custom config store and a custom secret store. +/// +/// Use this when a test exercises code that reads from config AND secret stores, +/// such as `request_signing::signing` and `request_signing::rotation`. +pub(crate) fn build_services_with_config_and_secret( + config_store: impl PlatformConfigStore + 'static, + secret_store: impl PlatformSecretStore + 'static, +) -> RuntimeServices { + RuntimeServices::builder() + .config_store(Arc::new(config_store)) + .secret_store(Arc::new(secret_store)) + .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() +} +``` + +- [ ] **Step 4: Run the test to confirm it passes** + +```bash +cargo test --package trusted-server-core platform::test_support::tests::build_services_with_config_and_secret +``` + +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add crates/trusted-server-core/src/platform/test_support.rs +git commit -m "Add build_services_with_config_and_secret to test_support" +``` + +--- + +### Task 2: Create `management_api.rs` in the adapter + +**Why:** Move the Fastly management API transport (currently in `api_client.rs` in core) to the adapter, where Fastly SDK usage is appropriate. Returns `PlatformError` instead of `TrustedServerError`. + +**Credential security note (from spec):** The Fastly API token is read from the `api-keys` secret store, key `api_key`. Log store IDs and operation names only — never the token or secret value. + +**Files:** +- Create: `crates/trusted-server-adapter-fastly/src/management_api.rs` +- Modify: `crates/trusted-server-adapter-fastly/src/main.rs` — add `mod management_api;` + +- [ ] **Step 1: Write the new module** + +Create `crates/trusted-server-adapter-fastly/src/management_api.rs`: + +```rust +//! Fastly management API transport for store write operations. +//! +//! Provides [`FastlyManagementApiClient`], which wraps the Fastly REST +//! management API for write operations on config and secret stores. +//! Used by [`super::platform::FastlyPlatformConfigStore`] and +//! [`super::platform::FastlyPlatformSecretStore`] to back store write methods. +//! +//! # Credentials +//! +//! The Fastly API token is read from the `api-keys` secret store under the +//! `api_key` entry. The token must have config-store write and secret-store +//! write permissions only — no service-level admin or purge permissions. +//! +//! # Security +//! +//! Credential values are never logged. Log messages include store IDs and +//! operation names only. + +use std::io::Read; + +use error_stack::{Report, ResultExt}; +use fastly::{Request, Response}; +use http::StatusCode; +use trusted_server_core::backend::BackendConfig; +use trusted_server_core::platform::{PlatformError, PlatformSecretStore, StoreName}; + +use crate::platform::FastlyPlatformSecretStore; + +const FASTLY_API_HOST: &str = "https://api.fastly.com"; +const API_KEYS_STORE: &str = "api-keys"; +const API_KEY_ENTRY: &str = "api_key"; + +pub(crate) fn build_config_item_payload(value: &str) -> String { + format!("item_value={}", urlencoding::encode(value)) +} + +/// HTTP client for Fastly management API write operations. +/// +/// Backs the `put`/`delete` methods of [`FastlyPlatformConfigStore`] and +/// the `create`/`delete` methods of [`FastlyPlatformSecretStore`]. +pub(crate) struct FastlyManagementApiClient { + api_key: Vec, + base_url: &'static str, + backend_name: String, +} + +impl FastlyManagementApiClient { + /// Initialize the client by reading the API token from the `api-keys` secret store. + /// + /// # Errors + /// + /// Returns [`PlatformError::Backend`] if the management API backend cannot + /// be registered, or [`PlatformError::SecretStore`] if the API key cannot + /// be read. + pub(crate) fn new() -> Result> { + let backend_name = BackendConfig::from_url(FASTLY_API_HOST, true) + .change_context(PlatformError::Backend) + .attach("failed to register Fastly management API backend")?; + + let api_key = FastlyPlatformSecretStore + .get_bytes(&StoreName::from(API_KEYS_STORE), API_KEY_ENTRY) + .change_context(PlatformError::SecretStore) + .attach("failed to read Fastly API key from secret store")?; + + log::debug!("FastlyManagementApiClient: initialized for management API operations"); + + Ok(Self { + api_key, + base_url: FASTLY_API_HOST, + backend_name, + }) + } + + fn make_request( + &self, + method: &str, + path: &str, + body: Option, + 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 { + "GET" => Request::get(&url), + "POST" => Request::post(&url), + "PUT" => Request::put(&url), + "DELETE" => Request::delete(&url), + _ => { + return Err(Report::new(PlatformError::ConfigStore) + .attach(format!("unsupported HTTP method: {}", method))) + } + }; + + request = request + .with_header("Fastly-Key", api_key_str) + .with_header("Accept", "application/json"); + + if let Some(body_content) = body { + request = request + .with_header("Content-Type", content_type) + .with_body(body_content); + } + + request.send(&self.backend_name).map_err(|e| { + Report::new(PlatformError::ConfigStore) + .attach(format!("management API request failed: {}", e)) + }) + } + + /// Update or create a config store item. + /// + /// # Errors + /// + /// Returns an error if the API request fails or returns a non-OK status. + pub(crate) fn update_config_item( + &self, + store_id: &str, + key: &str, + value: &str, + ) -> Result<(), Report> { + let path = format!("/resources/stores/config/{}/item/{}", store_id, key); + let payload = build_config_item_payload(value); + + let mut response = self.make_request( + "PUT", + &path, + Some(payload), + "application/x-www-form-urlencoded", + )?; + + let mut buf = String::new(); + response + .get_body_mut() + .read_to_string(&mut buf) + .change_context(PlatformError::ConfigStore)?; + + if response.get_status() == StatusCode::OK { + log::debug!( + "FastlyManagementApiClient: updated config key '{}' in store '{}'", + key, + store_id + ); + Ok(()) + } else { + Err(Report::new(PlatformError::ConfigStore).attach(format!( + "config item update failed with HTTP {} for key '{}' in store '{}'", + response.get_status(), + key, + store_id + ))) + } + } + + /// Delete a config store item. + /// + /// # Errors + /// + /// Returns an error if the API request fails or returns an unexpected status. + pub(crate) fn delete_config_item( + &self, + store_id: &str, + key: &str, + ) -> Result<(), Report> { + let path = format!("/resources/stores/config/{}/item/{}", store_id, key); + + let mut response = self.make_request("DELETE", &path, None, "application/json")?; + + let mut buf = String::new(); + response + .get_body_mut() + .read_to_string(&mut buf) + .change_context(PlatformError::ConfigStore)?; + + if response.get_status() == StatusCode::OK + || response.get_status() == StatusCode::NO_CONTENT + { + log::debug!( + "FastlyManagementApiClient: deleted config key '{}' from store '{}'", + key, + store_id + ); + Ok(()) + } else { + Err(Report::new(PlatformError::ConfigStore).attach(format!( + "config item delete failed with HTTP {} for key '{}' in store '{}'", + response.get_status(), + key, + store_id + ))) + } + } + + /// Create or overwrite a secret store entry. + /// + /// # Errors + /// + /// Returns an error if the API request fails or returns a non-OK status. + pub(crate) fn create_secret( + &self, + store_id: &str, + secret_name: &str, + 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 + }); + + let mut response = + self.make_request("POST", &path, Some(payload.to_string()), "application/json")?; + + let mut buf = String::new(); + response + .get_body_mut() + .read_to_string(&mut buf) + .change_context(PlatformError::SecretStore)?; + + if response.get_status() == StatusCode::OK { + log::debug!( + "FastlyManagementApiClient: created secret '{}' in store '{}'", + secret_name, + store_id + ); + Ok(()) + } else { + Err(Report::new(PlatformError::SecretStore).attach(format!( + "secret create failed with HTTP {} for name '{}' in store '{}'", + response.get_status(), + secret_name, + store_id + ))) + } + } + + /// Delete a secret store entry. + /// + /// # Errors + /// + /// Returns an error if the API request fails or returns an unexpected status. + pub(crate) fn delete_secret( + &self, + store_id: &str, + secret_name: &str, + ) -> Result<(), Report> { + let path = format!( + "/resources/stores/secret/{}/secrets/{}", + store_id, secret_name + ); + + let mut response = self.make_request("DELETE", &path, None, "application/json")?; + + let mut buf = String::new(); + response + .get_body_mut() + .read_to_string(&mut buf) + .change_context(PlatformError::SecretStore)?; + + if response.get_status() == StatusCode::OK + || response.get_status() == StatusCode::NO_CONTENT + { + log::debug!( + "FastlyManagementApiClient: deleted secret '{}' from store '{}'", + secret_name, + store_id + ); + Ok(()) + } else { + Err(Report::new(PlatformError::SecretStore).attach(format!( + "secret delete failed with HTTP {} for name '{}' in store '{}'", + response.get_status(), + secret_name, + store_id + ))) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn build_config_item_payload_url_encodes_reserved_characters() { + let payload = + build_config_item_payload(r#"value with spaces + symbols &= {"kid":"a+b"}"#); + + 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" + ); + } +} +``` + +- [ ] **Step 2: Add `mod management_api;` to `main.rs`** + +In `crates/trusted-server-adapter-fastly/src/main.rs`, add near the top (alongside the other `mod` declarations): + +```rust +mod management_api; +``` + +- [ ] **Step 3: Run the payload test** + +```bash +cargo test --package trusted-server-adapter-fastly management_api -- --nocapture +``` + +Expected: `build_config_item_payload_url_encodes_reserved_characters` passes. + +- [ ] **Step 4: Commit** + +```bash +git add crates/trusted-server-adapter-fastly/src/management_api.rs crates/trusted-server-adapter-fastly/src/main.rs +git commit -m "Add FastlyManagementApiClient to adapter" +``` + +--- + +### Task 3: Implement `FastlyPlatformConfigStore` write methods + +**Why:** Replace the `NotImplemented` stubs in `platform.rs` with real calls to `FastlyManagementApiClient`. The existing `NotImplemented` test for secret store (`fastly_platform_secret_store_create_returns_not_implemented`, `fastly_platform_secret_store_delete_returns_not_implemented`) must be deleted now that the real implementation lands. Check if there are equivalent config store tests to delete too. + +**Files:** +- Modify: `crates/trusted-server-adapter-fastly/src/platform.rs` + +- [ ] **Step 1: Delete `NotImplemented` tests for secret store writes** + +In `platform.rs` tests, find and delete these two tests (they assert the old stub behavior that no longer holds): +- `fastly_platform_secret_store_create_returns_not_implemented` +- `fastly_platform_secret_store_delete_returns_not_implemented` + +There are no analogous `NotImplemented` tests for `FastlyPlatformConfigStore::put/delete` — only the secret store stubs have them. No config-store equivalent to search for. + +- [ ] **Step 2: Update `FastlyPlatformConfigStore::put` and `delete`** + +In `platform.rs`, replace: + +```rust +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> { + Err(Report::new(PlatformError::NotImplemented)) +} +``` + +With: + +```rust +fn put( + &self, + store_id: &StoreId, + key: &str, + value: &str, +) -> Result<(), Report> { + let client = crate::management_api::FastlyManagementApiClient::new()?; + client.update_config_item(store_id.as_ref(), key, value) +} + +fn delete(&self, store_id: &StoreId, key: &str) -> Result<(), Report> { + let client = crate::management_api::FastlyManagementApiClient::new()?; + client.delete_config_item(store_id.as_ref(), key) +} +``` + +- [ ] **Step 3: Update `FastlyPlatformSecretStore::create` and `delete`** + +Replace: + +```rust +fn create( + &self, + _store_id: &StoreId, + _name: &str, + _value: &str, +) -> Result<(), Report> { + Err(Report::new(PlatformError::NotImplemented)) +} + +fn delete(&self, _store_id: &StoreId, _name: &str) -> Result<(), Report> { + Err(Report::new(PlatformError::NotImplemented)) +} +``` + +With: + +```rust +fn create( + &self, + store_id: &StoreId, + name: &str, + value: &str, +) -> Result<(), Report> { + let client = crate::management_api::FastlyManagementApiClient::new()?; + client.create_secret(store_id.as_ref(), name, value) +} + +fn delete(&self, store_id: &StoreId, name: &str) -> Result<(), Report> { + let client = crate::management_api::FastlyManagementApiClient::new()?; + client.delete_secret(store_id.as_ref(), name) +} +``` + +- [ ] **Step 4: Verify adapter compiles and remaining tests pass** + +```bash +cargo test --package trusted-server-adapter-fastly -- --nocapture +``` + +Expected: all tests pass (the `NotImplemented` tests were deleted; remaining tests still pass). + +- [ ] **Step 5: Commit** + +```bash +git add crates/trusted-server-adapter-fastly/src/platform.rs +git commit -m "Implement FastlyPlatformConfigStore and FastlyPlatformSecretStore write methods via management API" +``` + +--- + +### Task 4: Migrate `rotation.rs` to `RuntimeServices` + +**Why:** `KeyRotationManager` currently holds `FastlyConfigStore` (reads) and `FastlyApiClient` (writes) as fields. Replace both with `&RuntimeServices` passed to each method. + +**New design:** +- Drop `config_store: FastlyConfigStore` and `api_client: FastlyApiClient` fields +- Keep `config_store_id: StoreId` and `secret_store_id: StoreId` (passed to write methods) +- `new()` is now infallible (no API key fetch at construction time) +- All `rotate_key`, `list_active_keys`, `deactivate_key`, `delete_key` accept `services: &RuntimeServices` +- Reads use `JWKS_STORE_NAME` (edge-visible name); writes use the stored `StoreId` values + +**Files:** +- Modify: `crates/trusted-server-core/src/request_signing/rotation.rs` + +- [ ] **Step 1: Write failing tests that define the new API** + +Replace the `#[cfg(test)]` module in `rotation.rs` with: + +```rust +#[cfg(test)] +mod tests { + use std::collections::HashMap; + use std::sync::Mutex; + + use error_stack::Report; + + use crate::platform::test_support::build_services_with_config_and_secret; + use crate::platform::{ + PlatformConfigStore, PlatformError, PlatformSecretStore, StoreId, StoreName, + }; + use crate::request_signing::Keypair; + + use super::*; + + // --------------------------------------------------------------------------- + // Spy stores: record put/create/delete calls, serve preset get values + // --------------------------------------------------------------------------- + + struct SpyConfigStore { + data: Mutex>, + puts: Mutex>, + deletes: Mutex>, + } + + impl SpyConfigStore { + fn new(initial: HashMap) -> Self { + Self { + data: Mutex::new(initial), + puts: Mutex::new(vec![]), + deletes: Mutex::new(vec![]), + } + } + } + + impl PlatformConfigStore for SpyConfigStore { + fn get(&self, _: &StoreName, key: &str) -> Result> { + self.data + .lock() + .expect("should lock data") + .get(key) + .cloned() + .ok_or_else(|| Report::new(PlatformError::ConfigStore)) + } + + fn put( + &self, + store_id: &StoreId, + key: &str, + value: &str, + ) -> Result<(), Report> { + self.puts.lock().expect("should lock puts").push(( + store_id.to_string(), + key.to_string(), + value.to_string(), + )); + self.data + .lock() + .expect("should lock data") + .insert(key.to_string(), value.to_string()); + Ok(()) + } + + fn delete(&self, store_id: &StoreId, key: &str) -> Result<(), Report> { + self.deletes + .lock() + .expect("should lock deletes") + .push((store_id.to_string(), key.to_string())); + self.data + .lock() + .expect("should lock data") + .remove(key); + Ok(()) + } + } + + struct SpySecretStore { + creates: Mutex>, + deletes: Mutex>, + } + + impl SpySecretStore { + fn new() -> Self { + Self { + creates: Mutex::new(vec![]), + deletes: Mutex::new(vec![]), + } + } + } + + impl PlatformSecretStore for SpySecretStore { + fn get_bytes(&self, _: &StoreName, _: &str) -> Result, Report> { + Err(Report::new(PlatformError::SecretStore)) + } + + fn create( + &self, + store_id: &StoreId, + name: &str, + value: &str, + ) -> Result<(), Report> { + self.creates.lock().expect("should lock creates").push(( + store_id.to_string(), + name.to_string(), + value.to_string(), + )); + Ok(()) + } + + fn delete(&self, store_id: &StoreId, name: &str) -> Result<(), Report> { + self.deletes + .lock() + .expect("should lock deletes") + .push((store_id.to_string(), name.to_string())); + Ok(()) + } + } + + // --------------------------------------------------------------------------- + // Tests + // --------------------------------------------------------------------------- + + #[test] + fn generate_date_based_kid_has_correct_format() { + let kid = generate_date_based_kid(); + assert!(kid.starts_with("ts-"), "should start with 'ts-'"); + assert!(kid.len() >= 13, "should be at least 13 characters"); + let parts: Vec<&str> = kid.split('-').collect(); + assert_eq!(parts.len(), 4, "should have 4 dash-separated parts"); + assert_eq!(parts[0], "ts", "first part should be 'ts'"); + } + + #[test] + fn new_is_infallible_and_stores_ids() { + let manager = KeyRotationManager::new("cfg-store-123", "sec-store-456"); + assert_eq!( + manager.config_store_id.as_ref(), + "cfg-store-123", + "should store config_store_id" + ); + assert_eq!( + manager.secret_store_id.as_ref(), + "sec-store-456", + "should store secret_store_id" + ); + } + + #[test] + fn rotate_key_stores_private_key_via_secret_store_create() { + let config_store = SpyConfigStore::new(HashMap::new()); + let secret_store = SpySecretStore::new(); + let services = + build_services_with_config_and_secret(config_store, secret_store); + + let manager = KeyRotationManager::new("cfg-id", "sec-id"); + let result = manager.rotate_key(&services, Some("new-kid".to_string())); + + assert!(result.is_ok(), "should succeed when stores accept writes"); + let rotation = result.expect("should produce rotation result"); + assert_eq!(rotation.new_kid, "new-kid", "should use the provided kid"); + assert!( + rotation.active_kids.contains(&"new-kid".to_string()), + "should include new kid in active kids" + ); + } + + #[test] + fn deactivate_key_fails_when_only_one_key_remains() { + let mut data = HashMap::new(); + data.insert("active-kids".to_string(), "only-key".to_string()); + let config_store = SpyConfigStore::new(data); + let secret_store = SpySecretStore::new(); + let services = + build_services_with_config_and_secret(config_store, secret_store); + + let manager = KeyRotationManager::new("cfg-id", "sec-id"); + let result = manager.deactivate_key(&services, "only-key"); + + assert!( + result.is_err(), + "should fail to deactivate the last active key" + ); + } + + #[test] + fn key_rotation_result_structure_is_valid() { + let jwk = Keypair::generate().get_jwk("test-key".to_string()); + let result = KeyRotationResult { + new_kid: "ts-2024-01-01".to_string(), + previous_kid: Some("ts-2023-12-31".to_string()), + active_kids: vec![ + "ts-2023-12-31".to_string(), + "ts-2024-01-01".to_string(), + ], + jwk: jwk.clone(), + }; + + assert_eq!(result.new_kid, "ts-2024-01-01"); + assert_eq!(result.previous_kid, Some("ts-2023-12-31".to_string())); + assert_eq!(result.active_kids.len(), 2); + assert_eq!(result.jwk.prm.kid, Some("test-key".to_string())); + } +} +``` + +- [ ] **Step 2: Run tests to confirm they fail (expected compile error)** + +```bash +cargo test --package trusted-server-core request_signing::rotation 2>&1 | head -30 +``` + +Expected: compile error — `KeyRotationManager::new` still returns `Result`, and `rotate_key` doesn't take `services`. + +- [ ] **Step 3: Rewrite `rotation.rs`** + +Replace the entire file with the following (preserving `generate_date_based_kid` and `KeyRotationResult`): + +```rust +//! Key rotation management for request signing. +//! +//! This module provides functionality for rotating signing keys, managing key +//! lifecycle, and storing keys via platform store primitives through +//! [`RuntimeServices`]. + +use std::sync::LazyLock; + +use base64::{engine::general_purpose, Engine}; +use ed25519_dalek::SigningKey; +use error_stack::{Report, ResultExt}; +use jose_jwk::Jwk; + +use crate::error::TrustedServerError; +use crate::platform::{RuntimeServices, StoreId, StoreName}; +use crate::request_signing::JWKS_CONFIG_STORE_NAME; + +use super::Keypair; + +static JWKS_STORE_NAME: LazyLock = + LazyLock::new(|| StoreName::from(JWKS_CONFIG_STORE_NAME)); + +#[derive(Debug, Clone)] +pub struct KeyRotationResult { + pub new_kid: String, + pub previous_kid: Option, + pub active_kids: Vec, + pub jwk: Jwk, +} + +/// Manages signing key lifecycle using platform store primitives. +/// +/// Reads use the edge-visible store name ([`JWKS_CONFIG_STORE_NAME`]). +/// Writes use the management API store identifiers supplied at construction. +pub struct KeyRotationManager { + /// Management API store ID for config store writes. + config_store_id: StoreId, + /// Management API store ID for secret store writes. + secret_store_id: StoreId, +} + +impl KeyRotationManager { + /// Creates a new key rotation manager. + /// + /// The `config_store_id` and `secret_store_id` are platform management API + /// identifiers used for write operations. Edge reads use the store names + /// defined in [`JWKS_CONFIG_STORE_NAME`] and + /// [`crate::request_signing::SIGNING_SECRET_STORE_NAME`]. + #[must_use] + pub fn new( + config_store_id: impl Into, + secret_store_id: impl Into, + ) -> Self { + Self { + config_store_id: StoreId::from(config_store_id.into()), + secret_store_id: StoreId::from(secret_store_id.into()), + } + } + + /// Rotates the signing key by generating a new keypair and storing it. + /// + /// # Errors + /// + /// Returns an error if key storage or update operations fail. + pub fn rotate_key( + &self, + services: &RuntimeServices, + kid: Option, + ) -> Result> { + let new_kid = kid.unwrap_or_else(generate_date_based_kid); + + let keypair = Keypair::generate(); + let jwk = keypair.get_jwk(new_kid.clone()); + let previous_kid = services + .config_store() + .get(&JWKS_STORE_NAME, "current-kid") + .ok(); + + self.store_private_key(services, &new_kid, &keypair.signing_key)?; + self.store_public_jwk(services, &new_kid, &jwk)?; + + let active_kids = match &previous_kid { + Some(prev) if prev != &new_kid => vec![prev.clone(), new_kid.clone()], + _ => vec![new_kid.clone()], + }; + + self.update_current_kid(services, &new_kid)?; + self.update_active_kids(services, &active_kids)?; + + Ok(KeyRotationResult { + new_kid, + previous_kid, + active_kids, + jwk, + }) + } + + fn store_private_key( + &self, + services: &RuntimeServices, + kid: &str, + signing_key: &SigningKey, + ) -> Result<(), Report> { + let key_b64 = general_purpose::STANDARD.encode(signing_key.as_bytes()); + + services + .secret_store() + .create(&self.secret_store_id, kid, &key_b64) + .change_context(TrustedServerError::Configuration { + message: format!("failed to store private key '{}'", kid), + }) + } + + fn store_public_jwk( + &self, + services: &RuntimeServices, + kid: &str, + jwk: &Jwk, + ) -> Result<(), Report> { + let jwk_json = serde_json::to_string(jwk).map_err(|e| { + Report::new(TrustedServerError::Configuration { + message: format!("failed to serialize JWK: {}", e), + }) + })?; + + services + .config_store() + .put(&self.config_store_id, kid, &jwk_json) + .change_context(TrustedServerError::Configuration { + message: format!("failed to store public JWK '{}'", kid), + }) + } + + fn update_current_kid( + &self, + services: &RuntimeServices, + kid: &str, + ) -> Result<(), Report> { + services + .config_store() + .put(&self.config_store_id, "current-kid", kid) + .change_context(TrustedServerError::Configuration { + message: "failed to update current-kid".into(), + }) + } + + fn update_active_kids( + &self, + services: &RuntimeServices, + active_kids: &[String], + ) -> Result<(), Report> { + let active_kids_str = active_kids.join(","); + + services + .config_store() + .put(&self.config_store_id, "active-kids", &active_kids_str) + .change_context(TrustedServerError::Configuration { + message: "failed to update active-kids".into(), + }) + } + + /// Lists all active key IDs. + /// + /// # Errors + /// + /// Returns an error if the active keys cannot be retrieved from the config store. + pub fn list_active_keys( + &self, + services: &RuntimeServices, + ) -> Result, Report> { + let active_kids_str = services + .config_store() + .get(&JWKS_STORE_NAME, "active-kids") + .change_context(TrustedServerError::Configuration { + message: "failed to read active-kids from config store".into(), + })?; + + Ok(active_kids_str + .split(',') + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect()) + } + + /// Deactivates a key by removing it from the active keys list. + /// + /// # Errors + /// + /// Returns an error if this would deactivate the last active key, or if the update fails. + pub fn deactivate_key( + &self, + services: &RuntimeServices, + kid: &str, + ) -> Result<(), Report> { + let mut active_kids = self.list_active_keys(services)?; + active_kids.retain(|k| k != kid); + + if active_kids.is_empty() { + return Err(Report::new(TrustedServerError::Configuration { + message: "cannot deactivate the last active key".into(), + })); + } + + self.update_active_kids(services, &active_kids) + } + + /// Deletes a key by deactivating it and removing it from storage. + /// + /// # Errors + /// + /// Returns an error if deactivation fails or if the key cannot be deleted from storage. + pub fn delete_key( + &self, + services: &RuntimeServices, + kid: &str, + ) -> Result<(), Report> { + self.deactivate_key(services, kid)?; + + services + .config_store() + .delete(&self.config_store_id, kid) + .change_context(TrustedServerError::Configuration { + message: "failed to delete JWK from config store".into(), + })?; + + services + .secret_store() + .delete(&self.secret_store_id, kid) + .change_context(TrustedServerError::Configuration { + message: "failed to delete signing key from secret store".into(), + })?; + + Ok(()) + } +} + +#[must_use] +pub fn generate_date_based_kid() -> String { + use chrono::Utc; + format!("ts-{}", Utc::now().format("%Y-%m-%d")) +} +``` + +(Append the test module from Step 1 at the bottom.) + +- [ ] **Step 4: Run rotation tests** + +```bash +cargo test --package trusted-server-core request_signing::rotation -- --nocapture +``` + +Expected: all tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add crates/trusted-server-core/src/request_signing/rotation.rs +git commit -m "Migrate KeyRotationManager from FastlyApiClient to RuntimeServices store primitives" +``` + +--- + +### Task 5: Migrate `signing.rs` to `RuntimeServices` + +**Why:** Three items in `signing.rs` still construct `FastlyConfigStore`/`FastlySecretStore` directly. Replace all three with `RuntimeServices`. The existing viceroy-dependent tests are replaced with proper unit tests using stub stores. + +**Changed signatures:** +- `get_current_key_id()` → `get_current_key_id(services: &RuntimeServices)` +- `RequestSigner::from_config()` → `RequestSigner::from_services(services: &RuntimeServices)` (rename to make the break explicit) +- `verify_signature(payload, sig, kid)` → `verify_signature(payload, sig, kid, services: &RuntimeServices)` + +**Files:** +- Modify: `crates/trusted-server-core/src/request_signing/signing.rs` + +- [ ] **Step 1: Write failing tests for the new API** + +Replace the entire `#[cfg(test)]` module in `signing.rs` with the following (before updating the production code, so the tests fail to compile): + +```rust +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use error_stack::Report; + + use crate::platform::test_support::build_services_with_config_and_secret; + use crate::platform::{PlatformConfigStore, PlatformError, PlatformSecretStore, StoreId, StoreName}; + + use super::*; + + // --------------------------------------------------------------------------- + // Stub stores with preset data + // --------------------------------------------------------------------------- + + struct StubConfigStore(HashMap); + + impl PlatformConfigStore for StubConfigStore { + fn get(&self, _: &StoreName, key: &str) -> Result> { + self.0 + .get(key) + .cloned() + .ok_or_else(|| 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)) + } + } + + struct StubSecretStore(HashMap>); + + impl PlatformSecretStore for StubSecretStore { + fn get_bytes(&self, _: &StoreName, key: &str) -> Result, Report> { + self.0 + .get(key) + .cloned() + .ok_or_else(|| Report::new(PlatformError::SecretStore)) + } + + 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)) + } + } + + /// Build `RuntimeServices` with a real Ed25519 keypair pre-loaded in the + /// stub stores. Returns the `kid` used so callers can reference it. + fn build_signing_services() -> crate::platform::RuntimeServices { + use base64::{engine::general_purpose, Engine}; + use ed25519_dalek::SigningKey; + use rand::rngs::OsRng; + + let signing_key = SigningKey::generate(&mut OsRng); + let key_b64 = general_purpose::STANDARD.encode(signing_key.as_bytes()); + let verifying_key = signing_key.verifying_key(); + let x_b64 = general_purpose::URL_SAFE_NO_PAD.encode(verifying_key.as_bytes()); + let jwk_json = format!( + r#"{{"kty":"OKP","crv":"Ed25519","x":"{}","kid":"test-kid","alg":"EdDSA"}}"#, + x_b64 + ); + + let mut config_data = HashMap::new(); + config_data.insert("current-kid".to_string(), "test-kid".to_string()); + config_data.insert("test-kid".to_string(), jwk_json); + + let mut secret_data = HashMap::new(); + secret_data.insert("test-kid".to_string(), key_b64.into_bytes()); + + build_services_with_config_and_secret( + StubConfigStore(config_data), + StubSecretStore(secret_data), + ) + } + + // --------------------------------------------------------------------------- + // Tests + // --------------------------------------------------------------------------- + + #[test] + fn from_services_loads_kid_from_config_store() { + let services = build_signing_services(); + let signer = RequestSigner::from_services(&services) + .expect("should create signer from services"); + + assert_eq!(signer.kid, "test-kid", "should load kid from config store"); + } + + #[test] + fn sign_produces_non_empty_url_safe_base64_signature() { + let services = build_signing_services(); + let signer = RequestSigner::from_services(&services) + .expect("should create signer from services"); + + let signature = signer + .sign(b"these pretzels are making me thirsty") + .expect("should sign payload"); + + assert!(!signature.is_empty(), "should produce non-empty signature"); + assert!(signature.len() > 32, "should produce a full-length signature"); + } + + #[test] + fn sign_and_verify_roundtrip_succeeds() { + let services = build_signing_services(); + let signer = RequestSigner::from_services(&services) + .expect("should create signer from services"); + let payload = b"test payload for verification"; + + let signature = signer.sign(payload).expect("should sign payload"); + let verified = verify_signature(payload, &signature, &signer.kid, &services) + .expect("should attempt verification"); + + assert!(verified, "should verify a valid signature"); + } + + #[test] + fn verify_returns_false_for_wrong_payload() { + let services = build_signing_services(); + let signer = RequestSigner::from_services(&services) + .expect("should create signer from services"); + let signature = signer.sign(b"original").expect("should sign"); + + let verified = verify_signature(b"wrong payload", &signature, &signer.kid, &services) + .expect("should attempt verification"); + + assert!(!verified, "should not verify signature for wrong payload"); + } + + #[test] + fn verify_errors_for_unknown_kid() { + let services = build_signing_services(); + let signer = RequestSigner::from_services(&services) + .expect("should create signer from services"); + let signature = signer.sign(b"payload").expect("should sign"); + + let result = verify_signature(b"payload", &signature, "nonexistent-kid", &services); + + assert!(result.is_err(), "should error for unknown kid"); + } + + #[test] + fn verify_errors_for_malformed_signature() { + let services = build_signing_services(); + let signer = RequestSigner::from_services(&services) + .expect("should create signer from services"); + + let result = verify_signature(b"payload", "not-valid-base64!!!", &signer.kid, &services); + + assert!(result.is_err(), "should error for malformed signature"); + } + + #[test] + fn signing_params_build_payload_serializes_all_fields() { + let params = SigningParams { + request_id: "req-123".to_string(), + request_host: "example.com".to_string(), + request_scheme: "https".to_string(), + timestamp: 1706900000, + }; + + let payload = params.build_payload("kid-abc").expect("should build payload"); + let parsed: serde_json::Value = + serde_json::from_str(&payload).expect("should be valid JSON"); + + assert_eq!(parsed["version"], SIGNING_VERSION); + assert_eq!(parsed["kid"], "kid-abc"); + assert_eq!(parsed["host"], "example.com"); + assert_eq!(parsed["scheme"], "https"); + assert_eq!(parsed["id"], "req-123"); + assert_eq!(parsed["ts"], 1706900000); + } + + #[test] + fn signing_params_new_creates_recent_timestamp() { + let params = SigningParams::new( + "req-123".to_string(), + "example.com".to_string(), + "https".to_string(), + ); + + let now_ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("should get system time") + .as_millis() as u64; + + assert!( + params.timestamp <= now_ms, + "timestamp should not be in the future" + ); + assert!( + params.timestamp >= now_ms - 60_000, + "timestamp should be within the last minute" + ); + } + + #[test] + fn sign_request_enhanced_produces_verifiable_signature() { + let services = build_signing_services(); + let signer = RequestSigner::from_services(&services) + .expect("should create signer from services"); + let params = SigningParams::new( + "auction-123".to_string(), + "publisher.com".to_string(), + "https".to_string(), + ); + + let signature = signer.sign_request(¶ms).expect("should sign request"); + let payload = params.build_payload(&signer.kid).expect("should build payload"); + + let verified = + verify_signature(payload.as_bytes(), &signature, &signer.kid, &services) + .expect("should verify"); + + assert!(verified, "enhanced request signature should be verifiable"); + } + + #[test] + fn sign_request_different_hosts_produce_different_signatures() { + let services = build_signing_services(); + let signer = RequestSigner::from_services(&services) + .expect("should create signer from services"); + + let params1 = SigningParams { + request_id: "req-1".to_string(), + request_host: "host1.com".to_string(), + request_scheme: "https".to_string(), + timestamp: 1706900000, + }; + let params2 = SigningParams { + request_id: "req-1".to_string(), + request_host: "host2.com".to_string(), + request_scheme: "https".to_string(), + timestamp: 1706900000, + }; + + let sig1 = signer.sign_request(¶ms1).expect("should sign params1"); + let sig2 = signer.sign_request(¶ms2).expect("should sign params2"); + + assert_ne!( + sig1, sig2, + "different hosts should produce different signatures" + ); + } +} +``` + +- [ ] **Step 2: Run to confirm compile failure** + +```bash +cargo test --package trusted-server-core request_signing::signing 2>&1 | head -20 +``` + +Expected: compile error — `from_services`, `verify_signature` with 4 args not found. + +- [ ] **Step 3: Rewrite `signing.rs` production code** + +Replace the imports, `LazyLock` statics, and function bodies. Key changes: + +**Imports — replace:** +```rust +use crate::storage::{FastlyConfigStore, FastlySecretStore}; +``` +**With:** +```rust +use std::sync::LazyLock; + +use crate::platform::{RuntimeServices, StoreName}; +``` + +**Add after imports:** +```rust +static JWKS_STORE_NAME: LazyLock = + LazyLock::new(|| StoreName::from(JWKS_CONFIG_STORE_NAME)); + +static SIGNING_STORE_NAME: LazyLock = + LazyLock::new(|| StoreName::from(SIGNING_SECRET_STORE_NAME)); +``` + +**Replace `get_current_key_id`:** +```rust +pub fn get_current_key_id( + services: &RuntimeServices, +) -> Result> { + services + .config_store() + .get(&JWKS_STORE_NAME, "current-kid") + .change_context(TrustedServerError::Configuration { + message: "failed to read current-kid from config store".into(), + }) +} +``` + +**Replace `RequestSigner::from_config` with `from_services`:** +```rust +pub fn from_services( + services: &RuntimeServices, +) -> Result> { + let key_id = services + .config_store() + .get(&JWKS_STORE_NAME, "current-kid") + .change_context(TrustedServerError::Configuration { + message: "failed to get current-kid".into(), + })?; + + let key_bytes = services + .secret_store() + .get_bytes(&SIGNING_STORE_NAME, &key_id) + .change_context(TrustedServerError::Configuration { + message: format!("failed to get signing key for kid: {}", key_id), + })?; + + let signing_key = parse_ed25519_signing_key(key_bytes)?; + + Ok(Self { + key: signing_key, + kid: key_id, + }) +} +``` + +**Replace `verify_signature` — add `services: &RuntimeServices` parameter and replace `FastlyConfigStore::new(...)` with `services.config_store().get(&JWKS_STORE_NAME, kid)`.** + +Full new signature: +```rust +pub fn verify_signature( + payload: &[u8], + signature_b64: &str, + kid: &str, + services: &RuntimeServices, +) -> Result> { + let jwk_json = services + .config_store() + .get(&JWKS_STORE_NAME, kid) + .change_context(TrustedServerError::Configuration { + message: format!("failed to get JWK for kid: {}", kid), + })?; + // ... rest of verification logic unchanged ... +} +``` + +- [ ] **Step 4: Run signing tests** + +```bash +cargo test --package trusted-server-core request_signing::signing -- --nocapture +``` + +Expected: all tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add crates/trusted-server-core/src/request_signing/signing.rs +git commit -m "Migrate signing.rs from FastlyConfigStore/FastlySecretStore to RuntimeServices" +``` + +--- + +### Task 6: Update `endpoints.rs` to accept `&RuntimeServices` + +**Why:** Three handlers don't receive `&RuntimeServices`: `handle_verify_signature`, `handle_rotate_key`, `handle_deactivate_key`. They need it to pass to `verify_signature`, `KeyRotationManager` methods, and (for verify) `RequestSigner::from_services`. + +**Note:** `fastly::{Request, Response}` and `fastly::mime` remain — type migration is Phase 2 (PR 12). + +**Files:** +- Modify: `crates/trusted-server-core/src/request_signing/endpoints.rs` + +- [ ] **Step 1: Update `handle_verify_signature` signature and body** + +Change: +```rust +pub fn handle_verify_signature( + _settings: &Settings, + mut req: Request, +) -> Result> { +``` + +To: +```rust +pub fn handle_verify_signature( + _settings: &Settings, + services: &RuntimeServices, + mut req: Request, +) -> Result> { +``` + +Update the `verify_signature` call: +```rust +let verification_result = signing::verify_signature( + verify_req.payload.as_bytes(), + &verify_req.signature, + &verify_req.kid, + services, +); +``` + +- [ ] **Step 2: Update `handle_rotate_key` signature and body** + +Change signature to add `services: &RuntimeServices` as second parameter. Update the `KeyRotationManager` usage: + +```rust +// Before: +let manager = KeyRotationManager::new(config_store_id, secret_store_id).change_context(...)?; +match manager.rotate_key(rotate_req.kid) { ... } +manager.list_active_keys().unwrap_or_else(...) + +// After: +let manager = KeyRotationManager::new(config_store_id, secret_store_id); +match manager.rotate_key(services, rotate_req.kid) { ... } +manager.list_active_keys(services).unwrap_or_else(...) +``` + +Remove the `.change_context(...)` on `KeyRotationManager::new(...)` — it's now infallible. + +- [ ] **Step 3: Update `handle_deactivate_key` signature and body** + +Same pattern: add `services: &RuntimeServices`, update all `manager.*` calls to pass `services`: +- `manager.delete_key(&deactivate_req.kid)` → `manager.delete_key(services, &deactivate_req.kid)` +- `manager.deactivate_key(&deactivate_req.kid)` → `manager.deactivate_key(services, &deactivate_req.kid)` +- `manager.list_active_keys()` → `manager.list_active_keys(services)` + +Remove the `.change_context(...)` on `KeyRotationManager::new(...)`. + +- [ ] **Step 4: Update `endpoints.rs` tests** + +The tests in `endpoints.rs` that call `handle_verify_signature`, `handle_rotate_key`, `handle_deactivate_key` must be updated to pass a `&RuntimeServices`. Use `noop_services()` (from `test_support`) for rotation/deactivation tests (they test error paths that don't reach the stores). For `test_handle_verify_signature_valid` and `test_handle_verify_signature_invalid`, build a `RuntimeServices` with actual key material using `build_signing_services` (inline the helper or import logic). + +Also update `RequestSigner::from_config()` calls in test helpers to `RequestSigner::from_services(&services)`. + +**Add this helper to the `#[cfg(test)]` block at the top of `endpoints.rs` tests** (it cannot be imported from `signing.rs` because that function lives inside a `#[cfg(test)]` private module): + +```rust +/// Build `RuntimeServices` pre-loaded with a real Ed25519 keypair for +/// testing signature creation and verification in endpoint handlers. +fn build_signing_services_for_test() -> crate::platform::RuntimeServices { + use std::collections::HashMap; + + use base64::{engine::general_purpose, Engine}; + use ed25519_dalek::SigningKey; + use error_stack::Report; + use rand::rngs::OsRng; + + use crate::platform::test_support::build_services_with_config_and_secret; + use crate::platform::{ + PlatformConfigStore, PlatformError, PlatformSecretStore, StoreId, StoreName, + }; + + struct MapConfigStore(HashMap); + impl PlatformConfigStore for MapConfigStore { + fn get(&self, _: &StoreName, key: &str) -> Result> { + self.0.get(key).cloned().ok_or_else(|| 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)) + } + } + + struct MapSecretStore(HashMap>); + impl PlatformSecretStore for MapSecretStore { + fn get_bytes(&self, _: &StoreName, key: &str) -> Result, Report> { + self.0.get(key).cloned().ok_or_else(|| Report::new(PlatformError::SecretStore)) + } + 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)) + } + } + + let signing_key = SigningKey::generate(&mut OsRng); + let key_b64 = general_purpose::STANDARD.encode(signing_key.as_bytes()); + let x_b64 = general_purpose::URL_SAFE_NO_PAD.encode(signing_key.verifying_key().as_bytes()); + let jwk_json = format!( + r#"{{"kty":"OKP","crv":"Ed25519","x":"{}","kid":"test-kid","alg":"EdDSA"}}"#, + x_b64 + ); + + let mut cfg = HashMap::new(); + cfg.insert("current-kid".to_string(), "test-kid".to_string()); + cfg.insert("test-kid".to_string(), jwk_json); + + let mut sec = HashMap::new(); + sec.insert("test-kid".to_string(), key_b64.into_bytes()); + + build_services_with_config_and_secret(MapConfigStore(cfg), MapSecretStore(sec)) +} +``` + +Pattern for verify tests: +```rust +// In test_handle_verify_signature_valid: +let services = build_signing_services_for_test(); +let signer = crate::request_signing::RequestSigner::from_services(&services) + .expect("should create signer from services"); +// ... build req as before ... +let mut resp = handle_verify_signature(&settings, &services, req) + .expect("should handle verification request"); +``` + +For rotation/deactivation tests, `noop_services()` is fine — these tests use the `match result { Ok => log, Err => log }` pattern and do not assert against store state. The `noop_services()` causes `KeyRotationManager` methods to fail at the store read/write level, which is the expected behavior in a test environment without real stores: +```rust +let services = noop_services(); +let result = handle_rotate_key(&settings, &services, req); +// existing match-and-log pattern works unchanged +``` + +- [ ] **Step 5: Run endpoints tests** + +```bash +cargo test --package trusted-server-core request_signing::endpoints -- --nocapture +``` + +Expected: all tests pass. + +- [ ] **Step 6: Commit** + +```bash +git add crates/trusted-server-core/src/request_signing/endpoints.rs +git commit -m "Add RuntimeServices parameter to handle_verify_signature, handle_rotate_key, handle_deactivate_key" +``` + +--- + +### Task 7: Update `main.rs` to pass `runtime_services` to updated handlers + +**Why:** The adapter `main.rs` calls the three handlers without `runtime_services`. Add it. `runtime_services` is already in scope in all call sites. + +**Files:** +- Modify: `crates/trusted-server-adapter-fastly/src/main.rs` + +- [ ] **Step 1: Update the three handler call sites** + +Find (approximate lines, verify exact lines before editing): + +```rust +(Method::POST, "/verify-signature") => handle_verify_signature(settings, req), +(Method::POST, "/admin/keys/rotate") => handle_rotate_key(settings, req), +(Method::POST, "/admin/keys/deactivate") => handle_deactivate_key(settings, req), +``` + +Replace with: + +```rust +(Method::POST, "/verify-signature") => { + handle_verify_signature(settings, runtime_services, req) +} +(Method::POST, "/admin/keys/rotate") => { + handle_rotate_key(settings, runtime_services, req) +} +(Method::POST, "/admin/keys/deactivate") => { + handle_deactivate_key(settings, runtime_services, req) +} +``` + +- [ ] **Step 2: Verify the full workspace compiles** + +```bash +cargo check --workspace +``` + +Expected: no errors. + +- [ ] **Step 3: Commit** + +```bash +git add crates/trusted-server-adapter-fastly/src/main.rs +git commit -m "Pass runtime_services to signing endpoint handlers in main.rs" +``` + +--- + +### Task 8: Delete `api_client.rs` and clean up `storage/mod.rs` + +**Why:** `api_client.rs` is now fully superseded by `management_api.rs` in the adapter. No core code references `FastlyApiClient` anymore (verified by rotation.rs migration). + +**Files:** +- Delete: `crates/trusted-server-core/src/storage/api_client.rs` +- Modify: `crates/trusted-server-core/src/storage/mod.rs` + +- [ ] **Step 1: Verify zero legacy storage imports remain in `request_signing/`** + +```bash +grep -r "FastlyApiClient\|FastlyConfigStore\|FastlySecretStore" crates/trusted-server-core/src/request_signing/ +``` + +Expected: no output. If any matches appear, fix those call sites before continuing. + +Also verify no remaining references to `FastlyApiClient` anywhere in core: + +```bash +grep -r "FastlyApiClient" crates/trusted-server-core/src/ +``` + +Expected: no output. + +- [ ] **Step 2: Delete `api_client.rs`** + +```bash +rm crates/trusted-server-core/src/storage/api_client.rs +``` + +- [ ] **Step 3: Update `storage/mod.rs`** + +Remove the `api_client` module declaration and re-export. Change from: + +```rust +//! 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; +``` + +To: + +```rust +//! 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`] and +//! [`crate::platform::PlatformSecretStore`] via [`crate::platform::RuntimeServices`]. + +pub(crate) mod config_store; +pub(crate) mod secret_store; + +pub use config_store::FastlyConfigStore; +pub use secret_store::FastlySecretStore; +``` + +- [ ] **Step 4: Run the full workspace test suite** + +```bash +cargo test --workspace +``` + +Expected: all tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add crates/trusted-server-core/src/storage/mod.rs +git rm crates/trusted-server-core/src/storage/api_client.rs +git commit -m "Delete storage/api_client.rs from core; remove FastlyApiClient" +``` + +--- + +### Task 9: Run CI gates + +- [ ] **Step 1: Format check** + +```bash +cargo fmt --all -- --check +``` + +If it fails, fix with `cargo fmt --all` and re-run. + +- [ ] **Step 2: Clippy** + +```bash +cargo clippy --workspace --all-targets --all-features -- -D warnings +``` + +Fix any lints before proceeding. + +- [ ] **Step 3: Full test suite** + +```bash +cargo test --workspace +``` + +Expected: all tests pass. + +- [ ] **Step 4: Commit any lint/format fixes** + +```bash +git add -A +git commit -m "Fix clippy lints and formatting" +``` + +Only create this commit if there are actual changes. + +--- + +## Acceptance Checklist + +Verify all of the following before considering PR 9 complete: + +- [ ] `crates/trusted-server-core/src/storage/api_client.rs` no longer exists +- [ ] `crates/trusted-server-adapter-fastly/src/management_api.rs` exists +- [ ] `grep -r "FastlyApiClient\|from crate::storage::api" crates/trusted-server-core/src/request_signing/` returns no matches +- [ ] `grep -r "FastlyConfigStore\|FastlySecretStore" crates/trusted-server-core/src/request_signing/` returns no matches +- [ ] `cargo test --workspace` passes +- [ ] `cargo clippy --workspace --all-targets --all-features -- -D warnings` passes +- [ ] `cargo fmt --all -- --check` passes +- [ ] `handle_verify_signature`, `handle_rotate_key`, `handle_deactivate_key` in `endpoints.rs` all accept `&RuntimeServices` +- [ ] `FastlyPlatformConfigStore::put/delete` and `FastlyPlatformSecretStore::create/delete` in `platform.rs` no longer return `PlatformError::NotImplemented` From e13537b278ae8603593ca7906ba3cf2066a7a0e8 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Tue, 31 Mar 2026 18:53:00 +0530 Subject: [PATCH 02/14] Add build_services_with_config_and_secret to test_support --- .../src/platform/test_support.rs | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/crates/trusted-server-core/src/platform/test_support.rs b/crates/trusted-server-core/src/platform/test_support.rs index 91aa2e3d..ec93e803 100644 --- a/crates/trusted-server-core/src/platform/test_support.rs +++ b/crates/trusted-server-core/src/platform/test_support.rs @@ -257,6 +257,29 @@ impl PlatformGeo for NoopGeo { } } +/// Build a [`RuntimeServices`] instance with a custom config store and a custom secret store. +/// +/// Use this when a test exercises code that reads from config AND secret stores, +/// such as `request_signing::signing` and `request_signing::rotation`. +pub(crate) fn build_services_with_config_and_secret( + config_store: impl PlatformConfigStore + 'static, + secret_store: impl PlatformSecretStore + 'static, +) -> RuntimeServices { + RuntimeServices::builder() + .config_store(Arc::new(config_store)) + .secret_store(Arc::new(secret_store)) + .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 build_services_with_config( config_store: impl PlatformConfigStore + 'static, ) -> RuntimeServices { @@ -458,4 +481,17 @@ mod tests { let name = stub.ensure(&spec).expect("should return a backend name"); assert_eq!(name, "stub-backend", "should return fixed name"); } + + #[test] + fn build_services_with_config_and_secret_uses_provided_stores() { + // Arrange: noop stores + let services = build_services_with_config_and_secret(NoopConfigStore, NoopSecretStore); + + // Act: both stores return Unsupported (confirming the injected impls are active) + let config_result = services.config_store().get(&StoreName::from("s"), "k"); + let secret_result = services.secret_store().get_bytes(&StoreName::from("s"), "k"); + + assert!(config_result.is_err(), "should delegate to injected config store"); + assert!(secret_result.is_err(), "should delegate to injected secret store"); + } } From 2c0c4eb0e86863509772e0da15c3ff94e78e8bfc Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Wed, 1 Apr 2026 13:32:50 +0530 Subject: [PATCH 03/14] Add FastlyManagementApiClient to adapter --- .../trusted-server-adapter-fastly/Cargo.toml | 1 + .../trusted-server-adapter-fastly/src/main.rs | 1 + .../src/management_api.rs | 296 ++++++++++++++++++ 3 files changed, 298 insertions(+) create mode 100644 crates/trusted-server-adapter-fastly/src/management_api.rs diff --git a/crates/trusted-server-adapter-fastly/Cargo.toml b/crates/trusted-server-adapter-fastly/Cargo.toml index 7be7c275..2d1191cb 100644 --- a/crates/trusted-server-adapter-fastly/Cargo.toml +++ b/crates/trusted-server-adapter-fastly/Cargo.toml @@ -20,6 +20,7 @@ log-fastly = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } trusted-server-core = { path = "../trusted-server-core" } +urlencoding = { workspace = true } trusted-server-js = { path = "../js" } [dev-dependencies] diff --git a/crates/trusted-server-adapter-fastly/src/main.rs b/crates/trusted-server-adapter-fastly/src/main.rs index b9a1a814..5c7b2ae2 100644 --- a/crates/trusted-server-adapter-fastly/src/main.rs +++ b/crates/trusted-server-adapter-fastly/src/main.rs @@ -28,6 +28,7 @@ use trusted_server_core::settings::Settings; use trusted_server_core::settings_data::get_settings; mod error; +mod management_api; mod platform; use crate::error::to_error_response; diff --git a/crates/trusted-server-adapter-fastly/src/management_api.rs b/crates/trusted-server-adapter-fastly/src/management_api.rs new file mode 100644 index 00000000..94de4581 --- /dev/null +++ b/crates/trusted-server-adapter-fastly/src/management_api.rs @@ -0,0 +1,296 @@ +//! Fastly management API transport for store write operations. +//! +//! Provides [`FastlyManagementApiClient`], which wraps the Fastly REST +//! management API for write operations on config and secret stores. +//! Used by [`super::platform::FastlyPlatformConfigStore`] and +//! [`super::platform::FastlyPlatformSecretStore`] to back store write methods. +//! +//! # Credentials +//! +//! The Fastly API token is read from the `api-keys` secret store under the +//! `api_key` entry. The token must have config-store write and secret-store +//! write permissions only — no service-level admin or purge permissions. +//! +//! # Security +//! +//! Credential values are never logged. Log messages include store IDs and +//! operation names only. + +use std::io::Read; + +use error_stack::{Report, ResultExt}; +use fastly::{Request, Response}; +use fastly::http::StatusCode; +use trusted_server_core::platform::{PlatformError, PlatformSecretStore, StoreName}; + +use crate::platform::FastlyPlatformSecretStore; + +const FASTLY_API_HOST: &str = "https://api.fastly.com"; +const API_KEYS_STORE: &str = "api-keys"; +const API_KEY_ENTRY: &str = "api_key"; + +pub(crate) fn build_config_item_payload(value: &str) -> String { + format!("item_value={}", urlencoding::encode(value)) +} + +/// HTTP client for Fastly management API write operations. +/// +/// Backs the `put`/`delete` methods of [`FastlyPlatformConfigStore`] and +/// the `create`/`delete` methods of [`FastlyPlatformSecretStore`]. +pub(crate) struct FastlyManagementApiClient { + api_key: Vec, + base_url: &'static str, + backend_name: String, +} + +impl FastlyManagementApiClient { + /// Initialize the client by reading the API token from the `api-keys` secret store. + /// + /// # Errors + /// + /// Returns [`PlatformError::Backend`] if the management API backend cannot + /// be registered, or [`PlatformError::SecretStore`] if the API key cannot + /// be read. + pub(crate) fn new() -> Result> { + use trusted_server_core::backend::BackendConfig; + + let backend_name = BackendConfig::from_url(FASTLY_API_HOST, true) + .change_context(PlatformError::Backend) + .attach("failed to register Fastly management API backend")?; + + let api_key = FastlyPlatformSecretStore + .get_bytes(&StoreName::from(API_KEYS_STORE), API_KEY_ENTRY) + .change_context(PlatformError::SecretStore) + .attach("failed to read Fastly API key from secret store")?; + + log::debug!("FastlyManagementApiClient: initialized for management API operations"); + + Ok(Self { + api_key, + base_url: FASTLY_API_HOST, + backend_name, + }) + } + + fn make_request( + &self, + method: &str, + path: &str, + body: Option, + 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 { + "GET" => Request::get(&url), + "POST" => Request::post(&url), + "PUT" => Request::put(&url), + "DELETE" => Request::delete(&url), + _ => { + return Err(Report::new(PlatformError::ConfigStore) + .attach(format!("unsupported HTTP method: {}", method))) + } + }; + + request = request + .with_header("Fastly-Key", api_key_str) + .with_header("Accept", "application/json"); + + if let Some(body_content) = body { + request = request + .with_header("Content-Type", content_type) + .with_body(body_content); + } + + request.send(&self.backend_name).map_err(|e| { + Report::new(PlatformError::ConfigStore) + .attach(format!("management API request failed: {}", e)) + }) + } + + /// Update or create a config store item. + /// + /// # Errors + /// + /// Returns an error if the API request fails or returns a non-OK status. + pub(crate) fn update_config_item( + &self, + store_id: &str, + key: &str, + value: &str, + ) -> Result<(), Report> { + let path = format!("/resources/stores/config/{}/item/{}", store_id, key); + let payload = build_config_item_payload(value); + + let mut response = self.make_request( + "PUT", + &path, + Some(payload), + "application/x-www-form-urlencoded", + )?; + + let mut buf = String::new(); + response + .get_body_mut() + .read_to_string(&mut buf) + .change_context(PlatformError::ConfigStore)?; + + if response.get_status() == StatusCode::OK { + log::debug!( + "FastlyManagementApiClient: updated config key '{}' in store '{}'", + key, + store_id + ); + Ok(()) + } else { + Err(Report::new(PlatformError::ConfigStore).attach(format!( + "config item update failed with HTTP {} for key '{}' in store '{}'", + response.get_status(), + key, + store_id + ))) + } + } + + /// Delete a config store item. + /// + /// # Errors + /// + /// Returns an error if the API request fails or returns an unexpected status. + pub(crate) fn delete_config_item( + &self, + store_id: &str, + key: &str, + ) -> Result<(), Report> { + let path = format!("/resources/stores/config/{}/item/{}", store_id, key); + + let mut response = self.make_request("DELETE", &path, None, "application/json")?; + + let mut buf = String::new(); + response + .get_body_mut() + .read_to_string(&mut buf) + .change_context(PlatformError::ConfigStore)?; + + if response.get_status() == StatusCode::OK + || response.get_status() == StatusCode::NO_CONTENT + { + log::debug!( + "FastlyManagementApiClient: deleted config key '{}' from store '{}'", + key, + store_id + ); + Ok(()) + } else { + Err(Report::new(PlatformError::ConfigStore).attach(format!( + "config item delete failed with HTTP {} for key '{}' in store '{}'", + response.get_status(), + key, + store_id + ))) + } + } + + /// Create or overwrite a secret store entry. + /// + /// # Errors + /// + /// Returns an error if the API request fails or returns a non-OK status. + pub(crate) fn create_secret( + &self, + store_id: &str, + secret_name: &str, + 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 + }); + + let mut response = + self.make_request("POST", &path, Some(payload.to_string()), "application/json")?; + + let mut buf = String::new(); + response + .get_body_mut() + .read_to_string(&mut buf) + .change_context(PlatformError::SecretStore)?; + + if response.get_status() == StatusCode::OK { + log::debug!( + "FastlyManagementApiClient: created secret '{}' in store '{}'", + secret_name, + store_id + ); + Ok(()) + } else { + Err(Report::new(PlatformError::SecretStore).attach(format!( + "secret create failed with HTTP {} for name '{}' in store '{}'", + response.get_status(), + secret_name, + store_id + ))) + } + } + + /// Delete a secret store entry. + /// + /// # Errors + /// + /// Returns an error if the API request fails or returns an unexpected status. + pub(crate) fn delete_secret( + &self, + store_id: &str, + secret_name: &str, + ) -> Result<(), Report> { + let path = format!( + "/resources/stores/secret/{}/secrets/{}", + store_id, secret_name + ); + + let mut response = self.make_request("DELETE", &path, None, "application/json")?; + + let mut buf = String::new(); + response + .get_body_mut() + .read_to_string(&mut buf) + .change_context(PlatformError::SecretStore)?; + + if response.get_status() == StatusCode::OK + || response.get_status() == StatusCode::NO_CONTENT + { + log::debug!( + "FastlyManagementApiClient: deleted secret '{}' from store '{}'", + secret_name, + store_id + ); + Ok(()) + } else { + Err(Report::new(PlatformError::SecretStore).attach(format!( + "secret delete failed with HTTP {} for name '{}' in store '{}'", + response.get_status(), + secret_name, + store_id + ))) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn build_config_item_payload_url_encodes_reserved_characters() { + let payload = + build_config_item_payload(r#"value with spaces + symbols &= {"kid":"a+b"}"#); + + 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" + ); + } +} From f6b00c81c16badbde2fdc1510a19ef4d3bd1f9d0 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Wed, 1 Apr 2026 13:36:47 +0530 Subject: [PATCH 04/14] Implement FastlyPlatformConfigStore and FastlyPlatformSecretStore write methods via management API Replace FastlyApiClient with FastlyManagementApiClient in the put/delete methods of FastlyPlatformConfigStore and the create/delete methods of FastlyPlatformSecretStore. Remove the now-unused FastlyApiClient import. --- .../src/platform.rs | 36 ++++++------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/crates/trusted-server-adapter-fastly/src/platform.rs b/crates/trusted-server-adapter-fastly/src/platform.rs index 93d86dc5..b934aee9 100644 --- a/crates/trusted-server-adapter-fastly/src/platform.rs +++ b/crates/trusted-server-adapter-fastly/src/platform.rs @@ -22,8 +22,6 @@ use trusted_server_core::platform::{ PlatformResponse, PlatformSecretStore, PlatformSelectResult, RuntimeServices, StoreId, StoreName, }; -use trusted_server_core::storage::FastlyApiClient; - pub(crate) use trusted_server_core::platform::UnavailableKvStore; // --------------------------------------------------------------------------- @@ -38,8 +36,8 @@ pub(crate) use trusted_server_core::platform::UnavailableKvStore; /// /// # 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 +/// `put` and `delete` construct a [`FastlyManagementApiClient`] 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 @@ -67,19 +65,13 @@ 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) + let client = crate::management_api::FastlyManagementApiClient::new()?; + client.update_config_item(store_id.as_ref(), key, value) } 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) + let client = crate::management_api::FastlyManagementApiClient::new()?; + client.delete_config_item(store_id.as_ref(), key) } } @@ -95,7 +87,7 @@ impl PlatformConfigStore for FastlyPlatformConfigStore { /// /// # Write cost /// -/// `create` and `delete` have the same per-call [`FastlyApiClient`] cost +/// `create` and `delete` have the same per-call [`FastlyManagementApiClient`] cost /// described on [`FastlyPlatformConfigStore`]. pub struct FastlyPlatformSecretStore; @@ -138,19 +130,13 @@ impl PlatformSecretStore for FastlyPlatformSecretStore { 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) + let client = crate::management_api::FastlyManagementApiClient::new()?; + client.create_secret(store_id.as_ref(), name, value) } 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) + let client = crate::management_api::FastlyManagementApiClient::new()?; + client.delete_secret(store_id.as_ref(), name) } } From ec62970d6f6e61f7197bd58c88f7d9e0a62ca25d Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Wed, 1 Apr 2026 15:45:04 +0530 Subject: [PATCH 05/14] Migrate KeyRotationManager from FastlyApiClient to RuntimeServices store primitives --- .../trusted-server-adapter-fastly/src/main.rs | 4 +- .../src/request_signing/endpoints.rs | 34 +- .../src/request_signing/rotation.rs | 381 +++++++++++++----- 3 files changed, 293 insertions(+), 126 deletions(-) diff --git a/crates/trusted-server-adapter-fastly/src/main.rs b/crates/trusted-server-adapter-fastly/src/main.rs index 5c7b2ae2..aa902767 100644 --- a/crates/trusted-server-adapter-fastly/src/main.rs +++ b/crates/trusted-server-adapter-fastly/src/main.rs @@ -156,8 +156,8 @@ async fn route_request( // Key rotation admin endpoints // Keep in sync with Settings::ADMIN_ENDPOINTS in crates/trusted-server-core/src/settings.rs - (Method::POST, "/admin/keys/rotate") => handle_rotate_key(settings, req), - (Method::POST, "/admin/keys/deactivate") => handle_deactivate_key(settings, req), + (Method::POST, "/admin/keys/rotate") => handle_rotate_key(settings, runtime_services, req), + (Method::POST, "/admin/keys/deactivate") => handle_deactivate_key(settings, runtime_services, req), // Unified auction endpoint (returns creative HTML inline) (Method::POST, "/auction") => { diff --git a/crates/trusted-server-core/src/request_signing/endpoints.rs b/crates/trusted-server-core/src/request_signing/endpoints.rs index 3b14317e..b49c9b31 100644 --- a/crates/trusted-server-core/src/request_signing/endpoints.rs +++ b/crates/trusted-server-core/src/request_signing/endpoints.rs @@ -147,6 +147,7 @@ pub struct RotateKeyResponse { /// Returns an error if the request signing settings are missing, JSON parsing fails, or key rotation fails. pub fn handle_rotate_key( settings: &Settings, + services: &RuntimeServices, mut req: Request, ) -> Result> { let (config_store_id, secret_store_id) = match &settings.request_signing { @@ -168,13 +169,9 @@ pub fn handle_rotate_key( })? }; - let manager = KeyRotationManager::new(config_store_id, secret_store_id).change_context( - TrustedServerError::Configuration { - message: "failed to create KeyRotationManager".into(), - }, - )?; + let manager = KeyRotationManager::new(config_store_id, secret_store_id); - match manager.rotate_key(rotate_req.kid) { + match manager.rotate_key(services, rotate_req.kid) { Ok(result) => { let jwk_value = serde_json::to_value(&result.jwk).map_err(|e| { Report::new(TrustedServerError::Configuration { @@ -251,6 +248,7 @@ pub struct DeactivateKeyResponse { /// Returns an error if the request signing settings are missing, JSON parsing fails, or key deactivation fails. pub fn handle_deactivate_key( settings: &Settings, + services: &RuntimeServices, mut req: Request, ) -> Result> { let (config_store_id, secret_store_id) = match &settings.request_signing { @@ -269,21 +267,17 @@ pub fn handle_deactivate_key( 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(), - }, - )?; + let manager = KeyRotationManager::new(config_store_id, secret_store_id); let result = if deactivate_req.delete { - manager.delete_key(&deactivate_req.kid) + manager.delete_key(services, &deactivate_req.kid) } else { - manager.deactivate_key(&deactivate_req.kid) + manager.deactivate_key(services, &deactivate_req.kid) }; match result { Ok(()) => { - let remaining_keys = manager.list_active_keys().unwrap_or_else(|e| { + let remaining_keys = manager.list_active_keys(services).unwrap_or_else(|e| { log::warn!("failed to list active keys after deactivation: {}", e); vec![] }); @@ -475,7 +469,7 @@ mod tests { let settings = crate::test_support::tests::create_test_settings(); let req = Request::new(Method::POST, "https://test.com/admin/keys/rotate"); - let result = handle_rotate_key(&settings, req); + let result = handle_rotate_key(&settings, &noop_services(), req); match result { Ok(mut resp) => { let body = resp.take_body_str(); @@ -503,7 +497,7 @@ mod tests { let mut req = Request::new(Method::POST, "https://test.com/admin/keys/rotate"); req.set_body(body_json); - let result = handle_rotate_key(&settings, req); + let result = handle_rotate_key(&settings, &noop_services(), req); match result { Ok(mut resp) => { let body = resp.take_body_str(); @@ -525,7 +519,7 @@ mod tests { let mut req = Request::new(Method::POST, "https://test.com/admin/keys/rotate"); req.set_body("invalid json"); - let result = handle_rotate_key(&settings, req); + let result = handle_rotate_key(&settings, &noop_services(), req); assert!(result.is_err(), "Invalid JSON should return error"); } @@ -543,7 +537,7 @@ mod tests { let mut req = Request::new(Method::POST, "https://test.com/admin/keys/deactivate"); req.set_body(body_json); - let result = handle_deactivate_key(&settings, req); + let result = handle_deactivate_key(&settings, &noop_services(), req); match result { Ok(mut resp) => { let body = resp.take_body_str(); @@ -573,7 +567,7 @@ mod tests { let mut req = Request::new(Method::POST, "https://test.com/admin/keys/deactivate"); req.set_body(body_json); - let result = handle_deactivate_key(&settings, req); + let result = handle_deactivate_key(&settings, &noop_services(), req); match result { Ok(mut resp) => { let body = resp.take_body_str(); @@ -595,7 +589,7 @@ mod tests { let mut req = Request::new(Method::POST, "https://test.com/admin/keys/deactivate"); req.set_body("invalid json"); - let result = handle_deactivate_key(&settings, req); + let result = handle_deactivate_key(&settings, &noop_services(), req); assert!(result.is_err(), "Invalid JSON should return error"); } diff --git a/crates/trusted-server-core/src/request_signing/rotation.rs b/crates/trusted-server-core/src/request_signing/rotation.rs index 252ccd59..2a72507d 100644 --- a/crates/trusted-server-core/src/request_signing/rotation.rs +++ b/crates/trusted-server-core/src/request_signing/rotation.rs @@ -1,7 +1,10 @@ //! Key rotation management for request signing. //! -//! This module provides functionality for rotating signing keys, managing key lifecycle, -//! and storing keys in Fastly Config and Secret stores. +//! This module provides functionality for rotating signing keys, managing key +//! lifecycle, and storing keys via platform store primitives through +//! [`RuntimeServices`]. + +use std::sync::LazyLock; use base64::{engine::general_purpose, Engine}; use ed25519_dalek::SigningKey; @@ -9,11 +12,14 @@ use error_stack::{Report, ResultExt}; use jose_jwk::Jwk; use crate::error::TrustedServerError; +use crate::platform::{RuntimeServices, StoreId, StoreName}; use crate::request_signing::JWKS_CONFIG_STORE_NAME; -use crate::storage::{FastlyApiClient, FastlyConfigStore}; use super::Keypair; +static JWKS_STORE_NAME: LazyLock = + LazyLock::new(|| StoreName::from(JWKS_CONFIG_STORE_NAME)); + #[derive(Debug, Clone)] pub struct KeyRotationResult { pub new_kid: String, @@ -22,45 +28,33 @@ pub struct KeyRotationResult { pub jwk: Jwk, } -#[allow(deprecated)] +/// Manages signing key lifecycle using platform store primitives. +/// +/// Reads use the edge-visible store name ([`JWKS_CONFIG_STORE_NAME`]). +/// Writes use the management API store identifiers supplied at construction. pub struct KeyRotationManager { - /// Edge-side config store for reading JWKS (uses store name). - config_store: FastlyConfigStore, - /// Management API client for writing to stores (uses store IDs). - api_client: FastlyApiClient, - /// Fastly API store ID for config store writes. - config_store_id: String, - /// Fastly API store ID for secret store writes. - secret_store_id: String, + /// Management API store ID for config store writes. + config_store_id: StoreId, + /// Management API store ID for secret store writes. + secret_store_id: StoreId, } -#[allow(deprecated)] impl KeyRotationManager { /// Creates a new key rotation manager. /// - /// The `config_store_id` and `secret_store_id` are Fastly management API + /// The `config_store_id` and `secret_store_id` are platform management API /// identifiers used for write operations. Edge reads use the store names - /// defined in [`JWKS_CONFIG_STORE_NAME`] and [`crate::request_signing::SIGNING_SECRET_STORE_NAME`]. - /// - /// # Errors - /// - /// Returns an error if the API client cannot be initialized. + /// defined in [`JWKS_CONFIG_STORE_NAME`] and + /// [`crate::request_signing::SIGNING_SECRET_STORE_NAME`]. + #[must_use] pub fn new( config_store_id: impl Into, secret_store_id: impl Into, - ) -> Result> { - let config_store_id = config_store_id.into(); - let secret_store_id = secret_store_id.into(); - - let config_store = FastlyConfigStore::new(JWKS_CONFIG_STORE_NAME); - let api_client = FastlyApiClient::new()?; - - Ok(Self { - config_store, - api_client, - config_store_id, - secret_store_id, - }) + ) -> Self { + Self { + config_store_id: StoreId::from(config_store_id.into()), + secret_store_id: StoreId::from(secret_store_id.into()), + } } /// Rotates the signing key by generating a new keypair and storing it. @@ -70,24 +64,28 @@ impl KeyRotationManager { /// Returns an error if key storage or update operations fail. pub fn rotate_key( &self, + services: &RuntimeServices, kid: Option, ) -> Result> { let new_kid = kid.unwrap_or_else(generate_date_based_kid); let keypair = Keypair::generate(); let jwk = keypair.get_jwk(new_kid.clone()); - let previous_kid = self.config_store.get("current-kid").ok(); + let previous_kid = services + .config_store() + .get(&JWKS_STORE_NAME, "current-kid") + .ok(); - self.store_private_key(&new_kid, &keypair.signing_key)?; - self.store_public_jwk(&new_kid, &jwk)?; + self.store_private_key(services, &new_kid, &keypair.signing_key)?; + self.store_public_jwk(services, &new_kid, &jwk)?; let active_kids = match &previous_kid { Some(prev) if prev != &new_kid => vec![prev.clone(), new_kid.clone()], _ => vec![new_kid.clone()], }; - self.update_current_kid(&new_kid)?; - self.update_active_kids(&active_kids)?; + self.update_current_kid(services, &new_kid)?; + self.update_active_kids(services, &active_kids)?; Ok(KeyRotationResult { new_kid, @@ -99,48 +97,65 @@ impl KeyRotationManager { fn store_private_key( &self, + services: &RuntimeServices, kid: &str, signing_key: &SigningKey, ) -> Result<(), Report> { - let key_bytes = signing_key.as_bytes(); - let key_b64 = general_purpose::STANDARD.encode(key_bytes); + let key_b64 = general_purpose::STANDARD.encode(signing_key.as_bytes()); - self.api_client - .create_secret(&self.secret_store_id, kid, &key_b64) + services + .secret_store() + .create(&self.secret_store_id, kid, &key_b64) .change_context(TrustedServerError::Configuration { - message: format!("Failed to store private key '{}'", kid), + message: format!("failed to store private key '{}'", kid), }) } - fn store_public_jwk(&self, kid: &str, jwk: &Jwk) -> Result<(), Report> { + fn store_public_jwk( + &self, + services: &RuntimeServices, + kid: &str, + jwk: &Jwk, + ) -> Result<(), Report> { let jwk_json = serde_json::to_string(jwk).map_err(|e| { Report::new(TrustedServerError::Configuration { - message: format!("Failed to serialize JWK: {}", e), + message: format!("failed to serialize JWK: {}", e), }) })?; - self.api_client - .update_config_item(&self.config_store_id, kid, &jwk_json) + services + .config_store() + .put(&self.config_store_id, kid, &jwk_json) .change_context(TrustedServerError::Configuration { - message: format!("Failed to store public JWK '{}'", kid), + message: format!("failed to store public JWK '{}'", kid), }) } - fn update_current_kid(&self, kid: &str) -> Result<(), Report> { - self.api_client - .update_config_item(&self.config_store_id, "current-kid", kid) + fn update_current_kid( + &self, + services: &RuntimeServices, + kid: &str, + ) -> Result<(), Report> { + services + .config_store() + .put(&self.config_store_id, "current-kid", kid) .change_context(TrustedServerError::Configuration { - message: "Failed to update current-kid".into(), + message: "failed to update current-kid".into(), }) } - fn update_active_kids(&self, active_kids: &[String]) -> Result<(), Report> { + fn update_active_kids( + &self, + services: &RuntimeServices, + active_kids: &[String], + ) -> Result<(), Report> { let active_kids_str = active_kids.join(","); - self.api_client - .update_config_item(&self.config_store_id, "active-kids", &active_kids_str) + services + .config_store() + .put(&self.config_store_id, "active-kids", &active_kids_str) .change_context(TrustedServerError::Configuration { - message: "Failed to update active-kids".into(), + message: "failed to update active-kids".into(), }) } @@ -149,16 +164,22 @@ impl KeyRotationManager { /// # Errors /// /// Returns an error if the active keys cannot be retrieved from the config store. - pub fn list_active_keys(&self) -> Result, Report> { - let active_kids_str = self.config_store.get("active-kids")?; + pub fn list_active_keys( + &self, + services: &RuntimeServices, + ) -> Result, Report> { + let active_kids_str = services + .config_store() + .get(&JWKS_STORE_NAME, "active-kids") + .change_context(TrustedServerError::Configuration { + message: "failed to read active-kids from config store".into(), + })?; - let active_kids: Vec = active_kids_str + Ok(active_kids_str .split(',') .map(|s| s.trim().to_string()) .filter(|s| !s.is_empty()) - .collect(); - - Ok(active_kids) + .collect()) } /// Deactivates a key by removing it from the active keys list. @@ -166,18 +187,21 @@ impl KeyRotationManager { /// # Errors /// /// Returns an error if this would deactivate the last active key, or if the update fails. - pub fn deactivate_key(&self, kid: &str) -> Result<(), Report> { - let mut active_kids = self.list_active_keys()?; - + pub fn deactivate_key( + &self, + services: &RuntimeServices, + kid: &str, + ) -> Result<(), Report> { + let mut active_kids = self.list_active_keys(services)?; active_kids.retain(|k| k != kid); if active_kids.is_empty() { return Err(Report::new(TrustedServerError::Configuration { - message: "Cannot deactivate the last active key".into(), + message: "cannot deactivate the last active key".into(), })); } - self.update_active_kids(&active_kids) + self.update_active_kids(services, &active_kids) } /// Deletes a key by deactivating it and removing it from storage. @@ -185,25 +209,32 @@ impl KeyRotationManager { /// # Errors /// /// Returns an error if deactivation fails or if the key cannot be deleted from storage. - pub fn delete_key(&self, kid: &str) -> Result<(), Report> { - self.deactivate_key(kid)?; + pub fn delete_key( + &self, + services: &RuntimeServices, + kid: &str, + ) -> Result<(), Report> { + self.deactivate_key(services, kid)?; - self.api_client - .delete_config_item(&self.config_store_id, kid) + services + .config_store() + .delete(&self.config_store_id, kid) .change_context(TrustedServerError::Configuration { - message: "Failed to delete JWK from ConfigStore".into(), + message: "failed to delete JWK from config store".into(), })?; - self.api_client - .delete_secret(&self.secret_store_id, kid) + services + .secret_store() + .delete(&self.secret_store_id, kid) .change_context(TrustedServerError::Configuration { - message: "Failed to delete secret from SecretStore".into(), + message: "failed to delete signing key from secret store".into(), })?; Ok(()) } } +/// Generates a date-based key ID in the format `ts-YYYY-MM-DD`. #[must_use] pub fn generate_date_based_kid() -> String { use chrono::Utc; @@ -212,56 +243,198 @@ pub fn generate_date_based_kid() -> String { #[cfg(test)] mod tests { + use std::collections::HashMap; + use std::sync::Mutex; + + use error_stack::Report; + + use crate::platform::test_support::build_services_with_config_and_secret; + use crate::platform::{ + PlatformConfigStore, PlatformError, PlatformSecretStore, StoreId, StoreName, + }; use crate::request_signing::Keypair; use super::*; + // --------------------------------------------------------------------------- + // Spy stores: record put/create/delete calls, serve preset get values + // --------------------------------------------------------------------------- + + struct SpyConfigStore { + data: Mutex>, + puts: Mutex>, + deletes: Mutex>, + } + + impl SpyConfigStore { + fn new(initial: HashMap) -> Self { + Self { + data: Mutex::new(initial), + puts: Mutex::new(vec![]), + deletes: Mutex::new(vec![]), + } + } + } + + impl PlatformConfigStore for SpyConfigStore { + fn get(&self, _: &StoreName, key: &str) -> Result> { + self.data + .lock() + .expect("should lock data") + .get(key) + .cloned() + .ok_or_else(|| Report::new(PlatformError::ConfigStore)) + } + + fn put( + &self, + store_id: &StoreId, + key: &str, + value: &str, + ) -> Result<(), Report> { + self.puts.lock().expect("should lock puts").push(( + store_id.to_string(), + key.to_string(), + value.to_string(), + )); + self.data + .lock() + .expect("should lock data") + .insert(key.to_string(), value.to_string()); + Ok(()) + } + + fn delete(&self, store_id: &StoreId, key: &str) -> Result<(), Report> { + self.deletes + .lock() + .expect("should lock deletes") + .push((store_id.to_string(), key.to_string())); + self.data + .lock() + .expect("should lock data") + .remove(key); + Ok(()) + } + } + + struct SpySecretStore { + creates: Mutex>, + deletes: Mutex>, + } + + impl SpySecretStore { + fn new() -> Self { + Self { + creates: Mutex::new(vec![]), + deletes: Mutex::new(vec![]), + } + } + } + + impl PlatformSecretStore for SpySecretStore { + fn get_bytes(&self, _: &StoreName, _: &str) -> Result, Report> { + Err(Report::new(PlatformError::SecretStore)) + } + + fn create( + &self, + store_id: &StoreId, + name: &str, + value: &str, + ) -> Result<(), Report> { + self.creates.lock().expect("should lock creates").push(( + store_id.to_string(), + name.to_string(), + value.to_string(), + )); + Ok(()) + } + + fn delete(&self, store_id: &StoreId, name: &str) -> Result<(), Report> { + self.deletes + .lock() + .expect("should lock deletes") + .push((store_id.to_string(), name.to_string())); + Ok(()) + } + } + + // --------------------------------------------------------------------------- + // Tests + // --------------------------------------------------------------------------- + #[test] - fn test_generate_date_based_kid() { + fn generate_date_based_kid_has_correct_format() { let kid = generate_date_based_kid(); - // Verify format: ts-YYYY-MM-DD - assert!(kid.starts_with("ts-")); - assert!(kid.len() >= 13); + assert!(kid.starts_with("ts-"), "should start with 'ts-'"); + assert!(kid.len() >= 13, "should be at least 13 characters"); let parts: Vec<&str> = kid.split('-').collect(); - assert_eq!(parts.len(), 4); - assert_eq!(parts[0], "ts"); + assert_eq!(parts.len(), 4, "should have 4 dash-separated parts"); + assert_eq!(parts[0], "ts", "first part should be 'ts'"); } #[test] - fn test_key_rotation_manager_creation() { - let result = KeyRotationManager::new("test-config-store-id", "test-secret-store-id"); - match result { - Ok(manager) => { - assert_eq!(manager.config_store_id, "test-config-store-id"); - assert_eq!(manager.secret_store_id, "test-secret-store-id"); - } - Err(e) => { - println!("Expected error in test environment: {}", e); - } - } + fn new_is_infallible_and_stores_ids() { + let manager = KeyRotationManager::new("cfg-store-123", "sec-store-456"); + assert_eq!( + manager.config_store_id.as_ref(), + "cfg-store-123", + "should store config_store_id" + ); + assert_eq!( + manager.secret_store_id.as_ref(), + "sec-store-456", + "should store secret_store_id" + ); } #[test] - fn test_list_active_keys() { - let result = KeyRotationManager::new("test-config-store-id", "test-secret-store-id"); - if let Ok(manager) = result { - match manager.list_active_keys() { - Ok(keys) => { - assert!(!keys.is_empty(), "Should have at least one active key"); - } - Err(e) => println!("Expected error in test environment: {}", e), - } - } + fn rotate_key_stores_private_key_via_secret_store_create() { + let config_store = SpyConfigStore::new(HashMap::new()); + let secret_store = SpySecretStore::new(); + let services = + build_services_with_config_and_secret(config_store, secret_store); + + let manager = KeyRotationManager::new("cfg-id", "sec-id"); + let result = manager.rotate_key(&services, Some("new-kid".to_string())); + + assert!(result.is_ok(), "should succeed when stores accept writes"); + let rotation = result.expect("should produce rotation result"); + assert_eq!(rotation.new_kid, "new-kid", "should use the provided kid"); + assert!( + rotation.active_kids.contains(&"new-kid".to_string()), + "should include new kid in active kids" + ); } #[test] - fn test_key_rotation_result_structure() { - let jwk = Keypair::generate().get_jwk("test-key".to_string()); + fn deactivate_key_fails_when_only_one_key_remains() { + let mut data = HashMap::new(); + data.insert("active-kids".to_string(), "only-key".to_string()); + let config_store = SpyConfigStore::new(data); + let secret_store = SpySecretStore::new(); + let services = + build_services_with_config_and_secret(config_store, secret_store); + + let manager = KeyRotationManager::new("cfg-id", "sec-id"); + let result = manager.deactivate_key(&services, "only-key"); + + assert!( + result.is_err(), + "should fail to deactivate the last active key" + ); + } + #[test] + fn key_rotation_result_structure_is_valid() { + let jwk = Keypair::generate().get_jwk("test-key".to_string()); let result = KeyRotationResult { new_kid: "ts-2024-01-01".to_string(), previous_kid: Some("ts-2023-12-31".to_string()), - active_kids: vec!["ts-2023-12-31".to_string(), "ts-2024-01-01".to_string()], + active_kids: vec![ + "ts-2023-12-31".to_string(), + "ts-2024-01-01".to_string(), + ], jwk: jwk.clone(), }; From 27a0949de367ae69adbd6aeb0283680e0c5a9a2a Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Wed, 1 Apr 2026 18:05:32 +0530 Subject: [PATCH 06/14] Migrate signing.rs from FastlyConfigStore/FastlySecretStore to RuntimeServices --- .../trusted-server-adapter-fastly/src/main.rs | 4 +- .../src/integrations/prebid.rs | 1 + .../src/request_signing/endpoints.rs | 94 ++++-- .../src/request_signing/signing.rs | 312 +++++++++++++----- 4 files changed, 297 insertions(+), 114 deletions(-) diff --git a/crates/trusted-server-adapter-fastly/src/main.rs b/crates/trusted-server-adapter-fastly/src/main.rs index aa902767..2180c676 100644 --- a/crates/trusted-server-adapter-fastly/src/main.rs +++ b/crates/trusted-server-adapter-fastly/src/main.rs @@ -152,7 +152,9 @@ async fn route_request( } // Signature verification endpoint - (Method::POST, "/verify-signature") => handle_verify_signature(settings, req), + (Method::POST, "/verify-signature") => { + handle_verify_signature(settings, runtime_services, req) + } // Key rotation admin endpoints // Keep in sync with Settings::ADMIN_ENDPOINTS in crates/trusted-server-core/src/settings.rs diff --git a/crates/trusted-server-core/src/integrations/prebid.rs b/crates/trusted-server-core/src/integrations/prebid.rs index a60bd1ec..a78d4ea1 100644 --- a/crates/trusted-server-core/src/integrations/prebid.rs +++ b/crates/trusted-server-core/src/integrations/prebid.rs @@ -1009,6 +1009,7 @@ impl AuctionProvider for PrebidAuctionProvider { { if request_signing_config.enabled { let request_info = RequestInfo::from_request(context.request, context.client_info); + #[allow(deprecated)] let signer = RequestSigner::from_config()?; let params = SigningParams::new(request.id.clone(), request_info.host, request_info.scheme); diff --git a/crates/trusted-server-core/src/request_signing/endpoints.rs b/crates/trusted-server-core/src/request_signing/endpoints.rs index b49c9b31..93aa6eea 100644 --- a/crates/trusted-server-core/src/request_signing/endpoints.rs +++ b/crates/trusted-server-core/src/request_signing/endpoints.rs @@ -76,6 +76,7 @@ pub struct VerifySignatureResponse { /// Returns an error if the request body cannot be parsed as JSON or if verification fails. pub fn handle_verify_signature( _settings: &Settings, + services: &RuntimeServices, mut req: Request, ) -> Result> { let body = req.take_body_str(); @@ -88,6 +89,7 @@ pub fn handle_verify_signature( verify_req.payload.as_bytes(), &verify_req.signature, &verify_req.kid, + services, ); let response = match verification_result { @@ -334,16 +336,69 @@ pub fn handle_deactivate_key( #[cfg(test)] mod tests { + use std::collections::HashMap; + use error_stack::Report; use crate::platform::{ - test_support::{build_services_with_config, noop_services}, - PlatformConfigStore, PlatformError, StoreId, StoreName, + test_support::{build_services_with_config, build_services_with_config_and_secret, noop_services}, + PlatformConfigStore, PlatformError, PlatformSecretStore, StoreId, StoreName, }; use super::*; use fastly::http::{Method, StatusCode}; + /// Build `RuntimeServices` pre-loaded with a real Ed25519 keypair for + /// testing signature creation and verification in endpoint handlers. + fn build_signing_services_for_test() -> crate::platform::RuntimeServices { + use base64::{engine::general_purpose, Engine}; + use ed25519_dalek::SigningKey; + use rand::rngs::OsRng; + + struct MapConfigStore(HashMap); + impl PlatformConfigStore for MapConfigStore { + fn get(&self, _: &StoreName, key: &str) -> Result> { + self.0.get(key).cloned().ok_or_else(|| 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)) + } + } + + struct MapSecretStore(HashMap>); + impl PlatformSecretStore for MapSecretStore { + fn get_bytes(&self, _: &StoreName, key: &str) -> Result, Report> { + self.0.get(key).cloned().ok_or_else(|| Report::new(PlatformError::SecretStore)) + } + 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)) + } + } + + let signing_key = SigningKey::generate(&mut OsRng); + let key_b64 = general_purpose::STANDARD.encode(signing_key.as_bytes()); + let x_b64 = general_purpose::URL_SAFE_NO_PAD.encode(signing_key.verifying_key().as_bytes()); + let jwk_json = format!( + r#"{{"kty":"OKP","crv":"Ed25519","x":"{}","kid":"test-kid","alg":"EdDSA"}}"#, + x_b64 + ); + + let mut cfg = HashMap::new(); + cfg.insert("current-kid".to_string(), "test-kid".to_string()); + cfg.insert("test-kid".to_string(), jwk_json); + + let mut sec = HashMap::new(); + sec.insert("test-kid".to_string(), key_b64.into_bytes()); + + build_services_with_config_and_secret(MapConfigStore(cfg), MapSecretStore(sec)) + } + /// Config store stub that returns a minimal JWKS with one Ed25519 key. struct StubJwksConfigStore; @@ -367,19 +422,19 @@ mod tests { Err(Report::new(PlatformError::Unsupported)) } } + #[test] fn test_handle_verify_signature_valid() { let settings = crate::test_support::tests::create_test_settings(); + let services = build_signing_services_for_test(); - // First, create a valid signature let payload = "test message"; - let signer = crate::request_signing::RequestSigner::from_config() - .expect("should create signer from config"); + let signer = crate::request_signing::RequestSigner::from_services(&services) + .expect("should create signer from services"); let signature = signer .sign(payload.as_bytes()) .expect("should sign payload"); - // Create verification request let verify_req = VerifySignatureRequest { payload: payload.to_string(), signature, @@ -390,9 +445,8 @@ mod tests { let mut req = Request::new(Method::POST, "https://test.com/verify-signature"); req.set_body(body); - // Handle the request - let mut resp = - handle_verify_signature(&settings, req).expect("should handle verification request"); + let mut resp = handle_verify_signature(&settings, &services, req) + .expect("should handle verification request"); assert_eq!(resp.get_status(), StatusCode::OK); assert_eq!( resp.get_content_type(), @@ -400,12 +454,11 @@ mod tests { "should return application/json content type" ); - // Parse response let resp_body = resp.take_body_str(); let verify_resp: VerifySignatureResponse = serde_json::from_str(&resp_body).expect("should deserialize verify response"); - assert!(verify_resp.verified, "Signature should be verified"); + assert!(verify_resp.verified, "should verify a valid signature"); assert_eq!(verify_resp.kid, signer.kid); assert!(verify_resp.error.is_none()); } @@ -413,15 +466,15 @@ mod tests { #[test] fn test_handle_verify_signature_invalid() { let settings = crate::test_support::tests::create_test_settings(); - let signer = crate::request_signing::RequestSigner::from_config() - .expect("should create signer from config"); + let services = build_signing_services_for_test(); + + let signer = crate::request_signing::RequestSigner::from_services(&services) + .expect("should create signer from services"); - // Create a signature for a different payload let wrong_signature = signer .sign(b"different payload") .expect("should sign different payload"); - // Create request with signature that does not match the payload let verify_req = VerifySignatureRequest { payload: "test message".to_string(), signature: wrong_signature, @@ -432,9 +485,8 @@ mod tests { let mut req = Request::new(Method::POST, "https://test.com/verify-signature"); req.set_body(body); - // Handle the request - let mut resp = - handle_verify_signature(&settings, req).expect("should handle verification request"); + let mut resp = handle_verify_signature(&settings, &services, req) + .expect("should handle verification request"); assert_eq!(resp.get_status(), StatusCode::OK); assert_eq!( resp.get_content_type(), @@ -442,12 +494,11 @@ mod tests { "should return application/json content type" ); - // Parse response let resp_body = resp.take_body_str(); let verify_resp: VerifySignatureResponse = serde_json::from_str(&resp_body).expect("should deserialize verify response"); - assert!(!verify_resp.verified, "Invalid signature should not verify"); + assert!(!verify_resp.verified, "should not verify an invalid signature"); assert_eq!(verify_resp.kid, signer.kid); assert!(verify_resp.error.is_some()); } @@ -459,8 +510,7 @@ mod tests { let mut req = Request::new(Method::POST, "https://test.com/verify-signature"); req.set_body("not valid json"); - // Should return an error response - let result = handle_verify_signature(&settings, req); + let result = handle_verify_signature(&settings, &noop_services(), req); assert!(result.is_err(), "Malformed JSON should error"); } diff --git a/crates/trusted-server-core/src/request_signing/signing.rs b/crates/trusted-server-core/src/request_signing/signing.rs index a30226bb..e85d6bd5 100644 --- a/crates/trusted-server-core/src/request_signing/signing.rs +++ b/crates/trusted-server-core/src/request_signing/signing.rs @@ -1,7 +1,9 @@ //! Request signing and verification utilities. //! //! This module provides Ed25519-based signing and verification of HTTP requests -//! using keys stored in Fastly Config and Secret stores. +//! using keys stored via platform store primitives. + +use std::sync::LazyLock; use base64::{engine::general_purpose, Engine}; use ed25519_dalek::{Signature, Signer as Ed25519Signer, SigningKey, Verifier, VerifyingKey}; @@ -9,18 +11,29 @@ use error_stack::{Report, ResultExt}; use serde::Serialize; use crate::error::TrustedServerError; +use crate::platform::{RuntimeServices, StoreName}; use crate::request_signing::{JWKS_CONFIG_STORE_NAME, SIGNING_SECRET_STORE_NAME}; -use crate::storage::{FastlyConfigStore, FastlySecretStore}; + +static JWKS_STORE_NAME: LazyLock = + LazyLock::new(|| StoreName::from(JWKS_CONFIG_STORE_NAME)); + +static SIGNING_STORE_NAME: LazyLock = + LazyLock::new(|| StoreName::from(SIGNING_SECRET_STORE_NAME)); /// Retrieves the current active key ID from the config store. /// /// # Errors /// /// Returns an error if the config store cannot be accessed or the current-kid key is not found. -#[allow(deprecated)] -pub fn get_current_key_id() -> Result> { - let store = FastlyConfigStore::new(JWKS_CONFIG_STORE_NAME); - store.get("current-kid") +pub fn get_current_key_id( + services: &RuntimeServices, +) -> Result> { + services + .config_store() + .get(&JWKS_STORE_NAME, "current-kid") + .change_context(TrustedServerError::Configuration { + message: "failed to read current-kid from config store".into(), + }) } fn parse_ed25519_signing_key(key_bytes: Vec) -> Result> { @@ -120,20 +133,60 @@ impl RequestSigner { /// # Errors /// /// Returns an error if the key ID cannot be retrieved or the key cannot be parsed. + /// + /// # Deprecation + /// + /// Use [`Self::from_services`] instead. This method will be removed when + /// [`crate::auction::types::AuctionContext`] gains a `RuntimeServices` field. #[allow(deprecated)] + #[deprecated(since = "0.1.0", note = "use from_services instead")] pub fn from_config() -> Result> { + use crate::storage::{FastlyConfigStore, FastlySecretStore}; let config_store = FastlyConfigStore::new(JWKS_CONFIG_STORE_NAME); let key_id = config_store .get("current-kid") .change_context(TrustedServerError::Configuration { - message: "Failed to get current-kid".into(), + message: "failed to get current-kid".into(), })?; let secret_store = FastlySecretStore::new(SIGNING_SECRET_STORE_NAME); let key_bytes = secret_store .get(&key_id) - .attach(format!("Failed to get signing key for kid: {}", key_id))?; + .change_context(TrustedServerError::Configuration { + message: format!("failed to get signing key for kid: {}", key_id), + })?; + + let signing_key = parse_ed25519_signing_key(key_bytes)?; + + Ok(Self { + key: signing_key, + kid: key_id, + }) + } + + /// Creates a `RequestSigner` from the current key ID stored in platform stores. + /// + /// # Errors + /// + /// Returns an error if the key ID cannot be retrieved or the key cannot be parsed. + pub fn from_services( + services: &RuntimeServices, + ) -> Result> { + let key_id = services + .config_store() + .get(&JWKS_STORE_NAME, "current-kid") + .change_context(TrustedServerError::Configuration { + message: "failed to get current-kid".into(), + })?; + + let key_bytes = services + .secret_store() + .get_bytes(&SIGNING_STORE_NAME, &key_id) + .change_context(TrustedServerError::Configuration { + message: format!("failed to get signing key for kid: {}", key_id), + })?; + let signing_key = parse_ed25519_signing_key(key_bytes)?; Ok(Self { @@ -175,17 +228,17 @@ impl RequestSigner { /// # Errors /// /// Returns an error if the JWK cannot be retrieved, parsed, or if signature verification fails. -#[allow(deprecated)] pub fn verify_signature( payload: &[u8], signature_b64: &str, kid: &str, + services: &RuntimeServices, ) -> Result> { - let store = FastlyConfigStore::new(JWKS_CONFIG_STORE_NAME); - let jwk_json = store - .get(kid) + let jwk_json = services + .config_store() + .get(&JWKS_STORE_NAME, kid) .change_context(TrustedServerError::Configuration { - message: format!("Failed to get JWK for kid: {}", kid), + message: format!("failed to get JWK for kid: {}", kid), })?; let jwk: serde_json::Value = serde_json::from_str(&jwk_json).map_err(|e| { @@ -242,88 +295,159 @@ pub fn verify_signature( #[cfg(test)] mod tests { + use std::collections::HashMap; + + use error_stack::Report; + + use crate::platform::test_support::build_services_with_config_and_secret; + use crate::platform::{PlatformConfigStore, PlatformError, PlatformSecretStore, StoreId, StoreName}; + use super::*; - #[test] - fn test_request_signer_sign() { - // Report unwraps print full error chain on test failure - // Note: unwrapping a Report prints it nicely if test fails. - let signer = RequestSigner::from_config().expect("should create signer from config"); - let signature = signer - .sign(b"these pretzels are making me thirsty") - .expect("should sign payload"); - assert!(!signature.is_empty()); - assert!(signature.len() > 32); + // --------------------------------------------------------------------------- + // Stub stores with preset data + // --------------------------------------------------------------------------- + + struct StubConfigStore(HashMap); + + impl PlatformConfigStore for StubConfigStore { + fn get(&self, _: &StoreName, key: &str) -> Result> { + self.0 + .get(key) + .cloned() + .ok_or_else(|| 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)) + } } - #[test] - fn test_request_signer_from_config() { - let signer = RequestSigner::from_config().expect("should create signer from config"); - assert!(!signer.kid.is_empty()); + struct StubSecretStore(HashMap>); + + impl PlatformSecretStore for StubSecretStore { + fn get_bytes(&self, _: &StoreName, key: &str) -> Result, Report> { + self.0 + .get(key) + .cloned() + .ok_or_else(|| Report::new(PlatformError::SecretStore)) + } + + 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)) + } + } + + fn build_signing_services() -> crate::platform::RuntimeServices { + use base64::{engine::general_purpose, Engine}; + use ed25519_dalek::SigningKey; + use rand::rngs::OsRng; + + let signing_key = SigningKey::generate(&mut OsRng); + let key_b64 = general_purpose::STANDARD.encode(signing_key.as_bytes()); + let verifying_key = signing_key.verifying_key(); + let x_b64 = general_purpose::URL_SAFE_NO_PAD.encode(verifying_key.as_bytes()); + let jwk_json = format!( + r#"{{"kty":"OKP","crv":"Ed25519","x":"{}","kid":"test-kid","alg":"EdDSA"}}"#, + x_b64 + ); + + let mut config_data = HashMap::new(); + config_data.insert("current-kid".to_string(), "test-kid".to_string()); + config_data.insert("test-kid".to_string(), jwk_json); + + let mut secret_data = HashMap::new(); + secret_data.insert("test-kid".to_string(), key_b64.into_bytes()); + + build_services_with_config_and_secret( + StubConfigStore(config_data), + StubSecretStore(secret_data), + ) } #[test] - fn test_sign_and_verify() { - let payload = b"test payload for verification"; - let signer = RequestSigner::from_config().expect("should create signer from config"); - let signature = signer.sign(payload).expect("should sign payload"); + fn from_services_loads_kid_from_config_store() { + let services = build_signing_services(); + let signer = RequestSigner::from_services(&services) + .expect("should create signer from services"); - let result = - verify_signature(payload, &signature, &signer.kid).expect("should verify signature"); - assert!(result, "Signature should be valid"); + assert_eq!(signer.kid, "test-kid", "should load kid from config store"); } #[test] - fn test_verify_invalid_signature() { - let payload = b"test payload"; - let signer = RequestSigner::from_config().expect("should create signer from config"); + fn sign_produces_non_empty_url_safe_base64_signature() { + let services = build_signing_services(); + let signer = RequestSigner::from_services(&services) + .expect("should create signer from services"); - let wrong_signature = signer - .sign(b"different payload") - .expect("should sign different payload"); + let signature = signer + .sign(b"these pretzels are making me thirsty") + .expect("should sign payload"); - let result = verify_signature(payload, &wrong_signature, &signer.kid) + assert!(!signature.is_empty(), "should produce non-empty signature"); + assert!(signature.len() > 32, "should produce a full-length signature"); + } + + #[test] + fn sign_and_verify_roundtrip_succeeds() { + let services = build_signing_services(); + let signer = RequestSigner::from_services(&services) + .expect("should create signer from services"); + let payload = b"test payload for verification"; + + let signature = signer.sign(payload).expect("should sign payload"); + let verified = verify_signature(payload, &signature, &signer.kid, &services) .expect("should attempt verification"); - assert!(!result, "Invalid signature should not verify"); + + assert!(verified, "should verify a valid signature"); } #[test] - fn test_verify_wrong_payload() { - let original_payload = b"original payload"; - let signer = RequestSigner::from_config().expect("should create signer from config"); - let signature = signer - .sign(original_payload) - .expect("should sign original payload"); + fn verify_returns_false_for_wrong_payload() { + let services = build_signing_services(); + let signer = RequestSigner::from_services(&services) + .expect("should create signer from services"); + let signature = signer.sign(b"original").expect("should sign"); - let wrong_payload = b"wrong payload"; - let result = verify_signature(wrong_payload, &signature, &signer.kid) + let verified = verify_signature(b"wrong payload", &signature, &signer.kid, &services) .expect("should attempt verification"); - assert!(!result, "Signature should not verify with wrong payload"); + + assert!(!verified, "should not verify signature for wrong payload"); } #[test] - fn test_verify_missing_key() { - let payload = b"test payload"; - let signer = RequestSigner::from_config().expect("should create signer from config"); - let signature = signer.sign(payload).expect("should sign payload"); - let nonexistent_kid = "nonexistent-key-id"; + fn verify_errors_for_unknown_kid() { + let services = build_signing_services(); + let signer = RequestSigner::from_services(&services) + .expect("should create signer from services"); + let signature = signer.sign(b"payload").expect("should sign"); - let result = verify_signature(payload, &signature, nonexistent_kid); - assert!(result.is_err(), "Should error for missing key"); + let result = verify_signature(b"payload", &signature, "nonexistent-kid", &services); + + assert!(result.is_err(), "should error for unknown kid"); } #[test] - fn test_verify_malformed_signature() { - let payload = b"test payload"; - let signer = RequestSigner::from_config().expect("should create signer from config"); - let malformed_signature = "not-valid-base64!!!"; + fn verify_errors_for_malformed_signature() { + let services = build_signing_services(); + let signer = RequestSigner::from_services(&services) + .expect("should create signer from services"); + + let result = verify_signature(b"payload", "not-valid-base64!!!", &signer.kid, &services); - let result = verify_signature(payload, malformed_signature, &signer.kid); - assert!(result.is_err(), "Should error for malformed signature"); + assert!(result.is_err(), "should error for malformed signature"); } #[test] - fn test_signing_params_build_payload() { + fn signing_params_build_payload_serializes_all_fields() { let params = SigningParams { request_id: "req-123".to_string(), request_host: "example.com".to_string(), @@ -331,11 +455,10 @@ mod tests { timestamp: 1706900000, }; - let payload = params - .build_payload("kid-abc") - .expect("should build payload"); + let payload = params.build_payload("kid-abc").expect("should build payload"); let parsed: serde_json::Value = serde_json::from_str(&payload).expect("should be valid JSON"); + assert_eq!(parsed["version"], SIGNING_VERSION); assert_eq!(parsed["kid"], "kid-abc"); assert_eq!(parsed["host"], "example.com"); @@ -345,46 +468,54 @@ mod tests { } #[test] - fn test_signing_params_new_creates_timestamp() { + fn signing_params_new_creates_recent_timestamp() { let params = SigningParams::new( "req-123".to_string(), "example.com".to_string(), "https".to_string(), ); - assert_eq!(params.request_id, "req-123"); - assert_eq!(params.request_host, "example.com"); - assert_eq!(params.request_scheme, "https"); - // Timestamp should be recent (within last minute), in milliseconds let now_ms = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) - .unwrap() + .expect("should get system time") .as_millis() as u64; - assert!(params.timestamp <= now_ms); - assert!(params.timestamp >= now_ms - 60_000); + + assert!( + params.timestamp <= now_ms, + "timestamp should not be in the future" + ); + assert!( + params.timestamp >= now_ms - 60_000, + "timestamp should be within the last minute" + ); } #[test] - fn test_sign_request_enhanced() { - let signer = RequestSigner::from_config().unwrap(); + fn sign_request_enhanced_produces_verifiable_signature() { + let services = build_signing_services(); + let signer = RequestSigner::from_services(&services) + .expect("should create signer from services"); let params = SigningParams::new( "auction-123".to_string(), "publisher.com".to_string(), "https".to_string(), ); - let signature = signer.sign_request(¶ms).unwrap(); - assert!(!signature.is_empty()); + let signature = signer.sign_request(¶ms).expect("should sign request"); + let payload = params.build_payload(&signer.kid).expect("should build payload"); - // Verify the signature is valid by reconstructing the payload - let payload = params.build_payload(&signer.kid).unwrap(); - let result = verify_signature(payload.as_bytes(), &signature, &signer.kid).unwrap(); - assert!(result, "Enhanced signature should be valid"); + let verified = + verify_signature(payload.as_bytes(), &signature, &signer.kid, &services) + .expect("should verify"); + + assert!(verified, "enhanced request signature should be verifiable"); } #[test] - fn test_sign_request_different_params_different_signature() { - let signer = RequestSigner::from_config().unwrap(); + fn sign_request_different_hosts_produce_different_signatures() { + let services = build_signing_services(); + let signer = RequestSigner::from_services(&services) + .expect("should create signer from services"); let params1 = SigningParams { request_id: "req-1".to_string(), @@ -392,20 +523,19 @@ mod tests { request_scheme: "https".to_string(), timestamp: 1706900000, }; - let params2 = SigningParams { request_id: "req-1".to_string(), - request_host: "host2.com".to_string(), // Different host + request_host: "host2.com".to_string(), request_scheme: "https".to_string(), timestamp: 1706900000, }; - let sig1 = signer.sign_request(¶ms1).unwrap(); - let sig2 = signer.sign_request(¶ms2).unwrap(); + let sig1 = signer.sign_request(¶ms1).expect("should sign params1"); + let sig2 = signer.sign_request(¶ms2).expect("should sign params2"); assert_ne!( sig1, sig2, - "Different hosts should produce different signatures" + "different hosts should produce different signatures" ); } } From 5b6555f467ba5a0d1379eac591f79e4f93c2982b Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Wed, 1 Apr 2026 18:07:36 +0530 Subject: [PATCH 07/14] Delete storage/api_client.rs from core; remove FastlyApiClient --- .../src/storage/api_client.rs | 291 ------------------ crates/trusted-server-core/src/storage/mod.rs | 7 +- 2 files changed, 2 insertions(+), 296 deletions(-) delete mode 100644 crates/trusted-server-core/src/storage/api_client.rs diff --git a/crates/trusted-server-core/src/storage/api_client.rs b/crates/trusted-server-core/src/storage/api_client.rs deleted file mode 100644 index 81a2d57b..00000000 --- a/crates/trusted-server-core/src/storage/api_client.rs +++ /dev/null @@ -1,291 +0,0 @@ -//! 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::{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"; - -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 -/// 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::PlatformConfigStore`] and -/// [`crate::platform::PlatformSecretStore`] write methods instead. -pub struct FastlyApiClient { - api_key: Vec, - base_url: &'static str, - backend_name: String, -} - -impl FastlyApiClient { - /// Creates a new Fastly API client using the default secret store. - /// - /// # Errors - /// - /// 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 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. - pub fn from_secret_store( - store_name: &str, - key_name: &str, - ) -> Result> { - 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"); - - Ok(Self { - api_key, - base_url: FASTLY_API_HOST, - backend_name, - }) - } - - fn make_request( - &self, - method: &str, - path: &str, - body: Option, - 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 { - "GET" => Request::get(&url), - "POST" => Request::post(&url), - "PUT" => Request::put(&url), - "DELETE" => Request::delete(&url), - _ => { - return Err(Report::new(TrustedServerError::Configuration { - message: format!("unsupported HTTP method: {}", method), - })) - } - }; - - request = request - .with_header("Fastly-Key", api_key_str) - .with_header("Accept", "application/json"); - - if let Some(body_content) = body { - request = request - .with_header("Content-Type", content_type) - .with_body(body_content); - } - - request.send(&self.backend_name).map_err(|e| { - Report::new(TrustedServerError::Configuration { - message: format!("failed to send API request: {}", e), - }) - }) - } - - /// Updates a configuration item in a Fastly config store. - /// - /// # Errors - /// - /// Returns an error if the API request fails or returns a non-OK status. - pub fn update_config_item( - &self, - store_id: &str, - key: &str, - value: &str, - ) -> Result<(), Report> { - let path = format!("/resources/stores/config/{}/item/{}", store_id, key); - let payload = build_config_item_payload(value); - - let mut response = self.make_request( - "PUT", - &path, - Some(payload), - "application/x-www-form-urlencoded", - )?; - - let mut buf = String::new(); - response - .get_body_mut() - .read_to_string(&mut buf) - .change_context(TrustedServerError::Configuration { - message: "failed to read config store API response".into(), - })?; - - if response.get_status() == StatusCode::OK { - Ok(()) - } else { - Err(Report::new(TrustedServerError::Configuration { - message: format!( - "failed to update config item: HTTP {} - {}", - response.get_status(), - buf - ), - })) - } - } - - /// Creates a secret in a Fastly secret store. - /// - /// # Errors - /// - /// Returns an error if the API request fails or returns a non-OK status. - pub fn create_secret( - &self, - store_id: &str, - secret_name: &str, - 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 - }); - - let mut response = - self.make_request("POST", &path, Some(payload.to_string()), "application/json")?; - - let mut buf = String::new(); - response - .get_body_mut() - .read_to_string(&mut buf) - .change_context(TrustedServerError::Configuration { - message: "failed to read secret store API response".into(), - })?; - - if response.get_status() == StatusCode::OK { - Ok(()) - } else { - Err(Report::new(TrustedServerError::Configuration { - message: format!( - "failed to create secret: HTTP {} - {}", - response.get_status(), - buf - ), - })) - } - } - - /// Deletes a configuration item from a Fastly config store. - /// - /// # Errors - /// - /// 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, - key: &str, - ) -> Result<(), Report> { - let path = format!("/resources/stores/config/{}/item/{}", store_id, key); - - let mut response = self.make_request("DELETE", &path, None, "application/json")?; - - let mut buf = String::new(); - response - .get_body_mut() - .read_to_string(&mut buf) - .change_context(TrustedServerError::Configuration { - message: "failed to read config store delete API response".into(), - })?; - - if response.get_status() == StatusCode::OK - || response.get_status() == StatusCode::NO_CONTENT - { - Ok(()) - } else { - Err(Report::new(TrustedServerError::Configuration { - message: format!( - "failed to delete config item: HTTP {} - {}", - response.get_status(), - buf - ), - })) - } - } - - /// Deletes a secret from a Fastly secret store. - /// - /// # Errors - /// - /// 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, - secret_name: &str, - ) -> Result<(), Report> { - let path = format!( - "/resources/stores/secret/{}/secrets/{}", - store_id, secret_name - ); - - let mut response = self.make_request("DELETE", &path, None, "application/json")?; - - let mut buf = String::new(); - response - .get_body_mut() - .read_to_string(&mut buf) - .change_context(TrustedServerError::Configuration { - message: "failed to read secret store delete API response".into(), - })?; - - if response.get_status() == StatusCode::OK - || response.get_status() == StatusCode::NO_CONTENT - { - Ok(()) - } else { - Err(Report::new(TrustedServerError::Configuration { - message: format!( - "failed to delete secret: HTTP {} - {}", - response.get_status(), - buf - ), - })) - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn config_item_payload_url_encodes_reserved_characters() { - let payload = build_config_item_payload(r#"value with spaces + symbols &= {"kid":"a+b"}"#); - - 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/mod.rs b/crates/trusted-server-core/src/storage/mod.rs index 6010ae2e..b0679b9d 100644 --- a/crates/trusted-server-core/src/storage/mod.rs +++ b/crates/trusted-server-core/src/storage/mod.rs @@ -2,14 +2,11 @@ //! //! 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`]. +//! use [`crate::platform::PlatformConfigStore`] and +//! [`crate::platform::PlatformSecretStore`] 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; From 0a8915cc31bcc0256fd0b70125dbbdbef88e210a Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Wed, 1 Apr 2026 18:11:16 +0530 Subject: [PATCH 08/14] Fix formatting after CI gate check --- Cargo.lock | 1 + .../trusted-server-adapter-fastly/src/main.rs | 4 +- .../src/management_api.rs | 5 +- .../src/platform.rs | 2 +- .../src/platform/test_support.rs | 14 +++- .../src/request_signing/endpoints.rs | 25 +++++-- .../src/request_signing/rotation.rs | 21 ++---- .../src/request_signing/signing.rs | 69 ++++++++++--------- 8 files changed, 81 insertions(+), 60 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8cc043d8..263fcbd1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2792,6 +2792,7 @@ dependencies = [ "serde_json", "trusted-server-core", "trusted-server-js", + "urlencoding", ] [[package]] diff --git a/crates/trusted-server-adapter-fastly/src/main.rs b/crates/trusted-server-adapter-fastly/src/main.rs index 2180c676..345a6bbf 100644 --- a/crates/trusted-server-adapter-fastly/src/main.rs +++ b/crates/trusted-server-adapter-fastly/src/main.rs @@ -159,7 +159,9 @@ async fn route_request( // Key rotation admin endpoints // Keep in sync with Settings::ADMIN_ENDPOINTS in crates/trusted-server-core/src/settings.rs (Method::POST, "/admin/keys/rotate") => handle_rotate_key(settings, runtime_services, req), - (Method::POST, "/admin/keys/deactivate") => handle_deactivate_key(settings, runtime_services, req), + (Method::POST, "/admin/keys/deactivate") => { + handle_deactivate_key(settings, runtime_services, req) + } // Unified auction endpoint (returns creative HTML inline) (Method::POST, "/auction") => { diff --git a/crates/trusted-server-adapter-fastly/src/management_api.rs b/crates/trusted-server-adapter-fastly/src/management_api.rs index 94de4581..a436430b 100644 --- a/crates/trusted-server-adapter-fastly/src/management_api.rs +++ b/crates/trusted-server-adapter-fastly/src/management_api.rs @@ -19,8 +19,8 @@ use std::io::Read; use error_stack::{Report, ResultExt}; -use fastly::{Request, Response}; use fastly::http::StatusCode; +use fastly::{Request, Response}; use trusted_server_core::platform::{PlatformError, PlatformSecretStore, StoreName}; use crate::platform::FastlyPlatformSecretStore; @@ -284,8 +284,7 @@ mod tests { #[test] fn build_config_item_payload_url_encodes_reserved_characters() { - let payload = - build_config_item_payload(r#"value with spaces + symbols &= {"kid":"a+b"}"#); + let payload = build_config_item_payload(r#"value with spaces + symbols &= {"kid":"a+b"}"#); assert_eq!( payload, diff --git a/crates/trusted-server-adapter-fastly/src/platform.rs b/crates/trusted-server-adapter-fastly/src/platform.rs index b934aee9..8d25098f 100644 --- a/crates/trusted-server-adapter-fastly/src/platform.rs +++ b/crates/trusted-server-adapter-fastly/src/platform.rs @@ -16,13 +16,13 @@ use fastly::{ConfigStore, Request, SecretStore}; use trusted_server_core::backend::BackendConfig; use trusted_server_core::geo::geo_from_fastly; +pub(crate) use trusted_server_core::platform::UnavailableKvStore; use trusted_server_core::platform::{ ClientInfo, GeoInfo, PlatformBackend, PlatformBackendSpec, PlatformConfigStore, PlatformError, PlatformGeo, PlatformHttpClient, PlatformHttpRequest, PlatformKvStore, PlatformPendingRequest, PlatformResponse, PlatformSecretStore, PlatformSelectResult, RuntimeServices, StoreId, StoreName, }; -pub(crate) use trusted_server_core::platform::UnavailableKvStore; // --------------------------------------------------------------------------- // FastlyPlatformConfigStore diff --git a/crates/trusted-server-core/src/platform/test_support.rs b/crates/trusted-server-core/src/platform/test_support.rs index ec93e803..15ca210c 100644 --- a/crates/trusted-server-core/src/platform/test_support.rs +++ b/crates/trusted-server-core/src/platform/test_support.rs @@ -489,9 +489,17 @@ mod tests { // Act: both stores return Unsupported (confirming the injected impls are active) let config_result = services.config_store().get(&StoreName::from("s"), "k"); - let secret_result = services.secret_store().get_bytes(&StoreName::from("s"), "k"); + let secret_result = services + .secret_store() + .get_bytes(&StoreName::from("s"), "k"); - assert!(config_result.is_err(), "should delegate to injected config store"); - assert!(secret_result.is_err(), "should delegate to injected secret store"); + assert!( + config_result.is_err(), + "should delegate to injected config store" + ); + assert!( + secret_result.is_err(), + "should delegate to injected secret store" + ); } } diff --git a/crates/trusted-server-core/src/request_signing/endpoints.rs b/crates/trusted-server-core/src/request_signing/endpoints.rs index 93aa6eea..5d37c8ca 100644 --- a/crates/trusted-server-core/src/request_signing/endpoints.rs +++ b/crates/trusted-server-core/src/request_signing/endpoints.rs @@ -341,7 +341,9 @@ mod tests { use error_stack::Report; use crate::platform::{ - test_support::{build_services_with_config, build_services_with_config_and_secret, noop_services}, + test_support::{ + build_services_with_config, build_services_with_config_and_secret, noop_services, + }, PlatformConfigStore, PlatformError, PlatformSecretStore, StoreId, StoreName, }; @@ -358,7 +360,10 @@ mod tests { struct MapConfigStore(HashMap); impl PlatformConfigStore for MapConfigStore { fn get(&self, _: &StoreName, key: &str) -> Result> { - self.0.get(key).cloned().ok_or_else(|| Report::new(PlatformError::ConfigStore)) + self.0 + .get(key) + .cloned() + .ok_or_else(|| Report::new(PlatformError::ConfigStore)) } fn put(&self, _: &StoreId, _: &str, _: &str) -> Result<(), Report> { Err(Report::new(PlatformError::Unsupported)) @@ -370,8 +375,15 @@ mod tests { struct MapSecretStore(HashMap>); impl PlatformSecretStore for MapSecretStore { - fn get_bytes(&self, _: &StoreName, key: &str) -> Result, Report> { - self.0.get(key).cloned().ok_or_else(|| Report::new(PlatformError::SecretStore)) + fn get_bytes( + &self, + _: &StoreName, + key: &str, + ) -> Result, Report> { + self.0 + .get(key) + .cloned() + .ok_or_else(|| Report::new(PlatformError::SecretStore)) } fn create(&self, _: &StoreId, _: &str, _: &str) -> Result<(), Report> { Err(Report::new(PlatformError::Unsupported)) @@ -498,7 +510,10 @@ mod tests { let verify_resp: VerifySignatureResponse = serde_json::from_str(&resp_body).expect("should deserialize verify response"); - assert!(!verify_resp.verified, "should not verify an invalid signature"); + assert!( + !verify_resp.verified, + "should not verify an invalid signature" + ); assert_eq!(verify_resp.kid, signer.kid); assert!(verify_resp.error.is_some()); } diff --git a/crates/trusted-server-core/src/request_signing/rotation.rs b/crates/trusted-server-core/src/request_signing/rotation.rs index 2a72507d..3059dfd7 100644 --- a/crates/trusted-server-core/src/request_signing/rotation.rs +++ b/crates/trusted-server-core/src/request_signing/rotation.rs @@ -47,10 +47,7 @@ impl KeyRotationManager { /// defined in [`JWKS_CONFIG_STORE_NAME`] and /// [`crate::request_signing::SIGNING_SECRET_STORE_NAME`]. #[must_use] - pub fn new( - config_store_id: impl Into, - secret_store_id: impl Into, - ) -> Self { + pub fn new(config_store_id: impl Into, secret_store_id: impl Into) -> Self { Self { config_store_id: StoreId::from(config_store_id.into()), secret_store_id: StoreId::from(secret_store_id.into()), @@ -309,10 +306,7 @@ mod tests { .lock() .expect("should lock deletes") .push((store_id.to_string(), key.to_string())); - self.data - .lock() - .expect("should lock data") - .remove(key); + self.data.lock().expect("should lock data").remove(key); Ok(()) } } @@ -392,8 +386,7 @@ mod tests { fn rotate_key_stores_private_key_via_secret_store_create() { let config_store = SpyConfigStore::new(HashMap::new()); let secret_store = SpySecretStore::new(); - let services = - build_services_with_config_and_secret(config_store, secret_store); + let services = build_services_with_config_and_secret(config_store, secret_store); let manager = KeyRotationManager::new("cfg-id", "sec-id"); let result = manager.rotate_key(&services, Some("new-kid".to_string())); @@ -413,8 +406,7 @@ mod tests { data.insert("active-kids".to_string(), "only-key".to_string()); let config_store = SpyConfigStore::new(data); let secret_store = SpySecretStore::new(); - let services = - build_services_with_config_and_secret(config_store, secret_store); + let services = build_services_with_config_and_secret(config_store, secret_store); let manager = KeyRotationManager::new("cfg-id", "sec-id"); let result = manager.deactivate_key(&services, "only-key"); @@ -431,10 +423,7 @@ mod tests { let result = KeyRotationResult { new_kid: "ts-2024-01-01".to_string(), previous_kid: Some("ts-2023-12-31".to_string()), - active_kids: vec![ - "ts-2023-12-31".to_string(), - "ts-2024-01-01".to_string(), - ], + active_kids: vec!["ts-2023-12-31".to_string(), "ts-2024-01-01".to_string()], jwk: jwk.clone(), }; diff --git a/crates/trusted-server-core/src/request_signing/signing.rs b/crates/trusted-server-core/src/request_signing/signing.rs index e85d6bd5..b85f9d57 100644 --- a/crates/trusted-server-core/src/request_signing/signing.rs +++ b/crates/trusted-server-core/src/request_signing/signing.rs @@ -151,11 +151,12 @@ impl RequestSigner { })?; let secret_store = FastlySecretStore::new(SIGNING_SECRET_STORE_NAME); - let key_bytes = secret_store - .get(&key_id) - .change_context(TrustedServerError::Configuration { - message: format!("failed to get signing key for kid: {}", key_id), - })?; + let key_bytes = + secret_store + .get(&key_id) + .change_context(TrustedServerError::Configuration { + message: format!("failed to get signing key for kid: {}", key_id), + })?; let signing_key = parse_ed25519_signing_key(key_bytes)?; @@ -170,9 +171,7 @@ impl RequestSigner { /// # Errors /// /// Returns an error if the key ID cannot be retrieved or the key cannot be parsed. - pub fn from_services( - services: &RuntimeServices, - ) -> Result> { + pub fn from_services(services: &RuntimeServices) -> Result> { let key_id = services .config_store() .get(&JWKS_STORE_NAME, "current-kid") @@ -300,7 +299,9 @@ mod tests { use error_stack::Report; use crate::platform::test_support::build_services_with_config_and_secret; - use crate::platform::{PlatformConfigStore, PlatformError, PlatformSecretStore, StoreId, StoreName}; + use crate::platform::{ + PlatformConfigStore, PlatformError, PlatformSecretStore, StoreId, StoreName, + }; use super::*; @@ -376,8 +377,8 @@ mod tests { #[test] fn from_services_loads_kid_from_config_store() { let services = build_signing_services(); - let signer = RequestSigner::from_services(&services) - .expect("should create signer from services"); + let signer = + RequestSigner::from_services(&services).expect("should create signer from services"); assert_eq!(signer.kid, "test-kid", "should load kid from config store"); } @@ -385,22 +386,25 @@ mod tests { #[test] fn sign_produces_non_empty_url_safe_base64_signature() { let services = build_signing_services(); - let signer = RequestSigner::from_services(&services) - .expect("should create signer from services"); + let signer = + RequestSigner::from_services(&services).expect("should create signer from services"); let signature = signer .sign(b"these pretzels are making me thirsty") .expect("should sign payload"); assert!(!signature.is_empty(), "should produce non-empty signature"); - assert!(signature.len() > 32, "should produce a full-length signature"); + assert!( + signature.len() > 32, + "should produce a full-length signature" + ); } #[test] fn sign_and_verify_roundtrip_succeeds() { let services = build_signing_services(); - let signer = RequestSigner::from_services(&services) - .expect("should create signer from services"); + let signer = + RequestSigner::from_services(&services).expect("should create signer from services"); let payload = b"test payload for verification"; let signature = signer.sign(payload).expect("should sign payload"); @@ -413,8 +417,8 @@ mod tests { #[test] fn verify_returns_false_for_wrong_payload() { let services = build_signing_services(); - let signer = RequestSigner::from_services(&services) - .expect("should create signer from services"); + let signer = + RequestSigner::from_services(&services).expect("should create signer from services"); let signature = signer.sign(b"original").expect("should sign"); let verified = verify_signature(b"wrong payload", &signature, &signer.kid, &services) @@ -426,8 +430,8 @@ mod tests { #[test] fn verify_errors_for_unknown_kid() { let services = build_signing_services(); - let signer = RequestSigner::from_services(&services) - .expect("should create signer from services"); + let signer = + RequestSigner::from_services(&services).expect("should create signer from services"); let signature = signer.sign(b"payload").expect("should sign"); let result = verify_signature(b"payload", &signature, "nonexistent-kid", &services); @@ -438,8 +442,8 @@ mod tests { #[test] fn verify_errors_for_malformed_signature() { let services = build_signing_services(); - let signer = RequestSigner::from_services(&services) - .expect("should create signer from services"); + let signer = + RequestSigner::from_services(&services).expect("should create signer from services"); let result = verify_signature(b"payload", "not-valid-base64!!!", &signer.kid, &services); @@ -455,7 +459,9 @@ mod tests { timestamp: 1706900000, }; - let payload = params.build_payload("kid-abc").expect("should build payload"); + let payload = params + .build_payload("kid-abc") + .expect("should build payload"); let parsed: serde_json::Value = serde_json::from_str(&payload).expect("should be valid JSON"); @@ -493,8 +499,8 @@ mod tests { #[test] fn sign_request_enhanced_produces_verifiable_signature() { let services = build_signing_services(); - let signer = RequestSigner::from_services(&services) - .expect("should create signer from services"); + let signer = + RequestSigner::from_services(&services).expect("should create signer from services"); let params = SigningParams::new( "auction-123".to_string(), "publisher.com".to_string(), @@ -502,11 +508,12 @@ mod tests { ); let signature = signer.sign_request(¶ms).expect("should sign request"); - let payload = params.build_payload(&signer.kid).expect("should build payload"); + let payload = params + .build_payload(&signer.kid) + .expect("should build payload"); - let verified = - verify_signature(payload.as_bytes(), &signature, &signer.kid, &services) - .expect("should verify"); + let verified = verify_signature(payload.as_bytes(), &signature, &signer.kid, &services) + .expect("should verify"); assert!(verified, "enhanced request signature should be verifiable"); } @@ -514,8 +521,8 @@ mod tests { #[test] fn sign_request_different_hosts_produce_different_signatures() { let services = build_signing_services(); - let signer = RequestSigner::from_services(&services) - .expect("should create signer from services"); + let signer = + RequestSigner::from_services(&services).expect("should create signer from services"); let params1 = SigningParams { request_id: "req-1".to_string(), From 2f1cc979c2e690f52cbe86d954e643519742b186 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Thu, 2 Apr 2026 10:39:25 +0530 Subject: [PATCH 09/14] Add services to AuctionContext; remove deprecated from_config shim Thread RuntimeServices into AuctionContext so auction providers can access platform stores directly. Update PrebidAuctionProvider to use RequestSigner::from_services(context.services) instead of the now- removed from_config() shim. All construction sites and test helpers updated accordingly. This satisfies the final acceptance criterion of #490: no FastlyConfigStore/FastlySecretStore construction remains in the request_signing/ modules. --- .../src/auction/endpoints.rs | 1 + .../src/auction/orchestrator.rs | 5 +++ .../trusted-server-core/src/auction/types.rs | 4 +- .../src/integrations/prebid.rs | 10 ++++- .../src/request_signing/signing.rs | 38 ------------------- 5 files changed, 17 insertions(+), 41 deletions(-) diff --git a/crates/trusted-server-core/src/auction/endpoints.rs b/crates/trusted-server-core/src/auction/endpoints.rs index a679aad0..c0576fd2 100644 --- a/crates/trusted-server-core/src/auction/endpoints.rs +++ b/crates/trusted-server-core/src/auction/endpoints.rs @@ -89,6 +89,7 @@ pub async fn handle_auction( client_info: &services.client_info, timeout_ms: settings.auction.timeout_ms, provider_responses: None, + services, }; // Run the auction diff --git a/crates/trusted-server-core/src/auction/orchestrator.rs b/crates/trusted-server-core/src/auction/orchestrator.rs index b08d5cfb..d5281c7a 100644 --- a/crates/trusted-server-core/src/auction/orchestrator.rs +++ b/crates/trusted-server-core/src/auction/orchestrator.rs @@ -148,6 +148,7 @@ impl AuctionOrchestrator { client_info: context.client_info, timeout_ms: remaining_ms, provider_responses: Some(&provider_responses), + services: context.services, }; let start_time = Instant::now(); @@ -325,6 +326,7 @@ impl AuctionOrchestrator { client_info: context.client_info, timeout_ms: effective_timeout, provider_responses: context.provider_responses, + services: context.services, }; log::info!( @@ -677,12 +679,15 @@ mod tests { req: &'a Request, client_info: &'a crate::platform::ClientInfo, ) -> AuctionContext<'a> { + let services: &'static crate::platform::RuntimeServices = + Box::leak(Box::new(noop_services())); AuctionContext { settings, request: req, client_info, timeout_ms: 2000, provider_responses: None, + services, } } diff --git a/crates/trusted-server-core/src/auction/types.rs b/crates/trusted-server-core/src/auction/types.rs index 9f25e85a..1882fe62 100644 --- a/crates/trusted-server-core/src/auction/types.rs +++ b/crates/trusted-server-core/src/auction/types.rs @@ -6,7 +6,7 @@ use std::collections::HashMap; use crate::auction::context::ContextValue; use crate::geo::GeoInfo; -use crate::platform::ClientInfo; +use crate::platform::{ClientInfo, RuntimeServices}; use crate::settings::Settings; /// Represents a unified auction request across all providers. @@ -108,6 +108,8 @@ pub struct AuctionContext<'a> { /// Provider responses from the bidding phase, used by mediators. /// This is `None` for regular bidders and `Some` when calling a mediator. pub provider_responses: Option<&'a [AuctionResponse]>, + /// Platform services (config store, secret store, etc.) for use by providers. + pub services: &'a RuntimeServices, } /// Response from a single auction provider. diff --git a/crates/trusted-server-core/src/integrations/prebid.rs b/crates/trusted-server-core/src/integrations/prebid.rs index a78d4ea1..87278d55 100644 --- a/crates/trusted-server-core/src/integrations/prebid.rs +++ b/crates/trusted-server-core/src/integrations/prebid.rs @@ -1009,8 +1009,7 @@ impl AuctionProvider for PrebidAuctionProvider { { if request_signing_config.enabled { let request_info = RequestInfo::from_request(context.request, context.client_info); - #[allow(deprecated)] - let signer = RequestSigner::from_config()?; + let signer = RequestSigner::from_services(context.services)?; let params = SigningParams::new(request.id.clone(), request_info.host, request_info.scheme); let signature = signer.sign_request(¶ms)?; @@ -1286,12 +1285,16 @@ mod tests { request: &'a Request, client_info: &'a crate::platform::ClientInfo, ) -> AuctionContext<'a> { + use crate::platform::test_support::noop_services; + let services: &'static crate::platform::RuntimeServices = + Box::leak(Box::new(noop_services())); AuctionContext { settings, request, client_info, timeout_ms: 1000, provider_responses: None, + services, } } @@ -2852,6 +2855,7 @@ server_url = "https://prebid.example" config: PrebidIntegrationConfig, request: &AuctionRequest, ) -> OpenRtbRequest { + use crate::platform::test_support::noop_services; let provider = PrebidAuctionProvider::new(config); let settings = make_settings(); let fastly_req = Request::new(Method::POST, "https://example.com/auction"); @@ -2860,12 +2864,14 @@ server_url = "https://prebid.example" tls_protocol: None, tls_cipher: None, }; + let services = noop_services(); let context = AuctionContext { settings: &settings, request: &fastly_req, client_info: &client_info, timeout_ms: 1000, provider_responses: None, + services: &services, }; provider.to_openrtb(request, &context, None) } diff --git a/crates/trusted-server-core/src/request_signing/signing.rs b/crates/trusted-server-core/src/request_signing/signing.rs index b85f9d57..6d78feab 100644 --- a/crates/trusted-server-core/src/request_signing/signing.rs +++ b/crates/trusted-server-core/src/request_signing/signing.rs @@ -128,44 +128,6 @@ impl SigningParams { } impl RequestSigner { - /// Creates a `RequestSigner` from the current key ID stored in config. - /// - /// # Errors - /// - /// Returns an error if the key ID cannot be retrieved or the key cannot be parsed. - /// - /// # Deprecation - /// - /// Use [`Self::from_services`] instead. This method will be removed when - /// [`crate::auction::types::AuctionContext`] gains a `RuntimeServices` field. - #[allow(deprecated)] - #[deprecated(since = "0.1.0", note = "use from_services instead")] - pub fn from_config() -> Result> { - use crate::storage::{FastlyConfigStore, FastlySecretStore}; - let config_store = FastlyConfigStore::new(JWKS_CONFIG_STORE_NAME); - let key_id = - config_store - .get("current-kid") - .change_context(TrustedServerError::Configuration { - message: "failed to get current-kid".into(), - })?; - - let secret_store = FastlySecretStore::new(SIGNING_SECRET_STORE_NAME); - let key_bytes = - secret_store - .get(&key_id) - .change_context(TrustedServerError::Configuration { - message: format!("failed to get signing key for kid: {}", key_id), - })?; - - let signing_key = parse_ed25519_signing_key(key_bytes)?; - - Ok(Self { - key: signing_key, - kid: key_id, - }) - } - /// Creates a `RequestSigner` from the current key ID stored in platform stores. /// /// # Errors From ba141fa3c1de30279f609e022c72b253b088bbc1 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Thu, 2 Apr 2026 10:45:31 +0530 Subject: [PATCH 10/14] Fix prettier formatting in PR9 plan document --- ...31-pr9-wire-signing-to-store-primitives.md | 46 +++++++++++++++---- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/docs/superpowers/plans/2026-03-31-pr9-wire-signing-to-store-primitives.md b/docs/superpowers/plans/2026-03-31-pr9-wire-signing-to-store-primitives.md index be478529..c03de5fa 100644 --- a/docs/superpowers/plans/2026-03-31-pr9-wire-signing-to-store-primitives.md +++ b/docs/superpowers/plans/2026-03-31-pr9-wire-signing-to-store-primitives.md @@ -14,22 +14,24 @@ Before touching anything, read these files to understand the current state: -| File | Status | Notes | -|------|--------|-------| -| `crates/trusted-server-core/src/storage/api_client.rs` | **Delete** | Contains `FastlyApiClient` — HTTP calls to `api.fastly.com`. Used only by `rotation.rs`. | -| `crates/trusted-server-core/src/request_signing/rotation.rs` | **Migrate** | Uses `FastlyConfigStore` (reads) + `FastlyApiClient` (writes). Main migration target. | -| `crates/trusted-server-core/src/request_signing/signing.rs` | **Migrate** | Uses `FastlyConfigStore` + `FastlySecretStore` in 3 places. | -| `crates/trusted-server-core/src/request_signing/endpoints.rs` | **Update** | `handle_verify_signature`, `handle_rotate_key`, `handle_deactivate_key` don't receive `&RuntimeServices`. | -| `crates/trusted-server-core/src/request_signing/jwks.rs` | Already migrated ✓ | Uses `RuntimeServices`. No changes needed. | -| `crates/trusted-server-adapter-fastly/src/platform.rs` | **Update** | `FastlyPlatformConfigStore::put/delete` and `FastlyPlatformSecretStore::create/delete` return `PlatformError::NotImplemented`. | -| `crates/trusted-server-adapter-fastly/src/main.rs` | **Update** | Three call sites pass handlers without `runtime_services`. | +| File | Status | Notes | +| ------------------------------------------------------------- | ------------------ | ------------------------------------------------------------------------------------------------------------------------------ | +| `crates/trusted-server-core/src/storage/api_client.rs` | **Delete** | Contains `FastlyApiClient` — HTTP calls to `api.fastly.com`. Used only by `rotation.rs`. | +| `crates/trusted-server-core/src/request_signing/rotation.rs` | **Migrate** | Uses `FastlyConfigStore` (reads) + `FastlyApiClient` (writes). Main migration target. | +| `crates/trusted-server-core/src/request_signing/signing.rs` | **Migrate** | Uses `FastlyConfigStore` + `FastlySecretStore` in 3 places. | +| `crates/trusted-server-core/src/request_signing/endpoints.rs` | **Update** | `handle_verify_signature`, `handle_rotate_key`, `handle_deactivate_key` don't receive `&RuntimeServices`. | +| `crates/trusted-server-core/src/request_signing/jwks.rs` | Already migrated ✓ | Uses `RuntimeServices`. No changes needed. | +| `crates/trusted-server-adapter-fastly/src/platform.rs` | **Update** | `FastlyPlatformConfigStore::put/delete` and `FastlyPlatformSecretStore::create/delete` return `PlatformError::NotImplemented`. | +| `crates/trusted-server-adapter-fastly/src/main.rs` | **Update** | Three call sites pass handlers without `runtime_services`. | ## File Map ### Delete + - `crates/trusted-server-core/src/storage/api_client.rs` ### Modify (core) + - `crates/trusted-server-core/src/storage/mod.rs` — remove `api_client` submodule + re-export - `crates/trusted-server-core/src/platform/test_support.rs` — add `build_services_with_config_and_secret` - `crates/trusted-server-core/src/request_signing/rotation.rs` — replace `FastlyConfigStore`/`FastlyApiClient` with `RuntimeServices` @@ -37,9 +39,11 @@ Before touching anything, read these files to understand the current state: - `crates/trusted-server-core/src/request_signing/endpoints.rs` — add `&RuntimeServices` to three handlers ### Create (adapter) + - `crates/trusted-server-adapter-fastly/src/management_api.rs` — Fastly management API transport (absorbs `api_client.rs` logic, returns `PlatformError`) ### Modify (adapter) + - `crates/trusted-server-adapter-fastly/src/platform.rs` — implement `put`/`delete` for config, `create`/`delete` for secrets - `crates/trusted-server-adapter-fastly/src/main.rs` — pass `runtime_services` to three handlers @@ -52,6 +56,7 @@ Before touching anything, read these files to understand the current state: **Why:** Tasks 4 and 5 need a `RuntimeServices` with both a custom config store AND a custom secret store. The current `build_services_with_config` only customises the config store. **Files:** + - Modify: `crates/trusted-server-core/src/platform/test_support.rs` - [ ] **Step 1: Write a failing test that calls `build_services_with_config_and_secret`** @@ -132,6 +137,7 @@ git commit -m "Add build_services_with_config_and_secret to test_support" **Credential security note (from spec):** The Fastly API token is read from the `api-keys` secret store, key `api_key`. Log store IDs and operation names only — never the token or secret value. **Files:** + - Create: `crates/trusted-server-adapter-fastly/src/management_api.rs` - Modify: `crates/trusted-server-adapter-fastly/src/main.rs` — add `mod management_api;` @@ -467,11 +473,13 @@ git commit -m "Add FastlyManagementApiClient to adapter" **Why:** Replace the `NotImplemented` stubs in `platform.rs` with real calls to `FastlyManagementApiClient`. The existing `NotImplemented` test for secret store (`fastly_platform_secret_store_create_returns_not_implemented`, `fastly_platform_secret_store_delete_returns_not_implemented`) must be deleted now that the real implementation lands. Check if there are equivalent config store tests to delete too. **Files:** + - Modify: `crates/trusted-server-adapter-fastly/src/platform.rs` - [ ] **Step 1: Delete `NotImplemented` tests for secret store writes** In `platform.rs` tests, find and delete these two tests (they assert the old stub behavior that no longer holds): + - `fastly_platform_secret_store_create_returns_not_implemented` - `fastly_platform_secret_store_delete_returns_not_implemented` @@ -575,6 +583,7 @@ git commit -m "Implement FastlyPlatformConfigStore and FastlyPlatformSecretStore **Why:** `KeyRotationManager` currently holds `FastlyConfigStore` (reads) and `FastlyApiClient` (writes) as fields. Replace both with `&RuntimeServices` passed to each method. **New design:** + - Drop `config_store: FastlyConfigStore` and `api_client: FastlyApiClient` fields - Keep `config_store_id: StoreId` and `secret_store_id: StoreId` (passed to write methods) - `new()` is now infallible (no API key fetch at construction time) @@ -582,6 +591,7 @@ git commit -m "Implement FastlyPlatformConfigStore and FastlyPlatformSecretStore - Reads use `JWKS_STORE_NAME` (edge-visible name); writes use the stored `StoreId` values **Files:** + - Modify: `crates/trusted-server-core/src/request_signing/rotation.rs` - [ ] **Step 1: Write failing tests that define the new API** @@ -1074,11 +1084,13 @@ git commit -m "Migrate KeyRotationManager from FastlyApiClient to RuntimeService **Why:** Three items in `signing.rs` still construct `FastlyConfigStore`/`FastlySecretStore` directly. Replace all three with `RuntimeServices`. The existing viceroy-dependent tests are replaced with proper unit tests using stub stores. **Changed signatures:** + - `get_current_key_id()` → `get_current_key_id(services: &RuntimeServices)` - `RequestSigner::from_config()` → `RequestSigner::from_services(services: &RuntimeServices)` (rename to make the break explicit) - `verify_signature(payload, sig, kid)` → `verify_signature(payload, sig, kid, services: &RuntimeServices)` **Files:** + - Modify: `crates/trusted-server-core/src/request_signing/signing.rs` - [ ] **Step 1: Write failing tests for the new API** @@ -1353,10 +1365,13 @@ Expected: compile error — `from_services`, `verify_signature` with 4 args not Replace the imports, `LazyLock` statics, and function bodies. Key changes: **Imports — replace:** + ```rust use crate::storage::{FastlyConfigStore, FastlySecretStore}; ``` + **With:** + ```rust use std::sync::LazyLock; @@ -1364,6 +1379,7 @@ use crate::platform::{RuntimeServices, StoreName}; ``` **Add after imports:** + ```rust static JWKS_STORE_NAME: LazyLock = LazyLock::new(|| StoreName::from(JWKS_CONFIG_STORE_NAME)); @@ -1373,6 +1389,7 @@ static SIGNING_STORE_NAME: LazyLock = ``` **Replace `get_current_key_id`:** + ```rust pub fn get_current_key_id( services: &RuntimeServices, @@ -1387,6 +1404,7 @@ pub fn get_current_key_id( ``` **Replace `RequestSigner::from_config` with `from_services`:** + ```rust pub fn from_services( services: &RuntimeServices, @@ -1417,6 +1435,7 @@ pub fn from_services( **Replace `verify_signature` — add `services: &RuntimeServices` parameter and replace `FastlyConfigStore::new(...)` with `services.config_store().get(&JWKS_STORE_NAME, kid)`.** Full new signature: + ```rust pub fn verify_signature( payload: &[u8], @@ -1458,11 +1477,13 @@ git commit -m "Migrate signing.rs from FastlyConfigStore/FastlySecretStore to Ru **Note:** `fastly::{Request, Response}` and `fastly::mime` remain — type migration is Phase 2 (PR 12). **Files:** + - Modify: `crates/trusted-server-core/src/request_signing/endpoints.rs` - [ ] **Step 1: Update `handle_verify_signature` signature and body** Change: + ```rust pub fn handle_verify_signature( _settings: &Settings, @@ -1471,6 +1492,7 @@ pub fn handle_verify_signature( ``` To: + ```rust pub fn handle_verify_signature( _settings: &Settings, @@ -1480,6 +1502,7 @@ pub fn handle_verify_signature( ``` Update the `verify_signature` call: + ```rust let verification_result = signing::verify_signature( verify_req.payload.as_bytes(), @@ -1510,6 +1533,7 @@ Remove the `.change_context(...)` on `KeyRotationManager::new(...)` — it's now - [ ] **Step 3: Update `handle_deactivate_key` signature and body** Same pattern: add `services: &RuntimeServices`, update all `manager.*` calls to pass `services`: + - `manager.delete_key(&deactivate_req.kid)` → `manager.delete_key(services, &deactivate_req.kid)` - `manager.deactivate_key(&deactivate_req.kid)` → `manager.deactivate_key(services, &deactivate_req.kid)` - `manager.list_active_keys()` → `manager.list_active_keys(services)` @@ -1586,6 +1610,7 @@ fn build_signing_services_for_test() -> crate::platform::RuntimeServices { ``` Pattern for verify tests: + ```rust // In test_handle_verify_signature_valid: let services = build_signing_services_for_test(); @@ -1597,6 +1622,7 @@ let mut resp = handle_verify_signature(&settings, &services, req) ``` For rotation/deactivation tests, `noop_services()` is fine — these tests use the `match result { Ok => log, Err => log }` pattern and do not assert against store state. The `noop_services()` causes `KeyRotationManager` methods to fail at the store read/write level, which is the expected behavior in a test environment without real stores: + ```rust let services = noop_services(); let result = handle_rotate_key(&settings, &services, req); @@ -1625,6 +1651,7 @@ git commit -m "Add RuntimeServices parameter to handle_verify_signature, handle_ **Why:** The adapter `main.rs` calls the three handlers without `runtime_services`. Add it. `runtime_services` is already in scope in all call sites. **Files:** + - Modify: `crates/trusted-server-adapter-fastly/src/main.rs` - [ ] **Step 1: Update the three handler call sites** @@ -1673,6 +1700,7 @@ git commit -m "Pass runtime_services to signing endpoint handlers in main.rs" **Why:** `api_client.rs` is now fully superseded by `management_api.rs` in the adapter. No core code references `FastlyApiClient` anymore (verified by rotation.rs migration). **Files:** + - Delete: `crates/trusted-server-core/src/storage/api_client.rs` - Modify: `crates/trusted-server-core/src/storage/mod.rs` From 1a0c0b633fa684bc3a3253524b2b8328a3131f9a Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Mon, 6 Apr 2026 15:25:27 +0530 Subject: [PATCH 11/14] Address PR findings --- .../src/management_api.rs | 32 +++++++++---------- .../src/auction/orchestrator.rs | 6 ++-- .../src/integrations/prebid.rs | 7 ++-- .../src/request_signing/jwks.rs | 9 ++---- .../src/request_signing/mod.rs | 9 ++++++ .../src/request_signing/rotation.rs | 12 ++----- .../src/request_signing/signing.rs | 12 ++----- 7 files changed, 39 insertions(+), 48 deletions(-) diff --git a/crates/trusted-server-adapter-fastly/src/management_api.rs b/crates/trusted-server-adapter-fastly/src/management_api.rs index a436430b..5735532e 100644 --- a/crates/trusted-server-adapter-fastly/src/management_api.rs +++ b/crates/trusted-server-adapter-fastly/src/management_api.rs @@ -19,7 +19,6 @@ use std::io::Read; use error_stack::{Report, ResultExt}; -use fastly::http::StatusCode; use fastly::{Request, Response}; use trusted_server_core::platform::{PlatformError, PlatformSecretStore, StoreName}; @@ -38,7 +37,7 @@ pub(crate) fn build_config_item_payload(value: &str) -> String { /// Backs the `put`/`delete` methods of [`FastlyPlatformConfigStore`] and /// the `create`/`delete` methods of [`FastlyPlatformSecretStore`]. pub(crate) struct FastlyManagementApiClient { - api_key: Vec, + api_key: String, base_url: &'static str, backend_name: String, } @@ -59,7 +58,7 @@ impl FastlyManagementApiClient { .attach("failed to register Fastly management API backend")?; let api_key = FastlyPlatformSecretStore - .get_bytes(&StoreName::from(API_KEYS_STORE), API_KEY_ENTRY) + .get_string(&StoreName::from(API_KEYS_STORE), API_KEY_ENTRY) .change_context(PlatformError::SecretStore) .attach("failed to read Fastly API key from secret store")?; @@ -80,7 +79,6 @@ impl FastlyManagementApiClient { 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 { "GET" => Request::get(&url), @@ -94,7 +92,7 @@ impl FastlyManagementApiClient { }; request = request - .with_header("Fastly-Key", api_key_str) + .with_header("Fastly-Key", &self.api_key) .with_header("Accept", "application/json"); if let Some(body_content) = body { @@ -136,7 +134,7 @@ impl FastlyManagementApiClient { .read_to_string(&mut buf) .change_context(PlatformError::ConfigStore)?; - if response.get_status() == StatusCode::OK { + if response.get_status().is_success() { log::debug!( "FastlyManagementApiClient: updated config key '{}' in store '{}'", key, @@ -145,8 +143,9 @@ impl FastlyManagementApiClient { Ok(()) } else { Err(Report::new(PlatformError::ConfigStore).attach(format!( - "config item update failed with HTTP {} for key '{}' in store '{}'", + "config item update failed with HTTP {} - {} for key '{}' in store '{}'", response.get_status(), + buf.trim(), key, store_id ))) @@ -173,9 +172,7 @@ impl FastlyManagementApiClient { .read_to_string(&mut buf) .change_context(PlatformError::ConfigStore)?; - if response.get_status() == StatusCode::OK - || response.get_status() == StatusCode::NO_CONTENT - { + if response.get_status().is_success() { log::debug!( "FastlyManagementApiClient: deleted config key '{}' from store '{}'", key, @@ -184,8 +181,9 @@ impl FastlyManagementApiClient { Ok(()) } else { Err(Report::new(PlatformError::ConfigStore).attach(format!( - "config item delete failed with HTTP {} for key '{}' in store '{}'", + "config item delete failed with HTTP {} - {} for key '{}' in store '{}'", response.get_status(), + buf.trim(), key, store_id ))) @@ -218,7 +216,7 @@ impl FastlyManagementApiClient { .read_to_string(&mut buf) .change_context(PlatformError::SecretStore)?; - if response.get_status() == StatusCode::OK { + if response.get_status().is_success() { log::debug!( "FastlyManagementApiClient: created secret '{}' in store '{}'", secret_name, @@ -227,8 +225,9 @@ impl FastlyManagementApiClient { Ok(()) } else { Err(Report::new(PlatformError::SecretStore).attach(format!( - "secret create failed with HTTP {} for name '{}' in store '{}'", + "secret create failed with HTTP {} - {} for name '{}' in store '{}'", response.get_status(), + buf.trim(), secret_name, store_id ))) @@ -258,9 +257,7 @@ impl FastlyManagementApiClient { .read_to_string(&mut buf) .change_context(PlatformError::SecretStore)?; - if response.get_status() == StatusCode::OK - || response.get_status() == StatusCode::NO_CONTENT - { + if response.get_status().is_success() { log::debug!( "FastlyManagementApiClient: deleted secret '{}' from store '{}'", secret_name, @@ -269,8 +266,9 @@ impl FastlyManagementApiClient { Ok(()) } else { Err(Report::new(PlatformError::SecretStore).attach(format!( - "secret delete failed with HTTP {} for name '{}' in store '{}'", + "secret delete failed with HTTP {} - {} for name '{}' in store '{}'", response.get_status(), + buf.trim(), secret_name, store_id ))) diff --git a/crates/trusted-server-core/src/auction/orchestrator.rs b/crates/trusted-server-core/src/auction/orchestrator.rs index d5281c7a..e6969b1f 100644 --- a/crates/trusted-server-core/src/auction/orchestrator.rs +++ b/crates/trusted-server-core/src/auction/orchestrator.rs @@ -674,13 +674,15 @@ mod tests { crate::settings::Settings::from_toml(&settings_str).expect("should parse test settings") } + static TEST_SERVICES: std::sync::LazyLock = + std::sync::LazyLock::new(noop_services); + fn create_test_context<'a>( settings: &'a crate::settings::Settings, req: &'a Request, client_info: &'a crate::platform::ClientInfo, ) -> AuctionContext<'a> { - let services: &'static crate::platform::RuntimeServices = - Box::leak(Box::new(noop_services())); + let services: &'static crate::platform::RuntimeServices = &TEST_SERVICES; AuctionContext { settings, request: req, diff --git a/crates/trusted-server-core/src/integrations/prebid.rs b/crates/trusted-server-core/src/integrations/prebid.rs index 87278d55..a5fea62c 100644 --- a/crates/trusted-server-core/src/integrations/prebid.rs +++ b/crates/trusted-server-core/src/integrations/prebid.rs @@ -1280,14 +1280,15 @@ mod tests { } } + static TEST_SERVICES: std::sync::LazyLock = + std::sync::LazyLock::new(crate::platform::test_support::noop_services); + fn create_test_auction_context<'a>( settings: &'a Settings, request: &'a Request, client_info: &'a crate::platform::ClientInfo, ) -> AuctionContext<'a> { - use crate::platform::test_support::noop_services; - let services: &'static crate::platform::RuntimeServices = - Box::leak(Box::new(noop_services())); + let services: &'static crate::platform::RuntimeServices = &TEST_SERVICES; AuctionContext { settings, request, diff --git a/crates/trusted-server-core/src/request_signing/jwks.rs b/crates/trusted-server-core/src/request_signing/jwks.rs index 5c4dda94..cbc2b387 100644 --- a/crates/trusted-server-core/src/request_signing/jwks.rs +++ b/crates/trusted-server-core/src/request_signing/jwks.rs @@ -3,8 +3,6 @@ //! 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::{ @@ -14,11 +12,8 @@ use jose_jwk::{ use rand::rngs::OsRng; 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)); +use crate::platform::RuntimeServices; +use crate::request_signing::JWKS_STORE_NAME; /// An Ed25519 keypair used for request signing. pub struct Keypair { diff --git a/crates/trusted-server-core/src/request_signing/mod.rs b/crates/trusted-server-core/src/request_signing/mod.rs index 41507940..3c7b3e59 100644 --- a/crates/trusted-server-core/src/request_signing/mod.rs +++ b/crates/trusted-server-core/src/request_signing/mod.rs @@ -33,6 +33,15 @@ pub const JWKS_CONFIG_STORE_NAME: &str = "jwks_store"; /// `[local_server.secret_stores]`. pub const SIGNING_SECRET_STORE_NAME: &str = "signing_keys"; +use crate::platform::StoreName; +use std::sync::LazyLock; + +pub static JWKS_STORE_NAME: LazyLock = + LazyLock::new(|| StoreName::from(JWKS_CONFIG_STORE_NAME)); + +pub static SIGNING_STORE_NAME: LazyLock = + LazyLock::new(|| StoreName::from(SIGNING_SECRET_STORE_NAME)); + pub use discovery::*; pub use endpoints::*; pub use jwks::*; diff --git a/crates/trusted-server-core/src/request_signing/rotation.rs b/crates/trusted-server-core/src/request_signing/rotation.rs index 3059dfd7..20eccfe3 100644 --- a/crates/trusted-server-core/src/request_signing/rotation.rs +++ b/crates/trusted-server-core/src/request_signing/rotation.rs @@ -4,21 +4,15 @@ //! lifecycle, and storing keys via platform store primitives through //! [`RuntimeServices`]. -use std::sync::LazyLock; - use base64::{engine::general_purpose, Engine}; use ed25519_dalek::SigningKey; use error_stack::{Report, ResultExt}; use jose_jwk::Jwk; -use crate::error::TrustedServerError; -use crate::platform::{RuntimeServices, StoreId, StoreName}; -use crate::request_signing::JWKS_CONFIG_STORE_NAME; - use super::Keypair; - -static JWKS_STORE_NAME: LazyLock = - LazyLock::new(|| StoreName::from(JWKS_CONFIG_STORE_NAME)); +use crate::error::TrustedServerError; +use crate::platform::{RuntimeServices, StoreId}; +use crate::request_signing::JWKS_STORE_NAME; #[derive(Debug, Clone)] pub struct KeyRotationResult { diff --git a/crates/trusted-server-core/src/request_signing/signing.rs b/crates/trusted-server-core/src/request_signing/signing.rs index 6d78feab..054389e2 100644 --- a/crates/trusted-server-core/src/request_signing/signing.rs +++ b/crates/trusted-server-core/src/request_signing/signing.rs @@ -3,22 +3,14 @@ //! This module provides Ed25519-based signing and verification of HTTP requests //! using keys stored via platform store primitives. -use std::sync::LazyLock; - use base64::{engine::general_purpose, Engine}; use ed25519_dalek::{Signature, Signer as Ed25519Signer, SigningKey, Verifier, VerifyingKey}; use error_stack::{Report, ResultExt}; use serde::Serialize; use crate::error::TrustedServerError; -use crate::platform::{RuntimeServices, StoreName}; -use crate::request_signing::{JWKS_CONFIG_STORE_NAME, SIGNING_SECRET_STORE_NAME}; - -static JWKS_STORE_NAME: LazyLock = - LazyLock::new(|| StoreName::from(JWKS_CONFIG_STORE_NAME)); - -static SIGNING_STORE_NAME: LazyLock = - LazyLock::new(|| StoreName::from(SIGNING_SECRET_STORE_NAME)); +use crate::platform::RuntimeServices; +use crate::request_signing::{JWKS_STORE_NAME, SIGNING_STORE_NAME}; /// Retrieves the current active key ID from the config store. /// From 7365ec403304fd876eb025ff259728260e87fa0f Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Wed, 8 Apr 2026 21:33:21 +0530 Subject: [PATCH 12/14] Address PR review findings --- Cargo.lock | 1 + .../trusted-server-adapter-fastly/Cargo.toml | 1 + .../src/management_api.rs | 292 +++++++++++------- .../src/platform.rs | 14 +- .../src/platform/test_support.rs | 145 ++++++++- .../src/request_signing/endpoints.rs | 142 ++++----- .../src/request_signing/rotation.rs | 12 +- .../src/request_signing/signing.rs | 117 ++----- 8 files changed, 430 insertions(+), 294 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 263fcbd1..cfe84fd7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2779,6 +2779,7 @@ name = "trusted-server-adapter-fastly" version = "0.1.0" dependencies = [ "async-trait", + "base64", "chrono", "edgezero-adapter-fastly", "edgezero-core", diff --git a/crates/trusted-server-adapter-fastly/Cargo.toml b/crates/trusted-server-adapter-fastly/Cargo.toml index 2d1191cb..42a36208 100644 --- a/crates/trusted-server-adapter-fastly/Cargo.toml +++ b/crates/trusted-server-adapter-fastly/Cargo.toml @@ -8,6 +8,7 @@ workspace = true [dependencies] async-trait = { workspace = true } +base64 = { workspace = true } chrono = { workspace = true } edgezero-adapter-fastly = { workspace = true, features = ["fastly"] } edgezero-core = { workspace = true } diff --git a/crates/trusted-server-adapter-fastly/src/management_api.rs b/crates/trusted-server-adapter-fastly/src/management_api.rs index 5735532e..2d40fea4 100644 --- a/crates/trusted-server-adapter-fastly/src/management_api.rs +++ b/crates/trusted-server-adapter-fastly/src/management_api.rs @@ -18,6 +18,7 @@ use std::io::Read; +use base64::{engine::general_purpose, Engine as _}; use error_stack::{Report, ResultExt}; use fastly::{Request, Response}; use trusted_server_core::platform::{PlatformError, PlatformSecretStore, StoreName}; @@ -27,15 +28,84 @@ use crate::platform::FastlyPlatformSecretStore; const FASTLY_API_HOST: &str = "https://api.fastly.com"; const API_KEYS_STORE: &str = "api-keys"; const API_KEY_ENTRY: &str = "api_key"; +const SECRET_UPSERT_METHOD: &str = "PUT"; +const ERROR_BODY_LIMIT: usize = 200; + +fn encode_path_segment(value: &str) -> String { + urlencoding::encode(value).into_owned() +} pub(crate) fn build_config_item_payload(value: &str) -> String { format!("item_value={}", urlencoding::encode(value)) } +pub(crate) fn build_config_item_path(store_id: &str, key: &str) -> String { + format!( + "/resources/stores/config/{}/item/{}", + encode_path_segment(store_id), + encode_path_segment(key) + ) +} + +fn build_secret_collection_path(store_id: &str) -> String { + format!( + "/resources/stores/secret/{}/secrets", + encode_path_segment(store_id) + ) +} + +fn build_secret_path(store_id: &str, secret_name: &str) -> String { + format!( + "/resources/stores/secret/{}/secrets/{}", + encode_path_segment(store_id), + encode_path_segment(secret_name) + ) +} + +fn build_secret_payload(secret_name: &str, secret_value: &str) -> String { + serde_json::json!({ + "name": secret_name, + "secret": general_purpose::STANDARD.encode(secret_value.as_bytes()), + }) + .to_string() +} + +fn truncate_error_body(body: &str) -> String { + body.trim().chars().take(ERROR_BODY_LIMIT).collect() +} + +fn check_response( + response: &mut Response, + error_kind: fn() -> PlatformError, + operation: &str, + entity_description: &str, + store_id: &str, +) -> Result<(), Report> { + let mut body = String::new(); + response + .get_body_mut() + .read_to_string(&mut body) + .change_context(error_kind())?; + + if response.get_status().is_success() { + return Ok(()); + } + + Err(Report::new(error_kind()).attach(format!( + "{} failed with HTTP {} - {} for {} in store '{}'", + operation, + response.get_status(), + truncate_error_body(&body), + entity_description, + store_id + ))) +} + /// HTTP client for Fastly management API write operations. /// -/// Backs the `put`/`delete` methods of [`FastlyPlatformConfigStore`] and -/// the `create`/`delete` methods of [`FastlyPlatformSecretStore`]. +/// Backs the `put`/`delete` methods of +/// [`super::platform::FastlyPlatformConfigStore`] and the `create`/`delete` +/// methods of [`super::platform::FastlyPlatformSecretStore`]. pub(crate) struct FastlyManagementApiClient { api_key: String, base_url: &'static str, @@ -118,7 +188,7 @@ impl FastlyManagementApiClient { key: &str, value: &str, ) -> Result<(), Report> { - let path = format!("/resources/stores/config/{}/item/{}", store_id, key); + let path = build_config_item_path(store_id, key); let payload = build_config_item_payload(value); let mut response = self.make_request( @@ -128,28 +198,21 @@ impl FastlyManagementApiClient { "application/x-www-form-urlencoded", )?; - let mut buf = String::new(); - response - .get_body_mut() - .read_to_string(&mut buf) - .change_context(PlatformError::ConfigStore)?; - - if response.get_status().is_success() { - log::debug!( - "FastlyManagementApiClient: updated config key '{}' in store '{}'", - key, - store_id - ); - Ok(()) - } else { - Err(Report::new(PlatformError::ConfigStore).attach(format!( - "config item update failed with HTTP {} - {} for key '{}' in store '{}'", - response.get_status(), - buf.trim(), - key, - store_id - ))) - } + let entity_description = format!("key '{}'", key); + check_response( + &mut response, + || PlatformError::ConfigStore, + "config item update", + &entity_description, + store_id, + )?; + + log::debug!( + "FastlyManagementApiClient: updated config key '{}' in store '{}'", + key, + store_id + ); + Ok(()) } /// Delete a config store item. @@ -162,32 +225,25 @@ impl FastlyManagementApiClient { store_id: &str, key: &str, ) -> Result<(), Report> { - let path = format!("/resources/stores/config/{}/item/{}", store_id, key); + let path = build_config_item_path(store_id, key); let mut response = self.make_request("DELETE", &path, None, "application/json")?; - let mut buf = String::new(); - response - .get_body_mut() - .read_to_string(&mut buf) - .change_context(PlatformError::ConfigStore)?; - - if response.get_status().is_success() { - log::debug!( - "FastlyManagementApiClient: deleted config key '{}' from store '{}'", - key, - store_id - ); - Ok(()) - } else { - Err(Report::new(PlatformError::ConfigStore).attach(format!( - "config item delete failed with HTTP {} - {} for key '{}' in store '{}'", - response.get_status(), - buf.trim(), - key, - store_id - ))) - } + let entity_description = format!("key '{}'", key); + check_response( + &mut response, + || PlatformError::ConfigStore, + "config item delete", + &entity_description, + store_id, + )?; + + log::debug!( + "FastlyManagementApiClient: deleted config key '{}' from store '{}'", + key, + store_id + ); + Ok(()) } /// Create or overwrite a secret store entry. @@ -201,37 +257,31 @@ impl FastlyManagementApiClient { secret_name: &str, 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 - }); - - let mut response = - self.make_request("POST", &path, Some(payload.to_string()), "application/json")?; - - let mut buf = String::new(); - response - .get_body_mut() - .read_to_string(&mut buf) - .change_context(PlatformError::SecretStore)?; - - if response.get_status().is_success() { - log::debug!( - "FastlyManagementApiClient: created secret '{}' in store '{}'", - secret_name, - store_id - ); - Ok(()) - } else { - Err(Report::new(PlatformError::SecretStore).attach(format!( - "secret create failed with HTTP {} - {} for name '{}' in store '{}'", - response.get_status(), - buf.trim(), - secret_name, - store_id - ))) - } + let path = build_secret_collection_path(store_id); + let payload = build_secret_payload(secret_name, secret_value); + + let mut response = self.make_request( + SECRET_UPSERT_METHOD, + &path, + Some(payload), + "application/json", + )?; + + let entity_description = format!("name '{}'", secret_name); + check_response( + &mut response, + || PlatformError::SecretStore, + "secret upsert", + &entity_description, + store_id, + )?; + + log::debug!( + "FastlyManagementApiClient: upserted secret '{}' in store '{}'", + secret_name, + store_id + ); + Ok(()) } /// Delete a secret store entry. @@ -244,35 +294,25 @@ impl FastlyManagementApiClient { store_id: &str, secret_name: &str, ) -> Result<(), Report> { - let path = format!( - "/resources/stores/secret/{}/secrets/{}", - store_id, secret_name - ); + let path = build_secret_path(store_id, secret_name); let mut response = self.make_request("DELETE", &path, None, "application/json")?; - let mut buf = String::new(); - response - .get_body_mut() - .read_to_string(&mut buf) - .change_context(PlatformError::SecretStore)?; - - if response.get_status().is_success() { - log::debug!( - "FastlyManagementApiClient: deleted secret '{}' from store '{}'", - secret_name, - store_id - ); - Ok(()) - } else { - Err(Report::new(PlatformError::SecretStore).attach(format!( - "secret delete failed with HTTP {} - {} for name '{}' in store '{}'", - response.get_status(), - buf.trim(), - secret_name, - store_id - ))) - } + let entity_description = format!("name '{}'", secret_name); + check_response( + &mut response, + || PlatformError::SecretStore, + "secret delete", + &entity_description, + store_id, + )?; + + log::debug!( + "FastlyManagementApiClient: deleted secret '{}' from store '{}'", + secret_name, + store_id + ); + Ok(()) } } @@ -290,4 +330,46 @@ mod tests { "should URL-encode config item values in form payloads" ); } + + #[test] + fn build_config_item_path_url_encodes_store_id_and_key() { + let path = build_config_item_path("store/id", "current?kid+#1"); + + assert_eq!( + path, "/resources/stores/config/store%2Fid/item/current%3Fkid%2B%231", + "should percent-encode reserved path characters" + ); + } + + #[test] + fn secret_upsert_method_uses_put() { + assert_eq!( + SECRET_UPSERT_METHOD, "PUT", + "should use PUT so secret writes can overwrite existing entries" + ); + } + + #[test] + fn build_secret_payload_base64_encodes_raw_secret_value() { + let payload = build_secret_payload("signing-key", "raw-secret-value"); + let json: serde_json::Value = + serde_json::from_str(&payload).expect("should serialize secret payload as JSON"); + + assert_eq!(json["name"], "signing-key"); + assert_eq!( + json["secret"], + base64::engine::general_purpose::STANDARD.encode("raw-secret-value"), + "should base64-encode the secret payload for the Fastly API" + ); + } + + #[test] + fn truncate_error_body_limits_length_after_trimming() { + let body = format!(" {} ", "a".repeat(250)); + + let truncated = truncate_error_body(&body); + + assert_eq!(truncated.len(), 200, "should cap error bodies at 200 chars"); + assert_eq!(truncated, "a".repeat(200), "should trim before truncating"); + } } diff --git a/crates/trusted-server-adapter-fastly/src/platform.rs b/crates/trusted-server-adapter-fastly/src/platform.rs index 8d25098f..90fcece1 100644 --- a/crates/trusted-server-adapter-fastly/src/platform.rs +++ b/crates/trusted-server-adapter-fastly/src/platform.rs @@ -36,12 +36,13 @@ use trusted_server_core::platform::{ /// /// # Write cost /// -/// `put` and `delete` construct a [`FastlyManagementApiClient`] on every call, -/// which opens the `"api-keys"` secret store to read the management API key. On +/// `put` and `delete` construct a +/// [`crate::management_api::FastlyManagementApiClient`] 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. +/// should be aware that each call performs a synchronous outbound API request +/// to the Fastly management API. pub struct FastlyPlatformConfigStore; impl PlatformConfigStore for FastlyPlatformConfigStore { @@ -87,8 +88,9 @@ impl PlatformConfigStore for FastlyPlatformConfigStore { /// /// # Write cost /// -/// `create` and `delete` have the same per-call [`FastlyManagementApiClient`] cost -/// described on [`FastlyPlatformConfigStore`]. +/// `create` and `delete` have the same per-call +/// [`crate::management_api::FastlyManagementApiClient`] cost described on +/// [`FastlyPlatformConfigStore`]. pub struct FastlyPlatformSecretStore; impl PlatformSecretStore for FastlyPlatformSecretStore { diff --git a/crates/trusted-server-core/src/platform/test_support.rs b/crates/trusted-server-core/src/platform/test_support.rs index 15ca210c..04b9f4f8 100644 --- a/crates/trusted-server-core/src/platform/test_support.rs +++ b/crates/trusted-server-core/src/platform/test_support.rs @@ -1,14 +1,18 @@ -use std::collections::VecDeque; +use std::collections::{HashMap, VecDeque}; use std::net::IpAddr; use std::sync::{Arc, Mutex}; +use base64::{engine::general_purpose, Engine as _}; +use ed25519_dalek::SigningKey; use error_stack::{Report, ResultExt}; +use rand::rngs::OsRng; use super::{ ClientInfo, GeoInfo, PlatformBackend, PlatformBackendSpec, PlatformConfigStore, PlatformError, PlatformGeo, PlatformHttpClient, PlatformHttpRequest, PlatformPendingRequest, PlatformResponse, PlatformSecretStore, PlatformSelectResult, RuntimeServices, StoreId, StoreName, }; +use crate::request_signing::{JWKS_STORE_NAME, SIGNING_STORE_NAME}; pub(crate) struct NoopConfigStore; @@ -56,6 +60,74 @@ impl PlatformSecretStore for NoopSecretStore { } } +pub(crate) struct HashMapConfigStore { + data: HashMap, +} + +impl HashMapConfigStore { + pub(crate) fn new(data: HashMap) -> Self { + Self { data } + } +} + +impl PlatformConfigStore for HashMapConfigStore { + fn get(&self, _store_name: &StoreName, key: &str) -> Result> { + self.data + .get(key) + .cloned() + .ok_or_else(|| 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)) + } +} + +pub(crate) struct HashMapSecretStore { + data: HashMap>, +} + +impl HashMapSecretStore { + pub(crate) fn new(data: HashMap>) -> Self { + Self { data } + } +} + +impl PlatformSecretStore for HashMapSecretStore { + fn get_bytes( + &self, + _store_name: &StoreName, + key: &str, + ) -> Result, Report> { + self.data + .get(key) + .cloned() + .ok_or_else(|| Report::new(PlatformError::SecretStore)) + } + + 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 { @@ -280,6 +352,28 @@ pub(crate) fn build_services_with_config_and_secret( .build() } +pub(crate) fn build_request_signing_services() -> RuntimeServices { + let signing_key = SigningKey::generate(&mut OsRng); + let key_b64 = general_purpose::STANDARD.encode(signing_key.as_bytes()); + let x_b64 = general_purpose::URL_SAFE_NO_PAD.encode(signing_key.verifying_key().as_bytes()); + let jwk_json = format!( + r#"{{"kty":"OKP","crv":"Ed25519","x":"{}","kid":"test-kid","alg":"EdDSA"}}"#, + x_b64 + ); + + let mut config_data = HashMap::new(); + config_data.insert("current-kid".to_string(), "test-kid".to_string()); + config_data.insert("test-kid".to_string(), jwk_json); + + let mut secret_data = HashMap::new(); + secret_data.insert("test-kid".to_string(), key_b64.into_bytes()); + + build_services_with_config_and_secret( + HashMapConfigStore::new(config_data), + HashMapSecretStore::new(secret_data), + ) +} + pub(crate) fn build_services_with_config( config_store: impl PlatformConfigStore + 'static, ) -> RuntimeServices { @@ -502,4 +596,53 @@ mod tests { "should delegate to injected secret store" ); } + + #[test] + fn hash_map_stores_return_preset_values() { + let mut config = HashMap::new(); + config.insert("current-kid".to_string(), "test-kid".to_string()); + + let mut secrets = HashMap::new(); + secrets.insert("test-kid".to_string(), b"secret-material".to_vec()); + + let services = build_services_with_config_and_secret( + HashMapConfigStore::new(config), + HashMapSecretStore::new(secrets), + ); + + assert_eq!( + services + .config_store() + .get(&JWKS_STORE_NAME, "current-kid") + .expect("should read current-kid from config test store"), + "test-kid" + ); + assert_eq!( + services + .secret_store() + .get_bytes(&SIGNING_STORE_NAME, "test-kid") + .expect("should read signing key bytes from secret test store"), + b"secret-material".to_vec() + ); + } + + #[test] + fn build_request_signing_services_provides_current_kid_and_signing_key() { + let services = build_request_signing_services(); + + let kid = services + .config_store() + .get(&JWKS_STORE_NAME, "current-kid") + .expect("should expose current-kid in config store"); + let key_bytes = services + .secret_store() + .get_bytes(&SIGNING_STORE_NAME, &kid) + .expect("should expose signing key bytes in secret store"); + + assert_eq!(kid, "test-kid", "should use the standard signing test kid"); + assert!( + !key_bytes.is_empty(), + "should provide key material for the current signing key" + ); + } } diff --git a/crates/trusted-server-core/src/request_signing/endpoints.rs b/crates/trusted-server-core/src/request_signing/endpoints.rs index 5d37c8ca..57080625 100644 --- a/crates/trusted-server-core/src/request_signing/endpoints.rs +++ b/crates/trusted-server-core/src/request_signing/endpoints.rs @@ -52,18 +52,27 @@ pub fn handle_trusted_server_discovery( .with_body(json)) } +/// JSON request body for the signature verification endpoint. #[derive(Debug, Deserialize, Serialize)] pub struct VerifySignatureRequest { + /// Canonical payload that was signed. pub payload: String, + /// Base64-encoded Ed25519 signature to verify. pub signature: String, + /// Key identifier used to look up the public JWK. pub kid: String, } +/// JSON response body for the signature verification endpoint. #[derive(Debug, Deserialize, Serialize)] pub struct VerifySignatureResponse { + /// Whether signature verification succeeded. pub verified: bool, + /// Key identifier that was used during verification. pub kid: String, + /// Human-readable verification result summary. pub message: String, + /// Error detail when verification fails unexpectedly. #[serde(skip_serializing_if = "Option::is_none")] pub error: Option, } @@ -124,24 +133,52 @@ pub fn handle_verify_signature( .with_body(response_json)) } +/// JSON request body for the key-rotation endpoint. #[derive(Debug, Deserialize, Serialize)] pub struct RotateKeyRequest { + /// Optional explicit key identifier for the new signing key. #[serde(skip_serializing_if = "Option::is_none")] pub kid: Option, } +/// JSON response body for the key-rotation endpoint. #[derive(Debug, Deserialize, Serialize)] pub struct RotateKeyResponse { + /// Whether the rotation operation succeeded. pub success: bool, + /// Human-readable summary of the rotation result. pub message: String, + /// Newly generated or supplied key identifier. pub new_kid: String, + /// Previously active key identifier, if one existed. pub previous_kid: Option, + /// Active key identifiers after the rotation completes. pub active_kids: Vec, + /// Public JWK associated with the newly active key. pub jwk: serde_json::Value, + /// Error detail when rotation fails. #[serde(skip_serializing_if = "Option::is_none")] pub error: Option, } +fn signing_store_ids(settings: &Settings) -> Result<(&str, &str), Report> { + settings + .request_signing + .as_ref() + .map(|setting| { + ( + setting.config_store_id.as_str(), + setting.secret_store_id.as_str(), + ) + }) + .ok_or_else(|| { + TrustedServerError::Configuration { + message: "missing signing storage configuration".to_string(), + } + .into() + }) +} + /// Rotates the current active kid by generating and saving a new one /// /// # Errors @@ -152,15 +189,7 @@ pub fn handle_rotate_key( services: &RuntimeServices, mut req: Request, ) -> Result> { - let (config_store_id, secret_store_id) = match &settings.request_signing { - Some(setting) => (&setting.config_store_id, &setting.secret_store_id), - None => { - return Err(TrustedServerError::Configuration { - message: "missing signing storage configuration".to_string(), - } - .into()); - } - }; + let (config_store_id, secret_store_id) = signing_store_ids(settings)?; let body = req.take_body_str(); let rotate_req: RotateKeyRequest = if body.is_empty() { @@ -225,20 +254,30 @@ pub fn handle_rotate_key( } } +/// JSON request body for the key-deactivation endpoint. #[derive(Debug, Deserialize, Serialize)] pub struct DeactivateKeyRequest { + /// Key identifier to deactivate or delete. pub kid: String, + /// Whether the key should be deleted from storage after deactivation. #[serde(default)] pub delete: bool, } +/// JSON response body for the key-deactivation endpoint. #[derive(Debug, Deserialize, Serialize)] pub struct DeactivateKeyResponse { + /// Whether the deactivation or deletion succeeded. pub success: bool, + /// Human-readable summary of the operation result. pub message: String, + /// Key identifier that was deactivated or deleted. pub deactivated_kid: String, + /// Whether the key was deleted from storage. pub deleted: bool, + /// Active key identifiers remaining after the operation. pub remaining_active_kids: Vec, + /// Error detail when the operation fails. #[serde(skip_serializing_if = "Option::is_none")] pub error: Option, } @@ -253,15 +292,7 @@ pub fn handle_deactivate_key( services: &RuntimeServices, mut req: Request, ) -> Result> { - let (config_store_id, secret_store_id) = match &settings.request_signing { - Some(setting) => (&setting.config_store_id, &setting.secret_store_id), - None => { - return Err(TrustedServerError::Configuration { - message: "missing signing storage configuration".to_string(), - } - .into()); - } - }; + let (config_store_id, secret_store_id) = signing_store_ids(settings)?; let body = req.take_body_str(); let deactivate_req: DeactivateKeyRequest = @@ -336,81 +367,14 @@ pub fn handle_deactivate_key( #[cfg(test)] mod tests { - use std::collections::HashMap; - - use error_stack::Report; - use crate::platform::{ - test_support::{ - build_services_with_config, build_services_with_config_and_secret, noop_services, - }, - PlatformConfigStore, PlatformError, PlatformSecretStore, StoreId, StoreName, + test_support::{build_request_signing_services, build_services_with_config, noop_services}, + PlatformConfigStore, PlatformError, StoreId, StoreName, }; use super::*; use fastly::http::{Method, StatusCode}; - /// Build `RuntimeServices` pre-loaded with a real Ed25519 keypair for - /// testing signature creation and verification in endpoint handlers. - fn build_signing_services_for_test() -> crate::platform::RuntimeServices { - use base64::{engine::general_purpose, Engine}; - use ed25519_dalek::SigningKey; - use rand::rngs::OsRng; - - struct MapConfigStore(HashMap); - impl PlatformConfigStore for MapConfigStore { - fn get(&self, _: &StoreName, key: &str) -> Result> { - self.0 - .get(key) - .cloned() - .ok_or_else(|| 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)) - } - } - - struct MapSecretStore(HashMap>); - impl PlatformSecretStore for MapSecretStore { - fn get_bytes( - &self, - _: &StoreName, - key: &str, - ) -> Result, Report> { - self.0 - .get(key) - .cloned() - .ok_or_else(|| Report::new(PlatformError::SecretStore)) - } - 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)) - } - } - - let signing_key = SigningKey::generate(&mut OsRng); - let key_b64 = general_purpose::STANDARD.encode(signing_key.as_bytes()); - let x_b64 = general_purpose::URL_SAFE_NO_PAD.encode(signing_key.verifying_key().as_bytes()); - let jwk_json = format!( - r#"{{"kty":"OKP","crv":"Ed25519","x":"{}","kid":"test-kid","alg":"EdDSA"}}"#, - x_b64 - ); - - let mut cfg = HashMap::new(); - cfg.insert("current-kid".to_string(), "test-kid".to_string()); - cfg.insert("test-kid".to_string(), jwk_json); - - let mut sec = HashMap::new(); - sec.insert("test-kid".to_string(), key_b64.into_bytes()); - - build_services_with_config_and_secret(MapConfigStore(cfg), MapSecretStore(sec)) - } - /// Config store stub that returns a minimal JWKS with one Ed25519 key. struct StubJwksConfigStore; @@ -438,7 +402,7 @@ mod tests { #[test] fn test_handle_verify_signature_valid() { let settings = crate::test_support::tests::create_test_settings(); - let services = build_signing_services_for_test(); + let services = build_request_signing_services(); let payload = "test message"; let signer = crate::request_signing::RequestSigner::from_services(&services) @@ -478,7 +442,7 @@ mod tests { #[test] fn test_handle_verify_signature_invalid() { let settings = crate::test_support::tests::create_test_settings(); - let services = build_signing_services_for_test(); + let services = build_request_signing_services(); let signer = crate::request_signing::RequestSigner::from_services(&services) .expect("should create signer from services"); diff --git a/crates/trusted-server-core/src/request_signing/rotation.rs b/crates/trusted-server-core/src/request_signing/rotation.rs index 20eccfe3..fd2dacc7 100644 --- a/crates/trusted-server-core/src/request_signing/rotation.rs +++ b/crates/trusted-server-core/src/request_signing/rotation.rs @@ -14,17 +14,22 @@ use crate::error::TrustedServerError; use crate::platform::{RuntimeServices, StoreId}; use crate::request_signing::JWKS_STORE_NAME; +/// Result of a key rotation operation. #[derive(Debug, Clone)] pub struct KeyRotationResult { + /// Newly generated or supplied key identifier. pub new_kid: String, + /// Previously active key identifier, if one existed. pub previous_kid: Option, + /// Active key identifiers after rotation completes. pub active_kids: Vec, + /// Public JWK associated with the newly active key. pub jwk: Jwk, } /// Manages signing key lifecycle using platform store primitives. /// -/// Reads use the edge-visible store name ([`JWKS_CONFIG_STORE_NAME`]). +/// Reads use the edge-visible store name ([`super::JWKS_CONFIG_STORE_NAME`]). /// Writes use the management API store identifiers supplied at construction. pub struct KeyRotationManager { /// Management API store ID for config store writes. @@ -38,7 +43,7 @@ impl KeyRotationManager { /// /// The `config_store_id` and `secret_store_id` are platform management API /// identifiers used for write operations. Edge reads use the store names - /// defined in [`JWKS_CONFIG_STORE_NAME`] and + /// defined in [`super::JWKS_CONFIG_STORE_NAME`] and /// [`crate::request_signing::SIGNING_SECRET_STORE_NAME`]. #[must_use] pub fn new(config_store_id: impl Into, secret_store_id: impl Into) -> Self { @@ -92,6 +97,9 @@ impl KeyRotationManager { kid: &str, signing_key: &SigningKey, ) -> Result<(), Report> { + // The platform secret-store write interface is string-based, so signing + // keys are persisted as base64 text. The Fastly adapter applies its own + // transport-level base64 encoding when calling the management API. let key_b64 = general_purpose::STANDARD.encode(signing_key.as_bytes()); services diff --git a/crates/trusted-server-core/src/request_signing/signing.rs b/crates/trusted-server-core/src/request_signing/signing.rs index 054389e2..eb3ddb96 100644 --- a/crates/trusted-server-core/src/request_signing/signing.rs +++ b/crates/trusted-server-core/src/request_signing/signing.rs @@ -28,6 +28,13 @@ pub fn get_current_key_id( }) } +/// Parses an Ed25519 signing key from secret-store bytes. +/// +/// Request-signing rotation stores private keys as base64 text in the secret +/// store so Fastly can decode them on write, while some tests may inject raw +/// 32-byte key material directly. This reader accepts both encodings: +/// exactly 32 bytes are treated as raw key bytes, and longer values are +/// treated as base64-encoded bytes. fn parse_ed25519_signing_key(key_bytes: Vec) -> Result> { let bytes = if key_bytes.len() > 32 { general_purpose::STANDARD.decode(&key_bytes).map_err(|_| { @@ -48,8 +55,10 @@ fn parse_ed25519_signing_key(key_bytes: Vec) -> Result { /// Parameters for enhanced request signing #[derive(Debug, Clone)] pub struct SigningParams { + /// Request identifier to bind into the signature payload. pub request_id: String, + /// Host header value expected by the receiving service. pub request_host: String, + /// Request scheme bound into the signature payload. pub request_scheme: String, + /// Signature timestamp in Unix milliseconds. pub timestamp: u64, } @@ -96,8 +109,8 @@ impl SigningParams { /// Builds the canonical payload string for signing. /// - /// The payload is a JSON-serialized [`SigningPayload`] to prevent signature - /// confusion attacks that could exploit delimiter-based formats. + /// The payload is JSON-serialized to prevent signature confusion attacks + /// that could exploit delimiter-based formats. /// /// # Errors /// @@ -126,10 +139,8 @@ impl RequestSigner { /// /// Returns an error if the key ID cannot be retrieved or the key cannot be parsed. pub fn from_services(services: &RuntimeServices) -> Result> { - let key_id = services - .config_store() - .get(&JWKS_STORE_NAME, "current-kid") - .change_context(TrustedServerError::Configuration { + let key_id = + get_current_key_id(services).change_context(TrustedServerError::Configuration { message: "failed to get current-kid".into(), })?; @@ -248,89 +259,13 @@ pub fn verify_signature( #[cfg(test)] mod tests { - use std::collections::HashMap; - - use error_stack::Report; - - use crate::platform::test_support::build_services_with_config_and_secret; - use crate::platform::{ - PlatformConfigStore, PlatformError, PlatformSecretStore, StoreId, StoreName, - }; + use crate::platform::test_support::build_request_signing_services; use super::*; - // --------------------------------------------------------------------------- - // Stub stores with preset data - // --------------------------------------------------------------------------- - - struct StubConfigStore(HashMap); - - impl PlatformConfigStore for StubConfigStore { - fn get(&self, _: &StoreName, key: &str) -> Result> { - self.0 - .get(key) - .cloned() - .ok_or_else(|| 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)) - } - } - - struct StubSecretStore(HashMap>); - - impl PlatformSecretStore for StubSecretStore { - fn get_bytes(&self, _: &StoreName, key: &str) -> Result, Report> { - self.0 - .get(key) - .cloned() - .ok_or_else(|| Report::new(PlatformError::SecretStore)) - } - - 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)) - } - } - - fn build_signing_services() -> crate::platform::RuntimeServices { - use base64::{engine::general_purpose, Engine}; - use ed25519_dalek::SigningKey; - use rand::rngs::OsRng; - - let signing_key = SigningKey::generate(&mut OsRng); - let key_b64 = general_purpose::STANDARD.encode(signing_key.as_bytes()); - let verifying_key = signing_key.verifying_key(); - let x_b64 = general_purpose::URL_SAFE_NO_PAD.encode(verifying_key.as_bytes()); - let jwk_json = format!( - r#"{{"kty":"OKP","crv":"Ed25519","x":"{}","kid":"test-kid","alg":"EdDSA"}}"#, - x_b64 - ); - - let mut config_data = HashMap::new(); - config_data.insert("current-kid".to_string(), "test-kid".to_string()); - config_data.insert("test-kid".to_string(), jwk_json); - - let mut secret_data = HashMap::new(); - secret_data.insert("test-kid".to_string(), key_b64.into_bytes()); - - build_services_with_config_and_secret( - StubConfigStore(config_data), - StubSecretStore(secret_data), - ) - } - #[test] fn from_services_loads_kid_from_config_store() { - let services = build_signing_services(); + let services = build_request_signing_services(); let signer = RequestSigner::from_services(&services).expect("should create signer from services"); @@ -339,7 +274,7 @@ mod tests { #[test] fn sign_produces_non_empty_url_safe_base64_signature() { - let services = build_signing_services(); + let services = build_request_signing_services(); let signer = RequestSigner::from_services(&services).expect("should create signer from services"); @@ -356,7 +291,7 @@ mod tests { #[test] fn sign_and_verify_roundtrip_succeeds() { - let services = build_signing_services(); + let services = build_request_signing_services(); let signer = RequestSigner::from_services(&services).expect("should create signer from services"); let payload = b"test payload for verification"; @@ -370,7 +305,7 @@ mod tests { #[test] fn verify_returns_false_for_wrong_payload() { - let services = build_signing_services(); + let services = build_request_signing_services(); let signer = RequestSigner::from_services(&services).expect("should create signer from services"); let signature = signer.sign(b"original").expect("should sign"); @@ -383,7 +318,7 @@ mod tests { #[test] fn verify_errors_for_unknown_kid() { - let services = build_signing_services(); + let services = build_request_signing_services(); let signer = RequestSigner::from_services(&services).expect("should create signer from services"); let signature = signer.sign(b"payload").expect("should sign"); @@ -395,7 +330,7 @@ mod tests { #[test] fn verify_errors_for_malformed_signature() { - let services = build_signing_services(); + let services = build_request_signing_services(); let signer = RequestSigner::from_services(&services).expect("should create signer from services"); @@ -452,7 +387,7 @@ mod tests { #[test] fn sign_request_enhanced_produces_verifiable_signature() { - let services = build_signing_services(); + let services = build_request_signing_services(); let signer = RequestSigner::from_services(&services).expect("should create signer from services"); let params = SigningParams::new( @@ -474,7 +409,7 @@ mod tests { #[test] fn sign_request_different_hosts_produce_different_signatures() { - let services = build_signing_services(); + let services = build_request_signing_services(); let signer = RequestSigner::from_services(&services).expect("should create signer from services"); From 079a97f8a4faf0258a56e7da2324c0276eda3d39 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Thu, 9 Apr 2026 10:52:37 +0530 Subject: [PATCH 13/14] Address review findings --- .../src/management_api.rs | 34 ++++++++-- crates/trusted-server-core/src/auction/mod.rs | 2 + .../src/auction/orchestrator.rs | 25 ++----- .../src/auction/test_support.rs | 26 +++++++ .../src/integrations/prebid.rs | 14 +--- .../src/request_signing/endpoints.rs | 59 ++++++++++++++-- .../src/request_signing/jwks.rs | 19 +----- .../src/request_signing/mod.rs | 52 +++++++++++--- .../src/request_signing/rotation.rs | 67 ++++++++++++++----- 9 files changed, 209 insertions(+), 89 deletions(-) create mode 100644 crates/trusted-server-core/src/auction/test_support.rs diff --git a/crates/trusted-server-adapter-fastly/src/management_api.rs b/crates/trusted-server-adapter-fastly/src/management_api.rs index 2d40fea4..b65ebee4 100644 --- a/crates/trusted-server-adapter-fastly/src/management_api.rs +++ b/crates/trusted-server-adapter-fastly/src/management_api.rs @@ -147,6 +147,7 @@ impl FastlyManagementApiClient { path: &str, body: Option, content_type: &str, + error_kind: fn() -> PlatformError, ) -> Result> { let url = format!("{}{}", self.base_url, path); @@ -156,7 +157,7 @@ impl FastlyManagementApiClient { "PUT" => Request::put(&url), "DELETE" => Request::delete(&url), _ => { - return Err(Report::new(PlatformError::ConfigStore) + return Err(Report::new(error_kind()) .attach(format!("unsupported HTTP method: {}", method))) } }; @@ -172,8 +173,7 @@ impl FastlyManagementApiClient { } request.send(&self.backend_name).map_err(|e| { - Report::new(PlatformError::ConfigStore) - .attach(format!("management API request failed: {}", e)) + Report::new(error_kind()).attach(format!("management API request failed: {}", e)) }) } @@ -196,6 +196,7 @@ impl FastlyManagementApiClient { &path, Some(payload), "application/x-www-form-urlencoded", + || PlatformError::ConfigStore, )?; let entity_description = format!("key '{}'", key); @@ -227,7 +228,9 @@ impl FastlyManagementApiClient { ) -> Result<(), Report> { let path = build_config_item_path(store_id, key); - let mut response = self.make_request("DELETE", &path, None, "application/json")?; + let mut response = self.make_request("DELETE", &path, None, "application/json", || { + PlatformError::ConfigStore + })?; let entity_description = format!("key '{}'", key); check_response( @@ -265,6 +268,7 @@ impl FastlyManagementApiClient { &path, Some(payload), "application/json", + || PlatformError::SecretStore, )?; let entity_description = format!("name '{}'", secret_name); @@ -296,7 +300,9 @@ impl FastlyManagementApiClient { ) -> Result<(), Report> { let path = build_secret_path(store_id, secret_name); - let mut response = self.make_request("DELETE", &path, None, "application/json")?; + let mut response = self.make_request("DELETE", &path, None, "application/json", || { + PlatformError::SecretStore + })?; let entity_description = format!("name '{}'", secret_name); check_response( @@ -372,4 +378,22 @@ mod tests { assert_eq!(truncated.len(), 200, "should cap error bodies at 200 chars"); assert_eq!(truncated, "a".repeat(200), "should trim before truncating"); } + + #[test] + fn create_secret_uses_secret_store_error_for_transport_failures() { + let client = FastlyManagementApiClient { + api_key: "test-api-key".to_string(), + base_url: FASTLY_API_HOST, + backend_name: "missing-management-backend".to_string(), + }; + + let err = client + .create_secret("store-id", "secret-name", "secret-value") + .expect_err("should fail when the management API backend is unavailable"); + + assert!( + matches!(err.current_context(), &PlatformError::SecretStore), + "should classify secret transport failures as secret-store errors" + ); + } } diff --git a/crates/trusted-server-core/src/auction/mod.rs b/crates/trusted-server-core/src/auction/mod.rs index 6fa9cc2d..acf6d520 100644 --- a/crates/trusted-server-core/src/auction/mod.rs +++ b/crates/trusted-server-core/src/auction/mod.rs @@ -19,6 +19,8 @@ pub mod endpoints; pub mod formats; pub mod orchestrator; pub mod provider; +#[cfg(test)] +pub(crate) mod test_support; pub mod types; pub use config::AuctionConfig; diff --git a/crates/trusted-server-core/src/auction/orchestrator.rs b/crates/trusted-server-core/src/auction/orchestrator.rs index e6969b1f..5a69260b 100644 --- a/crates/trusted-server-core/src/auction/orchestrator.rs +++ b/crates/trusted-server-core/src/auction/orchestrator.rs @@ -617,8 +617,9 @@ impl OrchestrationResult { #[cfg(test)] mod tests { use crate::auction::config::AuctionConfig; + use crate::auction::test_support::create_test_auction_context; use crate::auction::types::{ - AdFormat, AdSlot, AuctionContext, AuctionRequest, Bid, MediaType, PublisherInfo, UserInfo, + AdFormat, AdSlot, AuctionRequest, Bid, MediaType, PublisherInfo, UserInfo, }; use crate::platform::test_support::noop_services; use crate::test_support::tests::crate_test_settings_str; @@ -674,25 +675,6 @@ mod tests { crate::settings::Settings::from_toml(&settings_str).expect("should parse test settings") } - static TEST_SERVICES: std::sync::LazyLock = - std::sync::LazyLock::new(noop_services); - - fn create_test_context<'a>( - settings: &'a crate::settings::Settings, - req: &'a Request, - client_info: &'a crate::platform::ClientInfo, - ) -> AuctionContext<'a> { - let services: &'static crate::platform::RuntimeServices = &TEST_SERVICES; - AuctionContext { - settings, - request: req, - client_info, - timeout_ms: 2000, - provider_responses: None, - services, - } - } - #[test] fn filters_winning_bids_below_floor() { let orchestrator = AuctionOrchestrator::new(AuctionConfig::default()); @@ -780,7 +762,7 @@ mod tests { let request = create_test_auction_request(); let settings = create_test_settings(); let req = Request::get("https://test.com/test"); - let context = create_test_context( + let context = create_test_auction_context( &settings, &req, &crate::platform::ClientInfo { @@ -788,6 +770,7 @@ mod tests { tls_protocol: None, tls_cipher: None, }, + 2000, ); let result = orchestrator diff --git a/crates/trusted-server-core/src/auction/test_support.rs b/crates/trusted-server-core/src/auction/test_support.rs new file mode 100644 index 00000000..2c5b5438 --- /dev/null +++ b/crates/trusted-server-core/src/auction/test_support.rs @@ -0,0 +1,26 @@ +use std::sync::LazyLock; + +use fastly::Request; + +use super::AuctionContext; +use crate::platform::{test_support::noop_services, ClientInfo, RuntimeServices}; +use crate::settings::Settings; + +static TEST_SERVICES: LazyLock = LazyLock::new(noop_services); + +pub(crate) fn create_test_auction_context<'a>( + settings: &'a Settings, + request: &'a Request, + client_info: &'a ClientInfo, + timeout_ms: u32, +) -> AuctionContext<'a> { + let services: &'static RuntimeServices = &TEST_SERVICES; + AuctionContext { + settings, + request, + client_info, + timeout_ms, + provider_responses: None, + services, + } +} diff --git a/crates/trusted-server-core/src/integrations/prebid.rs b/crates/trusted-server-core/src/integrations/prebid.rs index a5fea62c..5daf35ad 100644 --- a/crates/trusted-server-core/src/integrations/prebid.rs +++ b/crates/trusted-server-core/src/integrations/prebid.rs @@ -1212,6 +1212,7 @@ pub fn register_auction_provider( #[cfg(test)] mod tests { use super::*; + use crate::auction::test_support::create_test_auction_context as shared_test_auction_context; use crate::auction::types::{ AdFormat, AdSlot, AuctionContext, AuctionRequest, DeviceInfo, PublisherInfo, UserInfo, }; @@ -1280,23 +1281,12 @@ mod tests { } } - static TEST_SERVICES: std::sync::LazyLock = - std::sync::LazyLock::new(crate::platform::test_support::noop_services); - fn create_test_auction_context<'a>( settings: &'a Settings, request: &'a Request, client_info: &'a crate::platform::ClientInfo, ) -> AuctionContext<'a> { - let services: &'static crate::platform::RuntimeServices = &TEST_SERVICES; - AuctionContext { - settings, - request, - client_info, - timeout_ms: 1000, - provider_responses: None, - services, - } + shared_test_auction_context(settings, request, client_info, 1000) } fn config_from_settings( diff --git a/crates/trusted-server-core/src/request_signing/endpoints.rs b/crates/trusted-server-core/src/request_signing/endpoints.rs index 57080625..8644c36b 100644 --- a/crates/trusted-server-core/src/request_signing/endpoints.rs +++ b/crates/trusted-server-core/src/request_signing/endpoints.rs @@ -82,7 +82,8 @@ pub struct VerifySignatureResponse { /// /// # Errors /// -/// Returns an error if the request body cannot be parsed as JSON or if verification fails. +/// Returns an error if the request body cannot be parsed as JSON or if the +/// response body cannot be serialized. pub fn handle_verify_signature( _settings: &Settings, services: &RuntimeServices, @@ -114,12 +115,15 @@ pub fn handle_verify_signature( message: "Signature verification failed".into(), error: Some("Invalid signature".into()), }, - Err(e) => VerifySignatureResponse { - verified: false, - kid: verify_req.kid, - message: "Verification error".into(), - error: Some(format!("{}", e)), - }, + Err(e) => { + log::warn!("signature verification failed: {e}"); + VerifySignatureResponse { + verified: false, + kid: verify_req.kid, + message: "Verification error".into(), + error: Some("internal verification error".into()), + } + } }; let response_json = serde_json::to_string(&response).map_err(|e| { @@ -482,6 +486,47 @@ mod tests { assert!(verify_resp.error.is_some()); } + #[test] + fn test_handle_verify_signature_hides_internal_error_details() { + let settings = crate::test_support::tests::create_test_settings(); + + let verify_req = VerifySignatureRequest { + payload: "test message".to_string(), + signature: "any-signature".to_string(), + kid: "missing-kid".to_string(), + }; + + let body = serde_json::to_string(&verify_req).expect("should serialize verify request"); + let mut req = Request::new(Method::POST, "https://test.com/verify-signature"); + req.set_body(body); + + let services = noop_services(); + let mut resp = handle_verify_signature(&settings, &services, req) + .expect("should return a verification response for internal errors"); + + assert_eq!(resp.get_status(), StatusCode::OK, "should return 200 OK"); + + let resp_body = resp.take_body_str(); + let verify_resp: VerifySignatureResponse = + serde_json::from_str(&resp_body).expect("should deserialize verify response"); + + assert!( + !verify_resp.verified, + "should mark internal verification errors as unverified" + ); + assert_eq!(verify_resp.kid, "missing-kid"); + assert_eq!(verify_resp.message, "Verification error"); + assert_eq!( + verify_resp.error.as_deref(), + Some("internal verification error"), + "should return a generic error to unauthenticated callers" + ); + assert!( + !resp_body.contains("failed"), + "should not leak internal error details in the response body" + ); + } + #[test] fn test_handle_verify_signature_malformed_request() { 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 cbc2b387..8d206639 100644 --- a/crates/trusted-server-core/src/request_signing/jwks.rs +++ b/crates/trusted-server-core/src/request_signing/jwks.rs @@ -13,7 +13,7 @@ use rand::rngs::OsRng; use crate::error::TrustedServerError; use crate::platform::RuntimeServices; -use crate::request_signing::JWKS_STORE_NAME; +use crate::request_signing::{read_active_kids, JWKS_STORE_NAME}; /// An Ed25519 keypair used for request signing. pub struct Keypair { @@ -70,25 +70,12 @@ 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 active_kids_str = services - .config_store() - .get(&JWKS_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 - .split(',') - .map(str::trim) - .filter(|s| !s.is_empty()) - .collect(); - + let active_kids = read_active_kids(services)?; let mut jwks = Vec::new(); for kid in active_kids { let jwk = services .config_store() - .get(&JWKS_STORE_NAME, kid) + .get(&JWKS_STORE_NAME, &kid) .change_context(TrustedServerError::Configuration { message: format!("failed to get JWK for kid: {}", kid), })?; diff --git a/crates/trusted-server-core/src/request_signing/mod.rs b/crates/trusted-server-core/src/request_signing/mod.rs index 3c7b3e59..88076709 100644 --- a/crates/trusted-server-core/src/request_signing/mod.rs +++ b/crates/trusted-server-core/src/request_signing/mod.rs @@ -5,15 +5,27 @@ //! //! # Store names vs store IDs //! -//! Fastly stores have two identifiers: +//! Platform stores have two identifiers: //! //! - **Store name** ([`JWKS_CONFIG_STORE_NAME`], [`SIGNING_SECRET_STORE_NAME`]): -//! used at the edge for reads via `ConfigStore::open` / `SecretStore::open`. -//! These are configured in `fastly.toml`. +//! used for runtime reads via [`crate::platform::PlatformConfigStore::get`] +//! and [`crate::platform::PlatformSecretStore::get_bytes`] through +//! [`crate::platform::RuntimeServices`]. These names are configured in +//! `fastly.toml` for the Fastly adapter. //! -//! - **Store ID** (`RequestSigning::config_store_id`, `RequestSigning::secret_store_id`): -//! used by the Fastly management API for writes (creating, updating, and -//! deleting items). These are set in `trusted-server.toml`. +//! - **Store ID**: used for write operations via +//! [`crate::platform::PlatformConfigStore::put`] / +//! [`crate::platform::PlatformConfigStore::delete`] and +//! [`crate::platform::PlatformSecretStore::create`] / +//! [`crate::platform::PlatformSecretStore::delete`]. These identifiers come +//! from the request-signing settings in `trusted-server.toml`. + +use std::sync::LazyLock; + +use error_stack::{Report, ResultExt}; + +use crate::error::TrustedServerError; +use crate::platform::{RuntimeServices, StoreName}; pub mod discovery; pub mod endpoints; @@ -21,27 +33,45 @@ pub mod jwks; pub mod rotation; pub mod signing; -/// Config store name for JWKS public keys (edge reads via `ConfigStore::open`). +/// Config store name for JWKS public keys used by runtime read operations. /// /// This must match the store name declared in `fastly.toml` under /// `[local_server.config_stores]`. pub const JWKS_CONFIG_STORE_NAME: &str = "jwks_store"; -/// Secret store name for Ed25519 signing keys (edge reads via `SecretStore::open`). +/// Secret store name for Ed25519 signing keys used by runtime read operations. /// /// This must match the store name declared in `fastly.toml` under /// `[local_server.secret_stores]`. pub const SIGNING_SECRET_STORE_NAME: &str = "signing_keys"; -use crate::platform::StoreName; -use std::sync::LazyLock; - +/// Lazily constructed [`StoreName`] for JWKS config-store reads. pub static JWKS_STORE_NAME: LazyLock = LazyLock::new(|| StoreName::from(JWKS_CONFIG_STORE_NAME)); +/// Lazily constructed [`StoreName`] for signing-key secret-store reads. pub static SIGNING_STORE_NAME: LazyLock = LazyLock::new(|| StoreName::from(SIGNING_SECRET_STORE_NAME)); +fn parse_active_kids(active_kids: &str) -> Vec { + active_kids + .split(',') + .map(|kid| kid.trim().to_string()) + .filter(|kid| !kid.is_empty()) + .collect() +} + +fn read_active_kids(services: &RuntimeServices) -> Result, Report> { + services + .config_store() + .get(&JWKS_STORE_NAME, "active-kids") + .change_context(TrustedServerError::Configuration { + message: "failed to read active-kids from config store".into(), + }) + .attach("while fetching active kids list") + .map(|active_kids| parse_active_kids(&active_kids)) +} + pub use discovery::*; pub use endpoints::*; pub use jwks::*; diff --git a/crates/trusted-server-core/src/request_signing/rotation.rs b/crates/trusted-server-core/src/request_signing/rotation.rs index fd2dacc7..818b8bff 100644 --- a/crates/trusted-server-core/src/request_signing/rotation.rs +++ b/crates/trusted-server-core/src/request_signing/rotation.rs @@ -9,7 +9,7 @@ use ed25519_dalek::SigningKey; use error_stack::{Report, ResultExt}; use jose_jwk::Jwk; -use super::Keypair; +use super::{read_active_kids, Keypair}; use crate::error::TrustedServerError; use crate::platform::{RuntimeServices, StoreId}; use crate::request_signing::JWKS_STORE_NAME; @@ -75,10 +75,15 @@ impl KeyRotationManager { self.store_private_key(services, &new_kid, &keypair.signing_key)?; self.store_public_jwk(services, &new_kid, &jwk)?; - let active_kids = match &previous_kid { - Some(prev) if prev != &new_kid => vec![prev.clone(), new_kid.clone()], - _ => vec![new_kid.clone()], - }; + let mut active_kids = read_active_kids(services).unwrap_or_default(); + if let Some(prev) = &previous_kid { + if prev != &new_kid && !active_kids.iter().any(|kid| kid == prev) { + active_kids.push(prev.clone()); + } + } + if !active_kids.iter().any(|kid| kid == &new_kid) { + active_kids.push(new_kid.clone()); + } self.update_current_kid(services, &new_kid)?; self.update_active_kids(services, &active_kids)?; @@ -167,18 +172,7 @@ impl KeyRotationManager { &self, services: &RuntimeServices, ) -> Result, Report> { - let active_kids_str = services - .config_store() - .get(&JWKS_STORE_NAME, "active-kids") - .change_context(TrustedServerError::Configuration { - message: "failed to read active-kids from config store".into(), - })?; - - Ok(active_kids_str - .split(',') - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - .collect()) + read_active_kids(services) } /// Deactivates a key by removing it from the active keys list. @@ -402,6 +396,45 @@ mod tests { ); } + #[test] + fn rotate_key_preserves_existing_active_kids() { + let mut data = HashMap::new(); + data.insert("current-kid".to_string(), "kid-b".to_string()); + data.insert("active-kids".to_string(), "kid-a, kid-b".to_string()); + + let config_store = SpyConfigStore::new(data); + let secret_store = SpySecretStore::new(); + let services = build_services_with_config_and_secret(config_store, secret_store); + + let manager = KeyRotationManager::new("cfg-id", "sec-id"); + let rotation = manager + .rotate_key(&services, Some("kid-c".to_string())) + .expect("should rotate key successfully"); + + assert_eq!( + rotation.active_kids, + vec![ + "kid-a".to_string(), + "kid-b".to_string(), + "kid-c".to_string() + ], + "should preserve previously active keys and append the new kid" + ); + + let active_kids = manager + .list_active_keys(&services) + .expect("should read back updated active kids"); + assert_eq!( + active_kids, + vec![ + "kid-a".to_string(), + "kid-b".to_string(), + "kid-c".to_string() + ], + "should store the full active kid list after rotation" + ); + } + #[test] fn deactivate_key_fails_when_only_one_key_remains() { let mut data = HashMap::new(); From 3924a98647550deba5114363d4980408c68ec377 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Fri, 10 Apr 2026 12:22:31 +0530 Subject: [PATCH 14/14] Address review findings --- .../src/request_signing/mod.rs | 54 ++++++++++++++++++- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/crates/trusted-server-core/src/request_signing/mod.rs b/crates/trusted-server-core/src/request_signing/mod.rs index 88076709..d5d0e79c 100644 --- a/crates/trusted-server-core/src/request_signing/mod.rs +++ b/crates/trusted-server-core/src/request_signing/mod.rs @@ -46,11 +46,11 @@ pub const JWKS_CONFIG_STORE_NAME: &str = "jwks_store"; pub const SIGNING_SECRET_STORE_NAME: &str = "signing_keys"; /// Lazily constructed [`StoreName`] for JWKS config-store reads. -pub static JWKS_STORE_NAME: LazyLock = +pub(crate) static JWKS_STORE_NAME: LazyLock = LazyLock::new(|| StoreName::from(JWKS_CONFIG_STORE_NAME)); /// Lazily constructed [`StoreName`] for signing-key secret-store reads. -pub static SIGNING_STORE_NAME: LazyLock = +pub(crate) static SIGNING_STORE_NAME: LazyLock = LazyLock::new(|| StoreName::from(SIGNING_SECRET_STORE_NAME)); fn parse_active_kids(active_kids: &str) -> Vec { @@ -77,3 +77,53 @@ pub use endpoints::*; pub use jwks::*; pub use rotation::*; pub use signing::*; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_active_kids_splits_comma_separated_kids() { + let result = parse_active_kids("kid-a,kid-b,kid-c"); + assert_eq!(result, vec!["kid-a", "kid-b", "kid-c"]); + } + + #[test] + fn parse_active_kids_trims_whitespace_around_each_kid() { + let result = parse_active_kids(" kid-a , kid-b "); + assert_eq!(result, vec!["kid-a", "kid-b"]); + } + + #[test] + fn parse_active_kids_skips_empty_segments() { + let result = parse_active_kids("kid-a,,kid-b"); + assert_eq!(result, vec!["kid-a", "kid-b"]); + } + + #[test] + fn parse_active_kids_skips_whitespace_only_segments() { + let result = parse_active_kids(" kid-a , , kid-b "); + assert_eq!(result, vec!["kid-a", "kid-b"]); + } + + #[test] + fn parse_active_kids_returns_empty_for_empty_string() { + let result = parse_active_kids(""); + assert!(result.is_empty(), "should return no kids for empty input"); + } + + #[test] + fn parse_active_kids_returns_empty_for_only_commas() { + let result = parse_active_kids(",,,"); + assert!( + result.is_empty(), + "should return no kids when input is only commas" + ); + } + + #[test] + fn parse_active_kids_handles_single_kid() { + let result = parse_active_kids("only-kid"); + assert_eq!(result, vec!["only-kid"]); + } +}