Skip to content

Commit 603ce7f

Browse files
authored
fix: keep MCP daemon loopback-only by default (#46)
1 parent 7aa24b9 commit 603ce7f

7 files changed

Lines changed: 156 additions & 14 deletions

File tree

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,8 @@ agent_notes = false
144144

145145
- `hunk diff --agent-context <file>` loads inline agent rationale from a JSON sidecar
146146
- `hunk mcp serve` runs the local MCP daemon for agent-to-diff communication
147+
- Hunk keeps the daemon loopback-only by default
148+
- if you intentionally need remote access, set `HUNK_MCP_UNSAFE_ALLOW_REMOTE=1` and choose a non-loopback `HUNK_MCP_HOST`
147149

148150
## Performance notes
149151

src/core/cli.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,8 +353,9 @@ async function parseMcpCommand(tokens: string[]): Promise<ParsedCliInput> {
353353
"Run the local Hunk MCP daemon and websocket session broker.",
354354
"",
355355
"Environment:",
356-
" HUNK_MCP_HOST bind host (default 127.0.0.1)",
357-
" HUNK_MCP_PORT bind port (default 47657)",
356+
" HUNK_MCP_HOST bind host (default 127.0.0.1; loopback only unless explicitly overridden)",
357+
" HUNK_MCP_PORT bind port (default 47657)",
358+
" HUNK_MCP_UNSAFE_ALLOW_REMOTE set to 1 to allow non-loopback binding (unsafe)",
358359
].join("\n") + "\n",
359360
};
360361
}

src/mcp/client.ts

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import type {
77
SessionCommandResult,
88
SessionServerMessage,
99
} from "./types";
10-
import { HUNK_SESSION_SOCKET_PATH, resolveHunkMcpConfig } from "./config";
10+
import { HUNK_SESSION_SOCKET_PATH, resolveHunkMcpConfig, type ResolvedHunkMcpConfig } from "./config";
1111
import { isHunkDaemonHealthy, isLoopbackPortReachable, launchHunkDaemon, waitForHunkDaemonHealth } from "./daemonLauncher";
1212

