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;

jamubc added a commit that referenced this pull request Jun 11, 2026
- Route inlined @file/changeMode prompts via stdin on the agy backend,
  mirroring the gemini path (#27, #77) — passing whole inlined files as
  an argv element exceeded OS command-line limits (E2BIG).
- Tolerate non-zero exits from `agy -p`: descend the recovery ladder
  (PTY, transcript) instead of aborting; only ENOENT aborts immediately.
- Bound executeCommand with a 20-minute default timeout so a hung CLI
  cannot permanently wedge the serialized agy queue or the server.
- Backport the symlink-aware realpath re-check into
  assertSafeFileReferences, closing the CVE-2026-0755 gap on the gemini
  path (an in-root symlink to an outside target was still inlined).
- Probe %LOCALAPPDATA%\Antigravity on Windows before giving up on agy
  resolution, matching the guidance the ENOENT message already gives.
- Deduplicate changeMode preprocessing into prepareChangeModePrompt so
  the gemini and agy prompt bodies cannot drift; fold the repeated
  stdout/PTY parse block into one replyFrom helper.

Tests: 94 passing (timeout behaviour, symlink guard, file:->@ rewrite).
jamubc added a commit that referenced this pull request Jun 18, 2026
- Route inlined @file/changeMode prompts via stdin on the agy backend,
  mirroring the gemini path (#27, #77) — passing whole inlined files as
  an argv element exceeded OS command-line limits (E2BIG).
- Tolerate non-zero exits from `agy -p`: descend the recovery ladder
  (PTY, transcript) instead of aborting; only ENOENT aborts immediately.
- Bound executeCommand with a 20-minute default timeout so a hung CLI
  cannot permanently wedge the serialized agy queue or the server.
- Backport the symlink-aware realpath re-check into
  assertSafeFileReferences, closing the CVE-2026-0755 gap on the gemini
  path (an in-root symlink to an outside target was still inlined).
- Probe %LOCALAPPDATA%\Antigravity on Windows before giving up on agy
  resolution, matching the guidance the ENOENT message already gives.
- Deduplicate changeMode preprocessing into prepareChangeModePrompt so
  the gemini and agy prompt bodies cannot drift; fold the repeated
  stdout/PTY parse block into one replyFrom helper.

Tests: 94 passing (timeout behaviour, symlink guard, file:->@ rewrite).
jamubc added a commit that referenced this pull request Jun 18, 2026
* docs: deep-dive migration analysis for Gemini CLI -> Antigravity CLI (agy)

Maps the concrete divergences between the gemini and agy CLIs that affect a
non-interactive MCP caller (empty -p stdout, Flash-only print mode, @file
inlining, sandbox/approval semantics, sessions/concurrency, packaging/auth)
and proposes phased solutions building on the pluggable backend from #78.

Preliminary start to the migration ahead of the 2026-06-18 Gemini CLI
retirement. Refs discussion #90.

* feat(backends): pluggable gemini/agy backend seam with agy migration phases

Implements the migration mapped in docs/migration/antigravity-cli.md so the
tool keeps working past the 2026-06-18 Gemini CLI retirement.

- Backend seam (src/backends/): Backend interface + getBackend()/runWithBackend(),
  selected via GEMINI_MCP_BACKEND; gemini stays the default. ask-gemini and
  brainstorm now route through it. (Phase 0)
- agy backend: capability-gated model selection (Flash-only; never passes --model,
  skips the now-meaningless Pro->Flash fallback) with a user-facing notice;
  self-inlines @file references so determinism and the CVE-2026-0755 guard survive;
  truthful sandbox notice since -p does not isolate tool execution. (Phase 1)
- Transcript recovery (agyTranscript.ts): works around the empty agy -p stdout bug
  by reading JSONL, with a SQLite fallback behind one interface and start-time-bounded
  discovery so stale replies are never returned. (Phase 2)
- Prefers stdout when non-empty so the fallback self-retires once agy fixes -p;
  DEFAULT_BACKEND is a one-line flip for the eventual cutover. (Phase 3/4)
- Backend-aware executable resolution (AGY_CLI_PATH) and ENOENT guidance.
- README + migration doc document GEMINI_MCP_BACKEND / AGY_CLI_PATH.
- 19 new unit tests (76 total) for selection, capability gating, arg/prompt
  building, and transcript extraction.

Refs #90.

* review: harden agy backend and fix public-facing docs

Full review pass over the migration work.

Hardening:
- @file inlining now does a realpathSync symlink re-check so an in-root symlink
  pointing outside the project root is refused, not inlined (defense-in-depth on
  the CVE-2026-0755 guard); scoped to the inlining path. New test covers it.
- agy transcript reader wraps the JSONL read in try/catch and falls through to the
  SQLite reader; SQLite lookup probes both the logs dir and conversations/<id>.db.

Docs / public-facing:
- wire the migration page into the VitePress sidebar + nav (was undiscoverable)
- remove a stray tag accidentally left at the end of the doc
- qualify the 'empty -p stdout' claim to non-TTY/headless/Windows contexts
- standardize 'Gemini 3.5 Flash' wording; fix en-dashes; collapse a wrapped code span
- note backend-dependent model availability in models.md and add a retirement FAQ entry

Build clean, lint clean, 77 tests passing.

* feat(backends): complete agy migration phases 3 and 4

Phase 3 — converge on stdout, self-retire the transcript scrape:
- agyCapabilities.ts probes `agy --help` once per process (fail-safe, 4s
  timeout) so the backend adapts to whatever agy build is installed instead of
  hardcoding 1.0.x assumptions.
- agyOutput.ts adds a clean JSON-stdout reader (parseAgyJsonResponse) used when
  the build advertises --output-format json, plus an opt-in POSIX PTY recovery
  path (runAgyUnderPty via script(1), AGY_MCP_PTY=1) that coaxes a TTY-only build
  into printing real stdout without reading any private files. Args are
  POSIX-quoted, preserving the non-PTY path's injection safety.
- agyBackend.run now escalates best->last-resort: JSON stdout -> plain stdout ->
  opt-in PTY -> transcript. As agy fixes print-mode the probe shifts us up the
  ladder with no code change.

Phase 4 — date-aware cutover:
- resolveDefaultBackend() returns gemini until 2026-06-18 and agy thereafter
  (gemini is retired, so agy is the only live option); GEMINI_MCP_BACKEND always
  overrides. backendSelection() surfaces the post-retirement auto-switch notice
  and a once-per-process nudge in the final countdown.

Docs updated (phase statuses, config table, AGY_MCP_PTY). 90 tests passing,
build and lint clean.

* fix(backends): harden agy execution paths after full review

- Route inlined @file/changeMode prompts via stdin on the agy backend,
  mirroring the gemini path (#27, #77) — passing whole inlined files as
  an argv element exceeded OS command-line limits (E2BIG).
- Tolerate non-zero exits from `agy -p`: descend the recovery ladder
  (PTY, transcript) instead of aborting; only ENOENT aborts immediately.
- Bound executeCommand with a 20-minute default timeout so a hung CLI
  cannot permanently wedge the serialized agy queue or the server.
- Backport the symlink-aware realpath re-check into
  assertSafeFileReferences, closing the CVE-2026-0755 gap on the gemini
  path (an in-root symlink to an outside target was still inlined).
- Probe %LOCALAPPDATA%\Antigravity on Windows before giving up on agy
  resolution, matching the guidance the ENOENT message already gives.
- Deduplicate changeMode preprocessing into prepareChangeModePrompt so
  the gemini and agy prompt bodies cannot drift; fold the repeated
  stdout/PTY parse block into one replyFrom helper.

Tests: 94 passing (timeout behaviour, symlink guard, file:->@ rewrite).

* feat(agy): configurable timeout and stale-reply guard

Add GEMINI_MCP_TIMEOUT (minutes, default 45) and derive agy's --print-timeout from it, so long agent runs are not capped at agy's 5m default. Guard transcript recovery so a fast agy failure never returns a stale reply from a previous conversation.

* chore(deps): bump dompurify to 3.4.11 (#101)

Raise the override floor and lockfile to 3.4.11, closing the SAFE_FOR_TEMPLATES bypass advisory.

* docs: add antigravity (agy) badge and 1.1.8 changelog

* fix(agy): surface agy's real error instead of a silent empty reply

executeCommand dropped stderr on a clean exit, so an agy quota or auth failure (exit 0, empty stdout, message on stderr) reached the caller as an empty reply plus a misleading generic hint. Reject with the stderr text instead, and drop the incorrect agy -i suggestion.

* docs: refresh migration status and correct the agy output description

Mark the migration active for 1.1.8 with the 2026-06-18 cutover, document GEMINI_MCP_TIMEOUT and AGY_PRINT_TIMEOUT, and fix the claim that replies come from transcript files (stdout is the primary path).

* fix: address review findings on PTY cleanup, @file inlining, and sqlite parsing

Kill the PTY's whole process group on timeout so script's sh and agy children are not orphaned. Narrow @file inlining to whitespace-preceded tokens so emails are not mangled, while keeping the CVE-2026-0755 guard broad. Skip non-JSON SQLite cells before JSON.parse to avoid thrown-exception overhead.
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