Skip to content

fix: harden hooks, MCP server, and config (security audit) #893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
brodsky754 wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix: harden hooks, MCP server, and config (security audit) #893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions mempalace/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,9 @@ def set_entity_languages(self, languages):
self._file_config["entity_languages"] = normalized
self._config_dir.mkdir(parents=True, exist_ok=True)
try:
with open(self._config_file, "w", encoding="utf-8") as f:
# Owner-only create with no world-readable TOCTOU window.
fd = os.open(str(self._config_file), os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
with os.fdopen(fd, "w", encoding="utf-8") as f:
json.dump(self._file_config, f, indent=2, ensure_ascii=False)
except OSError:
pass
Expand Down Expand Up @@ -551,7 +553,9 @@ def set_embedding_model(self, model: str) -> None:
self._file_config["embedding_model"] = str(model).strip().lower()
self._config_dir.mkdir(parents=True, exist_ok=True)
try:
with open(self._config_file, "w", encoding="utf-8") as f:
# Owner-only create with no world-readable TOCTOU window.
fd = os.open(str(self._config_file), os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
with os.fdopen(fd, "w", encoding="utf-8") as f:
json.dump(self._file_config, f, indent=2, ensure_ascii=False)
except OSError:
pass
Expand Down Expand Up @@ -602,7 +606,9 @@ def set_hook_setting(self, key: str, value: bool):
self._file_config["hooks"] = {}
self._file_config["hooks"][key] = value
try:
with open(self._config_file, "w", encoding="utf-8") as f:
# Owner-only create with no world-readable TOCTOU window.
fd = os.open(str(self._config_file), os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
with os.fdopen(fd, "w", encoding="utf-8") as f:
json.dump(self._file_config, f, indent=2, ensure_ascii=False)
except OSError:
pass
Expand Down Expand Up @@ -630,13 +636,11 @@ def init(self):
"topic_wings": DEFAULT_TOPIC_WINGS,
"hall_keywords": DEFAULT_HALL_KEYWORDS,
}
with open(self._config_file, "w") as f:
# Create atomically with owner-only perms so there is never a
# window where the freshly written config is world-readable.
fd = os.open(str(self._config_file), os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600)
with os.fdopen(fd, "w") as f:
json.dump(default_config, f, indent=2)
# Restrict config file to owner read/write only
try:
self._config_file.chmod(0o600)
except (OSError, NotImplementedError):
pass
return self._config_file

def save_people_map(self, people_map):
Expand All @@ -646,7 +650,9 @@ def save_people_map(self, people_map):
people_map: Dict mapping name variants to canonical names.
"""
self._config_dir.mkdir(parents=True, exist_ok=True)
with open(self._people_map_file, "w") as f:
# Owner-only create with no world-readable TOCTOU window.
fd = os.open(str(self._people_map_file), os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
with os.fdopen(fd, "w") as f:
json.dump(people_map, f, indent=2)
try:
self._people_map_file.chmod(0o600)
Expand Down
60 changes: 37 additions & 23 deletions mempalace/mcp_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -769,9 +769,9 @@ def tool_status():
r = m.get("room", "unknown")
wings[w] = wings.get(w, 0) + 1
rooms[r] = rooms.get(r, 0) + 1
except Exception as e:
except Exception:
logger.exception("tool_status metadata fetch failed")
result["error"] = str(e)
result["error"] = "Internal error"
result["partial"] = True
return result

Expand Down Expand Up @@ -821,9 +821,9 @@ def tool_list_wings():
m = m or {}
w = m.get("wing", "unknown")
wings[w] = wings.get(w, 0) + 1
except Exception as e:
except Exception:
logger.exception("tool_list_wings metadata fetch failed")
result["error"] = str(e)
result["error"] = "Internal error"
result["partial"] = True
return result

Expand All @@ -845,9 +845,9 @@ def tool_list_rooms(wing: str = None):
m = m or {}
r = m.get("room", "unknown")
rooms[r] = rooms.get(r, 0) + 1
except Exception as e:
except Exception:
logger.exception("tool_list_rooms metadata fetch failed")
result["error"] = str(e)
result["error"] = "Internal error"
result["partial"] = True
return result

Expand All @@ -867,9 +867,9 @@ def tool_get_taxonomy():
if w not in taxonomy:
taxonomy[w] = {}
taxonomy[w][r] = taxonomy[w].get(r, 0) + 1
except Exception as e:
except Exception:
logger.exception("tool_get_taxonomy metadata fetch failed")
result["error"] = str(e)
result["error"] = "Internal error"
result["partial"] = True
return result

Expand Down Expand Up @@ -1235,8 +1235,15 @@ def tool_add_drawer(
"chunks": len(chunk_ids),
"chunk_ids": chunk_ids,
}
except Exception as e:
except RuntimeError as e:
# Readback-failure is raised deliberately with a fixed, actionable
# message (no internal details interpolated) — surface it so callers
# can self-repair (reconnect / repair) rather than see a blank error.
logger.exception("tool_add_drawer readback failed")
return {"success": False, "error": str(e)}
except Exception:
logger.exception("tool_add_drawer failed")
return {"success": False, "error": "Internal error"}


def tool_delete_drawer(drawer_id: str):
Expand Down Expand Up @@ -1268,8 +1275,9 @@ def tool_delete_drawer(drawer_id: str):
_metadata_cache = None
logger.info(f"Deleted drawer: {drawer_id}")
return {"success": True, "drawer_id": drawer_id}
except Exception as e:
return {"success": False, "error": str(e)}
except Exception:
logger.exception("tool_delete_drawer failed")
return {"success": False, "error": "Internal error"}


def tool_sync(project_dir: str = None, wing: str = None, apply: bool = False):
Expand Down Expand Up @@ -1337,8 +1345,9 @@ def tool_get_drawer(drawer_id: str):
"room": safe_meta.get("room", ""),
"metadata": safe_meta,
}
except Exception as e:
return {"error": str(e)}
except Exception:
logger.exception("tool_get_drawer failed")
return {"error": "Internal error"}


def tool_list_drawers(wing: str = None, room: str = None, limit: int = 20, offset: int = 0):
Expand Down Expand Up @@ -1396,8 +1405,9 @@ def tool_list_drawers(wing: str = None, room: str = None, limit: int = 20, offse
"offset": offset,
"limit": limit,
}
except Exception as e:
return {"error": str(e)}
except Exception:
logger.exception("tool_list_drawers failed")
return {"error": "Internal error"}


def tool_update_drawer(drawer_id: str, content: str = None, wing: str = None, room: str = None):
Expand Down Expand Up @@ -1465,8 +1475,9 @@ def tool_update_drawer(drawer_id: str, content: str = None, wing: str = None, ro
"wing": new_meta.get("wing", ""),
"room": new_meta.get("room", ""),
}
except Exception as e:
return {"success": False, "error": str(e)}
except Exception:
logger.exception("tool_update_drawer failed")
return {"success": False, "error": "Internal error"}


# ==================== KNOWLEDGE GRAPH ====================
Expand Down Expand Up @@ -1721,8 +1732,9 @@ def tool_diary_write(agent_name: str, entry: str, topic: str = "general", wing:
"chunks": len(chunk_ids),
"chunk_ids": chunk_ids,
}
except Exception as e:
return {"success": False, "error": str(e)}
except Exception:
logger.exception("tool_diary_write failed")
return {"success": False, "error": "Internal error"}


def tool_diary_read(agent_name: str, last_n: int = 10, wing: str = ""):
Expand Down Expand Up @@ -1811,8 +1823,9 @@ def tool_hook_settings(silent_save: bool = None, desktop_toast: bool = None):

try:
config = MempalaceConfig()
except Exception as e:
return {"success": False, "error": str(e)}
except Exception:
logger.exception("tool_hook_settings failed")
return {"success": False, "error": "Internal error"}

changed = []
if silent_save is not None:
Expand Down Expand Up @@ -1958,8 +1971,9 @@ def tool_reconnect():
"vector_disabled": _vector_disabled,
"vector_disabled_reason": _vector_disabled_reason,
}
except Exception as e:
return {"success": False, "error": str(e)}
except Exception:
logger.exception("tool_reconnect failed")
return {"success": False, "error": "Internal error"}


# ==================== MCP PROTOCOL ====================
Expand Down