1313
const DAEMON_LAUNCH_COOLDOWN_MS = 5_000;
@@ -31,7 +31,6 @@ export class HunkHostClient {
3131
private startupPromise: Promise<void> | null = null;
3232
private lastDaemonLaunchStartedAt = 0;
3333
private lastConnectionWarning: string | null = null;
34-
private readonly config = resolveHunkMcpConfig();
3534

3635
constructor(
3736
private readonly registration: HunkSessionRegistration,
@@ -73,13 +72,18 @@ export class HunkHostClient {
7372
this.websocket = null;
7473
}
7574

75+
private resolveConfig() {
76+
return resolveHunkMcpConfig();
77+
}
78+
7679
private async ensureDaemonAndConnect() {
77-
await this.ensureDaemonAvailable();
78-
this.connect();
80+
const config = this.resolveConfig();
81+
await this.ensureDaemonAvailable(config);
82+
this.connect(config);
7983
}
8084

81-
private async ensureDaemonAvailable() {
82-
if (await isHunkDaemonHealthy(this.config)) {
85+
private async ensureDaemonAvailable(config: ResolvedHunkMcpConfig) {
86+
if (await isHunkDaemonHealthy(config)) {
8387
this.lastConnectionWarning = null;
8488
return;
8589
}
@@ -91,7 +95,7 @@ export class HunkHostClient {
9195
}
9296

9397
const ready = await waitForHunkDaemonHealth({
94-
config: this.config,
98+
config,
9599
timeoutMs: shouldLaunch ? DAEMON_STARTUP_TIMEOUT_MS : 1_500,
96100
});
97101

@@ -100,16 +104,16 @@ export class HunkHostClient {
100104
return;
101105
}
102106

103-
const portReachable = await isLoopbackPortReachable(this.config);
107+
const portReachable = await isLoopbackPortReachable(config);
104108
if (portReachable) {
105109
throw new Error(
106-
`Hunk MCP port ${this.config.host}:${this.config.port} is already in use by another process. ` +
110+
`Hunk MCP port ${config.host}:${config.port} is already in use by another process. ` +
107111
`Stop the conflicting process or set HUNK_MCP_PORT to a different loopback port.`,
108112
);
109113
}
110114

111115
throw new Error(
112-
`Timed out waiting for the Hunk MCP daemon on ${this.config.host}:${this.config.port}. ` +
116+
`Timed out waiting for the Hunk MCP daemon on ${config.host}:${config.port}. ` +
113117
`Hunk will retry in the background.`,
114118
);
115119
}
@@ -128,12 +132,12 @@ export class HunkHostClient {
128132
});
129133
}
130134

131-
private connect() {
135+
private connect(config: ResolvedHunkMcpConfig) {
132136
if (this.stopped || this.websocket) {
133137
return;
134138
}
135139

136-
const websocket = new WebSocket(`${this.config.wsOrigin}${HUNK_SESSION_SOCKET_PATH}`);
140+
const websocket = new WebSocket(`${config.wsOrigin}${HUNK_SESSION_SOCKET_PATH}`);
137141
this.websocket = websocket;
138142

139143
websocket.onopen = () => {

src/mcp/config.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1+
import { isIP } from "node:net";
2+
13
const DEFAULT_HUNK_MCP_HOST = "127.0.0.1";
24
const DEFAULT_HUNK_MCP_PORT = 47657;
35
export const HUNK_MCP_PATH = "/mcp";
46
export const HUNK_SESSION_SOCKET_PATH = "/session";
7+
export const HUNK_MCP_UNSAFE_ALLOW_REMOTE_ENV = "HUNK_MCP_UNSAFE_ALLOW_REMOTE";
58

69
export interface ResolvedHunkMcpConfig {
710
host: string;
@@ -10,12 +13,51 @@ export interface ResolvedHunkMcpConfig {
1013
wsOrigin: string;
1114
}
1215

16+
/** Return whether one bind host stays on the local loopback interface. */
17+
export function isLoopbackHost(host: string) {
18+
const normalized = host.trim().toLowerCase();
19+
20+
if (normalized.length === 0) {
21+
return false;
22+
}
23+
24+
if (normalized === "localhost" || normalized === "::1" || normalized === "0:0:0:0:0:0:0:1") {
25+
return true;
26+
}
27+
28+
if (normalized.startsWith("[") && normalized.endsWith("]")) {
29+
return isLoopbackHost(normalized.slice(1, -1));
30+
}
31+
32+
if (normalized.startsWith("::ffff:")) {
33+
return isLoopbackHost(normalized.slice("::ffff:".length));
34+
}
35+
36+
if (isIP(normalized) === 4) {
37+
return normalized.startsWith("127.");
38+
}
39+
40+
return false;
41+
}
42+
43+
/** Return whether the user explicitly opted into exposing the daemon beyond loopback. */
44+
export function allowsUnsafeRemoteMcp(env: NodeJS.ProcessEnv = process.env) {
45+
return env[HUNK_MCP_UNSAFE_ALLOW_REMOTE_ENV] === "1";
46+
}
47+
1348
/** Resolve the loopback host/port Hunk should use for the local MCP daemon. */
1449
export function resolveHunkMcpConfig(env: NodeJS.ProcessEnv = process.env): ResolvedHunkMcpConfig {
1550
const host = env.HUNK_MCP_HOST?.trim() || DEFAULT_HUNK_MCP_HOST;
1651
const parsedPort = Number.parseInt(env.HUNK_MCP_PORT ?? "", 10);
1752
const port = Number.isInteger(parsedPort) && parsedPort > 0 ? parsedPort : DEFAULT_HUNK_MCP_PORT;
1853

54+
if (!isLoopbackHost(host) && !allowsUnsafeRemoteMcp(env)) {
55+
throw new Error(
56+
`Hunk MCP refuses to bind ${host}:${port} because the daemon is local-only by default. ` +
57+
`Use a loopback host such as 127.0.0.1, localhost, or ::1, or set ${HUNK_MCP_UNSAFE_ALLOW_REMOTE_ENV}=1 if you intentionally want remote access.`,
58+
);
59+
}
60+
1961
return {
2062
host,
2163
port,

test/mcp-client.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type { HunkSessionRegistration, HunkSessionSnapshot } from "../src/mcp/ty
66
const originalHost = process.env.HUNK_MCP_HOST;
77
const originalPort = process.env.HUNK_MCP_PORT;
88
const originalDisable = process.env.HUNK_MCP_DISABLE;
9+
const originalUnsafeRemote = process.env.HUNK_MCP_UNSAFE_ALLOW_REMOTE;
910
const originalConsoleError = console.error;
1011

1112
function createRegistration(): HunkSessionRegistration {
@@ -74,10 +75,40 @@ afterEach(() => {
7475
process.env.HUNK_MCP_DISABLE = originalDisable;
7576
}
7677

78+
if (originalUnsafeRemote === undefined) {
79+
delete process.env.HUNK_MCP_UNSAFE_ALLOW_REMOTE;
80+
} else {
81+
process.env.HUNK_MCP_UNSAFE_ALLOW_REMOTE = originalUnsafeRemote;
82+
}
83+
7784
console.error = originalConsoleError;
7885
});
7986

8087
describe("Hunk MCP client", () => {
88+
test("logs one actionable warning when MCP is configured for a non-loopback host without opt-in", async () => {
89+
process.env.HUNK_MCP_HOST = "0.0.0.0";
90+
process.env.HUNK_MCP_PORT = "47657";
91+
delete process.env.HUNK_MCP_UNSAFE_ALLOW_REMOTE;
92+
delete process.env.HUNK_MCP_DISABLE;
93+
94+
const messages: string[] = [];
95+
console.error = (...args: unknown[]) => {
96+
messages.push(args.map((value) => String(value)).join(" "));
97+
};
98+
99+
const client = new HunkHostClient(createRegistration(), createSnapshot());
100+
101+
try {
102+
client.start();
103+
await waitUntil("non-loopback MCP warning", () => messages.length === 1);
104+
105+
expect(messages[0]).toContain("[hunk:mcp] Hunk MCP refuses to bind 0.0.0.0:47657 because the daemon is local-only by default.");
106+
expect(messages[0]).toContain("HUNK_MCP_UNSAFE_ALLOW_REMOTE=1");
107+
} finally {
108+
client.stop();
109+
}
110+
}, 10_000);
111+
81112
test("logs one actionable warning when a non-Hunk listener owns the MCP port", async () => {
82113
const conflictingListener = createServer((_request, response) => {
83114
response.writeHead(404, { "content-type": "text/plain" });

test/mcp-config.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import { describe, expect, test } from "bun:test";
2+
import {
3+
HUNK_MCP_UNSAFE_ALLOW_REMOTE_ENV,
4+
allowsUnsafeRemoteMcp,
5+
isLoopbackHost,
6+
resolveHunkMcpConfig,
7+
} from "../src/mcp/config";
8+
9+
describe("Hunk MCP config", () => {
10+
test("accepts loopback hosts without an unsafe override", () => {
11+
expect(isLoopbackHost("127.0.0.1")).toBe(true);
12+
expect(isLoopbackHost("127.1.2.3")).toBe(true);
13+
expect(isLoopbackHost("localhost")).toBe(true);
14+
expect(isLoopbackHost("::1")).toBe(true);
15+
expect(isLoopbackHost("0:0:0:0:0:0:0:1")).toBe(true);
16+
expect(isLoopbackHost("::ffff:127.0.0.1")).toBe(true);
17+
expect(isLoopbackHost("0.0.0.0")).toBe(false);
18+
expect(isLoopbackHost("192.168.1.20")).toBe(false);
19+
expect(isLoopbackHost("example.com")).toBe(false);
20+
});
21+
22+
test("refuses non-loopback binds unless the unsafe override is enabled", () => {
23+
expect(() =>
24+
resolveHunkMcpConfig({
25+
HUNK_MCP_HOST: "0.0.0.0",
26+
HUNK_MCP_PORT: "49000",
27+
}),
28+
).toThrow("local-only by default");
29+
30+
expect(
31+
resolveHunkMcpConfig({
32+
HUNK_MCP_HOST: "0.0.0.0",
33+
HUNK_MCP_PORT: "49000",
34+
[HUNK_MCP_UNSAFE_ALLOW_REMOTE_ENV]: "1",
35+
}),
36+
).toMatchObject({
37+
host: "0.0.0.0",
38+
port: 49000,
39+
});
40+
});
41+
42+
test("reports whether unsafe remote MCP access was explicitly enabled", () => {
43+
expect(allowsUnsafeRemoteMcp({})).toBe(false);
44+
expect(allowsUnsafeRemoteMcp({ [HUNK_MCP_UNSAFE_ALLOW_REMOTE_ENV]: "0" })).toBe(false);
45+
expect(allowsUnsafeRemoteMcp({ [HUNK_MCP_UNSAFE_ALLOW_REMOTE_ENV]: "1" })).toBe(true);
46+
});
47+
});

test/mcp-server.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { serveHunkMcpServer } from "../src/mcp/server";
44

55
const originalHost = process.env.HUNK_MCP_HOST;
66
const originalPort = process.env.HUNK_MCP_PORT;
7+
const originalUnsafeRemote = process.env.HUNK_MCP_UNSAFE_ALLOW_REMOTE;
78

89
afterEach(() => {
910
if (originalHost === undefined) {
@@ -17,9 +18,23 @@ afterEach(() => {
1718
} else {
1819
process.env.HUNK_MCP_PORT = originalPort;
1920
}
21+
22+
if (originalUnsafeRemote === undefined) {
23+
delete process.env.HUNK_MCP_UNSAFE_ALLOW_REMOTE;
24+
} else {
25+
process.env.HUNK_MCP_UNSAFE_ALLOW_REMOTE = originalUnsafeRemote;
26+
}
2027
});
2128

2229
describe("Hunk MCP server", () => {
30+
test("refuses non-loopback binding unless explicitly allowed", () => {
31+
process.env.HUNK_MCP_HOST = "0.0.0.0";
32+
process.env.HUNK_MCP_PORT = "47657";
33+
delete process.env.HUNK_MCP_UNSAFE_ALLOW_REMOTE;
34+
35+
expect(() => serveHunkMcpServer()).toThrow("local-only by default");
36+
});
37+
2338
test("reports a clear error when the daemon port is already in use", async () => {
2439
const listener = createServer(() => undefined);
2540
await new Promise<void>((resolve, reject) => {

0 commit comments

Comments
 (0)