[ENG-623] feat: remote mcp#150
Open
miguelangaranocurrents wants to merge 4 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughAdds a stateless hosted HTTP transport for the MCP server ( ChangesHTTP MCP Transport and Publish Pipeline
Sequence Diagram(s)sequenceDiagram
participant Client as MCP Client
participant createHttpServer as createHttpServer
participant handleMcpPost as handleMcpPost
participant requestContext as requestContext (AsyncLocalStorage)
participant createMcpServer as createMcpServer
participant fetchApi as fetchApi / Currents API
Client->>createHttpServer: POST /mcp (Authorization: Bearer <apiKey>)
createHttpServer->>handleMcpPost: req, res
handleMcpPost->>handleMcpPost: readJsonBody → parsed body
handleMcpPost->>handleMcpPost: extractApiKey(req) → apiKey
handleMcpPost->>createMcpServer: createMcpServer()
handleMcpPost->>requestContext: run({ apiKey }, async () => ...)
requestContext->>fetchApi: Authorization: Bearer <apiKey> (via getApiKey())
fetchApi-->>requestContext: Currents API response
requestContext-->>handleMcpPost: MCP response written to res
handleMcpPost-->>Client: JSON-RPC response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
mcp-server/src/http.ts (1)
24-31: 💤 Low valueConsider handling edge cases in Bearer token extraction.
The current implementation returns the raw header value for invalid Bearer formats (e.g.,
"Bearer"with no token returns"Bearer"). While this aligns with the design philosophy of forwarding tokens to the Currents API for validation, it may pass through malformed headers.Consider either:
- Returning
undefinedfor invalid Bearer headers- Adding validation to ensure a non-empty token exists after "Bearer"
- Documenting the intentional passthrough behavior
🛡️ Example validation approach
export function extractApiKey(req: IncomingMessage): string | undefined { const auth = req.headers.authorization?.trim(); if (!auth) { return undefined; } const match = /^Bearer\s+(.+)$/i.exec(auth); - return (match ? match[1] : auth).trim(); + if (match) { + const token = match[1].trim(); + return token || undefined; // reject empty tokens + } + // Accept raw tokens without Bearer prefix + return auth.trim() || undefined; }🤖 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 `@mcp-server/src/http.ts` around lines 24 - 31, The extractApiKey function in the Bearer token extraction logic does not validate that a valid token actually exists after the "Bearer" prefix. Currently, malformed headers like "Bearer" with no token are passed through as-is (returning "Bearer" itself). Update the function to either return undefined when the Bearer prefix is present but no token follows it, or add validation to ensure the matched group contains a non-empty token before returning it. This prevents passing malformed authentication headers downstream while maintaining the passthrough behavior for non-Bearer authorization schemes.README.md (1)
115-117: 💤 Low valueAdd language specifier to fenced code block.
The code block showing the Authorization header format is missing a language specifier. While not critical, adding
httportextimproves rendering and satisfies linting rules.📝 Suggested fix
-``` +```http Authorization: Bearer <your-currents-api-key></details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@README.mdaround lines 115 - 117, The fenced code block displaying the
Authorization header format is missing a language specifier on the opening
backticks. Add a language identifier such ashttportextimmediately after
the opening triple backticks (changetohttp) to improve markdown
rendering and satisfy linting requirements.</details> <!-- cr-comment:v1:c93917ece7dab6edc19f8b7d --> _Source: Linters/SAST tools_ </blockquote></details> <details> <summary>.github/workflows/publish.yaml (2)</summary><blockquote> `102-104`: _⚡ Quick win_ **Consider disabling credential persistence for security.** The checkout action does not set `persist-credentials: false`, allowing the GitHub token to persist in the repository's git config. If subsequent steps are compromised, the token could be misused. <details> <summary>🔒 Suggested hardening</summary> ```diff - uses: actions/checkout@v4 with: ref: ${{ github.event.pull_request.head.sha || github.ref }} + persist-credentials: false ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish.yaml around lines 102 - 104, Add the `persist-credentials: false` parameter to the `with` section of the actions/checkout@v4 action to prevent the GitHub token from persisting in the git config. This ensures that if any subsequent workflow steps are compromised, the token cannot be misused by an attacker. ``` </details> <!-- cr-comment:v1:f2f5c547d3fbe8c377f2d297 --> _Source: Linters/SAST tools_ --- `102-102`: _⚖️ Poor tradeoff_ **Consider pinning actions to commit SHAs for supply-chain security.** The workflow uses semantic version tags (`@v3`, `@v4`, `@v5`) rather than immutable commit SHAs. While tags are more readable, they are mutable and could be compromised. Pinning to SHAs with a comment noting the version improves supply-chain security. Example: ```yaml - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 ``` Also applies to: 112-112, 114-114, 122-122, 129-129 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish.yaml at line 102, Replace all GitHub Actions version tags with immutable commit SHAs for supply-chain security in the publish.yaml workflow. In `.github/workflows/publish.yaml` at lines 102, 112, 114, 122, and 129, change each action reference from semantic version tags (such as `@v4`, `@v3`, `@v5`) to specific commit SHAs by looking up the commit hash for each action version and including the version as a comment for readability. For example, replace `actions/checkout@v4` with the format `actions/checkout@<commit-sha> # v4.1.1` where the commit SHA is the immutable hash corresponding to that version tag. ``` </details> <!-- cr-comment:v1:00b88ca52a278690646004a0 --> _Source: Linters/SAST tools_ </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>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 @.github/workflows/publish.yaml:
- Line 20: The
packages: writepermission at the workflow level (line 20)
should be removed and kept only at the job level where it is actually needed.
Delete thepackages: writeline from the workflow-level permissions section,
since thepublish-imagejob already correctly declares this permission at
lines 97-99. This ensures the permission is scoped only to the job that requires
it, adhering to the principle of least privilege.
Nitpick comments:
In @.github/workflows/publish.yaml:
- Around line 102-104: Add the
persist-credentials: falseparameter to the
withsection of the actions/checkout@v4 action to prevent the GitHub token
from persisting in the git config. This ensures that if any subsequent workflow
steps are compromised, the token cannot be misused by an attacker.- Line 102: Replace all GitHub Actions version tags with immutable commit SHAs
for supply-chain security in the publish.yaml workflow. In
.github/workflows/publish.yamlat lines 102, 112, 114, 122, and 129, change
each action reference from semantic version tags (such as@v4,@v3,@v5) to
specific commit SHAs by looking up the commit hash for each action version and
including the version as a comment for readability. For example, replace
actions/checkout@v4with the formatactions/checkout@<commit-sha> # v4.1.1
where the commit SHA is the immutable hash corresponding to that version tag.In
@mcp-server/src/http.ts:
- Around line 24-31: The extractApiKey function in the Bearer token extraction
logic does not validate that a valid token actually exists after the "Bearer"
prefix. Currently, malformed headers like "Bearer" with no token are passed
through as-is (returning "Bearer" itself). Update the function to either return
undefined when the Bearer prefix is present but no token follows it, or add
validation to ensure the matched group contains a non-empty token before
returning it. This prevents passing malformed authentication headers downstream
while maintaining the passthrough behavior for non-Bearer authorization schemes.In
@README.md:
- Around line 115-117: The fenced code block displaying the Authorization header
format is missing a language specifier on the opening backticks. Add a language
identifier such ashttportextimmediately after the opening triple
backticks (changetohttp) to improve markdown rendering and satisfy
linting requirements.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `6b031673-0034-4f74-9c2d-18c9294ed71a` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between e42e9fccd302f94942fdc6aac42ba5769a61008e and 9a997ccec09a30dc4b83690d5056b6098780be87. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `mcp-server/package-lock.json` is excluded by `!**/package-lock.json` </details> <details> <summary>📒 Files selected for processing (14)</summary> * `.dockerignore` * `.github/workflows/README.md` * `.github/workflows/publish.yaml` * `Dockerfile` * `README.md` * `mcp-server/package.json` * `mcp-server/src/http.test.ts` * `mcp-server/src/http.ts` * `mcp-server/src/lib/context.test.ts` * `mcp-server/src/lib/context.ts` * `mcp-server/src/lib/request.ts` * `mcp-server/src/server.test.ts` * `mcp-server/src/server.ts` * `mcp-server/tsdown.config.ts` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remote MCP HTTP server + container publish
Summary
Adds a Streamable HTTP transport so the Currents MCP server can run as a hosted remote endpoint (e.g. for Claude web/mobile connectors), while keeping the existing stdio transport unchanged for local clients.
The same Publish NPM Package workflow now also builds and pushes a Docker image to GHCR. For
betaandlatestchannels, a successful push triggers deployment in the private infrastructure repo viarepository_dispatch.Motivation
Today the server only speaks stdio — fine for Claude Desktop and Cursor, but remote connectors need an HTTPS URL. The goal is one package, one set of tools, two entry points: stdio for local use, HTTP for hosted use.
Authentication for the hosted path is intentionally not implemented in the MCP server. Callers pass their Currents API key as a Bearer token; the server forwards it to the Currents API, which remains the sole auth authority.
Architecture
dist/index.mjsCURRENTS_API_KEYenv var (required at startup)dist/http.mjsAuthorization: Bearer <key>per requestKey design decisions
Per-request context via
AsyncLocalStorage— The HTTP server is multi-tenant: concurrent requests may carry different API keys. Rather than threading a key through every tool handler,getApiKey()inrequest.tsreads from request-scoped context (HTTP) or falls back to the env var (stdio). Tool handlers and their signatures are untouched.Stateless HTTP — Each
POST /mcpcreates a freshMcpServer+StreamableHTTPServerTransportwithsessionIdGenerator: undefined. No session state, no cross-request key bleed.GET/DELETEon/mcpreturn 405.Server factory —
createMcpServer()replaces the module-level singleton so both transports can instantiate servers independently.What to review
Runtime (
mcp-server/src/)lib/context.tslib/request.tsgetApiKey()server.tshttp.ts/healthz, error handlingPackaging and Docker
package.jsonstart:httpscript andcurrents-mcp-httpbin; stdio bin unchangedtsdown.config.tshttpbuild entryDockerfileCURRENTS_API_URLfixed to include/v1; no baked-in API key.dockerignoreCI/CD (
.github/workflows/publish.yaml)New
publish-imagejob runs after npm publish:ghcr.io/currents-dev/currents-mcp:<version>and:<channel>beta/latestonly: resolve GHCR digest, firerepository_dispatch(mcp-image-published) tocurrents-dev/currentsWorth checking: dispatch runs after push (digest must exist), failures are not swallowed, and
alpha/ semver-only publishes do not dispatch.Tests
context.test.ts— concurrent ALS isolationhttp.test.ts— Bearer parsing and outbound auth header passthroughserver.test.ts— updated to call factory explicitlyDocs
README.md— remote endpoint usage for end users (client config, Bearer auth, local run).github/workflows/README.md— GHCR publish +DISPATCH_TOKENsecretBackward compatibility
npx @currents/mcp, Claude Desktop config) are unaffectedTest plan
After merge, before first
beta/latestpublish:DISPATCH_TOKENrepo secret (fine-grained PAT, Contents read/write oncurrents-dev/currents)GITHUB_TOKENto write packagesDeploy integration
latestbetaalpha, semver,oldversionDispatch payload:
{ "tag": "<channel>", "digest": "sha256:..." }.Summary by CodeRabbit
GET /healthzandPOST /mcpsupport, using Bearer token authorization.currents-mcp-httpexecutable andstart:httpcommand..dockerignore.