fix(mcp): basename source_file in tool_get_drawer responses
The MCP `mempalace_get_drawer` tool returned the entire raw drawer metadata blob to any connected client, and the `source_file` field in that blob is the absolute filesystem path written by the miners (`miner.py`, `convo_miner.py` — `source_file = str(filepath)`). On a single-user local deployment this is self-disclosure, but in nested-agent or multi-server MCP topologies the client is a separate trust domain and the host's directory layout has no documented client-side use. Mirror the mitigation that `searcher.search_memories()` already applies on its own return path: reduce `source_file` to its basename via `Path(source_file).name` before handing the metadata to the client. Citations still work — the directory layout does not leak. Companion to #1 (omit palace_path from tool_status). Same threat class, different surface: - mempalace_status — palace dir path → fixed in #1 - mempalace_get_drawer — per-drawer source_file path → this PR Other read tools were audited and do not leak host paths: - mempalace_search — already basenames source_file - mempalace_list_drawers — returns wing/room/preview only - mempalace_diary_read — date/timestamp/topic/content only - mempalace_reconnect — success/message/drawers only - mempalace_kg_* — entity/predicate strings, counts - mempalace_check_duplicate — wing/room/preview only Changes: - mempalace/mcp_server.py: tool_get_drawer() now basenames metadata.source_file - tests/test_mcp_server.py: regression test asserting the absolute path and its parent directory do not appear anywhere in the response - website/reference/mcp-tools.md: clarify the documented return shape
This commit is contained in:
@@ -531,6 +531,45 @@ class TestWriteTools:
|
||||
result = tool_get_drawer("nonexistent_drawer")
|
||||
assert "error" in result
|
||||
|
||||
def test_get_drawer_does_not_leak_absolute_source_file_path(
|
||||
self, monkeypatch, config, palace_path, collection, kg
|
||||
):
|
||||
"""tool_get_drawer must not expose the absolute filesystem path
|
||||
that the miners write into ``source_file``. Same threat class as
|
||||
the palace_path leak in mempalace_status: in nested-agent or
|
||||
multi-server MCP topologies the client is a separate trust
|
||||
domain, and the directory layout of the host has no documented
|
||||
client-side use. Basename is enough for citation."""
|
||||
_patch_mcp_server(monkeypatch, config, kg)
|
||||
|
||||
secret_dir = "/private/home/alice/secret-research/2026"
|
||||
absolute_source = f"{secret_dir}/notes.md"
|
||||
collection.add(
|
||||
ids=["drawer_leak_probe"],
|
||||
documents=["verbatim drawer body for leak probe"],
|
||||
metadatas=[
|
||||
{
|
||||
"wing": "research",
|
||||
"room": "notes",
|
||||
"source_file": absolute_source,
|
||||
"chunk_index": 0,
|
||||
"added_by": "miner",
|
||||
"filed_at": "2026-05-03T00:00:00",
|
||||
}
|
||||
],
|
||||
)
|
||||
|
||||
from mempalace.mcp_server import tool_get_drawer
|
||||
|
||||
result = tool_get_drawer("drawer_leak_probe")
|
||||
assert result["drawer_id"] == "drawer_leak_probe"
|
||||
assert result["metadata"]["source_file"] == "notes.md"
|
||||
# Defense-in-depth: no field anywhere in the response should
|
||||
# contain the absolute path or its parent directory.
|
||||
serialized = json.dumps(result)
|
||||
assert absolute_source not in serialized
|
||||
assert secret_dir not in serialized
|
||||
|
||||
def test_list_drawers(self, monkeypatch, config, palace_path, seeded_collection, kg):
|
||||
_patch_mcp_server(monkeypatch, config, kg)
|
||||
from mempalace.mcp_server import tool_list_drawers
|
||||
|
||||
Reference in New Issue
Block a user