diff --git a/CHANGELOG.md b/CHANGELOG.md index 0264025..fca05aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- The check that keeps saved images inside the workspace now follows links in + the part of the path that already exists when the folders below it do not + exist yet. A save directory whose existing part was a link leading out of the + workspace, followed by folders still to be created, passed the check, so the + folders and the image were created on the far side of the link. The same gap + refused new nested folders in a workspace opened through a link. + ## [0.9.0] - 2026-09-18 **Breaking: VS Code 1.138 or later is now required**, up from 1.137, in step with diff --git a/src/security/path-validator.ts b/src/security/path-validator.ts index a039d20..ea23a20 100644 --- a/src/security/path-validator.ts +++ b/src/security/path-validator.ts @@ -28,24 +28,8 @@ export async function validatePathInsideWorkspace( // Normalize both paths const normalizedWorkspace = normalizePath(realWorkspaceRoot); - // Check if target path exists - let realTargetPath: string; - try { - // If target exists, resolve its real path - realTargetPath = await fs.realpath(targetPath); - } catch { - // Target doesn't exist yet - validate parent directory - const parentDir = path.dirname(targetPath); - try { - const realParentDir = await fs.realpath(parentDir); - realTargetPath = path.join(realParentDir, path.basename(targetPath)); - } catch { - // Parent doesn't exist - check the constructed path - // This is acceptable for new directories that will be created - const resolvedTarget = path.resolve(workspaceRoot, targetPath); - realTargetPath = resolvedTarget; - } - } + // Resolve the target through links as far as it exists + const realTargetPath = await resolveExistingPrefix(targetPath, workspaceRoot); const normalizedTarget = normalizePath(realTargetPath); @@ -72,6 +56,41 @@ export async function validatePathInsideWorkspace( } } +/** + * Resolve a path through links as far as it exists, then append the rest. + * + * `realpath` fails on a path that does not exist yet, so the nearest ancestor + * that does exist is resolved instead and the missing segments are appended to + * it. Resolving less than that — only the parent, or nothing once the parent is + * missing too — lets a link in the existing part lead out of the workspace + * unseen, while a workspace that is itself reached through a link stops looking + * like it contains its own new folders. + * + * @param targetPath - The path to resolve; a relative path is taken from the workspace root + * @param workspaceRoot - The workspace root path + * @returns The real path of the existing part, followed by the missing segments + */ +async function resolveExistingPrefix(targetPath: string, workspaceRoot: string): Promise { + const absolute = path.isAbsolute(targetPath) ? targetPath : path.join(workspaceRoot, targetPath); + const missing: string[] = []; + let current = absolute; + + for (;;) { + try { + const real = await fs.realpath(current); + return path.join(real, ...missing.reverse()); + } catch { + const parent = path.dirname(current); + if (parent === current) { + // Not even the root resolved: compare the path as given + return path.resolve(absolute); + } + missing.push(path.basename(current)); + current = parent; + } + } +} + /** * Check if a path contains parent directory traversal (..) * diff --git a/test/security/path-validator-workspace.test.ts b/test/security/path-validator-workspace.test.ts new file mode 100644 index 0000000..2d80595 --- /dev/null +++ b/test/security/path-validator-workspace.test.ts @@ -0,0 +1,100 @@ +import * as fs from 'fs/promises'; +import * as os from 'os'; +import * as path from 'path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { PathValidationError } from '../../src/core/errors'; +import { validatePathInsideWorkspace } from '../../src/security/path-validator'; + +// A real directory tree rather than mocks: what this function exists for — a +// link that leads out of the workspace — only shows up on a real file system. +// Directory links are junctions, which Windows creates without privileges; +// elsewhere the type argument is ignored. +describe('validatePathInsideWorkspace', () => { + let root: string; + let workspace: string; + let outside: string; + let linkOut: string; + let linkedWorkspace: string; + + beforeAll(async () => { + root = await fs.mkdtemp(path.join(os.tmpdir(), 'clipshot-containment-')); + workspace = path.join(root, 'workspace'); + outside = path.join(root, 'outside'); + await fs.mkdir(path.join(workspace, 'images'), { recursive: true }); + await fs.mkdir(outside); + await fs.writeFile(path.join(workspace, 'images', 'inside.png'), ''); + await fs.writeFile(path.join(outside, 'secret.txt'), ''); + + linkOut = path.join(workspace, 'link-out'); + await fs.symlink(outside, linkOut, 'junction'); + + linkedWorkspace = path.join(root, 'workspace-link'); + await fs.symlink(workspace, linkedWorkspace, 'junction'); + }); + + afterAll(async () => { + await fs.rm(root, { recursive: true, force: true }); + }); + + describe('inside the workspace', () => { + it('accepts an existing file', async () => { + await expect( + validatePathInsideWorkspace(path.join(workspace, 'images', 'inside.png'), workspace) + ).resolves.toBe(true); + }); + + it('accepts a file that does not exist yet in an existing folder', async () => { + await expect( + validatePathInsideWorkspace(path.join(workspace, 'images', 'new.png'), workspace) + ).resolves.toBe(true); + }); + + it('accepts folders that do not exist yet, several levels deep', async () => { + await expect( + validatePathInsideWorkspace(path.join(workspace, 'new', 'deeper', 'image.png'), workspace) + ).resolves.toBe(true); + }); + + // A workspace opened through a link (a symlinked home directory, macOS's + // /var, an 8.3 short name on Windows) resolves to a different string than + // the path the caller builds from it. The check has to compare real paths + // on both sides, including for folders that do not exist yet. + it('accepts new folders under a workspace that is itself reached through a link', async () => { + await expect( + validatePathInsideWorkspace( + path.join(linkedWorkspace, 'new', 'deeper', 'image.png'), + linkedWorkspace + ) + ).resolves.toBe(true); + }); + }); + + describe('outside the workspace', () => { + it('rejects an absolute path outside', async () => { + await expect( + validatePathInsideWorkspace(path.join(outside, 'secret.txt'), workspace) + ).rejects.toBeInstanceOf(PathValidationError); + }); + + it('rejects an existing file reached through a link that leads outside', async () => { + await expect( + validatePathInsideWorkspace(path.join(linkOut, 'secret.txt'), workspace) + ).rejects.toBeInstanceOf(PathValidationError); + }); + + it('rejects a new file directly under a link that leads outside', async () => { + await expect( + validatePathInsideWorkspace(path.join(linkOut, 'new.png'), workspace) + ).rejects.toBeInstanceOf(PathValidationError); + }); + + // The case the folder-creating callers reach: `writeAtomic` and `ensureDir` + // validate first and then `mkdir -p`, so accepting this would create the + // folders — and write the image — on the far side of the link. + it('rejects new folders under a link that leads outside', async () => { + await expect( + validatePathInsideWorkspace(path.join(linkOut, 'new', 'deeper', 'image.png'), workspace) + ).rejects.toBeInstanceOf(PathValidationError); + }); + }); +});