fix: cap client-driven traversal depth (var-length Cypher + MCP tools)

src/cypher/cypher.c

@@ -8,6 +8,8 @@
 #include "cypher/cypher.h"
 #include "store/store.h"
 #include "foundation/platform.h"
+#include "foundation/limits.h"
+#include "foundation/log.h"
 
 enum {
     CYP_BUF_16 = 16,
@@ -22,7 +24,6 @@ enum {
     CYP_MAX_VARS = 16,     /* max Cypher variables in a query */
     CYP_MAX_EDGE_VARS = 8, /* max edge variables */
     CYP_GROWTH_10 = 10,    /* binding growth factor */
-    CYP_MAX_DEPTH = 10,    /* max variable-length path depth */
     CYP_CHAR_IDX1 = 1,     /* second character index (e.g. op[1]) */
     CYP_EBUF_MASK = 7,
     CYP_NODE_COLS = 4, /* columns per node var: name, qn, label, file */
@@ -2886,7 +2887,20 @@ static void expand_var_length(cbm_store_t *store, cbm_rel_pattern_t *rel,
                               cbm_node_pattern_t *target_node, binding_t *b, cbm_node_t *src,
                               const char *to_var, binding_t *new_bindings, int *new_count,
                               int max_new, int *match_count) {
-    int max_depth = rel->max_hops > 0 ? rel->max_hops : CYP_MAX_DEPTH;
+    /* Clamp BOTH the explicit (`*1..N`) and unbounded (`*`, `*..m`) forms to the
+     * engine ceiling: an explicit N above the cap was previously honoured
+     * verbatim, driving cbm_store_bfs to an unbounded hop count (#887). WARN on
+     * clamp — never a silent truncation. */
+    int depth_cap = cbm_cypher_max_depth();
+    int max_depth = rel->max_hops > 0 ? rel->max_hops : depth_cap;
+    if (max_depth > depth_cap) {
+        char req_buf[16];
+        char cap_buf[16];
+        snprintf(req_buf, sizeof(req_buf), "%d", max_depth);
+        snprintf(cap_buf, sizeof(cap_buf), "%d", depth_cap);
+        cbm_log_warn("cypher.depth_capped", "requested", req_buf, "cap", cap_buf);
+        max_depth = depth_cap;
+    }
     cbm_traverse_result_t tr = {0};
     const char *dir = rel->direction ? rel->direction : "outbound";
     cbm_store_bfs(store, src->id, dir, rel->types, rel->type_count, max_depth, CBM_PERCENT, &tr);

src/foundation/limits.c

@@ -4,6 +4,7 @@
 #include "foundation/limits.h"
 
 #include <errno.h>
+#include <limits.h>
 #include <stdlib.h>
 
 long cbm_max_file_bytes(void) {
@@ -24,3 +25,32 @@ long cbm_max_file_bytes(void) {
     }
     return default_cap;
 }
+
+/* Shared env-int parser: a positive integer in [1, INT_MAX], else the fallback.
+ * Read fresh each call (see cbm_max_file_bytes rationale — cheap, test-friendly,
+ * no stale memoized copy across runs). */
+static int env_positive_int(const char *name, int fallback) {
+    const char *raw = getenv(name);
+    if (raw && raw[0]) {
+        errno = 0;
+        char *end = NULL;
+        long v = strtol(raw, &end, 10);
+        if (errno == 0 && end != raw && *end == '\0' && v > 0 && v <= INT_MAX) {
+            return (int)v;
+        }
+        /* Unparseable / non-positive / out-of-range → safe default. */
+    }
+    return fallback;
+}
+
+int cbm_cypher_max_depth(void) {
+    /* 10 — generous for a code call/def graph; an explicit `*1..N` above this is
+     * WARN-capped, never an unbounded (cyclic-graph DoS) traversal. */
+    return env_positive_int("CBM_CYPHER_MAX_DEPTH", 10);
+}
+
+int cbm_mcp_max_depth(void) {
+    /* 15 — ceiling for client-driven MCP graph traversals (trace_call_path,
+     * detect_changes); the caller's `depth` is WARN-clamped to this. */
+    return env_positive_int("CBM_MCP_MAX_DEPTH", 15);
+}

src/foundation/limits.h

@@ -30,4 +30,17 @@ typedef enum {
  * leaking across runs. */
 long cbm_max_file_bytes(void);
 
+/* Maximum variable-length path depth for the Cypher engine (the `*min..max`
+ * hop ceiling). BOTH the explicit (`*1..N`) and unbounded (`*`, `*..m`) forms
+ * are clamped to this, so `[:CALLS*1..1000000]` degrades to a WARN-and-cap
+ * rather than an unbounded (cyclic-graph DoS) traversal. Override with
+ * CBM_CYPHER_MAX_DEPTH (a positive integer). Default 10. */
+int cbm_cypher_max_depth(void);
+
+/* Maximum traversal depth for client-driven MCP graph tools (trace_call_path,
+ * detect_changes): the client `depth` argument is WARN-clamped to this so an
+ * arbitrarily large value cannot drive an unbounded BFS over the shared store.
+ * Override with CBM_MCP_MAX_DEPTH (a positive integer). Default 15. */
+int cbm_mcp_max_depth(void);
+
 #endif /* CBM_LIMITS_H */

src/mcp/mcp.c

@@ -14,7 +14,6 @@ enum {
     MCP_TIMEOUT_MS = 1000,
     MCP_HALF_SEC_US = 500000,
     MCP_MAX_ROWS = 100,
-    MCP_MAX_DEPTH = 15,
     MCP_COL_2 = 2,
     MCP_COL_3 = 3,
     MCP_COL_4 = 4,
@@ -54,6 +53,7 @@ enum {
 #include "foundation/compat_fs.h"
 #include "foundation/compat_thread.h"
 #include "foundation/log.h"
+#include "foundation/limits.h"
 #include "mcp/index_supervisor.h"
 #include "foundation/str_util.h"
 #include "foundation/dump_verify.h"
@@ -2933,6 +2933,22 @@ static void bfs_union_same_name(cbm_store_t *store, const cbm_node_t *nodes, int
     }
 }
 
+/* Clamp a client-supplied traversal depth to the MCP ceiling (cbm_mcp_max_depth),
+ * WARN-logging when it does so — never a silent truncation (#887). An unclamped
+ * `depth` would drive the shared cbm_store_bfs to an arbitrary hop count. */
+static int clamp_mcp_depth(int depth, const char *tool) {
+    int cap = cbm_mcp_max_depth();
+    if (depth > cap) {
+        char req_buf[16];
+        char cap_buf[16];
+        snprintf(req_buf, sizeof(req_buf), "%d", depth);
+        snprintf(cap_buf, sizeof(cap_buf), "%d", cap);
+        cbm_log_warn("mcp.depth_capped", "tool", tool, "requested", req_buf, "cap", cap_buf);
+        return cap;
+    }
+    return depth;
+}
+
 static char *handle_trace_call_path(cbm_mcp_server_t *srv, const char *args) {
     char *func_name = cbm_mcp_get_string_arg(args, "function_name");
     char *project = get_project_arg(args);
@@ -2941,6 +2957,7 @@ static char *handle_trace_call_path(cbm_mcp_server_t *srv, const char *args) {
     char *mode = cbm_mcp_get_string_arg(args, "mode");
     char *param_name = cbm_mcp_get_string_arg(args, "parameter_name");
     int depth = cbm_mcp_get_int_arg(args, "depth", MCP_DEFAULT_DEPTH);
+    depth = clamp_mcp_depth(depth, "trace_call_path");
     bool risk_labels = cbm_mcp_get_bool_arg(args, "risk_labels");
     bool include_tests = cbm_mcp_get_bool_arg(args, "include_tests");
 
@@ -5201,6 +5218,7 @@ static char *handle_detect_changes(cbm_mcp_server_t *srv, const char *args) {
     char *since = cbm_mcp_get_string_arg(args, "since");
     char *scope = cbm_mcp_get_string_arg(args, "scope");
     int depth = cbm_mcp_get_int_arg(args, "depth", MCP_DEFAULT_BFS_DEPTH);
+    depth = clamp_mcp_depth(depth, "detect_changes");
 
     /* scope: "files" = just changed files, "symbols" = files + symbols (default) */
     bool want_symbols = !scope || strcmp(scope, "symbols") == 0 || strcmp(scope, "impact") == 0;

tests/test_cypher.c

@@ -7,6 +7,7 @@
 #include "test_framework.h"
 #include <cypher/cypher.h>
 #include <store/store.h>
+#include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
 
@@ -834,6 +835,73 @@ TEST(cypher_exec_variable_length) {
     PASS();
 }
 
+/* Reproduce-first (#887): an EXPLICIT variable-length upper bound must still be
+ * capped at the engine ceiling (cbm_cypher_max_depth(), default 10). On
+ * origin/main, expand_var_length honoured an explicit `*1..N` verbatim (only the
+ * unbounded `*` / `*..m` forms were capped), so `[:CALLS*1..N]` passed N straight
+ * to cbm_store_bfs — an unbounded traversal (a DoS on cyclic graphs). RED before
+ * the clamp: a *1..12 walk over a 13-node chain
+ * returns all 12 hops (N01..N12). GREEN after: it stops at the depth-10 ceiling
+ * (N01..N10); N11/N12 are never emitted. max_rows=64 keeps the binding-expansion
+ * cap (bind_cap*10) well above the hop count, so DEPTH — not the binding cap — is
+ * the bound under test. */
+TEST(cypher_exec_var_length_explicit_bound_capped) {
+    cbm_store_t *s = cbm_store_open_memory();
+    cbm_store_upsert_project(s, "test", "/tmp/test");
+
+    /* Linear chain N00 -CALLS-> N01 -> ... -> N12 (13 nodes, one node per hop). */
+    int64_t ids[13];
+    for (int i = 0; i < 13; i++) {
+        char name[8];
+        char qn[24];
+        snprintf(name, sizeof(name), "N%02d", i);
+        snprintf(qn, sizeof(qn), "test.N%02d", i);
+        cbm_node_t n = {.project = "test",
+                        .label = "Function",
+                        .name = name,
+                        .qualified_name = qn,
+                        .file_path = "chain.go"};
+        ids[i] = cbm_store_upsert_node(s, &n);
+    }
+    for (int i = 0; i < 12; i++) {
+        cbm_edge_t e = {
+            .project = "test", .source_id = ids[i], .target_id = ids[i + 1], .type = "CALLS"};
+        cbm_store_insert_edge(s, &e);
+    }
+
+    cbm_cypher_result_t r = {0};
+    int rc = cbm_cypher_execute(s,
+                                "MATCH (a:Function {name: \"N00\"})-[:CALLS*1..12]->"
+                                "(x:Function) RETURN x.name",
+                                "test", 64, &r);
+    ASSERT_EQ(rc, 0);
+
+    /* Capped at 10 hops → exactly N01..N10; N11/N12 are beyond the ceiling. */
+    ASSERT_EQ(r.row_count, 10);
+    bool saw_n10 = false;
+    bool saw_n11 = false;
+    bool saw_n12 = false;
+    for (int i = 0; i < r.row_count; i++) {
+        const char *v = r.rows[i][0];
+        if (v && strcmp(v, "N10") == 0) {
+            saw_n10 = true;
+        }
+        if (v && strcmp(v, "N11") == 0) {
+            saw_n11 = true;
+        }
+        if (v && strcmp(v, "N12") == 0) {
+            saw_n12 = true;
+        }
+    }
+    ASSERT_TRUE(saw_n10);  /* within the ceiling — proves the traversal really ran */
+    ASSERT_FALSE(saw_n11); /* clamped away */
+    ASSERT_FALSE(saw_n12);
+
+    cbm_cypher_result_free(&r);
+    cbm_store_close(s);
+    PASS();
+}
+
 TEST(cypher_exec_defines_edge) {
     cbm_store_t *s = setup_cypher_store();
     cbm_cypher_result_t r = {0};
@@ -2581,6 +2649,7 @@ SUITE(cypher) {
     RUN_TEST(cypher_exec_limit);
     RUN_TEST(cypher_exec_order_by);
     RUN_TEST(cypher_exec_variable_length);
+    RUN_TEST(cypher_exec_var_length_explicit_bound_capped);
     RUN_TEST(cypher_exec_defines_edge);
     RUN_TEST(cypher_exec_no_results);
     RUN_TEST(cypher_exec_where_numeric);

tests/test_mcp.c

@@ -1001,6 +1001,62 @@ TEST(tool_trace_call_path_prefers_definition) {
     PASS();
 }
 
+/* Reproduce-first (#887): the client-supplied `depth` on trace_call_path must be
+ * clamped to the MCP ceiling (cbm_mcp_max_depth(), default 15). On origin/main
+ * an MCP_MAX_DEPTH=15 constant was defined but never applied — `depth` flowed
+ * straight into bfs_union_same_name, so an unbounded value drives the shared
+ * cbm_store_bfs to arbitrary depth. Over an 18-node call chain, depth=1000
+ * reaches n16/n17 (RED); with the clamp the walk stops at hop 15, so n15 is
+ * reached but n16 is not (GREEN). Quoted tokens ("n15"/"n16") match only the
+ * node-name field, never the qualified_name (preceded by '.'), so the boundary
+ * check is exact. */
+TEST(tool_trace_call_path_depth_clamped) {
+    cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
+    cbm_store_t *st = cbm_mcp_server_store(srv);
+    const char *proj = "depth-proj";
+    cbm_mcp_server_set_project(srv, proj);
+    cbm_store_upsert_project(st, proj, "/tmp/depth");
+
+    /* Linear call chain n00 -CALLS-> n01 -> ... -> n17 (18 nodes). */
+    int64_t ids[18];
+    for (int i = 0; i < 18; i++) {
+        char name[8];
+        char qn[32];
+        snprintf(name, sizeof(name), "n%02d", i);
+        snprintf(qn, sizeof(qn), "depth-proj.n%02d", i);
+        cbm_node_t n = {.project = proj,
+                        .label = "Function",
+                        .name = name,
+                        .qualified_name = qn,
+                        .file_path = "chain.c",
+                        .start_line = 1,
+                        .end_line = 2};
+        ids[i] = cbm_store_upsert_node(st, &n);
+    }
+    for (int i = 0; i < 17; i++) {
+        cbm_edge_t e = {
+            .project = proj, .source_id = ids[i], .target_id = ids[i + 1], .type = "CALLS"};
+        cbm_store_insert_edge(st, &e);
+    }
+
+    char *resp = cbm_mcp_server_handle(
+        srv, "{\"jsonrpc\":\"2.0\",\"id\":71,\"method\":\"tools/call\","
+             "\"params\":{\"name\":\"trace_call_path\",\"arguments\":{\"function_name\":\"n00\","
+             "\"project\":\"depth-proj\",\"direction\":\"outbound\",\"depth\":1000}}}");
+    ASSERT_NOT_NULL(resp);
+    char *inner = extract_text_content(resp);
+    ASSERT_NOT_NULL(inner);
+
+    /* Reached within the ceiling (proves the traversal ran) but clamped at 15. */
+    ASSERT_NOT_NULL(strstr(inner, "\"n15\""));
+    ASSERT_NULL(strstr(inner, "\"n16\""));
+
+    free(inner);
+    free(resp);
+    cbm_mcp_server_free(srv);
+    PASS();
+}
+
 /* Reproduce-first (#650, distilled): two GENUINELY-DIFFERENT same-named functions
  * whose bodies differ in length score differently, so the old exact-tie check did
  * not flag them ambiguous — and bfs_union_same_name (#546) then merged the caller
@@ -4711,6 +4767,7 @@ SUITE(mcp) {
     RUN_TEST(tool_trace_missing_function_name);
     RUN_TEST(tool_trace_call_path_ambiguous);
     RUN_TEST(tool_trace_call_path_prefers_definition);
+    RUN_TEST(tool_trace_call_path_depth_clamped);
     RUN_TEST(tool_trace_call_path_distinct_defs_not_over_unioned);
     RUN_TEST(tool_trace_call_path_dts_stub_unions_with_impl);
     RUN_TEST(tool_delete_project_not_found);