Windows cbm_popen wrapped _popen, whose CreateProcess(bInheritHandles=TRUE)
leaks every inheritable handle - including the UI listening socket and
Winsock/AFD helpers - into git children. Git-for-Windows' MSYS2 runtime
walks inherited handles with NtQueryObject at startup and deadlocks on
socket/AFD handles, wedging the single-threaded UI server (#798).
Read-mode cbm_popen now spawns via CreateProcessW with STARTUPINFOEXW and
PROC_THREAD_ATTRIBUTE_HANDLE_LIST restricted to the stdout pipe write end
and a NUL handle for stdin/stderr; cbm_pclose reaps the child via a handle
table + WaitForSingleObject + GetExitCodeProcess.
Distilled from #799 with three corrections:
- no silent fallback to _popen: a failed isolated spawn logs a structured
warning (compat.popen_isolated_failed stage/gle/errno) and returns NULL
(all call sites handle NULL); the composed command line is heap-allocated
instead of a fixed 2048-byte buffer whose overflow forced the fallback
- cmd.exe resolved explicitly from %COMSPEC% (GetSystemDirectoryW\cmd.exe
fallback) and passed as lpApplicationName - no search-path lookup
- GetExitCodeProcess failure reports -1 instead of success; a failed NUL
open fails the spawn instead of feeding INVALID_HANDLE_VALUE into
STARTF_USESTDHANDLES
Adds two windows-only regression tests (test_security.c) driving the
cbm_popen/cbm_pclose round-trip against real git and asserting the isolated
path was taken via the cbm_popen_last_was_isolated() hook.
Closes#798
Co-authored-by: Flipper <jacobphilipp@ymail.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Distilled from PR #702 (fixes#697):
- install.ps1: replace the try/catch that swallowed `codebase-memory-mcp
install` failures with a $LASTEXITCODE check that reports the exit
code and fails the installer, instead of silently leaving no coding
agent configured.
- README: document Unblock-File for the Mark-of-the-Web restriction
that blocks the downloaded install.ps1, plus the execution-policy
escape hatch.
- cbm_exec_no_shell (Windows): switch from _spawnvp, whose CRT does not
quote arguments containing spaces (the taskkill filter "IMAGENAME eq
codebase-memory-mcp.exe" arrived as three bare tokens), to
CreateProcessW over a two-pass MSVC-convention quoted command line
(cbm_build_cmdline, exposed for tests via compat_fs_internal.h).
- UTF-8-correct widening: the quoted command line is assembled in UTF-8
bytes and converted once via cbm_utf8_to_wide, so non-ASCII arguments
(e.g. a non-ASCII %USERPROFILE%) survive intact instead of being
byte-widened as Latin-1 mojibake.
- tests: regression guard for the #697 taskkill filter, MSVC quoting
edge cases (empty arg, embedded quote, trailing backslashes), UTF-8
widening guards (2-byte sequence with explicit code points; mixed
2-/3-byte round-trip via cbm_utf8_to_wide), and live CreateProcessW
spawn tests. All Windows-only, exercised by the Windows CI leg.
Co-authored-by: ShauryaaSharma <shauryasofficial27@gmail.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The Windows search path landed in 82a9052 wraps shell args in cmd.exe-
level "powershell -Command \"...'%s'...\"". PowerShell's single quotes
hold the inner '%s' interpolation, but a literal " inside the user-
supplied value can close the cmd.exe outer quote. With ' ; | & $ `
already blocked, that wouldn't reach RCE on its own — but < and > were
unblocked, so a quote-break followed by cmd.exe redirection (e.g.
*">C:\evil.txt") would expose a file-write primitive.
Block " < > unconditionally. The validator's contract is "safe inside
single quotes for shell interpolation"; on POSIX these aren't strictly
necessary (single quotes hold), but on Windows the cmd.exe→powershell
wrapping makes them load-bearing, and unconditional blocking keeps the
validator simple — no ifdef-laddered policy and no surprises if a future
caller invokes it from a different shell context.
Three tests added to tests/test_security.c covering each new char.
