diff --git a/docs/extension.md b/docs/extension.md index 34595c027a..4c580597ec 100644 --- a/docs/extension.md +++ b/docs/extension.md @@ -36,11 +36,13 @@ gemini extensions uninstall gemini-cli-security Extensions are, by default, enabled across all workspaces. You can disable an extension entirely or for specific workspace. -For example, `gemini extensions disable extension-name` will disable the extension at the user level, so it will be disabled everywhere. `gemini extensions disable extension-name --scope=Workspace` will only disable the extension in the current workspace. +For example, `gemini extensions disable extension-name` will disable the extension at the user level, so it will be disabled everywhere. `gemini extensions disable extension-name --scope=workspace` will only disable the extension in the current workspace. ### Enabling an extension -You can re-enable extensions using `gemini extensions enable extension-name`. Note that if an extension is disabled at the user-level, enabling it at the workspace level will not do anything. +You can enable extensions using `gemini extensions enable extension-name`. You can also enable an extension for a specific workspace using `gemini extensions enable extension-name --scope=workspace` from within that workspace. + +This is useful if you have an extension disabled at the top-level and only enabled in specific places. ### Updating an extension diff --git a/packages/cli/src/commands/extensions/disable.ts b/packages/cli/src/commands/extensions/disable.ts index 19727233c4..0a88ce08f2 100644 --- a/packages/cli/src/commands/extensions/disable.ts +++ b/packages/cli/src/commands/extensions/disable.ts @@ -11,12 +11,16 @@ import { getErrorMessage } from '../../utils/errors.js'; interface DisableArgs { name: string; - scope: SettingScope; + scope?: string; } export function handleDisable(args: DisableArgs) { try { - disableExtension(args.name, args.scope); + if (args.scope?.toLowerCase() === 'workspace') { + disableExtension(args.name, SettingScope.Workspace); + } else { + disableExtension(args.name, SettingScope.User); + } console.log( `Extension "${args.name}" successfully disabled for scope "${args.scope}".`, ); @@ -39,13 +43,28 @@ export const disableCommand: CommandModule = { describe: 'The scope to disable the extenison in.', type: 'string', default: SettingScope.User, - choices: [SettingScope.User, SettingScope.Workspace], }) - .check((_argv) => true), + .check((argv) => { + if ( + argv.scope && + !Object.values(SettingScope) + .map((s) => s.toLowerCase()) + .includes((argv.scope as string).toLowerCase()) + ) { + throw new Error( + `Invalid scope: ${argv.scope}. Please use one of ${Object.values( + SettingScope, + ) + .map((s) => s.toLowerCase()) + .join(', ')}.`, + ); + } + return true; + }), handler: (argv) => { handleDisable({ name: argv['name'] as string, - scope: argv['scope'] as SettingScope, + scope: argv['scope'] as string, }); }, }; diff --git a/packages/cli/src/commands/extensions/enable.ts b/packages/cli/src/commands/extensions/enable.ts index 6bf3b71ff4..0691b86a60 100644 --- a/packages/cli/src/commands/extensions/enable.ts +++ b/packages/cli/src/commands/extensions/enable.ts @@ -11,13 +11,16 @@ import { SettingScope } from '../../config/settings.js'; interface EnableArgs { name: string; - scope?: SettingScope; + scope?: string; } export function handleEnable(args: EnableArgs) { try { - const scope = args.scope ? args.scope : SettingScope.User; - enableExtension(args.name, scope); + if (args.scope?.toLowerCase() === 'workspace') { + enableExtension(args.name, SettingScope.Workspace); + } else { + enableExtension(args.name, SettingScope.User); + } if (args.scope) { console.log( `Extension "${args.name}" successfully enabled for scope "${args.scope}".`, @@ -45,13 +48,28 @@ export const enableCommand: CommandModule = { describe: 'The scope to enable the extenison in. If not set, will be enabled in all scopes.', type: 'string', - choices: [SettingScope.User, SettingScope.Workspace], }) - .check((_argv) => true), + .check((argv) => { + if ( + argv.scope && + !Object.values(SettingScope) + .map((s) => s.toLowerCase()) + .includes((argv.scope as string).toLowerCase()) + ) { + throw new Error( + `Invalid scope: ${argv.scope}. Please use one of ${Object.values( + SettingScope, + ) + .map((s) => s.toLowerCase()) + .join(', ')}.`, + ); + } + return true; + }), handler: (argv) => { handleEnable({ name: argv['name'] as string, - scope: argv['scope'] as SettingScope, + scope: argv['scope'] as string, }); }, }; diff --git a/packages/cli/src/config/extensions/extensionEnablement.test.ts b/packages/cli/src/config/extensions/extensionEnablement.test.ts index 874b68b8c9..8923f6e023 100644 --- a/packages/cli/src/config/extensions/extensionEnablement.test.ts +++ b/packages/cli/src/config/extensions/extensionEnablement.test.ts @@ -4,11 +4,11 @@ * SPDX-License-Identifier: Apache-2.0 */ +import * as path from 'node:path'; import fs from 'node:fs'; import os from 'node:os'; -import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { ExtensionEnablementManager } from './extensionEnablement.js'; +import { ExtensionEnablementManager, Override } from './extensionEnablement.js'; // Helper to create a temporary directory for testing function createTestDir() { @@ -48,7 +48,7 @@ describe('ExtensionEnablementManager', () => { }); it('should enable a path based on an override rule', () => { - manager.disable('ext-test', true, '*'); // Disable globally + manager.disable('ext-test', true, '/'); manager.enable('ext-test', true, '/home/user/projects/'); expect(manager.isEnabled('ext-test', '/home/user/projects/my-app')).toBe( true, @@ -56,7 +56,7 @@ describe('ExtensionEnablementManager', () => { }); it('should disable a path based on a disable override rule', () => { - manager.enable('ext-test', true, '*'); // Enable globally + manager.enable('ext-test', true, '/'); manager.disable('ext-test', true, '/home/user/projects/'); expect(manager.isEnabled('ext-test', '/home/user/projects/my-app')).toBe( false, @@ -78,65 +78,253 @@ describe('ExtensionEnablementManager', () => { false, ); }); + + it('should handle', () => { + manager.enable('ext-test', true, '/home/user/projects'); + manager.disable('ext-test', false, '/home/user/projects/my-app'); + expect(manager.isEnabled('ext-test', '/home/user/projects/my-app')).toBe( + false, + ); + expect( + manager.isEnabled('ext-test', '/home/user/projects/something-else'), + ).toBe(true); + }); }); describe('includeSubdirs', () => { it('should add a glob when enabling with includeSubdirs', () => { manager.enable('ext-test', true, '/path/to/dir'); const config = manager.readConfig(); - expect(config['ext-test'].overrides).toContain('/path/to/dir*'); + expect(config['ext-test'].overrides).toContain('/path/to/dir/*'); }); it('should not add a glob when enabling without includeSubdirs', () => { manager.enable('ext-test', false, '/path/to/dir'); const config = manager.readConfig(); - expect(config['ext-test'].overrides).toContain('/path/to/dir'); - expect(config['ext-test'].overrides).not.toContain('/path/to/dir*'); + expect(config['ext-test'].overrides).toContain('/path/to/dir/'); + expect(config['ext-test'].overrides).not.toContain('/path/to/dir/*'); }); it('should add a glob when disabling with includeSubdirs', () => { manager.disable('ext-test', true, '/path/to/dir'); const config = manager.readConfig(); - expect(config['ext-test'].overrides).toContain('!/path/to/dir*'); + expect(config['ext-test'].overrides).toContain('!/path/to/dir/*'); }); it('should remove conflicting glob rule when enabling without subdirs', () => { manager.enable('ext-test', true, '/path/to/dir'); // Adds /path/to/dir* manager.enable('ext-test', false, '/path/to/dir'); // Should remove the glob const config = manager.readConfig(); - expect(config['ext-test'].overrides).toContain('/path/to/dir'); - expect(config['ext-test'].overrides).not.toContain('/path/to/dir*'); + expect(config['ext-test'].overrides).toContain('/path/to/dir/'); + expect(config['ext-test'].overrides).not.toContain('/path/to/dir/*'); }); it('should remove conflicting non-glob rule when enabling with subdirs', () => { manager.enable('ext-test', false, '/path/to/dir'); // Adds /path/to/dir manager.enable('ext-test', true, '/path/to/dir'); // Should remove the non-glob const config = manager.readConfig(); - expect(config['ext-test'].overrides).toContain('/path/to/dir*'); - expect(config['ext-test'].overrides).not.toContain('/path/to/dir'); + expect(config['ext-test'].overrides).toContain('/path/to/dir/*'); + expect(config['ext-test'].overrides).not.toContain('/path/to/dir/'); }); it('should remove conflicting rules when disabling', () => { manager.enable('ext-test', true, '/path/to/dir'); // enabled with glob manager.disable('ext-test', false, '/path/to/dir'); // disabled without const config = manager.readConfig(); - expect(config['ext-test'].overrides).toContain('!/path/to/dir'); - expect(config['ext-test'].overrides).not.toContain('/path/to/dir*'); + expect(config['ext-test'].overrides).toContain('!/path/to/dir/'); + expect(config['ext-test'].overrides).not.toContain('/path/to/dir/*'); }); it('should correctly evaluate isEnabled with subdirs', () => { - manager.disable('ext-test', true, '*'); + manager.disable('ext-test', true, '/'); manager.enable('ext-test', true, '/path/to/dir'); - expect(manager.isEnabled('ext-test', '/path/to/dir')).toBe(true); - expect(manager.isEnabled('ext-test', '/path/to/dir/sub')).toBe(true); - expect(manager.isEnabled('ext-test', '/path/to/another')).toBe(false); + expect(manager.isEnabled('ext-test', '/path/to/dir/')).toBe(true); + expect(manager.isEnabled('ext-test', '/path/to/dir/sub/')).toBe(true); + expect(manager.isEnabled('ext-test', '/path/to/another/')).toBe(false); }); it('should correctly evaluate isEnabled without subdirs', () => { - manager.disable('ext-test', true, '*'); + manager.disable('ext-test', true, '/*'); manager.enable('ext-test', false, '/path/to/dir'); expect(manager.isEnabled('ext-test', '/path/to/dir')).toBe(true); expect(manager.isEnabled('ext-test', '/path/to/dir/sub')).toBe(false); }); }); + + describe('pruning child rules', () => { + it('should remove child rules when enabling a parent with subdirs', () => { + // Pre-existing rules for children + manager.enable('ext-test', false, '/path/to/dir/subdir1'); + manager.disable('ext-test', true, '/path/to/dir/subdir2'); + manager.enable('ext-test', false, '/path/to/another/dir'); + + // Enable the parent directory + manager.enable('ext-test', true, '/path/to/dir'); + + const config = manager.readConfig(); + const overrides = config['ext-test'].overrides; + + // The new parent rule should be present + expect(overrides).toContain(`/path/to/dir/*`); + + // Child rules should be removed + expect(overrides).not.toContain('/path/to/dir/subdir1/'); + expect(overrides).not.toContain(`!/path/to/dir/subdir2/*`); + + // Unrelated rules should remain + expect(overrides).toContain('/path/to/another/dir/'); + }); + + it('should remove child rules when disabling a parent with subdirs', () => { + // Pre-existing rules for children + manager.enable('ext-test', false, '/path/to/dir/subdir1'); + manager.disable('ext-test', true, '/path/to/dir/subdir2'); + manager.enable('ext-test', false, '/path/to/another/dir'); + + // Disable the parent directory + manager.disable('ext-test', true, '/path/to/dir'); + + const config = manager.readConfig(); + const overrides = config['ext-test'].overrides; + + // The new parent rule should be present + expect(overrides).toContain(`!/path/to/dir/*`); + + // Child rules should be removed + expect(overrides).not.toContain('/path/to/dir/subdir1/'); + expect(overrides).not.toContain(`!/path/to/dir/subdir2/*`); + + // Unrelated rules should remain + expect(overrides).toContain('/path/to/another/dir/'); + }); + + it('should not remove child rules if includeSubdirs is false', () => { + manager.enable('ext-test', false, '/path/to/dir/subdir1'); + manager.enable('ext-test', false, '/path/to/dir'); // Not including subdirs + + const config = manager.readConfig(); + const overrides = config['ext-test'].overrides; + + expect(overrides).toContain('/path/to/dir/subdir1/'); + expect(overrides).toContain('/path/to/dir/'); + }); + }); + + it('should enable a path based on an enable override', () => { + manager.disable('ext-test', true, '/Users/chrstn'); + manager.enable('ext-test', true, '/Users/chrstn/gemini-cli'); + + expect(manager.isEnabled('ext-test', '/Users/chrstn/gemini-cli')).toBe( + true, + ); + }); + + it('should ignore subdirs', () => { + manager.disable('ext-test', false, '/Users/chrstn'); + expect(manager.isEnabled('ext-test', '/Users/chrstn/gemini-cli')).toBe( + true, + ); + }); +}); + +describe('Override', () => { + it('should create an override from input', () => { + const override = Override.fromInput('/path/to/dir', true); + expect(override.baseRule).toBe(`/path/to/dir/`); + expect(override.isDisable).toBe(false); + expect(override.includeSubdirs).toBe(true); + }); + + it('should create a disable override from input', () => { + const override = Override.fromInput('!/path/to/dir', false); + expect(override.baseRule).toBe(`/path/to/dir/`); + expect(override.isDisable).toBe(true); + expect(override.includeSubdirs).toBe(false); + }); + + it('should create an override from a file rule', () => { + const override = Override.fromFileRule('/path/to/dir'); + expect(override.baseRule).toBe('/path/to/dir'); + expect(override.isDisable).toBe(false); + expect(override.includeSubdirs).toBe(false); + }); + + it('should create a disable override from a file rule', () => { + const override = Override.fromFileRule('!/path/to/dir/'); + expect(override.isDisable).toBe(true); + expect(override.baseRule).toBe('/path/to/dir/'); + expect(override.includeSubdirs).toBe(false); + }); + + it('should create an override with subdirs from a file rule', () => { + const override = Override.fromFileRule('/path/to/dir/*'); + expect(override.baseRule).toBe('/path/to/dir/'); + expect(override.isDisable).toBe(false); + expect(override.includeSubdirs).toBe(true); + }); + + it('should correctly identify conflicting overrides', () => { + const override1 = Override.fromInput('/path/to/dir', true); + const override2 = Override.fromInput('/path/to/dir', false); + expect(override1.conflictsWith(override2)).toBe(true); + }); + + it('should correctly identify non-conflicting overrides', () => { + const override1 = Override.fromInput('/path/to/dir', true); + const override2 = Override.fromInput('/path/to/another/dir', true); + expect(override1.conflictsWith(override2)).toBe(false); + }); + + it('should correctly identify equal overrides', () => { + const override1 = Override.fromInput('/path/to/dir', true); + const override2 = Override.fromInput('/path/to/dir', true); + expect(override1.isEqualTo(override2)).toBe(true); + }); + + it('should correctly identify unequal overrides', () => { + const override1 = Override.fromInput('/path/to/dir', true); + const override2 = Override.fromInput('!/path/to/dir', true); + expect(override1.isEqualTo(override2)).toBe(false); + }); + + it('should generate the correct regex', () => { + const override = Override.fromInput('/path/to/dir', true); + const regex = override.asRegex(); + expect(regex.test('/path/to/dir/')).toBe(true); + expect(regex.test('/path/to/dir/subdir')).toBe(true); + expect(regex.test('/path/to/another/dir')).toBe(false); + }); + + it('should correctly identify child overrides', () => { + const parent = Override.fromInput('/path/to/dir', true); + const child = Override.fromInput('/path/to/dir/subdir', false); + expect(child.isChildOf(parent)).toBe(true); + }); + + it('should correctly identify child overrides with glob', () => { + const parent = Override.fromInput('/path/to/dir/*', true); + const child = Override.fromInput('/path/to/dir/subdir', false); + expect(child.isChildOf(parent)).toBe(true); + }); + + it('should correctly identify non-child overrides', () => { + const parent = Override.fromInput('/path/to/dir', true); + const other = Override.fromInput('/path/to/another/dir', false); + expect(other.isChildOf(parent)).toBe(false); + }); + + it('should generate the correct output string', () => { + const override = Override.fromInput('/path/to/dir', true); + expect(override.output()).toBe(`/path/to/dir/*`); + }); + + it('should generate the correct output string for a disable override', () => { + const override = Override.fromInput('!/path/to/dir', false); + expect(override.output()).toBe(`!/path/to/dir/`); + }); + + it('should disable a path based on a disable override rule', () => { + const override = Override.fromInput('!/path/to/dir', false); + expect(override.output()).toBe(`!/path/to/dir/`); + }); }); diff --git a/packages/cli/src/config/extensions/extensionEnablement.ts b/packages/cli/src/config/extensions/extensionEnablement.ts index b32fece9c1..967b6381e9 100644 --- a/packages/cli/src/config/extensions/extensionEnablement.ts +++ b/packages/cli/src/config/extensions/extensionEnablement.ts @@ -15,6 +15,80 @@ export interface AllExtensionsEnablementConfig { [extensionName: string]: ExtensionEnablementConfig; } +export class Override { + constructor( + public baseRule: string, + public isDisable: boolean, + public includeSubdirs: boolean, + ) {} + + static fromInput(inputRule: string, includeSubdirs: boolean): Override { + const isDisable = inputRule.startsWith('!'); + let baseRule = isDisable ? inputRule.substring(1) : inputRule; + baseRule = ensureLeadingAndTrailingSlash(baseRule); + return new Override(baseRule, isDisable, includeSubdirs); + } + + static fromFileRule(fileRule: string): Override { + const isDisable = fileRule.startsWith('!'); + let baseRule = isDisable ? fileRule.substring(1) : fileRule; + const includeSubdirs = baseRule.endsWith('*'); + baseRule = includeSubdirs + ? baseRule.substring(0, baseRule.length - 1) + : baseRule; + return new Override(baseRule, isDisable, includeSubdirs); + } + + conflictsWith(other: Override): boolean { + if (this.baseRule === other.baseRule) { + return ( + this.includeSubdirs !== other.includeSubdirs || + this.isDisable !== other.isDisable + ); + } + return false; + } + + isEqualTo(other: Override): boolean { + return ( + this.baseRule === other.baseRule && + this.includeSubdirs === other.includeSubdirs && + this.isDisable === other.isDisable + ); + } + + asRegex(): RegExp { + return globToRegex(`${this.baseRule}${this.includeSubdirs ? '*' : ''}`); + } + + isChildOf(parent: Override) { + if (!parent.includeSubdirs) { + return false; + } + return parent.asRegex().test(this.baseRule); + } + + output(): string { + return `${this.isDisable ? '!' : ''}${this.baseRule}${this.includeSubdirs ? '*' : ''}`; + } + + matchesPath(path: string) { + return this.asRegex().test(path); + } +} + +const ensureLeadingAndTrailingSlash = function (dirPath: string): string { + // Normalize separators to forward slashes for consistent matching across platforms. + let result = dirPath.replace(/\\/g, '/'); + if (result.charAt(0) !== '/') { + result = '/' + result; + } + if (result.charAt(result.length - 1) !== '/') { + result = result + '/'; + } + return result; +}; + /** * Converts a glob pattern to a RegExp object. * This is a simplified implementation that supports `*`. @@ -25,7 +99,7 @@ export interface AllExtensionsEnablementConfig { function globToRegex(glob: string): RegExp { const regexString = glob .replace(/[.+?^${}()|[\]\\]/g, '\\$&') // Escape special regex characters - .replace(/\*/g, '.*'); // Convert * to .* + .replace(/(\/?)\*/g, '($1.*)?'); // Convert * to optional group return new RegExp(`^${regexString}$`); } @@ -52,16 +126,13 @@ export class ExtensionEnablementManager { const extensionConfig = config[extensionName]; // Extensions are enabled by default. let enabled = true; - - for (const rule of extensionConfig?.overrides ?? []) { - const isDisableRule = rule.startsWith('!'); - const globPattern = isDisableRule ? rule.substring(1) : rule; - const regex = globToRegex(globPattern); - if (regex.test(currentPath)) { - enabled = !isDisableRule; + const allOverrides = extensionConfig?.overrides ?? []; + for (const rule of allOverrides) { + const override = Override.fromFileRule(rule); + if (override.matchesPath(ensureLeadingAndTrailingSlash(currentPath))) { + enabled = !override.isDisable; } } - return enabled; } @@ -96,24 +167,19 @@ export class ExtensionEnablementManager { if (!config[extensionName]) { config[extensionName] = { overrides: [] }; } - - const pathWithGlob = `${scopePath}*`; - const pathWithoutGlob = scopePath; - - const newPath = includeSubdirs ? pathWithGlob : pathWithoutGlob; - const conflictingPath = includeSubdirs ? pathWithoutGlob : pathWithGlob; - - config[extensionName].overrides = config[extensionName].overrides.filter( - (rule) => - rule !== conflictingPath && - rule !== `!${conflictingPath}` && - rule !== `!${newPath}`, - ); - - if (!config[extensionName].overrides.includes(newPath)) { - config[extensionName].overrides.push(newPath); - } - + const override = Override.fromInput(scopePath, includeSubdirs); + const overrides = config[extensionName].overrides.filter((rule) => { + const fileOverride = Override.fromFileRule(rule); + if ( + fileOverride.conflictsWith(override) || + fileOverride.isEqualTo(override) + ) { + return false; // Remove conflicts and equivalent values. + } + return !fileOverride.isChildOf(override); + }); + overrides.push(override.output()); + config[extensionName].overrides = overrides; this.writeConfig(config); } @@ -122,30 +188,7 @@ export class ExtensionEnablementManager { includeSubdirs: boolean, scopePath: string, ): void { - const config = this.readConfig(); - if (!config[extensionName]) { - config[extensionName] = { overrides: [] }; - } - - const pathWithGlob = `${scopePath}*`; - const pathWithoutGlob = scopePath; - - const targetPath = includeSubdirs ? pathWithGlob : pathWithoutGlob; - const newRule = `!${targetPath}`; - const conflictingPath = includeSubdirs ? pathWithoutGlob : pathWithGlob; - - config[extensionName].overrides = config[extensionName].overrides.filter( - (rule) => - rule !== conflictingPath && - rule !== `!${conflictingPath}` && - rule !== targetPath, - ); - - if (!config[extensionName].overrides.includes(newRule)) { - config[extensionName].overrides.push(newRule); - } - - this.writeConfig(config); + this.enable(extensionName, includeSubdirs, `!${scopePath}`); } remove(extensionName: string): void {