From 85532247fa96b529e5766a65038008320e85fe48 Mon Sep 17 00:00:00 2001 From: Ulises Millan Guerrero Date: Fri, 21 Aug 2026 13:50:35 -0600 Subject: [PATCH 1/2] fix(workspace): treat '/' and '\' as the same separator in root matching The Windows sensitive-root escape hatch broke on separator spelling: the grant is recorded from cbm_canonical_path (backslashes) while the indexer hands the candidate normalized to '/'. ws_is_ancestor_or_equal compared bytes, so an approved 'Program Files' root never matched the candidate and --approve-sensitive could not lift the refusal. Compare component boundaries with separator equivalence, consistent with ws_is_sep used everywhere else in this module. The containment rule (a separator byte is never part of a component name) is unchanged. Fixes #1718 Signed-off-by: Ulises Millan Guerrero --- src/foundation/workspace.c | 20 +++++++-- tests/test_workspace.c | 91 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 3 deletions(-) diff --git a/src/foundation/workspace.c b/src/foundation/workspace.c index a85e17822..8c6b34b8d 100644 --- a/src/foundation/workspace.c +++ b/src/foundation/workspace.c @@ -212,7 +212,16 @@ static bool ws_any_component_matches(const char *path, const char *const *names, } /* True when b is a or lives under a. Compares on a separator boundary so - * "/a/bc" is not treated as living under "/a/b". */ + * "/a/bc" is not treated as living under "/a/b". + * + * '/' and '\' are equivalent here, as they are everywhere else in this module + * (ws_is_sep). The Windows sensitive-grant escape hatch (#1718) records the + * grant from cbm_canonical_path — backslashes — while the indexer candidate + * arrives normalized to '/'. A byte-exact comparison made the grant never + * match the candidate on Windows, so --approve-sensitive could not lift a + * "Program Files" refusal. Comparing on the separator-equivalence boundary + * keeps the containment semantics (a separator byte is never "part of a + * component name") while erasing the platform spelling difference. */ static bool ws_is_ancestor_or_equal(const char *a, const char *b) { if (!a || !b || !a[0] || !b[0]) { return false; @@ -221,8 +230,13 @@ static bool ws_is_ancestor_or_equal(const char *a, const char *b) { while (la > 1 && ws_is_sep(a[la - 1])) { la--; } - if (strncmp(a, b, la) != 0) { - return false; + for (size_t i = 0; i < la; i++) { + if (a[i] == b[i]) { + continue; + } + if (!(ws_is_sep(a[i]) && ws_is_sep(b[i]))) { + return false; + } } return b[la] == '\0' || ws_is_sep(b[la]); } diff --git a/tests/test_workspace.c b/tests/test_workspace.c index 736b4477f..89af1d215 100644 --- a/tests/test_workspace.c +++ b/tests/test_workspace.c @@ -261,6 +261,96 @@ TEST(ws_manifest_approval_refuses_overbroad_requests) { PASS(); } +/* #1718: the sensitive-root escape hatch broke on Windows separator spelling. + * The grant is recorded from cbm_canonical_path — backslashes — while the + * indexer hands the candidate already normalized to '/'. A byte-exact + * comparison made the grant never match the candidate, so an approved + * "Program Files" root stayed refused. '/' and '\' are equivalent everywhere + * else in this module (ws_is_sep); the boundary comparison must treat them + * equally. The classify_root home rule uses the same ws_paths_equal, so a + * mixed-separator home spelling is the string-level regression check; the + * FS-backed grant round-trip lives in the _WIN32 half, where the real + * canonicalization exercises it. */ +TEST(ws_approve_sensitive_matches_across_separator_styles) { + /* canonical spelled with '\' vs home spelled with '/' — must classify as + * the same directory. Pre-fix: strncmp mismatch at the separator byte, so + * the home rule never matched and an ordinary path under "C:\Users" fell + * through to CBM_WS_ALLOW. */ + ASSERT_EQ(cbm_workspace_classify_root("C:\\Users\\dev", "C:/Users/dev", CACHE), + CBM_WS_DENY_SENSITIVE); + ASSERT_EQ(cbm_workspace_classify_root("C:/Users/dev", "C:\\Users\\dev", CACHE), + CBM_WS_DENY_SENSITIVE); + /* A sibling that differs after the same separator boundary is still a + * different directory: "/a/bc" must never look like "/a/b". */ + ASSERT_EQ(cbm_workspace_classify_root("C:\\Users\\devx", "C:/Users/dev", CACHE), CBM_WS_ALLOW); +#ifdef _WIN32 + /* Full grant round-trip on real filesystem paths: the grant stored with + * '\' must lift the exact candidate the indexer passes with '/'. The + * sensitive class is exercised by a credential-named directory (".ssh"), + * which is DENY_SENSITIVE at any depth — equivalent to "Program Files" + * for the refusal path under test. */ + char *cache = th_mktempdir("cbm_ws_sw"); + ASSERT(cache != NULL); + char dir[1024]; + snprintf(dir, sizeof(dir), "%s/.ssh/cfg", cache); + ASSERT_TRUE(cbm_mkdir_p(dir, 0700)); + char path[1024]; + snprintf(path, sizeof(path), "%s/allowed_roots", cache); + + char grant[2048]; + snprintf(grant, sizeof(grant), "!%s\\.ssh\\cfg\n", cache); + FILE *f = fopen(path, "w"); + ASSERT_NOT_NULL(f); + (void)fprintf(f, "%s", grant); + (void)fclose(f); + + char candidate[2048]; + snprintf(candidate, sizeof(candidate), "%s/.ssh/cfg", cache); + /* Convert to '/' spelling, exactly as cbm_normalize_path_sep does for the + * indexer's candidate. */ + char *p; + for (p = candidate; *p; p++) { + if (*p == '\\') { + *p = '/'; + } + } + char err[1024]; + err[0] = '\0'; + ASSERT_TRUE(cbm_workspace_root_allowed(candidate, NULL, cache, NULL, err, sizeof(err))); + + /* An unapproved sibling under the same sensitive tree stays refused. It + * lies outside every granted root, so the refusal is the boundary wording + * (kept verbatim by contract), not the sensitive hint. */ + char sibling[2048]; + snprintf(sibling, sizeof(sibling), "%s/.ssh/keys", cache); + for (p = sibling; *p; p++) { + if (*p == '\\') { + *p = '/'; + } + } + err[0] = '\0'; + ASSERT_FALSE(cbm_workspace_root_allowed(sibling, NULL, cache, NULL, err, sizeof(err))); + ASSERT_TRUE(strstr(err, "outside the allowed root") != NULL); + + /* A strict descendant INSIDE the approved sensitive grant is contained + * (boundary passes) but not the exact grant, so the sensitive refusal + * applies and the hint names the flag. */ + char descendant[2048]; + snprintf(descendant, sizeof(descendant), "%s/.ssh/cfg/sub", cache); + for (p = descendant; *p; p++) { + if (*p == '\\') { + *p = '/'; + } + } + err[0] = '\0'; + ASSERT_FALSE(cbm_workspace_root_allowed(descendant, NULL, cache, NULL, err, sizeof(err))); + ASSERT_TRUE(strstr(err, "--approve-sensitive") != NULL); + + th_cleanup(cache); +#endif + PASS(); +} + SUITE(workspace) { RUN_TEST(ws_manifest_absent_is_not_an_error); RUN_TEST(ws_manifest_parses_entries_and_skips_comments); @@ -278,4 +368,5 @@ SUITE(workspace) { RUN_TEST(ws_posix_matching_is_case_sensitive); RUN_TEST(ws_null_context_disables_dependent_checks); RUN_TEST(ws_every_verdict_has_a_reason); + RUN_TEST(ws_approve_sensitive_matches_across_separator_styles); } From 0286569e635e045963d03bbdef8aeb6af2ad31bb Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 20 Oct 2026 06:05:10 +0200 Subject: [PATCH 2/2] test(workspace): create the descendant on disk; write the grant via cbm_fopen The Windows half of ws_approve_sensitive_matches_across_separator_styles asserted the sensitive-root hint for /.ssh/cfg/sub without creating that directory. Containment canonicalizes the candidate, which on Windows opens it (CreateFileW, OPEN_EXISTING), so a missing descendant was reported as outside the allowed root and the assertion failed on the Windows shard. Create it the way its parent is created. The grant file is now written through cbm_fopen, the wrapper every repository or user path goes through, in binary mode. Signed-off-by: Martin Vogel --- tests/test_workspace.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_workspace.c b/tests/test_workspace.c index b45b28ec1..cba81e76a 100644 --- a/tests/test_workspace.c +++ b/tests/test_workspace.c @@ -452,7 +452,7 @@ TEST(ws_approve_sensitive_matches_across_separator_styles) { char grant[2048]; snprintf(grant, sizeof(grant), "!%s\\.ssh\\cfg\n", cache); - FILE *f = fopen(path, "w"); + FILE *f = cbm_fopen(path, "wb"); ASSERT_NOT_NULL(f); (void)fprintf(f, "%s", grant); (void)fclose(f); @@ -495,6 +495,10 @@ TEST(ws_approve_sensitive_matches_across_separator_styles) { *p = '/'; } } + /* Containment canonicalizes the candidate, which on Windows opens it + * (CreateFileW, OPEN_EXISTING): a descendant that is not on disk is + * "outside the allowed root" instead of reaching the sensitive hint. */ + ASSERT_TRUE(cbm_mkdir_p(descendant, 0700)); err[0] = '\0'; ASSERT_FALSE(cbm_workspace_root_allowed(descendant, NULL, cache, NULL, err, sizeof(err))); ASSERT_TRUE(strstr(err, "--approve-sensitive") != NULL);