Skip to content

Commit 28aaa39

Browse files
authored
Merge pull request #1485 from DeusData/fix/1427-exec-no-shell-window
fix(windows): suppress the console window on the last spawn site (#1427)
2 parents 98f292e + cc1d60d commit 28aaa39

3 files changed

Lines changed: 136 additions & 1 deletion

File tree

‎scripts/test.sh‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,9 @@ bash "$ROOT/tests/test_parallel_harness_contract.sh"
230230
echo "=== Step 0j: venue parity contract (one harness, every venue) ==="
231231
bash "$ROOT/tests/test_venue_parity_contract.sh"
232232

233+
echo "=== Step 0k: spawn console-window contract (#1427) ==="
234+
bash "$ROOT/tests/test_spawn_no_window_contract.sh"
235+
233236
# Verify compiler supports target arch
234237
verify_compiler "$CC"
235238

‎src/foundation/compat_fs.c‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,14 @@ int cbm_exec_no_shell(const char *const *argv) {
686686
memset(&pi, 0, sizeof(pi));
687687
si.cb = sizeof(si);
688688

689-
if (!CreateProcessW(NULL, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi)) {
689+
/* CREATE_NO_WINDOW: the third and last spawn site that still needed it
690+
* (#1427). Without it every helper routed through here — git, codesign,
691+
* open — flashes a console window, and under a stdio MCP session with
692+
* auto_watch those steal focus while the user is typing. The other three
693+
* CreateProcessW sites already set it: subprocess.c and cbm_popen_isolated
694+
* via #1448, and the detached daemon spawn in daemon/bootstrap.c, which has
695+
* had it since it was written. */
696+
if (!CreateProcessW(NULL, cmdline, NULL, NULL, FALSE, CREATE_NO_WINDOW, NULL, NULL, &si, &pi)) {
690697
free(cmdline);
691698
return CBM_NOT_FOUND;
692699
}
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
#!/usr/bin/env bash
2+
# Contract: every CreateProcessW spawn must suppress its console window.
3+
#
4+
# This exists because the fix for #1427 was applied THREE times and still missed
5+
# a site. The reported symptom is a console window stealing focus mid-typing on a
6+
# stdio MCP session with auto_watch. The flag that prevents it, CREATE_NO_WINDOW,
7+
# has to be set at each spawn independently, and the sites do not look alike:
8+
#
9+
# daemon/bootstrap.c had it from the start (DETACHED_PROCESS | ... | CREATE_NO_WINDOW)
10+
# subprocess.c added by #1448 (flags variable)
11+
# compat_fs.c :313 added by #1448 (inline literal, popen path)
12+
# compat_fs.c :689 MISSED by #1448 (dwCreationFlags was a bare 0)
13+
#
14+
# The missed one is the helper that runs git / codesign / open, i.e. the one a
15+
# user meets most often. Nobody was wrong to miss it: it is a different function
16+
# from the popen site in the same file, and reviewing "does the PR add the flag"
17+
# says nothing about the sites the PR does not touch. Only a whole-tree property
18+
# catches that, so this asserts the property rather than any one call.
19+
#
20+
# Python, not shell: this needs the 6th argument of a call spanning lines, and
21+
# the enclosing function's flags variable when that argument is not a literal.
22+
set -euo pipefail
23+
24+
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
25+
cd "$ROOT"
26+
27+
python3 - "$ROOT" <<'PY'
28+
import pathlib
29+
import re
30+
import sys
31+
32+
root = pathlib.Path(sys.argv[1])
33+
34+
35+
def strip_comments(text):
36+
"""Blank out C comments, preserving line count and column positions.
37+
38+
Mandatory, not tidiness: the first version of this contract matched the flag
39+
name anywhere near the call, so the explanatory COMMENT sitting above the
40+
fixed call site satisfied it — the test passed with the fix reverted. A
41+
contract that can be satisfied by prose about the contract is a false guard.
42+
"""
43+
out = []
44+
i, n = 0, len(text)
45+
while i < n:
46+
two = text[i:i + 2]
47+
if two == "/*":
48+
j = text.find("*/", i + 2)
49+
j = n if j < 0 else j + 2
50+
out.append("".join(c if c == "\n" else " " for c in text[i:j]))
51+
i = j
52+
elif two == "//":
53+
j = text.find("\n", i)
54+
j = n if j < 0 else j
55+
out.append(" " * (j - i))
56+
i = j
57+
else:
58+
out.append(text[i])
59+
i += 1
60+
return "".join(out)
61+
62+
# A site may opt out only with a justified, at-the-case entry (O10): the exact
63+
# "file:function" plus WHY. Empty on purpose — every current site sets the flag.
64+
ALLOWLIST = {
65+
# "src/foo/bar.c:some_function": "why a console window is required here",
66+
}
67+
68+
FLAG = "CREATE_NO_WINDOW"
69+
failures = []
70+
checked = 0
71+
72+
for path in sorted(root.glob("src/**/*.c")):
73+
raw = path.read_text(encoding="utf-8", errors="replace")
74+
if "CreateProcessW(" not in raw:
75+
continue
76+
# Comments are blanked FIRST, so neither the call scan nor the flag check can
77+
# be satisfied by prose. Line numbers still map to the real file.
78+
text = strip_comments(raw)
79+
lines = text.splitlines()
80+
for idx, line in enumerate(lines):
81+
if "CreateProcessW(" not in line:
82+
continue
83+
checked += 1
84+
85+
# Enclosing function: nearest preceding line that starts in column 0 and
86+
# looks like a definition. Good enough — these files are plain C.
87+
func = "<unknown>"
88+
for back in range(idx, -1, -1):
89+
m = re.match(r"^(?:static\s+)?[A-Za-z_][\w \t*]*\b(\w+)\s*\(", lines[back])
90+
if m and not lines[back].lstrip().startswith(("*", "//", "/*")):
91+
func = m.group(1)
92+
break
93+
rel = path.relative_to(root).as_posix()
94+
key = f"{rel}:{func}"
95+
if key in ALLOWLIST:
96+
continue
97+
98+
# The call may wrap across lines; take a window around it, plus the
99+
# enclosing function body, so a `DWORD flags = ...` assignment counts.
100+
call_window = "\n".join(lines[idx:idx + 4])
101+
body_start = max(0, idx - 60)
102+
body_window = "\n".join(lines[body_start:idx + 4])
103+
104+
if FLAG in call_window or FLAG in body_window:
105+
continue
106+
failures.append(
107+
f"{rel}:{idx + 1} ({func}) spawns without {FLAG}\n"
108+
f" {line.strip()}"
109+
)
110+
111+
if checked == 0:
112+
print("FAIL: no CreateProcessW call sites found — this contract has stopped "
113+
"checking anything (did the spawn layer move?)")
114+
sys.exit(1)
115+
116+
if failures:
117+
print(f"FAIL: {len(failures)} CreateProcessW site(s) can pop a console window (#1427):\n")
118+
for f in failures:
119+
print(f" - {f}")
120+
print(f"\nAdd {FLAG} to dwCreationFlags. If a site genuinely needs a console, "
121+
"add it to ALLOWLIST in this file with the reason.")
122+
sys.exit(1)
123+
124+
print(f"OK: all {checked} CreateProcessW site(s) set {FLAG}")
125+
PY

0 commit comments

Comments
 (0)