Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
177 changes: 175 additions & 2 deletions src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,18 @@ impl CommandHost {
return Err(Error::Uncertain("HOST_RESPONSE_TOO_LARGE".into()));
}
let status = child.wait().await?;
let value: Value = serde_json::from_slice(&bytes)?;
// A failed host may also print malformed output. Parse leniently
// so its exit identity is not replaced by a JSON parse error.
let parsed = serde_json::from_slice::<Value>(&bytes);
if !status.success() && parsed.is_err() {
let code = status
.code()
.map_or_else(|| "signal".to_string(), |code| code.to_string());
return Err(invalid(format!(
"HOST_COMMAND_FAILED: {action} (exit {code}): host response was not valid JSON"
)));
}
let value = parsed?;
if value.get("protocolVersion") == Some(&json!(1))
&& value.get("ok") == Some(&json!(false))
&& value.get("uncertain") == Some(&json!(true))
Expand All @@ -147,7 +158,22 @@ impl CommandHost {
)));
}
if !status.success() {
return Err(invalid(format!("HOST_COMMAND_FAILED: {action}")));
// Keep the host's own reason and exit code (issue slock#8609):
// without them a rollback only says "HOST_COMMAND_FAILED". The
// host writes this text for operators; it is bounded and
// stripped of control characters before it reaches receipts.
let code = status
.code()
.map_or_else(|| "signal".to_string(), |code| code.to_string());
let detail = value
.get("error")
.and_then(Value::as_str)
.map(host_error_detail)
.filter(|detail| !detail.is_empty())
.map_or_else(String::new, |detail| format!(": {detail}"));
return Err(invalid(format!(
"HOST_COMMAND_FAILED: {action} (exit {code}){detail}"
)));
}
if value.get("protocolVersion") != Some(&json!(1))
|| value.get("ok") != Some(&json!(true))
Expand Down Expand Up @@ -242,3 +268,150 @@ impl Host for CommandHost {
Ok(e)
}
}

const HOST_ERROR_DETAIL_MAX_CHARS: usize = 240;

/// A host-supplied failure reason, reduced to one bounded printable line.
const CREDENTIAL_MARKERS: &[&str] = &[
"authorization",
"bearer",
"token",
"secret",
"password",
"passwd",
"api_key",
"api key",
"api-key",
"apikey",
"access_key",
"access key",
"access-key",
"client_secret",
"client secret",
"client-secret",
"cookie",
"database_url",
"database-url",
];

fn has_credential_marker(line: &str) -> bool {
let line = line.to_ascii_lowercase();
CREDENTIAL_MARKERS
.iter()
.any(|marker| line.contains(marker))
}

/// `scheme://user@host` or `scheme://user:pass@host`: a token used as the
/// username is as sensitive as a password.
fn has_url_userinfo(line: &str) -> bool {
let mut rest = line;
while let Some(index) = rest.find("://") {
let after = &rest[index + 3..];
let authority = after
.split(|c: char| c.is_whitespace() || matches!(c, '/' | '?' | '#'))
.next()
.unwrap_or_default();
if authority.rfind('@').is_some_and(|at| at > 0) {
return true;
}
rest = after;
}
false
}

fn redact_opaque_word(word: &str) -> &str {
let opaque = word.len() >= 24
&& word
.chars()
.all(|c| c.is_ascii_alphanumeric() || "._-+".contains(c));
if opaque { "<redacted>" } else { word }
}

/// The host's reason reaches the terminal and the receipt, so redact it before
/// it is shortened. Redaction works on whole lines of the raw text: a line that
/// names a credential or carries URL userinfo is replaced entirely (a key and
/// its value may be split by spaces, quotes or a line break), and a credential
/// line ending in `:` or `=` also takes the next non-empty line.
fn redact_host_error(raw: &str) -> String {
let mut out: Vec<&str> = Vec::new();
let mut redact_next = false;
for line in raw.lines() {
let trimmed = line.trim();
if trimmed.is_empty() {
continue;
}
let carried = std::mem::take(&mut redact_next);
let credential = has_credential_marker(line);
if credential && matches!(trimmed.chars().last(), Some(':' | '=')) {
redact_next = true;
}
if carried || credential || has_url_userinfo(line) {
out.push("<redacted>");
continue;
}
out.extend(line.split_whitespace().map(redact_opaque_word));
}
out.join(" ")
}

fn host_error_detail(raw: &str) -> String {
let redacted = redact_host_error(raw);
let mut out = String::new();
let mut chars = 0;
for ch in redacted.chars() {
let ch = if ch.is_control() { ' ' } else { ch };
if ch == ' ' && out.ends_with(' ') {
continue;
}
if chars == HOST_ERROR_DETAIL_MAX_CHARS {
out.push('…');
break;
}
out.push(ch);
chars += 1;
}
out.trim().to_string()
}

