A pass over places where a bound was either miscalculated or missing.
UI response builders (src/ui/http_server.c) mixed the clamping helper
http_appendf with raw cursor indexing. Route the per-line separators and
quotes through the helper too, so the cursor cannot leave the buffer once
output saturates it, and terminate explicitly afterwards — the helper pins
the cursor and then writes nothing, which would otherwise leave the "%s"
reply with no terminator in range. The log buffer was budgeted at
LOG_LINE_MAX + 10 per line while JSON escaping can double every byte, so
size it for the escaped worst case. Job status entries now escape their path
and error fields, which were interpolated raw.
Cypher (src/cypher/cypher.c): the WHERE grammar descends once per nested '('
and once per NOT, so parse depth followed the query text rather than
anything bounded. Track depth on parser_t and refuse past 256 levels with a
parse error. Bounding parse depth also bounds the resulting tree, so the
recursive evaluator inherits the limit.
Gitignore (src/discover/gitignore.c): '**' retries the remainder at every
position and consecutive groups multiply, so match cost is exponential in
the number of groups. Thread a step budget through the matcher and give up
past 20000 steps, reporting no-match so a pathological pattern fails to
ignore rather than ignoring the wrong files. glob_match keeps its name so
recursion_whitelist.h still describes the functions that recurse; the new
non-recursive wrapper that seeds the budget is glob_match_bounded.
Call extraction (internal/cbm/extract_calls.c): extract_fp_callee recursed
once per applied argument, so stack use followed the parse-tree depth of the
indexed file. Rewritten as a left-spine loop. Reassigning node/nk before
continuing leaves the fall-through cases where the recursive form left them,
so behaviour is unchanged and the extraction suite is untouched.
CALLS props (src/pipeline/pass_calls.c): `cap - pos - PAIR_LEN` wraps once
pos reaches cap - PAIR_LEN and stops bounding the copy below. Guard
additively, matching the closing write in the same function.
Shell-bound paths (src/foundation/str_util.*): the three git shell-out sites
each wrap a repo path in cmd.exe-compatible double quotes, where %VAR%,
!VAR! and ^ remain active. Two carried private copies of the check that
rejects those three on Windows and the third used the bare validator.
Promote the stricter form to cbm_validate_shell_path_arg and route all three
through it, so the copies cannot drift apart again.
Tests cover the buffer bounds, the parse-depth refusal, matcher termination
and the validator. The bounds tests run in a forked child so a violation
surfaces as a signal instead of silently.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Closes the gap flagged in the distill review (test_security.c: "NOT proven
here: the full UI repro — listening socket + MSYS2 handle walk under a
single-threaded server"). Test-only; no production code changes.
Guard 1 — deterministic socket-isolation probe (any Windows, RED-able):
popen_isolates_listening_socket (tests/test_security.c) opens a real
inheritable AFD/listening socket — the exact handle class that deadlocked
git in #798 — then spawns THIS binary through cbm_popen (a cmd.exe
grandchild, git's spawn shape) in a new __cbm_sockprobe re-exec mode
(tests/test_main.c). The child reports via exit code whether that socket
handle is live in its address space: isolated spawn -> getsockopt fails ->
exit 0 (GREEN); raw-_popen regression leaks it transitively through
cmd.exe -> getsockopt succeeds -> exit 42 (RED). Verified RED with a local
spawn-inherits-all revert, GREEN with the isolated spawn.
Guard 2 — end-to-end liveness under live UI sockets (tests/test_httpd.c):
- ui_server_list_projects_responds_under_watchdog: POST /rpc list_projects
to the running single-threaded UI server with a client SO_RCVTIMEO
watchdog; a wedge -> no 200 -> hard FAIL, never an infinite CI hang.
- git_context_resolve_no_hang_under_live_ui_sockets: while the UI server
holds live listening/AFD handles in-process, run cbm_git_context_resolve
(the exact path list_projects takes) on a worker thread under a
WaitForSingleObject(30s) watchdog; a hang -> FAIL. SKIPs where git cannot
init a repo via system().
All three run in the existing security/httpd suites on the test-windows CI
leg; no new build wiring.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018g9f9jr5S46BfKaDvDykBp
Signed-off-by: Flipper <jacobphilipp@ymail.com>
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.
Martin Vogel