chore: add mcp server skills - #636
Conversation
📝 WalkthroughWalkthroughAdds two documentation skills for building remote and bundled MCP servers, covering design workflows, deployment, authentication, primitives, tool contracts, packaging, validation, and local security. ChangesMCP server guidance
MCPB packaging
Estimated code review effort: 3 (Moderate) | ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoAdd MCP server development skills (remote HTTP + MCPB packaging)
AI Description
Diagram
High-Level Assessment
Files changed (13)
|
Unit Test Results 1 files 32 suites 3s ⏱️ Results for commit 2e71974. ♻️ This comment has been updated with latest results. |
![gitstream-cm[bot]](https://avatars.githubusercontent.com/in/230441?s=60&v=4){kind=small}
There was a problem hiding this comment.
✨ PR Review
The PR adds two MCP server skill trees with comprehensive documentation. The content is well-structured and technically accurate, but there are a few concrete issues to address before merging.
3 issues detected:
🔒 Security - Both scaffold tool handlers pass untrusted `path` input directly to `join(ROOT, path)` with no traversal guard, making path escapes like `../../etc/passwd` trivially exploitable in any server built from this template.
Details: The list_files and read_file scaffold tool handlers use join(ROOT, path) directly without any path-containment check, even though the skill's own text (and references/local-security.md) explicitly identifies this as the #1 local MCP bug. Developers routinely copy scaffold code verbatim; shipping insecure scaffold is effectively shipping a vulnerable template.
File: .agents/skills/build-mcpb/SKILL.md (101-118)
🐞 Bug - The `computedHash` in `skills-lock.json` and the locally committed skill files can silently diverge, making the lock untrustworthy as an integrity check.
Details: skills-lock.json records both new skills (build-mcp-server, build-mcpb) as sourced from anthropics/claude-plugins-official on GitHub, with computedHash values that must match the remote files. The actual skill files are simultaneously committed locally under .agents/skills/. If the local content diverges from what is at those remote paths — or if the hashes were computed against a different revision — the lock file verification will fail or silently track the wrong content.
File: skills-lock.json (4-15)
🧹 Maintainability - No changeset file is present despite the project policy that every change — including docs and internal tooling — must include one.
Details: Project rules require a changeset for every source-code or dependency change — at minimum npx changeset --empty for internal-only additions like skill documentation. No .changeset/ file appears in this PR's diff, which will break the release pipeline's changeset check.
File: skills-lock.json (1-21)
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds two new skills, build-mcp-server and build-mcpb, along with detailed reference guides for Model Context Protocol (MCP) server development and packaging. The review comments identify several critical API mismatches and security issues in the code examples: connecting the transport inside the Express handler will fail on subsequent requests, the McpServer class lacks getClientCapabilities() and elicitInput() methods, and path.resolve does not resolve symlinks for path traversal checks. These issues should be addressed to ensure the guides are accurate and secure.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| app.post("/mcp", async (req, res) => { | ||
| const transport = new StreamableHTTPServerTransport({ | ||
| sessionIdGenerator: undefined, // stateless | ||
| }); | ||
| res.on("close", () => transport.close()); | ||
| await server.connect(transport); | ||
| await transport.handleRequest(req, res, req.body); | ||
| }); |
There was a problem hiding this comment.
In @modelcontextprotocol/sdk, an McpServer instance can only be connected to a single transport at a time. Calling server.connect(transport) inside the Express request handler will throw an error on any subsequent request because the server is already connected. Additionally, calling transport.close() on the response close event would close the transport for the entire server.
The correct pattern is to instantiate the StreamableHTTPServerTransport globally, connect the server to it once, and then call transport.handleRequest(req, res, req.body) inside the route handler.
| app.post("/mcp", async (req, res) => { | |
| const transport = new StreamableHTTPServerTransport({ | |
| sessionIdGenerator: undefined, // stateless | |
| }); | |
| res.on("close", () => transport.close()); | |
| await server.connect(transport); | |
| await transport.handleRequest(req, res, req.body); | |
| }); | |
| const transport = new StreamableHTTPServerTransport({ | |
| sessionIdGenerator: undefined, // stateless | |
| }); | |
| await server.connect(transport); | |
| app.post("/mcp", async (req, res) => { | |
| await transport.handleRequest(req, res, req.body); | |
| }); |
| server.registerTool("delete_all", { | ||
| description: "Delete all items after confirmation", | ||
| inputSchema: {}, | ||
| }, async ({}, extra) => { | ||
| const caps = server.getClientCapabilities(); | ||
| if (caps?.elicitation) { | ||
| const r = await server.elicitInput({ | ||
| mode: "form", | ||
| message: "Delete all items? This cannot be undone.", | ||
| requestedSchema: { | ||
| type: "object", | ||
| properties: { confirm: { type: "boolean", title: "Confirm deletion" } }, | ||
| required: ["confirm"], | ||
| }, | ||
| }); | ||
| if (r.action === "accept" && r.content?.confirm) { | ||
| await deleteAll(); | ||
| return { content: [{ type: "text", text: "Deleted." }] }; | ||
| } | ||
| return { content: [{ type: "text", text: "Cancelled." }] }; | ||
| } | ||
| // Fallback: return text asking Claude to relay the question | ||
| return { content: [{ type: "text", text: "Confirmation required. Please ask the user: 'Delete all items? This cannot be undone.' Then call this tool again with their answer." }] }; | ||
| }); |
There was a problem hiding this comment.
In the official @modelcontextprotocol/sdk (TypeScript), the McpServer class does not expose getClientCapabilities() or elicitInput() methods. Additionally, the extra parameter in the tool handler only provides an AbortSignal (as extra.signal) and does not grant access to the underlying session or client capabilities.
Using these non-existent methods will result in runtime errors. If elicitation is a proposed or custom extension, please clarify which SDK version or wrapper is required, or provide the correct way to access the session/client capabilities using the standard SDK (e.g., by using the lower-level Server class instead of McpServer).
| ```typescript | ||
| const caps = server.getClientCapabilities(); | ||
| if (caps?.roots) { | ||
| const { roots } = await server.server.listRoots(); | ||
| // roots: [{ uri: "file:///home/user/project", name: "My Project" }] |
There was a problem hiding this comment.
In @modelcontextprotocol/sdk, the McpServer class does not have a getClientCapabilities() method. Instead, the client capabilities are stored as a property on the underlying Server instance, which can be accessed via server.server.clientCapabilities after initialization.
| ```typescript | |
| const caps = server.getClientCapabilities(); | |
| if (caps?.roots) { | |
| const { roots } = await server.server.listRoots(); | |
| // roots: [{ uri: "file:///home/user/project", name: "My Project" }] | |
| const caps = server.server.clientCapabilities; | |
| if (caps?.roots) { | |
| const { roots } = await server.server.listRoots(); | |
| // roots: [{ uri: "file:///home/user/project", name: "My Project" }] | |
| } |
| } | ||
| ``` | ||
|
|
||
| `resolve` normalizes `..`, symlink segments, etc. `relative` tells you if the result left the root. Don't just `String.includes("..")` — that misses encoded and symlink-based escapes. |
There was a problem hiding this comment.
The statement that path.resolve normalizes symlink segments is incorrect. path.resolve() only performs string-based path resolution (resolving . and .. segments) and does not access the filesystem to resolve symlinks. If a user-controlled path points to a symlink that targets a location outside the root, path.resolve and path.relative will not detect the escape.
To safely handle symlinks, you should use fs.realpath (or fs.promises.realpath) on the resolved path before performing the containment check.
| 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()]; | ||
| } | ||
| }; | ||
| ``` |
There was a problem hiding this comment.
In @modelcontextprotocol/sdk, the McpServer class does not have a getClientCapabilities() method. Instead, the client capabilities are stored as a property on the underlying Server instance, which can be accessed via server.server.clientCapabilities after initialization.
| 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()]; | |
| } | |
| }; | |
| ``` | |
| let allowedRoots: string[] = []; | |
| server.server.oninitialized = async () => { | |
| const caps = server.server.clientCapabilities; | |
| 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()]; | |
| } | |
| }; |
|
Fixed the failing — charlied/pr-check-repair |
Code Review by Qodo
1. Missing build-mcp-app skill
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| 1. **Remote HTTP, no UI** → Scaffold inline using `references/remote-http-scaffold.md` (portable) or `references/deploy-cloudflare-workers.md` (fastest deploy). This skill can finish the job. | ||
| 2. **MCP app (UI widgets)** → Summarize the decisions so far, then load the **`build-mcp-app`** skill. | ||
| 3. **MCPB (bundled local)** → Summarize the decisions so far, then load the **`build-mcpb`** skill. |
There was a problem hiding this comment.
1. Missing build-mcp-app skill 🐞 Bug ≡ Correctness
build-mcp-server and build-mcpb instruct a handoff to a build-mcp-app skill, but skills-lock.json does not register any build-mcp-app entry, so the handoff cannot resolve. This breaks the documented “MCP app / widgets” workflow path and leaves versions.md referencing files that do not exist in this repo.
Agent Prompt
### Issue description
The docs for `build-mcp-server` and `build-mcpb` instruct the runtime to load a `build-mcp-app` skill for UI/widget scenarios, but the repo does not register that skill in `skills-lock.json`. This creates an unresolved handoff path.
### Issue Context
If `build-mcp-app` is intended to exist, it should be added to `skills-lock.json` (and the corresponding skill content added under `.agents/skills/`). If it is intentionally out of scope for this repo, remove/replace the `build-mcp-app` handoff instructions and references (including in `versions.md`) so users aren’t directed to a missing skill.
### Fix Focus Areas
- skills-lock.json[1-20]
- .agents/skills/build-mcp-server/SKILL.md[43-47]
- .agents/skills/build-mcp-server/SKILL.md[159-166]
- .agents/skills/build-mcpb/SKILL.md[165-170]
- .agents/skills/build-mcp-server/references/versions.md[5-12]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| 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. |
There was a problem hiding this comment.
2. Broken relative doc links 🐞 Bug ⚙ Maintainability
local-security.md links to build-mcp-server references using ../build-mcp-server/..., which resolves to a non-existent path from .agents/skills/build-mcpb/references/. This causes the referenced security guidance links (tool design + elicitation) to be broken.
Agent Prompt
### Issue description
The relative paths in `.agents/skills/build-mcpb/references/local-security.md` are off by one directory level. From within `build-mcpb/references/`, `../build-mcp-server/...` points under `build-mcpb/` instead of sibling `build-mcp-server/`.
### Issue Context
Update these references to go up two levels (likely `../../build-mcp-server/...`) so they correctly resolve to `.agents/skills/build-mcp-server/references/...`.
### Fix Focus Areas
- .agents/skills/build-mcpb/references/local-security.md[106-108]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Tick the box to add this pull request to the merge queue (same as
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #636 +/- ##
=======================================
Coverage 92.94% 92.94%
=======================================
Files 35 35
Lines 1559 1559
Branches 392 392
=======================================
Hits 1449 1449
Misses 48 48
Partials 62 62 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Code Review by Qodo
1. Missing build-mcp-app skill
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| 1. **Remote HTTP, no UI** → Scaffold inline using `references/remote-http-scaffold.md` (portable) or `references/deploy-cloudflare-workers.md` (fastest deploy). This skill can finish the job. | ||
| 2. **MCP app (UI widgets)** → Summarize the decisions so far, then load the **`build-mcp-app`** skill. | ||
| 3. **MCPB (bundled local)** → Summarize the decisions so far, then load the **`build-mcpb`** skill. |
There was a problem hiding this comment.
1. Missing build-mcp-app skill 🐞 Bug ≡ Correctness
build-mcp-server instructs the agent to hand off to a build-mcp-app skill for UI widgets, but the repo (and skills-lock.json) does not define that skill, so the UI/widgets path cannot be completed when needed.
Agent Prompt
## Issue description
`build-mcp-server` directs users to load a `build-mcp-app` skill, but that skill is not present/registered. This creates a dead-end for the “MCP app (UI widgets)” route.
## Issue Context
The skill set added in this PR includes `build-mcp-server` and `build-mcpb`, and updates `skills-lock.json`, but does not include any `build-mcp-app` skill entry or files.
## Fix Focus Areas
- .agents/skills/build-mcp-server/SKILL.md[163-165]
- .agents/skills/build-mcp-server/references/versions.md[5-12]
- skills-lock.json[1-46]
## What to change
Choose one:
1) **Add the missing skill**: introduce `.agents/skills/build-mcp-app/SKILL.md` (+ needed references), and add a `build-mcp-app` entry in `skills-lock.json`.
2) **Remove the dependency**: update `build-mcp-server/SKILL.md` and `versions.md` to not reference `build-mcp-app`, and instead keep widget guidance inline or point to an existing skill/file that actually exists in this repo.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| You are guiding a developer through designing and building an MCP server that works seamlessly with Claude. MCP servers come in many forms — picking the wrong shape early causes painful rewrites later. Your first job is **discovery, not code**. | ||
|
|
||
| **Load Claude-specific context first.** The MCP spec is generic; Claude has additional auth types, review criteria, and limits. Before answering questions or scaffolding, fetch `https://claude.com/docs/llms-full.txt` (the full export of the Claude connector docs) so your guidance reflects Claude's actual constraints. | ||
|
|
||
| Do not start scaffolding until you have answers to the questions in Phase 1. If the user's opening message already answers them, acknowledge that and skip straight to the recommendation. |
There was a problem hiding this comment.
2. Hard network fetch requirement 🐞 Bug ☼ Reliability
build-mcp-server requires fetching https://claude.com/docs/llms-full.txt before proceeding, which can block the skill entirely in sandboxed/offline environments where outbound network isn’t available.
Agent Prompt
## Issue description
The skill mandates an external network fetch before it will provide guidance. In restricted environments this will fail and prevents the skill from operating.
## Issue Context
This repo already documents cases where network calls fail in sandboxed environments.
## Fix Focus Areas
- .agents/skills/build-mcp-server/SKILL.md[9-13]
- AGENTS.md[103-107]
## What to change
- Reword the instruction to: “If network access is available, fetch …; otherwise proceed using the bundled reference files in `references/`.”
- Optionally add a brief fallback checklist of the key Claude-specific constraints that the fetch was meant to capture, so the skill remains usable offline.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| 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. |
There was a problem hiding this comment.
3. Broken cross-skill references 🐞 Bug ⚙ Maintainability
local-security.md references ../build-mcp-server/... paths that don’t match the repo’s .agents/skills/<skill>/... layout, so the referenced files won’t be found when someone tries to follow them.
Agent Prompt
## Issue description
The doc uses incorrect relative paths to sibling skill docs (`../build-mcp-server/...`). Given the actual directory layout, these paths won’t resolve.
## Issue Context
Other cross-skill references in this PR use a repo-root-ish convention like `build-mcpb/references/local-security.md` (no `../`).
## Fix Focus Areas
- .agents/skills/build-mcpb/references/local-security.md[106-108]
## What to change
- Replace `../build-mcp-server/references/tool-design.md` with either:
- `build-mcp-server/references/tool-design.md` (consistent with other docs), or
- the correct relative path from `build-mcpb/references/` (likely `../../build-mcp-server/references/tool-design.md`).
- Do the same for the elicitation reference (`../build-mcp-server/references/elicitation.md`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
✨ PR Review
The PR adds two well-structured agent skills (build-mcp-server, build-mcpb) with thorough reference documentation. A changeset file has been added (resolving the previous concern), but two previously identified issues remain, and a new security gap exists in the remote HTTP scaffold.
2 issues detected:
🔒 Security - The scaffold omits the Origin header validation that its own checklist marks as a spec MUST, shipping a template that is vulnerable to DNS rebinding by default. 🛠️
Details: The Express scaffold for the /mcp endpoint does not validate the Origin header before processing requests. The file's own deployment checklist (line 206) explicitly flags this as a spec MUST requirement for DNS rebinding prevention. Developers copying this scaffold verbatim will ship a server that is vulnerable to DNS rebinding attacks, where a malicious page on a controlled domain can silently call tools on the user's locally-running MCP server.
File: .agents/skills/build-mcp-server/references/remote-http-scaffold.md (65-65)
🛠️ A suggested code correction is included in the review comments.
🔒 Security - `join(ROOT, path)` does not normalize `..` segments, so a path like `../../etc/passwd` escapes the configured root directory.
Details: The list_files and read_file scaffold tool handlers join the user-supplied path directly to ROOT via join(ROOT, path) without any path-containment check. The companion references/local-security.md (also in this PR) identifies this exact pattern as the #1 local MCP bug and provides a ready-made safeJoin helper. Shipping an insecure scaffold alongside the security reference that condemns it means developers who copy the scaffold but skim the reference will produce vulnerable servers.
File: .agents/skills/build-mcpb/SKILL.md (101-118)
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how
| const app = express(); | ||
| app.use(express.json()); | ||
|
|
||
| app.post("/mcp", async (req, res) => { |
There was a problem hiding this comment.
🔒 Security - Missing Origin Validation: Add an Origin check at the top of the app.post("/mcp", ...) handler. Accept requests only from an explicit allowlist (e.g. https://claude.ai, http://localhost) or reject any Origin that does not match the server's own host. For example:
const ALLOWED_ORIGINS = new Set([
"https://claude.ai",
`http://localhost:${process.env.PORT ?? 3000}`,
]);
app.post("/mcp", async (req, res) => {
const origin = req.headers.origin;
if (origin && !ALLOWED_ORIGINS.has(origin)) {
res.status(403).json({ error: "Forbidden origin" });
return;
}
// ... existing handler
});| app.post("/mcp", async (req, res) => { | |
| const ALLOWED_ORIGINS = new Set([ | |
| "https://claude.ai", | |
| `http://localhost:${process.env.PORT ?? 3000}`, | |
| ]); | |
| app.post("/mcp", async (req, res) => { | |
| const origin = req.headers.origin; | |
| if (origin && !ALLOWED_ORIGINS.has(origin)) { | |
| res.status(403).json({ error: "Forbidden origin" }); | |
| return; | |
| } |
Is this review accurate? Use 👍 or 👎 to rate it
If you want to tell us more, use /gs feedback e.g. /gs feedback this review doesn't make sense, I disagree, and it keeps repeating over and over
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/skills/build-mcp-server/references/auth.md:
- Around line 25-34: Update the “Tier 1: No auth / static API key” section to
identify UPSTREAM_API_KEY as upstream/server-to-server authentication, not
remote MCP client authentication. Remove or qualify the claim that it works for
remote servers, and explicitly require client authentication for remote
deployments or document the authless threat model.
- Around line 49-53: Add the text language tag to the untyped Markdown fences
containing the protocol diagram and examples:
.agents/skills/build-mcp-server/references/auth.md lines 49-53, and
.agents/skills/build-mcp-server/references/tool-design.md lines 23-27, 35-37,
and 45-48. Preserve the fenced content unchanged.
In @.agents/skills/build-mcp-server/references/deploy-cloudflare-workers.md:
- Line 100: Update the MyMCP class to provide a typed Env generic to McpAgent,
and change init() to read the upstream API key from this.env.UPSTREAM_API_KEY
instead of the out-of-scope env variable.
In @.agents/skills/build-mcp-server/references/elicitation.md:
- Around line 39-46: Update the confirmation handling around the existing
r.action branch to process accepted submissions before cancellation: return
“Deleted.” for confirm: true, and an explicit declined/not-deleted response for
confirm: false. Preserve “Cancelled.” only for cancellation or other
non-accepted actions, and leave the fallback prompt unchanged.
- Around line 27-46: The no-elicitation branch in the tool handler is not
executable because the empty tool schema cannot carry the user’s confirmation on
retry. Update the tool’s input schema and handler around the visible elicitation
flow to accept an explicit confirmation argument and perform deletion when
confirmed, or replace the fallback with a terminal response stating that
elicitation is required; apply the same correction to the corresponding second
example.
In @.agents/skills/build-mcp-server/references/remote-http-scaffold.md:
- Around line 39-41: Make both minimal working scaffolds runnable by defining or
injecting the undeclared upstream adapters: update the upstreamApi usage at
.agents/skills/build-mcp-server/references/remote-http-scaffold.md#L39-L41 and
the upstream_api usage at
.agents/skills/build-mcp-server/references/remote-http-scaffold.md#L97-L100 with
small stubs or factories, or explicitly mark each adapter as pseudocode and show
where users must provide it.
- Around line 65-71: Update the /mcp handler around
StreamableHTTPServerTransport to validate the request’s Origin against an
explicit allowlist before calling server.connect or transport.handleRequest.
Reject disallowed origins, while preserving dispatch for allowed requests and
the existing transport close cleanup.
- Around line 65-72: Update the `/mcp` route alongside the existing POST handler
to explicitly handle GET requests, returning the protocol-appropriate response
(or HTTP 405 if GET is unsupported) instead of leaving them to fall through to a
404. Preserve the current POST transport flow unchanged.
In @.agents/skills/build-mcp-server/references/resources-and-prompts.md:
- Around line 55-69: Update the JavaScript resource handler and Python read_file
example to define an approved workspace root, resolve the URI-derived path
against it, and verify the resolved path remains contained within that root
before reading. Reject traversal and symlink escapes, then read only the
validated resolved path.
In @.agents/skills/build-mcp-server/references/tool-design.md:
- Around line 137-149: Update both the TypeScript and Python delete_file
examples to include the required title and readOnlyHint annotations alongside
the existing destructiveHint and idempotentHint values. Set readOnlyHint
consistently with the destructive delete operation and provide a clear
human-readable title, preserving the rest of each example unchanged.
In @.agents/skills/build-mcp-server/references/versions.md:
- Around line 7-10: Update the CIMD/DCR status entry in the versions index to
reference lines 36, 40, and 57 in auth.md instead of 20, 24, and 41, leaving the
surrounding version and source entries unchanged.
In @.agents/skills/build-mcp-server/SKILL.md:
- Around line 117-122: Update both fenced code blocks in the build-MCP-server
skill documentation to include an explicit language identifier, using text or
another appropriate identifier, while preserving their existing contents.
- Around line 49-53: Split the “What auth does the upstream service use?”
section into distinct guidance for upstream API authentication and MCP-host
authorization. Keep upstream OAuth focused on the server’s client flow to the
upstream service, and move CIMD/DCR and the reference to auth.md under the MCP
server’s OAuth surface for Claude.
In @.agents/skills/build-mcpb/references/local-security.md:
- Around line 81-91: Update the execFile example to prevent Git option injection
by placing "--" before the untrusted branch argument, and add validation of
branch against an allowlisted ref format before invocation. Apply this guidance
to the surrounding CLI-wrapping instructions, preserving the no-shell
requirement.
- Around line 112-130: Update the resource-limit example around MAX_BYTES to
avoid calling readFile(path) before enforcing the limit: inspect file metadata
and use a bounded read or stream, while retaining a TOCTOU-safe maximum for
files that grow or change during reading. Preserve the existing oversized-file
response and truncated UTF-8 content behavior.
- Around line 53-75: Update the root authorization flow around allowedRoots to
convert file:// URIs with fileURLToPath instead of URL.pathname, and refresh the
allowed roots when the client’s roots change or per request so revoked roots are
no longer authorized. Preserve the existing ROOT_DIR/process.cwd fallback when
roots are unavailable, and continue validating every requested path against the
current allowed set.
In @.agents/skills/build-mcpb/references/manifest-schema.md:
- Around line 58-63: Update the “Substitution variables” section to include the
supported ${DESKTOP}, ${DOCUMENTS}, ${DOWNLOADS}, and ${pathSeparator} (or ${/})
variables alongside the existing entries, preserving the note that substitutions
apply only in args and env.
In @.agents/skills/build-mcpb/SKILL.md:
- Around line 135-149: Update the Node.js build commands in the JavaScript and
Python sections to avoid unpinned executable npx dependencies: invoke
lockfile-backed local binaries or specify exact package versions for both
esbuild and `@anthropic-ai/mcpb`, preserving the existing build and packaging
flow.
- Around line 94-119: Enforce canonical root containment for both filesystem
tools: in .agents/skills/build-mcpb/SKILL.md at lines 94-119, replace direct
join(ROOT, path) usage in list_files and read_file with the shared safe path
helper before readdir/readFile; in
.agents/skills/build-mcpb/references/local-security.md at lines 18-28, update
safeJoin to realpath both the configured root and candidate and reject
candidates outside the canonical root, including symlink escapes.
- Around line 7-13: Update
.agents/skills/build-mcpb/references/manifest-schema.md at lines 50-56 to
include uv in the server.type contract and clarify that mcp_config is the launch
configuration. Update .agents/skills/build-mcpb/SKILL.md at lines 7-13 so its
runtime and packaging guidance documents the same supported node, python, uv,
and binary modes; both sites require direct documentation changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2bd1a45a-ad8f-4ad1-b59f-d165cd4823d6
📒 Files selected for processing (14)
.agents/skills/build-mcp-server/SKILL.md.agents/skills/build-mcp-server/references/auth.md.agents/skills/build-mcp-server/references/deploy-cloudflare-workers.md.agents/skills/build-mcp-server/references/elicitation.md.agents/skills/build-mcp-server/references/remote-http-scaffold.md.agents/skills/build-mcp-server/references/resources-and-prompts.md.agents/skills/build-mcp-server/references/server-capabilities.md.agents/skills/build-mcp-server/references/tool-design.md.agents/skills/build-mcp-server/references/versions.md.agents/skills/build-mcpb/SKILL.md.agents/skills/build-mcpb/references/local-security.md.agents/skills/build-mcpb/references/manifest-schema.md.changeset/pretty-teeth-jam.mdskills-lock.json
| ### Tier 1: No auth / static API key | ||
|
|
||
| Server reads a key from env. User provides it once at setup. Done. | ||
|
|
||
| ```typescript | ||
| const apiKey = process.env.UPSTREAM_API_KEY; | ||
| if (!apiKey) throw new Error("UPSTREAM_API_KEY not set"); | ||
| ``` | ||
|
|
||
| Works for local stdio, MCPB, and remote servers alike. If this is all you need, stop here. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not present an upstream API key as remote MCP authentication.
The example only authenticates the server to an upstream service; it does not validate incoming MCP clients. As written, “works for … remote servers” could lead to an exposed remote endpoint protected only by a shared server credential. Label this as upstream/server-to-server authentication and require client authentication for remote deployments, or explicitly document the authless threat model.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/build-mcp-server/references/auth.md around lines 25 - 34,
Update the “Tier 1: No auth / static API key” section to identify
UPSTREAM_API_KEY as upstream/server-to-server authentication, not remote MCP
client authentication. Remove or qualify the claim that it works for remote
servers, and explicitly require client authentication for remote deployments or
document the authless threat model.
| ``` | ||
| ┌─────────┐ client_id=https://... ┌──────────────┐ upstream OAuth ┌──────────┐ | ||
| │ MCP host│ ──────────────────────> │ Your MCP srv │ ─────────────────> │ Upstream │ | ||
| └─────────┘ <─── bearer token ───── └──────────────┘ <── access token ──└──────────┘ | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language tags to all untyped Markdown fences.
.agents/skills/build-mcp-server/references/auth.md#L49-L53: add a suitable tag such astextto the protocol diagram fence..agents/skills/build-mcp-server/references/tool-design.md#L23-L27: addtext..agents/skills/build-mcp-server/references/tool-design.md#L35-L37: addtext..agents/skills/build-mcp-server/references/tool-design.md#L45-L48: addtext.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 49-49: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
📍 Affects 2 files
.agents/skills/build-mcp-server/references/auth.md#L49-L53(this comment).agents/skills/build-mcp-server/references/tool-design.md#L23-L27.agents/skills/build-mcp-server/references/tool-design.md#L35-L37.agents/skills/build-mcp-server/references/tool-design.md#L45-L48
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/build-mcp-server/references/auth.md around lines 49 - 53, Add
the text language tag to the untyped Markdown fences containing the protocol
diagram and examples: .agents/skills/build-mcp-server/references/auth.md lines
49-53, and .agents/skills/build-mcp-server/references/tool-design.md lines
23-27, 35-37, and 45-48. Preserve the fenced content unchanged.
Source: Linters/SAST tools
|
|
||
| `wrangler deploy` prints the live URL. That's the URL users paste into Claude. | ||
|
|
||
| Secrets (upstream API keys): `npx wrangler secret put UPSTREAM_API_KEY`, then read `env.UPSTREAM_API_KEY` inside `init()`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced markdown around the flagged line.
file=".agents/skills/build-mcp-server/references/deploy-cloudflare-workers.md"
wc -l "$file"
sed -n '1,180p' "$file"
# Search for other Cloudflare/MCP agent examples in the repo that show env access.
rg -n --hidden --glob '!**/.git/**' -e 'McpAgent|init\(|env\.UPSTREAM_API_KEY|this\.env|typed Env|export interface Env|type Env' .agents . || trueRepository: chrisdoc/hevy-mcp
Length of output: 7465
🌐 Web query:
Cloudflare McpAgent env access this.env typed Env init() bindings documentation
💡 Result:
In the Cloudflare Agents SDK, the McpAgent class is used to build stateful Model Context Protocol (MCP) servers backed by Durable Objects [1][2]. Accessing Environment Bindings To access environment variables and service bindings within an McpAgent, you use the this.env property [3][4]. For type safety, you define an interface (e.g., Env or WorkerEnv) containing your bindings and pass it as a generic type argument when extending the McpAgent class [5][3][4]: export class MyMCP extends McpAgent<Env, State, {}> { async init { // Access bindings via this.env const mySecret = this.env.MY_SECRET; } } The init Method The init method is a lifecycle hook where you initialize your server instance, register tools, and define resources [5][4]. Because McpAgent instances are backed by Durable Objects, the init method is the recommended place for deterministic setup tasks such as configuring singletons, token validation, or initializing your McpServer instance [4]. It is executed when the Durable Object instance is first created [4]. Key Implementation Details: 1. Typed Environment: By extending McpAgent<Env>, TypeScript will correctly provide autocompletion and type checking for properties on this.env [3][4]. 2. State Management: The state and setState properties allow for persistent state management within the Durable Object [1][5]. 3. Deployment: The static McpAgent.serve() method is the standard way to expose your MCP server as a Worker handler [1][5]. If you find that this.env is typed as unknown, ensure that you have passed the Env type interface as a generic argument to the McpAgent class definition [6][3]. If you are using service bindings and experiencing issues with RPC method types, ensure your project configuration (e.g., wrangler.toml and tsconfig.json) is correctly set up to generate and include the necessary types [6].
Citations:
- 1: https://developers.cloudflare.com/agents/api-reference/mcp-agent-api/index.md
- 2: https://developers.cloudflare.com/agents/guides/remote-mcp-server/index.md
- 3: https://cloudflare-docs.justalittlebyte.ovh/agents/api-reference/agents-api/
- 4: https://github.com/bm1549/remote-mcp-cloudflare
- 5: https://github.com/cloudflare/agents/blob/main/examples/mcp/README.md
- 6:
wrangler typesfor service bindings missing rpc methods cloudflare/workers-sdk#8902
Use this.env inside init()
env isn’t in scope in MyMCP.init(). Read the secret via this.env.UPSTREAM_API_KEY and add a typed Env generic on McpAgent for type safety.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/build-mcp-server/references/deploy-cloudflare-workers.md at
line 100, Update the MyMCP class to provide a typed Env generic to McpAgent, and
change init() to read the upstream API key from this.env.UPSTREAM_API_KEY
instead of the out-of-scope env variable.
Source: MCP tools
| }, async ({}, extra) => { | ||
| const caps = server.getClientCapabilities(); | ||
| if (caps?.elicitation) { | ||
| const r = await server.elicitInput({ | ||
| mode: "form", | ||
| message: "Delete all items? This cannot be undone.", | ||
| requestedSchema: { | ||
| type: "object", | ||
| properties: { confirm: { type: "boolean", title: "Confirm deletion" } }, | ||
| required: ["confirm"], | ||
| }, | ||
| }); | ||
| if (r.action === "accept" && r.content?.confirm) { | ||
| await deleteAll(); | ||
| return { content: [{ type: "text", text: "Deleted." }] }; | ||
| } | ||
| return { content: [{ type: "text", text: "Cancelled." }] }; | ||
| } | ||
| // Fallback: return text asking Claude to relay the question | ||
| return { content: [{ type: "text", text: "Confirmation required. Please ask the user: 'Delete all items? This cannot be undone.' Then call this tool again with their answer." }] }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the no-elicitation fallback executable.
Both examples use an empty/no-argument tool, then tell Claude to ask the user and call the tool again with the answer. There is no confirm input to carry that answer, so the retry reaches the same fallback and cannot complete. Add an explicit confirmation argument and branch on it, or return a terminal response that elicitation is required instead of promising a retry.
Also applies to: 55-65
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/build-mcp-server/references/elicitation.md around lines 27 -
46, The no-elicitation branch in the tool handler is not executable because the
empty tool schema cannot carry the user’s confirmation on retry. Update the
tool’s input schema and handler around the visible elicitation flow to accept an
explicit confirmation argument and perform deletion when confirmed, or replace
the fallback with a terminal response stating that elicitation is required;
apply the same correction to the corresponding second example.
| if (r.action === "accept" && r.content?.confirm) { | ||
| await deleteAll(); | ||
| return { content: [{ type: "text", text: "Deleted." }] }; | ||
| } | ||
| return { content: [{ type: "text", text: "Cancelled." }] }; | ||
| } | ||
| // Fallback: return text asking Claude to relay the question | ||
| return { content: [{ type: "text", text: "Confirmation required. Please ask the user: 'Delete all items? This cannot be undone.' Then call this tool again with their answer." }] }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Distinguish accepted false from cancellation.
An accepted form with confirm: false is a valid user submission, but this code returns "Cancelled.", conflating it with decline and cancel. Handle r.action first, then report an explicit declined/not-deleted result for confirm: false.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/build-mcp-server/references/elicitation.md around lines 39 -
46, Update the confirmation handling around the existing r.action branch to
process accepted submissions before cancellation: return “Deleted.” for confirm:
true, and an explicit declined/not-deleted response for confirm: false. Preserve
“Cancelled.” only for cancellation or other non-accepted actions, and leave the
fallback prompt unchanged.
| ## Resource limits | ||
|
|
||
| Claude will happily ask to read a 4GB log file. Cap everything: | ||
|
|
||
| ```typescript | ||
| const MAX_BYTES = 1_000_000; | ||
| const buf = await readFile(path); | ||
| if (buf.length > MAX_BYTES) { | ||
| return { | ||
| content: [{ | ||
| type: "text", | ||
| text: `File is ${buf.length} bytes — too large. Showing first ${MAX_BYTES}:\n\n` | ||
| + buf.subarray(0, MAX_BYTES).toString("utf8"), | ||
| }], | ||
| }; | ||
| } | ||
| ``` | ||
|
|
||
| Same for directory listings (cap entry count), search results (cap matches), and anything else unbounded. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound the read before allocating the file.
readFile(path) loads the entire file into memory before buf.length is checked, so a 4 GB file can still exhaust the process. Check metadata and use a bounded stream/read operation; retain a TOCTOU-safe limit for files that change while being read.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/build-mcpb/references/local-security.md around lines 112 -
130, Update the resource-limit example around MAX_BYTES to avoid calling
readFile(path) before enforcing the limit: inspect file metadata and use a
bounded read or stream, while retaining a TOCTOU-safe maximum for files that
grow or change during reading. Preserve the existing oversized-file response and
truncated UTF-8 content behavior.
| **Substitution variables** (in `args` and `env` only): | ||
| - `${__dirname}` — absolute path to the unpacked bundle directory | ||
| - `${user_config.<key>}` — value the user entered at install time | ||
| - `${HOME}` — user's home directory | ||
|
|
||
| **There are no auto-prefixed env vars.** The env var names your server reads are exactly what you declare in `mcp_config.env`. If you write `"ROOT_DIR": "${user_config.rootDir}"`, your server reads `process.env.ROOT_DIR`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## Local file excerpt\n'
sed -n '1,140p' .agents/skills/build-mcpb/references/manifest-schema.md
printf '\n## Search for substitution variables in repository\n'
rg -n '\$\{DESKTOP\}|\$\{DOCUMENTS\}|\$\{DOWNLOADS\}|\$\{pathSeparator\}|\$\{__dirname\}|\$\{user_config\.|auto-prefixed env vars|Substitution variables' .agents/skills/build-mcpb -S || true
printf '\n## Upstream manifest reference excerpt\n'
python3 - <<'PY'
import urllib.request
url = "https://raw.githubusercontent.com/anthropics/mcpb/main/MANIFEST.md"
with urllib.request.urlopen(url, timeout=20) as r:
text = r.read().decode("utf-8", "replace")
for needle in ["${DESKTOP}", "${DOCUMENTS}", "${DOWNLOADS}", "${pathSeparator}", "Substitution variables", "auto-prefixed env vars"]:
idx = text.find(needle)
if idx >= 0:
start = max(0, idx - 250)
end = min(len(text), idx + 700)
print(f"\n--- {needle} ---")
print(text[start:end])
PYRepository: chrisdoc/hevy-mcp
Length of output: 9425
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import subprocess, textwrap, os, sys
url = "https://raw.githubusercontent.com/anthropics/mcpb/main/MANIFEST.md"
cmd = ["curl", "-L", "-k", "--fail", "--silent", "--show-error", url]
p = subprocess.run(cmd, capture_output=True, text=True)
print("curl_exit=", p.returncode)
if p.returncode != 0:
print(p.stderr)
sys.exit(p.returncode)
text = p.stdout
for needle in ["${DESKTOP}", "${DOCUMENTS}", "${DOWNLOADS}", "${pathSeparator}", "Substitution variables", "auto-prefixed env vars"]:
idx = text.find(needle)
print(f"\n## {needle}")
if idx >= 0:
start = max(0, idx - 300)
end = min(len(text), idx + 900)
print(text[start:end])
else:
print("NOT FOUND")
PYRepository: chrisdoc/hevy-mcp
Length of output: 5118
Document the full substitution-variable set. This list is incomplete: MANIFEST.md also supports ${DESKTOP}, ${DOCUMENTS}, ${DOWNLOADS}, and ${pathSeparator} (or ${/}), so the cross-platform bundle guidance here is missing required options.
🧰 Tools
🪛 LanguageTool
[style] ~63-~63: Consider an alternative for the overused word “exactly”.
Context: ...The env var names your server reads are exactly what you declare in mcp_config.env. I...
(EXACTLY_PRECISELY)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/build-mcpb/references/manifest-schema.md around lines 58 -
63, Update the “Substitution variables” section to include the supported
${DESKTOP}, ${DOCUMENTS}, ${DOWNLOADS}, and ${pathSeparator} (or ${/}) variables
alongside the existing entries, preserving the note that substitutions apply
only in args and env.
Source: MCP tools
| # Build an MCPB (Bundled Local MCP Server) | ||
|
|
||
| MCPB is a local MCP server **packaged with its runtime**. The user installs one file; it runs without needing Node, Python, or any toolchain on their machine. It's the sanctioned way to distribute local MCP servers. | ||
|
|
||
| > MCPB is the **secondary** distribution path. Anthropic recommends remote MCP servers for directory listing — see https://claude.com/docs/connectors/building/what-to-build. | ||
|
|
||
| **Use MCPB when the server must run on the user's machine** — reading local files, driving a desktop app, talking to localhost services, OS-level APIs. If your server only hits cloud APIs, you almost certainly want a remote HTTP server instead (see `build-mcp-server`). Don't pay the MCPB packaging tax for something that could be a URL. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files '.agents/skills/build-mcpb/SKILL.md' '.agents/skills/build-mcpb/references/manifest-schema.md' '.agents/skills/build-mcpb/references/local-security.md'
printf '\n== SKILL.md outline ==\n'
ast-grep outline .agents/skills/build-mcpb/SKILL.md || true
printf '\n== manifest-schema.md outline ==\n'
ast-grep outline .agents/skills/build-mcpb/references/manifest-schema.md || true
printf '\n== SKILL.md relevant lines ==\n'
sed -n '1,180p' .agents/skills/build-mcpb/SKILL.md | cat -n | sed -n '1,180p'
printf '\n== manifest-schema.md relevant lines ==\n'
sed -n '1,140p' .agents/skills/build-mcpb/references/manifest-schema.md | cat -n | sed -n '1,140p'
printf '\n== local-security.md relevant lines ==\n'
sed -n '1,120p' .agents/skills/build-mcpb/references/local-security.md | cat -n | sed -n '1,120p'
printf '\n== Search for uv/binary/type/command mentions ==\n'
rg -n '\buv\b|\bbinary\b|server\.type|mcp_config\.command|entry_point|node|python' .agents/skills/build-mcpb -SRepository: chrisdoc/hevy-mcp
Length of output: 23623
🌐 Web query:
MCPB v0.4 schema uv server.type node python binary
💡 Result:
In the MCPB v0.4 manifest schema, the server.type field supports four distinct types for defining how an MCP server is executed: node, python, binary, and uv [1][2]. The uv server type, introduced in v0.4, is specifically designed for Python extensions [1][2][3]. It enables cross-platform support by removing the need to bundle dependencies (such as lib/ or venv/ directories) within the.mcpb bundle [2][4]. Instead, dependencies must be declared in a pyproject.toml file, and the host application automatically manages the installation and execution using the UV runtime [2][5]. Regarding the mcp_config requirement, while the v0.4 schema initially required mcp_config for all server types, subsequent updates made it optional for the uv type [6][7]. For node, python, and binary types, mcp_config remains mandatory to specify the execution command and arguments [1][2]. In summary: - node: Node.js server with bundled dependencies [1][2]. - python: Python server with bundled dependencies (e.g., via server/lib or venv) [1][2]. - binary: Compiled executable that is self-contained [1][2]. - uv: Python server using the UV runtime; requires pyproject.toml; mcp_config is optional for this type [1][2][4].
Citations:
- 1: https://github.com/modelcontextprotocol/mcpb/blob/HEAD/MANIFEST.md
- 2: https://github.com/modelcontextprotocol/mcpb/blob/main/MANIFEST.md
- 3: automatic python package bundling... or just use
uv runmodelcontextprotocol/mcpb#22 - 4: https://gitcode.com/gh_mirrors/dx/mcpb/blob/main/MANIFEST.md
- 5: https://github.com/anthropics/mcpb?tab=readme-ov-file
- 6: Make server.mcp_config optional for uv server type (strict 0.4 schema) modelcontextprotocol/mcpb#270
- 7: fix: make mcp_config required for all server types in v0.4 schema modelcontextprotocol/mcpb#173
Add uv to the MCPB type contract.
.agents/skills/build-mcpb/references/manifest-schema.md:server.typestill stops atnode/python/binary; adduvand clarify thatmcp_configis the launch config..agents/skills/build-mcpb/SKILL.md: keep the runtime guidance aligned with the manifest contract so the documented packaging modes match the supported schema.
🧰 Tools
🪛 SkillSpector (2.3.7)
[warning] 137: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 139: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 148: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 177: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 180: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 183: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 184: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 187: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[error] 89: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 73: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
📍 Affects 2 files
.agents/skills/build-mcpb/SKILL.md#L7-L13(this comment).agents/skills/build-mcpb/references/manifest-schema.md#L50-L56
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/build-mcpb/SKILL.md around lines 7 - 13, Update
.agents/skills/build-mcpb/references/manifest-schema.md at lines 50-56 to
include uv in the server.type contract and clarify that mcp_config is the launch
configuration. Update .agents/skills/build-mcpb/SKILL.md at lines 7-13 so its
runtime and packaging guidance documents the same supported node, python, uv,
and binary modes; both sites require direct documentation changes.
Source: MCP tools
| server.registerTool( | ||
| "list_files", | ||
| { | ||
| description: "List files in a directory under the configured root.", | ||
| inputSchema: { path: z.string().default(".") }, | ||
| annotations: { readOnlyHint: true }, | ||
| }, | ||
| async ({ path }) => { | ||
| const entries = await readdir(join(ROOT, path), { withFileTypes: true }); | ||
| const list = entries.map(e => ({ name: e.name, dir: e.isDirectory() })); | ||
| return { content: [{ type: "text", text: JSON.stringify(list, null, 2) }] }; | ||
| }, | ||
| ); | ||
|
|
||
| server.registerTool( | ||
| "read_file", | ||
| { | ||
| description: "Read a file's contents. Path is relative to the configured root.", | ||
| inputSchema: { path: z.string() }, | ||
| annotations: { readOnlyHint: true }, | ||
| }, | ||
| async ({ path }) => { | ||
| const text = await readFile(join(ROOT, path), "utf8"); | ||
| return { content: [{ type: "text", text }] }; | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File list ==\n'
git ls-files .agents/skills/build-mcpb/SKILL.md .agents/skills/build-mcpb/references/local-security.md
printf '\n== SKILL.md excerpt ==\n'
sed -n '80,140p' .agents/skills/build-mcpb/SKILL.md
printf '\n== local-security.md excerpt ==\n'
sed -n '1,80p' .agents/skills/build-mcpb/references/local-security.md
printf '\n== Search for filesystem helpers/usages ==\n'
rg -n "safeJoin|realpath|resolve\\(|relative\\(|join\\(ROOT, path\\)|read_file|list_files" .agents/skills/build-mcpbRepository: chrisdoc/hevy-mcp
Length of output: 6200
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const fs = require('node:fs');
const fsp = require('node:fs/promises');
const path = require('node:path');
const os = require('node:os');
(async () => {
const base = await fsp.mkdtemp(path.join(os.tmpdir(), 'mcpb-path-'));
const root = path.join(base, 'root');
const outside = path.join(base, 'outside');
const link = path.join(root, 'link');
await fsp.mkdir(root);
await fsp.mkdir(outside);
await fsp.writeFile(path.join(outside, 'secret.txt'), 'secret');
await fsp.symlink(outside, link);
const userPath = 'link/secret.txt';
const lexical = path.resolve(root, userPath);
const rel = path.relative(root, lexical);
console.log(JSON.stringify({
root,
outside,
userPath,
lexical,
relative: rel,
pathEscapeDetected: rel.startsWith('..') || path.isAbsolute(rel),
realpathLink: await fsp.realpath(link),
realpathTarget: await fsp.realpath(path.join(root, userPath)),
}, null, 2));
await fsp.rm(base, { recursive: true, force: true });
})().catch(err => {
console.error(err);
process.exit(1);
});
JSRepository: chrisdoc/hevy-mcp
Length of output: 515
Enforce real root containment for filesystem tools.
.agents/skills/build-mcpb/SKILL.md: replacejoin(ROOT, path)with a shared helper that resolves the target,realpaths both root and candidate, and rejects anything outside the root beforereaddir/readFile..agents/skills/build-mcpb/references/local-security.md: updatesafeJointo compare canonical paths, not justresolve/relative, so symlink escapes are blocked too.
🧰 Tools
🪛 SkillSpector (2.3.7)
[warning] 137: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 139: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 148: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 177: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 180: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 183: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 184: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 187: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[error] 89: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 73: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
📍 Affects 2 files
.agents/skills/build-mcpb/SKILL.md#L94-L119(this comment).agents/skills/build-mcpb/references/local-security.md#L18-L28
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/build-mcpb/SKILL.md around lines 94 - 119, Enforce canonical
root containment for both filesystem tools: in
.agents/skills/build-mcpb/SKILL.md at lines 94-119, replace direct join(ROOT,
path) usage in list_files and read_file with the shared safe path helper before
readdir/readFile; in .agents/skills/build-mcpb/references/local-security.md at
lines 18-28, update safeJoin to realpath both the configured root and candidate
and reject candidates outside the canonical root, including symlink escapes.
Source: MCP tools
| ```bash | ||
| npm install | ||
| npx esbuild src/index.ts --bundle --platform=node --outfile=server/index.js | ||
| # or: copy node_modules wholesale if native deps resist bundling | ||
| npx @anthropic-ai/mcpb pack | ||
| ``` | ||
|
|
||
| `mcpb pack` zips the directory and validates `manifest.json` against the schema. | ||
|
|
||
| ### Python | ||
|
|
||
| ```bash | ||
| pip install -t server/vendor -r requirements.txt | ||
| npx @anthropic-ai/mcpb pack | ||
| ``` |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin every executable npx dependency.
These copy-paste commands resolve packages at execution time, undermining the reproducibility promised by this PR and allowing a later upstream publication to change what runs. Pin exact versions or invoke lockfile-backed local binaries.
Also applies to: 175-187
🧰 Tools
🪛 SkillSpector (2.3.7)
[warning] 137: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 139: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 148: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 177: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 180: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 183: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 184: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 187: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[error] 89: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 73: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/build-mcpb/SKILL.md around lines 135 - 149, Update the
Node.js build commands in the JavaScript and Python sections to avoid unpinned
executable npx dependencies: invoke lockfile-backed local binaries or specify
exact package versions for both esbuild and `@anthropic-ai/mcpb`, preserving the
existing build and packaging flow.
Source: Linters/SAST tools
2 participants
Primary changes
build-mcp-serverandbuild-mcpbagent skills with their supporting reference material.skills-lock.json.Reviewer walkthrough
.agents/skills/build-mcp-server/SKILL.mdand.agents/skills/build-mcpb/SKILL.mdfor the entry-point workflows.references/documents and confirm the corresponding lockfile entries.Correctness and invariants
Testing and QA
✨ PR Description
Purpose: Add comprehensive MCP server development skills documentation to guide building Model Context Protocol servers with deployment patterns, tool design, and security best practices.
Main changes:
build-mcp-serverskill with five-phase guidance covering use-case discovery, deployment models (remote HTTP, MCPB, local stdio), tool patterns, and framework selectionbuild-mcpbskill documenting bundled local MCP server packaging, manifest configuration, and security requirements for filesystem/OS-level integrationsGenerated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Description using Guidelines Learn how
Summary by CodeRabbit
New Features
Documentation