From ac08f3b3d0c6b44861d49f5f5b8af7fc7129eea9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kyle=20=F0=9F=90=86?= Date: Sun, 8 Feb 2026 16:30:14 -0500 Subject: [PATCH 1/8] fix: address security vulnerabilities --- dashboard/src/api.ts | 10 ++-- package-lock.json | 19 +++++--- proxy/package-lock.json | 13 ++++-- proxy/src/routes/proxy.ts | 8 ++-- src/bots/templates.ts | 28 ++++++------ src/secrets/manager.ts | 11 ++++- src/server.ts | 96 +++++++++++++++++++++++++++++++-------- 7 files changed, 133 insertions(+), 52 deletions(-) diff --git a/dashboard/src/api.ts b/dashboard/src/api.ts index 1694333..41e8e9e 100644 --- a/dashboard/src/api.ts +++ b/dashboard/src/api.ts @@ -180,11 +180,15 @@ export async function fetchProxyHealth(): Promise { } export async function fetchDynamicModels(baseUrl: string, apiKey?: string): Promise { - let url = `${API_BASE}/models/discover?baseUrl=${encodeURIComponent(baseUrl)}`; + const body: { baseUrl: string; apiKey?: string } = { baseUrl }; if (apiKey) { - url += `&apiKey=${encodeURIComponent(apiKey)}`; + body.apiKey = apiKey; } - const response = await fetch(url, { headers: getAuthHeaders() }); + const response = await fetch(`${API_BASE}/models/discover`, { + method: 'POST', + headers: { 'Content-Type': 'application/json', ...getAuthHeaders() }, + body: JSON.stringify(body), + }); const data = await handleResponse<{ models: string[] }>(response); return data.models; } diff --git a/package-lock.json b/package-lock.json index adf45c3..f4efd5a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1067,9 +1067,9 @@ } }, "node_modules/@isaacs/brace-expansion": { - "version": "5.0.0", - "resolved": "https://registry.npmjs.org/@isaacs/brace-expansion/-/brace-expansion-5.0.0.tgz", - "integrity": "sha512-ZT55BDLV0yv0RBm2czMiZ+SqCGO7AvmOM3G/w2xhVPH+te0aKgFjmBvGlL1dH+ql2tgGO3MVrbb3jCKyvpgnxA==", + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/@isaacs/brace-expansion/-/brace-expansion-5.0.1.tgz", + "integrity": "sha512-WMz71T1JS624nWj2n2fnYAuPovhv7EUhk69R6i9dsVyzxt5eM3bjwvgk9L+APE1TRscGysAVMANkB0jh0LQZrQ==", "license": "MIT", "dependencies": { "@isaacs/balanced-match": "^4.0.1" @@ -1729,6 +1729,7 @@ "integrity": "sha512-BtE0k6cjwjLZoZixN0t5AKP0kSzlGu7FctRXYuPAm//aaiZhmfq1JwdYpYr1brzEspYyFeF+8XF5j2VK6oalrA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.54.0", "@typescript-eslint/types": "8.54.0", @@ -2098,6 +2099,7 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -2821,6 +2823,7 @@ "integrity": "sha512-LEyamqS7W5HB3ujJyvi0HQK/dtVINZvd5mAAp9eT5S/ujByGjiZLCzPcHVzuXbpJDJF/cxwHlfceVUDZ2lnSTw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -3096,9 +3099,9 @@ "license": "BSD-3-Clause" }, "node_modules/fastify": { - "version": "5.7.2", - "resolved": "https://registry.npmjs.org/fastify/-/fastify-5.7.2.tgz", - "integrity": "sha512-dBJolW+hm6N/yJVf6J5E1BxOBNkuXNl405nrfeR8SpvGWG3aCC2XDHyiFBdow8Win1kj7sjawQc257JlYY6M/A==", + "version": "5.7.4", + "resolved": "https://registry.npmjs.org/fastify/-/fastify-5.7.4.tgz", + "integrity": "sha512-e6l5NsRdaEP8rdD8VR0ErJASeyaRbzXYpmkrpr2SuvuMq6Si3lvsaVy5C+7gLanEkvjpMDzBXWE5HPeb/hgTxA==", "funding": [ { "type": "github", @@ -4075,6 +4078,7 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -5080,6 +5084,7 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -5153,6 +5158,7 @@ "integrity": "sha512-o5a9xKjbtuhY6Bi5S3+HvbRERmouabWbyUcpXXUA1u+GNUKoROi9byOJ8M0nHbHYHkYICiMlqxkg1KkYmm25Sw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.21.3", "postcss": "^8.4.43", @@ -5666,6 +5672,7 @@ "integrity": "sha512-MSmPM9REYqDGBI8439mA4mWhV5sKmDlBKWIYbA3lRb2PTHACE0mgKwA8yQ2xq9vxDTuk4iPrECBAEW2aoFXY0Q==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@vitest/expect": "2.1.9", "@vitest/mocker": "2.1.9", diff --git a/proxy/package-lock.json b/proxy/package-lock.json index f319f92..111b337 100644 --- a/proxy/package-lock.json +++ b/proxy/package-lock.json @@ -1490,6 +1490,7 @@ "integrity": "sha512-BtE0k6cjwjLZoZixN0t5AKP0kSzlGu7FctRXYuPAm//aaiZhmfq1JwdYpYr1brzEspYyFeF+8XF5j2VK6oalrA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.54.0", "@typescript-eslint/types": "8.54.0", @@ -1833,6 +1834,7 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -2743,6 +2745,7 @@ "integrity": "sha512-LEyamqS7W5HB3ujJyvi0HQK/dtVINZvd5mAAp9eT5S/ujByGjiZLCzPcHVzuXbpJDJF/cxwHlfceVUDZ2lnSTw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -3042,9 +3045,9 @@ "license": "BSD-3-Clause" }, "node_modules/fastify": { - "version": "5.7.2", - "resolved": "https://registry.npmjs.org/fastify/-/fastify-5.7.2.tgz", - "integrity": "sha512-dBJolW+hm6N/yJVf6J5E1BxOBNkuXNl405nrfeR8SpvGWG3aCC2XDHyiFBdow8Win1kj7sjawQc257JlYY6M/A==", + "version": "5.7.4", + "resolved": "https://registry.npmjs.org/fastify/-/fastify-5.7.4.tgz", + "integrity": "sha512-e6l5NsRdaEP8rdD8VR0ErJASeyaRbzXYpmkrpr2SuvuMq6Si3lvsaVy5C+7gLanEkvjpMDzBXWE5HPeb/hgTxA==", "funding": [ { "type": "github", @@ -3954,6 +3957,7 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -4791,6 +4795,7 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -4865,6 +4870,7 @@ "integrity": "sha512-o5a9xKjbtuhY6Bi5S3+HvbRERmouabWbyUcpXXUA1u+GNUKoROi9byOJ8M0nHbHYHkYICiMlqxkg1KkYmm25Sw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.21.3", "postcss": "^8.4.43", @@ -5004,6 +5010,7 @@ "integrity": "sha512-MSmPM9REYqDGBI8439mA4mWhV5sKmDlBKWIYbA3lRb2PTHACE0mgKwA8yQ2xq9vxDTuk4iPrECBAEW2aoFXY0Q==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@vitest/expect": "2.1.9", "@vitest/mocker": "2.1.9", diff --git a/proxy/src/routes/proxy.ts b/proxy/src/routes/proxy.ts index 15fd985..bd4d93c 100644 --- a/proxy/src/routes/proxy.ts +++ b/proxy/src/routes/proxy.ts @@ -73,12 +73,14 @@ export function registerProxyRoutes( keyId = keySelection.keyId; } + const FORWARDED_HEADERS = ['content-type', 'accept', 'user-agent']; const headers: Record = {}; - for (const [key, value] of Object.entries(req.headers)) { + for (const name of FORWARDED_HEADERS) { + const value = req.headers[name]; if (typeof value === 'string') { - headers[key] = value; + headers[name] = value; } else if (Array.isArray(value)) { - headers[key] = value[0]; + headers[name] = value[0]; } } diff --git a/src/bots/templates.ts b/src/bots/templates.ts index 921eec3..da0dc16 100644 --- a/src/bots/templates.ts +++ b/src/bots/templates.ts @@ -195,18 +195,16 @@ export function createBotWorkspace(dataDir: string, config: BotWorkspaceConfig): const botDir = join(dataDir, 'bots', config.botHostname); const workspaceDir = join(botDir, 'workspace'); - // Create directories with permissions for bot container (runs as uid 1000) - mkdirSync(botDir, { recursive: true, mode: 0o777 }); - mkdirSync(workspaceDir, { recursive: true, mode: 0o777 }); - // Ensure parent dir has correct permissions (recursive: true doesn't set mode on existing dirs) - chmodSync(botDir, 0o777); - chmodSync(workspaceDir, 0o777); + mkdirSync(botDir, { recursive: true, mode: 0o755 }); + mkdirSync(workspaceDir, { recursive: true, mode: 0o755 }); + chmodSync(botDir, 0o755); + chmodSync(workspaceDir, 0o755); // Write openclaw.json at root of bot directory (OPENCLAW_STATE_DIR) const openclawConfig = generateOpenclawConfig(config); const configPath = join(botDir, 'openclaw.json'); writeFileSync(configPath, JSON.stringify(openclawConfig, null, 2)); - chmodSync(configPath, 0o666); + chmodSync(configPath, 0o644); // Write only persona files — OpenClaw's ensureAgentWorkspace() will create // AGENTS.md, BOOTSTRAP.md, TOOLS.md, HEARTBEAT.md from its own templates @@ -215,29 +213,29 @@ export function createBotWorkspace(dataDir: string, config: BotWorkspaceConfig): const identityPath = join(workspaceDir, 'IDENTITY.md'); writeFileSync(soulPath, generateSoulMd(config.persona)); writeFileSync(identityPath, generateIdentityMd(config.persona)); - chmodSync(soulPath, 0o666); - chmodSync(identityPath, 0o666); + chmodSync(soulPath, 0o644); + chmodSync(identityPath, 0o644); // OpenClaw runs as uid 1000 (node user), so we need to set ownership const OPENCLAW_UID = 1000; const OPENCLAW_GID = 1000; const agentDir = join(botDir, 'agents', 'main', 'agent'); - mkdirSync(agentDir, { recursive: true, mode: 0o777 }); - chmodSync(agentDir, 0o777); + mkdirSync(agentDir, { recursive: true, mode: 0o755 }); + chmodSync(agentDir, 0o755); tryChown(agentDir, OPENCLAW_UID, OPENCLAW_GID); // Pre-create sessions directory for OpenClaw runtime use const sessionsDir = join(botDir, 'agents', 'main', 'sessions'); - mkdirSync(sessionsDir, { recursive: true, mode: 0o777 }); - chmodSync(sessionsDir, 0o777); + mkdirSync(sessionsDir, { recursive: true, mode: 0o755 }); + chmodSync(sessionsDir, 0o755); tryChown(sessionsDir, OPENCLAW_UID, OPENCLAW_GID); // Pre-create sandbox directory for OpenClaw code execution // OpenClaw hardcodes /app/workspace for sandbox operations const sandboxDir = join(botDir, 'sandbox'); - mkdirSync(sandboxDir, { recursive: true, mode: 0o777 }); - chmodSync(sandboxDir, 0o777); + mkdirSync(sandboxDir, { recursive: true, mode: 0o755 }); + chmodSync(sandboxDir, 0o755); tryChown(sandboxDir, OPENCLAW_UID, OPENCLAW_GID); } diff --git a/src/secrets/manager.ts b/src/secrets/manager.ts index bee5fc9..4f9caa1 100644 --- a/src/secrets/manager.ts +++ b/src/secrets/manager.ts @@ -8,8 +8,8 @@ import { mkdirSync, writeFileSync, readFileSync, rmSync } from 'node:fs'; import { join } from 'node:path'; -/** Hostname regex for validation - prevents directory traversal attacks */ const HOSTNAME_REGEX = /^[a-z0-9-]{1,64}$/; +const SECRET_NAME_REGEX = /^[A-Z0-9_]{1,64}$/; /** * Returns the root directory for secrets storage. @@ -64,7 +64,10 @@ export function createBotSecretsDir(hostname: string): string { export function writeSecret(hostname: string, name: string, value: string): void { validateHostname(hostname); - // Ensure bot directory exists + if (!SECRET_NAME_REGEX.test(name)) { + throw new Error(`Invalid secret name: ${name}`); + } + const botDir = createBotSecretsDir(hostname); const filePath = join(botDir, name); @@ -82,6 +85,10 @@ export function writeSecret(hostname: string, name: string, value: string): void export function readSecret(hostname: string, name: string): string | undefined { validateHostname(hostname); + if (!SECRET_NAME_REGEX.test(name)) { + throw new Error(`Invalid secret name: ${name}`); + } + const secretsRoot = getSecretsRoot(); const filePath = join(secretsRoot, hostname, name); diff --git a/src/server.ts b/src/server.ts index 844cafc..55f4249 100644 --- a/src/server.ts +++ b/src/server.ts @@ -114,6 +114,49 @@ function toDockerHostUrl(url: string): string { return url.replace(/\blocalhost\b|127\.0\.0\.1/g, 'host.docker.internal'); } +function isPrivateUrl(urlStr: string): boolean { + let parsed: URL; + try { + parsed = new URL(urlStr); + } catch { + return true; + } + + if (!['http:', 'https:'].includes(parsed.protocol)) { + return true; + } + + const hostname = parsed.hostname.toLowerCase(); + + if ( + hostname === 'localhost' || + hostname === '127.0.0.1' || + hostname === '::1' || + hostname === '[::1]' || + hostname === '0.0.0.0' || + hostname.endsWith('.local') || + hostname.endsWith('.internal') + ) { + return true; + } + + const ipv4Match = hostname.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/); + if (ipv4Match) { + const [, a, b, c, d] = ipv4Match.map(Number); + if (a === 10) return true; + if (a === 172 && b >= 16 && b <= 31) return true; + if (a === 192 && b === 168) return true; + if (a === 169 && b === 254) return true; + if (a === 127) return true; + if (a === 0) return true; + if (a === 100 && b >= 64 && b <= 127) return true; + if (a === 198 && b >= 18 && b <= 19) return true; + if (a === 240 || (a === 255 && b === 255 && c === 255 && d === 255)) return true; + } + + return false; +} + async function resolveHostPaths(config: ReturnType): Promise<{ hostDataDir: string; hostSecretsDir: string; @@ -146,7 +189,18 @@ export async function buildServer(): Promise { // Register security headers await server.register(fastifyHelmet, { - contentSecurityPolicy: false, + contentSecurityPolicy: { + directives: { + defaultSrc: ["'self'"], + scriptSrc: ["'self'"], + styleSrc: ["'self'", "'unsafe-inline'"], + imgSrc: ["'self'", "data:"], + connectSrc: ["'self'"], + fontSrc: ["'self'"], + objectSrc: ["'none'"], + frameAncestors: ["'none'"], + }, + }, }); // Register rate limiting @@ -265,6 +319,11 @@ export async function buildServer(): Promise { return { error: 'Missing required field: name' }; } + if (!/^[a-zA-Z0-9 _.\-]{1,128}$/.test(body.name)) { + reply.code(400); + return { error: 'Bot name must be 1-128 characters: letters, numbers, spaces, underscores, dots, hyphens' }; + } + if (!body.hostname) { reply.code(400); return { error: 'Missing required field: hostname' }; @@ -377,15 +436,6 @@ export async function buildServer(): Promise { `PORT=${port}`, ]; - // Add channel tokens - for (const channel of body.channels) { - if (channel.channelType === 'telegram') { - environment.push(`TELEGRAM_BOT_TOKEN=${channel.token}`); - } else if (channel.channelType === 'discord') { - environment.push(`DISCORD_TOKEN=${channel.token}`); - } - } - const containerId = await docker.createContainer(bot.hostname, bot.id, { image: config.openclawImage, environment, @@ -611,26 +661,33 @@ export async function buildServer(): Promise { } }); - // Dynamic model discovery for any OpenAI-compatible provider (e.g., Ollama) - // Fetches from the provider's /v1/models endpoint. - server.get<{ Querystring: { baseUrl?: string; apiKey?: string } }>('/api/models/discover', async (request, reply) => { - const baseUrl = request.query.baseUrl; + server.post<{ Body: { baseUrl?: string; apiKey?: string } }>('/api/models/discover', async (request, reply) => { + const baseUrl = request.body.baseUrl; if (!baseUrl) { reply.code(400); - return { error: 'Missing baseUrl query parameter' }; + return { error: 'Missing baseUrl in request body' }; + } + + if (isPrivateUrl(baseUrl)) { + reply.code(400); + return { error: 'Requests to private/internal addresses are not allowed' }; } try { - // Translate localhost → host.docker.internal for fetches from inside Docker const fetchBase = toDockerHostUrl(baseUrl); - // Append /models to the base URL, preserving path (e.g. /v1 → /v1/models) + + if (isPrivateUrl(fetchBase)) { + reply.code(400); + return { error: 'Requests to private/internal addresses are not allowed' }; + } + const url = fetchBase.replace(/\/+$/, '') + '/models'; const controller = new AbortController(); const timeout = setTimeout(() => { controller.abort(); }, 5000); const headers: Record = {}; - if (request.query.apiKey) { - headers.Authorization = `Bearer ${request.query.apiKey}`; + if (request.body.apiKey) { + headers.Authorization = `Bearer ${request.body.apiKey}`; } const response = await fetch(url, { signal: controller.signal, headers }); @@ -644,7 +701,6 @@ export async function buildServer(): Promise { const models = (data.data ?? []).map((m: { id: string }) => m.id); return { models }; } catch { - // Connection refused, timeout, etc. — graceful fallback return { models: [] }; } }); From 8a2004e8dc362a79e475e5d362d27960caac82df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kyle=20=F0=9F=90=86?= Date: Sun, 8 Feb 2026 16:40:06 -0500 Subject: [PATCH 2/8] fix: harden SSRF protection, input validation, and resource management --- src/bots/templates.ts | 22 +++--- src/server.ts | 163 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 146 insertions(+), 39 deletions(-) diff --git a/src/bots/templates.ts b/src/bots/templates.ts index da0dc16..fd6ad6b 100644 --- a/src/bots/templates.ts +++ b/src/bots/templates.ts @@ -6,6 +6,7 @@ import { mkdirSync, writeFileSync, chmodSync, chownSync, rmSync } from 'node:fs'; import { join } from 'node:path'; +import { validateHostname } from '../secrets/manager.js'; /** * Try to change file ownership, but gracefully skip if not permitted. @@ -195,44 +196,42 @@ export function createBotWorkspace(dataDir: string, config: BotWorkspaceConfig): const botDir = join(dataDir, 'bots', config.botHostname); const workspaceDir = join(botDir, 'workspace'); + // OpenClaw runs as uid 1000 (node user), so we need to set ownership + const OPENCLAW_UID = 1000; + const OPENCLAW_GID = 1000; + mkdirSync(botDir, { recursive: true, mode: 0o755 }); mkdirSync(workspaceDir, { recursive: true, mode: 0o755 }); chmodSync(botDir, 0o755); chmodSync(workspaceDir, 0o755); + tryChown(botDir, OPENCLAW_UID, OPENCLAW_GID); + tryChown(workspaceDir, OPENCLAW_UID, OPENCLAW_GID); - // Write openclaw.json at root of bot directory (OPENCLAW_STATE_DIR) const openclawConfig = generateOpenclawConfig(config); const configPath = join(botDir, 'openclaw.json'); writeFileSync(configPath, JSON.stringify(openclawConfig, null, 2)); chmodSync(configPath, 0o644); + tryChown(configPath, OPENCLAW_UID, OPENCLAW_GID); - // Write only persona files — OpenClaw's ensureAgentWorkspace() will create - // AGENTS.md, BOOTSTRAP.md, TOOLS.md, HEARTBEAT.md from its own templates - // (using writeFileIfMissing / wx flag, so our files won't be overwritten). const soulPath = join(workspaceDir, 'SOUL.md'); const identityPath = join(workspaceDir, 'IDENTITY.md'); writeFileSync(soulPath, generateSoulMd(config.persona)); writeFileSync(identityPath, generateIdentityMd(config.persona)); chmodSync(soulPath, 0o644); chmodSync(identityPath, 0o644); - - // OpenClaw runs as uid 1000 (node user), so we need to set ownership - const OPENCLAW_UID = 1000; - const OPENCLAW_GID = 1000; + tryChown(soulPath, OPENCLAW_UID, OPENCLAW_GID); + tryChown(identityPath, OPENCLAW_UID, OPENCLAW_GID); const agentDir = join(botDir, 'agents', 'main', 'agent'); mkdirSync(agentDir, { recursive: true, mode: 0o755 }); chmodSync(agentDir, 0o755); tryChown(agentDir, OPENCLAW_UID, OPENCLAW_GID); - // Pre-create sessions directory for OpenClaw runtime use const sessionsDir = join(botDir, 'agents', 'main', 'sessions'); mkdirSync(sessionsDir, { recursive: true, mode: 0o755 }); chmodSync(sessionsDir, 0o755); tryChown(sessionsDir, OPENCLAW_UID, OPENCLAW_GID); - // Pre-create sandbox directory for OpenClaw code execution - // OpenClaw hardcodes /app/workspace for sandbox operations const sandboxDir = join(botDir, 'sandbox'); mkdirSync(sandboxDir, { recursive: true, mode: 0o755 }); chmodSync(sandboxDir, 0o755); @@ -258,6 +257,7 @@ export function getBotWorkspacePath(dataDir: string, hostname: string): string { * @param hostname - Bot hostname */ export function deleteBotWorkspace(dataDir: string, hostname: string): void { + validateHostname(hostname); const botDir = join(dataDir, 'bots', hostname); rmSync(botDir, { recursive: true, force: true }); } diff --git a/src/server.ts b/src/server.ts index 55f4249..05b8fd5 100644 --- a/src/server.ts +++ b/src/server.ts @@ -5,6 +5,7 @@ import fastifyHelmet from '@fastify/helmet'; import { join, resolve } from 'node:path'; import { existsSync } from 'node:fs'; import { randomBytes, timingSafeEqual } from 'node:crypto'; +import { promises as dns } from 'node:dns'; import { getConfig } from './config.js'; import { initDb } from './db/index.js'; @@ -114,6 +115,49 @@ function toDockerHostUrl(url: string): string { return url.replace(/\blocalhost\b|127\.0\.0\.1/g, 'host.docker.internal'); } +const VALID_PROVIDER_IDS = new Set([ + 'openai', 'anthropic', 'google', 'venice', 'openrouter', 'ollama', 'grok', + 'deepseek', 'mistral', 'groq', 'cerebras', 'fireworks', 'togetherai', + 'deepinfra', 'perplexity', 'nvidia', 'minimax', 'moonshot', 'scaleway', + 'nebius', 'ovhcloud', 'huggingface', +]); + +const VALID_CHANNEL_TYPES = new Set(['telegram', 'discord']); + +const MODEL_REGEX = /^[a-zA-Z0-9._:/-]{1,128}$/; + +function isPrivateIp(ip: string): boolean { + // Handle IPv6-mapped IPv4 (e.g., ::ffff:127.0.0.1) + const v4Mapped = ip.match(/^::ffff:(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/i); + const normalizedIp = v4Mapped ? v4Mapped[1] : ip; + + // IPv6 loopback + if (normalizedIp === '::1') return true; + + // IPv6 link-local + if (normalizedIp.toLowerCase().startsWith('fe80:')) return true; + + // IPv6 unique local (fc00::/7) + const firstChar = normalizedIp.toLowerCase(); + if (firstChar.startsWith('fc') || firstChar.startsWith('fd')) return true; + + const ipv4Match = normalizedIp.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/); + if (ipv4Match) { + const [, a, b, c, d] = ipv4Match.map(Number); + if (a === 10) return true; + if (a === 172 && b >= 16 && b <= 31) return true; + if (a === 192 && b === 168) return true; + if (a === 169 && b === 254) return true; + if (a === 127) return true; + if (a === 0) return true; + if (a === 100 && b >= 64 && b <= 127) return true; + if (a === 198 && b >= 18 && b <= 19) return true; + if (a === 240 || (a === 255 && b === 255 && c === 255 && d === 255)) return true; + } + + return false; +} + function isPrivateUrl(urlStr: string): boolean { let parsed: URL; try { @@ -126,13 +170,10 @@ function isPrivateUrl(urlStr: string): boolean { return true; } - const hostname = parsed.hostname.toLowerCase(); + const hostname = parsed.hostname.toLowerCase().replace(/^\[|\]$/g, ''); if ( hostname === 'localhost' || - hostname === '127.0.0.1' || - hostname === '::1' || - hostname === '[::1]' || hostname === '0.0.0.0' || hostname.endsWith('.local') || hostname.endsWith('.internal') @@ -140,23 +181,67 @@ function isPrivateUrl(urlStr: string): boolean { return true; } - const ipv4Match = hostname.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/); - if (ipv4Match) { - const [, a, b, c, d] = ipv4Match.map(Number); - if (a === 10) return true; - if (a === 172 && b >= 16 && b <= 31) return true; - if (a === 192 && b === 168) return true; - if (a === 169 && b === 254) return true; - if (a === 127) return true; - if (a === 0) return true; - if (a === 100 && b >= 64 && b <= 127) return true; - if (a === 198 && b >= 18 && b <= 19) return true; - if (a === 240 || (a === 255 && b === 255 && c === 255 && d === 255)) return true; - } + if (isPrivateIp(hostname)) return true; return false; } +const MAX_RESPONSE_BODY_BYTES = 1024 * 1024; // 1MB + +async function readLimitedBody(response: Response, maxBytes: number): Promise { + const reader = response.body?.getReader(); + if (!reader) throw new Error('No response body'); + + const chunks: Uint8Array[] = []; + let totalBytes = 0; + + while (true) { + const { done, value } = await reader.read(); + if (done) break; + totalBytes += value.byteLength; + if (totalBytes > maxBytes) { + reader.cancel(); + throw new Error('Response body exceeds size limit'); + } + chunks.push(value); + } + + const combined = new Uint8Array(totalBytes); + let offset = 0; + for (const chunk of chunks) { + combined.set(chunk, offset); + offset += chunk.byteLength; + } + return new TextDecoder().decode(combined); +} + +async function resolveAndValidateUrl(urlStr: string): Promise { + const parsed = new URL(urlStr); + const hostname = parsed.hostname.replace(/^\[|\]$/g, ''); + + // If hostname is already an IP literal, isPrivateUrl already checked it + if (/^[\d.]+$/.test(hostname) || hostname.includes(':')) return; + + let addresses: string[]; + try { + const results4 = await dns.resolve4(hostname).catch(() => [] as string[]); + const results6 = await dns.resolve6(hostname).catch(() => [] as string[]); + addresses = [...results4, ...results6]; + } catch { + throw new Error('DNS resolution failed'); + } + + if (addresses.length === 0) { + throw new Error('DNS resolution returned no addresses'); + } + + for (const addr of addresses) { + if (isPrivateIp(addr)) { + throw new Error('Resolved address is private'); + } + } +} + async function resolveHostPaths(config: ReturnType): Promise<{ hostDataDir: string; hostSecretsDir: string; @@ -345,6 +430,24 @@ export async function buildServer(): Promise { return { error: 'At least one channel is required' }; } + for (const provider of body.providers) { + if (!VALID_PROVIDER_IDS.has(provider.providerId)) { + reply.code(400); + return { error: `Invalid provider: ${provider.providerId}` }; + } + if (!MODEL_REGEX.test(provider.model)) { + reply.code(400); + return { error: `Invalid model name: ${provider.model}` }; + } + } + + for (const channel of body.channels) { + if (!VALID_CHANNEL_TYPES.has(channel.channelType)) { + reply.code(400); + return { error: `Invalid channel type: ${channel.channelType}` }; + } + } + // Check for duplicate hostname if (getBotByHostname(body.hostname)) { reply.code(409); @@ -673,17 +776,19 @@ export async function buildServer(): Promise { return { error: 'Requests to private/internal addresses are not allowed' }; } - try { - const fetchBase = toDockerHostUrl(baseUrl); + const fetchBase = toDockerHostUrl(baseUrl); - if (isPrivateUrl(fetchBase)) { - reply.code(400); - return { error: 'Requests to private/internal addresses are not allowed' }; - } + if (isPrivateUrl(fetchBase)) { + reply.code(400); + return { error: 'Requests to private/internal addresses are not allowed' }; + } - const url = fetchBase.replace(/\/+$/, '') + '/models'; - const controller = new AbortController(); - const timeout = setTimeout(() => { controller.abort(); }, 5000); + const url = fetchBase.replace(/\/+$/, '') + '/models'; + const controller = new AbortController(); + const timeout = setTimeout(() => { controller.abort(); }, 5000); + + try { + await resolveAndValidateUrl(url); const headers: Record = {}; if (request.body.apiKey) { @@ -691,17 +796,19 @@ export async function buildServer(): Promise { } const response = await fetch(url, { signal: controller.signal, headers }); - clearTimeout(timeout); if (!response.ok) { return { models: [] }; } - const data = await response.json() as { data?: { id: string }[] }; + const bodyText = await readLimitedBody(response, MAX_RESPONSE_BODY_BYTES); + const data = JSON.parse(bodyText) as { data?: { id: string }[] }; const models = (data.data ?? []).map((m: { id: string }) => m.id); return { models }; } catch { return { models: [] }; + } finally { + clearTimeout(timeout); } }); From e554203827199e5fdc8891a7fa6f9cf85c9d6d9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kyle=20=F0=9F=90=86?= Date: Sun, 8 Feb 2026 16:46:24 -0500 Subject: [PATCH 3/8] fix: pin resolved IP to prevent DNS rebinding, simplify security helpers --- src/bots/templates.ts | 57 +++++++++++++++---------------------------- src/server.ts | 43 +++++++++++++++----------------- 2 files changed, 40 insertions(+), 60 deletions(-) diff --git a/src/bots/templates.ts b/src/bots/templates.ts index fd6ad6b..6936bf0 100644 --- a/src/bots/templates.ts +++ b/src/bots/templates.ts @@ -16,14 +16,19 @@ function tryChown(path: string, uid: number, gid: number): void { try { chownSync(path, uid, gid); } catch (err) { - if ((err as NodeJS.ErrnoException).code === 'EPERM') { - // Not running as root - skip chown (acceptable in dev/CI) - return; - } + if ((err as NodeJS.ErrnoException).code === 'EPERM') return; throw err; } } +const OPENCLAW_UID = 1000; +const OPENCLAW_GID = 1000; + +function setOwnership(path: string, mode: number): void { + chmodSync(path, mode); + tryChown(path, OPENCLAW_UID, OPENCLAW_GID); +} + export interface BotPersona { name: string; identity: string; @@ -195,47 +200,25 @@ function generateIdentityMd(persona: BotPersona): string { export function createBotWorkspace(dataDir: string, config: BotWorkspaceConfig): void { const botDir = join(dataDir, 'bots', config.botHostname); const workspaceDir = join(botDir, 'workspace'); + const agentDir = join(botDir, 'agents', 'main', 'agent'); + const sessionsDir = join(botDir, 'agents', 'main', 'sessions'); + const sandboxDir = join(botDir, 'sandbox'); - // OpenClaw runs as uid 1000 (node user), so we need to set ownership - const OPENCLAW_UID = 1000; - const OPENCLAW_GID = 1000; - - mkdirSync(botDir, { recursive: true, mode: 0o755 }); - mkdirSync(workspaceDir, { recursive: true, mode: 0o755 }); - chmodSync(botDir, 0o755); - chmodSync(workspaceDir, 0o755); - tryChown(botDir, OPENCLAW_UID, OPENCLAW_GID); - tryChown(workspaceDir, OPENCLAW_UID, OPENCLAW_GID); + for (const dir of [botDir, workspaceDir, agentDir, sessionsDir, sandboxDir]) { + mkdirSync(dir, { recursive: true, mode: 0o755 }); + setOwnership(dir, 0o755); + } - const openclawConfig = generateOpenclawConfig(config); const configPath = join(botDir, 'openclaw.json'); - writeFileSync(configPath, JSON.stringify(openclawConfig, null, 2)); - chmodSync(configPath, 0o644); - tryChown(configPath, OPENCLAW_UID, OPENCLAW_GID); + writeFileSync(configPath, JSON.stringify(generateOpenclawConfig(config), null, 2)); + setOwnership(configPath, 0o644); const soulPath = join(workspaceDir, 'SOUL.md'); const identityPath = join(workspaceDir, 'IDENTITY.md'); writeFileSync(soulPath, generateSoulMd(config.persona)); writeFileSync(identityPath, generateIdentityMd(config.persona)); - chmodSync(soulPath, 0o644); - chmodSync(identityPath, 0o644); - tryChown(soulPath, OPENCLAW_UID, OPENCLAW_GID); - tryChown(identityPath, OPENCLAW_UID, OPENCLAW_GID); - - const agentDir = join(botDir, 'agents', 'main', 'agent'); - mkdirSync(agentDir, { recursive: true, mode: 0o755 }); - chmodSync(agentDir, 0o755); - tryChown(agentDir, OPENCLAW_UID, OPENCLAW_GID); - - const sessionsDir = join(botDir, 'agents', 'main', 'sessions'); - mkdirSync(sessionsDir, { recursive: true, mode: 0o755 }); - chmodSync(sessionsDir, 0o755); - tryChown(sessionsDir, OPENCLAW_UID, OPENCLAW_GID); - - const sandboxDir = join(botDir, 'sandbox'); - mkdirSync(sandboxDir, { recursive: true, mode: 0o755 }); - chmodSync(sandboxDir, 0o755); - tryChown(sandboxDir, OPENCLAW_UID, OPENCLAW_GID); + setOwnership(soulPath, 0o644); + setOwnership(identityPath, 0o644); } /** diff --git a/src/server.ts b/src/server.ts index 05b8fd5..0eb9089 100644 --- a/src/server.ts +++ b/src/server.ts @@ -131,15 +131,11 @@ function isPrivateIp(ip: string): boolean { const v4Mapped = ip.match(/^::ffff:(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/i); const normalizedIp = v4Mapped ? v4Mapped[1] : ip; - // IPv6 loopback if (normalizedIp === '::1') return true; - // IPv6 link-local - if (normalizedIp.toLowerCase().startsWith('fe80:')) return true; - - // IPv6 unique local (fc00::/7) - const firstChar = normalizedIp.toLowerCase(); - if (firstChar.startsWith('fc') || firstChar.startsWith('fd')) return true; + const lowerIp = normalizedIp.toLowerCase(); + if (lowerIp.startsWith('fe80:')) return true; + if (lowerIp.startsWith('fc') || lowerIp.startsWith('fd')) return true; const ipv4Match = normalizedIp.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/); if (ipv4Match) { @@ -181,9 +177,7 @@ function isPrivateUrl(urlStr: string): boolean { return true; } - if (isPrivateIp(hostname)) return true; - - return false; + return isPrivateIp(hostname); } const MAX_RESPONSE_BODY_BYTES = 1024 * 1024; // 1MB @@ -215,22 +209,19 @@ async function readLimitedBody(response: Response, maxBytes: number): Promise { +async function resolveAndValidateUrl(urlStr: string): Promise<{ resolvedUrl: string; originalHost: string }> { const parsed = new URL(urlStr); const hostname = parsed.hostname.replace(/^\[|\]$/g, ''); // If hostname is already an IP literal, isPrivateUrl already checked it - if (/^[\d.]+$/.test(hostname) || hostname.includes(':')) return; - - let addresses: string[]; - try { - const results4 = await dns.resolve4(hostname).catch(() => [] as string[]); - const results6 = await dns.resolve6(hostname).catch(() => [] as string[]); - addresses = [...results4, ...results6]; - } catch { - throw new Error('DNS resolution failed'); + if (/^[\d.]+$/.test(hostname) || hostname.includes(':')) { + return { resolvedUrl: urlStr, originalHost: hostname }; } + const results4 = await dns.resolve4(hostname).catch(() => [] as string[]); + const results6 = await dns.resolve6(hostname).catch(() => [] as string[]); + const addresses = [...results4, ...results6]; + if (addresses.length === 0) { throw new Error('DNS resolution returned no addresses'); } @@ -240,6 +231,12 @@ async function resolveAndValidateUrl(urlStr: string): Promise { throw new Error('Resolved address is private'); } } + + // Pin to the first resolved IP to prevent DNS rebinding + const pinnedIp = addresses[0]; + const pinnedHost = pinnedIp.includes(':') ? `[${pinnedIp}]` : pinnedIp; + parsed.hostname = pinnedHost; + return { resolvedUrl: parsed.toString(), originalHost: hostname }; } async function resolveHostPaths(config: ReturnType): Promise<{ @@ -788,14 +785,14 @@ export async function buildServer(): Promise { const timeout = setTimeout(() => { controller.abort(); }, 5000); try { - await resolveAndValidateUrl(url); + const { resolvedUrl, originalHost } = await resolveAndValidateUrl(url); - const headers: Record = {}; + const headers: Record = { Host: originalHost }; if (request.body.apiKey) { headers.Authorization = `Bearer ${request.body.apiKey}`; } - const response = await fetch(url, { signal: controller.signal, headers }); + const response = await fetch(resolvedUrl, { signal: controller.signal, headers }); if (!response.ok) { return { models: [] }; From ad908afb3eaa1471531942bacd559858ee561f60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kyle=20=F0=9F=90=86?= Date: Sun, 8 Feb 2026 16:51:27 -0500 Subject: [PATCH 4/8] fix: resolve eslint errors in security helpers --- src/server.ts | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/server.ts b/src/server.ts index 0eb9089..b209820 100644 --- a/src/server.ts +++ b/src/server.ts @@ -128,7 +128,7 @@ const MODEL_REGEX = /^[a-zA-Z0-9._:/-]{1,128}$/; function isPrivateIp(ip: string): boolean { // Handle IPv6-mapped IPv4 (e.g., ::ffff:127.0.0.1) - const v4Mapped = ip.match(/^::ffff:(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/i); + const v4Mapped = /^::ffff:(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/i.exec(ip); const normalizedIp = v4Mapped ? v4Mapped[1] : ip; if (normalizedIp === '::1') return true; @@ -137,7 +137,7 @@ function isPrivateIp(ip: string): boolean { if (lowerIp.startsWith('fe80:')) return true; if (lowerIp.startsWith('fc') || lowerIp.startsWith('fd')) return true; - const ipv4Match = normalizedIp.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/); + const ipv4Match = /^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/.exec(normalizedIp); if (ipv4Match) { const [, a, b, c, d] = ipv4Match.map(Number); if (a === 10) return true; @@ -183,18 +183,20 @@ function isPrivateUrl(urlStr: string): boolean { const MAX_RESPONSE_BODY_BYTES = 1024 * 1024; // 1MB async function readLimitedBody(response: Response, maxBytes: number): Promise { - const reader = response.body?.getReader(); - if (!reader) throw new Error('No response body'); + const body = response.body; + if (!body) throw new Error('No response body'); + const reader = body.getReader(); const chunks: Uint8Array[] = []; let totalBytes = 0; - while (true) { - const { done, value } = await reader.read(); - if (done) break; + for (;;) { + const result = await reader.read(); + if (result.done) break; + const value = result.value as Uint8Array; totalBytes += value.byteLength; if (totalBytes > maxBytes) { - reader.cancel(); + void reader.cancel(); throw new Error('Response body exceeds size limit'); } chunks.push(value); @@ -401,7 +403,7 @@ export async function buildServer(): Promise { return { error: 'Missing required field: name' }; } - if (!/^[a-zA-Z0-9 _.\-]{1,128}$/.test(body.name)) { + if (!/^[a-zA-Z0-9 _.-]{1,128}$/.test(body.name)) { reply.code(400); return { error: 'Bot name must be 1-128 characters: letters, numbers, spaces, underscores, dots, hyphens' }; } From ce5981cb9fcc1dabfa157d38b6a246fbad50a403 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Mon, 9 Feb 2026 17:34:24 +0000 Subject: [PATCH 5/8] fix: harden SSRF and model validation from security review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Reject '..' in model names to prevent path traversal - Add missing IPv6 reserved ranges (unspecified, documentation, TEREDO, discard, NAT64) - Validate IPv4 octets are ≤255, treating invalid IPs as private Co-Authored-By: Claude Opus 4.6 --- src/server.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/server.ts b/src/server.ts index b209820..b614908 100644 --- a/src/server.ts +++ b/src/server.ts @@ -124,7 +124,7 @@ const VALID_PROVIDER_IDS = new Set([ const VALID_CHANNEL_TYPES = new Set(['telegram', 'discord']); -const MODEL_REGEX = /^[a-zA-Z0-9._:/-]{1,128}$/; +const MODEL_REGEX = /^(?!.*\.\.)[a-zA-Z0-9._:/-]{1,128}$/; function isPrivateIp(ip: string): boolean { // Handle IPv6-mapped IPv4 (e.g., ::ffff:127.0.0.1) @@ -136,10 +136,16 @@ function isPrivateIp(ip: string): boolean { const lowerIp = normalizedIp.toLowerCase(); if (lowerIp.startsWith('fe80:')) return true; if (lowerIp.startsWith('fc') || lowerIp.startsWith('fd')) return true; + if (lowerIp === '::') return true; + if (lowerIp.startsWith('2001:db8:')) return true; + if (lowerIp.startsWith('2001:0:')) return true; + if (lowerIp.startsWith('100:')) return true; + if (lowerIp.startsWith('64:ff9b:')) return true; const ipv4Match = /^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/.exec(normalizedIp); if (ipv4Match) { const [, a, b, c, d] = ipv4Match.map(Number); + if (a > 255 || b > 255 || c > 255 || d > 255) return true; if (a === 10) return true; if (a === 172 && b >= 16 && b <= 31) return true; if (a === 192 && b === 168) return true; From 3bff1e8dc8340d2bc7c4ac95912daaee667ee9cc Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Mon, 9 Feb 2026 17:35:19 +0000 Subject: [PATCH 6/8] chore: bump version to 1.0.2 Co-Authored-By: Claude Opus 4.6 --- dashboard/package-lock.json | 4 ++-- dashboard/package.json | 2 +- package-lock.json | 11 ++--------- package.json | 2 +- proxy/package-lock.json | 11 ++--------- proxy/package.json | 2 +- 6 files changed, 9 insertions(+), 23 deletions(-) diff --git a/dashboard/package-lock.json b/dashboard/package-lock.json index d6a9577..a8c9cc5 100644 --- a/dashboard/package-lock.json +++ b/dashboard/package-lock.json @@ -1,12 +1,12 @@ { "name": "botmaker-dashboard", - "version": "1.0.1", + "version": "1.0.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "botmaker-dashboard", - "version": "1.0.1", + "version": "1.0.2", "dependencies": { "react": "^18.3.1", "react-dom": "^18.3.1" diff --git a/dashboard/package.json b/dashboard/package.json index 977134c..1a53e0d 100644 --- a/dashboard/package.json +++ b/dashboard/package.json @@ -1,7 +1,7 @@ { "name": "botmaker-dashboard", "private": true, - "version": "1.0.1", + "version": "1.0.2", "type": "module", "scripts": { "dev": "vite", diff --git a/package-lock.json b/package-lock.json index f4efd5a..04bbdf4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "botmaker", - "version": "1.0.1", + "version": "1.0.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "botmaker", - "version": "1.0.1", + "version": "1.0.2", "license": "MIT", "dependencies": { "@fastify/basic-auth": "^6.0.0", @@ -1729,7 +1729,6 @@ "integrity": "sha512-BtE0k6cjwjLZoZixN0t5AKP0kSzlGu7FctRXYuPAm//aaiZhmfq1JwdYpYr1brzEspYyFeF+8XF5j2VK6oalrA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.54.0", "@typescript-eslint/types": "8.54.0", @@ -2099,7 +2098,6 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -2823,7 +2821,6 @@ "integrity": "sha512-LEyamqS7W5HB3ujJyvi0HQK/dtVINZvd5mAAp9eT5S/ujByGjiZLCzPcHVzuXbpJDJF/cxwHlfceVUDZ2lnSTw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -4078,7 +4075,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -5084,7 +5080,6 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", - "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -5158,7 +5153,6 @@ "integrity": "sha512-o5a9xKjbtuhY6Bi5S3+HvbRERmouabWbyUcpXXUA1u+GNUKoROi9byOJ8M0nHbHYHkYICiMlqxkg1KkYmm25Sw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "^0.21.3", "postcss": "^8.4.43", @@ -5672,7 +5666,6 @@ "integrity": "sha512-MSmPM9REYqDGBI8439mA4mWhV5sKmDlBKWIYbA3lRb2PTHACE0mgKwA8yQ2xq9vxDTuk4iPrECBAEW2aoFXY0Q==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/expect": "2.1.9", "@vitest/mocker": "2.1.9", diff --git a/package.json b/package.json index f6c6b8e..9dab8c5 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "botmaker", - "version": "1.0.1", + "version": "1.0.2", "description": "Web UI for creating and managing OpenClaw bots", "author": "Jeff Garzik", "license": "MIT", diff --git a/proxy/package-lock.json b/proxy/package-lock.json index 111b337..ea17495 100644 --- a/proxy/package-lock.json +++ b/proxy/package-lock.json @@ -1,12 +1,12 @@ { "name": "keyring-proxy", - "version": "1.0.1", + "version": "1.0.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "keyring-proxy", - "version": "1.0.1", + "version": "1.0.2", "dependencies": { "@fastify/helmet": "^13.0.2", "@fastify/rate-limit": "^10.3.0", @@ -1490,7 +1490,6 @@ "integrity": "sha512-BtE0k6cjwjLZoZixN0t5AKP0kSzlGu7FctRXYuPAm//aaiZhmfq1JwdYpYr1brzEspYyFeF+8XF5j2VK6oalrA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.54.0", "@typescript-eslint/types": "8.54.0", @@ -1834,7 +1833,6 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -2745,7 +2743,6 @@ "integrity": "sha512-LEyamqS7W5HB3ujJyvi0HQK/dtVINZvd5mAAp9eT5S/ujByGjiZLCzPcHVzuXbpJDJF/cxwHlfceVUDZ2lnSTw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -3957,7 +3954,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -4795,7 +4791,6 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", - "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -4870,7 +4865,6 @@ "integrity": "sha512-o5a9xKjbtuhY6Bi5S3+HvbRERmouabWbyUcpXXUA1u+GNUKoROi9byOJ8M0nHbHYHkYICiMlqxkg1KkYmm25Sw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "^0.21.3", "postcss": "^8.4.43", @@ -5010,7 +5004,6 @@ "integrity": "sha512-MSmPM9REYqDGBI8439mA4mWhV5sKmDlBKWIYbA3lRb2PTHACE0mgKwA8yQ2xq9vxDTuk4iPrECBAEW2aoFXY0Q==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/expect": "2.1.9", "@vitest/mocker": "2.1.9", diff --git a/proxy/package.json b/proxy/package.json index 2ec9eeb..7973f11 100644 --- a/proxy/package.json +++ b/proxy/package.json @@ -1,6 +1,6 @@ { "name": "keyring-proxy", - "version": "1.0.1", + "version": "1.0.2", "description": "HTTP forward proxy for BotMaker - validates bot tokens, injects API keys", "type": "module", "main": "dist/index.js", From 43ee1a1532fe31296d62ea77ddda11a45f435f80 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Mon, 9 Feb 2026 18:49:12 +0000 Subject: [PATCH 7/8] fix: widen workspace permissions when chown fails When tryChown gets EPERM (non-root host), directories kept 0755 owned by the host user, leaving container UID 1000 without write access. Now falls back to permissive mode (0777 dirs, 0666 files) so bots can always write to their workspace. Co-Authored-By: Claude Opus 4.6 --- src/bots/templates.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/bots/templates.ts b/src/bots/templates.ts index 5f49806..5cbf688 100644 --- a/src/bots/templates.ts +++ b/src/bots/templates.ts @@ -12,11 +12,12 @@ import { validateHostname } from '../secrets/manager.js'; * Try to change file ownership, but gracefully skip if not permitted. * chown requires root privileges; in CI/dev environments we may not have them. */ -function tryChown(path: string, uid: number, gid: number): void { +function tryChown(path: string, uid: number, gid: number): boolean { try { chownSync(path, uid, gid); + return true; } catch (err) { - if ((err as NodeJS.ErrnoException).code === 'EPERM') return; + if ((err as NodeJS.ErrnoException).code === 'EPERM') return false; throw err; } } @@ -25,8 +26,9 @@ const OPENCLAW_UID = 1000; const OPENCLAW_GID = 1000; function setOwnership(path: string, mode: number): void { - chmodSync(path, mode); - tryChown(path, OPENCLAW_UID, OPENCLAW_GID); + const owned = tryChown(path, OPENCLAW_UID, OPENCLAW_GID); + // If chown failed, widen permissions so container UID 1000 can still write + chmodSync(path, owned ? mode : mode | 0o022); } export interface BotPersona { From de0513103cbb960970acfe49522599ca96b23146 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Mon, 9 Feb 2026 19:15:15 +0000 Subject: [PATCH 8/8] fix: address security review feedback - Allowlist localhost/127.0.0.1/host.docker.internal for model discovery so Ollama and local providers work on admin-authenticated endpoint - Preserve original hostname in resolveAndValidateUrl so HTTPS TLS/SNI and certificate validation work correctly - Expand VALID_CHANNEL_TYPES to match all 30 dashboard channel types - Add 12 tests for /api/models/discover endpoint covering auth, SSRF blocking, localhost allowlist, and input validation Co-Authored-By: Claude Opus 4.6 --- src/discover.test.ts | 235 +++++++++++++++++++++++++++++++++++++++++++ src/server.ts | 34 ++++--- 2 files changed, 257 insertions(+), 12 deletions(-) create mode 100644 src/discover.test.ts diff --git a/src/discover.test.ts b/src/discover.test.ts new file mode 100644 index 0000000..1f3e620 --- /dev/null +++ b/src/discover.test.ts @@ -0,0 +1,235 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import type { FastifyInstance } from 'fastify'; +import { sessions } from './server.js'; + +interface ErrorResponse { + error: string; +} + +async function createTestServer(): Promise { + process.env.ADMIN_PASSWORD = 'test-password-long'; + vi.doMock('./services/DockerService.js', () => ({ + DockerService: class { + getContainerStatus = vi.fn().mockResolvedValue(null); + getAllContainerStats = vi.fn().mockResolvedValue([]); + getVolumeMountpoint = vi.fn().mockResolvedValue('/tmp/test'); + }, + })); + vi.doMock('./db/index.js', () => ({ + initDb: vi.fn(), + getDb: vi.fn().mockReturnValue({ transaction: vi.fn(() => vi.fn()) }), + })); + vi.doMock('./bots/store.js', () => ({ + listBots: vi.fn().mockReturnValue([]), + })); + vi.doMock('./services/ReconciliationService.js', () => ({ + ReconciliationService: class { + reconcileOnStartup = vi.fn().mockResolvedValue({ + orphanedContainers: [], + orphanedWorkspaces: [], + orphanedSecrets: [], + }); + }, + })); + + const { buildServer } = await import('./server.js'); + return buildServer(); +} + +async function getAuthToken(server: FastifyInstance): Promise { + const response = await server.inject({ + method: 'POST', + url: '/api/login', + payload: { password: 'test-password-long' }, + }); + const body = JSON.parse(response.body) as { token: string }; + return body.token; +} + +describe('/api/models/discover', () => { + const originalEnv = { ...process.env }; + + beforeEach(() => { + sessions.clear(); + vi.resetModules(); + }); + + afterEach(() => { + process.env = { ...originalEnv }; + sessions.clear(); + vi.resetModules(); + }); + + it('should require authentication', async () => { + const server = await createTestServer(); + const response = await server.inject({ + method: 'POST', + url: '/api/models/discover', + payload: { baseUrl: 'https://api.openai.com/v1' }, + }); + expect(response.statusCode).toBe(401); + await server.close(); + }); + + it('should reject missing baseUrl', async () => { + const server = await createTestServer(); + const token = await getAuthToken(server); + const response = await server.inject({ + method: 'POST', + url: '/api/models/discover', + payload: {}, + headers: { Authorization: `Bearer ${token}` }, + }); + expect(response.statusCode).toBe(400); + const body = JSON.parse(response.body) as ErrorResponse; + expect(body.error).toMatch(/baseUrl/i); + await server.close(); + }); + + it('should reject private IPv4 addresses (10.x)', async () => { + const server = await createTestServer(); + const token = await getAuthToken(server); + const response = await server.inject({ + method: 'POST', + url: '/api/models/discover', + payload: { baseUrl: 'http://10.0.0.1/v1' }, + headers: { Authorization: `Bearer ${token}` }, + }); + expect(response.statusCode).toBe(400); + const body = JSON.parse(response.body) as ErrorResponse; + expect(body.error).toMatch(/private/i); + await server.close(); + }); + + it('should reject private IPv4 addresses (192.168.x)', async () => { + const server = await createTestServer(); + const token = await getAuthToken(server); + const response = await server.inject({ + method: 'POST', + url: '/api/models/discover', + payload: { baseUrl: 'http://192.168.1.1/v1' }, + headers: { Authorization: `Bearer ${token}` }, + }); + expect(response.statusCode).toBe(400); + const body = JSON.parse(response.body) as ErrorResponse; + expect(body.error).toMatch(/private/i); + await server.close(); + }); + + it('should allow localhost for local discovery (Ollama)', async () => { + const server = await createTestServer(); + const token = await getAuthToken(server); + const response = await server.inject({ + method: 'POST', + url: '/api/models/discover', + payload: { baseUrl: 'http://localhost:11434/v1' }, + headers: { Authorization: `Bearer ${token}` }, + }); + // Allowed through SSRF check; fetch fails → returns empty models + expect(response.statusCode).toBe(200); + await server.close(); + }); + + it('should allow 127.0.0.1 for local discovery', async () => { + const server = await createTestServer(); + const token = await getAuthToken(server); + const response = await server.inject({ + method: 'POST', + url: '/api/models/discover', + payload: { baseUrl: 'http://127.0.0.1:11434/v1' }, + headers: { Authorization: `Bearer ${token}` }, + }); + expect(response.statusCode).toBe(200); + await server.close(); + }); + + it('should reject non-http protocols', async () => { + const server = await createTestServer(); + const token = await getAuthToken(server); + const response = await server.inject({ + method: 'POST', + url: '/api/models/discover', + payload: { baseUrl: 'ftp://example.com/v1' }, + headers: { Authorization: `Bearer ${token}` }, + }); + expect(response.statusCode).toBe(400); + const body = JSON.parse(response.body) as ErrorResponse; + expect(body.error).toMatch(/private/i); + await server.close(); + }); + + it('should reject link-local IPv6', async () => { + const server = await createTestServer(); + const token = await getAuthToken(server); + const response = await server.inject({ + method: 'POST', + url: '/api/models/discover', + payload: { baseUrl: 'http://[fe80::1]/v1' }, + headers: { Authorization: `Bearer ${token}` }, + }); + expect(response.statusCode).toBe(400); + const body = JSON.parse(response.body) as ErrorResponse; + expect(body.error).toMatch(/private/i); + await server.close(); + }); + + it('should reject IPv6 loopback', async () => { + const server = await createTestServer(); + const token = await getAuthToken(server); + const response = await server.inject({ + method: 'POST', + url: '/api/models/discover', + payload: { baseUrl: 'http://[::1]/v1' }, + headers: { Authorization: `Bearer ${token}` }, + }); + expect(response.statusCode).toBe(400); + const body = JSON.parse(response.body) as ErrorResponse; + expect(body.error).toMatch(/private/i); + await server.close(); + }); + + it('should reject .local domains', async () => { + const server = await createTestServer(); + const token = await getAuthToken(server); + const response = await server.inject({ + method: 'POST', + url: '/api/models/discover', + payload: { baseUrl: 'http://myhost.local/v1' }, + headers: { Authorization: `Bearer ${token}` }, + }); + expect(response.statusCode).toBe(400); + const body = JSON.parse(response.body) as ErrorResponse; + expect(body.error).toMatch(/private/i); + await server.close(); + }); + + it('should reject 172.16.x private range', async () => { + const server = await createTestServer(); + const token = await getAuthToken(server); + const response = await server.inject({ + method: 'POST', + url: '/api/models/discover', + payload: { baseUrl: 'http://172.16.0.1/v1' }, + headers: { Authorization: `Bearer ${token}` }, + }); + expect(response.statusCode).toBe(400); + const body = JSON.parse(response.body) as ErrorResponse; + expect(body.error).toMatch(/private/i); + await server.close(); + }); + + it('should reject invalid URL', async () => { + const server = await createTestServer(); + const token = await getAuthToken(server); + const response = await server.inject({ + method: 'POST', + url: '/api/models/discover', + payload: { baseUrl: 'not-a-url' }, + headers: { Authorization: `Bearer ${token}` }, + }); + expect(response.statusCode).toBe(400); + const body = JSON.parse(response.body) as ErrorResponse; + expect(body.error).toMatch(/private/i); + await server.close(); + }); +}); diff --git a/src/server.ts b/src/server.ts index b614908..58643fc 100644 --- a/src/server.ts +++ b/src/server.ts @@ -115,6 +115,16 @@ function toDockerHostUrl(url: string): string { return url.replace(/\blocalhost\b|127\.0\.0\.1/g, 'host.docker.internal'); } +/** Check if a URL targets a local discovery host (localhost, 127.0.0.1, host.docker.internal). */ +function isLocalDiscoveryUrl(urlStr: string): boolean { + try { + const hostname = new URL(urlStr).hostname.toLowerCase(); + return hostname === 'localhost' || hostname === '127.0.0.1' || hostname === 'host.docker.internal'; + } catch { + return false; + } +} + const VALID_PROVIDER_IDS = new Set([ 'openai', 'anthropic', 'google', 'venice', 'openrouter', 'ollama', 'grok', 'deepseek', 'mistral', 'groq', 'cerebras', 'fireworks', 'togetherai', @@ -122,7 +132,13 @@ const VALID_PROVIDER_IDS = new Set([ 'nebius', 'ovhcloud', 'huggingface', ]); -const VALID_CHANNEL_TYPES = new Set(['telegram', 'discord']); +const VALID_CHANNEL_TYPES = new Set([ + 'telegram', 'discord', 'slack', 'signal', 'whatsapp', 'matrix', 'nostr', + 'twitter', 'facebook', 'instagram', 'teams', 'line', 'wechat', 'viber', + 'kik', 'twitch', 'reddit', 'mastodon', 'bluesky', 'rocketchat', + 'mattermost', 'zulip', 'irc', 'xmpp', 'sms', 'email', 'googlechat', + 'webex', 'web', 'webhook', +]); const MODEL_REGEX = /^(?!.*\.\.)[a-zA-Z0-9._:/-]{1,128}$/; @@ -240,11 +256,9 @@ async function resolveAndValidateUrl(urlStr: string): Promise<{ resolvedUrl: str } } - // Pin to the first resolved IP to prevent DNS rebinding - const pinnedIp = addresses[0]; - const pinnedHost = pinnedIp.includes(':') ? `[${pinnedIp}]` : pinnedIp; - parsed.hostname = pinnedHost; - return { resolvedUrl: parsed.toString(), originalHost: hostname }; + // All resolved addresses validated as non-private. + // Preserve original hostname so HTTPS TLS/SNI and cert validation work. + return { resolvedUrl: urlStr, originalHost: hostname }; } async function resolveHostPaths(config: ReturnType): Promise<{ @@ -776,14 +790,10 @@ export async function buildServer(): Promise { return { error: 'Missing baseUrl in request body' }; } - if (isPrivateUrl(baseUrl)) { - reply.code(400); - return { error: 'Requests to private/internal addresses are not allowed' }; - } - const fetchBase = toDockerHostUrl(baseUrl); + const isLocal = isLocalDiscoveryUrl(baseUrl) || isLocalDiscoveryUrl(fetchBase); - if (isPrivateUrl(fetchBase)) { + if (!isLocal && (isPrivateUrl(baseUrl) || isPrivateUrl(fetchBase))) { reply.code(400); return { error: 'Requests to private/internal addresses are not allowed' }; }