Merge fix/b4-provision — exec redaction
This commit is contained in:
commit
da147c9a1b
2 changed files with 121 additions and 7 deletions
|
|
@ -2,10 +2,44 @@
|
|||
//!
|
||||
//! Centralises `std::process::Command` so both Hetzner and Vultr backends
|
||||
//! have a single JSON-exec path. Makes test-time PATH injection uniform.
|
||||
//!
|
||||
//! DO NOT pass secrets as CLI args — env-only per RULE 0.8. Error
|
||||
//! messages redact argv to `<bin> <N args>` and truncate stderr to 200
|
||||
//! chars to avoid info-disclosure in logs (future-proofing against
|
||||
//! accidental `--api-key $X` refactors + vultr-cli stderr leaking
|
||||
//! request URL query params).
|
||||
|
||||
use anyhow::{anyhow, Context, Result};
|
||||
use std::process::Command;
|
||||
|
||||
/// Max stderr length retained in error messages before truncation.
|
||||
const STDERR_MAX: usize = 200;
|
||||
|
||||
/// Redact CLI args to `<bin> <N args>` — never echo full argv.
|
||||
/// Protects against future secret-in-arg refactors (RULE 0.8).
|
||||
fn redact_args(bin: &str, args: &[&str]) -> String {
|
||||
format!("{bin} <{} args>", args.len())
|
||||
}
|
||||
|
||||
/// Truncate stderr to `STDERR_MAX` chars, UTF-8 safe (char-boundary aware).
|
||||
/// Vultr-cli stderr may echo request URLs with enumeration-useful query
|
||||
/// params; truncation limits leakage into Claude logs / CI artefacts.
|
||||
fn truncate_stderr(s: &str) -> String {
|
||||
let s = s.trim();
|
||||
if s.chars().count() <= STDERR_MAX {
|
||||
return s.to_string();
|
||||
}
|
||||
let mut out = String::with_capacity(STDERR_MAX + 20);
|
||||
for (i, ch) in s.chars().enumerate() {
|
||||
if i >= STDERR_MAX {
|
||||
break;
|
||||
}
|
||||
out.push(ch);
|
||||
}
|
||||
out.push_str("... (truncated)");
|
||||
out
|
||||
}
|
||||
|
||||
/// Run `bin args…` and return parsed JSON on exit code 0.
|
||||
/// Returns `Ok(None)` when the child exits non-zero (caller decides if
|
||||
/// that's an error or an "absent" signal).
|
||||
|
|
@ -37,10 +71,10 @@ pub fn run_json_strict(bin: &str, args: &[&str]) -> Result<Option<serde_json::Va
|
|||
if !output.status.success() {
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
return Err(anyhow!(
|
||||
"`{bin} {}` failed (code {:?}): {}",
|
||||
args.join(" "),
|
||||
"`{}` failed (code {:?}): {}",
|
||||
redact_args(bin, args),
|
||||
output.status.code(),
|
||||
stderr.trim()
|
||||
truncate_stderr(&stderr)
|
||||
));
|
||||
}
|
||||
let stdout = String::from_utf8(output.stdout)
|
||||
|
|
@ -62,10 +96,10 @@ pub fn run_void(bin: &str, args: &[&str]) -> Result<()> {
|
|||
if !output.status.success() {
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
return Err(anyhow!(
|
||||
"`{bin} {}` failed (code {:?}): {}",
|
||||
args.join(" "),
|
||||
"`{}` failed (code {:?}): {}",
|
||||
redact_args(bin, args),
|
||||
output.status.code(),
|
||||
stderr.trim()
|
||||
truncate_stderr(&stderr)
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@
|
|||
//! the real v1 / v3 CLI output shapes. The Backend impls then parse these
|
||||
//! exactly as they would production output.
|
||||
|
||||
use kei_provision::{resolve, CreateOpts};
|
||||
use kei_provision::{exec, resolve, CreateOpts};
|
||||
use std::fs;
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
use std::path::Path;
|
||||
|
|
@ -39,6 +39,20 @@ fn install_fake_fail(dir: &Path, bin: &str) {
|
|||
fs::set_permissions(&path, perms).unwrap();
|
||||
}
|
||||
|
||||
/// Install a fake that emits `stderr_text` to stderr then exits non-zero.
|
||||
/// Used to test error-redaction paths in `run_void` / `run_json_strict`.
|
||||
fn install_fake_stderr(dir: &Path, bin: &str, stderr_text: &str) {
|
||||
let path = dir.join(bin);
|
||||
// `cat <<'EOF' 1>&2` preserves literal text including URLs + secrets.
|
||||
let script = format!(
|
||||
"#!/usr/bin/env bash\ncat <<'EOF' 1>&2\n{stderr_text}\nEOF\nexit 1\n"
|
||||
);
|
||||
fs::write(&path, script).expect("write fake");
|
||||
let mut perms = fs::metadata(&path).unwrap().permissions();
|
||||
perms.set_mode(0o755);
|
||||
fs::set_permissions(&path, perms).unwrap();
|
||||
}
|
||||
|
||||
/// Prepend tempdir to PATH so the fake binary wins, but keep the rest of
|
||||
/// PATH so `#!/usr/bin/env bash` can still find `bash`.
|
||||
fn prep_env(dir: &Path, token_var: &str) {
|
||||
|
|
@ -182,3 +196,69 @@ fn create_opts_default_is_none_everywhere() {
|
|||
assert!(o.firewall.is_none());
|
||||
assert!(o.user_data_path.is_none());
|
||||
}
|
||||
|
||||
// ---- security: error-message redaction (MEDIUM info-disclosure) ----
|
||||
|
||||
#[test]
|
||||
fn error_redacts_full_argv_out_of_message() {
|
||||
// If a future refactor ever routes `--api-key sk-live-ABCDEF...` as
|
||||
// an inline arg, the failure path must NOT leak it into logs.
|
||||
let _guard = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner());
|
||||
let dir = TempDir::new().unwrap();
|
||||
install_fake_stderr(dir.path(), "fakebin", "boom");
|
||||
prep_env(dir.path(), "IGNORED_TOKEN");
|
||||
|
||||
let err = exec::run_void(
|
||||
"fakebin",
|
||||
&["--api-key", "sk-live-SECRET-DO-NOT-LEAK", "--region", "fsn1"],
|
||||
)
|
||||
.unwrap_err()
|
||||
.to_string();
|
||||
|
||||
// No full arg list; no secret-looking token.
|
||||
assert!(!err.contains("sk-live-SECRET-DO-NOT-LEAK"), "leaked: {err}");
|
||||
assert!(!err.contains("--api-key"), "leaked flag: {err}");
|
||||
// But the count + binary name MUST remain for debugging.
|
||||
assert!(err.contains("fakebin"), "missing bin: {err}");
|
||||
assert!(err.contains("4 args"), "missing count: {err}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn error_truncates_long_stderr_to_200_chars_plus_marker() {
|
||||
let _guard = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner());
|
||||
let dir = TempDir::new().unwrap();
|
||||
// 400 ASCII chars — must truncate.
|
||||
let long: String = "A".repeat(400);
|
||||
install_fake_stderr(dir.path(), "fakebin2", &long);
|
||||
prep_env(dir.path(), "IGNORED_TOKEN");
|
||||
|
||||
let err = exec::run_void("fakebin2", &["--op", "create"])
|
||||
.unwrap_err()
|
||||
.to_string();
|
||||
|
||||
assert!(err.contains("(truncated)"), "no marker: {err}");
|
||||
// Full 400-byte blob must not be present.
|
||||
assert!(!err.contains(&"A".repeat(400)), "not truncated: {err}");
|
||||
// 200 retained is allowed.
|
||||
assert!(err.contains(&"A".repeat(200)), "kept too little: {err}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn error_truncation_is_utf8_safe_on_cyrillic() {
|
||||
// Cyrillic chars are multi-byte — a naive byte-slice would panic
|
||||
// on a non-char-boundary. Assert the helper survives + produces
|
||||
// well-formed UTF-8.
|
||||
let _guard = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner());
|
||||
let dir = TempDir::new().unwrap();
|
||||
let cyr: String = "Ошибка".repeat(80); // ~480 chars, multi-byte
|
||||
install_fake_stderr(dir.path(), "fakebin3", &cyr);
|
||||
prep_env(dir.path(), "IGNORED_TOKEN");
|
||||
|
||||
let err = exec::run_void("fakebin3", &["--any"])
|
||||
.unwrap_err()
|
||||
.to_string();
|
||||
|
||||
// Didn't panic, is valid UTF-8 (it's a Rust String), contains marker.
|
||||
assert!(err.contains("(truncated)"), "no marker: {err}");
|
||||
assert!(err.contains("Ошибка"), "lost cyrillic: {err}");
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue