fix(windows): unswallow installer failures, quote CreateProcessW args, document MotW
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>
This commit is contained in:
Martin Vogel
@@ -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=<path>` (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
|
||||
```
|
||||
|
||||
|
||||
+7
-5
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+130
-1
@@ -6,6 +6,7 @@
|
||||
*/
|
||||
#include "foundation/constants.h"
|
||||
#include "foundation/compat_fs.h"
|
||||
#include "foundation/compat_fs_internal.h"
|
||||
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
@@ -197,11 +198,139 @@ 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. Argument
|
||||
* bytes are treated as UTF-8 and converted to wide via cbm_utf8_to_wide,
|
||||
* so non-ASCII arguments (e.g. a non-ASCII %USERPROFILE%) survive intact.
|
||||
* 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);
|
||||
}
|
||||
}
|
||||
|
||||
/* Build the quoted command line in UTF-8 first, then widen it as a
|
||||
* whole via cbm_utf8_to_wide. Every character the quoting logic acts
|
||||
* on (space, tab, '"', '\\') is ASCII and, by UTF-8's design, never
|
||||
* appears inside a multibyte sequence, so operating on raw bytes here
|
||||
* is safe and keeps multibyte argument bytes intact for conversion. */
|
||||
char *buf = (char *)malloc(total);
|
||||
if (!buf) {
|
||||
return NULL;
|
||||
}
|
||||
|
||||
/* Second pass: write the command line bytes. */
|
||||
char *w = buf;
|
||||
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++ = ' ';
|
||||
}
|
||||
if (needs_quote) {
|
||||
*w++ = '"';
|
||||
size_t backslashes = 0;
|
||||
for (const char *p = arg; *p; p++) {
|
||||
if (*p == '\\') {
|
||||
backslashes++;
|
||||
*w++ = '\\';
|
||||
} else if (*p == '"') {
|
||||
/* Double the preceding backslashes, then escape the quote. */
|
||||
for (size_t b = 0; b < backslashes; b++) {
|
||||
*w++ = '\\';
|
||||
}
|
||||
*w++ = '\\';
|
||||
*w++ = '"';
|
||||
backslashes = 0;
|
||||
} else {
|
||||
backslashes = 0;
|
||||
*w++ = *p;
|
||||
}
|
||||
}
|
||||
/* Double trailing backslashes before the closing quote. */
|
||||
for (size_t b = 0; b < backslashes; b++) {
|
||||
*w++ = '\\';
|
||||
}
|
||||
*w++ = '"';
|
||||
} else {
|
||||
for (const char *p = arg; *p; p++) {
|
||||
*w++ = *p;
|
||||
}
|
||||
}
|
||||
}
|
||||
*w = '\0';
|
||||
|
||||
wchar_t *out = cbm_utf8_to_wide(buf);
|
||||
free(buf);
|
||||
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 */
|
||||
|
||||
@@ -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 <wchar.h>
|
||||
|
||||
/*
|
||||
* 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 */
|
||||
@@ -12,6 +12,12 @@
|
||||
#include <cypher/cypher.h>
|
||||
#include "../src/foundation/str_util.h"
|
||||
#include "../src/foundation/compat_fs.h"
|
||||
#ifdef _WIN32
|
||||
#include "../src/foundation/compat_fs_internal.h"
|
||||
#include "../src/foundation/win_utf8.h"
|
||||
#include <wchar.h>
|
||||
#include <stdlib.h>
|
||||
#endif
|
||||
|
||||
#include <string.h>
|
||||
#include <sys/stat.h>
|
||||
@@ -364,6 +370,153 @@ 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();
|
||||
}
|
||||
|
||||
TEST(cmdline_utf8_arg_is_widened_not_latin1) {
|
||||
/* A non-ASCII argument (e.g. a destination under a non-ASCII
|
||||
* %USERPROFILE%) must be decoded as UTF-8, not byte-widened as
|
||||
* Latin-1. Here "caf\xc3\xa9 dir" is UTF-8 for "café dir": the two
|
||||
* bytes C3 A9 must collapse to the single wide code point U+00E9,
|
||||
* not survive as U+00C3 U+00A9. The embedded space also forces
|
||||
* quoting, so this pins both quoting and correct widening at once. */
|
||||
const char *argv[] = {"cd", "caf\xc3\xa9 dir", NULL};
|
||||
const wchar_t expected[] = {L'c', L'd', L' ', L'"', L'c', L'a', L'f',
|
||||
0x00E9, L' ', L'd', L'i', L'r', L'"', L'\0'};
|
||||
ASSERT_CMDLINE(argv, expected);
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(cmdline_utf8_multibyte_roundtrips_via_utf8_to_wide) {
|
||||
/* Round-trip guard across wider multibyte shapes: 2-byte umlauts
|
||||
* (U+00E4, U+00FC) and 3-byte CJK (U+4E16, U+754C). The argument
|
||||
* "t\xc3\xa4st_\xc3\xbcmlaut \xe4\xb8\x96\xe7\x95\x8c" is UTF-8 for
|
||||
* "täst_ümlaut 世界"; its space forces quoting. The built command
|
||||
* line must equal cbm_utf8_to_wide applied to the same fully-quoted
|
||||
* UTF-8 command line — under Latin-1 byte-widening every UTF-8 byte
|
||||
* would surface as its own bogus code point and the compare fails. */
|
||||
const char *argv[] = {"echo", "t\xc3\xa4st_\xc3\xbcmlaut \xe4\xb8\x96\xe7\x95\x8c", NULL};
|
||||
wchar_t *expected =
|
||||
cbm_utf8_to_wide("echo \"t\xc3\xa4st_\xc3\xbcmlaut \xe4\xb8\x96\xe7\x95\x8c\"");
|
||||
wchar_t *actual = cbm_build_cmdline(argv);
|
||||
int ok = (expected != NULL && actual != NULL && wcscmp(actual, expected) == 0);
|
||||
free(expected);
|
||||
free(actual);
|
||||
if (!ok) {
|
||||
FAIL("multibyte UTF-8 arg did not round-trip through cbm_build_cmdline");
|
||||
}
|
||||
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 +570,20 @@ 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);
|
||||
RUN_TEST(cmdline_utf8_arg_is_widened_not_latin1);
|
||||
RUN_TEST(cmdline_utf8_multibyte_roundtrips_via_utf8_to_wide);
|
||||
/* 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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user