Skip to content

Fix CLI exit paths to preserve Zeroize semantics #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
darkmaster0345 merged 1 commit into from
Jun 19, 2026
Merged

Fix CLI exit paths to preserve Zeroize semantics #29

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
55 changes: 34 additions & 21 deletions neuron-encrypt/src/bin/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,19 +105,17 @@ enum Command {
},
}

fn read_password(password_file: &Option<PathBuf>) -> Zeroizing<String> {
fn read_password(password_file: &Option<PathBuf>) -> Result<Zeroizing<String>, (ExitCode, String)> {
if let Some(path) = password_file {
let bytes = fs::read(path).unwrap_or_else(|e| {
eprintln!("Error: Cannot read password file: {e}");
std::process::exit(ExitCode::BadInput as i32);
});
let mut pw = Zeroizing::new(String::from_utf8(bytes).unwrap_or_else(|e| {
eprintln!("Error: Password file contains invalid UTF-8: {e}");
std::process::exit(ExitCode::BadInput as i32);
}));
let bytes = fs::read(path).map_err(|e| {
(ExitCode::BadInput, format!("Cannot read password file: {e}"))
})?;
let mut pw = Zeroizing::new(String::from_utf8(bytes).map_err(|e| {
(ExitCode::BadInput, format!("Password file contains invalid UTF-8: {e}"))
})?);
Comment on lines +113 to +115

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Zeroize invalid UTF-8 password bytes

When --password-file points at a file whose secret contains any invalid UTF-8 byte, String::from_utf8 returns the original file contents inside FromUtf8Error; this handler formats the error and drops that buffer before it is ever wrapped in Zeroizing. That leaves the password file bytes in the allocator on this new non-exiting error path, so the hardening does not actually wipe the sensitive buffer for invalid password files.

Useful? React with 👍 / 👎.

let trimmed_len = pw.trim_end_matches(&['\r', '\n'][..]).len();
pw.truncate(trimmed_len);
return pw;
return Ok(pw);
}

if let Ok(pw) = std::env::var("NEURON_PASSWORD") {
Expand All @@ -127,7 +125,7 @@ fn read_password(password_file: &Option<PathBuf>) -> Zeroizing<String> {
eprintln!(
"[WARN] Prefer --password-file with a 0600-permission file for sensitive use."
);
return Zeroizing::new(pw);
return Ok(Zeroizing::new(pw));
}

loop {
Expand All @@ -137,11 +135,10 @@ fn read_password(password_file: &Option<PathBuf>) -> Zeroizing<String> {
eprintln!("Error: Passphrase cannot be empty.");
continue;
}
return Zeroizing::new(pw);
return Ok(Zeroizing::new(pw));
}
Err(e) => {
eprintln!("Error reading passphrase: {e}");
std::process::exit(ExitCode::BadInput as i32);
return Err((ExitCode::BadInput, format!("Error reading passphrase: {e}")));
}
}
}
Expand Down Expand Up @@ -171,8 +168,8 @@ fn constant_time_eq(a: &[u8], b: &[u8]) -> bool {
final_result == 0
}

fn read_password_confirmed(password_file: &Option<PathBuf>) -> Result<Zeroizing<String>, String> {
let pass1 = read_password(password_file);
fn read_password_confirmed(password_file: &Option<PathBuf>) -> Result<Zeroizing<String>, (ExitCode, String)> {
let pass1 = read_password(password_file)?;
let pass2 = loop {
match rpassword::prompt_password("Confirm passphrase: ") {
Ok(pw) => {
Expand All @@ -182,11 +179,11 @@ fn read_password_confirmed(password_file: &Option<PathBuf>) -> Result<Zeroizing<
}
break Zeroizing::new(pw);
}
Err(e) => return Err(format!("Error reading passphrase: {e}")),
Err(e) => return Err((ExitCode::BadInput, format!("Error reading passphrase: {e}"))),
}
};
if !constant_time_eq(pass1.as_bytes(), pass2.as_bytes()) {
return Err("Passphrases do not match".to_owned());
return Err((ExitCode::BadInput, "Passphrases do not match".to_owned()));
}
Ok(pass1)
}
Expand Down Expand Up @@ -435,13 +432,29 @@ fn run() -> Result<(), ExitCode> {
{
match read_password_confirmed(&cli.password_file) {
Ok(password) => password,
Err(msg) => {
Err((code, msg)) => {
Comment on lines 432 to +435

@sourcery-ai sourcery-ai Bot Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): JSON mode does not emit structured error output for read_password_confirmed, unlike the non-confirm path.

In this branch, read_password_confirmed errors only print to stderr and return an exit code, even when cli.json is set, whereas the non-confirm branch now emits a JSON error before returning. This inconsistency means JSON-mode callers get structured errors only for some password failures (e.g., mismatched passphrases). Please mirror the JSON emission here so all password-related errors produce a JSON payload when --json is enabled.

eprintln!("Error: {msg}");
return Err(ExitCode::BadInput);
return Err(code);
}
}
}
_ => read_password(&cli.password_file),
_ => match read_password(&cli.password_file) {
Ok(password) => password,
Err((code, msg)) => {
if cli.json {
emit_json(&JsonResult {
status: "error".into(),
output_path: None,
bytes_processed: None,
duration_ms: start.elapsed().as_millis(),
sha256: None,
error: Some(msg.clone()),
});
}
eprintln!("Error: {msg}");
return Err(code);
}
}
};

if password.len() < crypto::MIN_PASSWORD_LEN {
Expand Down
Loading