From 2899250c5293e6b40a92562e77249978fa7cab68 Mon Sep 17 00:00:00 2001 From: Mustapha Date: Thu, 16 Jul 2026 18:44:29 +0100 Subject: [PATCH 1/2] fix(mcp): reject misleading ADR sections writes Signed-off-by: Mustapha --- src/mcp/mcp.c | 30 +++++++++++++++++++++++++++--- tests/test_mcp.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 804222436..4a5da9254 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -583,9 +583,10 @@ static const tool_def_t TOOLS[] = { {"manage_adr", "Manage ADR", "Create or update Architecture Decision Records", "{\"type\":\"object\",\"properties\":{\"project\":{\"type\":\"string\"},\"mode\":{\"type\":" - "\"string\",\"enum\":[\"get\",\"update\",\"sections\"]},\"content\":{\"type\":\"string\"}," - "\"sections\":{\"type\":\"array\",\"items\":{\"type\":\"string\"}}},\"required\":[\"project\"]" - "}"}, + "\"string\",\"enum\":[\"get\",\"update\",\"sections\"],\"description\":\"update replaces " + "the entire ADR document; sections only lists existing headings\"},\"content\":{\"type\":\"string\"," + "\"description\":\"Complete replacement document required by update\"}},\"additionalProperties\":false," + "\"required\":[\"project\"]}"}, {"ingest_traces", "Ingest traces", "Ingest runtime traces to enhance the knowledge graph", "{\"type\":\"object\",\"properties\":{\"traces\":{\"type\":\"array\",\"items\":{\"type\":" @@ -7689,6 +7690,28 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) { mode_str = heap_strdup("get"); } + /* `sections` used to be advertised as an input argument, but it was never + * consumed: mode=update still replaced the whole document. Reject the old + * shape explicitly before opening a store so a stale client cannot mistake + * a whole-document replacement for a section-scoped update. */ + bool has_sections_arg = false; + yyjson_doc *args_doc = yyjson_read(args, strlen(args), 0); + if (args_doc) { + yyjson_val *args_root = yyjson_doc_get_root(args_doc); + has_sections_arg = args_root && yyjson_is_obj(args_root) && + yyjson_obj_get(args_root, "sections") != NULL; + yyjson_doc_free(args_doc); + } + if (has_sections_arg) { + free(project); + free(mode_str); + free(content); + return cbm_mcp_text_result( + "{\"status\":\"invalid_arguments\",\"error\":\"The sections argument is not an " + "update primitive and has been removed. No ADR write was performed.\"}", + true); + } + /* ADRs are stored in the SQLite store (project_summaries), the SAME * backend the UI /api/adr endpoints use — so writes via the MCP tool and * the UI are visible to each other (#256). */ @@ -7753,6 +7776,7 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) { if ((strcmp(mode_str, "update") == 0 || strcmp(mode_str, "store") == 0) && content) { if (cbm_store_adr_store(store, project, content) == CBM_STORE_OK) { yyjson_mut_obj_add_str(doc, root_obj, "status", "updated"); + yyjson_mut_obj_add_str(doc, root_obj, "semantics", "whole_document_replaced"); } else { yyjson_mut_obj_add_str(doc, root_obj, "status", "write_error"); is_error = true; diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 4e1d5eeb3..6b044c86a 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -3028,6 +3028,38 @@ TEST(tool_manage_adr_unified_backend_issue256) { PASS(); } +TEST(tool_manage_adr_rejects_removed_sections_argument) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *st = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(st); + ASSERT_EQ(cbm_store_upsert_project(st, "adr-sections-guard", "/tmp/adr-sections-guard"), + CBM_STORE_OK); + cbm_mcp_server_set_project(srv, "adr-sections-guard"); + ASSERT_EQ(cbm_store_adr_store(st, "adr-sections-guard", "## PURPOSE\nOriginal ADR.\n"), + CBM_STORE_OK); + + char *resp = cbm_mcp_server_handle( + srv, "{\"jsonrpc\":\"2.0\",\"id\":122,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"manage_adr\",\"arguments\":{" + "\"project\":\"adr-sections-guard\",\"mode\":\"update\"," + "\"sections\":[\"PURPOSE\"],\"content\":\"## PURPOSE\\nReplacement ADR.\\n\"}}}"); + ASSERT_NOT_NULL(resp); + ASSERT_NOT_NULL(strstr(resp, "invalid_arguments")); + ASSERT_NOT_NULL(strstr(resp, "No ADR write was performed")); + ASSERT_NOT_NULL(strstr(resp, "\"isError\":true")); + free(resp); + + cbm_adr_t adr; + memset(&adr, 0, sizeof(adr)); + ASSERT_EQ(cbm_store_adr_get(st, "adr-sections-guard", &adr), CBM_STORE_OK); + ASSERT_STR_EQ(adr.content, "## PURPOSE\nOriginal ADR.\n"); + cbm_store_adr_free(&adr); + + cbm_mcp_server_free(srv); + PASS(); +} + TEST(tool_index_repository_reports_store_backed_adr) { char tmp_dir[256]; snprintf(tmp_dir, sizeof(tmp_dir), "/tmp/cbm-index-adr-test-XXXXXX"); @@ -6455,6 +6487,7 @@ SUITE(mcp) { RUN_TEST(tool_manage_adr_no_project); RUN_TEST(tool_manage_adr_get_with_existing_adr); RUN_TEST(tool_manage_adr_unified_backend_issue256); + RUN_TEST(tool_manage_adr_rejects_removed_sections_argument); RUN_TEST(tool_index_repository_reports_store_backed_adr); RUN_TEST(tool_index_repository_dot_uses_absolute_project_key_and_preserves_adr); RUN_TEST(index_repository_cli_name_override_issue823); From ae255d240ef7f5296e521bdb7db6cf5518c9beab Mon Sep 17 00:00:00 2001 From: Mustapha Date: Thu, 16 Jul 2026 21:01:51 +0100 Subject: [PATCH 2/2] style(mcp): satisfy clang-format Signed-off-by: Mustapha --- src/mcp/mcp.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 4a5da9254..cd697a07e 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -584,8 +584,9 @@ static const tool_def_t TOOLS[] = { {"manage_adr", "Manage ADR", "Create or update Architecture Decision Records", "{\"type\":\"object\",\"properties\":{\"project\":{\"type\":\"string\"},\"mode\":{\"type\":" "\"string\",\"enum\":[\"get\",\"update\",\"sections\"],\"description\":\"update replaces " - "the entire ADR document; sections only lists existing headings\"},\"content\":{\"type\":\"string\"," - "\"description\":\"Complete replacement document required by update\"}},\"additionalProperties\":false," + "the entire ADR document; sections only lists existing " + "headings\"},\"content\":{\"type\":\"string\",\"description\":\"Complete replacement document " + "required by update\"}},\"additionalProperties\":false," "\"required\":[\"project\"]}"}, {"ingest_traces", "Ingest traces", "Ingest runtime traces to enhance the knowledge graph", @@ -7698,8 +7699,8 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) { yyjson_doc *args_doc = yyjson_read(args, strlen(args), 0); if (args_doc) { yyjson_val *args_root = yyjson_doc_get_root(args_doc); - has_sections_arg = args_root && yyjson_is_obj(args_root) && - yyjson_obj_get(args_root, "sections") != NULL; + has_sections_arg = + args_root && yyjson_is_obj(args_root) && yyjson_obj_get(args_root, "sections") != NULL; yyjson_doc_free(args_doc); } if (has_sections_arg) {