Skip to content

Mitigate Windows GH CLI command injection #1874

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
juliusmarminge wants to merge 3 commits into
base: main
Choose a base branch
from
+3 −1
Open

Mitigate Windows GH CLI command injection #1874

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc5871a
Mitigate Windows GH CLI command injection
juliusmarminge Apr 10, 2026
1fd6244
Merge branch 'main' into codex/fix-github-cli-command-injection-vulne…
juliusmarminge Apr 10, 2026
ce1b679
Disable shell by default in process runner
juliusmarminge Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/server/src/git/Layers/GitHubCli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ const makeGitHubCli = Effect.sync(() => {
runProcess("gh", input.args, {
cwd: input.cwd,
timeoutMs: input.timeoutMs ?? DEFAULT_TIMEOUT_MS,
shell: false,
Copy link
Copy Markdown
Contributor

@cursor cursor bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shell:false breaks gh .cmd shims on Windows

Medium Severity

Setting shell: false for gh invocations on Windows prevents Node.js spawn from executing .cmd/.bat shims. If gh is installed via a package manager that creates .cmd shims (rather than placing gh.exe directly on PATH), the spawn call will fail with ENOENT. The original shell: true on Windows existed precisely to handle this — the openai/codex repo itself has issue #16337 about this exact pattern. While most gh installations use .exe, this is a regression for some Windows setups.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1fd6244. Configure here.

}),
catch: (error) => normalizeGitHubCliError("execute", error),
});
Expand Down
3 changes: 2 additions & 1 deletion apps/server/src/processRunner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export interface ProcessRunOptions {
allowNonZeroExit?: boolean | undefined;
maxBufferBytes?: number | undefined;
outputMode?: "error" | "truncate" | undefined;
shell?: boolean | undefined;
}

export interface ProcessRunResult {
Expand Down Expand Up @@ -139,7 +140,7 @@ export async function runProcess(
cwd: options.cwd,
env: options.env,
stdio: "pipe",
shell: process.platform === "win32",
shell: options.shell ?? process.platform === "win32",
Copy link
Copy Markdown
Contributor

@cursor cursor bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing parentheses in nullish coalescing expression

Low Severity

The expression options.shell ?? process.platform === "win32" relies on === having higher precedence than ?? to evaluate correctly. While functionally equivalent to options.shell ?? (process.platform === "win32"), the PR description itself explicitly includes the parentheses. Adding them would make the intent unambiguous, especially since this is a security-sensitive change preventing command injection. Developers unfamiliar with ?? precedence may misread this as (options.shell ?? process.platform) === "win32".

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ce1b679. Configure here.

});

let stdout = "";
Expand Down
Loading