diff --git a/README.md b/README.md index c1c17f9f8..209953859 100644 --- a/README.md +++ b/README.md @@ -59,11 +59,16 @@ Invoke-WebRequest -Uri https://raw.githubusercontent.com/DeusData/codebase-memor # 2. (Optional but recommended) Inspect the script notepad install.ps1 -# 3. Run it +# 3. Unblock the downloaded file (removes Mark-of-the-Web restriction added by browsers/Invoke-WebRequest) +Unblock-File .\install.ps1 + +# 4. Run it .\install.ps1 ``` +> **Note:** If you see a script execution policy error, run `Set-ExecutionPolicy -Scope Process Bypass` first, or invoke with `PowerShell -ExecutionPolicy Bypass -File .\install.ps1`. + Options: `--ui` (graph visualization), `--skip-config` (binary only, no agent setup), `--dir=` (custom location). Restart your coding agent. Say **"Index this project"** — done. @@ -86,6 +91,7 @@ Restart your coding agent. Say **"Index this project"** — done. Windows (PowerShell): ```powershell Expand-Archive codebase-memory-mcp-windows-amd64.zip -DestinationPath . + Unblock-File .\install.ps1 .\install.ps1 ``` diff --git a/install.ps1 b/install.ps1 index 52668e407..0378903e7 100644 --- a/install.ps1 +++ b/install.ps1 @@ -131,11 +131,13 @@ if ($SkipConfig) { } else { Write-Host "" Write-Host "Configuring coding agents..." - try { - & $Dest install -y 2>&1 | Write-Host - } catch { - Write-Host "Agent configuration failed (non-fatal)." - Write-Host "Run manually: codebase-memory-mcp install" + & $Dest install -y 2>&1 | Write-Host + if ($LASTEXITCODE -ne 0) { + Write-Host "" + Write-Host "error: agent configuration failed (exit code $LASTEXITCODE)" -ForegroundColor Red + Write-Host "The binary was installed, but no coding agents were configured." + Write-Host "Run manually to configure: `"$Dest`" install" + exit 1 } } diff --git a/src/foundation/compat_fs.c b/src/foundation/compat_fs.c index 886f5c459..22f09be56 100644 --- a/src/foundation/compat_fs.c +++ b/src/foundation/compat_fs.c @@ -6,6 +6,7 @@ */ #include "foundation/constants.h" #include "foundation/compat_fs.h" +#include "foundation/compat_fs_internal.h" #include #include @@ -197,11 +198,129 @@ int cbm_rmdir(const char *path) { return ret; } +/* Build a properly-quoted Windows command line from an argv array. + * Returns a heap-allocated wide string, or NULL on allocation failure. + * Quoting follows the MSVC CRT convention: arguments containing spaces, + * tabs, or double-quotes are wrapped in double-quotes, with backslashes + * before a closing quote doubled and the quote itself escaped. + * Declared in compat_fs_internal.h so the test suite can drive it. */ +wchar_t *cbm_build_cmdline(const char *const *argv) { + /* First pass: compute required buffer size. */ + size_t total = 1; /* NUL terminator */ + for (int i = 0; argv[i]; i++) { + const char *arg = argv[i]; + bool needs_quote = (arg[0] == '\0'); + for (const char *p = arg; *p; p++) { + if (*p == ' ' || *p == '\t' || *p == '"') { + needs_quote = true; + } + } + if (i > 0) { + total++; /* space separator */ + } + if (needs_quote) { + total += 2; /* opening and closing quote */ + size_t backslashes = 0; + for (const char *p = arg; *p; p++) { + if (*p == '\\') { + backslashes++; + } else if (*p == '"') { + total += backslashes + 1; /* double backslashes + escape backslash */ + backslashes = 0; + } else { + backslashes = 0; + } + total++; + } + /* Trailing backslashes before closing quote must be doubled. */ + total += backslashes; + } else { + total += strlen(arg); + } + } + + wchar_t *out = (wchar_t *)malloc(total * sizeof(wchar_t)); + if (!out) { + return NULL; + } + + /* Second pass: write the command line. */ + wchar_t *w = out; + for (int i = 0; argv[i]; i++) { + const char *arg = argv[i]; + bool needs_quote = (arg[0] == '\0'); + for (const char *p = arg; *p; p++) { + if (*p == ' ' || *p == '\t' || *p == '"') { + needs_quote = true; + break; + } + } + if (i > 0) { + *w++ = L' '; + } + if (needs_quote) { + *w++ = L'"'; + size_t backslashes = 0; + for (const char *p = arg; *p; p++) { + if (*p == '\\') { + backslashes++; + *w++ = L'\\'; + } else if (*p == '"') { + /* Double the preceding backslashes, then escape the quote. */ + for (size_t b = 0; b < backslashes; b++) { + *w++ = L'\\'; + } + *w++ = L'\\'; + *w++ = L'"'; + backslashes = 0; + } else { + backslashes = 0; + *w++ = (wchar_t)(unsigned char)*p; + } + } + /* Double trailing backslashes before the closing quote. */ + for (size_t b = 0; b < backslashes; b++) { + *w++ = L'\\'; + } + *w++ = L'"'; + } else { + for (const char *p = arg; *p; p++) { + *w++ = (wchar_t)(unsigned char)*p; + } + } + } + *w = L'\0'; + return out; +} + int cbm_exec_no_shell(const char *const *argv) { if (!argv || !argv[0]) { return CBM_NOT_FOUND; } - return (int)_spawnvp(_P_WAIT, argv[0], argv); + + wchar_t *cmdline = cbm_build_cmdline(argv); + if (!cmdline) { + return CBM_NOT_FOUND; + } + + STARTUPINFOW si; + PROCESS_INFORMATION pi; + memset(&si, 0, sizeof(si)); + memset(&pi, 0, sizeof(pi)); + si.cb = sizeof(si); + + if (!CreateProcessW(NULL, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi)) { + free(cmdline); + return CBM_NOT_FOUND; + } + free(cmdline); + + WaitForSingleObject(pi.hProcess, INFINITE); + DWORD exit_code = (DWORD)CBM_NOT_FOUND; + GetExitCodeProcess(pi.hProcess, &exit_code); + CloseHandle(pi.hProcess); + CloseHandle(pi.hThread); + return (int)exit_code; } #else /* POSIX */ diff --git a/src/foundation/compat_fs_internal.h b/src/foundation/compat_fs_internal.h new file mode 100644 index 000000000..47ad53090 --- /dev/null +++ b/src/foundation/compat_fs_internal.h @@ -0,0 +1,36 @@ +/* + * compat_fs_internal.h — Internal helpers exposed for testing. + * + * These functions are implementation details of compat_fs.c; they are + * declared here only so that the test suite can drive them directly. + * Production code outside compat_fs.c should use the public APIs in + * compat_fs.h instead. + */ +#ifndef CBM_FOUNDATION_COMPAT_FS_INTERNAL_H +#define CBM_FOUNDATION_COMPAT_FS_INTERNAL_H + +#ifdef _WIN32 + +#include + +/* + * Build a properly-quoted Windows command line from a NULL-terminated + * argv array. This is the quoting step underlying cbm_exec_no_shell on + * Windows: it is what turns {"taskkill", "/FI", "IMAGENAME eq foo.exe"} + * into `taskkill /FI "IMAGENAME eq foo.exe"` rather than three bare + * tokens (the #697 regression). + * + * Quoting follows the MSVC/CommandLineToArgvW convention: an argument is + * wrapped in double-quotes when it is empty or contains a space, tab, or + * double-quote; backslashes immediately before a quote (literal or the + * closing one) are doubled, and embedded double-quotes are escaped with a + * backslash. + * + * Returns a heap-allocated wide string the caller must free(), or NULL on + * allocation failure. + */ +wchar_t *cbm_build_cmdline(const char *const *argv); + +#endif /* _WIN32 */ + +#endif /* CBM_FOUNDATION_COMPAT_FS_INTERNAL_H */ diff --git a/tests/test_security.c b/tests/test_security.c index 789809fc4..85be3f971 100644 --- a/tests/test_security.c +++ b/tests/test_security.c @@ -12,6 +12,11 @@ #include #include "../src/foundation/str_util.h" #include "../src/foundation/compat_fs.h" +#ifdef _WIN32 +#include "../src/foundation/compat_fs_internal.h" +#include +#include +#endif #include #include @@ -364,6 +369,118 @@ TEST(exec_no_shell_captures_exit_code) { PASS(); } +#else /* _WIN32 */ + +/* ────────────────────────────────────────────────────────────────── + * WINDOWS COMMAND-LINE QUOTING (cbm_build_cmdline) + * + * Regression guard for #697: cbm_exec_no_shell used _spawnvp, whose + * MinGW CRT did not quote arguments containing spaces. The taskkill + * filter "IMAGENAME eq codebase-memory-mcp.exe" was passed as three + * bare tokens, so taskkill printed + * ERROR: Invalid argument/option - 'eq'. + * on every install. cbm_build_cmdline now performs MSVC-convention + * quoting; these tests pin that behaviour so it cannot silently + * regress. The quoting is pure logic, so it runs deterministically in + * CI on the windows-latest test job. + * ────────────────────────────────────────────────────────────────── */ + +/* Assert that cbm_build_cmdline(argv) produces `expected` (wide). */ +#define ASSERT_CMDLINE(argv, expected) \ + do { \ + wchar_t *_cl = cbm_build_cmdline(argv); \ + ASSERT_NOT_NULL(_cl); \ + if (wcscmp(_cl, (expected)) != 0) { \ + free(_cl); \ + FAIL("cbm_build_cmdline produced an unexpected command line"); \ + } \ + free(_cl); \ + } while (0) + +TEST(cmdline_taskkill_filter_is_single_quoted_token) { + /* The exact #697 regression: the filter value contains spaces and + * must survive as ONE quoted argument, not three bare words. */ + const char *argv[] = {"taskkill", "/FI", "IMAGENAME eq codebase-memory-mcp.exe", NULL}; + ASSERT_CMDLINE(argv, L"taskkill /FI \"IMAGENAME eq codebase-memory-mcp.exe\""); + PASS(); +} + +TEST(cmdline_simple_args_are_not_quoted) { + /* Arguments with no spaces/tabs/quotes stay bare. */ + const char *argv[] = {"foo", "bar", "baz", NULL}; + ASSERT_CMDLINE(argv, L"foo bar baz"); + PASS(); +} + +TEST(cmdline_single_arg_no_trailing_space) { + const char *argv[] = {"codebase-memory-mcp.exe", NULL}; + ASSERT_CMDLINE(argv, L"codebase-memory-mcp.exe"); + PASS(); +} + +TEST(cmdline_empty_arg_becomes_empty_quotes) { + /* An empty argument must be preserved as "" so argv positions line up. */ + const char *argv[] = {"cmd", "", "tail", NULL}; + ASSERT_CMDLINE(argv, L"cmd \"\" tail"); + PASS(); +} + +TEST(cmdline_embedded_quote_is_escaped) { + /* A literal double-quote is escaped as \" inside the quoted token. */ + const char *argv[] = {"echo", "a\"b", NULL}; + ASSERT_CMDLINE(argv, L"echo \"a\\\"b\""); + PASS(); +} + +TEST(cmdline_trailing_backslashes_doubled_before_close_quote) { + /* Per the MSVC convention, backslashes immediately before the closing + * quote are doubled so the quote is not accidentally escaped. The arg + * `C:\dir with space\` becomes "C:\dir with space\\". */ + const char *argv[] = {"type", "C:\\dir with space\\", NULL}; + ASSERT_CMDLINE(argv, L"type \"C:\\dir with space\\\\\""); + PASS(); +} + +TEST(cmdline_null_argv_returns_null) { + /* Defensive: builder over an empty argv still yields a valid (empty) + * string rather than crashing. */ + const char *argv[] = {NULL}; + wchar_t *cl = cbm_build_cmdline(argv); + ASSERT_NOT_NULL(cl); + ASSERT_EQ((int)wcslen(cl), 0); + free(cl); + PASS(); +} + +#undef ASSERT_CMDLINE + +/* ────────────────────────────────────────────────────────────────── + * WINDOWS SHELL-FREE EXECUTION (cbm_exec_no_shell, CreateProcessW path) + * + * Exercises the live CreateProcessW code path end-to-end via cmd.exe so + * the Windows spawn path is not left entirely uncovered by CI. + * ────────────────────────────────────────────────────────────────── */ + +TEST(exec_no_shell_win_exit_zero) { + const char *argv[] = {"cmd", "/c", "exit 0", NULL}; + int rc = cbm_exec_no_shell(argv); + ASSERT_EQ(rc, 0); + PASS(); +} + +TEST(exec_no_shell_win_captures_exit_code) { + const char *argv[] = {"cmd", "/c", "exit 42", NULL}; + int rc = cbm_exec_no_shell(argv); + ASSERT_EQ(rc, 42); + PASS(); +} + +TEST(exec_no_shell_win_null_argv_returns_error) { + int rc = cbm_exec_no_shell(NULL); + ASSERT_NEQ(rc, 0); + PASS(); +} + #endif /* _WIN32 */ /* ══════════════════════════════════════════════════════════════════ @@ -417,5 +534,18 @@ SUITE(security) { RUN_TEST(exec_no_shell_nonexistent_command); RUN_TEST(exec_no_shell_null_argv_returns_error); RUN_TEST(exec_no_shell_captures_exit_code); +#else + /* Windows command-line quoting (regression guard for #697) */ + RUN_TEST(cmdline_taskkill_filter_is_single_quoted_token); + RUN_TEST(cmdline_simple_args_are_not_quoted); + RUN_TEST(cmdline_single_arg_no_trailing_space); + RUN_TEST(cmdline_empty_arg_becomes_empty_quotes); + RUN_TEST(cmdline_embedded_quote_is_escaped); + RUN_TEST(cmdline_trailing_backslashes_doubled_before_close_quote); + RUN_TEST(cmdline_null_argv_returns_null); + /* Live CreateProcessW spawn path */ + RUN_TEST(exec_no_shell_win_exit_zero); + RUN_TEST(exec_no_shell_win_captures_exit_code); + RUN_TEST(exec_no_shell_win_null_argv_returns_error); #endif }