feat(security): Improve secrets handling

CONTRIBUTING.md

@@ -140,6 +140,14 @@ tooling; otherwise follow the example's README.
   describe what well-named code already says.
 - **Backwards compatibility:** don't add compatibility shims for behavior
   the task didn't ask for. See `AGENTS.md` §3, rule 6.
+- **Secrets in server memory:** every long-lived secret-bearing field in
+  `crates/server` must use a wrapper from `crates/server/src/secret/`
+  (`FixedKey<N>`, `SecretBytes`, `SecretString`, `CredentialUrl`). Read a
+  secret-bearing env var and wrap it in a single expression — never bind
+  the raw `String` to a config field or local. Reviewers must confirm this
+  for any new secret-shaped field; the compile-time assertions in
+  `crates/server/src/secret/tests.rs` enforce that wrappers can never be
+  logged (`Display`), serialized (`Serialize`), or placed in a response DTO.
 
 ## Docs
 

crates/miden-keystore/Cargo.toml

@@ -8,6 +8,7 @@ miden-protocol = { workspace = true }
 thiserror = { workspace = true }
 hex = { workspace = true }
 rand = "0.9.2"
+zeroize = "1.7"
 
 [dev-dependencies]
 rand_chacha = { workspace = true }

crates/miden-keystore/src/ecdsa_keystore.rs

@@ -7,6 +7,7 @@ use std::hash::{DefaultHasher, Hash, Hasher};
 use std::io::{BufRead, BufReader, BufWriter, Write};
 use std::path::{Path, PathBuf};
 use std::sync::Mutex;
+use zeroize::Zeroizing;
 
 type Result<T> = std::result::Result<T, KeyStoreError>;
 
@@ -62,13 +63,10 @@ impl FilesystemEcdsaKeyStore {
             })?;
 
         let mut writer = BufWriter::new(file);
