mirror of
https://github.com/bahdotsh/wrkflw.git
synced 2026-05-18 05:05:35 +02:00
fix(executor): harden container config against credential leaks and empty volumes
ContainerCredentials had a derived Debug impl that would happily dump passwords into logs, panic output, and anywhere else Debug gets called. That's *exactly* the kind of thing that bites you at 3am when someone adds a debug trace and suddenly credentials show up in plaintext in your log aggregator. Replace the derived Debug with a manual impl that redacts the password field. While at it, add a guard for empty volume specs that would otherwise produce undefined Docker behavior, a note about the splitn limitation with Windows paths, and fix clippy warnings on the test assertions.
This commit is contained in:
@@ -2279,6 +2279,11 @@ fn prepare_container_mounts(
|
||||
let mut owned_volume_paths: Vec<VolumePathPair> = Vec::new();
|
||||
if let Some(container_volumes) = container_config.and_then(|c| c.volumes.as_ref()) {
|
||||
for vol_spec in container_volumes {
|
||||
if vol_spec.is_empty() {
|
||||
wrkflw_logging::warning("skipping empty volume spec");
|
||||
continue;
|
||||
}
|
||||
// NOTE: splitn(3, ':') won't correctly handle Windows-style host paths (e.g. C:\data:/container)
|
||||
let parts: Vec<&str> = vol_spec.splitn(3, ':').collect();
|
||||
match parts.len() {
|
||||
3 => {
|
||||
@@ -3046,7 +3051,7 @@ mod tests {
|
||||
let (_volumes, github_mount) = prepare_container_mounts(&mut step_env, &job_env, None);
|
||||
|
||||
// Env vars should NOT be remapped
|
||||
assert!(step_env.get("GITHUB_ENV").is_none());
|
||||
assert!(!step_env.contains_key("GITHUB_ENV"));
|
||||
|
||||
// Should mount the parent directory identity-mapped
|
||||
let (host, container) = github_mount.unwrap();
|
||||
@@ -3166,9 +3171,9 @@ mod tests {
|
||||
// GITHUB_ENV should be remapped
|
||||
assert_eq!(step_env.get("GITHUB_ENV").unwrap(), "/github/workflow/env");
|
||||
// Others should NOT be inserted (no phantom paths)
|
||||
assert!(step_env.get("GITHUB_OUTPUT").is_none());
|
||||
assert!(step_env.get("GITHUB_PATH").is_none());
|
||||
assert!(step_env.get("GITHUB_STEP_SUMMARY").is_none());
|
||||
assert!(!step_env.contains_key("GITHUB_OUTPUT"));
|
||||
assert!(!step_env.contains_key("GITHUB_PATH"));
|
||||
assert!(!step_env.contains_key("GITHUB_STEP_SUMMARY"));
|
||||
}
|
||||
|
||||
// --- container env precedence tests ---
|
||||
|
||||
@@ -73,7 +73,7 @@ where
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize, Serialize, Clone)]
|
||||
#[derive(Deserialize, Serialize, Clone)]
|
||||
pub struct ContainerCredentials {
|
||||
#[serde(default)]
|
||||
pub username: Option<String>,
|
||||
@@ -81,6 +81,15 @@ pub struct ContainerCredentials {
|
||||
pub password: Option<String>,
|
||||
}
|
||||
|
||||
impl std::fmt::Debug for ContainerCredentials {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.debug_struct("ContainerCredentials")
|
||||
.field("username", &self.username)
|
||||
.field("password", &"[REDACTED]")
|
||||
.finish()
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize, Serialize, Clone)]
|
||||
pub struct JobContainer {
|
||||
pub image: String,
|
||||
|
||||
Reference in New Issue
Block a user