diff --git a/packages/desktop/packages/server-core/src/handlers/rpc/workspace.image-path.test.ts b/packages/desktop/packages/server-core/src/handlers/rpc/workspace.image-path.test.ts new file mode 100644 index 00000000000..d7271f6a4e0 --- /dev/null +++ b/packages/desktop/packages/server-core/src/handlers/rpc/workspace.image-path.test.ts @@ -0,0 +1,163 @@ +import { afterEach, beforeEach, describe, expect, it, mock } from 'bun:test' +import { existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { RPC_CHANNELS } from '@craft-agent/shared/protocol' +import type { HandlerFn, RpcServer } from '@craft-agent/server-core/transport' +import type { HandlerDeps } from '../handler-deps' + +let workspaceRoot = '' + +mock.module('@craft-agent/shared/config', () => ({ + getWorkspaceByNameOrId: () => ({ + id: 'workspace-1', + name: 'Workspace', + rootPath: workspaceRoot, + }), + addWorkspace: mock(() => null), + setActiveWorkspace: mock(() => {}), + updateWorkspaceRemoteServer: mock(() => {}), +})) + +const { registerWorkspaceCoreHandlers } = await import('./workspace') + +function createWorkspaceImageHandlers() { + const handlers = new Map() + const server: RpcServer = { + handle(channel, handler) { + handlers.set(channel, handler) + }, + push() {}, + async invokeClient() { + return undefined + }, + } + + const deps: HandlerDeps = { + sessionManager: {} as HandlerDeps['sessionManager'], + oauthFlowStore: {} as HandlerDeps['oauthFlowStore'], + platform: { + appRootPath: '/', + resourcesPath: '/', + isPackaged: false, + appVersion: '0.0.0-test', + isDebugMode: true, + logger: { + info: () => {}, + warn: () => {}, + error: () => {}, + debug: () => {}, + }, + imageProcessor: { + getMetadata: async () => null, + process: async (buffer: Buffer) => buffer, + }, + }, + } + + registerWorkspaceCoreHandlers(server, deps) + + const readImage = handlers.get(RPC_CHANNELS.workspace.READ_IMAGE) + const writeImage = handlers.get(RPC_CHANNELS.workspace.WRITE_IMAGE) + + if (!readImage || !writeImage) { + throw new Error('workspace image handlers not registered') + } + + return { readImage, writeImage } +} + +describe('workspace image path boundaries', () => { + let rootDir: string + let outsideDir: string + + beforeEach(() => { + rootDir = mkdtempSync(join(tmpdir(), 'qwen-workspace-image-')) + workspaceRoot = join(rootDir, 'workspace') + outsideDir = join(rootDir, 'outside') + mkdirSync(workspaceRoot) + mkdirSync(outsideDir) + }) + + afterEach(() => { + rmSync(rootDir, { recursive: true, force: true }) + workspaceRoot = '' + }) + + it('returns null for missing optional images inside the workspace', async () => { + const { readImage } = createWorkspaceImageHandlers() + + await expect(readImage({ clientId: 'c1', workspaceId: null, webContentsId: null }, 'workspace-1', 'missing.svg')).resolves.toBeNull() + }) + + it.skipIf(process.platform === 'win32')('rejects image reads that escape through a symlink', async () => { + const outsideImage = join(outsideDir, 'icon.svg') + writeFileSync(outsideImage, '') + symlinkSync(outsideImage, join(workspaceRoot, 'icon.svg'), 'file') + + const { readImage } = createWorkspaceImageHandlers() + + await expect(readImage({ clientId: 'c1', workspaceId: null, webContentsId: null }, 'workspace-1', 'icon.svg')).rejects.toThrow( + 'outside workspace directory', + ) + }) + + it('rejects image writes that escape through a symlinked parent directory', async () => { + const outsideImage = join(outsideDir, 'icon.svg') + symlinkSync(outsideDir, join(workspaceRoot, 'linked-outside'), process.platform === 'win32' ? 'junction' : 'dir') + + const { writeImage } = createWorkspaceImageHandlers() + + await expect( + writeImage( + { clientId: 'c1', workspaceId: null, webContentsId: null }, + 'workspace-1', + 'linked-outside/icon.svg', + Buffer.from('').toString('base64'), + 'image/svg+xml', + ), + ).rejects.toThrow('outside workspace directory') + + expect(existsSync(outsideImage)).toBe(false) + }) + + it.skipIf(process.platform === 'win32')('rejects image writes through a broken final symlink', async () => { + const outsideImage = join(outsideDir, 'created.svg') + symlinkSync(outsideImage, join(workspaceRoot, 'icon.svg'), 'file') + + const { writeImage } = createWorkspaceImageHandlers() + + await expect( + writeImage( + { clientId: 'c1', workspaceId: null, webContentsId: null }, + 'workspace-1', + 'icon.svg', + Buffer.from('').toString('base64'), + 'image/svg+xml', + ), + ).rejects.toThrow('outside workspace directory') + + expect(existsSync(outsideImage)).toBe(false) + }) + + it('allows overwriting an image when the workspace root is a symlink', async () => { + const linkedWorkspaceRoot = join(rootDir, 'workspace-link') + symlinkSync(workspaceRoot, linkedWorkspaceRoot, process.platform === 'win32' ? 'junction' : 'dir') + workspaceRoot = linkedWorkspaceRoot + + const realImage = join(rootDir, 'workspace', 'icon.svg') + writeFileSync(realImage, 'old') + + const { writeImage } = createWorkspaceImageHandlers() + + await writeImage( + { clientId: 'c1', workspaceId: null, webContentsId: null }, + 'workspace-1', + 'icon.svg', + Buffer.from('new').toString('base64'), + 'image/svg+xml', + ) + + expect(readFileSync(realImage, 'utf8')).toBe('new') + }) +}) diff --git a/packages/desktop/packages/server-core/src/handlers/rpc/workspace.ts b/packages/desktop/packages/server-core/src/handlers/rpc/workspace.ts index 07cf9ebb947..5bd0556f631 100644 --- a/packages/desktop/packages/server-core/src/handlers/rpc/workspace.ts +++ b/packages/desktop/packages/server-core/src/handlers/rpc/workspace.ts @@ -1,8 +1,8 @@ import { execFile } from 'node:child_process' -import { existsSync } from 'node:fs' +import { existsSync, lstatSync, realpathSync } from 'node:fs' import { mkdir, writeFile } from 'node:fs/promises' import { homedir } from 'os' -import { dirname, join } from 'path' +import { dirname, isAbsolute, join, relative, resolve } from 'path' import { promisify } from 'node:util' import { RPC_CHANNELS } from '@craft-agent/shared/protocol' import { getWorkspaceByNameOrId, addWorkspace, setActiveWorkspace, updateWorkspaceRemoteServer } from '@craft-agent/shared/config' @@ -45,6 +45,67 @@ function getWorktreeDirName(branchName: string): string { .replace(/^[.-]+|[.-]+$/g, '') || 'worktree' } +function isPathWithinDirectory(baseDir: string, targetPath: string): boolean { + const relativePath = relative(baseDir, targetPath) + return relativePath === '' || (!relativePath.startsWith('..') && !isAbsolute(relativePath)) +} + +function pathEntryExists(path: string): boolean { + try { + lstatSync(path) + return true + } catch (error) { + const code = error && typeof error === 'object' ? (error as { code?: unknown }).code : undefined + return code !== 'ENOENT' && code !== 'ENOTDIR' + } +} + +function realpathOrNull(path: string): string | null { + try { + return realpathSync.native(path) + } catch { + return null + } +} + +function isExistingWorkspacePath(workspaceRoot: string, targetPath: string): boolean { + const resolvedRoot = resolve(workspaceRoot) + const resolvedTarget = resolve(targetPath) + + if (!isPathWithinDirectory(resolvedRoot, resolvedTarget)) return false + + const realRoot = realpathOrNull(resolvedRoot) + const realTarget = realpathOrNull(resolvedTarget) + if (!realRoot || !realTarget) return false + + return isPathWithinDirectory(realRoot, realTarget) +} + +function isWorkspacePathAllowingMissingTarget(workspaceRoot: string, targetPath: string): boolean { + const resolvedRoot = resolve(workspaceRoot) + const resolvedTarget = resolve(targetPath) + + if (!isPathWithinDirectory(resolvedRoot, resolvedTarget)) return false + + const realRoot = realpathOrNull(resolvedRoot) + if (!realRoot) return false + + if (pathEntryExists(resolvedTarget)) { + return isExistingWorkspacePath(workspaceRoot, resolvedTarget) + } + + let current = dirname(resolvedTarget) + while (true) { + if (pathEntryExists(current)) { + const realCurrent = realpathOrNull(current) + return !!realCurrent && isPathWithinDirectory(realRoot, realCurrent) + } + const parent = dirname(current) + if (parent === current) return false + current = parent + } +} + async function localBranchExists(repoRoot: string, branchName: string): Promise { try { await git(repoRoot, ['show-ref', '--verify', '--quiet', `refs/heads/${branchName}`]) @@ -297,8 +358,8 @@ export function registerWorkspaceCoreHandlers(server: RpcServer, deps: HandlerDe // Resolve path relative to workspace root const absolutePath = normalize(join(workspace.rootPath, relativePath)) - // Double-check the resolved path is still within workspace - if (!absolutePath.startsWith(workspace.rootPath)) { + // Double-check the resolved path is still within workspace, including symlink targets. + if (!isWorkspacePathAllowingMissingTarget(workspace.rootPath, absolutePath)) { throw new Error('Invalid path: outside workspace directory') } @@ -351,8 +412,8 @@ export function registerWorkspaceCoreHandlers(server: RpcServer, deps: HandlerDe // Resolve path relative to workspace root const absolutePath = normalize(join(workspace.rootPath, relativePath)) - // Double-check the resolved path is still within workspace - if (!absolutePath.startsWith(workspace.rootPath)) { + // Double-check the resolved path is still within workspace, including symlink targets. + if (!isWorkspacePathAllowingMissingTarget(workspace.rootPath, absolutePath)) { throw new Error('Invalid path: outside workspace directory') }