Skip to content

feat(pairing-cli): source --nsec - reads the key from stdin, never argv - #6849

Open
lodar wants to merge 1 commit into
block:mainfrom
5dive-ai:upstream-pr/pair-nsec-stdin
Open

feat(pairing-cli): source --nsec - reads the key from stdin, never argv#6849
lodar wants to merge 1 commit into
block:mainfrom
5dive-ai:upstream-pr/pair-nsec-stdin

Conversation

@lodar

@lodar lodar commented Aug 26, 2026

Copy link
Copy Markdown

Summary

buzz-pair source transfers a real secret key, and today its only form is --nsec <bech32> — an argv element. argv is world-readable in /proc/<pid>/cmdline for the entire life of the process, which for a source session is up to the 120s it spends waiting for the target to appear. Any local user, and anything that snapshots the process table, gets the key.

--nsec - takes the key from the first line of stdin instead. It is read before any session I/O and through the same buffered io::stdin() handle the SAS prompt already uses, a line at a time, so the interactive y/n answer is simply the next line and the existing flow is unchanged:

{ printf '%s\n' "$nsec"; cat; } | buzz-pair source --nsec - --relay wss://...

--nsec <bech32> keeps working exactly as before; this only adds the - form. The clap help text names both - and the word stdin, so a caller can probe source --help and refuse to hand a key to a build that does not support it.

Related issue

None found — I searched open issues and PRs for stdin/argv key handling in the pairing CLI and turned up nothing. The closest neighbour is #2610 (test(pairing-cli): add unit coverage for buzz-pairing-cli), which is adjacent rather than overlapping.

Note for whoever triages: this touches the same resolve_payload function as my other pairing-cli PR (the HTTPS pairing fix). They are independent and either can be taken alone — whichever lands second takes a small textual conflict, and I will rebase it.

Testing

  • Manual: piping a key in with --nsec - pairs normally and the SAS prompt still reads the next stdin line; --nsec <bech32> is byte-for-byte the previous behaviour; ps//proc/<pid>/cmdline during a live source session holds the literal - and no key material.
  • rustfmt --check clean; lint, unit tests, the Windows build and the cross-compile matrix ran green on this exact tree in my own CI before opening.
  • No unit test, deliberately, and I would rather say so than paper over it: the read goes through the process-global io::stdin() handle, so covering it means either injecting a BufRead into resolve_payload — a refactor that would collide with the other pairing-cli PR — or driving the built binary from an integration test. Say which you would prefer and I will add it here.

No UI surface is touched.

The branch is based on f88cda9eb, which is behind main; crates/buzz-pairing-cli/src/main.rs has not been touched upstream since, so it applies cleanly.

…argv

`buzz-pair source` transfers a real secret key. Today its only form is
`--nsec <bech32>`: an argv element, and argv is world-readable in
`/proc/<pid>/cmdline` for the entire life of the process — which for a `source`
session is up to the 120s it spends waiting for the target to show up. Any
local user, and anything that snapshots the process table, gets the key.

`--nsec -` takes the key from the first line of stdin instead. It is read before
any session I/O and through the same buffered `io::stdin()` handle the SAS
prompt already uses, a line at a time, so the interactive y/n answer is simply
the next line and the existing flow is unchanged:

    { printf '%s\n' "$nsec"; cat; } | buzz-pair source --nsec - --relay wss://...

`--nsec <bech32>` keeps working exactly as before; this only adds the `-` form.
The clap help text names both `-` and the word stdin so a caller can probe
`source --help` and refuse to hand a key to a build that does not support it.

Signed-off-by: lodar <markounik@gmail.com>
@lodar
lodar requested a review from a team as a code owner August 26, 2026 03:07

@Chessing234 Chessing234 left a comment

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.

the threat here is real and the fix is the right shape — a source session waits up to 120s for the target, and argv is readable from /proc/<pid>/cmdline for that whole window. i checked the stdin claim too: read_line goes through io::stdin(), which is the process-global buffered handle, and read_yes_no calls the same function, so the SAS answer really is just the next line. the doc example works.

the gap is that the key still ends up in memory the crate doesn't zeroize.

fn read_line() -> Result<String, CliError> {
    let stdin = io::stdin();
    let mut line = String::new();
    stdin.lock().read_line(&mut line)?;
    Ok(line.trim_end_matches('\n').trim_end_matches('\r').to_string())
}

Zeroizing::new(read_line()?) wraps only the value that comes back. by then the nsec exists in at least two other places that are dropped without wiping: line itself, and — because read_line grows the String as it reads — any earlier buffer it reallocated away from. on top of that Stdin's internal BufReader holds the raw bytes it read (its buffer is 8 KiB and lives for the process), so the nsec sits in that buffer until enough later input overwrites it, which for this tool is one short y.

a crate that already reaches for Zeroizing and SecretKey::parse(s.as_str()) is clearly trying to get this right, so it's worth closing: read into a Zeroizing<String> directly and zero the tail explicitly rather than round-tripping through to_string():

fn read_line_secret() -> Result<Zeroizing<String>, CliError> {
    let mut line = Zeroizing::new(String::with_capacity(128));
    io::stdin().lock().read_line(&mut line)?;
    let end = line.trim_end_matches(['\n', '\r']).len();
    line.truncate(end);   // truncate leaves the tail bytes, but Zeroizing wipes the whole buffer on drop
    Ok(line)
}

with_capacity up front matters — without it the realloc during read leaves a copy behind that nothing owns. leave read_line as-is for the y/n prompt and use the secret variant only for --nsec -.

two smaller ones:

an interactive --nsec - echoes the key. if someone runs it without a pipe (which the flag invites — it's the obvious thing to try), the terminal echoes the nsec as they paste it and it lands in the scrollback. worth either detecting a tty and reading with echo off, or refusing a tty with a message pointing at the pipe form from the doc comment.

empty stdin gives a confusing error. read_line on a closed stdin returns Ok(""), which then fails as InvalidNsec("...") — the user reads that as "my key is malformed" when the real problem is that nothing arrived. a if s.is_empty() { return Err(...) } with its own message would save a debugging session.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants