fix(windows): fail closed for shell-backed stdio launches

[mohanagy]

Closes #217

Summary

• Resolve Windows stdio commands to direct .exe or .com paths before the MCP SDK.
• Reject command shells, .cmd/.bat shims, shebang-reinterpreted files, and npx-backed Windows presets rather than allowing a cmd.exe fallback.
• Use the same effective case-insensitive Windows environment for doctor readiness and runtime launch resolution.
• Document the supported Windows paths and explicit preset requirement.

Validation

• npm test (1,497 passed; 27 skipped)
• npm run test:core (410 passed; 22 skipped)
• npm run test:coverage (1,497 passed; 27 skipped; thresholds passed)
• npm run lint, npm run typecheck, npm run build
• npm run smoke:cli, npm run check:pack, npm run test:package (23 passed)

Summary by CodeRabbit

• New Features

  • Added safer Windows STDIO launching using only direct .exe or .com executables.
  • Added clear rejection of command shims, shell executables, and npx-based presets on Windows.
  • Windows setup now requires an explicit supported preset.

• Documentation

  • Updated Windows compatibility, setup, security, preset, and changelog guidance.
  • Documented direct-executable and remote MCP alternatives for Windows.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 22 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: eff4fc66-8c0d-451e-91a4-38579f0ad132

📥 Commits

Reviewing files that changed from the base of the PR and between 606857f and 3083532.

📒 Files selected for processing (8)

• src/cli/doctor.ts
• src/config/presets.ts
• src/upstream/upstream-process-manager.ts
• tests/config-runtime-parity.test.ts
• tests/doctor.test.ts
• tests/package-contract.test.ts
• tests/preset-catalog.test.ts
• tests/presets.test.ts

📝 Walkthrough

Walkthrough

Windows STDIO launches now resolve only direct .exe or .com executables before spawning. Shell executables, command shims, and Windows npx-backed presets are rejected, with platform-aware init, onboarding, doctor checks, tests, and documentation updated accordingly.

Changes

Windows STDIO launch hardening

Layer / File(s)	Summary
Direct executable resolution policy src/secrets/executable-resolver.ts, src/upstream/windows-stdio-command.ts, tests/executable-resolver.test.ts, tests/windows-stdio-command.test.ts	Resolves Windows commands only to executable .exe/.com paths and rejects shims, shells, relative paths, and shebang-based files.
Runtime and doctor integration src/upstream/upstream-process-manager.ts, src/cli/doctor.ts, tests/upstream-manager.test.ts, tests/doctor.test.ts	Applies merged environments and validated Windows command resolution before STDIO transport startup and executable readiness reporting.
Preset and init platform policy src/config/presets.ts, src/cli/init.ts, tests/preset-catalog.test.ts, tests/init-command.test.ts, tests/setup-command.test.ts	Rejects Windows npx-backed presets and requires explicit Windows init presets while retaining non-Windows generic defaults.
Import and onboarding boundaries src/setup/client-entry-import.ts, src/console/console-application-service.ts, tests/client-entry-import.test.ts, tests/console-*	Uses safe imported configurations, reports Windows preset rejection, and adapts connector tests to platform-supported presets.
Documentation and behavior contracts README.md, docs/*.md, CHANGELOG.md, tests/windows-stdio-docs-contract.test.ts	Documents direct-executable requirements, Windows preset restrictions, and refusal to invoke cmd.exe; contract tests verify the wording.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant InitOrOnboarding
  participant buildPresetConfig
  participant UpstreamProcessManager
  participant resolveWindowsStdioCommand
  participant StdioClientTransport
  InitOrOnboarding->>buildPresetConfig: build selected preset
  buildPresetConfig-->>InitOrOnboarding: config or Windows preset rejection
  UpstreamProcessManager->>resolveWindowsStdioCommand: resolve configured STDIO command
  resolveWindowsStdioCommand-->>UpstreamProcessManager: direct executable or startup failure
  UpstreamProcessManager->>StdioClientTransport: start with resolved command and args

Loading

Possibly related PRs

• mohanagy/miftah#215: Adds the client-entry import flow refined here with stricter Windows executable handling.
• mohanagy/miftah#218: Adds related Windows direct .exe/.com restrictions for local STDIO presets.
• mohanagy/miftah#51: Also modifies UpstreamProcessManager, the runtime layer updated here for Windows STDIO resolution.

Poem

A rabbit hops where safe commands may run,
No shell-shim trick can spoil the fun.
.exe and .com lead the way,
While npx waits for a different day.
Direct paths thump with a sturdy beat—
Windows launches now stay neat!

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
Docstring Coverage	❌ Error	Docstring coverage is 21.74% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately reflects the main Windows fail-closed stdio launch change.
Description check	✅ Passed	The description covers the summary and validation sections well, but the Security impact section from the template is missing.
Linked Issues check	✅ Passed	The PR implements the required Windows fail-closed launch policy, rejects shell/shim fallbacks, adds regression coverage, and updates the affected docs for #217.
Out of Scope Changes check	✅ Passed	The changes appear scoped to Windows stdio launch policy, related tests, and documentation, with no clear unrelated additions.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/217-windows-direct-stdio

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cli/doctor.ts`:
- Around line 132-135: Update environmentForTarget to obtain and merge the same
runtime-resolved environment used when launching the executable, rather than
merging raw upstream and profile env maps. Reuse the existing
environment-resolution path and ensure executable preflight sees expanded
values, including referenced Path entries; add a regression covering an expanded
Windows Path.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b3df4c1e-3ea9-49c6-9f46-1c735367514e

📥 Commits

Reviewing files that changed from the base of the PR and between 645bce0 and 606857f.

📒 Files selected for processing (25)

• CHANGELOG.md
• README.md
• docs/cli.md
• docs/presets-and-clients.md
• docs/security.md
• src/cli/doctor.ts
• src/cli/init.ts
• src/config/presets.ts
• src/console/console-application-service.ts
• src/secrets/executable-resolver.ts
• src/setup/client-entry-import.ts
• src/upstream/upstream-process-manager.ts
• src/upstream/windows-stdio-command.ts
• tests/client-entry-import.test.ts
• tests/console-application-service.test.ts
• tests/console-dashboard-application-service.test.ts
• tests/console-server.test.ts
• tests/doctor.test.ts
• tests/executable-resolver.test.ts
• tests/init-command.test.ts
• tests/preset-catalog.test.ts
• tests/setup-command.test.ts
• tests/upstream-manager.test.ts
• tests/windows-stdio-command.test.ts
• tests/windows-stdio-docs-contract.test.ts

Dismissed: CodeRabbit is rate-limited. Its sole inline thread is resolved and outdated on the current head; the correction is covered by focused tests and independent security review.