Skip to content

Commit 7fa5b07

Browse files
committed
feat: enforce one workspace boundary on every indexing entry point
Routes the MCP index_repository handler and the graph UI's POST /api/index through a single decision function. The UI route previously checked only that root_path was a directory, so an operator's configured boundary held on one entry point and not the other; it now canonicalizes first and applies the same policy, answering 403 with the reason. The decision is two-tier, because a bare default-deny would refuse every first run and a bare opt-in leaves the default open: - Breadth is always enforced, with nothing configured. Filesystem, drive and share roots, top-level system trees, the home directory itself and credential directories are refused as indexing roots out of the box. - Containment in a declared root applies once CBM_ALLOWED_ROOT is set or a grant exists, and is evaluated first so a path outside a configured root is reported as exactly that. Three things the tests caught, each a real defect rather than a test fix: - On macOS /etc, /tmp and /var are firmlinked under /private, so canonicalizing "/etc" yields "/private/etc" and counted two deep — sailing past a minimum of two, missing the very path being guarded. Depth now discounts a leading "private" component. - An earlier draft refused any root containing the cache directory. That was over-claimed: the indexer only parses recognised source files and a graph database is binary SQLite it would never extract. Refusing a whole root is also the wrong remedy where the concern does hold — not walking the cache is. Removed, with the reasoning recorded at the site. - Rewording the refusal to "outside every allowed root" broke an assertion matching "outside the allowed root", and that test's early return skipped its CBM_ALLOWED_ROOT cleanup, leaking the variable into every later test in the suite. The original wording is kept and guidance appended instead. Worth remembering: these contracts match strings, not properties. Docs updated in the same change, since both env-var tables said "unset imposes no restriction" and that is no longer true: CONFIGURATION.md and README.md describe the two tiers, and CONFIGURATION.md lists the always- refused roots along with the two limits that matter — this constrains scope rather than sensitivity, and the credential list is a denylist that raises the cost of a mistake rather than closing the class. SECURITY.md's supported- versions table was still on 0.8.x and now reads 0.9.x. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent 03e9355 commit 7fa5b07

9 files changed

Lines changed: 435 additions & 42 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -670,7 +670,7 @@ codebase-memory-mcp config reset auto_index # reset to default
670670

671671
| Variable | Default | Description |
672672
|----------|---------|-------------|
673-
| `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. |
673+
| `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. |
674674
| `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. |
675675
| `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`. |
676676
| `CBM_DOWNLOAD_URL` | *(GitHub releases)* | Override the download URL for updates. Used for testing or self-hosted deployments. |

‎SECURITY.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,8 @@ sha256sum -c checksums.txt
172172

173173
| Version | Supported |
174174
|---------|-----------|
175-
| Latest `0.8.x` | Yes — security fixes land in the newest release |
176-
| < 0.8 | No — please upgrade to the latest release |
175+
| Latest `0.9.x` | Yes — security fixes land in the newest release |
176+
| < 0.9 | No — please upgrade to the latest release |
177177

178178
Only the latest release is supported. Security fixes are shipped in a new
179179
patched release rather than backported to older versions; upgrading to the

‎docs/CONFIGURATION.md‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ These environment variables affect runtime behavior:
116116

117117
| Variable | Default | Description |
118118
|---|---|---|
119-
| `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). |
119+
| `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. |
120120
| `CBM_CACHE_DIR` | `~/.cache/codebase-memory-mcp` | Override the cache directory used for indexes, `_config.db`, and UI `config.json`. |
121121
| `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. |
122122
| `CBM_DOWNLOAD_URL` | GitHub releases | Override the update download URL. |
@@ -125,6 +125,25 @@ These environment variables affect runtime behavior:
125125

126126
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.
127127

128+
129+
### Roots that are always refused
130+
131+
Independently of `CBM_ALLOWED_ROOT`, some directories are refused as an indexing
132+
root because they are too broad or too sensitive to index as a unit:
133+
134+
- a filesystem root, a Windows drive root, or a UNC share root;
135+
- a top-level system tree — `/etc`, `/var`, `/usr`, `/home`, `/Users`, and on
136+
Windows `C:\Windows`, `C:\Users`, `C:\ProgramData`, `C:\Program Files`;
137+
- your home directory itself (directories *below* it are fine);
138+
- a credential directory at any depth — `.ssh`, `.aws`, `.gnupg`, `.kube`,
139+
`.docker`, `.netrc`, `.git-credentials`, `.password-store`, macOS `Keychains`.
140+
141+
Two limits are worth stating plainly. This constrains *scope*, not
142+
*sensitivity*: inside a root that is allowed, every file the process can read may
143+
be indexed and later returned. And the credential list is a denylist, so it
144+
raises the cost of a mistake rather than closing the class — a directory it does
145+
not name is permitted.
146+
128147
## 5. Agent and Editor Integration Files
129148

