From 15f8026983115d01848aef192691c3a4308b6a30 Mon Sep 17 00:00:00 2001 From: matt korwel Date: Mon, 23 Mar 2026 13:34:09 -0700 Subject: [PATCH] fix(cli): prevent subcommand shadowing and skip auth for commands (#23177) --- packages/cli/src/config/config.test.ts | 35 +++++ packages/cli/src/config/config.ts | 145 ++++++++++++------- packages/cli/src/config/extension-manager.ts | 10 +- packages/cli/src/gemini.tsx | 4 +- 4 files changed, 135 insertions(+), 59 deletions(-) diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index 2325711ad0..f312ddde4f 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -322,6 +322,41 @@ describe('parseArguments', () => { }, ); + describe('isCommand middleware', () => { + it.each([ + { cmd: 'mcp list', expected: true }, + { cmd: 'extensions list', expected: true }, + { cmd: 'extension list', expected: true }, + { cmd: 'skills list', expected: true }, + { cmd: 'skill list', expected: true }, + { cmd: 'hooks migrate', expected: true }, + { cmd: 'hook migrate', expected: true }, + { cmd: 'some query', expected: undefined }, + { cmd: 'hello world', expected: undefined }, + ])( + 'should set isCommand to $expected for "$cmd"', + async ({ cmd, expected }) => { + process.argv = ['node', 'script.js', ...cmd.split(' ')]; + const settings = createTestMergedSettings({ + admin: { + mcp: { enabled: true }, + }, + experimental: { + extensionManagement: true, + }, + skills: { + enabled: true, + }, + hooksConfig: { + enabled: true, + }, + }); + const parsedArgs = await parseArguments(settings); + expect(parsedArgs.isCommand).toBe(expected); + }, + ); + }); + it.each([ { description: 'should allow --prompt without --prompt-interactive', diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 227ad4e8ed..fa6d16fc72 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -163,12 +163,104 @@ export async function parseArguments( .usage( 'Usage: gemini [options] [command]\n\nGemini CLI - Defaults to interactive mode. Use -p/--prompt for non-interactive (headless) mode.', ) + .option('isCommand', { + type: 'boolean', + hidden: true, + description: 'Internal flag to indicate if a subcommand is being run', + }) .option('debug', { alias: 'd', type: 'boolean', description: 'Run in debug mode (open debug console with F12)', default: false, }) + .middleware((argv) => { + const commandModules = [ + mcpCommand, + extensionsCommand, + skillsCommand, + hooksCommand, + ]; + + const subcommands = commandModules.flatMap((mod) => { + const names: string[] = []; + + const cmd = mod.command; + if (cmd) { + if (Array.isArray(cmd)) { + for (const c of cmd) { + names.push(String(c).split(' ')[0]); + } + } else { + names.push(String(cmd).split(' ')[0]); + } + } + + const aliases = mod.aliases; + if (aliases) { + if (Array.isArray(aliases)) { + for (const a of aliases) { + names.push(String(a).split(' ')[0]); + } + } else { + names.push(String(aliases).split(' ')[0]); + } + } + + return names; + }); + + const firstArg = argv._[0]; + if (typeof firstArg === 'string' && subcommands.includes(firstArg)) { + argv['isCommand'] = true; + } + }, true) + // Ensure validation flows through .fail() for clean UX + .fail((msg, err) => { + if (err) throw err; + throw new Error(msg); + }) + .check((argv) => { + // The 'query' positional can be a string (for one arg) or string[] (for multiple). + // This guard safely checks if any positional argument was provided. + const queryArg = argv['query']; + const query = + typeof queryArg === 'string' || Array.isArray(queryArg) + ? queryArg + : undefined; + const hasPositionalQuery = Array.isArray(query) + ? query.length > 0 + : !!query; + + if (argv['prompt'] && hasPositionalQuery) { + return 'Cannot use both a positional prompt and the --prompt (-p) flag together'; + } + if (argv['prompt'] && argv['promptInteractive']) { + return 'Cannot use both --prompt (-p) and --prompt-interactive (-i) together'; + } + if (argv['yolo'] && argv['approvalMode']) { + return 'Cannot use both --yolo (-y) and --approval-mode together. Use --approval-mode=yolo instead.'; + } + + const outputFormat = argv['outputFormat']; + if ( + typeof outputFormat === 'string' && + !['text', 'json', 'stream-json'].includes(outputFormat) + ) { + return `Invalid values:\n Argument: output-format, Given: "${outputFormat}", Choices: "text", "json", "stream-json"`; + } + if (argv['worktree'] && !settings.experimental?.worktrees) { + return 'The --worktree flag is only available when experimental.worktrees is enabled in your settings.'; + } + return true; + }); + + yargsInstance.command(mcpCommand); + yargsInstance.command(extensionsCommand); + yargsInstance.command(skillsCommand); + yargsInstance.command(hooksCommand); + + yargsInstance .command('$0 [query..]', 'Launch Gemini CLI', (yargsInstance) => yargsInstance .positional('query', { @@ -352,59 +444,6 @@ export async function parseArguments( description: 'Suppress the security warning when using --raw-output.', }), ) - // Register MCP subcommands - .command(mcpCommand) - // Ensure validation flows through .fail() for clean UX - .fail((msg, err) => { - if (err) throw err; - throw new Error(msg); - }) - .check((argv) => { - // The 'query' positional can be a string (for one arg) or string[] (for multiple). - // This guard safely checks if any positional argument was provided. - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - const query = argv['query'] as string | string[] | undefined; - const hasPositionalQuery = Array.isArray(query) - ? query.length > 0 - : !!query; - - if (argv['prompt'] && hasPositionalQuery) { - return 'Cannot use both a positional prompt and the --prompt (-p) flag together'; - } - if (argv['prompt'] && argv['promptInteractive']) { - return 'Cannot use both --prompt (-p) and --prompt-interactive (-i) together'; - } - if (argv['yolo'] && argv['approvalMode']) { - return 'Cannot use both --yolo (-y) and --approval-mode together. Use --approval-mode=yolo instead.'; - } - if ( - argv['outputFormat'] && - !['text', 'json', 'stream-json'].includes( - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - argv['outputFormat'] as string, - ) - ) { - return `Invalid values:\n Argument: output-format, Given: "${argv['outputFormat']}", Choices: "text", "json", "stream-json"`; - } - if (argv['worktree'] && !settings.experimental?.worktrees) { - return 'The --worktree flag is only available when experimental.worktrees is enabled in your settings.'; - } - return true; - }); - - if (settings.experimental?.extensionManagement) { - yargsInstance.command(extensionsCommand); - } - - if (settings.skills?.enabled ?? true) { - yargsInstance.command(skillsCommand); - } - // Register hooks command if hooks are enabled - if (settings.hooksConfig.enabled) { - yargsInstance.command(hooksCommand); - } - - yargsInstance .version(await getVersion()) // This will enable the --version flag based on package.json .alias('v', 'version') .help() diff --git a/packages/cli/src/config/extension-manager.ts b/packages/cli/src/config/extension-manager.ts index 04487bc5f8..65b3539794 100644 --- a/packages/cli/src/config/extension-manager.ts +++ b/packages/cli/src/config/extension-manager.ts @@ -614,7 +614,7 @@ Would you like to attempt to install via "git clone" instead?`, this.loadingPromise = (async () => { try { - if (this.settings.admin.extensions.enabled === false) { + if (this.settings.admin?.extensions?.enabled === false) { this.loadedExtensions = []; return this.loadedExtensions; } @@ -824,11 +824,11 @@ Would you like to attempt to install via "git clone" instead?`, } if (config.mcpServers) { - if (this.settings.admin.mcp.enabled === false) { + if (this.settings.admin?.mcp?.enabled === false) { config.mcpServers = undefined; } else { // Apply admin allowlist if configured - const adminAllowlist = this.settings.admin.mcp.config; + const adminAllowlist = this.settings.admin?.mcp?.config; if (adminAllowlist && Object.keys(adminAllowlist).length > 0) { const result = applyAdminAllowlist( config.mcpServers, @@ -1298,7 +1298,9 @@ export async function inferInstallMetadata( source.startsWith('http://') || source.startsWith('https://') || source.startsWith('git@') || - source.startsWith('sso://') + source.startsWith('sso://') || + source.startsWith('github:') || + source.startsWith('gitlab:') ) { return { source, diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 65a0d13a58..5bd9944f63 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -334,7 +334,7 @@ export async function main() { // the sandbox because the sandbox will interfere with the Oauth2 web // redirect. let initialAuthFailed = false; - if (!settings.merged.security.auth.useExternal) { + if (!settings.merged.security.auth.useExternal && !argv.isCommand) { try { if ( partialConfig.isInteractive() && @@ -386,7 +386,7 @@ export async function main() { await runDeferredCommand(settings.merged); // hop into sandbox if we are outside and sandboxing is enabled - if (!process.env['SANDBOX']) { + if (!process.env['SANDBOX'] && !argv.isCommand) { const memoryArgs = settings.merged.advanced.autoConfigureMemory ? getNodeMemoryArgs(isDebugMode) : [];