diff --git a/graph-ui/agents/hooks/atlas-trace.py b/graph-ui/agents/hooks/atlas-trace.py index 347f1ffac..86c9cde4a 100644 --- a/graph-ui/agents/hooks/atlas-trace.py +++ b/graph-ui/agents/hooks/atlas-trace.py @@ -21,12 +21,6 @@ import tempfile DEFAULT_FILE = os.path.join(os.path.expanduser('~'), '.atlas-trace', 'events.jsonl') -# Was von einem Befehl oder einem Suchmuster aufgeschrieben wird. Laenger waere -# in einem Instrument von 320 Pixeln ohnehin nicht zu lesen, und kuerzer -# verloere die Datei, um die es geht. -DETAIL_CAP = 180 - - def trace_file(): return os.environ.get('ATLAS_TRACE_FILE') or DEFAULT_FILE @@ -228,12 +222,6 @@ def main(): if not isinstance(args, dict): args = {} file_path = args.get('file_path') or args.get('notebook_path') or '' - detail = ( - args.get('command') - or args.get('pattern') - or args.get('description') - or '' - ) run = str(payload.get('session_id') or uuid.uuid4()) root = str(payload.get('cwd') or os.getcwd()) project = os.environ.get('ATLAS_PROJECT', os.path.basename(root.rstrip(os.sep))) @@ -246,7 +234,8 @@ def main(): 'phase': 'end', 'tool': tool, 'path': str(file_path or args.get('path') or ''), - 'detail': str(detail)[:DETAIL_CAP], + # Commands, patterns and descriptions may contain credentials. + 'detail': '', 'source': 'tool-hook', } span = line_span(file_path, args) if tool == 'Read' else None diff --git a/graph-ui/agents/hooks/test_atlas_trace.py b/graph-ui/agents/hooks/test_atlas_trace.py index 19b84441e..cc43594b5 100644 --- a/graph-ui/agents/hooks/test_atlas_trace.py +++ b/graph-ui/agents/hooks/test_atlas_trace.py @@ -1,6 +1,7 @@ """Reproducible local producer tests; no external network or agent accounts.""" import concurrent.futures import importlib.util +import io import json import os from pathlib import Path @@ -31,6 +32,66 @@ class TraceTests(unittest.TestCase): hook.enqueue(db, 'fixture', {'run': 'concurrent-run', 'seq': 0, 'ts': number, 'agent': 'fixture', 'tool': 'Read', 'phase': 'end'}) + def assert_private_tool_input_is_omitted(self, tool, field, value, private_marker): + root = Path(self.directory.name) + source = root / 'src' / 'example.py' + payload = {'tool_name': tool, 'session_id': 'privacy-run', 'cwd': str(root), + 'tool_input': {field: value, 'file_path': str(source)}} + generated = [] + original_enqueue = hook.enqueue + + def capture_enqueue(db, project, row): + generated.append(json.loads(json.dumps(row))) + original_enqueue(db, project, row) + + with patch.dict(os.environ, {'ATLAS_PROJECT': 'fixture', 'ATLAS_AGENT_NAME': 'fixture'}), \ + patch.object(hook.sys, 'argv', ['atlas-trace.py']), \ + patch.object(hook.sys, 'stdin', io.StringIO(json.dumps(payload))), \ + patch.object(hook.time, 'time', return_value=1234.5), \ + patch.object(hook, 'enqueue', side_effect=capture_enqueue), \ + patch.object(hook.urllib.request, 'build_opener') as transport: + transport.return_value.open.side_effect = OSError('offline fixture') + self.assertEqual(hook.main(), 0) + transport.return_value.open.assert_called_once() + request = transport.return_value.open.call_args.args[0] + + with hook.open_outbox() as db: + pending = db.execute('SELECT project,payload FROM pending').fetchall() + self.assertEqual(len(generated), 1) + self.assertEqual(len(pending), 1) + self.assertEqual(pending[0][0], 'fixture') + outbound = json.loads(request.data) + self.assertEqual(outbound['project'], 'fixture') + self.assertEqual(len(outbound['events']), 1) + for surface, event, serialized in ( + ('generated', generated[0], json.dumps(generated[0])), + ('persisted', json.loads(pending[0][1]), pending[0][1]), + ('outgoing', outbound['events'][0], request.data.decode())): + with self.subTest(surface=surface): + self.assertEqual(event['tool'], tool) + self.assertEqual(event['path'], os.path.join('src', 'example.py')) + self.assertEqual(event['agent'], 'fixture') + self.assertEqual(event['run'], 'privacy-run') + self.assertEqual(event['phase'], 'end') + self.assertEqual(event['source'], 'tool-hook') + self.assertEqual(event['ts'], 1234500) + self.assertNotIn(private_marker, serialized) + self.assertNotIn(value, serialized) + + def test_command_private_values_never_enter_event_payloads(self): + marker = 'SYNTHETIC_COMMAND_PRIVATE_VALUE_71E5' + self.assert_private_tool_input_is_omitted( + 'Bash', 'command', 'AUTH_TOKEN=' + marker + ' example-tool --quiet', marker) + + def test_search_patterns_never_enter_event_payloads(self): + marker = 'SYNTHETIC_PATTERN_PRIVATE_VALUE_83A6' + self.assert_private_tool_input_is_omitted('Grep', 'pattern', marker, marker) + + def test_description_private_values_never_enter_event_payloads(self): + marker = 'SYNTHETIC_DESCRIPTION_PRIVATE_VALUE_92B7' + self.assert_private_tool_input_is_omitted( + 'Bash', 'description', 'Inspect the value ' + marker, marker) + def test_concurrent_hooks_allocate_unique_sequences_and_keep_pending_after_failure(self): with hook.open_outbox(): pass diff --git a/scripts/ci/test-ui.sh b/scripts/ci/test-ui.sh old mode 100644 new mode 100755 diff --git a/src/cli/runtime_settings.c b/src/cli/runtime_settings.c index 04a291c8e..1ee4ace7c 100644 --- a/src/cli/runtime_settings.c +++ b/src/cli/runtime_settings.c @@ -103,9 +103,6 @@ static const setting_t settings[] = { "integer", NULL, 0, INT_MAX, NULL, "internal/cbm/lsp/ts_lsp.c"), ENV("CBM_LSP_MAX_WALK_DEPTH", "Resolver walk depth", "Maximum recursive resolver walk depth.", "Indexing", "integer", "512", 1, INT_MAX, NULL, "internal/cbm/lsp/scope.h"), - ENV("CBM_WALK_DEFS_MAX", "Definition walk frame limit", - "Maximum extraction definition-walk frames.", "Indexing", "integer", "8388608", 1, INT_MAX, - NULL, "internal/cbm/extract_defs.c"), ENV("CBM_INDEX_MAX_RESTARTS", "Index worker restart limit", "Maximum worker restarts. Values start at one to preserve the daemon and MCP consumers' " "shared behavior.", diff --git a/src/foundation/git_env.c b/src/foundation/git_env.c index b53f0ecbc..7807fe7a6 100644 --- a/src/foundation/git_env.c +++ b/src/foundation/git_env.c @@ -82,6 +82,15 @@ static bool git_env_name_should_strip(const char *name, size_t name_len) { git_env_name_equals(name, name_len, "GIT_DIFF_OPTS"); } +bool cbm_git_env_entry_should_strip(const char *entry) { + if (!entry) { + return false; + } + const char *eq = strchr(entry, '='); + size_t name_len = eq ? (size_t)(eq - entry) : strlen(entry); + return git_env_name_should_strip(entry, name_len); +} + #ifdef _WIN32 /* Wide entry name → should the child omit it? Names we match are ASCII, so any @@ -135,17 +144,11 @@ wchar_t *cbm_git_child_env_block(void) { #else -static bool git_env_entry_should_strip(const char *entry) { - const char *eq = strchr(entry, '='); - size_t name_len = eq ? (size_t)(eq - entry) : strlen(entry); - return git_env_name_should_strip(entry, name_len); -} - char **cbm_git_child_envp(void) { size_t count = 0; size_t bytes = 0; for (char **e = environ; e && *e; e++) { - if (!git_env_entry_should_strip(*e)) { + if (!cbm_git_env_entry_should_strip(*e)) { count++; bytes += strlen(*e) + 1; } @@ -158,7 +161,7 @@ char **cbm_git_child_envp(void) { char *strings = (char *)envp + table; size_t slot = 0; for (char **e = environ; e && *e && slot < count; e++) { - if (!git_env_entry_should_strip(*e)) { + if (!cbm_git_env_entry_should_strip(*e)) { size_t len = strlen(*e) + 1; memcpy(strings, *e, len); envp[slot++] = strings; diff --git a/src/foundation/git_env.h b/src/foundation/git_env.h index 2f81c00b8..f614bea51 100644 --- a/src/foundation/git_env.h +++ b/src/foundation/git_env.h @@ -30,6 +30,10 @@ extern const char *const cbm_git_repo_env_vars[CBM_GIT_REPO_ENV_VAR_COUNT]; * The name compare is case-insensitive on Windows (its environment is). */ bool cbm_git_env_entry_is_repo_local(const char *entry); +/* True when an entry must be omitted from a git child: repository-local variables + * and GIT_DIFF_OPTS. The name compare is case-insensitive on Windows. */ +bool cbm_git_env_entry_should_strip(const char *entry); + #ifdef _WIN32 /* A CREATE_UNICODE_ENVIRONMENT block: the current environment minus the * repository-local git variables and GIT_DIFF_OPTS. One allocation; release with diff --git a/src/foundation/subprocess.c b/src/foundation/subprocess.c index f0deb7e49..4b86feffe 100644 --- a/src/foundation/subprocess.c +++ b/src/foundation/subprocess.c @@ -506,7 +506,7 @@ static bool cbm_subprocess_copy_environment(cbm_subprocess_t *process, const cha entries[index++] = entry; } for (size_t i = 0; i < override_count; i++) { - if (process->strip_git_repo_env && cbm_git_env_entry_is_repo_local(overrides[i])) { + if (process->strip_git_repo_env && cbm_git_env_entry_should_strip(overrides[i])) { continue; } converted[i] = cbm_utf8_to_wide(overrides[i]); @@ -583,7 +583,7 @@ static bool cbm_subprocess_copy_environment(cbm_subprocess_t *process, const cha } } for (size_t i = 0; i < override_count; i++) { - if (process->strip_git_repo_env && cbm_git_env_entry_is_repo_local(overrides[i])) { + if (process->strip_git_repo_env && cbm_git_env_entry_should_strip(overrides[i])) { continue; } size_t name_length = (size_t)(strchr(overrides[i], '=') - overrides[i]); diff --git a/src/foundation/subprocess.h b/src/foundation/subprocess.h index e534dba1e..4dc8f1870 100644 --- a/src/foundation/subprocess.h +++ b/src/foundation/subprocess.h @@ -65,7 +65,8 @@ typedef struct { * nonempty (case-insensitive on Windows). NULL or an empty * list preserves normal inheritance. Executable lookup * continues to use the caller's PATH. strip_git_repo_env - * also removes repository-local entries from overrides. */ + * also removes repository-local entries and GIT_DIFF_OPTS + * from overrides. */ const char *windows_cmd_payload; /* Windows-only cmd.exe command text. When set, bin must * be an absolute path ending in cmd.exe and argv must be * NULL. The fixed /D /S /V:OFF /C prefix is added while diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index a7d7351cc..f10ab0cb3 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -878,11 +878,11 @@ typedef struct { * for write-side opens (index_repository, manage_adr writes). That is what * makes readOnlyHint=true honest for them and lets plan-mode clients expose * them (the "read-only analysis tools" surface described in #1100). - * get_file_outline reads through the same resolve_store() path, and - * ingest_traces only validates and counts its input (edge creation is not - * implemented), so both are read-only too (#2118). Only handlers that write - * user-visible state -- index_repository, manage_adr, delete_project -- may - * clear readOnlyHint. */ + * get_file_outline reads through the same resolve_store() path (#2118). + * Only handlers that write user-visible state -- index_repository, manage_adr, + * delete_project, ingest_traces -- may clear readOnlyHint. Trace ingestion + * accumulates observation counts on retries and can evict old runs, so it is + * neither read-only nor idempotent and must advertise destructive effects. */ static const tool_annotation_def_t TOOL_ANNOTATIONS[] = { {"index_repository", false, false, true, false}, {"search_graph", true, false, true, false}, @@ -900,7 +900,7 @@ static const tool_annotation_def_t TOOL_ANNOTATIONS[] = { {"check_index_coverage", true, false, true, false}, {"detect_changes", true, false, true, false}, {"manage_adr", false, true, false, false}, - {"ingest_traces", true, false, true, false}, + {"ingest_traces", false, true, false, false}, }; static const tool_annotation_def_t *mcp_tool_annotations(const char *name) { @@ -19551,8 +19551,34 @@ enum { * back so the producer can fix its naming instead of silently losing data. */ static char *handle_ingest_traces(cbm_mcp_server_t *srv, const char *args) { char *project = get_project_arg(args); + bool mutation_held = false; + if (project) { + mutation_held = mcp_project_mutation_begin(srv, project); + if (!mutation_held) { + free(project); + return cbm_mcp_text_result("project operation cancelled or blocked by an active index", + true); + } + if (mcp_request_cancelled(srv)) { + mcp_project_mutation_end(srv, project); + free(project); + return cbm_mcp_text_result("project operation cancelled for this request", true); + } + } + + /* Hold the lease before resolving a cached generation or opening the + * writer, and through both observation updates and retention eviction. */ cbm_store_t *store = resolve_store(srv, project); - REQUIRE_STORE(store, project); + if (!store) { + char *error = build_no_store_error_checked(srv, project); + char *result = cbm_mcp_text_result(error, true); + free(error); + if (mutation_held) { + mcp_project_mutation_end(srv, project); + } + free(project); + return result; + } yyjson_doc *adoc = yyjson_read(args, strlen(args), 0); yyjson_val *aroot = adoc ? yyjson_doc_get_root(adoc) : NULL; @@ -19561,6 +19587,9 @@ static char *handle_ingest_traces(cbm_mcp_server_t *srv, const char *args) { if (trace_count == 0U || trace_count > INGEST_TRACES_MAX) { if (adoc) yyjson_doc_free(adoc); + if (mutation_held) { + mcp_project_mutation_end(srv, project); + } free(project); return cbm_mcp_text_result( "traces is required: a non-empty array (max 10000) of {path: [qualified_name, ...]} " @@ -19578,6 +19607,9 @@ static char *handle_ingest_traces(cbm_mcp_server_t *srv, const char *args) { store = open_store_for_write(srv, store, &owned_rw); if (!store) { yyjson_doc_free(adoc); + if (mutation_held) { + mcp_project_mutation_end(srv, project); + } free(project); return cbm_mcp_text_result("could not open the project store for writing", true); } @@ -19591,6 +19623,9 @@ static char *handle_ingest_traces(cbm_mcp_server_t *srv, const char *args) { if (owned_rw) cbm_store_close(owned_rw); yyjson_doc_free(adoc); + if (mutation_held) { + mcp_project_mutation_end(srv, project); + } free(project); return cbm_mcp_text_result("store is not queryable", true); } @@ -19720,6 +19755,9 @@ static char *handle_ingest_traces(cbm_mcp_server_t *srv, const char *args) { yyjson_doc_free(adoc); if (owned_rw) cbm_store_close(owned_rw); + if (mutation_held) { + mcp_project_mutation_end(srv, project); + } free(project); char *result = cbm_mcp_text_result(json, false); diff --git a/src/ui/http_server.c b/src/ui/http_server.c index 8de5d9ff0..d67908db6 100644 --- a/src/ui/http_server.c +++ b/src/ui/http_server.c @@ -2990,7 +2990,8 @@ static bool rpc_tool_is_read_only(const char *name) { static bool rpc_manage_adr_is_read(yyjson_val *params) { yyjson_val *arguments = json_unique_member(params, "arguments"); if (!arguments) - return true; /* no arguments → default mode "get" */ + /* Only an absent arguments member defaults to "get". A duplicate is ambiguous. */ + return yyjson_obj_get(params, "arguments") == NULL; if (!yyjson_is_obj(arguments)) return false; bool mode_seen = false; diff --git a/tests/test_httpd.c b/tests/test_httpd.c index ada1c26d3..b7447abed 100644 --- a/tests/test_httpd.c +++ b/tests/test_httpd.c @@ -2236,6 +2236,41 @@ TEST(ui_server_mutations_require_json_content_type) { PASS(); } +TEST(ui_server_rpc_rejects_duplicate_adr_arguments) { + th_server_t ts; + ASSERT_EQ(th_server_start(&ts), 0); + static const struct { + const char *params; + int expected_status; + } cases[] = { + {"{\"name\":\"manage_adr\"}", 200}, + {"{\"name\":\"manage_adr\",\"arguments\":{\"mode\":\"update\"}," + "\"arguments\":{\"mode\":\"get\"}}", + 403}, + {"{\"name\":\"manage_adr\",\"arguments\":{\"mode\":\"get\"}," + "\"arguments\":{\"mode\":\"update\"}}", + 403}, + }; + int statuses[sizeof(cases) / sizeof(cases[0])] = {0}; + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + char body[512], request[1024], response[8192]; + snprintf(body, sizeof(body), + "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"tools/call\",\"params\":%s}", + cases[i].params); + snprintf(request, sizeof(request), + "POST /rpc HTTP/1.1\r\nContent-Type: application/json\r\n" + "Content-Length: %zu\r\n\r\n%s", + strlen(body), body); + int received = th_http(cbm_http_server_port(ts.srv), request, response, sizeof(response)); + statuses[i] = received > 0 ? th_status(response) : 0; + } + th_server_stop(&ts); + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + ASSERT_EQ(statuses[i], cases[i].expected_status); + } + PASS(); +} + TEST(ui_server_rpc_allows_only_ui_read_tools) { th_server_t ts; ASSERT_EQ(th_server_start(&ts), 0); @@ -3992,6 +4027,7 @@ SUITE(httpd) { RUN_TEST(ui_server_same_origin_request_is_allowed); RUN_TEST(ui_server_rejects_foreign_and_null_origins); RUN_TEST(ui_server_mutations_require_json_content_type); + RUN_TEST(ui_server_rpc_rejects_duplicate_adr_arguments); RUN_TEST(ui_server_rpc_allows_only_ui_read_tools); RUN_TEST(ui_server_atlas_routes_are_wired); RUN_TEST(ui_server_oversized_body_rejected); diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 6007ce610..4c2aeaad8 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -1427,10 +1427,9 @@ TEST(mcp_tools_have_behavior_annotations) { {"check_index_coverage", true, false, true, false}, {"detect_changes", true, false, true, false}, {"manage_adr", false, true, false, false}, - /* ingest_traces only validates and counts its input; it writes - * nothing, so it is read-only and idempotent until edge creation - * actually lands (#2118). */ - {"ingest_traces", true, false, true, false}, + /* Observation counts accumulate on retries, and ingestion can evict + * old runs to enforce retention. Both effects change stored data. */ + {"ingest_traces", false, true, false, false}, }; char *json = cbm_mcp_tools_list(); @@ -1513,10 +1512,12 @@ static bool tool_in_list(const char *name, const char *const *list, size_t count TEST(mcp_tool_annotations_only_mutators_are_writable_issue2118) { /* Handlers that write user-visible state: index_repository (builds the * index), manage_adr (writes/overwrites ADR sections), delete_project - * (removes the index). Everything else must be read-only. */ - static const char *const mutating[] = {"index_repository", "manage_adr", "delete_project"}; + * (removes the index), ingest_traces (accumulates observations and evicts + * old runs). Everything else must be read-only. */ + static const char *const mutating[] = {"index_repository", "manage_adr", "delete_project", + "ingest_traces"}; /* Of those, only the ones that can remove/overwrite existing data. */ - static const char *const destructive[] = {"manage_adr", "delete_project"}; + static const char *const destructive[] = {"manage_adr", "delete_project", "ingest_traces"}; const size_t n_mut = sizeof(mutating) / sizeof(mutating[0]); const size_t n_des = sizeof(destructive) / sizeof(destructive[0]); @@ -6028,6 +6029,195 @@ TEST(tool_index_status_no_project) { PASS(); } +static const char *const mcp_ingest_guard_request = + "{\"project\":\"guard-traces\",\"label\":\"guarded\",\"traces\":[" + "{\"path\":[\"guard-traces::alpha\",\"guard-traces::beta\"]}]}"; + +static cbm_mcp_server_t *mcp_ingest_guard_server(void) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + cbm_store_t *store = srv ? cbm_mcp_server_store(srv) : NULL; + if (!store || + cbm_store_upsert_project(store, "guard-traces", "/tmp/guard-traces") != CBM_STORE_OK) { + cbm_mcp_server_free(srv); + return NULL; + } + cbm_mcp_server_set_project(srv, "guard-traces"); + static const char *const names[] = {"alpha", "beta"}; + for (size_t i = 0; i < sizeof(names) / sizeof(names[0]); i++) { + char qn[64]; + snprintf(qn, sizeof(qn), "guard-traces::%s", names[i]); + cbm_node_t node = {.project = "guard-traces", + .label = "Function", + .name = names[i], + .qualified_name = qn, + .file_path = "trace.c", + .start_line = 1, + .end_line = 2}; + if (cbm_store_upsert_node(store, &node) <= 0) { + cbm_mcp_server_free(srv); + return NULL; + } + } + return srv; +} + +static long long mcp_ingest_guard_observed_count(cbm_mcp_server_t *srv) { + long long count = -1; + if (cbm_store_observed_lookup(cbm_mcp_server_store(srv), "guard-traces", "guard-traces::alpha", + "guard-traces::beta", &count, NULL, 0, NULL, 0) != CBM_STORE_OK) { + return -1; + } + return count; +} + +TEST(tool_ingest_traces_mutation_guard_denial_prevents_writes) { + cbm_mcp_server_t *srv = mcp_ingest_guard_server(); + ASSERT_NOT_NULL(srv); + mcp_mutation_guard_probe_t probe = {.deny_begin_call = 1}; + cbm_mcp_server_set_project_mutation_guard(srv, mcp_mutation_guard_probe_begin, + mcp_mutation_guard_probe_end, &probe); + + char *resp = cbm_mcp_handle_tool(srv, "ingest_traces", mcp_ingest_guard_request); + bool blocked = resp && strstr(resp, "\"isError\":true") && strstr(resp, "blocked"); + long long count = mcp_ingest_guard_observed_count(srv); + free(resp); + cbm_mcp_server_free(srv); + + ASSERT_TRUE(blocked); + ASSERT_EQ(count, 0); + ASSERT_EQ(probe.begin_count, 1); + ASSERT_EQ(probe.end_count, 0); + ASSERT_STR_EQ(probe.begin_projects[0], "guard-traces"); + PASS(); +} + +TEST(tool_ingest_traces_mutation_guard_balances_repeated_writes) { + cbm_mcp_server_t *srv = mcp_ingest_guard_server(); + ASSERT_NOT_NULL(srv); + mcp_mutation_guard_probe_t probe = {0}; + cbm_mcp_server_set_project_mutation_guard(srv, mcp_mutation_guard_probe_begin, + mcp_mutation_guard_probe_end, &probe); + + bool stored = true; + for (int i = 0; i < 2; i++) { + char *resp = cbm_mcp_handle_tool(srv, "ingest_traces", mcp_ingest_guard_request); + char *inner = resp ? extract_text_content(resp) : NULL; + stored = stored && inner && strstr(inner, "\"status\":\"stored\"") && + strstr(inner, "\"pairs_stored\":1") && strstr(inner, "\"paths_stored\":1"); + free(inner); + free(resp); + } + long long count = mcp_ingest_guard_observed_count(srv); + cbm_mcp_server_free(srv); + + ASSERT_TRUE(stored); + ASSERT_EQ(count, 2); + ASSERT_EQ(probe.begin_count, 2); + ASSERT_EQ(probe.end_count, 2); + for (int i = 0; i < 2; i++) { + ASSERT_STR_EQ(probe.begin_projects[i], "guard-traces"); + ASSERT_STR_EQ(probe.end_projects[i], "guard-traces"); + } + PASS(); +} + +TEST(tool_ingest_traces_mutation_guard_cancel_releases_without_writes) { + cbm_mcp_server_t *srv = mcp_ingest_guard_server(); + ASSERT_NOT_NULL(srv); + mcp_mutation_guard_probe_t probe = {.cancel_on_begin_call = 1, .cancel_server = srv}; + cbm_mcp_server_set_project_mutation_guard(srv, mcp_mutation_guard_probe_begin, + mcp_mutation_guard_probe_end, &probe); + + char *resp = cbm_mcp_handle_tool(srv, "ingest_traces", mcp_ingest_guard_request); + bool cancelled = resp && strstr(resp, "\"isError\":true") && strstr(resp, "cancelled"); + long long count = mcp_ingest_guard_observed_count(srv); + free(resp); + cbm_mcp_server_free(srv); + + ASSERT_TRUE(cancelled); + ASSERT_TRUE(probe.cancel_attempted); + ASSERT_TRUE(probe.cancel_accepted); + ASSERT_EQ(count, 0); + ASSERT_EQ(probe.begin_count, 1); + ASSERT_EQ(probe.end_count, 1); + ASSERT_STR_EQ(probe.begin_projects[0], "guard-traces"); + ASSERT_STR_EQ(probe.end_projects[0], "guard-traces"); + PASS(); +} + +TEST(tool_ingest_traces_mutation_guard_missing_store_releases) { + char cache[256]; + snprintf(cache, sizeof(cache), "%s/cbm-mcp-traces-guard-XXXXXX", cbm_tmpdir()); + ASSERT_NOT_NULL(cbm_mkdtemp(cache)); + const char *saved_cache = getenv("CBM_CACHE_DIR"); + char *saved_cache_copy = saved_cache ? strdup(saved_cache) : NULL; + ASSERT_EQ(cbm_setenv("CBM_CACHE_DIR", cache, 1), 0); + + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + mcp_mutation_guard_probe_t probe = {0}; + cbm_mcp_server_set_project_mutation_guard(srv, mcp_mutation_guard_probe_begin, + mcp_mutation_guard_probe_end, &probe); + char *resp = cbm_mcp_handle_tool(srv, "ingest_traces", mcp_ingest_guard_request); + bool missing = resp && strstr(resp, "\"isError\":true") && + (strstr(resp, "not found") || strstr(resp, "not indexed")); + char path[512]; + snprintf(path, sizeof(path), "%s/guard-traces.db", cache); + bool created = cbm_file_exists(path); + free(resp); + cbm_mcp_server_free(srv); + cleanup_project_db(cache, "guard-traces"); + restore_cache_dir(saved_cache_copy); + free(saved_cache_copy); + cbm_rmdir(cache); + + ASSERT_TRUE(missing); + ASSERT_FALSE(created); + ASSERT_EQ(probe.begin_count, 1); + ASSERT_EQ(probe.end_count, 1); + ASSERT_STR_EQ(probe.begin_projects[0], "guard-traces"); + ASSERT_STR_EQ(probe.end_projects[0], "guard-traces"); + PASS(); +} + +/* Reject exactly the endpoint lookup at prepare time; this exercises the + * handler's query failure exit without corrupting a database or timing a lock. */ +static int mcp_ingest_guard_deny_nodes(void *context, int action, const char *table, + const char *column, const char *database, + const char *trigger) { + (void)context; + (void)column; + (void)database; + (void)trigger; + return action == SQLITE_READ && table && strcmp(table, "nodes") == 0 ? SQLITE_DENY : SQLITE_OK; +} + +TEST(tool_ingest_traces_mutation_guard_query_failure_releases) { + cbm_mcp_server_t *srv = mcp_ingest_guard_server(); + ASSERT_NOT_NULL(srv); + mcp_mutation_guard_probe_t probe = {0}; + cbm_mcp_server_set_project_mutation_guard(srv, mcp_mutation_guard_probe_begin, + mcp_mutation_guard_probe_end, &probe); + sqlite3 *db = cbm_store_get_db(cbm_mcp_server_store(srv)); + ASSERT_NOT_NULL(db); + ASSERT_EQ(sqlite3_set_authorizer(db, mcp_ingest_guard_deny_nodes, NULL), SQLITE_OK); + char *resp = cbm_mcp_handle_tool(srv, "ingest_traces", mcp_ingest_guard_request); + int reset_result = sqlite3_set_authorizer(db, NULL, NULL); + bool failed = resp && strstr(resp, "\"isError\":true") && strstr(resp, "not queryable"); + long long count = mcp_ingest_guard_observed_count(srv); + free(resp); + cbm_mcp_server_free(srv); + + ASSERT_EQ(reset_result, SQLITE_OK); + ASSERT_TRUE(failed); + ASSERT_EQ(count, 0); + ASSERT_EQ(probe.begin_count, 1); + ASSERT_EQ(probe.end_count, 1); + ASSERT_STR_EQ(probe.begin_projects[0], "guard-traces"); + ASSERT_STR_EQ(probe.end_projects[0], "guard-traces"); + PASS(); +} + /* ingest_traces stores observed runtime calls qn-keyed (reindex-proof): * paths decompose into pairs, counts accumulate per run label, unresolved * names are reported (not silently dropped), whole runs evict together. */ @@ -24473,6 +24663,11 @@ SUITE(mcp) { /* Kept separate so daemon-coordination regressions can be iterated without * running the much larger MCP behavior suite. */ SUITE(mcp_mutation_guard) { + RUN_TEST(tool_ingest_traces_mutation_guard_denial_prevents_writes); + RUN_TEST(tool_ingest_traces_mutation_guard_balances_repeated_writes); + RUN_TEST(tool_ingest_traces_mutation_guard_cancel_releases_without_writes); + RUN_TEST(tool_ingest_traces_mutation_guard_missing_store_releases); + RUN_TEST(tool_ingest_traces_mutation_guard_query_failure_releases); RUN_TEST(tool_delete_project_mutation_guard_blocks_then_releases); RUN_TEST(tool_index_repository_mutation_guard_blocks_before_local_worker); RUN_TEST(tool_manage_adr_mutation_guard_balances_success); diff --git a/tests/test_runtime_settings.c b/tests/test_runtime_settings.c index 857cf10d0..c319c00e3 100644 --- a/tests/test_runtime_settings.c +++ b/tests/test_runtime_settings.c @@ -114,6 +114,7 @@ TEST(runtime_settings_strict_allowlist_and_types) { settings_fixture_t fixture; ASSERT_TRUE(fixture_open(&fixture)); const char *invalid[] = {"{\"CBM_TEST_CRASH_ON\":\"1\"}", + "{\"CBM_WALK_DEFS_MAX\":\"1024\"}", "{\"CBM_CACHE_DIR\":\"/tmp/no\"}", "{\"CBM_WORKERS\":4}", "{\"CBM_WORKERS\":\"4\",\"CBM_WORKERS\":\"8\"}", @@ -132,6 +133,7 @@ TEST(runtime_settings_strict_allowlist_and_types) { yyjson_doc *snapshot = read_snapshot(fixture.path); ASSERT_NOT_NULL(snapshot); ASSERT_NULL(item(snapshot, "CBM_TEST_CRASH_ON")); + ASSERT_NULL(item(snapshot, "CBM_WALK_DEFS_MAX")); ASSERT_FALSE(yyjson_get_bool(yyjson_obj_get(item(snapshot, "CBM_CACHE_DIR"), "editable"))); yyjson_doc_free(snapshot); fixture_close(&fixture);