diff --git a/packages/core/src/policy/config.test.ts b/packages/core/src/policy/config.test.ts index 9cee9997b24..20389a91354 100644 --- a/packages/core/src/policy/config.test.ts +++ b/packages/core/src/policy/config.test.ts @@ -115,8 +115,13 @@ describe('createPolicyEngineConfig', () => { vi.doMock('node:fs/promises', () => ({ ...actualFs, - default: { ...actualFs, readdir: mockReaddir }, + default: { + ...actualFs, + readdir: mockReaddir, + realpath: vi.fn(async (p) => p), + }, readdir: mockReaddir, + realpath: vi.fn(async (p) => p), })); // Mock Storage to avoid actual filesystem access for policy dirs during tests if needed, @@ -486,10 +491,12 @@ describe('createPolicyEngineConfig', () => { readdir: mockReaddir, readFile: mockReadFile, stat: mockStat, + realpath: vi.fn(async (p) => p), }, readdir: mockReaddir, readFile: mockReadFile, stat: mockStat, + realpath: vi.fn(async (p) => p), })); vi.resetModules(); const { createPolicyEngineConfig: createConfig } = await import( @@ -705,10 +712,12 @@ priority = 150 readFile: mockReadFile, readdir: mockReaddir, stat: mockStat, + realpath: vi.fn(async (p) => p), }, readFile: mockReadFile, readdir: mockReaddir, stat: mockStat, + realpath: vi.fn(async (p) => p), })); vi.resetModules(); @@ -834,10 +843,12 @@ required_context = ["environment"] readFile: mockReadFile, readdir: mockReaddir, stat: mockStat, + realpath: vi.fn(async (p) => p), }, readFile: mockReadFile, readdir: mockReaddir, stat: mockStat, + realpath: vi.fn(async (p) => p), })); vi.resetModules(); @@ -956,10 +967,12 @@ name = "invalid-name" readFile: mockReadFile, readdir: mockReaddir, stat: mockStat, + realpath: vi.fn(async (p) => p), }, readFile: mockReadFile, readdir: mockReaddir, stat: mockStat, + realpath: vi.fn(async (p) => p), })); vi.resetModules(); @@ -1021,8 +1034,13 @@ name = "invalid-name" ); vi.doMock('node:fs/promises', () => ({ ...actualFs, - default: { ...actualFs, readdir: mockReaddir }, + default: { + ...actualFs, + readdir: mockReaddir, + realpath: vi.fn(async (p) => p), + }, readdir: mockReaddir, + realpath: vi.fn(async (p) => p), })); const { createPolicyEngineConfig } = await import('./config.js'); @@ -1122,10 +1140,12 @@ modes = ["plan"] readFile: mockReadFile, readdir: mockReaddir, stat: mockStat, + realpath: vi.fn(async (p) => p), }, readFile: mockReadFile, readdir: mockReaddir, stat: mockStat, + realpath: vi.fn(async (p) => p), })); vi.resetModules(); diff --git a/packages/core/src/policy/integrity.ts b/packages/core/src/policy/integrity.ts index e8716ed4381..f40833cd525 100644 --- a/packages/core/src/policy/integrity.ts +++ b/packages/core/src/policy/integrity.ts @@ -43,7 +43,7 @@ export class PolicyIntegrityManager { policyDir: string, ): Promise { const { hash: currentHash, fileCount } = - await PolicyIntegrityManager.calculateIntegrityHash(policyDir); + await PolicyIntegrityManager.calculateIntegrityHash(policyDir, scope); const storedData = await this.loadIntegrityData(); const key = this.getIntegrityKey(scope, identifier); const storedHash = storedData[key]; @@ -86,9 +86,28 @@ export class PolicyIntegrityManager { */ private static async calculateIntegrityHash( policyDir: string, + scope: string, ): Promise<{ hash: string; fileCount: number }> { try { - const files = await readPolicyFiles(policyDir); + // Map scope to tierName for readPolicyFiles + const tierName = + scope === 'user' || scope === 'admin' || scope === 'default' + ? scope + : 'workspace'; + + const { files, errors: readErrors } = await readPolicyFiles( + policyDir, + tierName, + ); + + if (readErrors.length > 0) { + const errorDetails = readErrors + .map((e) => ` - ${e.filePath}: ${e.details}`) + .join('\n'); + debugLogger.error( + `[PolicyIntegrity] Unreadable policy files found in ${policyDir}, they will be excluded from the integrity hash:\n${errorDetails}`, + ); + } // Sort files by path to ensure deterministic hashing files.sort((a, b) => a.path.localeCompare(b.path)); diff --git a/packages/core/src/policy/toml-loader.ts b/packages/core/src/policy/toml-loader.ts index f5c176dc25b..1ecfc3fbcf7 100644 --- a/packages/core/src/policy/toml-loader.ts +++ b/packages/core/src/policy/toml-loader.ts @@ -24,6 +24,7 @@ import path from 'node:path'; import toml from '@iarna/toml'; import { z, type ZodError } from 'zod'; import { isNodeError } from '../utils/errors.js'; +import { isWithinRoot } from '../utils/fileUtils.js'; import { MCP_TOOL_PREFIX, formatMcpToolName } from '../tools/mcp-tool.js'; /** @@ -155,7 +156,8 @@ export interface PolicyFile { */ export async function readPolicyFiles( policyPath: string, -): Promise { + tierName: 'default' | 'extension' | 'user' | 'workspace' | 'admin', +): Promise<{ files: PolicyFile[]; errors: PolicyFileError[] }> { let filesToLoad: string[] = []; let baseDir = ''; @@ -165,7 +167,11 @@ export async function readPolicyFiles( baseDir = policyPath; const dirEntries = await fs.readdir(policyPath, { withFileTypes: true }); filesToLoad = dirEntries - .filter((entry) => entry.isFile() && entry.name.endsWith('.toml')) + .filter( + (entry) => + (entry.isFile() || entry.isSymbolicLink()) && + entry.name.endsWith('.toml'), + ) .map((entry) => entry.name); } else if (stats.isFile() && policyPath.endsWith('.toml')) { baseDir = path.dirname(policyPath); @@ -173,18 +179,53 @@ export async function readPolicyFiles( } } catch (e) { if (isNodeError(e) && e.code === 'ENOENT') { - return []; + return { files: [], errors: [] }; } throw e; } const results: PolicyFile[] = []; + const errors: PolicyFileError[] = []; for (const file of filesToLoad) { const filePath = path.join(baseDir, file); - const content = await fs.readFile(filePath, 'utf-8'); - results.push({ path: filePath, content }); + try { + const realFilePath = await fs.realpath(filePath); + + // For workspace policies, ensure the symlink doesn't "escape" the policy directory + if (tierName === 'workspace') { + const realBaseDir = await fs.realpath(baseDir); + if (!isWithinRoot(realFilePath, realBaseDir)) { + errors.push({ + filePath, + fileName: file, + tier: tierName, + errorType: 'file_read', + message: + 'Security violation: Symbolic link points outside of the workspace policy directory.', + details: `Symlink ${file} resolves to a path outside of the policy folder. Workspace policies are restricted for security.`, + suggestion: + 'To use global policies, place them in your user directory (~/.gemini/policies/) instead.', + }); + continue; + } + } + + const content = await fs.readFile(realFilePath, 'utf-8'); + results.push({ path: filePath, content }); + } catch (e) { + errors.push({ + filePath, + fileName: file, + tier: tierName, + errorType: 'file_read', + message: 'Failed to read policy file', + details: isNodeError(e) ? e.message : String(e), + suggestion: + 'Check if the file is a broken symbolic link or an unreadable directory.', + }); + } } - return results; + return { files: results, errors }; } /** @@ -332,7 +373,9 @@ export async function loadPoliciesFromToml( let policyFiles: PolicyFile[] = []; try { - policyFiles = await readPolicyFiles(p); + const readResult = await readPolicyFiles(p, tierName); + policyFiles = readResult.files; + errors.push(...readResult.errors); } catch (e) { errors.push({ filePath: p, diff --git a/packages/core/src/policy/workspace-policy.test.ts b/packages/core/src/policy/workspace-policy.test.ts index 0a277bc072c..b90fca58295 100644 --- a/packages/core/src/policy/workspace-policy.test.ts +++ b/packages/core/src/policy/workspace-policy.test.ts @@ -117,6 +117,8 @@ priority = 10 return ''; }); + const mockRealpath = vi.fn(async (p) => p); + vi.doMock('node:fs/promises', () => ({ ...actualFs, default: { @@ -124,10 +126,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'); @@ -197,6 +201,8 @@ decision="allow" priority=10`, ); + const mockRealpath = vi.fn(async (p) => p); + vi.doMock('node:fs/promises', () => ({ ...actualFs, default: { @@ -204,10 +210,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'); @@ -262,6 +270,8 @@ decision="allow" priority=500`, ); + const mockRealpath = vi.fn(async (p) => p); + vi.doMock('node:fs/promises', () => ({ ...actualFs, default: { @@ -269,10 +279,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');