fix(mcp): stop quarantining an anonymous temp db on invalid project names (#1425)
A project name failing cbm_validate_project_name made project_db_path() return an empty path, which cbm_store_open_path_query passed to SQLite - and SQLite opens "" as an anonymous temp database. The healthy temp db then failed the integrity check (no projects table) and quarantine_corrupt_store rendered ".corrupt.<hex>" from the empty prefix: a RELATIVE path, dropped as a 4 KB file into whatever directory the daemon was started from, on every such query. The caller only ever saw a clean 'project not found', so nothing pointed at the litter. Two guards, per the reporter's analysis: resolve_store_internal skips the direct open on an empty path and falls through to the existing fallback scan (which can still resolve legacy dbs whose internal name predates validation), and quarantine_corrupt_store refuses an empty path outright (belt-and-braces - nothing at such a path is worth quarantining). Regression test: tools/call search_graph with project "bad name" from a temp cwd asserts the clean not-found error AND that no .corrupt.* file appears in the cwd. RED before (litter created), GREEN after, RED again on revert. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
+16
-2
@@ -2003,6 +2003,14 @@ static bool quarantine_corrupt_store(cbm_mcp_server_t *srv, const char *project,
|
||||
char *backup_out, size_t backup_out_size) {
|
||||
char backup[CBM_SZ_2K];
|
||||
char pending[CBM_SZ_2K];
|
||||
/* #1425 belt-and-braces: an empty store path would render the backup as a
|
||||
* bare relative ".corrupt.<hex>" in the process cwd. There is nothing at
|
||||
* such a path worth quarantining. */
|
||||
if (!path || !path[0]) {
|
||||
cbm_log_error("store.auto_clean_failed", "project", project, "path", "", "reason",
|
||||
"empty store path");
|
||||
return false;
|
||||
}
|
||||
if (!reserve_unique_corrupt_pending(path, pending, sizeof(pending), backup, sizeof(backup))) {
|
||||
cbm_log_error("store.auto_clean_failed", "project", project, "path", path, "reason",
|
||||
"cannot reserve unique backup");
|
||||
@@ -2090,10 +2098,16 @@ static cbm_store_t *resolve_store_internal(cbm_mcp_server_t *srv, const char *pr
|
||||
}
|
||||
|
||||
/* Open project's .db file — query-only open (no SQLITE_OPEN_CREATE) to
|
||||
* prevent ghost .db file creation for unknown/unindexed projects. */
|
||||
* prevent ghost .db file creation for unknown/unindexed projects.
|
||||
* #1425: an invalid project name yields an empty path. SQLite opens ""
|
||||
* as an anonymous temp db, which then fails the integrity check and
|
||||
* quarantines a db that never existed — as a RELATIVE .corrupt.<hex>
|
||||
* file in the daemon's cwd. Skip the direct open entirely; the fallback
|
||||
* scan below still resolves legacy dbs whose internal name predates
|
||||
* validation. */
|
||||
char path[CBM_SZ_1K];
|
||||
project_db_path(project, path, sizeof(path));
|
||||
srv->store = cbm_store_open_path_query(path);
|
||||
srv->store = path[0] ? cbm_store_open_path_query(path) : NULL;
|
||||
if (srv->store) {
|
||||
/* Check DB integrity — back up (never silently delete) a corrupt DB */
|
||||
if (!cbm_store_check_integrity(srv->store)) {
|
||||
|
||||
@@ -2551,6 +2551,53 @@ TEST(tool_trace_call_path_not_found) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* Regression for #1425: a project name that fails validation must produce a
|
||||
* clean "not found" error and NOTHING else. project_db_path() yields "" for
|
||||
* such names; SQLite opens "" as an anonymous temp db, its integrity check
|
||||
* fails, and quarantine rendered "".corrupt.<hex> - a RELATIVE path dropped
|
||||
* into the daemon's cwd on every such query. */
|
||||
TEST(tool_call_invalid_project_name_leaves_no_corrupt_litter_issue1425) {
|
||||
char tmpdir[256];
|
||||
snprintf(tmpdir, sizeof(tmpdir), "/tmp/mcp-litter-XXXXXX");
|
||||
if (!cbm_mkdtemp(tmpdir))
|
||||
FAIL("cbm_mkdtemp failed");
|
||||
char oldcwd[CBM_SZ_1K];
|
||||
if (!cbm_getcwd(oldcwd, sizeof(oldcwd)))
|
||||
FAIL("getcwd failed");
|
||||
if (cbm_chdir(tmpdir) != 0)
|
||||
FAIL("chdir failed");
|
||||
|
||||
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
|
||||
char *resp =
|
||||
cbm_mcp_server_handle(srv, "{\"jsonrpc\":\"2.0\",\"id\":30,\"method\":\"tools/call\","
|
||||
"\"params\":{\"name\":\"search_graph\","
|
||||
"\"arguments\":{\"name_pattern\":\"x\","
|
||||
"\"project\":\"bad name\"}}}");
|
||||
bool clean_error = resp && strstr(resp, "not found") != NULL;
|
||||
free(resp);
|
||||
cbm_mcp_server_free(srv);
|
||||
|
||||
int litter = 0;
|
||||
cbm_dir_t *dir = cbm_opendir(tmpdir);
|
||||
if (dir) {
|
||||
cbm_dirent_t *entry;
|
||||
while ((entry = cbm_readdir(dir)) != NULL) {
|
||||
if (strstr(entry->name, ".corrupt.")) {
|
||||
litter++;
|
||||
}
|
||||
}
|
||||
cbm_closedir(dir);
|
||||
}
|
||||
if (cbm_chdir(oldcwd) != 0)
|
||||
FAIL("chdir back failed");
|
||||
th_rmtree(tmpdir);
|
||||
if (!clean_error)
|
||||
FAIL("invalid project name must produce a clean not-found error");
|
||||
if (litter != 0)
|
||||
FAIL("invalid project name must not quarantine an anonymous temp db into cwd (#1425)");
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(tool_trace_missing_function_name) {
|
||||
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
|
||||
|
||||
@@ -10352,6 +10399,7 @@ SUITE(mcp) {
|
||||
|
||||
/* Tool handlers with validation */
|
||||
RUN_TEST(tool_trace_call_path_not_found);
|
||||
RUN_TEST(tool_call_invalid_project_name_leaves_no_corrupt_litter_issue1425);
|
||||
RUN_TEST(tool_trace_missing_function_name);
|
||||
RUN_TEST(tool_trace_call_path_ambiguous);
|
||||
RUN_TEST(tool_trace_union_records_min_hop_across_seeds);
|
||||
|
||||
Reference in New Issue
Block a user