Skip to content

fix(mcp)!: trim context, format LLM-readable, scope identity to host (#1278)#1381

Open
alighazi288 wants to merge 2 commits into
MemMachine:mainfrom
alighazi288:fix/issue-1278-mcp-scoping-and-formatting
Open

fix(mcp)!: trim context, format LLM-readable, scope identity to host (#1278)#1381
alighazi288 wants to merge 2 commits into
MemMachine:mainfrom
alighazi288:fix/issue-1278-mcp-scoping-and-formatting

Conversation

@alighazi288

Copy link
Copy Markdown
Contributor

Purpose of the change

Resolves context bloat and identity spoofing vulnerabilities in the MCP server, specifically addressing the two hard requirements raised in the thread for issue #1278:

  1. LLM-readable formatting
  2. Scoped handle / security

Description

This patch resolves the security and context issues in #1278.

Changes

  • Identity Scoping (Security) Removed org_id, proj_id, and user_id parameters from mcp_add_memory, mcp_search_memory, and mcp_delete_memory signatures. Identity is now strictly host-bound via env vars or HTTP headers. Because FastMCP automatically derives its input-schema from these signatures, spoof attempts are now outright rejected with a ToolError rather than silently dropped.

  • Context Trimming (Performance) mcp_search_memory now returns a token-efficient, formatted string using internal helpers (_format_episode_line, _format_episodic_section, _format_semantic_section) instead of a raw Pydantic JSON dump.

  • Housekeeping Fixed mcp_search_memory default top_k from 20 to 5 to match the docstring.

Compatibility Shield

REST API Unchanged. Programmatic clients keep the structured SearchResult.

MCP Hosts Breaking. Hosts passing identity as MCP tool arguments will now receive a ToolError. Hosts that rely on MM_*_ID env vars or org-id/proj-id/user-id request headers keep working unchanged.

Fixes/Closes

Fixes #1278

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Security (improves security without changing functionality)

How Has This Been Tested?

  • Unit Test

  • Added host_identity_env fixture to simulate host-bound identity.

  • Verified FastMCP schema validation rejects attempted spoofs and the downstream service is never invoked.

  • Guarded the security contract at both Python-signature and wire-schema levels.

  • Verified new return types and ensured episodic + semantic sections render correctly without leaking UIDs, scores, or metadata IDs.

Test Results:

uv run ruff check packages/server/src/memmachine_server/server/api_v2/mcp.py \
                  packages/server/server_tests/memmachine_server/server/api_v2/test_mcp.py
uv run ruff format --check ...
uv run complexipy packages/server/src/memmachine_server/server/api_v2/mcp.py
uv run pytest packages/server/server_tests/memmachine_server/server/api_v2/ -v
# 255 passed (28 in test_mcp.py). 1182-test server suite + 229-test client suite remain green.

Checklist

  • I have signed the commit(s) within this pull request
  • My code follows the style guidelines of this project (See STYLE_GUIDE.md)
  • I have performed a self-review of my own code
  • I have commented my code
  • My changes generate no new warnings
  • I have added unit tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have checked my code and corrected any misspellings

Maintainer Checklist

  • Confirmed all checks passed
  • Contributor has signed the commit(s)
  • Reviewed the code
  • Run, Tested, and Verified the change(s) work as expected

Further comments

This implements a hard removal of identity parameters rather than a soft-deprecation. This follows the explicit direction from issue #1297 to "disregard backward compatibility" for the MCP server, instantly closing the identity spoofing surface.

…emMachine#1278)

Resolves context bloat and identity spoofing vulnerabilities in the MCP server.

- Identity Scoping: Removed org_id, proj_id, and user_id parameters from mcp_add_memory, mcp_search_memory, and mcp_delete_memory. Identity is now strictly host-bound via env vars or HTTP headers. FastMCP now outright rejects LLM spoof attempts with a ToolError.
- Context Trimming: mcp_search_memory now returns a token-efficient, formatted string using internal helpers instead of a raw Pydantic JSON dump.
- Housekeeping: Fixed mcp_search_memory default top_k from 20 to 5 to match documented intent.

BREAKING CHANGE: MCP tool signatures no longer accept identity parameters. Hosts passing these via the LLM will receive a ToolError and must migrate to MM_*_ID env vars or request headers.

Closes MemMachine#1278

Signed-off-by: alighazi288 <51366992+alighazi288@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens MCP security by preventing per-call identity spoofing (org/project/user) and reduces LLM context bloat by returning a compact, LLM-readable string from search_memory instead of a verbose structured payload.

Changes:

  • Removed org_id / proj_id / user_id tool arguments from MCP tool signatures so identity is host-bound (env vars in stdio; HTTP headers in HTTP mode).
  • Changed mcp_search_memory to return a compact formatted string (episodic + semantic sections) and reduced default top_k from 20 → 5.
  • Added/updated unit tests to assert schema-level rejection of spoofed identity params and to validate formatted output.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
packages/server/src/memmachine_server/server/api_v2/mcp.py Removes identity args from MCP tools; adds LLM-readable formatting helpers; trims search_memory output and default top_k.
packages/server/server_tests/memmachine_server/server/api_v2/test_mcp.py Adds fixtures and tests for host-bound identity, schema rejection of spoof params, and formatted string output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +139 to +143
return (
f"[{date_str} at {time_str}] {episode.producer_id}: "
f"{json.dumps(episode.content)}"
)
return f"{episode.producer_id}: {json.dumps(episode.content)}"
Comment on lines +167 to +170
structured: dict[str, dict[str, str]] = {}
for feature in features:
structured.setdefault(feature.tag, {})[feature.feature_name] = feature.value
return "[Semantic Memory]\n" + json.dumps(structured)
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.

[Bug]: Context creeping, even after simple search,

3 participants