diff --git a/README.md b/README.md index 26526eff..926d13d7 100644 --- a/README.md +++ b/README.md @@ -670,7 +670,7 @@ codebase-memory-mcp config reset auto_index # reset to default | Variable | Default | Description | |----------|---------|-------------| -| `CBM_ALLOWED_ROOT` | *(unset)* | Restrict `index_repository` to paths within this directory. When set, a `repo_path` that resolves (after symlink / `..` resolution) outside this root is refused; unset imposes no restriction. Useful when the server may be driven by an untrusted caller, e.g. agentic or multi-tenant deployments. | +| `CBM_ALLOWED_ROOT` | *(unset)* | Confine `index_repository` to paths within this directory. When set, a `repo_path` that resolves (after symlink / `..` resolution) outside this root is refused, and the same check now applies to the graph UI's `POST /api/index` route rather than only to the MCP tool. Unset imposes no *containment* restriction — but see the always-on limits below, which apply whether or not this is set. Useful when the server may be driven by an untrusted caller, e.g. agentic or multi-tenant deployments. | | `CBM_CACHE_DIR` | `~/.cache/codebase-memory-mcp` | Override the database storage directory. All project indexes and config are stored here. One account can use only one canonical cache root at a time; close active CBM sessions/commands before switching it. | | `CBM_DIAGNOSTICS` | `false` | Set to `1` or `true` to enable the shared daemon's periodic `snapshot.json` and retained `trajectory.ndjson` below a fresh owner-private directory in the system temp directory. Exact paths are logged by `diagnostics.start`. | | `CBM_DOWNLOAD_URL` | *(GitHub releases)* | Override the download URL for updates. Used for testing or self-hosted deployments. | diff --git a/SECURITY.md b/SECURITY.md index d5153f62..d00b3c55 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -172,8 +172,8 @@ sha256sum -c checksums.txt | Version | Supported | |---------|-----------| -| Latest `0.8.x` | Yes — security fixes land in the newest release | -| < 0.8 | No — please upgrade to the latest release | +| Latest `0.9.x` | Yes — security fixes land in the newest release | +| < 0.9 | No — please upgrade to the latest release | Only the latest release is supported. Security fixes are shipped in a new patched release rather than backported to older versions; upgrading to the diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index e8dc608c..e0a58953 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -116,7 +116,7 @@ These environment variables affect runtime behavior: | Variable | Default | Description | |---|---|---| -| `CBM_ALLOWED_ROOT` | *(unset)* | Restrict `index_repository` to paths within this directory. When set, a `repo_path` that resolves (after symlink / `..` resolution) outside this root is refused; unset imposes no restriction. Useful when the server may be driven by an untrusted caller (agentic or multi-tenant deployments). | +| `CBM_ALLOWED_ROOT` | *(unset)* | Confine `index_repository` to paths within this directory. When set, a `repo_path` that resolves (after symlink / `..` resolution) outside this root is refused, and the same check now applies to the graph UI's `POST /api/index` route rather than only to the MCP tool. Unset imposes no *containment* restriction — but see the always-on limits below, which apply whether or not this is set. Useful when the server may be driven by an untrusted caller, e.g. agentic or multi-tenant deployments. | | `CBM_CACHE_DIR` | `~/.cache/codebase-memory-mcp` | Override the cache directory used for indexes, `_config.db`, and UI `config.json`. | | `CBM_DIAGNOSTICS` | `false` | Enable periodic `snapshot.json` and retained `trajectory.ndjson` below a fresh owner-private directory in the system temp directory. The daemon records the randomized paths in the `diagnostics.start` discovery record (a single JSON line) in `${CBM_CACHE_DIR}/logs/cbm-daemon.log`; that one record is emitted even when `CBM_LOG_LEVEL` suppresses ordinary logging, so the paths always remain discoverable. | | `CBM_DOWNLOAD_URL` | GitHub releases | Override the update download URL. | @@ -125,6 +125,25 @@ These environment variables affect runtime behavior: Environment used by daemon-owned components—such as diagnostics, daemon logging, and process-wide indexing resource limits—is captured from the first daemon-backed session that starts the daemon. Later sessions join the existing process and cannot replace those values. To change them, close every daemon-backed session, update the relevant agent configurations consistently, and restart a session. `CBM_ALLOWED_ROOT` remains session-specific, a conflicting `CBM_CACHE_DIR` is rejected, and one-shot CLI commands use their own current environment without starting the daemon. + +### Roots that are always refused + +Independently of `CBM_ALLOWED_ROOT`, some directories are refused as an indexing +root because they are too broad or too sensitive to index as a unit: + +- a filesystem root, a Windows drive root, or a UNC share root; +- a top-level system tree — `/etc`, `/var`, `/usr`, `/home`, `/Users`, and on + Windows `C:\Windows`, `C:\Users`, `C:\ProgramData`, `C:\Program Files`; +- your home directory itself (directories *below* it are fine); +- a credential directory at any depth — `.ssh`, `.aws`, `.gnupg`, `.kube`, + `.docker`, `.netrc`, `.git-credentials`, `.password-store`, macOS `Keychains`. + +Two limits are worth stating plainly. This constrains *scope*, not +*sensitivity*: inside a root that is allowed, every file the process can read may +be indexed and later returned. And the credential list is a denylist, so it +raises the cost of a mistake rather than closing the class — a directory it does +not name is permitted. + ## 5. Agent and Editor Integration Files The `install` command can also write MCP entries and instruction blocks into agent/editor config files such as Claude Code, Codex, Gemini, VS Code, Cursor, Zed, and others. diff --git a/src/foundation/workspace.c b/src/foundation/workspace.c index 0519c09e..12e3affc 100644 --- a/src/foundation/workspace.c +++ b/src/foundation/workspace.c @@ -4,6 +4,13 @@ */ #include "foundation/workspace.h" +#include "foundation/compat.h" +#include "foundation/compat_fs.h" +#include "foundation/platform.h" + +#include + +#include #include enum { @@ -72,6 +79,14 @@ int cbm_workspace_path_depth(const char *canonical_path) { } int depth = 0; const char *p = canonical_path + prefix; + /* macOS firmlinks /etc, /tmp and /var under /private, so canonicalizing any + * of them adds a component: "/etc" resolves to "/private/etc" and would count + * as two deep, sailing past a minimum of two — exactly the path the reports + * demonstrate. Treat a leading "private" as transparent so depth reflects the + * tree the user named. "/private/tmp/proj" still counts two and is allowed. */ + if (strncmp(p, "private", 7) == 0 && (ws_is_sep(p[7]) || p[7] == '\0')) { + p += 7; + } while (*p) { while (ws_is_sep(*p)) { p++; @@ -222,11 +237,19 @@ cbm_ws_verdict_t cbm_workspace_classify_root(const char *canonical_path, const c return CBM_WS_DENY_TOO_SHALLOW; } - /* Indexing a tree that holds the cache would pull every other project's - * graph database into this project's index. Never overridable. */ - if (cache_dir && cache_dir[0] && ws_is_ancestor_or_equal(canonical_path, cache_dir)) { - return CBM_WS_DENY_ABSOLUTE; - } + /* No rule here for "this root contains the cache directory". + * + * An earlier draft refused such roots outright, on the theory that indexing + * them would absorb every other project's graph database. Two things make + * that wrong. The indexer only parses recognised source files, and a graph + * database is binary SQLite it would never extract; and refusing an entire + * root is the wrong remedy even where the concern holds — the right one is to + * not walk the cache. Excluding the cache from discovery is tracked + * separately; classifying the root is not the place for it. + * + * cache_dir stays in the signature because the exclusion work will need it + * and because callers already have it to hand. */ + (void)cache_dir; if (ws_any_component_matches(canonical_path, WS_CREDENTIAL_NAMES, sizeof(WS_CREDENTIAL_NAMES) / sizeof(WS_CREDENTIAL_NAMES[0]), @@ -263,3 +286,242 @@ const char *cbm_workspace_verdict_reason(cbm_ws_verdict_t verdict) { bool cbm_workspace_verdict_is_overridable(cbm_ws_verdict_t verdict) { return verdict == CBM_WS_DENY_SENSITIVE; } + +/* ── Grant store ──────────────────────────────────────────────────────────── */ + +enum { WS_LINE_MAX = 4096 }; + +/* A line may carry a leading '!' meaning "a person explicitly approved this + * sensitive root". The marker is stored rather than inferred so re-classifying + * later (a new credential name is added to the list, say) cannot silently + * upgrade an ordinary grant into a sensitive one. */ +static const char WS_SENSITIVE_MARK = '!'; + +bool cbm_workspace_grant_path(const char *cache_dir, char *out, size_t out_sz) { + if (!cache_dir || !cache_dir[0] || !out || out_sz == 0) { + return false; + } + int n = snprintf(out, out_sz, "%s/allowed_roots", cache_dir); + return n > 0 && (size_t)n < out_sz; +} + +/* Walk the grant file, invoking visit for each entry. visit returns true to stop. + * Returns the number of entries seen. */ +static int ws_grant_walk(const char *cache_dir, + bool (*visit)(const char *root, bool sensitive, void *ctx), void *ctx) { + char store[WS_LINE_MAX]; + if (!cbm_workspace_grant_path(cache_dir, store, sizeof(store))) { + return 0; + } + FILE *f = cbm_fopen(store, "r"); + if (!f) { + return 0; + } + char line[WS_LINE_MAX]; + int seen = 0; + while (fgets(line, (int)sizeof(line), f)) { + size_t len = strlen(line); + while (len > 0 && (line[len - 1] == '\n' || line[len - 1] == '\r')) { + line[--len] = '\0'; + } + if (len == 0 || line[0] == '#') { + continue; + } + bool sensitive = line[0] == WS_SENSITIVE_MARK; + const char *root = sensitive ? line + 1 : line; + if (!root[0]) { + continue; + } + seen++; + if (visit && visit(root, sensitive, ctx)) { + break; + } + } + (void)fclose(f); + return seen; +} + +typedef struct { + const char *candidate; + bool contained; /* candidate is at or below some granted root */ + bool exact_sensitive; /* candidate is itself a grant marked sensitive */ +} ws_match_t; + +static bool ws_match_visit(const char *root, bool sensitive, void *ctx) { + ws_match_t *m = ctx; + if (cbm_path_within_root(root, m->candidate)) { + m->contained = true; + if (sensitive && ws_paths_equal(root, m->candidate)) { + m->exact_sensitive = true; + } + } + return false; /* visit every entry: a later one may be the exact match */ +} + +typedef struct { + char *out; + size_t out_sz; + size_t used; +} ws_list_t; + +static bool ws_list_visit(const char *root, bool sensitive, void *ctx) { + ws_list_t *l = ctx; + int n = snprintf(l->out + l->used, l->out_sz - l->used, "%s%s\n", + sensitive ? "(approved) " : "", root); + if (n > 0 && (size_t)n < l->out_sz - l->used) { + l->used += (size_t)n; + } + return false; +} + +bool cbm_workspace_grant_list(const char *cache_dir, char *out, size_t out_sz) { + if (!out || out_sz == 0) { + return false; + } + out[0] = '\0'; + ws_list_t l = {out, out_sz, 0}; + return ws_grant_walk(cache_dir, ws_list_visit, &l) > 0; +} + +bool cbm_workspace_grant_add(const char *cache_dir, const char *home_dir, + const char *canonical_path, bool approve_sensitive, char *err, + size_t err_sz) { + if (err && err_sz) { + err[0] = '\0'; + } + if (!cache_dir || !cache_dir[0] || !canonical_path || !canonical_path[0]) { + if (err) { + snprintf(err, err_sz, "no path given"); + } + return false; + } + + cbm_ws_verdict_t verdict = cbm_workspace_classify_root(canonical_path, home_dir, cache_dir); + if (verdict != CBM_WS_ALLOW) { + if (!cbm_workspace_verdict_is_overridable(verdict)) { + if (err) { + snprintf(err, err_sz, "%s", cbm_workspace_verdict_reason(verdict)); + } + return false; + } + if (!approve_sensitive) { + /* Refuse by default and name the flag, rather than asking a question + * whose answer we cannot authenticate. */ + if (err) { + snprintf(err, err_sz, "%s; re-run with --approve-sensitive if that is intended", + cbm_workspace_verdict_reason(verdict)); + } + return false; + } + } + + /* Already present is success, not a duplicate error. */ + ws_match_t existing = {canonical_path, false, false}; + (void)ws_grant_walk(cache_dir, ws_match_visit, &existing); + if (existing.contained) { + return true; + } + + char store[WS_LINE_MAX]; + if (!cbm_workspace_grant_path(cache_dir, store, sizeof(store))) { + if (err) { + snprintf(err, err_sz, "cache path too long"); + } + return false; + } + FILE *f = cbm_fopen(store, "a"); + if (!f) { + if (err) { + snprintf(err, err_sz, "cannot write %s", store); + } + return false; + } + bool sensitive = verdict == CBM_WS_DENY_SENSITIVE; + (void)fprintf(f, "%s%s\n", sensitive ? "!" : "", canonical_path); + bool ok = fclose(f) == 0; + if (!ok && err) { + snprintf(err, err_sz, "cannot write %s", store); + } + return ok; +} + +bool cbm_workspace_root_allowed(const char *canonical_path, const char *home_dir, + const char *cache_dir, const char *configured_root, char *err, + size_t err_sz) { + if (err && err_sz) { + err[0] = '\0'; + } + if (!canonical_path || !canonical_path[0]) { + if (err) { + snprintf(err, err_sz, "no repository path given"); + } + return false; + } + + ws_match_t match = {canonical_path, false, false}; + int grants = ws_grant_walk(cache_dir, ws_match_visit, &match); + + /* A configured root behaves as an additional grant so existing + * CBM_ALLOWED_ROOT deployments keep working unchanged. */ + bool configured = configured_root && configured_root[0]; + bool configured_contains = configured && cbm_path_within_root(configured_root, canonical_path); + + /* Containment first when a boundary has actually been declared. A path + * outside a configured root is best explained as exactly that, and the + * wording is what callers and the shell contracts already match on. Breadth + * then applies to paths that ARE inside the declared root but are still too + * broad to index as one unit. */ + bool boundary_declared = grants > 0 || configured; + if (boundary_declared && !match.contained && !configured_contains) { + if (err) { + /* Keep the "outside the allowed root" wording: changing it broke an + * assertion whose early return then leaked CBM_ALLOWED_ROOT into + * every later test in that suite. Guidance is appended, not + * substituted. */ + snprintf(err, err_sz, + "%s is outside the allowed root. To allow it, run: " + "codebase-memory-mcp allow-root %s", + canonical_path, canonical_path); + } + return false; + } + + cbm_ws_verdict_t verdict = cbm_workspace_classify_root(canonical_path, home_dir, cache_dir); + if (verdict == CBM_WS_ALLOW) { + return true; + } + /* An explicit human approval recorded for exactly this path is the only thing + * that lifts a sensitive refusal. Absolute and shallow refusals cannot be + * lifted at all. */ + if (verdict == CBM_WS_DENY_SENSITIVE && match.exact_sensitive) { + return true; + } + if (err) { + if (cbm_workspace_verdict_is_overridable(verdict)) { + snprintf(err, err_sz, + "%s: %s. To index it anyway, run: codebase-memory-mcp allow-root " + "--approve-sensitive %s", + canonical_path, cbm_workspace_verdict_reason(verdict), canonical_path); + } else { + snprintf(err, err_sz, "%s: %s", canonical_path, cbm_workspace_verdict_reason(verdict)); + } + } + return false; +} + +/* ── Environment helpers ──────────────────────────────────────────────────── */ + +/* Callers should not each re-derive these; a caller that resolved the home + * directory differently would classify the same path differently. */ +const char *cbm_workspace_home_dir(void) { + const char *home = getenv("HOME"); + if (home && home[0]) { + return home; + } + home = getenv("USERPROFILE"); + return (home && home[0]) ? home : NULL; +} + +const char *cbm_workspace_cache_dir(void) { + return cbm_resolve_cache_dir(); +} diff --git a/src/foundation/workspace.h b/src/foundation/workspace.h index 0d0511a9..ef1a2e5e 100644 --- a/src/foundation/workspace.h +++ b/src/foundation/workspace.h @@ -51,4 +51,57 @@ bool cbm_workspace_verdict_is_overridable(cbm_ws_verdict_t verdict); * 0 (the share root itself). Exposed for tests. */ int cbm_workspace_path_depth(const char *canonical_path); +/* Canonical containment check: is abs_path at or below root_path, after symlink + * and junction resolution? The definition currently lives in src/mcp/mcp.c; it is + * declared here so the MCP handler, the HTTP UI route and the CLI all reach the + * same implementation. Two entry points evaluating the same policy separately is + * what produced the UI/MCP divergence in the first place. */ +bool cbm_path_within_root(const char *root_path, const char *abs_path); + +/* + * ── Grant store ────────────────────────────────────────────────────────────── + * + * The authorization half. Grants live in a user-level file under the cache + * directory and are only ever written by an explicit human action (the CLI), so + * neither an indexed repository nor a tool caller can widen its own boundary. + * + * Enforcement is deliberately two-tier, because default-deny with an empty store + * would refuse every first run: + * + * - The breadth policy above is ALWAYS enforced. Out of the box that already + * refuses "/", "/etc", "$HOME", "~/.ssh" and friends — the paths the reports + * actually demonstrate — with no configuration at all. + * - Containment in a granted root is enforced ONLY once the store is non-empty + * or a root is configured. Declaring your first root is therefore what turns + * on true confinement, and it never silently loosens: adding a root can only + * narrow what was previously unrestricted. + */ + +/* Absolute path of the grant file for a cache directory. */ +bool cbm_workspace_grant_path(const char *cache_dir, char *out, size_t out_sz); + +/* Record canonical_path as an allowed root. approve_sensitive records an explicit + * human override for a CBM_WS_DENY_SENSITIVE path; absolute denials and shallow + * paths are never grantable. Returns false and fills err on refusal. */ +bool cbm_workspace_grant_add(const char *cache_dir, const char *home_dir, + const char *canonical_path, bool approve_sensitive, char *err, + size_t err_sz); + +/* Newline-separated granted roots into out. True when at least one exists. */ +bool cbm_workspace_grant_list(const char *cache_dir, char *out, size_t out_sz); + +/* The whole decision, used by every entry point that accepts a repo path. + * canonical_path must already be canonicalized. configured_root is the legacy + * CBM_ALLOWED_ROOT / session policy, treated as an additional grant, or NULL. + * On refusal, err receives a message that names the command which would fix it. */ +bool cbm_workspace_root_allowed(const char *canonical_path, const char *home_dir, + const char *cache_dir, const char *configured_root, char *err, + size_t err_sz); + +/* The home and cache directories the policy should be evaluated against. Shared + * so two callers cannot derive them differently and reach different verdicts for + * the same path. Either may return NULL. */ +const char *cbm_workspace_home_dir(void); +const char *cbm_workspace_cache_dir(void); + #endif /* CBM_FOUNDATION_WORKSPACE_H */ diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 7db4bc41..63dfa75c 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -64,6 +64,7 @@ enum { #include "mcp/index_supervisor.h" #include "mcp/compact_out.h" #include "foundation/str_util.h" +#include "foundation/workspace.h" #include "foundation/dump_verify.h" #include "foundation/compat_regex.h" #include "pipeline/artifact.h" @@ -7895,17 +7896,24 @@ static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) { repo_path = canonicalize_repo_path_if_exists(repo_path); - /* Optional workspace boundary. Embedded/daemon sessions always use their - * explicit policy, including an explicit NULL meaning unrestricted. A - * standalone server retains the process-wide CBM_ALLOWED_ROOT fallback. */ + /* Workspace boundary. Embedded/daemon sessions supply their explicit policy, + * including an explicit NULL meaning unrestricted; a standalone server falls + * back to the process-wide CBM_ALLOWED_ROOT. The decision itself lives in one + * shared function so this handler and the HTTP UI indexing route cannot drift + * apart — they had, and the divergence was the defect. */ const char *allowed_root = srv->allowed_root_policy_set ? srv->allowed_root : getenv("CBM_ALLOWED_ROOT"); - if (allowed_root && allowed_root[0] && repo_path && - !cbm_path_within_root(allowed_root, repo_path)) { + /* repo_path is legitimately absent when the caller names an already-known + * project instead; the root is resolved downstream. Only a path supplied here + * is classified here — the previous check had the same tolerance. */ + char boundary_err[CBM_SZ_1K]; + if (repo_path && repo_path[0] && + !cbm_workspace_root_allowed(repo_path, cbm_workspace_home_dir(), cbm_workspace_cache_dir(), + allowed_root, boundary_err, sizeof(boundary_err))) { free(mode_str); free(name_override); free(repo_path); - return cbm_mcp_text_result("repo_path is outside the allowed root", true); + return cbm_mcp_text_result(boundary_err, true); } if (mode_str && strcmp(mode_str, "cross-repo-intelligence") == 0) { diff --git a/src/ui/http_server.c b/src/ui/http_server.c index 5e734d0a..991a34cd 100644 --- a/src/ui/http_server.c +++ b/src/ui/http_server.c @@ -35,6 +35,7 @@ #include "foundation/compat_thread.h" #include "foundation/subprocess.h" /* cbm_build_win_cmdline — shared MS-CRT arg quoting */ #include "foundation/win_utf8.h" /* cbm_utf8_to_wide — CreateProcessW wide cmdline (#423/#20) */ +#include "foundation/workspace.h" #include #include @@ -1052,6 +1053,28 @@ static void handle_index_start(cbm_http_server_t *server, cbm_http_conn_t *c, return; } + /* Same workspace boundary the MCP indexing tool applies, through the same + * function. This route used to check only that the path was a directory, so + * 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 boundary_err[1024]; + if (!cbm_canonical_path(rpath, canonical_root, sizeof(canonical_root))) { + yyjson_doc_free(doc); + cbm_http_replyf(c, 400, g_cors_json, "{\"error\":\"cannot resolve root_path\"}"); + return; + } + if (!cbm_workspace_root_allowed(canonical_root, cbm_workspace_home_dir(), + cbm_workspace_cache_dir(), getenv("CBM_ALLOWED_ROOT"), + boundary_err, sizeof(boundary_err))) { + yyjson_doc_free(doc); + char escaped[1024]; + cbm_json_escape(escaped, (int)sizeof(escaped), boundary_err); + cbm_http_replyf(c, 403, g_cors_json, "{\"error\":\"%s\"}", escaped); + return; + } + /* Find free job slot */ int slot = -1; for (int i = 0; i < MAX_INDEX_JOBS; i++) { diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 85213468..ebe5732f 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -2940,12 +2940,11 @@ TEST(tool_trace_call_path_prefers_definition) { TEST(trace_evidence_strategy_class_vocabulary_is_closed) { /* Every strategy string assigned anywhere in src/ + internal/ as of this * commit, plus the two literals pass_calls.c writes directly. */ - static const char *const lsp[] = {"lsp_direct", "lsp_base_dispatch", - "lsp_embed_dispatch", "lsp_implicit_this", - "lsp_inherited_dispatch", "lsp_method_dispatch", - "lsp_proc_macro", "lsp_smart_ptr_dispatch", - "lsp_strategy_cross_file", "lsp_trait_dispatch", - "lsp_type_dispatch", "lsp_virtual_dispatch"}; + static const char *const lsp[] = { + "lsp_direct", "lsp_base_dispatch", "lsp_embed_dispatch", + "lsp_implicit_this", "lsp_inherited_dispatch", "lsp_method_dispatch", + "lsp_proc_macro", "lsp_smart_ptr_dispatch", "lsp_strategy_cross_file", + "lsp_trait_dispatch", "lsp_type_dispatch", "lsp_virtual_dispatch"}; for (size_t i = 0; i < sizeof(lsp) / sizeof(lsp[0]); i++) { const char *cls = cbm_mcp_edge_strategy_class(lsp[i]); ASSERT_NOT_NULL(cls); @@ -6076,8 +6075,8 @@ TEST(tool_index_repository_unknown_project_name_still_requires_repo_path) { cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); ASSERT_NOT_NULL(srv); - char *resp = cbm_mcp_handle_tool(srv, "index_repository", - "{\"project\":\"never-indexed-project\"}"); + char *resp = + cbm_mcp_handle_tool(srv, "index_repository", "{\"project\":\"never-indexed-project\"}"); ASSERT_NOT_NULL(resp); ASSERT_NOT_NULL(strstr(resp, "repo_path is required")); free(resp); @@ -6609,13 +6608,13 @@ TEST(detect_changes_seeds_only_touched_symbol_issue1363) { "def bar():\n" " y = 2\n" " return y\n"), - 0); + 0); /* `git -C` with double quotes, not `cd '' &&`: single quotes are not * quoting characters for cmd.exe, and identity/branch/signing come from -c * so the fixture does not depend on the machine's global git config. The * assertions below read `base: main`, so pin init.defaultBranch. */ -#define DC1363_GITCFG \ +#define DC1363_GITCFG \ "-c user.name=t -c user.email=t@t.io -c init.defaultBranch=main -c commit.gpgsign=false" char cmd[1200]; const char *steps[] = {"init -q", "add -A", "commit -q -m init"}; @@ -6644,7 +6643,7 @@ TEST(detect_changes_seeds_only_touched_symbol_issue1363) { "def bar():\n" " y = 2\n" " return y\n"), - 0); + 0); char *project = cbm_project_name_from_path(repo); ASSERT_NOT_NULL(project); @@ -6691,7 +6690,7 @@ TEST(detect_changes_zero_overlap_falls_back_issue1363) { " return 2\n"), 0); -#define DC1363B_GITCFG \ +#define DC1363B_GITCFG \ "-c user.name=t -c user.email=t@t.io -c init.defaultBranch=main -c commit.gpgsign=false" char cmd[1200]; const char *steps[] = {"init -q", "add -A", "commit -q -m init"}; @@ -10003,6 +10002,38 @@ TEST(detect_changes_rejects_windows_cmd_metacharacters_in_project_root) { #endif } +/* With no boundary configured at all, index_repository must still refuse roots + * that are too broad or too sensitive to index as a unit. This is the part that + * holds out of the box: the paths the advisories actually demonstrate are refused + * without anyone setting an environment variable first. */ +TEST(index_repository_refuses_overbroad_roots_by_default) { + const char *saved = getenv("CBM_ALLOWED_ROOT"); + char *saved_copy = saved ? strdup(saved) : NULL; + cbm_unsetenv("CBM_ALLOWED_ROOT"); + + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + + /* A top-level system tree: refused on breadth, with no configuration. */ + char *resp = cbm_mcp_handle_tool(srv, "index_repository", "{\"repo_path\":\"/etc\"}"); + ASSERT_NOT_NULL(resp); + ASSERT_TRUE(strstr(resp, "too broad") != NULL); + free(resp); + + /* The filesystem root is refused outright and is never overridable. */ + resp = cbm_mcp_handle_tool(srv, "index_repository", "{\"repo_path\":\"/\"}"); + ASSERT_NOT_NULL(resp); + ASSERT_TRUE(strstr(resp, "cannot be indexed") != NULL); + free(resp); + + cbm_mcp_server_free(srv); + if (saved_copy) { + cbm_setenv("CBM_ALLOWED_ROOT", saved_copy, 1); + free(saved_copy); + } + PASS(); +} + /* Opt-in workspace boundary: when CBM_ALLOWED_ROOT is set, index_repository * must refuse a repo_path that resolves outside it. Unset (the default) imposes * no restriction. */ @@ -10292,6 +10323,7 @@ SUITE(mcp) { RUN_TEST(detect_changes_rejects_option_like_base_branch); RUN_TEST(detect_changes_rejects_windows_cmd_metacharacters_in_base_branch); RUN_TEST(detect_changes_rejects_windows_cmd_metacharacters_in_project_root); + RUN_TEST(index_repository_refuses_overbroad_roots_by_default); RUN_TEST(index_repository_honors_allowed_root); /* JSON-RPC parsing */ RUN_TEST(jsonrpc_parse_request); diff --git a/tests/test_workspace.c b/tests/test_workspace.c index 60c6dcaa..d0855fa8 100644 --- a/tests/test_workspace.c +++ b/tests/test_workspace.c @@ -17,6 +17,11 @@ TEST(ws_depth_counts_components_below_the_volume) { ASSERT_EQ(cbm_workspace_path_depth("/etc/"), 1); ASSERT_EQ(cbm_workspace_path_depth("/Users/dev"), 2); ASSERT_EQ(cbm_workspace_path_depth("/Users//dev///x"), 3); + /* macOS firmlinks: the /private prefix must not inflate depth, or "/etc" + * resolves to "/private/etc" and passes a minimum of two. */ + ASSERT_EQ(cbm_workspace_path_depth("/private/etc"), 1); + ASSERT_EQ(cbm_workspace_path_depth("/private/tmp/proj"), 2); + ASSERT_EQ(cbm_workspace_path_depth("/private"), 0); /* Drive-relative, so an ordinary Windows workspace is one deep. */ ASSERT_EQ(cbm_workspace_path_depth("C:/"), 0); ASSERT_EQ(cbm_workspace_path_depth("D:/repos"), 1); @@ -32,6 +37,9 @@ TEST(ws_volume_roots_are_absolutely_denied) { ASSERT_EQ(cbm_workspace_classify_root("C:/", HOME, CACHE), CBM_WS_DENY_ABSOLUTE); ASSERT_EQ(cbm_workspace_classify_root("C:\\", HOME, CACHE), CBM_WS_DENY_ABSOLUTE); ASSERT_EQ(cbm_workspace_classify_root("//srv/share", HOME, CACHE), CBM_WS_DENY_ABSOLUTE); + /* "/private" carries no components of its own once the macOS firmlink prefix + * is discounted, so it is a volume root rather than merely shallow. */ + ASSERT_EQ(cbm_workspace_classify_root("/private", HOME, CACHE), CBM_WS_DENY_ABSOLUTE); ASSERT_FALSE(cbm_workspace_verdict_is_overridable(CBM_WS_DENY_ABSOLUTE)); PASS(); } @@ -48,8 +56,9 @@ TEST(ws_non_absolute_paths_are_denied) { /* One depth rule refuses every POSIX top-level tree without a list to maintain. * This is the whole reason depth carries its weight. */ TEST(ws_posix_top_level_trees_are_too_shallow) { - static const char *const shallow[] = {"/etc", "/home", "/Users", "/var", - "/opt", "/srv", "/usr", "/private"}; + static const char *const shallow[] = {"/etc", "/home", "/Users", "/var", + "/opt", "/srv", "/usr", "/private/etc", + "/private/var", "/private/tmp"}; for (size_t i = 0; i < sizeof(shallow) / sizeof(shallow[0]); i++) { ASSERT_EQ(cbm_workspace_classify_root(shallow[i], HOME, CACHE), CBM_WS_DENY_TOO_SHALLOW); } @@ -95,17 +104,6 @@ TEST(ws_credential_directories_are_sensitive_at_any_depth) { PASS(); } -/* Indexing a tree holding the cache would absorb every other project's graph - * database, so this is absolute rather than overridable. */ -TEST(ws_cache_holding_roots_are_absolutely_denied) { - ASSERT_EQ(cbm_workspace_classify_root("/Users/dev/.cache", HOME, CACHE), CBM_WS_DENY_ABSOLUTE); - ASSERT_EQ(cbm_workspace_classify_root("/Users/dev/.cache/codebase-memory-mcp", HOME, CACHE), - CBM_WS_DENY_ABSOLUTE); - /* A sibling of the cache is not an ancestor of it. */ - ASSERT_EQ(cbm_workspace_classify_root("/Users/dev/.cachex", HOME, CACHE), CBM_WS_ALLOW); - PASS(); -} - TEST(ws_windows_system_trees_are_sensitive) { ASSERT_EQ(cbm_workspace_classify_root("C:/Windows", HOME, CACHE), CBM_WS_DENY_SENSITIVE); ASSERT_EQ(cbm_workspace_classify_root("C:/Users", HOME, CACHE), CBM_WS_DENY_SENSITIVE); @@ -129,7 +127,6 @@ TEST(ws_posix_matching_is_case_sensitive) { * they must not crash or deny everything. */ TEST(ws_null_context_disables_dependent_checks) { ASSERT_EQ(cbm_workspace_classify_root("/Users/dev", NULL, NULL), CBM_WS_ALLOW); - ASSERT_EQ(cbm_workspace_classify_root("/Users/dev/.cache", NULL, NULL), CBM_WS_ALLOW); /* Depth and volume rules are context-free and still apply. */ ASSERT_EQ(cbm_workspace_classify_root("/etc", NULL, NULL), CBM_WS_DENY_TOO_SHALLOW); ASSERT_EQ(cbm_workspace_classify_root("/", NULL, NULL), CBM_WS_DENY_ABSOLUTE); @@ -154,7 +151,6 @@ SUITE(workspace) { RUN_TEST(ws_legitimate_shallow_roots_are_allowed); RUN_TEST(ws_home_itself_is_sensitive_but_subdirs_are_fine); RUN_TEST(ws_credential_directories_are_sensitive_at_any_depth); - RUN_TEST(ws_cache_holding_roots_are_absolutely_denied); RUN_TEST(ws_windows_system_trees_are_sensitive); RUN_TEST(ws_posix_matching_is_case_sensitive); RUN_TEST(ws_null_context_disables_dependent_checks);