Skip to content

fix(scripts): eliminate stdout pollution in run.cmd polyglot#145

Merged
aeneasr merged 1 commit intoory:mainfrom
hypn4:fix/run-cmd-stdout-pollution
May 5, 2026
Merged

fix(scripts): eliminate stdout pollution in run.cmd polyglot#145
aeneasr merged 1 commit intoory:mainfrom
hypn4:fix/run-cmd-stdout-pollution

Conversation

@hypn4
Copy link
Copy Markdown
Contributor

@hypn4 hypn4 commented May 3, 2026

Summary

scripts/run.cmd is a sh/cmd polyglot whose first two lines (#!/bin/sh
and # 2>NUL & @goto batch) were echoed to stdout by cmd.exe before
@echo off took effect, prepending ~92 bytes of prompt-prefixed text to
the lumen binary's output. This broke MCP stdio JSON-RPC framing for any
MCP host that spawns run.cmd directly via the .cmd file-extension
association — including Claude Code on native Windows.

Replace the polyglot prologue with a label-based pattern that cmd.exe
does not echo, and add a regression test that catches stdout pollution
from a fresh-shell launch.

Refs: #121.

The bug

Affected environment

Any MCP host that spawns run.cmd on Windows. Reproduced on:

  • Claude Code (native Windows install, v0.0.39 lumen plugin):
    claude mcp list reported plugin:lumen:lumen ... ✗ Failed to connect.
    The same routing applies to other hosts that start the MCP server via
    the .cmd file association (Cursor's .cursor/mcp.json uses the same
    run.cmd entry point).

Why stdout was polluted

When cmd.exe runs a .cmd file with the default @echo on state, it
echoes each command line to stdout (prompt-prefixed, e.g.
C:\path>command) before executing it. The previous polyglot turned
echo off only after the goto:

#!/bin/sh                      <- echoed to stdout (prompt + 9 bytes)
# 2>NUL & @goto batch          <- echoed to stdout (prompt + 22 bytes)
exec "$(dirname "$0")/run.sh" "$@"

:batch
@echo off                      <- echo only off from here

Measured (Windows 11, default cmd.exe):

invocation stdout bytes stderr bytes
cmd /c scripts/run.cmd version 99 93
cmd /c scripts/run.bat version 7 0

The 92-byte stdout prefix looks like:

[blank]
C:\path>#!/bin/sh
[blank]
C:\path>#   2>NUL  &
0.0.39

MCP stdio clients parse JSON-RPC framing from the first byte of stdout;
any leading non-JSON bytes fail the handshake before the lumen binary
ever speaks.

Why CI didn't catch it

scripts/test_run_cmd_windows.bat (added in #121) had two relevant
gaps:

  1. The stderr assertion only checked for absence of the historical -S
    substring (with a comment noting the shebang error was "harmless"),
    so it passed even when stderr contained a different error.
  2. The stdout test used for /f "tokens=*" %%i in ('run.cmd ...') do set "OUTPUT=%%i", which overwrites OUTPUT on each line and keeps
    only the last stdout line. The polluting prompt-echo lines came
    first and were silently discarded.

The fix

scripts/run.cmd

Replace the prologue with a label-style line that cmd.exe does not echo
(labels are skipped without echoing even with @echo on), while still
working as a heredoc start in sh:

:<<"::CMDLITERAL"
@echo off
goto :batch
::CMDLITERAL
exec "$(dirname "$0")/run.sh" "$@"

:batch
@echo off
set "SCRIPT_DIR=%~dp0"
call "%SCRIPT_DIR%run.bat" %*
exit /b %ERRORLEVEL%
  • cmd.exe path: line 1 is parsed as a label (silent), line 2
    @echo off turns echo off, line 3 goto :batch jumps to the batch
    block. No stdout pollution.
  • sh path: line 1 is : (POSIX no-op) followed by a quoted heredoc
    with delimiter ::CMDLITERAL; the heredoc consumes lines 2–3 until
    the matching terminator on line 4. After the heredoc closes, sh runs
    the exec line.
  • Byte 0 is now :, so OS execve returns ENOEXEC. POSIX-compliant
    runners that go through libc execvp() fall back to /bin/sh on
    ENOEXEC, which interprets the file via the sh path above (verified on
    WSL Ubuntu 24.04).

scripts/test_run_cmd_windows.bat

  • Test 3 (tightened): assert stderr is exactly 0 bytes (the new
    prologue produces no command-not-found errors at all), instead of
    only checking absence of -S.
  • Test 4 (new): spawn run.cmd via cmd /c to match how MCP hosts
    launch it (a fresh cmd.exe with default @echo on), capture full
    stdout to a file, and assert (a) the first non-empty line is
    delegated:stdio and (b) the total line count is exactly 1. Both
    checks must pass — first-line alone misses cases where extra lines
    appear after the expected output, and line-count alone misses
    reordering.

The cmd /c invocation is critical: call run.cmd would inherit the
parent test script's @echo off and mask the bug.

Test plan

  • scripts\test_run_cmd_windows.bat with the fix on Windows 11 →
    4/4 PASS
  • scripts\test_run_cmd_windows.bat after reverting run.cmd to
    upstream mainTest 3 FAIL (93-byte stderr), Test 4
    FAIL
    (5 lines, first line is the prompt-echoed #!/bin/sh) —
    confirms the new tests are real regression guards
  • scripts/test_run_cmd.sh on WSL Ubuntu 24.04 — Unix delegation
    still passes (execvp ENOEXEC → /bin/sh fallback)
  • Patched run.cmd with CRLF line endings (matches Windows
    checkout under core.autocrlf=true) tested on /bin/sh,
    /bin/dash, and direct execve on Linux — all PASS (POSIX sh
    treats trailing \r consistently on the heredoc opener and
    terminator, so they match)
  • End-to-end: applied the patched run.cmd to a local Claude
    Code plugin cache, restarted, /mcp reconnected
    plugin:lumen:lumen and the mcp__lumen__semantic_search tool
    became callable

Summary by CodeRabbit

  • Chores
    • Improved internal script structure for cross-platform command execution
    • Enhanced test coverage for Windows command execution behavior

cmd.exe echoed the polyglot's first two lines (`#!/bin/sh` and
`# 2>NUL & @goto batch`) to stdout before `@echo off` took effect,
breaking MCP stdio JSON-RPC framing for hosts that spawn run.cmd
via .cmd extension association (Claude Code, Cursor).

Replace the prologue with `:<<"::CMDLITERAL"`: cmd.exe treats it
as a non-echoing label, while sh treats it as a quoted heredoc
that consumes the cmd-only block.

Add a regression test to test_run_cmd_windows.bat that spawns
run.cmd via `cmd /c` and asserts stdout is exactly the run.bat
output — the existing `for /f` test only kept the last stdout
line, masking any prefix pollution.

Refs: ory#121
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 3, 2026

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 3, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9854831a-2bc6-426b-8025-2f12de7b40af

📥 Commits

Reviewing files that changed from the base of the PR and between 5206287 and fb5259f.

📒 Files selected for processing (2)
  • scripts/run.cmd
  • scripts/test_run_cmd_windows.bat

📝 Walkthrough

Walkthrough

The pull request restructures the run.cmd polyglot script from a POSIX shell shebang-based approach to a bash heredoc-style wrapper using :<<"::CMDLITERAL" syntax. Test coverage for the Windows batch path is tightened to validate stderr and stdout cleanliness, ensuring no unwanted output pollution from the polyglot mechanism.

Changes

Polyglot Restructure and Test Validation

Layer / File(s) Summary
Polyglot Mechanism Rewrite
scripts/run.cmd
Changes from #!/bin/sh shebang with 2>NUL & @goto batch escape to bash heredoc-style :<<"::CMDLITERAL" wrapper that embeds @echo off and batch label, preserving both bash (exec to run.sh) and batch (call to run.bat) execution paths.
Test Assertions
scripts/test_run_cmd_windows.bat
Test 3 replaces stderr pattern matching with explicit empty-file validation (STDERR_SIZE == 0); Test 4 added to verify stdout contains exactly one line (delegated:stdio) with no command-echo pollution, both capturing and reporting full output on failure.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title directly and clearly addresses the main change: eliminating stdout pollution in the run.cmd polyglot script, which is the core objective of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

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

Comment thread scripts/run.cmd
@@ -1,5 +1,7 @@
#!/bin/sh
# 2>NUL & @goto batch
:<<"::CMDLITERAL"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't that in turn breaking linux again?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Looking at this PR's CI, Script tests (ubuntu-latest) and (macos-latest) both pass — those jobs run scripts/test_run_cmd.sh, which does "${TMP_DIR}/run.cmd" stdio --flag (direct exec on the patched file, not bash run.cmd).

My understanding of the mechanism: byte 0 is now : instead of #!, so execve() returns ENOEXEC, and POSIX shells handle this by re-invoking the file under /bin/sh. sh then sees :<<"::CMDLITERAL" as no-op : + quoted heredoc, consumes the cmd block, and runs exec ... run.sh "$@" as before.

I also tested locally on WSL Ubuntu 24.04 with CRLF line endings (/bin/sh, /bin/dash, and direct execve — all clean).

Wouldn't that suggest Linux/macOS still work, or is there a path the CI isn't exercising that you'd want verified?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, I also checked it out locally and it works!

@aeneasr aeneasr merged commit 2c1f63b into ory:main May 5, 2026
10 checks passed
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.

3 participants