fix: block symlink escapes in skills file ops
This commit is contained in:
@@ -4,6 +4,19 @@ import path from 'path';
|
|||||||
import { executeFileOps } from '../file-ops.js';
|
import { executeFileOps } from '../file-ops.js';
|
||||||
import { createTempDir, cleanup } from './test-helpers.js';
|
import { createTempDir, cleanup } from './test-helpers.js';
|
||||||
|
|
||||||
|
function shouldSkipSymlinkTests(err: unknown): boolean {
|
||||||
|
return !!(
|
||||||
|
err &&
|
||||||
|
typeof err === 'object' &&
|
||||||
|
'code' in err &&
|
||||||
|
(
|
||||||
|
(err as { code?: string }).code === 'EPERM' ||
|
||||||
|
(err as { code?: string }).code === 'EACCES' ||
|
||||||
|
(err as { code?: string }).code === 'ENOSYS'
|
||||||
|
)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
describe('file-ops', () => {
|
describe('file-ops', () => {
|
||||||
let tmpDir: string;
|
let tmpDir: string;
|
||||||
const originalCwd = process.cwd();
|
const originalCwd = process.cwd();
|
||||||
@@ -90,4 +103,53 @@ describe('file-ops', () => {
|
|||||||
expect(result.success).toBe(false);
|
expect(result.success).toBe(false);
|
||||||
expect(result.errors.length).toBeGreaterThan(0);
|
expect(result.errors.length).toBeGreaterThan(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('move rejects symlink escape to outside project root', () => {
|
||||||
|
const outsideDir = createTempDir();
|
||||||
|
|
||||||
|
try {
|
||||||
|
fs.symlinkSync(outsideDir, path.join(tmpDir, 'linkdir'));
|
||||||
|
} catch (err) {
|
||||||
|
cleanup(outsideDir);
|
||||||
|
if (shouldSkipSymlinkTests(err)) return;
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
|
||||||
|
fs.writeFileSync(path.join(tmpDir, 'source.ts'), 'content');
|
||||||
|
|
||||||
|
const result = executeFileOps([
|
||||||
|
{ type: 'move', from: 'source.ts', to: 'linkdir/pwned.ts' },
|
||||||
|
], tmpDir);
|
||||||
|
|
||||||
|
expect(result.success).toBe(false);
|
||||||
|
expect(result.errors.some((e) => e.includes('escapes project root'))).toBe(true);
|
||||||
|
expect(fs.existsSync(path.join(tmpDir, 'source.ts'))).toBe(true);
|
||||||
|
expect(fs.existsSync(path.join(outsideDir, 'pwned.ts'))).toBe(false);
|
||||||
|
|
||||||
|
cleanup(outsideDir);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('delete rejects symlink escape to outside project root', () => {
|
||||||
|
const outsideDir = createTempDir();
|
||||||
|
const outsideFile = path.join(outsideDir, 'victim.ts');
|
||||||
|
fs.writeFileSync(outsideFile, 'secret');
|
||||||
|
|
||||||
|
try {
|
||||||
|
fs.symlinkSync(outsideDir, path.join(tmpDir, 'linkdir'));
|
||||||
|
} catch (err) {
|
||||||
|
cleanup(outsideDir);
|
||||||
|
if (shouldSkipSymlinkTests(err)) return;
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
|
||||||
|
const result = executeFileOps([
|
||||||
|
{ type: 'delete', path: 'linkdir/victim.ts' },
|
||||||
|
], tmpDir);
|
||||||
|
|
||||||
|
expect(result.success).toBe(false);
|
||||||
|
expect(result.errors.some((e) => e.includes('escapes project root'))).toBe(true);
|
||||||
|
expect(fs.existsSync(outsideFile)).toBe(true);
|
||||||
|
|
||||||
|
cleanup(outsideDir);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -2,11 +2,65 @@ import fs from 'fs';
|
|||||||
import path from 'path';
|
import path from 'path';
|
||||||
import type { FileOperation, FileOpsResult } from './types.js';
|
import type { FileOperation, FileOpsResult } from './types.js';
|
||||||
|
|
||||||
|
function isWithinRoot(rootPath: string, targetPath: string): boolean {
|
||||||
|
return targetPath === rootPath || targetPath.startsWith(rootPath + path.sep);
|
||||||
|
}
|
||||||
|
|
||||||
|
function nearestExistingPathOrSymlink(candidateAbsPath: string): string {
|
||||||
|
let current = candidateAbsPath;
|
||||||
|
while (true) {
|
||||||
|
try {
|
||||||
|
fs.lstatSync(current);
|
||||||
|
return current;
|
||||||
|
} catch {
|
||||||
|
const parent = path.dirname(current);
|
||||||
|
if (parent === current) {
|
||||||
|
throw new Error(`Invalid file operation path: "${candidateAbsPath}"`);
|
||||||
|
}
|
||||||
|
current = parent;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function resolveRealPathWithSymlinkAwareAnchor(candidateAbsPath: string): string {
|
||||||
|
const anchorPath = nearestExistingPathOrSymlink(candidateAbsPath);
|
||||||
|
const anchorStat = fs.lstatSync(anchorPath);
|
||||||
|
let realAnchor: string;
|
||||||
|
|
||||||
|
if (anchorStat.isSymbolicLink()) {
|
||||||
|
const linkTarget = fs.readlinkSync(anchorPath);
|
||||||
|
const linkResolved = path.resolve(path.dirname(anchorPath), linkTarget);
|
||||||
|
realAnchor = fs.realpathSync(linkResolved);
|
||||||
|
} else {
|
||||||
|
realAnchor = fs.realpathSync(anchorPath);
|
||||||
|
}
|
||||||
|
|
||||||
|
const relativeRemainder = path.relative(anchorPath, candidateAbsPath);
|
||||||
|
return relativeRemainder
|
||||||
|
? path.resolve(realAnchor, relativeRemainder)
|
||||||
|
: realAnchor;
|
||||||
|
}
|
||||||
|
|
||||||
function safePath(projectRoot: string, relativePath: string): string | null {
|
function safePath(projectRoot: string, relativePath: string): string | null {
|
||||||
const resolved = path.resolve(projectRoot, relativePath);
|
if (typeof relativePath !== 'string' || relativePath.trim() === '') {
|
||||||
if (!resolved.startsWith(projectRoot + path.sep) && resolved !== projectRoot) {
|
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const root = path.resolve(projectRoot);
|
||||||
|
const resolved = path.resolve(root, relativePath);
|
||||||
|
if (!isWithinRoot(root, resolved)) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
if (resolved === root) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
const realRoot = fs.realpathSync(root);
|
||||||
|
const realParent = resolveRealPathWithSymlinkAwareAnchor(path.dirname(resolved));
|
||||||
|
if (!isWithinRoot(realRoot, realParent)) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
return resolved;
|
return resolved;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user