Hardening pass: bounds, containment, and input validation across MCP/CLI/UI

README.md

@@ -476,6 +476,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_CACHE_DIR` | `~/.cache/codebase-memory-mcp` | Override the database storage directory. All project indexes and config are stored here. |
 | `CBM_DIAGNOSTICS` | `false` | Set to `1` or `true` to enable periodic diagnostics output to `/tmp/cbm-diagnostics-<pid>.json`. |
 | `CBM_DOWNLOAD_URL` | *(GitHub releases)* | Override the download URL for updates. Used for testing or self-hosted deployments. |

docs/CONFIGURATION.md

@@ -110,6 +110,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_CACHE_DIR` | `~/.cache/codebase-memory-mcp` | Override the cache directory used for indexes, `_config.db`, and UI `config.json`. |
 | `CBM_DIAGNOSTICS` | `false` | Enable periodic diagnostics output to `/tmp/cbm-diagnostics-<pid>.json`. |
 | `CBM_DOWNLOAD_URL` | GitHub releases | Override the update download URL. |

internal/cbm/zstd_store.c

@@ -5,6 +5,7 @@
 #include "zstd_store.h"
 
 #include <stddef.h>
+#include <stdint.h>
 
 int cbm_zstd_compress(const char *src, int srcLen, char *dst, int dstCap, int level) {
     size_t rc = ZSTD_compress(dst, (size_t)dstCap, src, (size_t)srcLen, level);
@@ -14,12 +15,20 @@ int cbm_zstd_compress(const char *src, int srcLen, char *dst, int dstCap, int le
     return (int)rc;
 }
 
-int cbm_zstd_decompress(const char *src, int srcLen, char *dst, int dstCap) {
-    size_t rc = ZSTD_decompress(dst, (size_t)dstCap, src, (size_t)srcLen);
+int64_t cbm_zstd_decompress(const char *src, size_t srcLen, char *dst, size_t dstCap) {
+    size_t rc = ZSTD_decompress(dst, dstCap, src, srcLen);
     if (ZSTD_isError(rc)) {
         return 0;
     }
-    return (int)rc;
+    return (int64_t)rc;
+}
+
+size_t cbm_zstd_frame_content_size(const char *src, size_t srcLen) {
+    unsigned long long n = ZSTD_getFrameContentSize(src, srcLen);
+    if (n == ZSTD_CONTENTSIZE_UNKNOWN || n == ZSTD_CONTENTSIZE_ERROR) {
+        return 0;
+    }
+    return (size_t)n;
 }
 
 size_t cbm_zstd_compress_bound(int inputSize) {

internal/cbm/zstd_store.h

@@ -2,14 +2,22 @@
 #define CBM_ZSTD_STORE_H
 
 #include <stddef.h>
+#include <stdint.h>
 
 // Zstd compression at specified level (1=fast .. 22=best).
 // Returns compressed size on success, 0 on error.
 int cbm_zstd_compress(const char *src, int srcLen, char *dst, int dstCap, int level);
 
-// Zstd decompression.
-// Returns decompressed size on success, 0 on error.
-int cbm_zstd_decompress(const char *src, int srcLen, char *dst, int dstCap);
+// Zstd decompression. srcLen/dstCap are size_t so a >2 GiB destination capacity
+// is never truncated (a large DB artifact, or a crafted one, would otherwise
+// wrap through int and desync the real buffer size from the decoder capacity).
+// Returns decompressed size on success (>0), 0 on error.
+int64_t cbm_zstd_decompress(const char *src, size_t srcLen, char *dst, size_t dstCap);
+
+// Declared decompressed content size of a zstd frame (from its header), or 0
+// when unknown/unreadable. Lets the caller size the destination from the frame
+// itself instead of trusting a separate, attacker-controllable size field.
+size_t cbm_zstd_frame_content_size(const char *src, size_t srcLen);
 
 // Maximum compressed size bound for given input size.
 size_t cbm_zstd_compress_bound(int inputSize);

src/cli/cli.c

@@ -2968,15 +2968,16 @@ static bool prompt_yn(const char *question) {
 /* Minimum line length in checksums.txt: CBM_SZ_64 hex + 2 spaces + 1 char filename */
 #define CHECKSUM_LINE_MIN (SHA256_HEX_LEN + 2)
 
-/* Compute SHA-CBM_SZ_256 of a file using platform tools (sha256sum/shasum).
- * Writes CBM_SZ_64-char hex digest + NUL to out. Returns 0 on success. */
-static int sha256_file(const char *path, char *out, size_t out_size) {
+/* Compute the SHA-256 of a file using platform tools (sha256sum/shasum).
+ * Writes a 64-char hex digest + NUL to out. Returns 0 on success. Not static:
+ * exercised directly by the self-update checksum regression test. */
+int cbm_cli_sha256_file(const char *path, char *out, size_t out_size) {
     if (out_size < SHA256_BUF_SIZE) {
         return CLI_ERR;
     }
     char cmd[CLI_BUF_1K];
 #ifdef __APPLE__
-    snprintf(cmd, sizeof(cmd), "shasum -a CBM_SZ_256 '%s' 2>/dev/null", path);
+    snprintf(cmd, sizeof(cmd), "shasum -a 256 '%s' 2>/dev/null", path);
 #else
     snprintf(cmd, sizeof(cmd), "sha256sum '%s' 2>/dev/null", path);
 #endif
@@ -3107,8 +3108,8 @@ static int verify_download_checksum(const char *archive_path, const char *archiv
     }
 
     char actual[SHA256_BUF_SIZE] = {0};
-    if (sha256_file(archive_path, actual, sizeof(actual)) != 0) {
-        (void)fprintf(stderr, "warning: sha256sum/shasum not available — skipping verification\n");
+    if (cbm_cli_sha256_file(archive_path, actual, sizeof(actual)) != 0) {
+        (void)fprintf(stderr, "error: could not compute checksum (sha256 tool unavailable)\n");
         return CLI_ERR;
     }
 
@@ -4250,8 +4251,12 @@ static int download_verify_install(const char *url, const char *ext, const char
     const char *portable = (strcmp(os, "linux") == 0) ? "-portable" : "";
     snprintf(archive_name, sizeof(archive_name), "codebase-memory-mcp-%s%s-%s%s.%s",
              want_ui ? "ui-" : "", os, arch, portable, ext);
+    /* Fail closed: install only a positively-verified download. A mismatch,
+     * a missing checksum entry, or an unavailable hash tool (crc != 0) all
+     * abort rather than install an unverified binary. */
     int crc = verify_download_checksum(tmp_archive, archive_name);
-    if (crc == CLI_TRUE) {
+    if (crc != 0) {
+        (void)fprintf(stderr, "error: refusing to install an unverified download\n");
         cbm_unlink(tmp_archive);
         return CLI_TRUE;
     }

src/cypher/cypher.c

@@ -1652,6 +1652,16 @@ static int parse_return_or_with(parser_t *p, cbm_return_clause_t **out, bool is_
 
     } while (check(p, TOK_COMMA));
 
+    /* Projection is materialized per row into fixed-width stack arrays sized at
+     * CBM_SZ_32 columns (execute_return_simple and its siblings). Bound the
+     * parsed item count to that width so an over-wide RETURN is rejected here
+     * instead of writing past those arrays downstream. */
+    if (r->count > CBM_SZ_32) {
+        free(r->items);
+        free(r);
+        return CBM_NOT_FOUND;
+    }
+
 tail:
     /* Optional ORDER BY */
     if (match(p, TOK_ORDER)) {

src/discover/discover.c

@@ -606,9 +606,20 @@ static int wide_stat(const char *path, struct stat *st) {
 #endif
 }
 
-/* Stat a path, skipping symlinks. Returns 0 on success, -1 to skip. */
+/* Stat a path, skipping symlinks (POSIX) and junctions / reparse points
+ * (Windows). Returns 0 on success, -1 to skip. Skipping reparse points keeps
+ * discovery from walking through a junction that points outside the project
+ * root, mirroring the POSIX S_ISLNK skip. */
 static int safe_stat(const char *abs_path, struct stat *st) {
 #ifdef _WIN32
+    wchar_t *wpath = cbm_utf8_to_wide(abs_path);
+    if (wpath) {
+        DWORD attr = GetFileAttributesW(wpath);
+        free(wpath);
+        if (attr != INVALID_FILE_ATTRIBUTES && (attr & FILE_ATTRIBUTE_REPARSE_POINT)) {
+            return CBM_NOT_FOUND;
+        }
+    }
     return wide_stat(abs_path, st);
 #else
     if (lstat(abs_path, st) != 0) {

src/mcp/mcp.c

@@ -3842,6 +3842,8 @@ char *cbm_mcp_index_run_supervised_path(const char *root_path) {
     return index_run_supervised_path(NULL, root_path);
 }
 
+bool cbm_path_within_root(const char *root_path, const char *abs_path); /* defined below */
+
 static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) {
     /* Supervisor gate: run the index in a crash/hang-isolating worker subprocess
      * unless this process IS the worker or the kill switch (CBM_INDEX_SUPERVISOR=0)
@@ -3866,6 +3868,20 @@ 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: when CBM_ALLOWED_ROOT is set (agentic /
+     * multi-tenant deployments where repo_path may be influenced by an
+     * untrusted caller), refuse to index a path that resolves outside it.
+     * Unset by default, so the standard "index the path I gave you" behaviour
+     * is unchanged. */
+    const char *allowed_root = getenv("CBM_ALLOWED_ROOT");
+    if (allowed_root && allowed_root[0] && repo_path &&
+        !cbm_path_within_root(allowed_root, repo_path)) {
+        free(mode_str);
+        free(name_override);
+        free(repo_path);
+        return cbm_mcp_text_result("repo_path is outside the allowed root", true);
+    }
+
     if (mode_str && strcmp(mode_str, "cross-repo-intelligence") == 0) {
         free(mode_str);
         free(name_override);
@@ -4080,19 +4096,19 @@ static yyjson_doc *enrich_node_properties(yyjson_mut_doc *doc, yyjson_mut_val *o
 /* Resolve an absolute path from root_path + file_path, verify containment,
  * and read source lines. Sets *out_abs_path (caller frees). Returns source
  * string (caller frees) or NULL if path is invalid/unreadable. */
-static char *resolve_snippet_source(const char *root_path, const char *file_path, int start,
-                                    int end, char **out_abs_path) {
-    *out_abs_path = NULL;
-    if (!root_path || !file_path) {
-        return NULL;
+/* True only when abs_path, after realpath/_fullpath resolution (which collapses
+ * `..` and resolves symlinks/junctions), stays within root_path. This is the
+ * single containment guard every MCP file-read sink must pass before reading a
+ * file into a tool response: both the snippet path (resolve_snippet_source) and
+ * the search path (attach_result_source) route through it, so a result whose
+ * indexed path escapes the project root — via a `..` segment, or a symlink /
+ * Windows junction picked up during discovery — is never read back out. */
+bool cbm_path_within_root(const char *root_path, const char *abs_path) {
+    if (!root_path || !abs_path) {
+        return false;
     }
-    size_t apsz = strlen(root_path) + strlen(file_path) + MCP_SEPARATOR;
-    char *abs_path = malloc(apsz);
-    snprintf(abs_path, apsz, "%s/%s", root_path, file_path);
-
     char real_root[CBM_SZ_4K];
     char real_file[CBM_SZ_4K];
-    bool path_ok = false;
 #ifdef _WIN32
     if (_fullpath(real_root, root_path, sizeof(real_root)) &&
         _fullpath(real_file, abs_path, sizeof(real_file))) {
@@ -4104,11 +4120,24 @@ static char *resolve_snippet_source(const char *root_path, const char *file_path
         size_t root_len = strlen(real_root);
         if (strncmp(real_file, real_root, root_len) == 0 &&
             (real_file[root_len] == '/' || real_file[root_len] == '\0')) {
-            path_ok = true;
+            return true;
         }
     }
+    return false;
+}
+
+static char *resolve_snippet_source(const char *root_path, const char *file_path, int start,
+                                    int end, char **out_abs_path) {
+    *out_abs_path = NULL;
+    if (!root_path || !file_path) {
+        return NULL;
+    }
+    size_t apsz = strlen(root_path) + strlen(file_path) + MCP_SEPARATOR;
+    char *abs_path = malloc(apsz);
+    snprintf(abs_path, apsz, "%s/%s", root_path, file_path);
+
     *out_abs_path = abs_path;
-    if (path_ok) {
+    if (cbm_path_within_root(root_path, abs_path)) {
         return read_file_lines(abs_path, start, end);
     }
     return NULL;
@@ -4570,6 +4599,14 @@ static void attach_result_source(yyjson_mut_doc *doc, yyjson_mut_val *item, sear
     char abs_path[CBM_SZ_1K];
     snprintf(abs_path, sizeof(abs_path), "%s/%s", root_path, r->file);
 
+    /* Containment: a search result whose indexed path resolves outside the
+     * project root (a `..` segment, or a symlink/junction that discovery
+     * followed) must not be read back into the response. Same guard the
+     * snippet path already uses. */
+    if (!cbm_path_within_root(root_path, abs_path)) {
+        return;
+    }
+
     if (mode == MODE_FULL) {
         char *source = read_file_lines(abs_path, r->start_line, r->end_line);
         if (source) {
@@ -4938,6 +4975,14 @@ static bool write_scoped_filelist(cbm_mcp_server_t *srv, const char *project, co
     int written = 0;
     if (fl) {
         for (int fi = 0; fi < indexed_count; fi++) {
+            /* A source path never legitimately contains a newline or carriage
+             * return. Those bytes are exactly the record separator on the
+             * Windows filelist (and would split naive line readers elsewhere),
+             * so a crafted indexed path with an embedded newline could inject
+             * an extra entry into the scan set. Skip such paths entirely. */
+            if (strpbrk(indexed_files[fi], "\r\n") != NULL) {
+                continue;
+            }
             if (has_path_filter && path_regex) {
 #ifdef _WIN32
                 cbm_normalize_path_sep(indexed_files[fi]);
@@ -5352,8 +5397,12 @@ static char *handle_detect_changes(cbm_mcp_server_t *srv, const char *args) {
         base_branch = heap_strdup("main");
     }
 
-    /* Reject shell metacharacters in user-supplied branch name */
-    if (!cbm_validate_shell_arg(base_branch)) {
+    /* Reject shell metacharacters, and a leading '-', in the user-supplied
+     * branch name. base_branch is spliced into `git diff --name-only
+     * "<base>"...HEAD`; a value starting with '-' would be read by git as an
+     * option rather than a ref (e.g. `--output=<path>` writes the diff to an
+     * arbitrary file). A real git ref never begins with '-'. */
+    if (!cbm_validate_shell_arg(base_branch) || base_branch[0] == '-') {
         free(project);
         free(base_branch);
         free(scope);

src/pipeline/artifact.c

@@ -15,6 +15,13 @@ enum {
 };
 #define ART_BYTES_PER_MB ((size_t)1024 * 1024)
 
+/* Generous ceiling on an imported artifact's decompressed size. Real indexes
+ * (a full Linux-kernel DB is ~14 GB) fit comfortably; a frame that declares
+ * more than this is rejected before any allocation so a crafted content size
+ * can neither trigger a runaway allocation nor be used to desync the decoder
+ * capacity from the destination buffer. */
+#define ART_MAX_DECOMPRESSED_BYTES ((size_t)64 * 1024 * ART_BYTES_PER_MB)
+
 #include "pipeline/artifact.h"
 #include "store/store.h"
 #include "foundation/platform.h"
@@ -614,16 +621,29 @@ int cbm_artifact_import(const char *repo_path, const char *cache_db_path) {
     }
 
     /* Decompress */
-    char *decompressed = malloc(original_size);
+    /* Size the destination from the zstd frame's own content-size header, not
+     * from the separately-stored (attacker-controllable) original_size field.
+     * The allocation and the decoder capacity are then the SAME size_t value,
+     * so a crafted size can never make the capacity exceed the real buffer
+     * (the int-truncation that used to do exactly that is gone with the size_t
+     * signature). Require the metadata field to agree, and cap the total. */
+    size_t frame_size = cbm_zstd_frame_content_size(compressed, clen);
+    if (frame_size == 0 || frame_size > ART_MAX_DECOMPRESSED_BYTES || frame_size != original_size) {
+        free(compressed);
+        cbm_log_error("artifact.import", "err", "bad_decompressed_size");
+        return CBM_NOT_FOUND;
+    }
+
+    char *decompressed = malloc(frame_size);
     if (!decompressed) {
         free(compressed);
         return CBM_NOT_FOUND;
     }
 
-    int dlen = cbm_zstd_decompress(compressed, (int)clen, decompressed, (int)original_size);
+    int64_t dlen = cbm_zstd_decompress(compressed, clen, decompressed, frame_size);
     free(compressed);
 
-    if (dlen <= 0) {
+    if (dlen <= 0 || (size_t)dlen != frame_size) {
         free(decompressed);
         cbm_log_error("artifact.import", "err", "zstd_decompress");
         return CBM_NOT_FOUND;