fix: sanitize topic parameter in tool_diary_write#936
Merged
igorls merged 1 commit intoMemPalace:developfrom Apr 26, 2026
Merged
fix: sanitize topic parameter in tool_diary_write#936igorls merged 1 commit intoMemPalace:developfrom
igorls merged 1 commit intoMemPalace:developfrom
Conversation
agent_name and entry are validated via sanitize_name/sanitize_content, but topic is stored raw into ChromaDB metadata. Apply the same sanitize_name guard to reject null bytes, path traversal, and oversized payloads.
Contributor
Author
bensig
approved these changes
Apr 26, 2026
Collaborator
bensig
left a comment
There was a problem hiding this comment.
+1 — exact fix shaun0927 proposed in #1154. Same defensive pattern as wing/room in tool_add_drawer. Diff is +1/-0; the new sanitize_name(topic, "topic") runs alongside the existing agent_name and entry sanitizers in the same try/except, so ValueError handling is unchanged.
Ran the existing test_mcp_server.py suite locally on develop: 64 passed. CI matrix should match.
Ready to merge.
jphein
added a commit
to jphein/mempalace
that referenced
this pull request
Apr 26, 2026
Upstream PR MemPalace#936 (@shaun0927, commit 5bf8260) added sanitize_name(topic, "topic") to tool_diary_write before metadata write — null-byte / path-traversal / size guards. The 2026-04-26 upstream/develop sync brought the commit in, but the merge resolution in our `tool_diary_write` (which had been modified by the checkpoint-split phases A–D) kept our version and dropped the sanitize line. Verified non-mangling for our routing values: sanitize_name passes "checkpoint", "auto-save", "CHECKPOINT", "general" through unchanged, so the `if topic in _CHECKPOINT_TOPICS` check below remains correct. Suite 1395/1395. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Scorpion1221
pushed a commit
to Scorpion1221/mempalace
that referenced
this pull request
May 2, 2026
Scorpion1221
pushed a commit
to Scorpion1221/mempalace
that referenced
this pull request
May 2, 2026
Sync with upstream/develop via cherry-pick of 9 critical bugfixes: - HNSW index bloat prevention (MemPalace#1191) - SIGSEGV guards on collection reopen (MemPalace#1262, MemPalace#1289) - blob-seq marker fast-path (MemPalace#1177) - palace_graph None-metadata guard (MemPalace#1201) - palace_graph security tunnels (MemPalace#1168) - hyphenated wing name normalization (MemPalace#1197) - searcher _tokenize None guard (MemPalace#1198) - mcp_server diary topic sanitize (MemPalace#936) Tests: 1459 default + 113 benchmark + 6/7 stress all pass. (random-kill stress test was failing on dev pre-merge; not regressed.)
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
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.
What does this PR do?
tool_diary_writevalidatesagent_nameviasanitize_name()(line 926) andentryviasanitize_content()(line 927), but storestopicraw into ChromaDB metadata at line 967. Any string — including null bytes, path traversal sequences, or megabyte-length payloads — passes through unchecked.This is consistent with the input validation pattern already applied to every other MCP tool parameter (e.g.
wing,room,agent_nameintool_add_drawer;subject/predicate/objectintool_kg_addviasanitize_kg_value).Refs: #934 (Finding 4)
How to test
Checklist
ruff check mempalace/mcp_server.py— passedruff format --check mempalace/mcp_server.py— passed