From ccef3bb8ab46b2b1e6215c3357ee6e11dae5a20c Mon Sep 17 00:00:00 2001 From: Lawyered Date: Sun, 22 Feb 2026 00:00:13 -0500 Subject: [PATCH] fix: block symlink escapes in skills file ops --- skills-engine/__tests__/file-ops.test.ts | 62 ++++++++++++++++++++++++ skills-engine/file-ops.ts | 58 +++++++++++++++++++++- 2 files changed, 118 insertions(+), 2 deletions(-) diff --git a/skills-engine/__tests__/file-ops.test.ts b/skills-engine/__tests__/file-ops.test.ts index 1375c1a..ee9a0bb 100644 --- a/skills-engine/__tests__/file-ops.test.ts +++ b/skills-engine/__tests__/file-ops.test.ts @@ -4,6 +4,19 @@ import path from 'path'; import { executeFileOps } from '../file-ops.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', () => { let tmpDir: string; const originalCwd = process.cwd(); @@ -90,4 +103,53 @@ describe('file-ops', () => { expect(result.success).toBe(false); 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); + }); }); diff --git a/skills-engine/file-ops.ts b/skills-engine/file-ops.ts index 65b2f01..ea24c5f 100644 --- a/skills-engine/file-ops.ts +++ b/skills-engine/file-ops.ts @@ -2,11 +2,65 @@ import fs from 'fs'; import path from 'path'; 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 { - const resolved = path.resolve(projectRoot, relativePath); - if (!resolved.startsWith(projectRoot + path.sep) && resolved !== projectRoot) { + if (typeof relativePath !== 'string' || relativePath.trim() === '') { 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; }