diff --git a/Cargo.lock b/Cargo.lock index 5287c2fc897..6ba1128220e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6754,6 +6754,7 @@ dependencies = [ "async-trait", "base64 0.23.1", "hex", + "log", "regex", "reqwest", "rustls-pki-types", diff --git a/crates/mise-sigstore/Cargo.toml b/crates/mise-sigstore/Cargo.toml index 4b7dc39ffdd..866e81f9147 100644 --- a/crates/mise-sigstore/Cargo.toml +++ b/crates/mise-sigstore/Cargo.toml @@ -15,6 +15,7 @@ path = "src/lib.rs" async-trait = "0.1" base64 = "0.23" hex = "0.4" +log = "0.4" regex = "1" reqwest = { version = "0.13", default-features = false, features = ["json"] } serde = { version = "1", features = ["derive"] } diff --git a/crates/mise-sigstore/src/github.rs b/crates/mise-sigstore/src/github.rs index 91ff518fb42..e17e8725895 100644 --- a/crates/mise-sigstore/src/github.rs +++ b/crates/mise-sigstore/src/github.rs @@ -92,6 +92,19 @@ pub async fn verify_github_attestation_sources( } let artifact = tokio::fs::read(artifact_path).await?; + verify_github_attestation_sources_for_artifact( + Artifact::from(artifact.as_slice()), + attestations, + signer_workflow, + ) + .await +} + +pub(crate) async fn verify_github_attestation_sources_for_artifact( + artifact: Artifact<'_>, + attestations: &[Attestation], + signer_workflow: Option<&str>, +) -> Result> { let mut trust_roots = TrustRoots::default(); let mut sources = Vec::new(); let mut errors = Vec::new(); @@ -102,12 +115,15 @@ pub async fn verify_github_attestation_sources( let bundle = match serde_json::from_value::(bundle_value.clone()) { Ok(bundle) => bundle, Err(e) => { - errors.push(e.to_string()); + let error_message = e.to_string(); + let error = AttestationError::Json(e); + log_attestation_method_failure(&error); + errors.push(error_message); continue; } }; match verify_bundle_with_trust_roots( - Artifact::from(artifact.as_slice()), + artifact.clone(), &bundle, signer_workflow, &mut trust_roots, @@ -116,21 +132,46 @@ pub async fn verify_github_attestation_sources( { Ok(()) => match bundle_source_repository(&bundle) { Some(repository) => sources.push(repository), - None => errors.push("verified attestation names no source repository".to_string()), + None => { + const ERROR: &str = "verified attestation names no source repository"; + log::debug!( + "cached GitHub attestation verification method failed; continuing with other attestations: {ERROR}" + ); + errors.push(ERROR.to_string()); + } }, - Err(e) => errors.push(e.to_string()), + Err(error) => { + log_attestation_method_failure(&error); + errors.push(error.to_string()); + } } } if sources.is_empty() { + log::debug!( + "cached GitHub attestation verification failed overall: no attestation verified" + ); return Err(AttestationError::Verification(join_error_strings( errors, || "No valid attestations found".to_string(), ))); } + if !errors.is_empty() { + log::debug!( + "cached GitHub attestation verification succeeded overall despite {} failed attestation method(s)", + errors.len() + ); + } Ok(sources) } +pub(crate) fn log_attestation_method_failure(error: &AttestationError) { + log::debug!( + "cached GitHub attestation verification method failed; continuing with other attestations: {}", + error.diagnostic_summary() + ); +} + pub(crate) async fn verify_github_attestation_inner( request: GithubAttestationRequest<'_>, ) -> Result { diff --git a/crates/mise-sigstore/src/model.rs b/crates/mise-sigstore/src/model.rs index 3282f7d2fe3..d0a9c9dd0d2 100644 --- a/crates/mise-sigstore/src/model.rs +++ b/crates/mise-sigstore/src/model.rs @@ -42,6 +42,30 @@ impl From for AttestationError { } } +impl AttestationError { + /// Return a safe, bounded summary for diagnostic logs. + /// + /// Verification errors can be built from configured mirror responses, so + /// logging their free-form contents risks leaking credentials or query + /// tokens. Preserve only a useful category; keep the original error intact + /// for callers that need to return it. + pub fn diagnostic_summary(&self) -> &'static str { + match self { + Self::Api(_) => "GitHub attestation API request failed", + Self::Verification(_) | Self::Sigstore(_) => { + "attestation signature verification failed" + } + Self::WorkflowMismatch(_) => "signer workflow mismatch", + Self::SubjectMismatch(_) => "artifact subject mismatch", + Self::UnsupportedFormat(_) => "unsupported attestation format", + Self::NoAttestations => "no attestations found", + Self::Io(_) => "artifact I/O failed", + Self::Http(_) => "network request failed", + Self::Json(_) => "invalid attestation JSON", + } + } +} + pub type Result = std::result::Result; #[derive(Debug, Clone)] diff --git a/crates/mise-sigstore/src/tests/github.rs b/crates/mise-sigstore/src/tests/github.rs new file mode 100644 index 00000000000..b0e3d8f96b8 --- /dev/null +++ b/crates/mise-sigstore/src/tests/github.rs @@ -0,0 +1,163 @@ +use super::*; +use std::sync::{ + Mutex, Once, + atomic::{AtomicU64, Ordering}, +}; + +#[derive(Clone, Debug)] +struct CapturedLog { + message: String, + sequence: u64, + thread_id: std::thread::ThreadId, +} + +struct LogCapture { + first_sequence: u64, + thread_id: std::thread::ThreadId, +} + +struct CapturingLogger { + next_sequence: AtomicU64, + records: Mutex>, +} + +static CAPTURING_LOGGER: CapturingLogger = CapturingLogger { + next_sequence: AtomicU64::new(0), + records: Mutex::new(Vec::new()), +}; +static LOGGER_INIT: Once = Once::new(); + +impl log::Log for CapturingLogger { + fn enabled(&self, metadata: &log::Metadata<'_>) -> bool { + metadata.level() <= log::Level::Debug + } + + fn log(&self, record: &log::Record<'_>) { + if self.enabled(record.metadata()) && record.target() == "mise_sigstore::github" { + self.records.lock().unwrap().push(CapturedLog { + message: record.args().to_string(), + sequence: self.next_sequence.fetch_add(1, Ordering::Relaxed), + thread_id: std::thread::current().id(), + }); + } + } + + fn flush(&self) {} +} + +fn capture_logs() -> LogCapture { + LOGGER_INIT.call_once(|| { + log::set_logger(&CAPTURING_LOGGER).unwrap(); + log::set_max_level(log::LevelFilter::Debug); + }); + LogCapture { + first_sequence: CAPTURING_LOGGER.next_sequence.load(Ordering::Relaxed), + thread_id: std::thread::current().id(), + } +} + +fn captured_messages(capture: &LogCapture) -> Vec { + CAPTURING_LOGGER + .records + .lock() + .unwrap() + .iter() + .filter(|record| { + record.sequence >= capture.first_sequence && record.thread_id == capture.thread_id + }) + .map(|record| record.message.clone()) + .collect() +} + +fn fixture_attestation(json: &str) -> Attestation { + serde_json::from_value(serde_json::json!({ + "bundle": serde_json::from_str::(json).unwrap(), + })) + .unwrap() +} + +fn invalid_attestation() -> Attestation { + serde_json::from_value(serde_json::json!({ + "bundle": {"not": "a sigstore bundle"}, + })) + .unwrap() +} + +fn fixture_digest() -> Sha256Hash { + Sha256Hash::from_hex("b4058dece685259910d3aba5782445996eea79dbdb3cf952a6eb81aadf0373ff") + .unwrap() +} + +#[tokio::test] +async fn github_attestation_sources_succeeds_when_one_method_fails() { + let capture = capture_logs(); + let digest = fixture_digest(); + let sources = crate::github::verify_github_attestation_sources_for_artifact( + Artifact::from(&digest), + &[ + invalid_attestation(), + fixture_attestation(include_str!( + "../../tests/fixtures/github_build_provenance_jdx_mise.json" + )), + ], + None, + ) + .await + .unwrap(); + + assert_eq!(sources, ["jdx/mise"]); + let messages = captured_messages(&capture); + assert!(messages.iter().any(|message| message.contains( + "cached GitHub attestation verification method failed; continuing with other attestations: invalid attestation JSON" + ))); + assert!(messages.iter().any(|message| message.contains( + "cached GitHub attestation verification succeeded overall despite 1 failed attestation method(s)" + ))); +} + +#[tokio::test] +async fn github_attestation_sources_fails_when_all_methods_fail() { + let capture = capture_logs(); + let digest = fixture_digest(); + let error = crate::github::verify_github_attestation_sources_for_artifact( + Artifact::from(&digest), + &[invalid_attestation()], + None, + ) + .await + .unwrap_err(); + + assert!(matches!(error, AttestationError::Verification(_))); + let messages = captured_messages(&capture); + assert!(messages.iter().any(|message| message.contains( + "cached GitHub attestation verification method failed; continuing with other attestations: invalid attestation JSON" + ))); + assert!(messages.iter().any(|message| message + == "cached GitHub attestation verification failed overall: no attestation verified")); +} + +#[test] +fn attestation_diagnostic_logs_do_not_leak_secrets() { + let capture = capture_logs(); + let error = AttestationError::Verification( + "TUF fetch failed at https://user:password@mirror.example/root.json?token=query-token&mode=refresh; Authorization: Basic encoded-credentials; GITHUB_TOKEN=bare-token; API_KEY=\"quoted token with spaces\"; nested_api_key='another quoted token'; Bearer bearer-token".to_string(), + ); + crate::github::log_attestation_method_failure(&error); + let messages = captured_messages(&capture); + assert_eq!(messages.len(), 1); + let message = &messages[0]; + + for secret in [ + "user", + "password", + "query-token", + "encoded-credentials", + "bare-token", + "quoted token with spaces", + "another quoted token", + "bearer-token", + ] { + assert!(!message.contains(secret), "leaked {secret}: {message}"); + } + assert!(message.contains("attestation signature verification failed")); +} diff --git a/crates/mise-sigstore/src/tests/mod.rs b/crates/mise-sigstore/src/tests/mod.rs index ebf5e42c482..dbcf1d803db 100644 --- a/crates/mise-sigstore/src/tests/mod.rs +++ b/crates/mise-sigstore/src/tests/mod.rs @@ -12,6 +12,7 @@ const GENUINE_INTOTO_ENVELOPE: &str = include_str!("../../tests/fixtures/sops_v3 mod certificate; mod cosign; +mod github; mod retry; mod slsa; mod trust; diff --git a/crates/mise-util/src/github/sigstore.rs b/crates/mise-util/src/github/sigstore.rs index 118d0023683..06f1ea40e93 100644 --- a/crates/mise-util/src/github/sigstore.rs +++ b/crates/mise-util/src/github/sigstore.rs @@ -249,7 +249,8 @@ async fn verify_attestation_uncached( "mise-versions GitHub attestations did not verify for {owner}/{repo}; falling back to GitHub API" ), CachedAttestationVerification::Retry(Some(err)) => debug!( - "mise-versions GitHub attestations did not verify for {owner}/{repo}; falling back to GitHub API: {err}" + "mise-versions GitHub attestation verification failed overall for {owner}/{repo}; falling back to GitHub API: {}", + err.diagnostic_summary() ), } }