Skip to content

Make the MCP server project-aware to prevent cross-project data leaks#519

Open
frozenraindrop wants to merge 1 commit into
nashsu:mainfrom
frozenraindrop:mcp-project-guidance
Open

Make the MCP server project-aware to prevent cross-project data leaks#519
frozenraindrop wants to merge 1 commit into
nashsu:mainfrom
frozenraindrop:mcp-project-guidance

Conversation

@frozenraindrop

Copy link
Copy Markdown

What this fixes

When multiple projects exist, an AI assistant using the llm-wiki MCP server had no clear signal about which project it should be working with. That opened the door for a request meant for one project to accidentally pull data from a different one, without ever checking in with the user first.

What changed

  1. Added usage guidance for AI assistants. The MCP server now tells connecting AI clients, up front: always use the currently active project unless the user says otherwise, and never poke around in a different project without asking first.
  2. Added a visible warning when it matters. When there's more than one project registered, the status/projects responses now include a clear warning naming the active project and reminding the assistant to check with the user before targeting a different one — right at the moment it's making that decision, not buried in documentation.

What this doesn't fix yet

These two changes rely on the assistant reading and following the guidance — they're a strong nudge, not a hard guarantee. A separate, more involved fix (binding a session to one project, or having the server ask the user directly when it's unclear) would close this properly. That's being filed as a follow-up issue rather than bundled into this PR.

Testing

Type-checked and built cleanly (tsc --noEmit, npm run build). No behavior changes for the single-project case.

@chuenchen309 chuenchen309 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.

Read through this and checked it out locally — the design is sound and the cross-project-leak concern is real (the MCP tools otherwise expose every registered project's contents with no signal about which one is authoritative).

A few things I verified:

  • instructions is a valid ServerOptions field in the SDK version pinned here, so the server-level guidance is delivered through the supported channel rather than a custom hack.
  • The { ...health, ...projects, ...multiProjectWarning(...) } spread is safe — neither the health payload nor the projects response carries a warning key, so nothing gets clobbered.
  • multiProjectWarning correctly no-ops for the single-project case (length <= 1), so existing single-project users see zero behavior change.
  • tsc --noEmit is clean and the existing test suite still passes.

The layering is nice: server-level instructions + the per-tool project_id: 'current' default enforce the isolation mechanically, and the warning on status/projects is a visible belt-and-suspenders on exactly the two discovery tools where an agent would first notice multiple projects exist.

LGTM from my read.

@chuenchen309

Copy link
Copy Markdown
Contributor

Following up on my own note about where the warning lands — thinking about it more, there's one coverage gap worth considering rather than the two enumeration tools being sufficient on their own.

The warning only surfaces on llm_wiki_status and llm_wiki_projects, but the tools that actually read cross-project data — files, file_content, reviews, search, chat, graph, rescan — all take project_id and say nothing when it resolves to a non-current project. An assistant that jumps straight to search/chat (or whose client doesn't surface instructions, which is advisory) gets no signal at the exact moment it crosses the boundary.

projectId(args) is already the single choke point for all seven, so returning the same warning from there would put it where the decision is actually made. Not blocking — the current PR is a real improvement as-is — just where I'd extend it next.

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.

2 participants