Skip to content

[codex] Avoid shell for system executables#2950

Merged
juliusmarminge merged 1 commit into
mainfrom
codex/remove-shell-for-system-executables
Jun 4, 2026
Merged

[codex] Avoid shell for system executables#2950
juliusmarminge merged 1 commit into
mainfrom
codex/remove-shell-for-system-executables

Conversation

@juliusmarminge
Copy link
Copy Markdown
Member

@juliusmarminge juliusmarminge commented Jun 4, 2026

Summary

Removes shell: process.platform === "win32" from direct system executable launches where the command is not a package-manager or user-installed .cmd shim.

Details

  • Runs Git repository identity probes without shell mode.
  • Runs process diagnostics and terminal subprocess PowerShell probes directly via powershell.exe argv.
  • Uses ssh.exe and tailscale.exe on Windows so those executable launches can avoid cmd.exe while still resolving the Windows binary name.
  • Leaves package-manager/provider/editor command paths untouched because those may legitimately be .cmd shims.

Validation

  • vp check passed. It reported existing react(no-unstable-nested-components) warnings outside this change.
  • vp run typecheck passed.

Note

Avoid shell when spawning system executables for SSH, Tailscale, and git commands

  • Removes the shell: true option from all ChildProcess.make and ProcessRunner.run calls across the server and packages, so processes are spawned directly rather than via a shell.
  • Introduces SSH_COMMAND in packages/ssh/src/command.ts and TAILSCALE_COMMAND in packages/tailscale/src/tailscale.ts, each selecting the .exe variant on Windows and the plain name elsewhere.
  • Updates SSH tunnel, Tailscale status/command runners, git identity resolution, diagnostics, and environment label commands to use the platform-specific executable names.
  • Behavioral Change: on Windows, these processes no longer run through cmd.exe; the executable must be on PATH directly.

Macroscope summarized 1fbde6e.

Related issues

Refs #2537. This removes the SSH/Tailscale shell-wrapped executable launches called out there, but does not close the issue because OpenCode/provider process cleanup and shim-resolution work remains.


Note

Medium Risk
Changes Windows process launch semantics for SSH, Tailscale, git, and diagnostics; failures are possible if executables are not on PATH without cmd.exe resolution.

Overview
Stops wrapping known system executables in a Windows shell when spawning child processes. shell: process.platform === "win32" is removed from diagnostics (ps / powershell.exe), ProcessRunner git and host-label probes, terminal subprocess inspection, and SSH/Tailscale ChildProcess spawns.

SSH and Tailscale now use platform constants (SSH_COMMAND, TAILSCALE_COMMAND) so Windows launches ssh.exe / tailscale.exe directly while Unix keeps ssh / tailscale. Package-manager or .cmd shim paths are intentionally unchanged per the PR description.

On Windows, these calls no longer go through cmd.exe; the binary must be resolvable on PATH without shell indirection.

Reviewed by Cursor Bugbot for commit 1fbde6e. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8db7cbd5-92ab-4bca-b3ab-91e31bc824ab

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/remove-shell-for-system-executables

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Jun 4, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

🚀 Expo continuous deployment is ready!

  • Project → t3-code
  • Platforms → android, ios
  • Scheme → t3code-preview
  🤖 Android 🍎 iOS
Fingerprint 251694208975e82c56bd5289d014d27db782d79c 88f9ec34318a3522ac20299028ec60ca0080a665
Build Details Build Permalink
DetailsDistribution: INTERNAL
Build profile: preview:dev
Runtime version: 251694208975e82c56bd5289d014d27db782d79c
App version: 0.1.0
Git commit: fff460b4abc01f29f5b2cdb5bf25d955066816ee
Build Permalink
DetailsDistribution: INTERNAL
Build profile: preview:dev
Runtime version: 88f9ec34318a3522ac20299028ec60ca0080a665
App version: 0.1.0
Git commit: fff460b4abc01f29f5b2cdb5bf25d955066816ee
Update Details Update Permalink
DetailsBranch: pr-2950
Runtime version: 251694208975e82c56bd5289d014d27db782d79c
Git commit: da86e18e58a07a6139ccab9e41888445367c86f6
Update Permalink
DetailsBranch: pr-2950
Runtime version: 88f9ec34318a3522ac20299028ec60ca0080a665
Git commit: da86e18e58a07a6139ccab9e41888445367c86f6
Update QR

@juliusmarminge juliusmarminge marked this pull request as ready for review June 4, 2026 22:20
@macroscopeapp
Copy link
Copy Markdown
Contributor

macroscopeapp Bot commented Jun 4, 2026

Approvability

Verdict: Approved

Mechanical refactor that removes shell intermediation on Windows by using explicit .exe suffixes for system executables (ssh, tailscale). The change is consistent, low-risk, and maintains equivalent behavior.

You can customize Macroscope's approvability policy. Learn more.

@juliusmarminge juliusmarminge merged commit 300f7fd into main Jun 4, 2026
21 checks passed
@juliusmarminge juliusmarminge deleted the codex/remove-shell-for-system-executables branch June 4, 2026 22:25
J-Giggles pushed a commit to J-Giggles/t3code that referenced this pull request Jun 5, 2026
Co-authored-by: Julius Marminge <julius@mac.lan>
(cherry picked from commit 300f7fd)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant