diff --git a/Dockerfile b/Dockerfile index 74e54d8a..04986a4c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -31,7 +31,7 @@ RUN test -n "${VERSION}" \ && test "${BUILD_DATE}" != unknown \ && apt-get update \ && apt-get install -y --no-install-recommends \ - ca-certificates=20230311+deb12u1 \ + ca-certificates \ tini=0.19.0-1+b3 \ && rm -rf /var/lib/apt/lists/* \ && groupadd --gid 10001 extenddb \ diff --git a/crates/storage-postgres/src/bootstrapper.rs b/crates/storage-postgres/src/bootstrapper.rs index d10a8255..ce483826 100755 --- a/crates/storage-postgres/src/bootstrapper.rs +++ b/crates/storage-postgres/src/bootstrapper.rs @@ -13,8 +13,8 @@ use async_trait::async_trait; use extenddb_storage::bootstrapper::{ AdminBootstrapResult, BootstrapConfig, Bootstrapper, helpers::{ - check_conflict, extract_arg, generate_account_id, generate_encryption_key, - generate_random_password, hash_password_async, + check_conflict, check_conflict_redacted, extract_arg, generate_account_id, + generate_encryption_key, generate_random_password, hash_password_async, }, }; use extenddb_storage::management_store::{OpError, OpResult}; @@ -688,7 +688,7 @@ impl PostgresBootstrapper { check_conflict(pg_host.as_ref(), &parts.host, "--pg-host")?; check_conflict(pg_port.as_ref(), &parts.port, "--pg-port")?; check_conflict(extenddb_user.as_ref(), &parts.user, "--extenddb-user")?; - check_conflict(extenddb_pass.as_ref(), &parts.password, "--extenddb-pass")?; + check_conflict_redacted(extenddb_pass.as_ref(), &parts.password, "--extenddb-pass")?; if let Some(ref cli_catalog) = catalog_db && cli_catalog != &parts.database diff --git a/crates/storage/src/bootstrapper.rs b/crates/storage/src/bootstrapper.rs index e0b1f8bf..39266884 100755 --- a/crates/storage/src/bootstrapper.rs +++ b/crates/storage/src/bootstrapper.rs @@ -235,6 +235,10 @@ pub mod helpers { } /// Check that a CLI arg, if provided, matches the config value. + /// + /// The error message includes both values, so this must only be used for + /// non-sensitive settings (hosts, ports, usernames). For secrets, use + /// [`check_conflict_redacted`]. pub fn check_conflict( cli_val: Option<&T>, config_val: &T, @@ -250,6 +254,25 @@ pub mod helpers { Ok(()) } + /// Like [`check_conflict`], but the error message never contains either + /// value. Use for passwords and other secrets: the message lands on + /// stderr, and in a container stderr is the log stream, which is commonly + /// shipped to aggregators. + pub fn check_conflict_redacted( + cli_val: Option<&T>, + config_val: &T, + flag: &str, + ) -> Result<(), crate::error::StorageError> { + if let Some(v) = cli_val + && v != config_val + { + return Err(crate::error::StorageError::Internal(format!( + "{flag} conflicts with the value in the config file (values redacted)" + ))); + } + Ok(()) + } + /// Extract a CLI argument value by flag name. #[must_use] pub fn extract_arg(args: &[String], flag: &str) -> Option { @@ -432,6 +455,28 @@ pub mod helpers { assert!(result.is_err(), "Should error when numeric values differ"); } + #[test] + fn test_check_conflict_redacted_never_leaks_either_value() { + let cli_secret = "hunter2-cli".to_string(); + let config_secret = "hunter2-config".to_string(); + let result = + check_conflict_redacted(Some(&cli_secret), &config_secret, "--extenddb-pass"); + let err = result.unwrap_err().to_string(); + assert!(err.contains("--extenddb-pass"), "flag must be named: {err}"); + assert!(!err.contains("hunter2-cli"), "CLI secret leaked: {err}"); + assert!( + !err.contains("hunter2-config"), + "config secret leaked: {err}" + ); + } + + #[test] + fn test_check_conflict_redacted_ok_paths() { + let same = "s3cret".to_string(); + assert!(check_conflict_redacted(Some(&same), &same, "--extenddb-pass").is_ok()); + assert!(check_conflict_redacted(None, &same, "--extenddb-pass").is_ok()); + } + #[test] fn test_extract_arg_found() { let args = vec![