fix: security hardening — shell injection, SSL bypass, error leaks, read-only MCP mode#34
Closed
kushjoy wants to merge 1 commit intoMemPalace:developfrom
Closed
fix: security hardening — shell injection, SSL bypass, error leaks, read-only MCP mode#34kushjoy wants to merge 1 commit intoMemPalace:developfrom
kushjoy wants to merge 1 commit intoMemPalace:developfrom
Conversation
IgorTavcar
added a commit
to IgorTavcar/mempalace
that referenced
this pull request
Apr 7, 2026
Cherry-picked from upstream PR MemPalace#34 (kushjoy). - Fix shell injection in mempal_save_hook.sh (env var instead of interpolation) - Remove SSL certificate verification bypass in benchmarks - Remove hardcoded personal API key paths from benchmark code - Replace str(e) in MCP error responses with generic messages (prevents path leakage) - Replace truncated MD5 IDs with uuid4 (collision safety) - Add --read-only MCP flag to block write tools from untrusted clients - Add PII-sensitive files to .gitignore Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
adv3nt3
added a commit
to adv3nt3/mempalace
that referenced
this pull request
Apr 7, 2026
Add usedforsecurity=False to hashlib.md5() calls in miner.py and convo_miner.py to document that MD5 is used for deterministic ID generation, not cryptographic security. Preserves stable drawer IDs for backward compatibility with existing palaces. Swapping to SHA-256 would change the ID formula and make existing drawers unreachable on re-ingestion. PR MemPalace#34 covers the MD5 sites in knowledge_graph.py and mcp_server.py. Verified: usedforsecurity kwarg is supported since Python 3.9 (project target per pyproject.toml line 10), confirmed via Context7 CPython docs.
5 tasks
Collaborator
|
Thanks for the thorough security audit! A few of these have already been merged separately:
The remaining changes are valuable though — especially removing the ssl bypass in benchmarks, the hardcoded key file paths in longmemeval_bench.py, and the .gitignore additions. Could you split those into a smaller focused PR? The current diff overlaps too much with merged work and would be hard to land cleanly. |
This was referenced Apr 8, 2026
kushjoy
force-pushed
the
fix/security-hardening
branch
from
4d7f9b8 to
87300ed
Compare
Author
|
@bensig noted. Focused security fixes — no overlap with #114 or #62.
|
Collaborator
|
@kushjoy 1 conflict to fix pls |
Collaborator
|
@kushjoy pls rebase |
kushjoy
force-pushed
the
fix/security-hardening
branch
from
87300ed to
a0a7a06
Compare
Author
|
done |
web3guru888
approved these changes
Apr 11, 2026
web3guru888
left a comment
web3guru888
left a comment
There was a problem hiding this comment.
🔧 Review of #34 — fix: security hardening — shell injection, SSL bypass, error leaks, read-only MCP mode
Scope: +9/−33 · 3 file(s)
.gitignore(modified: +5/−0)benchmarks/convomem_bench.py(modified: +0/−4)benchmarks/longmemeval_bench.py(modified: +4/−29)
🟢 Approved — clean, well-structured PR. Good work @kushjoy!
🏛️ Reviewed by MemPalace-AGI · Autonomous research system with perfect memory · Showcase: Truth Palace of Atlantis
- convomem_bench.py: drop ssl._create_unverified_context bypass; SSL validation should be configured at the environment/CA level - longmemeval_bench.py: remove ~/.config/lu/keys.json fallback; API keys must come from --llm-key arg or ANTHROPIC_API_KEY env var only - .gitignore: exclude entities.json, known_names.json, identity.txt, *.sqlite3, *.db to prevent accidental credential/data commits Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
kushjoy
force-pushed
the
fix/security-hardening
branch
from
a0a7a06 to
b48e6cc
Compare
Author
|
small update pushed for gitignore to rebase. @bensig lets merge :) |
Collaborator
|
Closing — this is superseded by recently merged PRs to develop. Thank you for the contribution! |
Merged
7 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
3 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.
Summary
Security hardening across 6 files, addressing findings from a full codebase audit.
hooks/mempal_save_hook.sh):$TRANSCRIPT_PATHwas string-interpolated directly into an inline Python script — a path containing a single quote couldinject arbitrary code. Fixed by passing it via env var.
$SESSION_IDsanitized withtrbefore use as a filename component to prevent path traversal.benchmarks/convomem_bench.py):ssl._create_unverified_contextwas globally disabling certificate verification, enabling MITM on all HTTPS connectionsincluding API key transmission. Removed.
benchmarks/longmemeval_bench.py): Hardcoded personal key names (lu_key,anthropic_milla,anthropic_claude_code_main) and a personalconfig path (
~/.config/lu/keys.json) were published in the repo. Simplified_load_api_key()to use only the standardANTHROPIC_API_KEYenv var.mempalace/mcp_server.py):str(e)was returned directly in MCP responses, exposing internal file paths. Replaced with generic messages; realerrors still logged server-side.
mempalace/mcp_server.py): Added--read-onlyflag that blocks all write tools (mempalace_add_drawer,mempalace_kg_add,mempalace_diary_write)at the dispatch layer, mitigating memory poisoning from untrusted MCP clients.
mempalace/mcp_server.py,mempalace/knowledge_graph.py): MD5 truncated to 16 hex chars was used for ID generation — collisions could cause silentdata overwrites. Replaced with
uuid4..gitignoregaps: Addedentities.json,known_names.json,identity.txt,*.sqlite3,*.dbto prevent personal data files from being accidentally committed.Test plan
pytest tests/ -v)