fix(mcp): omit absolute filesystem paths from MCP tool responses#1325
Merged
fix(mcp): omit absolute filesystem paths from MCP tool responses#1325
Conversation
The MCP `mempalace_status` tool was returning the server's absolute `_config.palace_path` to any connected client on both the main (ChromaDB-backed) path and the sqlite fallback path that runs when HNSW divergence is detected (#1222). 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 absolute path has no documented client-side use. Clients that legitimately need the palace path continue to have three documented channels: the `MEMPALACE_PALACE_PATH` env var (primary) or its legacy `MEMPAL_PALACE_PATH` alias, the `~/.mempalace/config.json` file, and the `--palace` CLI flag on most subcommands. Also corrects stale docs that claimed `mempalace_reconnect` returned a `palace_path` field; the code returns `{success, message, drawers, vector_disabled[, vector_disabled_reason]}` on success, plus a no-palace shape and an exception shape. - mempalace/mcp_server.py: drop palace_path from tool_status() and _tool_status_via_sqlite() result dicts - website/reference/mcp-tools.md: update documented return shapes for mempalace_status (fix) and mempalace_reconnect (stale-docs correction) Authored-by: Aaron Salsitz (ICCI LLC, @icciaaron). Claude Code was used as an authoring and review-orchestration tool, with human-in-the-loop oversight at every step: Aaron wrote the prompts, reviewed each draft, called for three independent review passes (drafting / post-rebase technical / CISA-aligned disclosure-leak), and verified the final patch behavior before commit.
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
xcarbo
added a commit
to xcarbo/mempalace
that referenced
this pull request
May 4, 2026
Catches up on a month of upstream work. Highlights pulled in: - MemPalace#1306 searcher: hybrid candidate union (vector ∪ BM25 reranking pool) - MemPalace#1325 mcp security: omit absolute paths from tool_get_drawer / tool_status - MemPalace#1322 chroma: wire quarantine_stale_hnsw to prevent SIGSEGV on stale HNSW - MemPalace#1320 mcp: forward valid_to / source params in kg_add / kg_invalidate - MemPalace#1321 cli: honor --palace flag in cmd_init - MemPalace#1314 kg temporal params fix - MemPalace#1244 cli: cmd_compress writes to mempalace_closets so palace can read - MemPalace#1243 mcp: case-insensitive agent name in diary_write/diary_read - MemPalace#1303 mcp_server: pass embedding_function= on collection reopen - MemPalace#1076/MemPalace#1077 hooks: quote CLAUDE_PLUGIN_ROOT / CODEX_PLUGIN_ROOT in hooks.json - Various ruff format passes on touched files Conflict resolution (CHANGELOG.md only — code files all auto-merged): - 3.3.5 unreleased section header from upstream kept above 3.3.4 - 3.3.4 section: kept our 2026-04-30 release date; merged upstream's new MemPalace#1299 SIGSEGV-on-default-EF entry in alongside our existing topic-tunnels (MemPalace#1194/MemPalace#1195/MemPalace#1197), HNSW-bloat (MemPalace#1191), max_seq_id (MemPalace#1135), and auto-ingest (MemPalace#1230/MemPalace#1231) entries. Kept our richer topic-tunnels detail (upstream's version was a strict subset). xdev patches preserved (still on this branch, untouched by merge): - 6ef44cb fix(hooks): route CC transcripts via convo_miner with cwd-based wings - 3fad61d fix(config): allow leading dash in wing names Not pushed to origin — run tests locally and decide when to push. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two related fixes for absolute filesystem paths leaking from MCP tool responses to connected clients. 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.
Originally drafted on a private GHSA fork; bundled and brought to the public repo because the GHSA repo's PR/merge UI is currently broken on GitHub's side. No exploitation in the wild — disclosure path is local-only and tied to MCP topology.
Fixes
mempalace_statuspalace_path(absolute palace dir) on both ChromaDB and #1222 sqlite-fallback pathsmempalace_get_drawermetadata.source_file(absolute path written by the miners)Path(source_file).namebefore returning, mirroring whatsearcher.search_memories()already doesDocumented client-side channels for clients that legitimately need the palace path are unchanged: the
MEMPALACE_PALACE_PATHenv var (or its legacyMEMPAL_PALACE_PATHalias),~/.mempalace/config.json, and the--palaceCLI flag.Audit of remaining MCP tools
I scanned every entry in
TOOLS. None of these leak host paths:mempalace_search— already basenamessource_fileand strips the internal_source_file_fullfieldmempalace_list_drawers— wing/room/content_preview only, no metadatamempalace_diary_read— date/timestamp/topic/content onlymempalace_reconnect— success/message/drawers/vector_disabled onlymempalace_kg_*— entity strings, predicates, countsmempalace_check_duplicate— wing/room/preview onlymempalace_traverse_graph,mempalace_find_tunnels,mempalace_graph_stats,mempalace_kg_stats— graph data onlyadd_drawer,update_drawer,delete_drawer,kg_add,kg_invalidate,diary_write) — drawer_id/wing/room/triple_id onlymempalace_hook_settings— booleansmempalace_memories_filed_away— count/timestamp onlyStale-docs correction (folded in)
mempalace_reconnectwas documented as returning{success, palace_path}— actually returns four distinct shapes ({success, message, drawers, vector_disabled[, vector_disabled_reason]}on success, plus no-palace and exception variants).website/reference/mcp-tools.mdupdated to match.Commits
b2f259c— fix(mcp): omit palace_path from tool_status responses (+ docs) — authored by @icciaaron7fc260f— fix(mcp): basename source_file in tool_get_drawer responses — companion fix from the same audit passTest results
uv run pytest tests/ -q --ignore=tests/benchmarks→ 1471 passed, 1 skippeduvx --from 'ruff>=0.4.0,<0.5' ruff checkandruff format --checkon touched files → cleantest_get_drawer_does_not_leak_absolute_source_file_pathasserts neither the absolute path nor its parent directory appears anywhere in the JSON-serialised response