Skip to content

feat(windows): stdin prompt passing + windowsHide (harvested from #27)#77

Open
jamubc wants to merge 1 commit into
security/cve-2026-0755from
windows/stdin-windowshide
Open

feat(windows): stdin prompt passing + windowsHide (harvested from #27)#77
jamubc wants to merge 1 commit into
security/cve-2026-0755from
windows/stdin-windowshide

Conversation

@jamubc

@jamubc jamubc commented May 30, 2026

Copy link
Copy Markdown
Owner

Summary

Harvests the still-valuable, already-tested Windows fixes from #27 (main-windows-patch) onto the current security branch, so they land instead of sitting in a long-running branch. Authored here as small, focused changes on top of the CVE-2026-0755 fix.

Stacked on security/cve-2026-0755 (#76) so it keeps the security fixes and doesn't reintroduce the broken -p quoting. GitHub will auto-retarget this to main once #76 merges; review/merge #76 first.

What it does

  • stdin prompt passingchangeMode and @file prompts are sent to the Gemini CLI on stdin instead of the -p flag (useStdin = changeMode || prompt.includes('@')). This:
    • sidesteps cmd.exe argument parsing on Windows entirely (the prompt never becomes a shell token), and
    • avoids the OS command-line length limit, which large @file/changeMode prompts can exceed.
    • assertSafeFileReferences() still runs first, so @file containment applies to the stdin path too.
  • windowsHide — suppresses the popup console window when spawning on Windows.
  • Simple prompts still use -p, passed verbatim (Windows quoting handled by commandExecutor's quoteForCmd).

Provenance

This is the tested approach from #27 — the stdin routing and windowsHide were verified there on Windows. Reconciled with the security model (no \"${prompt}\" wrapping; containment preserved). #27 will be closed pointing here.

Test plan

  • npm run build (tsc) passes.
  • Windows smoke test (was previously verified in Main windows patch #27): changeMode + @file prompts via stdin, simple prompt via -p, no popup console window.

Harvested from the tested Windows work in #27. changeMode and @file prompts are
routed to the Gemini CLI on stdin instead of -p, which sidesteps cmd.exe argument
parsing on Windows and avoids the OS command-line length limit on large prompts;
@file containment still runs first. windowsHide suppresses the popup console
window on Windows. Builds on the CVE-2026-0755 fix, so no broken quoting is
reintroduced on the -p path.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request enhances Windows robustness by routing complex prompts (such as those with '@' or 'changeMode') to the Gemini CLI via stdin instead of the -p command-line flag, avoiding cmd.exe parsing issues and command-line length limits. It also adds windowsHide: true to suppress popup console windows on Windows. The review feedback suggests two key improvements: registering an error handler on childProcess.stdin to prevent unhandled EPIPE or EINVAL exceptions from crashing the Node.js process, and routing prompts via stdin if they exceed a length threshold (e.g., 4000 characters) to further safeguard against Windows command-line length limits.

Comment on lines +43 to +46
if (stdinData !== undefined && childProcess.stdin) {
childProcess.stdin.write(stdinData);
childProcess.stdin.end();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

When writing to childProcess.stdin, if the child process exits or crashes immediately, writing to or ending the stream can throw an EPIPE or EINVAL error. If there is no 'error' event listener registered on childProcess.stdin, this error will propagate as an unhandled exception and crash the entire Node.js process. Registering a no-op error handler on childProcess.stdin prevents this crash.

Suggested change
if (stdinData !== undefined && childProcess.stdin) {
childProcess.stdin.write(stdinData);
childProcess.stdin.end();
}
if (stdinData !== undefined && childProcess.stdin) {
childProcess.stdin.on("error", () => {});
childProcess.stdin.write(stdinData);
childProcess.stdin.end();
}

// parsing on Windows. Simple prompts use -p verbatim (commandExecutor handles
// Windows quoting); no manual quoting — that only injects literal quote
// characters and corrupts @file references (#66, CVE-2026-0755).
const useStdin = !!changeMode || prompt_processed.includes('@');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To prevent command-line length limit issues on Windows (which has an 8191-character limit for cmd.exe), we should also route the prompt via stdin if the prompt length exceeds a safe threshold (e.g., 4000 characters), even if it doesn't contain @ or changeMode.

Suggested change
const useStdin = !!changeMode || prompt_processed.includes('@');
const useStdin = !!changeMode || prompt_processed.includes('@') || prompt_processed.length > 4000;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant