fix(windows): stop subprocess console-window popups + add CI guard (#53791)

* fix(windows): stop subprocess console-window popups + add CI guard

The single biggest source of Windows 'terminal popup' bug reports was bare
subprocess.run/Popen calls spawning a console window. The compat helpers
(windows_hide_flags / windows_detach_popen_kwargs) already existed but the
footgun checker had no rule to stop new bare calls from reintroducing the flash.

- scripts/check-windows-footguns.py: new AST-based rule flagging subprocess
  calls that can create a new console — output-redirection-aware (capture/
  redirect/check_output exempt) and POSIX-only-program-aware (launchctl/
  systemctl/brew/etc. exempt). Comprehensive on real popups, no annotation
  burden on calls that can't flash.
- Swept all genuine window-spawning sites through windows_hide_flags()/
  windows_detach_popen_kwargs(); marked intentionally-visible launches
  (editor/terminal/foreground re-exec) with '# windows-footgun: ok'.
- tests/scripts/test_windows_footgun_subprocess_rule.py: behavior-contract
  tests + full-repo cleanliness invariant.
- CONTRIBUTING.md: documents the rule + the helper pattern.

* test: accept creationflags kwarg in psutil_android fake_subprocess_run

The Windows no-window sweep added creationflags=windows_hide_flags() to
install_psutil_android.py's subprocess.run call; the test's fake stub had a
fixed (cmd) signature and raised TypeError on the new kwarg.
This commit is contained in:
Teknium 2026-06-27 13:03:51 -07:00 committed by GitHub
parent 3b44a3c8bb
commit ef17cd204d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
22 changed files with 445 additions and 34 deletions

View file

@ -819,6 +819,31 @@ that touches the OS, assume *any* platform can hit your code path.
_quote_cmd_script_arg` and `_quote_schtasks_arg` for the reference
pair.
17. **Every `subprocess` call that spawns a console program needs a
no-window flag on Windows — and CI now enforces it.** A bare
`subprocess.run(["git", ...])` / `Popen(...)` of a console app flashes a
cmd window on Windows unless the child either inherits the parent's stdio
(output is captured/redirected) or is spawned with a no-window
creationflag. This was the single biggest source of "terminal popups"
bug reports. Use the helpers in `hermes_cli/_subprocess_compat.py` (both
no-op on POSIX):
```python
from hermes_cli._subprocess_compat import (
windows_hide_flags, windows_detach_popen_kwargs,
)
# short-lived / captured spawn:
subprocess.run(cmd, creationflags=windows_hide_flags())
# detached background daemon:
subprocess.Popen(cmd, **windows_detach_popen_kwargs())
```
`scripts/check-windows-footguns.py` flags any subprocess call that can
create a new console (AST-based, output-redirection-aware). Calls that
capture/redirect output, use `check_output`, or run a POSIX-only program
(`launchctl`, `systemctl`, `brew`, …) are exempt automatically — no
annotation needed. If a visible window is genuinely intended (interactive
editor/terminal launch, foreground re-exec), add `# windows-footgun: ok`
on the call line.
### Testing cross-platform
Tests that use POSIX-only syscalls need a skip marker. Common ones:

View file

@ -1274,7 +1274,7 @@ def run_oauth_setup_token() -> Optional[str]:
# concern does not apply to an interactive login the user explicitly
# invokes. noqa: subprocess-stdin
try:
subprocess.run([claude_path, "setup-token"])
subprocess.run([claude_path, "setup-token"]) # windows-footgun: ok — claude setup-token is interactive OAuth
except (KeyboardInterrupt, EOFError):
return None

View file

@ -2260,11 +2260,11 @@ class CLICommandsMixin:
if initial_text:
fh.write(initial_text)
try:
subprocess.call([*shlex.split(editor), path])
subprocess.call([*shlex.split(editor), path]) # windows-footgun: ok — $EDITOR launch is interactive/foreground
except Exception:
# Fall back to a bare invocation (editor value may not be a
# simple argv-splittable string on some platforms).
subprocess.call(f"{editor} {shlex.quote(path)}", shell=True)
subprocess.call(f"{editor} {shlex.quote(path)}", shell=True) # windows-footgun: ok — $EDITOR launch is interactive/foreground
with open(path, "r", encoding="utf-8") as fh:
raw = fh.read()
finally:

View file

@ -7019,7 +7019,7 @@ def edit_config():
return
print(f"Opening {config_path} in {editor}...")
subprocess.run([editor, str(config_path)])
subprocess.run([editor, str(config_path)]) # windows-footgun: ok — $EDITOR launch is interactive/foreground
def set_config_value(key: str, value: str):

View file

@ -23,6 +23,7 @@ import sys
from pathlib import Path
from hermes_constants import agent_browser_runnable
from hermes_cli._subprocess_compat import windows_hide_flags
_IS_WINDOWS = platform.system() == "Windows"
@ -152,6 +153,7 @@ def ensure_dependency(
result = subprocess.run(
cmd,
env=run_env,
creationflags=windows_hide_flags(),
)
if result.returncode != 0:
return False

View file

@ -12,6 +12,7 @@ import shlex
import shutil
import signal
import subprocess
from hermes_cli._subprocess_compat import windows_hide_flags
import sys
import textwrap
import time
@ -3385,7 +3386,7 @@ def systemd_status(deep: bool = False, system: bool = False, full: bool = False)
]
if full:
log_cmd.append("-l")
subprocess.run(log_cmd, timeout=10)
subprocess.run(log_cmd, timeout=10, creationflags=windows_hide_flags())
# =============================================================================

View file

@ -258,6 +258,10 @@ import json
import shutil
import stat
import subprocess
from hermes_cli._subprocess_compat import (
windows_detach_popen_kwargs,
windows_hide_flags,
)
from pathlib import Path
from typing import Optional
@ -2105,7 +2109,7 @@ def _launch_tui(
code: Optional[int] = None
try:
try:
code = subprocess.call(argv, cwd=str(cwd), env=env)
code = subprocess.call(argv, cwd=str(cwd), env=env) # windows-footgun: ok — foreground TUI hand-off, console is intentional
except KeyboardInterrupt:
code = 130
@ -2618,6 +2622,7 @@ def cmd_whatsapp(args):
],
cwd=str(bridge_dir),
env=with_hermes_node_path(),
creationflags=windows_hide_flags(),
)
except KeyboardInterrupt:
pass
@ -5289,7 +5294,7 @@ def _redownload_electron_dist(
if mirror:
dl_env["ELECTRON_MIRROR"] = mirror
try:
subprocess.run([node, str(installer)], cwd=str(electron_dir), env=dl_env, check=False)
subprocess.run([node, str(installer)], cwd=str(electron_dir), env=dl_env, check=False, creationflags=windows_hide_flags())
except OSError:
return False
return _electron_dist_ok(project_root)
@ -5409,7 +5414,7 @@ def _desktop_macos_relaunchable_fixup(desktop_dir: Path) -> None:
return
try:
subprocess.run(["xattr", "-cr", str(app)], check=False)
subprocess.run([codesign, "--force", "--deep", "--sign", "-", str(app)], check=False)
subprocess.run([codesign, "--force", "--deep", "--sign", "-", str(app)], check=False, creationflags=windows_hide_flags())
except Exception as exc:
print(f" (warning: macOS relaunch fixup skipped: {exc})")
@ -5474,7 +5479,7 @@ def _desktop_linux_sandbox_fixup(packaged_executable: Path) -> bool:
print("→ Configuring Electron Linux sandbox helper (sudo required)...")
for command in ([sudo, "chown", "root:root", str(sandbox)], [sudo, "chmod", "4755", str(sandbox)]):
if subprocess.run(command, check=False).returncode != 0:
if subprocess.run(command, check=False, creationflags=windows_hide_flags()).returncode != 0:
print(f"✗ Failed to configure Electron's Linux sandbox helper: {sandbox}")
return False
return True
@ -5585,7 +5590,7 @@ def cmd_gui(args: argparse.Namespace):
stopped = _stop_desktop_processes_locking_build(desktop_dir)
if stopped:
print(f" ⚠ Stopped running desktop app to free the build output (pid {', '.join(map(str, stopped))})")
build_result = subprocess.run([npm, "run", build_script], cwd=desktop_dir, env=env, check=False)
build_result = subprocess.run([npm, "run", build_script], cwd=desktop_dir, env=env, check=False, creationflags=windows_hide_flags())
if (
build_result.returncode != 0
and not source_mode
@ -5612,7 +5617,7 @@ def cmd_gui(args: argparse.Namespace):
# The purge can't remove a win-unpacked tree whose Hermes.exe
# is still locked by a running instance; stop it before retry.
_stop_desktop_processes_locking_build(desktop_dir)
build_result = subprocess.run([npm, "run", build_script], cwd=desktop_dir, env=env, check=False)
build_result = subprocess.run([npm, "run", build_script], cwd=desktop_dir, env=env, check=False, creationflags=windows_hide_flags())
if (
build_result.returncode != 0
and not source_mode
@ -5628,7 +5633,7 @@ def cmd_gui(args: argparse.Namespace):
if not _electron_dist_ok(PROJECT_ROOT):
_redownload_electron_dist(PROJECT_ROOT, env, mirror=mirror)
_stop_desktop_processes_locking_build(desktop_dir)
build_result = subprocess.run([npm, "run", build_script], cwd=desktop_dir, env=mirror_env, check=False)
build_result = subprocess.run([npm, "run", build_script], cwd=desktop_dir, env=mirror_env, check=False, creationflags=windows_hide_flags())
if build_result.returncode != 0:
print("✗ Desktop GUI build failed")
print(f" Run manually: cd apps/desktop && npm run {build_script}")
@ -5670,7 +5675,7 @@ def cmd_gui(args: argparse.Namespace):
if source_mode:
print("→ Launching Hermes Desktop from source build...")
launch_result = subprocess.run([npm, "exec", "--", "electron", "."], cwd=desktop_dir, env=env, check=False)
launch_result = subprocess.run([npm, "exec", "--", "electron", "."], cwd=desktop_dir, env=env, check=False, creationflags=windows_hide_flags())
sys.exit(launch_result.returncode)
if packaged_executable is None:
@ -5682,7 +5687,7 @@ def cmd_gui(args: argparse.Namespace):
sys.exit(1)
print(f"→ Launching packaged Hermes Desktop: {packaged_executable}")
launch_result = subprocess.run([str(packaged_executable)], cwd=desktop_dir, env=env, check=False)
launch_result = subprocess.run([str(packaged_executable)], cwd=desktop_dir, env=env, check=False, creationflags=windows_hide_flags())
sys.exit(launch_result.returncode)
@ -6226,6 +6231,7 @@ def _update_via_zip(args):
[sys.executable, "-m", "ensurepip", "--upgrade", "--default-pip"],
cwd=PROJECT_ROOT,
check=True,
creationflags=windows_hide_flags(),
)
_install_python_dependencies_with_optional_fallback(pip_cmd)
@ -6315,6 +6321,7 @@ def _stash_local_changes_if_needed(git_cmd: list[str], cwd: Path) -> Optional[st
git_cmd + ["stash", "push", "--include-untracked", "-m", stash_name],
cwd=cwd,
check=True,
creationflags=windows_hide_flags(),
)
stash_ref = subprocess.run(
git_cmd + ["rev-parse", "--verify", "refs/stash"],
@ -6735,6 +6742,7 @@ def _sync_with_upstream_if_needed(git_cmd: list[str], cwd: Path) -> None:
git_cmd + ["pull", "--ff-only", "upstream", "main"],
cwd=cwd,
check=True,
creationflags=windows_hide_flags(),
)
except subprocess.CalledProcessError:
print(
@ -7006,6 +7014,7 @@ def _run_install_with_heartbeat(
cwd=PROJECT_ROOT,
check=True,
env=env,
creationflags=windows_hide_flags(),
)
finally:
done.set()
@ -7803,6 +7812,7 @@ def _ensure_uv_for_termux(pip_cmd: list[str]) -> str | None:
pip_cmd + ["install", "uv", "--only-binary", ":all:"],
cwd=PROJECT_ROOT,
check=False,
creationflags=windows_hide_flags(),
)
if result.returncode != 0:
return None
@ -8904,7 +8914,7 @@ def _cmd_update_pip(args):
cmd = [sys.executable, "-m", "pip", "install", "--upgrade", "hermes-agent"]
print(f"→ Running: {' '.join(cmd)}")
run_kwargs = {}
run_kwargs = {"creationflags": windows_hide_flags()}
if export_virtualenv:
run_kwargs["env"] = {**os.environ, "VIRTUAL_ENV": sys.prefix}
result = subprocess.run(cmd, **run_kwargs)
@ -9377,6 +9387,7 @@ def _cmd_update_impl(args, gateway_mode: bool):
[sys.executable, "-m", "ensurepip", "--upgrade", "--default-pip"],
cwd=PROJECT_ROOT,
check=True,
creationflags=windows_hide_flags(),
)
if _is_termux_env():
install_group = "termux-all"
@ -11468,7 +11479,7 @@ def cmd_dashboard(args):
# re-executing the dashboard for a non-default profile. Use
# subprocess.Popen + sys.exit() on Windows to avoid the crash.
if sys.platform == "win32":
proc = subprocess.Popen(reexec_argv, env=env)
proc = subprocess.Popen(reexec_argv, env=env) # windows-footgun: ok — foreground re-exec, child owns the console
sys.exit(proc.wait())
else:
os.execvpe(sys.executable, reexec_argv, env)

View file

@ -41,6 +41,7 @@ from hermes_cli.config import (
save_env_value,
)
from hermes_cli.cli_output import prompt as _prompt_input
from hermes_cli._subprocess_compat import windows_hide_flags
_MANIFEST_VERSION = 1
@ -364,7 +365,7 @@ def _run_bootstrap(cwd: Path, commands: List[str]) -> None:
"""
for cmd in commands:
print(color(f" $ {cmd}", Colors.DIM))
proc = subprocess.run(cmd, cwd=str(cwd), shell=True)
proc = subprocess.run(cmd, cwd=str(cwd), shell=True, creationflags=windows_hide_flags())
if proc.returncode != 0:
raise CatalogError(
f"bootstrap step failed (exit {proc.returncode}): {cmd}"
@ -399,6 +400,7 @@ def _do_git_install(entry: CatalogEntry) -> Path:
if not is_sha_ref:
proc = subprocess.run(
[git, "clone", "--depth", "1", "--branch", install.ref, install.url, str(dest)],
creationflags=windows_hide_flags(),
)
if proc.returncode == 0:
pass
@ -410,10 +412,10 @@ def _do_git_install(entry: CatalogEntry) -> Path:
is_sha_ref = True # treat the same as a SHA ref from here
if is_sha_ref:
proc = subprocess.run([git, "clone", install.url, str(dest)])
proc = subprocess.run([git, "clone", install.url, str(dest)], creationflags=windows_hide_flags())
if proc.returncode != 0:
raise CatalogError(f"git clone failed for {install.url}")
proc = subprocess.run([git, "-C", str(dest), "checkout", install.ref])
proc = subprocess.run([git, "-C", str(dest), "checkout", install.ref], creationflags=windows_hide_flags())
if proc.returncode != 0:
raise CatalogError(f"git checkout {install.ref} failed")

View file

@ -185,7 +185,7 @@ def relaunch(
# Windows: subprocess + exit, because execvp can't swap to .cmd/.exe shims.
import subprocess
try:
result = subprocess.run(new_argv)
result = subprocess.run(new_argv) # windows-footgun: ok — re-exec replaces the foreground process
sys.exit(result.returncode)
except KeyboardInterrupt:
sys.exit(130)

View file

@ -160,6 +160,7 @@ from hermes_cli.cli_output import ( # noqa: E402
print_warning,
)
from hermes_cli.secret_prompt import masked_secret_prompt # noqa: E402
from hermes_cli._subprocess_compat import windows_hide_flags
def is_interactive_stdin() -> bool:
@ -805,11 +806,11 @@ def _install_neutts_deps() -> bool:
if prompt_yes_no("Install espeak-ng now?", True):
try:
if sys.platform == "darwin":
subprocess.run(["brew", "install", "espeak-ng"], check=True)
subprocess.run(["brew", "install", "espeak-ng"], check=True, creationflags=windows_hide_flags())
elif sys.platform == "win32":
subprocess.run(["choco", "install", "espeak-ng", "-y"], check=True)
subprocess.run(["choco", "install", "espeak-ng", "-y"], check=True, creationflags=windows_hide_flags())
else:
subprocess.run(["sudo", "apt", "install", "-y", "espeak-ng"], check=True)
subprocess.run(["sudo", "apt", "install", "-y", "espeak-ng"], check=True, creationflags=windows_hide_flags())
print_success("espeak-ng installed")
except (subprocess.CalledProcessError, FileNotFoundError) as e:
print_warning(f"Could not install espeak-ng automatically: {e}")
@ -827,6 +828,7 @@ def _install_neutts_deps() -> bool:
subprocess.run(
[sys.executable, "-m", "pip", "install", "-U", "neutts[all]", "--quiet"],
check=True, timeout=300,
creationflags=windows_hide_flags(),
)
print_success("neutts installed successfully")
return True
@ -852,6 +854,7 @@ def _install_kittentts_deps() -> bool:
subprocess.run(
[sys.executable, "-m", "pip", "install", "-U", wheel_url, "soundfile", "--quiet"],
check=True, timeout=300,
creationflags=windows_hide_flags(),
)
print_success("kittentts installed successfully")
return True

View file

@ -228,6 +228,7 @@ def _checklist_toolset_keys(platform: str) -> Set[str]:
# module shares the same data. Kept as dict-of-dicts for backward
# compatibility with existing ``PLATFORMS[key]["label"]`` access patterns.
from hermes_cli.platforms import PLATFORMS as _PLATFORMS_REGISTRY
from hermes_cli._subprocess_compat import windows_hide_flags
PLATFORMS = {
k: {"label": info.label, "default_toolset": info.default_toolset}
@ -886,7 +887,7 @@ def _run_cua_driver_installer(label: str = "Installing", verbose: bool = True) -
# debuggable. Verbose installs (interactive `computer-use install`)
# keep streaming live.
if verbose:
result = subprocess.run(install_cmd, shell=use_shell, timeout=300, env=_cua_driver_env())
result = subprocess.run(install_cmd, shell=use_shell, timeout=300, env=_cua_driver_env(), creationflags=windows_hide_flags())
else:
result = subprocess.run(
install_cmd, shell=use_shell, timeout=300, env=_cua_driver_env(),

View file

@ -10533,7 +10533,7 @@ async def open_profile_terminal_endpoint(name: str):
command = _profile_setup_command(name)
if sys.platform.startswith("win"):
subprocess.Popen(["cmd.exe", "/c", "start", "", command])
subprocess.Popen(["cmd.exe", "/c", "start", "", command]) # windows-footgun: ok — open terminal for user (Windows branch)
elif sys.platform == "darwin":
escaped = command.replace("\\", "\\\\").replace('"', '\\"')
applescript = (
@ -10542,7 +10542,7 @@ async def open_profile_terminal_endpoint(name: str):
f'do script "{escaped}"\n'
"end tell"
)
subprocess.Popen(["osascript", "-e", applescript])
subprocess.Popen(["osascript", "-e", applescript]) # windows-footgun: ok — open Terminal.app (macOS, visible by design)
else:
terminal_commands = [
("x-terminal-emulator", ["x-terminal-emulator", "-e", "sh", "-lc", command]),
@ -10562,7 +10562,7 @@ async def open_profile_terminal_endpoint(name: str):
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
) == 0:
subprocess.Popen(popen_args)
subprocess.Popen(popen_args) # windows-footgun: ok — open OS terminal for user
break
else:
raise HTTPException(

View file

@ -522,7 +522,8 @@ def _ensure_ollama(models: list[str]) -> bool:
print(f" Pulling '{model}'... (this may take a few minutes)")
try:
subprocess.run([ollama_bin or "ollama", "pull", model], timeout=600,
stdin=subprocess.DEVNULL)
stdin=subprocess.DEVNULL,
creationflags=windows_hide_flags())
print(f" ✓ Model '{model}' pulled")
except Exception as e:
print(f" Warning: Could not pull '{model}': {e}")

View file

@ -679,6 +679,7 @@ class VoiceReceiver:
check=True,
timeout=10,
stdin=subprocess.DEVNULL,
creationflags=windows_hide_flags(),
)
finally:
try:

View file

@ -387,6 +387,7 @@ def _install_sidecar() -> int:
[npm, "ci"],
cwd=str(_SIDECAR_DIR),
check=False,
creationflags=windows_hide_flags(),
)
if proc.returncode != 0:
print(f" npm ci failed — falling back to: {npm} install")
@ -394,6 +395,7 @@ def _install_sidecar() -> int:
[npm, "install"],
cwd=str(_SIDECAR_DIR),
check=False,
creationflags=windows_hide_flags(),
)
if proc.returncode != 0:
print("npm install failed", file=sys.stderr)

View file

@ -20,6 +20,7 @@ import re
import secrets
import shutil
import subprocess
from hermes_cli._subprocess_compat import windows_detach_popen_kwargs
import threading
import time
import uuid
@ -542,7 +543,7 @@ class RaftAdapter(BasePlatformAdapter):
env = {**os.environ, "RAFT_CHANNEL_TOKEN": self._bridge_token}
try:
self._bridge_process = subprocess.Popen(
cmd, env=env, stdin=subprocess.DEVNULL
cmd, env=env, stdin=subprocess.DEVNULL, **windows_detach_popen_kwargs()
)
logger.info("[raft] Spawned bridge pid=%d profile=%s endpoint=%s", self._bridge_process.pid, profile, endpoint)
except Exception:

View file

@ -29,6 +29,7 @@ Suppress an intentional use (e.g. tests or platform-gated code) with:
from __future__ import annotations
import argparse
import ast
import os
import re
import subprocess
@ -327,6 +328,187 @@ FOOTGUNS: list[Footgun] = [
]
# -----------------------------------------------------------------------------
# AST-based rule: subprocess calls that flash a console window on Windows
# -----------------------------------------------------------------------------
#
# This is the high-volume Windows complaint: every `subprocess.run(...)` /
# `subprocess.Popen(...)` of a console program on Windows briefly flashes a
# cmd window unless the child either (a) inherits the parent's stdio handles
# via output redirection, or (b) is spawned with a no-window creationflag
# (CREATE_NO_WINDOW / DETACHED_PROCESS). The fix landscape already exists in
# `hermes_cli/_subprocess_compat.py` (windows_hide_flags / windows_detach_*),
# but nothing stopped new bare calls from re-introducing the popup — so the
# bug kept coming back PR after PR. This rule is the chokepoint.
#
# It is AST-based (not regex) because the deciding factor — whether the call
# redirects stdout/stderr — frequently lives several lines below the
# `subprocess.run(` opener, which a line-oriented regex cannot see.
#
# Comprehensive, not restrictive: a call is only flagged when it can ACTUALLY
# create a new console. Calls that capture or redirect output (capture_output=,
# stdout=, stderr=), or use check_output (which always captures), cannot pop a
# window and are silently ignored — no suppression comment needed. The intent
# is that the overwhelming majority of subprocess calls require no change at
# all; only the genuine window-spawners do.
# The subprocess functions that can spawn a child process.
_SUBPROCESS_FUNCS = frozenset({"run", "Popen", "call", "check_call", "check_output"})
# Module aliases we recognise as the stdlib subprocess module.
_SUBPROCESS_ALIASES = frozenset({"subprocess", "sp"})
# Executables that simply do not exist on Windows. A subprocess call whose
# program is one of these can never create a Windows console window, so the
# no-window flag is irrelevant — flagging them would force pointless
# suppression comments on macOS/Linux-only service-management and packaging
# code (launchctl, systemctl, brew, codesign …). Matched against the FIRST
# element of a list/tuple argv literal only; anything dynamic still gets
# flagged (we can't prove it's POSIX-only).
_POSIX_ONLY_PROGRAMS = frozenset(
{
"launchctl",
"systemctl",
"journalctl",
"loginctl",
"osascript",
"codesign",
"xattr",
"defaults",
"brew",
"apt",
"apt-get",
"dpkg",
"pacman",
"dnf",
"yum",
"sudo",
"open", # macOS `open`
"tail",
"sw_vers",
"scutil",
"diskutil",
"hdiutil",
"dscl",
}
)
SUBPROCESS_FOOTGUN_NAME = "subprocess without Windows no-window flag"
SUBPROCESS_FOOTGUN_MESSAGE = (
"subprocess.run/Popen/call on Windows flashes a console (cmd) window "
"unless the child inherits stdio (output is captured/redirected) or is "
"spawned with a no-window creationflag. This is the #1 source of Windows "
"'terminal popup' bug reports."
)
SUBPROCESS_FOOTGUN_FIX = (
"Pass creationflags=windows_hide_flags() (for short-lived/captured spawns) "
"or **windows_detach_popen_kwargs() (for detached daemons) from "
"hermes_cli._subprocess_compat (both no-op on POSIX). If a visible window "
"is intended (interactive launch, shell hand-off), add "
"'# windows-footgun: ok' on the call line."
)
def _call_attr_name(node: ast.Call) -> str | None:
"""Return 'run'/'Popen'/... when node is subprocess.<func>(...), else None."""
f = node.func
if not isinstance(f, ast.Attribute):
return None
if f.attr not in _SUBPROCESS_FUNCS:
return None
mod = getattr(f.value, "id", None)
if mod not in _SUBPROCESS_ALIASES:
return None
return f.attr
def _suppresses_window(node: ast.Call, func_name: str) -> bool:
"""True if this subprocess call cannot create a new console window.
A child that inherits or redirects the parent's stdio reuses the parent
console no new window is created, so CREATE_NO_WINDOW is moot. We treat
the following as window-safe:
* check_output always captures stdout
* capture_output=... captures both streams
* stdout= or stderr= at least one stream redirected/inherited
* creationflags=... author is already managing the console
* **<spread> kwargs may carry a _subprocess_compat helper;
flag-via-spread is the recommended fix, so we
must not penalise it. (We accept the small
false-negative: a spread that happens to omit
creationflags. The alternative flagging every
**kwargs call would punish the correct fix.)
"""
if func_name == "check_output":
return True
explicit = {kw.arg for kw in node.keywords if kw.arg}
if explicit & {"stdout", "stderr", "capture_output", "creationflags"}:
return True
if any(kw.arg is None for kw in node.keywords): # **kwargs spread
return True
if _is_posix_only_program(node):
return True
return False
def _is_posix_only_program(node: ast.Call) -> bool:
"""True if the call's program is a statically-known POSIX-only executable.
Only inspects a literal list/tuple first arg whose first element is a
string constant (e.g. ``["launchctl", "bootout", target]``). Dynamic
argv (variables, f-strings) is treated as unknown and still flagged.
"""
if not node.args:
return False
first = node.args[0]
if isinstance(first, (ast.List, ast.Tuple)) and first.elts:
head = first.elts[0]
if isinstance(head, ast.Constant) and isinstance(head.value, str):
prog = head.value.rsplit("/", 1)[-1]
return prog in _POSIX_ONLY_PROGRAMS
return False
def scan_subprocess_window_footguns(
path: Path, text: str
) -> list[tuple[int, str, Footgun]]:
"""AST pass: flag subprocess calls that can flash a Windows console.
Honours the same `# windows-footgun: ok` line suppression as the regex
rules. Returns the same (lineno, line, Footgun) shape so results merge
cleanly into scan_file's output.
"""
try:
tree = ast.parse(text)
except SyntaxError:
return []
lines = text.splitlines()
rule = Footgun(
name=SUBPROCESS_FOOTGUN_NAME,
pattern=re.compile(r"^$"), # unused; AST-driven
message=SUBPROCESS_FOOTGUN_MESSAGE,
fix=SUBPROCESS_FOOTGUN_FIX,
)
out: list[tuple[int, str, Footgun]] = []
for node in ast.walk(tree):
if not isinstance(node, ast.Call):
continue
func_name = _call_attr_name(node)
if func_name is None:
continue
if _suppresses_window(node, func_name):
continue
lineno = node.lineno
line = lines[lineno - 1] if 0 <= lineno - 1 < len(lines) else ""
# Inline suppression — check the opener line AND, for multi-line calls,
# any line in the call's span (a developer may mark the closing paren).
end = getattr(node, "end_lineno", lineno) or lineno
span = lines[lineno - 1 : end]
if any(SUPPRESS_MARKER.search(l) for l in span):
continue
out.append((lineno, line.rstrip(), rule))
return out
def should_scan_file(path: Path) -> bool:
"""Return True if this file is in scope for the checker."""
# Skip the excluded dirs
@ -416,6 +598,11 @@ def scan_file(path: Path, footguns: list[Footgun]) -> list[tuple[int, str, Footg
return []
matches: list[tuple[int, str, Footgun]] = []
# AST-based rule (subprocess console-window footgun). Runs only on Python
# source; merges into the same result list as the regex rules below.
if path.suffix in {".py", ".pyw", ".pyi"}:
matches.extend(scan_subprocess_window_footguns(path, text))
# Track whether we're inside a triple-quoted string (docstring/raw block).
# Simple state machine — handles both ''' and """, toggled by the FIRST
# triple-quote we see; we don't try to handle nested or f-string cases.
@ -548,6 +735,12 @@ def print_rules() -> None:
print(f" {fg.message}")
print(f" Fix: {fg.fix}")
print()
# AST-based rule (not in the regex FOOTGUNS list).
n = len(FOOTGUNS) + 1
print(f"{n:2}. {SUBPROCESS_FOOTGUN_NAME} (AST-based)")
print(f" {SUBPROCESS_FOOTGUN_MESSAGE}")
print(f" Fix: {SUBPROCESS_FOOTGUN_FIX}")
print()
def main(argv: list[str]) -> int:

View file

@ -42,6 +42,7 @@ from hermes_cli.psutil_android import (
PsutilAndroidInstallError,
prepare_patched_psutil_sdist,
)
from hermes_cli._subprocess_compat import windows_hide_flags
@ -90,7 +91,7 @@ def main() -> int:
cmd = install_cmd_prefix + ["install", "--no-build-isolation", str(src_root)]
print(f" $ {' '.join(cmd)}")
result = subprocess.run(cmd)
result = subprocess.run(cmd, creationflags=windows_hide_flags())
if result.returncode != 0:
return result.returncode

View file

@ -109,7 +109,7 @@ def test_install_psutil_android_script_uses_patched_tree(tmp_path, monkeypatch,
shutil.copyfile(archive, dest)
return str(dest), None
def fake_subprocess_run(cmd: list[str]):
def fake_subprocess_run(cmd: list[str], **kwargs):
src_root = Path(cmd[-1])
patched = (src_root / "psutil" / "_common.py").read_text(encoding="utf-8")
assert REPLACEMENT in patched

View file

@ -0,0 +1,164 @@
"""Tests for the subprocess console-window rule in check-windows-footguns.py.
These assert behavior contracts of the AST rule which call shapes get
flagged and which are correctly exempt NOT a snapshot of how many sites
the repo currently has. The rule's job: flag subprocess calls that can spawn
a NEW Windows console window, ignore the ones that physically cannot.
"""
from __future__ import annotations
import importlib.util
import sys
from pathlib import Path
import pytest
# The checker lives at scripts/check-windows-footguns.py (hyphenated, not a
# normal importable module name) — load it by path.
_REPO_ROOT = Path(__file__).resolve().parents[2]
_CHECKER_PATH = _REPO_ROOT / "scripts" / "check-windows-footguns.py"
@pytest.fixture(scope="module")
def checker():
spec = importlib.util.spec_from_file_location("_wf_checker", _CHECKER_PATH)
mod = importlib.util.module_from_spec(spec)
# Register before exec so the module's dataclasses can resolve their
# __module__ via sys.modules (dataclasses._is_type looks it up there).
sys.modules["_wf_checker"] = mod
spec.loader.exec_module(mod)
return mod
def _flag(checker, src: str) -> list[int]:
"""Return the line numbers the subprocess rule flags for a source string."""
hits = checker.scan_subprocess_window_footguns(Path("x.py"), src)
return [lineno for (lineno, _line, _fg) in hits]
# --- Calls that SHOULD be flagged (can pop a Windows console) --------------
@pytest.mark.parametrize(
"src",
[
'subprocess.run(["git", "status"])',
'subprocess.Popen(["node", "x.js"])',
'subprocess.call(["npm", "run", "build"])',
'subprocess.check_call(["python", "setup.py"])',
"subprocess.run(cmd)", # dynamic argv, no redirection
'sp.run(["foo"])', # `sp` alias
],
)
def test_flags_bare_window_spawning_calls(checker, src):
assert _flag(checker, src) == [1], src
def test_flags_multiline_call_without_redirection(checker):
src = (
"subprocess.run(\n"
" [npm, 'run', 'build'],\n"
" cwd=desktop_dir,\n"
" check=False,\n"
")\n"
)
assert _flag(checker, src) == [1]
# --- Calls that should NOT be flagged (no new console possible) ------------
@pytest.mark.parametrize(
"src",
[
# output captured / redirected -> inherits parent console, no popup
'subprocess.run(["git", "x"], capture_output=True)',
'subprocess.run(["git", "x"], stdout=subprocess.PIPE)',
'subprocess.run(["git", "x"], stderr=subprocess.DEVNULL)',
# check_output always captures stdout
'subprocess.check_output(["git", "rev-parse", "HEAD"])',
# already managing the console
'subprocess.run(["x"], creationflags=windows_hide_flags())',
# ** spread may carry a helper -> not penalised
"subprocess.Popen(argv, **windows_detach_popen_kwargs())",
"subprocess.run(cmd, **run_kwargs)",
],
)
def test_exempts_window_safe_calls(checker, src):
assert _flag(checker, src) == [], src
@pytest.mark.parametrize(
"src",
[
'subprocess.run(["launchctl", "bootout", target])',
'subprocess.run(["systemctl", "status", svc])',
'subprocess.run(["brew", "install", "espeak-ng"])',
'subprocess.run(["codesign", "--sign", "-", app])',
'subprocess.run(["/usr/bin/sudo", "chmod", "4755", p])', # path-qualified
],
)
def test_exempts_posix_only_programs(checker, src):
"""launchctl/systemctl/brew/etc. don't exist on Windows -> can't pop a
Windows console, so they must not require a creationflag or suppression."""
assert _flag(checker, src) == [], src
def test_inline_suppression_marker(checker):
src = 'subprocess.run(["git", "x"]) # windows-footgun: ok\n'
assert _flag(checker, src) == []
def test_inline_suppression_on_multiline_closing_paren(checker):
src = (
"subprocess.run(\n"
" [npm, 'run', 'build'],\n"
" cwd=d,\n"
") # windows-footgun: ok\n"
)
assert _flag(checker, src) == []
def test_non_subprocess_calls_ignored(checker):
# A .run() on something that isn't the subprocess module is not our concern.
src = "loop.run(coro)\nclient.run()\n"
assert _flag(checker, src) == []
def test_syntax_error_returns_empty(checker):
assert _flag(checker, "def (:\n") == []
def test_repo_is_clean_of_window_footguns(checker):
"""Full-repo invariant: no unsuppressed window-spawning subprocess calls
remain in shippable Python packages. This is the chokepoint the rule
exists to hold."""
roots = [
_REPO_ROOT / d
for d in (
"hermes_cli",
"gateway",
"tools",
"cron",
"agent",
"plugins",
"scripts",
"acp_adapter",
"acp_registry",
)
]
roots = [r for r in roots if r.exists()]
offenders: list[str] = []
for path in checker.iter_files(roots):
if path.suffix not in {".py", ".pyw", ".pyi"}:
continue
try:
text = path.read_text(encoding="utf-8", errors="replace")
except OSError:
continue
for lineno, _line, _fg in checker.scan_subprocess_window_footguns(path, text):
offenders.append(f"{path.relative_to(_REPO_ROOT)}:{lineno}")
assert not offenders, "Unsuppressed Windows console footguns:\n" + "\n".join(
offenders
)

View file

@ -29,6 +29,7 @@ import shutil
import subprocess
import sys
from typing import Any, Dict, List, Optional
from hermes_cli._subprocess_compat import windows_hide_flags
# Platforms with a cua-driver runtime backend (mirrors the toolset platform_gate).
_RUNTIME_PLATFORMS = frozenset({"darwin", "win32", "linux"})
@ -180,6 +181,7 @@ def request_permissions_grant(driver_cmd: Optional[str] = None) -> int:
[binary, "permissions", "grant"],
env=_child_env(),
stdin=subprocess.DEVNULL,
creationflags=windows_hide_flags(),
).returncode
)
except KeyboardInterrupt: # pragma: no cover - interactive

