diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index f628497778..cebe5047ad 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -92,6 +92,13 @@ their corresponding top-level category object in your `settings.json` file. - **Default:** `[]` - **Requires restart:** Yes +#### `adminPolicyPaths` + +- **`adminPolicyPaths`** (array): + - **Description:** Additional admin policy files or directories to load. + - **Default:** `[]` + - **Requires restart:** Yes + #### `general` - **`general.preferredEditor`** (string): diff --git a/docs/reference/policy-engine.md b/docs/reference/policy-engine.md index 2ea23d4be4..54db8dec2e 100644 --- a/docs/reference/policy-engine.md +++ b/docs/reference/policy-engine.md @@ -191,9 +191,13 @@ User, and (if configured) Admin directories. #### System-wide policies (Admin) -Administrators can enforce system-wide policies (Tier 3) that override all user -and default settings. These policies must be placed in specific, secure -directories: +Administrators can enforce system-wide policies (Tier 4) that override all user +and default settings. These policies can be loaded from standard system +locations or supplemental paths. + +##### Standard Locations + +These are the default paths the CLI searches for admin policies: | OS | Policy Directory Path | | :---------- | :------------------------------------------------ | @@ -201,10 +205,25 @@ directories: | **macOS** | `/Library/Application Support/GeminiCli/policies` | | **Windows** | `C:\ProgramData\gemini-cli\policies` | -**Security Requirements:** +##### Supplemental Admin Policies -To prevent privilege escalation, the CLI enforces strict security checks on -admin directories. If checks fail, system policies are **ignored**. +Administrators can also specify supplemental policy paths using: + +- The `--admin-policy` command-line flag. +- The `adminPolicyPaths` setting in a system settings file. + +These supplemental policies are assigned the same **Admin** tier (Base 4) as +policies in standard locations. + +**Security Guard**: Supplemental admin policies are **ignored** if any `.toml` +policy files are found in the standard system location. This prevents flag-based +overrides when a central system policy has already been established. + +#### Security Requirements + +To prevent privilege escalation, the CLI enforces strict security checks on the +**standard system policy directory**. If checks fail, the policies in that +directory are **ignored**. - **Linux / macOS:** Must be owned by `root` (UID 0) and NOT writable by group or others (e.g., `chmod 755`). @@ -214,6 +233,11 @@ admin directories. If checks fail, system policies are **ignored**. for non-admin groups. You may need to "Disable inheritance" in Advanced Security Settings._ +**Note:** Supplemental admin policies (provided via `--admin-policy` or +`adminPolicyPaths` settings) are **NOT** subject to these strict ownership +checks, as they are explicitly provided by the user or administrator in their +current execution context. + ### TOML rule schema Here is a breakdown of the fields available in a TOML policy rule: diff --git a/package-lock.json b/package-lock.json index 7e0a853d15..0dc1ce4951 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2195,6 +2195,7 @@ "integrity": "sha512-t54CUOsFMappY1Jbzb7fetWeO0n6K0k/4+/ZpkS+3Joz8I4VcvY9OiEBFRYISqaI2fq5sCiPtAjRDOzVYG8m+Q==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@octokit/auth-token": "^6.0.0", "@octokit/graphql": "^9.0.2", @@ -2375,6 +2376,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/api/-/api-1.9.0.tgz", "integrity": "sha512-3giAOQvZiH5F9bMlMiv8+GSPMeqg0dbaeo58/0SlA9sxSqZhnUtxzX9/2FzyhS9sWQf5S0GJE0AKBrFqjpeYcg==", "license": "Apache-2.0", + "peer": true, "engines": { "node": ">=8.0.0" } @@ -2424,6 +2426,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/core/-/core-2.5.0.tgz", "integrity": "sha512-ka4H8OM6+DlUhSAZpONu0cPBtPPTQKxbxVzC4CzVx5+K4JnroJVBtDzLAMx4/3CDTJXRvVFhpFjtl4SaiTNoyQ==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@opentelemetry/semantic-conventions": "^1.29.0" }, @@ -2798,6 +2801,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/resources/-/resources-2.5.0.tgz", "integrity": "sha512-F8W52ApePshpoSrfsSk1H2yJn9aKjCrbpQF1M9Qii0GHzbfVeFUB+rc3X4aggyZD8x9Gu3Slua+s6krmq6Dt8g==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@opentelemetry/core": "2.5.0", "@opentelemetry/semantic-conventions": "^1.29.0" @@ -2831,6 +2835,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-metrics/-/sdk-metrics-2.5.0.tgz", "integrity": "sha512-BeJLtU+f5Gf905cJX9vXFQorAr6TAfK3SPvTFqP+scfIpDQEJfRaGJWta7sJgP+m4dNtBf9y3yvBKVAZZtJQVA==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@opentelemetry/core": "2.5.0", "@opentelemetry/resources": "2.5.0" @@ -2885,6 +2890,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-trace-base/-/sdk-trace-base-2.5.0.tgz", "integrity": "sha512-VzRf8LzotASEyNDUxTdaJ9IRJ1/h692WyArDBInf5puLCjxbICD6XkHgpuudis56EndyS7LYFmtTMny6UABNdQ==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@opentelemetry/core": "2.5.0", "@opentelemetry/resources": "2.5.0", @@ -4048,6 +4054,7 @@ "integrity": "sha512-6mDvHUFSjyT2B2yeNx2nUgMxh9LtOWvkhIU3uePn2I2oyNymUAX1NIsdgviM4CH+JSrp2D2hsMvJOkxY+0wNRA==", "devOptional": true, "license": "MIT", + "peer": true, "dependencies": { "csstype": "^3.0.2" } @@ -4322,6 +4329,7 @@ "integrity": "sha512-6sMvZePQrnZH2/cJkwRpkT7DxoAWh+g6+GFRK6bV3YQo7ogi3SX5rgF6099r5Q53Ma5qeT7LGmOmuIutF4t3lA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.35.0", "@typescript-eslint/types": "8.35.0", @@ -5195,6 +5203,7 @@ "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.15.0.tgz", "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -7726,6 +7735,7 @@ "integrity": "sha512-GsGizj2Y1rCWDu6XoEekL3RLilp0voSePurjZIkxL3wlm5o5EC9VpgaP7lrCvjnkuLvzFBQWB3vWB3K5KQTveQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.2.0", "@eslint-community/regexpp": "^4.12.1", @@ -8236,6 +8246,7 @@ "resolved": "https://registry.npmjs.org/express/-/express-5.2.1.tgz", "integrity": "sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==", "license": "MIT", + "peer": true, "dependencies": { "accepts": "^2.0.0", "body-parser": "^2.2.1", @@ -9503,6 +9514,7 @@ "resolved": "https://registry.npmjs.org/hono/-/hono-4.11.9.tgz", "integrity": "sha512-Eaw2YTGM6WOxA6CXbckaEvslr2Ne4NFsKrvc0v97JD5awbmeBLO5w9Ho9L9kmKonrwF9RJlW6BxT1PVv/agBHQ==", "license": "MIT", + "peer": true, "engines": { "node": ">=16.9.0" } @@ -9782,6 +9794,7 @@ "resolved": "https://registry.npmjs.org/@jrichman/ink/-/ink-6.4.11.tgz", "integrity": "sha512-93LQlzT7vvZ1XJcmOMwN4s+6W334QegendeHOMnEJBlhnpIzr8bws6/aOEHG8ZCuVD/vNeeea5m1msHIdAY6ig==", "license": "MIT", + "peer": true, "dependencies": { "@alcalzone/ansi-tokenize": "^0.2.1", "ansi-escapes": "^7.0.0", @@ -13368,6 +13381,7 @@ "resolved": "https://registry.npmjs.org/react/-/react-19.2.4.tgz", "integrity": "sha512-9nfp2hYpCwOjAN+8TZFGhtWEwgvWHXqESH8qT89AT/lWklpLON22Lc8pEtnpsZz7VmawabSU0gCjnj8aC0euHQ==", "license": "MIT", + "peer": true, "engines": { "node": ">=0.10.0" } @@ -13378,6 +13392,7 @@ "integrity": "sha512-ePrwPfxAnB+7hgnEr8vpKxL9cmnp7F322t8oqcPshbIQQhDKgFDW4tjhF2wjVbdXF9O/nyuy3sQWd9JGpiLPvA==", "devOptional": true, "license": "MIT", + "peer": true, "dependencies": { "shell-quote": "^1.6.1", "ws": "^7" @@ -15422,6 +15437,7 @@ "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -15645,7 +15661,8 @@ "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.8.1.tgz", "integrity": "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==", "dev": true, - "license": "0BSD" + "license": "0BSD", + "peer": true }, "node_modules/tsx": { "version": "4.20.3", @@ -15653,6 +15670,7 @@ "integrity": "sha512-qjbnuR9Tr+FJOMBqJCW5ehvIo/buZq7vH7qD7JziU98h6l3qGy0a/yPFjwO+y0/T7GFpNgNAvEcPPVfyT8rrPQ==", "devOptional": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "~0.25.0", "get-tsconfig": "^4.7.5" @@ -15812,6 +15830,7 @@ "integrity": "sha512-p1diW6TqL9L07nNxvRMM7hMMw4c5XOo/1ibL4aAIGmSAt9slTE1Xgw5KWuof2uTOvCg9BY7ZRi+GaF+7sfgPeQ==", "devOptional": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -16034,6 +16053,7 @@ "resolved": "https://registry.npmjs.org/vite/-/vite-7.2.2.tgz", "integrity": "sha512-BxAKBWmIbrDgrokdGZH1IgkIk/5mMHDreLDmCJ0qpyJaAteP8NvMhkwr/ZCQNqNH97bw/dANTE9PDzqwJghfMQ==", "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.5.0", @@ -16147,6 +16167,7 @@ "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -16159,6 +16180,7 @@ "resolved": "https://registry.npmjs.org/vitest/-/vitest-3.2.4.tgz", "integrity": "sha512-LUCP5ev3GURDysTWiP47wRRUpLKMOfPh+yKTx3kVIEiu5KOMeqzpnYNsKyOoVrULivR8tLcks4+lga33Whn90A==", "license": "MIT", + "peer": true, "dependencies": { "@types/chai": "^5.2.2", "@vitest/expect": "3.2.4", @@ -16800,6 +16822,7 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", "license": "MIT", + "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } @@ -17336,6 +17359,7 @@ "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 320e47a380..2ebc4d4b22 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -76,6 +76,7 @@ export interface CliArgs { yolo: boolean | undefined; approvalMode: string | undefined; policy: string[] | undefined; + adminPolicy: string[] | undefined; allowedMcpServerNames: string[] | undefined; allowedTools: string[] | undefined; acp?: boolean; @@ -97,6 +98,21 @@ export interface CliArgs { isCommand: boolean | undefined; } +/** + * Helper to coerce comma-separated or multiple flag values into a flat array. + */ +const coerceCommaSeparated = (values: string[]): string[] => { + if (values.length === 1 && values[0] === '') { + return ['']; + } + return values.flatMap((v) => + v + .split(',') + .map((s) => s.trim()) + .filter(Boolean), + ); +}; + export async function parseArguments( settings: MergedSettings, ): Promise { @@ -166,14 +182,15 @@ export async function parseArguments( nargs: 1, description: 'Additional policy files or directories to load (comma-separated or multiple --policy)', - coerce: (policies: string[]) => - // Handle comma-separated values - policies.flatMap((p) => - p - .split(',') - .map((s) => s.trim()) - .filter(Boolean), - ), + coerce: coerceCommaSeparated, + }) + .option('admin-policy', { + type: 'array', + string: true, + nargs: 1, + description: + 'Additional admin policy files or directories to load (comma-separated or multiple --admin-policy)', + coerce: coerceCommaSeparated, }) .option('acp', { type: 'boolean', @@ -189,11 +206,7 @@ export async function parseArguments( string: true, nargs: 1, description: 'Allowed MCP server names', - coerce: (mcpServerNames: string[]) => - // Handle comma-separated values - mcpServerNames.flatMap((mcpServerName) => - mcpServerName.split(',').map((m) => m.trim()), - ), + coerce: coerceCommaSeparated, }) .option('allowed-tools', { type: 'array', @@ -201,9 +214,7 @@ export async function parseArguments( nargs: 1, description: '[DEPRECATED: Use Policy Engine instead See https://geminicli.com/docs/core/policy-engine] Tools that are allowed to run without confirmation', - coerce: (tools: string[]) => - // Handle comma-separated values - tools.flatMap((tool) => tool.split(',').map((t) => t.trim())), + coerce: coerceCommaSeparated, }) .option('extensions', { alias: 'e', @@ -212,11 +223,7 @@ export async function parseArguments( nargs: 1, description: 'A list of extensions to use. If not provided, all extensions are used.', - coerce: (extensions: string[]) => - // Handle comma-separated values - extensions.flatMap((extension) => - extension.split(',').map((e) => e.trim()), - ), + coerce: coerceCommaSeparated, }) .option('list-extensions', { alias: 'l', @@ -258,9 +265,7 @@ export async function parseArguments( nargs: 1, description: 'Additional directories to include in the workspace (comma-separated or multiple --include-directories)', - coerce: (dirs: string[]) => - // Handle comma-separated values - dirs.flatMap((dir) => dir.split(',').map((d) => d.trim())), + coerce: coerceCommaSeparated, }) .option('screen-reader', { type: 'boolean', @@ -643,7 +648,8 @@ export async function loadCliConfig( ...settings.mcp, allowed: argv.allowedMcpServerNames ?? settings.mcp?.allowed, }, - policyPaths: argv.policy, + policyPaths: argv.policy ?? settings.policyPaths, + adminPolicyPaths: argv.adminPolicy ?? settings.adminPolicyPaths, }; const { workspacePoliciesDir, policyUpdateConfirmationRequest } = diff --git a/packages/cli/src/config/policy.ts b/packages/cli/src/config/policy.ts index bc22c928f8..4bbd396fba 100644 --- a/packages/cli/src/config/policy.ts +++ b/packages/cli/src/config/policy.ts @@ -61,6 +61,7 @@ export async function createPolicyEngineConfig( tools: settings.tools, mcpServers: settings.mcpServers, policyPaths: settings.policyPaths, + adminPolicyPaths: settings.adminPolicyPaths, workspacePoliciesDir, }; diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 390fa9d48a..007274dafc 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -134,6 +134,18 @@ export interface SettingsSchema { export type MemoryImportFormat = 'tree' | 'flat'; export type DnsResolutionOrder = 'ipv4first' | 'verbatim'; +const pathArraySetting = (label: string, description: string) => ({ + type: 'array' as const, + label, + category: 'Advanced' as const, + requiresRestart: true as const, + default: [] as string[], + description, + showInDialog: false as const, + items: { type: 'string' as const }, + mergeStrategy: MergeStrategy.UNION, +}); + /** * The canonical schema for all settings. * The structure of this object defines the structure of the `Settings` type. @@ -156,17 +168,15 @@ const SETTINGS_SCHEMA = { }, }, - policyPaths: { - type: 'array', - label: 'Policy Paths', - category: 'Advanced', - requiresRestart: true, - default: [] as string[], - description: 'Additional policy files or directories to load.', - showInDialog: false, - items: { type: 'string' }, - mergeStrategy: MergeStrategy.UNION, - }, + policyPaths: pathArraySetting( + 'Policy Paths', + 'Additional policy files or directories to load.', + ), + + adminPolicyPaths: pathArraySetting( + 'Admin Policy Paths', + 'Additional admin policy files or directories to load.', + ), general: { type: 'object', @@ -2677,7 +2687,9 @@ type InferSettings = { ? boolean : T[K]['default'] extends string ? string - : T[K]['default']; + : T[K]['default'] extends ReadonlyArray + ? U[] + : T[K]['default']; }; type InferMergedSettings = { @@ -2691,7 +2703,9 @@ type InferMergedSettings = { ? boolean : T[K]['default'] extends string ? string - : T[K]['default']; + : T[K]['default'] extends ReadonlyArray + ? U[] + : T[K]['default']; }; export type Settings = InferSettings; diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index 90c63651e7..9ae9da0dc3 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -481,6 +481,7 @@ describe('gemini.tsx main function kitty protocol', () => { yolo: undefined, approvalMode: undefined, policy: undefined, + adminPolicy: undefined, allowedMcpServerNames: undefined, allowedTools: undefined, experimentalAcp: undefined, diff --git a/packages/cli/src/ui/components/__snapshots__/ConfigInitDisplay.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/ConfigInitDisplay.test.tsx.snap index 1b14fadf55..28929deee5 100644 --- a/packages/cli/src/ui/components/__snapshots__/ConfigInitDisplay.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/ConfigInitDisplay.test.tsx.snap @@ -29,3 +29,9 @@ exports[`ConfigInitDisplay > updates message on McpClientUpdate event 1`] = ` Spinner Connecting to MCP servers... (1/2) - Waiting for: server2 " `; + +exports[`ConfigInitDisplay > updates message on McpClientUpdate event 2`] = ` +" +Spinner Connecting to MCP servers... (1/2) - Waiting for: server2 +" +`; diff --git a/packages/core/src/policy/config.test.ts b/packages/core/src/policy/config.test.ts index 9cee9997b2..42a76e9fe5 100644 --- a/packages/core/src/policy/config.test.ts +++ b/packages/core/src/policy/config.test.ts @@ -5,8 +5,9 @@ */ import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'; - import nodePath from 'node:path'; +import * as fs from 'node:fs/promises'; +import { type Dirent, type Stats, type PathLike } from 'node:fs'; import { ApprovalMode, @@ -15,6 +16,13 @@ import { type PolicySettings, } from './types.js'; import { isDirectorySecure } from '../utils/security.js'; +import { + createPolicyEngineConfig, + clearEmittedPolicyWarnings, +} from './config.js'; +import { Storage } from '../config/storage.js'; +import * as tomlLoader from './toml-loader.js'; +import { coreEvents } from '../utils/events.js'; vi.unmock('../config/storage.js'); @@ -22,29 +30,98 @@ vi.mock('../utils/security.js', () => ({ isDirectorySecure: vi.fn().mockResolvedValue({ secure: true }), })); +vi.mock('node:fs/promises', async (importOriginal) => { + const actual = await importOriginal(); + const mockFs = { + ...actual, + readdir: vi.fn(actual.readdir), + readFile: vi.fn(actual.readFile), + stat: vi.fn(actual.stat), + mkdir: vi.fn(actual.mkdir), + open: vi.fn(actual.open), + rename: vi.fn(actual.rename), + }; + return { + ...mockFs, + default: mockFs, + }; +}); + afterEach(() => { - vi.clearAllMocks(); - vi.restoreAllMocks(); - vi.doUnmock('node:fs/promises'); + vi.resetAllMocks(); }); describe('createPolicyEngineConfig', () => { + const MOCK_DEFAULT_DIR = '/tmp/mock/default/policies'; + beforeEach(async () => { - vi.resetModules(); - const { Storage } = await import('../config/storage.js'); - // Mock Storage to avoid picking up real user/system policies from the host environment + clearEmittedPolicyWarnings(); + // Mock Storage to avoid host environment contamination vi.spyOn(Storage, 'getUserPoliciesDir').mockReturnValue( '/non/existent/user/policies', ); vi.spyOn(Storage, 'getSystemPoliciesDir').mockReturnValue( '/non/existent/system/policies', ); - // Reset security check to default secure vi.mocked(isDirectorySecure).mockResolvedValue({ secure: true }); }); + /** + * Helper to mock a policy file in the filesystem. + */ + function mockPolicyFile(path: string, content: string) { + vi.mocked( + fs.readdir as (path: PathLike) => Promise, + ).mockImplementation(async (p) => { + if (nodePath.resolve(p.toString()) === nodePath.dirname(path)) { + return [ + { + name: nodePath.basename(path), + isFile: () => true, + isDirectory: () => false, + } as unknown as Dirent, + ]; + } + return ( + await vi.importActual( + 'node:fs/promises', + ) + ).readdir(p); + }); + + vi.mocked(fs.stat).mockImplementation(async (p) => { + if (nodePath.resolve(p.toString()) === nodePath.dirname(path)) { + return { + isDirectory: () => true, + isFile: () => false, + } as unknown as Stats; + } + if (nodePath.resolve(p.toString()) === path) { + return { + isDirectory: () => false, + isFile: () => true, + } as unknown as Stats; + } + return ( + await vi.importActual( + 'node:fs/promises', + ) + ).stat(p); + }); + + vi.mocked(fs.readFile).mockImplementation(async (p) => { + if (nodePath.resolve(p.toString()) === path) { + return content; + } + return ( + await vi.importActual( + 'node:fs/promises', + ) + ).readFile(p); + }); + } + 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); @@ -55,117 +132,71 @@ describe('createPolicyEngineConfig', () => { 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 = {}; + const loadPoliciesSpy = vi + .spyOn(tomlLoader, 'loadPoliciesFromToml') + .mockResolvedValue({ rules: [], checkers: [], errors: [] }); 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 NOT filter out insecure supplemental admin policy directories', async () => { + const adminPolicyDir = '/insecure/admin/policies'; + vi.mocked(isDirectorySecure).mockImplementation(async (path: string) => { + if (nodePath.resolve(path) === nodePath.resolve(adminPolicyDir)) { + return { secure: false, reason: 'Insecure directory' }; + } + return { secure: true }; + }); + + const loadPoliciesSpy = vi + .spyOn(tomlLoader, 'loadPoliciesFromToml') + .mockResolvedValue({ rules: [], checkers: [], errors: [] }); + + await createPolicyEngineConfig( + { adminPolicyPaths: [adminPolicyDir] }, + ApprovalMode.DEFAULT, + '/tmp/mock/default/policies', + ); + + const calledDirs = loadPoliciesSpy.mock.calls[0][0]; + expect(calledDirs).toContain(adminPolicyDir); + expect(calledDirs).toContain('/non/existent/system/policies'); 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( - 'node:fs/promises', - ); + vi.mocked( + fs.readdir as (path: PathLike) => Promise, + ).mockResolvedValue([]); - const mockReaddir = vi.fn( - async ( - path: string | Buffer | URL, - options?: Parameters[1], - ) => { - if ( - typeof path === 'string' && - nodePath - .normalize(path) - .includes(nodePath.normalize('.gemini/policies')) - ) { - // Return empty array for user policies - return [] as unknown as Awaited>; - } - return actualFs.readdir( - path, - options as Parameters[1], - ); - }, - ); - - vi.doMock('node:fs/promises', () => ({ - ...actualFs, - default: { ...actualFs, readdir: mockReaddir }, - readdir: mockReaddir, - })); - - // Mock Storage to avoid actual filesystem access for policy dirs during tests if needed, - // but for now relying on the fs mock above might be enough if it catches the right paths. - // Let's see if we need to mock Storage.getUserPoliciesDir etc. - - vi.resetModules(); - const { createPolicyEngineConfig } = await import('./config.js'); - - const settings: PolicySettings = {}; - // Pass a dummy default policies dir to avoid it trying to resolve __dirname relative to the test file in a weird way const config = await createPolicyEngineConfig( - settings, + {}, ApprovalMode.DEFAULT, - '/tmp/mock/default/policies', + MOCK_DEFAULT_DIR, ); expect(config.defaultDecision).toBe(PolicyDecision.ASK_USER); - // The order of the rules is not guaranteed, so we sort them by tool name. - config.rules?.sort((a, b) => - (a.toolName ?? '').localeCompare(b.toolName ?? ''), - ); - - // Since we are mocking an empty policy directory, we expect NO rules from TOML. - // Wait, the CLI test expected a bunch of default rules. Those must have come from - // the actual default policies directory in the CLI package. - // In the core package, we don't necessarily have those default policy files yet - // or we need to point to them. - // For this unit test, if we mock the default dir as empty, we should get NO rules - // if no settings are provided. - - // Actually, let's look at how CLI test gets them. It uses `__dirname` in `policy.ts`. - // If we want to test default rules, we need to provide them. - // For now, let's assert it's empty if we provide no TOML files, to ensure the *mechanism* works. - // Or better, mock one default rule to ensure it's loaded. - expect(config.rules).toEqual([]); - - vi.doUnmock('node:fs/promises'); - }, 30000); + }); it('should allow tools in tools.allowed', async () => { - const { createPolicyEngineConfig } = await import('./config.js'); - const settings: PolicySettings = { - tools: { allowed: ['run_shell_command'] }, - }; + vi.mocked( + fs.readdir as (path: PathLike) => Promise, + ).mockResolvedValue([]); const config = await createPolicyEngineConfig( - settings, + { tools: { allowed: ['run_shell_command'] } }, ApprovalMode.DEFAULT, - '/tmp/mock/default/policies', + MOCK_DEFAULT_DIR, ); const rule = config.rules?.find( (r) => @@ -177,14 +208,10 @@ describe('createPolicyEngineConfig', () => { }); it('should deny tools in tools.exclude', async () => { - const { createPolicyEngineConfig } = await import('./config.js'); - const settings: PolicySettings = { - tools: { exclude: ['run_shell_command'] }, - }; const config = await createPolicyEngineConfig( - settings, + { tools: { exclude: ['run_shell_command'] } }, ApprovalMode.DEFAULT, - '/tmp/mock/default/policies', + MOCK_DEFAULT_DIR, ); const rule = config.rules?.find( (r) => @@ -196,14 +223,10 @@ describe('createPolicyEngineConfig', () => { }); it('should allow tools from allowed MCP servers', async () => { - const { createPolicyEngineConfig } = await import('./config.js'); - const settings: PolicySettings = { - mcp: { allowed: ['my-server'] }, - }; const config = await createPolicyEngineConfig( - settings, + { mcp: { allowed: ['my-server'] } }, ApprovalMode.DEFAULT, - '/tmp/mock/default/policies', + MOCK_DEFAULT_DIR, ); const rule = config.rules?.find( (r) => r.mcpName === 'my-server' && r.decision === PolicyDecision.ALLOW, @@ -213,14 +236,10 @@ describe('createPolicyEngineConfig', () => { }); it('should deny tools from excluded MCP servers', async () => { - const { createPolicyEngineConfig } = await import('./config.js'); - const settings: PolicySettings = { - mcp: { excluded: ['my-server'] }, - }; const config = await createPolicyEngineConfig( - settings, + { mcp: { excluded: ['my-server'] } }, ApprovalMode.DEFAULT, - '/tmp/mock/default/policies', + MOCK_DEFAULT_DIR, ); const rule = config.rules?.find( (r) => r.mcpName === 'my-server' && r.decision === PolicyDecision.DENY, @@ -230,21 +249,15 @@ describe('createPolicyEngineConfig', () => { }); it('should allow tools from trusted MCP servers', async () => { - const { createPolicyEngineConfig } = await import('./config.js'); - const settings: PolicySettings = { - mcpServers: { - 'trusted-server': { - trust: true, - }, - 'untrusted-server': { - trust: false, + const config = await createPolicyEngineConfig( + { + mcpServers: { + 'trusted-server': { trust: true }, + 'untrusted-server': { trust: false }, }, }, - }; - const config = await createPolicyEngineConfig( - settings, ApprovalMode.DEFAULT, - '/tmp/mock/default/policies', + MOCK_DEFAULT_DIR, ); const trustedRule = config.rules?.find( @@ -263,22 +276,13 @@ describe('createPolicyEngineConfig', () => { }); it('should handle multiple MCP server configurations together', async () => { - const { createPolicyEngineConfig } = await import('./config.js'); - const settings: PolicySettings = { - mcp: { - allowed: ['allowed-server'], - excluded: ['excluded-server'], - }, - mcpServers: { - 'trusted-server': { - trust: true, - }, - }, - }; const config = await createPolicyEngineConfig( - settings, + { + mcp: { allowed: ['allowed-server'], excluded: ['excluded-server'] }, + mcpServers: { 'trusted-server': { trust: true } }, + }, ApprovalMode.DEFAULT, - '/tmp/mock/default/policies', + MOCK_DEFAULT_DIR, ); // Check allowed server @@ -307,24 +311,16 @@ describe('createPolicyEngineConfig', () => { }); it('should allow all tools in YOLO mode', async () => { - const { createPolicyEngineConfig } = await import('./config.js'); - const settings: PolicySettings = {}; - const config = await createPolicyEngineConfig(settings, ApprovalMode.YOLO); + const config = await createPolicyEngineConfig({}, ApprovalMode.YOLO); const rule = config.rules?.find( (r) => r.decision === PolicyDecision.ALLOW && !r.toolName, ); expect(rule).toBeDefined(); - // Priority 998 in default tier → 1.998 (999 reserved for ask_user exception) expect(rule?.priority).toBeCloseTo(1.998, 5); }); it('should allow edit tool in AUTO_EDIT mode', async () => { - const { createPolicyEngineConfig } = await import('./config.js'); - const settings: PolicySettings = {}; - const config = await createPolicyEngineConfig( - settings, - ApprovalMode.AUTO_EDIT, - ); + const config = await createPolicyEngineConfig({}, ApprovalMode.AUTO_EDIT); const rule = config.rules?.find( (r) => r.toolName === 'replace' && @@ -332,19 +328,19 @@ describe('createPolicyEngineConfig', () => { r.modes?.includes(ApprovalMode.AUTO_EDIT), ); expect(rule).toBeDefined(); - // Priority 15 in default tier → 1.015 expect(rule?.priority).toBeCloseTo(1.015, 5); }); it('should prioritize exclude over allow', async () => { - const { createPolicyEngineConfig } = await import('./config.js'); - const settings: PolicySettings = { - tools: { allowed: ['run_shell_command'], exclude: ['run_shell_command'] }, - }; const config = await createPolicyEngineConfig( - settings, + { + tools: { + allowed: ['run_shell_command'], + exclude: ['run_shell_command'], + }, + }, ApprovalMode.DEFAULT, - '/tmp/mock/default/policies', + MOCK_DEFAULT_DIR, ); const denyRule = config.rules?.find( (r) => @@ -356,13 +352,10 @@ describe('createPolicyEngineConfig', () => { r.toolName === 'run_shell_command' && r.decision === PolicyDecision.ALLOW, ); - expect(denyRule).toBeDefined(); - expect(allowRule).toBeDefined(); expect(denyRule!.priority).toBeGreaterThan(allowRule!.priority!); }); it('should prioritize specific tool allows over MCP server excludes', async () => { - const { createPolicyEngineConfig } = await import('./config.js'); const settings: PolicySettings = { mcp: { excluded: ['my-server'] }, tools: { allowed: ['mcp_my-server_specific-tool'] }, @@ -370,7 +363,7 @@ describe('createPolicyEngineConfig', () => { const config = await createPolicyEngineConfig( settings, ApprovalMode.DEFAULT, - '/tmp/mock/default/policies', + MOCK_DEFAULT_DIR, ); const serverDenyRule = config.rules?.find( @@ -425,6 +418,11 @@ describe('createPolicyEngineConfig', () => { }); it('should handle complex priority scenarios correctly', async () => { + mockPolicyFile( + nodePath.join(MOCK_DEFAULT_DIR, 'default.toml'), + '[[rule]]\ntoolName = "glob"\ndecision = "allow"\npriority = 50\n', + ); + const settings: PolicySettings = { tools: { allowed: ['mcp_trusted-server_tool1', 'other-tool'], // Priority 4.3 @@ -441,68 +439,12 @@ describe('createPolicyEngineConfig', () => { }, }; - // Mock a default policy for 'glob' to test priority override - const actualFs = - await vi.importActual( - 'node:fs/promises', - ); - const mockReaddir = vi.fn(async (p, _o) => { - if (typeof p === 'string' && p.includes('/tmp/mock/default/policies')) { - return [ - { - name: 'default.toml', - isFile: () => true, - isDirectory: () => false, - }, - ] as unknown as Awaited>; - } - return []; - }); - const mockStat = vi.fn(async (p) => { - if (typeof p === 'string' && p.includes('/tmp/mock/default/policies')) { - return { - isDirectory: () => true, - isFile: () => false, - } as unknown as Awaited>; - } - if (typeof p === 'string' && p.includes('default.toml')) { - return { - isDirectory: () => false, - isFile: () => true, - } as unknown as Awaited>; - } - return actualFs.stat(p); - }); - const mockReadFile = vi.fn(async (p, _o) => { - if (typeof p === 'string' && p.includes('default.toml')) { - return '[[rule]]\ntoolName = "glob"\ndecision = "allow"\npriority = 50\n'; - } - return ''; - }); - vi.doMock('node:fs/promises', () => ({ - ...actualFs, - default: { - ...actualFs, - readdir: mockReaddir, - readFile: mockReadFile, - stat: mockStat, - }, - readdir: mockReaddir, - readFile: mockReadFile, - stat: mockStat, - })); - vi.resetModules(); - const { createPolicyEngineConfig: createConfig } = await import( - './config.js' - ); - - const config = await createConfig( + const config = await createPolicyEngineConfig( settings, ApprovalMode.DEFAULT, - '/tmp/mock/default/policies', + MOCK_DEFAULT_DIR, ); - // Verify glob is denied even though default would allow it const globDenyRule = config.rules?.find( (r) => r.toolName === 'glob' && r.decision === PolicyDecision.DENY, ); @@ -534,26 +476,18 @@ describe('createPolicyEngineConfig', () => { expect( highestPriorityExcludes?.every((p) => p.decision === PolicyDecision.DENY), ).toBe(true); - - vi.doUnmock('node:fs/promises'); }); it('should handle MCP servers with undefined trust property', async () => { - const { createPolicyEngineConfig } = await import('./config.js'); - const settings: PolicySettings = { - mcpServers: { - 'no-trust-property': { - // trust property is undefined/missing - }, - 'explicit-false': { - trust: false, + const config = await createPolicyEngineConfig( + { + mcpServers: { + 'no-trust-property': {}, + 'explicit-false': { trust: false }, }, }, - }; - const config = await createPolicyEngineConfig( - settings, ApprovalMode.DEFAULT, - '/tmp/mock/default/policies', + MOCK_DEFAULT_DIR, ); // Neither server should have an allow rule @@ -572,48 +506,24 @@ describe('createPolicyEngineConfig', () => { }); it('should have YOLO allow-all rule beat write tool rules in YOLO mode', async () => { - vi.resetModules(); - vi.doUnmock('node:fs/promises'); - const { createPolicyEngineConfig: createConfig } = await import( - './config.js' - ); - // Re-mock Storage after resetModules because it was reloaded - const { Storage: FreshStorage } = await import('../config/storage.js'); - vi.spyOn(FreshStorage, 'getUserPoliciesDir').mockReturnValue( - '/non/existent/user/policies', - ); - vi.spyOn(FreshStorage, 'getSystemPoliciesDir').mockReturnValue( - '/non/existent/system/policies', + const config = await createPolicyEngineConfig( + { tools: { exclude: ['dangerous-tool'] } }, + ApprovalMode.YOLO, ); - const settings: PolicySettings = { - tools: { exclude: ['dangerous-tool'] }, - }; - // Use default policy dir (no third arg) to load real yolo.toml and write.toml - const config = await createConfig(settings, ApprovalMode.YOLO); - - // Should have the wildcard allow rule const wildcardRule = config.rules?.find( (r) => !r.toolName && r.decision === PolicyDecision.ALLOW, ); - expect(wildcardRule).toBeDefined(); - // Priority 998 in default tier → 1.998 (999 reserved for ask_user exception) - expect(wildcardRule?.priority).toBeCloseTo(1.998, 5); - - // Write tool ASK_USER rules are present (from write.toml) const writeToolRules = config.rules?.filter( (r) => ['run_shell_command'].includes(r.toolName || '') && r.decision === PolicyDecision.ASK_USER, ); - expect(writeToolRules).toBeDefined(); - expect(writeToolRules?.length).toBeGreaterThan(0); - // But YOLO allow-all rule has higher priority than all write tool rules + expect(wildcardRule).toBeDefined(); writeToolRules?.forEach((writeRule) => { expect(wildcardRule!.priority).toBeGreaterThan(writeRule.priority!); }); - // Should still have the exclude rule (from settings, user tier) const excludeRule = config.rules?.find( (r) => @@ -624,101 +534,21 @@ describe('createPolicyEngineConfig', () => { }); it('should support argsPattern in policy rules', async () => { - const actualFs = - await vi.importActual( - 'node:fs/promises', - ); - - const mockReaddir = vi.fn( - async ( - path: string | Buffer | URL, - options?: Parameters[1], - ) => { - if ( - typeof path === 'string' && - nodePath - .normalize(path) - .includes(nodePath.normalize('.gemini/policies')) - ) { - return [ - { - name: 'write.toml', - isFile: () => true, - isDirectory: () => false, - }, - ] as unknown as Awaited>; - } - return actualFs.readdir( - path, - options as Parameters[1], - ); - }, + mockPolicyFile( + nodePath.join(MOCK_DEFAULT_DIR, 'write.toml'), + ` + [[rule]] + toolName = "run_shell_command" + argsPattern = "\\"command\\":\\"git (status|diff|log)\\"" + decision = "allow" + priority = 150 + `, ); - const mockReadFile = vi.fn( - async ( - path: Parameters[0], - options: Parameters[1], - ) => { - if ( - typeof path === 'string' && - nodePath - .normalize(path) - .includes(nodePath.normalize('.gemini/policies/write.toml')) - ) { - return ` -[[rule]] -toolName = "run_shell_command" -argsPattern = "\\"command\\":\\"git (status|diff|log)\\"" -decision = "allow" -priority = 150 -`; - } - return actualFs.readFile(path, options); - }, - ); - - const mockStat = vi.fn( - async ( - path: Parameters[0], - options?: Parameters[1], - ) => { - if ( - typeof path === 'string' && - nodePath - .normalize(path) - .includes(nodePath.normalize('.gemini/policies')) - ) { - return { - isDirectory: () => true, - isFile: () => false, - } as unknown as Awaited>; - } - return actualFs.stat(path, options); - }, - ); - - vi.doMock('node:fs/promises', () => ({ - ...actualFs, - default: { - ...actualFs, - readFile: mockReadFile, - readdir: mockReaddir, - stat: mockStat, - }, - readFile: mockReadFile, - readdir: mockReaddir, - stat: mockStat, - })); - - vi.resetModules(); - const { createPolicyEngineConfig } = await import('./config.js'); - - const settings: PolicySettings = {}; const config = await createPolicyEngineConfig( - settings, + {}, ApprovalMode.DEFAULT, - '/tmp/mock/default/policies', + MOCK_DEFAULT_DIR, ); const rule = config.rules?.find( @@ -727,67 +557,17 @@ priority = 150 r.decision === PolicyDecision.ALLOW, ); expect(rule).toBeDefined(); - // Priority 150 in user tier → 4.150 - expect(rule?.priority).toBeCloseTo(4.15, 5); + // Priority 150 in default tier → 1.150 + expect(rule?.priority).toBeCloseTo(1.15, 5); expect(rule?.argsPattern).toBeInstanceOf(RegExp); expect(rule?.argsPattern?.test('{"command":"git status"}')).toBe(true); - expect(rule?.argsPattern?.test('{"command":"git diff"}')).toBe(true); - expect(rule?.argsPattern?.test('{"command":"git log"}')).toBe(true); expect(rule?.argsPattern?.test('{"command":"git commit"}')).toBe(false); - expect(rule?.argsPattern?.test('{"command":"git push"}')).toBe(false); - - vi.doUnmock('node:fs/promises'); }); it('should load safety_checker configuration from TOML', async () => { - const actualFs = - await vi.importActual( - 'node:fs/promises', - ); - - const mockReaddir = vi.fn( - async ( - path: string | Buffer | URL, - options?: Parameters[1], - ) => { - if ( - typeof path === 'string' && - nodePath - .normalize(path) - .includes(nodePath.normalize('.gemini/policies')) - ) { - return [ - { - name: 'safety.toml', - isFile: () => true, - isDirectory: () => false, - }, - ] as unknown as Awaited>; - } - return actualFs.readdir( - path, - options as Parameters[1], - ); - }, - ); - - const mockReadFile = vi.fn( - async ( - path: Parameters[0], - options: Parameters[1], - ) => { - if ( - typeof path === 'string' && - nodePath - .normalize(path) - .includes(nodePath.normalize('.gemini/policies/safety.toml')) - ) { - return ` -[[rule]] -toolName = "write_file" -decision = "allow" -priority = 10 - + mockPolicyFile( + nodePath.join(MOCK_DEFAULT_DIR, 'safety.toml'), + ` [[rule]] toolName = "write_file" decision = "allow" @@ -800,118 +580,31 @@ priority = 10 type = "in-process" name = "allowed-path" required_context = ["environment"] -[safety_checker.checker.config] -`; - } - return actualFs.readFile(path, options); - }, +`, ); - const mockStat = vi.fn( - async ( - path: Parameters[0], - options?: Parameters[1], - ) => { - if ( - typeof path === 'string' && - nodePath - .normalize(path) - .includes(nodePath.normalize('.gemini/policies')) - ) { - return { - isDirectory: () => true, - isFile: () => false, - } as unknown as Awaited>; - } - return actualFs.stat(path, options); - }, - ); - - vi.doMock('node:fs/promises', () => ({ - ...actualFs, - default: { - ...actualFs, - readFile: mockReadFile, - readdir: mockReaddir, - stat: mockStat, - }, - readFile: mockReadFile, - readdir: mockReaddir, - stat: mockStat, - })); - - vi.resetModules(); - const { createPolicyEngineConfig } = await import('./config.js'); - - const settings: PolicySettings = {}; const config = await createPolicyEngineConfig( - settings, + {}, ApprovalMode.DEFAULT, - '/tmp/mock/default/policies', + MOCK_DEFAULT_DIR, ); - const rule = config.rules?.find( - (r) => r.toolName === 'write_file' && r.decision === PolicyDecision.ALLOW, - ); - expect(rule).toBeDefined(); - + expect( + config.rules?.some( + (r) => + r.toolName === 'write_file' && r.decision === PolicyDecision.ALLOW, + ), + ).toBe(true); const checker = config.checkers?.find( (c) => c.toolName === 'write_file' && c.checker.type === 'in-process', ); - expect(checker).toBeDefined(); - expect(checker?.checker.type).toBe('in-process'); expect(checker?.checker.name).toBe(InProcessCheckerType.ALLOWED_PATH); - expect(checker?.checker.required_context).toEqual(['environment']); - - vi.doUnmock('node:fs/promises'); }); it('should reject invalid in-process checker names', async () => { - const actualFs = - await vi.importActual( - 'node:fs/promises', - ); - - const mockReaddir = vi.fn( - async ( - path: string | Buffer | URL, - options?: Parameters[1], - ) => { - if ( - typeof path === 'string' && - nodePath - .normalize(path) - .includes(nodePath.normalize('.gemini/policies')) - ) { - return [ - { - name: 'invalid_safety.toml', - isFile: () => true, - isDirectory: () => false, - }, - ] as unknown as Awaited>; - } - return actualFs.readdir( - path, - options as Parameters[1], - ); - }, - ); - - const mockReadFile = vi.fn( - async ( - path: Parameters[0], - options: Parameters[1], - ) => { - if ( - typeof path === 'string' && - nodePath - .normalize(path) - .includes( - nodePath.normalize('.gemini/policies/invalid_safety.toml'), - ) - ) { - return ` + mockPolicyFile( + nodePath.join(MOCK_DEFAULT_DIR, 'invalid_safety.toml'), + ` [[rule]] toolName = "write_file" decision = "allow" @@ -923,116 +616,38 @@ priority = 10 [safety_checker.checker] type = "in-process" name = "invalid-name" -`; - } - return actualFs.readFile(path, options); - }, +`, ); - const mockStat = vi.fn( - async ( - path: Parameters[0], - options?: Parameters[1], - ) => { - if ( - typeof path === 'string' && - nodePath - .normalize(path) - .includes(nodePath.normalize('.gemini/policies')) - ) { - return { - isDirectory: () => true, - isFile: () => false, - } as unknown as Awaited>; - } - return actualFs.stat(path, options); - }, - ); - - vi.doMock('node:fs/promises', () => ({ - ...actualFs, - default: { - ...actualFs, - readFile: mockReadFile, - readdir: mockReaddir, - stat: mockStat, - }, - readFile: mockReadFile, - readdir: mockReaddir, - stat: mockStat, - })); - - vi.resetModules(); - const { createPolicyEngineConfig } = await import('./config.js'); - - const settings: PolicySettings = {}; const config = await createPolicyEngineConfig( - settings, + {}, ApprovalMode.DEFAULT, - '/tmp/mock/default/policies', + MOCK_DEFAULT_DIR, ); - - // The rule should be rejected because 'invalid-name' is not in the enum - const rule = config.rules?.find((r) => r.toolName === 'write_file'); - expect(rule).toBeUndefined(); - - vi.doUnmock('node:fs/promises'); + expect( + config.rules?.find((r) => r.toolName === 'write_file'), + ).toBeUndefined(); }); it('should have default ASK_USER rule for discovered tools', async () => { - vi.resetModules(); - vi.doUnmock('node:fs/promises'); - const { createPolicyEngineConfig: createConfig } = await import( - './config.js' - ); - // Re-mock Storage after resetModules because it was reloaded - const { Storage: FreshStorage } = await import('../config/storage.js'); - vi.spyOn(FreshStorage, 'getUserPoliciesDir').mockReturnValue( - '/non/existent/user/policies', - ); - vi.spyOn(FreshStorage, 'getSystemPoliciesDir').mockReturnValue( - '/non/existent/system/policies', - ); - - const settings: PolicySettings = {}; - // Use default policy dir to load real discovered.toml - const config = await createConfig(settings, ApprovalMode.DEFAULT); - + const config = await createPolicyEngineConfig({}, ApprovalMode.DEFAULT); const discoveredRule = config.rules?.find( (r) => r.toolName === 'discovered_tool_*' && r.decision === PolicyDecision.ASK_USER, ); expect(discoveredRule).toBeDefined(); - // Priority 10 in default tier → 1.010 expect(discoveredRule?.priority).toBeCloseTo(1.01, 5); }); it('should normalize legacy "ShellTool" alias to "run_shell_command"', async () => { - vi.resetModules(); - - // Mock fs to return empty for policies - const actualFs = - await vi.importActual( - 'node:fs/promises', - ); - const mockReaddir = vi.fn( - async () => [] as unknown as Awaited>, - ); - vi.doMock('node:fs/promises', () => ({ - ...actualFs, - default: { ...actualFs, readdir: mockReaddir }, - readdir: mockReaddir, - })); - - const { createPolicyEngineConfig } = await import('./config.js'); - const settings: PolicySettings = { - tools: { allowed: ['ShellTool'] }, - }; + vi.mocked( + fs.readdir as (path: PathLike) => Promise, + ).mockResolvedValue([]); const config = await createPolicyEngineConfig( - settings, + { tools: { allowed: ['ShellTool'] } }, ApprovalMode.DEFAULT, - '/tmp/mock/default/policies', + MOCK_DEFAULT_DIR, ); const rule = config.rules?.find( (r) => @@ -1046,57 +661,12 @@ name = "invalid-name" }); it('should allow overriding Plan Mode deny with user policy', async () => { - const actualFs = - await vi.importActual( - 'node:fs/promises', - ); + const userPolicyDir = '/tmp/gemini-cli-test/user/policies'; + vi.spyOn(Storage, 'getUserPoliciesDir').mockReturnValue(userPolicyDir); - const mockReaddir = vi.fn( - async ( - path: string | Buffer | URL, - options?: Parameters[1], - ) => { - const normalizedPath = nodePath.normalize(path.toString()); - if (normalizedPath.includes('gemini-cli-test/user/policies')) { - return [ - { - name: 'user-plan.toml', - isFile: () => true, - isDirectory: () => false, - }, - ] as unknown as Awaited>; - } - return actualFs.readdir( - path, - options as Parameters[1], - ); - }, - ); - - const mockStat = vi.fn( - async ( - path: Parameters[0], - options?: Parameters[1], - ) => { - const normalizedPath = nodePath.normalize(path.toString()); - if (normalizedPath.includes('gemini-cli-test/user/policies')) { - return { - isDirectory: () => true, - isFile: () => false, - } as unknown as Awaited>; - } - return actualFs.stat(path, options); - }, - ); - - const mockReadFile = vi.fn( - async ( - path: Parameters[0], - options: Parameters[1], - ) => { - const normalizedPath = nodePath.normalize(path.toString()); - if (normalizedPath.includes('user-plan.toml')) { - return ` + mockPolicyFile( + nodePath.join(userPolicyDir, 'user-plan.toml'), + ` [[rule]] toolName = "run_shell_command" commandPrefix = ["git status", "git diff"] @@ -1109,48 +679,11 @@ toolName = "codebase_investigator" decision = "allow" priority = 100 modes = ["plan"] -`; - } - return actualFs.readFile(path, options); - }, +`, ); - vi.doMock('node:fs/promises', () => ({ - ...actualFs, - default: { - ...actualFs, - readFile: mockReadFile, - readdir: mockReaddir, - stat: mockStat, - }, - readFile: mockReadFile, - readdir: mockReaddir, - stat: mockStat, - })); - - vi.resetModules(); - - // Robustly mock Storage using doMock to ensure it persists through imports in config.js - vi.doMock('../config/storage.js', async () => { - const actual = await vi.importActual< - typeof import('../config/storage.js') - >('../config/storage.js'); - class MockStorage extends actual.Storage { - static override getUserPoliciesDir() { - return '/tmp/gemini-cli-test/user/policies'; - } - static override getSystemPoliciesDir() { - return '/tmp/gemini-cli-test/system/policies'; - } - } - return { ...actual, Storage: MockStorage }; - }); - - const { createPolicyEngineConfig } = await import('./config.js'); - - const settings: PolicySettings = {}; const config = await createPolicyEngineConfig( - settings, + {}, ApprovalMode.PLAN, nodePath.join(__dirname, 'policies'), ); @@ -1159,31 +692,57 @@ modes = ["plan"] (r) => r.toolName === 'run_shell_command' && r.decision === PolicyDecision.ALLOW && - r.modes?.includes(ApprovalMode.PLAN) && - r.argsPattern, + r.modes?.includes(ApprovalMode.PLAN), ); - expect(shellRules).toHaveLength(2); - expect( - shellRules?.some((r) => r.argsPattern?.test('{"command":"git status"}')), - ).toBe(true); - expect( - shellRules?.some((r) => r.argsPattern?.test('{"command":"git diff"}')), - ).toBe(true); - expect( - shellRules?.every( - (r) => !r.argsPattern?.test('{"command":"git commit"}'), - ), - ).toBe(true); + expect(shellRules?.length).toBeGreaterThan(0); + shellRules?.forEach((r) => expect(r.priority).toBeCloseTo(4.1, 5)); const subagentRule = config.rules?.find( (r) => r.toolName === 'codebase_investigator' && - r.decision === PolicyDecision.ALLOW && - r.modes?.includes(ApprovalMode.PLAN), + r.decision === PolicyDecision.ALLOW, ); expect(subagentRule).toBeDefined(); expect(subagentRule?.priority).toBeCloseTo(4.1, 5); + }); - vi.doUnmock('node:fs/promises'); + it('should deduplicate security warnings when called multiple times', async () => { + const systemPoliciesDir = '/tmp/gemini-cli-test/system/policies'; + vi.spyOn(Storage, 'getSystemPoliciesDir').mockReturnValue( + systemPoliciesDir, + ); + + vi.mocked( + fs.readdir as (path: PathLike) => Promise, + ).mockImplementation(async (path) => { + if (nodePath.resolve(path.toString()) === systemPoliciesDir) { + return ['policy.toml'] as string[]; + } + return [] as string[]; + }); + + const feedbackSpy = vi + .spyOn(coreEvents, 'emitFeedback') + .mockImplementation(() => {}); + + // First call + await createPolicyEngineConfig( + { adminPolicyPaths: ['/tmp/other/admin/policies'] }, + ApprovalMode.DEFAULT, + ); + expect(feedbackSpy).toHaveBeenCalledWith( + 'warning', + expect.stringContaining('Ignoring --admin-policy'), + ); + const count = feedbackSpy.mock.calls.length; + + // Second call + await createPolicyEngineConfig( + { adminPolicyPaths: ['/tmp/other/admin/policies'] }, + ApprovalMode.DEFAULT, + ); + expect(feedbackSpy.mock.calls.length).toBe(count); + + feedbackSpy.mockRestore(); }); }); diff --git a/packages/core/src/policy/config.ts b/packages/core/src/policy/config.ts index 7085da7e3e..435fb018d5 100644 --- a/packages/core/src/policy/config.ts +++ b/packages/core/src/policy/config.ts @@ -39,6 +39,26 @@ const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); export const DEFAULT_CORE_POLICIES_DIR = path.join(__dirname, 'policies'); +// Cache to prevent duplicate warnings in the same process +const emittedWarnings = new Set(); + +/** + * Emits a warning feedback event only once per process. + */ +function emitWarningOnce(message: string): void { + if (!emittedWarnings.has(message)) { + coreEvents.emitFeedback('warning', message); + emittedWarnings.add(message); + } +} + +/** + * Clears the emitted warnings cache. Used primarily for tests. + */ +export function clearEmittedPolicyWarnings(): void { + emittedWarnings.clear(); +} + // Policy tier constants for priority calculation export const DEFAULT_POLICY_TIER = 1; export const EXTENSION_POLICY_TIER = 2; @@ -89,33 +109,29 @@ export function getAlwaysAllowPriorityFraction(): number { * @param policyPaths Optional user-provided policy paths (from --policy flag). * When provided, these replace the default user policies directory. * @param workspacePoliciesDir Optional path to a directory containing workspace policies. + * @param adminPolicyPaths Optional admin-provided policy paths (from --admin-policy flag). + * When provided, these supplement the default system policies directory. */ export function getPolicyDirectories( defaultPoliciesDir?: string, policyPaths?: string[], workspacePoliciesDir?: string, + adminPolicyPaths?: string[], ): string[] { - const dirs = []; + return [ + // Admin tier (highest priority) + Storage.getSystemPoliciesDir(), + ...(adminPolicyPaths ?? []), - // Admin tier (highest priority) - dirs.push(Storage.getSystemPoliciesDir()); + // User tier (second highest priority) + ...(policyPaths ?? [Storage.getUserPoliciesDir()]), - // User tier (second highest priority) - if (policyPaths && policyPaths.length > 0) { - dirs.push(...policyPaths); - } else { - dirs.push(Storage.getUserPoliciesDir()); - } + // Workspace Tier (third highest) + workspacePoliciesDir, - // Workspace Tier (third highest) - if (workspacePoliciesDir) { - dirs.push(workspacePoliciesDir); - } - - // Default tier (lowest priority) - dirs.push(defaultPoliciesDir ?? DEFAULT_CORE_POLICIES_DIR); - - return dirs; + // Default tier (lowest priority) + defaultPoliciesDir ?? DEFAULT_CORE_POLICIES_DIR, + ].filter((dir): dir is string => !!dir); } /** @@ -124,37 +140,40 @@ export function getPolicyDirectories( */ export function getPolicyTier( dir: string, - defaultPoliciesDir?: string, - workspacePoliciesDir?: string, + context: { + defaultPoliciesDir?: string; + workspacePoliciesDir?: string; + adminPolicyPaths?: Set; + systemPoliciesDir: string; + userPoliciesDir: string; + }, ): number { - const USER_POLICIES_DIR = Storage.getUserPoliciesDir(); - const ADMIN_POLICIES_DIR = Storage.getSystemPoliciesDir(); - const normalizedDir = path.resolve(dir); - const normalizedUser = path.resolve(USER_POLICIES_DIR); - const normalizedAdmin = path.resolve(ADMIN_POLICIES_DIR); + if (normalizedDir === context.systemPoliciesDir) { + return ADMIN_POLICY_TIER; + } + if (context.adminPolicyPaths?.has(normalizedDir)) { + return ADMIN_POLICY_TIER; + } + if (normalizedDir === context.userPoliciesDir) { + return USER_POLICY_TIER; + } if ( - defaultPoliciesDir && - normalizedDir === path.resolve(defaultPoliciesDir) + context.workspacePoliciesDir && + normalizedDir === path.resolve(context.workspacePoliciesDir) + ) { + return WORKSPACE_POLICY_TIER; + } + if ( + context.defaultPoliciesDir && + normalizedDir === path.resolve(context.defaultPoliciesDir) ) { return DEFAULT_POLICY_TIER; } if (normalizedDir === path.resolve(DEFAULT_CORE_POLICIES_DIR)) { return DEFAULT_POLICY_TIER; } - if (normalizedDir === normalizedUser) { - return USER_POLICY_TIER; - } - if ( - workspacePoliciesDir && - normalizedDir === path.resolve(workspacePoliciesDir) - ) { - return WORKSPACE_POLICY_TIER; - } - if (normalizedDir === normalizedAdmin) { - return ADMIN_POLICY_TIER; - } return DEFAULT_POLICY_TIER; } @@ -178,21 +197,24 @@ export function formatPolicyError(error: PolicyFileError): string { /** * Filters out insecure policy directories (specifically the system policy directory). + * Supplemental admin policy paths are NOT subject to strict security checks as they + * are explicitly provided by the user/administrator via flags or settings. * Emits warnings if insecure directories are found. */ async function filterSecurePolicyDirectories( dirs: string[], + systemPoliciesDir: string, ): Promise { - 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 normalizedDir = path.resolve(dir); + const isSystemPolicy = normalizedDir === systemPoliciesDir; + + if (isSystemPolicy) { const { secure, reason } = await isDirectorySecure(dir); if (!secure) { const msg = `Security Warning: Skipping system policies from ${dir}: ${reason}`; - coreEvents.emitFeedback('warning', msg); + emitWarningOnce(msg); return null; } } @@ -271,40 +293,70 @@ export async function createPolicyEngineConfig( approvalMode: ApprovalMode, defaultPoliciesDir?: string, ): Promise { + const systemPoliciesDir = path.resolve(Storage.getSystemPoliciesDir()); + const userPoliciesDir = path.resolve(Storage.getUserPoliciesDir()); + let adminPolicyPaths = settings.adminPolicyPaths; + + // Security: Ignore supplemental admin policies if the system directory already contains policies. + // This prevents flag-based overrides when a central system policy is established. + if (adminPolicyPaths?.length) { + try { + const files = await fs.readdir(systemPoliciesDir); + if (files.some((f) => f.endsWith('.toml'))) { + const msg = `Security Warning: Ignoring --admin-policy because system policies are already defined in ${systemPoliciesDir}`; + emitWarningOnce(msg); + adminPolicyPaths = undefined; + } + } catch (e) { + if (!isNodeError(e) || e.code !== 'ENOENT') { + debugLogger.warn( + `Failed to check system policies in ${systemPoliciesDir}`, + e, + ); + } + } + } + const policyDirs = getPolicyDirectories( defaultPoliciesDir, settings.policyPaths, settings.workspacePoliciesDir, + adminPolicyPaths, ); - const securePolicyDirs = await filterSecurePolicyDirectories(policyDirs); - const normalizedAdminPoliciesDir = path.resolve( - Storage.getSystemPoliciesDir(), + const adminPolicyPathsSet = adminPolicyPaths + ? new Set(adminPolicyPaths.map((p) => path.resolve(p))) + : undefined; + + const securePolicyDirs = await filterSecurePolicyDirectories( + policyDirs, + systemPoliciesDir, ); + const tierContext = { + defaultPoliciesDir, + workspacePoliciesDir: settings.workspacePoliciesDir, + adminPolicyPaths: adminPolicyPathsSet, + systemPoliciesDir, + userPoliciesDir, + }; + + const userProvidedPaths = settings.policyPaths + ? new Set(settings.policyPaths.map((p) => path.resolve(p))) + : new Set(); + // Load policies from TOML files const { rules: tomlRules, checkers: tomlCheckers, errors, } = await loadPoliciesFromToml(securePolicyDirs, (p) => { - const tier = getPolicyTier( - p, - defaultPoliciesDir, - settings.workspacePoliciesDir, - ); + const normalizedPath = path.resolve(p); + const tier = getPolicyTier(normalizedPath, tierContext); - // If it's a user-provided path that isn't already categorized as ADMIN, - // treat it as USER tier. - if ( - settings.policyPaths?.some( - (userPath) => path.resolve(userPath) === path.resolve(p), - ) - ) { - const normalizedPath = path.resolve(p); - if (normalizedPath !== normalizedAdminPoliciesDir) { - return USER_POLICY_TIER; - } + // If it's a user-provided path that isn't already categorized as ADMIN, treat it as USER tier. + if (userProvidedPaths.has(normalizedPath) && tier !== ADMIN_POLICY_TIER) { + return USER_POLICY_TIER; } return tier; diff --git a/packages/core/src/policy/types.ts b/packages/core/src/policy/types.ts index 53a0433a15..6fa45630d9 100644 --- a/packages/core/src/policy/types.ts +++ b/packages/core/src/policy/types.ts @@ -311,6 +311,8 @@ export interface PolicySettings { mcpServers?: Record; // User provided policies that will replace the USER level policies in ~/.gemini/policies policyPaths?: string[]; + // Admin provided policies that will supplement the ADMIN level policies + adminPolicyPaths?: string[]; workspacePoliciesDir?: string; } diff --git a/schemas/settings.schema.json b/schemas/settings.schema.json index ec2873378e..27ac0bf51d 100644 --- a/schemas/settings.schema.json +++ b/schemas/settings.schema.json @@ -32,6 +32,16 @@ "type": "string" } }, + "adminPolicyPaths": { + "title": "Admin Policy Paths", + "description": "Additional admin policy files or directories to load.", + "markdownDescription": "Additional admin policy files or directories to load.\n\n- Category: `Advanced`\n- Requires restart: `yes`\n- Default: `[]`", + "default": [], + "type": "array", + "items": { + "type": "string" + } + }, "general": { "title": "General", "description": "General application settings.",