-        writer
-            .write_all(hex::encode(key_bytes).as_bytes())
-            .map_err(|error| {
-                KeyStoreError::StorageError(format!(
-                    "Failed to write key to file {filename}: {error}"
-                ))
-            })?;
+        let encoded = Zeroizing::new(hex::encode(key_bytes));
+        writer.write_all(encoded.as_bytes()).map_err(|error| {
+            KeyStoreError::StorageError(format!("Failed to write key to file {filename}: {error}"))
+        })?;
 
         writer.flush().map_err(|error| {
             KeyStoreError::StorageError(format!("Failed to flush key file {filename}: {error}"))
@@ -89,16 +87,16 @@ impl FilesystemEcdsaKeyStore {
             })?;
 
         let mut reader = BufReader::new(file);
-        let mut hex_encoded = String::new();
+        let mut hex_encoded = Zeroizing::new(String::new());
         reader.read_line(&mut hex_encoded).map_err(|error| {
             KeyStoreError::StorageError(format!("Failed to read key from file {filename}: {error}"))
         })?;
 
-        let key_bytes = hex::decode(hex_encoded.trim()).map_err(|error| {
+        let key_bytes = Zeroizing::new(hex::decode(hex_encoded.trim()).map_err(|error| {
             KeyStoreError::DecodingError(format!(
                 "Failed to decode hex key from file {filename}: {error}"
             ))
-        })?;
+        })?);
 
         SecretKey::read_from_bytes(&key_bytes).map_err(|error| {
             KeyStoreError::DecodingError(format!(

crates/miden-keystore/src/keystore.rs

@@ -8,6 +8,7 @@ use std::io::{BufRead, BufReader, BufWriter, Write};
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, RwLock};
 use thiserror::Error;
+use zeroize::Zeroizing;
 
 #[derive(Debug, Error)]
 pub enum KeyStoreError {
@@ -66,13 +67,10 @@ impl<R: RngCore + Send + Sync> FilesystemKeyStore<R> {
             })?;
 
         let mut writer = BufWriter::new(file);
-        writer
-            .write_all(hex::encode(key_bytes).as_bytes())
-            .map_err(|error| {
-                KeyStoreError::StorageError(format!(
-                    "Failed to write key to file {filename}: {error}"
-                ))
-            })?;
+        let encoded = Zeroizing::new(hex::encode(key_bytes));
+        writer.write_all(encoded.as_bytes()).map_err(|error| {
+            KeyStoreError::StorageError(format!("Failed to write key to file {filename}: {error}"))
+        })?;
 
         writer.flush().map_err(|error| {
             KeyStoreError::StorageError(format!("Failed to flush key file {filename}: {error}"))
@@ -93,16 +91,16 @@ impl<R: RngCore + Send + Sync> FilesystemKeyStore<R> {
             })?;
 
         let mut reader = BufReader::new(file);
-        let mut hex_encoded = String::new();
+        let mut hex_encoded = Zeroizing::new(String::new());
         reader.read_line(&mut hex_encoded).map_err(|error| {
             KeyStoreError::StorageError(format!("Failed to read key from file {filename}: {error}"))
         })?;
 
-        let key_bytes = hex::decode(hex_encoded.trim()).map_err(|error| {
+        let key_bytes = Zeroizing::new(hex::decode(hex_encoded.trim()).map_err(|error| {
             KeyStoreError::DecodingError(format!(
                 "Failed to decode hex key from file {filename}: {error}"
             ))
-        })?;
+        })?);
 
         SecretKey::read_from_bytes(&key_bytes).map_err(|error| {
             KeyStoreError::DecodingError(format!(

crates/server/Cargo.toml

@@ -25,7 +25,11 @@ hmac = "0.12"
 sha2 = "0.10"
 rand = "0.9"
 rand_chacha = { workspace = true }
+secrecy = { version = "0.10", default-features = false }
 serde = { workspace = true }
+subtle = "2.5"
+url = "2.5"
+zeroize = { version = "1.7", features = ["derive"] }
 serde_json = { workspace = true }
 tokio = { workspace = true, features = ["full"] }
 tonic = { workspace = true }
@@ -63,6 +67,7 @@ tonic-prost-build = { workspace = true }
 
 [dev-dependencies]
 futures = "0.3"
+static_assertions = "1.1"
 tempfile = { workspace = true }
 uuid = { version = "1.0", features = ["v4"] }
 miden-standards = { workspace = true }

crates/server/src/ack/miden_ecdsa/signer.rs

@@ -48,6 +48,13 @@ impl MidenEcdsaSigner {
 
 impl MidenEcdsaSigner {
     pub(crate) fn sign_with_server_key(&self, message: Word) -> crate::ack::Result<Signature> {
+        // Verified: miden-crypto 0.23.0 (pulled in by miden-protocol 0.14.5)
+        // implements `impl ZeroizeOnDrop for SecretKey {}` in
+        // src/dsa/ecdsa_k256_keccak/mod.rs (delegating to k256's
+        // ZeroizeOnDrop), so the per-call loaded key material is zeroed
+        // when the signing frame returns. The local file-read buffer inside
+        // miden-keystore is wrapped in zeroize::Zeroizing for the same
+        // reason.
         Ok(self.keystore.ecdsa_sign(self.server_pubkey_word, message)?)
     }
 

crates/server/src/ack/miden_falcon_rpo/signer.rs

@@ -49,6 +49,13 @@ impl MidenFalconRpoSigner {
 
 impl MidenFalconRpoSigner {
     pub(crate) fn sign_with_server_key(&self, message: Word) -> crate::ack::Result<Signature> {
+        // Verified: miden-crypto 0.23.0 (pulled in by miden-protocol 0.14.5)
+        // implements `impl ZeroizeOnDrop for SecretKey {}` in
+        // src/dsa/falcon512_poseidon2/keys/secret_key.rs (the SecretKey type
+        // here), so the per-call loaded key material is zeroed when the
+        // signing frame returns. The local file-read buffer inside
+        // miden-keystore is wrapped in zeroize::Zeroizing for the same
+        // reason.
         Ok(self.keystore.sign(self.server_pubkey_word, message)?)
     }
 

crates/server/src/ack/secrets_manager.rs

@@ -1,4 +1,5 @@
 use crate::error::{GuardianError, Result};
+use crate::secret::{SecretBytes, SecretString};
 use async_trait::async_trait;
 use aws_config::BehaviorVersion;
 use aws_sdk_secretsmanager::Client;
@@ -39,7 +40,7 @@ impl AwsSecretsManagerProvider {
         })
     }
 
-    async fn secret_string(&self, secret_id: &str) -> Result<String> {
+    async fn secret_string(&self, secret_id: &str) -> Result<SecretString> {
         let response = self
             .client
             .get_secret_value()
@@ -52,25 +53,30 @@ impl AwsSecretsManagerProvider {
                 ))
             })?;
 
-        response.secret_string().map(str::to_owned).ok_or_else(|| {
-            GuardianError::ConfigurationError(format!(
-                "Secret {secret_id} does not contain a secret string value"
-            ))
-        })
+        response
+            .secret_string()
+            .map(|s| SecretString::new(s.to_owned()))
+            .ok_or_else(|| {
+                GuardianError::ConfigurationError(format!(
+                    "Secret {secret_id} does not contain a secret string value"
+                ))
+            })
     }
 
     async fn parsed_secret_key<T, F>(&self, secret_id: &str, parser: F) -> Result<T>
     where
         F: FnOnce(&[u8]) -> std::result::Result<T, String>,
     {
         let secret_hex = self.secret_string(secret_id).await?;
-        let secret_bytes = hex::decode(secret_hex.trim()).map_err(|error| {
-            GuardianError::ConfigurationError(format!(
-                "Secret {secret_id} must contain valid hex-encoded key bytes: {error}"
-            ))
-        })?;
+        let secret_bytes = SecretBytes::new(
+            hex::decode(secret_hex.expose_secret().trim()).map_err(|error| {
+                GuardianError::ConfigurationError(format!(
+                    "Secret {secret_id} must contain valid hex-encoded key bytes: {error}"
+                ))
+            })?,
+        );
 
-        parser(&secret_bytes).map_err(|error| {
+        parser(secret_bytes.expose_secret()).map_err(|error| {
             GuardianError::ConfigurationError(format!(
                 "Secret {secret_id} does not contain a valid key: {error}"
             ))

crates/server/src/audit/postgres.rs

@@ -261,12 +261,15 @@ mod tests {
         use diesel::QueryDsl;
         use diesel_async::RunQueryDsl;
 
-        let database_url = std::env::var("DATABASE_URL")
-            .expect("DATABASE_URL must be set; this test is marked #[ignore] by default");
-        crate::storage::postgres::run_migrations(&database_url)
+        let database_url = crate::secret::CredentialUrl::new(
+            std::env::var("DATABASE_URL")
+                .expect("DATABASE_URL must be set; this test is marked #[ignore] by default"),
+        );
+        let raw_url = database_url.expose_secret();
+        crate::storage::postgres::run_migrations(raw_url)
             .await
             .expect("migrations run");
-        let pool = build_postgres_pool_lazy(&database_url, 1).expect("pool builds");
+        let pool = build_postgres_pool_lazy(raw_url, 1).expect("pool builds");
         // Sanity: prove the pool is reachable; if not, abort early
         // with a clear error rather than a confusing trigger failure.
         let _ = pool