Skip to content

Commit 1ef97f3

Browse files
aksOpsclaude
andcommitted
fix: run_cypher guard now correctly allows CALL db.* read-only procedures
Bug: query was uppercased but regex exemption checked lowercase db. — CALL db.* was still blocked despite the intended allowlist. Fix: - Use case-insensitive regex (Pattern.CASE_INSENSITIVE) instead of uppercasing the query — preserves original casing for Neo4j execution - CALL db.* explicitly allowed (fulltext search, indexes, schema) - All other CALL forms blocked (apoc.create, custom procedures) Tests updated: - Removed test expecting CALL db.indexes() to be blocked - Added: runCypherShouldAllowCallDbIndexes (positive) - Added: runCypherShouldAllowCallDbFulltextSearch (positive) - Added: runCypherShouldBlockNonDbCall (negative - apoc.create) - Added: runCypherShouldBlockCallCustomProcedure (negative - custom.mutate) - Existing mutation keyword tests still pass (CREATE, DELETE, SET, etc.) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent a7b8253 commit 1ef97f3

2 files changed

Lines changed: 59 additions & 12 deletions

File tree

src/main/java/io/github/randomcodespace/iq/mcp/McpTools.java

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -268,16 +268,26 @@ public String generateFlow(
268268
@McpTool(name = "run_cypher", description = "Execute a read-only Cypher query against the Neo4j graph database.")
269269
public String runCypher(
270270
@McpToolParam(description = "Cypher query string") String query) {
271-
// Block any mutation keywords anywhere in the query (defense-in-depth)
272-
String upper = query.trim().toUpperCase();
273-
List<String> BLOCKED_PATTERNS = List.of(
274-
"\\bCREATE\\b", "\\bDELETE\\b", "\\bDETACH\\b", "\\bSET\\b",
275-
"\\bREMOVE\\b", "\\bMERGE\\b", "\\bDROP\\b", "\\bFOREACH\\b",
276-
"\\bLOAD\\s+CSV\\b", "\\bCALL\\s+(?!db\\.)\\b");
277-
for (String pattern : BLOCKED_PATTERNS) {
278-
if (java.util.regex.Pattern.compile(pattern).matcher(upper).find()) {
279-
String keyword = pattern.replace("\\b", "").replace("\\s+", " ");
280-
return toJson(Map.of(PROP_ERROR, "Read-only queries only. Mutation keyword found: " + keyword));
271+
// Block mutation keywords (defense-in-depth). Uses case-insensitive matching
272+
// so the original query casing is preserved for Neo4j execution.
273+
// CALL db.* is explicitly allowed (read-only: fulltext search, schema, indexes).
274+
String trimmed = query.trim();
275+
List<java.util.regex.Pattern> BLOCKED_PATTERNS = List.of(
276+
java.util.regex.Pattern.compile("\\bCREATE\\b", java.util.regex.Pattern.CASE_INSENSITIVE),
277+
java.util.regex.Pattern.compile("\\bDELETE\\b", java.util.regex.Pattern.CASE_INSENSITIVE),
278+
java.util.regex.Pattern.compile("\\bDETACH\\b", java.util.regex.Pattern.CASE_INSENSITIVE),
279+
java.util.regex.Pattern.compile("\\bSET\\b", java.util.regex.Pattern.CASE_INSENSITIVE),
280+
java.util.regex.Pattern.compile("\\bREMOVE\\b", java.util.regex.Pattern.CASE_INSENSITIVE),
281+
java.util.regex.Pattern.compile("\\bMERGE\\b", java.util.regex.Pattern.CASE_INSENSITIVE),
282+
java.util.regex.Pattern.compile("\\bDROP\\b", java.util.regex.Pattern.CASE_INSENSITIVE),
283+
java.util.regex.Pattern.compile("\\bFOREACH\\b", java.util.regex.Pattern.CASE_INSENSITIVE),
284+
java.util.regex.Pattern.compile("\\bLOAD\\s+CSV\\b", java.util.regex.Pattern.CASE_INSENSITIVE),
285+
// Allow CALL db.* (read-only procedures: indexes, schema, fulltext search)
286+
// Block all other CALL forms (mutation procedures like apoc.create, apoc.merge)
287+
java.util.regex.Pattern.compile("\\bCALL\\s+(?!db\\.)", java.util.regex.Pattern.CASE_INSENSITIVE));
288+
for (var pattern : BLOCKED_PATTERNS) {
289+
if (pattern.matcher(trimmed).find()) {
290+
return toJson(Map.of(PROP_ERROR, "Read-only queries only. Mutation keyword found: " + pattern.pattern()));
281291
}
282292
}
283293
try {

src/test/java/io/github/randomcodespace/iq/mcp/McpToolsExpandedTest.java

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,10 +183,47 @@ void runCypherShouldBlockForeach() throws IOException {
183183
}
184184

185185
@Test
186-
void runCypherShouldBlockCall() throws IOException {
186+
void runCypherShouldBlockNonDbCall() throws IOException {
187+
// Non-db CALL statements (mutation procedures) must be blocked
188+
String result = mcpTools.runCypher("CALL apoc.create.node(['Label'], {name: 'test'})");
189+
Map<String, Object> parsed = parseJson(result);
190+
assertNotNull(parsed.get("error"), "Non-db CALL should be blocked");
191+
}
192+
193+
@Test
194+
void runCypherShouldBlockCallCustomProcedure() throws IOException {
195+
String result = mcpTools.runCypher("CALL custom.mutate()");
196+
Map<String, Object> parsed = parseJson(result);
197+
assertNotNull(parsed.get("error"), "Custom CALL should be blocked");
198+
}
199+
200+
@Test
201+
void runCypherShouldAllowCallDbIndexes() throws IOException {
202+
// CALL db.* is read-only (indexes, schema, fulltext search)
203+
Transaction tx = mock(Transaction.class);
204+
Result queryResult = mock(Result.class);
205+
when(graphDb.beginTx()).thenReturn(tx);
206+
when(tx.execute("CALL db.indexes()")).thenReturn(queryResult);
207+
when(queryResult.columns()).thenReturn(List.of("name"));
208+
when(queryResult.hasNext()).thenReturn(false);
209+
187210
String result = mcpTools.runCypher("CALL db.indexes()");
188211
Map<String, Object> parsed = parseJson(result);
189-
assertNotNull(parsed.get("error"));
212+
assertNotNull(parsed.get("rows"), "CALL db.indexes() should be allowed");
213+
}
214+
215+
@Test
216+
void runCypherShouldAllowCallDbFulltextSearch() throws IOException {
217+
Transaction tx = mock(Transaction.class);
218+
Result queryResult = mock(Result.class);
219+
when(graphDb.beginTx()).thenReturn(tx);
220+
when(tx.execute("CALL db.index.fulltext.queryNodes('lexical_index', 'search')")).thenReturn(queryResult);
221+
when(queryResult.columns()).thenReturn(List.of("node"));
222+
when(queryResult.hasNext()).thenReturn(false);
223+
224+
String result = mcpTools.runCypher("CALL db.index.fulltext.queryNodes('lexical_index', 'search')");
225+
Map<String, Object> parsed = parseJson(result);
226+
assertNotNull(parsed.get("rows"), "CALL db.index.fulltext should be allowed");
190227
}
191228

192229
@Test

0 commit comments

Comments
 (0)