Make API the primary SecretRegistry path; filesystem seeds the cache
SecretRegistry::new(fetcher) is now the primary constructor — a fetcher
is required, not optional. SecretRegistry::seed(secrets) pre-warms the
cache for the filesystem source so no fetches are needed at run time.
From impls (used by tests and the filesystem path) install a
not_found_fetcher so they continue to work without change.
In quire-ci, the SecretSource enum dissolves: both transport modes
install the API fetcher via SecretRegistry::new; only the filesystem
mode additionally seeds the cache from the bootstrap file.
https://claude.ai/code/session_01VCCZYDCHDbr4LgadiJ6QAi
diff --git a/quire-ci/src/main.rs b/quire-ci/src/main.rs
index 2a0be58..97d45c7 100644
--- a/quire-ci/src/main.rs
+++ b/quire-ci/src/main.rs
@@ -237,12 +237,15 @@ fn main() -> miette::Result<()> {
let miette_layer = MietteLayer::new();
telemetry::init_tracing(miette_layer, FmtMode::Plain)?;
- let source = if transport.mode == TransportMode::Api {
- SecretSource::Api(transport.session.clone())
- } else {
- SecretSource::Bootstrap(secrets)
+ let session = transport.session.clone();
+ let registry = {
+ let base = SecretRegistry::new(move |name| fetch_secret_from_api(&session, name));
+ if transport.mode == TransportMode::Filesystem {
+ base.seed(secrets)
+ } else {
+ base
+ }
};
- let registry = source.into_registry();
run_pipeline(
cli.workspace,
@@ -359,64 +362,41 @@ fn load_bootstrap(
Ok((bootstrap.git_dir, bootstrap.meta, secrets, bootstrap.sentry))
}
-/// How this run resolves secret values.
-///
-/// `Bootstrap` reads the revealed values baked into the bootstrap file
-/// by the orchestrator — the current default. `Api` ignores those values
-/// and fetches each secret on demand from quire-server instead.
+/// Fetch a single secret from quire-server.
///
-/// Once the `Api` path is validated in production, `Bootstrap` will be
-/// removed and the bootstrap file will stop carrying secret values.
-enum SecretSource {
- Bootstrap(HashMap<String, SecretString>),
- Api(ApiSession),
-}
-
-impl SecretSource {
- fn into_registry(self) -> SecretRegistry {
- match self {
- Self::Bootstrap(secrets) => SecretRegistry::from(secrets),
- Self::Api(session) => SecretRegistry::from(HashMap::new())
- .with_fallback(move |name| Self::fetch_from_api(&session, name)),
- }
- }
-
- /// Fetch a single secret from quire-server.
- ///
- /// Uses [`tokio::runtime::Handle::block_on`] to drive the async HTTP
- /// call from synchronous Lua callback context. Requires the caller to
- /// be on a thread that has entered a Tokio runtime (`rt.enter()` in
- /// `main` satisfies this).
- fn fetch_from_api(session: &ApiSession, name: &str) -> quire_core::secret::Result<String> {
- let url = format!(
- "{}/api/runs/{}/secrets/{}",
- session.server_url, session.run_id, name
- );
- let token = session.auth_token.clone();
- let name_owned = name.to_string();
-
- tokio::runtime::Handle::current().block_on(async move {
- let resp = reqwest::Client::new()
- .get(&url)
- .bearer_auth(&token)
- .send()
+/// Uses [`tokio::runtime::Handle::block_on`] to drive the async HTTP
+/// call from synchronous Lua callback context. Requires the caller to
+/// be on a thread that has entered a Tokio runtime (`rt.enter()` in
+/// `main` satisfies this).
+fn fetch_secret_from_api(session: &ApiSession, name: &str) -> quire_core::secret::Result<String> {
+ let url = format!(
+ "{}/api/runs/{}/secrets/{}",
+ session.server_url, session.run_id, name
+ );
+ let token = session.auth_token.clone();
+ let name_owned = name.to_string();
+
+ tokio::runtime::Handle::current().block_on(async move {
+ let resp = reqwest::Client::new()
+ .get(&url)
+ .bearer_auth(&token)
+ .send()
+ .await
+ .map_err(|e| SecretError::Resolve(e.to_string()))?;
+
+ let status = resp.status();
+ if status.is_success() {
+ resp.text()
.await
- .map_err(|e| SecretError::Resolve(e.to_string()))?;
-
- let status = resp.status();
- if status.is_success() {
- resp.text()
- .await
- .map_err(|e| SecretError::Resolve(e.to_string()))
- } else if status == reqwest::StatusCode::NOT_FOUND {
- Err(SecretError::UnknownSecret(name_owned))
- } else {
- Err(SecretError::Resolve(format!(
- "secret API returned {status} for {name_owned:?}"
- )))
- }
- })
- }
+ .map_err(|e| SecretError::Resolve(e.to_string()))
+ } else if status == reqwest::StatusCode::NOT_FOUND {
+ Err(SecretError::UnknownSecret(name_owned))
+ } else {
+ Err(SecretError::Resolve(format!(
+ "secret API returned {status} for {name_owned:?}"
+ )))
+ }
+ })
}
fn run_pipeline(
diff --git a/quire-core/src/ci/runtime.rs b/quire-core/src/ci/runtime.rs
index b75089f..4aa0410 100644
--- a/quire-core/src/ci/runtime.rs
+++ b/quire-core/src/ci/runtime.rs
@@ -131,8 +131,8 @@ impl Runtime {
/// Takes ownership of the pipeline (including its Lua VM). `meta`
/// provides the push data for `:quire/push` source outputs.
/// `registry` supplies secrets for `(secret :name)` calls; build
- /// it with [`SecretRegistry::with_fallback`] to enable API-backed
- /// fetching.
+ /// it via [`SecretRegistry::new`] with an API fetcher, optionally
+ /// pre-seeded via [`SecretRegistry::seed`] for the filesystem source.
///
/// Panics if any of the Lua table operations below fail. They run
/// against a freshly initialized VM with `String`/`&str` keys and
diff --git a/quire-core/src/secret.rs b/quire-core/src/secret.rs
index 6a60687..89a456e 100644
--- a/quire-core/src/secret.rs
+++ b/quire-core/src/secret.rs
@@ -161,46 +161,46 @@ impl Revealed {
/// revealed value or an error.
type SecretFetcher = Box<dyn Fn(&str) -> Result<String>>;
-/// Per-run secret store: holds declared secrets and their revealed
-/// values for both lookup and redaction.
+/// Per-run secret store. Resolves secret names to values via a fetcher
+/// closure, caching each result so the fetcher is called at most once
+/// per name. Revealed values are registered for 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`].
+/// The normal construction path installs an API fetcher and starts with
+/// an empty cache. The filesystem source pre-warms the cache via
+/// [`SecretRegistry::seed`] so no fetches are needed at run time.
///
/// Lifetime is bounded to a single CI run. Do not carry a registry
-/// across runs — values from previous runs would contaminate
+/// across runs — revealed 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.
+ /// Pull-through cache: name → secret. Pre-seeded for the filesystem
+ /// source; populated lazily for the API source.
+ cache: HashMap<String, SecretString>,
+ /// name → revealed value (opaque). Populated on first `(secret :name)` call.
revealed: HashMap<String, Revealed>,
- /// Optional fallback invoked when a name is not in `declared`.
- /// Intended for API transport: quire-ci installs a closure that
- /// fetches the value from the server. Names resolved via the
- /// fallback are registered for redaction just like declared ones.
- fallback: Option<SecretFetcher>,
+ /// Called when a name is absent from the cache. Always present — the
+ /// normal case is an API fetcher; tests and the filesystem source use
+ /// a closure that returns [`Error::UnknownSecret`].
+ fetcher: SecretFetcher,
}
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("cache", &self.cache.keys().collect::<Vec<_>>())
.field("revealed", &self.revealed.keys().collect::<Vec<_>>())
- .field("fallback", &self.fallback.as_ref().map(|_| "<fn>"))
+ .field("fetcher", &"<fn>")
.finish()
}
}
+fn not_found_fetcher(name: &str) -> Result<String> {
+ Err(Error::UnknownSecret(name.to_string()))
+}
+
impl From<HashMap<String, SecretString>> for SecretRegistry {
- fn from(declared: HashMap<String, SecretString>) -> Self {
- Self {
- declared,
- revealed: HashMap::new(),
- fallback: None,
- }
+ fn from(secrets: HashMap<String, SecretString>) -> Self {
+ Self::new(not_found_fetcher).seed(secrets)
}
}
@@ -212,35 +212,45 @@ impl From<Vec<(String, SecretString)>> for SecretRegistry {
impl From<Vec<(&str, &str)>> for SecretRegistry {
fn from(pairs: Vec<(&str, &str)>) -> Self {
- let declared: HashMap<String, SecretString> = pairs
+ let cache: HashMap<String, SecretString> = pairs
.into_iter()
.map(|(k, v)| (k.to_string(), SecretString::from(v)))
.collect();
- Self::from(declared)
+ Self::from(cache)
}
}
impl SecretRegistry {
- /// Install a fallback resolver called when a secret name is not
- /// found in the declared map. Intended for API transport: quire-ci
- /// installs a closure that fetches the value from quire-server.
+ /// Create a registry backed by `fetcher`. The cache starts empty;
+ /// use [`SecretRegistry::seed`] to pre-warm it.
///
- /// Values fetched via the fallback are cached back into `declared`,
- /// so the fallback is called at most once per name. The registry
- /// therefore acts as a pull-through cache: pre-populate it for the
- /// filesystem source, leave it empty for the API source.
- pub fn with_fallback<F>(mut self, fallback: F) -> Self
+ /// `fetcher` is called at most once per name — results are cached
+ /// back into the registry so subsequent lookups are local. Values
+ /// fetched through either path are registered for redaction.
+ pub fn new<F>(fetcher: F) -> Self
where
F: Fn(&str) -> Result<String> + 'static,
{
- self.fallback = Some(Box::new(fallback));
+ Self {
+ cache: HashMap::new(),
+ revealed: HashMap::new(),
+ fetcher: Box::new(fetcher),
+ }
+ }
+
+ /// Pre-warm the cache with an existing set of secrets. Intended for
+ /// the filesystem source, which receives all secrets up-front in the
+ /// bootstrap file. Pre-seeded names are served from the cache without
+ /// invoking the fetcher.
+ pub fn seed(mut self, secrets: HashMap<String, SecretString>) -> Self {
+ self.cache = secrets;
self
}
/// Resolve a secret by name, caching the revealed value for
- /// redaction. Checks `declared` first; if the name is absent and a
- /// fallback is installed, calls it. Returns `Err` if the name is
- /// unknown through both paths or the source can't be read.
+ /// redaction. Checks the cache first; on a miss, calls the fetcher
+ /// and stores the result. Returns `Err` if the name is unknown 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
@@ -255,15 +265,11 @@ impl SecretRegistry {
/// through [`redact`] (e.g. `sh` command args, ShOutput) or wrap
/// it in a type whose `Debug`/`Display` impl redacts.
pub fn resolve(&mut self, name: &str) -> Result<String> {
- let value = if let Some(secret) = self.declared.get(name) {
+ let value = if let Some(secret) = self.cache.get(name) {
secret.reveal()?.to_string()
} else {
- let fetched = if let Some(ref fallback) = self.fallback {
- fallback(name)?
- } else {
- return Err(Error::UnknownSecret(name.to_string()));
- };
- self.declared
+ let fetched = (self.fetcher)(name)?;
+ self.cache
.insert(name.to_string(), SecretString::from(fetched.clone()));
fetched
};
@@ -343,7 +349,7 @@ mod tests {
);
assert!(
debug_output.contains("github_token"),
- "SecretRegistry Debug should still surface declared names: {debug_output}"
+ "SecretRegistry Debug should still surface cached names: {debug_output}"
);
}
@@ -475,7 +481,7 @@ mod tests {
let call_count = Arc::new(AtomicUsize::new(0));
let counter = call_count.clone();
- let mut registry = SecretRegistry::from(HashMap::new()).with_fallback(move |name| {
+ let mut registry = SecretRegistry::new(move |name| {
counter.fetch_add(1, Ordering::SeqCst);
Ok(format!("fetched_{name}_abcdefgh"))
});