fix(cli): compare our binary path separator-insensitively in MCP ownership (#1582)
gotspatel's live opencode.json stores our entry with backslashes (`C:\...\codebase-memory-mcp.exe`) while the installer compares its own path with forward slashes — the same file on disk, refused over the separator spelling, so op=mcp_install failed on a correctly-installed machine (and on Windows the dead-path probe rightly reported the binary PRESENT, which turned the mismatch into a hard refusal). Ownership comparison now treats `\` and `/` as equal everywhere and folds case on Windows only, where the filesystem is case-insensitive; POSIX byte-exactness otherwise holds. The annotated entry that names this binary is recognised as already satisfied and preserved byte-for-byte. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
+31
-2
@@ -1672,16 +1672,45 @@ static size_t cbm_json_mcp_ownership_fields(cbm_json_mcp_schema_t schema, const
|
||||
* populated from config content. */
|
||||
static CBM_TLS const char *g_previous_managed_mcp_command = NULL;
|
||||
|
||||
/* Path-shape-insensitive equality for OUR OWN binary path (#1582): clients
|
||||
* and installers spell the same Windows file with different separators (the
|
||||
* entry stores `C:\...\cbm.exe`, the installer compares `C:/.../cbm.exe`),
|
||||
* and Windows filesystems are case-insensitive. Separators always compare
|
||||
* equal; case only folds on Windows. POSIX byte-exactness otherwise holds. */
|
||||
static bool cbm_json_mcp_paths_equal(const char *a, const char *b) {
|
||||
while (*a && *b) {
|
||||
char ca = *a;
|
||||
char cb = *b;
|
||||
if (ca == '\\') {
|
||||
ca = '/';
|
||||
}
|
||||
if (cb == '\\') {
|
||||
cb = '/';
|
||||
}
|
||||
#ifdef _WIN32
|
||||
ca = (char)tolower((unsigned char)ca);
|
||||
cb = (char)tolower((unsigned char)cb);
|
||||
#endif
|
||||
if (ca != cb) {
|
||||
return false;
|
||||
}
|
||||
a++;
|
||||
b++;
|
||||
}
|
||||
return *a == *b;
|
||||
}
|
||||
|
||||
static bool cbm_json_mcp_owned_command(const char *command, const char *expected_binary,
|
||||
const char *previous_managed_binary) {
|
||||
if (!command || command[0] == '\0') {
|
||||
return false;
|
||||
}
|
||||
if (expected_binary && expected_binary[0] && strcmp(command, expected_binary) == 0) {
|
||||
if (expected_binary && expected_binary[0] &&
|
||||
cbm_json_mcp_paths_equal(command, expected_binary)) {
|
||||
return true;
|
||||
}
|
||||
if (previous_managed_binary && previous_managed_binary[0] &&
|
||||
strcmp(command, previous_managed_binary) == 0) {
|
||||
cbm_json_mcp_paths_equal(command, previous_managed_binary)) {
|
||||
return true;
|
||||
}
|
||||
return strcmp(command, "codebase-memory-mcp") == 0 ||
|
||||
|
||||
@@ -3040,6 +3040,40 @@ TEST(cli_opencode_moved_entry_without_authority_refuses_issue1630) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(cli_opencode_owns_backslash_command_issue1582) {
|
||||
/* gotspatel's live file: the entry stores the Windows path with
|
||||
* backslashes while the installer compares its own path with forward
|
||||
* slashes — the same file, refused over the separator spelling. Ownership
|
||||
* comparison must be separator-insensitive. */
|
||||
char tmpdir[256];
|
||||
snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-oc-XXXXXX");
|
||||
if (!cbm_mkdtemp(tmpdir))
|
||||
FAIL("cbm_mkdtemp failed");
|
||||
char configpath[512];
|
||||
snprintf(configpath, sizeof(configpath), "%s/opencode.json", tmpdir);
|
||||
const char *initial = "{\n"
|
||||
" \"mcp\": {\n"
|
||||
" \"codebase-memory-mcp\": {\n"
|
||||
" \"enabled\": true,\n"
|
||||
" \"type\": \"local\",\n"
|
||||
" \"command\": [\"C:\\\\Users\\\\Admin\\\\Programs\\\\"
|
||||
"codebase-memory-mcp\\\\codebase-memory-mcp.exe\"]\n"
|
||||
" }\n"
|
||||
" }\n"
|
||||
"}\n";
|
||||
write_test_file(configpath, initial);
|
||||
ASSERT_EQ(
|
||||
cbm_upsert_opencode_mcp(
|
||||
"C:/Users/Admin/Programs/codebase-memory-mcp/codebase-memory-mcp.exe", configpath),
|
||||
0);
|
||||
const char *data = read_test_file(configpath);
|
||||
ASSERT_NOT_NULL(data);
|
||||
/* Already satisfied: the annotated entry names this binary — preserved. */
|
||||
ASSERT(strcmp(data, initial) == 0);
|
||||
test_rmdir_r(tmpdir);
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(cli_gemini_mcp_install) {
|
||||
/* Port of TestGeminiMCPInstall */
|
||||
char tmpdir[256];
|
||||
@@ -12973,6 +13007,7 @@ SUITE(cli) {
|
||||
RUN_TEST(cli_goose_block_carries_required_name_issue1675);
|
||||
RUN_TEST(cli_editor_mcp_field_repairs_annotated_entry_via_previous_issue1630);
|
||||
RUN_TEST(cli_opencode_moved_entry_without_authority_refuses_issue1630);
|
||||
RUN_TEST(cli_opencode_owns_backslash_command_issue1582);
|
||||
RUN_TEST(cli_gemini_mcp_install);
|
||||
RUN_TEST(cli_openclaw_mcp_install_uses_nested_servers);
|
||||
RUN_TEST(cli_openclaw_mcp_preserves_existing_config);
|
||||
|
||||
Reference in New Issue
Block a user