diff --git a/crates/goose-cli/src/commands/update.rs b/crates/goose-cli/src/commands/update.rs index 04272d205..7c600a92c 100644 --- a/crates/goose-cli/src/commands/update.rs +++ b/crates/goose-cli/src/commands/update.rs @@ -1,4 +1,8 @@ use anyhow::{bail, Context, Result}; +use reqwest::{ + header::{HeaderValue, AUTHORIZATION}, + StatusCode, +}; use sha2::{Digest, Sha256}; use sigstore_verify::trust_root::{TrustedRoot, SIGSTORE_PRODUCTION_TRUSTED_ROOT}; use sigstore_verify::types::{Bundle, Sha256Hash}; @@ -79,6 +83,32 @@ struct AttestationEntry { const GITHUB_ACTIONS_ISSUER: &str = "https://token.actions.githubusercontent.com"; +fn sanitized_token(token: Option<&str>) -> Option<&str> { + token.map(str::trim).filter(|tok| !tok.is_empty()) +} + +fn authorization_header_value(token: &str) -> Option { + HeaderValue::from_str(&format!("Bearer {token}")).ok() +} + +fn github_token() -> Option { + env::var("GITHUB_TOKEN") + .ok() + .and_then(|tok| sanitized_token(Some(&tok)).map(str::to_owned)) + .or_else(|| { + env::var("GH_TOKEN") + .ok() + .and_then(|tok| sanitized_token(Some(&tok)).map(str::to_owned)) + }) +} + +fn should_retry_attestations_without_token(status: StatusCode, token: Option<&str>) -> bool { + sanitized_token(token) + .and_then(authorization_header_value) + .is_some() + && matches!(status, StatusCode::UNAUTHORIZED | StatusCode::FORBIDDEN) +} + async fn fetch_attestations(digest: &str, token: Option<&str>) -> Result> { let url = format!( "https://api.github.com/repos/aaif-goose/goose/attestations/sha256:{digest}\ @@ -86,17 +116,14 @@ async fn fetch_attestations(digest: &str, token: Option<&str>) -> Result) -> Result, +) -> Result { + let mut req = client + .get(url) + .header("Accept", "application/vnd.github+json") + .header("X-GitHub-Api-Version", "2022-11-28") + .header("User-Agent", "goose-cli"); + + if let Some(value) = token.and_then(authorization_header_value) { + req = req.header(AUTHORIZATION, value); + } + + req.send().await.context("Failed to fetch attestations") +} + // Verify a single attestation bundle against the artifact digest and workflow. fn verify_bundle( bundle_json: &serde_json::Value, @@ -142,8 +187,8 @@ fn verify_bundle( Ok(()) } -/// Returns `Ok(true)` verified, `Ok(false)` skipped (soft warning), `Err` hard failure. -async fn verify_provenance(archive_data: &[u8], tag: &str) -> Result { +/// Returns `Ok(())` when the downloaded archive has verified provenance. +async fn verify_provenance(archive_data: &[u8], tag: &str) -> Result<()> { let digest = sha256_hex(archive_data); println!("Archive SHA-256: {digest}"); @@ -152,30 +197,19 @@ async fn verify_provenance(archive_data: &[u8], tag: &str) -> Result { _ => "release.yml", }; - let token = env::var("GITHUB_TOKEN") - .ok() - .or_else(|| env::var("GH_TOKEN").ok()); + let token = github_token(); println!("Verifying SLSA provenance via Sigstore..."); - let bundles = match fetch_attestations(&digest, token.as_deref()).await { - Ok(b) if b.is_empty() => { - eprintln!( - "Warning: No Sigstore attestation found for this build. \ - This may be expected for canary or nightly builds." - ); - return Ok(false); - } - Ok(b) => b, - Err(e) => { - eprintln!( - "Warning: Sigstore provenance check could not complete: {e}\n\ - This may be expected for releases published before provenance \ - attestations were enabled." - ); - return Ok(false); - } - }; + let bundles = fetch_attestations(&digest, token.as_deref()) + .await + .context( + "Sigstore provenance check could not complete; refusing to install unverifiable update", + )?; + + if bundles.is_empty() { + bail!("No Sigstore attestation found for downloaded archive; refusing to install unverifiable update"); + } let trusted_root = TrustedRoot::from_json(SIGSTORE_PRODUCTION_TRUSTED_ROOT) .context("Failed to load Sigstore trusted root")?; @@ -195,7 +229,7 @@ async fn verify_provenance(archive_data: &[u8], tag: &str) -> Result { ) { Ok(()) => { println!("Sigstore provenance verification passed."); - return Ok(true); + return Ok(()); } Err(e) => last_err = Some(e), } @@ -247,7 +281,7 @@ pub async fn update(canary: bool, reconfigure: bool) -> Result<()> { println!("Downloaded {} bytes.", bytes.len()); // --- Verify SLSA provenance via Sigstore -------------------------------- - let provenance_verified = verify_provenance(&bytes, tag).await?; + verify_provenance(&bytes, tag).await?; // --- Extract to temp dir (hardened against path traversal) -------------- let tmp_dir = tempfile::tempdir().context("Failed to create temp directory")?; @@ -274,11 +308,7 @@ pub async fn update(canary: bool, reconfigure: bool) -> Result<()> { #[cfg(target_os = "windows")] copy_dlls(&extracted_binary, ¤t_exe)?; - if provenance_verified { - println!("goose updated successfully (verified with Sigstore SLSA provenance)."); - } else { - println!("goose updated successfully."); - } + println!("goose updated successfully (verified with Sigstore SLSA provenance)."); // --- Reconfigure if requested ------------------------------------------- if reconfigure { @@ -737,6 +767,48 @@ mod tests { ); } + #[test] + fn test_sanitized_token_trims_blank_values() { + assert_eq!(sanitized_token(None), None); + assert_eq!(sanitized_token(Some("")), None); + assert_eq!(sanitized_token(Some(" ")), None); + assert_eq!(sanitized_token(Some(" token\n")), Some("token")); + } + + #[test] + fn test_authorization_header_value_rejects_malformed_tokens() { + assert!(authorization_header_value("token").is_some()); + assert!(authorization_header_value("bad\ntoken").is_none()); + } + + #[test] + fn test_attestation_lookup_retries_auth_failures_without_token() { + assert!(should_retry_attestations_without_token( + StatusCode::UNAUTHORIZED, + Some("token") + )); + assert!(should_retry_attestations_without_token( + StatusCode::FORBIDDEN, + Some("token") + )); + assert!(!should_retry_attestations_without_token( + StatusCode::INTERNAL_SERVER_ERROR, + Some("token") + )); + assert!(!should_retry_attestations_without_token( + StatusCode::UNAUTHORIZED, + Some("") + )); + assert!(!should_retry_attestations_without_token( + StatusCode::UNAUTHORIZED, + Some("bad\ntoken") + )); + assert!(!should_retry_attestations_without_token( + StatusCode::UNAUTHORIZED, + None + )); + } + // ----------------------------------------------------------------------- // Path validation and extraction hardening tests // ----------------------------------------------------------------------- @@ -808,13 +880,11 @@ mod tests { // ----------------------------------------------------------------------- #[tokio::test] - async fn test_verify_provenance_warns_on_missing_attestation() { + async fn test_verify_provenance_fails_closed_when_unverifiable() { let result = verify_provenance(b"not a real archive", "stable").await; - // Network failures and missing attestations are soft warnings: Ok(false), not hard errors. - assert_eq!( - result.ok(), - Some(false), - "verify_provenance should return Ok(false) when attestations cannot be fetched" + assert!( + result.is_err(), + "verify_provenance must fail closed when provenance cannot be verified" ); }