From 98c9c33ea0ef3369908b5ac312abde2c2a234f1f Mon Sep 17 00:00:00 2001 From: Wes Date: Mon, 6 Apr 2026 08:30:18 -0400 Subject: [PATCH] fix: correct zeroize annotation placement and avoid secret cloning in protected.rs (#108) S3Credentials had #[zeroize(skip)] on secret_access_key instead of access_key_id. The comment said "access_key_id is not sensitive" but the annotation was on the adjacent field. In practice ProtectedString has its own #[zeroize(drop)] so the secret was still zeroed on drop, but the annotation contradicted its own comment and would silently break if ProtectedString's drop behavior ever changed. ProtectedString::into_inner() cloned self.inner instead of moving it out. Use std::mem::take to swap the value out directly, avoiding the unnecessary heap allocation of the clone. // ticktockbent --- nora-registry/src/secrets/protected.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nora-registry/src/secrets/protected.rs b/nora-registry/src/secrets/protected.rs index 9582c21..be867ff 100644 --- a/nora-registry/src/secrets/protected.rs +++ b/nora-registry/src/secrets/protected.rs @@ -33,8 +33,8 @@ impl ProtectedString { } /// Consume and return the inner value - pub fn into_inner(self) -> Zeroizing { - Zeroizing::new(self.inner.clone()) + pub fn into_inner(mut self) -> Zeroizing { + Zeroizing::new(std::mem::take(&mut self.inner)) } /// Check if the secret is empty @@ -74,8 +74,8 @@ impl From<&str> for ProtectedString { #[derive(Clone, Zeroize)] #[zeroize(drop)] pub struct S3Credentials { - pub access_key_id: String, #[zeroize(skip)] // access_key_id is not sensitive + pub access_key_id: String, pub secret_access_key: ProtectedString, pub region: Option, }