Repository navigation
Security hardening: SSRF guards, bounded rate-limiter memory, OAuth token lifetime & scoped revocation #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e7b9d2c
ca4f1a1
c37f3b7
ef4cb5c
c433b0d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,11 +7,13 @@ import { | |
| } from "@modelcontextprotocol/sdk/server/auth/router.js"; | ||
| import { resolveAuth } from "../auth/resolve.js"; | ||
| import { AuthContext, AuthError } from "../auth/context.js"; | ||
| import { loadServerConfig } from "../config.js"; | ||
| import { loadConfigOrExit } from "./loadConfigOrExit.js"; | ||
| import { createLogger } from "../logger.js"; | ||
| import { LimiterPool } from "../netbird/limiterPool.js"; | ||
| import { verifyPat, type TokenVerification } from "../netbird/client.js"; | ||
| import { buildServer } from "../server.js"; | ||
| import { NetBirdOAuthProvider } from "../oauth/provider.js"; | ||
| import { loginRateLimiter } from "../oauth/loginRateLimit.js"; | ||
|
|
||
| /** | ||
| * CLOUD entrypoint. Speaks MCP over Streamable HTTP so the server can be hosted | ||
|
|
@@ -23,23 +25,55 @@ import { NetBirdOAuthProvider } from "../oauth/provider.js"; | |
| * Stateless: each request gets a fresh server instance, so one deployment safely | ||
| * serves many tenants and scales horizontally. | ||
| */ | ||
| const config = loadServerConfig(); | ||
| const config = loadConfigOrExit(); | ||
| const logger = createLogger(config.logLevel); | ||
| const { port, tokenHeader, urlHeader, oauthEnabled, publicBaseUrl, verifyPatOnLogin } = | ||
| const { port, tokenHeader, urlHeader, oauthEnabled, directPatEnabled, publicBaseUrl, verifyPatOnLogin, trustProxy } = | ||
| config.http; | ||
|
|
||
| const provider = new NetBirdOAuthProvider({ | ||
| logger, | ||
| verifyPatOnLogin, | ||
| maxRequestsPerMinute: config.maxRequestsPerMinute, | ||
| requestTimeoutMs: config.requestTimeoutMs, | ||
| allowedApiHosts: config.allowedApiHosts, | ||
| }); | ||
|
|
||
| // One rate limiter per tenant (keyed by a hash of the NetBird token, never the token | ||
| // itself), so NetBird's per-account limit is respected without cross-tenant interference. | ||
| const limiters = new LimiterPool(config.maxRequestsPerMinute); | ||
|
|
||
| // Verify a direct PAT against NetBird before it is trusted, reusing the tenant's | ||
| // own limiter so verification counts against the same per-account budget as its | ||
| // tool calls. Only the direct-PAT path uses this; OAuth bindings are pre-verified. | ||
| // An indeterminate result (or a surprise throw) fails open — the host is already | ||
| // allowlisted and NetBird refuses a bogus token on the real call — but is logged, | ||
| // so admitting a token without a clean verification is never silent. | ||
| const verifyDirectPat = async (auth: AuthContext): Promise<TokenVerification> => { | ||
| try { | ||
| const result = await verifyPat(auth, { | ||
| logger, | ||
| rateLimiter: limiters.get(auth.token), | ||
| timeoutMs: config.requestTimeoutMs, | ||
| }); | ||
| if (result === "unknown") { | ||
| logger.warn("direct-PAT verification indeterminate; admitting token unverified", { | ||
| baseUrl: auth.baseUrl, | ||
| }); | ||
| } | ||
| return result; | ||
| } catch (err) { | ||
| logger.warn("direct-PAT verification errored; admitting token unverified", { | ||
| baseUrl: auth.baseUrl, | ||
| message: (err as Error).message, | ||
| }); | ||
| return "unknown"; | ||
| } | ||
| }; | ||
|
|
||
| const app = express(); | ||
| // Behind a proxy this must be set so req.ip is the real client IP, which every | ||
| // per-IP rate limiter (OAuth routes, login form, per-source login verify) keys on. | ||
| app.set("trust proxy", trustProxy); | ||
|
Comment on lines
73
to
+76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
rg -n -B2 -A15 'trustProxy|TRUST_PROXY' src/config.ts .env.example README.md 2>/dev/nullRepository: netbirdio/netbird-mcp Length of output: 5701 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Read the trust-proxy parser and its references without executing repo code.
fd -a 'config\.ts$' . | sed 's#^\./##'
rg -n -B5 -A60 'parseTrustProxy|trustProxy' src/config.ts src/bin/http.ts
sed -n '1,220p' src/config.ts
sed -n '60,85p' src/bin/http.tsRepository: netbirdio/netbird-mcp Length of output: 21936 Restrict
🤖 Prompt for AI Agents |
||
| app.use(express.json({ limit: "1mb" })); | ||
| app.use(express.urlencoded({ extended: false })); // OAuth login form posts | ||
|
|
||
|
|
@@ -61,8 +95,10 @@ if (oauthEnabled) { | |
| resourceName: "NetBird", | ||
| }), | ||
| ); | ||
| // Our interactive login step (the page /authorize renders posts here). | ||
| app.post("/oauth/netbird-login", provider.handleLogin); | ||
| // Our interactive login step (the page /authorize renders posts here). Rate | ||
| // limited like the SDK's own auth routes, since each submission triggers an | ||
| // outbound PAT verification. | ||
| app.post("/oauth/netbird-login", loginRateLimiter(), provider.handleLogin); | ||
| } | ||
|
|
||
| app.post("/mcp", async (req, res) => { | ||
|
|
@@ -72,6 +108,9 @@ app.post("/mcp", async (req, res) => { | |
| provider: oauthEnabled ? provider : undefined, | ||
| tokenHeader, | ||
| urlHeader, | ||
| allowedApiHosts: config.allowedApiHosts, | ||
| directPatEnabled, | ||
| verifyPat: verifyDirectPat, | ||
| }); | ||
| } catch (err) { | ||
| if (err instanceof AuthError) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import { loadServerConfig, type ServerConfig } from "../config.js"; | ||
|
|
||
| /** | ||
| * Load config for an entrypoint, or fail fast. loadServerConfig throws on an | ||
| * unusable NETBIRD_API_URL, and no logger exists this early in boot, so surface | ||
| * the reason as a clean one-line fatal on stderr and refuse to start. Shared by | ||
| * both entrypoints so the boot-failure behaviour has one implementation. | ||
| */ | ||
| export function loadConfigOrExit(): ServerConfig { | ||
| try { | ||
| return loadServerConfig(); | ||
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : String(err); | ||
| process.stderr.write(`fatal: ${message}\n`); | ||
| process.exit(1); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Explicit
falsehere defeats the auto-on-when-OAuth-off behavior it documents.NETBIRD_ENABLE_DIRECT_PAT=falseis set to a literal value, not left blank. Inconfig.ts,boolEnv(env.NETBIRD_ENABLE_DIRECT_PAT, !oauthEnabled)only applies the!oauthEnabledfallback when the variable isundefined. Copying this template to.envunmodified and then settingNETBIRD_ENABLE_OAUTH=falsewill not auto-enable direct-PAT as the comment above promises — the operator is left with no HTTP auth path unless they also remember to flip this line.🩹 Proposed fix
🤖 Prompt for AI Agents