diff --git a/Dockerfile b/Dockerfile index d7fd5d313..f8902afa3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -11,7 +11,7 @@ RUN ARCH="$(dpkg --print-architecture)" \ *) echo "Unsupported architecture: ${ARCH}" >&2; exit 1 ;; \ esac \ && apt-get update && apt-get install -y xz-utils ca-certificates rsync \ - && curl -fsSLk https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-${NODE_ARCH}.tar.xz -o /tmp/node.tar.xz \ + && curl -fsSL https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-${NODE_ARCH}.tar.xz -o /tmp/node.tar.xz \ && tar -xJf /tmp/node.tar.xz -C /usr/local --strip-components=1 \ && rm /tmp/node.tar.xz \ && node --version \ diff --git a/skills/cloudflare-browser/scripts/cdp-client.js b/skills/cloudflare-browser/scripts/cdp-client.js index 93f4c6932..feb91e910 100755 --- a/skills/cloudflare-browser/scripts/cdp-client.js +++ b/skills/cloudflare-browser/scripts/cdp-client.js @@ -22,11 +22,19 @@ function createClient(options = {}) { } const workerUrl = (options.workerUrl || process.env.WORKER_URL).replace(/^https?:\/\//, ''); + // Keep query param for backwards compatibility, but also use Authorization header (preferred) const wsUrl = `wss://${workerUrl}/cdp?secret=${encodeURIComponent(CDP_SECRET)}`; const timeout = options.timeout || 60000; + // Security: Use Authorization header (preferred over query param to avoid logging secrets) + const WS_OPTIONS = { + headers: { + 'Authorization': `Bearer ${CDP_SECRET}`, + }, + }; + return new Promise((resolve, reject) => { - const ws = new WebSocket(wsUrl); + const ws = new WebSocket(wsUrl, WS_OPTIONS); let messageId = 1; const pending = new Map(); let targetId = null; diff --git a/skills/cloudflare-browser/scripts/screenshot.js b/skills/cloudflare-browser/scripts/screenshot.js index 522116b08..ebe994cb2 100755 --- a/skills/cloudflare-browser/scripts/screenshot.js +++ b/skills/cloudflare-browser/scripts/screenshot.js @@ -15,8 +15,16 @@ if (!CDP_SECRET) { } const WORKER_URL = process.env.WORKER_URL.replace(/^https?:\/\//, ''); +// Keep query param for backwards compatibility const WS_URL = `wss://${WORKER_URL}/cdp?secret=${encodeURIComponent(CDP_SECRET)}`; +// Security: Use Authorization header (preferred over query param to avoid logging secrets) +const WS_OPTIONS = { + headers: { + 'Authorization': `Bearer ${CDP_SECRET}`, + }, +}; + const url = process.argv[2]; const output = process.argv[3] || 'screenshot.png'; @@ -31,7 +39,7 @@ const pending = new Map(); async function main() { console.log(`Capturing screenshot of ${url}`); - const ws = new WebSocket(WS_URL); + const ws = new WebSocket(WS_URL, WS_OPTIONS); let targetResolve; const targetReady = new Promise(r => { targetResolve = r; }); diff --git a/skills/cloudflare-browser/scripts/video.js b/skills/cloudflare-browser/scripts/video.js index b1b148a92..d18d0a7be 100755 --- a/skills/cloudflare-browser/scripts/video.js +++ b/skills/cloudflare-browser/scripts/video.js @@ -19,8 +19,16 @@ if (!CDP_SECRET) { } const WORKER_URL = process.env.WORKER_URL.replace(/^https?:\/\//, ''); +// Keep query param for backwards compatibility const WS_URL = `wss://${WORKER_URL}/cdp?secret=${encodeURIComponent(CDP_SECRET)}`; +// Security: Use Authorization header (preferred over query param to avoid logging secrets) +const WS_OPTIONS = { + headers: { + 'Authorization': `Bearer ${CDP_SECRET}`, + }, +}; + // Parse args const args = process.argv.slice(2); const urlArg = args.find(a => !a.startsWith('--')); @@ -44,7 +52,7 @@ async function main() { console.log(`Creating video from ${urls.length} URL(s)`); console.log(`Output: ${output}, FPS: ${fps}, Scroll: ${doScroll}\n`); - const ws = new WebSocket(WS_URL); + const ws = new WebSocket(WS_URL, WS_OPTIONS); let targetResolve; const targetReady = new Promise(r => { targetResolve = r; }); diff --git a/src/auth/middleware.ts b/src/auth/middleware.ts index 0b170a995..0643fdece 100644 --- a/src/auth/middleware.ts +++ b/src/auth/middleware.ts @@ -79,8 +79,20 @@ export function createAccessMiddleware(options: AccessMiddlewareOptions) { // Get JWT const jwt = extractJWT(c); + const clientIP = c.req.header('CF-Connecting-IP') || 'unknown'; + const userAgent = c.req.header('User-Agent') || 'unknown'; if (!jwt) { + // Security: Log authentication failure + console.log(JSON.stringify({ + event: 'auth_failure', + reason: 'missing_jwt', + ip: clientIP, + userAgent, + path: new URL(c.req.url).pathname, + timestamp: new Date().toISOString(), + })); + if (type === 'html' && redirectOnMissing) { return c.redirect(`https://${teamDomain}`, 302); } @@ -107,8 +119,28 @@ export function createAccessMiddleware(options: AccessMiddlewareOptions) { try { const payload = await verifyAccessJWT(jwt, teamDomain, expectedAud); c.set('accessUser', { email: payload.email, name: payload.name }); + + // Security: Log successful authentication + console.log(JSON.stringify({ + event: 'auth_success', + email: payload.email, + ip: clientIP, + path: new URL(c.req.url).pathname, + timestamp: new Date().toISOString(), + })); + await next(); } catch (err) { + // Security: Log authentication failure with details + console.log(JSON.stringify({ + event: 'auth_failure', + reason: 'invalid_jwt', + error: err instanceof Error ? err.message : 'Unknown error', + ip: clientIP, + userAgent, + path: new URL(c.req.url).pathname, + timestamp: new Date().toISOString(), + })); console.error('Access JWT verification failed:', err); if (type === 'json') { diff --git a/src/gateway/process.ts b/src/gateway/process.ts index aa35e0696..cb7dfb2be 100644 --- a/src/gateway/process.ts +++ b/src/gateway/process.ts @@ -4,6 +4,34 @@ import { MOLTBOT_PORT, STARTUP_TIMEOUT_MS } from '../config'; import { buildEnvVars } from './env'; import { mountR2Storage } from './r2'; +/** + * Lock to prevent race conditions when starting the gateway + */ +let gatewayLock: Promise | null = null; + +/** + * Execute a function with a lock to prevent concurrent execution + */ +async function withGatewayLock(fn: () => Promise): Promise { + // Wait for any existing lock to complete + while (gatewayLock) { + await gatewayLock; + } + + // Create our lock + let releaseLock: () => void; + gatewayLock = new Promise((resolve) => { + releaseLock = resolve; + }); + + try { + return await fn(); + } finally { + gatewayLock = null; + releaseLock!(); + } +} + /** * Find an existing Moltbot gateway process * @@ -48,9 +76,10 @@ export async function findExistingMoltbotProcess(sandbox: Sandbox): Promise { - // Mount R2 storage for persistent data (non-blocking if not configured) - // R2 is used as a backup - the startup script will restore from it on boot - await mountR2Storage(sandbox, env); + return withGatewayLock(async () => { + // Mount R2 storage for persistent data (non-blocking if not configured) + // R2 is used as a backup - the startup script will restore from it on boot + await mountR2Storage(sandbox, env); // Check if Moltbot is already running or starting const existingProcess = await findExistingMoltbotProcess(sandbox); @@ -82,7 +111,8 @@ export async function ensureMoltbotGateway(sandbox: Sandbox, env: MoltbotEnv): P const command = '/usr/local/bin/start-moltbot.sh'; console.log('Starting process with command:', command); - console.log('Environment vars being passed:', Object.keys(envVars)); + // Security: Only log count of env vars, not their names (which could leak info) + console.log('Environment vars count:', Object.keys(envVars).length); let process: Process; try { @@ -121,4 +151,5 @@ export async function ensureMoltbotGateway(sandbox: Sandbox, env: MoltbotEnv): P console.log('[Gateway] Verifying gateway health...'); return process; + }); } diff --git a/src/gateway/r2.ts b/src/gateway/r2.ts index 302c61d7d..b32775a7b 100644 --- a/src/gateway/r2.ts +++ b/src/gateway/r2.ts @@ -2,6 +2,34 @@ import type { Sandbox } from '@cloudflare/sandbox'; import type { MoltbotEnv } from '../types'; import { R2_MOUNT_PATH, getR2BucketName } from '../config'; +/** + * Lock to prevent race conditions during mount operations + */ +let mountLock: Promise | null = null; + +/** + * Execute a function with a lock to prevent concurrent mount operations + */ +async function withMountLock(fn: () => Promise): Promise { + // Wait for any existing lock to complete + while (mountLock) { + await mountLock; + } + + // Create our lock + let releaseLock: () => void; + mountLock = new Promise((resolve) => { + releaseLock = resolve; + }); + + try { + return await fn(); + } finally { + mountLock = null; + releaseLock!(); + } +} + /** * Check if R2 is already mounted by looking at the mount table */ @@ -33,43 +61,45 @@ async function isR2Mounted(sandbox: Sandbox): Promise { * @returns true if mounted successfully, false otherwise */ export async function mountR2Storage(sandbox: Sandbox, env: MoltbotEnv): Promise { - // Skip if R2 credentials are not configured - if (!env.R2_ACCESS_KEY_ID || !env.R2_SECRET_ACCESS_KEY || !env.CF_ACCOUNT_ID) { - console.log('R2 storage not configured (missing R2_ACCESS_KEY_ID, R2_SECRET_ACCESS_KEY, or CF_ACCOUNT_ID)'); - return false; - } - - // Check if already mounted first - this avoids errors and is faster - if (await isR2Mounted(sandbox)) { - console.log('R2 bucket already mounted at', R2_MOUNT_PATH); - return true; - } + return withMountLock(async () => { + // Skip if R2 credentials are not configured + if (!env.R2_ACCESS_KEY_ID || !env.R2_SECRET_ACCESS_KEY || !env.CF_ACCOUNT_ID) { + console.log('R2 storage not configured (missing R2_ACCESS_KEY_ID, R2_SECRET_ACCESS_KEY, or CF_ACCOUNT_ID)'); + return false; + } - const bucketName = getR2BucketName(env); - try { - console.log('Mounting R2 bucket', bucketName, 'at', R2_MOUNT_PATH); - await sandbox.mountBucket(bucketName, R2_MOUNT_PATH, { - endpoint: `https://${env.CF_ACCOUNT_ID}.r2.cloudflarestorage.com`, - // Pass credentials explicitly since we use R2_* naming instead of AWS_* - credentials: { - accessKeyId: env.R2_ACCESS_KEY_ID, - secretAccessKey: env.R2_SECRET_ACCESS_KEY, - }, - }); - console.log('R2 bucket mounted successfully - moltbot data will persist across sessions'); - return true; - } catch (err) { - const errorMessage = err instanceof Error ? err.message : String(err); - console.log('R2 mount error:', errorMessage); - - // Check again if it's mounted - the error might be misleading + // Check if already mounted first - this avoids errors and is faster if (await isR2Mounted(sandbox)) { - console.log('R2 bucket is mounted despite error'); + console.log('R2 bucket already mounted at', R2_MOUNT_PATH); return true; } - - // Don't fail if mounting fails - moltbot can still run without persistent storage - console.error('Failed to mount R2 bucket:', err); - return false; - } + + const bucketName = getR2BucketName(env); + try { + console.log('Mounting R2 bucket', bucketName, 'at', R2_MOUNT_PATH); + await sandbox.mountBucket(bucketName, R2_MOUNT_PATH, { + endpoint: `https://${env.CF_ACCOUNT_ID}.r2.cloudflarestorage.com`, + // Pass credentials explicitly since we use R2_* naming instead of AWS_* + credentials: { + accessKeyId: env.R2_ACCESS_KEY_ID, + secretAccessKey: env.R2_SECRET_ACCESS_KEY, + }, + }); + console.log('R2 bucket mounted successfully - moltbot data will persist across sessions'); + return true; + } catch (err) { + const errorMessage = err instanceof Error ? err.message : String(err); + console.log('R2 mount error:', errorMessage); + + // Check again if it's mounted - the error might be misleading + if (await isR2Mounted(sandbox)) { + console.log('R2 bucket is mounted despite error'); + return true; + } + + // Don't fail if mounting fails - moltbot can still run without persistent storage + console.error('Failed to mount R2 bucket:', err); + return false; + } + }); } diff --git a/src/gateway/sync.ts b/src/gateway/sync.ts index a10c711a4..572e062d3 100644 --- a/src/gateway/sync.ts +++ b/src/gateway/sync.ts @@ -7,10 +7,39 @@ import { waitForProcess } from './utils'; export interface SyncResult { success: boolean; lastSync?: string; + checksum?: string; error?: string; details?: string; } +/** + * Lock to prevent race conditions during sync operations + */ +let syncLock: Promise | null = null; + +/** + * Execute a function with a lock to prevent concurrent sync operations + */ +async function withSyncLock(fn: () => Promise): Promise { + // Wait for any existing lock to complete + while (syncLock) { + await syncLock; + } + + // Create our lock + let releaseLock: () => void; + syncLock = new Promise((resolve) => { + releaseLock = resolve; + }); + + try { + return await fn(); + } finally { + syncLock = null; + releaseLock!(); + } +} + /** * Sync moltbot config from container to R2 for persistence. * @@ -25,10 +54,11 @@ export interface SyncResult { * @returns SyncResult with success status and optional error details */ export async function syncToR2(sandbox: Sandbox, env: MoltbotEnv): Promise { - // Check if R2 is configured - if (!env.R2_ACCESS_KEY_ID || !env.R2_SECRET_ACCESS_KEY || !env.CF_ACCOUNT_ID) { - return { success: false, error: 'R2 storage is not configured' }; - } + return withSyncLock(async () => { + // Check if R2 is configured + if (!env.R2_ACCESS_KEY_ID || !env.R2_SECRET_ACCESS_KEY || !env.CF_ACCOUNT_ID) { + return { success: false, error: 'R2 storage is not configured' }; + } // Mount R2 if not already mounted const mounted = await mountR2Storage(sandbox, env); @@ -59,7 +89,8 @@ export async function syncToR2(sandbox: Sandbox, env: MoltbotEnv): Promise ${R2_MOUNT_PATH}/.last-sync`; + // Security: Also generate SHA-256 checksum for integrity verification + const syncCmd = `rsync -r --no-times --delete --exclude='*.lock' --exclude='*.log' --exclude='*.tmp' /root/.clawdbot/ ${R2_MOUNT_PATH}/clawdbot/ && rsync -r --no-times --delete /root/clawd/skills/ ${R2_MOUNT_PATH}/skills/ && date -Iseconds > ${R2_MOUNT_PATH}/.last-sync && sha256sum /root/.clawdbot/clawdbot.json 2>/dev/null | cut -d' ' -f1 > ${R2_MOUNT_PATH}/.checksum`; try { const proc = await sandbox.startProcess(syncCmd); @@ -73,8 +104,14 @@ export async function syncToR2(sandbox: Sandbox, env: MoltbotEnv): Promise/dev/null || echo ""`); + await waitForProcess(checksumProc, 5000); + const checksumLogs = await checksumProc.getLogs(); + const checksum = checksumLogs.stdout?.trim() || undefined; + if (lastSync && lastSync.match(/^\d{4}-\d{2}-\d{2}/)) { - return { success: true, lastSync }; + return { success: true, lastSync, checksum }; } else { const logs = await proc.getLogs(); return { @@ -90,4 +127,5 @@ export async function syncToR2(sandbox: Sandbox, env: MoltbotEnv): Promise { newHeaders.set('X-Worker-Debug', 'proxy-to-moltbot'); newHeaders.set('X-Debug-Path', url.pathname); + // Security: Prevent cache poisoning for authenticated responses + // Add appropriate Cache-Control headers when authentication is involved + const hasAuthHeader = request.headers.get('Authorization') || request.headers.get('Cookie'); + const hasSetCookie = httpResponse.headers.get('Set-Cookie'); + if (hasAuthHeader || hasSetCookie) { + newHeaders.set('Cache-Control', 'private, no-store, must-revalidate'); + newHeaders.set('Vary', 'Authorization, Cookie'); + } + return new Response(httpResponse.body, { status: httpResponse.status, statusText: httpResponse.statusText, diff --git a/src/middleware/index.ts b/src/middleware/index.ts new file mode 100644 index 000000000..75da2c27d --- /dev/null +++ b/src/middleware/index.ts @@ -0,0 +1 @@ +export { rateLimit, adminRateLimit, cdpRateLimit, authRateLimit } from './ratelimit'; diff --git a/src/middleware/ratelimit.ts b/src/middleware/ratelimit.ts new file mode 100644 index 000000000..37971117b --- /dev/null +++ b/src/middleware/ratelimit.ts @@ -0,0 +1,88 @@ +import type { Context, Next } from 'hono'; +import type { AppEnv } from '../types'; + +interface RateLimitOptions { + maxRequests: number; + windowSec: number; + keyFn: (c: Context) => string; + message?: string; +} + +// In-memory store for rate limiting (per-isolate) +const rateLimitStore = new Map(); + +/** + * Creates a rate limiting middleware + */ +export function rateLimit(options: RateLimitOptions) { + const { maxRequests, windowSec, keyFn, message = 'Too many requests. Please try again later.' } = options; + + return async (c: Context, next: Next) => { + const key = keyFn(c); + const now = Date.now(); + const windowMs = windowSec * 1000; + + let entry = rateLimitStore.get(key); + + // Clean up expired entries periodically + if (rateLimitStore.size > 10000) { + for (const [k, v] of rateLimitStore) { + if (v.resetAt < now) { + rateLimitStore.delete(k); + } + } + } + + if (!entry || entry.resetAt < now) { + // New window + entry = { count: 1, resetAt: now + windowMs }; + rateLimitStore.set(key, entry); + } else { + entry.count++; + } + + // Set rate limit headers + c.header('X-RateLimit-Limit', String(maxRequests)); + c.header('X-RateLimit-Remaining', String(Math.max(0, maxRequests - entry.count))); + c.header('X-RateLimit-Reset', String(Math.ceil(entry.resetAt / 1000))); + + if (entry.count > maxRequests) { + return c.json({ error: message }, 429); + } + + return next(); + }; +} + +/** + * Rate limiter for admin API routes + * 30 requests per minute per IP + */ +export const adminRateLimit = rateLimit({ + maxRequests: 30, + windowSec: 60, + keyFn: (c) => `admin:${c.req.header('CF-Connecting-IP') || 'unknown'}`, + message: 'Too many admin API requests. Please try again later.', +}); + +/** + * Rate limiter for CDP routes + * 100 requests per minute per IP + */ +export const cdpRateLimit = rateLimit({ + maxRequests: 100, + windowSec: 60, + keyFn: (c) => `cdp:${c.req.header('CF-Connecting-IP') || 'unknown'}`, + message: 'Too many CDP requests. Please try again later.', +}); + +/** + * Rate limiter for authentication attempts + * 10 attempts per minute per IP + */ +export const authRateLimit = rateLimit({ + maxRequests: 10, + windowSec: 60, + keyFn: (c) => `auth:${c.req.header('CF-Connecting-IP') || 'unknown'}`, + message: 'Too many authentication attempts. Please try again later.', +}); diff --git a/src/routes/api.ts b/src/routes/api.ts index f11da34db..dcd085e29 100644 --- a/src/routes/api.ts +++ b/src/routes/api.ts @@ -3,6 +3,7 @@ import type { AppEnv } from '../types'; import { createAccessMiddleware } from '../auth'; import { ensureMoltbotGateway, findExistingMoltbotProcess, mountR2Storage, syncToR2, waitForProcess } from '../gateway'; import { R2_MOUNT_PATH } from '../config'; +import { adminRateLimit } from '../middleware'; // CLI commands can take 10-15 seconds to complete due to WebSocket connection overhead const CLI_TIMEOUT_MS = 20000; @@ -23,6 +24,20 @@ const adminApi = new Hono(); // Middleware: Verify Cloudflare Access JWT for all admin routes adminApi.use('*', createAccessMiddleware({ type: 'json' })); +// Middleware: Apply rate limiting to admin routes +adminApi.use('*', adminRateLimit); + +/** + * Sanitize requestId to prevent command injection + * Only allows alphanumeric characters, hyphens, and underscores + */ +function sanitizeRequestId(requestId: string): string | null { + if (!/^[a-zA-Z0-9_-]+$/.test(requestId)) { + return null; + } + return requestId; +} + // GET /api/admin/devices - List pending and paired devices adminApi.get('/devices', async (c) => { const sandbox = c.get('sandbox'); @@ -75,17 +90,32 @@ adminApi.get('/devices', async (c) => { adminApi.post('/devices/:requestId/approve', async (c) => { const sandbox = c.get('sandbox'); const requestId = c.req.param('requestId'); + const accessUser = c.get('accessUser'); + const clientIP = c.req.header('CF-Connecting-IP') || 'unknown'; if (!requestId) { return c.json({ error: 'requestId is required' }, 400); } + // Security: Validate requestId to prevent command injection + const sanitizedRequestId = sanitizeRequestId(requestId); + if (!sanitizedRequestId) { + console.log(JSON.stringify({ + event: 'device_approval_rejected', + reason: 'invalid_request_id', + requestId, + ip: clientIP, + timestamp: new Date().toISOString(), + })); + return c.json({ error: 'Invalid requestId format. Only alphanumeric characters, hyphens, and underscores are allowed.' }, 400); + } + try { // Ensure moltbot is running first await ensureMoltbotGateway(sandbox, c.env); // Run moltbot CLI to approve the device (CLI is still named clawdbot) - const proc = await sandbox.startProcess(`clawdbot devices approve ${requestId} --url ws://localhost:18789`); + const proc = await sandbox.startProcess(`clawdbot devices approve ${sanitizedRequestId} --url ws://localhost:18789`); await waitForProcess(proc, CLI_TIMEOUT_MS); const logs = await proc.getLogs(); @@ -95,9 +125,20 @@ adminApi.post('/devices/:requestId/approve', async (c) => { // Check for success indicators (case-insensitive, CLI outputs "Approved ...") const success = stdout.toLowerCase().includes('approved') || proc.exitCode === 0; + // Audit log for device approval + console.log(JSON.stringify({ + event: 'device_approved', + requestId: sanitizedRequestId, + approvedBy: accessUser?.email || 'unknown', + ip: clientIP, + success, + bulk: false, + timestamp: new Date().toISOString(), + })); + return c.json({ success, - requestId, + requestId: sanitizedRequestId, message: success ? 'Device approved' : 'Approval may have failed', stdout, stderr, @@ -111,6 +152,8 @@ adminApi.post('/devices/:requestId/approve', async (c) => { // POST /api/admin/devices/approve-all - Approve all pending devices adminApi.post('/devices/approve-all', async (c) => { const sandbox = c.get('sandbox'); + const accessUser = c.get('accessUser'); + const clientIP = c.req.header('CF-Connecting-IP') || 'unknown'; try { // Ensure moltbot is running first @@ -143,17 +186,47 @@ adminApi.post('/devices/approve-all', async (c) => { const results: Array<{ requestId: string; success: boolean; error?: string }> = []; for (const device of pending) { + // Security: Validate each requestId to prevent command injection + const sanitizedRequestId = sanitizeRequestId(device.requestId); + if (!sanitizedRequestId) { + console.log(JSON.stringify({ + event: 'device_approval_rejected', + reason: 'invalid_request_id', + requestId: device.requestId, + ip: clientIP, + bulk: true, + timestamp: new Date().toISOString(), + })); + results.push({ + requestId: device.requestId, + success: false, + error: 'Invalid requestId format', + }); + continue; + } + try { - const approveProc = await sandbox.startProcess(`clawdbot devices approve ${device.requestId} --url ws://localhost:18789`); + const approveProc = await sandbox.startProcess(`clawdbot devices approve ${sanitizedRequestId} --url ws://localhost:18789`); await waitForProcess(approveProc, CLI_TIMEOUT_MS); const approveLogs = await approveProc.getLogs(); const success = approveLogs.stdout?.toLowerCase().includes('approved') || approveProc.exitCode === 0; - results.push({ requestId: device.requestId, success }); + // Audit log for each device approval + console.log(JSON.stringify({ + event: 'device_approved', + requestId: sanitizedRequestId, + approvedBy: accessUser?.email || 'unknown', + ip: clientIP, + success, + bulk: true, + timestamp: new Date().toISOString(), + })); + + results.push({ requestId: sanitizedRequestId, success }); } catch (err) { results.push({ - requestId: device.requestId, + requestId: sanitizedRequestId, success: false, error: err instanceof Error ? err.message : 'Unknown error', }); diff --git a/src/routes/cdp.ts b/src/routes/cdp.ts index 1d78e4911..b74007591 100644 --- a/src/routes/cdp.ts +++ b/src/routes/cdp.ts @@ -1,6 +1,7 @@ import { Hono } from 'hono'; import type { AppEnv, MoltbotEnv } from '../types'; import puppeteer, { type Browser, type Page } from '@cloudflare/puppeteer'; +import { cdpRateLimit } from '../middleware'; /** * CDP (Chrome DevTools Protocol) WebSocket shim @@ -8,7 +9,8 @@ import puppeteer, { type Browser, type Page } from '@cloudflare/puppeteer'; * Implements a subset of the CDP protocol over WebSocket, translating commands * to Cloudflare Browser Rendering binding calls (Puppeteer interface). * - * Authentication: Pass secret as query param `?secret=` on WebSocket connect. + * Authentication: Pass secret via Authorization: Bearer header (preferred) or + * as query param `?secret=` on WebSocket connect (fallback for compatibility). * This route is intentionally NOT protected by Cloudflare Access. * * Supported CDP domains: @@ -23,6 +25,24 @@ import puppeteer, { type Browser, type Page } from '@cloudflare/puppeteer'; */ const cdp = new Hono(); +// Apply rate limiting to all CDP routes +cdp.use('*', cdpRateLimit); + +/** + * Extract CDP secret from Authorization header (preferred) or query param (fallback) + * Prefers Authorization header to avoid logging secrets in URLs + */ +function extractCDPSecret(c: { req: { header: (name: string) => string | undefined; url: string } }): string | null { + // Prefer Authorization: Bearer header + const authHeader = c.req.header('Authorization'); + if (authHeader?.startsWith('Bearer ')) { + return authHeader.substring(7); + } + // Fallback to query param for backwards compatibility + const url = new URL(c.req.url); + return url.searchParams.get('secret'); +} + /** * CDP Message types */ @@ -152,9 +172,8 @@ cdp.get('/', async (c) => { }); } - // Verify secret from query param - const url = new URL(c.req.url); - const providedSecret = url.searchParams.get('secret'); + // Verify secret from Authorization header (preferred) or query param (fallback) + const providedSecret = extractCDPSecret(c); const expectedSecret = c.env.CDP_SECRET; if (!expectedSecret) { @@ -201,9 +220,9 @@ cdp.get('/', async (c) => { * Authentication: Pass secret as query param `?secret=` */ cdp.get('/json/version', async (c) => { - // Verify secret from query param + // Verify secret from Authorization header (preferred) or query param (fallback) const url = new URL(c.req.url); - const providedSecret = url.searchParams.get('secret'); + const providedSecret = extractCDPSecret(c); const expectedSecret = c.env.CDP_SECRET; if (!expectedSecret) { @@ -247,9 +266,9 @@ cdp.get('/json/version', async (c) => { * Authentication: Pass secret as query param `?secret=` */ cdp.get('/json/list', async (c) => { - // Verify secret from query param + // Verify secret from Authorization header (preferred) or query param (fallback) const url = new URL(c.req.url); - const providedSecret = url.searchParams.get('secret'); + const providedSecret = extractCDPSecret(c); const expectedSecret = c.env.CDP_SECRET; if (!expectedSecret) { @@ -296,8 +315,8 @@ cdp.get('/json', async (c) => { const url = new URL(c.req.url); url.pathname = url.pathname.replace(/\/json\/?$/, '/json/list'); - // Verify secret from query param - const providedSecret = url.searchParams.get('secret'); + // Verify secret from Authorization header (preferred) or query param (fallback) + const providedSecret = extractCDPSecret(c); const expectedSecret = c.env.CDP_SECRET; if (!expectedSecret) { @@ -1265,6 +1284,23 @@ async function handleDOM( if (!selector) throw new Error(`Node not found: ${nodeId}`); + // Security: Validate file paths to prevent path traversal + const ALLOWED_BASE = '/root/clawd'; + for (const filePath of files) { + // Normalize path to detect traversal attempts + const normalized = filePath.replace(/\/+/g, '/').replace(/\.\./g, ''); + + // Block absolute paths outside allowed base + if (filePath.startsWith('/') && !normalized.startsWith(ALLOWED_BASE)) { + throw new Error(`Path not allowed: ${filePath}. Files must be within ${ALLOWED_BASE}`); + } + + // Block path traversal attempts + if (filePath.includes('..')) { + throw new Error(`Path traversal not allowed: ${filePath}`); + } + } + const element = await page.$(selector); if (element) { // Cast to input element handle for uploadFile @@ -1758,10 +1794,16 @@ async function handleFetch( const request = pending.request as unknown as { respond: (opts: Record) => Promise }; + // Security: Sanitize headers to prevent CRLF injection const headers: Record = {}; if (responseHeaders) { for (const h of responseHeaders) { - headers[h.name] = h.value; + // Remove CR, LF, and null bytes to prevent header injection + const name = String(h.name).replace(/[\r\n\x00]/g, ''); + const value = String(h.value).replace(/[\r\n\x00]/g, ''); + if (name) { + headers[name] = value; + } } } diff --git a/src/routes/debug.ts b/src/routes/debug.ts index 612eb6f55..0a27a87d9 100644 --- a/src/routes/debug.ts +++ b/src/routes/debug.ts @@ -94,13 +94,35 @@ debug.get('/processes', async (c) => { }); // GET /debug/gateway-api - Probe the moltbot gateway HTTP API +// Security: Only allow whitelisted paths to prevent SSRF debug.get('/gateway-api', async (c) => { const sandbox = c.get('sandbox'); const path = c.req.query('path') || '/'; const MOLTBOT_PORT = 18789; + // Whitelist of allowed paths to prevent SSRF + const ALLOWED_PATHS = new Set([ + '/', + '/health', + '/status', + '/version', + '/api/status', + '/api/version', + ]); + + // Normalize path and validate + const normalizedPath = path.replace(/\/+/g, '/').replace(/\/$/, '') || '/'; + + if (!ALLOWED_PATHS.has(normalizedPath)) { + return c.json({ + error: 'Path not allowed', + allowedPaths: Array.from(ALLOWED_PATHS), + provided: normalizedPath, + }, 403); + } + try { - const url = `http://localhost:${MOLTBOT_PORT}${path}`; + const url = `http://localhost:${MOLTBOT_PORT}${normalizedPath}`; const response = await sandbox.containerFetch(new Request(url), MOLTBOT_PORT); const contentType = response.headers.get('content-type') || ''; @@ -112,14 +134,14 @@ debug.get('/gateway-api', async (c) => { } return c.json({ - path, + path: normalizedPath, status: response.status, contentType, body, }); } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; - return c.json({ error: errorMessage, path }, 500); + return c.json({ error: errorMessage, path: normalizedPath }, 500); } }); @@ -204,11 +226,20 @@ debug.get('/logs', async (c) => { }); // GET /debug/ws-test - Interactive WebSocket debug page +// Security: Validate host header and escape output to prevent XSS debug.get('/ws-test', async (c) => { - const host = c.req.header('host') || 'localhost'; + let host = c.req.header('host') || 'localhost'; const protocol = c.req.header('x-forwarded-proto') || 'https'; const wsProtocol = protocol === 'https' ? 'wss' : 'ws'; + // Security: Validate host header to prevent injection + if (!/^[a-zA-Z0-9.-]+(:\d+)?$/.test(host)) { + host = 'localhost'; + } + + // Security: Use JSON.stringify for safe embedding in JavaScript + const safeWsUrl = JSON.stringify(`${wsProtocol}://${host}/`); + const html = ` @@ -241,13 +272,15 @@ debug.get('/ws-test', async (c) => {