From 77190b96e12c54022e91eb3a873698ae902552a3 Mon Sep 17 00:00:00 2001 From: Tamsi Date: Sun, 5 Jul 2026 22:56:43 +0200 Subject: [PATCH] fix(web): prevent workspace path traversal via prefix bypass Replace startsWith(root) checks with path.relative-based validation so sibling directories (e.g. mercury vs mercury-private) cannot be read, written, or used as terminal cwd. Also validate git unstage paths. --- src/utils/path-safety.test.ts | 23 +++++++++++++++++++++++ src/utils/path-safety.ts | 13 +++++++++++++ src/web/api/chat.ts | 5 +++-- src/web/api/workspace-ide.ts | 10 +++++++--- 4 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 src/utils/path-safety.test.ts create mode 100644 src/utils/path-safety.ts diff --git a/src/utils/path-safety.test.ts b/src/utils/path-safety.test.ts new file mode 100644 index 00000000..e0c96eea --- /dev/null +++ b/src/utils/path-safety.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, it } from 'vitest'; +import { join } from 'node:path'; +import { isPathInsideRoot } from './path-safety.js'; + +describe('isPathInsideRoot', () => { + const root = '/home/user/mercury'; + + it('allows the workspace root itself', () => { + expect(isPathInsideRoot(root, root)).toBe(true); + }); + + it('allows paths inside the workspace', () => { + expect(isPathInsideRoot(join(root, 'src/index.ts'), root)).toBe(true); + }); + + it('rejects prefix-bypass paths (mercury vs mercury-private)', () => { + expect(isPathInsideRoot('/home/user/mercury-private/secret.env', root)).toBe(false); + }); + + it('rejects parent traversal', () => { + expect(isPathInsideRoot(join(root, '..', 'outside.txt'), root)).toBe(false); + }); +}); diff --git a/src/utils/path-safety.ts b/src/utils/path-safety.ts new file mode 100644 index 00000000..ef5ce919 --- /dev/null +++ b/src/utils/path-safety.ts @@ -0,0 +1,13 @@ +import { resolve, relative, sep } from 'node:path'; + +/** + * Returns true when `candidate` resolves to `root` or a path strictly inside it. + * Uses path.relative instead of string prefix checks to avoid prefix bypass + * (e.g. /workspace vs /workspace-private). + */ +export function isPathInsideRoot(candidate: string, root: string): boolean { + const resolvedRoot = resolve(root); + const resolved = resolve(candidate); + const rel = relative(resolvedRoot, resolved); + return rel === '' || (!rel.startsWith('..') && !rel.startsWith(`..${sep}`)); +} diff --git a/src/web/api/chat.ts b/src/web/api/chat.ts index a467151b..ab9cf0af 100644 --- a/src/web/api/chat.ts +++ b/src/web/api/chat.ts @@ -4,6 +4,7 @@ import type { ProgrammingMode, ProgrammingModeState } from '../../core/programmi import { existsSync, readFileSync, writeFileSync, readdirSync, statSync } from 'node:fs'; import { join, extname, basename, relative, resolve } from 'node:path'; import { getMercuryHome, loadConfig, getActiveProviders } from '../../utils/config.js'; +import { isPathInsideRoot } from '../../utils/path-safety.js'; import { listThreads, loadThread, deleteThread as removeThread, appendMessage } from './chat-history.js'; let webChannel: WebChannel | null = null; @@ -261,7 +262,7 @@ chat.get('/api/workspace/tree', (c) => { const targetDir = subPath ? resolve(rootDir, subPath) : rootDir; // Security: ensure target is within workspace - if (!targetDir.startsWith(rootDir)) { + if (!isPathInsideRoot(targetDir, rootDir)) { return c.json({ error: 'Path outside workspace' }, 403); } @@ -315,7 +316,7 @@ chat.get('/api/workspace/file', (c) => { const fullPath = resolve(rootDir, filePath); // Security: ensure within workspace - if (!fullPath.startsWith(rootDir)) { + if (!isPathInsideRoot(fullPath, rootDir)) { return c.json({ error: 'Path outside workspace' }, 403); } diff --git a/src/web/api/workspace-ide.ts b/src/web/api/workspace-ide.ts index 6d999d09..8d243806 100644 --- a/src/web/api/workspace-ide.ts +++ b/src/web/api/workspace-ide.ts @@ -3,6 +3,7 @@ import { execSync, spawn } from 'node:child_process'; import { existsSync, readFileSync, writeFileSync, statSync } from 'node:fs'; import { join, resolve, relative, extname, basename } from 'node:path'; import { getMercuryHome, loadConfig } from '../../utils/config.js'; +import { isPathInsideRoot } from '../../utils/path-safety.js'; import { generateText } from 'ai'; import type { ProviderRegistry } from '../../providers/registry.js'; @@ -36,7 +37,7 @@ function git(args: string, cwd?: string): string { function isInsideWorkspace(filePath: string): boolean { const root = getWorkspaceRoot(); const resolved = resolve(root, filePath); - return resolved.startsWith(root); + return isPathInsideRoot(resolved, root); } const ide = new Hono(); @@ -192,6 +193,9 @@ ide.post('/api/git/unstage', async (c) => { const body = await c.req.json<{ files: string[] }>(); if (!body.files?.length) return c.json({ error: 'files array required' }, 400); const cwd = getWorkspaceRoot(); + for (const f of body.files) { + if (!isInsideWorkspace(f)) return c.json({ error: `File outside workspace: ${f}` }, 403); + } git(`reset HEAD ${body.files.map(f => `"${f}"`).join(' ')}`, cwd); return c.json({ success: true }); } catch (err: any) { @@ -250,7 +254,7 @@ ide.put('/api/workspace/file', async (c) => { if (!body.path) return c.json({ error: 'path is required' }, 400); const root = getWorkspaceRoot(); const fullPath = resolve(root, body.path); - if (!fullPath.startsWith(root)) { + if (!isPathInsideRoot(fullPath, root)) { return c.json({ error: 'Path outside workspace' }, 403); } writeFileSync(fullPath, body.content, 'utf8'); @@ -271,7 +275,7 @@ ide.post('/api/terminal/exec', async (c) => { const root = getWorkspaceRoot(); const cwd = body.cwd ? resolve(root, body.cwd) : root; - if (!cwd.startsWith(root)) { + if (!isPathInsideRoot(cwd, root)) { return c.json({ error: 'cwd outside workspace' }, 403); }