#[cfg(test)]
mod host_error_detail_tests {
use super::host_error_detail;

#[test]
fn credential_lines_are_redacted_before_display() {
for (raw, leaked) in [
("login failed: password = hunter2", "hunter2"),
("token : abc", "abc"),
("Authorization: Basic c2hvcnQ=", "c2hvcnQ="),
("Cookie: theme=dark; sid=abc123", "abc123"),
(
"clone https://ghp_shortTok@github.com/o/r.git failed",
"ghp_shortTok",
),
("connect postgres://u:pw123@h/db refused", "pw123"),
(
"config:\n password:\n\n short-secret-value\nnext",
"short-secret-value",
),
] {
let detail = host_error_detail(raw);
assert!(
!detail.contains(leaked),
"{raw:?} leaked {leaked:?} as {detail:?}"
);
assert!(detail.contains("<redacted>"), "{raw:?} -> {detail:?}");
}
}

#[test]
fn harmless_reason_is_kept() {
assert_eq!(
host_error_detail("launchctl bootstrap failed: Input/output error"),
"launchctl bootstrap failed: Input/output error",
);
assert_eq!(
host_error_detail("config:\n password:\n x\nservice exited"),
"config: <redacted> <redacted> service exited"
);
}
}
75 changes: 75 additions & 0 deletions tests/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,3 +237,78 @@ async fn rejected_upgrade_is_held_without_recovering_another_writers_transaction
assert!(recovery.recovery_file.is_some());
Ok(())
}

#[tokio::test]
async fn native_controller_failure_keeps_the_hosts_reason_and_exit_code() -> Result<()> {
let root = tempdir()?;
let mut host = CommandHost::new(
vec![fixture().into(), "--controller".into()],
root.path().join("state"),
3000,
)?;
host.cwd = Some(root.path().into());
fs::write(root.path().join("effect-failed"), b"fixture")?;
let error = host.probe().await.unwrap_err();
assert!(!error.is_uncertain(), "a reported failure is definite");
let message = error.to_string();
assert!(
message.contains("HOST_COMMAND_FAILED: probe (exit 3): candidate was not started"),
"{message}"
);
assert!(
!message.chars().any(char::is_control),
"control characters are stripped: {message:?}"
);
assert!(
message.chars().count() < 400,
"the host's reason is bounded: {}",
message.chars().count()
);
Ok(())
}

#[tokio::test]
async fn native_controller_failure_redacts_credentials_in_the_hosts_reason() -> Result<()> {
let root = tempdir()?;
let mut host = CommandHost::new(
vec![fixture().into(), "--controller".into()],
root.path().join("state"),
3000,
)?;
host.cwd = Some(root.path().into());
fs::write(root.path().join("effect-failed-credential"), b"fixture")?;
let message = host.probe().await.unwrap_err().to_string();
assert!(
message.contains("HOST_COMMAND_FAILED: probe (exit 3): login failed"),
"{message}"
);
assert!(
!message.contains("pw123"),
"the credential leaked: {message}"
);
assert!(message.contains("<redacted>"), "{message}");
Ok(())
}

#[tokio::test]
async fn native_controller_failure_with_malformed_output_keeps_the_exit_code() -> Result<()> {
let root = tempdir()?;
let mut host = CommandHost::new(
vec![fixture().into(), "--controller".into()],
root.path().join("state"),
3000,
)?;
host.cwd = Some(root.path().into());
fs::write(root.path().join("effect-failed-malformed"), b"fixture")?;
let error = host.probe().await.unwrap_err();
assert!(
!error.is_uncertain(),
"a non-zero exit is a definite failure"
);
let message = error.to_string();
assert!(
message.contains("HOST_COMMAND_FAILED: probe (exit 4): host response was not valid JSON"),
"{message}"
);
Ok(())
}
22 changes: 22 additions & 0 deletions tests/support/fixture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,28 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
std::io::stdout().write_all(&vec![b'x'; 70000])?;
return Ok(());
}
if root.join("effect-failed").exists() {
// A definite failure with the host's own message, a control
// character and a long tail (bounded by the carrier).
let message = format!("candidate was not started\u{1b}[31m: {}", "y".repeat(600));
println!(
"{}",
serde_json::json!({"protocolVersion":1,"ok":false,"uncertain":false,"error":message})
);
std::process::exit(3);
}
if root.join("effect-failed-credential").exists() {
let message = "login failed\nAuthorization: Bearer pw123";
println!(
"{}",
serde_json::json!({"protocolVersion":1,"ok":false,"uncertain":false,"error":message})
);
std::process::exit(3);
}
if root.join("effect-failed-malformed").exists() {
println!("host crashed before writing a response {{");
std::process::exit(4);
}
if root.join("effect-uncertain").exists() {
println!(
"{}",
Expand Down
Loading