Prevent writing outside of the workspace roots (#6178)

Co-authored-by: Jacob Richman <jacob314@gmail.com>
This commit is contained in:
Tommaso Sciortino 2025-08-14 15:59:37 -07:00 committed by GitHub
parent f47af1607a
commit 1a41ba7daf
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 258 additions and 251 deletions

View File

@ -4,280 +4,304 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect, beforeEach, vi } from 'vitest';
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import { WorkspaceContext } from './workspaceContext.js';
vi.mock('fs');
describe('WorkspaceContext', () => {
let workspaceContext: WorkspaceContext;
// Use path module to create platform-agnostic paths
const mockCwd = path.resolve(path.sep, 'home', 'user', 'project');
const mockExistingDir = path.resolve(
path.sep,
'home',
'user',
'other-project',
);
const mockNonExistentDir = path.resolve(
path.sep,
'home',
'user',
'does-not-exist',
);
const mockSymlinkDir = path.resolve(path.sep, 'home', 'user', 'symlink');
const mockRealPath = path.resolve(path.sep, 'home', 'user', 'real-directory');
describe('WorkspaceContext with real filesystem', () => {
let tempDir: string;
let cwd: string;
let otherDir: string;
beforeEach(() => {
vi.resetAllMocks();
// os.tmpdir() can return a path using a symlink (this is standard on macOS)
// Use fs.realpathSync to fully resolve the absolute path.
tempDir = fs.realpathSync(
fs.mkdtempSync(path.join(os.tmpdir(), 'workspace-context-test-')),
);
// Mock fs.existsSync
vi.mocked(fs.existsSync).mockImplementation((path) => {
const pathStr = path.toString();
return (
pathStr === mockCwd ||
pathStr === mockExistingDir ||
pathStr === mockSymlinkDir ||
pathStr === mockRealPath
);
});
cwd = path.join(tempDir, 'project');
otherDir = path.join(tempDir, 'other-project');
// Mock fs.statSync
vi.mocked(fs.statSync).mockImplementation((path) => {
const pathStr = path.toString();
if (pathStr === mockNonExistentDir) {
throw new Error('ENOENT');
}
return {
isDirectory: () => true,
} as fs.Stats;
});
fs.mkdirSync(cwd, { recursive: true });
fs.mkdirSync(otherDir, { recursive: true });
});
// Mock fs.realpathSync
vi.mocked(fs.realpathSync).mockImplementation((path) => {
const pathStr = path.toString();
if (pathStr === mockSymlinkDir) {
return mockRealPath;
}
return pathStr;
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
});
describe('initialization', () => {
it('should initialize with a single directory (cwd)', () => {
workspaceContext = new WorkspaceContext(mockCwd);
const workspaceContext = new WorkspaceContext(cwd);
const directories = workspaceContext.getDirectories();
expect(directories).toHaveLength(1);
expect(directories[0]).toBe(mockCwd);
expect(directories).toEqual([cwd]);
});
it('should validate and resolve directories to absolute paths', () => {
const absolutePath = path.join(mockCwd, 'subdir');
vi.mocked(fs.existsSync).mockImplementation(
(p) => p === mockCwd || p === absolutePath,
);
vi.mocked(fs.realpathSync).mockImplementation((p) => p.toString());
workspaceContext = new WorkspaceContext(mockCwd, [absolutePath]);
const workspaceContext = new WorkspaceContext(cwd, [otherDir]);
const directories = workspaceContext.getDirectories();
expect(directories).toContain(absolutePath);
expect(directories).toEqual([cwd, otherDir]);
});
it('should reject non-existent directories', () => {
const nonExistentDir = path.join(tempDir, 'does-not-exist');
expect(() => {
new WorkspaceContext(mockCwd, [mockNonExistentDir]);
new WorkspaceContext(cwd, [nonExistentDir]);
}).toThrow('Directory does not exist');
});
it('should handle empty initialization', () => {
workspaceContext = new WorkspaceContext(mockCwd, []);
const workspaceContext = new WorkspaceContext(cwd, []);
const directories = workspaceContext.getDirectories();
expect(directories).toHaveLength(1);
expect(directories[0]).toBe(mockCwd);
expect(fs.realpathSync(directories[0])).toBe(cwd);
});
});
describe('adding directories', () => {
beforeEach(() => {
workspaceContext = new WorkspaceContext(mockCwd);
});
it('should add valid directories', () => {
workspaceContext.addDirectory(mockExistingDir);
const workspaceContext = new WorkspaceContext(cwd);
workspaceContext.addDirectory(otherDir);
const directories = workspaceContext.getDirectories();
expect(directories).toHaveLength(2);
expect(directories).toContain(mockExistingDir);
expect(directories).toEqual([cwd, otherDir]);
});
it('should resolve relative paths to absolute', () => {
// Since we can't mock path.resolve, we'll test with absolute paths
workspaceContext.addDirectory(mockExistingDir);
const workspaceContext = new WorkspaceContext(cwd);
const relativePath = path.relative(cwd, otherDir);
workspaceContext.addDirectory(relativePath, cwd);
const directories = workspaceContext.getDirectories();
expect(directories).toContain(mockExistingDir);
expect(directories).toEqual([cwd, otherDir]);
});
it('should reject non-existent directories', () => {
const nonExistentDir = path.join(tempDir, 'does-not-exist');
const workspaceContext = new WorkspaceContext(cwd);
expect(() => {
workspaceContext.addDirectory(mockNonExistentDir);
workspaceContext.addDirectory(nonExistentDir);
}).toThrow('Directory does not exist');
});
it('should prevent duplicate directories', () => {
workspaceContext.addDirectory(mockExistingDir);
workspaceContext.addDirectory(mockExistingDir);
const workspaceContext = new WorkspaceContext(cwd);
workspaceContext.addDirectory(otherDir);
workspaceContext.addDirectory(otherDir);
const directories = workspaceContext.getDirectories();
expect(directories.filter((d) => d === mockExistingDir)).toHaveLength(1);
expect(directories).toHaveLength(2);
});
it('should handle symbolic links correctly', () => {
workspaceContext.addDirectory(mockSymlinkDir);
const realDir = path.join(tempDir, 'real');
fs.mkdirSync(realDir, { recursive: true });
const symlinkDir = path.join(tempDir, 'symlink-to-real');
fs.symlinkSync(realDir, symlinkDir, 'dir');
const workspaceContext = new WorkspaceContext(cwd);
workspaceContext.addDirectory(symlinkDir);
const directories = workspaceContext.getDirectories();
expect(directories).toContain(mockRealPath);
expect(directories).not.toContain(mockSymlinkDir);
expect(directories).toEqual([cwd, realDir]);
});
});
describe('path validation', () => {
beforeEach(() => {
workspaceContext = new WorkspaceContext(mockCwd, [mockExistingDir]);
it('should accept paths within workspace directories', () => {
const workspaceContext = new WorkspaceContext(cwd, [otherDir]);
const validPath1 = path.join(cwd, 'src', 'file.ts');
const validPath2 = path.join(otherDir, 'lib', 'module.js');
fs.mkdirSync(path.dirname(validPath1), { recursive: true });
fs.writeFileSync(validPath1, 'content');
fs.mkdirSync(path.dirname(validPath2), { recursive: true });
fs.writeFileSync(validPath2, 'content');
expect(workspaceContext.isPathWithinWorkspace(validPath1)).toBe(true);
expect(workspaceContext.isPathWithinWorkspace(validPath2)).toBe(true);
});
it('should accept paths within workspace directories', () => {
const validPath1 = path.join(mockCwd, 'src', 'file.ts');
const validPath2 = path.join(mockExistingDir, 'lib', 'module.js');
it('should accept non-existent paths within workspace directories', () => {
const workspaceContext = new WorkspaceContext(cwd, [otherDir]);
const validPath1 = path.join(cwd, 'src', 'file.ts');
const validPath2 = path.join(otherDir, 'lib', 'module.js');
expect(workspaceContext.isPathWithinWorkspace(validPath1)).toBe(true);
expect(workspaceContext.isPathWithinWorkspace(validPath2)).toBe(true);
});
it('should reject paths outside workspace', () => {
const invalidPath = path.resolve(
path.dirname(mockCwd),
'outside-workspace',
'file.txt',
);
const workspaceContext = new WorkspaceContext(cwd, [otherDir]);
const invalidPath = path.join(tempDir, 'outside-workspace', 'file.txt');
expect(workspaceContext.isPathWithinWorkspace(invalidPath)).toBe(false);
});
it('should resolve symbolic links before validation', () => {
const symlinkPath = path.join(mockCwd, 'symlink-file');
const realPath = path.join(mockCwd, 'real-file');
it('should reject non-existent paths outside workspace', () => {
const workspaceContext = new WorkspaceContext(cwd, [otherDir]);
const invalidPath = path.join(tempDir, 'outside-workspace', 'file.txt');
vi.mocked(fs.existsSync).mockReturnValue(true);
vi.mocked(fs.realpathSync).mockImplementation((p) => {
if (p === symlinkPath) {
return realPath;
}
return p.toString();
});
expect(workspaceContext.isPathWithinWorkspace(symlinkPath)).toBe(true);
expect(workspaceContext.isPathWithinWorkspace(invalidPath)).toBe(false);
});
it('should handle nested directories correctly', () => {
const nestedPath = path.join(
mockCwd,
'deeply',
'nested',
'path',
'file.txt',
);
const workspaceContext = new WorkspaceContext(cwd, [otherDir]);
const nestedPath = path.join(cwd, 'deeply', 'nested', 'path', 'file.txt');
expect(workspaceContext.isPathWithinWorkspace(nestedPath)).toBe(true);
});
it('should handle edge cases (root, parent references)', () => {
const rootPath = '/';
const parentPath = path.dirname(mockCwd);
const workspaceContext = new WorkspaceContext(cwd, [otherDir]);
const rootPath = path.parse(tempDir).root;
const parentPath = path.dirname(cwd);
expect(workspaceContext.isPathWithinWorkspace(rootPath)).toBe(false);
expect(workspaceContext.isPathWithinWorkspace(parentPath)).toBe(false);
});
it('should handle non-existent paths correctly', () => {
const nonExistentPath = path.join(mockCwd, 'does-not-exist.txt');
vi.mocked(fs.existsSync).mockImplementation((p) => p !== nonExistentPath);
// Should still validate based on path structure
const workspaceContext = new WorkspaceContext(cwd, [otherDir]);
const nonExistentPath = path.join(cwd, 'does-not-exist.txt');
expect(workspaceContext.isPathWithinWorkspace(nonExistentPath)).toBe(
true,
);
});
describe('with symbolic link', () => {
describe('in the workspace', () => {
let realDir: string;
let symlinkDir: string;
beforeEach(() => {
realDir = path.join(cwd, 'real-dir');
fs.mkdirSync(realDir, { recursive: true });
symlinkDir = path.join(cwd, 'symlink-file');
fs.symlinkSync(realDir, symlinkDir, 'dir');
});
it('should accept dir paths', () => {
const workspaceContext = new WorkspaceContext(cwd);
expect(workspaceContext.isPathWithinWorkspace(symlinkDir)).toBe(true);
});
it('should accept non-existent paths', () => {
const filePath = path.join(symlinkDir, 'does-not-exist.txt');
const workspaceContext = new WorkspaceContext(cwd);
expect(workspaceContext.isPathWithinWorkspace(filePath)).toBe(true);
});
it('should accept non-existent deep paths', () => {
const filePath = path.join(symlinkDir, 'deep', 'does-not-exist.txt');
const workspaceContext = new WorkspaceContext(cwd);
expect(workspaceContext.isPathWithinWorkspace(filePath)).toBe(true);
});
});
describe('outside the workspace', () => {
let realDir: string;
let symlinkDir: string;
beforeEach(() => {
realDir = path.join(tempDir, 'real-dir');
fs.mkdirSync(realDir, { recursive: true });
symlinkDir = path.join(cwd, 'symlink-file');
fs.symlinkSync(realDir, symlinkDir, 'dir');
});
it('should reject dir paths', () => {
const workspaceContext = new WorkspaceContext(cwd);
expect(workspaceContext.isPathWithinWorkspace(symlinkDir)).toBe(
false,
);
});
it('should reject non-existent paths', () => {
const filePath = path.join(symlinkDir, 'does-not-exist.txt');
const workspaceContext = new WorkspaceContext(cwd);
expect(workspaceContext.isPathWithinWorkspace(filePath)).toBe(false);
});
it('should reject non-existent deep paths', () => {
const filePath = path.join(symlinkDir, 'deep', 'does-not-exist.txt');
const workspaceContext = new WorkspaceContext(cwd);
expect(workspaceContext.isPathWithinWorkspace(filePath)).toBe(false);
});
it('should reject partially non-existent deep paths', () => {
const deepDir = path.join(symlinkDir, 'deep');
fs.mkdirSync(deepDir, { recursive: true });
const filePath = path.join(deepDir, 'does-not-exist.txt');
const workspaceContext = new WorkspaceContext(cwd);
expect(workspaceContext.isPathWithinWorkspace(filePath)).toBe(false);
});
});
it('should reject symbolic file links outside the workspace', () => {
const realFile = path.join(tempDir, 'real-file.txt');
fs.writeFileSync(realFile, 'content');
const symlinkFile = path.join(cwd, 'symlink-to-real-file');
fs.symlinkSync(realFile, symlinkFile, 'file');
const workspaceContext = new WorkspaceContext(cwd);
expect(workspaceContext.isPathWithinWorkspace(symlinkFile)).toBe(false);
});
it('should reject non-existent symbolic file links outside the workspace', () => {
const realFile = path.join(tempDir, 'real-file.txt');
const symlinkFile = path.join(cwd, 'symlink-to-real-file');
fs.symlinkSync(realFile, symlinkFile, 'file');
const workspaceContext = new WorkspaceContext(cwd);
expect(workspaceContext.isPathWithinWorkspace(symlinkFile)).toBe(false);
});
it('should handle circular symlinks gracefully', () => {
const workspaceContext = new WorkspaceContext(cwd);
const linkA = path.join(cwd, 'link-a');
const linkB = path.join(cwd, 'link-b');
// Create a circular dependency: linkA -> linkB -> linkA
fs.symlinkSync(linkB, linkA, 'dir');
fs.symlinkSync(linkA, linkB, 'dir');
// fs.realpathSync should throw ELOOP, and isPathWithinWorkspace should
// handle it gracefully and return false.
expect(workspaceContext.isPathWithinWorkspace(linkA)).toBe(false);
expect(workspaceContext.isPathWithinWorkspace(linkB)).toBe(false);
});
});
});
describe('getDirectories', () => {
it('should return a copy of directories array', () => {
workspaceContext = new WorkspaceContext(mockCwd);
const workspaceContext = new WorkspaceContext(cwd);
const dirs1 = workspaceContext.getDirectories();
const dirs2 = workspaceContext.getDirectories();
expect(dirs1).not.toBe(dirs2); // Different array instances
expect(dirs1).toEqual(dirs2); // Same content
});
});
describe('symbolic link security', () => {
beforeEach(() => {
workspaceContext = new WorkspaceContext(mockCwd);
});
it('should follow symlinks but validate resolved path', () => {
const symlinkInsideWorkspace = path.join(mockCwd, 'link-to-subdir');
const resolvedInsideWorkspace = path.join(mockCwd, 'subdir');
vi.mocked(fs.existsSync).mockReturnValue(true);
vi.mocked(fs.realpathSync).mockImplementation((p) => {
if (p === symlinkInsideWorkspace) {
return resolvedInsideWorkspace;
}
return p.toString();
});
expect(
workspaceContext.isPathWithinWorkspace(symlinkInsideWorkspace),
).toBe(true);
});
it('should prevent sandbox escape via symlinks', () => {
const symlinkEscape = path.join(mockCwd, 'escape-link');
const resolvedOutside = path.resolve(mockCwd, '..', 'outside-file');
vi.mocked(fs.existsSync).mockImplementation((p) => {
const pathStr = p.toString();
return (
pathStr === symlinkEscape ||
pathStr === resolvedOutside ||
pathStr === mockCwd
);
});
vi.mocked(fs.realpathSync).mockImplementation((p) => {
if (p.toString() === symlinkEscape) {
return resolvedOutside;
}
return p.toString();
});
vi.mocked(fs.statSync).mockImplementation(
(p) =>
({
isDirectory: () => p.toString() !== resolvedOutside,
}) as fs.Stats,
);
workspaceContext = new WorkspaceContext(mockCwd);
expect(workspaceContext.isPathWithinWorkspace(symlinkEscape)).toBe(false);
});
it('should handle circular symlinks', () => {
const circularLink = path.join(mockCwd, 'circular');
vi.mocked(fs.existsSync).mockReturnValue(true);
vi.mocked(fs.realpathSync).mockImplementation(() => {
throw new Error('ELOOP: too many symbolic links encountered');
});
// Should handle the error gracefully
expect(workspaceContext.isPathWithinWorkspace(circularLink)).toBe(false);
expect(dirs1).not.toBe(dirs2);
expect(dirs1).toEqual(dirs2);
});
});
});

View File

@ -4,6 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { isNodeError } from '../utils/errors.js';
import * as fs from 'fs';
import * as path from 'path';
@ -13,26 +14,21 @@ import * as path from 'path';
* in a single session.
*/
export class WorkspaceContext {
private directories: Set<string>;
private directories = new Set<string>();
private initialDirectories: Set<string>;
/**
* Creates a new WorkspaceContext with the given initial directory and optional additional directories.
* @param initialDirectory The initial working directory (usually cwd)
* @param directory The initial working directory (usually cwd)
* @param additionalDirectories Optional array of additional directories to include
*/
constructor(initialDirectory: string, additionalDirectories: string[] = []) {
this.directories = new Set<string>();
this.initialDirectories = new Set<string>();
this.addDirectoryInternal(initialDirectory);
this.addInitialDirectoryInternal(initialDirectory);
for (const dir of additionalDirectories) {
this.addDirectoryInternal(dir);
this.addInitialDirectoryInternal(dir);
constructor(directory: string, additionalDirectories: string[] = []) {
this.addDirectory(directory);
for (const additionalDirectory of additionalDirectories) {
this.addDirectory(additionalDirectory);
}
this.initialDirectories = new Set(this.directories);
}
/**
@ -41,16 +37,13 @@ export class WorkspaceContext {
* @param basePath Optional base path for resolving relative paths (defaults to cwd)
*/
addDirectory(directory: string, basePath: string = process.cwd()): void {
this.addDirectoryInternal(directory, basePath);
this.directories.add(this.resolveAndValidateDir(directory, basePath));
}
/**
* Internal method to add a directory with validation.
*/
private addDirectoryInternal(
private resolveAndValidateDir(
directory: string,
basePath: string = process.cwd(),
): void {
): string {
const absolutePath = path.isAbsolute(directory)
? directory
: path.resolve(basePath, directory);
@ -58,47 +51,12 @@ export class WorkspaceContext {
if (!fs.existsSync(absolutePath)) {
throw new Error(`Directory does not exist: ${absolutePath}`);
}
const stats = fs.statSync(absolutePath);
if (!stats.isDirectory()) {
throw new Error(`Path is not a directory: ${absolutePath}`);
}
let realPath: string;
try {
realPath = fs.realpathSync(absolutePath);
} catch (_error) {
throw new Error(`Failed to resolve path: ${absolutePath}`);
}
this.directories.add(realPath);
}
private addInitialDirectoryInternal(
directory: string,
basePath: string = process.cwd(),
): void {
const absolutePath = path.isAbsolute(directory)
? directory
: path.resolve(basePath, directory);
if (!fs.existsSync(absolutePath)) {
throw new Error(`Directory does not exist: ${absolutePath}`);
}
const stats = fs.statSync(absolutePath);
if (!stats.isDirectory()) {
throw new Error(`Path is not a directory: ${absolutePath}`);
}
let realPath: string;
try {
realPath = fs.realpathSync(absolutePath);
} catch (_error) {
throw new Error(`Failed to resolve path: ${absolutePath}`);
}
this.initialDirectories.add(realPath);
return fs.realpathSync(absolutePath);
}
/**
@ -116,7 +74,7 @@ export class WorkspaceContext {
setDirectories(directories: readonly string[]): void {
this.directories.clear();
for (const dir of directories) {
this.addDirectoryInternal(dir);
this.addDirectory(dir);
}
}
@ -127,29 +85,43 @@ export class WorkspaceContext {
*/
isPathWithinWorkspace(pathToCheck: string): boolean {
try {
const absolutePath = path.resolve(pathToCheck);
let resolvedPath = absolutePath;
if (fs.existsSync(absolutePath)) {
try {
resolvedPath = fs.realpathSync(absolutePath);
} catch (_error) {
return false;
}
}
const fullyResolvedPath = this.fullyResolvedPath(pathToCheck);
for (const dir of this.directories) {
if (this.isPathWithinRoot(resolvedPath, dir)) {
if (this.isPathWithinRoot(fullyResolvedPath, dir)) {
return true;
}
}
return false;
} catch (_error) {
return false;
}
}
/**
* Fully resolves a path, including symbolic links.
* If the path does not exist, it returns the fully resolved path as it would be
* if it did exist.
*/
private fullyResolvedPath(pathToCheck: string): string {
try {
return fs.realpathSync(pathToCheck);
} catch (e: unknown) {
if (
isNodeError(e) &&
e.code === 'ENOENT' &&
e.path &&
// realpathSync does not set e.path correctly for symlinks to
// non-existent files.
!this.isFileSymlink(e.path)
) {
// If it doesn't exist, e.path contains the fully resolved path.
return e.path;
}
throw e;
}
}
/**
* Checks if a path is within a given root directory.
* @param pathToCheck The absolute path to check
@ -167,4 +139,15 @@ export class WorkspaceContext {
!path.isAbsolute(relative)
);
}
/**
* Checks if a file path is a symbolic link that points to a file.
*/
private isFileSymlink(filePath: string): boolean {
try {
return !fs.readlinkSync(filePath).endsWith('/');
} catch (_error) {
return false;
}
}
}