Fix script generation defects found in review - #1
Merged
Conversation
Correctness: - mac: Codex telemetry wrote config.toml with >> (append), duplicating the whole config and stacking [otel] tables on re-run; use > (replace). - mac: Homebrew installer hung on the RETURN prompt (output redirected to log) or aborted in the curl|bash flow; set NONINTERACTIVE=1, keep output on the terminal, and reconnect run-mac.sh stdin to /dev/tty. - mac: Python was a no-op because /usr/bin/python3 (CLT stub) satisfied the has-check; add install_python that checks Homebrew's python3 path. - win: $script:Failed (Global) vs the scriptblock-local $Failed meant the summary never saw failures and always exited 0; use $script:Failed both places. - win: Codex CLI native installer's $env:CODEX_NON_INTERACTIVE was interpolated away before the child ran; set it in the parent scope. - Windows one-liner self-interpolated $env:DEV_SETUP_SCRIPT_B64 when pasted into PowerShell; drop $ via [Environment]::SetEnvironmentVariable. - git identity: only the missing field is written (was overwriting an existing name/email when just one was set). Security: - chmod 600 the mac telemetry file (may hold the OTLP header secret). - Keep the OTLP header value out of the shareable URL. - Polyglot BAT: a setting containing the marker could hijack PS extraction via LastIndexOf; split the header marker and switch to IndexOf. - Strip CR/LF from telemetry values (TOML/script-line breakout). Known limitation: unchecking telemetry and re-running does not remove previously written config (mac file / Windows env vars).
- run-mac.sh: [ -e /dev/tty ] only tests existence; /dev/tty exists even
without a controlling terminal (CI, cron, ssh without -t) where opening it
fails with ENXIO. Under set -euo pipefail the failed redirect aborted the
runner before the fallback. Probe openability instead:
{ : < /dev/tty; } 2>/dev/null. Caught independently by Codex and a
subagent review, both reproduced the abort in a no-tty context.
- Add regression assertions for every fix (mac/windows/public-runner suites):
codex telemetry replace-not-append, Homebrew NONINTERACTIVE, install_python
vs the CLT stub, telemetry chmod 600, git per-field guards, CR/LF strip,
$script:Failed scoping, Codex env in parent scope, split polyglot marker +
IndexOf hijack-resistance, and the /dev/tty openability probe.
- .gitignore: .DS_Store and prompts/ (untracked local prompt logs that hold
raw prompt history and local paths; must not leak into the public repo).
There was a problem hiding this comment.
Pull request overview
This PR fixes multiple verified defects in the generated installer scripts (macOS bash + Windows PowerShell/CMD), focusing on correctness, injection resistance, and UX reliability, with added regression tests to lock in the fixes.
Changes:
- Fix macOS script generation issues: Homebrew installer non-interactive behavior, reliable Python installation detection/installation, Codex telemetry config overwrite (not append), safer Git identity defaults, and tighter telemetry file permissions.
- Fix Windows script generation issues: reliable failure tracking, Codex CLI env var propagation, safer polyglot marker extraction (IndexOf + split marker), and safer one-liner command construction.
- Add regression tests for the above fixes and adjust the macOS public runner to reconnect stdin to
/dev/ttywhen available.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/builder.js |
Core fixes to generated macOS/Windows scripts (telemetry config write mode, Homebrew behavior, Python install, marker extraction, failure tracking, git defaults). |
src/App.jsx |
Prevent OTLP header secrets from being encoded into share URLs; harden Windows one-liner against $env: pre-expansion on paste. |
public/run-mac.sh |
Reconnect stdin to /dev/tty when possible to support interactive installers when invoked via `curl |
tests/mac.test.js |
Adds regression assertions for macOS script defect fixes (telemetry overwrite, brew noninteractive, Python install, chmod 600, CR/LF stripping, git defaults). |
tests/windows.test.js |
Adds regression assertions for Windows script defect fixes (script-scoped failure tracking, env propagation, marker extraction hardening, git defaults). |
tests/public-runners.test.js |
Adds regression assertions validating the /dev/tty redirection strategy and avoiding fragile existence checks. |
.gitignore |
Ignores .DS_Store and prompts/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
요약
생성 스크립트를 5개 관점(bash/mac, PowerShell/win, 보안, 인젝션, 전달·UX)으로 리뷰하고, 각 발견을 적대적으로 재검증한 뒤 확정된 결함을 수정했습니다. PowerShell/cmd 결함은 macOS에서 실행 불가라 Codex로 시맨틱 교차검증했습니다(3건 모두 REAL 확정).
수정한 결함
정확성
~/.codex/config.toml손상 —>>(append)로 써서 기존 사용자 설정 전체가 복제되고, 재실행마다[otel]테이블이 중첩돼 TOML이 깨졌습니다.>(replace)로 수정. (Windows 대응부는 이미 파일 교체 방식)curl | bash흐름에서 stdin이 TTY가 아니라 sudo가 실패했습니다.NONINTERACTIVE=1+ 출력을 터미널에 유지 +run-mac.shstdin을/dev/tty로 재연결./usr/bin/python3(CLT 스텁)가has python3를 통과해 brew Python이 설치 안 됐습니다. Homebrew python3 경로를 확인하는install_python로 교체.$script:Failed(→Global)와 스크립트블록-로컬$Failed불일치로 실패가 요약에 안 잡히고 항상exit 0. 양쪽을$script:Failed로 통일.$env:CODEX_NON_INTERACTIVE=1이 child 실행 전 보간돼 파싱 오류로 조용히 fallback. 부모 스코프에서 env 설정.$env:DEV_SETUP_SCRIPT_B64가 먼저 보간돼 깨졌습니다. 아우터 명령에서$를 제거([Environment]::SetEnvironmentVariable).보안
chmod 600추가.#__PS_SCRIPT_BELOW__가 들어가면LastIndexOf추출을 하이재킹할 수 있었습니다. 헤더 마커를 분할하고IndexOf로 전환.알려진 한계 (미수정)
검증
pnpm test/pnpm build/pnpm test:e2e모두 통과.[otel]1개,model보존, TOML VALID.gitName에 마커 주입 -> 헤더에 연속 마커 0개, 실제 구분자가 첫 매치.