Merge ci/redact into secret module
SecretRegistry and redact are secret-handling concerns that belong alongside SecretString. Moves UnknownSecret error variant to secret::Error to break the ci dependency. Assisted-by: GLM-5.1 via pi
diff --git a/src/ci/error.rs b/src/ci/error.rs
index d2912d8..62daaa4 100644
--- a/src/ci/error.rs
+++ b/src/ci/error.rs
@@ -80,9 +80,6 @@ pub enum Error {
#[source]
source: std::io::Error,
},
-
- #[error("unknown secret: {0:?}")]
- UnknownSecret(String),
}
pub type Result<T> = std::result::Result<T, Error>;
diff --git a/src/ci/mirror.rs b/src/ci/mirror.rs
index 1af69f2..22dc637 100644
--- a/src/ci/mirror.rs
+++ b/src/ci/mirror.rs
@@ -253,7 +253,7 @@ mod tests {
use crate::ci::pipeline::{Diagnostic, RustRunFn, compile};
use crate::ci::run::RunMeta;
use crate::ci::runtime::{ExecutorRuntime, RuntimeHandle};
- use crate::secret::SecretString;
+ use crate::secret::{Error as SecretError, SecretString};
/// Set up a bare git repo with one commit. Returns the tempdir,
/// the bare repo path, and the head SHA.
@@ -617,7 +617,7 @@ mod tests {
runtime.leave_job();
assert!(
- matches!(err, Error::UnknownSecret(ref name) if name == "missing"),
+ matches!(err, Error::Secret(SecretError::UnknownSecret(ref name)) if name == "missing"),
"expected UnknownSecret(\"missing\"), got: {err:?}"
);
}
diff --git a/src/ci/mod.rs b/src/ci/mod.rs
index 1d75abf..6d971a6 100644
--- a/src/ci/mod.rs
+++ b/src/ci/mod.rs
@@ -6,7 +6,6 @@ pub(crate) mod docker;
pub(crate) mod logs;
mod mirror;
mod pipeline;
-mod redact;
mod registration;
mod run;
mod runtime;
@@ -15,7 +14,6 @@ pub(crate) mod error;
pub use error::{Error, Result};
pub use pipeline::{DefinitionError, Diagnostic, Job, Pipeline, PipelineError, StructureError};
-pub use redact::{SecretRegistry, redact};
pub use run::{Executor, Run, RunMeta, RunState, Runs, materialize_workspace, reconcile_orphans};
/// A resolved commit reference.
diff --git a/src/ci/redact.rs b/src/ci/redact.rs
deleted file mode 100644
index 132d3cf..0000000
--- a/src/ci/redact.rs
+++ /dev/null
@@ -1,280 +0,0 @@
-//! Secret redaction for CI output surfaces.
-//!
-//! [`SecretRegistry`] holds both declared secrets (for lookup by name)
-//! and their revealed values (for output redaction). A single struct
-//! replaces the previous split between `Runtime.secrets` and a
-//! separate redaction registry.
-//!
-//! [`Revealed`] is an opaque wrapper without a [`Debug`] impl, so a
-//! revealed value can't accidentally land in `tracing::debug!` or a
-//! panic message. That's the only memory-side protection here: copies
-//! made earlier in the pipeline (the `String` returned by `reveal`,
-//! Lua VM internals, intermediate `format!` allocations) aren't
-//! zeroized, so the registry is best thought of as preventing
-//! accidental display, not preventing memory disclosure.
-
-use std::collections::HashMap;
-
-use crate::secret::SecretString;
-
-/// Opaque wrapper for a revealed secret value. No Debug impl.
-struct Revealed(String);
-
-impl Revealed {
- fn new(value: String) -> Self {
- Self(value)
- }
-
- fn as_str(&self) -> &str {
- &self.0
- }
-}
-
-// Explicitly no Debug impl — revealed values must never be printed.
-
-/// Per-run secret store: holds declared secrets and their revealed
-/// values for both lookup and redaction.
-///
-/// Constructed with the declared secrets from global config.
-/// As `(secret :name)` is called during CI execution, values are
-/// revealed and cached for redaction via [`redact`].
-///
-/// Lifetime is bounded to a single CI run. Do not carry a registry
-/// across runs — values from previous runs would contaminate
-/// redaction of unrelated output.
-pub struct SecretRegistry {
- /// name → declared secret (lazy reveal).
- declared: HashMap<String, SecretString>,
- /// name → revealed value (opaque, zeroed on drop).
- /// Populated on first `(secret :name)` call.
- revealed: HashMap<String, Revealed>,
-}
-
-impl std::fmt::Debug for SecretRegistry {
- fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
- f.debug_struct("SecretRegistry")
- .field("declared", &self.declared.keys().collect::<Vec<_>>())
- .field("revealed", &self.revealed.keys().collect::<Vec<_>>())
- .finish()
- }
-}
-
-impl From<HashMap<String, SecretString>> for SecretRegistry {
- fn from(declared: HashMap<String, SecretString>) -> Self {
- Self {
- declared,
- revealed: HashMap::new(),
- }
- }
-}
-
-impl From<Vec<(String, SecretString)>> for SecretRegistry {
- fn from(pairs: Vec<(String, SecretString)>) -> Self {
- Self::from(pairs.into_iter().collect::<HashMap<_, _>>())
- }
-}
-
-impl From<Vec<(&str, &str)>> for SecretRegistry {
- fn from(pairs: Vec<(&str, &str)>) -> Self {
- let declared: HashMap<String, SecretString> = pairs
- .into_iter()
- .map(|(k, v)| (k.to_string(), SecretString::from(v)))
- .collect();
- Self::from(declared)
- }
-}
-
-impl SecretRegistry {
- /// Resolve a declared secret by name, caching the revealed value
- /// for redaction. Returns `Err` if the name isn't declared or
- /// the source can't be read.
- ///
- /// Values shorter than 8 characters are returned to the caller
- /// but not registered for redaction — the false-positive rate on
- /// common short strings like "true" or "yes" is too high. A warn
- /// is emitted so an operator can see why a short token is showing
- /// up unredacted in CI output.
- pub fn resolve(&mut self, name: &str) -> super::error::Result<String> {
- let secret = self
- .declared
- .get(name)
- .ok_or_else(|| super::error::Error::UnknownSecret(name.to_string()))?;
- let value = secret.reveal()?.to_string();
- if value.len() >= 8 {
- self.revealed
- .insert(name.to_string(), Revealed::new(value.clone()));
- } else {
- tracing::warn!(
- secret = %name,
- length = value.len(),
- "secret value is shorter than the 8-byte minimum and will not be redacted from CI output"
- );
- }
- Ok(value)
- }
-
- /// Return revealed (name, value) pairs sorted by value length
- /// descending so longest matches are replaced first (prevents
- /// partial replacement of overlapping secrets). Equal-length
- /// values tiebreak on name, so two names that map to the same
- /// value redact deterministically.
- fn entries(&self) -> Vec<(&str, &str)> {
- let mut entries: Vec<_> = self
- .revealed
- .iter()
- .map(|(k, v)| (k.as_str(), v.as_str()))
- .collect();
- entries.sort_by(|a, b| b.1.len().cmp(&a.1.len()).then_with(|| a.0.cmp(b.0)));
- entries
- }
-
- pub fn has_redactions(&self) -> bool {
- !self.revealed.is_empty()
- }
-}
-
-/// Replace any revealed secret value in `text` with `{{ name }}`.
-///
-/// Longest values are replaced first to prevent partial matches.
-/// Returns the input unchanged when no secrets have been revealed.
-pub fn redact(text: &str, registry: &SecretRegistry) -> String {
- if !registry.has_redactions() {
- return text.to_string();
- }
- let mut result = text.to_string();
- for (name, value) in registry.entries() {
- let replacement = format!("{{{{ {} }}}}", name);
- result = result.replace(value, &replacement);
- }
- result
-}
-
-#[cfg(test)]
-mod tests {
- use super::*;
-
- #[test]
- fn redact_replaces_secret_value() {
- let mut reg = SecretRegistry::from(vec![("github_token", "ghp_abc123xyz")]);
- reg.resolve("github_token").unwrap();
- assert_eq!(
- redact("push with token ghp_abc123xyz failed", ®),
- "push with token {{ github_token }} failed"
- );
- }
-
- #[test]
- fn redact_handles_multiple_secrets() {
- let mut reg = SecretRegistry::from(vec![("token_a", "aaaaaaaa"), ("token_b", "bbbbbbbb")]);
- reg.resolve("token_a").unwrap();
- reg.resolve("token_b").unwrap();
- let result = redact("aaaaaaaa and bbbbbbbb", ®);
- assert_eq!(result, "{{ token_a }} and {{ token_b }}");
- }
-
- #[test]
- fn redact_longest_first_prevents_partial_overlap() {
- let mut reg =
- SecretRegistry::from(vec![("short", "abcdefgh"), ("long", "abcdefghijklmnop")]);
- reg.resolve("short").unwrap();
- reg.resolve("long").unwrap();
- assert_eq!(redact("abcdefghijklmnop here", ®), "{{ long }} here");
- }
-
- #[test]
- fn redact_returns_unchanged_when_nothing_revealed() {
- let reg = SecretRegistry::from(vec![("key", "secret_value")]);
- assert_eq!(redact("nothing to see", ®), "nothing to see");
- }
-
- #[test]
- fn redact_ignores_short_secrets() {
- let mut reg = SecretRegistry::from(vec![("tiny", "abcdefg")]);
- reg.resolve("tiny").unwrap();
- assert_eq!(redact("abcdefg is short", ®), "abcdefg is short");
- }
-
- #[test]
- fn redact_similar_but_not_equal_passes_through() {
- let mut reg = SecretRegistry::from(vec![("token", "ghp_abc123xyz")]);
- reg.resolve("token").unwrap();
- assert_eq!(
- redact("ghp_abc124xyz is close but not equal", ®),
- "ghp_abc124xyz is close but not equal"
- );
- }
-
- #[test]
- fn redact_replaces_all_occurrences() {
- let mut reg = SecretRegistry::from(vec![("key", "secret_password")]);
- reg.resolve("key").unwrap();
- assert_eq!(
- redact("secret_password secret_password secret_password", ®),
- "{{ key }} {{ key }} {{ key }}"
- );
- }
-
- #[test]
- fn minimum_length_is_8() {
- let mut reg = SecretRegistry::from(vec![("short", "1234567"), ("ok", "12345678")]);
- // 7 chars — too short for redaction
- reg.resolve("short").unwrap();
- assert_eq!(redact("1234567", ®), "1234567");
-
- // 8 chars — just enough
- reg.resolve("ok").unwrap();
- assert_eq!(redact("12345678", ®), "{{ ok }}");
- }
-
- #[test]
- fn resolve_errors_for_unknown_name() {
- let mut reg = SecretRegistry::from(HashMap::new());
- let err = reg.resolve("missing").unwrap_err();
- assert!(
- matches!(err, super::super::error::Error::UnknownSecret(ref n) if n == "missing"),
- "expected UnknownSecret, got: {err:?}"
- );
- }
-
- #[test]
- fn resolve_returns_value() {
- let mut reg = SecretRegistry::from(vec![("key", "hunter2")]);
- assert_eq!(reg.resolve("key").unwrap(), "hunter2");
- }
-
- #[test]
- fn redact_is_idempotent() {
- let mut reg = SecretRegistry::from(vec![("token", "ghp_long_secret_value")]);
- reg.resolve("token").unwrap();
- let input = "hello ghp_long_secret_value world";
- let first = redact(input, ®);
- let second = redact(&first, ®);
- assert_eq!(first, second);
- }
-
- #[test]
- fn redact_preserves_non_matching_text() {
- let mut reg = SecretRegistry::from(vec![("token", "ghp_long_secret_value")]);
- reg.resolve("token").unwrap();
- let input = "nothing to see here";
- assert_eq!(redact(input, ®), input);
- }
-
- #[test]
- fn redact_with_no_resolves_is_identity() {
- let reg = SecretRegistry::from(vec![("token", "ghp_long_secret_value")]);
- // No resolve — no redactions registered.
- let input = "contains ghp_long_secret_value but not resolved";
- assert_eq!(redact(input, ®), input);
- }
-
- #[test]
- fn redact_tiebreaks_equal_length_by_name() {
- // Two names with the same revealed value: alphabetical name wins.
- let mut reg =
- SecretRegistry::from(vec![("zzz_late", "samevalue"), ("aaa_early", "samevalue")]);
- reg.resolve("zzz_late").unwrap();
- reg.resolve("aaa_early").unwrap();
- assert_eq!(redact("samevalue", ®), "{{ aaa_early }}");
- }
-}
diff --git a/src/ci/runtime.rs b/src/ci/runtime.rs
index 8685986..6d95670 100644
--- a/src/ci/runtime.rs
+++ b/src/ci/runtime.rs
@@ -15,9 +15,7 @@ use mlua::{IntoLua, Lua, LuaSerdeExt};
use super::pipeline::{Job, Pipeline};
use super::run::{DockerLifecycle, RunMeta};
-use crate::secret::SecretString;
-
-use super::redact::{SecretRegistry, redact};
+use crate::secret::{SecretRegistry, SecretString, redact};
/// Per-sh timing: (index, started_at, finished_at).
pub(super) type ShTimings = Vec<(usize, Timestamp, Timestamp)>;
@@ -202,7 +200,7 @@ impl Runtime {
/// Errors if the name isn't declared or the secret's source
/// can't be read.
pub(super) fn secret(&self, name: &str) -> super::error::Result<String> {
- self.registry.borrow_mut().resolve(name)
+ self.registry.borrow_mut().resolve(name).map_err(Into::into)
}
/// Run `cmd` with `opts` and record its output against the
diff --git a/src/secret.rs b/src/secret.rs
index c948738..40efeb3 100644
--- a/src/secret.rs
+++ b/src/secret.rs
@@ -1,3 +1,4 @@
+use std::collections::HashMap;
use std::path::PathBuf;
use std::sync::OnceLock;
@@ -15,6 +16,9 @@ pub enum Error {
/// type), we can store a structured error instead of a string.
#[error("secret resolution failed: {0}")]
Resolve(String),
+
+ #[error("unknown secret: {0:?}")]
+ UnknownSecret(String),
}
pub type Result<T> = std::result::Result<T, Error>;
@@ -136,6 +140,140 @@ impl<'de> serde::Deserialize<'de> for SecretString {
}
}
+// ── Secret registry and redaction ───────────────────────────────
+
+/// Opaque wrapper for a revealed secret value. No Debug impl.
+struct Revealed(String);
+
+impl Revealed {
+ fn new(value: String) -> Self {
+ Self(value)
+ }
+
+ fn as_str(&self) -> &str {
+ &self.0
+ }
+}
+
+// Explicitly no Debug impl — revealed values must never be printed.
+
+/// Per-run secret store: holds declared secrets and their revealed
+/// values for both lookup and redaction.
+///
+/// Constructed with the declared secrets from global config.
+/// As `(secret :name)` is called during CI execution, values are
+/// revealed and cached for redaction via [`redact`].
+///
+/// Lifetime is bounded to a single CI run. Do not carry a registry
+/// across runs — values from previous runs would contaminate
+/// redaction of unrelated output.
+pub struct SecretRegistry {
+ /// name → declared secret (lazy reveal).
+ declared: HashMap<String, SecretString>,
+ /// name → revealed value (opaque, zeroed on drop).
+ /// Populated on first `(secret :name)` call.
+ revealed: HashMap<String, Revealed>,
+}
+
+impl std::fmt::Debug for SecretRegistry {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ f.debug_struct("SecretRegistry")
+ .field("declared", &self.declared.keys().collect::<Vec<_>>())
+ .field("revealed", &self.revealed.keys().collect::<Vec<_>>())
+ .finish()
+ }
+}
+
+impl From<HashMap<String, SecretString>> for SecretRegistry {
+ fn from(declared: HashMap<String, SecretString>) -> Self {
+ Self {
+ declared,
+ revealed: HashMap::new(),
+ }
+ }
+}
+
+impl From<Vec<(String, SecretString)>> for SecretRegistry {
+ fn from(pairs: Vec<(String, SecretString)>) -> Self {
+ Self::from(pairs.into_iter().collect::<HashMap<_, _>>())
+ }
+}
+
+impl From<Vec<(&str, &str)>> for SecretRegistry {
+ fn from(pairs: Vec<(&str, &str)>) -> Self {
+ let declared: HashMap<String, SecretString> = pairs
+ .into_iter()
+ .map(|(k, v)| (k.to_string(), SecretString::from(v)))
+ .collect();
+ Self::from(declared)
+ }
+}
+
+impl SecretRegistry {
+ /// Resolve a declared secret by name, caching the revealed value
+ /// for redaction. Returns `Err` if the name isn't declared or
+ /// the source can't be read.
+ ///
+ /// Values shorter than 8 characters are returned to the caller
+ /// but not registered for redaction — the false-positive rate on
+ /// common short strings like "true" or "yes" is too high. A warn
+ /// is emitted so an operator can see why a short token is showing
+ /// up unredacted in CI output.
+ pub fn resolve(&mut self, name: &str) -> Result<String> {
+ let secret = self
+ .declared
+ .get(name)
+ .ok_or_else(|| Error::UnknownSecret(name.to_string()))?;
+ let value = secret.reveal()?.to_string();
+ if value.len() >= 8 {
+ self.revealed
+ .insert(name.to_string(), Revealed::new(value.clone()));
+ } else {
+ tracing::warn!(
+ secret = %name,
+ length = value.len(),
+ "secret value is shorter than the 8-byte minimum and will not be redacted from CI output"
+ );
+ }
+ Ok(value)
+ }
+
+ /// Return revealed (name, value) pairs sorted by value length
+ /// descending so longest matches are replaced first (prevents
+ /// partial replacement of overlapping secrets). Equal-length
+ /// values tiebreak on name, so two names that map to the same
+ /// value redact deterministically.
+ fn entries(&self) -> Vec<(&str, &str)> {
+ let mut entries: Vec<_> = self
+ .revealed
+ .iter()
+ .map(|(k, v)| (k.as_str(), v.as_str()))
+ .collect();
+ entries.sort_by(|a, b| b.1.len().cmp(&a.1.len()).then_with(|| a.0.cmp(b.0)));
+ entries
+ }
+
+ pub fn has_redactions(&self) -> bool {
+ !self.revealed.is_empty()
+ }
+}
+
+/// Replace any revealed secret value in `text` with `{{ name }}`.
+///
+/// Longest values are replaced first to prevent partial matches.
+/// Returns the input unchanged when no secrets have been revealed.
+pub fn redact(text: &str, registry: &SecretRegistry) -> String {
+ if !registry.has_redactions() {
+ return text.to_string();
+ }
+ let mut result = text.to_string();
+ for (name, value) in registry.entries() {
+ let replacement = format!("{{{{ {} }}}}", name);
+ result = result.replace(value, &replacement);
+ }
+ result
+}
+
#[cfg(test)]
mod tests {
use super::*;
diff --git a/tests/property.rs b/tests/property.rs
index d16df78..e2299c0 100644
--- a/tests/property.rs
+++ b/tests/property.rs
@@ -3,9 +3,8 @@ use std::collections::HashMap;
use hegel::TestCase;
use hegel::generators::{integers, just, text, vecs};
use hegel::one_of;
-use quire::ci::{SecretRegistry, redact};
use quire::event::{PushEvent, PushRef};
-use quire::secret::SecretString;
+use quire::secret::{SecretRegistry, SecretString, redact};
const ZERO_SHA: &str = "0000000000000000000000000000000000000000";