From 35d8ccafd18fa73a746ca034058e47e09473ec9e Mon Sep 17 00:00:00 2001 From: Nikko Crocker Date: Thu, 29 Oct 2026 09:29:23 -0400 Subject: [PATCH] Fixes an issue where the Github Access Token is getting redacted before it is used. * Updates the github_toekn_redaction test to check the actual token is not redacted but that any logging is redacted --- src/deployment_specification/parse.rs | 28 +++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/deployment_specification/parse.rs b/src/deployment_specification/parse.rs index d3975cdc..5b7b2628 100644 --- a/src/deployment_specification/parse.rs +++ b/src/deployment_specification/parse.rs @@ -33,15 +33,15 @@ impl DeploymentSpec { } fn parse_deployment_spec_data(data: &str) -> Result { - let mut parsed: Value = serde_json::from_str(data) + let parsed: Value = serde_json::from_str(data) .map_err(|e| DeploymentSpecError::ParseError(format!("JSON parse error: {e}")))?; - // The token must be redacted before logging. - if let Some(token) = parsed.get_mut("GitHubAccessToken") { + // Redact a copy for logging; build the spec from the original. + let mut sanitized = parsed.clone(); + if let Some(token) = sanitized.get_mut("GitHubAccessToken") { *token = Value::String("REDACTED".to_string()); } - - debug!("Parse: {parsed}"); + debug!("Parse: {sanitized}"); DeploymentSpec::new(&parsed) } @@ -180,6 +180,9 @@ mod tests { assert_eq!(spec.deployment_id, "d-12345678"); } + /// The debug log must show the redacted spec, while the parsed spec must + /// keep the real token so the GitHub downloader can authenticate. + #[tracing_test::traced_test] #[test] fn github_token_redaction() { let data = json!({ @@ -203,6 +206,19 @@ mod tests { payload: serde_json::to_string(&data).unwrap(), }; - let _spec = DeploymentSpec::parse_with_env(&envelope, &dev_mode_env()).unwrap(); + let spec = DeploymentSpec::parse_with_env(&envelope, &dev_mode_env()).unwrap(); + match spec.revision { + RevisionLocation::GitHub { auth_token, anonymous, .. } => { + assert_eq!(auth_token.as_deref(), Some("secret-token")); + assert!(!anonymous); + }, + other => panic!("expected GitHub revision, got {other:?}"), + } + // The spec was actually logged (guards against a vacuous pass)... + assert!(logs_contain("Parse:"), "expected the spec debug log line"); + // ...with the token redacted... + assert!(logs_contain(r#""GitHubAccessToken":"REDACTED""#)); + // ...and the real token nowhere in the captured logs. + assert!(!logs_contain("secret-token"), "real token leaked into logs"); } }