Skip to content

Commit ec80e7d

Browse files
test(windows): add regression guard for #697 taskkill quoting
Expose cbm_build_cmdline via compat_fs_internal.h (following the system_info_internal.h "exposed for testing" pattern) and add Windows-only tests in test_security.c that pin the MSVC command-line quoting convention: - the exact #697 case: {"taskkill","/FI","IMAGENAME eq ...exe"} must become taskkill /FI "IMAGENAME eq ...exe" (one quoted token, not three bare words that made taskkill print "Invalid argument - eq") - unquoted simple args, empty-arg -> "", embedded-quote escaping, trailing-backslash doubling before the closing quote Also fills in the previously-empty Windows side of the cbm_exec_no_shell suite with live CreateProcessW exec tests (cmd /c exit N). These run in CI on the existing windows-latest test job, so the Windows spawn path is now covered rather than #ifndef _WIN32-excluded. The quoting logic is pure and deterministic; the seven cmdline cases were validated against the production algorithm under MinGW GCC 6.3. Signed-off-by: ShauryaaSharma <shauryasofficial27@gmail.com>
1 parent 7d9b292 commit ec80e7d

3 files changed

Lines changed: 170 additions & 2 deletions

File tree

src/foundation/compat_fs.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77
#include "foundation/constants.h"
88
#include "foundation/compat_fs.h"
9+
#include "foundation/compat_fs_internal.h"
910

