fix(security): enforce strict policy directory permissions (#17353)

Co-authored-by: Yuna Seol <yunaseol@google.com>

packages/core/src/policy/config.test.ts

@@ -10,6 +10,11 @@ import nodePath from 'node:path';
 
 import type { PolicySettings } from './types.js';
 import { ApprovalMode, PolicyDecision, InProcessCheckerType } from './types.js';
+import { isDirectorySecure } from '../utils/security.js';
+
+vi.mock('../utils/security.js', () => ({
+  isDirectorySecure: vi.fn().mockResolvedValue({ secure: true }),
+}));
 
 afterEach(() => {
   vi.clearAllMocks();
@@ -28,7 +33,53 @@ describe('createPolicyEngineConfig', () => {
     vi.spyOn(Storage, 'getSystemPoliciesDir').mockReturnValue(
       '/non/existent/system/policies',
     );
+    // Reset security check to default secure
+    vi.mocked(isDirectorySecure).mockResolvedValue({ secure: true });
   });
+
+  it('should filter out insecure system policy directories', async () => {
+    const { Storage } = await import('../config/storage.js');
+    const systemPolicyDir = '/insecure/system/policies';
+    vi.spyOn(Storage, 'getSystemPoliciesDir').mockReturnValue(systemPolicyDir);
+
+    vi.mocked(isDirectorySecure).mockImplementation(async (path: string) => {
+      if (nodePath.resolve(path) === nodePath.resolve(systemPolicyDir)) {
+        return { secure: false, reason: 'Insecure directory' };
+      }
+      return { secure: true };
+    });
+
+    // We need to spy on loadPoliciesFromToml to verify which directories were passed
+    // But it is not exported from config.js, it is imported.
+    // We can spy on the module it comes from.
+    const tomlLoader = await import('./toml-loader.js');
+    const loadPoliciesSpy = vi.spyOn(tomlLoader, 'loadPoliciesFromToml');
+    loadPoliciesSpy.mockResolvedValue({
+      rules: [],
+      checkers: [],
+      errors: [],
+    });
+
+    const { createPolicyEngineConfig } = await import('./config.js');
+    const settings: PolicySettings = {};
+
+    await createPolicyEngineConfig(
+      settings,
+      ApprovalMode.DEFAULT,
+      '/tmp/mock/default/policies',
+    );
+
+    // Verify loadPoliciesFromToml was called
+    expect(loadPoliciesSpy).toHaveBeenCalled();
+    const calledDirs = loadPoliciesSpy.mock.calls[0][0];
+
+    // The system directory should NOT be in the list
+    expect(calledDirs).not.toContain(systemPolicyDir);
+    // But other directories (user, default) should be there
+    expect(calledDirs).toContain('/non/existent/user/policies');
+    expect(calledDirs).toContain('/tmp/mock/default/policies');
+  });
+
   it('should return ASK_USER for write tools and ALLOW for read-only tools by default', async () => {
     const actualFs =
       await vi.importActual<typeof import('node:fs/promises')>(

packages/core/src/policy/config.ts

@@ -29,6 +29,8 @@ import { debugLogger } from '../utils/debugLogger.js';
 import { SHELL_TOOL_NAMES } from '../utils/shell-utils.js';
 import { SHELL_TOOL_NAME } from '../tools/tool-names.js';
 
+import { isDirectorySecure } from '../utils/security.js';
+
 const __filename = fileURLToPath(import.meta.url);
 const __dirname = path.dirname(__filename);
 export const DEFAULT_CORE_POLICIES_DIR = path.join(__dirname, 'policies');
@@ -112,19 +114,47 @@ export function formatPolicyError(error: PolicyFileError): string {
   return message;
 }
 
+/**
+ * Filters out insecure policy directories (specifically the system policy directory).
+ * Emits warnings if insecure directories are found.
+ */
+async function filterSecurePolicyDirectories(
+  dirs: string[],
+): Promise<string[]> {
+  const systemPoliciesDir = path.resolve(Storage.getSystemPoliciesDir());
+
+  const results = await Promise.all(
+    dirs.map(async (dir) => {
+      // Only check security for system policies
+      if (path.resolve(dir) === systemPoliciesDir) {
+        const { secure, reason } = await isDirectorySecure(dir);
+        if (!secure) {
+          const msg = `Security Warning: Skipping system policies from ${dir}: ${reason}`;
+          coreEvents.emitFeedback('warning', msg);
+          return null;
+        }
+      }
+      return dir;
+    }),
+  );
+
+  return results.filter((dir): dir is string => dir !== null);
+}
+
 export async function createPolicyEngineConfig(
   settings: PolicySettings,
   approvalMode: ApprovalMode,
   defaultPoliciesDir?: string,
 ): Promise<PolicyEngineConfig> {
   const policyDirs = getPolicyDirectories(defaultPoliciesDir);
+  const securePolicyDirs = await filterSecurePolicyDirectories(policyDirs);
 
   // Load policies from TOML files
   const {
     rules: tomlRules,
     checkers: tomlCheckers,
     errors,
-  } = await loadPoliciesFromToml(policyDirs, (dir) =>
+  } = await loadPoliciesFromToml(securePolicyDirs, (dir) =>
     getPolicyTier(dir, defaultPoliciesDir),
   );