mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-04 14:39:42 +02:00
fix(security): block DNS rebinding + cross-site CSRF + subagent-panel XSS
Adds an always-on Host-header allowlist and a cross-site Origin/CSRF guard, hardens the text/plain body parser, validates the WebSocket upgrade origin, and escapes AI-derived fields in the subagent panel. Closes the two CRITICALs and 5 HIGHs from the 2026-06-09 adversarial security review. - C1: no Host allowlist -> DNS rebinding drove the full API (RCE) on the default no-auth loopback install. New registerHostGuard rejects rebound custom domains; allows loopback, any IP literal, the bind host, *.ts.net / *.trycloudflare.com / *.cfargotunnel.com, the active managed tunnel, and CODEMAN_ALLOWED_HOSTS. - C2: a global text/plain parser JSON-parsed every body, enabling cross-site simple-request CSRF. Parser now keeps the raw string; /api/crash-diag self-parses; the global Origin guard rejects cross-site state changes. - H1/H3/H6: self-update, session create/input, and settings/tunnel toggles were CSRF-triggerable -> now covered by the Origin guard. - H4: the subagent activity panel injected raw AI tool names/inputs into innerHTML (executed under CSP 'unsafe-inline'). All sinks now escapeHtml'd. - H5: the WebSocket upgrade had no Origin/Host check (CSWSH) -> now validated. A missing Origin is allowed so curl/CLI and Claude Code hooks keep working; custom reverse-proxy domains need CODEMAN_ALLOWED_HOSTS=host,.suffix. Deferred: H2 (self-update tag signing, needs signing infra) and CSP 'unsafe-inline' removal (needs a nonce migration). Tests: test/network-host-guard.test.ts (19), test/routes/ws-routes.test.ts updated. Report: docs/reports/security-review-2026-06-09.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -12,6 +12,7 @@ import type { FastifyInstance, FastifyReply } from 'fastify';
|
||||
import { randomBytes, timingSafeEqual } from 'node:crypto';
|
||||
import { StaleExpirationMap } from '../../utils/index.js';
|
||||
import type { AuthSessionRecord } from '../ports/auth-port.js';
|
||||
import { isAllowedRequestHost, isAllowedRequestOrigin, type HostPolicy } from '../network-auth-policy.js';
|
||||
import {
|
||||
AUTH_SESSION_TTL_MS,
|
||||
MAX_AUTH_SESSIONS,
|
||||
@@ -157,6 +158,40 @@ export function registerAuthMiddleware(app: FastifyInstance, https: boolean): Au
|
||||
return state;
|
||||
}
|
||||
|
||||
/** Methods that don't change server state and so skip the cross-site Origin check. */
|
||||
const SAFE_HTTP_METHODS = new Set(['GET', 'HEAD', 'OPTIONS']);
|
||||
|
||||
/**
|
||||
* Register the anti-DNS-rebinding Host allowlist + cross-site (CSRF) Origin guard.
|
||||
*
|
||||
* This protects the API even on the default no-password install, where there is no
|
||||
* cookie/credential to gate on. It must be registered BEFORE the auth middleware so
|
||||
* forged cross-site or DNS-rebound requests are rejected up front. `getPolicy` is
|
||||
* evaluated per request so a tunnel started at runtime is reflected immediately.
|
||||
*
|
||||
* - Every request: the `Host` header must be in the allowlist (blocks DNS rebinding,
|
||||
* where a custom domain is rebound to 127.0.0.1 but still sends its own name).
|
||||
* - State-changing methods: the `Origin` (when the client sends one — i.e. a browser)
|
||||
* must be same-site (blocks cross-site CSRF, including the text/plain simple-request
|
||||
* trick). Non-browser clients (curl, Claude Code hooks) omit Origin and pass.
|
||||
*
|
||||
* WebSocket upgrades are validated separately in the ws route handler.
|
||||
*/
|
||||
export function registerHostGuard(app: FastifyInstance, getPolicy: () => HostPolicy): void {
|
||||
app.addHook('onRequest', (req, reply, done) => {
|
||||
const policy = getPolicy();
|
||||
if (!isAllowedRequestHost(req.headers.host, policy)) {
|
||||
reply.code(403).send('Forbidden: host not allowed');
|
||||
return;
|
||||
}
|
||||
if (!SAFE_HTTP_METHODS.has(req.method) && !isAllowedRequestOrigin(req.headers.origin, policy)) {
|
||||
reply.code(403).send('Forbidden: cross-site request blocked');
|
||||
return;
|
||||
}
|
||||
done();
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Register security headers and CORS middleware on every response.
|
||||
*/
|
||||
|
||||
@@ -19,3 +19,112 @@ export function isLoopbackBindHost(host: string): boolean {
|
||||
}
|
||||
return normalized.startsWith('::ffff:127.');
|
||||
}
|
||||
|
||||
/**
|
||||
* Hostname suffixes that are always accepted by the Host/Origin allowlist. These
|
||||
* are namespaces an external attacker cannot register DNS-rebinding records under
|
||||
* (tailscale MagicDNS, Cloudflare quick/named tunnels), so accepting them keeps
|
||||
* the project's documented tunnel access paths working without reopening the
|
||||
* rebinding hole. Extend per-deployment via CODEMAN_ALLOWED_HOSTS.
|
||||
*/
|
||||
export const DEFAULT_TRUSTED_HOST_SUFFIXES = ['.ts.net', '.trycloudflare.com', '.cfargotunnel.com'];
|
||||
|
||||
/** Policy inputs for the anti-DNS-rebinding Host allowlist + cross-site Origin guard. */
|
||||
export interface HostPolicy {
|
||||
/** The host the server is bound to (e.g. '127.0.0.1', '0.0.0.0', or a hostname). */
|
||||
bindHost: string;
|
||||
/** Extra allowed hosts: exact lowercased names, or a leading-dot '.suffix' for suffix matches. */
|
||||
allowedHosts: string[];
|
||||
/** Hostname of the currently-active Codeman-managed tunnel, if any. */
|
||||
tunnelHost?: string | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract the lowercased hostname from a Host/authority value, stripping the port
|
||||
* and IPv6 brackets. Returns null for empty/garbage input.
|
||||
*/
|
||||
export function parseAuthorityHostname(authority: string | undefined): string | null {
|
||||
if (!authority) return null;
|
||||
let h = authority.trim();
|
||||
if (!h) return null;
|
||||
if (h.startsWith('[')) {
|
||||
// [::1] or [::1]:3000
|
||||
const end = h.indexOf(']');
|
||||
if (end === -1) return null;
|
||||
return h.slice(1, end).toLowerCase() || null;
|
||||
}
|
||||
// host:port — only treat a single trailing colon as a port separator so a
|
||||
// bracketless IPv6 literal (multiple colons) is left intact.
|
||||
const first = h.indexOf(':');
|
||||
if (first !== -1 && first === h.lastIndexOf(':')) {
|
||||
h = h.slice(0, first);
|
||||
}
|
||||
return h.toLowerCase() || null;
|
||||
}
|
||||
|
||||
/** Build a HostPolicy from the bind host, CODEMAN_ALLOWED_HOSTS, and an active tunnel URL. */
|
||||
export function buildHostPolicy(bindHost: string, tunnelUrl?: string | null): HostPolicy {
|
||||
const allowedHosts = (process.env.CODEMAN_ALLOWED_HOSTS || '')
|
||||
.split(',')
|
||||
.map((s) => s.trim().toLowerCase())
|
||||
.filter(Boolean);
|
||||
let tunnelHost: string | null = null;
|
||||
if (tunnelUrl) {
|
||||
try {
|
||||
tunnelHost = new URL(tunnelUrl).hostname.toLowerCase();
|
||||
} catch {
|
||||
tunnelHost = null;
|
||||
}
|
||||
}
|
||||
return { bindHost, allowedHosts, tunnelHost };
|
||||
}
|
||||
|
||||
function matchesHost(hostname: string, policy: HostPolicy): boolean {
|
||||
// localhost is reserved (always resolves to loopback, not rebindable).
|
||||
if (hostname === 'localhost') return true;
|
||||
// Any IP literal: a literal address cannot be the target of DNS rebinding — the
|
||||
// browser connected straight to it, there is no name to re-point.
|
||||
if (isIP(hostname) !== 0) return true;
|
||||
const bind = parseAuthorityHostname(policy.bindHost);
|
||||
if (bind && hostname === bind) return true;
|
||||
if (policy.tunnelHost && hostname === policy.tunnelHost) return true;
|
||||
for (const suffix of DEFAULT_TRUSTED_HOST_SUFFIXES) {
|
||||
if (hostname === suffix.slice(1) || hostname.endsWith(suffix)) return true;
|
||||
}
|
||||
for (const entry of policy.allowedHosts) {
|
||||
if (entry.startsWith('.')) {
|
||||
if (hostname === entry.slice(1) || hostname.endsWith(entry)) return true;
|
||||
} else if (hostname === entry) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* True if a request's Host header is allowed. Blocks DNS-rebinding: a custom
|
||||
* domain rebound to a loopback/LAN address still carries its own name in Host,
|
||||
* which will not be in the allowlist.
|
||||
*/
|
||||
export function isAllowedRequestHost(hostHeader: string | undefined, policy: HostPolicy): boolean {
|
||||
const hostname = parseAuthorityHostname(hostHeader);
|
||||
if (!hostname) return false;
|
||||
return matchesHost(hostname, policy);
|
||||
}
|
||||
|
||||
/**
|
||||
* True if a request's Origin is allowed for a state-changing / WebSocket request.
|
||||
* A MISSING Origin is allowed: non-browser clients (curl, Claude Code hooks) omit
|
||||
* it, while browsers always attach it on cross-origin state-changing/WS requests —
|
||||
* so a forged cross-site request is caught while local automation keeps working.
|
||||
* The opaque origin 'null' (sandboxed iframe, data: URL) is rejected.
|
||||
*/
|
||||
export function isAllowedRequestOrigin(originHeader: string | undefined, policy: HostPolicy): boolean {
|
||||
if (originHeader === undefined || originHeader === '') return true;
|
||||
if (originHeader === 'null') return false;
|
||||
try {
|
||||
return matchesHost(new URL(originHeader).hostname.toLowerCase(), policy);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -802,13 +802,13 @@ Object.assign(CodemanApp.prototype, {
|
||||
const time = new Date(a.timestamp).toLocaleTimeString('en-US', { hour12: false });
|
||||
if (a.type === 'tool') {
|
||||
const toolDetail = this.getToolDetailExpanded(a.tool, a.input, a.fullInput, a.toolUseId);
|
||||
return `<div class="subagent-activity tool" data-tool-use-id="${a.toolUseId || ''}">
|
||||
return `<div class="subagent-activity tool" data-tool-use-id="${escapeHtml(a.toolUseId || '')}">
|
||||
<span class="time">${time}</span>
|
||||
<span class="icon">${this.getToolIcon(a.tool)}</span>
|
||||
<span class="name">${a.tool}</span>
|
||||
<span class="detail">${toolDetail.primary}</span>
|
||||
<span class="name">${escapeHtml(a.tool)}</span>
|
||||
<span class="detail">${escapeHtml(toolDetail.primary)}</span>
|
||||
${toolDetail.hasMore ? `<button class="tool-expand-btn" onclick="app.toggleToolParams('${escapeHtml(a.toolUseId)}')">▶</button>` : ''}
|
||||
${toolDetail.hasMore ? `<div class="tool-params-expanded" id="tool-params-${a.toolUseId}" style="display:none;"><pre>${escapeHtml(JSON.stringify(a.fullInput || a.input, null, 2))}</pre></div>` : ''}
|
||||
${toolDetail.hasMore ? `<div class="tool-params-expanded" id="tool-params-${escapeHtml(a.toolUseId)}" style="display:none;"><pre>${escapeHtml(JSON.stringify(a.fullInput || a.input, null, 2))}</pre></div>` : ''}
|
||||
</div>`;
|
||||
} else if (a.type === 'tool_result') {
|
||||
const icon = a.isError ? '❌' : '📄';
|
||||
@@ -818,7 +818,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
return `<div class="subagent-activity tool-result ${statusClass}">
|
||||
<span class="time">${time}</span>
|
||||
<span class="icon">${icon}</span>
|
||||
<span class="name">${a.tool || 'result'}</span>
|
||||
<span class="name">${escapeHtml(a.tool || 'result')}</span>
|
||||
<span class="detail">${escapeHtml(preview)}${sizeInfo}</span>
|
||||
</div>`;
|
||||
} else if (a.type === 'progress') {
|
||||
@@ -830,7 +830,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
return `<div class="subagent-activity progress${hookClass}">
|
||||
<span class="time">${time}</span>
|
||||
<span class="icon">${icon}</span>
|
||||
<span class="detail">${displayText}</span>
|
||||
<span class="detail">${escapeHtml(displayText)}</span>
|
||||
</div>`;
|
||||
} else if (a.type === 'message') {
|
||||
const preview = a.text.length > 100 ? a.text.substring(0, 100) + '...' : a.text;
|
||||
@@ -1400,7 +1400,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
return `<div class="activity-line">
|
||||
<span class="time">${time}</span>
|
||||
<span class="tool-icon">${this.getToolIcon(a.tool)}</span>
|
||||
<span class="tool-name">${a.tool}</span>
|
||||
<span class="tool-name">${escapeHtml(a.tool)}</span>
|
||||
<span class="tool-detail">${escapeHtml(this.getToolDetail(a.tool, a.input))}</span>
|
||||
</div>`;
|
||||
} else if (a.type === 'tool_result') {
|
||||
@@ -1411,7 +1411,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
return `<div class="activity-line result-line${statusClass}">
|
||||
<span class="time">${time}</span>
|
||||
<span class="tool-icon">${icon}</span>
|
||||
<span class="tool-name">${a.tool || '→'}</span>
|
||||
<span class="tool-name">${escapeHtml(a.tool || '→')}</span>
|
||||
<span class="tool-detail">${escapeHtml(preview)}${sizeInfo}</span>
|
||||
</div>`;
|
||||
} else if (a.type === 'progress') {
|
||||
|
||||
@@ -30,6 +30,7 @@ import { FastifyInstance } from 'fastify';
|
||||
import type { WebSocket } from 'ws';
|
||||
import type { SessionPort } from '../ports/session-port.js';
|
||||
import { MAX_INPUT_LENGTH } from '../../config/terminal-limits.js';
|
||||
import { isAllowedRequestHost, isAllowedRequestOrigin, type HostPolicy } from '../network-auth-policy.js';
|
||||
|
||||
/** Micro-batch interval for terminal output (ms). Short enough for low latency,
|
||||
* long enough to group Ink's rapid cursor-up redraw sequences into single frames. */
|
||||
@@ -58,8 +59,19 @@ const MAX_WS_PER_SESSION = 5;
|
||||
/** Track active WS connections per session for connection limiting. */
|
||||
const sessionWsCount = new Map<string, number>();
|
||||
|
||||
export function registerWsRoutes(app: FastifyInstance, ctx: SessionPort): void {
|
||||
export function registerWsRoutes(app: FastifyInstance, ctx: SessionPort, getHostPolicy: () => HostPolicy): void {
|
||||
app.get<{ Params: { id: string } }>('/ws/sessions/:id/terminal', { websocket: true }, (socket: WebSocket, req) => {
|
||||
// Reject cross-site WebSocket hijacking (CSWSH) and DNS-rebinding before doing
|
||||
// anything: the upgrade must come from an allowed Host and (when the browser
|
||||
// sends one — it always does for WS) a same-site Origin. Writing to this socket
|
||||
// injects keystrokes into a --dangerously-skip-permissions agent, so this gate
|
||||
// matters even on the default no-password install. See security review H5.
|
||||
const policy = getHostPolicy();
|
||||
if (!isAllowedRequestHost(req.headers.host, policy) || !isAllowedRequestOrigin(req.headers.origin, policy)) {
|
||||
socket.close(4003, 'Forbidden');
|
||||
return;
|
||||
}
|
||||
|
||||
const { id } = req.params;
|
||||
const session = ctx.sessions.get(id);
|
||||
|
||||
|
||||
+41
-15
@@ -102,9 +102,9 @@ import type { EventLoopMonitorHandle } from '../utils/index.js';
|
||||
import { MAX_CONCURRENT_SESSIONS, MAX_SSE_CLIENTS } from '../config/map-limits.js';
|
||||
import { SseEvent } from './sse-events.js';
|
||||
import type { ScheduledRun } from './ports/index.js';
|
||||
import { registerAuthMiddleware, registerSecurityHeaders } from './middleware/auth.js';
|
||||
import { registerAuthMiddleware, registerSecurityHeaders, registerHostGuard } from './middleware/auth.js';
|
||||
import { installRouteErrorHandler } from './route-error-handler.js';
|
||||
import { isExplicitlyEnabled, isLoopbackBindHost } from './network-auth-policy.js';
|
||||
import { isExplicitlyEnabled, isLoopbackBindHost, buildHostPolicy, type HostPolicy } from './network-auth-policy.js';
|
||||
import {
|
||||
registerPushRoutes,
|
||||
registerTeamRoutes,
|
||||
@@ -531,6 +531,14 @@ export class WebServer extends EventEmitter {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Current Host/Origin allowlist policy. Read per request so a tunnel started at
|
||||
* runtime (PUT /api/settings) is reflected without a restart.
|
||||
*/
|
||||
private getHostPolicy(): HostPolicy {
|
||||
return buildHostPolicy(this.host, this.tunnelManager.getUrl());
|
||||
}
|
||||
|
||||
private async setupRoutes(): Promise<void> {
|
||||
// multipart/form-data: parser is provided by @fastify/multipart (registered
|
||||
// below). Its parser is a no-op marker that leaves the body on req.raw, so
|
||||
@@ -547,6 +555,11 @@ export class WebServer extends EventEmitter {
|
||||
// Cookie plugin (needed for auth session tokens)
|
||||
await this.app.register(fastifyCookie);
|
||||
|
||||
// Anti-DNS-rebinding Host allowlist + cross-site (CSRF) Origin guard. Registered
|
||||
// before auth so forged cross-site / rebound requests are rejected up front, even
|
||||
// on the default no-password install. See docs/reports/security-review-2026-06-09.md.
|
||||
registerHostGuard(this.app, () => this.getHostPolicy());
|
||||
|
||||
// Auth middleware (Basic Auth + session cookies + rate limiting)
|
||||
const authState = registerAuthMiddleware(this.app, this.https);
|
||||
if (authState) {
|
||||
@@ -698,24 +711,28 @@ export class WebServer extends EventEmitter {
|
||||
// parseBody. Shared with the route test harness so test behavior matches prod.
|
||||
installRouteErrorHandler(this.app);
|
||||
|
||||
// Crash diagnostics beacon — frontend POSTs breadcrumbs, GET to read them
|
||||
// Crash diagnostics beacon — frontend POSTs breadcrumbs, GET to read them.
|
||||
// text/plain is used ONLY by this beacon (navigator.sendBeacon sends text/plain).
|
||||
// Keep the body as a RAW STRING and parse it inside the handler — a global
|
||||
// text/plain -> JSON parser would let a cross-site "simple request" (no CORS
|
||||
// preflight) submit JSON to any route. See security review C2.
|
||||
let _crashBreadcrumbs = '';
|
||||
this.app.addContentTypeParser('text/plain;charset=UTF-8', { parseAs: 'string' }, (_req, body, done) => {
|
||||
try {
|
||||
done(null, JSON.parse(body as string));
|
||||
} catch {
|
||||
done(null, { data: body });
|
||||
}
|
||||
done(null, body);
|
||||
});
|
||||
this.app.addContentTypeParser('text/plain', { parseAs: 'string' }, (_req, body, done) => {
|
||||
try {
|
||||
done(null, JSON.parse(body as string));
|
||||
} catch {
|
||||
done(null, { data: body });
|
||||
}
|
||||
done(null, body);
|
||||
});
|
||||
this.app.post('/api/crash-diag', (req, reply) => {
|
||||
_crashBreadcrumbs = String((req.body as { data?: string })?.data || '');
|
||||
const raw = typeof req.body === 'string' ? req.body : '';
|
||||
let data = raw;
|
||||
try {
|
||||
const parsed = JSON.parse(raw) as { data?: unknown };
|
||||
if (parsed && typeof parsed.data === 'string') data = parsed.data;
|
||||
} catch {
|
||||
/* not JSON — treat the raw beacon text as the breadcrumbs */
|
||||
}
|
||||
_crashBreadcrumbs = String(data || '');
|
||||
reply.code(204).send();
|
||||
});
|
||||
this.app.get('/api/crash-diag', (_req, reply) => {
|
||||
@@ -738,7 +755,7 @@ export class WebServer extends EventEmitter {
|
||||
registerPlanRoutes(this.app, ctx);
|
||||
registerClipboardRoutes(this.app, ctx);
|
||||
registerOrchestratorRoutes(this.app, ctx);
|
||||
registerWsRoutes(this.app, ctx);
|
||||
registerWsRoutes(this.app, ctx, () => this.getHostPolicy());
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1699,6 +1716,15 @@ export class WebServer extends EventEmitter {
|
||||
const displayHost = this.host === '0.0.0.0' ? 'localhost' : this.host;
|
||||
console.log(`Codeman web interface running at ${protocol}://${displayHost}:${this.port}`);
|
||||
|
||||
// Anti-DNS-rebinding Host allowlist is always on. Localhost, any bare IP, the
|
||||
// bind host, *.ts.net / *.trycloudflare.com / *.cfargotunnel.com, and the active
|
||||
// managed tunnel are accepted automatically; add any other domain you front this
|
||||
// with (e.g. a custom reverse-proxy host) via CODEMAN_ALLOWED_HOSTS=host1,.suffix.
|
||||
const extraAllowed = (process.env.CODEMAN_ALLOWED_HOSTS || '').trim();
|
||||
if (extraAllowed) {
|
||||
console.log(` Host allowlist also accepts: ${extraAllowed}`);
|
||||
}
|
||||
|
||||
// Codeman binds loopback (127.0.0.1) by default, which is safe out of the box.
|
||||
// If the user opts into a non-loopback bind (e.g. --host 0.0.0.0) WITHOUT a
|
||||
// password we no longer refuse to start — that surprised people whose setups
|
||||
|
||||
Reference in New Issue
Block a user