1011
#include <stdio.h>
1112
#include <stdlib.h>
@@ -201,8 +202,9 @@ int cbm_rmdir(const char *path) {
201202
* Returns a heap-allocated wide string, or NULL on allocation failure.
202203
* Quoting follows the MSVC CRT convention: arguments containing spaces,
203204
* tabs, or double-quotes are wrapped in double-quotes, with backslashes
204-
* before a closing quote doubled and the quote itself escaped. */
205-
static wchar_t *cbm_build_cmdline(const char *const *argv) {
205+
* before a closing quote doubled and the quote itself escaped.
206+
* Declared in compat_fs_internal.h so the test suite can drive it. */
207+
wchar_t *cbm_build_cmdline(const char *const *argv) {
206208
/* First pass: compute required buffer size. */
207209
size_t total = 1; /* NUL terminator */
208210
for (int i = 0; argv[i]; i++) {
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* compat_fs_internal.h — Internal helpers exposed for testing.
3+
*
4+
* These functions are implementation details of compat_fs.c; they are
5+
* declared here only so that the test suite can drive them directly.
6+
* Production code outside compat_fs.c should use the public APIs in
7+
* compat_fs.h instead.
8+
*/
9+
#ifndef CBM_FOUNDATION_COMPAT_FS_INTERNAL_H
10+
#define CBM_FOUNDATION_COMPAT_FS_INTERNAL_H
11+
12+
#ifdef _WIN32
13+
14+
#include <wchar.h>
15+
16+
/*
17+
* Build a properly-quoted Windows command line from a NULL-terminated
18+
* argv array. This is the quoting step underlying cbm_exec_no_shell on
19+
* Windows: it is what turns {"taskkill", "/FI", "IMAGENAME eq foo.exe"}
20+
* into `taskkill /FI "IMAGENAME eq foo.exe"` rather than three bare
21+
* tokens (the #697 regression).
22+
*
23+
* Quoting follows the MSVC/CommandLineToArgvW convention: an argument is
24+
* wrapped in double-quotes when it is empty or contains a space, tab, or
25+
* double-quote; backslashes immediately before a quote (literal or the
26+
* closing one) are doubled, and embedded double-quotes are escaped with a
27+
* backslash.
28+
*
29+
* Returns a heap-allocated wide string the caller must free(), or NULL on
30+
* allocation failure.
31+
*/
32+
wchar_t *cbm_build_cmdline(const char *const *argv);
33+
34+
#endif /* _WIN32 */
35+
36+
#endif /* CBM_FOUNDATION_COMPAT_FS_INTERNAL_H */

tests/test_security.c

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@
1212
#include <cypher/cypher.h>
1313
#include "../src/foundation/str_util.h"
1414
#include "../src/foundation/compat_fs.h"
15+
#ifdef _WIN32
16+
#include "../src/foundation/compat_fs_internal.h"
17+
#include <wchar.h>
18+
#include <stdlib.h>
19+
#endif
1520

1621
#include <string.h>
1722
#include <sys/stat.h>
@@ -364,6 +369,118 @@ TEST(exec_no_shell_captures_exit_code) {
364369
PASS();
365370
}
366371

372+
#else /* _WIN32 */
373+
374+
/* ──────────────────────────────────────────────────────────────────
375+
* WINDOWS COMMAND-LINE QUOTING (cbm_build_cmdline)
376+
*
377+
* Regression guard for #697: cbm_exec_no_shell used _spawnvp, whose
378+
* MinGW CRT did not quote arguments containing spaces. The taskkill
379+
* filter "IMAGENAME eq codebase-memory-mcp.exe" was passed as three
380+
* bare tokens, so taskkill printed
381+
* ERROR: Invalid argument/option - 'eq'.
382+
* on every install. cbm_build_cmdline now performs MSVC-convention
383+
* quoting; these tests pin that behaviour so it cannot silently
384+
* regress. The quoting is pure logic, so it runs deterministically in
385+
* CI on the windows-latest test job.
386+
* ────────────────────────────────────────────────────────────────── */
387+
388+
/* Assert that cbm_build_cmdline(argv) produces `expected` (wide). */
389+
#define ASSERT_CMDLINE(argv, expected) \
390+
do { \
391+
wchar_t *_cl = cbm_build_cmdline(argv); \
392+
ASSERT_NOT_NULL(_cl); \
393+
if (wcscmp(_cl, (expected)) != 0) { \
394+
free(_cl); \
395+
FAIL("cbm_build_cmdline produced an unexpected command line"); \
396+
} \
397+
free(_cl); \
398+
} while (0)
399+
400+
TEST(cmdline_taskkill_filter_is_single_quoted_token) {
401+
/* The exact #697 regression: the filter value contains spaces and
402+
* must survive as ONE quoted argument, not three bare words. */
403+
const char *argv[] = {"taskkill", "/FI", "IMAGENAME eq codebase-memory-mcp.exe", NULL};
404+
ASSERT_CMDLINE(argv, L"taskkill /FI \"IMAGENAME eq codebase-memory-mcp.exe\"");
405+
PASS();
406+
}
407+
408+
TEST(cmdline_simple_args_are_not_quoted) {
409+
/* Arguments with no spaces/tabs/quotes stay bare. */
410+
const char *argv[] = {"foo", "bar", "baz", NULL};
411+
ASSERT_CMDLINE(argv, L"foo bar baz");
412+
PASS();
413+
}
414+
415+
TEST(cmdline_single_arg_no_trailing_space) {
416+
const char *argv[] = {"codebase-memory-mcp.exe", NULL};
417+
ASSERT_CMDLINE(argv, L"codebase-memory-mcp.exe");
418+
PASS();
419+
}
420+
421+
TEST(cmdline_empty_arg_becomes_empty_quotes) {
422+
/* An empty argument must be preserved as "" so argv positions line up. */
423+
const char *argv[] = {"cmd", "", "tail", NULL};
424+
ASSERT_CMDLINE(argv, L"cmd \"\" tail");
425+
PASS();
426+
}
427+
428+
TEST(cmdline_embedded_quote_is_escaped) {
429+
/* A literal double-quote is escaped as \" inside the quoted token. */
430+
const char *argv[] = {"echo", "a\"b", NULL};
431+
ASSERT_CMDLINE(argv, L"echo \"a\\\"b\"");
432+
PASS();
433+
}
434+
435+
TEST(cmdline_trailing_backslashes_doubled_before_close_quote) {
436+
/* Per the MSVC convention, backslashes immediately before the closing
437+
* quote are doubled so the quote is not accidentally escaped. The arg
438+
* `C:\dir with space\` becomes "C:\dir with space\\". */
439+
const char *argv[] = {"type", "C:\\dir with space\\", NULL};
440+
ASSERT_CMDLINE(argv, L"type \"C:\\dir with space\\\\\"");
441+
PASS();
442+
}
443+
444+
TEST(cmdline_null_argv_returns_null) {
445+
/* Defensive: builder over an empty argv still yields a valid (empty)
446+
* string rather than crashing. */
447+
const char *argv[] = {NULL};
448+
wchar_t *cl = cbm_build_cmdline(argv);
449+
ASSERT_NOT_NULL(cl);
450+
ASSERT_EQ((int)wcslen(cl), 0);
451+
free(cl);
452+
PASS();
453+
}
454+
455+
#undef ASSERT_CMDLINE
456+
457+
/* ──────────────────────────────────────────────────────────────────
458+
* WINDOWS SHELL-FREE EXECUTION (cbm_exec_no_shell, CreateProcessW path)
459+
*
460+
* Exercises the live CreateProcessW code path end-to-end via cmd.exe so
461+
* the Windows spawn path is not left entirely uncovered by CI.
462+
* ────────────────────────────────────────────────────────────────── */
463+
464+
TEST(exec_no_shell_win_exit_zero) {
465+
const char *argv[] = {"cmd", "/c", "exit 0", NULL};
466+
int rc = cbm_exec_no_shell(argv);
467+
ASSERT_EQ(rc, 0);
468+
PASS();
469+
}
470+
471+
TEST(exec_no_shell_win_captures_exit_code) {
472+
const char *argv[] = {"cmd", "/c", "exit 42", NULL};
473+
int rc = cbm_exec_no_shell(argv);
474+
ASSERT_EQ(rc, 42);
475+
PASS();
476+
}
477+
478+
TEST(exec_no_shell_win_null_argv_returns_error) {
479+
int rc = cbm_exec_no_shell(NULL);
480+
ASSERT_NEQ(rc, 0);
481+
PASS();
482+
}
483+
367484
#endif /* _WIN32 */
368485

369486
/* ══════════════════════════════════════════════════════════════════
@@ -417,5 +534,18 @@ SUITE(security) {
417534
RUN_TEST(exec_no_shell_nonexistent_command);
418535
RUN_TEST(exec_no_shell_null_argv_returns_error);
419536
RUN_TEST(exec_no_shell_captures_exit_code);
537+
#else
538+
/* Windows command-line quoting (regression guard for #697) */
539+
RUN_TEST(cmdline_taskkill_filter_is_single_quoted_token);
540+
RUN_TEST(cmdline_simple_args_are_not_quoted);
541+
RUN_TEST(cmdline_single_arg_no_trailing_space);
542+
RUN_TEST(cmdline_empty_arg_becomes_empty_quotes);
543+
RUN_TEST(cmdline_embedded_quote_is_escaped);
544+
RUN_TEST(cmdline_trailing_backslashes_doubled_before_close_quote);
545+
RUN_TEST(cmdline_null_argv_returns_null);
546+
/* Live CreateProcessW spawn path */
547+
RUN_TEST(exec_no_shell_win_exit_zero);
548+
RUN_TEST(exec_no_shell_win_captures_exit_code);
549+
RUN_TEST(exec_no_shell_win_null_argv_returns_error);
420550
#endif
421551
}

0 commit comments

Comments
 (0)