From a65a328d903a8f0dae55bcbd1cd86d6fa64beac8 Mon Sep 17 00:00:00 2001 From: OrcaWin <293788423+OrcaWin@users.noreply.github.com> Date: Fri, 7 Aug 2026 17:12:50 -0700 Subject: [PATCH] fix(ai-vault): contain WSL session deletion --- src/main/ai-vault/session-delete.test.ts | 47 ++++++- src/main/ai-vault/session-delete.ts | 23 +++- src/main/wsl-contained-delete.ts | 130 ++++++++++++++++++ src/main/wsl-unc-delete-symlink-repro.test.ts | 79 +++++++++++ src/main/wsl-unc-delete.test.ts | 63 ++++++++- src/main/wsl-unc-delete.ts | 49 +++++-- src/main/wsl-unc-delete.wsl.test.ts | 82 +++++++++++ 7 files changed, 449 insertions(+), 24 deletions(-) create mode 100644 src/main/wsl-contained-delete.ts create mode 100644 src/main/wsl-unc-delete-symlink-repro.test.ts create mode 100644 src/main/wsl-unc-delete.wsl.test.ts diff --git a/src/main/ai-vault/session-delete.test.ts b/src/main/ai-vault/session-delete.test.ts index eb25b11a820..cacb0dbbda5 100644 --- a/src/main/ai-vault/session-delete.test.ts +++ b/src/main/ai-vault/session-delete.test.ts @@ -1,11 +1,22 @@ -import { join } from 'node:path' +import { join, resolve } from 'node:path' import { describe, expect, it, vi, beforeEach } from 'vitest' -const { lstatMock, realpathMock, trashItemMock, tryDeleteWslUncPathMock } = vi.hoisted(() => ({ +const { + lstatMock, + realpathMock, + trashItemMock, + tryDeleteWslUncPathMock, + WslDeleteValidationErrorMock +} = vi.hoisted(() => ({ lstatMock: vi.fn(), realpathMock: vi.fn(), trashItemMock: vi.fn(), - tryDeleteWslUncPathMock: vi.fn() + tryDeleteWslUncPathMock: vi.fn(), + WslDeleteValidationErrorMock: class extends Error { + constructor(readonly reason: 'path-outside-known-roots' | 'unexpected-target-kind') { + super(reason) + } + } })) vi.mock('node:fs/promises', () => ({ @@ -18,7 +29,8 @@ vi.mock('electron', () => ({ })) vi.mock('../wsl-unc-delete', () => ({ - tryDeleteWslUncPath: tryDeleteWslUncPathMock + tryDeleteWslUncPath: tryDeleteWslUncPathMock, + WslDeleteValidationError: WslDeleteValidationErrorMock })) import { deleteAiVaultSessionFile } from './session-delete' @@ -181,7 +193,10 @@ describe('deleteAiVaultSessionFile', () => { const result = await deleteAiVaultSessionFile(baseArgs(filePath)) expect(result).toEqual({ outcome: 'deleted' }) - expect(tryDeleteWslUncPathMock).toHaveBeenCalledWith(filePath, { recursive: false }) + expect(tryDeleteWslUncPathMock).toHaveBeenCalledWith(resolve(filePath), { + recursive: false, + approvedRoots: [resolve(GEMINI_ROOT)] + }) expect(lstatMock).not.toHaveBeenCalled() expect(trashItemMock).not.toHaveBeenCalled() }) @@ -201,8 +216,26 @@ describe('deleteAiVaultSessionFile', () => { }) expect(result).toEqual({ outcome: 'deleted' }) - expect(tryDeleteWslUncPathMock).toHaveBeenCalledWith(join(ROVO_ROOT, 'sess-1'), { - recursive: true + expect(tryDeleteWslUncPathMock).toHaveBeenCalledWith(resolve(ROVO_ROOT, 'sess-1'), { + recursive: true, + approvedRoots: [resolve(ROVO_ROOT)] + }) + expect(lstatMock).not.toHaveBeenCalled() + expect(trashItemMock).not.toHaveBeenCalled() + }) + + it('maps a WSL containment rejection to a delete rejection', async () => { + const filePath = join(GEMINI_ROOT, 'project-a', 'session-1.json') + tryDeleteWslUncPathMock.mockRejectedValue( + new WslDeleteValidationErrorMock('path-outside-known-roots') + ) + + const result = await deleteAiVaultSessionFile(baseArgs(filePath)) + + expect(result).toEqual({ + outcome: 'rejected', + agent: 'gemini', + reason: 'path-outside-known-roots' }) expect(lstatMock).not.toHaveBeenCalled() expect(trashItemMock).not.toHaveBeenCalled() diff --git a/src/main/ai-vault/session-delete.ts b/src/main/ai-vault/session-delete.ts index d52725ff0fb..d0ba15e8756 100644 --- a/src/main/ai-vault/session-delete.ts +++ b/src/main/ai-vault/session-delete.ts @@ -9,7 +9,7 @@ import { validateAiVaultSessionDeleteTarget, type ValidateAiVaultSessionDeleteTargetArgs } from './session-delete-target' -import { tryDeleteWslUncPath } from '../wsl-unc-delete' +import { tryDeleteWslUncPath, WslDeleteValidationError } from '../wsl-unc-delete' import { isENOENT } from '../ipc/filesystem-auth' // Trashes a validated session's paths, companions first so a part-way failure @@ -48,11 +48,22 @@ export async function deleteAiVaultSessionFile( async function removeOne( removal: AiVaultSessionDeleteRemoval ): Promise<'unexpected-target-kind' | 'path-outside-known-roots' | null> { - // First, because a WSL UNC path defeats both guards below: 9P's stat is - // unreliable and the volume has no Recycle Bin for trashItem. Its own - // `rm -f`/`-rf` is idempotent and never follows a symlink. - if (await tryDeleteWslUncPath(removal.path, { recursive: removal.kind === 'directory' })) { - return null + // WSL validates and removes inside the distro because 9P stat is unreliable + // and its virtual volume has no Recycle Bin for trashItem. + try { + if ( + await tryDeleteWslUncPath(removal.path, { + recursive: removal.kind === 'directory', + approvedRoots: removal.roots + }) + ) { + return null + } + } catch (error) { + if (error instanceof WslDeleteValidationError) { + return error.reason + } + throw error } let stats diff --git a/src/main/wsl-contained-delete.ts b/src/main/wsl-contained-delete.ts new file mode 100644 index 00000000000..248cc52b287 --- /dev/null +++ b/src/main/wsl-contained-delete.ts @@ -0,0 +1,130 @@ +import { posix } from 'node:path' +import type { WslPathInfo } from './wsl' + +const REJECTION_PREFIX = 'ORCA_WSL_DELETE_REJECT:' + +export type WslDeleteRejection = 'path-outside-known-roots' | 'unexpected-target-kind' + +export const WSL_CONTAINED_DELETE_SCRIPT = String.raw` +reject() { printf '${REJECTION_PREFIX}%s\n' "$1" >&2; exit 65; } +missing_entry() { + command ls -A -- . >/dev/null 2>&1 || reject inspection + for entry in ./* ./.[!.]* ./..?*; do + [ "$entry" = "./$1" ] && return 1 + done + return 0 +} +inspect() { + inspected=$(stat -c '%d:%i:%F' -- "$1" 2>/dev/null) && return 0 + missing_entry "$2" && exit 0 + reject inspection +} + +expected_kind=$1 +root_count=$2 +shift 2 +cd -P -- / || reject inspection + +while [ "$root_count" -gt 0 ]; do + component=$1 + shift + root_count=$((root_count - 1)) + inspect "$component" "$component" + entry_kind=$(printf '%s' "$inspected" | cut -d: -f3-) + case "$entry_kind" in directory|'symbolic link') ;; *) reject inspection ;; esac + cd -P -- "$component" || reject inspection +done + +root_physical=$(pwd -P) || reject inspection +[ "$root_physical" != / ] || reject containment +exec 3< . || reject inspection + +while [ "$#" -gt 1 ]; do + component=$1 + shift + inspected=$(stat -c '%d:%i:%F' -- "/proc/self/fd/3/$component" 2>/dev/null) || { + missing_entry "$component" && exit 0 + reject inspection + } + [ "$(printf '%s' "$inspected" | cut -d: -f3-)" = directory ] || reject symlink + expected_id=$(printf '%s' "$inspected" | cut -d: -f1-2) + cd -P -- "/proc/self/fd/3/$component" || reject inspection + actual_id=$(stat -Lc '%d:%i' -- . 2>/dev/null) || reject inspection + [ "$actual_id" = "$expected_id" ] || reject race + current_physical=$(pwd -P) || reject inspection + case "$current_physical/" in "$root_physical/"*) ;; *) reject containment ;; esac + exec 3<&- + exec 3< . || reject inspection +done + +[ "$#" -eq 1 ] || reject containment +leaf=$1 +leaf_path=/proc/self/fd/3/$leaf +inspected=$(stat -c '%d:%i:%F' -- "$leaf_path" 2>/dev/null) || { + missing_entry "$leaf" && exit 0 + reject inspection +} +actual_kind=$(printf '%s' "$inspected" | cut -d: -f3-) +[ "$actual_kind" = "$expected_kind" ] || reject kind + +if [ "$expected_kind" = directory ]; then + rm -rf -- "$leaf_path" +else + rm -f -- "$leaf_path" +fi +` + +export function containedDeleteCommand( + target: WslPathInfo, + approvedRoots: readonly string[], + parsePath: (path: string) => WslPathInfo | null, + recursive: boolean +): string[] | null { + const targetPath = posix.resolve(target.linuxPath) + const matchedRoot = approvedRoots + .map(parsePath) + .filter((root): root is WslPathInfo => root?.distro === target.distro) + .map((root) => posix.resolve(root.linuxPath)) + .filter((root) => isInside(root, targetPath) && root !== '/') + .sort((left, right) => right.length - left.length)[0] + if (!matchedRoot) { + return null + } + + const relativeParts = posix.relative(matchedRoot, targetPath).split('/').filter(Boolean) + if (relativeParts.length === 0) { + return null + } + const rootParts = matchedRoot.split('/').filter(Boolean) + return [ + 'sh', + '-c', + WSL_CONTAINED_DELETE_SCRIPT, + 'orca-wsl-delete', + recursive ? 'directory' : 'regular file', + String(rootParts.length), + ...rootParts, + ...relativeParts + ] +} + +export function rejectionFromWslDeleteStderr(stderr: string): WslDeleteRejection | null { + const marker = stderr + .split(/\r?\n/u) + .find((line) => line.startsWith(REJECTION_PREFIX)) + ?.slice(REJECTION_PREFIX.length) + if (!marker) { + return null + } + return marker === 'kind' ? 'unexpected-target-kind' : 'path-outside-known-roots' +} + +function isInside(root: string, target: string): boolean { + const relative = posix.relative(root, target) + return ( + relative !== '' && + relative !== '..' && + !relative.startsWith('../') && + !posix.isAbsolute(relative) + ) +} diff --git a/src/main/wsl-unc-delete-symlink-repro.test.ts b/src/main/wsl-unc-delete-symlink-repro.test.ts new file mode 100644 index 00000000000..ace95afe933 --- /dev/null +++ b/src/main/wsl-unc-delete-symlink-repro.test.ts @@ -0,0 +1,79 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { execFileMock } = vi.hoisted(() => ({ execFileMock: vi.fn() })) + +vi.mock('node:child_process', () => ({ execFile: execFileMock })) + +import { tryDeleteWslUncPath } from './wsl-unc-delete' + +const DISTRO = 'Ubuntu' +const FIXTURE_ROOT = '/tmp/orca-wsl-vault-delete-repro/vault' +const OUTSIDE_ROOT = '/tmp/orca-wsl-vault-delete-repro/outside' +const SENTINEL = `${OUTSIDE_ROOT}/unrelated-sentinel` +const LINK = `${FIXTURE_ROOT}/linked-project` + +function unc(linuxPath: string): string { + return `\\\\wsl.localhost\\${DISTRO}${linuxPath.replaceAll('/', '\\')}` +} + +function withWindows(run: () => Promise): Promise { + const original = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + return run().finally(() => { + if (original) { + Object.defineProperty(process, 'platform', original) + } + }) +} + +describe('WSL vault intermediate-symlink reproduction', () => { + let removalReached: boolean + let virtualEntries: Set + + beforeEach(() => { + removalReached = false + virtualEntries = new Set([ + `${OUTSIDE_ROOT}/session.json`, + `${OUTSIDE_ROOT}/session/inside`, + SENTINEL + ]) + execFileMock.mockReset() + execFileMock.mockImplementation((_binary, args: string[], _options, callback) => { + const separator = args.findIndex((arg) => arg === '--' || arg === '--exec') + const command = args.slice(separator + 1) + if (command[0] === 'rm') { + removalReached = true + callback(null, '', '') + return + } + + const error = Object.assign(new Error('Command failed'), { code: 65 }) + callback(error, '', 'Refusing WSL delete: symbolic-link path component') + }) + }) + + it.each([ + ['file-shaped', `${FIXTURE_ROOT}/linked-project/session.json`, false], + ['directory-shaped', `${FIXTURE_ROOT}/linked-project/session`, true] + ])('rejects a %s target before removal', async (_shape, target, recursive) => { + const options = { recursive, approvedRoots: [unc(FIXTURE_ROOT)] } + let rejection: unknown + + await withWindows(async () => { + try { + await tryDeleteWslUncPath(unc(target), options) + } catch (error) { + rejection = error + } + }) + + expect(removalReached).toBe(false) + expect(rejection).toEqual( + expect.objectContaining({ message: expect.stringContaining('symbolic-link') }) + ) + expect(LINK).toBe(`${FIXTURE_ROOT}/linked-project`) + expect(virtualEntries).toEqual( + new Set([`${OUTSIDE_ROOT}/session.json`, `${OUTSIDE_ROOT}/session/inside`, SENTINEL]) + ) + }) +}) diff --git a/src/main/wsl-unc-delete.test.ts b/src/main/wsl-unc-delete.test.ts index 932924b765d..4ca6c43312c 100644 --- a/src/main/wsl-unc-delete.test.ts +++ b/src/main/wsl-unc-delete.test.ts @@ -52,7 +52,15 @@ describe('tryDeleteWslUncPath', () => { expect(execFileMock).toHaveBeenCalledTimes(1) const [binary, spawnArgs] = execFileMock.mock.calls[0] expect(binary).toBe('wsl.exe') - expect(spawnArgs).toEqual(['-d', 'Ubuntu', '--', 'rm', '-f', '--', '/home/me/repo/file.txt']) + expect(spawnArgs).toEqual([ + '-d', + 'Ubuntu', + '--exec', + 'rm', + '-f', + '--', + '/home/me/repo/file.txt' + ]) }) it('passes -rf for a recursive directory delete', async () => { @@ -63,7 +71,56 @@ describe('tryDeleteWslUncPath', () => { }) const [, spawnArgs] = execFileMock.mock.calls[0] - expect(spawnArgs).toEqual(['-d', 'Ubuntu', '--', 'rm', '-rf', '--', '/home/me/repo/dir']) + expect(spawnArgs).toEqual(['-d', 'Ubuntu', '--exec', 'rm', '-rf', '--', '/home/me/repo/dir']) + }) + + it('runs approved-root deletion through the contained command', async () => { + await withPlatform('win32', async () => { + await expect( + tryDeleteWslUncPath('\\\\wsl.localhost\\Ubuntu\\home\\me\\vault\\real\\session', { + recursive: true, + approvedRoots: ['\\\\wsl.localhost\\Ubuntu\\home\\me\\vault'] + }) + ).resolves.toBe(true) + }) + + const [, spawnArgs] = execFileMock.mock.calls[0] + expect(spawnArgs.slice(0, 5)).toEqual(['-d', 'Ubuntu', '--exec', 'sh', '-c']) + expect(spawnArgs.slice(-7)).toEqual([ + 'directory', + '3', + 'home', + 'me', + 'vault', + 'real', + 'session' + ]) + }) + + it('rejects an inspection failure instead of falling back to direct rm', async () => { + execFileMock.mockImplementation((_cmd, _args, _opts, cb) => { + cb(new Error('Command failed'), '', 'ORCA_WSL_DELETE_REJECT:inspection') + }) + + await withPlatform('win32', async () => { + await expect( + tryDeleteWslUncPath('\\\\wsl.localhost\\Ubuntu\\home\\me\\vault\\session.json', { + approvedRoots: ['\\\\wsl.localhost\\Ubuntu\\home\\me\\vault'] + }) + ).rejects.toMatchObject({ reason: 'path-outside-known-roots' }) + }) + expect(execFileMock).toHaveBeenCalledTimes(1) + }) + + it('rejects a target without a same-distro containing root before spawning', async () => { + await withPlatform('win32', async () => { + await expect( + tryDeleteWslUncPath('\\\\wsl.localhost\\Ubuntu\\home\\me\\vault\\session.json', { + approvedRoots: ['\\\\wsl.localhost\\Debian\\home\\me\\vault'] + }) + ).rejects.toMatchObject({ reason: 'path-outside-known-roots' }) + }) + expect(execFileMock).not.toHaveBeenCalled() }) it('also handles the legacy \\\\wsl$ UNC prefix', async () => { @@ -72,7 +129,7 @@ describe('tryDeleteWslUncPath', () => { }) const [, spawnArgs] = execFileMock.mock.calls[0] - expect(spawnArgs).toEqual(['-d', 'Debian', '--', 'rm', '-f', '--', '/srv/app.log']) + expect(spawnArgs).toEqual(['-d', 'Debian', '--exec', 'rm', '-f', '--', '/srv/app.log']) }) it('surfaces the distro stderr when rm fails', async () => { diff --git a/src/main/wsl-unc-delete.ts b/src/main/wsl-unc-delete.ts index 69e55c916bc..9cc62c9b236 100644 --- a/src/main/wsl-unc-delete.ts +++ b/src/main/wsl-unc-delete.ts @@ -1,5 +1,20 @@ import { execFile } from 'node:child_process' import { parseWslPath } from './wsl' +import { + containedDeleteCommand, + rejectionFromWslDeleteStderr, + type WslDeleteRejection +} from './wsl-contained-delete' + +export class WslDeleteValidationError extends Error { + constructor( + readonly reason: WslDeleteRejection, + message: string + ) { + super(message) + this.name = 'WslDeleteValidationError' + } +} /** * Delete a file/directory that lives on a WSL distro's Linux filesystem, @@ -17,19 +32,33 @@ import { parseWslPath } from './wsl' */ export async function tryDeleteWslUncPath( targetPath: string, - options: { recursive?: boolean } = {} + options: { recursive?: boolean; approvedRoots?: readonly string[] } = {} ): Promise { const info = parseWslPath(targetPath) if (!info) { return false } - // Why: `rm -f` makes the delete idempotent — a missing file is success, - // matching the ENOENT-swallowing semantics of the trash path. `--` stops - // flag parsing so a Linux path that starts with `-` can't be read as an - // option. `-r` is added only for directory deletes the renderer confirmed. - const flags = options.recursive ? '-rf' : '-f' - await execFileWsl(info.distro, ['rm', flags, '--', info.linuxPath]) + let command: string[] + if (options.approvedRoots) { + const contained = containedDeleteCommand( + info, + options.approvedRoots, + parseWslPath, + options.recursive === true + ) + if (!contained) { + throw new WslDeleteValidationError( + 'path-outside-known-roots', + 'Refusing WSL delete outside approved roots' + ) + } + command = contained + } else { + const flags = options.recursive ? '-rf' : '-f' + command = ['rm', flags, '--', info.linuxPath] + } + await execFileWsl(info.distro, command) return true } @@ -37,7 +66,7 @@ function execFileWsl(distro: string, command: string[]): Promise { return new Promise((resolve, reject) => { execFile( 'wsl.exe', - ['-d', distro, '--', ...command], + ['-d', distro, '--exec', ...command], // Why: a generous bound so deleting a large directory tree on the WSL fs // doesn't abort mid-delete, while still capping a wedged wsl.exe. { encoding: 'utf-8', timeout: 30000 }, @@ -54,6 +83,10 @@ function execFileWsl(distro: string, command: string[]): Promise { function wslDeleteError(error: Error, stderr: string): Error { const detail = stderr.trim() + const rejection = rejectionFromWslDeleteStderr(stderr) + if (rejection) { + return new WslDeleteValidationError(rejection, `Refusing WSL delete: ${detail}`) + } if (!detail) { return error } diff --git a/src/main/wsl-unc-delete.wsl.test.ts b/src/main/wsl-unc-delete.wsl.test.ts new file mode 100644 index 00000000000..36175b62626 --- /dev/null +++ b/src/main/wsl-unc-delete.wsl.test.ts @@ -0,0 +1,82 @@ +import { execFile } from 'node:child_process' +import { promisify } from 'node:util' +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { tryDeleteWslUncPath } from './wsl-unc-delete' + +const execFileAsync = promisify(execFile) +const DISTRO = process.env.ORCA_WSL_TEST_DISTRO ?? 'Ubuntu-24.04' +const runRealWsl = process.platform === 'win32' && process.env.ORCA_REAL_WSL_DELETE_TEST === '1' + +function unc(linuxPath: string): string { + return `\\\\wsl.localhost\\${DISTRO}${linuxPath.replaceAll('/', '\\')}` +} + +async function wsl(command: string, ...args: string[]): Promise { + const result = await execFileAsync( + 'wsl.exe', + ['-d', DISTRO, '--exec', 'sh', '-c', command, 'orca-wsl-test', ...args], + { encoding: 'utf-8', timeout: 30000 } + ) + return result.stdout.trim() +} + +describe.skipIf(!runRealWsl)('WSL contained delete integration', () => { + let fixtureRoot: string + + beforeAll(async () => { + fixtureRoot = await wsl("mktemp -d -p /tmp 'orca-wsl-delete-real.XXXXXX'") + await wsl( + 'mkdir -p "$1/vault" "$1/outside/file-parent" "$1/outside/dir-parent/session" "$1/vault/benign/session" && ' + + 'ln -s "$1/outside/file-parent" "$1/vault/file-link" && ' + + 'ln -s "$1/outside/dir-parent" "$1/vault/dir-link" && ' + + 'printf sentinel > "$1/outside/file-parent/session.json" && ' + + 'printf nested > "$1/outside/dir-parent/session/inside" && ' + + 'printf unrelated > "$1/outside/unrelated" && ' + + 'printf benign > "$1/vault/benign/session/inside"', + fixtureRoot + ) + }) + + afterAll(async () => { + if (/^\/tmp\/orca-wsl-delete-real\.[A-Za-z0-9]+$/u.test(fixtureRoot)) { + await wsl('rm -rf -- "$1"', fixtureRoot) + } + }) + + it.each([ + ['file-shaped', 'file-link/session.json', false], + ['directory-shaped', 'dir-link/session', true] + ])('rejects a %s escape and preserves all outside entries', async (_shape, path, recursive) => { + const vaultRoot = `${fixtureRoot}/vault` + + await expect( + tryDeleteWslUncPath(unc(`${vaultRoot}/${path}`), { + recursive, + approvedRoots: [unc(vaultRoot)] + }) + ).rejects.toMatchObject({ reason: 'path-outside-known-roots' }) + + await expect( + wsl( + 'test -f "$1/outside/file-parent/session.json" && ' + + 'test -f "$1/outside/dir-parent/session/inside" && test -f "$1/outside/unrelated"', + fixtureRoot + ) + ).resolves.toBe('') + }) + + it('deletes a benign directory without touching an unrelated sentinel', async () => { + const vaultRoot = `${fixtureRoot}/vault` + + await expect( + tryDeleteWslUncPath(unc(`${vaultRoot}/benign/session`), { + recursive: true, + approvedRoots: [unc(vaultRoot)] + }) + ).resolves.toBe(true) + + await expect( + wsl('test ! -e "$1/vault/benign/session" && test -f "$1/outside/unrelated"', fixtureRoot) + ).resolves.toBe('') + }) +})