docs: add -s -- flags to curl|bash install to prevent stdin consumption#73830
docs: add -s -- flags to curl|bash install to prevent stdin consumption#73830unn-Known1 wants to merge 1 commit into
Conversation
…onsumption Fixes #73814 The install.sh script uses interactive prompts (via gum) that consume stdin. When the script is piped via 'curl | bash', stdin is occupied by the script stream, causing the prompts to consume portions of the script itself, resulting in truncated function names and indefinite hangs. Adding '-s --' flags prevents the bash subshell from allowing stdin to reach the script's interactive prompts. This is a standard pattern for piped installation scripts.
openclaw-barnacle
Bot
added
docs
|
Codex review: needs changes before merge. Summary Reproducibility: yes. for the PR's claimed fix: a focused pipe smoke shows a child process can still consume the next script line under both Real behavior proof Next step before merge Security Review findings
Review detailsBest possible solution: Harden installer subprocess stdin handling or document a tested caller pattern, then update all public install examples consistently with focused installer smoke coverage. Do we have a high-confidence way to reproduce the issue? Yes for the PR's claimed fix: a focused pipe smoke shows a child process can still consume the next script line under both Is this the best way to solve the issue? No. Full review comments:
Overall correctness: patch is incorrect Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 48b4e5b3614f. |
Greptile SummaryThis PR changes the install command from
Confidence Score: 3/5Not safe to merge — the documented fix does not solve the stated problem and introduces a misleading note. The sole change is a docs fix for a real bug (#73814), but the proposed fix ( docs/start/getting-started.md — both the command change and the new note need to be reconsidered. Prompt To Fix All With AIThis is a comment left during a code review.
Path: docs/start/getting-started.md
Line: 48-49
Comment:
**`-s --` does not prevent child-process stdin consumption**
`bash -s` tells bash to read commands from stdin, but this is already the default when no script file is passed — it is a no-op here. Crucially, child processes (like `gum`) inherit the parent's file descriptors unchanged, so they still read from the same pipe. The note on line 48 is therefore misleading: users who follow this command will still encounter the interactive-prompt hang described in the linked issue.
The correct caller-side fix is process substitution, which passes the script via a file descriptor rather than stdin, leaving stdin available for TTY prompts:
```bash
bash <(curl -fsSL https://openclaw.ai/install.sh)
```
Alternatively, the real fix belongs in `install.sh` itself — redirect `gum`'s stdin from `/dev/tty` (e.g. `gum choose < /dev/tty`) so the prompt is terminal-backed regardless of how the outer script is invoked.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "docs: add -s -- flags to curl|bash insta..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| The `-s --` flags prevent the installer from consuming stdin when piped via `curl | bash`. | ||
| Other install methods (Docker, Nix, npm): [Install](/install). |
There was a problem hiding this comment.
-s -- does not prevent child-process stdin consumption
bash -s tells bash to read commands from stdin, but this is already the default when no script file is passed — it is a no-op here. Crucially, child processes (like gum) inherit the parent's file descriptors unchanged, so they still read from the same pipe. The note on line 48 is therefore misleading: users who follow this command will still encounter the interactive-prompt hang described in the linked issue.
The correct caller-side fix is process substitution, which passes the script via a file descriptor rather than stdin, leaving stdin available for TTY prompts:
bash <(curl -fsSL https://openclaw.ai/install.sh)Alternatively, the real fix belongs in install.sh itself — redirect gum's stdin from /dev/tty (e.g. gum choose < /dev/tty) so the prompt is terminal-backed regardless of how the outer script is invoked.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/start/getting-started.md
Line: 48-49
Comment:
**`-s --` does not prevent child-process stdin consumption**
`bash -s` tells bash to read commands from stdin, but this is already the default when no script file is passed — it is a no-op here. Crucially, child processes (like `gum`) inherit the parent's file descriptors unchanged, so they still read from the same pipe. The note on line 48 is therefore misleading: users who follow this command will still encounter the interactive-prompt hang described in the linked issue.
The correct caller-side fix is process substitution, which passes the script via a file descriptor rather than stdin, leaving stdin available for TTY prompts:
```bash
bash <(curl -fsSL https://openclaw.ai/install.sh)
```
Alternatively, the real fix belongs in `install.sh` itself — redirect `gum`'s stdin from `/dev/tty` (e.g. `gum choose < /dev/tty`) so the prompt is terminal-backed regardless of how the outer script is invoked.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Closing in favor of a revised approach. The -s -- approach is insufficient per code review feedback — process substitution (bash <(curl -fsSL https://openclaw.ai/install.sh)) is the correct caller-side fix.
Greptile Review AcknowledgmentThank you for the P1/P2 feedback. This is noted for the next revision. Status: Merge conflict must be resolved first. Will address these comments after rebase. Automated acknowledgment |
2 participants
Summary
Fixes #73814
The install.sh script uses interactive prompts (via gum) that consume stdin. When the script is piped via curl | bash, stdin is occupied by the script stream, causing the prompts to consume portions of the script itself, resulting in truncated function names and indefinite hangs.
Solution
Added -s -- flags to the install command in the getting started documentation:
curl -fsSL https://openclaw.ai/install.sh | bash -s --The -s -- flags prevent the bash subshell from allowing stdin to reach the script interactive prompts. This is a standard pattern for piped installation scripts.
Testing
Disclosure
This contribution was developed with AI assistance.