From c1b9c451c7f124936e635835ca4dd0cf8bf7d5b3 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 6 Sep 2026 02:48:16 +0200 Subject: [PATCH 1/2] fix(daemon): keep one separator spelling for the session root on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows cbm_canonical_path answers in the platform's native backslash form, and the daemon stored that spelling verbatim for the client's session_root and allowed_root (cbm_mcp_server_set_session_context) and for the auto-index job's repo_path (application.c canonicalizes with the bare call at three sites). Every tool handler, however, normalizes separators after canonicalizing. One directory therefore had two names, and the exact comparison in application_index_args_equal never matched: an explicit index_repository request for the session root was refused as OPTIONS_CONFLICT instead of joining the auto-index job already running for it. Normalize the separator spelling of both session roots in set_session_context, and canonicalize plus normalize at the three application.c sites through one helper, application_canonical_root(). Separators fold on every platform, so the new test daemon_session_context_keeps_one_spelling_of_a_root binds wherever the suite runs; it fails on main with "C:\repos\cbm" != "C:/repos/cbm". Distilled from #1726 with co-author credit. Signed-off-by: Martin Vogel Co-authored-by: 刘冲 --- src/daemon/application.c | 21 ++++++++++++++++++--- src/mcp/mcp.c | 16 ++++++++++++++-- tests/test_daemon.c | 21 +++++++++++++++++++++ 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/daemon/application.c b/src/daemon/application.c index 69ff52ec7..8e3fea72f 100644 --- a/src/daemon/application.c +++ b/src/daemon/application.c @@ -379,6 +379,20 @@ static bool application_canonical_directory_exists(const char *path) { return cbm_path_info_utf8(path, &info) == 0 && info.is_directory; } +/* The daemon's one spelling of a repository root. cbm_canonical_path answers in + * the platform's native form (backslashes on Windows) while every tool handler + * normalizes the separators of the paths it canonicalizes. A root kept native + * is a second name for one directory, and the exact comparison downstream - + * index-args equality when a request joins the job already running for that + * root - never matches it. */ +static bool application_canonical_root(const char *path, char *out, size_t out_size) { + if (!cbm_canonical_path(path, out, out_size)) { + return false; + } + cbm_normalize_path_sep(out); + return true; +} + static cbm_daemon_application_watch_t *application_find_watch_locked( cbm_daemon_application_t *application, const char *project) { for (cbm_daemon_application_watch_t *watch = application->watches; watch; watch = watch->next) { @@ -2355,9 +2369,10 @@ static cbm_daemon_runtime_application_status_t application_set_context( } char canonical_root[APPLICATION_PATH_CAP] = {0}; char canonical_allowed[APPLICATION_PATH_CAP] = {0}; - bool canonical = cbm_canonical_path(root, canonical_root, sizeof(canonical_root)); + bool canonical = application_canonical_root(root, canonical_root, sizeof(canonical_root)); if (canonical && allowed_present) { - canonical = cbm_canonical_path(allowed, canonical_allowed, sizeof(canonical_allowed)); + canonical = + application_canonical_root(allowed, canonical_allowed, sizeof(canonical_allowed)); } canonical = canonical && application_canonical_directory_exists(canonical_root); bool set = @@ -3372,7 +3387,7 @@ static int application_background_index(cbm_daemon_application_t *application, return -1; } char canonical_root[APPLICATION_PATH_CAP]; - if (!cbm_canonical_path(root_path, canonical_root, sizeof(canonical_root)) || + if (!application_canonical_root(root_path, canonical_root, sizeof(canonical_root)) || !application_canonical_directory_exists(canonical_root)) { return -1; } diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 3259221c4..14da18e75 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -1765,7 +1765,18 @@ bool cbm_mcp_server_set_session_context(cbm_mcp_server_t *srv, const char *sessi return false; } - char *project = cbm_project_name_from_path(session_root); + /* Keep both roots in the one spelling every tool handler produces. Each + * handler normalizes separators after canonicalizing, so a root stored in + * the platform's native form (backslashes on Windows) is a second name for + * the same directory - and the daemon compares roots exactly. An explicit + * index_repository request for this root was refused as an options + * conflict instead of joining the auto-index job already running for it, + * because the job's repo_path never matched the request's. */ + char root[sizeof(srv->session_root)]; + snprintf(root, sizeof(root), "%s", session_root); + cbm_normalize_path_sep(root); + + char *project = cbm_project_name_from_path(root); if (!project || project[0] == '\0' || strlen(project) >= sizeof(srv->session_project)) { free(project); return false; @@ -1776,8 +1787,9 @@ bool cbm_mcp_server_set_session_context(cbm_mcp_server_t *srv, const char *sessi free(project); return false; } + cbm_normalize_path_sep(allowed_copy); - snprintf(srv->session_root, sizeof(srv->session_root), "%s", session_root); + snprintf(srv->session_root, sizeof(srv->session_root), "%s", root); snprintf(srv->session_project, sizeof(srv->session_project), "%s", project); free(project); diff --git a/tests/test_daemon.c b/tests/test_daemon.c index fb0d0095a..ff0bac8cf 100644 --- a/tests/test_daemon.c +++ b/tests/test_daemon.c @@ -467,6 +467,26 @@ TEST(daemon_sessions_keep_distinct_roots_and_allowed_root_policy) { PASS(); } +/* The daemon canonicalizes a client's root through the platform API, which on + * Windows answers in backslash form, while every tool handler normalizes the + * separators of the paths it canonicalizes. Stored in its native form, one + * directory had two names, and an explicit index_repository request was + * refused as an options conflict instead of joining the auto-index job already + * running for that root. Separators fold on every platform, so the check binds + * wherever the suite runs. */ +TEST(daemon_session_context_keeps_one_spelling_of_a_root) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_mcp_server_set_background_tasks(srv, false); + + ASSERT_TRUE(cbm_mcp_server_set_session_context(srv, "C:\\repos\\cbm", "C:\\repos")); + ASSERT_STR_EQ(cbm_mcp_server_session_root(srv), "C:/repos/cbm"); + ASSERT_STR_EQ(cbm_mcp_server_allowed_root(srv), "C:/repos"); + + cbm_mcp_server_free(srv); + PASS(); +} + SUITE(daemon) { RUN_TEST(daemon_client_ids_are_connection_bound); RUN_TEST(daemon_shared_job_survives_until_final_subscriber_disconnects); @@ -480,4 +500,5 @@ SUITE(daemon) { RUN_TEST(daemon_bridge_rejects_embedded_nul_body); RUN_TEST(daemon_bridge_rejects_oversized_headers); RUN_TEST(daemon_sessions_keep_distinct_roots_and_allowed_root_policy); + RUN_TEST(daemon_session_context_keeps_one_spelling_of_a_root); } From 9846c3f1711d557c78426913efaff340e37feb4e Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 6 Sep 2026 16:23:45 +0200 Subject: [PATCH 2/2] fix(daemon): compare session roots separator-equivalently without respelling the policy root The first cut on this branch (c1b9c451) respelled the session root and the allowed root with forward slashes on the way into the session policy (cbm_mcp_server_set_session_context) and at the three application.c sites that canonicalize a root. CI's Windows shard then failed two tests: tests/test_daemon_application.c:2256: ASSERT(sensitive_blocked) tests/test_daemon_application.c:2380: ASSERT(sensitive_blocked) Both set HOME to the cbm_canonical_path spelling of a directory - the platform's native form, backslashes on Windows - and expect a session rooted there to be refused for auto-index and for watch. The sensitive-root and allowed-root containment walk (ws_is_ancestor_or_equal in src/foundation/workspace.c) compares prefixes byte-exact, so a root that now read C:/Users/... no longer matched a HOME of C:\Users\...: the sensitive refusal was lost and $HOME was admitted. Respelling the policy root was the wrong layer. This commit keeps the policy roots in their canonical native spelling (mcp.c and the three application.c sites are back to what main runs) and folds the separator spelling at the one comparison the original defect lives in: application_index_args_equal, where the running job's args are compared with an explicit index_repository request's. The auto-index job spells repo_path the way the session policy holds it; the handler spells it with forward slashes. On Windows the two never matched, and the request was refused as OPTIONS_CONFLICT instead of joining the job already running for its root. The fold acts on the parsed copies only (yyjson_mut_obj_replace with a cbm_normalize_path_sep'd string), so nothing the daemon stores changes spelling and every other option stays exact. Why this is correct on Windows without a local Windows run: the policy spelling is untouched, so the containment path is byte-for-byte what main runs there today; cbm_normalize_path_sep folds backslashes on every platform, so the new fold is exercised by the new test wherever the suite runs. Tests: daemon_session_context_keeps_one_spelling_of_a_root asserted that the policy respells a root with forward slashes. That was the wrong contract: the policy's containment checks compare its spelling byte-exact against HOME and the granted roots, so a respelled root is exactly what stops matching them. It is replaced by daemon_session_context_keeps_the_policy_spelling_of_a_root, which binds the opposite: the policy stores the root as given. The comparison is exposed through application_internal.h as cbm_daemon_application_index_args_equal_for_test and bound by daemon_application_index_args_compare_repo_path_separator_equivalently: a backslash and a forward-slash repo_path compare equal; a different path, a sub-path, and a different mode still do not. Refs #1726 Signed-off-by: Martin Vogel --- src/daemon/application.c | 54 ++++++++++++++++++++----------- src/daemon/application_internal.h | 6 ++++ src/mcp/mcp.c | 16 ++------- tests/test_daemon.c | 22 ++++++------- tests/test_daemon_application.c | 25 ++++++++++++++ 5 files changed, 80 insertions(+), 43 deletions(-) diff --git a/src/daemon/application.c b/src/daemon/application.c index 8e3fea72f..d005f9217 100644 --- a/src/daemon/application.c +++ b/src/daemon/application.c @@ -379,20 +379,6 @@ static bool application_canonical_directory_exists(const char *path) { return cbm_path_info_utf8(path, &info) == 0 && info.is_directory; } -/* The daemon's one spelling of a repository root. cbm_canonical_path answers in - * the platform's native form (backslashes on Windows) while every tool handler - * normalizes the separators of the paths it canonicalizes. A root kept native - * is a second name for one directory, and the exact comparison downstream - - * index-args equality when a request joins the job already running for that - * root - never matches it. */ -static bool application_canonical_root(const char *path, char *out, size_t out_size) { - if (!cbm_canonical_path(path, out, out_size)) { - return false; - } - cbm_normalize_path_sep(out); - return true; -} - static cbm_daemon_application_watch_t *application_find_watch_locked( cbm_daemon_application_t *application, const char *project) { for (cbm_daemon_application_watch_t *watch = application->watches; watch; watch = watch->next) { @@ -1602,6 +1588,33 @@ static bool application_index_args_normalize_defaults(yyjson_mut_val *root) { return true; } +/* One directory is one root. The auto-index job spells repo_path the way the + * session policy holds it - the platform's native form, backslashes on + * Windows - while an explicit index_repository request arrives in the + * handler's forward-slash spelling. Compared byte-exact the two never matched + * on Windows, and the request was refused as an options conflict instead of + * joining the job already running for its root. The policy keeps its + * spelling: the sensitive-root and allowed-root containment checks match it + * byte-exact against HOME and the granted roots, and respelling it there + * admitted $HOME. So the fold happens here, on this comparison's private copy, + * and nothing the daemon stores changes. */ +static bool application_index_args_fold_repo_path(yyjson_mut_doc *document) { + yyjson_mut_val *root = yyjson_mut_doc_get_root(document); + yyjson_mut_val *repo_path = yyjson_mut_obj_get(root, "repo_path"); + if (!repo_path || !yyjson_mut_is_str(repo_path)) { + return true; + } + char *folded = strdup(yyjson_mut_get_str(repo_path)); + if (!folded) { + return false; + } + cbm_normalize_path_sep(folded); + yyjson_mut_val *key = yyjson_mut_str(document, "repo_path"); + yyjson_mut_val *value = yyjson_mut_strcpy(document, folded); + free(folded); + return key && value && yyjson_mut_obj_replace(root, key, value); +} + static bool application_index_args_equal(const char *left, const char *right) { if (!left || !right) { return false; @@ -1614,6 +1627,8 @@ static bool application_index_args_equal(const char *left, const char *right) { yyjson_mut_val *right_root = right_copy ? yyjson_mut_doc_get_root(right_copy) : NULL; bool equal = application_index_args_normalize_defaults(left_root) && application_index_args_normalize_defaults(right_root) && + application_index_args_fold_repo_path(left_copy) && + application_index_args_fold_repo_path(right_copy) && yyjson_mut_equals(left_root, right_root); yyjson_mut_doc_free(left_copy); yyjson_mut_doc_free(right_copy); @@ -1622,6 +1637,10 @@ static bool application_index_args_equal(const char *left, const char *right) { return equal; } +bool cbm_daemon_application_index_args_equal_for_test(const char *left, const char *right) { + return application_index_args_equal(left, right); +} + /* Caller holds application->mutex. Keeping watcher ownership validation and * this admission in the same critical section closes the unwatch race. */ static cbm_daemon_application_job_t *application_job_subscribe_locked( @@ -2369,10 +2388,9 @@ static cbm_daemon_runtime_application_status_t application_set_context( } char canonical_root[APPLICATION_PATH_CAP] = {0}; char canonical_allowed[APPLICATION_PATH_CAP] = {0}; - bool canonical = application_canonical_root(root, canonical_root, sizeof(canonical_root)); + bool canonical = cbm_canonical_path(root, canonical_root, sizeof(canonical_root)); if (canonical && allowed_present) { - canonical = - application_canonical_root(allowed, canonical_allowed, sizeof(canonical_allowed)); + canonical = cbm_canonical_path(allowed, canonical_allowed, sizeof(canonical_allowed)); } canonical = canonical && application_canonical_directory_exists(canonical_root); bool set = @@ -3387,7 +3405,7 @@ static int application_background_index(cbm_daemon_application_t *application, return -1; } char canonical_root[APPLICATION_PATH_CAP]; - if (!application_canonical_root(root_path, canonical_root, sizeof(canonical_root)) || + if (!cbm_canonical_path(root_path, canonical_root, sizeof(canonical_root)) || !application_canonical_directory_exists(canonical_root)) { return -1; } diff --git a/src/daemon/application_internal.h b/src/daemon/application_internal.h index 04f24421a..bd06e4efe 100644 --- a/src/daemon/application_internal.h +++ b/src/daemon/application_internal.h @@ -45,4 +45,10 @@ int cbm_daemon_application_busy_queue_waits_for_test(void); char *cbm_daemon_application_framable_response_for_test(char *response, const cbm_jsonrpc_request_t *request); +/* The job registry's index-argument equality: the running job's args against + * a request's. Exposed because the two repo_path spellings it folds - the + * session policy's native root and a handler's forward-slash one - only differ + * on Windows, while the fold itself runs on every platform. */ +bool cbm_daemon_application_index_args_equal_for_test(const char *left, const char *right); + #endif /* CBM_DAEMON_APPLICATION_INTERNAL_H */ diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 14da18e75..3259221c4 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -1765,18 +1765,7 @@ bool cbm_mcp_server_set_session_context(cbm_mcp_server_t *srv, const char *sessi return false; } - /* Keep both roots in the one spelling every tool handler produces. Each - * handler normalizes separators after canonicalizing, so a root stored in - * the platform's native form (backslashes on Windows) is a second name for - * the same directory - and the daemon compares roots exactly. An explicit - * index_repository request for this root was refused as an options - * conflict instead of joining the auto-index job already running for it, - * because the job's repo_path never matched the request's. */ - char root[sizeof(srv->session_root)]; - snprintf(root, sizeof(root), "%s", session_root); - cbm_normalize_path_sep(root); - - char *project = cbm_project_name_from_path(root); + char *project = cbm_project_name_from_path(session_root); if (!project || project[0] == '\0' || strlen(project) >= sizeof(srv->session_project)) { free(project); return false; @@ -1787,9 +1776,8 @@ bool cbm_mcp_server_set_session_context(cbm_mcp_server_t *srv, const char *sessi free(project); return false; } - cbm_normalize_path_sep(allowed_copy); - snprintf(srv->session_root, sizeof(srv->session_root), "%s", root); + snprintf(srv->session_root, sizeof(srv->session_root), "%s", session_root); snprintf(srv->session_project, sizeof(srv->session_project), "%s", project); free(project); diff --git a/tests/test_daemon.c b/tests/test_daemon.c index ff0bac8cf..04aece649 100644 --- a/tests/test_daemon.c +++ b/tests/test_daemon.c @@ -467,21 +467,21 @@ TEST(daemon_sessions_keep_distinct_roots_and_allowed_root_policy) { PASS(); } -/* The daemon canonicalizes a client's root through the platform API, which on - * Windows answers in backslash form, while every tool handler normalizes the - * separators of the paths it canonicalizes. Stored in its native form, one - * directory had two names, and an explicit index_repository request was - * refused as an options conflict instead of joining the auto-index job already - * running for that root. Separators fold on every platform, so the check binds - * wherever the suite runs. */ -TEST(daemon_session_context_keeps_one_spelling_of_a_root) { +/* The session policy keeps a root in the spelling the daemon canonicalized it + * to - the platform's native form, backslashes on Windows. The sensitive-root + * and allowed-root containment checks compare that spelling byte-exact against + * HOME and the granted roots, so a root respelled with forward slashes on the + * way in stopped matching them, and $HOME was admitted for auto-index and + * watch on Windows. One directory being one root for the job registry is + * folded at that comparison, never by respelling the policy. */ +TEST(daemon_session_context_keeps_the_policy_spelling_of_a_root) { cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); ASSERT_NOT_NULL(srv); cbm_mcp_server_set_background_tasks(srv, false); ASSERT_TRUE(cbm_mcp_server_set_session_context(srv, "C:\\repos\\cbm", "C:\\repos")); - ASSERT_STR_EQ(cbm_mcp_server_session_root(srv), "C:/repos/cbm"); - ASSERT_STR_EQ(cbm_mcp_server_allowed_root(srv), "C:/repos"); + ASSERT_STR_EQ(cbm_mcp_server_session_root(srv), "C:\\repos\\cbm"); + ASSERT_STR_EQ(cbm_mcp_server_allowed_root(srv), "C:\\repos"); cbm_mcp_server_free(srv); PASS(); @@ -500,5 +500,5 @@ SUITE(daemon) { RUN_TEST(daemon_bridge_rejects_embedded_nul_body); RUN_TEST(daemon_bridge_rejects_oversized_headers); RUN_TEST(daemon_sessions_keep_distinct_roots_and_allowed_root_policy); - RUN_TEST(daemon_session_context_keeps_one_spelling_of_a_root); + RUN_TEST(daemon_session_context_keeps_the_policy_spelling_of_a_root); } diff --git a/tests/test_daemon_application.c b/tests/test_daemon_application.c index fceeb7130..cea698fda 100644 --- a/tests/test_daemon_application.c +++ b/tests/test_daemon_application.c @@ -5382,6 +5382,30 @@ TEST(daemon_application_oversized_reply_is_a_jsonrpc_error_not_a_death) { PASS(); } +/* One directory is one root for the job registry. The auto-index job spells + * repo_path the way the session policy holds it - the platform's native form, + * backslashes on Windows - while an explicit index_repository request arrives + * in the handler's forward-slash spelling. Compared byte-exact the two never + * matched on Windows, and the request was refused as an options conflict + * instead of joining the job already running for its root. The fold runs on + * every platform, so this binds wherever the suite runs; every other option + * stays exact. */ +TEST(daemon_application_index_args_compare_repo_path_separator_equivalently) { + ASSERT_TRUE(cbm_daemon_application_index_args_equal_for_test( + "{\"repo_path\":\"C:\\\\repos\\\\cbm\"}", "{\"repo_path\":\"C:/repos/cbm\"}")); + ASSERT_TRUE(cbm_daemon_application_index_args_equal_for_test( + "{\"repo_path\":\"C:\\\\repos\\\\cbm\",\"mode\":\"full\"}", + "{\"mode\":\"full\",\"repo_path\":\"C:/repos/cbm\"}")); + ASSERT_FALSE(cbm_daemon_application_index_args_equal_for_test( + "{\"repo_path\":\"C:\\\\repos\\\\cbm\"}", "{\"repo_path\":\"C:/repos/cbm2\"}")); + ASSERT_FALSE(cbm_daemon_application_index_args_equal_for_test( + "{\"repo_path\":\"C:\\\\repos\\\\cbm\"}", "{\"repo_path\":\"C:/repos/cbm/sub\"}")); + ASSERT_FALSE(cbm_daemon_application_index_args_equal_for_test( + "{\"repo_path\":\"C:\\\\repos\\\\cbm\",\"mode\":\"incremental\"}", + "{\"repo_path\":\"C:/repos/cbm\"}")); + PASS(); +} + SUITE(daemon_application) { RUN_TEST(daemon_application_oversized_reply_is_a_jsonrpc_error_not_a_death); RUN_TEST(daemon_application_new_session_does_not_retain_initial_store); @@ -5400,6 +5424,7 @@ SUITE(daemon_application) { RUN_TEST(daemon_application_initialize_coalesces_auto_index_for_full_sessions); RUN_TEST(daemon_application_sensitive_root_blocks_auto_index_but_preserves_controls); RUN_TEST(daemon_application_sensitive_root_blocks_watch_but_preserves_controls); + RUN_TEST(daemon_application_index_args_compare_repo_path_separator_equivalently); RUN_TEST(daemon_application_auto_index_honors_tracked_file_limit); RUN_TEST(daemon_application_auto_index_file_count_handles_literal_metacharacter_path); RUN_TEST(daemon_application_auto_index_file_count_supports_non_git_roots);