Skip to content

Commit 7ba26f5

Browse files
committed
fix: sanitize graph comparison metadata
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent 079d8dc commit 7ba26f5

4 files changed

Lines changed: 119 additions & 24 deletions

File tree

‎src/mcp/mcp.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3022,8 +3022,8 @@ static yyjson_mut_val *compare_project_json(yyjson_mut_doc *doc, const char *pro
30223022
const cbm_graph_compare_project_t *metadata) {
30233023
yyjson_mut_val *object = yyjson_mut_obj(doc);
30243024
if (!object || !yyjson_mut_obj_add_strcpy(doc, object, "project", project) ||
3025-
!yyjson_mut_obj_add_strcpy(doc, object, "generation", metadata->generation) ||
3026-
!yyjson_mut_obj_add_strcpy(doc, object, "index_mode", metadata->index_mode) ||
3025+
!compare_add_identity_string(doc, object, "generation", metadata->generation) ||
3026+
!compare_add_identity_string(doc, object, "index_mode", metadata->index_mode) ||
30273027
!yyjson_mut_obj_add_sint(doc, object, "node_count", metadata->node_count) ||
30283028
!yyjson_mut_obj_add_sint(doc, object, "edge_count", metadata->edge_count)) {
30293029
return NULL;

‎src/store/store.c‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1213,8 +1213,11 @@ int cbm_store_rollback(cbm_store_t *s) {
12131213
typedef struct {
12141214
cbm_graph_compare_cancel_fn cancel;
12151215
void *context;
1216+
bool progress_cancelled;
12161217
} graph_compare_progress_t;
12171218

1219+
enum { GRAPH_COMPARE_PROGRESS_INTERVAL = 1000 };
1220+
12181221
typedef struct {
12191222
sqlite3_stmt *stmt;
12201223
cbm_graph_node_identity_t identity;
@@ -1230,6 +1233,7 @@ typedef struct {
12301233
#ifdef CBM_ENABLE_TEST_SEAMS
12311234
static atomic_int graph_compare_test_bind_countdown = ATOMIC_VAR_INIT(CBM_NOT_FOUND);
12321235
static atomic_int graph_compare_test_cancel_countdown = ATOMIC_VAR_INIT(CBM_NOT_FOUND);
1236+
static atomic_bool graph_compare_test_progress_cancel = ATOMIC_VAR_INIT(false);
12331237

12341238
void cbm_store_compare_test_fail_bind_after(int successful_binds) {
12351239
atomic_store(&graph_compare_test_bind_countdown, successful_binds);
@@ -1239,6 +1243,10 @@ void cbm_store_compare_test_cancel_after(int successful_checks) {
12391243
atomic_store(&graph_compare_test_cancel_countdown, successful_checks);
12401244
}
12411245

1246+
void cbm_store_compare_test_cancel_from_progress(bool enabled) {
1247+
atomic_store(&graph_compare_test_progress_cancel, enabled);
1248+
}
1249+
12421250
static bool graph_compare_test_countdown_fires(atomic_int *countdown) {
12431251
int remaining = atomic_load(countdown);
12441252
while (remaining >= 0) {
@@ -1261,7 +1269,25 @@ static bool graph_compare_is_cancelled(const graph_compare_progress_t *progress)
12611269
}
12621270

12631271
static int graph_compare_progress(void *context) {
1264-
return graph_compare_is_cancelled((const graph_compare_progress_t *)context) ? 1 : 0;
1272+
graph_compare_progress_t *progress = (graph_compare_progress_t *)context;
1273+
#ifdef CBM_ENABLE_TEST_SEAMS
1274+
if (atomic_load(&graph_compare_test_progress_cancel)) {
1275+
progress->progress_cancelled = true;
1276+
return true;
1277+
}
1278+
#endif
1279+
bool cancelled = graph_compare_is_cancelled(progress);
1280+
progress->progress_cancelled = cancelled;
1281+
return cancelled;
1282+
}
1283+
1284+
static int graph_compare_progress_interval(void) {
1285+
#ifdef CBM_ENABLE_TEST_SEAMS
1286+
if (atomic_load(&graph_compare_test_progress_cancel)) {
1287+
return 1;
1288+
}
1289+
#endif
1290+
return GRAPH_COMPARE_PROGRESS_INTERVAL;
12651291
}
12661292

12671293
static int graph_compare_bind_text(sqlite3_stmt *stmt, int column, const char *value) {
@@ -1594,8 +1620,10 @@ int cbm_store_compare_graphs(cbm_store_t *base_store, const char *base_project,
15941620
int rc = CBM_STORE_OK;
15951621
bool base_transaction = false;
15961622
bool target_transaction = false;
1597-
sqlite3_progress_handler(base_store->db, 1000, graph_compare_progress, &progress);
1598-
sqlite3_progress_handler(target_store->db, 1000, graph_compare_progress, &progress);
1623+
int progress_interval = graph_compare_progress_interval();
1624+
sqlite3_progress_handler(base_store->db, progress_interval, graph_compare_progress, &progress);
1625+
sqlite3_progress_handler(target_store->db, progress_interval, graph_compare_progress,
1626+
&progress);
15991627

16001628
rc = exec_sql(base_store, "BEGIN;");
16011629
if (rc == CBM_STORE_OK) {
@@ -1622,10 +1650,12 @@ int cbm_store_compare_graphs(cbm_store_t *base_store, const char *base_project,
16221650
rc = graph_compare_edges(base_store, base_project, target_store, target_project, &progress,
16231651
on_edge, out);
16241652
}
1625-
if (rc == CBM_STORE_OK && graph_compare_is_cancelled(&progress)) {
1653+
if (rc == CBM_STORE_OK &&
1654+
(progress.progress_cancelled || graph_compare_is_cancelled(&progress))) {
16261655
rc = CBM_STORE_CANCELLED;
16271656
}
1628-
if (rc != CBM_STORE_OK && graph_compare_is_cancelled(&progress)) {
1657+
if (rc != CBM_STORE_OK &&
1658+
(progress.progress_cancelled || graph_compare_is_cancelled(&progress))) {
16291659
rc = CBM_STORE_CANCELLED;
16301660
}
16311661

‎src/store/store.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,7 @@ int cbm_store_compare_graphs(cbm_store_t *base_store, const char *base_project,
317317
* the seam. */
318318
void cbm_store_compare_test_fail_bind_after(int successful_binds);
319319
void cbm_store_compare_test_cancel_after(int successful_checks);
320+
void cbm_store_compare_test_cancel_from_progress(bool enabled);
320321
#endif
321322

322323
/* ── Lifecycle ──────────────────────────────────────────────────── */

‎tests/test_mcp.c‎

Lines changed: 81 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7956,11 +7956,11 @@ typedef struct {
79567956
const char *file_path;
79577957
} compare_node_spec_t;
79587958

7959-
static bool compare_write_coverage(cbm_store_t *store, const char *project,
7960-
const char *generation) {
7959+
static bool compare_write_coverage_mode(cbm_store_t *store, const char *project,
7960+
const char *generation, const char *index_mode) {
79617961
cbm_coverage_meta_t meta = {
79627962
.generation = generation,
7963-
.index_mode = "full",
7963+
.index_mode = index_mode,
79647964
.recorded_at = "2026-08-28T00:00:00Z",
79657965
.recording_status = "complete",
79667966
.ignored_files_stored = 0,
@@ -7971,6 +7971,11 @@ static bool compare_write_coverage(cbm_store_t *store, const char *project,
79717971
return cbm_store_coverage_replace_ex(store, project, NULL, 0, &meta) == CBM_STORE_OK;
79727972
}
79737973

7974+
static bool compare_write_coverage(cbm_store_t *store, const char *project,
7975+
const char *generation) {
7976+
return compare_write_coverage_mode(store, project, generation, "full");
7977+
}
7978+
79747979
static bool compare_write_fixture_store(const char *path, const char *project, bool target) {
79757980
static const compare_node_spec_t base_nodes[] = {
79767981
{"pkg.common", "Function", "src/common.c"}, {"pkg.base", "Function", "src/base.c"},
@@ -8248,10 +8253,11 @@ static bool compare_write_single_node_store(const char *path, const char *projec
82488253
}
82498254

82508255
static bool compare_write_identity_store(const char *path, const char *project,
8251-
const char *generation, const char *source_qn,
8252-
const char *source_label, const char *source_file,
8253-
const char *target_qn, const char *target_label,
8254-
const char *target_file, const char *edge_type) {
8256+
const char *generation, const char *index_mode,
8257+
const char *source_qn, const char *source_label,
8258+
const char *source_file, const char *target_qn,
8259+
const char *target_label, const char *target_file,
8260+
const char *edge_type) {
82558261
cbm_store_t *store = cbm_store_open_path(path);
82568262
if (!store ||
82578263
cbm_store_upsert_project(store, project, "/tmp/compare-identity-525") != CBM_STORE_OK) {
@@ -8296,7 +8302,7 @@ static bool compare_write_identity_store(const char *path, const char *project,
82968302
};
82978303
ok = cbm_store_insert_edge(store, &edge) > 0;
82988304
}
8299-
ok = ok && compare_write_coverage(store, project, generation) &&
8305+
ok = ok && compare_write_coverage_mode(store, project, generation, index_mode) &&
83008306
cbm_store_prepare_for_publish(store) == CBM_STORE_OK;
83018307
cbm_store_close(store);
83028308
return ok;
@@ -8309,10 +8315,10 @@ TEST(tool_compare_graphs_normalizes_legacy_path_separators_issue525) {
83098315
char target_path[768];
83108316
snprintf(base_path, sizeof(base_path), "%s/sepbase525.db", fixture.cache);
83118317
snprintf(target_path, sizeof(target_path), "%s/septarget525.db", fixture.cache);
8312-
ASSERT_TRUE(compare_write_identity_store(base_path, "sepbase525", "sep-base", "pkg.source",
8313-
"Function", "src\\same.c", "pkg.target", "Function",
8314-
"src\\target.c", "CALLS"));
8315-
ASSERT_TRUE(compare_write_identity_store(target_path, "septarget525", "sep-target",
8318+
ASSERT_TRUE(compare_write_identity_store(base_path, "sepbase525", "sep-base", "full",
8319+
"pkg.source", "Function", "src\\same.c", "pkg.target",
8320+
"Function", "src\\target.c", "CALLS"));
8321+
ASSERT_TRUE(compare_write_identity_store(target_path, "septarget525", "sep-target", "full",
83168322
"pkg.source", "Function", "src/same.c", "pkg.target",
83178323
"Function", "src/target.c", "CALLS"));
83188324

@@ -8358,18 +8364,26 @@ TEST(tool_compare_graphs_sanitizes_legacy_invalid_utf8_issue525) {
83588364
"bad.c";
83598365
static const char replacement_type[] = "\xEF\xBF\xBD"
83608366
"EDGE";
8367+
static const char invalid_generation[] = "utf-\xFF"
8368+
"generation";
8369+
static const char invalid_index_mode[] = "\xFE"
8370+
"full";
8371+
static const char replacement_generation[] = "utf-\xEF\xBF\xBD"
8372+
"generation";
8373+
static const char replacement_index_mode[] = "\xEF\xBF\xBD"
8374+
"full";
83618375

83628376
compare_graphs_fixture_t fixture;
83638377
ASSERT_TRUE(compare_graphs_fixture_open(&fixture));
83648378
char base_path[768];
83658379
char target_path[768];
83668380
snprintf(base_path, sizeof(base_path), "%s/utfbase525.db", fixture.cache);
83678381
snprintf(target_path, sizeof(target_path), "%s/utftarget525.db", fixture.cache);
8368-
ASSERT_TRUE(compare_write_identity_store(base_path, "utfbase525", "utf-base", NULL, NULL, NULL,
8369-
NULL, NULL, NULL, NULL));
8370-
ASSERT_TRUE(compare_write_identity_store(target_path, "utftarget525", "utf-target", invalid_qn,
8371-
invalid_label, invalid_file, safe_qn, "Function",
8372-
"src/sink.c", invalid_type));
8382+
ASSERT_TRUE(compare_write_identity_store(base_path, "utfbase525", "utf-base", "full", NULL,
8383+
NULL, NULL, NULL, NULL, NULL, NULL));
8384+
ASSERT_TRUE(compare_write_identity_store(
8385+
target_path, "utftarget525", invalid_generation, invalid_index_mode, invalid_qn,
8386+
invalid_label, invalid_file, safe_qn, "Function", "src/sink.c", invalid_type));
83738387

83748388
cbm_mcp_server_t *server = cbm_mcp_server_new(NULL);
83758389
ASSERT_NOT_NULL(server);
@@ -8384,6 +8398,9 @@ TEST(tool_compare_graphs_sanitizes_legacy_invalid_utf8_issue525) {
83848398
yyjson_doc *doc = inner ? yyjson_read(inner, strlen(inner), 0) : NULL;
83858399
ASSERT_NOT_NULL(doc);
83868400
yyjson_val *root = yyjson_doc_get_root(doc);
8401+
yyjson_val *target = yyjson_obj_get(root, "target");
8402+
ASSERT_STR_EQ(yyjson_get_str(yyjson_obj_get(target, "generation")), replacement_generation);
8403+
ASSERT_STR_EQ(yyjson_get_str(yyjson_obj_get(target, "index_mode")), replacement_index_mode);
83878404
yyjson_val *items =
83888405
yyjson_obj_get(yyjson_obj_get(yyjson_obj_get(root, "nodes"), "added"), "items");
83898406
bool found_node = false;
@@ -8486,6 +8503,52 @@ TEST(tool_compare_graphs_midscan_cancel_restores_store_state_issue525) {
84868503
PASS();
84878504
}
84888505

8506+
TEST(tool_compare_graphs_progress_cancel_clears_handler_issue525) {
8507+
compare_graphs_fixture_t fixture;
8508+
ASSERT_TRUE(compare_graphs_fixture_open(&fixture));
8509+
cbm_mcp_server_t *server = cbm_mcp_server_new(NULL);
8510+
ASSERT_NOT_NULL(server);
8511+
8512+
cbm_store_compare_test_cancel_from_progress(true);
8513+
char *cancelled = compare_graphs_call(server, NULL);
8514+
ASSERT_NOT_NULL(cancelled);
8515+
ASSERT_NOT_NULL(strstr(cancelled, "\"isError\":true"));
8516+
ASSERT_NOT_NULL(strstr(cancelled, "cancelled"));
8517+
char *inner = extract_text_content(cancelled);
8518+
ASSERT_NOT_NULL(inner);
8519+
ASSERT_NULL(strstr(inner, "\"nodes\""));
8520+
free(inner);
8521+
free(cancelled);
8522+
8523+
cbm_store_t *base_store = cbm_store_open_path_query(fixture.base_path);
8524+
cbm_store_t *target_store = cbm_store_open_path_query(fixture.target_path);
8525+
ASSERT_NOT_NULL(base_store);
8526+
ASSERT_NOT_NULL(target_store);
8527+
cbm_graph_compare_result_t result = {0};
8528+
ASSERT_EQ(cbm_store_compare_graphs(base_store, "base525", target_store, "target525", 100, NULL,
8529+
NULL, NULL, NULL, &result),
8530+
CBM_STORE_CANCELLED);
8531+
ASSERT_EQ(result.nodes_added_total, 0);
8532+
ASSERT_EQ(result.nodes_removed_total, 0);
8533+
ASSERT_EQ(result.edges_added_total, 0);
8534+
ASSERT_EQ(result.edges_removed_total, 0);
8535+
8536+
static const char expensive_query[] =
8537+
"WITH RECURSIVE sequence(value) AS (VALUES(0) UNION ALL "
8538+
"SELECT value+1 FROM sequence WHERE value<5000) SELECT sum(value) FROM sequence;";
8539+
int base_query_rc = cbm_store_exec(base_store, expensive_query);
8540+
int target_query_rc = cbm_store_exec(target_store, expensive_query);
8541+
cbm_store_compare_test_cancel_from_progress(false);
8542+
cbm_store_close(target_store);
8543+
cbm_store_close(base_store);
8544+
ASSERT_EQ(base_query_rc, CBM_STORE_OK);
8545+
ASSERT_EQ(target_query_rc, CBM_STORE_OK);
8546+
8547+
cbm_mcp_server_free(server);
8548+
ASSERT_TRUE(compare_graphs_fixture_close(&fixture));
8549+
PASS();
8550+
}
8551+
84898552
TEST(tool_compare_graphs_enforces_encoded_budget_issue525) {
84908553
compare_graphs_fixture_t fixture;
84918554
ASSERT_TRUE(compare_graphs_fixture_open(&fixture));
@@ -12295,6 +12358,7 @@ SUITE(mcp) {
1229512358
RUN_TEST(tool_compare_graphs_sanitizes_legacy_invalid_utf8_issue525);
1229612359
RUN_TEST(tool_compare_graphs_bind_failures_are_atomic_issue525);
1229712360
RUN_TEST(tool_compare_graphs_midscan_cancel_restores_store_state_issue525);
12361+
RUN_TEST(tool_compare_graphs_progress_cancel_clears_handler_issue525);
1229812362
RUN_TEST(tool_compare_graphs_enforces_encoded_budget_issue525);
1229912363
RUN_TEST(tool_compare_graphs_validation_and_scan_cap_are_atomic_issue525);
1230012364
RUN_TEST(tool_compare_graphs_cancel_and_readonly_handles_release_issue525);

0 commit comments

Comments
 (0)