Skip to content

fix(mcp): refuse config and ack writes in read-only mode (#2101) - #2103

Open
mvalentsev wants to merge 1 commit into
MemPalace:developfrom
mvalentsev:fix/2101-read-only-gate
Open

fix(mcp): refuse config and ack writes in read-only mode (#2101)#2103
mvalentsev wants to merge 1 commit into
MemPalace:developfrom
mvalentsev:fix/2101-read-only-gate

Conversation

@mvalentsev

Copy link
Copy Markdown
Contributor

What does this PR do?

Closes #2101.

Read-only mode gated on _MUTATING_TOOLS, which is the palace-write set: the peer-writer lease uses it to decide which calls need the palace mine lock. Two tools change state without touching the palace, so they are rightly absent from it and were served on a --read-only server anyway:

  • mempalace_hook_settings rewrites ~/.mempalace/config.json via MempalaceConfig.set_hook_setting. service.WRITE_TOOLS already classifies it as a write, and the daemon uses that as an allowlist, so read-only was the odd one out.
  • mempalace_memories_filed_away unlinks ~/.mempalace/hook_state/last_checkpoint on both branches. Consuming that file is the tool's contract and it takes no arguments, but it still deletes state that outlives the process for a client with no write access.

This adds _READ_ONLY_REFUSED_TOOLS and points the dispatch gate and the tools/list filter at it. _MUTATING_TOOLS and the peer-writer path are untouched.

#1930 fixed this class by adding the missing names to _MUTATING_TOOLS directly, which worked because checkpoint and delete_by_source are palace writes. Neither tool here is, so that move would put them under the lease: with a peer holding the lock, this server would refuse mempalace_hook_settings with -32001. test_peer_writer_guard_does_not_gate_hook_settings pins that it does not.

To find the rest of the class I grepped every write primitive in mcp_server.py, mapped each to its handler, then read the handlers outside _MUTATING_TOOLS and followed them into config.py, knowledge_graph.py and backends/. Nothing else of that shape came up. mempalace_reconnect stays reachable on purpose: it is not write-free either, since clearing ChromaBackend._quarantined_paths lets the reopen rename a segment directory, but it is the only way a read-only server picks up an external writer's changes, and _SQLITE_INTEGRITY_ALLOWED_TOOLS already keeps it reachable for recovery.

Three costs, stated rather than buried:

  • The gate is by tool name, and mempalace_hook_settings with no arguments is a read ("Call with no args to view"). A read-only server can no longer report its own hook settings, and there is no CLI equivalent. tools/list cannot advertise a tool as half-available, so hiding and refusing keeps both sides symmetric. Happy to make the gate argument-aware instead if you prefer.
  • integrations/openclaw/SKILL.md tells agents to call mempalace_memories_filed_away at session start, so that now no-ops under read-only. That file describes the full 36-tool surface, which read-only exists to reduce, and every other hidden tool breaks its instructions the same way.
  • The set means "refuse what a client asked to change", not "nothing past here writes": opening the palace or the knowledge graph materialises files on its own, which no name-based gate can express.

The two --read-only help strings and the matching row in the remote-server guide said "the mutating tools", which now names the narrower set, so they say "the tools that change state" instead.

How to test

With ~/.mempalace/config.json holding {"hooks": {"desktop_toast": false}}:

mempalace serve --read-only --host 127.0.0.1 --port 8765 &

curl -s localhost:8765/mcp -H 'Content-Type: application/json' \
  -d '{"jsonrpc":"2.0","id":1,"method":"tools/list"}' | grep -c mempalace_hook_settings

curl -s localhost:8765/mcp -H 'Content-Type: application/json' \
  -d '{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"mempalace_hook_settings","arguments":{"desktop_toast":true}}}'

cat ~/.mempalace/config.json

Before: prints 1, the call succeeds, desktop_toast flips to true. After: prints 0, the call returns -32003, the file is byte-identical. Same over stdio (mempalace-mcp --read-only) and with MEMPALACE_MCP_READ_ONLY=1.

python -m pytest tests/test_mcp_http_transport.py tests/test_mcp_server.py -v

Five tests are new. The two named after the tools each make the same call twice, once with the gate off to show the write lands and once with it on to show the file is byte-identical, so neither survival assertion can pass vacuously. test_read_only_refuses_exactly_the_refused_set asks the gate which tools it refuses and compares that with the set, which also catches a renamed tool leaving a dead name behind. test_read_only_refuses_every_daemon_write_tool keeps read-only from being laxer than service.WRITE_TOOLS; it is the one that would have caught the original gap.

Checklist

  • Tests pass (python -m pytest tests/ -v)
  • No hardcoded paths
  • Linter passes (ruff check .)

)

Read-only gated on _MUTATING_TOOLS, which is the palace-write set the
peer-writer lease consults to decide which calls need the palace mine
lock. Two tools change state without touching the palace, so they are
correctly absent from that set and were served on a --read-only server
anyway: mempalace_hook_settings rewrites ~/.mempalace/config.json via
MempalaceConfig.set_hook_setting, and mempalace_memories_filed_away
unlinks ~/.mempalace/hook_state/last_checkpoint on both branches.

Add _READ_ONLY_REFUSED_TOOLS and point the dispatch gate and the
tools/list filter at it. _MUTATING_TOOLS and the peer-writer path are
unchanged: adding the two names there instead would put a config-only
tool under the palace lease, so a server that lost the lease to a peer
would answer -32001 for a call the lease has no say over.

mempalace_reconnect stays reachable on purpose and the comment records
why, since clearing ChromaBackend._quarantined_paths lets the reopen
rename a segment directory. The two --read-only help strings and the
matching row in the remote-server guide said "the mutating tools",
which now names the narrower set.
@mvalentsev
mvalentsev marked this pull request as ready for review July 29, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP --read-only gates on the palace-write set, so mempalace_hook_settings still rewrites config.json

1 participant