Add Folder Trust Support To Hooks (#15325)

packages/cli/src/config/policy-engine.integration.test.ts

@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { describe, it, expect } from 'vitest';
+import { describe, it, expect, vi } from 'vitest';
 import {
   ApprovalMode,
   PolicyDecision,
@@ -13,6 +13,21 @@ import {
 import { createPolicyEngineConfig } from './policy.js';
 import type { Settings } from './settings.js';
 
+// Mock Storage to ensure tests are hermetic and don't read from user's home directory
+vi.mock('@google/gemini-cli-core', async (importOriginal) => {
+  const actual =
+    await importOriginal<typeof import('@google/gemini-cli-core')>();
+  const Storage = actual.Storage;
+  // Monkey-patch static methods
+  Storage.getUserPoliciesDir = () => '/non-existent/user/policies';
+  Storage.getSystemPoliciesDir = () => '/non-existent/system/policies';
+
+  return {
+    ...actual,
+    Storage,
+  };
+});
+
 describe('Policy Engine Integration Tests', () => {
   describe('Policy configuration produces valid PolicyEngine config', () => {
     it('should create a working PolicyEngine from basic settings', async () => {

packages/cli/src/config/trustedFolders.ts

@@ -161,7 +161,6 @@ export function loadTrustedFolders(): LoadedTrustedFolders {
   const userConfig: Record<string, TrustLevel> = {};
 
   const userPath = getTrustedFoldersPath();
-
   // Load user trusted folders
   try {
     if (fs.existsSync(userPath)) {

packages/core/src/hooks/hookPlanner.test.ts

@@ -7,8 +7,7 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { HookPlanner } from './hookPlanner.js';
 import type { HookRegistry, HookRegistryEntry } from './hookRegistry.js';
-import { HookEventName, HookType } from './types.js';
-import { ConfigSource } from './hookRegistry.js';
+import { ConfigSource, HookEventName, HookType } from './types.js';
 
 // Mock debugLogger using vi.hoisted
 const mockDebugLogger = vi.hoisted(() => ({

packages/core/src/hooks/hookRegistry.test.ts

@@ -6,9 +6,9 @@
 
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import * as fs from 'node:fs';
-import { HookRegistry, ConfigSource } from './hookRegistry.js';
+import { HookRegistry } from './hookRegistry.js';
 import type { Storage } from '../config/storage.js';
-import { HookEventName, HookType } from './types.js';
+import { ConfigSource, HookEventName, HookType } from './types.js';
 import type { Config } from '../config/config.js';
 import type { HookDefinition } from './types.js';
 
@@ -47,6 +47,7 @@ describe('HookRegistry', () => {
       getExtensions: vi.fn().mockReturnValue([]),
       getHooks: vi.fn().mockReturnValue({}),
       getDisabledHooks: vi.fn().mockReturnValue([]),
+      isTrustedFolder: vi.fn().mockReturnValue(true),
     } as unknown as Config;
 
     hookRegistry = new HookRegistry(mockConfig);
@@ -68,6 +69,35 @@ describe('HookRegistry', () => {
       );
     });
 
+    it('should not load hooks if folder is not trusted', async () => {
+      vi.mocked(mockConfig.isTrustedFolder).mockReturnValue(false);
+      const mockHooksConfig = {
+        BeforeTool: [
+          {
+            hooks: [
+              {
+                type: 'command',
+                command: './hooks/test.sh',
+              },
+            ],
+          },
+        ],
+      };
+
+      vi.mocked(mockConfig.getHooks).mockReturnValue(
+        mockHooksConfig as unknown as {
+          [K in HookEventName]?: HookDefinition[];
+        },
+      );
+
+      await hookRegistry.initialize();
+
+      expect(hookRegistry.getAllHooks()).toHaveLength(0);
+      expect(mockDebugLogger.warn).toHaveBeenCalledWith(
+        'Project hooks disabled because the folder is not trusted.',
+      );
+    });
+
     it('should load hooks from project configuration', async () => {
       const mockHooksConfig = {
         BeforeTool: [

packages/core/src/hooks/hookRegistry.ts

@@ -6,19 +6,9 @@
 
 import type { Config } from '../config/config.js';
 import type { HookDefinition, HookConfig } from './types.js';
-import { HookEventName } from './types.js';
+import { HookEventName, ConfigSource } from './types.js';
 import { debugLogger } from '../utils/debugLogger.js';
 
-/**
- * Configuration source levels in precedence order (highest to lowest)
- */
-export enum ConfigSource {
-  Project = 'project',
-  User = 'user',
-  System = 'system',
-  Extensions = 'extensions',
-}
-
 /**
  * Hook registry entry with source information
  */
@@ -111,7 +101,13 @@ export class HookRegistry {
     // Get hooks from the main config (this comes from the merged settings)
     const configHooks = this.config.getHooks();
     if (configHooks) {
-      this.processHooksConfiguration(configHooks, ConfigSource.Project);
+      if (this.config.isTrustedFolder()) {
+        this.processHooksConfiguration(configHooks, ConfigSource.Project);
+      } else {
+        debugLogger.warn(
+          'Project hooks disabled because the folder is not trusted.',
+        );
+      }
     }
 
     // Get hooks from extensions
@@ -189,6 +185,9 @@ export class HookRegistry {
         } as HookRegistryEntry);
         const isDisabled = disabledHooks.includes(hookName);
 
+        // Add source to hook config
+        hookConfig.source = source;
+
         this.entries.push({
           config: hookConfig,
           source,

packages/core/src/hooks/hookRunner.test.ts

@@ -11,6 +11,8 @@ import { HookEventName, HookType } from './types.js';
 import type { HookConfig } from './types.js';
 import type { HookInput } from './types.js';
 import type { Readable, Writable } from 'node:stream';
+import type { Config } from '../config/config.js';
+import { ConfigSource } from './types.js';
 
 // Mock type for the child_process spawn
 type MockChildProcessWithoutNullStreams = ChildProcessWithoutNullStreams & {
@@ -53,6 +55,7 @@ vi.stubGlobal('console', mockConsole);
 describe('HookRunner', () => {
   let hookRunner: HookRunner;
   let mockSpawn: MockChildProcessWithoutNullStreams;
+  let mockConfig: Config;
 
   const mockInput: HookInput = {
     session_id: 'test-session',
@@ -65,7 +68,11 @@ describe('HookRunner', () => {
   beforeEach(() => {
     vi.resetAllMocks();
 
-    hookRunner = new HookRunner();
+    mockConfig = {
+      isTrustedFolder: vi.fn().mockReturnValue(true),
+    } as unknown as Config;
+
+    hookRunner = new HookRunner(mockConfig);
 
     // Mock spawn with accessible mock functions
     const mockStdoutOn = vi.fn();
@@ -100,6 +107,89 @@ describe('HookRunner', () => {
   });
 
   describe('executeHook', () => {
+    describe('security checks', () => {
+      it('should block project hooks in untrusted folders', async () => {
+        vi.mocked(mockConfig.isTrustedFolder).mockReturnValue(false);
+
+        const projectHookConfig: HookConfig = {
+          type: HookType.Command,
+          command: './hooks/test.sh',
+          source: ConfigSource.Project,
+        };
+
+        const result = await hookRunner.executeHook(
+          projectHookConfig,
+          HookEventName.BeforeTool,
+          mockInput,
+        );
+
+        expect(result.success).toBe(false);
+        expect(result.error?.message).toContain(
+          'Security: Blocked execution of project hook in untrusted folder',
+        );
+        expect(mockDebugLogger.warn).toHaveBeenCalledWith(
+          expect.stringContaining('Security: Blocked execution'),
+        );
+        expect(spawn).not.toHaveBeenCalled();
+      });
+
+      it('should allow project hooks in trusted folders', async () => {
+        vi.mocked(mockConfig.isTrustedFolder).mockReturnValue(true);
+
+        const projectHookConfig: HookConfig = {
+          type: HookType.Command,
+          command: './hooks/test.sh',
+          source: ConfigSource.Project,
+        };
+
+        // Mock successful execution
+        mockSpawn.mockProcessOn.mockImplementation(
+          (event: string, callback: (code: number) => void) => {
+            if (event === 'close') {
+              setTimeout(() => callback(0), 10);
+            }
+          },
+        );
+
+        const result = await hookRunner.executeHook(
+          projectHookConfig,
+          HookEventName.BeforeTool,
+          mockInput,
+        );
+
+        expect(result.success).toBe(true);
+        expect(spawn).toHaveBeenCalled();
+      });
+
+      it('should allow non-project hooks even in untrusted folders', async () => {
+        vi.mocked(mockConfig.isTrustedFolder).mockReturnValue(false);
+
+        const systemHookConfig: HookConfig = {
+          type: HookType.Command,
+          command: './hooks/test.sh',
+          source: ConfigSource.System,
+        };
+
+        // Mock successful execution
+        mockSpawn.mockProcessOn.mockImplementation(
+          (event: string, callback: (code: number) => void) => {
+            if (event === 'close') {
+              setTimeout(() => callback(0), 10);
+            }
+          },
+        );
+
+        const result = await hookRunner.executeHook(
+          systemHookConfig,
+          HookEventName.BeforeTool,
+          mockInput,
+        );
+
+        expect(result.success).toBe(true);
+        expect(spawn).toHaveBeenCalled();
+      });
+    });
+
     describe('command hooks', () => {
       const commandConfig: HookConfig = {
         type: HookType.Command,

packages/core/src/hooks/hookRunner.ts

@@ -6,7 +6,8 @@
 
 import { spawn } from 'node:child_process';
 import type { HookConfig } from './types.js';
-import { HookEventName } from './types.js';
+import { HookEventName, ConfigSource } from './types.js';
+import type { Config } from '../config/config.js';
 import type {
   HookInput,
   HookOutput,
@@ -39,7 +40,11 @@ const EXIT_CODE_NON_BLOCKING_ERROR = 1;
  * Hook runner that executes command hooks
  */
 export class HookRunner {
-  constructor() {}
+  private readonly config: Config;
+
+  constructor(config: Config) {
+    this.config = config;
+  }
 
   /**
    * Execute a single hook
@@ -51,6 +56,23 @@ export class HookRunner {
   ): Promise<HookExecutionResult> {
     const startTime = Date.now();
 
+    // Secondary security check: Ensure project hooks are not executed in untrusted folders
+    if (
+      hookConfig.source === ConfigSource.Project &&
+      !this.config.isTrustedFolder()
+    ) {
+      const errorMessage =
+        'Security: Blocked execution of project hook in untrusted folder';
+      debugLogger.warn(errorMessage);
+      return {
+        hookConfig,
+        eventName,
+        success: false,
+        error: new Error(errorMessage),
+        duration: 0,
+      };
+    }
+
     try {
       return await this.executeCommandHook(
         hookConfig,

packages/core/src/hooks/hookSystem.ts

@@ -31,7 +31,7 @@ export class HookSystem {
 
     // Initialize components
     this.hookRegistry = new HookRegistry(config);
-    this.hookRunner = new HookRunner();
+    this.hookRunner = new HookRunner(config);
     this.hookAggregator = new HookAggregator();
     this.hookPlanner = new HookPlanner(this.hookRegistry);
     this.hookEventHandler = new HookEventHandler(

packages/core/src/hooks/index.ts

@@ -17,7 +17,7 @@ export { HookEventHandler } from './hookEventHandler.js';
 
 // Export interfaces and enums
 export type { HookRegistryEntry } from './hookRegistry.js';
-export { ConfigSource } from './hookRegistry.js';
+export { ConfigSource } from './types.js';
 export type { AggregatedHookResult } from './hookAggregator.js';
 export type { HookEventContext } from './hookPlanner.js';
 

packages/core/src/hooks/types.ts

@@ -17,6 +17,16 @@ import type {
 } from './hookTranslator.js';
 import { defaultHookTranslator } from './hookTranslator.js';
 
+/**
+ * Configuration source levels in precedence order (highest to lowest)
+ */
+export enum ConfigSource {
+  Project = 'project',
+  User = 'user',
+  System = 'system',
+  Extensions = 'extensions',
+}
+
 /**
  * Event names for the hook system
  */
@@ -43,6 +53,7 @@ export interface CommandHookConfig {
   name?: string;
   description?: string;
   timeout?: number;
+  source?: ConfigSource;
 }
 
 export type HookConfig = CommandHookConfig;