Skip to content

Commit 079d8dc

Browse files
committed
fix: harden graph comparison identities
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent cf6e620 commit 079d8dc

4 files changed

Lines changed: 336 additions & 16 deletions

File tree

‎src/mcp/mcp.c‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2918,13 +2918,26 @@ static bool compare_parse_arguments(const char *args, char **base_project, char
29182918
return true;
29192919
}
29202920

2921+
static char *sanitize_utf8_lossy(const char *s);
2922+
2923+
static bool compare_add_identity_string(yyjson_mut_doc *doc, yyjson_mut_val *object,
2924+
const char *key, const char *value) {
2925+
char *sanitized = sanitize_utf8_lossy(value);
2926+
if (!sanitized) {
2927+
return false;
2928+
}
2929+
bool ok = yyjson_mut_obj_add_strcpy(doc, object, key, sanitized);
2930+
free(sanitized);
2931+
return ok;
2932+
}
2933+
29212934
static yyjson_mut_val *compare_node_json(yyjson_mut_doc *doc,
29222935
const cbm_graph_node_identity_t *node) {
29232936
yyjson_mut_val *object = yyjson_mut_obj(doc);
29242937
if (!object ||
2925-
!yyjson_mut_obj_add_strcpy(doc, object, "qualified_name", node->qualified_name) ||
2926-
!yyjson_mut_obj_add_strcpy(doc, object, "label", node->label) ||
2927-
!yyjson_mut_obj_add_strcpy(doc, object, "file_path", node->file_path)) {
2938+
!compare_add_identity_string(doc, object, "qualified_name", node->qualified_name) ||
2939+
!compare_add_identity_string(doc, object, "label", node->label) ||
2940+
!compare_add_identity_string(doc, object, "file_path", node->file_path)) {
29282941
return NULL;
29292942
}
29302943
return object;
@@ -2992,8 +3005,8 @@ static bool compare_edge_callback(void *context, bool added,
29923005
yyjson_mut_val *target = doc ? compare_node_json(doc, &edge->target) : NULL;
29933006
bool ok = item && source && target && yyjson_mut_obj_add_val(doc, item, "source", source) &&
29943007
yyjson_mut_obj_add_val(doc, item, "target", target) &&
2995-
yyjson_mut_obj_add_strcpy(doc, item, "type", edge->type) &&
2996-
yyjson_mut_obj_add_strcpy(doc, item, "local_name_gen", edge->local_name_gen);
3008+
compare_add_identity_string(doc, item, "type", edge->type) &&
3009+
compare_add_identity_string(doc, item, "local_name_gen", edge->local_name_gen);
29973010
if (ok) {
29983011
yyjson_mut_doc_set_root(doc, item);
29993012
}

‎src/store/store.c‎

Lines changed: 62 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1227,14 +1227,52 @@ typedef struct {
12271227
bool has_row;
12281228
} graph_edge_cursor_t;
12291229

1230+
#ifdef CBM_ENABLE_TEST_SEAMS
1231+
static atomic_int graph_compare_test_bind_countdown = ATOMIC_VAR_INIT(CBM_NOT_FOUND);
1232+
static atomic_int graph_compare_test_cancel_countdown = ATOMIC_VAR_INIT(CBM_NOT_FOUND);
1233+
1234+
void cbm_store_compare_test_fail_bind_after(int successful_binds) {
1235+
atomic_store(&graph_compare_test_bind_countdown, successful_binds);
1236+
}
1237+
1238+
void cbm_store_compare_test_cancel_after(int successful_checks) {
1239+
atomic_store(&graph_compare_test_cancel_countdown, successful_checks);
1240+
}
1241+
1242+
static bool graph_compare_test_countdown_fires(atomic_int *countdown) {
1243+
int remaining = atomic_load(countdown);
1244+
while (remaining >= 0) {
1245+
int next = remaining == 0 ? CBM_NOT_FOUND : remaining - 1;
1246+
if (atomic_compare_exchange_weak(countdown, &remaining, next)) {
1247+
return remaining == 0;
1248+
}
1249+
}
1250+
return false;
1251+
}
1252+
#endif
1253+
12301254
static bool graph_compare_is_cancelled(const graph_compare_progress_t *progress) {
1255+
#ifdef CBM_ENABLE_TEST_SEAMS
1256+
if (graph_compare_test_countdown_fires(&graph_compare_test_cancel_countdown)) {
1257+
return true;
1258+
}
1259+
#endif
12311260
return progress && progress->cancel && progress->cancel(progress->context);
12321261
}
12331262

12341263
static int graph_compare_progress(void *context) {
12351264
return graph_compare_is_cancelled((const graph_compare_progress_t *)context) ? 1 : 0;
12361265
}
12371266

1267+
static int graph_compare_bind_text(sqlite3_stmt *stmt, int column, const char *value) {
1268+
#ifdef CBM_ENABLE_TEST_SEAMS
1269+
if (graph_compare_test_countdown_fires(&graph_compare_test_bind_countdown)) {
1270+
return SQLITE_NOMEM;
1271+
}
1272+
#endif
1273+
return bind_text(stmt, column, value);
1274+
}
1275+
12381276
/* SQLite's BINARY collation compares the unsigned bytes of the UTF-8 text.
12391277
* Keep the merge comparator byte-for-byte equivalent so neither cursor can
12401278
* be advanced past a row that SQLite considers distinct. */
@@ -1282,7 +1320,11 @@ static int graph_count_rows(cbm_store_t *store, const char *sql, const char *pro
12821320
store_set_error_sqlite(store, "compare count prepare");
12831321
return CBM_STORE_ERR;
12841322
}
1285-
bind_text(stmt, SKIP_ONE, project);
1323+
if (graph_compare_bind_text(stmt, SKIP_ONE, project) != SQLITE_OK) {
1324+
store_set_error(store, "compare count bind failed");
1325+
sqlite3_finalize(stmt);
1326+
return CBM_STORE_ERR;
1327+
}
12861328
int step_rc = sqlite3_step(stmt);
12871329
if (step_rc != SQLITE_ROW) {
12881330
store_set_error_sqlite(store, "compare count");
@@ -1388,35 +1430,46 @@ static int graph_edge_cursor_step(cbm_store_t *store, graph_edge_cursor_t *curso
13881430
static int graph_prepare_node_cursor(cbm_store_t *store, const char *project,
13891431
graph_node_cursor_t *cursor) {
13901432
static const char sql[] =
1391-
"SELECT qualified_name,label,coalesce(file_path,'') FROM nodes WHERE project=?1 "
1433+
"SELECT qualified_name,label,replace(coalesce(file_path,''),'\\','/') "
1434+
"FROM nodes WHERE project=?1 "
13921435
"ORDER BY qualified_name COLLATE BINARY,label COLLATE BINARY,"
1393-
"coalesce(file_path,'') COLLATE BINARY;";
1436+
"replace(coalesce(file_path,''),'\\','/') COLLATE BINARY;";
13941437
if (sqlite3_prepare_v2(store->db, sql, CBM_NOT_FOUND, &cursor->stmt, NULL) != SQLITE_OK) {
13951438
store_set_error_sqlite(store, "compare node prepare");
13961439
return CBM_STORE_ERR;
13971440
}
1398-
bind_text(cursor->stmt, SKIP_ONE, project);
1441+
if (graph_compare_bind_text(cursor->stmt, SKIP_ONE, project) != SQLITE_OK) {
1442+
store_set_error(store, "compare node bind failed");
1443+
sqlite3_finalize(cursor->stmt);
1444+
cursor->stmt = NULL;
1445+
return CBM_STORE_ERR;
1446+
}
13991447
return graph_node_cursor_step(store, cursor);
14001448
}
14011449

14021450
static int graph_prepare_edge_cursor(cbm_store_t *store, const char *project,
14031451
graph_edge_cursor_t *cursor) {
14041452
static const char sql[] =
1405-
"SELECT sn.qualified_name,sn.label,coalesce(sn.file_path,''),"
1406-
"tn.qualified_name,tn.label,coalesce(tn.file_path,''),e.type,"
1453+
"SELECT sn.qualified_name,sn.label,replace(coalesce(sn.file_path,''),'\\','/'),"
1454+
"tn.qualified_name,tn.label,replace(coalesce(tn.file_path,''),'\\','/'),e.type,"
14071455
"coalesce(e.local_name_gen,'') FROM edges e "
14081456
"JOIN nodes sn ON sn.id=e.source_id JOIN nodes tn ON tn.id=e.target_id "
14091457
"WHERE e.project=?1 ORDER BY "
14101458
"sn.qualified_name COLLATE BINARY,sn.label COLLATE BINARY,"
1411-
"coalesce(sn.file_path,'') COLLATE BINARY,"
1459+
"replace(coalesce(sn.file_path,''),'\\','/') COLLATE BINARY,"
14121460
"tn.qualified_name COLLATE BINARY,tn.label COLLATE BINARY,"
1413-
"coalesce(tn.file_path,'') COLLATE BINARY,"
1461+
"replace(coalesce(tn.file_path,''),'\\','/') COLLATE BINARY,"
14141462
"e.type COLLATE BINARY,coalesce(e.local_name_gen,'') COLLATE BINARY;";
14151463
if (sqlite3_prepare_v2(store->db, sql, CBM_NOT_FOUND, &cursor->stmt, NULL) != SQLITE_OK) {
14161464
store_set_error_sqlite(store, "compare edge prepare");
14171465
return CBM_STORE_ERR;
14181466
}
1419-
bind_text(cursor->stmt, SKIP_ONE, project);
1467+
if (graph_compare_bind_text(cursor->stmt, SKIP_ONE, project) != SQLITE_OK) {
1468+
store_set_error(store, "compare edge bind failed");
1469+
sqlite3_finalize(cursor->stmt);
1470+
cursor->stmt = NULL;
1471+
return CBM_STORE_ERR;
1472+
}
14201473
return graph_edge_cursor_step(store, cursor);
14211474
}
14221475

‎src/store/store.h‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,9 +282,12 @@ typedef bool (*cbm_graph_compare_node_fn)(void *context, bool added,
282282
typedef bool (*cbm_graph_compare_edge_fn)(void *context, bool added,
283283
const cbm_graph_edge_identity_t *edge);
284284

285+
#define CBM_GRAPH_COMPARE_GENERATION_SIZE 128
286+
#define CBM_GRAPH_COMPARE_INDEX_MODE_SIZE 32
287+
285288
typedef struct {
286-
char generation[128];
287-
char index_mode[32];
289+
char generation[CBM_GRAPH_COMPARE_GENERATION_SIZE];
290+
char index_mode[CBM_GRAPH_COMPARE_INDEX_MODE_SIZE];
288291
int64_t node_count;
289292
int64_t edge_count;
290293
} cbm_graph_compare_project_t;
@@ -308,6 +311,14 @@ int cbm_store_compare_graphs(cbm_store_t *base_store, const char *base_project,
308311
cbm_graph_compare_node_fn on_node, cbm_graph_compare_edge_fn on_edge,
309312
void *context, cbm_graph_compare_result_t *out);
310313

314+
#ifdef CBM_ENABLE_TEST_SEAMS
315+
/* Deterministic one-shot comparison fault seams. Values count successful
316+
* comparison binds/cancellation checks before the injected event; -1 disables
317+
* the seam. */
318+
void cbm_store_compare_test_fail_bind_after(int successful_binds);
319+
void cbm_store_compare_test_cancel_after(int successful_checks);
320+
#endif
321+
311322
/* ── Lifecycle ──────────────────────────────────────────────────── */
312323

313324
/* Open an in-memory database (for testing). */

0 commit comments

Comments
 (0)