Skip to content

Commit 9306e07

Browse files
fix(cli): suppress terminal echo for interactive secret prompts (#45)
* fix(cli): suppress terminal echo for interactive secret prompts `vault login` / `vault unlock` echoed the master password in clear text as it was typed (visible on screen and in scrollback); the PIN, the add/edit login password, the card number/CVV, and the identity SSN/passport/license prompts shared the same flaw. Add a `NoEcho` RAII guard over `rustix::termios` that clears terminal ECHO for the duration of every interactive secret read and restores the prior attributes on drop (including on early return / panic). rustix was already in the tree transitively via `secmem-proc`, so no new crate is added and the crate keeps `#![forbid(unsafe_code)]`. Interactive entry now also submits on Enter — the master-password path previously read until EOF, so a typed password sat until Ctrl-D. Piped / redirected input is unchanged (`pass show | vault login` still reads the whole stream); non-secret prompts (the register server picker, account email, the ephemeral authenticator code) still echo by design. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * build(deps): bump quinn-proto 0.11.14 -> 0.11.15 (RUSTSEC-2026-0185) Remote memory-exhaustion (DoS) advisory in quinn-proto's out-of-order stream reassembly, published 2026-06-22. quinn-proto is a phantom Cargo.lock entry — an unenabled QUIC/HTTP3 path of reqwest; Vault speaks HTTP/2 only, so it never enters the build graph and the flaw is unreachable here — but cargo audit scans the lockfile literally, so pull the patched release to keep the supply-chain gate green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f244c86 commit 9306e07

5 files changed

Lines changed: 149 additions & 48 deletions

File tree

CHANGELOG.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,29 @@ range may break in any release.
88

99
## [Unreleased]
1010

11+
### Security
12+
13+
- **Interactive secret prompts no longer echo to the terminal.** `vault login`
14+
/ `vault unlock` printed the **master password in clear text** as it was
15+
typed (visible on screen and in scrollback); the PIN, the `add`/`edit` login
16+
password, and the card number/CVV and identity SSN/passport/license prompts
17+
had the same flaw. The CLI now disables terminal `ECHO` for the duration of
18+
every interactive secret read (a `NoEcho` RAII guard over `rustix::termios`,
19+
restored on drop — including on error/panic; no new dependency, no `unsafe`).
20+
Interactive entry now also **submits on Enter** (the master-password path
21+
previously read until EOF, so a typed password sat until `Ctrl-D`). Piped /
22+
redirected input is unchanged — `pass show | vault login` still reads the
23+
whole stream — and non-secret prompts (the register server picker, account
24+
email, the ephemeral authenticator code) still echo by design.
25+
26+
- **Bumped `quinn-proto` 0.11.14 → 0.11.15 (RUSTSEC-2026-0185).** A remote
27+
memory-exhaustion (DoS) advisory in `quinn-proto`'s out-of-order stream
28+
reassembly, published 2026-06-22. `quinn-proto` is a phantom `Cargo.lock`
29+
entry — an unenabled QUIC/HTTP3 path of `reqwest`; Vault speaks HTTP/2 only,
30+
so it never enters the build graph and the flaw is unreachable — but
31+
`cargo audit` scans the lockfile literally, so the patched release is pulled
32+
in to keep the supply-chain gate green.
33+
1134
### Added
1235

1336
- **EncString fuzz soak passed (PRD §11.4 / RELEASING.md gate #1).** A ≥ 24 h

Cargo.lock

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,11 @@ dirs = "5"
8383
linux-keyutils = "0.2"
8484
time = { version = "0.3", features = ["serde", "serde-well-known"] }
8585
fs2 = "0.4"
86+
# Terminal control — disable echo while reading secrets at an interactive
87+
# prompt (master password, PIN, card number/CVV, identity SSN/passport/license).
88+
# Pure-Rust syscalls, so the CLI keeps `forbid(unsafe_code)`; already in the tree
89+
# transitively via `secmem-proc`. MIT / Apache-2.0; GPL-3.0-or-later compatible.
90+
rustix = { version = "1", features = ["termios"] }
8691

8792
# TUI — ratatui + crossterm (MIT; GPL-3.0-or-later compatible). No cursive
8893
# (license-tree hygiene per PRD §7.2).

crates/vault-cli/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ clap = { workspace = true }
3434
dirs = { workspace = true }
3535
serde = { workspace = true }
3636
serde_json = { workspace = true }
37+
rustix = { workspace = true }
3738
tempfile = { workspace = true }
3839
tokio = { workspace = true }
3940
toml = { workspace = true }

crates/vault-cli/src/main.rs

Lines changed: 117 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use vault_config as config;
1616

1717
use std::fs::OpenOptions;
1818
use std::io::{self, BufRead, BufReader, IsTerminal, Read, Write};
19+
use std::os::fd::AsFd;
1920
use std::path::{Path, PathBuf};
2021

2122
use clap::{Parser, Subcommand};
@@ -632,15 +633,15 @@ impl IdentityArgs {
632633
company: self.company,
633634
ssn: self
634635
.ssn
635-
.then(|| read_tty_line("SSN / national id: ").map(String::into_bytes))
636+
.then(|| read_tty_secret("SSN / national id: ").map(String::into_bytes))
636637
.flatten(),
637638
passport_number: self
638639
.passport
639-
.then(|| read_tty_line("Passport number: ").map(String::into_bytes))
640+
.then(|| read_tty_secret("Passport number: ").map(String::into_bytes))
640641
.flatten(),
641642
license_number: self
642643
.license
643-
.then(|| read_tty_line("License number: ").map(String::into_bytes))
644+
.then(|| read_tty_secret("License number: ").map(String::into_bytes))
644645
.flatten(),
645646
email: self.email,
646647
phone: self.phone,
@@ -1060,10 +1061,69 @@ async fn password_unlock(
10601061
}
10611062
}
10621063

1064+
/// RAII guard that disables terminal ECHO on `fd` for its lifetime, restoring
1065+
/// the prior attributes on drop — including on early return or panic.
1066+
///
1067+
/// [`new`](NoEcho::new) returns `None` when `fd` is not a terminal or its
1068+
/// attributes can't be read; callers then fall back to a plain (echoing) read,
1069+
/// which is the only safe degradation and is harmless for piped input (nothing
1070+
/// is displayed there anyway). Built on `rustix::termios`, so no `unsafe` is
1071+
/// introduced and the crate keeps `#![forbid(unsafe_code)]`.
1072+
struct NoEcho<Fd: AsFd> {
1073+
fd: Fd,
1074+
original: rustix::termios::Termios,
1075+
}
1076+
1077+
impl<Fd: AsFd> NoEcho<Fd> {
1078+
fn new(fd: Fd) -> Option<Self> {
1079+
use rustix::termios::{LocalModes, OptionalActions, tcgetattr, tcsetattr};
1080+
1081+
let original = tcgetattr(&fd).ok()?;
1082+
let mut modified = original.clone();
1083+
// Clear ECHO only; ICANON stays on so the read is still line-buffered
1084+
// (Enter submits, Backspace edits) — we just stop the typed characters
1085+
// from being printed back to the screen.
1086+
modified.local_modes.remove(LocalModes::ECHO);
1087+
// `Flush` (TCSAFLUSH): apply now and discard any already-typed input,
1088+
// so a character typed (and echoed) before the guard took effect can't
1089+
// leak into the secret.
1090+
tcsetattr(&fd, OptionalActions::Flush, &modified).ok()?;
1091+
Some(Self { fd, original })
1092+
}
1093+
}
1094+
1095+
impl<Fd: AsFd> Drop for NoEcho<Fd> {
1096+
fn drop(&mut self) {
1097+
let _ = rustix::termios::tcsetattr(
1098+
&self.fd,
1099+
rustix::termios::OptionalActions::Now,
1100+
&self.original,
1101+
);
1102+
}
1103+
}
1104+
10631105
/// Prompt on the controlling terminal (`/dev/tty`) and read one line, so it
10641106
/// works even when stdin was piped/consumed (the password path). `None` if the
10651107
/// terminal can't be opened or the line is empty (treated as "abort").
1108+
///
1109+
/// Echoing variant — for non-secret prompts (menu choices, server URL, account
1110+
/// email). For secrets entered on the terminal (card number/CVV, identity
1111+
/// SSN/passport/license) use [`read_tty_secret`], which suppresses echo.
10661112
fn read_tty_line(prompt: &str) -> Option<String> {
1113+
read_tty(prompt, false)
1114+
}
1115+
1116+
/// Like [`read_tty_line`], but disables terminal echo while reading — for
1117+
/// secrets entered on `/dev/tty` so they never appear on screen or in
1118+
/// scrollback.
1119+
fn read_tty_secret(prompt: &str) -> Option<String> {
1120+
read_tty(prompt, true)
1121+
}
1122+
1123+
/// Shared `/dev/tty` line reader. When `secret` is set, echo is suppressed for
1124+
/// the read (a [`NoEcho`] guard) and a newline is emitted afterward, since the
1125+
/// user's un-echoed Enter otherwise leaves the cursor on the prompt line.
1126+
fn read_tty(prompt: &str, secret: bool) -> Option<String> {
10671127
let tty = OpenOptions::new()
10681128
.read(true)
10691129
.write(true)
@@ -1074,8 +1134,15 @@ fn read_tty_line(prompt: &str) -> Option<String> {
10741134
let _ = write!(w, "{prompt}");
10751135
let _ = w.flush();
10761136
}
1137+
let no_echo = secret.then(|| NoEcho::new(tty.as_fd())).flatten();
10771138
let mut line = String::new();
1078-
BufReader::new(tty).read_line(&mut line).ok()?;
1139+
let read = BufReader::new(&tty).read_line(&mut line).ok();
1140+
if no_echo.is_some() {
1141+
let mut w = &tty;
1142+
let _ = writeln!(w);
1143+
}
1144+
drop(no_echo); // restore echo before returning
1145+
read?;
10791146
let trimmed = line.trim_end_matches(['\n', '\r']).to_owned();
10801147
line.zeroize();
10811148
if trimmed.is_empty() {
@@ -1525,10 +1592,10 @@ async fn cmd_add(ep: Endpoint<'_>, args: AddArgs) -> Result<(), u8> {
15251592
Some(CardWrite {
15261593
cardholder: args.cardholder,
15271594
brand: args.brand,
1528-
number: read_tty_line("Card number: ").map(String::into_bytes),
1595+
number: read_tty_secret("Card number: ").map(String::into_bytes),
15291596
exp_month,
15301597
exp_year,
1531-
code: read_tty_line("CVV (leave empty for none): ").map(String::into_bytes),
1598+
code: read_tty_secret("CVV (leave empty for none): ").map(String::into_bytes),
15321599
})
15331600
} else {
15341601
None
@@ -1634,13 +1701,13 @@ async fn cmd_edit(ep: Endpoint<'_>, args: EditArgs) -> Result<(), u8> {
16341701
brand: args.brand,
16351702
number: args
16361703
.number
1637-
.then(|| read_tty_line("New card number: ").map(String::into_bytes))
1704+
.then(|| read_tty_secret("New card number: ").map(String::into_bytes))
16381705
.flatten(),
16391706
exp_month,
16401707
exp_year,
16411708
code: args
16421709
.code
1643-
.then(|| read_tty_line("New CVV: ").map(String::into_bytes))
1710+
.then(|| read_tty_secret("New CVV: ").map(String::into_bytes))
16441711
.flatten(),
16451712
})
16461713
} else {
@@ -1715,21 +1782,31 @@ fn generate_pw(len: usize) -> Result<Zeroizing<String>, u8> {
17151782
})
17161783
}
17171784

1718-
/// Read an optional secret from stdin. Returns `None` for empty input (after a
1719-
/// single trailing newline). Prompts on a TTY; never echoes via argv.
1720-
fn read_secret(prompt: &str) -> Result<Option<Vec<u8>>, u8> {
1785+
/// Read a secret from stdin, returning `None` for empty input (after stripping
1786+
/// one trailing newline). On an interactive terminal: print `prompt` to stderr,
1787+
/// suppress echo for the duration (a [`NoEcho`] guard), read a single line
1788+
/// (Enter submits), then emit a newline. When stdin is piped/redirected the
1789+
/// whole stream is read (no prompt, no echo concern) so piped secrets — e.g.
1790+
/// `pass show | vault login` — keep working.
1791+
fn read_stdin_secret(prompt: &str) -> io::Result<Option<Vec<u8>>> {
17211792
let stdin = io::stdin();
1793+
let mut buf = String::new();
17221794
if stdin.is_terminal() {
17231795
let mut stderr = io::stderr();
17241796
let _ = write!(stderr, "{prompt}");
17251797
let _ = stderr.flush();
1798+
let no_echo = NoEcho::new(stdin.as_fd());
1799+
let read = stdin.lock().read_line(&mut buf);
1800+
if no_echo.is_some() {
1801+
let _ = writeln!(io::stderr());
1802+
}
1803+
drop(no_echo); // restore echo before propagating any read error
1804+
read?;
1805+
} else {
1806+
stdin.lock().read_to_string(&mut buf)?;
17261807
}
1727-
let mut buf = String::new();
1728-
let read_res = stdin.lock().read_to_string(&mut buf);
1729-
if let Err(e) = read_res {
1730-
eprintln!("vault: failed to read input: {e}");
1731-
return Err(2);
1732-
}
1808+
// Strip exactly one trailing newline (terminals send one); preserve any
1809+
// deliberate trailing whitespace beyond that.
17331810
if buf.ends_with('\n') {
17341811
buf.pop();
17351812
if buf.ends_with('\r') {
@@ -1745,6 +1822,15 @@ fn read_secret(prompt: &str) -> Result<Option<Vec<u8>>, u8> {
17451822
Ok(Some(bytes))
17461823
}
17471824

1825+
/// Read an optional secret from stdin. Returns `None` for empty input. Echo is
1826+
/// suppressed on an interactive terminal; never echoes via argv.
1827+
fn read_secret(prompt: &str) -> Result<Option<Vec<u8>>, u8> {
1828+
read_stdin_secret(prompt).map_err(|e| {
1829+
eprintln!("vault: failed to read input: {e}");
1830+
2u8
1831+
})
1832+
}
1833+
17481834
async fn cmd_get(ep: Endpoint<'_>, name: String, field: Field, json: bool) -> Result<(), u8> {
17491835
let mut stream = connect(ep).await?;
17501836
let req = Request::Get {
@@ -1894,38 +1980,23 @@ fn resolve_register_email(cli: Option<String>) -> Result<String, u8> {
18941980
Err(2)
18951981
}
18961982

1897-
/// Read the master password. Prompts on a TTY with no echo guarantee yet
1898-
/// (M3 ships without `rpassword` to keep the dep tree slim — interactive
1899-
/// users should redirect from a tool like `pass` or `gpg --decrypt`).
1983+
/// Read the master password. On an interactive terminal the input is **not**
1984+
/// echoed (a [`NoEcho`] guard) and a single line is read (Enter submits). When
1985+
/// stdin is piped/redirected the entire stream is taken as the password, so
1986+
/// `pass show | vault login` and the "stdin consumed, 2FA read from `/dev/tty`"
1987+
/// flow keep working. Empty input is rejected.
19001988
fn read_password() -> Result<Vec<u8>, u8> {
1901-
let stdin = io::stdin();
1902-
if stdin.is_terminal() {
1903-
let mut stderr = io::stderr();
1904-
let _ = write!(stderr, "Master password: ");
1905-
let _ = stderr.flush();
1906-
}
1907-
let mut buf = String::new();
1908-
let read_res = stdin.lock().read_to_string(&mut buf);
1909-
if let Err(e) = read_res {
1910-
eprintln!("vault: failed to read password: {e}");
1911-
return Err(2);
1912-
}
1913-
// Strip exactly one trailing newline (typical from terminals); preserve
1914-
// any deliberate trailing whitespace beyond that.
1915-
if buf.ends_with('\n') {
1916-
buf.pop();
1917-
if buf.ends_with('\r') {
1918-
buf.pop();
1989+
match read_stdin_secret("Master password: ") {
1990+
Ok(Some(bytes)) => Ok(bytes),
1991+
Ok(None) => {
1992+
eprintln!("vault: empty password");
1993+
Err(2)
1994+
}
1995+
Err(e) => {
1996+
eprintln!("vault: failed to read password: {e}");
1997+
Err(2)
19191998
}
19201999
}
1921-
if buf.is_empty() {
1922-
eprintln!("vault: empty password");
1923-
buf.zeroize();
1924-
return Err(2);
1925-
}
1926-
let bytes = buf.as_bytes().to_vec();
1927-
buf.zeroize();
1928-
Ok(bytes)
19292000
}
19302001

19312002
/// Print a `{ "<action>": true }` acknowledgement under `--json`; stay silent

0 commit comments

Comments
 (0)