From 33c88add8a0882778c58644bb342cf83063ef19b Mon Sep 17 00:00:00 2001 From: Christian Date: Fri, 6 Mar 2026 08:15:07 -0500 Subject: [PATCH 1/2] Replace hardcoded store name strings with constants in request signing Scattered string literals for store names ("jwks_store", "signing_keys") made the request signing module fragile and inconsistent with the config-driven store IDs used by the admin endpoints. Define JWKS_CONFIG_STORE_NAME and SIGNING_SECRET_STORE_NAME constants as the single source of truth for edge-side store names, and document the distinction between store names (edge reads) and store IDs (API writes). Closes #399 --- crates/common/src/request_signing/jwks.rs | 3 ++- crates/common/src/request_signing/mod.rs | 24 +++++++++++++++++++ crates/common/src/request_signing/rotation.rs | 13 +++++++++- crates/common/src/request_signing/signing.rs | 9 +++---- 4 files changed, 43 insertions(+), 6 deletions(-) diff --git a/crates/common/src/request_signing/jwks.rs b/crates/common/src/request_signing/jwks.rs index 27115899..24b86fb4 100644 --- a/crates/common/src/request_signing/jwks.rs +++ b/crates/common/src/request_signing/jwks.rs @@ -13,6 +13,7 @@ use rand::rngs::OsRng; use crate::error::TrustedServerError; use crate::fastly_storage::FastlyConfigStore; +use crate::request_signing::JWKS_CONFIG_STORE_NAME; pub struct Keypair { pub signing_key: SigningKey, @@ -60,7 +61,7 @@ impl Keypair { /// /// Returns an error if the config store cannot be accessed or if active keys cannot be retrieved. pub fn get_active_jwks() -> Result> { - let store = FastlyConfigStore::new("jwks_store"); + let store = FastlyConfigStore::new(JWKS_CONFIG_STORE_NAME); let active_kids_str = store .get("active-kids") .attach("while fetching active kids list")?; diff --git a/crates/common/src/request_signing/mod.rs b/crates/common/src/request_signing/mod.rs index c6904e41..41507940 100644 --- a/crates/common/src/request_signing/mod.rs +++ b/crates/common/src/request_signing/mod.rs @@ -2,6 +2,18 @@ //! //! This module provides cryptographic signing capabilities using Ed25519 keys, //! including JWKS management, key rotation, and signature verification. +//! +//! # Store names vs store IDs +//! +//! Fastly 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`. +//! +//! - **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`. pub mod discovery; pub mod endpoints; @@ -9,6 +21,18 @@ pub mod jwks; pub mod rotation; pub mod signing; +/// Config store name for JWKS public keys (edge reads via `ConfigStore::open`). +/// +/// 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`). +/// +/// This must match the store name declared in `fastly.toml` under +/// `[local_server.secret_stores]`. +pub const SIGNING_SECRET_STORE_NAME: &str = "signing_keys"; + pub use discovery::*; pub use endpoints::*; pub use jwks::*; diff --git a/crates/common/src/request_signing/rotation.rs b/crates/common/src/request_signing/rotation.rs index 8ddca3a7..46c93c06 100644 --- a/crates/common/src/request_signing/rotation.rs +++ b/crates/common/src/request_signing/rotation.rs @@ -10,6 +10,9 @@ use jose_jwk::Jwk; use crate::error::TrustedServerError; use crate::fastly_storage::{FastlyApiClient, FastlyConfigStore}; +use crate::request_signing::JWKS_CONFIG_STORE_NAME; +#[allow(unused_imports)] +use crate::request_signing::SIGNING_SECRET_STORE_NAME; use super::Keypair; @@ -22,15 +25,23 @@ pub struct KeyRotationResult { } 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, } impl KeyRotationManager { /// Creates a new key rotation manager. /// + /// The `config_store_id` and `secret_store_id` are Fastly management API + /// identifiers used for write operations. Edge reads use the store names + /// defined in [`JWKS_CONFIG_STORE_NAME`] and [`SIGNING_SECRET_STORE_NAME`]. + /// /// # Errors /// /// Returns an error if the API client cannot be initialized. @@ -41,7 +52,7 @@ impl KeyRotationManager { let config_store_id = config_store_id.into(); let secret_store_id = secret_store_id.into(); - let config_store = FastlyConfigStore::new("jwks_store"); + let config_store = FastlyConfigStore::new(JWKS_CONFIG_STORE_NAME); let api_client = FastlyApiClient::new()?; Ok(Self { diff --git a/crates/common/src/request_signing/signing.rs b/crates/common/src/request_signing/signing.rs index 7086bfdf..c256da02 100644 --- a/crates/common/src/request_signing/signing.rs +++ b/crates/common/src/request_signing/signing.rs @@ -10,6 +10,7 @@ use serde::Serialize; use crate::error::TrustedServerError; use crate::fastly_storage::{FastlyConfigStore, FastlySecretStore}; +use crate::request_signing::{JWKS_CONFIG_STORE_NAME, SIGNING_SECRET_STORE_NAME}; /// Retrieves the current active key ID from the config store. /// @@ -17,7 +18,7 @@ use crate::fastly_storage::{FastlyConfigStore, FastlySecretStore}; /// /// Returns an error if the config store cannot be accessed or the current-kid key is not found. pub fn get_current_key_id() -> Result> { - let store = FastlyConfigStore::new("jwks_store"); + let store = FastlyConfigStore::new(JWKS_CONFIG_STORE_NAME); store.get("current-kid") } @@ -119,7 +120,7 @@ impl RequestSigner { /// /// Returns an error if the key ID cannot be retrieved or the key cannot be parsed. pub fn from_config() -> Result> { - let config_store = FastlyConfigStore::new("jwks_store"); + let config_store = FastlyConfigStore::new(JWKS_CONFIG_STORE_NAME); let key_id = config_store .get("current-kid") @@ -127,7 +128,7 @@ impl RequestSigner { message: "Failed to get current-kid".into(), })?; - let secret_store = FastlySecretStore::new("signing_keys"); + 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))?; @@ -177,7 +178,7 @@ pub fn verify_signature( signature_b64: &str, kid: &str, ) -> Result> { - let store = FastlyConfigStore::new("jwks_store"); + let store = FastlyConfigStore::new(JWKS_CONFIG_STORE_NAME); let jwk_json = store .get(kid) .change_context(TrustedServerError::Configuration { From 81fd86cdee7a317f3cedb01ca59c196ba1178c4c Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 9 Mar 2026 14:49:21 -0500 Subject: [PATCH 2/2] Address PR review: remove unused import and clarify test store IDs - Drop #[allow(unused_imports)] on SIGNING_SECRET_STORE_NAME and use a fully-qualified path in the doc comment instead - Rename test store ID literals from 'jwks_store'/'signing_keys' to 'test-config-store-id'/'test-secret-store-id' to make the semantic distinction between store names (edge reads) and store IDs (management API) explicit --- crates/common/src/request_signing/rotation.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/crates/common/src/request_signing/rotation.rs b/crates/common/src/request_signing/rotation.rs index 46c93c06..da3abd9e 100644 --- a/crates/common/src/request_signing/rotation.rs +++ b/crates/common/src/request_signing/rotation.rs @@ -11,8 +11,6 @@ use jose_jwk::Jwk; use crate::error::TrustedServerError; use crate::fastly_storage::{FastlyApiClient, FastlyConfigStore}; use crate::request_signing::JWKS_CONFIG_STORE_NAME; -#[allow(unused_imports)] -use crate::request_signing::SIGNING_SECRET_STORE_NAME; use super::Keypair; @@ -40,7 +38,7 @@ impl KeyRotationManager { /// /// The `config_store_id` and `secret_store_id` are Fastly management API /// identifiers used for write operations. Edge reads use the store names - /// defined in [`JWKS_CONFIG_STORE_NAME`] and [`SIGNING_SECRET_STORE_NAME`]. + /// defined in [`JWKS_CONFIG_STORE_NAME`] and [`crate::request_signing::SIGNING_SECRET_STORE_NAME`]. /// /// # Errors /// @@ -229,11 +227,11 @@ mod tests { #[test] fn test_key_rotation_manager_creation() { - let result = KeyRotationManager::new("jwks_store", "signing_keys"); + let result = KeyRotationManager::new("test-config-store-id", "test-secret-store-id"); match result { Ok(manager) => { - assert_eq!(manager.config_store_id, "jwks_store"); - assert_eq!(manager.secret_store_id, "signing_keys"); + 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); @@ -243,7 +241,7 @@ mod tests { #[test] fn test_list_active_keys() { - let result = KeyRotationManager::new("jwks_store", "signing_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) => {