Sebastien Tardif 1fa1507fc4 fix(install): harden stdin consumers to prevent pipe corruption in curl | bash (#87799) ...

* fix(install): harden stdin consumers to prevent pipe corruption in curl | bash

Redirect stdin from /dev/null for non-interactive subprocesses (npm install,
openclaw daemon restart, openclaw plugins update, openclaw dashboard) and
from /dev/tty for interactive ones (openclaw onboard in bootstrap). Also
protect the fallback paths in run_with_spinner and run_quiet_step.

This prevents subprocesses from consuming the script stream when the
installer is piped via curl | bash, which causes truncated function names
and hangs (reported in #73814).

Unlike the global pipe guard in #82918 (closed as too broad), this approach
has no detection heuristics and no re-execution. Each subprocess simply gets
the correct stdin for its purpose.

Fixes #73814

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>

* chore: retrigger proof evaluation

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>

* fix: redirect stdin in run_with_spinner raw-mode fallback

The success-path raw-mode fallback in run_with_spinner invoked the
command with inherited stdin. In a curl | bash scenario, this allowed
the child process to consume bytes from the script stream, causing
truncation. Add < /dev/null to match the other two fallback paths.

* retrigger proof check

* fix(test): update gum fallback assertion for stdin redirect

* fix: redirect gum-wrapped stdin and preserve TTY for interactive commands

Address ClawSweeper P1 and P2 findings:

- P1: Redirect stdin from /dev/null on the normal gum spin path so child
  commands cannot consume the piped script stream (gum v0.17.0 passes
  os.Stdin to wrapped commands).

- P2: Use is_non_interactive_shell to conditionally redirect stdin in
  fallback paths. When running interactively (bash install.sh), commands
  that need user input (e.g. Homebrew prompts) keep terminal stdin.
  When piped (curl | bash), stdin is redirected from /dev/null.

- P2: Revert labeler.yml changes to keep the PR focused on installer
  stdin safety. Size-label best-effort handling belongs in a separate PR.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>

* chore: retrigger proof evaluation

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>

* retrigger proof check

* fix: base stdin isolation on stdin TTY check, not stdout

Replace is_non_interactive_shell with needs_stdin_isolation for stdin
redirection decisions. The new function checks stdin directly (! -t 0)
and NO_PROMPT, without checking stdout (-t 1). This ensures that
stdout redirection (e.g. install.sh > log.txt) does not suppress
interactive prompts when stdin is a terminal.

* test(install): add stdin isolation and NO_PROMPT coverage

Three focused tests for the needs_stdin_isolation function:

1. Verify needs_stdin_isolation returns true when stdin is piped
   (the core curl|bash scenario).
2. Verify needs_stdin_isolation returns true when NO_PROMPT=1 is set
   (explicit non-interactive override).
3. Verify run_quiet_step redirects subprocess stdin to /dev/null
   when running in a piped context, preventing script consumption.

* test(install): strengthen stdin isolation test with sentinel data

Replace the weak TTY check (which passes even without the fix since
the test pipe is also non-TTY) with a sentinel-based test: pipe
SENTINEL_DATA on stdin and verify the child process reads nothing,
proving run_quiet_step actually redirects stdin to /dev/null.

* test(install): add counterproof and cat-based stdin isolation tests

Add two new tests to prove the stdin isolation fix is necessary and
works correctly:

- counterproof: demonstrates that pipe data DOES leak to the child
  when stdin is not redirected through run_quiet_step, proving the
  /dev/null redirect is the isolation barrier
- cat-based test: uses cat (reads all of stdin) instead of read -t 1
  (timeout-based) for a deterministic assertion that run_quiet_step
  produces empty stdin

* fix: gate gum spin stdin redirect on needs_stdin_isolation

The gum spin command unconditionally redirected stdin from /dev/null,
which also killed interactive prompts for direct installs since gum
v0.17 passes os.Stdin to the wrapped command. Now only redirect
stdin when needs_stdin_isolation returns true (piped install context).

Adds focused tests verifying both paths: piped installs get /dev/null
redirect, direct interactive installs preserve terminal stdin.

* test: assert gum stdin is not /dev/null for direct interactive installs

The direct gum runtime test now reads the child command's stdin-source
log file and asserts stdin was NOT /dev/null, using device:inode
comparison (stat -f on macOS, stat -c on Linux) for reliable detection
across both platforms.

* retrigger proof check

* retrigger proof check with real installer evidence

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>

* fix(install): preserve interactive post-install stdin

* fix(install): route piped prompts through controlling tty

* fix(install): keep quiet piped steps noninteractive

* fix(install): avoid hidden prompts with redirected output

* fix(install): preserve visible prompt output state

---------

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Co-authored-by: Vincent Koc <25068+vincentkoc@users.noreply.github.com>

2026-07-11 14:00:53 +08:00

109 KiB

Executable File

Raw Blame History

View Raw