130149
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.

‎src/foundation/workspace.c‎

Lines changed: 267 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@
44
*/
55
#include "foundation/workspace.h"
66

7+
#include "foundation/compat.h"
8+
#include "foundation/compat_fs.h"
9+
#include "foundation/platform.h"
10+
11+
#include <stdlib.h>
12+
13+
#include <stdio.h>
714
#include <string.h>
815

916
enum {
@@ -72,6 +79,14 @@ int cbm_workspace_path_depth(const char *canonical_path) {
7279
}
7380
int depth = 0;
7481
const char *p = canonical_path + prefix;
82+
/* macOS firmlinks /etc, /tmp and /var under /private, so canonicalizing any
83+
* of them adds a component: "/etc" resolves to "/private/etc" and would count
84+
* as two deep, sailing past a minimum of two — exactly the path the reports
85+
* demonstrate. Treat a leading "private" as transparent so depth reflects the
86+
* tree the user named. "/private/tmp/proj" still counts two and is allowed. */
87+
if (strncmp(p, "private", 7) == 0 && (ws_is_sep(p[7]) || p[7] == '\0')) {
88+
p += 7;
89+
}
7590
while (*p) {
7691
while (ws_is_sep(*p)) {
7792
p++;
@@ -222,11 +237,19 @@ cbm_ws_verdict_t cbm_workspace_classify_root(const char *canonical_path, const c
222237
return CBM_WS_DENY_TOO_SHALLOW;
223238
}
224239

225-
/* Indexing a tree that holds the cache would pull every other project's
226-
* graph database into this project's index. Never overridable. */
227-
if (cache_dir && cache_dir[0] && ws_is_ancestor_or_equal(canonical_path, cache_dir)) {
228-
return CBM_WS_DENY_ABSOLUTE;
229-
}
240+
/* No rule here for "this root contains the cache directory".
241+
*
242+
* An earlier draft refused such roots outright, on the theory that indexing
243+
* them would absorb every other project's graph database. Two things make
244+
* that wrong. The indexer only parses recognised source files, and a graph
245+
* database is binary SQLite it would never extract; and refusing an entire
246+
* root is the wrong remedy even where the concern holds — the right one is to
247+
* not walk the cache. Excluding the cache from discovery is tracked
248+
* separately; classifying the root is not the place for it.
249+
*
250+
* cache_dir stays in the signature because the exclusion work will need it
251+
* and because callers already have it to hand. */
252+
(void)cache_dir;
230253

231254
if (ws_any_component_matches(canonical_path, WS_CREDENTIAL_NAMES,
232255
sizeof(WS_CREDENTIAL_NAMES) / sizeof(WS_CREDENTIAL_NAMES[0]),
@@ -263,3 +286,242 @@ const char *cbm_workspace_verdict_reason(cbm_ws_verdict_t verdict) {
263286
bool cbm_workspace_verdict_is_overridable(cbm_ws_verdict_t verdict) {
264287
return verdict == CBM_WS_DENY_SENSITIVE;
265288
}
289+
290+
/* ── Grant store ──────────────────────────────────────────────────────────── */
291+
292+
enum { WS_LINE_MAX = 4096 };
293+
294+
/* A line may carry a leading '!' meaning "a person explicitly approved this
295+
* sensitive root". The marker is stored rather than inferred so re-classifying
296+
* later (a new credential name is added to the list, say) cannot silently
297+
* upgrade an ordinary grant into a sensitive one. */
298+
static const char WS_SENSITIVE_MARK = '!';
299+
300+
bool cbm_workspace_grant_path(const char *cache_dir, char *out, size_t out_sz) {
301+
if (!cache_dir || !cache_dir[0] || !out || out_sz == 0) {
302+
return false;
303+
}
304+
int n = snprintf(out, out_sz, "%s/allowed_roots", cache_dir);
305+
return n > 0 && (size_t)n < out_sz;
306+
}
307+
308+
/* Walk the grant file, invoking visit for each entry. visit returns true to stop.
309+
* Returns the number of entries seen. */
310+
static int ws_grant_walk(const char *cache_dir,
311+
bool (*visit)(const char *root, bool sensitive, void *ctx), void *ctx) {
312+
char store[WS_LINE_MAX];
313+
if (!cbm_workspace_grant_path(cache_dir, store, sizeof(store))) {
314+
return 0;
315+
}
316+
FILE *f = cbm_fopen(store, "r");
317+
if (!f) {
318+
return 0;
319+
}
320+
char line[WS_LINE_MAX];
321+
int seen = 0;
322+
while (fgets(line, (int)sizeof(line), f)) {
323+
size_t len = strlen(line);
324+
while (len > 0 && (line[len - 1] == '\n' || line[len - 1] == '\r')) {
325+
line[--len] = '\0';
326+
}
327+
if (len == 0 || line[0] == '#') {
328+
continue;
329+
}
330+
bool sensitive = line[0] == WS_SENSITIVE_MARK;
331+
const char *root = sensitive ? line + 1 : line;
332+
if (!root[0]) {
333+
continue;
334+
}
335+
seen++;
336+
if (visit && visit(root, sensitive, ctx)) {
337+
break;
338+
}
339+
}
340+
(void)fclose(f);
341+
return seen;
342+
}
343+
344+
typedef struct {
345+
const char *candidate;
346+
bool contained; /* candidate is at or below some granted root */
347+
bool exact_sensitive; /* candidate is itself a grant marked sensitive */
348+
} ws_match_t;
349+
350+
static bool ws_match_visit(const char *root, bool sensitive, void *ctx) {
351+
ws_match_t *m = ctx;
352+
if (cbm_path_within_root(root, m->candidate)) {
353+
m->contained = true;
354+
if (sensitive && ws_paths_equal(root, m->candidate)) {
355+
m->exact_sensitive = true;
356+
}
357+
}
358+
return false; /* visit every entry: a later one may be the exact match */
359+
}
360+
361+
typedef struct {
362+
char *out;
363+
size_t out_sz;
364+
size_t used;
365+
} ws_list_t;
366+
367+
static bool ws_list_visit(const char *root, bool sensitive, void *ctx) {
368+
ws_list_t *l = ctx;
369+
int n = snprintf(l->out + l->used, l->out_sz - l->used, "%s%s\n",
370+
sensitive ? "(approved) " : "", root);
371+
if (n > 0 && (size_t)n < l->out_sz - l->used) {
372+
l->used += (size_t)n;
373+
}
374+
return false;
375+
}
376+
377+
bool cbm_workspace_grant_list(const char *cache_dir, char *out, size_t out_sz) {
378+
if (!out || out_sz == 0) {
379+
return false;
380+
}
381+
out[0] = '\0';
382+
ws_list_t l = {out, out_sz, 0};
383+
return ws_grant_walk(cache_dir, ws_list_visit, &l) > 0;
384+
}
385+
386+
bool cbm_workspace_grant_add(const char *cache_dir, const char *home_dir,
387+
const char *canonical_path, bool approve_sensitive, char *err,
388+
size_t err_sz) {
389+
if (err && err_sz) {
390+
err[0] = '\0';
391+
}
392+
if (!cache_dir || !cache_dir[0] || !canonical_path || !canonical_path[0]) {
393+
if (err) {
394+
snprintf(err, err_sz, "no path given");
395+
}
396+
return false;
397+
}
398+
399+
cbm_ws_verdict_t verdict = cbm_workspace_classify_root(canonical_path, home_dir, cache_dir);
400+
if (verdict != CBM_WS_ALLOW) {
401+
if (!cbm_workspace_verdict_is_overridable(verdict)) {
402+
if (err) {
403+
snprintf(err, err_sz, "%s", cbm_workspace_verdict_reason(verdict));
404+
}
405+
return false;
406+
}
407+
if (!approve_sensitive) {
408+
/* Refuse by default and name the flag, rather than asking a question
409+
* whose answer we cannot authenticate. */
410+
if (err) {
411+
snprintf(err, err_sz, "%s; re-run with --approve-sensitive if that is intended",
412+
cbm_workspace_verdict_reason(verdict));
413+
}
414+
return false;
415+
}
416+
}
417+
418+
/* Already present is success, not a duplicate error. */
419+
ws_match_t existing = {canonical_path, false, false};
420+
(void)ws_grant_walk(cache_dir, ws_match_visit, &existing);
421+
if (existing.contained) {
422+
return true;
423+
}
424+
425+
char store[WS_LINE_MAX];
426+
if (!cbm_workspace_grant_path(cache_dir, store, sizeof(store))) {
427+
if (err) {
428+
snprintf(err, err_sz, "cache path too long");
429+
}
430+
return false;
431+
}
432+
FILE *f = cbm_fopen(store, "a");
433+
if (!f) {
434+
if (err) {
435+
snprintf(err, err_sz, "cannot write %s", store);
436+
}
437+
return false;
438+
}
439+
bool sensitive = verdict == CBM_WS_DENY_SENSITIVE;
440+
(void)fprintf(f, "%s%s\n", sensitive ? "!" : "", canonical_path);
441+
bool ok = fclose(f) == 0;
442+
if (!ok && err) {
443+
snprintf(err, err_sz, "cannot write %s", store);
444+
}
445+
return ok;
446+
}
447+
448+
bool cbm_workspace_root_allowed(const char *canonical_path, const char *home_dir,
449+
const char *cache_dir, const char *configured_root, char *err,
450+
size_t err_sz) {
451+
if (err && err_sz) {
452+
err[0] = '\0';
453+
}
454+
if (!canonical_path || !canonical_path[0]) {
455+
if (err) {
456+
snprintf(err, err_sz, "no repository path given");
457+
}
458+
return false;
459+
}
460+
461+
ws_match_t match = {canonical_path, false, false};
462+
int grants = ws_grant_walk(cache_dir, ws_match_visit, &match);
463+
464+
/* A configured root behaves as an additional grant so existing
465+
* CBM_ALLOWED_ROOT deployments keep working unchanged. */
466+
bool configured = configured_root && configured_root[0];
467+
bool configured_contains = configured && cbm_path_within_root(configured_root, canonical_path);
468+
469+
/* Containment first when a boundary has actually been declared. A path
470+
* outside a configured root is best explained as exactly that, and the
471+
* wording is what callers and the shell contracts already match on. Breadth
472+
* then applies to paths that ARE inside the declared root but are still too
473+
* broad to index as one unit. */
474+
bool boundary_declared = grants > 0 || configured;
475+
if (boundary_declared && !match.contained && !configured_contains) {
476+
if (err) {
477+
/* Keep the "outside the allowed root" wording: changing it broke an
478+
* assertion whose early return then leaked CBM_ALLOWED_ROOT into
479+
* every later test in that suite. Guidance is appended, not
480+
* substituted. */
481+
snprintf(err, err_sz,
482+
"%s is outside the allowed root. To allow it, run: "
483+
"codebase-memory-mcp allow-root %s",
484+
canonical_path, canonical_path);
485+
}
486+
return false;
487+
}
488+
489+
cbm_ws_verdict_t verdict = cbm_workspace_classify_root(canonical_path, home_dir, cache_dir);
490+
if (verdict == CBM_WS_ALLOW) {
491+
return true;
492+
}
493+
/* An explicit human approval recorded for exactly this path is the only thing
494+
* that lifts a sensitive refusal. Absolute and shallow refusals cannot be
495+
* lifted at all. */
496+
if (verdict == CBM_WS_DENY_SENSITIVE && match.exact_sensitive) {
497+
return true;
498+
}
499+
if (err) {
500+
if (cbm_workspace_verdict_is_overridable(verdict)) {
501+
snprintf(err, err_sz,
502+
"%s: %s. To index it anyway, run: codebase-memory-mcp allow-root "
503+
"--approve-sensitive %s",
504+
canonical_path, cbm_workspace_verdict_reason(verdict), canonical_path);
505+
} else {
506+
snprintf(err, err_sz, "%s: %s", canonical_path, cbm_workspace_verdict_reason(verdict));
507+
}
508+
}
509+
return false;
510+
}
511+
512+
/* ── Environment helpers ──────────────────────────────────────────────────── */
513+
514+
/* Callers should not each re-derive these; a caller that resolved the home
515+
* directory differently would classify the same path differently. */
516+
const char *cbm_workspace_home_dir(void) {
517+
const char *home = getenv("HOME");
518+
if (home && home[0]) {
519+
return home;
520+
}
521+
home = getenv("USERPROFILE");
522+
return (home && home[0]) ? home : NULL;
523+
}
524+
525+
const char *cbm_workspace_cache_dir(void) {
526+
return cbm_resolve_cache_dir();
527+
}

0 commit comments

Comments
 (0)