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.
This commit is contained in:
Keith Schaab
2026-04-14 17:12:16 +00:00
parent 1bb41262b0
commit b55320926d
4 changed files with 188 additions and 23 deletions
+19 -2
View File
@@ -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<typeof import('node:fs/promises')>(
'node:fs/promises',
)
).realpath(p);
});
vi.mocked(fs.readFile).mockImplementation(async (p) => {
if (nodePath.resolve(p.toString()) === resolvedPath) {
return content;
@@ -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(`
+32 -15
View File
@@ -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<string> = new Set(),
): Promise<PolicyFile[]> {
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;
}
@@ -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<ReturnType<typeof actualFs.stat>>;
}
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<ReturnType<typeof actualFs.stat>>;
}
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<ReturnType<typeof actualFs.stat>>;
}
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');