-
Notifications
You must be signed in to change notification settings - Fork 136
feat: add security and reliability improvements #26
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
a0b47ab
387f46a
30360f9
a69c3fc
6ad0f5c
5d8e5d6
bd29d1f
ca3dbf5
fe9169f
08cb2a2
c7d9262
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| __pycache__/ | ||
| *.py[cod] | ||
| *$py.class | ||
| *.so | ||
| .Python | ||
| build/ | ||
| develop-eggs/ | ||
| dist/ | ||
| downloads/ | ||
| eggs/ | ||
| .eggs/ | ||
| lib/ | ||
| lib64/ | ||
| parts/ | ||
| sdist/ | ||
| var/ | ||
| wheels/ | ||
| *.egg-info/ | ||
| .installed.cfg | ||
| *.egg | ||
| .env | ||
| .venv | ||
| env/ | ||
| venv/ | ||
| .juno_task/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,10 +44,39 @@ Or with pip: | |
| pip install mem0-mcp-server | ||
| ``` | ||
|
|
||
| ### Provisional Deployment (from GitHub) | ||
|
|
||
| You can deploy directly from GitHub without installing: | ||
|
|
||
| ```json | ||
| { | ||
| "mcpServers": { | ||
| "mem0": { | ||
| "command": "uvx", | ||
| "args": [ | ||
| "--from", | ||
| "git+https://github.com/alfonsodg/[email protected]", | ||
| "mem0-mcp-server", | ||
| "--api-key=YOUR_MEM0_API_KEY", | ||
| "--user-id=your-user-id" | ||
| ], | ||
| "tools": ["*"] | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| Replace `YOUR_MEM0_API_KEY` with your actual Mem0 API key from https://app.mem0.ai | ||
|
|
||
| **Why CLI arguments instead of env vars?** | ||
| GitHub Copilot CLI (and some other MCP clients) have issues with environment variables in MCP server configurations. Using CLI arguments (`--api-key`, `--user-id`) ensures compatibility across all MCP clients. | ||
|
|
||
| ### Client Configuration | ||
|
|
||
| Add this configuration to your MCP client: | ||
|
|
||
| **Option 1: Using environment variables (recommended)** | ||
|
|
||
| ```json | ||
| { | ||
| "mcpServers": { | ||
|
|
@@ -63,6 +92,23 @@ Add this configuration to your MCP client: | |
| } | ||
| ``` | ||
|
|
||
| **Option 2: Using command-line arguments (for CLIs that don't support env)** | ||
|
|
||
| ```json | ||
| { | ||
| "mcpServers": { | ||
| "mem0": { | ||
| "command": "uvx", | ||
| "args": [ | ||
| "mem0-mcp-server", | ||
| "--api-key=m0-...", | ||
| "--user-id=your-handle" | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### Test with the Python Agent | ||
|
|
||
| <details> | ||
|
|
@@ -117,11 +163,19 @@ The Mem0 MCP server enables powerful memory capabilities for your AI application | |
|
|
||
| ### Environment Variables | ||
|
|
||
| - `MEM0_API_KEY` (required) – Mem0 platform API key. | ||
| - `MEM0_API_KEY` (required if not using `--api-key`) – Mem0 platform API key. | ||
| - `MEM0_DEFAULT_USER_ID` (optional) – default `user_id` injected into filters and write requests (defaults to `mem0-mcp`). | ||
| - `MEM0_ENABLE_GRAPH_DEFAULT` (optional) – Enable graph memories by default (defaults to `false`). | ||
| - `MEM0_DISABLE_DNS_REBINDING_PROTECTION` (optional) – Disable DNS rebinding protection for local development (defaults to `false`, protection enabled). | ||
| - `MEM0_MCP_AGENT_MODEL` (optional) – default LLM for the bundled agent example (defaults to `openai:gpt-4o-mini`). | ||
|
|
||
| ### Command-Line Arguments | ||
|
|
||
| - `--api-key` – Mem0 API key (overrides `MEM0_API_KEY` env var) | ||
| - `--user-id` – Default user ID (overrides `MEM0_DEFAULT_USER_ID` env var) | ||
|
|
||
| CLI arguments take precedence over environment variables. | ||
|
|
||
| ## Advanced Setup | ||
|
|
||
| <details> | ||
|
|
@@ -206,6 +260,9 @@ mem0-mcp-server | |
| # Or with uv | ||
| uv sync | ||
| uv run mem0-mcp-server | ||
|
|
||
| # Run tests | ||
| pytest | ||
| ``` | ||
|
|
||
| </details> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,8 +3,8 @@ requires = ["hatchling>=1.27.0"] | |||||
| build-backend = "hatchling.build" | ||||||
|
|
||||||
| [project] | ||||||
| name = "mem0-mcp-server" | ||||||
| version = "0.2.1" | ||||||
| name = "mem0-mcp-server-alfonsodg" | ||||||
|
||||||
| name = "mem0-mcp-server-alfonsodg" | |
| name = "mem0-mcp-server" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| [pytest] | ||
| testpaths = tests | ||
| python_files = test_*.py | ||
| python_classes = Test* | ||
| python_functions = test_* | ||
| addopts = -v --tb=short |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,9 +2,12 @@ | |||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| import argparse | ||||||||||||||||||||||||||||||||
| import json | ||||||||||||||||||||||||||||||||
| import logging | ||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||
| import time | ||||||||||||||||||||||||||||||||
| from functools import lru_cache | ||||||||||||||||||||||||||||||||
| from typing import Annotated, Any, Callable, Dict, Optional, TypeVar | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| from dotenv import load_dotenv | ||||||||||||||||||||||||||||||||
|
|
@@ -14,26 +17,15 @@ | |||||||||||||||||||||||||||||||
| from mem0.exceptions import MemoryError | ||||||||||||||||||||||||||||||||
| from pydantic import Field | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| try: # Support both package (`python -m mem0_mcp.server`) and script (`python mem0_mcp/server.py`) runs. | ||||||||||||||||||||||||||||||||
| from .schemas import ( | ||||||||||||||||||||||||||||||||
| AddMemoryArgs, | ||||||||||||||||||||||||||||||||
| ConfigSchema, | ||||||||||||||||||||||||||||||||
| DeleteAllArgs, | ||||||||||||||||||||||||||||||||
| DeleteEntitiesArgs, | ||||||||||||||||||||||||||||||||
| GetMemoriesArgs, | ||||||||||||||||||||||||||||||||
| SearchMemoriesArgs, | ||||||||||||||||||||||||||||||||
| ToolMessage, | ||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||
| except ImportError: # pragma: no cover - fallback for script execution | ||||||||||||||||||||||||||||||||
| from schemas import ( | ||||||||||||||||||||||||||||||||
| AddMemoryArgs, | ||||||||||||||||||||||||||||||||
| ConfigSchema, | ||||||||||||||||||||||||||||||||
| DeleteAllArgs, | ||||||||||||||||||||||||||||||||
| DeleteEntitiesArgs, | ||||||||||||||||||||||||||||||||
| GetMemoriesArgs, | ||||||||||||||||||||||||||||||||
| SearchMemoriesArgs, | ||||||||||||||||||||||||||||||||
| ToolMessage, | ||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||
| from .schemas import ( | ||||||||||||||||||||||||||||||||
| AddMemoryArgs, | ||||||||||||||||||||||||||||||||
| ConfigSchema, | ||||||||||||||||||||||||||||||||
| DeleteAllArgs, | ||||||||||||||||||||||||||||||||
| DeleteEntitiesArgs, | ||||||||||||||||||||||||||||||||
| GetMemoriesArgs, | ||||||||||||||||||||||||||||||||
| SearchMemoriesArgs, | ||||||||||||||||||||||||||||||||
| ToolMessage, | ||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| load_dotenv() | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
|
@@ -60,6 +52,10 @@ def decorator(func: Callable[..., T]) -> Callable[..., T]: # type: ignore[type- | |||||||||||||||||||||||||||||||
| smithery = _SmitheryFallback() # type: ignore[assignment] | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| # CLI arguments override environment variables | ||||||||||||||||||||||||||||||||
| _CLI_API_KEY: Optional[str] = None | ||||||||||||||||||||||||||||||||
| _CLI_DEFAULT_USER_ID: Optional[str] = None | ||||||||||||||||||||||||||||||||
|
Comment on lines
+55
to
+57
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| # graph remains off by default , also set the default user_id to "mem0-mcp" when nothing set | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
| # graph remains off by default , also set the default user_id to "mem0-mcp" when nothing set | |
| # graph remains off by default, also set the default user_id to "mem0-mcp" when nothing set |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API key validation only checks for the "m0-" prefix and a minimum length of 10 characters. This is a weak validation that could accept malformed keys like "m0-1234567" (which meets the requirements but may not be a valid format). Consider implementing more robust validation, such as checking for the expected total length or format of actual Mem0 API keys, or documenting the rationale for this minimal validation approach.
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API key validation is performed inside the cached function, which means that invalid API keys will be cached by the LRU cache decorator. Once an invalid key is validated and raises an exception, subsequent calls with the same invalid key will retrieve the cached exception behavior. However, since lru_cache caches based on function arguments and the function raises an exception (doesn't return a value), this actually won't cache the exception itself - it will re-raise it each time. But this design is inefficient: validation should occur before attempting to cache. Consider validating the API key before calling the cached client creation function.
| def _mem0_client(api_key: str) -> MemoryClient: | |
| """Create and cache MemoryClient instances with LRU eviction.""" | |
| if not api_key.startswith("m0-") or len(api_key) < 10: | |
| raise ValueError("Invalid MEM0_API_KEY format. Expected format: m0-...") | |
| return MemoryClient(api_key=api_key) | |
| def _cached_mem0_client(api_key: str) -> MemoryClient: | |
| """Create and cache MemoryClient instances with LRU eviction.""" | |
| return MemoryClient(api_key=api_key) | |
| def _mem0_client(api_key: str) -> MemoryClient: | |
| """Validate the API key, then obtain a cached MemoryClient instance.""" | |
| if not api_key.startswith("m0-") or len(api_key) < 10: | |
| raise ValueError("Invalid MEM0_API_KEY format. Expected format: m0-...") | |
| return _cached_mem0_client(api_key) |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The retry logic only retries on MemoryError exceptions with 5xx status codes. However, network-related errors (connection timeouts, DNS failures, etc.) that don't result in a MemoryError with a status code will not be retried. The comment on line 118 mentions "or network issues" but the code doesn't actually handle generic network exceptions. Consider catching additional exception types like requests.exceptions.RequestException or adding a broader exception handler for true network resilience.
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DNS rebinding protection is enabled by default (when ENV_DISABLE_DNS_REBINDING_PROTECTION is false), but the double-negative logic makes this confusing. The setting is named "DISABLE_DNS_REBINDING_PROTECTION" and then negated with "not", which is hard to reason about. Consider renaming the environment variable to MEM0_ENABLE_DNS_REBINDING_PROTECTION and removing the negation, or add a comment explaining the logic clearly.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| # Tests | ||
|
|
||
| ## Running Tests | ||
|
|
||
| ```bash | ||
| # Run all tests | ||
| pytest | ||
|
|
||
| # Run with verbose output | ||
| pytest -v | ||
|
|
||
| # Run specific test | ||
| pytest tests/test_basic.py::TestAPIKeyValidation | ||
| ``` | ||
|
|
||
| ## Test Coverage | ||
|
|
||
| **test_basic.py** - Core logic validation (13 tests) | ||
| - API key format validation | ||
| - Default filter injection logic | ||
| - Enable graph default behavior | ||
| - CLI argument parsing format | ||
| - Retry logic calculations | ||
| - Error code detection (4xx vs 5xx) | ||
|
|
||
| ## Test Results | ||
|
|
||
| ``` | ||
| 13 passed in 0.43s | ||
| ``` | ||
|
|
||
| All tests validate the core business logic without requiring external dependencies. | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| """Tests for mem0-mcp-server.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation references GitHub repository "alfonsodg/mem0-mcp.git" and tag "v0.2.3", which appears to be a personal fork. If this PR is intended to be merged into the upstream repository, these references should point to the official repository instead. This is inconsistent with a PR being submitted for the main project.