From bc49595d0a448f65d43ba1cf5a4439ec7c511317 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Feb 2026 09:40:03 +0000 Subject: [PATCH 1/5] Initial plan From 850335661d559babafee640398aacbe33f301483 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Feb 2026 09:43:31 +0000 Subject: [PATCH 2/5] fix: remove noisy console logs from useBeadsSubscription hook Co-authored-by: zenchantlive <103866469+zenchantlive@users.noreply.github.com> --- package-lock.json | 16 ++++++++++++++++ src/hooks/use-beads-subscription.ts | 11 ++--------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/package-lock.json b/package-lock.json index c2773da..8c25a11 100644 --- a/package-lock.json +++ b/package-lock.json @@ -77,6 +77,7 @@ "integrity": "sha512-CGOfOJqWjg2qW/Mb6zNsDm+u5vFQ8DxXfbM09z69p5Z6+mE1ikP2jUXw+j42Pf1XTYED2Rni5f95npYeuwMDQA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@babel/code-frame": "^7.29.0", "@babel/generator": "^7.29.0", @@ -1734,6 +1735,7 @@ "integrity": "sha512-akea+6bHYBBfA9uQqSYmlJXn61cTa+jbO87xVLCWbTqbWadRVmhxlXATaOjOgcBaWU4ePo0wB41KMFv3o35IXA==", "devOptional": true, "license": "Apache-2.0", + "peer": true, "dependencies": { "playwright": "1.58.2" }, @@ -1864,6 +1866,7 @@ "integrity": "sha512-ilcTH/UniCkMdtexkoCN0bI7pMcJDvmQFPvuPvmEaYA/NSfFTAgdUSLAoVjaRJm7+6PvcM+q1zYOwS4wTYMF9w==", "devOptional": true, "license": "MIT", + "peer": true, "dependencies": { "csstype": "^3.2.2" } @@ -1923,6 +1926,7 @@ "integrity": "sha512-4z2nCSBfVIMnbuu8uinj+f0o4qOeggYJLbjpPHka3KH1om7e+H9yLKTYgksTaHcGco+NClhhY2vyO3HsMH1RGw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.55.0", "@typescript-eslint/types": "8.55.0", @@ -2441,6 +2445,7 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -2860,6 +2865,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "baseline-browser-mapping": "^2.9.0", "caniuse-lite": "^1.0.30001759", @@ -3144,6 +3150,7 @@ "resolved": "https://registry.npmjs.org/d3-selection/-/d3-selection-3.0.0.tgz", "integrity": "sha512-fmTRWbNMmsmWq6xJV8D19U/gw/bwrHfNXxrIN+HfZgnzqTHp9jOmKMhsTUjXOJnZOdZY9Q28y4yebKzqDKlxlQ==", "license": "ISC", + "peer": true, "engines": { "node": ">=12" } @@ -3638,6 +3645,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", @@ -3823,6 +3831,7 @@ "integrity": "sha512-whOE1HFo/qJDyX4SnXzP4N6zOWn79WhnCUY/iDR0mPfQZO8wcYE4JClzI2oZrhBnnMUCBCHZhO6VQyoBU95mZA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@rtsao/scc": "^1.1.0", "array-includes": "^3.1.9", @@ -5085,6 +5094,7 @@ "integrity": "sha512-/imKNG4EbWNrVjoNC/1H5/9GFy+tqjGBHCaSsN+P2RnPqjsLmv6UD3Ej+Kj8nBWaRAwyk7kK5ZUc+OEatnTR3A==", "dev": true, "license": "MIT", + "peer": true, "bin": { "jiti": "bin/jiti.js" } @@ -5875,6 +5885,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "nanoid": "^3.3.11", "picocolors": "^1.1.1", @@ -6033,6 +6044,7 @@ "resolved": "https://registry.npmjs.org/react/-/react-19.2.1.tgz", "integrity": "sha512-DGrYcCWK7tvYMnWh79yrPHt+vdx9tY+1gPZa7nJQtO/p8bLTDaHp4dzwEhQB7pZ4Xe3ok4XKuEPrVuc+wlpkmw==", "license": "MIT", + "peer": true, "engines": { "node": ">=0.10.0" } @@ -6042,6 +6054,7 @@ "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-19.2.1.tgz", "integrity": "sha512-ibrK8llX2a4eOskq1mXKu/TGZj9qzomO+sNfO98M6d9zIPOEhlBkMkBUBLd1vgS0gQsLDBzA+8jJBVXDnfHmJg==", "license": "MIT", + "peer": true, "dependencies": { "scheduler": "^0.27.0" }, @@ -6893,6 +6906,7 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -7082,6 +7096,7 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -7389,6 +7404,7 @@ "integrity": "sha512-rftlrkhHZOcjDwkGlnUtZZkvaPHCsDATp4pGpuOOMDaTdDDXF91wuVDJoWoPsKX/3YPQ5fHuF3STjcYyKr+Qhg==", "dev": true, "license": "MIT", + "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } diff --git a/src/hooks/use-beads-subscription.ts b/src/hooks/use-beads-subscription.ts index eeb4a0c..c2509af 100644 --- a/src/hooks/use-beads-subscription.ts +++ b/src/hooks/use-beads-subscription.ts @@ -65,19 +65,13 @@ export function useBeadsSubscription( }, [projectRoot, onUpdate]); useEffect(() => { - console.log('[SSE] Connecting to event source for:', projectRoot); const source = new EventSource(`/api/events?projectRoot=${encodeURIComponent(projectRoot)}`); - source.onopen = () => { - console.log('[SSE] Connection opened'); - }; - source.onerror = (err) => { console.error('[SSE] Connection error:', err); }; - const onIssues = (event: MessageEvent) => { - console.log('🚨 SSE RECEIVED:', event.data); + const onIssues = () => { onUpdate?.(); void refresh({ silent: true }); }; @@ -85,11 +79,10 @@ export function useBeadsSubscription( source.addEventListener('issues', onIssues as EventListener); return () => { - console.log('[SSE] Closing connection'); source.removeEventListener('issues', onIssues as EventListener); source.close(); }; - }, [projectRoot, refresh]); + }, [projectRoot, refresh, onUpdate]); return { issues, refresh, updateLocal }; } From e46062b4f5f103a6156de7c415eac6c00c630856 Mon Sep 17 00:00:00 2001 From: openhands Date: Sat, 14 Feb 2026 16:36:27 +0000 Subject: [PATCH 3/5] fix: address critical security and stability issues - Fix path traversal vulnerabilities in API route validation functions - Fix path traversal in readiness-report.mjs artifact validation - Add file locking to prevent race conditions in agent-reservations.ts - Fix event ordering in ActivityEventBus by capturing snapshot before modification - Fix memory leaks in watcher.ts by explicitly removing chokidar listeners - Add command injection sanitization in mutations.ts Co-authored-by: openhands --- .../scripts/readiness-report.mjs | 13 +++++- src/app/api/activity/route.ts | 11 ++++- src/app/api/agents/[agentId]/stats/route.ts | 11 ++++- src/app/api/beads/read/route.ts | 11 ++++- src/app/api/sessions/route.ts | 13 ++++-- src/lib/agent-reservations.ts | 40 +++++++++++++++++++ src/lib/mutations.ts | 8 +++- src/lib/realtime.ts | 6 ++- src/lib/watcher.ts | 31 ++++++++++++-- 9 files changed, 130 insertions(+), 14 deletions(-) diff --git a/skills/beadboard-driver/scripts/readiness-report.mjs b/skills/beadboard-driver/scripts/readiness-report.mjs index fb6578b..191c8bf 100644 --- a/skills/beadboard-driver/scripts/readiness-report.mjs +++ b/skills/beadboard-driver/scripts/readiness-report.mjs @@ -1,6 +1,7 @@ #!/usr/bin/env node import fs from 'node:fs/promises'; +import path from 'node:path'; function parseArgs(argv) { const output = {}; @@ -43,8 +44,16 @@ async function withArtifactExistence(artifacts) { }; if (typeof artifact.path === 'string' && artifact.path.trim()) { try { - await fs.access(artifact.path); - item.exists = true; + // Validate path to prevent path traversal attacks + const resolved = path.resolve(artifact.path); + const normalized = path.normalize(resolved); + // Check that the path doesn't contain traversal patterns + if (normalized.includes('..') || path.sep !== '/' && normalized.includes('..\\')) { + item.exists = false; + } else { + await fs.access(resolved); + item.exists = true; + } } catch { item.exists = false; } diff --git a/src/app/api/activity/route.ts b/src/app/api/activity/route.ts index 99531b5..0a80c88 100644 --- a/src/app/api/activity/route.ts +++ b/src/app/api/activity/route.ts @@ -5,7 +5,16 @@ import { activityEventBus } from '../../../lib/realtime'; function isValidProjectRoot(root: string): boolean { try { const resolved = path.resolve(root); - return path.isAbsolute(resolved); + if (!path.isAbsolute(resolved)) { + return false; + } + // Prevent path traversal by ensuring resolved path doesn't escape the project + const normalized = path.normalize(resolved); + // Check that the path doesn't contain traversal patterns + if (normalized.includes('..') || path.sep !== '/' && normalized.includes('..\\')) { + return false; + } + return true; } catch { return false; } diff --git a/src/app/api/agents/[agentId]/stats/route.ts b/src/app/api/agents/[agentId]/stats/route.ts index 007c31b..e090883 100644 --- a/src/app/api/agents/[agentId]/stats/route.ts +++ b/src/app/api/agents/[agentId]/stats/route.ts @@ -7,7 +7,16 @@ import { getAgentMetrics } from '../../../../../lib/agent-sessions'; function isValidProjectRoot(root: string): boolean { try { const resolved = path.resolve(root); - return path.isAbsolute(resolved); + if (!path.isAbsolute(resolved)) { + return false; + } + // Prevent path traversal by ensuring resolved path doesn't escape the project + const normalized = path.normalize(resolved); + // Check that the path doesn't contain traversal patterns + if (normalized.includes('..') || path.sep !== '/' && normalized.includes('..\\')) { + return false; + } + return true; } catch { return false; } diff --git a/src/app/api/beads/read/route.ts b/src/app/api/beads/read/route.ts index db71509..6ab7c31 100644 --- a/src/app/api/beads/read/route.ts +++ b/src/app/api/beads/read/route.ts @@ -5,7 +5,16 @@ import { readIssuesFromDisk } from '../../../../lib/read-issues'; function isValidProjectRoot(root: string): boolean { try { const resolved = path.resolve(root); - return path.isAbsolute(resolved); + if (!path.isAbsolute(resolved)) { + return false; + } + // Prevent path traversal by ensuring resolved path doesn't escape the project + const normalized = path.normalize(resolved); + // Check that the path doesn't contain traversal patterns + if (normalized.includes('..') || path.sep !== '/' && normalized.includes('..\\')) { + return false; + } + return true; } catch { return false; } diff --git a/src/app/api/sessions/route.ts b/src/app/api/sessions/route.ts index bdbc083..60514ac 100644 --- a/src/app/api/sessions/route.ts +++ b/src/app/api/sessions/route.ts @@ -5,11 +5,18 @@ import { activityEventBus } from '../../../lib/realtime'; import { buildSessionTaskFeed, getCommunicationSummary } from '../../../lib/agent-sessions'; function isValidProjectRoot(root: string): boolean { - // Basic validation: path should not contain traversal patterns - // and should resolve to an absolute path try { const resolved = path.resolve(root); - return path.isAbsolute(resolved); + if (!path.isAbsolute(resolved)) { + return false; + } + // Prevent path traversal by ensuring resolved path doesn't escape the project + const normalized = path.normalize(resolved); + // Check that the path doesn't contain traversal patterns + if (normalized.includes('..') || path.sep !== '/' && normalized.includes('..\\')) { + return false; + } + return true; } catch { return false; } diff --git a/src/lib/agent-reservations.ts b/src/lib/agent-reservations.ts index 265c4f9..b0b206d 100644 --- a/src/lib/agent-reservations.ts +++ b/src/lib/agent-reservations.ts @@ -169,6 +169,30 @@ async function readActiveReservations(): Promise { } } +async function lockActiveReservations(): Promise { + // Ensure the directory and file exist before trying to lock + await fs.mkdir(path.dirname(activeReservationsPath()), { recursive: true }); + try { + const fd = await fs.open(activeReservationsPath(), 'r+'); + await fs.flock(fd, 'ex'); + return fd; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + // File doesn't exist, create it first + await fs.writeFile(activeReservationsPath(), JSON.stringify({ reservations: [] }), 'utf8'); + const fd = await fs.open(activeReservationsPath(), 'r+'); + await fs.flock(fd, 'ex'); + return fd; + } + throw error; + } +} + +async function unlockActiveReservations(fd: number): Promise { + await fs.flock(fd, 'un'); + await fs.close(fd); +} + async function atomicWriteJson(filePath: string, payload: string): Promise { await fs.mkdir(path.dirname(filePath), { recursive: true }); @@ -270,7 +294,11 @@ export async function reserveAgentScope( return invalid(command, 'INVALID_ARGS', `TTL must be an integer between ${MIN_TTL_MINUTES} and ${MAX_TTL_MINUTES} minutes.`); } + let lockFd: number | null = null; try { + // Acquire exclusive lock to prevent race conditions + lockFd = await lockActiveReservations(); + const now = deps.now ? deps.now() : new Date().toISOString(); const reservations = await readActiveReservations(); const existing = reservations.find((reservation) => reservation.scope === scope); @@ -320,6 +348,10 @@ export async function reserveAgentScope( return success(command, created); } catch (error) { return invalid(command, 'INTERNAL_ERROR', error instanceof Error ? error.message : 'Failed to reserve scope.'); + } finally { + if (lockFd !== null) { + await unlockActiveReservations(lockFd); + } } } @@ -340,7 +372,11 @@ export async function releaseAgentReservation( return invalid(command, 'INVALID_ARGS', 'Scope is required.'); } + let lockFd: number | null = null; try { + // Acquire exclusive lock to prevent race conditions + lockFd = await lockActiveReservations(); + const now = deps.now ? deps.now() : new Date().toISOString(); const reservations = await readActiveReservations(); const existing = reservations.find((reservation) => reservation.scope === scope); @@ -371,6 +407,10 @@ export async function releaseAgentReservation( return success(command, released); } catch (error) { return invalid(command, 'INTERNAL_ERROR', error instanceof Error ? error.message : 'Failed to release reservation.'); + } finally { + if (lockFd !== null) { + await unlockActiveReservations(lockFd); + } } } diff --git a/src/lib/mutations.ts b/src/lib/mutations.ts index 9b06a94..feedc6a 100644 --- a/src/lib/mutations.ts +++ b/src/lib/mutations.ts @@ -75,7 +75,13 @@ function asNonEmptyString(value: unknown, field: string): string { if (typeof value !== 'string' || !value.trim()) { throw new MutationValidationError(`"${field}" is required.`); } - return value.trim(); + const trimmed = value.trim(); + // Sanitize to prevent command injection - remove control characters and shell metacharacters + const sanitized = trimmed.replace(/[\x00-\x1f\x7f]/g, '').replace(/[;&|`$(){}[\]\\*?<>!#"'%\n\r]/g, ''); + if (!sanitized) { + throw new MutationValidationError(`"${field}" contains only invalid characters.`); + } + return sanitized; } function asOptionalString(value: unknown): string | undefined { diff --git a/src/lib/realtime.ts b/src/lib/realtime.ts index 1637631..e5b6be8 100644 --- a/src/lib/realtime.ts +++ b/src/lib/realtime.ts @@ -117,6 +117,9 @@ export class ActivityEventBus { }; this.nextEventId += 1; + // Capture history snapshot BEFORE modification for persistence + const historySnapshot = [...this.history]; + // Buffer history this.history.unshift(activity); if (this.history.length > this.MAX_HISTORY) { @@ -124,10 +127,9 @@ export class ActivityEventBus { } // Persist async with deduplication - wait for any pending save to complete - const currentHistory = [...this.history]; const persist = async () => { try { - await saveActivityHistory(currentHistory); + await saveActivityHistory(historySnapshot); } catch (error) { console.error('[ActivityEventBus] Failed to save history:', error); } diff --git a/src/lib/watcher.ts b/src/lib/watcher.ts index 553540b..ccaafcf 100644 --- a/src/lib/watcher.ts +++ b/src/lib/watcher.ts @@ -19,6 +19,11 @@ function getGlobalAgentMessagesPath(): string { interface WatchRegistration { projectRoot: string; watcher: FSWatcher; + handlers?: { + onAdd: (changedPath: string) => void; + onChange: (changedPath: string) => void; + onUnlink: (changedPath: string) => void; + }; } export interface WatchManagerOptions { @@ -119,13 +124,19 @@ export class IssuesWatchManager { this.queueCoalescedEvent(projectRoot, changedPath, kind); }; - watcher.on('add', (changedPath) => onFileEvent('add', changedPath)); - watcher.on('change', (changedPath) => onFileEvent('change', changedPath)); - watcher.on('unlink', (changedPath) => onFileEvent('unlink', changedPath)); + // Store references to event handlers for proper cleanup + const onAdd = (changedPath: string) => onFileEvent('add', changedPath); + const onChange = (changedPath: string) => onFileEvent('change', changedPath); + const onUnlink = (changedPath: string) => onFileEvent('unlink', changedPath); + + watcher.on('add', onAdd); + watcher.on('change', onChange); + watcher.on('unlink', onUnlink); this.registrations.set(projectKey, { projectRoot, watcher, + handlers: { onAdd, onChange, onUnlink }, }); } @@ -137,6 +148,14 @@ export class IssuesWatchManager { } this.coalescer.cancel(projectRoot); + + // Explicitly remove event listeners before closing to prevent memory leaks + if (registration.handlers) { + registration.watcher.removeListener('add', registration.handlers.onAdd); + registration.watcher.removeListener('change', registration.handlers.onChange); + registration.watcher.removeListener('unlink', registration.handlers.onUnlink); + } + this.registrations.delete(projectKey); await registration.watcher.close(); } @@ -145,6 +164,12 @@ export class IssuesWatchManager { const closeOps: Promise[] = []; for (const registration of this.registrations.values()) { + // Explicitly remove event listeners before closing to prevent memory leaks + if (registration.handlers) { + registration.watcher.removeListener('add', registration.handlers.onAdd); + registration.watcher.removeListener('change', registration.handlers.onChange); + registration.watcher.removeListener('unlink', registration.handlers.onUnlink); + } closeOps.push(registration.watcher.close()); } From 710556aa45750e6204360c804431c3992df34052 Mon Sep 17 00:00:00 2001 From: openhands Date: Sat, 14 Feb 2026 17:17:00 +0000 Subject: [PATCH 4/5] fix: replace non-standard flock() with portable file-based mutex The original implementation used fs.flock() which is not available in the Node.js fs/promises API. Replaced with a portable file-based mutex using exclusive file creation (flag: 'wx') with retry logic. This ensures the race condition fix for agent reservations works correctly across all Node.js versions and platforms. --- package-lock.json | 16 -------- src/lib/agent-reservations.ts | 69 +++++++++++++++++++++-------------- 2 files changed, 41 insertions(+), 44 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8c25a11..c2773da 100644 --- a/package-lock.json +++ b/package-lock.json @@ -77,7 +77,6 @@ "integrity": "sha512-CGOfOJqWjg2qW/Mb6zNsDm+u5vFQ8DxXfbM09z69p5Z6+mE1ikP2jUXw+j42Pf1XTYED2Rni5f95npYeuwMDQA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@babel/code-frame": "^7.29.0", "@babel/generator": "^7.29.0", @@ -1735,7 +1734,6 @@ "integrity": "sha512-akea+6bHYBBfA9uQqSYmlJXn61cTa+jbO87xVLCWbTqbWadRVmhxlXATaOjOgcBaWU4ePo0wB41KMFv3o35IXA==", "devOptional": true, "license": "Apache-2.0", - "peer": true, "dependencies": { "playwright": "1.58.2" }, @@ -1866,7 +1864,6 @@ "integrity": "sha512-ilcTH/UniCkMdtexkoCN0bI7pMcJDvmQFPvuPvmEaYA/NSfFTAgdUSLAoVjaRJm7+6PvcM+q1zYOwS4wTYMF9w==", "devOptional": true, "license": "MIT", - "peer": true, "dependencies": { "csstype": "^3.2.2" } @@ -1926,7 +1923,6 @@ "integrity": "sha512-4z2nCSBfVIMnbuu8uinj+f0o4qOeggYJLbjpPHka3KH1om7e+H9yLKTYgksTaHcGco+NClhhY2vyO3HsMH1RGw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.55.0", "@typescript-eslint/types": "8.55.0", @@ -2445,7 +2441,6 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -2865,7 +2860,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "baseline-browser-mapping": "^2.9.0", "caniuse-lite": "^1.0.30001759", @@ -3150,7 +3144,6 @@ "resolved": "https://registry.npmjs.org/d3-selection/-/d3-selection-3.0.0.tgz", "integrity": "sha512-fmTRWbNMmsmWq6xJV8D19U/gw/bwrHfNXxrIN+HfZgnzqTHp9jOmKMhsTUjXOJnZOdZY9Q28y4yebKzqDKlxlQ==", "license": "ISC", - "peer": true, "engines": { "node": ">=12" } @@ -3645,7 +3638,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", @@ -3831,7 +3823,6 @@ "integrity": "sha512-whOE1HFo/qJDyX4SnXzP4N6zOWn79WhnCUY/iDR0mPfQZO8wcYE4JClzI2oZrhBnnMUCBCHZhO6VQyoBU95mZA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@rtsao/scc": "^1.1.0", "array-includes": "^3.1.9", @@ -5094,7 +5085,6 @@ "integrity": "sha512-/imKNG4EbWNrVjoNC/1H5/9GFy+tqjGBHCaSsN+P2RnPqjsLmv6UD3Ej+Kj8nBWaRAwyk7kK5ZUc+OEatnTR3A==", "dev": true, "license": "MIT", - "peer": true, "bin": { "jiti": "bin/jiti.js" } @@ -5885,7 +5875,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "nanoid": "^3.3.11", "picocolors": "^1.1.1", @@ -6044,7 +6033,6 @@ "resolved": "https://registry.npmjs.org/react/-/react-19.2.1.tgz", "integrity": "sha512-DGrYcCWK7tvYMnWh79yrPHt+vdx9tY+1gPZa7nJQtO/p8bLTDaHp4dzwEhQB7pZ4Xe3ok4XKuEPrVuc+wlpkmw==", "license": "MIT", - "peer": true, "engines": { "node": ">=0.10.0" } @@ -6054,7 +6042,6 @@ "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-19.2.1.tgz", "integrity": "sha512-ibrK8llX2a4eOskq1mXKu/TGZj9qzomO+sNfO98M6d9zIPOEhlBkMkBUBLd1vgS0gQsLDBzA+8jJBVXDnfHmJg==", "license": "MIT", - "peer": true, "dependencies": { "scheduler": "^0.27.0" }, @@ -6906,7 +6893,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -7096,7 +7082,6 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", - "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -7404,7 +7389,6 @@ "integrity": "sha512-rftlrkhHZOcjDwkGlnUtZZkvaPHCsDATp4pGpuOOMDaTdDDXF91wuVDJoWoPsKX/3YPQ5fHuF3STjcYyKr+Qhg==", "dev": true, "license": "MIT", - "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } diff --git a/src/lib/agent-reservations.ts b/src/lib/agent-reservations.ts index b0b206d..6cb1ca5 100644 --- a/src/lib/agent-reservations.ts +++ b/src/lib/agent-reservations.ts @@ -169,28 +169,47 @@ async function readActiveReservations(): Promise { } } -async function lockActiveReservations(): Promise { - // Ensure the directory and file exist before trying to lock - await fs.mkdir(path.dirname(activeReservationsPath()), { recursive: true }); - try { - const fd = await fs.open(activeReservationsPath(), 'r+'); - await fs.flock(fd, 'ex'); - return fd; - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') { - // File doesn't exist, create it first - await fs.writeFile(activeReservationsPath(), JSON.stringify({ reservations: [] }), 'utf8'); - const fd = await fs.open(activeReservationsPath(), 'r+'); - await fs.flock(fd, 'ex'); - return fd; +// Simple mutex-based locking using a shared lock file to prevent race conditions +const LOCK_FILE_PATH = path.join(reservationsRoot(), '.lock'); + +async function lockActiveReservations(): Promise { + // Ensure the directory exists + await fs.mkdir(path.dirname(LOCK_FILE_PATH), { recursive: true }); + + // Use a simple file-based mutex - create file exclusively, fail if exists + let attempts = 0; + const maxAttempts = 100; + + while (attempts < maxAttempts) { + try { + await fs.writeFile(LOCK_FILE_PATH, String(process.pid), { flag: 'wx' }); + return; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'EEXIST') { + // Lock file exists, wait and retry + await new Promise(resolve => setTimeout(resolve, 50)); + attempts++; + continue; + } + throw error; } - throw error; } + throw new Error('Failed to acquire lock after maximum attempts'); } -async function unlockActiveReservations(fd: number): Promise { - await fs.flock(fd, 'un'); - await fs.close(fd); +async function unlockActiveReservations(): Promise { + try { + const content = await fs.readFile(LOCK_FILE_PATH, 'utf8'); + // Only release if we own the lock + if (content.trim() === String(process.pid)) { + await fs.unlink(LOCK_FILE_PATH); + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + throw error; + } + // Lock file doesn't exist, ignore + } } async function atomicWriteJson(filePath: string, payload: string): Promise { @@ -294,10 +313,9 @@ export async function reserveAgentScope( return invalid(command, 'INVALID_ARGS', `TTL must be an integer between ${MIN_TTL_MINUTES} and ${MAX_TTL_MINUTES} minutes.`); } - let lockFd: number | null = null; try { // Acquire exclusive lock to prevent race conditions - lockFd = await lockActiveReservations(); + await lockActiveReservations(); const now = deps.now ? deps.now() : new Date().toISOString(); const reservations = await readActiveReservations(); @@ -349,9 +367,7 @@ export async function reserveAgentScope( } catch (error) { return invalid(command, 'INTERNAL_ERROR', error instanceof Error ? error.message : 'Failed to reserve scope.'); } finally { - if (lockFd !== null) { - await unlockActiveReservations(lockFd); - } + await unlockActiveReservations(); } } @@ -372,10 +388,9 @@ export async function releaseAgentReservation( return invalid(command, 'INVALID_ARGS', 'Scope is required.'); } - let lockFd: number | null = null; try { // Acquire exclusive lock to prevent race conditions - lockFd = await lockActiveReservations(); + await lockActiveReservations(); const now = deps.now ? deps.now() : new Date().toISOString(); const reservations = await readActiveReservations(); @@ -408,9 +423,7 @@ export async function releaseAgentReservation( } catch (error) { return invalid(command, 'INTERNAL_ERROR', error instanceof Error ? error.message : 'Failed to release reservation.'); } finally { - if (lockFd !== null) { - await unlockActiveReservations(lockFd); - } + await unlockActiveReservations(); } } From 05357580aed6048f2856816e8b1e8ef2a0f1f295 Mon Sep 17 00:00:00 2001 From: openhands Date: Sat, 14 Feb 2026 17:57:12 +0000 Subject: [PATCH 5/5] Fix path traversal validation and mutation sanitization - Fix isValidProjectRoot() in 4 API routes to properly prevent path traversal by using path.relative() to ensure paths stay within allowed base directory (replaces ineffective normalized.includes('..') check) - Fix readiness-report.mjs to remove misleading path traversal validation that was ineffective after path.resolve() removes '..' segments - Fix asNonEmptyString() in mutations.ts to only remove control characters while preserving backslashes (for Windows paths) and punctuation (for user text) These changes address security review comments about ineffective path traversal checks and mutation input corruption. --- skills/beadboard-driver/scripts/readiness-report.mjs | 11 ++--------- src/app/api/activity/route.ts | 9 +++++---- src/app/api/agents/[agentId]/stats/route.ts | 9 +++++---- src/app/api/beads/read/route.ts | 9 +++++---- src/app/api/sessions/route.ts | 9 +++++---- src/lib/mutations.ts | 5 +++-- 6 files changed, 25 insertions(+), 27 deletions(-) diff --git a/skills/beadboard-driver/scripts/readiness-report.mjs b/skills/beadboard-driver/scripts/readiness-report.mjs index 191c8bf..2ecd7ce 100644 --- a/skills/beadboard-driver/scripts/readiness-report.mjs +++ b/skills/beadboard-driver/scripts/readiness-report.mjs @@ -44,16 +44,9 @@ async function withArtifactExistence(artifacts) { }; if (typeof artifact.path === 'string' && artifact.path.trim()) { try { - // Validate path to prevent path traversal attacks const resolved = path.resolve(artifact.path); - const normalized = path.normalize(resolved); - // Check that the path doesn't contain traversal patterns - if (normalized.includes('..') || path.sep !== '/' && normalized.includes('..\\')) { - item.exists = false; - } else { - await fs.access(resolved); - item.exists = true; - } + await fs.access(resolved); + item.exists = true; } catch { item.exists = false; } diff --git a/src/app/api/activity/route.ts b/src/app/api/activity/route.ts index 0a80c88..0e5a0f9 100644 --- a/src/app/api/activity/route.ts +++ b/src/app/api/activity/route.ts @@ -8,10 +8,11 @@ function isValidProjectRoot(root: string): boolean { if (!path.isAbsolute(resolved)) { return false; } - // Prevent path traversal by ensuring resolved path doesn't escape the project - const normalized = path.normalize(resolved); - // Check that the path doesn't contain traversal patterns - if (normalized.includes('..') || path.sep !== '/' && normalized.includes('..\\')) { + // Prevent path traversal by ensuring resolved path stays within the project root + const allowedBase = process.cwd(); + const relative = path.relative(allowedBase, resolved); + // If "resolved" is outside "allowedBase", "relative" will start with ".." + if (relative.startsWith('..') || path.isAbsolute(relative)) { return false; } return true; diff --git a/src/app/api/agents/[agentId]/stats/route.ts b/src/app/api/agents/[agentId]/stats/route.ts index e090883..7db8999 100644 --- a/src/app/api/agents/[agentId]/stats/route.ts +++ b/src/app/api/agents/[agentId]/stats/route.ts @@ -10,10 +10,11 @@ function isValidProjectRoot(root: string): boolean { if (!path.isAbsolute(resolved)) { return false; } - // Prevent path traversal by ensuring resolved path doesn't escape the project - const normalized = path.normalize(resolved); - // Check that the path doesn't contain traversal patterns - if (normalized.includes('..') || path.sep !== '/' && normalized.includes('..\\')) { + // Prevent path traversal by ensuring resolved path stays within the project root + const allowedBase = process.cwd(); + const relative = path.relative(allowedBase, resolved); + // If "resolved" is outside "allowedBase", "relative" will start with ".." + if (relative.startsWith('..') || path.isAbsolute(relative)) { return false; } return true; diff --git a/src/app/api/beads/read/route.ts b/src/app/api/beads/read/route.ts index 6ab7c31..8158396 100644 --- a/src/app/api/beads/read/route.ts +++ b/src/app/api/beads/read/route.ts @@ -8,10 +8,11 @@ function isValidProjectRoot(root: string): boolean { if (!path.isAbsolute(resolved)) { return false; } - // Prevent path traversal by ensuring resolved path doesn't escape the project - const normalized = path.normalize(resolved); - // Check that the path doesn't contain traversal patterns - if (normalized.includes('..') || path.sep !== '/' && normalized.includes('..\\')) { + // Prevent path traversal by ensuring resolved path stays within the project root + const allowedBase = process.cwd(); + const relative = path.relative(allowedBase, resolved); + // If "resolved" is outside "allowedBase", "relative" will start with ".." + if (relative.startsWith('..') || path.isAbsolute(relative)) { return false; } return true; diff --git a/src/app/api/sessions/route.ts b/src/app/api/sessions/route.ts index 60514ac..280eff5 100644 --- a/src/app/api/sessions/route.ts +++ b/src/app/api/sessions/route.ts @@ -10,10 +10,11 @@ function isValidProjectRoot(root: string): boolean { if (!path.isAbsolute(resolved)) { return false; } - // Prevent path traversal by ensuring resolved path doesn't escape the project - const normalized = path.normalize(resolved); - // Check that the path doesn't contain traversal patterns - if (normalized.includes('..') || path.sep !== '/' && normalized.includes('..\\')) { + // Prevent path traversal by ensuring resolved path stays within the project root + const allowedBase = process.cwd(); + const relative = path.relative(allowedBase, resolved); + // If "resolved" is outside "allowedBase", "relative" will start with ".." + if (relative.startsWith('..') || path.isAbsolute(relative)) { return false; } return true; diff --git a/src/lib/mutations.ts b/src/lib/mutations.ts index feedc6a..e94eae1 100644 --- a/src/lib/mutations.ts +++ b/src/lib/mutations.ts @@ -76,8 +76,9 @@ function asNonEmptyString(value: unknown, field: string): string { throw new MutationValidationError(`"${field}" is required.`); } const trimmed = value.trim(); - // Sanitize to prevent command injection - remove control characters and shell metacharacters - const sanitized = trimmed.replace(/[\x00-\x1f\x7f]/g, '').replace(/[;&|`$(){}[\]\\*?<>!#"'%\n\r]/g, ''); + // Remove control characters that could cause issues in command execution + // Preserve backslashes for Windows paths and punctuation for user text + const sanitized = trimmed.replace(/[\x00-\x1f\x7f]/g, ''); if (!sanitized) { throw new MutationValidationError(`"${field}" contains only invalid characters.`); }