From b55320926d65eb6aa3c8af2caf1575f531008a9f Mon Sep 17 00:00:00 2001 From: Keith Schaab Date: Tue, 14 Apr 2026 17:12:16 +0000 Subject: [PATCH] fix(core): support symlinks for workspace policies and handle broken links gracefully Updates the policy TOML loader to correctly follow symlinked files and directories. This ensures that users can symbolically link policies across workspaces or fallback paths without the loader failing. Key changes: - Refactored readPolicyFiles to follow symlinks using fs.realpath. - Added recursion for symlinked directories with a visitedPaths tracker to prevent infinite circular traversal. - Added error handling within the directory scanning loop to catch and ignore ENOENT errors, ensuring that broken symlinks do not silently abort the loading of other valid policies in the same directory. - Added comprehensive unit tests for standard symlink behavior, circular symlink protection, and broken symlink resilience. - Updated mocked fs calls in tests to support realpath. --- packages/core/src/policy/config.test.ts | 21 +++- packages/core/src/policy/toml-loader.test.ts | 101 ++++++++++++++++++ packages/core/src/policy/toml-loader.ts | 47 +++++--- .../core/src/policy/workspace-policy.test.ts | 42 ++++++-- 4 files changed, 188 insertions(+), 23 deletions(-) diff --git a/packages/core/src/policy/config.test.ts b/packages/core/src/policy/config.test.ts index 0d23eaaeed..d05da81581 100644 --- a/packages/core/src/policy/config.test.ts +++ b/packages/core/src/policy/config.test.ts @@ -41,6 +41,7 @@ vi.mock('node:fs/promises', async (importOriginal) => { mkdir: vi.fn(actual.mkdir), open: vi.fn(actual.open), rename: vi.fn(actual.rename), + realpath: vi.fn(actual.realpath), }; return { ...mockFs, @@ -92,13 +93,14 @@ describe('createPolicyEngineConfig', () => { }); vi.mocked(fs.stat).mockImplementation(async (p) => { - if (nodePath.resolve(p.toString()) === nodePath.dirname(resolvedPath)) { + const resolvedP = nodePath.resolve(p.toString()); + if (resolvedP === nodePath.dirname(resolvedPath)) { return { isDirectory: () => true, isFile: () => false, } as unknown as Stats; } - if (nodePath.resolve(p.toString()) === resolvedPath) { + if (resolvedP === resolvedPath) { return { isDirectory: () => false, isFile: () => true, @@ -111,6 +113,21 @@ describe('createPolicyEngineConfig', () => { ).stat(p); }); + vi.mocked(fs.realpath).mockImplementation(async (p) => { + const resolvedP = nodePath.resolve(p.toString()); + if ( + resolvedP === resolvedPath || + resolvedP === nodePath.dirname(resolvedPath) + ) { + return resolvedP; + } + return ( + await vi.importActual( + 'node:fs/promises', + ) + ).realpath(p); + }); + vi.mocked(fs.readFile).mockImplementation(async (p) => { if (nodePath.resolve(p.toString()) === resolvedPath) { return content; diff --git a/packages/core/src/policy/toml-loader.test.ts b/packages/core/src/policy/toml-loader.test.ts index 1d3c4e0eb6..f9232c5b80 100644 --- a/packages/core/src/policy/toml-loader.test.ts +++ b/packages/core/src/policy/toml-loader.test.ts @@ -686,6 +686,107 @@ priority = 100 }); }); + describe('Symlink support', () => { + it('should load a symlinked policy file', async () => { + const realFile = path.join(tempDir, 'real.txt'); // Not .toml + await fs.writeFile( + realFile, + '[[rule]]\ntoolName = "symlink-test"\ndecision = "allow"\npriority = 100\n', + ); + + const symlinkFile = path.join(tempDir, 'link.toml'); + await fs.symlink(realFile, symlinkFile); + + const getPolicyTier = (_dir: string) => 1; + const result = await loadPoliciesFromToml([tempDir], getPolicyTier); + + expect(result.errors).toHaveLength(0); + expect(result.rules).toHaveLength(1); + expect(result.rules[0].toolName).toBe('symlink-test'); + }); + + it('should load from a symlinked directory', async () => { + const realSubDir = path.join(tempDir, 'real-dir'); + await fs.mkdir(realSubDir); + await fs.writeFile( + path.join(realSubDir, 'policy.toml'), + '[[rule]]\ntoolName = "dir-link-test"\ndecision = "allow"\npriority = 100\n', + ); + + const symlinkDir = path.join(tempDir, 'link-dir'); + await fs.symlink(realSubDir, symlinkDir); + + const getPolicyTier = (_dir: string) => 1; + const result = await loadPoliciesFromToml([symlinkDir], getPolicyTier); + + expect(result.errors).toHaveLength(0); + expect(result.rules).toHaveLength(1); + expect(result.rules[0].toolName).toBe('dir-link-test'); + }); + + it('should load from a symlinked subdirectory (recursive)', async () => { + const realSubDir = path.join(tempDir, 'real-subdir'); + await fs.mkdir(realSubDir); + await fs.writeFile( + path.join(realSubDir, 'policy.toml'), + '[[rule]]\ntoolName = "subdir-link-test"\ndecision = "allow"\npriority = 100\n', + ); + + const symlinkSubDir = path.join(tempDir, 'link-subdir'); + await fs.symlink(realSubDir, symlinkSubDir); + + const getPolicyTier = (_dir: string) => 1; + // Load from tempDir, which contains link-subdir + const result = await loadPoliciesFromToml([tempDir], getPolicyTier); + + // Current implementation is NOT recursive, so this is expected to FAIL + // but once we add recursion and symlink following, it should PASS. + expect(result.rules.some((r) => r.toolName === 'subdir-link-test')).toBe( + true, + ); + }); + + it('should prevent circular symlink traversal', async () => { + const subDir = path.join(tempDir, 'circular-dir'); + await fs.mkdir(subDir); + await fs.writeFile( + path.join(subDir, 'policy.toml'), + '[[rule]]\ntoolName = "circular-test"\ndecision = "allow"\npriority = 100\n', + ); + + // Create a symlink back to its parent + await fs.symlink(subDir, path.join(subDir, 'link-back')); + + const getPolicyTier = (_dir: string) => 1; + const result = await loadPoliciesFromToml([subDir], getPolicyTier); + + // Should load policy.toml once and stop + expect( + result.rules.filter((r) => r.toolName === 'circular-test'), + ).toHaveLength(1); + expect(result.errors).toHaveLength(0); + }); + + it('should ignore broken symlinks and continue loading other policies', async () => { + const realFile = path.join(tempDir, 'real.toml'); + await fs.writeFile( + realFile, + '[[rule]]\ntoolName = "valid-test"\ndecision = "allow"\npriority = 100\n', + ); + + // Create a broken symlink (points to a non-existent file) + const brokenLink = path.join(tempDir, 'broken.toml'); + await fs.symlink(path.join(tempDir, 'does-not-exist.toml'), brokenLink); + + const getPolicyTier = (_dir: string) => 1; + const result = await loadPoliciesFromToml([tempDir], getPolicyTier); + + expect(result.errors).toHaveLength(0); + expect(result.rules).toHaveLength(1); + expect(result.rules[0].toolName).toBe('valid-test'); + }); + }); + describe('Tool name validation', () => { it('should warn for unrecognized tool names with suggestions', async () => { const result = await runLoadPoliciesFromToml(` diff --git a/packages/core/src/policy/toml-loader.ts b/packages/core/src/policy/toml-loader.ts index 977e8a399a..edf828238f 100644 --- a/packages/core/src/policy/toml-loader.ts +++ b/packages/core/src/policy/toml-loader.ts @@ -151,28 +151,51 @@ export interface PolicyFile { } /** - * Reads policy files from a directory or a single file. + * Reads policy files from a directory or a single file, following symlinks. + * Supports recursion and prevents circular symlink traversal. * * @param policyPath Path to a directory or a .toml file. + * @param visitedPaths Set of real paths already visited to prevent circularity. * @returns Array of PolicyFile objects. */ export async function readPolicyFiles( policyPath: string, + visitedPaths: Set = new Set(), ): Promise { - let filesToLoad: string[] = []; - let baseDir = ''; + const results: PolicyFile[] = []; try { + const realPath = await fs.realpath(policyPath); + if (visitedPaths.has(realPath)) { + return []; + } + visitedPaths.add(realPath); + const stats = await fs.stat(policyPath); if (stats.isDirectory()) { - baseDir = policyPath; const dirEntries = await fs.readdir(policyPath, { withFileTypes: true }); - filesToLoad = dirEntries - .filter((entry) => entry.isFile() && entry.name.endsWith('.toml')) - .map((entry) => entry.name); + for (const entry of dirEntries) { + const entryPath = path.join(policyPath, entry.name); + try { + const entryStats = await fs.stat(entryPath); + + if (entryStats.isDirectory()) { + // Recursive call + results.push(...(await readPolicyFiles(entryPath, visitedPaths))); + } else if (entryStats.isFile() && entry.name.endsWith('.toml')) { + const content = await fs.readFile(entryPath, 'utf-8'); + results.push({ path: entryPath, content }); + } + } catch (e) { + if (isNodeError(e) && e.code === 'ENOENT') { + continue; + } + throw e; + } + } } else if (stats.isFile() && policyPath.endsWith('.toml')) { - baseDir = path.dirname(policyPath); - filesToLoad = [path.basename(policyPath)]; + const content = await fs.readFile(policyPath, 'utf-8'); + results.push({ path: policyPath, content }); } } catch (e) { if (isNodeError(e) && e.code === 'ENOENT') { @@ -181,12 +204,6 @@ export async function readPolicyFiles( throw e; } - const results: PolicyFile[] = []; - for (const file of filesToLoad) { - const filePath = path.join(baseDir, file); - const content = await fs.readFile(filePath, 'utf-8'); - results.push({ path: filePath, content }); - } return results; } diff --git a/packages/core/src/policy/workspace-policy.test.ts b/packages/core/src/policy/workspace-policy.test.ts index d8f6297e1a..6e9be2d08a 100644 --- a/packages/core/src/policy/workspace-policy.test.ts +++ b/packages/core/src/policy/workspace-policy.test.ts @@ -47,14 +47,22 @@ describe('Workspace-Level Policies', () => { const mockRoot = nodePath.resolve('/mock/'); const mockStat = vi.fn(async (path: string) => { if (typeof path === 'string' && path.startsWith(mockRoot)) { + const isFile = path.endsWith('.toml'); return { - isDirectory: () => true, - isFile: () => false, + isDirectory: () => !isFile, + isFile: () => isFile, } as unknown as Awaited>; } return actualFs.stat(path); }); + const mockRealpath = vi.fn(async (path: string) => { + if (typeof path === 'string' && path.startsWith(mockRoot)) { + return path; + } + return actualFs.realpath(path); + }); + // Mock readdir to return a policy file for each tier const mockReaddir = vi.fn(async (path: string) => { const normalizedPath = nodePath.normalize(path); @@ -125,10 +133,12 @@ priority = 10 readdir: mockReaddir, readFile: mockReadFile, stat: mockStat, + realpath: mockRealpath, }, readdir: mockReaddir, readFile: mockReadFile, stat: mockStat, + realpath: mockRealpath, })); const { createPolicyEngineConfig } = await import('./config.js'); @@ -172,14 +182,22 @@ priority = 10 const mockRoot = nodePath.resolve('/mock/'); const mockStat = vi.fn(async (path: string) => { if (typeof path === 'string' && path.startsWith(mockRoot)) { + const isFile = path.endsWith('.toml'); return { - isDirectory: () => true, - isFile: () => false, + isDirectory: () => !isFile, + isFile: () => isFile, } as unknown as Awaited>; } return actualFs.stat(path); }); + const mockRealpath = vi.fn(async (path: string) => { + if (typeof path === 'string' && path.startsWith(mockRoot)) { + return path; + } + return actualFs.realpath(path); + }); + const mockReaddir = vi.fn(async (path: string) => { const normalizedPath = nodePath.normalize(path); if (normalizedPath.endsWith(nodePath.normalize('default/policies'))) @@ -206,10 +224,12 @@ priority=10`, readdir: mockReaddir, readFile: mockReadFile, stat: mockStat, + realpath: mockRealpath, }, readdir: mockReaddir, readFile: mockReadFile, stat: mockStat, + realpath: mockRealpath, })); const { createPolicyEngineConfig } = await import('./config.js'); @@ -238,14 +258,22 @@ priority=10`, const mockRoot = nodePath.resolve('/mock/'); const mockStat = vi.fn(async (path: string) => { if (typeof path === 'string' && path.startsWith(mockRoot)) { + const isFile = path.endsWith('.toml'); return { - isDirectory: () => true, - isFile: () => false, + isDirectory: () => !isFile, + isFile: () => isFile, } as unknown as Awaited>; } return actualFs.stat(path); }); + const mockRealpath = vi.fn(async (path: string) => { + if (typeof path === 'string' && path.startsWith(mockRoot)) { + return path; + } + return actualFs.realpath(path); + }); + const mockReaddir = vi.fn(async (path: string) => { const normalizedPath = nodePath.normalize(path); if (normalizedPath.endsWith(nodePath.normalize('workspace/policies'))) @@ -272,10 +300,12 @@ priority=500`, readdir: mockReaddir, readFile: mockReadFile, stat: mockStat, + realpath: mockRealpath, }, readdir: mockReaddir, readFile: mockReadFile, stat: mockStat, + realpath: mockRealpath, })); const { createPolicyEngineConfig } = await import('./config.js');