jpmschweitzerandClaude Fable 5
a4c299bf9b
/query/graph scoping was a documented no-op (graph_service returned the query unscoped) and neo4j_client permitted writes; a live probe showed a nonexistent user could read the whole graph. - Add Neo4jClient.execute_read() that opens the session with default_access_mode=READ_ACCESS so the database refuses writes even if validation is bypassed. - GraphService.execute_query() now rejects queries containing CREATE/MERGE/DELETE/DETACH/SET/REMOVE/DROP/FOREACH/LOAD or any CALL (conservative word-boundary denylist on the uppercased query) and executes through the read-only session; the no-op _scope_query_to_user is removed. - Remove the false user-scoping claims from /query/graph (main.py) and /graph/query docs and the CypherQueryRequest model: the endpoints are documented as admin/debug, unscoped read-only (per-tenant label injection for arbitrary Cypher would need a real parser; /graph/nodes remains the tenant-scoped path). - Offline unit tests: denylist coverage (incl. lowercase/multiline/CALL), word-boundary false-positive check, and READ_ACCESS session assertion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
113 lines
3.8 KiB
Python
113 lines
3.8 KiB
Python
"""
|
|
Offline unit tests for /query/graph hardening.
|
|
|
|
The raw Cypher endpoint must:
|
|
- reject queries containing write clauses or CALL procedures (denylist),
|
|
- execute allowed queries through the READ-ONLY client path
|
|
(Neo4jClient.execute_read), never the writable execute_query path.
|
|
"""
|
|
|
|
from unittest.mock import AsyncMock, MagicMock
|
|
|
|
import pytest
|
|
|
|
from src.services.graph_service import GraphService
|
|
|
|
|
|
@pytest.fixture
|
|
def mock_neo4j():
|
|
neo4j = MagicMock()
|
|
neo4j.execute_read = AsyncMock(return_value=[{"n": {"name": "x"}}])
|
|
neo4j.execute_query = AsyncMock(return_value=[{"n": {"name": "x"}}])
|
|
return neo4j
|
|
|
|
|
|
@pytest.fixture
|
|
def service(mock_neo4j):
|
|
return GraphService(neo4j_client=mock_neo4j, wikijs_client=MagicMock())
|
|
|
|
|
|
WRITE_QUERIES = [
|
|
"CREATE (n:Evil) RETURN n",
|
|
"MATCH (n) DELETE n",
|
|
"MATCH (n) DETACH DELETE n",
|
|
"MERGE (n:Evil {name: 'x'}) RETURN n",
|
|
"MATCH (n) SET n.pwned = true RETURN n",
|
|
"MATCH (n) REMOVE n:Document RETURN n",
|
|
"DROP INDEX my_index",
|
|
"FOREACH (x IN [1] | CREATE (:Evil))",
|
|
"LOAD CSV FROM 'file:///etc/passwd' AS row RETURN row",
|
|
"CALL db.labels()",
|
|
"CALL apoc.periodic.iterate('MATCH (n) RETURN n', 'DELETE n', {})",
|
|
"call dbms.components()",
|
|
"match (n) detach delete n", # lowercase
|
|
"MATCH (n)\nSET n.x = 1", # multiline
|
|
]
|
|
|
|
|
|
@pytest.mark.unit
|
|
class TestWriteClauseDenylist:
|
|
"""Write clauses and procedure calls must be rejected before execution."""
|
|
|
|
@pytest.mark.parametrize("query", WRITE_QUERIES)
|
|
async def test_write_query_rejected(self, service, mock_neo4j, query):
|
|
with pytest.raises(ValueError, match="read-only"):
|
|
await service.execute_query(query, {}, user="llm_tester")
|
|
|
|
mock_neo4j.execute_read.assert_not_awaited()
|
|
mock_neo4j.execute_query.assert_not_awaited()
|
|
|
|
async def test_read_query_allowed(self, service):
|
|
response = await service.execute_query(
|
|
"MATCH (n:Document) RETURN n LIMIT 5", {}, user="llm_tester"
|
|
)
|
|
assert response.count == 1
|
|
|
|
async def test_word_boundary_no_false_positive(self, service):
|
|
"""Words merely containing denylisted substrings must pass."""
|
|
response = await service.execute_query(
|
|
"MATCH (n:Document) WHERE n.title = 'dataSET dropped' RETURN n",
|
|
{},
|
|
user="llm_tester",
|
|
)
|
|
assert response.count == 1
|
|
|
|
|
|
@pytest.mark.unit
|
|
class TestReadOnlyExecution:
|
|
"""Allowed queries must run through the read-only session path."""
|
|
|
|
async def test_uses_execute_read_not_execute_query(self, service, mock_neo4j):
|
|
await service.execute_query(
|
|
"MATCH (n) RETURN n LIMIT 1", {"p": 1}, user="llm_tester"
|
|
)
|
|
|
|
mock_neo4j.execute_read.assert_awaited_once_with(
|
|
"MATCH (n) RETURN n LIMIT 1", {"p": 1}
|
|
)
|
|
mock_neo4j.execute_query.assert_not_awaited()
|
|
|
|
async def test_neo4j_client_read_session_access_mode(self):
|
|
"""Neo4jClient.execute_read must open the session with READ_ACCESS."""
|
|
from neo4j import READ_ACCESS
|
|
from src.clients.neo4j_client import Neo4jClient
|
|
|
|
client = Neo4jClient(uri="bolt://unused:7687", user="u", password="p")
|
|
|
|
session = MagicMock()
|
|
run_result = MagicMock()
|
|
run_result.data = AsyncMock(return_value=[{"ok": 1}])
|
|
session.run = AsyncMock(return_value=run_result)
|
|
session_cm = MagicMock()
|
|
session_cm.__aenter__ = AsyncMock(return_value=session)
|
|
session_cm.__aexit__ = AsyncMock(return_value=False)
|
|
|
|
driver = MagicMock()
|
|
driver.session = MagicMock(return_value=session_cm)
|
|
client._driver = driver
|
|
|
|
records = await client.execute_read("RETURN 1 AS ok")
|
|
|
|
assert records == [{"ok": 1}]
|
|
driver.session.assert_called_once_with(default_access_mode=READ_ACCESS)
|