From e4f9735dea30dcc799ca5153a666ecdf9a00ca00 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 25 Sep 2026 20:08:36 +0200 Subject: [PATCH 01/15] feat(cross-repo): report run status and skipped legacy stores (#2133) Two follow-ups to the #2133 wildcard fix, both about what cross-repo intelligence tells the caller. 1. "Never run" vs "ran, no links". An empty CROSS_* set used to mean either one, and no output surface could tell them apart. Every run now records a marker in the SOURCE store's store_meta (key cross_repo_last_run:, same per-project key pattern as coverage_shadow_fp:) once the previous CROSS_* generation has been cleaned: last_run_at, outcome (complete|cancelled|failed), targets, projects_scanned, total_cross_edges, skipped_projects. index_status and the UI layout (next to linked_projects) now carry a `cross_repo` object: {"status":"never_run"} or {"status":"ran", ...}. The marker lives beside the edges it describes, so a reindex that rebuilds the store drops both together. No new database files and no new freshness key. New store API cbm_store_meta_get/put. put seeds a missing store_meta through the generation path, because creating the table without db_uid/mutation_gen would make cbm_store_generation() report malformed metadata. 2. ["*"] skips a pre-#768 store (since #2343) but only logged it. The result now names each one, and the MCP response lists them: skipped_projects: [{name, reason:"pre_768_schema", hint:"reindex this project"}] (only emitted when non-empty). The cross_repo.project_unusable log line is unchanged. cr_project_exists() is split into cr_project_state() (usable / missing / legacy) so that the enumerator can tell a legacy store apart from a foreign .db file. New allocations go through mem_core. Refs #2133 Signed-off-by: Martin Vogel --- README.md | 2 + src/mcp/mcp.c | 16 ++++ src/pipeline/pass_cross_repo.c | 169 ++++++++++++++++++++++++++++++--- src/pipeline/pass_cross_repo.h | 24 +++++ src/store/store.c | 71 ++++++++++++++ src/store/store.h | 11 +++ src/ui/http_server.c | 4 + tests/test_cross_repo.c | 93 ++++++++++++++++++ tests/test_mcp.c | 97 +++++++++++++++++++ 9 files changed, 476 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 7564427ea..2500d6c51 100644 --- a/README.md +++ b/README.md @@ -213,6 +213,8 @@ The install script placed beside the binary is **reported, not deleted** — uni - **`CROSS_*` edges** link nodes across multiple repos indexed under the same store - **Multi-galaxy 3D UI layout** for cross-repo architecture visualization - **Cross-repo architecture summary** combining services, routes, and dependencies across the indexed fleet +- **Run status**: `index_status` (and the UI layout next to `linked_projects`) carries a `cross_repo` object, `{"status":"never_run"}` until `index_repository(mode="cross-repo-intelligence")` has run with that project as the source, then `"ran"` with `last_run_at`, `outcome`, `targets`, `projects_scanned`, `total_cross_edges` and `skipped_projects`. That is how you tell "never linked" apart from "linked, nothing matched". The status is stored in the project's own index next to its `CROSS_*` edges, so a reindex that rebuilds the index drops both at once and the status goes back to `never_run`. +- **Skipped stores**: `target_projects: ["*"]` skips an index built before the #768 schema change rather than failing the whole run. The response lists each one as `skipped_projects: [{"name", "reason": "pre_768_schema", "hint": "reindex this project"}]`. ### Edge types (selected) - `CALLS` — a callable is invoked at the source site diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 2ed29799b..e67595882 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -6906,6 +6906,7 @@ static char *handle_index_status(cbm_mcp_server_t *srv, const char *args) { } add_coverage_report(doc, root, store, project, have_proj_info ? proj_info.indexed_at : NULL, coverage_samples); + (void)cbm_cross_repo_add_status_json(doc, root, store, project); safe_str_free(&proj_info.name); safe_str_free(&proj_info.indexed_at); safe_str_free(&proj_info.root_path); @@ -10054,6 +10055,7 @@ static char *handle_cross_repo_mode(cbm_mcp_server_t *srv, const char *repo_path yyjson_doc_free(jdoc); if (result.no_targets || result.failed) { + cbm_cross_repo_result_free(&result); free(project); return cbm_mcp_text_result( result.no_targets @@ -10091,9 +10093,23 @@ static char *handle_cross_repo_mode(cbm_mcp_server_t *srv, const char *repo_path yyjson_mut_obj_add_int(doc, root, "cross_trpc_calls", result.trpc_edges); yyjson_mut_obj_add_int(doc, root, "total_cross_edges", total); yyjson_mut_obj_add_real(doc, root, "elapsed_ms", result.elapsed_ms); + /* ["*"] skips stores it cannot link (#2133); name them so a partial fleet + * is visible instead of silently smaller. Only emitted when non-empty. */ + if (result.skipped_count > 0) { + yyjson_mut_val *skipped = yyjson_mut_arr(doc); + for (int i = 0; i < result.skipped_count; i++) { + yyjson_mut_val *item = yyjson_mut_obj(doc); + yyjson_mut_obj_add_strcpy(doc, item, "name", result.skipped_projects[i].project); + yyjson_mut_obj_add_str(doc, item, "reason", result.skipped_projects[i].reason); + yyjson_mut_obj_add_str(doc, item, "hint", "reindex this project"); + yyjson_mut_arr_append(skipped, item); + } + yyjson_mut_obj_add_val(doc, root, "skipped_projects", skipped); + } char *json = yy_doc_to_str(doc); yyjson_mut_doc_free(doc); + cbm_cross_repo_result_free(&result); free(project); char *out = cbm_mcp_text_result(json, result.cancelled); free(json); diff --git a/src/pipeline/pass_cross_repo.c b/src/pipeline/pass_cross_repo.c index b86d8cee4..f645db349 100644 --- a/src/pipeline/pass_cross_repo.c +++ b/src/pipeline/pass_cross_repo.c @@ -13,12 +13,14 @@ #include "pipeline/pipeline_internal.h" // cbm_route_canon_path #include "foundation/constants.h" #include "foundation/log.h" +#include "foundation/mem_core.h" #include "foundation/platform.h" #include "foundation/compat.h" #include "foundation/compat_fs.h" #include "foundation/str_util.h" #include +#include #include #include #include @@ -153,27 +155,38 @@ static bool cr_store_has_exact_project(cbm_store_t *store, const char *project) return matches; } -/* True when `project` is a usable cross-repo input: its store holds exactly - * that project AND carries the schema the read-write open in +typedef enum { + CR_PROJECT_USABLE = 0, + CR_PROJECT_MISSING, /* no store, or the store is not exactly this project */ + CR_PROJECT_LEGACY, /* exact project, but a pre-#768 schema (reindex required) */ +} cr_project_state_t; + +/* Classify `project` as a cross-repo input: usable when its store holds + * exactly that project AND carries the schema the read-write open in * cr_open_existing_project requires. Checking only the first let a pre-#768 * store (still readable, so list_projects shows it) pass validation and the * ["*"] enumeration, then abort the whole run at its write open — after the * source's previous CROSS_* generation had already been deleted. The probe is * read-only, so the legacy store is left untouched for its reindex. (#2133) */ -static bool cr_project_exists(const char *project) { +static cr_project_state_t cr_project_state(const char *project) { char path[CR_PATH_BUF]; if (!cr_db_path(project, path, sizeof(path))) { - return false; + return CR_PROJECT_MISSING; } cbm_store_t *store = cbm_store_open_path_query(path); - bool exists = cr_store_has_exact_project(store, project); - if (exists && !cbm_store_edges_schema_current(store)) { + cr_project_state_t state = + cr_store_has_exact_project(store, project) ? CR_PROJECT_USABLE : CR_PROJECT_MISSING; + if (state == CR_PROJECT_USABLE && !cbm_store_edges_schema_current(store)) { cbm_log_warn("cross_repo.project_unusable", "project", project, "reason", "pre_768_schema_reindex_required"); - exists = false; + state = CR_PROJECT_LEGACY; } cbm_store_close(store); - return exists; + return state; +} + +static bool cr_project_exists(const char *project) { + return cr_project_state(project) == CR_PROJECT_USABLE; } static cbm_store_t *cr_open_existing_project(const char *project) { @@ -1139,8 +1152,33 @@ static cr_match_result_t match_typed_routes(cbm_store_t *src_store, const char * static void free_project_list(char **projects, int count); +/* Remember a store the ["*"] enumeration skipped, so the caller can name it + * instead of the run silently covering fewer projects than exist. */ +static bool cr_record_skip(cbm_cross_repo_result_t *result, const char *project, + const char *reason) { + if (!result) { + return true; + } + cbm_cross_repo_skip_t *grown = + cbm_realloc(CBM_MEM_CLASS_OTHER, result->skipped_projects, + ((size_t)result->skipped_count + 1U) * sizeof(*result->skipped_projects)); + if (!grown) { + return false; + } + result->skipped_projects = grown; + char *name = cbm_mem_strdup(CBM_MEM_CLASS_OTHER, project); + if (!name) { + return false; + } + grown[result->skipped_count].project = name; + grown[result->skipped_count].reason = reason; + result->skipped_count++; + return true; +} + /* When target_projects = ["*"], scan the cache directory for all .db files. */ -static int collect_all_projects(char ***out, cr_run_context_t *ctx) { +static int collect_all_projects(char ***out, cr_run_context_t *ctx, + cbm_cross_repo_result_t *result) { *out = NULL; const char *dir = cr_cache_dir(); cbm_dir_t *d = cbm_opendir(dir); @@ -1188,7 +1226,15 @@ static int collect_all_projects(char ***out, cr_run_context_t *ctx) { } memcpy(project, ent->name, project_length); project[project_length] = '\0'; - if (!cbm_validate_project_name(project) || !cr_project_exists(project)) { + if (!cbm_validate_project_name(project)) { + continue; + } + cr_project_state_t state = cr_project_state(project); + if (state == CR_PROJECT_LEGACY && !cr_record_skip(result, project, "pre_768_schema")) { + failed = true; + break; + } + if (state != CR_PROJECT_USABLE) { continue; } if (count >= cap) { @@ -1273,6 +1319,106 @@ static int collect_named_projects(const char **targets, int target_count, char * return unique_count; } +/* ── Last-run marker ─────────────────────────────────────────────── */ + +static bool cr_status_key(const char *project, char *buf, size_t bufsz) { + int written = snprintf(buf, bufsz, "cross_repo_last_run:%s", project); + return written > 0 && (size_t)written < bufsz; +} + +/* Record this run in the SOURCE store's store_meta, next to the CROSS_* + * edges it just rewrote: a reindex that rebuilds the store drops both + * together, so the marker can never outlive the links it describes. Written + * once the source's previous CROSS_* generation was cleaned, whatever the + * outcome, because from then on the old summary no longer matches the graph. + * Project names are validated ([A-Za-z0-9._-]) or "*", so no JSON escaping is + * needed. Best-effort: a marker failure is logged, never fails the run. */ +static void cr_record_last_run(cbm_store_t *src_store, const char *project, const char **targets, + int target_count, const cbm_cross_repo_result_t *result) { + char key[CR_QN_BUF]; + if (!cr_status_key(project, key, sizeof(key))) { + return; + } + size_t cap = CR_PROPS_BUF; + for (int i = 0; i < target_count; i++) { + cap += (targets[i] ? strlen(targets[i]) : 0) + CR_COL_4; + } + char *json = cbm_alloc(CBM_MEM_CLASS_OTHER, cap); + if (!json) { + return; + } + char when[CBM_SZ_32]; + time_t now = time(NULL); + struct tm tm_now; + if (!cbm_gmtime_r(&now, &tm_now) || + strftime(when, sizeof(when), "%Y-%m-%dT%H:%M:%SZ", &tm_now) == 0) { + when[0] = '\0'; + } + const char *outcome = result->failed ? "failed" : result->cancelled ? "cancelled" : "complete"; + size_t len = (size_t)snprintf( + json, cap, "{\"last_run_at\":\"%s\",\"outcome\":\"%s\",\"targets\":[", when, outcome); + for (int i = 0; i < target_count && len < cap; i++) { + len += (size_t)snprintf(json + len, cap - len, "%s\"%s\"", i ? "," : "", + targets[i] ? targets[i] : ""); + } + int total = result->http_edges + result->async_edges + result->channel_edges + + result->grpc_edges + result->graphql_edges + result->trpc_edges; + if (len < cap) { + len += (size_t)snprintf(json + len, cap - len, + "],\"projects_scanned\":%d,\"total_cross_edges\":%d," + "\"skipped_projects\":%d}", + result->projects_scanned, total, result->skipped_count); + } + if (len >= cap || cbm_store_meta_put(src_store, key, json) != CBM_STORE_OK) { + cbm_log_warn("cross_repo.status_unrecorded", "project", project); + } + cbm_free(CBM_MEM_CLASS_OTHER, json); +} + +bool cbm_cross_repo_add_status_json(struct yyjson_mut_doc *doc, struct yyjson_mut_val *parent, + cbm_store_t *store, const char *project) { + char key[CR_QN_BUF]; + if (!doc || !parent || !store || !project || !cr_status_key(project, key, sizeof(key))) { + return false; + } + char *raw = NULL; + int rc = cbm_store_meta_get(store, key, &raw); + yyjson_doc *recorded = rc == CBM_STORE_OK ? yyjson_read(raw, strlen(raw), 0) : NULL; + cbm_free(CBM_MEM_CLASS_STORE, raw); + yyjson_val *recorded_root = recorded ? yyjson_doc_get_root(recorded) : NULL; + yyjson_mut_val *status = NULL; + if (yyjson_is_obj(recorded_root)) { + status = yyjson_val_mut_copy(doc, recorded_root); + } + if (!status) { + status = yyjson_mut_obj(doc); + } + yyjson_doc_free(recorded); + if (!status) { + return false; + } + /* "never_run" is the honest reading of an absent marker: no run with this + * project as the source has completed its cleanup since the store was + * (re)built. A target-side run still leaves its CROSS_* edges here. */ + const char *state = rc == CBM_STORE_NOT_FOUND ? "never_run" + : yyjson_mut_obj_size(status) > 0 ? "ran" + : "unavailable"; + yyjson_mut_obj_add_str(doc, status, "status", state); + return yyjson_mut_obj_add_val(doc, parent, "cross_repo", status); +} + +void cbm_cross_repo_result_free(cbm_cross_repo_result_t *result) { + if (!result) { + return; + } + for (int i = 0; i < result->skipped_count; i++) { + cbm_free(CBM_MEM_CLASS_OTHER, result->skipped_projects[i].project); + } + cbm_free(CBM_MEM_CLASS_OTHER, result->skipped_projects); + result->skipped_projects = NULL; + result->skipped_count = 0; +} + static cr_run_status_t add_match_count(int *total, cr_match_result_t matched) { *total += matched.count; return matched.status; @@ -1304,7 +1450,7 @@ cbm_cross_repo_result_t cbm_cross_repo_match_cancellable(const char *project, int resolved_count = 0; if (target_count == SKIP_ONE && target_projects[0] && strcmp(target_projects[0], "*") == 0) { - resolved_count = collect_all_projects(&resolved, &run); + resolved_count = collect_all_projects(&resolved, &run, &result); } else { resolved_count = collect_named_projects(target_projects, target_count, &resolved, &run); } @@ -1442,6 +1588,7 @@ cbm_cross_repo_result_t cbm_cross_repo_match_cancellable(const char *project, result.projects_scanned++; } + cr_record_last_run(src_store, project, target_projects, target_count, &result); cbm_store_close(src_store); free_project_list(resolved, resolved_count); diff --git a/src/pipeline/pass_cross_repo.h b/src/pipeline/pass_cross_repo.h index 898283c3e..85c832b2d 100644 --- a/src/pipeline/pass_cross_repo.h +++ b/src/pipeline/pass_cross_repo.h @@ -9,6 +9,12 @@ #include +/* A store the ["*"] enumeration skipped instead of linking. */ +typedef struct { + char *project; /* heap-owned */ + const char *reason; /* static, e.g. "pre_768_schema" (reindex required) */ +} cbm_cross_repo_skip_t; + /* Result of a cross-repo matching run. */ typedef struct { int http_edges; /* CROSS_HTTP_CALLS edges created */ @@ -23,8 +29,26 @@ typedef struct { bool no_targets; /* failed: nothing but the source project resolved (#1133) */ bool cancelled; /* stopped at a bounded cancellation checkpoint */ bool partial_results; /* committed writes before cancellation were retained */ + cbm_cross_repo_skip_t *skipped_projects; /* ["*"] only; free with ..._result_free */ + int skipped_count; } cbm_cross_repo_result_t; +/* Release the heap-owned parts of a result (safe on a zeroed result). */ +void cbm_cross_repo_result_free(cbm_cross_repo_result_t *result); + +struct yyjson_mut_doc; +struct yyjson_mut_val; + +/* Add a "cross_repo" object to `parent` describing the last cross-repo run + * with `project` as the source, as recorded in that project's store: + * {"status":"never_run"} -- no run recorded since the store was (re)built + * {"status":"ran","last_run_at","outcome","targets","projects_scanned", + * "total_cross_edges","skipped_projects"} + * Read-only (works on a query store). Returns false only on invalid input or + * allocation failure. */ +bool cbm_cross_repo_add_status_json(struct yyjson_mut_doc *doc, struct yyjson_mut_val *parent, + cbm_store_t *store, const char *project); + /* Run cross-repo matching for `project` against `target_projects`. * If target_count == 1 and target_projects[0] == "*", matches against all * indexed projects. Writes CROSS_* edges bidirectionally into both the diff --git a/src/store/store.c b/src/store/store.c index d66ef0ffb..430efd8a3 100644 --- a/src/store/store.c +++ b/src/store/store.c @@ -2456,6 +2456,77 @@ int cbm_store_generation(cbm_store_t *s, char *buf, size_t bufsz) { return CBM_STORE_OK; } +int cbm_store_meta_get(cbm_store_t *s, const char *key, char **out) { + if (!out) { + return CBM_STORE_ERR; + } + *out = NULL; + if (!s || !s->db || !key) { + return CBM_STORE_ERR; + } + sqlite3_stmt *get = NULL; + /* A store that never advanced its generation has no store_meta table: + * that is "no value recorded", not an error. */ + if (sqlite3_prepare_v2(s->db, "SELECT v FROM store_meta WHERE k = ?1;", CBM_NOT_FOUND, &get, + NULL) != SQLITE_OK) { + sqlite3_finalize(get); + return CBM_STORE_NOT_FOUND; + } + bind_text(get, SKIP_ONE, key); + int rc = sqlite3_step(get); + if (rc == SQLITE_ROW) { + const char *value = (const char *)sqlite3_column_text(get, 0); + *out = cbm_mem_strdup(CBM_MEM_CLASS_STORE, value ? value : ""); + sqlite3_finalize(get); + return *out ? CBM_STORE_OK : CBM_STORE_ERR; + } + sqlite3_finalize(get); + return rc == SQLITE_DONE ? CBM_STORE_NOT_FOUND : CBM_STORE_ERR; +} + +int cbm_store_meta_put(cbm_store_t *s, const char *key, const char *value) { + static const char savepoint[] = "SAVEPOINT cbm_meta_put;"; + static const char rollback[] = "ROLLBACK TO cbm_meta_put;"; + static const char release[] = "RELEASE cbm_meta_put;"; + if (!s || !s->db || !key || !value) { + return CBM_STORE_ERR; + } + if (exec_sql(s, savepoint) != CBM_STORE_OK) { + return CBM_STORE_ERR; + } + /* store_meta is only ever created by the generation seed, which writes + * db_uid + mutation_gen with it. Creating the table here without them + * would make cbm_store_generation() report malformed metadata, so an + * absent table is seeded through that one path. */ + bool absent = false; + char uid[17] = {0}; + char counter[21] = {0}; + uint64_t counter_value = 0; + if (generation_metadata_read(s, &absent, uid, counter, &counter_value) != CBM_STORE_OK || + (absent && cbm_store_generation_advance(s) != CBM_STORE_OK)) { + return rollback_savepoint_preserving_error(s, rollback, release); + } + sqlite3_stmt *set = NULL; + if (sqlite3_prepare_v2(s->db, "INSERT OR REPLACE INTO store_meta (k, v) VALUES (?1, ?2);", + CBM_NOT_FOUND, &set, NULL) != SQLITE_OK) { + store_set_error_sqlite(s, "store_meta put prepare"); + sqlite3_finalize(set); + return rollback_savepoint_preserving_error(s, rollback, release); + } + bind_text(set, SKIP_ONE, key); + bind_text(set, ST_COL_2, value); + int rc = sqlite3_step(set); + sqlite3_finalize(set); + if (rc != SQLITE_DONE) { + store_set_error_sqlite(s, "store_meta put"); + return rollback_savepoint_preserving_error(s, rollback, release); + } + if (exec_sql(s, release) != CBM_STORE_OK) { + return rollback_savepoint_preserving_error(s, rollback, release); + } + return CBM_STORE_OK; +} + int cbm_store_get_project(cbm_store_t *s, const char *name, cbm_project_t *out) { sqlite3_stmt *stmt = prepare_cached(s, &s->stmt_get_project, diff --git a/src/store/store.h b/src/store/store.h index 06ea1fbb7..ca4f1eb58 100644 --- a/src/store/store.h +++ b/src/store/store.h @@ -462,6 +462,17 @@ int cbm_store_generation_advance(cbm_store_t *s); * an error. */ int cbm_store_generation(cbm_store_t *s, char *buf, size_t bufsz); +/* Per-store feature metadata in store_meta (key/value text). Keys are + * namespaced by the owning feature, e.g. "cross_repo_last_run:". + * get: CBM_STORE_OK with *out owned by the caller (release with + * cbm_free(CBM_MEM_CLASS_STORE, *out)), or + * CBM_STORE_NOT_FOUND when the key -- or store_meta itself -- is absent. + * Read-only: safe on a store opened for query. + * put: upserts the key; seeds store_meta through the generation path when a + * legacy store lacks it. Atomic (savepoint); needs a read-write store. */ +int cbm_store_meta_get(cbm_store_t *s, const char *key, char **out); +int cbm_store_meta_put(cbm_store_t *s, const char *key, const char *value); + /* Seal a fully-written staging database before atomic publication. * Raises synchronous to FULL, requires an exclusive TRUNCATE checkpoint to * complete, then leaves the database in verified DELETE journal mode so the diff --git a/src/ui/http_server.c b/src/ui/http_server.c index 66f1affb8..ae0dd2754 100644 --- a/src/ui/http_server.c +++ b/src/ui/http_server.c @@ -19,6 +19,7 @@ #include "ui/layout3d.h" #include "mcp/mcp.h" #include "store/store.h" +#include "pipeline/pass_cross_repo.h" #include "watcher/watcher.h" #include "cli/cli.h" #include "git/git_context.h" @@ -1729,6 +1730,9 @@ static void handle_layout(cbm_http_conn_t *c, const cbm_http_req_t *req) { free(linked[li]); } + /* Next to linked_projects: an empty list alone cannot tell "never linked" + * from "linked, nothing matched". */ + (void)cbm_cross_repo_add_status_json(mdoc, mroot, store, project); cbm_store_close(store); yyjson_mut_obj_add_val(mdoc, mroot, "linked_projects", lp_arr); diff --git a/tests/test_cross_repo.c b/tests/test_cross_repo.c index cb4ab0607..54c75cbd5 100644 --- a/tests/test_cross_repo.c +++ b/tests/test_cross_repo.c @@ -10,6 +10,7 @@ #include "pipeline/pipeline_internal.h" #include +#include #include #include #include @@ -401,6 +402,96 @@ TEST(cross_repo_named_pre768_target_fails_before_cleanup_issue2133) { PASS(); } +/* Render the "cross_repo" status object the MCP/UI surfaces attach for + * `project`, read from its store the way index_status does (query open). */ +static char *cross_repo_status_json(const cross_repo_fixture_t *fixture, const char *project) { + char path[512]; + if (!cross_repo_project_path(fixture, project, path, sizeof(path))) { + return NULL; + } + cbm_store_t *store = cbm_store_open_path_query(path); + if (!store) { + return NULL; + } + yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL); + yyjson_mut_val *root = yyjson_mut_obj(doc); + yyjson_mut_doc_set_root(doc, root); + char *json = NULL; + if (cbm_cross_repo_add_status_json(doc, root, store, project)) { + json = yyjson_mut_write(doc, 0, NULL); + } + yyjson_mut_doc_free(doc); + cbm_store_close(store); + return json; +} + +/* #2133 follow-up: an empty CROSS_* set could mean "never linked" or "linked, + * nothing matched" and no surface told them apart. The source store records + * each run; before any run the status says so explicitly. */ +TEST(cross_repo_status_distinguishes_never_run_from_ran_without_links) { + cross_repo_fixture_t fixture; + bool setup = cross_repo_fixture_begin(&fixture) && + cross_repo_create_project(&fixture, "status-src") && + cross_repo_create_project(&fixture, "status-api"); + if (!setup) { + cross_repo_fixture_end(&fixture); + FAIL("failed to seed status fixture"); + } + + char *before = cross_repo_status_json(&fixture, "status-src"); + const char *targets[] = {"status-api"}; + cbm_cross_repo_result_t result = cbm_cross_repo_match("status-src", targets, 1); + char *after = cross_repo_status_json(&fixture, "status-src"); + cross_repo_fixture_end(&fixture); + + bool before_never = before && strstr(before, "\"cross_repo\":{\"status\":\"never_run\"}"); + bool after_ran = + after && strstr(after, "\"status\":\"ran\"") && strstr(after, "\"outcome\":\"complete\"") && + strstr(after, "\"targets\":[\"status-api\"]") && strstr(after, "\"projects_scanned\":1") && + strstr(after, "\"total_cross_edges\":0") && strstr(after, "\"last_run_at\":\"20"); + free(before); + free(after); + cbm_cross_repo_result_free(&result); + + ASSERT_FALSE(result.failed); + ASSERT_TRUE(before_never); + ASSERT_TRUE(after_ran); + PASS(); +} + +/* #2133 follow-up: ["*"] skips a pre-#768 store (it cannot be linked until a + * reindex); the result must name it rather than silently covering fewer + * projects, and the recorded status counts it. */ +TEST(cross_repo_wildcard_reports_skipped_pre768_store) { + cross_repo_fixture_t fixture; + bool setup = cross_repo_fixture_begin(&fixture) && + cross_repo_seed_http_pair(&fixture, "skip-src", "skip-api", "/orders", "s") && + cross_repo_create_pre768_project(&fixture, "aa-pre768-store"); + if (!setup) { + cross_repo_fixture_end(&fixture); + FAIL("failed to seed skipped-store fixture"); + } + + const char *targets[] = {"*"}; + cbm_cross_repo_result_t result = cbm_cross_repo_match("skip-src", targets, 1); + char *status = cross_repo_status_json(&fixture, "skip-src"); + cross_repo_fixture_end(&fixture); + + bool named = result.skipped_count == 1 && result.skipped_projects && + strcmp(result.skipped_projects[0].project, "aa-pre768-store") == 0 && + strcmp(result.skipped_projects[0].reason, "pre_768_schema") == 0; + bool status_counts = + status && strstr(status, "\"skipped_projects\":1") && strstr(status, "\"targets\":[\"*\"]"); + free(status); + cbm_cross_repo_result_free(&result); + + ASSERT_FALSE(result.failed); + ASSERT_EQ(result.projects_scanned, 1); + ASSERT_TRUE(named); + ASSERT_TRUE(status_counts); + PASS(); +} + static bool cross_repo_seed_bounded_scan(const cross_repo_fixture_t *fixture, const char *source_project, const char *target_project) { enum { TEST_SCAN_ROWS = 4097 }; @@ -836,6 +927,8 @@ SUITE(cross_repo) { RUN_TEST(cross_repo_wildcard_keeps_projects_containing_internal_tokens); RUN_TEST(cross_repo_wildcard_skips_pre768_store_issue2133); RUN_TEST(cross_repo_named_pre768_target_fails_before_cleanup_issue2133); + RUN_TEST(cross_repo_status_distinguishes_never_run_from_ran_without_links); + RUN_TEST(cross_repo_wildcard_reports_skipped_pre768_store); RUN_TEST(cross_repo_scan_bound_counts_examined_rows_not_matches); RUN_TEST(cross_repo_propagates_delete_failure); RUN_TEST(cross_repo_failed_bidirectional_insert_is_not_counted); diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 7a2219c5a..883812157 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -13033,6 +13033,102 @@ TEST(tool_cross_repo_dedupes_targets_before_scanning_and_counting) { PASS(); } +/* A store written before the #768 edges.local_name_gen column: readable, but + * refused by every read-write open until a reindex. */ +static bool mcp_cross_repo_create_pre768_store(const char *cache, const char *project) { + char db_path[CBM_SZ_1K]; + snprintf(db_path, sizeof(db_path), "%s/%s.db", cache, project); + sqlite3 *db = NULL; + if (sqlite3_open(db_path, &db) != SQLITE_OK) { + sqlite3_close(db); + return false; + } + char sql[CBM_SZ_1K]; + snprintf(sql, sizeof(sql), + "CREATE TABLE projects(name TEXT PRIMARY KEY, indexed_at TEXT NOT NULL," + " root_path TEXT NOT NULL);" + "CREATE TABLE nodes(id INTEGER PRIMARY KEY AUTOINCREMENT, project TEXT NOT NULL," + " label TEXT NOT NULL, name TEXT NOT NULL, qualified_name TEXT NOT NULL," + " file_path TEXT DEFAULT '', start_line INTEGER DEFAULT 0," + " end_line INTEGER DEFAULT 0, properties TEXT DEFAULT '{}'," + " UNIQUE(project, qualified_name));" + "CREATE TABLE edges(id INTEGER PRIMARY KEY AUTOINCREMENT, project TEXT NOT NULL," + " source_id INTEGER NOT NULL, target_id INTEGER NOT NULL, type TEXT NOT NULL," + " properties TEXT DEFAULT '{}', UNIQUE(source_id, target_id, type));" + "INSERT INTO projects VALUES('%s', '2026-06-01T00:00:00Z', '/pre768');", + project); + bool ok = sqlite3_exec(db, sql, NULL, NULL, NULL) == SQLITE_OK; + sqlite3_close(db); + return ok; +} + +/* #2133 follow-up, both user-visible halves: index_status tells "never run" + * from "ran" via a `cross_repo` object, and a ["*"] run names the legacy + * store it skipped (with the reindex hint) instead of silently linking fewer + * projects. */ +TEST(tool_cross_repo_status_and_wildcard_skipped_projects) { + char cache[256]; + snprintf(cache, sizeof(cache), "%s/cbm-mcp-cross-status-XXXXXX", cbm_tmpdir()); + ASSERT_NOT_NULL(cbm_mkdtemp(cache)); + + const char *saved_cache = getenv("CBM_CACHE_DIR"); + char *saved_cache_copy = saved_cache ? strdup(saved_cache) : NULL; + cbm_setenv("CBM_CACHE_DIR", cache, 1); + + char *source_project = cbm_project_name_from_path(cache); + ASSERT_NOT_NULL(source_project); + const char *target_project = "cross-status-target"; + const char *legacy_project = "cross-status-pre768"; + ASSERT_TRUE(mcp_cross_repo_seed_http_match(cache, source_project, target_project, cache)); + ASSERT_TRUE(mcp_cross_repo_create_pre768_store(cache, legacy_project)); + + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + ASSERT_TRUE(cbm_mcp_server_set_session_context(srv, cache, NULL)); + + char status_args[CBM_SZ_1K]; + snprintf(status_args, sizeof(status_args), "{\"project\":\"%s\",\"format\":\"json\"}", + source_project); + char *before = cbm_mcp_handle_tool(srv, "index_status", status_args); + bool before_never = + response_contains_json_fragment(before, "\"cross_repo\":{\"status\":\"never_run\"}"); + + char args[CBM_SZ_2K]; + snprintf(args, sizeof(args), + "{\"repo_path\":\"%s\",\"mode\":\"cross-repo-intelligence\"," + "\"target_projects\":[\"*\"]}", + cache); + char *run = cbm_mcp_handle_tool(srv, "index_repository", args); + bool run_ok = run && strstr(run, "\"isError\":true") == NULL && + response_contains_json_fragment(run, "\"total_cross_edges\":1"); + bool skipped_named = response_contains_json_fragment( + run, "\"skipped_projects\":[{\"name\":\"cross-status-pre768\"," + "\"reason\":\"pre_768_schema\",\"hint\":\"reindex this project\"}]"); + + char *after = cbm_mcp_handle_tool(srv, "index_status", status_args); + bool after_ran = response_contains_json_fragment(after, "\"status\":\"ran\"") && + response_contains_json_fragment(after, "\"total_cross_edges\":1") && + response_contains_json_fragment(after, "\"skipped_projects\":1"); + + free(before); + free(run); + free(after); + cbm_mcp_server_free(srv); + cleanup_project_db(cache, source_project); + cleanup_project_db(cache, target_project); + cleanup_project_db(cache, legacy_project); + free(source_project); + restore_cache_dir(saved_cache_copy); + free(saved_cache_copy); + cbm_rmdir(cache); + + ASSERT_TRUE(before_never); + ASSERT_TRUE(run_ok); + ASSERT_TRUE(skipped_named); + ASSERT_TRUE(after_ran); + PASS(); +} + /* `name` is the documented index project-name override and must identify the * cross-repo source too. Deriving from repo_path here makes custom-named * projects impossible to rescan even though ordinary indexing created them. */ @@ -21212,6 +21308,7 @@ SUITE(mcp_mutation_guard) { RUN_TEST(tool_cross_repo_checks_cancellation_after_acquiring_leases); RUN_TEST(tool_cross_repo_missing_inputs_fail_without_creating_ghost_databases); RUN_TEST(tool_cross_repo_dedupes_targets_before_scanning_and_counting); + RUN_TEST(tool_cross_repo_status_and_wildcard_skipped_projects); RUN_TEST(tool_cross_repo_honors_source_name_override); RUN_TEST(tool_corrupt_store_cleanup_guard_is_balanced_and_not_nested); RUN_TEST(tool_corrupt_store_cleanup_guard_denial_preserves_db_and_wal); From bc66208be6a240a684ff1b4bcf6dab60dd048851 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Thu, 1 Oct 2026 19:11:51 +0200 Subject: [PATCH 02/15] style(cross-repo): narrow the scope of the status edge total CI's cppcheck flags `total` in the cross-repo status writer (variableScope): it is only read inside the `len < cap` branch. Move the declaration into that branch. No behaviour change. Signed-off-by: Martin Vogel --- src/pipeline/pass_cross_repo.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pipeline/pass_cross_repo.c b/src/pipeline/pass_cross_repo.c index f645db349..09384a8e9 100644 --- a/src/pipeline/pass_cross_repo.c +++ b/src/pipeline/pass_cross_repo.c @@ -1361,9 +1361,9 @@ static void cr_record_last_run(cbm_store_t *src_store, const char *project, cons len += (size_t)snprintf(json + len, cap - len, "%s\"%s\"", i ? "," : "", targets[i] ? targets[i] : ""); } - int total = result->http_edges + result->async_edges + result->channel_edges + - result->grpc_edges + result->graphql_edges + result->trpc_edges; if (len < cap) { + int total = result->http_edges + result->async_edges + result->channel_edges + + result->grpc_edges + result->graphql_edges + result->trpc_edges; len += (size_t)snprintf(json + len, cap - len, "],\"projects_scanned\":%d,\"total_cross_edges\":%d," "\"skipped_projects\":%d}", From 46a8f57f18e5e40b62c4d40d31062028fb336c48 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 2 Oct 2026 00:42:05 +0200 Subject: [PATCH 03/15] fix(ui): index job carries the resolved root, not the request spelling POST /api/index resolves the root_path of the request into a 4 KB buffer, but the index job kept the original request string in a 1 KB field. The job, the worker log lines, the 202 reply and /api/index-status therefore reported the request spelling instead of the resolved root, and a path past 1023 bytes reached the daemon cut off. Store the resolved path in the job, give the field the capacity of the buffer it is copied from, and answer 400 for a path that would not fit instead of storing a shortened one. The status listing's escape buffer follows the new field size. The job's downstream, application_background_index, re-resolves what it receives, so a resolved input is a fixed point there and the daemon keys the job by the same string the route worked with. Tests drive the live route: a root outside a configured allowed root is answered 403 and no job starts; a root spelled through a ".." segment is stored and forwarded in resolved form; a root past 1 KB is either refused or forwarded whole. The last case reaches the accepted branch on Linux (PATH_MAX 4096); on macOS the path cannot resolve and the refusal branch is asserted. Signed-off-by: Martin Vogel --- src/ui/http_server.c | 36 +++++-- tests/test_httpd.c | 238 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 259 insertions(+), 15 deletions(-) diff --git a/src/ui/http_server.c b/src/ui/http_server.c index 66f1affb8..7403897e7 100644 --- a/src/ui/http_server.c +++ b/src/ui/http_server.c @@ -160,6 +160,10 @@ static void handle_ui_config(cbm_http_conn_t *c, const cbm_http_req_t *req) { /* ── Server state ─────────────────────────────────────────────── */ #define MAX_INDEX_JOBS 4 +/* Capacity of the buffer POST /api/index resolves the root into, and of the + * job field that buffer is copied to: the job carries the resolved string + * whole, so the path handed to the daemon is the one the route checked. */ +#define INDEX_JOB_ROOT_CAP 4096 enum { HTTP_RUN_IDLE = 0, @@ -170,7 +174,7 @@ enum { typedef struct { cbm_http_server_t *server; - char root_path[1024]; + char root_path[INDEX_JOB_ROOT_CAP]; /* resolved form, see handle_index_start */ char project_name[256]; atomic_int status; /* 0=idle, 1=running, 2=done, 3=error */ char error_msg[256]; @@ -1182,7 +1186,7 @@ static void handle_index_start(cbm_http_server_t *server, cbm_http_conn_t *c, * it accepted roots the MCP path refused — an operator's boundary held on one * entry point and not the other. Canonicalize first: the policy is defined * over resolved paths, and a symlink would otherwise launder the verdict. */ - char canonical_root[4096]; + char canonical_root[INDEX_JOB_ROOT_CAP]; char boundary_err[1024]; if (!cbm_canonical_path(rpath, canonical_root, sizeof(canonical_root))) { yyjson_doc_free(doc); @@ -1198,6 +1202,15 @@ static void handle_index_start(cbm_http_server_t *server, cbm_http_conn_t *c, cbm_http_replyf(c, 403, g_cors_json, "{\"error\":\"%s\"}", escaped); return; } + /* The job carries exactly the string the checks above ran on. The field + * has the resolved buffer's capacity, so a resolved path always fits; the + * check keeps a resized field from ever handing the worker a cut-off path + * instead of the one that was checked. */ + if (strlen(canonical_root) >= INDEX_JOB_ROOT_CAP) { + yyjson_doc_free(doc); + cbm_http_replyf(c, 400, g_cors_json, "{\"error\":\"root_path too long\"}"); + return; + } /* Find free job slot */ int slot = -1; @@ -1226,7 +1239,7 @@ static void handle_index_start(cbm_http_server_t *server, cbm_http_conn_t *c, job->thread_started = false; } job->server = server; - snprintf(job->root_path, sizeof(job->root_path), "%s", rpath); + snprintf(job->root_path, sizeof(job->root_path), "%s", canonical_root); snprintf(job->project_name, sizeof(job->project_name), "%s", project_name); job->error_msg[0] = '\0'; atomic_store(&job->status, 1); @@ -1258,14 +1271,15 @@ static void handle_index_status(cbm_http_server_t *server, cbm_http_conn_t *c) { if (pos > 1) http_appendf(buf, sizeof(buf), &pos, ","); const char *ss = st == 1 ? "indexing" : st == 2 ? "done" : "error"; - /* root_path comes from POST /api/index and is up to 1023 bytes, so four - * occupied slots exceed this buffer. http_appendf pins pos to - * sizeof(buf) on truncation, so the separator and the close have to go - * through it as well rather than indexing raw. Both fields are free-form, - * so escape them — a quote in a path would otherwise end its JSON string - * early. */ - /* Escaping can double each byte: root_path is 1024, error_msg 256. */ - char esc_path[2048]; + /* root_path is the resolved root POST /api/index stored, up to + * INDEX_JOB_ROOT_CAP - 1 bytes, so one long path already exceeds this + * buffer. http_appendf pins pos to sizeof(buf) on truncation, so the + * separator and the close have to go through it as well rather than + * indexing raw. Both fields are free-form, so escape them — a quote in a + * path would otherwise end its JSON string early. */ + /* Escaping can double each byte: root_path is INDEX_JOB_ROOT_CAP, + * error_msg 256. */ + char esc_path[2 * INDEX_JOB_ROOT_CAP]; char esc_error[512]; cbm_json_escape(esc_path, (int)sizeof(esc_path), server->index_jobs[i].root_path); cbm_json_escape(esc_error, (int)sizeof(esc_error), diff --git a/tests/test_httpd.c b/tests/test_httpd.c index 322130c33..ee024a4d6 100644 --- a/tests/test_httpd.c +++ b/tests/test_httpd.c @@ -18,6 +18,7 @@ #include "../src/foundation/compat_thread.h" #include "../src/foundation/log.h" #include "../src/foundation/platform.h" +#include "../src/foundation/str_util.h" /* cbm_json_escape: expected index-status fragment */ #include "../src/cli/cli.h" #include "../src/daemon/host_internal.h" #include "../src/git/git_context.h" /* #798 follow-up: live-socket git-resolve repro */ @@ -29,6 +30,7 @@ #include #include +#include /* PATH_MAX: which platforms resolve a root past 1 KB */ #include #include #include @@ -627,7 +629,7 @@ static int th_server_start_with_watcher(th_server_t *ts, cbm_watcher_t *watcher) typedef struct { atomic_int calls; - char root_path[512]; + char root_path[4096]; /* the route resolves into a 4 KB buffer; record it whole */ char project_name[256]; } th_ui_index_executor_t; @@ -974,6 +976,10 @@ TEST(ui_server_process_kill_route_is_unavailable) { TEST(ui_server_routes_indexing_through_joinable_daemon_executor) { char *root = th_mktempdir("cbm_httpd_daemon_index"); ASSERT_NOT_NULL(root); + /* The executor receives the root in resolved form (the string the route + * checked), which on macOS differs from the /tmp spelling. */ + char resolved_root[4096]; + ASSERT_TRUE(cbm_canonical_path(root, resolved_root, sizeof(resolved_root))); th_ui_index_executor_t executor = {0}; atomic_init(&executor.calls, 0); th_server_t ts; @@ -998,7 +1004,7 @@ TEST(ui_server_routes_indexing_through_joinable_daemon_executor) { ASSERT_GT(response_length, 0); ASSERT_EQ(th_status(response), 202); ASSERT_TRUE(called); - ASSERT_STR_EQ(executor.root_path, root); + ASSERT_STR_EQ(executor.root_path, resolved_root); ASSERT_STR_EQ(executor.project_name, "ui-project"); th_cleanup(root); PASS(); @@ -1048,6 +1054,227 @@ TEST(ui_server_free_never_joins_active_index_worker) { PASS(); } +/* ── POST /api/index carries the resolved root ────────────────── */ + +/* The route resolves the request's root_path, runs the workspace check on the + * resolved form, and the job must then carry exactly that string: what was + * checked is what the daemon receives. These cases drive the real endpoint + * with the recording executor and read every place the stored root surfaces + * (the executor argument, the 202 reply, /api/index-status). */ + +typedef struct { + bool was_set; + char value[4096]; +} th_allowed_root_saved_t; + +/* Set (or, with NULL, clear) CBM_ALLOWED_ROOT and remember the previous value + * so the test can put it back before asserting — a failed assertion returns + * early, and a boundary left behind would leak into every later test. */ +static void th_allowed_root_set(th_allowed_root_saved_t *saved, const char *value) { + const char *current = getenv("CBM_ALLOWED_ROOT"); + saved->was_set = current != NULL; + snprintf(saved->value, sizeof(saved->value), "%s", current ? current : ""); + if (value) { + cbm_setenv("CBM_ALLOWED_ROOT", value, 1); + } else { + cbm_unsetenv("CBM_ALLOWED_ROOT"); + } +} + +static void th_allowed_root_restore(const th_allowed_root_saved_t *saved) { + if (saved->was_set) { + cbm_setenv("CBM_ALLOWED_ROOT", saved->value, 1); + } else { + cbm_unsetenv("CBM_ALLOWED_ROOT"); + } +} + +typedef struct { + th_ui_index_executor_t executor; + int status; /* HTTP status of the POST, -1 when no reply came */ + bool executor_called; /* only waited for after a 202 */ + char response[8192]; /* the raw POST reply */ + char index_status[16384]; /* GET /api/index-status, read after the POST */ +} th_ui_index_probe_t; + +/* Response body: everything after the header block, or "" when absent. */ +static const char *th_response_body(const char *response) { + const char *body = strstr(response, "\r\n\r\n"); + return body ? body + 4 : ""; +} + +/* Boot a server with the recording executor, POST one /api/index request, + * wait for the worker only when the route accepted the job, read + * /api/index-status and stop the server. Everything observed lands in the + * probe so the caller asserts after teardown. */ +static void th_ui_index_probe(th_ui_index_probe_t *probe, const char *root_path, + const char *project_name) { + memset(probe, 0, sizeof(*probe)); + atomic_init(&probe->executor.calls, 0); + probe->status = -1; + th_server_t ts; + ts.srv = cbm_http_server_new(0); + if (!ts.srv) { + return; + } + cbm_http_server_set_index_executor(ts.srv, th_ui_index_executor, &probe->executor); + if (th_server_thread_start(&ts.tid, ts.srv) != 0) { + (void)cbm_http_server_free(ts.srv); + return; + } + int port = cbm_http_server_port(ts.srv); + char body[8192]; + snprintf(body, sizeof(body), "{\"root_path\":\"%s\",\"project_name\":\"%s\"}", root_path, + project_name); + char request[8192 + 256]; + snprintf(request, sizeof(request), + "POST /api/index HTTP/1.1\r\nContent-Type: application/json\r\n" + "Content-Length: %zu\r\n\r\n%s", + strlen(body), body); + if (th_http(port, request, probe->response, sizeof(probe->response)) > 0) { + probe->status = th_status(probe->response); + } + if (probe->status == 202) { + probe->executor_called = th_wait_atomic_int(&probe->executor.calls, 1, 2000); + } + char status_request[128]; + snprintf(status_request, sizeof(status_request), + "GET /api/index-status HTTP/1.1\r\nHost: 127.0.0.1:%d\r\n\r\n", port); + (void)th_http(port, status_request, probe->index_status, sizeof(probe->index_status)); + th_server_stop(&ts); +} + +TEST(ui_server_index_start_refuses_root_outside_allowed) { + char *tmp = th_mktempdir("cbm_httpd_index_bound"); + ASSERT_NOT_NULL(tmp); + char base[256]; + snprintf(base, sizeof(base), "%s", tmp); + char allowed[512]; + char outside[512]; + snprintf(allowed, sizeof(allowed), "%s/allowed", base); + snprintf(outside, sizeof(outside), "%s/outside", base); + ASSERT_EQ(th_mkdir_p(allowed), 0); + ASSERT_EQ(th_mkdir_p(outside), 0); + /* The boundary is declared in resolved form, as allow-root records it. */ + char canonical_base[4096]; + ASSERT_TRUE(cbm_canonical_path(base, canonical_base, sizeof(canonical_base))); + char allowed_root[4096 + 16]; + snprintf(allowed_root, sizeof(allowed_root), "%s/allowed", canonical_base); + /* Spelled under the allowed root, resolving beside it: the verdict has to + * come from the resolved form. */ + char request_path[600]; + snprintf(request_path, sizeof(request_path), "%s/allowed/../outside", base); + + th_allowed_root_saved_t saved; + th_allowed_root_set(&saved, allowed_root); + th_ui_index_probe_t probe; + th_ui_index_probe(&probe, request_path, "bound"); + th_allowed_root_restore(&saved); + th_cleanup(base); + + ASSERT_EQ(probe.status, 403); + ASSERT_NOT_NULL(strstr(probe.response, "outside the allowed root")); + ASSERT_EQ(atomic_load(&probe.executor.calls), 0); + ASSERT_STR_EQ(th_response_body(probe.index_status), "[]"); + PASS(); +} + +TEST(ui_server_index_start_stores_resolved_root) { + char *tmp = th_mktempdir("cbm_httpd_index_root"); + ASSERT_NOT_NULL(tmp); + char base[256]; + snprintf(base, sizeof(base), "%s", tmp); + char inner[512]; + snprintf(inner, sizeof(inner), "%s/allowed/inner", base); + ASSERT_EQ(th_mkdir_p(inner), 0); + char canonical_base[4096]; + ASSERT_TRUE(cbm_canonical_path(base, canonical_base, sizeof(canonical_base))); + char allowed_root[4096 + 16]; + snprintf(allowed_root, sizeof(allowed_root), "%s/allowed", canonical_base); + /* The request names the allowed root through a ".." segment, so its + * spelling is not the resolved form (on macOS /tmp is itself a link to + * /private/tmp, so the prefix differs as well). */ + char request_path[600]; + snprintf(request_path, sizeof(request_path), "%s/allowed/inner/..", base); + ASSERT_TRUE(strcmp(request_path, allowed_root) != 0); + + th_allowed_root_saved_t saved; + th_allowed_root_set(&saved, allowed_root); + th_ui_index_probe_t probe; + th_ui_index_probe(&probe, request_path, "resolved"); + th_allowed_root_restore(&saved); + th_cleanup(base); + + ASSERT_EQ(probe.status, 202); + ASSERT_TRUE(probe.executor_called); + /* The daemon receives the string the check ran on, not the request + * spelling. */ + ASSERT_STR_EQ(probe.executor.root_path, allowed_root); + ASSERT_STR_EQ(probe.executor.project_name, "resolved"); + /* The 202 reply and the status listing report that same stored root. */ + char expected[8192 + 64]; + snprintf(expected, sizeof(expected), "\"path\":\"%s\"}", allowed_root); + ASSERT_NOT_NULL(strstr(probe.response, expected)); + char escaped[8192]; + cbm_json_escape(escaped, (int)sizeof(escaped), allowed_root); + snprintf(expected, sizeof(expected), "\"path\":\"%s\",\"error\"", escaped); + ASSERT_NOT_NULL(strstr(probe.index_status, expected)); + PASS(); +} + +TEST(ui_server_index_start_long_root_not_truncated) { + char *tmp = th_mktempdir("cbm_httpd_index_long"); + ASSERT_NOT_NULL(tmp); + char base[256]; + snprintf(base, sizeof(base), "%s", tmp); + /* A root well past 1 KB, the capacity the job used to hold, and well + * inside the 4 KB the route resolves; 200-byte components keep every + * component under NAME_MAX. */ + char component[201]; + memset(component, 'd', sizeof(component) - 1); + component[sizeof(component) - 1] = '\0'; + char deep[2048]; + int n = snprintf(deep, sizeof(deep), "%s", base); + while (n < 1400) { + n += snprintf(deep + n, sizeof(deep) - (size_t)n, "/%s", component); + } + bool created = th_mkdir_p(deep) == 0; + char canonical[4096]; + bool resolved = created && cbm_canonical_path(deep, canonical, sizeof(canonical)) != 0; + + /* No boundary here: the question is the length the job keeps. */ + th_allowed_root_saved_t saved; + th_allowed_root_set(&saved, NULL); + th_ui_index_probe_t probe; + th_ui_index_probe(&probe, deep, "long"); + th_allowed_root_restore(&saved); + th_cleanup(base); + + if (probe.status == 202) { + /* The platform resolved the path, so the worker has to receive it + * whole — the same string the route checked. */ + ASSERT_TRUE(resolved); + ASSERT_TRUE(probe.executor_called); + ASSERT_TRUE(strlen(probe.executor.root_path) > 1023); + ASSERT_STR_EQ(probe.executor.root_path, canonical); + } else { + /* The platform cannot resolve a path this long (macOS: PATH_MAX is + * 1024, so mkdir and stat refuse it before the route sees it); the + * route answers 400 and keeps nothing. */ + ASSERT_EQ(probe.status, 400); + ASSERT_EQ(atomic_load(&probe.executor.calls), 0); + ASSERT_STR_EQ(th_response_body(probe.index_status), "[]"); + } +#if defined(PATH_MAX) && PATH_MAX >= 2048 + /* Linux resolves it (PATH_MAX 4096), and that is the one platform where a + * cut-off could reach the worker; the accepted branch is the only honest + * outcome there, so the refusal branch may not stand in for it. */ + ASSERT_TRUE(created); + ASSERT_EQ(probe.status, 202); +#endif + PASS(); +} + TEST(ui_server_root_without_embedded_assets_is_not_found) { /* The frontend is linked into the image, so its availability is decided at * build time, not warmed at runtime: a binary built without --with-ui has @@ -2318,8 +2545,8 @@ TEST(ui_server_logs_escape_dense_no_overflow) { /* The index-status endpoint renders every active job into a fixed 2 KB stack * buffer. http_appendf clamps its own writes, but the separator and the closing - * bracket were raw indexes, so two jobs holding ~1 KB root paths (the field is - * 1024 bytes and the value comes straight from POST /api/index) pushed pos to + * bracket were raw indexes, so two jobs holding ~1 KB root paths (the field + * was 1024 bytes and now holds the 4 KB resolved root) pushed pos to * the clamp and the close then wrote past the buffer. Drive it through the real * endpoint with the index executor stubbed out, in a forked child so the * overflow surfaces as a killing signal. */ @@ -2458,6 +2685,9 @@ SUITE(httpd) { RUN_TEST(ui_server_process_kill_route_is_unavailable); RUN_TEST(ui_server_routes_indexing_through_joinable_daemon_executor); RUN_TEST(ui_server_free_never_joins_active_index_worker); + RUN_TEST(ui_server_index_start_refuses_root_outside_allowed); + RUN_TEST(ui_server_index_start_stores_resolved_root); + RUN_TEST(ui_server_index_start_long_root_not_truncated); RUN_TEST(ui_server_root_without_embedded_assets_is_not_found); RUN_TEST(ui_server_same_origin_request_is_allowed); RUN_TEST(ui_server_rejects_foreign_and_null_origins); From 7077695f2990818d300662cf9e3c4a3ceca601aa Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 2 Oct 2026 02:23:12 +0200 Subject: [PATCH 04/15] fix(ui): size the index-status body for the roots the table holds GET /api/index-status rendered every job into a fixed 2 KB stack buffer with a clamp. Now that a job carries its resolved root in a 4 KB field, one long root alone exceeded that buffer and the listing came back as a cut JSON body with status 200. Build the body in a heap buffer sized from the table: per slot the entry framing plus the root and the error message at cbm_json_escape's widest expansion. The two strings are escaped straight into the body, the append helpers keep their clamp as a second guard, and the buffer goes through the memory core like the other variable-size responses. The 202 reply of POST /api/index printed the stored root unescaped, so a quote or backslash in the path ended its JSON string early. It is now escaped the way the listing escapes it. Tests: with every slot holding a root as long as the platform resolves (just under the 4 KB field on Linux, under PATH_MAX on macOS), the listing parses as JSON and carries each root whole, first with one job and then with the table full; a root containing a quote comes back parseable from the 202 reply, naming the resolved path. Signed-off-by: Martin Vogel --- src/ui/http_server.c | 81 +++++++++++----- tests/test_httpd.c | 224 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 280 insertions(+), 25 deletions(-) diff --git a/src/ui/http_server.c b/src/ui/http_server.c index 7403897e7..f9f2ff0c6 100644 --- a/src/ui/http_server.c +++ b/src/ui/http_server.c @@ -28,6 +28,7 @@ #endif /* pipeline.h no longer needed — indexing runs as subprocess */ #include "foundation/log.h" +#include "foundation/mem_core.h" #include "foundation/platform.h" #include "foundation/secure_random.h" #include "foundation/sha256.h" @@ -164,6 +165,13 @@ static void handle_ui_config(cbm_http_conn_t *c, const cbm_http_req_t *req) { * job field that buffer is copied to: the job carries the resolved string * whole, so the path handed to the daemon is the one the route checked. */ #define INDEX_JOB_ROOT_CAP 4096 +#define INDEX_JOB_ERROR_CAP 256 +/* GET /api/index-status sizes its body from the table: per entry the framing + * below ({"slot":N,"status":"indexing","path":"","error":""} and a separator, + * well under 64 bytes) plus both strings at cbm_json_escape's widest expansion, + * six bytes for a control character (\u00XX). */ +#define INDEX_STATUS_ENTRY_FRAMING 64 +#define JSON_ESCAPE_MAX_EXPANSION 6 enum { HTTP_RUN_IDLE = 0, @@ -177,7 +185,7 @@ typedef struct { char root_path[INDEX_JOB_ROOT_CAP]; /* resolved form, see handle_index_start */ char project_name[256]; atomic_int status; /* 0=idle, 1=running, 2=done, 3=error */ - char error_msg[256]; + char error_msg[INDEX_JOB_ERROR_CAP]; cbm_thread_t thread; bool thread_started; atomic_int completed; @@ -1256,44 +1264,67 @@ static void handle_index_start(cbm_http_server_t *server, cbm_http_conn_t *c, } job->thread_started = true; + /* The reply names the stored root; escape it as the status listing does, + * or a quote or backslash in the path ends the JSON string early. */ + char esc_path[JSON_ESCAPE_MAX_EXPANSION * INDEX_JOB_ROOT_CAP]; + cbm_json_escape(esc_path, (int)sizeof(esc_path), job->root_path); cbm_http_replyf(c, 202, g_cors_json, "{\"status\":\"indexing\",\"slot\":%d,\"path\":\"%s\"}", - slot, job->root_path); + slot, esc_path); +} + +/* Append src JSON-escaped, under http_appendf's clamp contract: once *pos has + * reached bufsz nothing more is written. */ +static void http_append_json_escaped(char *buf, size_t bufsz, int *pos, const char *src) { + if (*pos < 0) { + return; + } + if ((size_t)*pos >= bufsz) { + *pos = (int)bufsz; + return; + } + *pos += cbm_json_escape(buf + *pos, (int)(bufsz - (size_t)*pos), src); } /* GET /api/index-status — returns status of all index jobs */ static void handle_index_status(cbm_http_server_t *server, cbm_http_conn_t *c) { - char buf[2048] = "["; - int pos = 1; + /* Size the body for a full table: every slot's framing plus its root and + * error message at the widest escape. A fixed 2 KB stack buffer used to + * clamp here, so one long resolved root (the field holds 4 KB) came back + * as a cut JSON body with status 200. The append helpers keep clamping, + * so this calculation is not the only thing keeping pos in range. */ + size_t buf_size = 2 + (size_t)MAX_INDEX_JOBS * (INDEX_STATUS_ENTRY_FRAMING + + JSON_ESCAPE_MAX_EXPANSION * + (INDEX_JOB_ROOT_CAP + INDEX_JOB_ERROR_CAP)); + char *buf = cbm_alloc(CBM_MEM_CLASS_OTHER, buf_size); + if (!buf) { + cbm_http_replyf(c, 500, g_cors_json, "{\"error\":\"oom\"}"); + return; + } + int pos = 0; + http_appendf(buf, buf_size, &pos, "["); for (int i = 0; i < MAX_INDEX_JOBS; i++) { int st = atomic_load(&server->index_jobs[i].status); if (st == 0) continue; if (pos > 1) - http_appendf(buf, sizeof(buf), &pos, ","); + http_appendf(buf, buf_size, &pos, ","); const char *ss = st == 1 ? "indexing" : st == 2 ? "done" : "error"; - /* root_path is the resolved root POST /api/index stored, up to - * INDEX_JOB_ROOT_CAP - 1 bytes, so one long path already exceeds this - * buffer. http_appendf pins pos to sizeof(buf) on truncation, so the - * separator and the close have to go through it as well rather than - * indexing raw. Both fields are free-form, so escape them — a quote in a - * path would otherwise end its JSON string early. */ - /* Escaping can double each byte: root_path is INDEX_JOB_ROOT_CAP, - * error_msg 256. */ - char esc_path[2 * INDEX_JOB_ROOT_CAP]; - char esc_error[512]; - cbm_json_escape(esc_path, (int)sizeof(esc_path), server->index_jobs[i].root_path); - cbm_json_escape(esc_error, (int)sizeof(esc_error), - st == 3 ? server->index_jobs[i].error_msg : ""); - http_appendf(buf, sizeof(buf), &pos, - "{\"slot\":%d,\"status\":\"%s\",\"path\":\"%s\",\"error\":\"%s\"}", i, ss, - esc_path, esc_error); + /* Both fields are free-form, so escape them — a quote in a path would + * otherwise end its JSON string early. */ + http_appendf(buf, buf_size, &pos, "{\"slot\":%d,\"status\":\"%s\",\"path\":\"", i, ss); + http_append_json_escaped(buf, buf_size, &pos, server->index_jobs[i].root_path); + http_appendf(buf, buf_size, &pos, "\",\"error\":\""); + http_append_json_escaped(buf, buf_size, &pos, + st == 3 ? server->index_jobs[i].error_msg : ""); + http_appendf(buf, buf_size, &pos, "\"}"); } - http_appendf(buf, sizeof(buf), &pos, "]"); - if ((size_t)pos >= sizeof(buf)) { - pos = (int)sizeof(buf) - 1; + http_appendf(buf, buf_size, &pos, "]"); + if ((size_t)pos >= buf_size) { + pos = (int)buf_size - 1; } buf[pos] = '\0'; - cbm_http_replyf(c, 200, g_cors_json, "%s", buf); + cbm_http_reply_buf(c, 200, g_cors_json, buf, (size_t)pos); + cbm_free(CBM_MEM_CLASS_OTHER, buf); } static void unwatch_project(cbm_http_server_t *srv, const char *name) { diff --git a/tests/test_httpd.c b/tests/test_httpd.c index ee024a4d6..d020dc7da 100644 --- a/tests/test_httpd.c +++ b/tests/test_httpd.c @@ -29,6 +29,7 @@ #include "ui/http_server.h" #include #include +#include /* the index replies are asserted as parsed JSON */ #include /* PATH_MAX: which platforms resolve a root past 1 KB */ #include @@ -1275,6 +1276,60 @@ TEST(ui_server_index_start_long_root_not_truncated) { PASS(); } +/* The 202 reply names the stored root; a quote or backslash in it has to + * travel escaped or the reply is not JSON. */ +TEST(ui_server_index_start_reply_escapes_path) { + char *tmp = th_mktempdir("cbm_httpd_index_quote"); + ASSERT_NOT_NULL(tmp); + char base[256]; + snprintf(base, sizeof(base), "%s", tmp); + char root[512]; +#ifdef _WIN32 + /* A quote is not a legal file-name byte on Windows; the separators of the + * resolved path already need escaping there. */ + snprintf(root, sizeof(root), "%s\\plain", base); +#else + snprintf(root, sizeof(root), "%s/q\"uote", base); +#endif + ASSERT_EQ(th_mkdir_p(root), 0); + char canonical[4096]; + ASSERT_TRUE(cbm_canonical_path(root, canonical, sizeof(canonical))); + /* The request body is JSON too, so the root goes in escaped. */ + char request_path[1024]; + cbm_json_escape(request_path, (int)sizeof(request_path), root); + + th_ui_index_probe_t probe; + th_ui_index_probe(&probe, request_path, "quoted"); + th_cleanup(base); + + bool parsed = false; + int slot = -1; + char reply_path[4096] = ""; + const char *body = th_response_body(probe.response); + yyjson_doc *doc = yyjson_read(body, strlen(body), 0); + if (doc) { + yyjson_val *obj = yyjson_doc_get_root(doc); + yyjson_val *path = yyjson_obj_get(obj, "path"); + yyjson_val *slot_val = yyjson_obj_get(obj, "slot"); + parsed = yyjson_is_obj(obj); + if (yyjson_is_str(path)) { + snprintf(reply_path, sizeof(reply_path), "%s", yyjson_get_str(path)); + } + if (yyjson_is_int(slot_val)) { + slot = yyjson_get_int(slot_val); + } + yyjson_doc_free(doc); + } + + ASSERT_EQ(probe.status, 202); + ASSERT_TRUE(probe.executor_called); + ASSERT_STR_EQ(probe.executor.root_path, canonical); + ASSERT_TRUE(parsed); + ASSERT_STR_EQ(reply_path, canonical); + ASSERT_TRUE(slot >= 0); + PASS(); +} + TEST(ui_server_root_without_embedded_assets_is_not_found) { /* The frontend is linked into the image, so its availability is decided at * build time, not warmed at runtime: a binary built without --with-ui has @@ -2644,6 +2699,173 @@ TEST(ui_server_index_status_long_paths_no_overflow) { #endif } +/* Grow path in place to exactly target bytes with 'd' components, none + * longer than 200 so every component stays under NAME_MAX. */ +static void th_pad_path_to(char *path, size_t cap, int target) { + char fill[201]; + memset(fill, 'd', sizeof(fill) - 1); + fill[sizeof(fill) - 1] = '\0'; + int n = (int)strlen(path); + while (n < target && (size_t)n + 2 < cap) { + int room = target - n - 1; /* after the '/' */ + int len = room > 200 ? 200 : room; + if (len <= 0) { + break; + } + n += snprintf(path + n, cap - (size_t)n, "/%.*s", len, fill); + } +} + +/* Parse an index-status body and compare each entry's "path" with the root + * expected for its slot. Results go to out-params so the caller can tear + * down before asserting. */ +static void th_index_status_check(const char *body, int expected_count, char expected[][4096], + bool *parsed, int *count, bool *paths_whole) { + *parsed = false; + *count = 0; + *paths_whole = false; + yyjson_doc *doc = yyjson_read(body, strlen(body), 0); + if (!doc) { + return; + } + yyjson_val *arr = yyjson_doc_get_root(doc); + if (yyjson_is_arr(arr)) { + *parsed = true; + *count = (int)yyjson_arr_size(arr); + bool whole = *count == expected_count; + size_t idx; + size_t max_entries; + yyjson_val *entry; + yyjson_arr_foreach(arr, idx, max_entries, entry) { + yyjson_val *slot = yyjson_obj_get(entry, "slot"); + yyjson_val *path = yyjson_obj_get(entry, "path"); + int s = yyjson_is_int(slot) ? yyjson_get_int(slot) : -1; + whole = whole && s >= 0 && s < expected_count && yyjson_is_str(path) && + strcmp(yyjson_get_str(path), expected[s]) == 0; + } + *paths_whole = whole; + } + yyjson_doc_free(doc); +} + +/* The listing is sized for what the job table holds. Every slot takes a root + * as long as the platform resolves; the body has to parse as JSON and carry + * each root whole, first with one job and then with the table full. Roots + * enter the table the way the clamp test's do: real directories through the + * route, held open by the blocking executor so every slot stays occupied. */ +TEST(ui_server_index_status_long_roots_render_whole) { + char *tmp = th_mktempdir("cbm_status_whole"); + ASSERT_NOT_NULL(tmp); + char base[256]; + snprintf(base, sizeof(base), "%s", tmp); + /* Linux: just under the 4 KB field; the 4 KB request body still wraps it. */ + int target = 4096 - 160; +#if defined(_WIN32) + /* MAX_PATH: long roots are not characterised there; the listing is still + * checked for shape and content with every slot occupied. */ + target = 200; +#elif defined(PATH_MAX) && PATH_MAX < 4096 + /* macOS: the longest root the route can resolve; four still exceed 2 KB. */ + target = PATH_MAX - 96; +#endif + char deep[MAX_TEST_INDEX_JOBS][4096]; + char expected[MAX_TEST_INDEX_JOBS][4096]; + for (int j = 0; j < MAX_TEST_INDEX_JOBS; j++) { + snprintf(deep[j], sizeof(deep[j]), "%s/%d", base, j); + th_pad_path_to(deep[j], sizeof(deep[j]), target); + ASSERT_EQ((int)strlen(deep[j]), target); + ASSERT_EQ(th_mkdir_p(deep[j]), 0); + ASSERT_TRUE(cbm_canonical_path(deep[j], expected[j], sizeof(expected[j]))); + } + + th_ui_blocking_index_executor_t executor = {0}; + atomic_init(&executor.calls, 0); + atomic_init(&executor.release, 0); + th_server_t ts; + ts.srv = cbm_http_server_new(0); + ASSERT_NOT_NULL(ts.srv); + cbm_http_server_set_index_executor(ts.srv, th_ui_blocking_index_executor, &executor); + ASSERT_EQ(th_server_thread_start(&ts.tid, ts.srv), 0); + int port = cbm_http_server_port(ts.srv); + char status_request[128]; + snprintf(status_request, sizeof(status_request), + "GET /api/index-status HTTP/1.1\r\nHost: 127.0.0.1:%d\r\n\r\n", port); + + int accepted = 0; + bool started = true; + bool one_parsed = false; + bool one_whole = false; + bool all_parsed = false; + bool all_whole = false; + int one_count = 0; + int all_count = 0; + size_t one_body_len = 0; + size_t all_body_len = 0; + char listing[65536]; + for (int j = 0; j < MAX_TEST_INDEX_JOBS; j++) { + char body[4600]; + snprintf(body, sizeof(body), "{\"root_path\":\"%s\",\"project_name\":\"p%d\"}", deep[j], j); + char request[4900]; + snprintf(request, sizeof(request), + "POST /api/index HTTP/1.1\r\nContent-Type: application/json\r\n" + "Content-Length: %zu\r\n\r\n%s", + strlen(body), body); + char response[8192]; + int rn = th_http(port, request, response, sizeof(response)); + if (rn > 0 && th_status(response) == 202) { + accepted++; + } + /* Wait for the state the listing depends on: this job running. */ + started = started && th_wait_atomic_int(&executor.calls, j + 1, 5000); + if (j == 0 || j == MAX_TEST_INDEX_JOBS - 1) { + int n = th_http(port, status_request, listing, sizeof(listing)); + const char *status_body = n > 0 ? th_response_body(listing) : ""; + if (j == 0) { + one_body_len = strlen(status_body); + th_index_status_check(status_body, 1, expected, &one_parsed, &one_count, + &one_whole); + } else { + all_body_len = strlen(status_body); + th_index_status_check(status_body, MAX_TEST_INDEX_JOBS, expected, &all_parsed, + &all_count, &all_whole); + } + } + } + /* Release the workers and tear down; free refuses until each worker has + * flagged completion, the same way the active-worker test waits. */ + atomic_store(&executor.release, 1); + cbm_http_server_stop(ts.srv); + ASSERT_EQ(cbm_thread_join(&ts.tid), 0); + bool freed = false; + uint64_t deadline = cbm_now_ms() + 2000; + while (!freed && cbm_now_ms() < deadline) { + freed = cbm_http_server_free(ts.srv); + if (!freed) + cbm_usleep(1000); + } + th_cleanup(base); + + ASSERT_TRUE(freed); + ASSERT_EQ(accepted, MAX_TEST_INDEX_JOBS); + ASSERT_TRUE(started); + if (!one_parsed) { + char m[128]; + snprintf(m, sizeof(m), "one-job index-status body of %zu bytes is not JSON", one_body_len); + FAIL(m); + } + ASSERT_EQ(one_count, 1); + ASSERT_TRUE(one_whole); + if (!all_parsed) { + char m[128]; + snprintf(m, sizeof(m), "full-table index-status body of %zu bytes is not JSON", + all_body_len); + FAIL(m); + } + ASSERT_EQ(all_count, MAX_TEST_INDEX_JOBS); + ASSERT_TRUE(all_whole); + PASS(); +} + /* ── Suite ────────────────────────────────────────────────────── */ SUITE(httpd) { @@ -2651,6 +2873,7 @@ SUITE(httpd) { RUN_TEST(ui_server_browse_wide_dir_no_overflow); RUN_TEST(ui_server_logs_escape_dense_no_overflow); RUN_TEST(ui_server_index_status_long_paths_no_overflow); + RUN_TEST(ui_server_index_status_long_roots_render_whole); /* Parser / helpers */ RUN_TEST(httpd_parse_simple_get); RUN_TEST(httpd_parse_security_headers_and_rejects_duplicates); @@ -2688,6 +2911,7 @@ SUITE(httpd) { RUN_TEST(ui_server_index_start_refuses_root_outside_allowed); RUN_TEST(ui_server_index_start_stores_resolved_root); RUN_TEST(ui_server_index_start_long_root_not_truncated); + RUN_TEST(ui_server_index_start_reply_escapes_path); RUN_TEST(ui_server_root_without_embedded_assets_is_not_found); RUN_TEST(ui_server_same_origin_request_is_allowed); RUN_TEST(ui_server_rejects_foreign_and_null_origins); From f62d36d083e5a1e814431a573bb65fd9fd9bd37b Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 2 Oct 2026 05:03:35 +0200 Subject: [PATCH 05/15] test(cross-repo): free every matcher result the tests receive Since #2133 a cross-repo result owns heap memory: a wildcard run records each store it skips in result.skipped_projects, and the caller releases it with cbm_cross_repo_result_free(). Fifteen tests in test_cross_repo.c received a result and never released it. The one that seeds a pre-#768 store next to a wildcard target records a skip, so it leaked the skip array and the skipped project name: 32 bytes in 2 allocations, allocated in cr_record_skip. Every assertion still passed. The macOS leak lane (test-lsan-macos) was the run that reported it, because it builds with Homebrew LLVM and sets detect_leaks=1. The ordinary macOS runs, the local ones included, are built with Apple clang, where leak detection is off and cannot be turned on, so they stayed green. Each test now releases its result right after the fixture is torn down and before the first assertion, which is the shape the two tests that already released theirs use. The release therefore also runs when an assertion fails, and the scalar fields the assertions read stay valid after it. The repro_issue523 cases release their results the same way. They name their targets, so they record no skip and did not leak, but they hold a result and follow the same ownership rule. Signed-off-by: Martin Vogel --- tests/repro/repro_issue523.c | 7 +++++++ tests/test_cross_repo.c | 23 +++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/tests/repro/repro_issue523.c b/tests/repro/repro_issue523.c index 82d45b38d..14191e004 100644 --- a/tests/repro/repro_issue523.c +++ b/tests/repro/repro_issue523.c @@ -195,6 +195,7 @@ TEST(repro_issue523_crossrepo_http_calls_edge) { /* ── Cleanup ──────────────────────────────────────────────── */ rh_cleanup(&client, NULL); rh_cleanup(&server, NULL); + cbm_cross_repo_result_free(&result); /* * WHY RED: result.http_edges == 0 on current code. @@ -315,6 +316,7 @@ TEST(repro_issue523_scheme_stripped_url_match) { rh_cleanup(&client, NULL); rh_cleanup(&server, NULL); + cbm_cross_repo_result_free(&r); ASSERT_GTE(r.http_edges, 1); PASS(); } @@ -355,6 +357,7 @@ TEST(repro_issue523_template_fuzzy_match) { rh_cleanup(&client, NULL); rh_cleanup(&server, NULL); + cbm_cross_repo_result_free(&r); ASSERT_GTE(r.http_edges, 1); PASS(); } @@ -398,6 +401,7 @@ TEST(repro_issue523_reverse_direction_match) { rh_cleanup(&client, NULL); rh_cleanup(&server, NULL); + cbm_cross_repo_result_free(&r); ASSERT_GTE(r.http_edges, 1); ASSERT_GTE(client_cross, 1); PASS(); @@ -464,6 +468,9 @@ TEST(repro_issue523_idempotent_cross_edges) { rh_cleanup(&client, NULL); rh_cleanup(&server, NULL); + cbm_cross_repo_result_free(&r1); + cbm_cross_repo_result_free(&r2); + cbm_cross_repo_result_free(&r3); ASSERT_EQ(client_1, 1); ASSERT_EQ(server_1, 1); diff --git a/tests/test_cross_repo.c b/tests/test_cross_repo.c index 54c75cbd5..23d74fb5b 100644 --- a/tests/test_cross_repo.c +++ b/tests/test_cross_repo.c @@ -271,12 +271,14 @@ TEST(cross_repo_null_target_fails_without_dereference) { const char *targets[] = {NULL}; cbm_cross_repo_result_t result = cbm_cross_repo_match("null-target-source", targets, 1); rejected = result.failed; + cbm_cross_repo_result_free(&result); #else fflush(NULL); pid_t child = fork(); if (child == 0) { const char *targets[] = {NULL}; cbm_cross_repo_result_t result = cbm_cross_repo_match("null-target-source", targets, 1); + cbm_cross_repo_result_free(&result); _exit(result.failed ? 0 : 2); } int status = 0; @@ -308,6 +310,7 @@ TEST(cross_repo_wildcard_keeps_projects_containing_internal_tokens) { const char *targets[] = {"*"}; cbm_cross_repo_result_t result = cbm_cross_repo_match("wildcard-source", targets, 1); cross_repo_fixture_end(&fixture); + cbm_cross_repo_result_free(&result); ASSERT_FALSE(result.failed); ASSERT_EQ(result.projects_scanned, 4); @@ -367,6 +370,7 @@ TEST(cross_repo_wildcard_skips_pre768_store_issue2133) { cbm_cross_repo_result_t result = cbm_cross_repo_match("wild-src", targets, 1); int edges = cross_repo_count_edges(&fixture, "wild-src", "CROSS_HTTP_CALLS"); cross_repo_fixture_end(&fixture); + cbm_cross_repo_result_free(&result); ASSERT_FALSE(result.failed); ASSERT_EQ(result.projects_scanned, 1); @@ -394,6 +398,8 @@ TEST(cross_repo_named_pre768_target_fails_before_cleanup_issue2133) { cbm_cross_repo_result_t result = cbm_cross_repo_match("named-src", with_pre768, 2); int after = cross_repo_count_edges(&fixture, "named-src", "CROSS_HTTP_CALLS"); cross_repo_fixture_end(&fixture); + cbm_cross_repo_result_free(&initial); + cbm_cross_repo_result_free(&result); ASSERT_FALSE(initial.failed); ASSERT_EQ(before, 1); @@ -579,6 +585,7 @@ TEST(cross_repo_scan_bound_counts_examined_rows_not_matches) { const char *target = "bounded-target"; cbm_cross_repo_result_t result = cbm_cross_repo_match("bounded-source", &target, 1); cross_repo_fixture_end(&fixture); + cbm_cross_repo_result_free(&result); ASSERT_FALSE(result.failed); ASSERT_EQ(result.projects_scanned, 1); @@ -608,6 +615,8 @@ TEST(cross_repo_propagates_delete_failure) { failed = cbm_cross_repo_match("delete-source", &target, 1); } cross_repo_fixture_end(&fixture); + cbm_cross_repo_result_free(&initial); + cbm_cross_repo_result_free(&failed); ASSERT_TRUE(trigger_created); ASSERT_TRUE(failed.failed); @@ -631,6 +640,7 @@ TEST(cross_repo_failed_bidirectional_insert_is_not_counted) { const char *target = "insert-target"; cbm_cross_repo_result_t result = cbm_cross_repo_match("insert-source", &target, 1); cross_repo_fixture_end(&fixture); + cbm_cross_repo_result_free(&result); ASSERT_TRUE(result.failed); ASSERT_EQ(result.http_edges, 0); @@ -683,6 +693,7 @@ TEST(cross_repo_cancel_mid_run_keeps_completed_target_and_stops_before_later_tar int later_target_edges = cross_repo_count_edges(&fixture, "cancel-target-c", "CROSS_HTTP_CALLS"); cross_repo_fixture_end(&fixture); + cbm_cross_repo_result_free(&result); ASSERT_EQ(hook.fired, 1); ASSERT_TRUE(result.cancelled); @@ -714,6 +725,8 @@ TEST(cross_repo_pre_cancel_preserves_existing_cross_edges) { cbm_cross_repo_match_cancellable("pre-cancel-source", &target, 1, &cancelled); int after = cross_repo_count_edges(&fixture, "pre-cancel-source", "CROSS_HTTP_CALLS"); cross_repo_fixture_end(&fixture); + cbm_cross_repo_result_free(&initial); + cbm_cross_repo_result_free(&result); ASSERT_FALSE(initial.failed); ASSERT_TRUE(before > 0); @@ -747,6 +760,8 @@ TEST(cross_repo_self_only_target_fails_and_keeps_edges_issue1133) { cbm_cross_repo_result_t result = cbm_cross_repo_match("self-source", &self, 1); int after = cross_repo_count_edges(&fixture, "self-source", "CROSS_HTTP_CALLS"); cross_repo_fixture_end(&fixture); + cbm_cross_repo_result_free(&initial); + cbm_cross_repo_result_free(&result); ASSERT_FALSE(initial.failed); ASSERT_TRUE(before > 0); @@ -770,6 +785,7 @@ TEST(cross_repo_wildcard_with_no_other_project_fails_issue1133) { const char *targets[] = {"*"}; cbm_cross_repo_result_t result = cbm_cross_repo_match("lonely-source", targets, 1); cross_repo_fixture_end(&fixture); + cbm_cross_repo_result_free(&result); ASSERT_TRUE(result.failed); ASSERT_TRUE(result.no_targets); @@ -820,6 +836,7 @@ TEST(cross_repo_accepts_project_with_missed_shadow_row_issue1609) { const char *target = "shadow-target"; cbm_cross_repo_result_t result = cbm_cross_repo_match("shadow-source", &target, 1); cross_repo_fixture_end(&fixture); + cbm_cross_repo_result_free(&result); ASSERT_FALSE(result.failed); ASSERT_EQ(result.projects_scanned, 1); @@ -847,6 +864,8 @@ TEST(cross_repo_caller_local_handler_blocks_cross_http_issue1459) { int mono_edges = cross_repo_count_edges(&fixture, "mono-1459", "CROSS_HTTP_CALLS"); int api_edges = cross_repo_count_edges(&fixture, "api-1459", "CROSS_HTTP_CALLS"); cross_repo_fixture_end(&fixture); + cbm_cross_repo_result_free(&fwd); + cbm_cross_repo_result_free(&rev); ASSERT_FALSE(fwd.failed); ASSERT_FALSE(rev.failed); @@ -880,6 +899,8 @@ TEST(cross_repo_methodless_call_to_local_get_route_blocks_cross_http_issue1459) int mono_edges = cross_repo_count_edges(&fixture, "mono2-1459", "CROSS_HTTP_CALLS"); int api_edges = cross_repo_count_edges(&fixture, "api3-1459", "CROSS_HTTP_CALLS"); cross_repo_fixture_end(&fixture); + cbm_cross_repo_result_free(&fwd); + cbm_cross_repo_result_free(&rev); ASSERT_FALSE(fwd.failed); ASSERT_FALSE(rev.failed); @@ -908,6 +929,8 @@ TEST(cross_repo_caller_without_local_handler_keeps_cross_http_issue1459) { const char *consumer = "client-1459"; cbm_cross_repo_result_t rev = cbm_cross_repo_match("api2-1459", &consumer, 1); cross_repo_fixture_end(&fixture); + cbm_cross_repo_result_free(&fwd); + cbm_cross_repo_result_free(&rev); ASSERT_FALSE(fwd.failed); ASSERT_FALSE(rev.failed); From 5692dbe29982a344ee2d712957f75ece23a6557e Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 2 Oct 2026 20:56:41 +0200 Subject: [PATCH 06/15] ci: retry MSan apt updates after transient repository errors Signed-off-by: Martin Vogel --- test-infrastructure/Dockerfile.msan | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test-infrastructure/Dockerfile.msan b/test-infrastructure/Dockerfile.msan index ce9b616d0..144abf279 100644 --- a/test-infrastructure/Dockerfile.msan +++ b/test-infrastructure/Dockerfile.msan @@ -51,17 +51,19 @@ RUN printf '%s\n' \ # The key is fetched with `wget -O ` rather than `-qO- > `: under # retry a redirect is opened once for all attempts, so a partial write from a # failed attempt would be prepended to the output of a later successful one. -RUN retry apt-get -o Acquire::Retries=3 update \ +# APT normally treats transient index-download failures as warnings and exits +# zero. Make every update fail on those errors so the outer retry can run. +RUN retry apt-get -o Acquire::Retries=3 --error-on=any update \ && retry apt-get -o Acquire::Retries=3 install -y --no-install-recommends wget gnupg ca-certificates \ && retry wget -q -O /etc/apt/trusted.gpg.d/apt.llvm.org.asc https://apt.llvm.org/llvm-snapshot.gpg.key \ && echo "deb http://apt.llvm.org/noble/ llvm-toolchain-noble-21 main" > /etc/apt/sources.list.d/llvm-21.list \ - && retry apt-get -o Acquire::Retries=3 update \ + && retry apt-get -o Acquire::Retries=3 --error-on=any update \ && retry apt-get -o Acquire::Retries=3 install -y --no-install-recommends \ clang-21 libclang-rt-21-dev llvm-21 \ && ln -sf /usr/bin/clang-21 /usr/bin/clang \ && ln -sf /usr/bin/clang++-21 /usr/bin/clang++ -RUN retry apt-get -o Acquire::Retries=3 update \ +RUN retry apt-get -o Acquire::Retries=3 --error-on=any update \ && retry apt-get -o Acquire::Retries=3 install -y --no-install-recommends \ cmake \ ninja-build \ From fa258d342734d56e88180017bf0d92222813645d Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 25 Sep 2026 18:37:16 +0200 Subject: [PATCH 07/15] fix(extraction): parse every file as if its last line were terminated (#2078) A file whose last byte is not "\n" leaves the construct on its final line unterminated for every grammar that treats the line ending as part of that construct. #1610/#1746 already excused the mild form, a ZERO-WIDTH MISSING newline at EOF. The severe form is a WIDTH-BEARING error that cannot be excused. In tree-sitter-markdown an opening fence, a bare list marker or an ATX heading as the last bytes costs the last line (`text\n` + fence -> parse_partial 2-2) or the whole file (a lone fence, `- ` -> parse_unusable). An unterminated one-line heading was lost silently: the root is ERROR, there is no Section and no flag. A Makefile whose last recipe line is unterminated lost the recipe. The same bytes plus one "\n" parse clean. The reporter confirmed this on 21 real files (43 -> 23 flagged after appending newlines). Supply the newline rather than excuse its absence. cbm_parse_source() feeds the parser one virtual "\n" at offset source_len (through the TSInput read callback, so the caller's buffer is never copied or written) and then applies one ts_tree_edit that deletes that byte again. tree-sitter's edit arithmetic clamps every node that reached into it back to the real end, bytes and row/column points both. So def line ranges, node text, call byte spans, parse-coverage ranges and the retained tree all see only real bytes and real lines. Empty files and files that already end in "\n" are parsed exactly as before. Every whole-file parse goes through the helper: the extraction parse, and the whole-file fallback re-parses in the C/C++, C#, Go, Java, Kotlin, PHP, Python, Rust and TS LSP passes. That keeps a spilled result (no retained tree) consistent with a cached one. Sub-range and synthetic parses (preprocessed C, Rust wrappers, Kotlin patches, injected Jinja/imports ranges) are unchanged. The two #1610/#1746 guards that pinned "an unterminated Makefile recipe is a width-bearing loss" now use a construct that is broken with or without a newline (an unclosed Python tuple at EOF). The recipe is no longer lost, so flagging it would be false. The new makefile_unterminated_recipe_is_parsed_issue2078 test pins that. Proof (this repo, vendored grammars excluded, before/after binaries): - unchanged tree, and the same tree with the final newline stripped from all 1,220 files: nodes, edges and parse_partial lists byte-identical before vs after; - EOF-class corpus (the repo plus each .md with a fence, list marker, heading or opening fence appended without a newline, plus an unterminated Makefile): parse_partial 122 -> 77, exactly the unchanged repo's list. The only graph deltas are the 45 dropped ::missed shadow File rows (+7 Folders, 52 CONTAINS edges) and 22 Section docstrings that now include their real last line. The other four constructs in #2078 (HTML bare `&`, bash heredoc + redirect + pipe, bash `$((10#$n))`, markdown empty `||` cell) are upstream grammar gaps. Reports for them are drafted separately. Refs #2078 (the EOF class; the remaining triggers are upstream grammar gaps) Signed-off-by: Martin Vogel --- internal/cbm/cbm.c | 84 ++++++++-- internal/cbm/cbm.h | 8 + internal/cbm/lsp/c_lsp.c | 4 +- internal/cbm/lsp/cs_lsp.c | 10 +- internal/cbm/lsp/go_lsp.c | 4 +- internal/cbm/lsp/java_lsp.c | 4 +- internal/cbm/lsp/kotlin_lsp.c | 2 +- internal/cbm/lsp/php_lsp.c | 2 +- internal/cbm/lsp/py_lsp.c | 4 +- internal/cbm/lsp/rust_lsp.c | 4 +- internal/cbm/lsp/ts_lsp.c | 4 +- tests/test_parse_coverage.c | 285 +++++++++++++++++++++++++++++++++- 12 files changed, 376 insertions(+), 39 deletions(-) diff --git a/internal/cbm/cbm.c b/internal/cbm/cbm.c index 4d01d0c69..035285c30 100644 --- a/internal/cbm/cbm.c +++ b/internal/cbm/cbm.c @@ -278,6 +278,8 @@ void cbm_channels_push(CBMChannelArray *arr, CBMArena *a, CBMChannel ch) { typedef struct { const char *string; uint32_t length; + /* #2078: serve one "\n" at offset `length` -- see cbm_parse_source. */ + bool virtual_newline; } CBMStringInput; static const char *cbm_string_read(void *payload, uint32_t byte, TSPoint point, @@ -285,6 +287,10 @@ static const char *cbm_string_read(void *payload, uint32_t byte, TSPoint point, (void)point; CBMStringInput *self = (CBMStringInput *)payload; if (byte >= self->length) { + if (self->virtual_newline && byte == self->length) { + *bytes_read = 1; + return "\n"; + } *bytes_read = 0; return ""; } @@ -292,6 +298,60 @@ static const char *cbm_string_read(void *payload, uint32_t byte, TSPoint point, return self->string + byte; } +/* Where the parser stands after reading all `len` real bytes. */ +static TSPoint cbm_source_end_point(const char *source, uint32_t len) { + TSPoint end = {0, 0}; + const char *p = source; + const char *stop = source + len; + const char *nl; + while (p < stop && (nl = memchr(p, '\n', (size_t)(stop - p))) != NULL) { + end.row++; + p = nl + 1; + } + end.column = (uint32_t)(stop - p); + return end; +} + +/* #2078: parse every file as if its last line were terminated. + * + * Many grammars treat the line terminator as part of the construct it ends -- + * a markdown fence, heading or list marker, a Makefile recipe, a Dockerfile + * instruction. A file whose last byte is not "\n" leaves that construct + * unterminated: at best a zero-width MISSING token (#1610/#1746 excuse those at + * EOF), at worst a WIDTH-BEARING error that costs the whole last line or, for a + * one-line file, the whole file, and sometimes a silent loss (an unterminated + * markdown heading produced no Section and no flag). The same bytes plus one + * "\n" parse clean, so the absent newline is supplied instead of excused. + * + * The parser reads one virtual "\n" past EOF; afterwards a single tree edit + * deletes it again. tree-sitter's own edit arithmetic then clamps every node + * that reached into the virtual byte back to the real end -- byte offsets AND + * row/column points -- so every consumer (def line ranges, node text, call + * byte spans, parse-coverage ranges, the retained tree the LSP passes reuse) + * sees only real bytes and real lines; nothing downstream needs to know. The + * caller's buffer is never copied or written. + * + * A file that is empty or already ends in "\n" is parsed exactly as before. */ +TSTree *cbm_parse_source(TSParser *parser, const char *source, uint32_t source_len, + TSParseOptions opts) { + CBMStringInput input = {source, source_len, source_len > 0 && source[source_len - 1] != '\n'}; + TSInput ts_input = {&input, cbm_string_read, TSInputEncodingUTF8, NULL}; + TSTree *tree = ts_parser_parse_with_options(parser, NULL, ts_input, opts); + if (tree && input.virtual_newline) { + TSPoint end = cbm_source_end_point(source, source_len); + TSInputEdit drop_virtual_newline = { + .start_byte = source_len, + .old_end_byte = source_len + 1, + .new_end_byte = source_len, + .start_point = end, + .old_end_point = {end.row + 1, 0}, + .new_end_point = end, + }; + ts_tree_edit(tree, &drop_virtual_newline); + } + return tree; +} + // --- Parse timeout callback --- /* Budget for the tree-sitter progress callback. The PRIMARY gate is per-thread @@ -1261,7 +1321,11 @@ static void cbm_error_regions_push(cbm_error_regions_t *acc, TSNode n) { * #1746: the Dockerfile grammar places that zero-width missing newline before * trailing whitespace rather than at raw EOF. Preserve the broad exact-EOF * rule above; only extend it past blanks when the missing token is specifically - * a newline. */ + * a newline. + * + * #2078: cbm_parse_source now supplies the absent final newline, so a MISSING + * newline at EOF no longer arises from it; the rule stays for the other + * zero-width terminators a grammar can leave at EOF. */ static bool cbm_is_blank_not_newline(char c) { return c == ' ' || c == '\t' || c == '\v' || c == '\f' || c == '\r'; } @@ -2221,15 +2285,7 @@ static CBMFileResult *extract_file_ex_body(const char *source, int source_len, C uint64_t t0 = now_ns(); - // Build string input + timeout options for parse_with_options - CBMStringInput str_input = {source, (uint32_t)source_len}; - TSInput ts_input = { - &str_input, - cbm_string_read, - TSInputEncodingUTF8, - NULL, - }; - + // Timeout options for parse_with_options TSParseOptions opts = {0}; CBMParseBudget budget = {0}; // cppcheck-suppress unreadVariable uint64_t budget_ns = 0; @@ -2257,12 +2313,14 @@ static CBMFileResult *extract_file_ex_body(const char *source, int source_len, C /* #1735: a SQL data dump's literal-only INSERT rows carry no graph content * but dominate its parse. Keep them out through included ranges; offsets * and positions of everything kept are unchanged. The parser is - * thread-local and reused, so the ranges are cleared right after. */ + * thread-local and reused, so the ranges are cleared right after. The last + * range ends at the real EOF, so the virtual final newline cbm_parse_source + * supplies (#2078) stays outside a ranged parse. */ CBMSqlKeptRanges sql_kept = {NULL, 0}; bool sql_ranged = language == CBM_LANG_SQL && cbm_sql_values_exclusion_on(rel_path) && cbm_sql_values_kept_ranges(source, (uint32_t)source_len, &sql_kept) && ts_parser_set_included_ranges(parser, sql_kept.items, sql_kept.count); - TSTree *tree = ts_parser_parse_with_options(parser, NULL, ts_input, opts); + TSTree *tree = cbm_parse_source(parser, source, (uint32_t)source_len, opts); if (sql_ranged) { (void)ts_parser_set_included_ranges(parser, NULL, 0); } @@ -2465,7 +2523,7 @@ static CBMFileResult *extract_file_ex_body(const char *source, int source_len, C TSParser *pp_parser = get_thread_parser(ts_lang, language); if (pp_parser) { ts_parser_reset(pp_parser); - CBMStringInput pp_input = {expanded, (uint32_t)expanded_len}; + CBMStringInput pp_input = {expanded, (uint32_t)expanded_len, false}; TSInput pp_ts_input = { &pp_input, cbm_string_read, diff --git a/internal/cbm/cbm.h b/internal/cbm/cbm.h index dc89a6b2d..3bd310d8b 100644 --- a/internal/cbm/cbm.h +++ b/internal/cbm/cbm.h @@ -819,6 +819,14 @@ void cbm_work_arena_keep_begin(void); /* Free the compaction scratch this thread kept (cbm_work_arena_release calls it). */ void cbm_result_compact_release_thread(void); +/* Parse one whole file as if its last line ended with "\n" (#2078). The + * parser sees the source plus one virtual newline when the last byte is not + * already one; the returned tree is then clamped back to `source_len`, so no + * node range, point or text reaches past the real bytes. Every whole-file parse + * goes through here, so a retained tree and a fallback re-parse agree. */ +TSTree *cbm_parse_source(TSParser *parser, const char *source, uint32_t source_len, + TSParseOptions opts); + // Extract all data from one file. Caller must call cbm_free_result(). // source must remain valid for the duration of the call. // timeout_micros: per-file tree-sitter parse budget in microseconds of the diff --git a/internal/cbm/lsp/c_lsp.c b/internal/cbm/lsp/c_lsp.c index 22d630020..73141b0f6 100644 --- a/internal/cbm/lsp/c_lsp.c +++ b/internal/cbm/lsp/c_lsp.c @@ -6101,7 +6101,7 @@ void cbm_run_c_lsp_cross_with_registry(CBMArena *arena, const char *source, int return; const TSLanguage *ts_lang = cpp_mode ? tree_sitter_cpp() : tree_sitter_c(); ts_parser_set_language(parser, ts_lang); - tree = ts_parser_parse_string(parser, NULL, source, source_len); + tree = cbm_parse_source(parser, source, (uint32_t)source_len, (TSParseOptions){0}); ts_parser_delete(parser); owns_tree = true; if (!tree) @@ -6151,7 +6151,7 @@ void cbm_run_c_lsp_cross(CBMArena *arena, const char *source, int source_len, co return; const TSLanguage *ts_lang = cpp_mode ? tree_sitter_cpp() : tree_sitter_c(); ts_parser_set_language(parser, ts_lang); - tree = ts_parser_parse_string(parser, NULL, source, source_len); + tree = cbm_parse_source(parser, source, (uint32_t)source_len, (TSParseOptions){0}); ts_parser_delete(parser); owns_tree = true; if (!tree) diff --git a/internal/cbm/lsp/cs_lsp.c b/internal/cbm/lsp/cs_lsp.c index fc3b548ef..749b126bf 100644 --- a/internal/cbm/lsp/cs_lsp.c +++ b/internal/cbm/lsp/cs_lsp.c @@ -3763,8 +3763,9 @@ void cbm_run_cs_lsp_cross_with_registry(CBMArena *arena, const char *source, int if (!parser) return; ts_parser_set_language(parser, tree_sitter_c_sharp()); - tree = ts_parser_parse_string( - parser, NULL, source, source_len > 0 ? (uint32_t)source_len : (uint32_t)strlen(source)); + tree = cbm_parse_source(parser, source, + source_len > 0 ? (uint32_t)source_len : (uint32_t)strlen(source), + (TSParseOptions){0}); ts_parser_delete(parser); owns = true; } @@ -3805,8 +3806,9 @@ void cbm_run_cs_lsp_cross(CBMArena *arena, const char *source, int source_len, if (!parser) return; ts_parser_set_language(parser, tree_sitter_c_sharp()); - tree = ts_parser_parse_string( - parser, NULL, source, source_len > 0 ? (uint32_t)source_len : (uint32_t)strlen(source)); + tree = cbm_parse_source(parser, source, + source_len > 0 ? (uint32_t)source_len : (uint32_t)strlen(source), + (TSParseOptions){0}); ts_parser_delete(parser); owns = true; } diff --git a/internal/cbm/lsp/go_lsp.c b/internal/cbm/lsp/go_lsp.c index ba7df163b..58c151a53 100644 --- a/internal/cbm/lsp/go_lsp.c +++ b/internal/cbm/lsp/go_lsp.c @@ -3224,7 +3224,7 @@ void cbm_run_go_lsp_cross(CBMArena *arena, const char *source, int source_len, if (!parser) return; ts_parser_set_language(parser, tree_sitter_go()); - tree = ts_parser_parse_string(parser, NULL, source, source_len); + tree = cbm_parse_source(parser, source, (uint32_t)source_len, (TSParseOptions){0}); owns_tree = true; if (!tree) { ts_parser_delete(parser); @@ -3630,7 +3630,7 @@ void cbm_run_go_lsp_cross_with_registry(CBMArena *arena, const char *source, int if (!parser) return; ts_parser_set_language(parser, tree_sitter_go()); - tree = ts_parser_parse_string(parser, NULL, source, source_len); + tree = cbm_parse_source(parser, source, (uint32_t)source_len, (TSParseOptions){0}); owns_tree = true; if (!tree) { ts_parser_delete(parser); diff --git a/internal/cbm/lsp/java_lsp.c b/internal/cbm/lsp/java_lsp.c index 9a79783ff..2b518d890 100644 --- a/internal/cbm/lsp/java_lsp.c +++ b/internal/cbm/lsp/java_lsp.c @@ -3931,7 +3931,7 @@ void cbm_run_java_lsp_cross_with_registry(CBMArena *arena, CBMFileResult *result return; } ts_parser_set_language(parser, tree_sitter_java()); - tree = ts_parser_parse_string(parser, NULL, source, (uint32_t)source_len); + tree = cbm_parse_source(parser, source, (uint32_t)source_len, (TSParseOptions){0}); ts_parser_delete(parser); owns_tree = true; } @@ -3985,7 +3985,7 @@ void cbm_run_java_lsp_cross(CBMArena *arena, const char *source, int source_len, if (!tree) { TSParser *parser = ts_parser_new(); ts_parser_set_language(parser, tree_sitter_java()); - tree = ts_parser_parse_string(parser, NULL, source, (uint32_t)source_len); + tree = cbm_parse_source(parser, source, (uint32_t)source_len, (TSParseOptions){0}); ts_parser_delete(parser); owns_tree = true; } diff --git a/internal/cbm/lsp/kotlin_lsp.c b/internal/cbm/lsp/kotlin_lsp.c index 0ca49afd5..cc6be1680 100644 --- a/internal/cbm/lsp/kotlin_lsp.c +++ b/internal/cbm/lsp/kotlin_lsp.c @@ -5415,7 +5415,7 @@ void cbm_run_kotlin_lsp_cross(CBMArena *arena, const char *source, int source_le return; } ts_parser_set_language(parser, tree_sitter_kotlin()); - tree = ts_parser_parse_string(parser, NULL, source, (uint32_t)source_len); + tree = cbm_parse_source(parser, source, (uint32_t)source_len, (TSParseOptions){0}); ts_parser_delete(parser); owns_tree = true; } diff --git a/internal/cbm/lsp/php_lsp.c b/internal/cbm/lsp/php_lsp.c index 4caf5ba0f..81347a94c 100644 --- a/internal/cbm/lsp/php_lsp.c +++ b/internal/cbm/lsp/php_lsp.c @@ -4498,7 +4498,7 @@ void cbm_run_php_lsp_cross(CBMArena *arena, const char *source, int source_len, if (!parser) return; ts_parser_set_language(parser, tree_sitter_php_only()); - tree = ts_parser_parse_string(parser, NULL, source, (uint32_t)source_len); + tree = cbm_parse_source(parser, source, (uint32_t)source_len, (TSParseOptions){0}); owns_tree = true; if (!tree) { ts_parser_delete(parser); diff --git a/internal/cbm/lsp/py_lsp.c b/internal/cbm/lsp/py_lsp.c index 70de5dce7..2b47fbf23 100644 --- a/internal/cbm/lsp/py_lsp.c +++ b/internal/cbm/lsp/py_lsp.c @@ -5110,7 +5110,7 @@ void cbm_run_py_lsp_cross(CBMArena *arena, const char *source, int source_len, if (!parser) return; ts_parser_set_language(parser, tree_sitter_python()); - tree = ts_parser_parse_string(parser, NULL, source, (uint32_t)source_len); + tree = cbm_parse_source(parser, source, (uint32_t)source_len, (TSParseOptions){0}); owns_tree = true; if (!tree) { ts_parser_delete(parser); @@ -5191,7 +5191,7 @@ void cbm_run_py_lsp_cross_with_registry(CBMArena *arena, const char *source, int if (!parser) return; ts_parser_set_language(parser, tree_sitter_python()); - tree = ts_parser_parse_string(parser, NULL, source, (uint32_t)source_len); + tree = cbm_parse_source(parser, source, (uint32_t)source_len, (TSParseOptions){0}); owns_tree = true; if (!tree) { ts_parser_delete(parser); diff --git a/internal/cbm/lsp/rust_lsp.c b/internal/cbm/lsp/rust_lsp.c index 68fb444a6..44680cb69 100644 --- a/internal/cbm/lsp/rust_lsp.c +++ b/internal/cbm/lsp/rust_lsp.c @@ -6377,7 +6377,7 @@ void cbm_run_rust_lsp_cross_with_registry(CBMArena *arena, const char *source, i if (!parser) return; ts_parser_set_language(parser, tree_sitter_rust()); - tree = ts_parser_parse_string(parser, NULL, source, source_len); + tree = cbm_parse_source(parser, source, (uint32_t)source_len, (TSParseOptions){0}); owns_tree = true; if (!tree) { ts_parser_delete(parser); @@ -6412,7 +6412,7 @@ void cbm_run_rust_lsp_cross_with_manifest(CBMArena *arena, const char *source, i if (!parser) return; ts_parser_set_language(parser, tree_sitter_rust()); - tree = ts_parser_parse_string(parser, NULL, source, source_len); + tree = cbm_parse_source(parser, source, (uint32_t)source_len, (TSParseOptions){0}); owns_tree = true; if (!tree) { ts_parser_delete(parser); diff --git a/internal/cbm/lsp/ts_lsp.c b/internal/cbm/lsp/ts_lsp.c index 967d06762..c25a58bbe 100644 --- a/internal/cbm/lsp/ts_lsp.c +++ b/internal/cbm/lsp/ts_lsp.c @@ -5899,7 +5899,7 @@ void cbm_run_ts_lsp_cross_with_registry(CBMArena *arena, const char *source, int jsx_mode ? (js_mode ? tree_sitter_javascript() : tree_sitter_tsx()) : (js_mode ? tree_sitter_javascript() : tree_sitter_typescript()); ts_parser_set_language(parser, lang); - tree = ts_parser_parse_string(parser, NULL, source, source_len); + tree = cbm_parse_source(parser, source, (uint32_t)source_len, (TSParseOptions){0}); ts_parser_delete(parser); if (!tree) return; @@ -6110,7 +6110,7 @@ void cbm_run_ts_lsp_cross(CBMArena *arena, const char *source, int source_len, jsx_mode ? (js_mode ? tree_sitter_javascript() : tree_sitter_tsx()) : (js_mode ? tree_sitter_javascript() : tree_sitter_typescript()); ts_parser_set_language(parser, lang); - tree = ts_parser_parse_string(parser, NULL, source, source_len); + tree = cbm_parse_source(parser, source, (uint32_t)source_len, (TSParseOptions){0}); ts_parser_delete(parser); if (!tree) return; diff --git a/tests/test_parse_coverage.c b/tests/test_parse_coverage.c index 09caeaa82..1cc2f8e98 100644 --- a/tests/test_parse_coverage.c +++ b/tests/test_parse_coverage.c @@ -507,11 +507,17 @@ TEST(real_error_before_eof_still_flagged_without_final_newline_issue1610) { } /* GUARD: a MISSING/ERROR node WITH WIDTH at EOF is a genuine loss and must - * still be flagged. A Makefile whose final recipe line lacks its newline really - * does drop the recipe from the tree — cbm's flag is honest there. */ + * still be flagged. The construct here is broken whether or not the line is + * terminated, so neither the zero-width rule nor the virtual final newline + * (#2078) may excuse it. + * + * This guard used a Makefile whose final recipe line lacks its newline, which + * really did drop the recipe from the tree. Since #2078 the parser sees that + * line terminated and the recipe parses, so there is no loss left to report -- + * see makefile_unterminated_recipe_is_parsed_issue2078. */ TEST(width_bearing_error_at_eof_still_flagged_issue1610) { - const char *src = "all:\n\techo hi"; /* no trailing newline; recipe is lost */ - CBMFileResult *r = do_extract(src, CBM_LANG_MAKEFILE, "Makefile"); + const char *src = "def ok():\n return 1\nx = (1,"; /* unclosed at EOF */ + CBMFileResult *r = do_extract(src, CBM_LANG_PYTHON, "eof.py"); ASSERT_NOT_NULL(r); bool flagged = r->parse_incomplete; cbm_free_result(r); @@ -521,6 +527,22 @@ TEST(width_bearing_error_at_eof_still_flagged_issue1610) { PASS(); } +/* #2078: the recipe the old #1610 guard lost is now parsed -- the file is not + * flagged, with or without a trailing blank. */ +TEST(makefile_unterminated_recipe_is_parsed_issue2078) { + const char *cases[] = {"all:\n\techo hi", "all:\n\techo hi "}; + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + CBMFileResult *r = do_extract(cases[i], CBM_LANG_MAKEFILE, "Makefile"); + ASSERT_NOT_NULL(r); + bool flagged = r->parse_incomplete; + cbm_free_result(r); + if (flagged) { + FAIL("an unterminated final recipe line must parse like a terminated one"); + } + } + PASS(); +} + /* #1838: tree-sitter-perl v1.0.0 rejects a valid line-oriented format and * reports the file as partial. The supported upstream v1.2.1 grammar accepts * the format and preserves declaration extraction beyond its dot terminator. */ @@ -611,11 +633,12 @@ TEST(real_error_before_eof_still_flagged_with_trailing_blank_issue1746) { PASS(); } -/* GUARD: a WIDTH-BEARING loss at EOF stays honest with a blank tail too — the - * Makefile recipe really is dropped, and only zero-width nodes are excused. */ +/* GUARD: a WIDTH-BEARING loss at EOF stays honest with a blank tail too, and + * only zero-width nodes are excused. (Formerly the unterminated Makefile + * recipe, which #2078 now parses -- see the #1610 guard above.) */ TEST(width_bearing_error_at_eof_still_flagged_with_trailing_blank_issue1746) { - const char *src = "all:\n\techo hi "; - CBMFileResult *r = do_extract(src, CBM_LANG_MAKEFILE, "Makefile"); + const char *src = "def ok():\n return 1\nx = (1, "; + CBMFileResult *r = do_extract(src, CBM_LANG_PYTHON, "eof.py"); ASSERT_NOT_NULL(r); bool flagged = r->parse_incomplete; cbm_free_result(r); @@ -1615,6 +1638,246 @@ TEST(cs_malformed_conditional_remains_partial_issue1748) { PASS(); } +/* ── #2078: every file is parsed as if it ended with a newline ─────────────── + * + * Several grammars need a line terminator that a file's last line may simply + * not have. tree-sitter-markdown is the sharpest case: an opening code fence, + * an ATX heading or a bare list marker as the final bytes of a file leaves a + * WIDTH-BEARING ERROR (so the #1610 zero-width rule cannot excuse it), and a + * file that is only "```" came back as a whole-file error. Appending one "\n" + * made every one of these clean -- the reporter proved it on 21 real files. + * + * cbm now feeds the parser one virtual "\n" past EOF when the last byte is not + * already a newline, then clamps the tree back to the real length, so no + * consumer sees a byte, a line or a range that is not in the file. */ +TEST(markdown_unterminated_last_line_is_clean_issue2078) { + const char *cases[] = { + "text\n```", /* opening fence at EOF (upstream PR #262, closed unmerged) */ + "```", /* the whole file is one opening fence */ + "# Top", /* ATX heading */ + "- ", /* bare list marker */ + }; + int flagged = 0; + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + CBMFileResult *r = do_extract(cases[i], CBM_LANG_MARKDOWN, "doc.md"); + ASSERT_NOT_NULL(r); + if (r->parse_incomplete || r->parse_unusable) { + fprintf(stderr, " case %zu flagged: partial=%d unusable=%d ranges=%s\n", i, + r->parse_incomplete, r->parse_unusable, + r->error_ranges ? r->error_ranges : "(none)"); + flagged++; + } + cbm_free_result(r); + } + if (flagged) { + FAIL("a markdown file must not be flagged only because its last line is unterminated"); + } + PASS(); +} + +/* Controls: the same bytes WITH the newline were already clean and stay so. */ +TEST(markdown_terminated_last_line_controls_unchanged_issue2078) { + const char *cases[] = {"text\n```\n", "```\n", "# Top\n", "- \n"}; + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + CBMFileResult *r = do_extract(cases[i], CBM_LANG_MARKDOWN, "doc.md"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->parse_incomplete); + ASSERT_FALSE(r->parse_unusable); + cbm_free_result(r); + } + PASS(); +} + +/* Everything extraction reports, as one string, so two results compare + * byte-for-byte: defs (every text field and range), calls (exact byte spans), + * imports and the coverage verdict. */ +typedef struct { + char *buf; + size_t cap; + size_t len; +} dump_buf_t; + +static void dump_str(dump_buf_t *b, const char *tag, const char *s) { + int n = snprintf(b->buf + b->len, b->cap - b->len, " %s=%s", tag, s ? s : "~"); + if (n > 0 && (size_t)n < b->cap - b->len) { + b->len += (size_t)n; + } +} + +static void dump_num(dump_buf_t *b, const char *tag, long v) { + int n = snprintf(b->buf + b->len, b->cap - b->len, " %s=%ld", tag, v); + if (n > 0 && (size_t)n < b->cap - b->len) { + b->len += (size_t)n; + } +} + +/* The Module def spans the whole file, and its end line follows the line + * convention for a terminated last line (#1967), so it is compared on its own + * rather than through the dump. */ +static char *dump_extraction(const CBMFileResult *r) { + dump_buf_t b = {(char *)calloc(1u << 16, 1), 1u << 16, 0}; + if (!b.buf) { + return NULL; + } + for (int i = 0; i < r->defs.count; i++) { + const CBMDefinition *d = &r->defs.items[i]; + if (d->label && strcmp(d->label, "Module") == 0) { + continue; + } + dump_str(&b, "\nD", d->qualified_name); + dump_str(&b, "name", d->name); + dump_str(&b, "label", d->label); + dump_num(&b, "start", d->start_line); + dump_num(&b, "end", d->end_line); + dump_num(&b, "lines", d->lines); + dump_num(&b, "cx", d->complexity); + dump_num(&b, "params", d->param_count); + dump_str(&b, "sig", d->signature); + dump_str(&b, "ret", d->return_type); + dump_str(&b, "doc", d->docstring); + dump_str(&b, "prof", d->structural_profile); + dump_str(&b, "tok", d->body_tokens); + } + for (int i = 0; i < r->calls.count; i++) { + const CBMCall *c = &r->calls.items[i]; + dump_str(&b, "\nC", c->callee_name); + dump_str(&b, "in", c->enclosing_func_qn); + dump_num(&b, "line", c->start_line); + dump_num(&b, "from", (long)c->site_start_byte); + dump_num(&b, "to", (long)c->site_end_byte); + dump_num(&b, "args", c->arg_count); + } + for (int i = 0; i < r->imports.count; i++) { + dump_str(&b, "\nI", r->imports.items[i].local_name); + dump_str(&b, "path", r->imports.items[i].module_path); + } + dump_str(&b, "\nP", r->error_ranges); + dump_num(&b, "partial", r->parse_incomplete); + dump_num(&b, "unusable", r->parse_unusable); + dump_str(&b, "", "\n"); + return b.buf; +} + +static uint32_t module_end_line(const CBMFileResult *r) { + for (int i = 0; i < r->defs.count; i++) { + if (r->defs.items[i].label && strcmp(r->defs.items[i].label, "Module") == 0) { + return r->defs.items[i].end_line; + } + } + return 0; +} + +static char *extract_dump(const char *src, CBMLanguage lang, const char *path, + uint32_t *module_end) { + CBMFileResult *r = do_extract(src, lang, path); + if (!r) { + return NULL; + } + char *d = dump_extraction(r); + if (module_end) { + *module_end = module_end_line(r); + } + cbm_free_result(r); + return d; +} + +/* GUARD: for code grammars a trailing newline is insignificant, so a file + * without one must extract byte-identically to the same file with one -- every + * def, range, text field and call byte span. The virtual newline may never + * show up as phantom content or a phantom line. */ +TEST(code_without_final_newline_extracts_identically_issue2078) { + struct { + const char *src; /* no trailing newline */ + CBMLanguage lang; + const char *path; + uint32_t lines; + } cases[] = { + {"#include \n\nstatic int beta(int x) {\n return x + 1;\n}\n\n" + "void alpha(void) { printf(\"a\"); beta(2); }", + CBM_LANG_C, "a.c", 7}, + {"import os\n\n\nclass K:\n def m(self, a):\n return os.path.join(a)\n\n\n" + "def f(x):\n \"\"\"doc\"\"\"\n return K().m(x)", + CBM_LANG_PYTHON, "a.py", 11}, + }; + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + size_t n = strlen(cases[i].src); + char *terminated = (char *)malloc(n + 2); + ASSERT_NOT_NULL(terminated); + memcpy(terminated, cases[i].src, n); + terminated[n] = '\n'; + terminated[n + 1] = '\0'; + uint32_t bare_module_end = 0; + char *bare = extract_dump(cases[i].src, cases[i].lang, cases[i].path, &bare_module_end); + char *term = extract_dump(terminated, cases[i].lang, cases[i].path, NULL); + free(terminated); + ASSERT_NOT_NULL(bare); + ASSERT_NOT_NULL(term); + bool same = strcmp(bare, term) == 0; + if (!same) { + fprintf(stderr, " %s differs:\n--- without final newline%s--- with final newline%s", + cases[i].path, bare, term); + } + free(bare); + free(term); + if (!same) { + FAIL("extraction must not depend on whether the last line is terminated"); + } + if (bare_module_end != cases[i].lines) { + fprintf(stderr, " %s: module ends on line %u, file has %u\n", cases[i].path, + bare_module_end, cases[i].lines); + FAIL("the virtual newline must not add a line to the module"); + } + } + PASS(); +} + +/* GUARD against phantom content: a heading on an unterminated last line is a + * one-line file. No def may end past line 1, no text may carry the virtual + * newline, and the result must equal the terminated file's. */ +TEST(virtual_newline_adds_no_phantom_line_or_text_issue2078) { + CBMFileResult *r = do_extract("# Top", CBM_LANG_MARKDOWN, "top.md"); + ASSERT_NOT_NULL(r); + bool found = has_def(r, "Top"); + for (int i = 0; i < r->defs.count; i++) { + const CBMDefinition *d = &r->defs.items[i]; + if (d->end_line > 1 || d->start_line > 1) { + fprintf(stderr, " def %s spans %u-%u\n", d->name ? d->name : "?", d->start_line, + d->end_line); + cbm_free_result(r); + FAIL("no def may reach past the last real line"); + } + if ((d->name && strchr(d->name, '\n')) || (d->signature && strchr(d->signature, '\n'))) { + cbm_free_result(r); + FAIL("no extracted text may contain the virtual newline"); + } + } + cbm_free_result(r); + if (!found) { + /* Before #2078 the unterminated heading was silently dropped: no + * Section, and no parse_partial flag to say so. */ + FAIL("the heading on an unterminated last line must be extracted as a Section"); + } + PASS(); +} + +/* GUARD: the empty file and a file that is exactly "\n" get no virtual + * newline (nothing is unterminated) and stay clean. */ +TEST(empty_and_newline_only_files_unchanged_issue2078) { + const char *srcs[] = {"", "\n"}; + CBMLanguage langs[] = {CBM_LANG_MARKDOWN, CBM_LANG_PYTHON, CBM_LANG_C}; + const char *paths[] = {"e.md", "e.py", "e.c"}; + for (size_t s = 0; s < 2; s++) { + for (size_t l = 0; l < 3; l++) { + CBMFileResult *r = do_extract(srcs[s], langs[l], paths[l]); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->parse_incomplete); + ASSERT_FALSE(r->parse_unusable); + cbm_free_result(r); + } + } + PASS(); +} + SUITE(parse_coverage) { RUN_TEST(c_ifdef_split_brace_sets_parse_incomplete); RUN_TEST(c_ifdef_split_brace_neighbors_still_extracted); @@ -1640,6 +1903,7 @@ SUITE(parse_coverage) { RUN_TEST(missing_final_newline_not_flagged_across_grammars_issue1610); RUN_TEST(real_error_before_eof_still_flagged_without_final_newline_issue1610); RUN_TEST(width_bearing_error_at_eof_still_flagged_issue1610); + RUN_TEST(makefile_unterminated_recipe_is_parsed_issue2078); RUN_TEST(c_ifdef_split_range_narrows_to_dropped_branch); RUN_TEST(c_ifdef_split_range_excludes_lines_the_preprocessor_explained); RUN_TEST(c_ifdef_split_range_never_starts_on_a_directive); @@ -1666,4 +1930,9 @@ SUITE(parse_coverage) { RUN_TEST(cs_collection_expression_in_conditional_is_complete_issue1748); RUN_TEST(cs_calls_inside_collection_expression_extracted_issue1748); RUN_TEST(cs_malformed_conditional_remains_partial_issue1748); + RUN_TEST(markdown_unterminated_last_line_is_clean_issue2078); + RUN_TEST(markdown_terminated_last_line_controls_unchanged_issue2078); + RUN_TEST(code_without_final_newline_extracts_identically_issue2078); + RUN_TEST(virtual_newline_adds_no_phantom_line_or_text_issue2078); + RUN_TEST(empty_and_newline_only_files_unchanged_issue2078); } From 1522c160510912b30d114e6c435ec17a52b8cdfc Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 4 Oct 2026 18:11:07 +0200 Subject: [PATCH 08/15] test(httpd): expect the resolved allowed root in its platform form ui_server_index_start_stores_resolved_root built its expected root as the resolved parent plus "/allowed". On Windows the server stores the resolved form of the directory itself, which uses backslashes throughout, so the comparison saw a mixed-separator string and failed on the windows-latest CLANG64 job. The test now resolves the allowed directory with cbm_canonical_path, as the server does, and compares the 202 reply against the JSON-escaped form, as the status-listing check already did; a backslash appears escaped in both bodies. Production is unchanged; on POSIX the expected strings are identical to before. Signed-off-by: Martin Vogel --- tests/test_httpd.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/test_httpd.c b/tests/test_httpd.c index d020dc7da..4408f4ef5 100644 --- a/tests/test_httpd.c +++ b/tests/test_httpd.c @@ -1188,10 +1188,13 @@ TEST(ui_server_index_start_stores_resolved_root) { char inner[512]; snprintf(inner, sizeof(inner), "%s/allowed/inner", base); ASSERT_EQ(th_mkdir_p(inner), 0); - char canonical_base[4096]; - ASSERT_TRUE(cbm_canonical_path(base, canonical_base, sizeof(canonical_base))); - char allowed_root[4096 + 16]; - snprintf(allowed_root, sizeof(allowed_root), "%s/allowed", canonical_base); + /* The resolved form of the allowed directory itself, as the server + * computes it. A "/allowed" suffix on a resolved parent is not that form + * on Windows, where the resolved path uses backslashes throughout. */ + char allowed_dir[512]; + snprintf(allowed_dir, sizeof(allowed_dir), "%s/allowed", base); + char allowed_root[4096]; + ASSERT_TRUE(cbm_canonical_path(allowed_dir, allowed_root, sizeof(allowed_root))); /* The request names the allowed root through a ".." segment, so its * spelling is not the resolved form (on macOS /tmp is itself a link to * /private/tmp, so the prefix differs as well). */ @@ -1212,12 +1215,13 @@ TEST(ui_server_index_start_stores_resolved_root) { * spelling. */ ASSERT_STR_EQ(probe.executor.root_path, allowed_root); ASSERT_STR_EQ(probe.executor.project_name, "resolved"); - /* The 202 reply and the status listing report that same stored root. */ - char expected[8192 + 64]; - snprintf(expected, sizeof(expected), "\"path\":\"%s\"}", allowed_root); - ASSERT_NOT_NULL(strstr(probe.response, expected)); + /* The 202 reply and the status listing report that same stored root, + * JSON-escaped (a Windows path carries backslashes). */ char escaped[8192]; cbm_json_escape(escaped, (int)sizeof(escaped), allowed_root); + char expected[8192 + 64]; + snprintf(expected, sizeof(expected), "\"path\":\"%s\"}", escaped); + ASSERT_NOT_NULL(strstr(probe.response, expected)); snprintf(expected, sizeof(expected), "\"path\":\"%s\",\"error\"", escaped); ASSERT_NOT_NULL(strstr(probe.index_status, expected)); PASS(); From c442028010bee9b4ae4e24905fad7fdcd8966b34 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 2 Oct 2026 21:36:08 +0200 Subject: [PATCH 09/15] fix(workspace): classify manifest entries in their resolved form A project's .cbmpathwhitelist lists extra roots it would like indexed. When such a manifest was approved, and again each time it was consulted, every entry was classified exactly as written in the file, while the directory that would actually be indexed is the one the entry resolves to. An entry that reached the home directory, a credential directory or the volume root through a link or through `..` segments was judged by its spelling and passed as an ordinary path; only the containment check resolved it. An entry naming a directory that did not exist was approved as well. Both cbm_workspace_manifest_approve and cbm_workspace_manifest_allows now resolve each entry with cbm_canonical_path, the resolver every other caller of the policy applies, and classify that form. Approval refuses an entry that is relative, does not exist or is not a directory, with a message that names the entry and, when the resolved directory differs from what was written, shows both. Approval stays bound to the manifest text; where an entry leads is read again when it is used, so a link that is retargeted after approval is judged by its new target. At use time an entry that no longer resolves matches nothing, and the other entries of the manifest are consulted as before. An ordinary, existing, absolute entry keeps its result. A manifest that names a directory which does not exist yet is now refused at approval and has to be approved once the directory is there. Signed-off-by: Martin Vogel --- src/foundation/workspace.c | 69 ++++++- src/foundation/workspace.h | 15 +- tests/test_workspace.c | 384 +++++++++++++++++++++++++++++++++++-- 3 files changed, 441 insertions(+), 27 deletions(-) diff --git a/src/foundation/workspace.c b/src/foundation/workspace.c index 40e009a03..eab52d48f 100644 --- a/src/foundation/workspace.c +++ b/src/foundation/workspace.c @@ -776,6 +776,43 @@ bool cbm_workspace_manifest_is_approved(const char *cache_dir, const char *proje return found; } +/* Resolve a manifest entry to the directory it names — the form the policy is + * defined over. The file holds the entry as a person wrote it, and a spelling + * can reach a directory through a link or through `..` segments; the directory + * that would be indexed is the resolved one, so that is the one classified, at + * approval and again at use. Only an absolute entry is accepted: the manifest + * is read by a long-lived process whose working directory is unrelated to the + * project, so a relative spelling has no base it could honestly be resolved + * against. Returns NULL on success, otherwise the reason the entry is unusable. + * out must hold WS_LINE_MAX bytes. */ +static const char *ws_manifest_entry_resolve(const char *entry, char *out, size_t out_sz) { + if (ws_volume_prefix_len(entry) == 0) { + return "must be an absolute path"; + } + if (!cbm_canonical_path(entry, out, out_sz)) { + return "must name an existing directory"; + } + if (!cbm_is_dir(out)) { + return "is not a directory"; + } + return NULL; +} + +/* "requested path : ", with the resolved form shown when it + * differs from what was written: the person typed one and the policy judged + * the other. */ +static void ws_manifest_entry_refusal(char *err, size_t err_sz, const char *entry, + const char *resolved, const char *reason) { + if (!err || err_sz == 0) { + return; + } + if (resolved && strcmp(resolved, entry) != 0) { + snprintf(err, err_sz, "requested path %s (resolves to %s): %s", entry, resolved, reason); + } else { + snprintf(err, err_sz, "requested path %s: %s", entry, reason); + } +} + bool cbm_workspace_manifest_approve(const char *cache_dir, const char *home_dir, const char *project_root, char *err, size_t err_sz) { if (err && err_sz) { @@ -800,14 +837,19 @@ bool cbm_workspace_manifest_approve(const char *cache_dir, const char *home_dir, } /* Approving a manifest must not become a way around the breadth policy: every - * requested entry has to stand on its own as an indexing root. */ + * requested entry has to stand on its own as an indexing root, judged by the + * directory it resolves to. */ for (int i = 0; i < m.count; i++) { - cbm_ws_verdict_t v = cbm_workspace_classify_root(m.entries[i], home_dir, cache_dir); + char resolved[WS_LINE_MAX]; + const char *unusable = ws_manifest_entry_resolve(m.entries[i], resolved, sizeof(resolved)); + if (unusable) { + ws_manifest_entry_refusal(err, err_sz, m.entries[i], NULL, unusable); + return false; + } + cbm_ws_verdict_t v = cbm_workspace_classify_root(resolved, home_dir, cache_dir); if (v != CBM_WS_ALLOW) { - if (err) { - snprintf(err, err_sz, "requested path %s: %s", m.entries[i], - cbm_workspace_verdict_reason(v)); - } + ws_manifest_entry_refusal(err, err_sz, m.entries[i], resolved, + cbm_workspace_verdict_reason(v)); return false; } } @@ -850,12 +892,19 @@ bool cbm_workspace_manifest_allows(const char *cache_dir, const char *home_dir, return false; } for (int i = 0; i < m.count; i++) { - /* Re-classify at use time as well as at approval time: the credential list - * may have grown since, and a stored approval must not outrank it. */ - if (cbm_workspace_classify_root(m.entries[i], home_dir, cache_dir) != CBM_WS_ALLOW) { + /* Resolve and re-classify at use time as well as at approval time: the + * credential list may have grown since, and a link may lead somewhere + * else than it did when the person approved it. A stored approval must + * not outrank either. An entry that no longer resolves matches nothing, + * and the other entries of the manifest are consulted as before. */ + char resolved[WS_LINE_MAX]; + if (ws_manifest_entry_resolve(m.entries[i], resolved, sizeof(resolved)) != NULL) { continue; } - if (cbm_path_within_root(m.entries[i], candidate)) { + if (cbm_workspace_classify_root(resolved, home_dir, cache_dir) != CBM_WS_ALLOW) { + continue; + } + if (cbm_path_within_root(resolved, candidate)) { return true; } } diff --git a/src/foundation/workspace.h b/src/foundation/workspace.h index 276079155..c4674696d 100644 --- a/src/foundation/workspace.h +++ b/src/foundation/workspace.h @@ -145,12 +145,19 @@ bool cbm_workspace_manifest_read(const char *project_root, cbm_ws_manifest_t *ou bool cbm_workspace_manifest_is_approved(const char *cache_dir, const char *project_root, const cbm_ws_manifest_t *manifest); -/* Record approval for the manifest currently on disk. Refuses when a requested - * entry would not be allowable as a root on its own — approving a manifest must - * not become a way around the breadth policy. */ +/* Record approval for the manifest currently on disk. Each entry is judged in + * its resolved form (links followed, `..` collapsed), since that is the + * directory that would be indexed; an entry that is relative, does not exist + * or is not a directory is refused, as is one that would not be allowable as a + * root on its own — approving a manifest must not become a way around the + * breadth policy. err names the entry as written and, when it differs, the + * directory it resolves to. */ bool cbm_workspace_manifest_approve(const char *cache_dir, const char *home_dir, const char *project_root, char *err, size_t err_sz); -/* True when candidate is at or below an APPROVED manifest entry of project_root. */ +/* True when candidate is at or below an APPROVED manifest entry of project_root. + * Entries are resolved and classified again here: approval binds to the + * manifest text, and where an entry leads is read at the time it is used. An + * entry that no longer resolves matches nothing. */ bool cbm_workspace_manifest_allows(const char *cache_dir, const char *home_dir, const char *project_root, const char *candidate); diff --git a/tests/test_workspace.c b/tests/test_workspace.c index afe6726e2..7f163db29 100644 --- a/tests/test_workspace.c +++ b/tests/test_workspace.c @@ -334,13 +334,29 @@ TEST(ws_manifest_rejects_control_characters) { /* THE property the design rests on: a manifest grants nothing until a person * approves it, and editing it lapses that approval rather than inheriting it. */ TEST(ws_manifest_approval_is_keyed_to_content) { - char *base = th_mktempdir("cbm_ws_m3"); - char *cache = th_mktempdir("cbm_ws_m3c"); - ASSERT(base != NULL); - ASSERT(cache != NULL); + /* th_mktempdir returns one static buffer: copy each result at once. */ + char base[256]; + char cache[256]; + char *created = th_mktempdir("cbm_ws_m3"); + ASSERT(created != NULL); + snprintf(base, sizeof(base), "%s", created); + created = th_mktempdir("cbm_ws_m3c"); + ASSERT(created != NULL); + snprintf(cache, sizeof(cache), "%s", created); + + /* Entries must name existing directories. */ + char sdk[1024]; + char protos[1024]; + snprintf(sdk, sizeof(sdk), "%s/sdk", base); + snprintf(protos, sizeof(protos), "%s/protos", base); + ASSERT_EQ(cbm_mkdir(sdk), 0); + ASSERT_EQ(cbm_mkdir(protos), 0); + char path[1024]; + char body[4096]; snprintf(path, sizeof(path), "%s/%s", base, CBM_WS_MANIFEST_NAME); - ws_write(path, "/opt/sdk\n"); + snprintf(body, sizeof(body), "%s\n", sdk); + ws_write(path, body); cbm_ws_manifest_t before; ASSERT_TRUE(cbm_workspace_manifest_read(base, &before)); @@ -352,7 +368,8 @@ TEST(ws_manifest_approval_is_keyed_to_content) { ASSERT_TRUE(cbm_workspace_manifest_is_approved(cache, base, &before)); /* Widen the requests, as a `git pull` would. Approval must lapse. */ - ws_write(path, "/opt/sdk\n/srv/protos\n"); + snprintf(body, sizeof(body), "%s\n%s\n", sdk, protos); + ws_write(path, body); cbm_ws_manifest_t after; ASSERT_TRUE(cbm_workspace_manifest_read(base, &after)); ASSERT_TRUE(strcmp(before.digest, after.digest) != 0); @@ -363,18 +380,35 @@ TEST(ws_manifest_approval_is_keyed_to_content) { PASS(); } -/* Approving a manifest must not become a route around the breadth policy. */ +/* Approving a manifest must not become a route around the breadth policy. The + * entry has to exist, so the overbroad directory is "/etc" on POSIX and, on + * Windows, where one component below the drive is already an ordinary + * workspace, the drive root of the temporary directory. */ TEST(ws_manifest_approval_refuses_overbroad_requests) { - char *base = th_mktempdir("cbm_ws_m4"); - char *cache = th_mktempdir("cbm_ws_m4c"); - ASSERT(base != NULL); - ASSERT(cache != NULL); + char base[256]; + char cache[256]; + char *created = th_mktempdir("cbm_ws_m4"); + ASSERT(created != NULL); + snprintf(base, sizeof(base), "%s", created); + created = th_mktempdir("cbm_ws_m4c"); + ASSERT(created != NULL); + snprintf(cache, sizeof(cache), "%s", created); + char path[1024]; + char body[1024]; + const char *refusal = NULL; snprintf(path, sizeof(path), "%s/%s", base, CBM_WS_MANIFEST_NAME); - ws_write(path, "/etc\n"); +#ifdef _WIN32 + snprintf(body, sizeof(body), "%.3s\n", base); + refusal = cbm_workspace_verdict_reason(CBM_WS_DENY_ABSOLUTE); +#else + snprintf(body, sizeof(body), "/etc\n"); + refusal = cbm_workspace_verdict_reason(CBM_WS_DENY_TOO_SHALLOW); +#endif + ws_write(path, body); char err[1024]; ASSERT_FALSE(cbm_workspace_manifest_approve(cache, HOME, base, err, sizeof(err))); - ASSERT_TRUE(strstr(err, "too broad") != NULL); + ASSERT_TRUE(strstr(err, refusal) != NULL); th_cleanup(base); th_cleanup(cache); PASS(); @@ -463,6 +497,323 @@ TEST(ws_linked_home_classified_as_home) { #endif } +/* ── Manifest entries are judged in their resolved form ──────────────────── */ + +enum { WS_T_PATH = 4096 }; + +/* th_mktempdir hands back one static buffer, so copy at once — and in canonical + * form, because the policy compares resolved spellings and /tmp is a link on + * macOS. */ +static bool ws_tempdir_canonical(const char *prefix, char *out, size_t out_sz) { + char *created = th_mktempdir(prefix); + return created && cbm_canonical_path(created, out, out_sz) == 1; +} + +static bool ws_join(char *out, size_t out_sz, const char *a, const char *b) { + int n = snprintf(out, out_sz, "%s/%s", a, b); + return n > 0 && (size_t)n < out_sz; +} + +/* Create / and return it in canonical form. */ +static bool ws_mkdir_canonical(const char *parent, const char *name, char *out, size_t out_sz) { + char made[WS_T_PATH]; + if (!ws_join(made, sizeof(made), parent, name) || cbm_mkdir(made) != 0) { + return false; + } + return cbm_canonical_path(made, out, out_sz) == 1; +} + +static void ws_manifest_write(const char *base, const char *body) { + char path[WS_T_PATH]; + if (ws_join(path, sizeof(path), base, CBM_WS_MANIFEST_NAME)) { + ws_write(path, body); + } +} + +static void ws_manifest_write_line(const char *base, const char *entry) { + char body[WS_T_PATH]; + int n = snprintf(body, sizeof(body), "%s\n", entry); + if (n > 0 && (size_t)n < sizeof(body)) { + ws_manifest_write(base, body); + } +} + +/* Put the manifest on record as approved by writing the store line directly, + * in the form cbm_workspace_manifest_approve writes (" "). + * This is the record an approval leaves behind when the entry was judged by + * its spelling, and the use-time check must stand on its own against it. */ +static bool ws_manifest_record_approval(const char *cache, const char *base) { + cbm_ws_manifest_t m; + if (!cbm_workspace_manifest_read(base, &m) || !m.present) { + return false; + } + char store[WS_T_PATH]; + char line[WS_T_PATH]; + if (!ws_join(store, sizeof(store), cache, "approved_manifests")) { + return false; + } + int n = snprintf(line, sizeof(line), "%s %s\n", m.digest, base); + if (n <= 0 || (size_t)n >= sizeof(line)) { + return false; + } + ws_write(store, line); + return cbm_workspace_manifest_is_approved(cache, base, &m); +} + +/* The directory a manifest entry names is the one that would be indexed, so + * that is the form the policy judges — at approval and again at use. Spelled + * through `..`, an entry reads as an ordinary path several components deep + * while naming the home directory or the volume root. */ +TEST(ws_manifest_entry_classified_in_resolved_form) { + char base[WS_T_PATH]; + char cache[WS_T_PATH]; + ASSERT_TRUE(ws_tempdir_canonical("cbm_ws_m5", base, sizeof(base))); + ASSERT_TRUE(ws_tempdir_canonical("cbm_ws_m5c", cache, sizeof(cache))); + + char home[WS_T_PATH]; + char src[WS_T_PATH]; + char home_sub[WS_T_PATH]; + ASSERT_TRUE(ws_mkdir_canonical(base, "home", home, sizeof(home))); + ASSERT_TRUE(ws_mkdir_canonical(base, "src", src, sizeof(src))); + ASSERT_TRUE(ws_mkdir_canonical(home, "notes", home_sub, sizeof(home_sub))); + + char entry[WS_T_PATH]; + char expected[WS_T_PATH]; + char err[1024]; + + /* "/src/../home" is the home directory. */ + ASSERT_TRUE(ws_join(entry, sizeof(entry), src, "../home")); + ws_manifest_write_line(base, entry); + ASSERT_FALSE(cbm_workspace_manifest_approve(cache, home, base, err, sizeof(err))); + ASSERT_NOT_NULL(strstr(err, entry)); + ASSERT(snprintf(expected, sizeof(expected), "(resolves to %s)", home) > 0); + ASSERT_NOT_NULL(strstr(err, expected)); + ASSERT_NOT_NULL(strstr(err, cbm_workspace_verdict_reason(CBM_WS_DENY_SENSITIVE))); + + /* An approval already on record for this spelling does not outrank the + * use-time check. */ + ASSERT_TRUE(ws_manifest_record_approval(cache, base)); + ASSERT_FALSE(cbm_workspace_manifest_allows(cache, home, base, home_sub)); + + /* Enough `..` segments reach the volume root from anywhere. */ + ASSERT_TRUE(ws_join(entry, sizeof(entry), base, "../../../../../../../../../../../..")); + ws_manifest_write_line(base, entry); + ASSERT_FALSE(cbm_workspace_manifest_approve(cache, home, base, err, sizeof(err))); + ASSERT_NOT_NULL(strstr(err, entry)); + ASSERT_NOT_NULL(strstr(err, cbm_workspace_verdict_reason(CBM_WS_DENY_ABSOLUTE))); + + th_cleanup(base); + th_cleanup(cache); + PASS(); +} + +/* An entry that does not resolve to a directory is refused at approval, naming + * the entry; at use it simply matches nothing, and the other entries of the + * same manifest keep working. */ +TEST(ws_manifest_unresolvable_entry_refused) { + char base[WS_T_PATH]; + char cache[WS_T_PATH]; + ASSERT_TRUE(ws_tempdir_canonical("cbm_ws_m6", base, sizeof(base))); + ASSERT_TRUE(ws_tempdir_canonical("cbm_ws_m6c", cache, sizeof(cache))); + + char entry[WS_T_PATH]; + char err[1024]; + + /* Missing. */ + ASSERT_TRUE(ws_join(entry, sizeof(entry), base, "missing")); + ws_manifest_write_line(base, entry); + ASSERT_FALSE(cbm_workspace_manifest_approve(cache, HOME, base, err, sizeof(err))); + ASSERT_NOT_NULL(strstr(err, entry)); + ASSERT_NOT_NULL(strstr(err, "must name an existing directory")); + + /* Present, but a file. */ + ASSERT_TRUE(ws_join(entry, sizeof(entry), base, "notes.txt")); + ws_write(entry, "not a directory\n"); + ws_manifest_write_line(base, entry); + ASSERT_FALSE(cbm_workspace_manifest_approve(cache, HOME, base, err, sizeof(err))); + ASSERT_NOT_NULL(strstr(err, entry)); + ASSERT_NOT_NULL(strstr(err, "is not a directory")); + + /* Relative: the file is read by a long-lived process whose working + * directory is unrelated to the project, so there is no base to resolve + * it against. */ + ws_manifest_write_line(base, "src"); + ASSERT_FALSE(cbm_workspace_manifest_approve(cache, HOME, base, err, sizeof(err))); + ASSERT_NOT_NULL(strstr(err, "requested path src:")); + ASSERT_NOT_NULL(strstr(err, "must be an absolute path")); + + /* At use: a directory that disappeared after approval matches nothing and + * does not take the rest of the manifest with it. */ + char present[WS_T_PATH]; + char present_sub[WS_T_PATH]; + char body[WS_T_PATH]; + ASSERT_TRUE(ws_mkdir_canonical(base, "present", present, sizeof(present))); + ASSERT_TRUE(ws_mkdir_canonical(present, "inc", present_sub, sizeof(present_sub))); + ASSERT_TRUE(ws_join(entry, sizeof(entry), base, "gone")); + ASSERT(snprintf(body, sizeof(body), "%s\n%s\n", entry, present) > 0); + ws_manifest_write(base, body); + ASSERT_TRUE(ws_manifest_record_approval(cache, base)); + ASSERT_TRUE(cbm_workspace_manifest_allows(cache, HOME, base, present_sub)); + ASSERT_FALSE(cbm_workspace_manifest_allows(cache, HOME, base, base)); + + th_cleanup(base); + th_cleanup(cache); + PASS(); +} + +/* The ordinary shapes keep their result: an absolute existing directory, an + * entry equal to the project root, an entry outside every root in the grant + * store (the manifest is consulted on its own), and on POSIX a link to an + * ordinary directory. */ +TEST(ws_manifest_ordinary_entries_unchanged) { + char base[WS_T_PATH]; + char cache[WS_T_PATH]; + ASSERT_TRUE(ws_tempdir_canonical("cbm_ws_m7", base, sizeof(base))); + ASSERT_TRUE(ws_tempdir_canonical("cbm_ws_m7c", cache, sizeof(cache))); + + char sdk[WS_T_PATH]; + char sdk_inc[WS_T_PATH]; + char granted[WS_T_PATH]; + ASSERT_TRUE(ws_mkdir_canonical(base, "sdk", sdk, sizeof(sdk))); + ASSERT_TRUE(ws_mkdir_canonical(sdk, "inc", sdk_inc, sizeof(sdk_inc))); + ASSERT_TRUE(ws_mkdir_canonical(base, "granted", granted, sizeof(granted))); + + char err[1024]; + /* The grant store names a root the sdk is not under. */ + ASSERT_TRUE(cbm_workspace_grant_add(cache, HOME, granted, false, err, sizeof(err))); + + ws_manifest_write_line(base, sdk); + ASSERT_TRUE(cbm_workspace_manifest_approve(cache, HOME, base, err, sizeof(err))); + ASSERT_STR_EQ(err, ""); + ASSERT_TRUE(cbm_workspace_manifest_allows(cache, HOME, base, sdk_inc)); + ASSERT_FALSE(cbm_workspace_manifest_allows(cache, HOME, base, granted)); + + /* Equal to the project root. */ + ws_manifest_write_line(base, base); + ASSERT_TRUE(cbm_workspace_manifest_approve(cache, HOME, base, err, sizeof(err))); + ASSERT_TRUE(cbm_workspace_manifest_allows(cache, HOME, base, base)); + ASSERT_TRUE(cbm_workspace_manifest_allows(cache, HOME, base, sdk_inc)); + +#ifndef _WIN32 + /* A link to an ordinary directory is that directory. Windows is not + * covered here: symlink(2) has no counterpart in the test helpers, and + * CreateSymbolicLinkW needs a privilege the CI runner does not hold; the + * resolved-form rules themselves are exercised on every platform by the + * `..` cases above. */ + char alias[WS_T_PATH]; + ASSERT_TRUE(ws_join(alias, sizeof(alias), base, "alias")); + ASSERT_EQ(symlink(sdk, alias), 0); + ws_manifest_write_line(base, alias); + ASSERT_TRUE(cbm_workspace_manifest_approve(cache, HOME, base, err, sizeof(err))); + ASSERT_TRUE(cbm_workspace_manifest_allows(cache, HOME, base, sdk_inc)); +#endif + + th_cleanup(base); + th_cleanup(cache); + PASS(); +} + +#ifndef _WIN32 +/* A link is judged by where it leads. Windows is not covered: symlink(2) has + * no counterpart in the test helpers and CreateSymbolicLinkW needs a privilege + * the CI runner does not hold; the resolved-form rules are exercised on every + * platform by ws_manifest_entry_classified_in_resolved_form. */ +TEST(ws_manifest_link_to_sensitive_directory_refused) { + char base[WS_T_PATH]; + char cache[WS_T_PATH]; + ASSERT_TRUE(ws_tempdir_canonical("cbm_ws_m8", base, sizeof(base))); + ASSERT_TRUE(ws_tempdir_canonical("cbm_ws_m8c", cache, sizeof(cache))); + + char home[WS_T_PATH]; + char home_sub[WS_T_PATH]; + char keys[WS_T_PATH]; + char keys_sub[WS_T_PATH]; + ASSERT_TRUE(ws_mkdir_canonical(base, "home", home, sizeof(home))); + ASSERT_TRUE(ws_mkdir_canonical(home, "notes", home_sub, sizeof(home_sub))); + ASSERT_TRUE(ws_mkdir_canonical(base, ".ssh", keys, sizeof(keys))); + ASSERT_TRUE(ws_mkdir_canonical(keys, "hosts", keys_sub, sizeof(keys_sub))); + + char to_home[WS_T_PATH]; + char to_keys[WS_T_PATH]; + ASSERT_TRUE(ws_join(to_home, sizeof(to_home), base, "to-home")); + ASSERT_TRUE(ws_join(to_keys, sizeof(to_keys), base, "to-keys")); + ASSERT_EQ(symlink(home, to_home), 0); + ASSERT_EQ(symlink(keys, to_keys), 0); + + char expected[WS_T_PATH]; + char err[1024]; + + ws_manifest_write_line(base, to_home); + ASSERT_FALSE(cbm_workspace_manifest_approve(cache, home, base, err, sizeof(err))); + ASSERT_NOT_NULL(strstr(err, to_home)); + ASSERT(snprintf(expected, sizeof(expected), "(resolves to %s)", home) > 0); + ASSERT_NOT_NULL(strstr(err, expected)); + ASSERT_NOT_NULL(strstr(err, cbm_workspace_verdict_reason(CBM_WS_DENY_SENSITIVE))); + + ws_manifest_write_line(base, to_keys); + ASSERT_FALSE(cbm_workspace_manifest_approve(cache, home, base, err, sizeof(err))); + ASSERT_NOT_NULL(strstr(err, to_keys)); + ASSERT(snprintf(expected, sizeof(expected), "(resolves to %s)", keys) > 0); + ASSERT_NOT_NULL(strstr(err, expected)); + ASSERT_NOT_NULL(strstr(err, cbm_workspace_verdict_reason(CBM_WS_DENY_SENSITIVE))); + + /* On record as approved: the use-time check still refuses both. */ + char body[WS_T_PATH]; + ASSERT(snprintf(body, sizeof(body), "%s\n%s\n", to_home, to_keys) > 0); + ws_manifest_write(base, body); + ASSERT_TRUE(ws_manifest_record_approval(cache, base)); + ASSERT_FALSE(cbm_workspace_manifest_allows(cache, home, base, home_sub)); + ASSERT_FALSE(cbm_workspace_manifest_allows(cache, home, base, keys_sub)); + + th_cleanup(base); + th_cleanup(cache); + PASS(); +} + +/* Approval binds to the manifest text; where a link leads is read again each + * time the manifest is used. A link approved while it led to an ordinary + * directory and later pointed at the home directory grants nothing there. + * POSIX only, for the reason given at the test above. */ +TEST(ws_manifest_link_retarget_is_seen_at_use) { + char base[WS_T_PATH]; + char cache[WS_T_PATH]; + ASSERT_TRUE(ws_tempdir_canonical("cbm_ws_m9", base, sizeof(base))); + ASSERT_TRUE(ws_tempdir_canonical("cbm_ws_m9c", cache, sizeof(cache))); + + char shared[WS_T_PATH]; + char shared_sub[WS_T_PATH]; + char home[WS_T_PATH]; + char home_sub[WS_T_PATH]; + ASSERT_TRUE(ws_mkdir_canonical(base, "shared", shared, sizeof(shared))); + ASSERT_TRUE(ws_mkdir_canonical(shared, "proto", shared_sub, sizeof(shared_sub))); + ASSERT_TRUE(ws_mkdir_canonical(base, "home", home, sizeof(home))); + ASSERT_TRUE(ws_mkdir_canonical(home, "notes", home_sub, sizeof(home_sub))); + + char extra[WS_T_PATH]; + ASSERT_TRUE(ws_join(extra, sizeof(extra), base, "extra")); + ASSERT_EQ(symlink(shared, extra), 0); + + char err[1024]; + ws_manifest_write_line(base, extra); + ASSERT_TRUE(cbm_workspace_manifest_approve(cache, home, base, err, sizeof(err))); + ASSERT_TRUE(cbm_workspace_manifest_allows(cache, home, base, shared_sub)); + + /* Same text, new target. */ + ASSERT_EQ(unlink(extra), 0); + ASSERT_EQ(symlink(home, extra), 0); + cbm_ws_manifest_t m; + ASSERT_TRUE(cbm_workspace_manifest_read(base, &m)); + ASSERT_TRUE(cbm_workspace_manifest_is_approved(cache, base, &m)); + + ASSERT_FALSE(cbm_workspace_manifest_allows(cache, home, base, home_sub)); + ASSERT_FALSE(cbm_workspace_manifest_allows(cache, home, base, shared_sub)); + + th_cleanup(base); + th_cleanup(cache); + PASS(); +} +#endif /* _WIN32 */ + SUITE(workspace) { RUN_TEST(ws_manifest_absent_is_not_an_error); RUN_TEST(ws_manifest_parses_entries_and_skips_comments); @@ -486,4 +837,11 @@ SUITE(workspace) { RUN_TEST(ws_every_verdict_has_a_reason); RUN_TEST(ws_home_dir_is_resolved); RUN_TEST(ws_linked_home_classified_as_home); + RUN_TEST(ws_manifest_entry_classified_in_resolved_form); + RUN_TEST(ws_manifest_unresolvable_entry_refused); + RUN_TEST(ws_manifest_ordinary_entries_unchanged); +#ifndef _WIN32 + RUN_TEST(ws_manifest_link_to_sensitive_directory_refused); + RUN_TEST(ws_manifest_link_retarget_is_seen_at_use); +#endif } From 4e490bef5af969a8f31a2cceda10a665f9f741d0 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 28 Sep 2026 19:34:20 +0200 Subject: [PATCH 10/15] fix(routes): HANDLES for Laravel array and invokable controller actions Why: Laravel names a route's controller as [UserController::class, 'show'], or, for an invokable controller, as GetCurrentUserController::class. The route handler scan only took identifiers and strings, so these routes never got a HANDLES edge. The cross-repo HTTP matcher reaches a route's handler through that edge, so apps written this way - the #1146 reporter's app among them - produced 0 CROSS_HTTP_CALLS. Resolving the class by short name is not enough either: the call pass's import map bound `use App\Http\Controllers\UserController` to Admin\UserController by name, even with a composer.json PSR-4 map present, and the generic resolver's multi-candidate fallback bound a vendor controller's show() to another class's show(). Fix: - extract_calls.c: for PHP, [X::class, 'm'] becomes the handler reference "Ns\X::m" and X::class becomes "Ns\X::__invoke". X is qualified the way PHP resolves it: through the file's `use` imports (aliases included), else the file namespace; a leading backslash is already absolute. - registry.c, cbm_registry_resolve_handler (both route passes): a class-qualified handler is placed on a method QN only by what places the class file - the composer.json PSR-4 root in the package map, else the one candidate whose folders mirror the namespace (App\Http\Controllers -> app/Http/Controllers). A class that is not in the repo gets no handler. A handler reference without "::" resolves exactly as before. Proof (release CLI built from this tree vs origin/main 1f9b4db1): - laravel.io: HANDLES 0 -> 4, one for each of the 5 route registrations main mints (two share a Route). The use statement tells Articles\ArticlesController apart from Admin\ArticlesController. Route count unchanged (39). With the slashless-path and withRouting fixes also applied: HANDLES 0 -> 58, all from PHP controllers, samples checked against source (aliased AdminArticlesController and AdminUsersController included); the 7 routes left without a handler use methods from a vendor trait (AuthenticatesUsers). - Reporter-shaped cross-repo check (Laravel 11 withRouting api:, invokable GetCurrentUserController under prefix('/users'), TS fetch('/api/users/me')): CROSS_HTTP_CALLS 0 with this fix alone (route /users/me, no /api mount) and 0 with the withRouting fix alone (no HANDLES); 2 with both applied. - krayin/laravel-crm (reduced to one route file): / <- Webkul\Admin\Http\ Controllers\Controller::redirectToLogin, placed through the PSR-4 root Webkul\Admin\ -> packages/Webkul/Admin/src. Tests (edge_types_probe): handles_laravel_class_handlers_issue1146, handles_laravel_class_handlers_parallel_issue1146 and handles_laravel_class_handlers_no_composer_issue1146 assert the exact HANDLES set. They include controls for a same-named Admin\UserController::show, an alias import, an unimported fully-qualified class, a package under a PSR-4 root whose folder does not mirror its namespace, a decoy __invoke, and two vendor controllers that must get no edge. RED on origin/main 3x, green, RED on revert. edge_types_probe, php_lsp, pipeline, parallel, route_canon and cross_repo pass (730); make lint-ci passes. Refs #1146 Signed-off-by: Martin Vogel --- internal/cbm/extract_calls.c | 124 +++++++++++++++++++++++ src/pipeline/pass_calls.c | 5 +- src/pipeline/pass_parallel.c | 3 +- src/pipeline/pipeline_internal.h | 14 +++ src/pipeline/registry.c | 168 +++++++++++++++++++++++++++++++ tests/test_edge_types_probe.c | 166 ++++++++++++++++++++++++++++++ 6 files changed, 477 insertions(+), 3 deletions(-) diff --git a/internal/cbm/extract_calls.c b/internal/cbm/extract_calls.c index f14efb766..4da0ece7a 100644 --- a/internal/cbm/extract_calls.c +++ b/internal/cbm/extract_calls.c @@ -2553,6 +2553,123 @@ static const char *normalize_string_handler(CBMArena *a, const char *raw) { return unq; } +/* True for a PHP class/namespace name: identifier segments joined by '\'. */ +static bool php_is_class_path(const char *s) { + if (!s || !s[0]) { + return false; + } + for (; *s; s++) { + if (!((*s >= 'a' && *s <= 'z') || (*s >= 'A' && *s <= 'Z') || (*s >= '0' && *s <= '9') || + *s == '_' || *s == '\\')) { + return false; + } + } + return true; +} + +/* Qualify a class name the way PHP resolves it: a leading '\' is already + * absolute; a first segment that a `use` clause imports (by alias or by last + * segment) is replaced by the imported name; anything else is relative to the + * file's namespace. Imports and the namespace are extracted before the call + * walk, so both are complete here. */ +static const char *php_qualify_class(CBMExtractCtx *ctx, const char *written) { + if (written[0] == '\\') { + return written + SKIP_ONE; + } + const char *sep = strchr(written, '\\'); + size_t first_len = sep ? (size_t)(sep - written) : strlen(written); + const CBMImportArray *imps = &ctx->result->imports; + for (int i = 0; i < imps->count; i++) { + const CBMImport *imp = &imps->items[i]; + const char *path = imp->module_path; + if (!imp->local_name || !path || strlen(imp->local_name) != first_len || + strncmp(imp->local_name, written, first_len) != 0) { + continue; + } + path += path[0] == '\\' ? SKIP_ONE : 0; + if (php_is_class_path(path)) { + return cbm_arena_sprintf(ctx->arena, "%s%s", path, sep ? sep : ""); + } + } + const char *ns = ctx->result->namespace_name; + return ns && ns[0] ? cbm_arena_sprintf(ctx->arena, "%s\\%s", ns, written) : written; +} + +/* The fully-qualified class a PHP `X::class` expression names (`use` imports + * and the file namespace applied). NULL for any other expression, including + * `static::class` and `self::class`. */ +static const char *php_class_literal_fqn(CBMExtractCtx *ctx, TSNode node) { + if (strcmp(ts_node_type(node), "class_constant_access_expression") != 0 || + ts_node_named_child_count(node) != PAIR_LEN) { + return NULL; + } + TSNode cls = ts_node_named_child(node, 0); + const char *ck = ts_node_type(cls); + const char *member = + cbm_node_text(ctx->arena, ts_node_named_child(node, SKIP_ONE), ctx->source); + if ((strcmp(ck, "name") != 0 && strcmp(ck, "qualified_name") != 0) || !member || + strcmp(member, "class") != 0) { + return NULL; + } + const char *text = cbm_node_text(ctx->arena, cls, ctx->source); + return php_is_class_path(text) ? php_qualify_class(ctx, text) : NULL; +} + +/* The value of a PHP array element written without a key; a null node for a + * keyed element (`'uses' => ...`) or anything else. */ +static TSNode php_unkeyed_element_value(TSNode elem) { + TSNode none = {0}; + if (strcmp(ts_node_type(elem), "array_element_initializer") != 0 || + ts_node_named_child_count(elem) != SKIP_ONE) { + return none; + } + return ts_node_named_child(elem, 0); +} + +static bool php_is_identifier(const char *s) { + if (!s || !((*s >= 'a' && *s <= 'z') || (*s >= 'A' && *s <= 'Z') || *s == '_')) { + return false; + } + for (; *s; s++) { + if (!((*s >= 'a' && *s <= 'z') || (*s >= 'A' && *s <= 'Z') || (*s >= '0' && *s <= '9') || + *s == '_')) { + return false; + } + } + return true; +} + +/* Laravel's class-based route handlers (#1146), as "Ns\Class::method": + * [UserController::class, 'show'] -> App\Http\Controllers\UserController::show + * GetCurrentUserController::class -> App\...\GetCurrentUserController::__invoke + * with the class qualified through the file's `use` imports; the route passes + * place it on a method QN (cbm_registry_resolve_handler). NULL for any other + * expression. */ +static const char *php_class_route_handler(CBMExtractCtx *ctx, TSNode arg) { + const char *kind = ts_node_type(arg); + if (strcmp(kind, "class_constant_access_expression") == 0) { + const char *cls = php_class_literal_fqn(ctx, arg); + return cls ? cbm_arena_sprintf(ctx->arena, "%s::__invoke", cls) : NULL; + } + if (strcmp(kind, "array_creation_expression") != 0 || + ts_node_named_child_count(arg) != PAIR_LEN) { + return NULL; + } + TSNode cls_node = php_unkeyed_element_value(ts_node_named_child(arg, 0)); + TSNode method_node = php_unkeyed_element_value(ts_node_named_child(arg, SKIP_ONE)); + if (ts_node_is_null(cls_node) || ts_node_is_null(method_node)) { + return NULL; + } + const char *cls = php_class_literal_fqn(ctx, cls_node); + const char *mk = ts_node_type(method_node); + if (!cls || (strcmp(mk, "string") != 0 && strcmp(mk, "encapsed_string") != 0)) { + return NULL; + } + const char *method = + strip_quotes(ctx->arena, cbm_node_text(ctx->arena, method_node, ctx->source)); + return php_is_identifier(method) ? cbm_arena_sprintf(ctx->arena, "%s::%s", cls, method) : NULL; +} + static const char *extract_handler_arg(CBMExtractCtx *ctx, TSNode args) { /* The LAST eligible argument wins, and every argument is examined. * Express, Fastify, gin and Laravel all put middleware between the route @@ -2570,6 +2687,13 @@ static const char *extract_handler_arg(CBMExtractCtx *ctx, TSNode args) { if (strcmp(ts_node_type(arg2), "argument") == 0 && ts_node_named_child_count(arg2) > 0) { arg2 = ts_node_named_child(arg2, 0); } + if (ctx->language == CBM_LANG_PHP) { + const char *class_handler = php_class_route_handler(ctx, arg2); + if (class_handler) { + handler = class_handler; + continue; + } + } const char *ak2 = ts_node_type(arg2); /* `name` = PHP bare identifier handler; string = Laravel string handler * ('showUsers' or 'Controller@method'). */ diff --git a/src/pipeline/pass_calls.c b/src/pipeline/pass_calls.c index 668919682..53f57025a 100644 --- a/src/pipeline/pass_calls.c +++ b/src/pipeline/pass_calls.c @@ -404,8 +404,9 @@ static void handle_route_registration(cbm_pipeline_ctx_t *ctx, const CBMCall *ca esc_fa); cbm_gbuf_insert_edge(ctx->gbuf, source_node->id, route_id, "CALLS", props); if (call->second_arg_name != NULL && call->second_arg_name[0] != '\0') { - cbm_resolution_t hres = cbm_registry_resolve(ctx->registry, call->second_arg_name, - module_qn, imp_keys, imp_vals, imp_count); + cbm_resolution_t hres = + cbm_registry_resolve_handler(ctx->registry, call->second_arg_name, module_qn, imp_keys, + imp_vals, imp_count, cbm_pipeline_get_pkgmap()); if (hres.qualified_name != NULL && hres.qualified_name[0] != '\0') { const cbm_gbuf_node_t *handler = cbm_gbuf_find_by_qn(ctx->gbuf, hres.qualified_name); if (handler != NULL) { diff --git a/src/pipeline/pass_parallel.c b/src/pipeline/pass_parallel.c index a8a9f3693..dfaea7161 100644 --- a/src/pipeline/pass_parallel.c +++ b/src/pipeline/pass_parallel.c @@ -2295,7 +2295,8 @@ static void emit_route_registration(cbm_gbuf_t *gbuf, const cbm_gbuf_node_t *sou esc_rp); cbm_gbuf_insert_edge(gbuf, source->id, rid, "CALLS", props); if (handler_ref && handler_ref[0] != '\0') { - cbm_resolution_t hres = cbm_registry_resolve(registry, handler_ref, module_qn, ik, iv, ic); + cbm_resolution_t hres = cbm_registry_resolve_handler(registry, handler_ref, module_qn, ik, + iv, ic, cbm_pipeline_get_pkgmap()); if (hres.qualified_name && hres.qualified_name[0] != '\0') { const cbm_gbuf_node_t *h = cbm_gbuf_find_by_qn(main_gbuf, hres.qualified_name); if (h) { diff --git a/src/pipeline/pipeline_internal.h b/src/pipeline/pipeline_internal.h index 7f266eae9..451776807 100644 --- a/src/pipeline/pipeline_internal.h +++ b/src/pipeline/pipeline_internal.h @@ -208,6 +208,20 @@ static inline int cbm_pipeline_relpath_is_excluded(const char *rel_path, char *c CBMHashTable *cbm_pipeline_get_pkgmap(void); void cbm_pipeline_set_pkgmap(CBMHashTable *map); +/* Resolve a route handler reference (the registry's answer for route passes). + * A class-qualified "Ns\\Class::method" reference — the extractor's form for + * Laravel `[Class::class, 'method']` and invokable `Class::class` handlers + * (#1146), with the class already qualified through the file's `use` imports + * and namespace — is placed on a method QN only by what places the class file: + * a composer.json PSR-4 root in psr4_roots (the package map), else the one + * candidate whose directories end with the class's namespace. Never by short + * name alone, so a vendor controller that is not in the repo gets no handler. + * A reference without "::" resolves exactly as cbm_registry_resolve does. */ +cbm_resolution_t cbm_registry_resolve_handler(const cbm_registry_t *r, const char *handler_ref, + const char *module_qn, const char **import_map_keys, + const char **import_map_vals, int import_map_count, + const CBMHashTable *psr4_roots); + /* Unified module resolver: relative → pkgmap → fqn_module fallback. * Handles bare specifiers via pkgmap lookup with prefix matching. * Caller must free() the returned string. */ diff --git a/src/pipeline/registry.c b/src/pipeline/registry.c index ff56a41ac..4159c19f4 100644 --- a/src/pipeline/registry.c +++ b/src/pipeline/registry.c @@ -25,6 +25,7 @@ enum { REG_MAX_CANDIDATES = 256 }; #define DEFAULT_CONFIDENCE 0.5 #include "pipeline/pipeline.h" +#include "pipeline/pipeline_internal.h" #include "cbm.h" /* cbm_label_is_relation — the resolve-time relation veto */ #include "foundation/compat.h" /* CBM_TLS */ #include "foundation/hash_table.h" @@ -1462,6 +1463,173 @@ cbm_resolution_t cbm_registry_resolve(const cbm_registry_t *r, const char *calle return res; } +/* ── Class-qualified route handlers (PHP, #1146) ─────────────────── */ + +/* Method QNs are path-based: ..... + * A PHP class is named by its namespace instead, so a "Ns\\Class::method" + * handler is placed on a QN by what places the class file itself. */ +#define CONF_PSR4_MEMBER 0.95 +#define CONF_NAMESPACE_PATH_MEMBER 0.90 + +static bool seg_eq_nocase(const char *a, size_t alen, const char *b, size_t blen) { + if (alen != blen) { + return false; + } + for (size_t i = 0; i < alen; i++) { + char ca = (char)((a[i] >= 'A' && a[i] <= 'Z') ? a[i] - 'A' + 'a' : a[i]); + char cb = (char)((b[i] >= 'A' && b[i] <= 'Z') ? b[i] - 'A' + 'a' : b[i]); + if (ca != cb) { + return false; + } + } + return true; +} + +/* Does qn end with ".."? Returns the offset of that tail or -1. */ +static int qn_class_member_tail(const char *qn, const char *cls, size_t cls_len, + const char *member) { + size_t qlen = strlen(qn); + size_t mlen = strlen(member); + if (qlen < cls_len + mlen + PAIR_LEN) { + return CBM_NOT_FOUND; + } + const char *m = qn + qlen - mlen; + const char *c = m - SKIP_ONE - cls_len; + if (strcmp(m, member) != 0 || m[-SKIP_ONE] != '.' || strncmp(c, cls, cls_len) != 0 || + c[-SKIP_ONE] != '.') { + return CBM_NOT_FOUND; + } + return (int)(c - SKIP_ONE - qn); +} + +/* PSR-4: the longest composer.json autoload root "Prefix\\" that prefixes the + * FQN maps the remaining namespace onto directories below the root's folder + * and the class onto .php — the file Composer's autoloader loads. + * Returns the registered method QN, or NULL (no root prefixes the FQN, or the + * autoloaded file holds no such method). */ +static const char *psr4_member_qn(const cbm_registry_t *r, const CBMHashTable *psr4_roots, + const char *fqn, size_t fqn_len, const char *cls, size_t cls_len, + const char *member) { + if (!psr4_roots || fqn_len >= CBM_SZ_512) { + return NULL; + } + char key[CBM_SZ_512]; + for (size_t cut = fqn_len; cut > 0; cut--) { + if (fqn[cut - SKIP_ONE] != '\\') { + continue; + } + memcpy(key, fqn, cut); /* "App\\" / "App\\Http\\" — keeps the trailing '\\' */ + key[cut] = '\0'; + const char *root_qn = (const char *)cbm_ht_get(psr4_roots, key); + if (!root_qn) { + continue; + } + char qn[CBM_SZ_1K]; + int n = snprintf(qn, sizeof(qn), "%s.%.*s.%.*s.%s", root_qn, (int)(fqn_len - cut), + fqn + cut, (int)cls_len, cls, member); + if (n <= 0 || (size_t)n >= sizeof(qn)) { + return NULL; + } + for (char *p = qn + strlen(root_qn); *p; p++) { + if (*p == '\\') { + *p = '.'; + } + } + return cbm_ht_get_key(r->exact, qn); + } + return NULL; +} + +/* Do the directory segments of qn (between the project root segment and the + * file stem, i.e. before offset tail = "..") end with every + * segment of the namespace fqn[0, ns_len)? Case-insensitive: App\\Http is + * conventionally app/Http. An empty namespace is trivially aligned. */ +static bool namespace_matches_dirs(const char *qn, int tail, const char *fqn, size_t ns_len) { + const char *root_dot = strchr(qn, '.'); + const char *stem_end = qn + tail; + if (!root_dot || stem_end <= root_dot) { + return false; + } + const char *dir_end = stem_end - SKIP_ONE; /* back over the file stem */ + while (*dir_end != '.') { + dir_end--; + } + size_t ns_end = ns_len; + while (ns_end > 0) { + size_t ns_start = ns_end; + while (ns_start > 0 && fqn[ns_start - SKIP_ONE] != '\\') { + ns_start--; + } + if (dir_end <= root_dot) { + return false; /* more namespace segments than directories */ + } + const char *dir_start = dir_end; + while (dir_start[-SKIP_ONE] != '.') { + dir_start--; + } + if (!seg_eq_nocase(fqn + ns_start, ns_end - ns_start, dir_start, + (size_t)(dir_end - dir_start))) { + return false; + } + dir_end = dir_start - SKIP_ONE; + ns_end = ns_start > 0 ? ns_start - SKIP_ONE : 0; + } + return true; +} + +/* Namespace/path alignment: the one "." candidate that sits + * where its namespace says. NULL when none or several do. */ +static const char *aligned_member_qn(const cbm_registry_t *r, const char *fqn, size_t ns_len, + const char *cls, size_t cls_len, const char *member) { + const qn_array_t *arr = cbm_ht_get(r->by_name, member); + const char *match = NULL; + for (int i = 0; arr && i < arr->count; i++) { + const char *qn = arr->items[i]; + int tail = qn_class_member_tail(qn, cls, cls_len, member); + if (tail == CBM_NOT_FOUND || !namespace_matches_dirs(qn, tail, fqn, ns_len)) { + continue; + } + if (match) { + return NULL; /* ambiguous */ + } + match = qn; + } + return match; +} + +cbm_resolution_t cbm_registry_resolve_handler(const cbm_registry_t *r, const char *handler_ref, + const char *module_qn, const char **import_map_keys, + const char **import_map_vals, int import_map_count, + const CBMHashTable *psr4_roots) { + const char *sep = handler_ref ? strstr(handler_ref, "::") : NULL; + if (!r || !sep) { + return cbm_registry_resolve(r, handler_ref, module_qn, import_map_keys, import_map_vals, + import_map_count); + } + const char *member = sep + PAIR_LEN; + size_t fqn_len = (size_t)(sep - handler_ref); + size_t ns_len = fqn_len; + while (ns_len > 0 && handler_ref[ns_len - SKIP_ONE] != '\\') { + ns_len--; + } + const char *cls = handler_ref + ns_len; + size_t cls_len = fqn_len - ns_len; + ns_len = ns_len > 0 ? ns_len - SKIP_ONE : 0; /* drop the separator before the class */ + if (cls_len == 0 || !member[0]) { + return empty_result(); + } + const char *qn = psr4_member_qn(r, psr4_roots, handler_ref, fqn_len, cls, cls_len, member); + if (qn) { + return (cbm_resolution_t){qn, "php_psr4", CONF_PSR4_MEMBER, REG_RESOLVED}; + } + qn = aligned_member_qn(r, handler_ref, ns_len, cls, cls_len, member); + if (qn) { + return (cbm_resolution_t){qn, "php_namespace_path", CONF_NAMESPACE_PATH_MEMBER, + REG_RESOLVED}; + } + return empty_result(); +} + cbm_resolution_t cbm_registry_resolve_lineage(const cbm_registry_t *r, const char *callee_name, const char *module_qn, const char **import_map_keys, const char **import_map_vals, int import_map_count) { diff --git a/tests/test_edge_types_probe.c b/tests/test_edge_types_probe.c index 5ed449b8d..999019c45 100644 --- a/tests/test_edge_types_probe.c +++ b/tests/test_edge_types_probe.c @@ -922,6 +922,169 @@ TEST(routes_laravel_withrouting_parallel_subdir_issue1146) { PASS(); } +/* Assert the exact HANDLES edge set as " -> " + * pairs. The tail is matched at a segment boundary against the handler's + * qualified name ("UserController.show" matches + * ".app.Http.Controllers.UserController.UserController.show"), so a + * same-named method on another class never satisfies it; the total count must + * match too, so a fabricated extra edge fails the assertion. */ +typedef struct { + const char *handler_tail; + const char *route; +} EtHandles; + +static int et_qn_has_tail(const char *qn, const char *tail) { + size_t ql = qn ? strlen(qn) : 0; + size_t tl = strlen(tail); + return ql >= tl && strcmp(qn + ql - tl, tail) == 0 && (ql == tl || qn[ql - tl - 1] == '.'); +} + +static int et_handles_exact(const EtFile *files, int nfiles, const EtHandles *want, int parallel) { + EtProj lp; + cbm_store_t *store = + parallel ? et_index_parallel(&lp, files, nfiles) : et_index_files(&lp, files, nfiles); + int wanted = 0; + int found[ET_ROUTE_ASSERT_MAX] = {0}; + while (want[wanted].handler_tail && wanted < ET_ROUTE_ASSERT_MAX) { + wanted++; + } + cbm_edge_t *edges = NULL; + int n = 0; + int ok = store != NULL && + cbm_store_find_edges_by_type(store, lp.project, "HANDLES", &edges, &n) == + CBM_STORE_OK && + n == wanted; + for (int i = 0; store && i < n; i++) { + cbm_node_t src = {0}; + cbm_node_t tgt = {0}; + int have_src = cbm_store_find_node_by_id(store, edges[i].source_id, &src) == CBM_STORE_OK; + int have_tgt = cbm_store_find_node_by_id(store, edges[i].target_id, &tgt) == CBM_STORE_OK; + int matched = 0; + for (int wi = 0; have_src && have_tgt && wi < wanted; wi++) { + if (!found[wi] && tgt.name && strcmp(tgt.name, want[wi].route) == 0 && + et_qn_has_tail(src.qualified_name, want[wi].handler_tail)) { + found[wi] = matched = 1; + break; + } + } + if (!matched) { + ok = 0; + fprintf(stderr, " [ET-HANDLES] unexpected %s -> %s\n", + have_src && src.qualified_name ? src.qualified_name : "", + have_tgt && tgt.name ? tgt.name : ""); + } + if (have_src) cbm_node_free_fields(&src); + if (have_tgt) cbm_node_free_fields(&tgt); + } + for (int wi = 0; wi < wanted; wi++) { + if (!found[wi]) { + ok = 0; + fprintf(stderr, " [ET-HANDLES] missing %s -> %s\n", want[wi].handler_tail, + want[wi].route); + } + } + if (!ok) { + fprintf(stderr, " [ET-HANDLES] FAIL (%s path) expected=%d actual=%d\n", + parallel ? "parallel" : "sequential", wanted, n); + } + if (edges) cbm_store_free_edges(edges, n); + et_cleanup(&lp, store); + return ok; +} + +/* #1146: Laravel's two class-based handler forms — + * Route::get('/x', [UserController::class, 'show']) (controller method) + * Route::get('/x', GetCurrentUserController::class) (invokable: __invoke) + * got no HANDLES edge, because the handler scan only took identifiers and + * strings. The cross-repo matcher needs HANDLES on the Route, so apps written + * this way (the #1146 reporter's included) produced 0 CROSS_HTTP_CALLS. + * Controls: an Admin\UserController::show with the same short name (the `use` + * statement picks the class), an alias import, a fully-qualified class that is + * not imported, a package class under a PSR-4 root whose folder does not + * mirror its namespace (Acme\Blog\ -> packages/blog/src/), a decoy __invoke on + * another class, and two vendor classes that are not in the repo — those must + * get no HANDLES edge at all rather than bind to some other class's + * same-named method. composer.json comes first so the no-composer variant + * below can index the same fixture without it. */ +static const EtFile et_laravel_class_handlers[] = { + {"composer.json", "{\"autoload\": {\"psr-4\": {\"App\\\\\": \"app/\", " + "\"Acme\\\\Blog\\\\\": \"packages/blog/src/\"}}}\n"}, + {"packages/blog/src/Http/PostsController.php", + " []]; }\n}\n"}, + {"app/Http/Controllers/UserController.php", + " $id]; }\n" + " public function index() { return []; }\n}\n"}, + {"app/Http/Controllers/Admin/UserController.php", + " $id]; }\n}\n"}, + {"app/Http/Controllers/PostController.php", + " $id]; }\n}\n"}, + {"app/Http/Controllers/GetCurrentUserController.php", + " true]; }\n}\n"}, + {"app/Http/Controllers/HealthController.php", + " true]; }\n}\n"}, + {"routes/api.php", + " MIN_FILES_FOR_PARALLEL), + * whose route emitter (pass_parallel.c) is a separate code path. */ +TEST(handles_laravel_class_handlers_parallel_issue1146) { + ASSERT_TRUE(et_handles_exact(et_laravel_class_handlers, ET_LARAVEL_CLASS_FILES, + et_laravel_class_handles, 1)); + PASS(); +} + +/* Without composer.json there is no PSR-4 map: a class is placed only where + * its namespace mirrors the folders (App\Http\Controllers -> app/Http/ + * Controllers). The package class, whose folder does not mirror its + * namespace, then gets no handler rather than a guessed one. */ +TEST(handles_laravel_class_handlers_no_composer_issue1146) { + static const EtHandles want[] = { + {"Controllers.UserController.UserController.show", "/users/{id}"}, + {"GetCurrentUserController.__invoke", "/me"}, + {"PostController.show", "/posts/{id}"}, + {NULL, NULL}}; + ASSERT_TRUE( + et_handles_exact(et_laravel_class_handlers + 1, ET_LARAVEL_CLASS_FILES - 1, want, 0)); + PASS(); +} + /* Rails (Ruby) — ActionDispatch router. The handler MUST be passed as a bare * identifier (not the idiomatic `to: 'list_items'` string, which extract_handler_arg * cannot capture). mapper.get resolves by name to the Mapper#get method whose QN @@ -2138,6 +2301,9 @@ SUITE(edge_types_probe) { RUN_TEST(routes_laravel_routeserviceprovider_control_issue1146); RUN_TEST(routes_laravel_withrouting_nonliteral_prefix_issue1146); RUN_TEST(routes_laravel_withrouting_parallel_subdir_issue1146); + RUN_TEST(handles_laravel_class_handlers_issue1146); + RUN_TEST(handles_laravel_class_handlers_parallel_issue1146); + RUN_TEST(handles_laravel_class_handlers_no_composer_issue1146); RUN_TEST(handles_rails_ruby); RUN_TEST(handles_actix_rust); From 42118ad1427ab2a59a502ec72028d803913c913b Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 4 Oct 2026 21:11:49 +0200 Subject: [PATCH 11/15] fix(routes): resolve PSR-4 handler classes through the class-file resolver Why: since 830f0ccf (#2509) the package map stores a composer.json PSR-4 prefix ("Acme\Blog\") as its repo-relative directory ("packages/blog/src"), not as a dotted QN root. The route-handler resolver still formatted that value as a QN root and built "packages/blog/src.Http.PostsController. PostsController.index", which names no node. The PSR-4 step therefore resolved nothing. Handlers whose folders mirror their namespace (App\Http\Controllers -> app/Http/Controllers) still got their edge through the namespace/folder fallback, but a package class under a root whose folder does not mirror its namespace got no HANDLES edge. That is the case of packages/blog/src in the test fixture and of krayin's Webkul\Admin\ -> packages/Webkul/Admin/src. Change: resolve the handler class the way the import pass resolves `use Ns\Class;` for PHP. - pass_pkgmap.c: psr4_file_node and resolve_php_psr4_class take the graph buffer instead of the pipeline ctx, since they only read ctx->gbuf. The new cbm_pipeline_psr4_member_qn runs resolve_php_psr4_class for the class FQN, takes the Class/Interface/Trait/Enum node of that name defined in the resolved file, and returns the QN of . when that node exists. - registry.c: psr4_member_qn delegates to it and no longer builds a QN from the package-map value. cbm_registry_resolve_handler takes the graph buffer in place of the package map. The namespace/folder fallback is unchanged. - pass_calls.c and pass_parallel.c pass the graph buffer they already use for the handler lookup: ctx->gbuf, and main_gbuf, which is read-only during the resolve phase. Proof: - handles_laravel_class_handlers_issue1146 and its _parallel twin (edge_types_probe): RED 3/3 before the change ("missing PostsController.index -> /blog", expected=4 actual=3; suite 75 passed, 2 failed). GREEN 3/3 after (77 passed). RED again with the production change reverted. GREEN again once it was restored. - Real input: no Laravel app with a non-mirroring PSR-4 root is available locally. koel/koel v9.11.1 (App\ -> app/) was indexed with the binary before and after, each under a private runtime and cache. The results are identical: 16,564 nodes, 60,177 edges, 15 Routes and 1 HANDLES (/demo/new-session -> NewSessionController.__invoke), with the same handler rows. - Suites: edge_types_probe 77, edge_imports 68, pipeline 317 and route_canon 11 all pass (473). make lint-ci passes. Refs #1146 Signed-off-by: Martin Vogel --- src/pipeline/pass_calls.c | 2 +- src/pipeline/pass_parallel.c | 4 +-- src/pipeline/pass_pkgmap.c | 52 +++++++++++++++++++++++++++----- src/pipeline/pipeline_internal.h | 20 +++++++++--- src/pipeline/registry.c | 49 +++++++++--------------------- 5 files changed, 77 insertions(+), 50 deletions(-) diff --git a/src/pipeline/pass_calls.c b/src/pipeline/pass_calls.c index 53f57025a..a40f8947c 100644 --- a/src/pipeline/pass_calls.c +++ b/src/pipeline/pass_calls.c @@ -406,7 +406,7 @@ static void handle_route_registration(cbm_pipeline_ctx_t *ctx, const CBMCall *ca if (call->second_arg_name != NULL && call->second_arg_name[0] != '\0') { cbm_resolution_t hres = cbm_registry_resolve_handler(ctx->registry, call->second_arg_name, module_qn, imp_keys, - imp_vals, imp_count, cbm_pipeline_get_pkgmap()); + imp_vals, imp_count, ctx->gbuf); if (hres.qualified_name != NULL && hres.qualified_name[0] != '\0') { const cbm_gbuf_node_t *handler = cbm_gbuf_find_by_qn(ctx->gbuf, hres.qualified_name); if (handler != NULL) { diff --git a/src/pipeline/pass_parallel.c b/src/pipeline/pass_parallel.c index dfaea7161..7514008ce 100644 --- a/src/pipeline/pass_parallel.c +++ b/src/pipeline/pass_parallel.c @@ -2295,8 +2295,8 @@ static void emit_route_registration(cbm_gbuf_t *gbuf, const cbm_gbuf_node_t *sou esc_rp); cbm_gbuf_insert_edge(gbuf, source->id, rid, "CALLS", props); if (handler_ref && handler_ref[0] != '\0') { - cbm_resolution_t hres = cbm_registry_resolve_handler(registry, handler_ref, module_qn, ik, - iv, ic, cbm_pipeline_get_pkgmap()); + cbm_resolution_t hres = + cbm_registry_resolve_handler(registry, handler_ref, module_qn, ik, iv, ic, main_gbuf); if (hres.qualified_name && hres.qualified_name[0] != '\0') { const cbm_gbuf_node_t *h = cbm_gbuf_find_by_qn(main_gbuf, hres.qualified_name); if (h) { diff --git a/src/pipeline/pass_pkgmap.c b/src/pipeline/pass_pkgmap.c index 9c4ace14b..71d3d3bec 100644 --- a/src/pipeline/pass_pkgmap.c +++ b/src/pipeline/pass_pkgmap.c @@ -2060,10 +2060,10 @@ typedef enum { /* The File node at exactly `rel_path`, or NULL. Looked up through the name * index (File nodes are named by basename) and matched on the full path, so * a same-named file in another directory never qualifies. */ -static const cbm_gbuf_node_t *psr4_file_node(const cbm_pipeline_ctx_t *ctx, const char *rel_path) { +static const cbm_gbuf_node_t *psr4_file_node(const cbm_gbuf_t *gbuf, const char *rel_path) { const cbm_gbuf_node_t **hits = NULL; int hit_count = 0; - if (cbm_gbuf_find_by_name(ctx->gbuf, path_leaf(rel_path), &hits, &hit_count) != 0 || !hits) { + if (cbm_gbuf_find_by_name(gbuf, path_leaf(rel_path), &hits, &hit_count) != 0 || !hits) { return NULL; } for (int i = 0; i < hit_count; i++) { @@ -2084,9 +2084,8 @@ static const cbm_gbuf_node_t *psr4_file_node(const cbm_pipeline_ctx_t *ctx, cons * to the namespace bucket, which binds it to whichever file of the namespace * came first. `use function` / `use const` name namespace members, not class * files, so they are not applicable. */ -static psr4_outcome_t resolve_php_psr4_class(const cbm_pipeline_ctx_t *ctx, - const char *source_file_qn, const CBMImport *imp, - const cbm_gbuf_node_t **out) { +static psr4_outcome_t resolve_php_psr4_class(const cbm_gbuf_t *gbuf, const char *source_file_qn, + const CBMImport *imp, const cbm_gbuf_node_t **out) { *out = NULL; CBMHashTable *pkgmap = cbm_pipeline_get_pkgmap(); if (!pkgmap || imp->kind != CBM_IMPORT_KIND_DEFAULT) { @@ -2124,7 +2123,7 @@ static psr4_outcome_t resolve_php_psr4_class(const cbm_pipeline_ctx_t *ctx, *c = '/'; } } - const cbm_gbuf_node_t *file = psr4_file_node(ctx, rel); + const cbm_gbuf_node_t *file = psr4_file_node(gbuf, rel); if (file && (!source_file_qn || !file->qualified_name || strcmp(file->qualified_name, source_file_qn) != 0)) { *out = file; @@ -2134,6 +2133,45 @@ static psr4_outcome_t resolve_php_psr4_class(const cbm_pipeline_ctx_t *ctx, return covered ? PSR4_UNRESOLVED : PSR4_NOT_APPLICABLE; } +static bool psr4_class_label(const char *label) { + return label && (strcmp(label, "Class") == 0 || strcmp(label, "Interface") == 0 || + strcmp(label, "Trait") == 0 || strcmp(label, "Enum") == 0); +} + +const char *cbm_pipeline_psr4_member_qn(const cbm_gbuf_t *gbuf, const char *class_fqn, + const char *member) { + if (!gbuf || !class_fqn || !member || !member[0]) { + return NULL; + } + const CBMImport imp = {.module_path = class_fqn, .kind = CBM_IMPORT_KIND_DEFAULT}; + const cbm_gbuf_node_t *file = NULL; + if (resolve_php_psr4_class(gbuf, NULL, &imp, &file) != PSR4_RESOLVED || !file->file_path) { + return NULL; + } + const char *cls = strrchr(class_fqn, '\\'); + cls = cls ? cls + SKIP_ONE : class_fqn; + const cbm_gbuf_node_t **hits = NULL; + int hit_count = 0; + if (cbm_gbuf_find_by_name(gbuf, cls, &hits, &hit_count) != 0 || !hits) { + return NULL; + } + for (int i = 0; i < hit_count; i++) { + const cbm_gbuf_node_t *c = hits[i]; + if (!c || !psr4_class_label(c->label) || !c->qualified_name || !c->file_path || + strcmp(c->file_path, file->file_path) != 0) { + continue; + } + char qn[CBM_SZ_1K]; + int w = snprintf(qn, sizeof(qn), "%s.%s", c->qualified_name, member); + if (w <= 0 || (size_t)w >= sizeof(qn)) { + return NULL; + } + const cbm_gbuf_node_t *m = cbm_gbuf_find_by_qn(gbuf, qn); + return m ? m->qualified_name : NULL; + } + return NULL; +} + const cbm_gbuf_node_t *cbm_pipeline_resolve_import_node(const cbm_pipeline_ctx_t *ctx, const char *source_rel, const char *source_file_qn, @@ -2154,7 +2192,7 @@ const cbm_gbuf_node_t *cbm_pipeline_resolve_import_node(const cbm_pipeline_ctx_t /* PHP class imports covered by a composer psr-4 prefix name exactly one * file; when it is absent the import stays unresolved (#1186). */ const cbm_gbuf_node_t *psr4_target = NULL; - switch (resolve_php_psr4_class(ctx, source_file_qn, imp, &psr4_target)) { + switch (resolve_php_psr4_class(ctx->gbuf, source_file_qn, imp, &psr4_target)) { case PSR4_RESOLVED: return psr4_target; case PSR4_UNRESOLVED: diff --git a/src/pipeline/pipeline_internal.h b/src/pipeline/pipeline_internal.h index 451776807..650f8a08d 100644 --- a/src/pipeline/pipeline_internal.h +++ b/src/pipeline/pipeline_internal.h @@ -213,14 +213,24 @@ void cbm_pipeline_set_pkgmap(CBMHashTable *map); * Laravel `[Class::class, 'method']` and invokable `Class::class` handlers * (#1146), with the class already qualified through the file's `use` imports * and namespace — is placed on a method QN only by what places the class file: - * a composer.json PSR-4 root in psr4_roots (the package map), else the one - * candidate whose directories end with the class's namespace. Never by short - * name alone, so a vendor controller that is not in the repo gets no handler. - * A reference without "::" resolves exactly as cbm_registry_resolve does. */ + * the composer.json PSR-4 class file in gbuf (cbm_pipeline_psr4_member_qn), + * else the one candidate whose directories end with the class's namespace. + * Never by short name alone, so a vendor controller that is not in the repo + * gets no handler. A reference without "::" resolves exactly as + * cbm_registry_resolve does. */ cbm_resolution_t cbm_registry_resolve_handler(const cbm_registry_t *r, const char *handler_ref, const char *module_qn, const char **import_map_keys, const char **import_map_vals, int import_map_count, - const CBMHashTable *psr4_roots); + const cbm_gbuf_t *gbuf); + +/* The QN of `member` on the PHP class `class_fqn` ("Ns\\Class", NUL- + * terminated), placed the way composer's PSR-4 autoloader places the class: + * the class file the import resolver picks for `use Ns\\Class;` (#1186), the + * class that file defines, then that class's member. NULL when no PSR-4 + * prefix covers the class, its file is absent, or the class has no such + * member. The returned string is owned by gbuf. Read-only on gbuf. */ +const char *cbm_pipeline_psr4_member_qn(const cbm_gbuf_t *gbuf, const char *class_fqn, + const char *member); /* Unified module resolver: relative → pkgmap → fqn_module fallback. * Handles bare specifiers via pkgmap lookup with prefix matching. diff --git a/src/pipeline/registry.c b/src/pipeline/registry.c index 4159c19f4..3e564f816 100644 --- a/src/pipeline/registry.c +++ b/src/pipeline/registry.c @@ -1502,42 +1502,21 @@ static int qn_class_member_tail(const char *qn, const char *cls, size_t cls_len, return (int)(c - SKIP_ONE - qn); } -/* PSR-4: the longest composer.json autoload root "Prefix\\" that prefixes the - * FQN maps the remaining namespace onto directories below the root's folder - * and the class onto .php — the file Composer's autoloader loads. - * Returns the registered method QN, or NULL (no root prefixes the FQN, or the - * autoloaded file holds no such method). */ -static const char *psr4_member_qn(const cbm_registry_t *r, const CBMHashTable *psr4_roots, - const char *fqn, size_t fqn_len, const char *cls, size_t cls_len, +/* PSR-4: the class file Composer's autoloader loads for the FQN, the class it + * defines, and that class's member — through the PSR-4 import resolver + * (cbm_pipeline_psr4_member_qn), so a root whose folder does not mirror its + * namespace (Acme\\Blog\\ -> packages/blog/src) places the class as well. + * Returns the member QN, or NULL (no root covers the FQN, its class file is + * absent, or the class holds no such member). */ +static const char *psr4_member_qn(const cbm_gbuf_t *gbuf, const char *fqn, size_t fqn_len, const char *member) { - if (!psr4_roots || fqn_len >= CBM_SZ_512) { + char cls_fqn[CBM_SZ_512]; + if (!gbuf || fqn_len >= sizeof(cls_fqn)) { return NULL; } - char key[CBM_SZ_512]; - for (size_t cut = fqn_len; cut > 0; cut--) { - if (fqn[cut - SKIP_ONE] != '\\') { - continue; - } - memcpy(key, fqn, cut); /* "App\\" / "App\\Http\\" — keeps the trailing '\\' */ - key[cut] = '\0'; - const char *root_qn = (const char *)cbm_ht_get(psr4_roots, key); - if (!root_qn) { - continue; - } - char qn[CBM_SZ_1K]; - int n = snprintf(qn, sizeof(qn), "%s.%.*s.%.*s.%s", root_qn, (int)(fqn_len - cut), - fqn + cut, (int)cls_len, cls, member); - if (n <= 0 || (size_t)n >= sizeof(qn)) { - return NULL; - } - for (char *p = qn + strlen(root_qn); *p; p++) { - if (*p == '\\') { - *p = '.'; - } - } - return cbm_ht_get_key(r->exact, qn); - } - return NULL; + memcpy(cls_fqn, fqn, fqn_len); + cls_fqn[fqn_len] = '\0'; + return cbm_pipeline_psr4_member_qn(gbuf, cls_fqn, member); } /* Do the directory segments of qn (between the project root segment and the @@ -1600,7 +1579,7 @@ static const char *aligned_member_qn(const cbm_registry_t *r, const char *fqn, s cbm_resolution_t cbm_registry_resolve_handler(const cbm_registry_t *r, const char *handler_ref, const char *module_qn, const char **import_map_keys, const char **import_map_vals, int import_map_count, - const CBMHashTable *psr4_roots) { + const cbm_gbuf_t *gbuf) { const char *sep = handler_ref ? strstr(handler_ref, "::") : NULL; if (!r || !sep) { return cbm_registry_resolve(r, handler_ref, module_qn, import_map_keys, import_map_vals, @@ -1618,7 +1597,7 @@ cbm_resolution_t cbm_registry_resolve_handler(const cbm_registry_t *r, const cha if (cls_len == 0 || !member[0]) { return empty_result(); } - const char *qn = psr4_member_qn(r, psr4_roots, handler_ref, fqn_len, cls, cls_len, member); + const char *qn = psr4_member_qn(gbuf, handler_ref, fqn_len, member); if (qn) { return (cbm_resolution_t){qn, "php_psr4", CONF_PSR4_MEMBER, REG_RESOLVED}; } From 2633334c59a8cdd27378d1d42dce20e88816be93 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 4 Oct 2026 22:06:47 +0200 Subject: [PATCH 12/15] ci: analyse and score only the newest main commit CodeQL SAST keyed its concurrency group by commit SHA on push events, so every merge to main got its own group and nothing was ever cancelled; OpenSSF Scorecard had no group at all. On 2026-10-04 a day of 36 merges queued one CodeQL analysis (40-100 minutes each) and one Scorecard run per merge, ahead of every pull request run: pull request CI waited hours for a runner, and a main analysis waited 4 hours. CodeQL now groups by event and by pull request number or ref, so pull requests keep one analysis each as before, and a newer push to main cancels the analysis of an older main commit. Scorecard groups by ref with the same cancellation. Only the newest main commit matters for both: code scanning tracks the default branch's latest analysis. DCO keeps its per-commit group on purpose: its push run on main is the check that catches a merge rewriting sign-offs, so a superseded push must still be checked, and a DCO job takes seconds. Signed-off-by: Martin Vogel --- .github/workflows/codeql.yml | 6 +++++- .github/workflows/scorecard.yml | 6 ++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 2430c6712..621d407fc 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -9,8 +9,12 @@ on: permissions: contents: read +# One analysis per pull request and one per pushed branch. A newer push to main +# cancels the analysis of an older main commit; only the newest commit's +# results matter for main, and a merge-heavy day otherwise queues one 40-100 +# minute analysis per merge ahead of every pull request run. concurrency: - group: codeql-${{ github.ref }}-${{ github.event.pull_request.number || github.sha }} + group: codeql-${{ github.event_name }}-${{ github.event.pull_request.number || github.ref }} cancel-in-progress: true jobs: diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index c2acbd8ac..fb0a52bbc 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -8,6 +8,12 @@ on: permissions: read-all +# Only the newest main commit is scored: a newer push (or the weekly run) +# cancels a pending or running score of an older commit. +concurrency: + group: scorecard-${{ github.ref }} + cancel-in-progress: true + jobs: scorecard: runs-on: ubuntu-latest From f8d8fcdefec1cb5d28a064b98270a7fe3a5c45ac Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 4 Oct 2026 22:27:29 +0200 Subject: [PATCH 13/15] test: keep ASan stack-use-after-return off outside the diag lane gcc's libasan turns detect_stack_use_after_return ON by default; clang's runtime leaves it off. On aarch64, gcc 13/14 free small fake frames without clearing their in-use flag, so once 8,192 frames of a size class have been used, every instrumented call scans the flag array (~8 us instead of ~6 ns) and the check silently stops detecting anything. CI's ubuntu-24.04-arm gcc leg runs the extraction suite in 688 s against a 900 s cap (16 s on amd64), and the deep extraction tests of #2478 push it over the cap. The lanes already document the intent: test-diag in _test.yml and `make -f Makefile.cbm diag` set detect_stack_use_after_return=1 explicitly, as one of the ASan checks that are off by default in every other lane. scripts/test.sh now appends detect_stack_use_after_return=0 to ASAN_OPTIONS (keeping any options already set, ':'-separated) unless ASAN_OPTIONS already mentions the flag, and exports it, so test-diag's explicit =1 wins. Lanes built with clang already ran with it off and are unchanged. The venue-parity contract (Step 0j) gains a layer that drives the real scripts/test.sh with a no-op make and a probe runner and asserts three cases: unset, other options kept with the flag appended, explicit =1 kept. Measured in the local Linux arm64 container (Colima, gcc 13.3, ASan + UBSan), wall time of `scripts/test.sh --suites extraction`: origin/main 268a9d88 without this change 628.6 s origin/main 268a9d88 with this change 5.1 s #2478 head 818ef0ec with this change 9.7 s Signed-off-by: Martin Vogel --- scripts/test.sh | 12 ++++++ tests/test_venue_parity_contract.sh | 65 ++++++++++++++++++++++++++++- 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/scripts/test.sh b/scripts/test.sh index 1741f8a1d..54962bf04 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -179,6 +179,18 @@ if [ "$SANITIZE_GIVEN" -eq 0 ] && [ "${MSYSTEM:-}" = "CLANGARM64" ]; then MAKE_ARGS+=("SANITIZE=-fsanitize=undefined -fsanitize-trap=undefined -fstack-protector-strong -fno-omit-frame-pointer") fi +# ASan stack-use-after-return runs in the test-diag lane only (_test.yml and +# `make -f Makefile.cbm diag` set detect_stack_use_after_return=1). gcc's +# libasan turns it ON by default, and on aarch64 gcc 13/14 never clear a freed +# fake frame's in-use flag: past 8,192 frames per size class every instrumented +# call scans that flag array and the check stops detecting anything (gcc arm64 +# ran `extraction` in 688 s, amd64 in 16 s). Off here; an explicit setting wins. +case "${ASAN_OPTIONS:-}" in + *detect_stack_use_after_return*) ;; + *) ASAN_OPTIONS="${ASAN_OPTIONS:+$ASAN_OPTIONS:}detect_stack_use_after_return=0" ;; +esac +export ASAN_OPTIONS + EXPECTED_SANITIZED=1 if [ "$SANITIZE_GIVEN" -eq 1 ]; then case "$SANITIZE_VALUE" in diff --git a/tests/test_venue_parity_contract.sh b/tests/test_venue_parity_contract.sh index 81e431a1e..cef82b33e 100755 --- a/tests/test_venue_parity_contract.sh +++ b/tests/test_venue_parity_contract.sh @@ -24,10 +24,13 @@ # 5. Interface probes — every entry must answer --help (exit 0, "Usage:") # and reject unknown flags with exit 2 + "Please consult --help." so an # agent can always discover exactly what a run will do. +# 6. Sanitizer runtime defaults — scripts/test.sh, not each venue, sets the +# ASan options every lane runs with (stack-use-after-return stays off +# unless a lane asks for it explicitly, as test-diag does). # # Usage: tests/test_venue_parity_contract.sh [repo-root] (root override so # the suite can prove the contract FAILS on a violation, not just passes; -# layer 5 runs only against the real repo root). +# layers 5 and 6 run only against the real repo root). set -euo pipefail @@ -455,3 +458,63 @@ scripts/memwaste.sh fi echo "interface probes OK ($(printf '%s' "$HELP_ENTRIES" | grep -c .) --help entries, $(printf '%s' "$STRICT_ENTRIES" | grep -c .) strict-flag entries)" fi + +# ── Layer 6: sanitizer runtime defaults (real repo root only). Drives the REAL +# scripts/test.sh in --suites mode with a no-op `make` first on PATH and a +# probe standing in for the test-runner: the probe answers the build-config +# handshake, then records the ASAN_OPTIONS it inherited. Every case sets or +# unsets ASAN_OPTIONS itself, because this contract runs INSIDE test.sh, whose +# own export would otherwise leak in. +if [ -d "$ROOT/scripts" ] && [ -f "$ROOT/scripts/test.sh" ]; then + asan_failures=0 + asan_tmp="$(mktemp -d)" + trap 'rm -rf "$asan_tmp"' EXIT + mkdir -p "$asan_tmp/bin" "$asan_tmp/build" + printf '#!/usr/bin/env bash\nexit 0\n' > "$asan_tmp/bin/make" + cat > "$asan_tmp/build/test-runner" <<'EOF' +#!/usr/bin/env bash +if [ "${1:-}" = "--build-config" ]; then + printf 'sanitized=1 test_seams=1\n' + exit 0 +fi +printf '%s' "${ASAN_OPTIONS-}" > "$CBM_ASAN_PROBE_OUT" +EOF + chmod +x "$asan_tmp/bin/make" "$asan_tmp/build/test-runner" + + # asan_case LABEL GIVEN WANT: GIVEN is the caller's ASAN_OPTIONS ("" + # for none), WANT the exact value the runner must receive. + asan_case() { + local label="$1" given="$2" want="$3" seen rc + rm -f "$asan_tmp/seen" + ( + if [ "$given" = "" ]; then + unset ASAN_OPTIONS + else + export ASAN_OPTIONS="$given" + fi + cd "$ROOT" && PATH="$asan_tmp/bin:$PATH" CBM_NO_CCACHE=1 \ + CBM_ASAN_PROBE_OUT="$asan_tmp/seen" \ + bash scripts/test.sh --suites probe "BUILD_DIR=$asan_tmp/build" + ) >"$asan_tmp/log" 2>&1 && rc=0 || rc=$? + seen="$(cat "$asan_tmp/seen" 2>/dev/null || printf '')" + if [ "$rc" -ne 0 ] || [ "$seen" != "$want" ]; then + echo "SANITIZER DEFAULTS: $label: runner got ASAN_OPTIONS=[$seen], want [$want] (test.sh rc=$rc)" >&2 + sed 's/^/ /' "$asan_tmp/log" >&2 + asan_failures=1 + fi + } + + asan_case "unset" "" \ + "detect_stack_use_after_return=0" + asan_case "other options kept" "detect_leaks=1:halt_on_error=1" \ + "detect_leaks=1:halt_on_error=1:detect_stack_use_after_return=0" + asan_case "explicit setting wins (test-diag)" \ + "detect_stack_use_after_return=1:strict_string_checks=1:detect_stack_use_after_scope=1" \ + "detect_stack_use_after_return=1:strict_string_checks=1:detect_stack_use_after_scope=1" + + if [ "$asan_failures" -ne 0 ]; then + echo "VENUE PARITY CONTRACT VIOLATED — sanitizer runtime defaults failed (layer 6)" >&2 + exit 1 + fi + echo "sanitizer runtime defaults OK (3 ASAN_OPTIONS cases through scripts/test.sh)" +fi From 70e73442ed3ced71619d561b901af92729ede595 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 5 Oct 2026 01:37:11 +0200 Subject: [PATCH 14/15] fix(routes): leave a PSR-4-covered handler without a class file unresolved Why: when a composer.json PSR-4 prefix covers a handler class but no class file exists for it, cbm_registry_resolve_handler fell through to the namespace/folder fallback and bound the route to a same-named class in any folder that mirrors the namespace, a class composer never loads. Main's import resolver stops in that case: resolve_php_psr4_class leaves the `use` import unresolved (#1186). The handler now does the same, so no guessed HANDLES edge is minted. Classes that no PSR-4 prefix covers keep the fallback unchanged. Change: - pipeline_internal.h: cbm_pipeline_psr4_member_qn returns a tri-state cbm_psr4_member_t (NOT_COVERED, RESOLVED, UNRESOLVED) and hands out the member QN through an out parameter, set only when RESOLVED. UNRESOLVED means a prefix covers the class but no class file holds the class with that member: the file is absent, or it lacks the class or the member. - pass_pkgmap.c: maps the outcome of resolve_php_psr4_class onto it; the class/member lookup in the resolved file moves into psr4_member_in_file. - registry.c: cbm_registry_resolve_handler returns unresolved on UNRESOLVED and runs the folder fallback only on NOT_COVERED. Proof: - New handles_laravel_psr4_absent_class_issue1146 and its _parallel twin (edge_types_probe) extend the issue-1146 fixture with a route to \Acme\Blog\Http\MissingController::index (covered by Acme\Blog\ -> packages/blog/src/, no class file) and one to PostsController::archive (class file exists, no archive method), plus namespace-mirroring decoys in legacy/Acme/Blog/Http that hold both members. The test asserts exactly the 4 HANDLES of the fixture and that both Route nodes exist. RED before the change on both paths with the 2 decoy edges (legacy...MissingController.index -> /blog/missing, legacy...PostsController.archive -> /blog/archive; expected=4 actual=6; suite 77 passed, 2 failed). GREEN after (79 passed). RED again with the production change reverted (the same 2 edges), GREEN once it was restored. - Real input: koel/koel v9.11.1 (App\ -> app/) indexed with the new binary under a private HOME, cache and runtime: 16,564 nodes, 60,177 edges, 15 Routes and 1 HANDLES (/demo/new-session -> NewSessionController.__invoke), identical to the numbers recorded for 42118ad1. - Suites: edge_types_probe 79, edge_imports 68, pipeline 317 and route_canon 11 all pass (475). make lint-ci passes. Refs #1146 Signed-off-by: Martin Vogel --- src/pipeline/pass_pkgmap.c | 41 ++++++++++++------ src/pipeline/pipeline_internal.h | 40 ++++++++++++------ src/pipeline/registry.c | 23 ++++++---- tests/test_edge_types_probe.c | 72 +++++++++++++++++++++++++++++++- 4 files changed, 141 insertions(+), 35 deletions(-) diff --git a/src/pipeline/pass_pkgmap.c b/src/pipeline/pass_pkgmap.c index 71d3d3bec..c82cdf645 100644 --- a/src/pipeline/pass_pkgmap.c +++ b/src/pipeline/pass_pkgmap.c @@ -2138,18 +2138,10 @@ static bool psr4_class_label(const char *label) { strcmp(label, "Trait") == 0 || strcmp(label, "Enum") == 0); } -const char *cbm_pipeline_psr4_member_qn(const cbm_gbuf_t *gbuf, const char *class_fqn, - const char *member) { - if (!gbuf || !class_fqn || !member || !member[0]) { - return NULL; - } - const CBMImport imp = {.module_path = class_fqn, .kind = CBM_IMPORT_KIND_DEFAULT}; - const cbm_gbuf_node_t *file = NULL; - if (resolve_php_psr4_class(gbuf, NULL, &imp, &file) != PSR4_RESOLVED || !file->file_path) { - return NULL; - } - const char *cls = strrchr(class_fqn, '\\'); - cls = cls ? cls + SKIP_ONE : class_fqn; +/* The QN of `member` on the class `cls` defined in `file_path`, or NULL when + * that file defines no such class or the class has no such member. */ +static const char *psr4_member_in_file(const cbm_gbuf_t *gbuf, const char *file_path, + const char *cls, const char *member) { const cbm_gbuf_node_t **hits = NULL; int hit_count = 0; if (cbm_gbuf_find_by_name(gbuf, cls, &hits, &hit_count) != 0 || !hits) { @@ -2158,7 +2150,7 @@ const char *cbm_pipeline_psr4_member_qn(const cbm_gbuf_t *gbuf, const char *clas for (int i = 0; i < hit_count; i++) { const cbm_gbuf_node_t *c = hits[i]; if (!c || !psr4_class_label(c->label) || !c->qualified_name || !c->file_path || - strcmp(c->file_path, file->file_path) != 0) { + strcmp(c->file_path, file_path) != 0) { continue; } char qn[CBM_SZ_1K]; @@ -2172,6 +2164,29 @@ const char *cbm_pipeline_psr4_member_qn(const cbm_gbuf_t *gbuf, const char *clas return NULL; } +cbm_psr4_member_t cbm_pipeline_psr4_member_qn(const cbm_gbuf_t *gbuf, const char *class_fqn, + const char *member, const char **out_qn) { + *out_qn = NULL; + if (!gbuf || !class_fqn || !member || !member[0]) { + return CBM_PSR4_MEMBER_NOT_COVERED; + } + const CBMImport imp = {.module_path = class_fqn, .kind = CBM_IMPORT_KIND_DEFAULT}; + const cbm_gbuf_node_t *file = NULL; + psr4_outcome_t placed = resolve_php_psr4_class(gbuf, NULL, &imp, &file); + if (placed == PSR4_NOT_APPLICABLE) { + return CBM_PSR4_MEMBER_NOT_COVERED; + } + /* Covered from here on: composer would load this file or none, so a + * missing file, class or member leaves the member unresolved. */ + if (placed != PSR4_RESOLVED || !file->file_path) { + return CBM_PSR4_MEMBER_UNRESOLVED; + } + const char *cls = strrchr(class_fqn, '\\'); + cls = cls ? cls + SKIP_ONE : class_fqn; + *out_qn = psr4_member_in_file(gbuf, file->file_path, cls, member); + return *out_qn ? CBM_PSR4_MEMBER_RESOLVED : CBM_PSR4_MEMBER_UNRESOLVED; +} + const cbm_gbuf_node_t *cbm_pipeline_resolve_import_node(const cbm_pipeline_ctx_t *ctx, const char *source_rel, const char *source_file_qn, diff --git a/src/pipeline/pipeline_internal.h b/src/pipeline/pipeline_internal.h index 650f8a08d..fb3ed1253 100644 --- a/src/pipeline/pipeline_internal.h +++ b/src/pipeline/pipeline_internal.h @@ -213,24 +213,38 @@ void cbm_pipeline_set_pkgmap(CBMHashTable *map); * Laravel `[Class::class, 'method']` and invokable `Class::class` handlers * (#1146), with the class already qualified through the file's `use` imports * and namespace — is placed on a method QN only by what places the class file: - * the composer.json PSR-4 class file in gbuf (cbm_pipeline_psr4_member_qn), - * else the one candidate whose directories end with the class's namespace. - * Never by short name alone, so a vendor controller that is not in the repo - * gets no handler. A reference without "::" resolves exactly as - * cbm_registry_resolve does. */ + * the composer.json PSR-4 class file in gbuf (cbm_pipeline_psr4_member_qn). + * When a PSR-4 prefix covers the class, that answer is final: a covered class + * whose file or member is absent stays unresolved, as its `use` import does + * (#1186). Only a class no prefix covers falls back to the one candidate whose + * directories end with the class's namespace. Never by short name alone, so a + * vendor controller that is not in the repo gets no handler. A reference + * without "::" resolves exactly as cbm_registry_resolve does. */ cbm_resolution_t cbm_registry_resolve_handler(const cbm_registry_t *r, const char *handler_ref, const char *module_qn, const char **import_map_keys, const char **import_map_vals, int import_map_count, const cbm_gbuf_t *gbuf); -/* The QN of `member` on the PHP class `class_fqn` ("Ns\\Class", NUL- - * terminated), placed the way composer's PSR-4 autoloader places the class: - * the class file the import resolver picks for `use Ns\\Class;` (#1186), the - * class that file defines, then that class's member. NULL when no PSR-4 - * prefix covers the class, its file is absent, or the class has no such - * member. The returned string is owned by gbuf. Read-only on gbuf. */ -const char *cbm_pipeline_psr4_member_qn(const cbm_gbuf_t *gbuf, const char *class_fqn, - const char *member); +/* Where composer's PSR-4 autoloader places a PHP class member. Tri-state, so + * "no prefix covers the class" (another strategy may answer) is never + * confused with "a prefix covers it but places nothing" (final). */ +typedef enum { + CBM_PSR4_MEMBER_NOT_COVERED = 0, /* no psr-4 prefix covers the class */ + CBM_PSR4_MEMBER_RESOLVED, /* *out_qn is the member's QN */ + CBM_PSR4_MEMBER_UNRESOLVED, /* covered, but no class file holds the + * class with this member: leave it + * unresolved, never guess elsewhere */ +} cbm_psr4_member_t; + +/* Place `member` on the PHP class `class_fqn` ("Ns\\Class", NUL-terminated) + * the way composer's PSR-4 autoloader places the class: the class file the + * import resolver picks for `use Ns\\Class;` (#1186), the class that file + * defines, then that class's member. *out_qn is set only on + * CBM_PSR4_MEMBER_RESOLVED (else NULL) and is owned by gbuf. Missing + * arguments or no package map give CBM_PSR4_MEMBER_NOT_COVERED. Read-only on + * gbuf. */ +cbm_psr4_member_t cbm_pipeline_psr4_member_qn(const cbm_gbuf_t *gbuf, const char *class_fqn, + const char *member, const char **out_qn); /* Unified module resolver: relative → pkgmap → fqn_module fallback. * Handles bare specifiers via pkgmap lookup with prefix matching. diff --git a/src/pipeline/registry.c b/src/pipeline/registry.c index 3e564f816..1e4e0819e 100644 --- a/src/pipeline/registry.c +++ b/src/pipeline/registry.c @@ -1506,17 +1506,17 @@ static int qn_class_member_tail(const char *qn, const char *cls, size_t cls_len, * defines, and that class's member — through the PSR-4 import resolver * (cbm_pipeline_psr4_member_qn), so a root whose folder does not mirror its * namespace (Acme\\Blog\\ -> packages/blog/src) places the class as well. - * Returns the member QN, or NULL (no root covers the FQN, its class file is - * absent, or the class holds no such member). */ -static const char *psr4_member_qn(const cbm_gbuf_t *gbuf, const char *fqn, size_t fqn_len, - const char *member) { + * Returns its tri-state outcome; *out_qn is the member QN when RESOLVED. */ +static cbm_psr4_member_t psr4_member_qn(const cbm_gbuf_t *gbuf, const char *fqn, size_t fqn_len, + const char *member, const char **out_qn) { char cls_fqn[CBM_SZ_512]; + *out_qn = NULL; if (!gbuf || fqn_len >= sizeof(cls_fqn)) { - return NULL; + return CBM_PSR4_MEMBER_NOT_COVERED; } memcpy(cls_fqn, fqn, fqn_len); cls_fqn[fqn_len] = '\0'; - return cbm_pipeline_psr4_member_qn(gbuf, cls_fqn, member); + return cbm_pipeline_psr4_member_qn(gbuf, cls_fqn, member, out_qn); } /* Do the directory segments of qn (between the project root segment and the @@ -1597,9 +1597,16 @@ cbm_resolution_t cbm_registry_resolve_handler(const cbm_registry_t *r, const cha if (cls_len == 0 || !member[0]) { return empty_result(); } - const char *qn = psr4_member_qn(gbuf, handler_ref, fqn_len, member); - if (qn) { + const char *qn = NULL; + switch (psr4_member_qn(gbuf, handler_ref, fqn_len, member, &qn)) { + case CBM_PSR4_MEMBER_RESOLVED: return (cbm_resolution_t){qn, "php_psr4", CONF_PSR4_MEMBER, REG_RESOLVED}; + case CBM_PSR4_MEMBER_UNRESOLVED: + /* A covered class lives where PSR-4 says or nowhere; a namespace- + * mirroring folder elsewhere would be a guessed edge (#1186). */ + return empty_result(); + case CBM_PSR4_MEMBER_NOT_COVERED: + break; } qn = aligned_member_qn(r, handler_ref, ns_len, cls, cls_len, member); if (qn) { diff --git a/tests/test_edge_types_probe.c b/tests/test_edge_types_probe.c index 999019c45..6dfa94e1c 100644 --- a/tests/test_edge_types_probe.c +++ b/tests/test_edge_types_probe.c @@ -939,7 +939,11 @@ static int et_qn_has_tail(const char *qn, const char *tail) { return ql >= tl && strcmp(qn + ql - tl, tail) == 0 && (ql == tl || qn[ql - tl - 1] == '.'); } -static int et_handles_exact(const EtFile *files, int nfiles, const EtHandles *want, int parallel) { +/* route_qns (optional, NULL-terminated): Route qualified_names that must + * exist, so a test asserting that a route gets NO handler cannot pass merely + * because the route itself was never extracted. */ +static int et_handles_exact_routes(const EtFile *files, int nfiles, const EtHandles *want, + int parallel, const char *const *route_qns) { EtProj lp; cbm_store_t *store = parallel ? et_index_parallel(&lp, files, nfiles) : et_index_files(&lp, files, nfiles); @@ -983,6 +987,16 @@ static int et_handles_exact(const EtFile *files, int nfiles, const EtHandles *wa want[wi].route); } } + for (int ri = 0; route_qns && route_qns[ri]; ri++) { + cbm_node_t route = {0}; + if (!store || + cbm_store_find_node_by_qn(store, lp.project, route_qns[ri], &route) != CBM_STORE_OK) { + ok = 0; + fprintf(stderr, " [ET-HANDLES] missing Route %s\n", route_qns[ri]); + } else { + cbm_node_free_fields(&route); + } + } if (!ok) { fprintf(stderr, " [ET-HANDLES] FAIL (%s path) expected=%d actual=%d\n", parallel ? "parallel" : "sequential", wanted, n); @@ -992,6 +1006,10 @@ static int et_handles_exact(const EtFile *files, int nfiles, const EtHandles *wa return ok; } +static int et_handles_exact(const EtFile *files, int nfiles, const EtHandles *want, int parallel) { + return et_handles_exact_routes(files, nfiles, want, parallel, NULL); +} + /* #1146: Laravel's two class-based handler forms — * Route::get('/x', [UserController::class, 'show']) (controller method) * Route::get('/x', GetCurrentUserController::class) (invokable: __invoke) @@ -1085,6 +1103,56 @@ TEST(handles_laravel_class_handlers_no_composer_issue1146) { PASS(); } +/* A PSR-4 prefix that covers a handler class decides where the class lives, + * as it does for the class's `use` import (#1186): when composer would load + * no file holding the member, the route stays without a handler. Both routes + * below are covered by Acme\Blog\ -> packages/blog/src/: + * /blog/missing MissingController has no class file there at all; + * /blog/archive PostsController's class file exists but has no archive(). + * legacy/Acme/Blog/Http mirrors the namespace and holds both members, so the + * namespace/folder fallback would bind both routes to a class composer never + * loads. It must not run for a covered class. These files are added to the + * fixture above, whose four handlers must keep their edges. */ +static const EtFile et_laravel_psr4_absent_extra[] = { + {"legacy/Acme/Blog/Http/MissingController.php", + " true]; }\n}\n"}, + {"legacy/Acme/Blog/Http/PostsController.php", + " true]; }\n}\n"}, + {"routes/blog.php", + " Date: Wed, 7 Oct 2026 18:36:15 +0200 Subject: [PATCH 15/15] fix(extraction): parse C++ branch views under the terminated-last-line rule Merging main brought extract_cpp_branch_views (conditional variants), which built its CBMStringInput with the two-field initializer. With this branch's virtual_newline field that is a missing initializer, an error under GCC's -Wextra -Werror (the shadow job's product build). A branch view keeps the raw source's length and line structure, so it now goes through cbm_parse_source like the raw parse: the same virtual newline, and the same edit that removes it again. Signed-off-by: Martin Vogel --- internal/cbm/cbm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/cbm/cbm.c b/internal/cbm/cbm.c index fb0d14099..210a0a067 100644 --- a/internal/cbm/cbm.c +++ b/internal/cbm/cbm.c @@ -2267,8 +2267,6 @@ static void extract_cpp_branch_views(const CBMExtractCtx *raw, const TSLanguage return; } ts_parser_reset(parser); - CBMStringInput input = {view, (uint32_t)raw->source_len}; - TSInput ts_input = {&input, cbm_string_read, TSInputEncodingUTF8, NULL}; TSParseOptions opts = {0}; CBMParseBudget budget = {0}; // cppcheck-suppress unreadVariable if (timeout_micros > 0) { @@ -2278,7 +2276,9 @@ static void extract_cpp_branch_views(const CBMExtractCtx *raw, const TSLanguage opts.payload = &budget; opts.progress_callback = cbm_timeout_cb; } - TSTree *tree = ts_parser_parse_with_options(parser, NULL, ts_input, opts); + /* A view keeps the raw source's length and line structure, so it is + * parsed under the same terminated-last-line rule (#2078). */ + TSTree *tree = cbm_parse_source(parser, view, (uint32_t)raw->source_len, opts); if (!tree) { return; }