mirror of
https://github.com/anthropics/claude-plugins-official.git
synced 2026-03-20 11:33:08 +00:00
fix(plugin): mcp-server-dev — correct APIs against spec, add missing primitives
Corrects fabricated/deprecated APIs: ext-apps App class model (not embedded resources), real MCPB v0.4 manifest (no permissions block exists), registerTool (not server.tool), @anthropic-ai/mcpb package name, CIMD preferred over DCR. Adds missing spec coverage: resources, prompts, elicitation (with capability check + fallback), sampling, roots, tool annotations, structured output, instructions field, progress/cancellation.
This commit is contained in:
@@ -1,8 +1,10 @@
|
||||
# Local MCP Security
|
||||
|
||||
An MCPB server runs as the user, with whatever permissions the manifest was granted. Claude drives it. That combination means: **tool inputs are untrusted**, even though they come from an AI the user trusts. A prompt-injected web page can make Claude call your `delete_file` tool with a path you didn't intend.
|
||||
**MCPB provides no sandbox.** There's no `permissions` block in the manifest, no filesystem scoping, no network allowlist enforced by the platform. The server process runs with the user's full privileges — it can read any file the user can, spawn any process, hit any network endpoint.
|
||||
|
||||
Defense in depth. Manifest permissions are the outer wall; validation in your tool handlers is the inner wall.
|
||||
Claude drives it. That combination means: **tool inputs are untrusted**, even though they come from an AI the user trusts. A prompt-injected web page can make Claude call your `delete_file` tool with a path you didn't intend.
|
||||
|
||||
Your tool handlers are the only defense. Everything below is about building that defense yourself.
|
||||
|
||||
---
|
||||
|
||||
@@ -39,6 +41,41 @@ def safe_join(root: Path, user_path: str) -> Path:
|
||||
|
||||
---
|
||||
|
||||
## Roots — ask the host, don't hardcode
|
||||
|
||||
Before hardcoding `ROOT` from a config env var, check if the host supports `roots/list`. This is the spec-native way to get user-approved workspace boundaries.
|
||||
|
||||
```typescript
|
||||
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
|
||||
|
||||
const server = new McpServer({ name: "...", version: "..." });
|
||||
|
||||
let allowedRoots: string[] = [];
|
||||
server.server.oninitialized = async () => {
|
||||
const caps = server.getClientCapabilities();
|
||||
if (caps?.roots) {
|
||||
const { roots } = await server.server.listRoots();
|
||||
allowedRoots = roots.map(r => new URL(r.uri).pathname);
|
||||
} else {
|
||||
allowedRoots = [process.env.ROOT_DIR ?? process.cwd()];
|
||||
}
|
||||
};
|
||||
```
|
||||
|
||||
```python
|
||||
# fastmcp — inside a tool handler
|
||||
async def my_tool(ctx: Context) -> str:
|
||||
try:
|
||||
roots = await ctx.list_roots()
|
||||
allowed = [urlparse(r.uri).path for r in roots]
|
||||
except Exception:
|
||||
allowed = [os.environ.get("ROOT_DIR", os.getcwd())]
|
||||
```
|
||||
|
||||
If roots are available, use them. If not, fall back to config. Either way, validate every path against the allowed set.
|
||||
|
||||
---
|
||||
|
||||
## Command injection
|
||||
|
||||
If you spawn processes, **never pass user input through a shell**.
|
||||
@@ -62,11 +99,13 @@ Split read and write into separate tools. Most workflows only need read. A tool
|
||||
```
|
||||
list_files ← safe to call freely
|
||||
read_file ← safe to call freely
|
||||
write_file ← separate tool, separate permission, separate scrutiny
|
||||
write_file ← separate tool, separate scrutiny
|
||||
delete_file ← consider not shipping this at all
|
||||
```
|
||||
|
||||
If you ship write/delete, consider requiring a confirmation widget (see `build-mcp-app`) so the user explicitly approves each destructive call.
|
||||
Pair this with tool annotations — `readOnlyHint: true` on every read tool, `destructiveHint: true` on delete/overwrite tools. Hosts surface these in permission UI (auto-approve reads, confirm-dialog destructive). See `../build-mcp-server/references/tool-design.md`.
|
||||
|
||||
If you ship write/delete, consider requiring explicit confirmation via elicitation (see `../build-mcp-server/references/elicitation.md`) or a confirmation widget (see `build-mcp-app`) so the user approves each destructive call.
|
||||
|
||||
---
|
||||
|
||||
@@ -94,7 +133,7 @@ Same for directory listings (cap entry count), search results (cap matches), and
|
||||
|
||||
## Secrets
|
||||
|
||||
- **Config secrets** (`secret: true` in manifest): host stores in OS keychain, delivers via env var. Don't log them. Don't include them in tool results.
|
||||
- **Config secrets** (`sensitive: true` in manifest `user_config`): host stores in OS keychain, delivers via env var. Don't log them. Don't include them in tool results.
|
||||
- **Never store secrets in plaintext files.** If the host's keychain integration isn't enough, use `keytar` (Node) / `keyring` (Python) yourself.
|
||||
- **Tool results flow into the chat transcript.** Anything you return, the user (and any log export) can see. Redact before returning.
|
||||
|
||||
@@ -104,8 +143,7 @@ Same for directory listings (cap entry count), search results (cap matches), and
|
||||
|
||||
- [ ] Every path parameter goes through containment check
|
||||
- [ ] No `exec()` / `shell=True` — `execFile` / array-argv only
|
||||
- [ ] Write/delete split from read tools
|
||||
- [ ] Write/delete split from read tools; `readOnlyHint`/`destructiveHint` annotations set
|
||||
- [ ] Size caps on file reads, listing lengths, search results
|
||||
- [ ] Manifest permissions match actual code behavior (no over-requesting)
|
||||
- [ ] Secrets never logged or returned in tool results
|
||||
- [ ] Tested with adversarial inputs: `../../etc/passwd`, `; rm -rf ~`, 10GB file
|
||||
|
||||
Reference in New Issue
Block a user