fix: security hardening, performance, and input validation

[anthonyonazure]

Summary

Comprehensive audit of the MCP server surface area, addressing security, performance, and functional issues:

• Security (5 fixes): Sanitize all KG tool inputs via sanitize_name(), validate date/direction params, replace WAL plaintext content previews with SHA-256 hashes, sanitize error messages returned to MCP clients to prevent leaking system paths and DB internals
• Performance (2 fixes): Add 2-second TTL metadata cache so status/wings/rooms/taxonomy share one collection scan instead of four independent 10K fetches; pass cached ChromaDB collection to search_memories() instead of creating a new PersistentClient per call
• Functional (4 fixes): Fix drawer ID hash collision (hash full content instead of content[:100]), remove duplicate cache variable initialization, replace all silent except: pass with logger.error(), expose query_relationship() as mempalace_kg_query_relationship MCP tool

Files changed

File	Changes
mempalace/mcp_server.py	Input validation, metadata cache, WAL hashing, error sanitization, new tool
mempalace/searcher.py	Accept optional collection param, sanitize error message
tests/conftest.py	Clear metadata cache between tests
tests/test_mcp_server.py	+16 new tests (input validation, error sanitization, WAL, cache, new tool)
tests/test_searcher.py	Update error message assertion to match sanitized output

Breaking changes

• WAL log entries now use content_hash instead of content_preview — tools that parse the WAL for content previews will need updating
• search_memories() has a new optional collection parameter (backwards compatible)
• Error messages returned to MCP clients are now generic (no longer contain raw exception text)

Test plan

• All 554 tests pass (up from 534 — 16 new tests added)
• Ruff lint clean
• Verified path traversal, null bytes, invalid dates, and invalid direction all rejected
• Verified drawer ID uniqueness with same-prefix different-suffix content
• Verified WAL no longer contains plaintext content
• Verified error messages don't leak system paths
• Verified metadata cache invalidates on writes
• Verified mempalace_kg_query_relationship tool registered and functional

[web3guru888]

Comprehensive audit with real fixes across security, performance, and correctness. This is exactly the kind of hardening the MCP surface needed.

Approved with one minor concern flagged below.

---

What's solid:

• Metadata cache (_META_CACHE, 2s TTL): Smart approach. Coalescing status/wings/rooms/taxonomy from four independent 10K scans into one cached fetch is a real win for installations with large palaces. Cache invalidation on tool_add_drawer, tool_delete_drawer, and tool_diary_write looks complete — all three write paths correctly call _invalidate_meta_cache().

• drawer_id hash over full content (removing [:100] truncation): This was a latent collision bug. Two drawers differing only after character 100 in the same wing/room would have been silently overwritten. Correct fix.

• WAL content → SHA-256 hash: Right call. WAL files are often left world-readable and could contain sensitive content. The test in TestWALSecurity covers exactly the right scenario.

• Error message sanitization: "Search failed" / "Duplicate check failed" / "Failed to file drawer" are generic without being useless. Good pattern.

• _validate_date + sanitize_name on KG tools: Path traversal rejection tested, date format validated. The direction enum check is clean.

• tool_kg_query_relationship: Useful new tool — querying by predicate is a common pattern and it was missing. Validated inputs, follows the same count/facts response shape as tool_kg_query.

• collection=col passed to search_memories: Avoids creating a second PersistentClient per search call, which was the root cause of the WAL flush issue in #538.

One concern:

tool_list_rooms previously pushed where={"wing": wing} to ChromaDB when a wing filter was specified. The new version fetches all metadata from cache and filters in Python. For the common case (single wing query, 10K drawer palace) this is slower — you load all metadata then discard most of it. The cache helps when tool_list_rooms is called right after tool_status, but not when called standalone with a wing argument.

Suggest keeping the ChromaDB where filter for the non-cached path when wing is specified, or accepting this as a known tradeoff in the PR description (the 2s TTL means it'll often be cached anyway in burst-query scenarios).

Not a blocker — correctness is preserved. Just worth documenting.

---

260 new test lines covering the new behaviors: input validation, cache invalidation, WAL hashing, error sanitization, and the new relationship query tool. That's the right way to ship a hardening PR.

[web3guru888]

Good set of hardening changes — a few notes from our integration work:

Metadata cache — the concept is solid. Calling tool_status(), tool_list_wings(), and tool_list_rooms() in the same agent loop currently triggers redundant full-collection scans, so caching the result for 2s is the right shape. One caveat: _get_all_metadata() still hard-codes limit=10000, so the cache is caching a potentially-truncated snapshot rather than a complete one. For palaces beyond 10K drawers this doesn't fix the underlying bug — it just makes the wrong answer faster. The paginated fetch pattern from #484/_fetch_all_metadata() should be the data source here. Cache the complete result, not the clipped one.

Cache invalidation — _invalidate_meta_cache() called after writes is exactly right. Stale cache after an add/upsert is a nasty class of bug (agent lists rooms, doesn't see the one it just wrote, writes again). This is the part of the implementation I'd prioritize keeping.

Security: error message sanitization — good call. Leaking ChromaDB stack traces or raw file paths to MCP clients is a real concern in multi-tenant or remotely-hosted setups. The sanitized surface should also cover the new sanitize_name() call sites — make sure malformed names don't bubble back through exception paths.

add() → upsert() in convo_miner.py — same fix as mrdeeme's #542 and the pattern correction in #298. This one keeps appearing across forks, which suggests it should be part of the official migration guide somewhere ("if you're on <0.6, you were calling add() — switch to upsert()").

Overlap with #540/#542 — there's meaningful surface overlap with #540 (KG path injection fix) and #542 (comprehensive hardening including the same upsert fix + Unicode normalization). Worth coordinating with those authors or the maintainer to decide which changes land where, so they don't conflict on merge.

[bensig]

Overlaps with #387 (security hardening) and #542. The KG sanitization and WAL hash changes are good ideas — if #542 doesn't cover them, a focused follow-up PR would be welcome.