fix(workspace): compare the home directory in its canonical form

cbm_workspace_home_dir() returned the raw HOME (or USERPROFILE) string,
while cbm_workspace_classify_root() compares it against a canonical,
link-resolved path. Where the home directory is reached through a link
(/home kept on another volume, a macOS firmlink, a HOME that is itself a
link) the two spellings differ and the home rule never fired, so the home
directory was not classified as the home directory.

The helper now resolves the value through cbm_canonical_path, the same
resolver every caller applies to the path it classifies, and falls back
to the raw value when the directory cannot be resolved. The result lives
in a per-thread buffer recomputed on each call, as cbm_resolve_cache_dir
does, so a changed environment is seen at once. The environment is read
with cbm_safe_getenv so a non-ASCII USERPROFILE arrives as UTF-8.

The ancestor comparison now treats "/" and "\" as the same separator: on
Windows the resolver returns backslashes while some callers normalize to
forward slashes, and both spell one directory.

Two tests cover the helper: one with a plain temp directory (already a
firmlink on macOS), one with a home reached through a symbolic link
(POSIX; creating a link needs a privilege on Windows). The existing
explicit-approval test in the mcp suite now presents its session root in
resolved form, as the daemon does; with the raw spelling it would have
passed as an ordinary root and stopped exercising the approval path.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
Martin Vogel
2026-10-02 05:34:41 +02:00
parent cb7d937638
commit 2e6dc0877c
3 changed files with 142 additions and 9 deletions
+44 -7
View File
@@ -6,6 +6,7 @@
#include "foundation/compat.h"
#include "foundation/compat_fs.h"
#include "foundation/constants.h"
#include "foundation/platform.h"
#include "foundation/sha256.h"
@@ -239,6 +240,22 @@ static bool ws_is_windows_user_programs_tree(const char *path) {
return true;
}
/* strncmp that treats "/" and "\" as the same character. On Windows a
* canonical path arrives with backslashes from the resolver and with forward
* slashes from callers that normalize, and both spell one directory. Stops at
* the end of either string like strncmp does. */
static bool ws_prefix_equal(const char *a, const char *b, size_t len) {
for (size_t i = 0; i < len; i++) {
if (a[i] != b[i] && !(ws_is_sep(a[i]) && ws_is_sep(b[i]))) {
return false;
}
if (a[i] == '\0') {
break;
}
}
return true;
}
/* 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". */
static bool ws_is_ancestor_or_equal(const char *a, const char *b) {
@@ -249,7 +266,7 @@ 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) {
if (!ws_prefix_equal(a, b, la)) {
return false;
}
return b[la] == '\0' || ws_is_sep(b[la]);
@@ -600,14 +617,34 @@ bool cbm_workspace_root_allowed(const char *canonical_path, const char *home_dir
/* ── Environment helpers ──────────────────────────────────────────────────── */
/* Callers should not each re-derive these; a caller that resolved the home
* directory differently would classify the same path differently. */
* directory differently would classify the same path differently.
*
* The value is the CANONICAL form of $HOME (then %USERPROFILE%), through the
* same resolver callers apply to the path they classify. The policy compares
* the two byte for byte, and a home reached through a link — "/home" kept on
* another volume, a macOS firmlink, a HOME that is itself a link — differs
* from its resolved form, so the raw value would never match and the home
* rule would not fire. A home that cannot be resolved (it does not exist)
* falls back to the raw value, which is all the comparison could use anyway.
*
* Storage is per thread and recomputed on every call, as cbm_resolve_cache_dir
* does: no lock, a changed environment is seen at once, and the pointer stays
* valid until the next call on the same thread. */
const char *cbm_workspace_home_dir(void) {
const char *home = getenv("HOME");
if (home && home[0]) {
return home;
static CBM_TLS char resolved[CBM_SZ_4K];
char raw[CBM_SZ_4K];
const char *home = cbm_safe_getenv("HOME", raw, sizeof(raw), NULL);
if (!home || !home[0]) {
home = cbm_safe_getenv("USERPROFILE", raw, sizeof(raw), NULL);
}
home = getenv("USERPROFILE");
return (home && home[0]) ? home : NULL;
if (!home || !home[0]) {
return NULL;
}
if (!cbm_canonical_path(home, resolved, sizeof(resolved))) {
memcpy(resolved, raw, strlen(raw) + 1);
}
cbm_normalize_path_sep(resolved);
return resolved;
}
const char *cbm_workspace_cache_dir(void) {
+8 -2
View File
@@ -1952,8 +1952,14 @@ TEST(mcp_issue403_sensitive_root_stops_before_discovery_count) {
}
TEST(mcp_issue403_explicit_approval_preserves_auto_index) {
char *sensitive_home = th_mktempdir("cbm_mcp_403_home");
ASSERT_NOT_NULL(sensitive_home);
char *created = th_mktempdir("cbm_mcp_403_home");
ASSERT_NOT_NULL(created);
/* The session root is presented in its resolved form, as the daemon does,
* because the home helper hands out the resolved home: with the raw
* spelling the two differ on macOS (/tmp is a firmlink) and the root
* would pass as an ordinary one, never reaching the approval path. */
char sensitive_home[4096];
ASSERT_TRUE(cbm_canonical_path(created, sensitive_home, sizeof(sensitive_home)));
const char *saved_home = getenv("HOME");
char *saved_home_copy = saved_home ? strdup(saved_home) : NULL;
cbm_setenv("HOME", sensitive_home, 1);
+90
View File
@@ -9,8 +9,13 @@
#include "test_helpers.h"
#include "foundation/workspace.h"
#include "foundation/compat_fs.h"
#include "foundation/platform.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#ifndef _WIN32
#include <unistd.h>
#endif
static const char *HOME = "/Users/dev";
static const char *CACHE = "/Users/dev/.cache/codebase-memory-mcp";
@@ -375,6 +380,89 @@ TEST(ws_manifest_approval_refuses_overbroad_requests) {
PASS();
}
/* ── Environment helpers ────────────────────────────────────────────────── */
static char *ws_env_save(const char *name) {
const char *value = getenv(name);
return value ? strdup(value) : NULL;
}
static void ws_env_restore(const char *name, char *saved) {
if (saved) {
(void)cbm_setenv(name, saved, 1);
free(saved);
} else {
(void)cbm_unsetenv(name);
}
}
/* The policy compares the home directory against canonical paths, so the
* helper must hand out the canonical form. A plain temp directory already
* shows the difference: on macOS /tmp is a firmlink to /private/tmp, and on
* Windows the temp path may carry a short (8.3) component. */
TEST(ws_home_dir_is_resolved) {
char *created = th_mktempdir("cbm_ws_home");
ASSERT_NOT_NULL(created);
char real[256];
snprintf(real, sizeof(real), "%s", created);
char expected[4096];
ASSERT_TRUE(cbm_canonical_path(real, expected, sizeof(expected)));
cbm_normalize_path_sep(expected);
char *saved_home = ws_env_save("HOME");
ASSERT_EQ(cbm_setenv("HOME", real, 1), 0);
const char *home = cbm_workspace_home_dir();
bool resolved = home && strcmp(home, expected) == 0;
ws_env_restore("HOME", saved_home);
th_cleanup(real);
ASSERT_TRUE(resolved);
PASS();
}
/* A home directory reached through a link — "/home" kept on another volume,
* an account whose HOME is itself a link — is still the home directory when
* a caller presents its resolved path, and must be refused as such. */
TEST(ws_linked_home_classified_as_home) {
#ifdef _WIN32
/* symlink() does not exist on Windows, and creating a symbolic link there
* needs a privilege an ordinary account (and the CI runner) does not hold.
* A directory junction (cmd.exe mklink /J) is the Windows shape of the
* same case and would need a cmd.exe fixture; the resolution the case
* depends on is covered on Windows by ws_home_dir_is_resolved, and the
* link itself is exercised on POSIX. */
SKIP_PLATFORM("Windows: symlink() unavailable; links need a privilege");
#else
char *created = th_mktempdir("cbm_ws_linked_home");
ASSERT_NOT_NULL(created);
char base[256];
snprintf(base, sizeof(base), "%s", created);
char real[512];
char link[512];
snprintf(real, sizeof(real), "%s/real", base);
snprintf(link, sizeof(link), "%s/link", base);
ASSERT_EQ(cbm_mkdir(real), 0);
ASSERT_EQ(symlink(real, link), 0);
char canonical_real[4096];
ASSERT_TRUE(cbm_canonical_path(real, canonical_real, sizeof(canonical_real)));
/* The fixture proves something only when the two spellings differ. */
ASSERT_TRUE(strcmp(link, canonical_real) != 0);
char *saved_home = ws_env_save("HOME");
ASSERT_EQ(cbm_setenv("HOME", link, 1), 0);
const char *home = cbm_workspace_home_dir();
bool resolved = home && strcmp(home, canonical_real) == 0;
cbm_ws_verdict_t verdict = cbm_workspace_classify_root(canonical_real, home, NULL);
ws_env_restore("HOME", saved_home);
th_cleanup(base);
ASSERT_TRUE(resolved);
ASSERT_EQ(verdict, CBM_WS_DENY_SENSITIVE);
PASS();
#endif
}
SUITE(workspace) {
RUN_TEST(ws_manifest_absent_is_not_an_error);
RUN_TEST(ws_manifest_parses_entries_and_skips_comments);
@@ -396,4 +484,6 @@ 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_home_dir_is_resolved);
RUN_TEST(ws_linked_home_classified_as_home);
}