feat: enable cors#28
Conversation
This allows the MCP server to be used e.g. with locally running llama.cpp
📝 WalkthroughWalkthroughThis PR adds CORS (Cross-Origin Resource Sharing) support to the HTTP server mode of the CLI. It introduces the ChangesHTTP CORS middleware addition
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![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
🤖 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 `@src/index.ts`:
- Around line 47-62: The CORS config currently uses origin: "*" which is too
permissive; replace it with a configurable allowlist read from env (e.g.
process.env.CORS_ORIGINS) and have app.use(cors(...)) validate incoming origins
against that list, defaulting to localhost origins like http://localhost:3000
and http://127.0.0.1:3000 when the env var is absent; update the cors config in
the app.use(cors({...})) block (keeping exposedHeaders and allowedHeaders) so
only origins present in the allowlist are accepted and all others are rejected.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: caadc168-15f9-4520-8ee6-b0d8a113b8c7
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
package.jsonsrc/index.ts
| app.use( | ||
| cors({ | ||
| origin: "*", | ||
|
|
||
| exposedHeaders: ["mcp-session-id"], | ||
|
|
||
| methods: ["GET", "POST", "OPTIONS"], | ||
|
|
||
| allowedHeaders: [ | ||
| "Content-Type", | ||
| "Authorization", | ||
| "mcp-session-id", | ||
| "mcp-protocol-version", | ||
| ], | ||
| }), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For the corsnpm middleware, what is the recommended secure pattern for restrictingorigin in local API servers that should only be callable from localhost apps?
💡 Result:
To securely restrict the origin in the cors npm middleware for local development, the recommended pattern is to explicitly define an allowlist of trusted origins rather than using a wildcard () [1][2][3]. For a local API server that should only be callable from specific localhost applications, you should configure the origin option using a function or an array of strings [1][4][5]. Recommended Implementation Pattern: const cors = require('cors'); const allowedOrigins = ['http://localhost:3000', 'http://localhost:5173']; const corsOptions = { origin: (origin, callback) => { // Allow requests with no origin (e.g., server-to-server, Postman, curl) if (!origin) return callback(null, true); if (allowedOrigins.includes(origin)) { callback(null, true); } else { callback(new Error('Origin not allowed by CORS')); } }, credentials: true // Required if your app uses cookies or authorization headers }; app.use(cors(corsOptions)); Key Security Considerations: 1. CORS is a Browser-Only Mechanism: CORS is enforced by browsers to protect users, not by the server to block requests [1][2][6]. Your server will still receive and process requests from non-browser clients (like curl, Postman, or other backend servers) regardless of your CORS configuration [1][7][8]. 2. Avoid Wildcards: Never use origin: '' in production or when your API handles sensitive data or credentials [3][9][10]. 3. Handle Missing Origins: The origin header is often undefined for non-browser requests [1][3]. The pattern above allows these requests by default; if you need to strictly block non-browser traffic, you must implement additional authentication or authorization logic (e.g., API keys or tokens) outside of the CORS middleware [3][10]. 4. Development Alternatives: For local development, many developers prefer using a proxy (e.g., Vite's server.proxy or Webpack's devServer.proxy) to route frontend requests through the same origin as the backend, which avoids CORS issues entirely during development [3][9][11].
Citations:
- 1: https://www.npmjs.com/package/cors
- 2: https://expressjs.com/en/resources/middleware/cors.html
- 3: https://stackdevtools.com/how-to-fix-cors-error-react-localhost/
- 4: https://www.stackhawk.com/blog/nodejs-cors-guide-what-it-is-and-how-to-enable-it/
- 5: https://github.com/expressjs/cors?query=workflow%3Aci
- 6: https://dev.to/akash_shukla/cors-why-it-exists-how-it-works-how-to-fix-common-issues-102c
- 7: https://npmx.dev/package/cors
- 8: https://dev.to/prathameshdhadbale/how-frontend-talks-to-backend-rest-json-and-cors-explained-without-the-jargon-5a8d
- 9: https://headersnap.com/blog/cors-debugging-local-development/
- 10: https://stackoverflow.com/questions/39042799/cors-localhost-as-allowed-origin-in-production
- 11: https://dev.to/andypotts/avoiding-cors-errors-on-localhost-in-2020-4mfn
🏁 Script executed:
cat -n src/index.ts | head -80Repository: getAlby/mcp
Length of output: 2807
🏁 Script executed:
# Check if there are any authentication/authorization middleware in the codebase
rg -i "auth|jwt|token|middleware" src/ --max-count=10Repository: getAlby/mcp
Length of output: 1120
🏁 Script executed:
# Look for any environment variable usage or config files
fd -e ".env*" -e "config*" | head -20Repository: getAlby/mcp
Length of output: 37
🏁 Script executed:
# Check package.json to understand project purpose
cat package.jsonRepository: getAlby/mcp
Length of output: 1211
Restrict CORS origins; wildcard is too permissive for wallet-control endpoints.
Line 49 currently allows any origin ("*"), which permits arbitrary websites to call this wallet API from a browser context. Although Bearer token authentication is enforced, CORS should still be restrictive as a defense-in-depth measure. Switch to an explicit allowlist (configurable via env) and default to localhost origins.
Proposed hardening diff
app.use(
cors({
- origin: "*",
+ origin: (origin, callback) => {
+ const configured = (process.env.CORS_ALLOWED_ORIGINS || "")
+ .split(",")
+ .map((v) => v.trim())
+ .filter(Boolean);
+ const defaults = [
+ "http://localhost:3000",
+ "http://127.0.0.1:3000",
+ ];
+ const allowlist = new Set(configured.length ? configured : defaults);
+
+ // Non-browser clients may omit Origin
+ if (!origin || allowlist.has(origin)) return callback(null, true);
+ return callback(new Error("CORS origin not allowed"));
+ },🤖 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 `@src/index.ts` around lines 47 - 62, The CORS config currently uses origin:
"*" which is too permissive; replace it with a configurable allowlist read from
env (e.g. process.env.CORS_ORIGINS) and have app.use(cors(...)) validate
incoming origins against that list, defaulting to localhost origins like
http://localhost:3000 and http://127.0.0.1:3000 when the env var is absent;
update the cors config in the app.use(cors({...})) block (keeping exposedHeaders
and allowedHeaders) so only origins present in the allowlist are accepted and
all others are rejected.
There was a problem hiding this comment.
how do you expect it to be used if we restrict it? it's a public API that can be executed from anywhere, and it requires a NWC connection secret to be used.
There was a problem hiding this comment.
@rolznz, that's a fair point. Since the NWC connection string already acts as the authentication credential — without it, no wallet operations can be performed — the wildcard origin: "*" is appropriate here. Restricting origins would break the intended use case of allowing any LLM frontend or locally running service (like llama.cpp) to connect freely.
I'll withdraw this concern.
(∪・ω・)っ
✏️ Learnings added
Learnt from: rolznz
Repo: getAlby/mcp PR: 28
File: src/index.ts:47-62
Timestamp: 2026-05-16T06:06:08.014Z
Learning: In the `getAlby/mcp` repository (`src/index.ts`), the HTTP server intentionally uses `origin: "*"` in the CORS configuration. The API is designed to be publicly accessible from any origin (including arbitrary LLM frontends and local services like llama.cpp), and security is enforced via the NWC (Nostr Wallet Connect) connection string secret — not by restricting CORS origins. Do not flag wildcard CORS origin as a security issue in this repo.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
1 participant
This allows the MCP server to be used e.g. with locally running llama.cpp
Summary by CodeRabbit
New Features
Bug Fixes