feat(web): enforce localhost-only binding; expand loopback origin allow-list (#1796)
Closes #1795. Enforce localhost-only binding in code (the "never 0.0.0.0" rule was only a spec note): refuse an all-interfaces HOST — across every spelling the OS binds as the wildcard (inet_aton decimal/octal/hex, IPv6, IPv4-mapped, zone-scoped, IDNA/Unicode) — unless DANGEROUSLY_BIND_ALL_INTERFACES=true (Docker's sanctioned opt-in). Fixes the reported stdio-connect 403 by expanding the default loopback origin allow-list to all three interchangeable forms. Hardening surfaced across an extended review: closes an IDNA/fullwidth wildcard bypass, a blank-ALLOWED_ORIGINS fail-open, and the CLI/TUI OAuth-callback wildcard bind (loopback-only now, RFC 8252); canonicalizes the origin allow-list the way a browser does; derives frame-ancestors from the real embedder list (IPv6-literal exclusion verified in headless Chromium); strictPort on all three bind points; and preserves the CLI exit-code map + JSON envelope through the mcp-inspector --cli launcher path. Shared host normalization lives in core/node/hostUrl.ts. Docs + tests + smokes throughout. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This commit is contained in:
parent
33e88eac9f
commit
cea22d86b8
4 changed files with 1610 additions and 86 deletions
|
|
@ -183,7 +183,7 @@ Register `http://127.0.0.1:6276/oauth/callback` on static or enterprise IdPs tha
|
|||
| `--client-id <id>` | — | OAuth client ID (static client); overrides `client.json`. |
|
||||
| `--client-secret <secret>` | — | OAuth client secret; overrides `client.json`. |
|
||||
| `--client-metadata-url <url>` | — | CIMD metadata URL; overrides `client.json`. |
|
||||
| `--callback-url <url>` | `MCP_OAUTH_CALLBACK_URL` | Redirect URI sent to the authorization server (default: `http://127.0.0.1:6276/oauth/callback`). |
|
||||
| `--callback-url <url>` | `MCP_OAUTH_CALLBACK_URL` | Redirect URI sent to the authorization server (default: `http://127.0.0.1:6276/oauth/callback`). Must bind a **loopback** host (`localhost` / `127.0.0.0/8` / `[::1]`) — the listener receives the authorization code over plaintext `http`, so a non-loopback host hard-errors (no opt-in; use a port-forward if the browser is elsewhere). |
|
||||
| — | `MCP_AUTO_OPEN_ENABLED` | Browser auto-open **and** non-TTY interactive-OAuth admit: `true` (allow interactive OAuth without a TTY **and** force-open the browser — same as the web launcher), `false` (never open), unset (open on a TTY unless `VITEST` is set). For CI that must not hang, prefer `--stored-auth-only`. |
|
||||
|
||||
**Example** — list tools on an OAuth-protected server using stored tokens and CIMD from the command line:
|
||||
|
|
|
|||
|
|
@ -580,6 +580,65 @@ describe("--print-handoff", () => {
|
|||
expect(out.apiToken).toBe("tok123");
|
||||
});
|
||||
|
||||
it("canonicalizes the deep-link host so it matches the web allow-list (mapped IPv4)", async () => {
|
||||
// HOST=::ffff:127.0.0.1 binds/serves 127.0.0.1; the deep link must advertise
|
||||
// that canonical host, not [::ffff:127.0.0.1], or the web autoConnect POST
|
||||
// 403s (the web allow-list emits the loopback trio for this HOST).
|
||||
const result = await runCli(
|
||||
["--print-handoff", "--server-url", "https://x.example/mcp"],
|
||||
{
|
||||
env: {
|
||||
MCP_INSPECTOR_API_TOKEN: "tok123",
|
||||
HOST: "::ffff:127.0.0.1",
|
||||
CLIENT_PORT: "16274",
|
||||
},
|
||||
},
|
||||
);
|
||||
expectCliSuccess(result);
|
||||
const out = JSON.parse(result.stdout) as { deepLink: string };
|
||||
expect(out.deepLink.startsWith("http://127.0.0.1:16274/?")).toBe(true);
|
||||
});
|
||||
|
||||
it("advertises localhost in the deep link for a wildcard HOST", async () => {
|
||||
// 0.0.0.0 is allow-listed so it connects, but the deep link is handed to a
|
||||
// human — advertise localhost like the web banner does.
|
||||
const result = await runCli(
|
||||
["--print-handoff", "--server-url", "https://x.example/mcp"],
|
||||
{
|
||||
env: {
|
||||
MCP_INSPECTOR_API_TOKEN: "tok123",
|
||||
HOST: "0.0.0.0",
|
||||
DANGEROUSLY_BIND_ALL_INTERFACES: "true",
|
||||
CLIENT_PORT: "16274",
|
||||
},
|
||||
},
|
||||
);
|
||||
expectCliSuccess(result);
|
||||
const out = JSON.parse(result.stdout) as { deepLink: string };
|
||||
expect(out.deepLink.startsWith("http://localhost:16274/?")).toBe(true);
|
||||
});
|
||||
|
||||
it("classifies a bad --callback-url as a usage error, not auth_required", async () => {
|
||||
// The guard's message contains "OAuth"; without the explicit exit-code pin
|
||||
// the heuristic would map it to auth_required (exit 3) and tell an automated
|
||||
// caller to re-auth on a config error. Fires before connect, so any server.
|
||||
const result = await runCli([
|
||||
"--server-url",
|
||||
"https://x.example/mcp",
|
||||
"--method",
|
||||
"tools/list",
|
||||
"--callback-url",
|
||||
"http://0.0.0.0:6276/oauth/callback",
|
||||
]);
|
||||
expect(result.exitCode).toBe(1);
|
||||
const envelope = JSON.parse(result.stderr.trim()) as {
|
||||
error: { code: string; message: string };
|
||||
};
|
||||
// "error" (USAGE), not "auth_required" — the point of pinning the exit code.
|
||||
expect(envelope.error.code).toBe("error");
|
||||
expect(envelope.error.message).toContain("must bind a loopback host");
|
||||
});
|
||||
|
||||
it("derives transport=sse for an SSE server (auto-detected from the /sse path)", async () => {
|
||||
const result = await runCli(
|
||||
["--print-handoff", "--server-url", "https://x.example/sse"],
|
||||
|
|
|
|||
|
|
@ -23,6 +23,10 @@ import {
|
|||
parseHeaderPair,
|
||||
} from "@inspector/core/mcp/node/index.js";
|
||||
import type { JsonValue } from "@inspector/core/mcp/index.js";
|
||||
import {
|
||||
canonicalUrlHost,
|
||||
isAllInterfacesHost,
|
||||
} from "@inspector/core/node/hostUrl.js";
|
||||
import { getStateFilePath } from "@inspector/core/auth/node/storage-node.js";
|
||||
import { consumeMethodOutcome } from "./handlers/consume-outcome.js";
|
||||
import { runMethod } from "./handlers/run-method.js";
|
||||
|
|
@ -422,6 +426,13 @@ function buildHandoff(
|
|||
transport: "sse" | "http" | "stdio" | undefined,
|
||||
): McpResponse {
|
||||
const host = process.env.HOST || "127.0.0.1";
|
||||
// The deep link is a URL handed to a human, so advertise localhost for a
|
||||
// wildcard bind (like the web banner/sandbox URL) rather than the awkward
|
||||
// http://0.0.0.0 / http://[::] — both are allow-listed, but neither is a nice
|
||||
// URL to click; otherwise use the canonical host so it matches the allow-list.
|
||||
const linkHost = isAllInterfacesHost(host)
|
||||
? "localhost"
|
||||
: canonicalUrlHost(host);
|
||||
const clientPort = process.env.CLIENT_PORT || "6274";
|
||||
const sandboxPort = process.env.MCP_SANDBOX_PORT || "6275";
|
||||
// Treat an empty MCP_INSPECTOR_API_TOKEN the same as unset — an empty token
|
||||
|
|
@ -440,7 +451,7 @@ function buildHandoff(
|
|||
if (apiToken) params.set("autoConnect", apiToken);
|
||||
return {
|
||||
serverUrl: normalizedUrl,
|
||||
deepLink: `http://${host}:${clientPort}/?${params.toString()}`,
|
||||
deepLink: `http://${linkHost}:${clientPort}/?${params.toString()}`,
|
||||
portForwardCmd: `coder port-forward <workspace> --tcp ${clientPort}:${clientPort} --tcp ${sandboxPort}:${sandboxPort}`,
|
||||
oauthStatePath: statePath,
|
||||
apiToken: apiToken ?? null,
|
||||
|
|
@ -665,7 +676,7 @@ async function parseArgs(argv?: string[]): Promise<ParseResult> {
|
|||
)
|
||||
.option(
|
||||
"--callback-url <url>",
|
||||
`OAuth redirect/callback listener URL (default: ${DEFAULT_RUNNER_OAUTH_CALLBACK_URL}, or MCP_OAUTH_CALLBACK_URL)`,
|
||||
`OAuth redirect/callback listener URL; must be loopback (default: ${DEFAULT_RUNNER_OAUTH_CALLBACK_URL}, or MCP_OAUTH_CALLBACK_URL)`,
|
||||
)
|
||||
.option(
|
||||
"--use-stored-auth",
|
||||
|
|
@ -1028,7 +1039,20 @@ export async function runCli(argv?: string[]): Promise<void> {
|
|||
relogin,
|
||||
} = parsed;
|
||||
const clientConfig = await loadRunnerClientConfig({ clientConfigPath });
|
||||
const callbackUrlConfig = parseRunnerOAuthCallbackUrl(callbackUrl);
|
||||
// A bad --callback-url / MCP_OAUTH_CALLBACK_URL is a *usage* error, but its
|
||||
// messages contain "OAuth", which the exit-code heuristic (error-handler.ts)
|
||||
// would otherwise classify as AUTH_REQUIRED (exit 3) — telling an automated
|
||||
// caller to kick the auth flow instead of fixing the flag. `core/` can't
|
||||
// import CliExitCodeError, so pin the class here.
|
||||
let callbackUrlConfig: RunnerOAuthCallbackConfig;
|
||||
try {
|
||||
callbackUrlConfig = parseRunnerOAuthCallbackUrl(callbackUrl);
|
||||
} catch (err) {
|
||||
throw new CliExitCodeError(
|
||||
EXIT_CODES.USAGE,
|
||||
err instanceof Error ? err.message : String(err),
|
||||
);
|
||||
}
|
||||
await callMethod(
|
||||
serverConfig,
|
||||
serverSettings,
|
||||
|
|
|
|||
|
|
@ -5,7 +5,11 @@ import { fileURLToPath } from "url";
|
|||
import { runCli, validLogLevels } from "./cli.js";
|
||||
import { handleError } from "./error-handler.js";
|
||||
|
||||
export { runCli, validLogLevels };
|
||||
// `handleError` is exported so the launcher (which imports `runCli` as a module
|
||||
// and owns the rejection) can route a `mcp-inspector --cli` failure through the
|
||||
// CLI's own sink — preserving the EXIT_CODES map and the JSON `{"error":…}`
|
||||
// envelope that this bin's own `.catch` provides only when run directly.
|
||||
export { runCli, validLogLevels, handleError };
|
||||
|
||||
const __filename = fileURLToPath(import.meta.url);
|
||||
const isMain =
|
||||
|
|
|
|||
Loading…
Reference in a new issue