From 7a85e210cd07b07d5bb1d8b6fd2ea68f73082e97 Mon Sep 17 00:00:00 2001 From: KayanoLiam Date: Fri, 9 Oct 2026 21:20:24 +0900 Subject: [PATCH 1/3] Fix GCS external account credentials without an explicit format --- src/cache/gcs.rs | 217 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 216 insertions(+), 1 deletion(-) diff --git a/src/cache/gcs.rs b/src/cache/gcs.rs index 0c83d3d11b..2956ae9c45 100644 --- a/src/cache/gcs.rs +++ b/src/cache/gcs.rs @@ -15,6 +15,8 @@ use crate::cache::CacheMode; use crate::errors::*; +use base64::Engine; +use base64::prelude::BASE64_STANDARD; use opendal::Operator; use opendal::{OperationContext, services::Gcs}; use opendal_layer_logging::LoggingLayer; @@ -54,7 +56,10 @@ impl GCSCache { } if let Some(path) = cred_path { - builder = builder.credential_path(path); + builder = match credential_with_default_format(path) { + Some(credential) => builder.credential(&credential), + None => builder.credential_path(path), + }; } if let Some(cred_url) = credential_url { @@ -78,6 +83,28 @@ impl GCSCache { } } +// AIP-4117 defaults an omitted credential_source.format to plain text, but +// reqsign-google currently requires it for file and URL sources. Normalize only +// those credentials in memory, leaving other formats and validation to OpenDAL. +// Remove this workaround once OpenDAL includes the released fix for +// https://github.com/apache/reqsign/pull/910 +fn credential_with_default_format(path: &str) -> Option { + let content = std::fs::read(path).ok()?; + let mut credential: serde_json::Value = serde_json::from_slice(&content).ok()?; + if credential.get("type")?.as_str()? != "external_account" { + return None; + } + let source = credential.get_mut("credential_source")?.as_object_mut()?; + if source.contains_key("format") + || source.contains_key("environment_id") + || !(source.contains_key("file") || source.contains_key("url")) + { + return None; + } + source.insert("format".to_owned(), serde_json::json!({"type": "text"})); + Some(BASE64_STANDARD.encode(serde_json::to_vec(&credential).ok()?)) +} + /// Fetch token from TaskCluster for GCS authentication /// /// This feature is required to run [mozilla's CI](https://searchfox.org/mozilla-central/source/build/mozconfig.cache#67-84): @@ -113,3 +140,191 @@ struct TaskClusterToken { access_token: String, expire_time: String, } + +#[cfg(test)] +mod tests { + use super::*; + use opendal::{Buffer, HttpBody, HttpTransport, HttpTransporter}; + use serde_json::json; + use std::collections::HashMap; + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + + struct FederationTransport { + subject_token: String, + scope: &'static str, + exchanges: Arc, + } + + impl HttpTransport for FederationTransport { + async fn fetch( + &self, + req: http::Request, + ) -> opendal::Result> { + let (status, body) = match req.uri().to_string().as_str() { + "https://issuer.example/token" => { + assert_eq!(req.method(), http::Method::GET); + assert_eq!(req.headers()["x-test-identity"], "test-identity"); + (200, self.subject_token.clone()) + } + "https://sts.googleapis.com/v1/token" => { + assert_eq!(req.method(), http::Method::POST); + let body = req.body().to_bytes(); + let form: HashMap<_, _> = url::form_urlencoded::parse(&body).collect(); + assert_eq!(form["subject_token"], "test-subject-token"); + assert_eq!(form["scope"], self.scope); + assert_eq!( + form["grant_type"], + "urn:ietf:params:oauth:grant-type:token-exchange" + ); + self.exchanges.fetch_add(1, Ordering::SeqCst); + ( + 200, + json!({ + "access_token": "test-access-token", + "token_type": "Bearer", + "expires_in": 3600 + }) + .to_string(), + ) + } + url if url.starts_with("https://storage.googleapis.com/") => { + assert_eq!(req.headers()["authorization"], "Bearer test-access-token"); + (404, String::new()) + } + url => panic!("unexpected request, including credential fallback: {url}"), + }; + let size = body.len() as u64; + let stream = futures::stream::iter([Ok(Buffer::from(body))]); + Ok(http::Response::builder() + .status(status) + .body(HttpBody::new(stream, Some(size))) + .unwrap()) + } + } + + async fn check_external_account(source_type: &str, format: Option) { + for mode in [CacheMode::ReadOnly, CacheMode::ReadWrite] { + let dir = tempfile::tempdir().unwrap(); + let token_path = dir.path().join("subject-token"); + let subject_token = if format.as_ref().and_then(|f| f["type"].as_str()) == Some("json") + { + json!({"id_token": "test-subject-token"}).to_string() + } else { + "test-subject-token\n".to_owned() + }; + std::fs::write(&token_path, &subject_token).unwrap(); + let mut source = match source_type { + "file" => json!({"file": token_path}), + "url" => json!({ + "url": "https://issuer.example/token", + "headers": {"x-test-identity": "test-identity"} + }), + _ => unreachable!(), + }; + if let Some(format) = &format { + source["format"] = format.clone(); + } + let credential = json!({ + "type": "external_account", + "audience": "//iam.googleapis.com/projects/123/locations/global/workloadIdentityPools/test/providers/test", + "subject_token_type": "urn:ietf:params:oauth:token-type:jwt", + "token_url": "https://sts.googleapis.com/v1/token", + "credential_source": source + }) + .to_string(); + let credential_path = dir.path().join("external-account.json"); + std::fs::write(&credential_path, &credential).unwrap(); + let exchanges = Arc::new(AtomicUsize::new(0)); + let op = GCSCache::build( + "test-bucket", + "", + credential_path.to_str(), + None, + mode, + None, + ) + .unwrap() + .with_context( + OperationContext::new().with_http_transport(HttpTransporter::new( + FederationTransport { + subject_token, + scope: rw_to_scope(mode), + exchanges: exchanges.clone(), + }, + )), + ); + + for _ in 0..2 { + let err = op.read(".sccache_check").await.unwrap_err(); + assert_eq!(err.kind(), opendal::ErrorKind::NotFound, "{err:?}"); + } + // Repeated requests reuse the exchanged access token. + assert_eq!(exchanges.load(Ordering::SeqCst), 1); + assert_eq!( + std::fs::read_to_string(credential_path).unwrap(), + credential + ); + } + } + + #[test] + fn test_credential_with_default_format_leaves_other_credentials_unchanged() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("credential.json"); + assert!(credential_with_default_format(path.to_str().unwrap()).is_none()); + for credential in [ + "not json".to_owned(), + json!({"type": "service_account", "private_key": "unchanged"}).to_string(), + json!({"type": "authorized_user", "refresh_token": "unchanged"}).to_string(), + json!({"type": "external_account"}).to_string(), + ] { + std::fs::write(&path, credential).unwrap(); + assert!(credential_with_default_format(path.to_str().unwrap()).is_none()); + } + for source in [ + json!({"file": "token", "format": {"type": "text"}}), + json!({"url": "https://issuer.example/token", "format": { + "type": "json", "subject_token_field_name": "id_token" + }}), + json!({"file": "token", "format": null}), + json!({"file": "token", "format": {}}), + json!({"file": "token", "format": {"type": "invalid"}}), + json!({"environment_id": "aws1", "url": "http://169.254.169.254"}), + json!({"executable": {"command": "get-token"}}), + json!({}), + ] { + let credential = json!({ + "type": "external_account", + "credential_source": source + }); + std::fs::write(&path, credential.to_string()).unwrap(); + assert!( + credential_with_default_format(path.to_str().unwrap()).is_none(), + "{credential}" + ); + } + } + + #[tokio::test] + async fn test_external_account_file_without_format() { + check_external_account("file", None).await; + } + + #[tokio::test] + async fn test_external_account_url_without_format() { + check_external_account("url", None).await; + } + + #[tokio::test] + async fn test_external_account_explicit_formats() { + for source_type in ["file", "url"] { + for format in [ + json!({"type": "text"}), + json!({"type": "json", "subject_token_field_name": "id_token"}), + ] { + check_external_account(source_type, Some(format)).await; + } + } + } +} From 3b66a4eee46da06b7a6a7ce5f4de469dad5d2bbe Mon Sep 17 00:00:00 2001 From: KayanoLiam Date: Sat, 10 Oct 2026 17:09:07 +0800 Subject: [PATCH 2/3] gcs: normalize credentials selected through the environment --- src/cache/gcs.rs | 66 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 21 deletions(-) diff --git a/src/cache/gcs.rs b/src/cache/gcs.rs index 2956ae9c45..f0f244d618 100644 --- a/src/cache/gcs.rs +++ b/src/cache/gcs.rs @@ -55,7 +55,8 @@ impl GCSCache { builder = builder.service_account(service_account); } - if let Some(path) = cred_path { + let env_cred_path = std::env::var("GOOGLE_APPLICATION_CREDENTIALS").ok(); + if let Some(path) = cred_path.or(env_cred_path.as_deref()) { builder = match credential_with_default_format(path) { Some(credential) => builder.credential(&credential), None => builder.credential_path(path), @@ -83,10 +84,8 @@ impl GCSCache { } } -// AIP-4117 defaults an omitted credential_source.format to plain text, but -// reqsign-google currently requires it for file and URL sources. Normalize only -// those credentials in memory, leaving other formats and validation to OpenDAL. -// Remove this workaround once OpenDAL includes the released fix for +// Default omitted file/URL formats to text per AIP-4117 until OpenDAL ships +// the reqsign-google fix: // https://github.com/apache/reqsign/pull/910 fn credential_with_default_format(path: &str) -> Option { let content = std::fs::read(path).ok()?; @@ -203,7 +202,11 @@ mod tests { } } - async fn check_external_account(source_type: &str, format: Option) { + async fn check_external_account( + source_type: &str, + format: Option, + credentials_from_env: bool, + ) { for mode in [CacheMode::ReadOnly, CacheMode::ReadWrite] { let dir = tempfile::tempdir().unwrap(); let token_path = dir.path().join("subject-token"); @@ -236,14 +239,26 @@ mod tests { let credential_path = dir.path().join("external-account.json"); std::fs::write(&credential_path, &credential).unwrap(); let exchanges = Arc::new(AtomicUsize::new(0)); - let op = GCSCache::build( - "test-bucket", - "", - credential_path.to_str(), - None, - mode, - None, - ) + // An explicit credential path must take precedence over the environment. + let env_path = if credentials_from_env { + credential_path.clone() + } else { + dir.path().join("unused-credentials.json") + }; + let op = temp_env::with_var("GOOGLE_APPLICATION_CREDENTIALS", Some(env_path), || { + GCSCache::build( + "test-bucket", + "", + if credentials_from_env { + None + } else { + credential_path.to_str() + }, + None, + mode, + None, + ) + }) .unwrap() .with_context( OperationContext::new().with_http_transport(HttpTransporter::new( @@ -261,10 +276,6 @@ mod tests { } // Repeated requests reuse the exchanged access token. assert_eq!(exchanges.load(Ordering::SeqCst), 1); - assert_eq!( - std::fs::read_to_string(credential_path).unwrap(), - credential - ); } } @@ -308,12 +319,22 @@ mod tests { #[tokio::test] async fn test_external_account_file_without_format() { - check_external_account("file", None).await; + check_external_account("file", None, false).await; } #[tokio::test] async fn test_external_account_url_without_format() { - check_external_account("url", None).await; + check_external_account("url", None, false).await; + } + + #[tokio::test] + async fn test_external_account_file_without_format_from_env() { + check_external_account("file", None, true).await; + } + + #[tokio::test] + async fn test_external_account_url_without_format_from_env() { + check_external_account("url", None, true).await; } #[tokio::test] @@ -323,7 +344,10 @@ mod tests { json!({"type": "text"}), json!({"type": "json", "subject_token_field_name": "id_token"}), ] { - check_external_account(source_type, Some(format)).await; + for credentials_from_env in [false, true] { + check_external_account(source_type, Some(format.clone()), credentials_from_env) + .await; + } } } } From f4540adc508b18415a7a546f7802ce29068e5508 Mon Sep 17 00:00:00 2001 From: KayanoLiam Date: Sun, 11 Oct 2026 00:10:54 +0900 Subject: [PATCH 3/3] gcs: ignore empty credential environment paths --- src/cache/gcs.rs | 68 ++++++++++++++++++++++++------------------------ 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/src/cache/gcs.rs b/src/cache/gcs.rs index f0f244d618..2c1d2f6db4 100644 --- a/src/cache/gcs.rs +++ b/src/cache/gcs.rs @@ -55,7 +55,9 @@ impl GCSCache { builder = builder.service_account(service_account); } - let env_cred_path = std::env::var("GOOGLE_APPLICATION_CREDENTIALS").ok(); + let env_cred_path = std::env::var("GOOGLE_APPLICATION_CREDENTIALS") + .ok() + .filter(|s| !s.is_empty()); if let Some(path) = cred_path.or(env_cred_path.as_deref()) { builder = match credential_with_default_format(path) { Some(credential) => builder.credential(&credential), @@ -238,44 +240,42 @@ mod tests { .to_string(); let credential_path = dir.path().join("external-account.json"); std::fs::write(&credential_path, &credential).unwrap(); - let exchanges = Arc::new(AtomicUsize::new(0)); // An explicit credential path must take precedence over the environment. - let env_path = if credentials_from_env { - credential_path.clone() + let (env_paths, key_path) = if credentials_from_env { + (vec![Some(credential_path.clone())], None) } else { - dir.path().join("unused-credentials.json") - }; - let op = temp_env::with_var("GOOGLE_APPLICATION_CREDENTIALS", Some(env_path), || { - GCSCache::build( - "test-bucket", - "", - if credentials_from_env { - None - } else { - credential_path.to_str() - }, - None, - mode, - None, + ( + vec![ + Some(dir.path().join("unused-credentials.json")), + Some("".into()), + None, + ], + credential_path.to_str(), ) - }) - .unwrap() - .with_context( - OperationContext::new().with_http_transport(HttpTransporter::new( - FederationTransport { - subject_token, - scope: rw_to_scope(mode), - exchanges: exchanges.clone(), - }, - )), - ); + }; + for env_path in env_paths { + let exchanges = Arc::new(AtomicUsize::new(0)); + let op = temp_env::with_var("GOOGLE_APPLICATION_CREDENTIALS", env_path, || { + GCSCache::build("test-bucket", "", key_path, None, mode, None) + }) + .unwrap() + .with_context( + OperationContext::new().with_http_transport(HttpTransporter::new( + FederationTransport { + subject_token: subject_token.clone(), + scope: rw_to_scope(mode), + exchanges: exchanges.clone(), + }, + )), + ); - for _ in 0..2 { - let err = op.read(".sccache_check").await.unwrap_err(); - assert_eq!(err.kind(), opendal::ErrorKind::NotFound, "{err:?}"); + for _ in 0..2 { + let err = op.read(".sccache_check").await.unwrap_err(); + assert_eq!(err.kind(), opendal::ErrorKind::NotFound, "{err:?}"); + } + // Repeated requests reuse the exchanged access token. + assert_eq!(exchanges.load(Ordering::SeqCst), 1); } - // Repeated requests reuse the exchanged access token. - assert_eq!(exchanges.load(Ordering::SeqCst), 1); } }