Skip to content

Commit c583b24

Browse files
claudeBenWestgate
authored andcommitted
cli: Keep Core selection off pipes and redirected stderr
Connecting before the search made two problems easier to hit. With damaged input piped to `ms32 correct` and two Core networks running, the network prompt read the exhausted pipe forever. With stderr redirected, "Using Bitcoin Core on ..." and a blank line came before `interactive confirmation required`, which the security model says must be the only message. Ask for a network only when stdin is a terminal; a pipe now gets "More than one local Bitcoin Core network is running." Print Core's messages only when stderr is a terminal. The gate test's fake now reports like the real one, so it catches the extra output. Claude-Session: https://claude.ai/code/session_015CuLXqAvAovfoVcUmmogwa
1 parent 799bffe commit c583b24

3 files changed

Lines changed: 39 additions & 9 deletions

File tree

‎src/codex32/cli.py‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -442,12 +442,14 @@ def _initialize_wallet(
442442

443443

444444
def _connected_core(fallback: str | None = None) -> BitcoinCore:
445+
# A pipe can't choose a network, and redirected stderr keeps only the gate's error.
445446
try:
446447
core = BitcoinCore.connect(
447-
lambda prompt: _text(prompt, optional=True),
448-
lambda message: _print(message, err=True),
448+
(lambda prompt: _text(prompt, optional=True)) if sys.stdin.isatty() else None,
449+
(lambda message: _print(message, err=True)) if sys.stderr.isatty() else None,
449450
)
450-
_print("", err=True)
451+
if sys.stderr.isatty():
452+
_print("", err=True)
451453
return core
452454
except KeyboardInterrupt as error:
453455
raise _CoreSelectionInterrupted from error

‎tests/test_cli.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import contextlib
55
import importlib
66
import io
7+
import json
78
import re
89
import subprocess
910
import sys
@@ -123,6 +124,9 @@ def initialize(
123124
return "test-wallet"
124125

125126

127+
_REAL_CONNECT = BitcoinCore.connect
128+
129+
126130
@pytest.fixture(autouse=True)
127131
def _offline_core(monkeypatch):
128132
monkeypatch.setattr("codex32.cli.BitcoinCore.connect", lambda *args, **kwargs: _FakeBitcoinCore())
@@ -320,6 +324,30 @@ def test_missing_core_for_wallet_setup_offers_no_fallback(monkeypatch: pytest.Mo
320324
assert str(failure.value) == "bitcoin-cli was not found.\nThis command gives Bitcoin Core the master key."
321325

322326

327+
def test_piped_input_never_answers_the_core_network_choice(
328+
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
329+
) -> None:
330+
def run(command: list[str], **_options: object) -> subprocess.CompletedProcess[str]:
331+
chain = command[1].removeprefix("-chain=")
332+
if chain not in ("main", "signet"):
333+
return subprocess.CompletedProcess(command, 1, "", "")
334+
response = {"version": 320000} if command[-1] == "getnetworkinfo" else {"chain": chain}
335+
return subprocess.CompletedProcess(command, 0, json.dumps(response), "")
336+
337+
damaged = VECTOR_1["secret_s"].replace("x", "q", 1)
338+
reads = iter((damaged,)) # A second read would be the pipe's EOF answering the network prompt.
339+
monkeypatch.setattr("codex32._bitcoin_core.shutil.which", lambda _name: "/reviewed/bitcoin-cli")
340+
monkeypatch.setattr(subprocess, "run", run)
341+
monkeypatch.setattr(BitcoinCore, "connect", _REAL_CONNECT)
342+
monkeypatch.setattr(sys, "stdin", io.StringIO(damaged))
343+
monkeypatch.setattr("codex32._cli_input._stdin", lambda: next(reads))
344+
345+
assert ms_main(["correct"]) == 3
346+
assert capsys.readouterr().err.startswith(
347+
"ms32 correct: More than one local Bitcoin Core network is running.\nThis command uses Bitcoin Core"
348+
)
349+
350+
323351
def test_check_does_not_derive_wallet_keys(monkeypatch: pytest.MonkeyPatch) -> None:
324352
cli_module = importlib.import_module("codex32.cli")
325353

‎tests/test_correction_disclosure.py‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,12 @@ def test_public_api_full_checksum_erasure_completion_carries_risk(source, degree
9797
@pytest.mark.parametrize("entrypoint", (cli.main, cli.ms_main))
9898
@pytest.mark.parametrize("plain", (False, True))
9999
def test_noninteractive_gate_emits_only_operational_error(entrypoint, plain):
100-
stdout, stderr, connected = io.StringIO(), io.StringIO(), []
100+
stdout, stderr = io.StringIO(), io.StringIO()
101101

102-
def connect(*args):
103-
connected.append(_FakeBitcoinCore())
104-
return connected[-1]
102+
def connect(ask=None, tell=None):
103+
if tell is not None:
104+
tell("Using Bitcoin Core on signet.")
105+
return _FakeBitcoinCore()
105106

106107
with (
107108
patch.object(sys, "stdin", io.StringIO(VECTOR_1["secret_s"][:-1] + "?")),
@@ -113,8 +114,7 @@ def connect(*args):
113114
status = entrypoint(["correct", *(["--plain"] if plain else [])])
114115
prog = "codex32" if entrypoint is cli.main else "ms32"
115116
assert status == 3 and stdout.getvalue() == ""
116-
# ms32 connects to Core before searching; that prints only a blank line.
117-
assert stderr.getvalue() == "\n" * len(connected) + f"{prog}: interactive confirmation required\n"
117+
assert stderr.getvalue() == f"{prog}: interactive confirmation required\n"
118118

119119

120120
@pytest.mark.parametrize("answer", ("y", "Y", "yes", "Yes", "n", "", "other", None))

0 commit comments

Comments
 (0)