From 1ee976562081e0de60d168c19d8031fa4e78b518 Mon Sep 17 00:00:00 2001 From: JasonBuildAI Date: Mon, 12 Oct 2026 00:47:01 +0800 Subject: [PATCH 1/4] fix(claude-goal-mode): quote the interpreter path baked into the adapter commands The installer bakes the interpreter it selected into the `--harden` PreToolUse hook, the statusline and the `/loopx` command (including its `allowed-tools` rule) unquoted. Claude Code runs those strings through a shell, so on a host whose interpreter lives under a path with a space - the python.org all-users install, or any profile directory with a space - the shell splits the path at the first space: the hook exits non-zero with empty stdout and the tool call proceeds ungated (the documented fail-closed `should_run` gate fails OPEN), the statusline renders nothing and `/loopx` never runs. PR #6229 named this residual. `_shell_path()` quotes a token that is not a plain path and leaves a plain path byte-identical to the previous rendering, so existing installs are unchanged. Verified on native Windows with the interpreter under a spaced directory: the exact installed hook command exits 1 with `'' is not recognized as an internal or external command` on the base commit and exits 0 with its `{}` no-op decision on this commit; the new tests fail before the change and pass after. Signed-off-by: JasonBuildAI --- loopx/claude_goal_mode/README.md | 7 ++ loopx/claude_goal_mode/scripts/install.py | 22 ++++- ...est_claude_goal_install_python_launcher.py | 87 ++++++++++++++++++- 3 files changed, 113 insertions(+), 3 deletions(-) diff --git a/loopx/claude_goal_mode/README.md b/loopx/claude_goal_mode/README.md index fd3c99b9cf..04b3ec8941 100644 --- a/loopx/claude_goal_mode/README.md +++ b/loopx/claude_goal_mode/README.md @@ -70,6 +70,13 @@ UTF-8) an unpinned gate would decode the event into mojibake, miss the project goal and answer `{}` — no gate at all — and `/loopx status` would raise on its own state glyphs. `tests/test_claude_goal_mode_stdio_utf8.py` guards all three. +Claude Code also runs those baked command strings through a shell, so an +interpreter path that contains a space - the python.org all-users install, or a +profile directory with a space - is quoted before the installer bakes it. +Unquoted, the shell splits the path at the first space and the gate, the +statusline and `/loopx` silently do nothing. +`tests/test_claude_goal_install_python_launcher.py` guards that. + ```bash python3 loopx/claude_goal_mode/scripts/install.py --scope project --project /path/to/project --harden ``` diff --git a/loopx/claude_goal_mode/scripts/install.py b/loopx/claude_goal_mode/scripts/install.py index 324f9048ed..ff7cecf58c 100644 --- a/loopx/claude_goal_mode/scripts/install.py +++ b/loopx/claude_goal_mode/scripts/install.py @@ -21,6 +21,7 @@ import argparse import json +import re import shutil import subprocess import sys @@ -34,6 +35,23 @@ def _p(*parts) -> str: return str(PLUGIN_ROOT.joinpath(*parts)).replace("\\", "/") +_PLAIN_SHELL_PATH = re.compile(r"[A-Za-z0-9._/\\:@+-]+\Z") + + +def _shell_path(value: str) -> str: + """Render one path token of a baked command string. + + Claude Code runs the hook / statusline / `/loopx` command strings through a + shell, which splits an unquoted program path at the first space: on a host + where the confirmed-runnable interpreter lives under `C:\\Program Files` (or + any profile directory with a space) the baked command never starts Python + and the surface silently does nothing. Quote a token that is no longer a + plain path; a plain path keeps the exact previous rendering.""" + if _PLAIN_SHELL_PATH.fullmatch(value): + return value + return f'"{value}"' + + def _launches_python(py: str) -> bool: """True when running `py` actually starts an interpreter. @@ -74,7 +92,7 @@ def deep_merge(base: dict, add: dict) -> dict: def hardening_block() -> dict: """The OPTIONAL per-tool gate + statusline. Not installed unless --harden.""" - py = _python_cmd() + py = _shell_path(_python_cmd()) return { "hooks": { "PreToolUse": [{"matcher": "*", "hooks": [ @@ -217,7 +235,7 @@ def install_mcp( def command_md() -> str: entry = _p("scripts", "goalmode_cmd.py") - py = _python_cmd() + py = _shell_path(_python_cmd()) return ( "---\n" "description: loopx goal-mode setup (NOT Claude Code's built-in /goal). " diff --git a/tests/test_claude_goal_install_python_launcher.py b/tests/test_claude_goal_install_python_launcher.py index 7c65145709..c44503d075 100644 --- a/tests/test_claude_goal_install_python_launcher.py +++ b/tests/test_claude_goal_install_python_launcher.py @@ -17,6 +17,13 @@ The installer must only bake an interpreter it has confirmed can launch Python. PR #6166 named this residual as its out-of-scope successor. + +The same baked strings interpolate that interpreter unquoted, so a confirmed +runnable interpreter under a path that contains a space - the python.org +all-users install (`C:\Program Files\Python312`), or any profile directory with +a space - produces a command the shell splits at the first space: the gate +fails OPEN, the statusline renders nothing and `/loopx` never runs. PR #6229 +named that residual. """ from __future__ import annotations @@ -81,7 +88,7 @@ def test_baked_commands_do_not_carry_the_dead_alias(alias_host: Path) -> None: assert sys.executable in hook_cmd and alias not in hook_cmd assert sys.executable in status_cmd and alias not in status_cmd - assert f"allowed-tools: Bash({sys.executable}:*)" in command + assert f"allowed-tools: Bash({claude_install._shell_path(sys.executable)}:*)" in command assert alias not in command @@ -138,3 +145,81 @@ class Result: monkeypatch.setattr(claude_install.subprocess, "run", lambda *args, **kwargs: Result()) assert claude_install._launches_python("python3") is True + + +def _spaced_interpreter(tmp_path: Path) -> Path: + """A real launcher that lives under a path containing a space.""" + home = tmp_path / "Program Files" + home.mkdir(parents=True, exist_ok=True) + if sys.platform == "win32": + launcher = home / "python.bat" + launcher.write_text(f'@echo off\r\n"{sys.executable}" %*\r\n', encoding="ascii") + else: + launcher = home / "python3" + launcher.write_text(f'#!/bin/sh\nexec "{sys.executable}" "$@"\n', encoding="utf-8") + launcher.chmod(0o755) + return launcher + + +@pytest.fixture() +def spaced_host(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + """A PATH whose `python3` is a runnable interpreter under a spaced path.""" + launcher = _spaced_interpreter(tmp_path) + + def which(name: str) -> str | None: + return str(launcher) if name == "python3" else None + + monkeypatch.setattr(claude_install.shutil, "which", which) + return launcher + + +def test_shell_path_quotes_only_a_token_a_shell_would_split() -> None: + """A plain path keeps its old rendering; a splittable one is quoted.""" + assert claude_install._shell_path("/usr/bin/python3") == "/usr/bin/python3" + assert ( + claude_install._shell_path(r"C:\Users\me\Python311\python.exe") + == r"C:\Users\me\Python311\python.exe" + ) + assert claude_install._shell_path("/opt/my python/bin/python3") == '"/opt/my python/bin/python3"' + assert claude_install._shell_path(r"C:\Program Files (x86)\Python311\python.exe") == ( + r'"C:\Program Files (x86)\Python311\python.exe"' + ) + + +def test_baked_surfaces_quote_a_spaced_interpreter(spaced_host: Path) -> None: + launcher = str(spaced_host) + hardening = claude_install.hardening_block() + hook_cmd = hardening["hooks"]["PreToolUse"][0]["hooks"][0]["command"] + status_cmd = hardening["statusLine"]["command"] + command = claude_install.command_md() + + assert hook_cmd.startswith(f'"{launcher}" ') + assert status_cmd.startswith(f'"{launcher}" ') + # The /loopx permission rule must carry the same token the command runs. + assert f'allowed-tools: Bash("{launcher}":*)' in command + assert f'!`"{launcher}" "{claude_install._p("scripts", "goalmode_cmd.py")}"' in command + + +def test_installed_gate_command_runs_from_a_spaced_interpreter( + spaced_host: Path, tmp_path: Path +) -> None: + """Run exactly the hook command `--harden` installs, the way Claude Code does. + + With the spaced interpreter path unquoted, the shell splits it at the first + space and exits non-zero with nothing on stdout; the event never reaches the + gate, which Claude Code treats as a non-blocking hook error - the tool call + proceeds ungated. + """ + settings = tmp_path / "claude" / "settings.json" + claude_install.add_hardening(dry=False, settings_path=settings) + command = json.loads(settings.read_text(encoding="utf-8"))["hooks"]["PreToolUse"][0]["hooks"][0][ + "command" + ] + + project = tmp_path / "project" + project.mkdir() + event = json.dumps({"cwd": str(project), "tool_name": "Read", "tool_input": {}}) + result = subprocess.run(command, shell=True, input=event.encode("utf-8"), capture_output=True) + + assert result.returncode == 0, (result.returncode, result.stdout, result.stderr) + assert json.loads(result.stdout.decode("utf-8")) == {} From 01783cd7a9fc1688e22bf962b6b6c2efac4874f5 Mon Sep 17 00:00:00 2001 From: JasonBuildAI Date: Tue, 13 Oct 2026 00:34:49 +0800 Subject: [PATCH 2/4] fix(claude-goal-mode): encode baked adapter paths for the host shell The review head wrapped a non-plain interpreter path in double quotes only. Under /bin/sh that is not a literal token: `$NAME` still expands inside it and an inner `"` closes the token, so a confirmed-runnable interpreter under `User $LABEL Home` or `User "quoted" Home` left the hook failing (rc 127, empty stdout - the gate fails OPEN), the statusline rendering nothing and /loopx never running. Encode every path a baked command string carries - the interpreter and the script it starts - as one literal token for the shell that actually runs the string: shlex.quote under /bin/sh, double quotes under cmd.exe. A plain Windows path keeps its previous unquoted rendering, so settings.json and the /loopx permission prefix do not churn. cmd.exe keeps one residual: %NAME% still expands inside double quotes. Tests now read the generated hook, statusline and /loopx commands back out of the installed files and execute them through the host shell for a plain path, an ordinary spaced path and literal $/quote paths. Signed-off-by: JasonBuildAI --- loopx/claude_goal_mode/README.md | 18 +- loopx/claude_goal_mode/scripts/install.py | 54 +++-- ...est_claude_goal_install_python_launcher.py | 227 +++++++++++++++--- 3 files changed, 240 insertions(+), 59 deletions(-) diff --git a/loopx/claude_goal_mode/README.md b/loopx/claude_goal_mode/README.md index 04b3ec8941..53e7460c54 100644 --- a/loopx/claude_goal_mode/README.md +++ b/loopx/claude_goal_mode/README.md @@ -70,12 +70,18 @@ UTF-8) an unpinned gate would decode the event into mojibake, miss the project goal and answer `{}` — no gate at all — and `/loopx status` would raise on its own state glyphs. `tests/test_claude_goal_mode_stdio_utf8.py` guards all three. -Claude Code also runs those baked command strings through a shell, so an -interpreter path that contains a space - the python.org all-users install, or a -profile directory with a space - is quoted before the installer bakes it. -Unquoted, the shell splits the path at the first space and the gate, the -statusline and `/loopx` silently do nothing. -`tests/test_claude_goal_install_python_launcher.py` guards that. +Claude Code also runs those baked command strings through the host shell +(`cmd.exe` on Windows, `/bin/sh` elsewhere), so the installer encodes every +path it bakes - the interpreter and the script it starts - as one literal token +for that shell: `shlex.quote`'s single-quoted form under `/bin/sh`, double +quotes under `cmd.exe`, and a plain Windows path keeps its previous unquoted +rendering. Unencoded, the shell splits the path at the first space, expands +`$NAME`, or closes the token at an inner `"`: the `--harden` gate fails OPEN, +the statusline renders nothing and `/loopx` never runs. One `cmd.exe` residual +stays: `%NAME%` still expands inside double quotes. +`tests/test_claude_goal_install_python_launcher.py` executes the installed +hook, statusline and `/loopx` commands for a plain path, an ordinary spaced +path and literal `$`/quote paths. ```bash python3 loopx/claude_goal_mode/scripts/install.py --scope project --project /path/to/project --harden diff --git a/loopx/claude_goal_mode/scripts/install.py b/loopx/claude_goal_mode/scripts/install.py index ff7cecf58c..2d971e9fd7 100644 --- a/loopx/claude_goal_mode/scripts/install.py +++ b/loopx/claude_goal_mode/scripts/install.py @@ -21,7 +21,9 @@ import argparse import json +import os import re +import shlex import shutil import subprocess import sys @@ -35,21 +37,41 @@ def _p(*parts) -> str: return str(PLUGIN_ROOT.joinpath(*parts)).replace("\\", "/") -_PLAIN_SHELL_PATH = re.compile(r"[A-Za-z0-9._/\\:@+-]+\Z") +_PLAIN_CMD_PATH = re.compile(r"[A-Za-z0-9._/\\:@+-]+\Z") + + +def _is_windows_shell() -> bool: + """Whether the baked command strings run under cmd.exe instead of `/bin/sh`. + + Claude Code executes hook / statusline / command strings through the host + shell: cmd.exe on Windows (the split-at-the-first-space failure in issue + #6249 is cmd.exe's message), `/bin/sh` elsewhere. The two disagree about + what an unquoted - and even a double-quoted - token means, so the encoding + follows the shell that will actually run the string.""" + return os.name == "nt" + + +def _shell_literal(value: str) -> str: + """Render `value` as one token the target shell reads back byte-for-byte. + + POSIX double quotes are not literal: `$NAME` still expands inside them and + an inner `"` closes the token, so `/bin/sh` targets get `shlex.quote`'s + single-quoted form. cmd.exe has no way to escape a `"` inside a quoted + token, but a Windows path cannot contain one; cmd's `%NAME%` expansion + inside quotes stays a documented residual.""" + if _is_windows_shell(): + return f'"{value}"' + return shlex.quote(value) def _shell_path(value: str) -> str: - """Render one path token of a baked command string. - - Claude Code runs the hook / statusline / `/loopx` command strings through a - shell, which splits an unquoted program path at the first space: on a host - where the confirmed-runnable interpreter lives under `C:\\Program Files` (or - any profile directory with a space) the baked command never starts Python - and the surface silently does nothing. Quote a token that is no longer a - plain path; a plain path keeps the exact previous rendering.""" - if _PLAIN_SHELL_PATH.fullmatch(value): + """`_shell_literal` for a path, except that a plain Windows path keeps the + installer's previous unquoted rendering (no churn in `settings.json` or in + the `/loopx` permission prefix). A path a shell would split, expand or + dequote - spaces, `$`, quotes - is encoded, not merely wrapped.""" + if _is_windows_shell() and _PLAIN_CMD_PATH.fullmatch(value): return value - return f'"{value}"' + return _shell_literal(value) def _launches_python(py: str) -> bool: @@ -93,12 +115,14 @@ def deep_merge(base: dict, add: dict) -> dict: def hardening_block() -> dict: """The OPTIONAL per-tool gate + statusline. Not installed unless --harden.""" py = _shell_path(_python_cmd()) + gate = _shell_literal(_p("hooks", "goal_policy.py")) + status = _shell_literal(_p("statusline", "goal_status.py")) return { "hooks": { "PreToolUse": [{"matcher": "*", "hooks": [ - {"type": "command", "command": f'{py} "{_p("hooks", "goal_policy.py")}"', "timeout": 10}]}], + {"type": "command", "command": f"{py} {gate}", "timeout": 10}]}], }, - "statusLine": {"type": "command", "command": f'{py} "{_p("statusline", "goal_status.py")}"'}, + "statusLine": {"type": "command", "command": f"{py} {status}"}, } @@ -234,7 +258,7 @@ def install_mcp( # ---- /loopx command ---------------------------------------------------------- def command_md() -> str: - entry = _p("scripts", "goalmode_cmd.py") + entry = _shell_literal(_p("scripts", "goalmode_cmd.py")) py = _shell_path(_python_cmd()) return ( "---\n" @@ -245,7 +269,7 @@ def command_md() -> str: f"allowed-tools: Bash({py}:*)\n" "---\n\n" "Run the loopx setup helper and read its output:\n\n" - f"!`{py} \"{entry}\" $ARGUMENTS`\n\n" + f"!`{py} {entry} $ARGUMENTS`\n\n" "The output is loopx control-plane SETUP / status info — it is the COMPLETE, user-facing " "result. Show it to the user VERBATIM and STOP. Do NOT do any work, claim todos, write " "files, or loop here: the WORK runs when the user types Claude Code's native `/loop` (which " diff --git a/tests/test_claude_goal_install_python_launcher.py b/tests/test_claude_goal_install_python_launcher.py index c44503d075..561b8244f6 100644 --- a/tests/test_claude_goal_install_python_launcher.py +++ b/tests/test_claude_goal_install_python_launcher.py @@ -21,9 +21,12 @@ The same baked strings interpolate that interpreter unquoted, so a confirmed runnable interpreter under a path that contains a space - the python.org all-users install (`C:\Program Files\Python312`), or any profile directory with -a space - produces a command the shell splits at the first space: the gate -fails OPEN, the statusline renders nothing and `/loopx` never runs. PR #6229 -named that residual. +a space - produces a command the shell splits at the first space. PR #6229 +named that residual; PR #6250 keeps every baked token literal in the shell that +actually runs it (cmd.exe on Windows, `/bin/sh` elsewhere), where double quotes +alone still expand `$NAME` and dequote an inner `"`. These tests read the +generated commands back out of the installed files and execute them through the +host shell. """ from __future__ import annotations @@ -101,12 +104,12 @@ def test_installed_gate_command_actually_runs_python(alias_host: Path, tmp_path: """ settings = tmp_path / "claude" / "settings.json" claude_install.add_hardening(dry=False, settings_path=settings) - command = json.loads(settings.read_text(encoding="utf-8"))["hooks"]["PreToolUse"][0]["hooks"][0]["command"] + hook_cmd, _ = _installed_commands(settings) project = tmp_path / "project" project.mkdir() event = json.dumps({"cwd": str(project), "tool_name": "Read", "tool_input": {}}) - result = subprocess.run(command, shell=True, input=event.encode("utf-8"), capture_output=True) + result = _host_shell_run(hook_cmd, event, cwd=project) assert result.returncode == 0, (result.returncode, result.stdout, result.stderr) assert json.loads(result.stdout.decode("utf-8")) == {} @@ -147,9 +150,9 @@ class Result: assert claude_install._launches_python("python3") is True -def _spaced_interpreter(tmp_path: Path) -> Path: - """A real launcher that lives under a path containing a space.""" - home = tmp_path / "Program Files" +def _literal_interpreter(tmp_path: Path, dirname: str) -> Path: + """A real launcher for `sys.executable` that lives under `tmp_path / dirname`.""" + home = tmp_path / dirname home.mkdir(parents=True, exist_ok=True) if sys.platform == "win32": launcher = home / "python.bat" @@ -161,65 +164,213 @@ def _spaced_interpreter(tmp_path: Path) -> Path: return launcher +# Directory names `_launches_python` accepts but the host shell would not read +# as one literal token: spaces split, `$` expands, and an inner `"` closes the +# token under `/bin/sh`. A Windows file name cannot contain `"`. +_LITERAL_DIRNAMES = [ + "Program Files", + "User $LABEL Home", + pytest.param( + 'User "quoted" Home', + marks=pytest.mark.skipif( + sys.platform == "win32", reason='a Windows file name cannot contain "' + ), + ), +] + + @pytest.fixture() -def spaced_host(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: - """A PATH whose `python3` is a runnable interpreter under a spaced path.""" - launcher = _spaced_interpreter(tmp_path) +def literal_host(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + """Select a literal launcher as PATH's `python3` for one directory name.""" - def which(name: str) -> str | None: - return str(launcher) if name == "python3" else None + def make(dirname: str) -> Path: + launcher = _literal_interpreter(tmp_path, dirname) + monkeypatch.setattr( + claude_install.shutil, + "which", + lambda name: str(launcher) if name == "python3" else None, + ) + return launcher - monkeypatch.setattr(claude_install.shutil, "which", which) - return launcher + return make + + +def _host_shell_run(command: str, stdin: str, cwd: Path) -> subprocess.CompletedProcess[bytes]: + """Run one baked command string through the shell Claude Code uses. + + Hook / statusline / command strings go through cmd.exe on Windows and + `/bin/sh` elsewhere, with the session or event JSON on stdin. + """ + data = stdin.encode("utf-8") + if sys.platform == "win32": + return subprocess.run(command, shell=True, input=data, capture_output=True, cwd=cwd) + return subprocess.run(["/bin/sh", "-c", command], input=data, capture_output=True, cwd=cwd) + + +def _installed_commands(settings_path: Path) -> tuple[str, str]: + """The hook and statusline command strings read back from settings.json.""" + settings = json.loads(settings_path.read_text(encoding="utf-8")) + hook = settings["hooks"]["PreToolUse"][0]["hooks"][0]["command"] + status = settings["statusLine"]["command"] + return hook, status + + +def _expected_interpreter_token(launcher: str, *, plain: bool) -> str: + """The token `_shell_path` must bake on this host for `launcher`.""" + if plain: + return launcher + if sys.platform == "win32": + return f'"{launcher}"' + return f"'{launcher}'" -def test_shell_path_quotes_only_a_token_a_shell_would_split() -> None: - """A plain path keeps its old rendering; a splittable one is quoted.""" +def test_shell_path_encodes_for_posix_sh(monkeypatch: pytest.MonkeyPatch) -> None: + """Under `/bin/sh` the token must keep `$`, `"` and `\\` as data, not syntax.""" + monkeypatch.setattr(claude_install, "_is_windows_shell", lambda: False) assert claude_install._shell_path("/usr/bin/python3") == "/usr/bin/python3" + assert claude_install._shell_path("/opt/my python/bin/python3") == "'/opt/my python/bin/python3'" + assert ( + claude_install._shell_path("/home/User $LABEL Home/python3") + == "'/home/User $LABEL Home/python3'" + ) + assert ( + claude_install._shell_path('/home/User "quoted" Home/python3') + == '\'/home/User "quoted" Home/python3\'' + ) + assert claude_install._shell_path(r"C:\Users\me\python.exe") == r"'C:\Users\me\python.exe'" + + +def test_shell_path_encodes_for_cmd_exe(monkeypatch: pytest.MonkeyPatch) -> None: + """cmd.exe groups spaces with double quotes and reads `$` and `\\` literally. + + A plain Windows path keeps the installer's previous unquoted rendering. + """ + monkeypatch.setattr(claude_install, "_is_windows_shell", lambda: True) assert ( claude_install._shell_path(r"C:\Users\me\Python311\python.exe") == r"C:\Users\me\Python311\python.exe" ) - assert claude_install._shell_path("/opt/my python/bin/python3") == '"/opt/my python/bin/python3"' assert claude_install._shell_path(r"C:\Program Files (x86)\Python311\python.exe") == ( r'"C:\Program Files (x86)\Python311\python.exe"' ) + assert claude_install._shell_path(r"C:\Users\User $LABEL Home\python.bat") == ( + r'"C:\Users\User $LABEL Home\python.bat"' + ) + +@pytest.mark.skipif(sys.platform == "win32", reason="runs the rendering through /bin/sh") +@pytest.mark.parametrize( + "path", + [ + "/usr/bin/python3", + "/opt/my python/bin/python3", + "/home/User $LABEL Home/python3", + '/home/User "quoted" Home/python3', + "/home/it's/python3", + "/home/back\\slash/python3", + ], +) +def test_posix_rendering_round_trips_through_sh(path: str) -> None: + """`/bin/sh` must expand the encoded token back to exactly the original path.""" + rendered = claude_install._shell_path(path) + result = subprocess.run(["/bin/sh", "-c", f"printf %s {rendered}"], capture_output=True) + assert result.returncode == 0, result.stderr + assert result.stdout.decode("utf-8") == path + + +@pytest.mark.parametrize("dirname", _LITERAL_DIRNAMES) +def test_baked_surfaces_keep_the_interpreter_token_literal(literal_host, dirname: str) -> None: + launcher = str(literal_host(dirname)) + token = _expected_interpreter_token(launcher, plain=False) -def test_baked_surfaces_quote_a_spaced_interpreter(spaced_host: Path) -> None: - launcher = str(spaced_host) hardening = claude_install.hardening_block() - hook_cmd = hardening["hooks"]["PreToolUse"][0]["hooks"][0]["command"] - status_cmd = hardening["statusLine"]["command"] - command = claude_install.command_md() + assert hardening["hooks"]["PreToolUse"][0]["hooks"][0]["command"].startswith(f"{token} ") + assert hardening["statusLine"]["command"].startswith(f"{token} ") - assert hook_cmd.startswith(f'"{launcher}" ') - assert status_cmd.startswith(f'"{launcher}" ') + command = claude_install.command_md() # The /loopx permission rule must carry the same token the command runs. - assert f'allowed-tools: Bash("{launcher}":*)' in command - assert f'!`"{launcher}" "{claude_install._p("scripts", "goalmode_cmd.py")}"' in command + assert f"allowed-tools: Bash({token}:*)" in command + body = next(line for line in command.splitlines() if line.startswith("!`")) + assert body.startswith(f"!`{token} ") + +def test_a_plain_interpreter_path_keeps_its_previous_rendering(literal_host) -> None: + launcher = str(literal_host("PlainPython311")) + assert _expected_interpreter_token(launcher, plain=True) == launcher + + hardening = claude_install.hardening_block() + assert hardening["hooks"]["PreToolUse"][0]["hooks"][0]["command"].startswith(f"{launcher} ") + assert hardening["statusLine"]["command"].startswith(f"{launcher} ") + assert f"allowed-tools: Bash({launcher}:*)" in claude_install.command_md() -def test_installed_gate_command_runs_from_a_spaced_interpreter( - spaced_host: Path, tmp_path: Path + +@pytest.mark.parametrize("dirname", _LITERAL_DIRNAMES) +def test_installed_gate_command_runs_through_a_literal_interpreter( + literal_host, dirname: str, tmp_path: Path ) -> None: - """Run exactly the hook command `--harden` installs, the way Claude Code does. + """Execute the hook command exactly as `--harden` installed it. - With the spaced interpreter path unquoted, the shell splits it at the first - space and exits non-zero with nothing on stdout; the event never reaches the - gate, which Claude Code treats as a non-blocking hook error - the tool call - proceeds ungated. + A token the shell splits, expands or dequotes never reaches Python: the + gate exits non-zero with empty stdout (rc 127 under `/bin/sh`), which + Claude Code treats as a non-blocking hook error - the tool call proceeds + ungated. """ + literal_host(dirname) settings = tmp_path / "claude" / "settings.json" claude_install.add_hardening(dry=False, settings_path=settings) - command = json.loads(settings.read_text(encoding="utf-8"))["hooks"]["PreToolUse"][0]["hooks"][0][ - "command" - ] + hook_cmd, _ = _installed_commands(settings) project = tmp_path / "project" project.mkdir() event = json.dumps({"cwd": str(project), "tool_name": "Read", "tool_input": {}}) - result = subprocess.run(command, shell=True, input=event.encode("utf-8"), capture_output=True) + result = _host_shell_run(hook_cmd, event, cwd=project) assert result.returncode == 0, (result.returncode, result.stdout, result.stderr) assert json.loads(result.stdout.decode("utf-8")) == {} + + +@pytest.mark.parametrize("dirname", _LITERAL_DIRNAMES) +def test_installed_statusline_command_renders_through_a_literal_interpreter( + literal_host, dirname: str, tmp_path: Path +) -> None: + """Execute the statusline command read back from settings.json. + + Claude Code feeds the session JSON on stdin; with no goal here the + statusline must render nothing - but only after the interpreter starts. + """ + literal_host(dirname) + settings = tmp_path / "claude" / "settings.json" + claude_install.add_hardening(dry=False, settings_path=settings) + _, status_cmd = _installed_commands(settings) + + project = tmp_path / "project" + project.mkdir() + result = _host_shell_run(status_cmd, json.dumps({"cwd": str(project)}), cwd=project) + + assert result.returncode == 0, (result.returncode, result.stdout, result.stderr) + assert result.stdout.decode("utf-8").strip() == "" + + +@pytest.mark.parametrize("dirname", _LITERAL_DIRNAMES) +def test_installed_slash_command_runs_through_a_literal_interpreter( + literal_host, dirname: str, tmp_path: Path +) -> None: + """Execute the `/loopx` body line from the installed command file. + + Claude Code substitutes `$ARGUMENTS` textually and runs the line through + the host shell; `status` is the cheapest real argument, and with no goal + here the helper says so and exits 0. + """ + literal_host(dirname) + cmd_dir = tmp_path / "claude" / "commands" + claude_install.install_command(dry=False, cmd_dir=cmd_dir) + text = (cmd_dir / "loopx.md").read_text(encoding="utf-8") + line = next(line for line in text.splitlines() if line.startswith("!`") and line.endswith("`")) + command = line[2:-1].replace("$ARGUMENTS", "status") + + project = tmp_path / "project" + project.mkdir() + result = _host_shell_run(command, "", cwd=project) + + assert result.returncode == 0, (result.returncode, result.stdout, result.stderr) + assert "no goal in this project yet" in result.stdout.decode("utf-8") From 88ae94a2ac080ce3c5f94359c6d1809a556c4908 Mon Sep 17 00:00:00 2001 From: JasonBuildAI Date: Tue, 13 Oct 2026 10:28:58 +0800 Subject: [PATCH 3/4] fix(claude-goal-mode): encode baked commands for Claude Code's actual shell Claude Code runs shell-form hooks and the statusline through Git Bash on Windows when Git Bash is installed, PowerShell when it is not, and /bin/sh on macOS/Linux - never cmd.exe (Git Bash switch in 2.1.47, PowerShell fallback in 2.1.120). os.name == "nt" therefore encoded for the wrong shell: Git Bash eats the backslashes of unquoted Windows paths, and the reviewer's 12-combination probe failed 9 of them with rc 127 and empty hook output. Detect the shell the way Claude Code does - CLAUDE_CODE_GIT_BASH_PATH, the Git installation, bash on PATH, the default Git for Windows locations - rejecting the WSL System32 alias by probing `uname -s` for MINGW/MSYS/CYGWIN. Encode the interpreter and the script it starts as one literal token for that shell: shlex-quoted for /bin/sh and Git Bash (Windows paths are always quoted now), and `& '...'` with `''` escaping for PowerShell. The /loopx `!` line runs through the Bash tool on every host, so it stays POSIX-encoded; a Windows host without Git Bash cannot run it at all, and re-running install.py --harden after Git Bash availability changes re-encodes for the new shell. Correct the README's cmd.exe claim and execute the installed hook, statusline and /loopx commands through the selected shell: a directory junction reproduces a real spaced interpreter install under Git Bash, and a forced PowerShell branch runs the installed strings through real powershell.exe. Signed-off-by: JasonBuildAI --- loopx/claude_goal_mode/README.md | 32 ++- loopx/claude_goal_mode/scripts/install.py | 115 +++++--- ...est_claude_goal_install_python_launcher.py | 245 +++++++++++++----- 3 files changed, 285 insertions(+), 107 deletions(-) diff --git a/loopx/claude_goal_mode/README.md b/loopx/claude_goal_mode/README.md index 8258c428a9..9f5e81448b 100644 --- a/loopx/claude_goal_mode/README.md +++ b/loopx/claude_goal_mode/README.md @@ -76,18 +76,28 @@ UTF-8) an unpinned gate would decode the event into mojibake, miss the project goal and answer `{}` — no gate at all — and `/loopx status` would raise on its own state glyphs. `tests/test_claude_goal_mode_stdio_utf8.py` guards all three. -Claude Code also runs those baked command strings through the host shell -(`cmd.exe` on Windows, `/bin/sh` elsewhere), so the installer encodes every -path it bakes - the interpreter and the script it starts - as one literal token -for that shell: `shlex.quote`'s single-quoted form under `/bin/sh`, double -quotes under `cmd.exe`, and a plain Windows path keeps its previous unquoted -rendering. Unencoded, the shell splits the path at the first space, expands -`$NAME`, or closes the token at an inner `"`: the `--harden` gate fails OPEN, -the statusline renders nothing and `/loopx` never runs. One `cmd.exe` residual -stays: `%NAME%` still expands inside double quotes. +Claude Code also runs those baked command strings through a shell - `/bin/sh` +on macOS/Linux, Git Bash on Windows when it is installed, PowerShell on a +Windows host without Git Bash (hooks reference, "Exec form and shell form"; +Claude Code switched Windows hooks from `cmd.exe` to Git Bash in 2.1.47 and +added the PowerShell fallback in 2.1.120). The installer detects that same +choice and encodes every path it bakes - the interpreter and the script it +starts - as one literal token for it: `shlex.quote`'s single-quoted form under +`/bin/sh` and Git Bash, a single-quoted `& '...'` token under PowerShell. A +Windows path is always quoted, because Git Bash eats the backslashes of a bare +`C:\Users\me\Python\python.exe`. Unencoded, the shell splits the path at the +first space, expands `$NAME`, drops backslashes, or closes the token at an +inner `"`: the `--harden` gate fails OPEN, the statusline renders nothing and +`/loopx` never runs. Two boundaries stay explicit: the `/loopx` `!` line runs +through the Bash tool, so it is always POSIX-encoded and a Windows host without +Git Bash cannot run it at all; and the shell choice is made when the installer +runs - installing or removing Git Bash later, or pointing Claude Code at one +with `CLAUDE_CODE_GIT_BASH_PATH` in an environment the installer cannot see, +changes which shell Claude Code picks, so re-run `install.py --harden` after +that change. `tests/test_claude_goal_install_python_launcher.py` executes the installed -hook, statusline and `/loopx` commands for a plain path, an ordinary spaced -path and literal `$`/quote paths. +hook, statusline and `/loopx` commands through the selected shell for a plain +path, an ordinary spaced path and literal `$`/quote paths. ```bash python3 loopx/claude_goal_mode/scripts/install.py --scope project --project /path/to/project --harden diff --git a/loopx/claude_goal_mode/scripts/install.py b/loopx/claude_goal_mode/scripts/install.py index 2d971e9fd7..b1936c4672 100644 --- a/loopx/claude_goal_mode/scripts/install.py +++ b/loopx/claude_goal_mode/scripts/install.py @@ -22,7 +22,6 @@ import argparse import json import os -import re import shlex import shutil import subprocess @@ -37,41 +36,89 @@ def _p(*parts) -> str: return str(PLUGIN_ROOT.joinpath(*parts)).replace("\\", "/") -_PLAIN_CMD_PATH = re.compile(r"[A-Za-z0-9._/\\:@+-]+\Z") +def _is_git_bash(path: str) -> bool: + """True when `path` is the MSYS/Cygwin bash Claude Code dispatches to. + + The WSL `bash.exe` that sits on some PATHs is a Linux shell, not the Git + Bash Claude Code spawns for a Windows command string, and its `uname` says + `Linux`; the System32 alias is rejected by name and everything else by + asking the candidate for its `uname`.""" + system32 = Path(os.environ.get("SystemRoot", r"C:\Windows")) / "System32" + resolved = os.path.normcase(os.path.realpath(path)) + system32_resolved = os.path.normcase(os.path.realpath(system32)) + if resolved == system32_resolved or resolved.startswith(system32_resolved + os.sep): + return False + try: + probe = subprocess.run([path, "-c", "uname -s"], capture_output=True, timeout=10) + except (OSError, subprocess.SubprocessError): + return False + return probe.returncode == 0 and probe.stdout.decode("utf-8", "replace").startswith( + ("MINGW", "MSYS", "CYGWIN") + ) + + +def _git_bash() -> str | None: + """The Git Bash Claude Code runs a shell-form command string with, if any. + + Claude Code passes a hook or statusline string to Git Bash on Windows when + Git Bash is installed, to PowerShell when it is not, and to `/bin/sh` on + macOS/Linux (Claude Code hooks reference, "Exec form and shell form"). It + finds Git Bash through its `CLAUDE_CODE_GIT_BASH_PATH` override, the Git + installation or a `bash` on PATH, or a default Git for Windows location; + this mirrors those sources so the encoding can follow the same choice, and + a stale override falls through to the next source the way a non-bash + override is ignored there.""" + if sys.platform != "win32": + return None + candidates: list[str] = [] + override = os.environ.get("CLAUDE_CODE_GIT_BASH_PATH") + if override: + candidates.append(override) + git = shutil.which("git") + if git: + root = Path(git).resolve().parent.parent + candidates += [str(root / "bin" / "bash.exe"), str(root / "usr" / "bin" / "bash.exe")] + on_path = shutil.which("bash") + if on_path: + candidates.append(on_path) + for base, sub in ( + (os.environ.get("ProgramFiles"), "Git"), + (os.environ.get("ProgramFiles(x86)"), "Git"), + (os.environ.get("LocalAppData"), "Programs/Git"), + ): + if base: + candidates.append(str(Path(base) / sub / "bin" / "bash.exe")) + for candidate in candidates: + if os.path.isfile(candidate) and _is_git_bash(candidate): + return candidate + return None -def _is_windows_shell() -> bool: - """Whether the baked command strings run under cmd.exe instead of `/bin/sh`. +def _uses_powershell() -> bool: + """Whether Claude Code runs these command strings through PowerShell. - Claude Code executes hook / statusline / command strings through the host - shell: cmd.exe on Windows (the split-at-the-first-space failure in issue - #6249 is cmd.exe's message), `/bin/sh` elsewhere. The two disagree about - what an unquoted - and even a double-quoted - token means, so the encoding - follows the shell that will actually run the string.""" - return os.name == "nt" + A Windows host without Git Bash gets PowerShell for shell-form hooks and + the statusline; every other host runs `/bin/sh` or Git Bash.""" + return sys.platform == "win32" and _git_bash() is None -def _shell_literal(value: str) -> str: - """Render `value` as one token the target shell reads back byte-for-byte. +def _shell_literal(value: str, *, powershell: bool) -> str: + """Render `value` as one literal token the consumer shell reads back whole. - POSIX double quotes are not literal: `$NAME` still expands inside them and - an inner `"` closes the token, so `/bin/sh` targets get `shlex.quote`'s - single-quoted form. cmd.exe has no way to escape a `"` inside a quoted - token, but a Windows path cannot contain one; cmd's `%NAME%` expansion - inside quotes stays a documented residual.""" - if _is_windows_shell(): - return f'"{value}"' + Double quotes are not literal in either shell: `$NAME` expands inside them + and an inner `"` can close the token. Single quotes are literal in both, so + POSIX/Git Bash targets get `shlex.quote`'s form and PowerShell a `'...'` + string with its `''` escape. A bare Windows path would lose its + backslashes under Git Bash, so those are quoted too.""" + if powershell: + return "'" + value.replace("'", "''") + "'" return shlex.quote(value) -def _shell_path(value: str) -> str: - """`_shell_literal` for a path, except that a plain Windows path keeps the - installer's previous unquoted rendering (no churn in `settings.json` or in - the `/loopx` permission prefix). A path a shell would split, expand or - dequote - spaces, `$`, quotes - is encoded, not merely wrapped.""" - if _is_windows_shell() and _PLAIN_CMD_PATH.fullmatch(value): - return value - return _shell_literal(value) +def _shell_command(value: str, *, powershell: bool) -> str: + """`_shell_literal` in command position: PowerShell needs its `&` operator.""" + token = _shell_literal(value, powershell=powershell) + return f"& {token}" if powershell else token def _launches_python(py: str) -> bool: @@ -114,9 +161,10 @@ def deep_merge(base: dict, add: dict) -> dict: def hardening_block() -> dict: """The OPTIONAL per-tool gate + statusline. Not installed unless --harden.""" - py = _shell_path(_python_cmd()) - gate = _shell_literal(_p("hooks", "goal_policy.py")) - status = _shell_literal(_p("statusline", "goal_status.py")) + powershell = _uses_powershell() + py = _shell_command(_python_cmd(), powershell=powershell) + gate = _shell_literal(_p("hooks", "goal_policy.py"), powershell=powershell) + status = _shell_literal(_p("statusline", "goal_status.py"), powershell=powershell) return { "hooks": { "PreToolUse": [{"matcher": "*", "hooks": [ @@ -258,8 +306,11 @@ def install_mcp( # ---- /loopx command ---------------------------------------------------------- def command_md() -> str: - entry = _shell_literal(_p("scripts", "goalmode_cmd.py")) - py = _shell_path(_python_cmd()) + # The `!` line runs through the Bash tool - `sh`/Git Bash on every host - so + # it is always a POSIX literal, never the PowerShell form; a Windows host + # without Git Bash cannot run `!` lines at all (README). + entry = _shell_literal(_p("scripts", "goalmode_cmd.py"), powershell=False) + py = _shell_literal(_python_cmd(), powershell=False) return ( "---\n" "description: loopx goal-mode setup (NOT Claude Code's built-in /goal). " diff --git a/tests/test_claude_goal_install_python_launcher.py b/tests/test_claude_goal_install_python_launcher.py index 561b8244f6..ad099fce62 100644 --- a/tests/test_claude_goal_install_python_launcher.py +++ b/tests/test_claude_goal_install_python_launcher.py @@ -22,16 +22,20 @@ runnable interpreter under a path that contains a space - the python.org all-users install (`C:\Program Files\Python312`), or any profile directory with a space - produces a command the shell splits at the first space. PR #6229 -named that residual; PR #6250 keeps every baked token literal in the shell that -actually runs it (cmd.exe on Windows, `/bin/sh` elsewhere), where double quotes -alone still expand `$NAME` and dequote an inner `"`. These tests read the -generated commands back out of the installed files and execute them through the -host shell. +named that residual and issue #6249 reported it; PR #6250 keeps every baked +token literal in the shell that actually runs it - `/bin/sh` on macOS/Linux, +Git Bash on Windows when it is installed, PowerShell on a Windows host without +Git Bash. cmd.exe was never that shell for current Claude Code, and it read +`$`, quotes and backslashes differently from the Git Bash that does run these +strings. These tests read the generated commands back out of the installed +files and execute them through the selected shell. """ from __future__ import annotations import json +import shutil +import shlex import subprocess import sys from pathlib import Path @@ -91,7 +95,7 @@ def test_baked_commands_do_not_carry_the_dead_alias(alias_host: Path) -> None: assert sys.executable in hook_cmd and alias not in hook_cmd assert sys.executable in status_cmd and alias not in status_cmd - assert f"allowed-tools: Bash({claude_install._shell_path(sys.executable)}:*)" in command + assert f"allowed-tools: Bash({shlex.quote(sys.executable)}:*)" in command assert alias not in command @@ -150,24 +154,44 @@ class Result: assert claude_install._launches_python("python3") is True -def _literal_interpreter(tmp_path: Path, dirname: str) -> Path: - """A real launcher for `sys.executable` that lives under `tmp_path / dirname`.""" +def _literal_interpreter(tmp_path: Path, dirname: str, request: pytest.FixtureRequest) -> Path: + """A real launcher for `sys.executable` under `tmp_path / dirname`. + + On Windows the launcher is the running interpreter's base install directory + behind a directory junction - the shape issue #6249's reproduction used. A + copied `python.exe` cannot find its DLLs, a venv `python.exe` needs the + `pyvenv.cfg` of the venv it was started from, and a `.bat` shim does not + work: Git Bash hands it to cmd.exe, which splits a spaced shim path itself. + The junction is dropped in teardown (`rmdir` unlinks it without touching the + target); pytest's own `tmp_path` cleanup refuses to delete one. + """ home = tmp_path / dirname - home.mkdir(parents=True, exist_ok=True) if sys.platform == "win32": - launcher = home / "python.bat" - launcher.write_text(f'@echo off\r\n"{sys.executable}" %*\r\n', encoding="ascii") - else: - launcher = home / "python3" - launcher.write_text(f'#!/bin/sh\nexec "{sys.executable}" "$@"\n', encoding="utf-8") - launcher.chmod(0o755) + probe = subprocess.run( + ["cmd.exe", "/d", "/c", "mklink", "/J", str(home), sys.base_prefix], + capture_output=True, + ) + if probe.returncode != 0: + pytest.skip(f"cannot create a directory junction: {probe.stdout!r} {probe.stderr!r}") + + def _unlink() -> None: + subprocess.run(["cmd.exe", "/d", "/c", "rmdir", str(home)], capture_output=True) + + request.addfinalizer(_unlink) + return home / "python.exe" + home.mkdir(parents=True, exist_ok=True) + launcher = home / "python3" + launcher.write_text(f'#!/bin/sh\nexec "{sys.executable}" "$@"\n', encoding="utf-8") + launcher.chmod(0o755) return launcher -# Directory names `_launches_python` accepts but the host shell would not read -# as one literal token: spaces split, `$` expands, and an inner `"` closes the -# token under `/bin/sh`. A Windows file name cannot contain `"`. -_LITERAL_DIRNAMES = [ +# Directory names the consumer shell must read back as one literal token, plus a +# plain one: a space used to split the token, `$` expanded, a backslash was +# eaten under Git Bash, and an inner `"` - which no Windows file name can hold - +# closed the token under `/bin/sh`. +_INTERPRETER_DIRNAMES = [ + "PlainPython311", "Program Files", "User $LABEL Home", pytest.param( @@ -180,15 +204,16 @@ def _literal_interpreter(tmp_path: Path, dirname: str) -> Path: @pytest.fixture() -def literal_host(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): +def literal_host(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, request: pytest.FixtureRequest): """Select a literal launcher as PATH's `python3` for one directory name.""" + real_which = shutil.which def make(dirname: str) -> Path: - launcher = _literal_interpreter(tmp_path, dirname) + launcher = _literal_interpreter(tmp_path, dirname, request) monkeypatch.setattr( claude_install.shutil, "which", - lambda name: str(launcher) if name == "python3" else None, + lambda name: str(launcher) if name == "python3" else real_which(name), ) return launcher @@ -198,12 +223,24 @@ def make(dirname: str) -> Path: def _host_shell_run(command: str, stdin: str, cwd: Path) -> subprocess.CompletedProcess[bytes]: """Run one baked command string through the shell Claude Code uses. - Hook / statusline / command strings go through cmd.exe on Windows and - `/bin/sh` elsewhere, with the session or event JSON on stdin. + A shell-form hook or statusline string goes to `/bin/sh` on macOS/Linux, to + Git Bash on Windows when it is installed, and to PowerShell when it is not; + `subprocess.run(..., shell=True)` would run cmd.exe on Windows, which is not + that shell, so the selected one is spawned explicitly. The event or session + JSON goes in on stdin. """ data = stdin.encode("utf-8") + if claude_install._uses_powershell(): + return subprocess.run( + ["powershell", "-NoProfile", "-Command", command], + input=data, + capture_output=True, + cwd=cwd, + ) if sys.platform == "win32": - return subprocess.run(command, shell=True, input=data, capture_output=True, cwd=cwd) + bash = claude_install._git_bash() + assert bash, "a Windows rendering without Git Bash must be the PowerShell form" + return subprocess.run([bash, "-c", command], input=data, capture_output=True, cwd=cwd) return subprocess.run(["/bin/sh", "-c", command], input=data, capture_output=True, cwd=cwd) @@ -215,47 +252,80 @@ def _installed_commands(settings_path: Path) -> tuple[str, str]: return hook, status -def _expected_interpreter_token(launcher: str, *, plain: bool) -> str: - """The token `_shell_path` must bake on this host for `launcher`.""" - if plain: - return launcher - if sys.platform == "win32": - return f'"{launcher}"' - return f"'{launcher}'" +def _expected_command_token(launcher: str) -> str: + """The command token `hardening_block` must bake on this host for `launcher`.""" + if claude_install._uses_powershell(): + return f"& '{launcher}'" + return shlex.quote(launcher) -def test_shell_path_encodes_for_posix_sh(monkeypatch: pytest.MonkeyPatch) -> None: - """Under `/bin/sh` the token must keep `$`, `"` and `\\` as data, not syntax.""" - monkeypatch.setattr(claude_install, "_is_windows_shell", lambda: False) - assert claude_install._shell_path("/usr/bin/python3") == "/usr/bin/python3" - assert claude_install._shell_path("/opt/my python/bin/python3") == "'/opt/my python/bin/python3'" +def test_shell_literal_encodes_for_posix_sh() -> None: + """Under `/bin/sh` and Git Bash the token must keep `$`, `"` and `\\` as data.""" + literal = claude_install._shell_literal + assert literal("/usr/bin/python3", powershell=False) == "/usr/bin/python3" + assert literal("/opt/my python/bin/python3", powershell=False) == "'/opt/my python/bin/python3'" assert ( - claude_install._shell_path("/home/User $LABEL Home/python3") + literal("/home/User $LABEL Home/python3", powershell=False) == "'/home/User $LABEL Home/python3'" ) assert ( - claude_install._shell_path('/home/User "quoted" Home/python3') + literal('/home/User "quoted" Home/python3', powershell=False) == '\'/home/User "quoted" Home/python3\'' ) - assert claude_install._shell_path(r"C:\Users\me\python.exe") == r"'C:\Users\me\python.exe'" + # Git Bash eats an unquoted backslash, so even a plain Windows path is quoted. + assert literal(r"C:\Users\me\python.exe", powershell=False) == r"'C:\Users\me\python.exe'" -def test_shell_path_encodes_for_cmd_exe(monkeypatch: pytest.MonkeyPatch) -> None: - """cmd.exe groups spaces with double quotes and reads `$` and `\\` literally. +def test_shell_literal_and_command_encode_for_powershell() -> None: + """PowerShell needs `'...'` with `''` escaping, and `&` in command position. - A plain Windows path keeps the installer's previous unquoted rendering. + Double quotes would still expand `$LABEL`, and a bare path is not a command + at all outside command position. """ - monkeypatch.setattr(claude_install, "_is_windows_shell", lambda: True) - assert ( - claude_install._shell_path(r"C:\Users\me\Python311\python.exe") - == r"C:\Users\me\Python311\python.exe" - ) - assert claude_install._shell_path(r"C:\Program Files (x86)\Python311\python.exe") == ( - r'"C:\Program Files (x86)\Python311\python.exe"' + literal = claude_install._shell_literal + command = claude_install._shell_command + assert literal(r"C:\Program Files (x86)\Python311\python.exe", powershell=True) == ( + r"'C:\Program Files (x86)\Python311\python.exe'" ) - assert claude_install._shell_path(r"C:\Users\User $LABEL Home\python.bat") == ( - r'"C:\Users\User $LABEL Home\python.bat"' + assert command(r"C:\Users\User $LABEL Home\python.exe", powershell=True) == ( + r"& 'C:\Users\User $LABEL Home\python.exe'" ) + assert command("/usr/bin/python3", powershell=True) == "& '/usr/bin/python3'" + assert literal("C:/it's/python.exe", powershell=True) == "'C:/it''s/python.exe'" + + +def test_shell_choice_follows_git_bash(monkeypatch: pytest.MonkeyPatch) -> None: + """PowerShell only on a Windows host that has no Git Bash for Claude Code. + + Claude Code passes a shell-form command string to Git Bash whenever Git Bash + is installed on Windows, so the installer must not classify the host by + `os.name` alone; everywhere else the shell is `/bin/sh`. + """ + monkeypatch.setattr(claude_install, "_git_bash", lambda: None) + assert claude_install._uses_powershell() is (sys.platform == "win32") + monkeypatch.setattr(claude_install, "_git_bash", lambda: r"C:\Git\bin\bash.exe") + assert claude_install._uses_powershell() is False + + +@pytest.mark.skipif(sys.platform != "win32", reason="Git Bash discovery is Windows-only") +def test_git_bash_rejects_the_wsl_alias(monkeypatch: pytest.MonkeyPatch) -> None: + r"""`C:\Windows\System32\bash.exe` is the WSL shell, not the Git Bash Claude spawns.""" + monkeypatch.setenv("CLAUDE_CODE_GIT_BASH_PATH", r"C:\Windows\System32\bash.exe") + monkeypatch.setattr(claude_install.shutil, "which", lambda name: None) + for var in ("ProgramFiles", "ProgramFiles(x86)", "LocalAppData"): + monkeypatch.delenv(var, raising=False) + assert claude_install._git_bash() is None + + +@pytest.mark.skipif(sys.platform != "win32", reason="Git Bash discovery is Windows-only") +def test_git_bash_is_a_runnable_msys_shell() -> None: + """Whatever the discovery accepts must answer `uname -s` as an MSYS/Cygwin bash.""" + bash = claude_install._git_bash() + if bash is None: + pytest.skip("this host has no Git Bash") + probe = subprocess.run([bash, "-c", "uname -s"], capture_output=True, timeout=10) + assert probe.returncode == 0, probe.stderr + assert probe.stdout.decode("utf-8", "replace").startswith(("MINGW", "MSYS", "CYGWIN")) @pytest.mark.skipif(sys.platform == "win32", reason="runs the rendering through /bin/sh") @@ -272,31 +342,37 @@ def test_shell_path_encodes_for_cmd_exe(monkeypatch: pytest.MonkeyPatch) -> None ) def test_posix_rendering_round_trips_through_sh(path: str) -> None: """`/bin/sh` must expand the encoded token back to exactly the original path.""" - rendered = claude_install._shell_path(path) + rendered = claude_install._shell_literal(path, powershell=False) result = subprocess.run(["/bin/sh", "-c", f"printf %s {rendered}"], capture_output=True) assert result.returncode == 0, result.stderr assert result.stdout.decode("utf-8") == path -@pytest.mark.parametrize("dirname", _LITERAL_DIRNAMES) +@pytest.mark.parametrize("dirname", _INTERPRETER_DIRNAMES) def test_baked_surfaces_keep_the_interpreter_token_literal(literal_host, dirname: str) -> None: launcher = str(literal_host(dirname)) - token = _expected_interpreter_token(launcher, plain=False) + token = _expected_command_token(launcher) hardening = claude_install.hardening_block() assert hardening["hooks"]["PreToolUse"][0]["hooks"][0]["command"].startswith(f"{token} ") assert hardening["statusLine"]["command"].startswith(f"{token} ") + # The `/loopx` `!` line runs through the Bash tool on every host, so that + # surface is always POSIX-encoded even where the hook uses PowerShell. + posix_token = shlex.quote(launcher) command = claude_install.command_md() # The /loopx permission rule must carry the same token the command runs. - assert f"allowed-tools: Bash({token}:*)" in command + assert f"allowed-tools: Bash({posix_token}:*)" in command body = next(line for line in command.splitlines() if line.startswith("!`")) - assert body.startswith(f"!`{token} ") + assert body.startswith(f"!`{posix_token} ") -def test_a_plain_interpreter_path_keeps_its_previous_rendering(literal_host) -> None: +@pytest.mark.skipif( + sys.platform == "win32", reason="a Windows path is always quoted for Git Bash" +) +def test_a_plain_posix_path_keeps_its_previous_rendering(literal_host) -> None: launcher = str(literal_host("PlainPython311")) - assert _expected_interpreter_token(launcher, plain=True) == launcher + assert claude_install._shell_literal(launcher, powershell=False) == launcher hardening = claude_install.hardening_block() assert hardening["hooks"]["PreToolUse"][0]["hooks"][0]["command"].startswith(f"{launcher} ") @@ -304,7 +380,7 @@ def test_a_plain_interpreter_path_keeps_its_previous_rendering(literal_host) -> assert f"allowed-tools: Bash({launcher}:*)" in claude_install.command_md() -@pytest.mark.parametrize("dirname", _LITERAL_DIRNAMES) +@pytest.mark.parametrize("dirname", _INTERPRETER_DIRNAMES) def test_installed_gate_command_runs_through_a_literal_interpreter( literal_host, dirname: str, tmp_path: Path ) -> None: @@ -329,7 +405,7 @@ def test_installed_gate_command_runs_through_a_literal_interpreter( assert json.loads(result.stdout.decode("utf-8")) == {} -@pytest.mark.parametrize("dirname", _LITERAL_DIRNAMES) +@pytest.mark.parametrize("dirname", _INTERPRETER_DIRNAMES) def test_installed_statusline_command_renders_through_a_literal_interpreter( literal_host, dirname: str, tmp_path: Path ) -> None: @@ -351,15 +427,15 @@ def test_installed_statusline_command_renders_through_a_literal_interpreter( assert result.stdout.decode("utf-8").strip() == "" -@pytest.mark.parametrize("dirname", _LITERAL_DIRNAMES) +@pytest.mark.parametrize("dirname", _INTERPRETER_DIRNAMES) def test_installed_slash_command_runs_through_a_literal_interpreter( literal_host, dirname: str, tmp_path: Path ) -> None: """Execute the `/loopx` body line from the installed command file. Claude Code substitutes `$ARGUMENTS` textually and runs the line through - the host shell; `status` is the cheapest real argument, and with no goal - here the helper says so and exits 0. + the Bash tool (`sh`/Git Bash); `status` is the cheapest real argument, and + with no goal here the helper says so and exits 0. """ literal_host(dirname) cmd_dir = tmp_path / "claude" / "commands" @@ -374,3 +450,44 @@ def test_installed_slash_command_runs_through_a_literal_interpreter( assert result.returncode == 0, (result.returncode, result.stdout, result.stderr) assert "no goal in this project yet" in result.stdout.decode("utf-8") + + +@pytest.mark.skipif(sys.platform != "win32", reason="powershell.exe is the Windows shell") +def test_installed_commands_run_under_powershell_without_git_bash( + literal_host, monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """The PowerShell form must start the same interpreter on a host with no Git Bash. + + The host running the test may have Git Bash (CI Windows does), so the + PowerShell branch is forced here and the installed strings are executed + through a real `powershell.exe` - the shell Claude Code falls back to when + Git Bash is absent - with the event and session JSON on stdin. + """ + launcher = str(literal_host("Program Files")) + monkeypatch.setattr(claude_install, "_uses_powershell", lambda: True) + + settings = tmp_path / "claude" / "settings.json" + claude_install.add_hardening(dry=False, settings_path=settings) + hook_cmd, status_cmd = _installed_commands(settings) + assert hook_cmd.startswith(f"& '{launcher}'") + assert status_cmd.startswith(f"& '{launcher}'") + + project = tmp_path / "project" + project.mkdir() + + def run(command: str, stdin: str) -> subprocess.CompletedProcess[bytes]: + return subprocess.run( + ["powershell", "-NoProfile", "-Command", command], + input=stdin.encode("utf-8"), + capture_output=True, + cwd=project, + ) + + event = json.dumps({"cwd": str(project), "tool_name": "Read", "tool_input": {}}) + hook = run(hook_cmd, event) + assert hook.returncode == 0, (hook.returncode, hook.stdout, hook.stderr) + assert json.loads(hook.stdout.decode("utf-8-sig")) == {} + + status = run(status_cmd, json.dumps({"cwd": str(project)})) + assert status.returncode == 0, (status.returncode, status.stdout, status.stderr) + assert status.stdout.decode("utf-8-sig").strip() == "" From 9056c0bfcdaaee1cd85b819409ddd8de6f69ecf4 Mon Sep 17 00:00:00 2001 From: JasonBuildAI Date: Tue, 13 Oct 2026 12:11:38 +0800 Subject: [PATCH 4/4] fix(claude-goal-mode): ignore Git Bash overrides Claude Code would ignore Claude Code 2.1.219+ only honors CLAUDE_CODE_GIT_BASH_PATH when the file name is one of bash.exe, sh.exe, bash or sh (compared case-insensitively); any other name is ignored and Git Bash detection continues as if the variable were unset. The installer accepted any existing override whose `uname -s` answered MINGW/MSYS/CYGWIN, so a host with a runnable shell under an unsupported name and no other Git Bash was classified as Git Bash: the hook and statusline were baked with the POSIX single-quoted form while the consumer dispatched the same strings to PowerShell. Filter the override by the consumer's accepted names in the existing _git_bash selector and let detection continue to the same fallback sources. The new tests pin a Windows host on every platform: unsupported names are ignored, including when a git-derived Git Bash exists; the four accepted names win case-insensitively; and the generated hook/statusline carry the PowerShell `& '...'` form for the rejected override, the POSIX form for an accepted override and for the auto-detected fallback. The Windows execution test now runs that rejected-override fallback through a real powershell.exe. Signed-off-by: JasonBuildAI --- loopx/claude_goal_mode/README.md | 5 +- loopx/claude_goal_mode/scripts/install.py | 16 +- ...est_claude_goal_install_python_launcher.py | 178 ++++++++++++++++-- 3 files changed, 183 insertions(+), 16 deletions(-) diff --git a/loopx/claude_goal_mode/README.md b/loopx/claude_goal_mode/README.md index 9f5e81448b..107e5a694f 100644 --- a/loopx/claude_goal_mode/README.md +++ b/loopx/claude_goal_mode/README.md @@ -94,7 +94,10 @@ Git Bash cannot run it at all; and the shell choice is made when the installer runs - installing or removing Git Bash later, or pointing Claude Code at one with `CLAUDE_CODE_GIT_BASH_PATH` in an environment the installer cannot see, changes which shell Claude Code picks, so re-run `install.py --harden` after -that change. +that change. Claude Code honors that override only for a file named `bash.exe`, +`sh.exe`, `bash` or `sh`; anything else, existing or not, is ignored and +detection proceeds as if it were unset, and the installer follows the same +rule. `tests/test_claude_goal_install_python_launcher.py` executes the installed hook, statusline and `/loopx` commands through the selected shell for a plain path, an ordinary spaced path and literal `$`/quote paths. diff --git a/loopx/claude_goal_mode/scripts/install.py b/loopx/claude_goal_mode/scripts/install.py index b1936c4672..d8ddd21131 100644 --- a/loopx/claude_goal_mode/scripts/install.py +++ b/loopx/claude_goal_mode/scripts/install.py @@ -57,6 +57,13 @@ def _is_git_bash(path: str) -> bool: ) +# Claude Code honors `CLAUDE_CODE_GIT_BASH_PATH` only when the file name is one +# of these (env-vars reference, 2.1.219+); any other name is ignored and Git +# Bash detection continues as if the variable were unset - even when the file +# exists and runs. +_GIT_BASH_OVERRIDE_NAMES = frozenset({"bash.exe", "sh.exe", "bash", "sh"}) + + def _git_bash() -> str | None: """The Git Bash Claude Code runs a shell-form command string with, if any. @@ -65,14 +72,15 @@ def _git_bash() -> str | None: macOS/Linux (Claude Code hooks reference, "Exec form and shell form"). It finds Git Bash through its `CLAUDE_CODE_GIT_BASH_PATH` override, the Git installation or a `bash` on PATH, or a default Git for Windows location; - this mirrors those sources so the encoding can follow the same choice, and - a stale override falls through to the next source the way a non-bash - override is ignored there.""" + this mirrors those sources so the encoding can follow the same choice. Like + that consumer, an override whose file name is not `bash.exe`, `sh.exe`, + `bash` or `sh` (compared case-insensitively) is ignored and detection + continues as if it were unset.""" if sys.platform != "win32": return None candidates: list[str] = [] override = os.environ.get("CLAUDE_CODE_GIT_BASH_PATH") - if override: + if override and Path(override).name.lower() in _GIT_BASH_OVERRIDE_NAMES: candidates.append(override) git = shutil.which("git") if git: diff --git a/tests/test_claude_goal_install_python_launcher.py b/tests/test_claude_goal_install_python_launcher.py index ad099fce62..83abf03f77 100644 --- a/tests/test_claude_goal_install_python_launcher.py +++ b/tests/test_claude_goal_install_python_launcher.py @@ -25,10 +25,12 @@ named that residual and issue #6249 reported it; PR #6250 keeps every baked token literal in the shell that actually runs it - `/bin/sh` on macOS/Linux, Git Bash on Windows when it is installed, PowerShell on a Windows host without -Git Bash. cmd.exe was never that shell for current Claude Code, and it read -`$`, quotes and backslashes differently from the Git Bash that does run these -strings. These tests read the generated commands back out of the installed -files and execute them through the selected shell. +Git Bash - and the `CLAUDE_CODE_GIT_BASH_PATH` override only counts there when +the file is named `bash.exe`, `sh.exe`, `bash` or `sh`, the same rule this +installer applies. cmd.exe was never that shell for current Claude Code, and it +read `$`, quotes and backslashes differently from the Git Bash that does run +these strings. These tests read the generated commands back out of the +installed files and execute them through the selected shell. """ from __future__ import annotations @@ -317,6 +319,152 @@ def test_git_bash_rejects_the_wsl_alias(monkeypatch: pytest.MonkeyPatch) -> None assert claude_install._git_bash() is None +def _git_bash_host( + monkeypatch: pytest.MonkeyPatch, which, override: str | None = None +) -> None: + """Pin `_git_bash` to a Windows host whose only discovery sources are the + `which` lookup and `override`. + + Pinning `sys.platform` covers the Windows-only rule on every host, and the + Git-for-Windows default locations are cleared so a result can only come + from the stub or the override. + """ + monkeypatch.setattr(claude_install.sys, "platform", "win32") + monkeypatch.setattr(claude_install.shutil, "which", which) + if override is None: + monkeypatch.delenv("CLAUDE_CODE_GIT_BASH_PATH", raising=False) + else: + monkeypatch.setenv("CLAUDE_CODE_GIT_BASH_PATH", override) + for var in ("ProgramFiles", "ProgramFiles(x86)", "LocalAppData"): + monkeypatch.delenv(var, raising=False) + + +@pytest.mark.parametrize("name", ["custom-shell.exe", "git-bash.exe", "bash.cmd", "wsl-bash"]) +def test_git_bash_ignores_override_names_claude_rejects( + name: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A `CLAUDE_CODE_GIT_BASH_PATH` name Claude Code does not accept is unset. + + The env-vars reference (2.1.219+) only honors `bash.exe`, `sh.exe`, `bash` + and `sh`: any other name is ignored and detection continues, even though the + file exists and runs. Accepting such an override here would render POSIX + commands while the consumer dispatches the same string to PowerShell. + """ + override = tmp_path / name + override.write_bytes(b"") + _git_bash_host(monkeypatch, lambda _name: None, override=str(override)) + # Only the consumer's name rule can reject the file: it exists and would + # pass the MSYS probe. + monkeypatch.setattr(claude_install, "_is_git_bash", lambda _path: True) + assert claude_install._git_bash() is None + + +@pytest.mark.parametrize("name", ["bash.exe", "sh.exe", "bash", "sh", "BASH.EXE"]) +def test_git_bash_honors_override_names_claude_accepts( + name: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The four accepted names win; the consumer lowercases, and so does this.""" + override = tmp_path / "Git" / "bin" / name + override.parent.mkdir(parents=True) + override.write_bytes(b"") + _git_bash_host(monkeypatch, lambda _name: None, override=str(override)) + monkeypatch.setattr(claude_install, "_is_git_bash", lambda _path: True) + assert claude_install._git_bash() == str(override) + + +def test_git_bash_ignores_a_rejected_name_but_keeps_detecting( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An ignored override must not stop the auto-detection the consumer falls + back to: a `git` on PATH still resolves its Git Bash.""" + override = tmp_path / "custom-shell.exe" + override.write_bytes(b"") + git = tmp_path / "Git" / "cmd" / "git.exe" + git.parent.mkdir(parents=True) + git.write_bytes(b"") + fallback = tmp_path / "Git" / "bin" / "bash.exe" + fallback.parent.mkdir(parents=True) + fallback.write_bytes(b"") + _git_bash_host( + monkeypatch, lambda name: str(git) if name == "git" else None, override=str(override) + ) + monkeypatch.setattr(claude_install, "_is_git_bash", lambda _path: True) + assert Path(claude_install._git_bash()).resolve() == fallback.resolve() + + +def test_rejected_override_renders_the_powershell_fallback( + literal_host, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A runnable `custom-shell.exe` must not flip the renderer to POSIX while + the consumer ignores it and falls back. + + With no other Git Bash, Claude Code dispatches to PowerShell, so the + installed hook and statusline must carry its `& '...'` command form. + """ + launcher = str(literal_host("Program Files")) + override = tmp_path / "custom-shell.exe" + override.write_bytes(b"") + _git_bash_host( + monkeypatch, lambda name: launcher if name == "python3" else None, override=str(override) + ) + monkeypatch.setattr(claude_install, "_is_git_bash", lambda _path: True) + + assert claude_install._uses_powershell() is True + hardening = claude_install.hardening_block() + hook = hardening["hooks"]["PreToolUse"][0]["hooks"][0]["command"] + status = hardening["statusLine"]["command"] + assert hook.startswith(f"& '{launcher}' ") + assert status.startswith(f"& '{launcher}' ") + + +def test_accepted_override_renders_the_literal_posix_form( + literal_host, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An override name the consumer honors keeps the literal POSIX rendering.""" + launcher = str(literal_host("Program Files")) + override = tmp_path / "Git" / "bin" / "BASH.EXE" + override.parent.mkdir(parents=True) + override.write_bytes(b"") + _git_bash_host( + monkeypatch, lambda name: launcher if name == "python3" else None, override=str(override) + ) + monkeypatch.setattr(claude_install, "_is_git_bash", lambda _path: True) + + assert claude_install._uses_powershell() is False + token = shlex.quote(launcher) + hardening = claude_install.hardening_block() + hook = hardening["hooks"]["PreToolUse"][0]["hooks"][0]["command"] + status = hardening["statusLine"]["command"] + assert hook.startswith(f"{token} ") + assert status.startswith(f"{token} ") + + +def test_git_derived_fallback_renders_the_literal_posix_form( + literal_host, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Auto-detected Git Bash (no override) still renders the literal form.""" + launcher = str(literal_host("Program Files")) + git = tmp_path / "Git" / "cmd" / "git.exe" + git.parent.mkdir(parents=True) + git.write_bytes(b"") + bash = tmp_path / "Git" / "bin" / "bash.exe" + bash.parent.mkdir(parents=True) + bash.write_bytes(b"") + _git_bash_host( + monkeypatch, + lambda name: launcher if name == "python3" else (str(git) if name == "git" else None), + ) + monkeypatch.setattr(claude_install, "_is_git_bash", lambda _path: True) + + assert claude_install._uses_powershell() is False + token = shlex.quote(launcher) + hardening = claude_install.hardening_block() + hook = hardening["hooks"]["PreToolUse"][0]["hooks"][0]["command"] + status = hardening["statusLine"]["command"] + assert hook.startswith(f"{token} ") + assert status.startswith(f"{token} ") + + @pytest.mark.skipif(sys.platform != "win32", reason="Git Bash discovery is Windows-only") def test_git_bash_is_a_runnable_msys_shell() -> None: """Whatever the discovery accepts must answer `uname -s` as an MSYS/Cygwin bash.""" @@ -456,16 +604,24 @@ def test_installed_slash_command_runs_through_a_literal_interpreter( def test_installed_commands_run_under_powershell_without_git_bash( literal_host, monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: - """The PowerShell form must start the same interpreter on a host with no Git Bash. - - The host running the test may have Git Bash (CI Windows does), so the - PowerShell branch is forced here and the installed strings are executed - through a real `powershell.exe` - the shell Claude Code falls back to when - Git Bash is absent - with the event and session JSON on stdin. + """The rejected-override fallback must start the interpreter under the real + PowerShell. + + This host may have Git Bash (CI Windows does), so every discovery source is + pinned empty and the `CLAUDE_CODE_GIT_BASH_PATH` override carries a name + Claude Code ignores, leaving the real selector on the PowerShell fallback. + The installed strings are executed through a real `powershell.exe` - the + shell Claude Code uses when Git Bash is absent - with the event and session + JSON on stdin. """ launcher = str(literal_host("Program Files")) - monkeypatch.setattr(claude_install, "_uses_powershell", lambda: True) + override = tmp_path / "custom-shell.exe" + override.write_bytes(b"") + _git_bash_host( + monkeypatch, lambda name: launcher if name == "python3" else None, override=str(override) + ) + assert claude_install._uses_powershell() is True settings = tmp_path / "claude" / "settings.json" claude_install.add_hardening(dry=False, settings_path=settings) hook_cmd, status_cmd = _installed_commands(settings)