View file

@ -1871,7 +1871,7 @@ def _generate_neutts(text: str, output_path: str, tts_config: Dict[str, Any]) ->
ffmpeg = shutil.which("ffmpeg")
if ffmpeg:
conv_cmd = [ffmpeg, "-i", wav_path, "-y", "-loglevel", "error", output_path]
subprocess.run(conv_cmd, check=True, timeout=30, stdin=subprocess.DEVNULL)
subprocess.run(conv_cmd, check=True, timeout=30, stdin=subprocess.DEVNULL, creationflags=windows_hide_flags())
os.remove(wav_path)
else:
# No ffmpeg — just rename the WAV to the expected path
@ -2050,7 +2050,7 @@ def _generate_piper_tts(text: str, output_path: str, tts_config: Dict[str, Any])
ffmpeg = shutil.which("ffmpeg")
if ffmpeg:
conv_cmd = [ffmpeg, "-i", wav_path, "-y", "-loglevel", "error", output_path]
subprocess.run(conv_cmd, check=True, timeout=30, stdin=subprocess.DEVNULL)
subprocess.run(conv_cmd, check=True, timeout=30, stdin=subprocess.DEVNULL, creationflags=windows_hide_flags())
try:
os.remove(wav_path)
except OSError:
@ -2116,7 +2116,7 @@ def _generate_kittentts(text: str, output_path: str, tts_config: Dict[str, Any])
ffmpeg = shutil.which("ffmpeg")
if ffmpeg:
conv_cmd = [ffmpeg, "-i", wav_path, "-y", "-loglevel", "error", output_path]
subprocess.run(conv_cmd, check=True, timeout=30, stdin=subprocess.DEVNULL)
subprocess.run(conv_cmd, check=True, timeout=30, stdin=subprocess.DEVNULL, creationflags=windows_hide_flags())
os.remove(wav_path)
else:
# No ffmpeg — rename the WAV to the expected path
@ -2812,6 +2812,7 @@ if __name__ == "__main__":
# Registry
# ---------------------------------------------------------------------------
from tools.registry import registry, tool_error
from hermes_cli._subprocess_compat import windows_hide_flags
TTS_SCHEMA = {
"name": "text_to_speech",