Follow-up to #881. cbm_build_win_cmdline() returned false from inside its loop on
buffer overflow without NUL-terminating buf, although its header contract states the
buffer holds a string. No live bug — both call sites (cbm_run_win and the UI
http_server index spawn) ignore buf when the function returns false — but a future
caller trusting the documented contract could read an unterminated (possibly
uninitialized) buffer. Set buf[0]='\0' on the overflow path so it is always a valid
string, and correct the header comment to match.
Reproduce-first: the new win_cmdline_overflow_leaves_empty_string test feeds an argv
that overflows a small cap and asserts buf[0]=='\0'. It is RED on the pre-fix code
(buf[0] holds the first quoted byte '"' = 0x22) and GREEN with the fix.
Also tidies two review nits in the same files:
- fix the stale cbm_cmdline_put comment (it returns pos UNCHANGED on overflow;
the overflow is signalled via *ovf, not an advancing pos),
- bounds-guard the reference CommandLineToArgvW parser in the round-trip test.
Refs: #881
Signed-off-by: Flipper <jacobphilipp@ymail.com>
Fixes the Windows-only smoke-windows (standard + ui) failure in the release dry
run, where index_repository reported outcome=exit_nonzero and "Indexing worker
crashed on a file" on big_templated.hpp. It is not the #424 allocator class and
not big_templated.hpp — the file parses fine; the worker never reaches it.
This is the Windows residual of #838. That issue fixed the CLI parser to accept
the supervisor's raw-JSON-positional + --response-out flag layout, verified with a
PowerShell repro where the JSON argument arrives intact. But the real supervisor
spawns the worker via CreateProcessA, whose command-line re-parse strips the JSON's
quotes, so on Windows the now-fixed parser still receives corrupted JSON.
On Windows the index supervisor spawns the worker as
`<self> cli --index-worker index_repository <args_json> ...`, but cbm_run_win
built the CreateProcess command line by wrapping each argv element in bare quotes
with no escaping. Windows re-parses that single string back into argv and strips
the inner quotes, so the JSON argument {"repo_path":"..."} arrived at the child as
{repo_path:...} — invalid JSON. The worker exited non-zero at arg parse
("repo_path is required"); the supervisor classified exit_nonzero and blamed the
last-marked file (big_templated.hpp), producing the misleading crash message.
POSIX is unaffected: cbm_run_posix passes the argv array straight to execv with no
re-parse, which is why only smoke-windows was red while Linux/macOS indexed the
same fixture fine.
Fix: cbm_build_win_cmdline() applies the Microsoft C runtime quoting rules
(quote-wrap + escape embedded quotes and their preceding backslash runs) so the
child re-parses byte-identical argv. It is defined unconditionally (pure string
logic, no Windows headers) and unit-tested on every platform via a round-trip
against a reference CommandLineToArgvW parser.
Also in this change:
- ui/http_server.c: the UI index spawn had a byte-for-byte duplicate of the same
bug (its own "%s" cmdline). Route it through cbm_build_win_cmdline too.
- mcp/index_supervisor.c: log the worker's raw exit_code and KEEP its log on any
failure (delete only on a clean run). Previously the log was always deleted and
only outcome+signal were logged, so a non-zero worker left nothing to diagnose —
the CI blind spot that hid this bug behind a generic message.
A separate hardening of pipeline/artifact.c's single-quoted git shell-outs (a
different bug class — popen command-string quoting, not CreateProcess argv) is
split into a follow-up PR with its own guard test, per review.
Verified on a CLANG64 build: the supervised big_templated.hpp fixture now indexes
clean (reap outcome=clean exit_code=0); subprocess suite 11/11, affected suites
(git_context, index_resilience, ui, httpd, mcp, security, str_util) green.
Refs: #838, #423
Signed-off-by: Flipper <jacobphilipp@ymail.com>
New src/foundation/subprocess.{h,c}: spawn a child, tail its output, and
classify how it ended — clean / exit-nonzero / crash / hang / killed — from
POSIX WIFSIGNALED/WTERMSIG and the Windows NTSTATUS exception exit codes
(0xC0000005 access violation, 0xC00000FD stack overflow). Adds an EINTR-safe
reap loop, a quiet-timeout that kills and reports a hung child (no new output
within the window), and partial-line-safe log tailing.
Generalized from the ad-hoc index spawn in src/ui/http_server.c so the
crash/hang supervisor can reuse one primitive across platforms. The exit-code
to outcome mapping lives in a pure cbm_proc_classify() so the Windows
crash-code path is unit-tested on every platform, not just an untested
Windows branch.
tests/test_subprocess.c: 13 tests — the classifier on all platforms plus real
POSIX spawn/reap of clean, non-zero, SIGSEGV-crash and hang children. Green
under ASan/UBSan.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Flipper