mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix: COD-29 fail closed for unauthenticated network binds
This commit is contained in:
+11
-3
@@ -483,21 +483,29 @@ program
|
||||
program
|
||||
.command('web')
|
||||
.description('Start the web interface')
|
||||
.option('-H, --host <host>', 'Host to bind to', process.env.CODEMAN_HOST || '127.0.0.1')
|
||||
.option('-p, --port <port>', 'Port to listen on (env: CODEMAN_PORT)', process.env.CODEMAN_PORT || '3000')
|
||||
.option('--https', 'Enable HTTPS with self-signed certificate (only needed for remote access, not localhost)')
|
||||
.option('--title-hostname <hostname>', 'Override the hostname shown in the browser title')
|
||||
.option(
|
||||
'--allow-unauthenticated-network',
|
||||
'Allow non-loopback web access without CODEMAN_PASSWORD (dangerous; terminal control is exposed)'
|
||||
)
|
||||
.action(async (options) => {
|
||||
const { startWebServer } = await import('./web/server.js');
|
||||
const host = options.host;
|
||||
const port = parseInt(options.port, 10);
|
||||
const https = !!options.https;
|
||||
const titleHostname = options.titleHostname;
|
||||
const allowUnauthenticatedNetwork = !!options.allowUnauthenticatedNetwork;
|
||||
const protocol = https ? 'https' : 'http';
|
||||
const displayHost = host === '0.0.0.0' ? 'localhost' : host;
|
||||
|
||||
console.log(chalk.cyan(`Starting Codeman web interface on port ${port}${https ? ' (HTTPS)' : ''}...`));
|
||||
console.log(chalk.cyan(`Starting Codeman web interface on ${displayHost}:${port}${https ? ' (HTTPS)' : ''}...`));
|
||||
|
||||
try {
|
||||
const server = await startWebServer(port, https, false, titleHostname);
|
||||
console.log(chalk.green(`\n✓ Web interface running at ${protocol}://localhost:${port}`));
|
||||
const server = await startWebServer(port, https, false, host, titleHostname, allowUnauthenticatedNetwork);
|
||||
console.log(chalk.green(`\n✓ Web interface running at ${protocol}://${displayHost}:${port}`));
|
||||
if (https) {
|
||||
console.log(chalk.yellow(' Note: Accept the self-signed certificate in your browser on first visit'));
|
||||
}
|
||||
|
||||
+58
-12
@@ -42,6 +42,7 @@ import { execSync } from 'node:child_process';
|
||||
import { hostname as getHostname } from 'node:os';
|
||||
import { dataPath } from '../config/instance.js';
|
||||
import { EventEmitter } from 'node:events';
|
||||
import { isIP } from 'node:net';
|
||||
import { Session, type BackgroundTask } from '../session.js';
|
||||
import type { ClaudeMode, SessionState } from '../types.js';
|
||||
import { RespawnController, RespawnConfig } from '../respawn-controller.js';
|
||||
@@ -122,6 +123,26 @@ import {
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
|
||||
const EXPLICIT_TRUE_VALUES = new Set(['1', 'true', 'yes', 'on']);
|
||||
|
||||
function isExplicitlyEnabled(value: string | undefined): boolean {
|
||||
return value !== undefined && EXPLICIT_TRUE_VALUES.has(value.trim().toLowerCase());
|
||||
}
|
||||
|
||||
function isLoopbackBindHost(host: string): boolean {
|
||||
const normalized = host
|
||||
.trim()
|
||||
.toLowerCase()
|
||||
.replace(/^\[(.*)\]$/, '$1');
|
||||
if (normalized === 'localhost' || normalized === '::1' || normalized === '0:0:0:0:0:0:0:1') {
|
||||
return true;
|
||||
}
|
||||
if (isIP(normalized) === 4 && normalized.startsWith('127.')) {
|
||||
return true;
|
||||
}
|
||||
return normalized.startsWith('::ffff:127.');
|
||||
}
|
||||
|
||||
// Bounded, predictable shape for SSE client identifiers: alphanumerics, `_`, `-`.
|
||||
// Length range covers crypto.randomUUID() (36 chars) plus any short stable IDs,
|
||||
// while capping growth of `sseClientsById` and blocking pathological inputs.
|
||||
@@ -191,6 +212,7 @@ export class WebServer extends EventEmitter {
|
||||
private sse: SseStreamManager;
|
||||
private store = getStore();
|
||||
private port: number;
|
||||
private host: string;
|
||||
private https: boolean;
|
||||
private testMode: boolean;
|
||||
private mux: TerminalMultiplexer;
|
||||
@@ -232,6 +254,10 @@ export class WebServer extends EventEmitter {
|
||||
private pushStore: PushSubscriptionStore = new PushSubscriptionStore();
|
||||
private teamWatcher: TeamWatcher = new TeamWatcher();
|
||||
private _orchestratorLoop: import('../orchestrator-loop.js').OrchestratorLoop | null = null;
|
||||
private readonly titleHostname: string;
|
||||
private readonly windowTitle: string;
|
||||
private readonly indexHtmlTemplate: string;
|
||||
private readonly allowUnauthenticatedNetwork: boolean;
|
||||
private _pasteImageGcStop: (() => void) | null = null;
|
||||
private _eventLoopMonitor: EventLoopMonitorHandle | null = null;
|
||||
private teamWatcherHandlers: {
|
||||
@@ -240,15 +266,22 @@ export class WebServer extends EventEmitter {
|
||||
teamRemoved: (config: unknown) => void;
|
||||
taskUpdated: (data: unknown) => void;
|
||||
} | null = null;
|
||||
private readonly titleHostname: string;
|
||||
private readonly windowTitle: string;
|
||||
private readonly indexHtmlTemplate: string;
|
||||
constructor(port: number = 3000, https: boolean = false, testMode: boolean = false, titleHostname?: string) {
|
||||
constructor(
|
||||
port: number = 3000,
|
||||
https: boolean = false,
|
||||
testMode: boolean = false,
|
||||
host: string = '127.0.0.1',
|
||||
titleHostname?: string,
|
||||
allowUnauthenticatedNetwork: boolean = false
|
||||
) {
|
||||
super();
|
||||
this.setMaxListeners(0);
|
||||
this.host = host;
|
||||
this.port = port;
|
||||
this.https = https;
|
||||
this.testMode = testMode;
|
||||
this.allowUnauthenticatedNetwork =
|
||||
allowUnauthenticatedNetwork || isExplicitlyEnabled(process.env.CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK);
|
||||
this.titleHostname = titleHostname || getHostname();
|
||||
this.windowTitle = `codeman:${this.titleHostname}`;
|
||||
this.indexHtmlTemplate = readFileSync(join(__dirname, 'public', 'index.html'), 'utf-8');
|
||||
@@ -1646,6 +1679,13 @@ export class WebServer extends EventEmitter {
|
||||
}
|
||||
|
||||
async start(): Promise<void> {
|
||||
if (!isLoopbackBindHost(this.host) && !process.env.CODEMAN_PASSWORD && !this.allowUnauthenticatedNetwork) {
|
||||
throw new Error(
|
||||
'Refusing to start Codeman on a non-loopback host without CODEMAN_PASSWORD. ' +
|
||||
'Set CODEMAN_PASSWORD or explicitly allow unauthenticated network access.'
|
||||
);
|
||||
}
|
||||
|
||||
await this.setupRoutes();
|
||||
|
||||
const lifecycleLog = getLifecycleLog();
|
||||
@@ -1672,19 +1712,23 @@ export class WebServer extends EventEmitter {
|
||||
this._eventLoopMonitor = startEventLoopMonitor();
|
||||
}
|
||||
|
||||
await this.app.listen({ port: this.port, host: '0.0.0.0' });
|
||||
await this.app.listen({ port: this.port, host: this.host });
|
||||
const protocol = this.https ? 'https' : 'http';
|
||||
console.log(`Codeman web interface running at ${protocol}://localhost:${this.port}`);
|
||||
const displayHost = this.host === '0.0.0.0' ? 'localhost' : this.host;
|
||||
console.log(`Codeman web interface running at ${protocol}://${displayHost}:${this.port}`);
|
||||
|
||||
// Security warning: server binds to 0.0.0.0 (all interfaces) — warn if no auth configured
|
||||
if (!process.env.CODEMAN_PASSWORD) {
|
||||
if (!isLoopbackBindHost(this.host) && !process.env.CODEMAN_PASSWORD && this.allowUnauthenticatedNetwork) {
|
||||
console.warn('\n⚠ WARNING: No CODEMAN_PASSWORD set — server is accessible without authentication.');
|
||||
console.warn(' Anyone on your network can access and control Claude sessions.');
|
||||
console.warn(' Set CODEMAN_PASSWORD environment variable to enable auth.\n');
|
||||
console.warn(
|
||||
' This was explicitly allowed by --allow-unauthenticated-network or CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK.\n'
|
||||
);
|
||||
}
|
||||
|
||||
// Set API URL for child processes (MCP server, spawned sessions)
|
||||
process.env.CODEMAN_API_URL = `${protocol}://localhost:${this.port}`;
|
||||
const apiHost =
|
||||
this.host === '0.0.0.0' || this.host === 'localhost' || this.host === '::1' ? '127.0.0.1' : this.host;
|
||||
process.env.CODEMAN_API_URL = `${protocol}://${apiHost}:${this.port}`;
|
||||
|
||||
// Start scheduled runs cleanup timer
|
||||
this.cleanup.setInterval(
|
||||
@@ -2176,9 +2220,11 @@ export async function startWebServer(
|
||||
port: number = 3000,
|
||||
https: boolean = false,
|
||||
testMode: boolean = false,
|
||||
titleHostname?: string
|
||||
host: string = '127.0.0.1',
|
||||
titleHostname?: string,
|
||||
allowUnauthenticatedNetwork: boolean = false
|
||||
): Promise<WebServer> {
|
||||
const server = new WebServer(port, https, testMode, titleHostname);
|
||||
const server = new WebServer(port, https, testMode, host, titleHostname, allowUnauthenticatedNetwork);
|
||||
await server.start();
|
||||
return server;
|
||||
}
|
||||
|
||||
+47
-17
@@ -3,12 +3,12 @@
|
||||
* 1. Timing-safe password comparison (timingSafeEqual)
|
||||
* 2. Hook event endpoint restricted to localhost
|
||||
* 3. Session cookie TTL refresh on access
|
||||
* 4. Startup warning when no password configured
|
||||
* 4. Startup fails closed when network-bound without auth
|
||||
* 5. SSE client limit enforcement
|
||||
* 6. Logout endpoint invalidates session
|
||||
* 7. Settings schema rejects unknown fields
|
||||
*
|
||||
* Port: 3160 (auth tests), 3161 (no-auth tests)
|
||||
* Port: 3160 (auth tests), 3161 (loopback no-auth tests), 3162 (network override tests)
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { WebServer } from '../src/web/server.js';
|
||||
@@ -16,6 +16,7 @@ import { SettingsUpdateSchema } from '../src/web/schemas.js';
|
||||
|
||||
const AUTH_PORT = 3160;
|
||||
const NOAUTH_PORT = 3161;
|
||||
const NETWORK_OVERRIDE_PORT = 3162;
|
||||
const TEST_USER = 'admin';
|
||||
const TEST_PASS = 'test-password-12345';
|
||||
|
||||
@@ -270,34 +271,63 @@ describe('Settings Schema Security', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('No-Auth Server Warning', () => {
|
||||
describe('No-Auth Server Startup Policy', () => {
|
||||
let server: WebServer;
|
||||
let consoleWarnSpy: string[] = [];
|
||||
const originalWarn = console.warn;
|
||||
|
||||
beforeAll(async () => {
|
||||
delete process.env.CODEMAN_PASSWORD;
|
||||
delete process.env.CODEMAN_USERNAME;
|
||||
consoleWarnSpy = [];
|
||||
console.warn = (...args: unknown[]) => {
|
||||
consoleWarnSpy.push(args.map(String).join(' '));
|
||||
};
|
||||
server = new WebServer(NOAUTH_PORT, false, true);
|
||||
delete process.env.CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK;
|
||||
server = new WebServer(NOAUTH_PORT, false, true, '127.0.0.1');
|
||||
await server.start();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
console.warn = originalWarn;
|
||||
delete process.env.CODEMAN_PASSWORD;
|
||||
delete process.env.CODEMAN_USERNAME;
|
||||
delete process.env.CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK;
|
||||
await server.stop();
|
||||
});
|
||||
|
||||
it('should warn when no CODEMAN_PASSWORD is set', () => {
|
||||
const hasWarning = consoleWarnSpy.some(msg => msg.includes('No CODEMAN_PASSWORD set'));
|
||||
expect(hasWarning).toBe(true);
|
||||
});
|
||||
|
||||
it('should allow requests without auth when no password configured', async () => {
|
||||
it('allows loopback requests without auth when no password is configured', async () => {
|
||||
const res = await fetch(`http://localhost:${NOAUTH_PORT}/api/status`);
|
||||
expect(res.status).toBe(200);
|
||||
});
|
||||
|
||||
it('rejects non-loopback startup without a password or explicit override', async () => {
|
||||
const networkServer = new WebServer(0, false, true, '0.0.0.0');
|
||||
|
||||
await expect(networkServer.start()).rejects.toThrow(/CODEMAN_PASSWORD/);
|
||||
|
||||
await networkServer.stop();
|
||||
});
|
||||
|
||||
it('allows non-loopback startup when CODEMAN_PASSWORD is configured', async () => {
|
||||
process.env.CODEMAN_PASSWORD = TEST_PASS;
|
||||
const networkServer = new WebServer(0, false, true, '0.0.0.0');
|
||||
|
||||
await networkServer.start();
|
||||
await networkServer.stop();
|
||||
|
||||
delete process.env.CODEMAN_PASSWORD;
|
||||
});
|
||||
|
||||
it('allows non-loopback startup with the explicit unauthenticated-network override', async () => {
|
||||
const networkServer = new WebServer(NETWORK_OVERRIDE_PORT, false, true, '0.0.0.0', undefined, true);
|
||||
|
||||
await networkServer.start();
|
||||
const res = await fetch(`http://localhost:${NETWORK_OVERRIDE_PORT}/api/status`);
|
||||
expect(res.status).toBe(200);
|
||||
await networkServer.stop();
|
||||
});
|
||||
|
||||
it('allows non-loopback startup with the explicit unauthenticated-network env override', async () => {
|
||||
process.env.CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK = 'true';
|
||||
const networkServer = new WebServer(0, false, true, '0.0.0.0');
|
||||
|
||||
await networkServer.start();
|
||||
await networkServer.stop();
|
||||
|
||||
delete process.env.CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK;
|
||||
});
|
||||
});
|
||||
|
||||
+32
-14
@@ -5,6 +5,7 @@
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { program } from '../src/cli.js';
|
||||
|
||||
describe('CLI Command Parsing', () => {
|
||||
describe('Command Structure', () => {
|
||||
@@ -72,11 +73,11 @@ describe('CLI Command Parsing', () => {
|
||||
];
|
||||
|
||||
const findCommand = (name: string): Command | undefined => {
|
||||
return commands.find(c => c.name === name || c.aliases.includes(name));
|
||||
return commands.find((c) => c.name === name || c.aliases.includes(name));
|
||||
};
|
||||
|
||||
const findSubcommand = (parent: Command, name: string): Command | undefined => {
|
||||
return parent.subcommands?.find(c => c.name === name || c.aliases.includes(name));
|
||||
return parent.subcommands?.find((c) => c.name === name || c.aliases.includes(name));
|
||||
};
|
||||
|
||||
it('should find commands by name', () => {
|
||||
@@ -103,7 +104,7 @@ describe('CLI Command Parsing', () => {
|
||||
});
|
||||
|
||||
it('should have descriptions for all commands', () => {
|
||||
commands.forEach(cmd => {
|
||||
commands.forEach((cmd) => {
|
||||
expect(cmd.description).toBeTruthy();
|
||||
});
|
||||
});
|
||||
@@ -267,13 +268,13 @@ describe('CLI Command Parsing', () => {
|
||||
});
|
||||
|
||||
it('should have defaults for web flags', () => {
|
||||
webFlags.forEach(flag => {
|
||||
webFlags.forEach((flag) => {
|
||||
expect(flag.default).toBeDefined();
|
||||
});
|
||||
});
|
||||
|
||||
it('should have defaults for tui flags', () => {
|
||||
tuiFlags.forEach(flag => {
|
||||
tuiFlags.forEach((flag) => {
|
||||
expect(flag.default).toBeDefined();
|
||||
});
|
||||
});
|
||||
@@ -322,7 +323,7 @@ describe('CLI Command Parsing', () => {
|
||||
help += `${description}\n`;
|
||||
if (options.length > 0) {
|
||||
help += '\nOptions:\n';
|
||||
options.forEach(opt => {
|
||||
options.forEach((opt) => {
|
||||
help += ` ${opt}\n`;
|
||||
});
|
||||
}
|
||||
@@ -345,6 +346,15 @@ describe('CLI Command Parsing', () => {
|
||||
expect(help).toContain('--host');
|
||||
});
|
||||
|
||||
it('documents the unauthenticated network override in real web command help', () => {
|
||||
const webCommand = program.commands.find((command) => command.name() === 'web');
|
||||
expect(webCommand).toBeDefined();
|
||||
|
||||
const help = webCommand!.helpInformation();
|
||||
expect(help).toContain('--allow-unauthenticated-network');
|
||||
expect(help).toMatch(/without\s+CODEMAN_PASSWORD/);
|
||||
});
|
||||
|
||||
it('should format properly', () => {
|
||||
const help = generateHelp('test', 'Test command', ['--flag']);
|
||||
const lines = help.split('\n');
|
||||
@@ -474,22 +484,27 @@ describe('CLI Output Formatting', () => {
|
||||
}
|
||||
|
||||
const formatRow = (values: string[], columns: Column[]): string => {
|
||||
return values.map((val, i) => {
|
||||
return values
|
||||
.map((val, i) => {
|
||||
const width = columns[i]?.width || 10;
|
||||
return val.padEnd(width).substring(0, width);
|
||||
}).join(' ');
|
||||
})
|
||||
.join(' ');
|
||||
};
|
||||
|
||||
const formatTable = (headers: string[], rows: string[][], widths: number[]): string => {
|
||||
const columns = headers.map((h, i) => ({ header: h, width: widths[i] }));
|
||||
const headerRow = formatRow(headers, columns);
|
||||
const separator = columns.map(c => '-'.repeat(c.width)).join(' ');
|
||||
const dataRows = rows.map(row => formatRow(row, columns));
|
||||
const separator = columns.map((c) => '-'.repeat(c.width)).join(' ');
|
||||
const dataRows = rows.map((row) => formatRow(row, columns));
|
||||
return [headerRow, separator, ...dataRows].join('\n');
|
||||
};
|
||||
|
||||
it('should format single row', () => {
|
||||
const columns = [{ header: 'ID', width: 10 }, { header: 'Status', width: 8 }];
|
||||
const columns = [
|
||||
{ header: 'ID', width: 10 },
|
||||
{ header: 'Status', width: 8 },
|
||||
];
|
||||
const row = formatRow(['123', 'active'], columns);
|
||||
expect(row).toBe('123 active ');
|
||||
});
|
||||
@@ -503,7 +518,10 @@ describe('CLI Output Formatting', () => {
|
||||
it('should format complete table', () => {
|
||||
const table = formatTable(
|
||||
['ID', 'Status'],
|
||||
[['1', 'active'], ['2', 'idle']],
|
||||
[
|
||||
['1', 'active'],
|
||||
['2', 'idle'],
|
||||
],
|
||||
[5, 8]
|
||||
);
|
||||
expect(table).toContain('ID');
|
||||
@@ -587,7 +605,7 @@ describe('CLI Output Formatting', () => {
|
||||
});
|
||||
|
||||
it('should format normal costs with 2 decimals', () => {
|
||||
expect(formatCost(1.50)).toBe('$1.50');
|
||||
expect(formatCost(1.5)).toBe('$1.50');
|
||||
expect(formatCost(0.05)).toBe('$0.05');
|
||||
});
|
||||
|
||||
@@ -633,7 +651,7 @@ describe('CLI Output Formatting', () => {
|
||||
|
||||
describe('List Formatting', () => {
|
||||
const formatList = (items: string[], bullet: string = '-'): string => {
|
||||
return items.map(item => `${bullet} ${item}`).join('\n');
|
||||
return items.map((item) => `${bullet} ${item}`).join('\n');
|
||||
};
|
||||
|
||||
const formatNumberedList = (items: string[]): string => {
|
||||
|
||||
Reference in New Issue
Block a user