feat(cli): add security consent prompts for skill installation (#16549)

This commit is contained in:
N. Taylor Mullen
2026-01-14 17:47:02 -08:00
committed by GitHub
parent 5bdfe1a1fa
commit a81500a929
7 changed files with 296 additions and 101 deletions

View File

@@ -7,11 +7,18 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
const mockInstallSkill = vi.hoisted(() => vi.fn());
const mockRequestConsentNonInteractive = vi.hoisted(() => vi.fn());
const mockSkillsConsentString = vi.hoisted(() => vi.fn());
vi.mock('../../utils/skillUtils.js', () => ({
installSkill: mockInstallSkill,
}));
vi.mock('../../config/extensions/consent.js', () => ({
requestConsentNonInteractive: mockRequestConsentNonInteractive,
skillsConsentString: mockSkillsConsentString,
}));
vi.mock('@google/gemini-cli-core', () => ({
debugLogger: { log: vi.fn(), error: vi.fn() },
}));
@@ -23,6 +30,8 @@ describe('skill install command', () => {
beforeEach(() => {
vi.clearAllMocks();
vi.spyOn(process, 'exit').mockImplementation(() => undefined as never);
mockSkillsConsentString.mockResolvedValue('Mock Consent String');
mockRequestConsentNonInteractive.mockResolvedValue(true);
});
describe('installCommand', () => {
@@ -37,9 +46,10 @@ describe('skill install command', () => {
});
it('should call installSkill with correct arguments for user scope', async () => {
mockInstallSkill.mockResolvedValue([
{ name: 'test-skill', location: '/mock/user/skills/test-skill' },
]);
mockInstallSkill.mockImplementation(async (_s, _sc, _p, _ol, rc) => {
await rc([]);
return [{ name: 'test-skill', location: '/mock/user/skills/test-skill' }];
});
await handleInstall({
source: 'https://example.com/repo.git',
@@ -51,6 +61,7 @@ describe('skill install command', () => {
'user',
undefined,
expect.any(Function),
expect.any(Function),
);
expect(debugLogger.log).toHaveBeenCalledWith(
expect.stringContaining('Successfully installed skill: test-skill'),
@@ -58,6 +69,47 @@ describe('skill install command', () => {
expect(debugLogger.log).toHaveBeenCalledWith(
expect.stringContaining('location: /mock/user/skills/test-skill'),
);
expect(mockRequestConsentNonInteractive).toHaveBeenCalledWith(
'Mock Consent String',
);
});
it('should skip prompt and log consent when --consent is provided', async () => {
mockInstallSkill.mockImplementation(async (_s, _sc, _p, _ol, rc) => {
await rc([]);
return [{ name: 'test-skill', location: '/mock/user/skills/test-skill' }];
});
await handleInstall({
source: 'https://example.com/repo.git',
consent: true,
});
expect(mockRequestConsentNonInteractive).not.toHaveBeenCalled();
expect(debugLogger.log).toHaveBeenCalledWith(
'You have consented to the following:',
);
expect(debugLogger.log).toHaveBeenCalledWith('Mock Consent String');
expect(mockInstallSkill).toHaveBeenCalled();
});
it('should abort installation if consent is denied', async () => {
mockRequestConsentNonInteractive.mockResolvedValue(false);
mockInstallSkill.mockImplementation(async (_s, _sc, _p, _ol, rc) => {
if (!(await rc([]))) {
throw new Error('Skill installation cancelled by user.');
}
return [];
});
await handleInstall({
source: 'https://example.com/repo.git',
});
expect(debugLogger.error).toHaveBeenCalledWith(
'Skill installation cancelled by user.',
);
expect(process.exit).toHaveBeenCalledWith(1);
});
it('should call installSkill with correct arguments for workspace scope and subpath', async () => {
@@ -76,6 +128,7 @@ describe('skill install command', () => {
'workspace',
'my-skills-dir',
expect.any(Function),
expect.any(Function),
);
});

View File

@@ -5,24 +5,43 @@
*/
import type { CommandModule } from 'yargs';
import { debugLogger } from '@google/gemini-cli-core';
import { debugLogger, type SkillDefinition } from '@google/gemini-cli-core';
import { getErrorMessage } from '../../utils/errors.js';
import { exitCli } from '../utils.js';
import { installSkill } from '../../utils/skillUtils.js';
import chalk from 'chalk';
import {
requestConsentNonInteractive,
skillsConsentString,
} from '../../config/extensions/consent.js';
interface InstallArgs {
source: string;
scope?: 'user' | 'workspace';
path?: string;
consent?: boolean;
}
export async function handleInstall(args: InstallArgs) {
try {
const { source } = args;
const { source, consent } = args;
const scope = args.scope ?? 'user';
const subpath = args.path;
const requestConsent = async (
skills: SkillDefinition[],
targetDir: string,
) => {
if (consent) {
debugLogger.log('You have consented to the following:');
debugLogger.log(await skillsConsentString(skills, source, targetDir));
return true;
}
return requestConsentNonInteractive(
await skillsConsentString(skills, source, targetDir),
);
};
const installedSkills = await installSkill(
source,
scope,
@@ -30,6 +49,7 @@ export async function handleInstall(args: InstallArgs) {
(msg) => {
debugLogger.log(msg);
},
requestConsent,
);
for (const skill of installedSkills) {
@@ -68,6 +88,12 @@ export const installCommand: CommandModule = {
'Sub-path within the repository to install from (only used for git repository sources).',
type: 'string',
})
.option('consent', {
describe:
'Acknowledge the security risks of installing a skill and skip the confirmation prompt.',
type: 'boolean',
default: false,
})
.check((argv) => {
if (!argv.source) {
throw new Error('The source argument must be provided.');
@@ -79,6 +105,7 @@ export const installCommand: CommandModule = {
source: argv['source'] as string,
scope: argv['scope'] as 'user' | 'workspace',
path: argv['path'] as string | undefined,
consent: argv['consent'] as boolean | undefined,
});
await exitCli();
},

View File

@@ -1298,10 +1298,11 @@ describe('extension tests', () => {
expect(mockRequestConsent).toHaveBeenCalledWith(
`Installing extension "my-local-extension".
${INSTALL_WARNING_MESSAGE}
This extension will run the following MCP servers:
* test-server (local): node dobadthing \\u001b[12D\\u001b[K server.js
* test-server-2 (remote): https://google.com`,
* test-server-2 (remote): https://google.com
${INSTALL_WARNING_MESSAGE}`,
);
});

View File

@@ -191,12 +191,13 @@ describe('consent', () => {
const expectedConsentString = [
'Installing extension "test-ext".',
INSTALL_WARNING_MESSAGE,
'This extension will run the following MCP servers:',
' * server1 (local): npm start',
' * server2 (remote): https://remote.com',
'This extension will append info to your gemini.md context using my-context.md',
'This extension will exclude the following core tools: tool1,tool2',
'',
INSTALL_WARNING_MESSAGE,
].join('\n');
expect(requestConsent).toHaveBeenCalledWith(expectedConsentString);
@@ -324,7 +325,6 @@ describe('consent', () => {
const expectedConsentString = [
'Installing extension "test-ext".',
INSTALL_WARNING_MESSAGE,
'This extension will run the following MCP servers:',
' * server1 (local): npm start',
' * server2 (remote): https://remote.com',
@@ -332,13 +332,17 @@ describe('consent', () => {
'This extension will exclude the following core tools: tool1,tool2',
'',
chalk.bold('Agent Skills:'),
SKILLS_WARNING_MESSAGE,
'This extension will install the following agent skills:',
'\nThis extension will install the following agent skills:\n',
` * ${chalk.bold('skill1')}: desc1`,
` (Location: ${skill1.location}) (2 items in directory)`,
` * ${chalk.bold('skill2')}: desc2`,
` (Location: ${skill2.location}) (1 items in directory)`,
chalk.dim(` (Source: ${skill1.location}) (2 items in directory)`),
'',
` * ${chalk.bold('skill2')}: desc2`,
chalk.dim(` (Source: ${skill2.location}) (1 items in directory)`),
'',
'',
INSTALL_WARNING_MESSAGE,
'',
SKILLS_WARNING_MESSAGE,
].join('\n');
expect(requestConsent).toHaveBeenCalledWith(expectedConsentString);
@@ -375,10 +379,42 @@ describe('consent', () => {
expect(requestConsent).toHaveBeenCalledWith(
expect.stringContaining(
` (Location: ${skill.location}) ${chalk.red('⚠️ (Could not count items in directory)')}`,
` (Source: ${skill.location}) ${chalk.red('⚠️ (Could not count items in directory)')}`,
),
);
});
});
});
describe('skillsConsentString', () => {
it('should generate a consent string for skills', async () => {
const skill1Dir = path.join(tempDir, 'skill1');
await fs.mkdir(skill1Dir, { recursive: true });
await fs.writeFile(path.join(skill1Dir, 'SKILL.md'), 'body1');
const skill1: SkillDefinition = {
name: 'skill1',
description: 'desc1',
location: path.join(skill1Dir, 'SKILL.md'),
body: 'body1',
};
const { skillsConsentString } = await import('./consent.js');
const consentString = await skillsConsentString(
[skill1],
'https://example.com/repo.git',
'/mock/target/dir',
);
expect(consentString).toContain(
'Installing agent skill(s) from "https://example.com/repo.git".',
);
expect(consentString).toContain('Install Destination: /mock/target/dir');
expect(consentString).toContain('\n' + SKILLS_WARNING_MESSAGE);
expect(consentString).toContain(` * ${chalk.bold('skill1')}: desc1`);
expect(consentString).toContain(
chalk.dim(`(Source: ${skill1.location}) (1 items in directory)`),
);
});
});
});

View File

@@ -21,6 +21,27 @@ export const SKILLS_WARNING_MESSAGE = chalk.yellow(
"Agent skills inject specialized instructions and domain-specific knowledge into the agent's system prompt. This can change how the agent interprets your requests and interacts with your environment. Review the skill definitions at the location(s) provided below to ensure they meet your security standards.",
);
/**
* Builds a consent string for installing agent skills.
*/
export async function skillsConsentString(
skills: SkillDefinition[],
source: string,
targetDir?: string,
): Promise<string> {
const output: string[] = [];
output.push(`Installing agent skill(s) from "${source}".`);
output.push('\nThe following agent skill(s) will be installed:\n');
output.push(...(await renderSkillsList(skills)));
if (targetDir) {
output.push(`Install Destination: ${targetDir}`);
}
output.push('\n' + SKILLS_WARNING_MESSAGE);
return output.join('\n');
}
/**
* Requests consent from the user to perform an action, by reading a Y/n
* character from stdin.
@@ -120,7 +141,6 @@ async function extensionConsentString(
const output: string[] = [];
const mcpServerEntries = Object.entries(sanitizedConfig.mcpServers || {});
output.push(`Installing extension "${sanitizedConfig.name}".`);
output.push(INSTALL_WARNING_MESSAGE);
if (mcpServerEntries.length) {
output.push('This extension will run the following MCP servers:');
@@ -149,23 +169,37 @@ async function extensionConsentString(
}
if (skills.length > 0) {
output.push(`\n${chalk.bold('Agent Skills:')}`);
output.push(SKILLS_WARNING_MESSAGE);
output.push('This extension will install the following agent skills:');
for (const skill of skills) {
output.push(` * ${chalk.bold(skill.name)}: ${skill.description}`);
const skillDir = path.dirname(skill.location);
let fileCountStr = '';
try {
const skillDirItems = await fs.readdir(skillDir);
fileCountStr = ` (${skillDirItems.length} items in directory)`;
} catch {
fileCountStr = ` ${chalk.red('⚠️ (Could not count items in directory)')}`;
}
output.push(` (Location: ${skill.location})${fileCountStr}`);
output.push('\nThis extension will install the following agent skills:\n');
output.push(...(await renderSkillsList(skills)));
}
output.push('\n' + INSTALL_WARNING_MESSAGE);
if (skills.length > 0) {
output.push('\n' + SKILLS_WARNING_MESSAGE);
}
return output.join('\n');
}
/**
* Shared logic for formatting a list of agent skills for a consent prompt.
*/
async function renderSkillsList(skills: SkillDefinition[]): Promise<string[]> {
const output: string[] = [];
for (const skill of skills) {
output.push(` * ${chalk.bold(skill.name)}: ${skill.description}`);
const skillDir = path.dirname(skill.location);
let fileCountStr = '';
try {
const skillDirItems = await fs.readdir(skillDir);
fileCountStr = ` (${skillDirItems.length} items in directory)`;
} catch {
fileCountStr = ` ${chalk.red('⚠️ (Could not count items in directory)')}`;
}
output.push(chalk.dim(` (Source: ${skill.location})${fileCountStr}`));
output.push('');
}
return output.join('\n');
return output;
}
/**

View File

@@ -78,4 +78,33 @@ describe('skillUtils', () => {
const installedExists = await fs.stat(installedPath).catch(() => null);
expect(installedExists?.isDirectory()).toBe(true);
});
it('should abort installation if consent is rejected', async () => {
const mockSkillDir = path.join(tempDir, 'mock-skill-source');
const skillSubDir = path.join(mockSkillDir, 'test-skill');
await fs.mkdir(skillSubDir, { recursive: true });
await fs.writeFile(
path.join(skillSubDir, 'SKILL.md'),
'---\nname: test-skill\ndescription: test\n---\nbody',
);
const requestConsent = vi.fn().mockResolvedValue(false);
await expect(
installSkill(
mockSkillDir,
'workspace',
undefined,
() => {},
requestConsent,
),
).rejects.toThrow('Skill installation cancelled by user.');
expect(requestConsent).toHaveBeenCalled();
// Verify it was NOT copied
const installedPath = path.join(tempDir, '.gemini/skills', 'test-skill');
const installedExists = await fs.stat(installedPath).catch(() => null);
expect(installedExists).toBeNull();
});
});

View File

@@ -6,7 +6,11 @@
import { SettingScope } from '../config/settings.js';
import type { SkillActionResult } from './skillSettings.js';
import { Storage, loadSkillsFromDir } from '@google/gemini-cli-core';
import {
Storage,
loadSkillsFromDir,
type SkillDefinition,
} from '@google/gemini-cli-core';
import { cloneFromGit } from '../config/extensions/github.js';
import extract from 'extract-zip';
import * as fs from 'node:fs/promises';
@@ -79,6 +83,10 @@ export async function installSkill(
scope: 'user' | 'workspace',
subpath: string | undefined,
onLog: (msg: string) => void,
requestConsent: (
skills: SkillDefinition[],
targetDir: string,
) => Promise<boolean> = () => Promise.resolve(true),
): Promise<Array<{ name: string; location: string }>> {
let sourcePath = source;
let tempDirToClean: string | undefined = undefined;
@@ -90,85 +98,92 @@ export async function installSkill(
const isSkillFile = source.toLowerCase().endsWith('.skill');
if (isGitUrl) {
tempDirToClean = await fs.mkdtemp(path.join(os.tmpdir(), 'gemini-skill-'));
sourcePath = tempDirToClean;
try {
if (isGitUrl) {
tempDirToClean = await fs.mkdtemp(
path.join(os.tmpdir(), 'gemini-skill-'),
);
sourcePath = tempDirToClean;
onLog(`Cloning skill from ${source}...`);
// Reuse existing robust git cloning utility from extension manager.
await cloneFromGit(
{
source,
type: 'git',
},
tempDirToClean,
);
} else if (isSkillFile) {
tempDirToClean = await fs.mkdtemp(path.join(os.tmpdir(), 'gemini-skill-'));
sourcePath = tempDirToClean;
onLog(`Cloning skill from ${source}...`);
// Reuse existing robust git cloning utility from extension manager.
await cloneFromGit(
{
source,
type: 'git',
},
tempDirToClean,
);
} else if (isSkillFile) {
tempDirToClean = await fs.mkdtemp(
path.join(os.tmpdir(), 'gemini-skill-'),
);
sourcePath = tempDirToClean;
onLog(`Extracting skill from ${source}...`);
await extract(path.resolve(source), { dir: tempDirToClean });
}
onLog(`Extracting skill from ${source}...`);
await extract(path.resolve(source), { dir: tempDirToClean });
}
// If a subpath is provided, resolve it against the cloned/local root.
if (subpath) {
sourcePath = path.join(sourcePath, subpath);
}
// If a subpath is provided, resolve it against the cloned/local root.
if (subpath) {
sourcePath = path.join(sourcePath, subpath);
}
sourcePath = path.resolve(sourcePath);
sourcePath = path.resolve(sourcePath);
// Quick security check to prevent directory traversal out of temp dir when cloning
if (tempDirToClean && !sourcePath.startsWith(path.resolve(tempDirToClean))) {
// Quick security check to prevent directory traversal out of temp dir when cloning
if (
tempDirToClean &&
!sourcePath.startsWith(path.resolve(tempDirToClean))
) {
throw new Error('Invalid path: Directory traversal not allowed.');
}
onLog(`Searching for skills in ${sourcePath}...`);
const skills = await loadSkillsFromDir(sourcePath);
if (skills.length === 0) {
throw new Error(
`No valid skills found in ${source}${subpath ? ` at path "${subpath}"` : ''}. Ensure a SKILL.md file exists with valid frontmatter.`,
);
}
const workspaceDir = process.cwd();
const storage = new Storage(workspaceDir);
const targetDir =
scope === 'workspace'
? storage.getProjectSkillsDir()
: Storage.getUserSkillsDir();
if (!(await requestConsent(skills, targetDir))) {
throw new Error('Skill installation cancelled by user.');
}
await fs.mkdir(targetDir, { recursive: true });
const installedSkills: Array<{ name: string; location: string }> = [];
for (const skill of skills) {
const skillName = skill.name;
const skillDir = path.dirname(skill.location);
const destPath = path.join(targetDir, skillName);
const exists = await fs.stat(destPath).catch(() => null);
if (exists) {
onLog(`Skill "${skillName}" already exists. Overwriting...`);
await fs.rm(destPath, { recursive: true, force: true });
}
await fs.cp(skillDir, destPath, { recursive: true });
installedSkills.push({ name: skillName, location: destPath });
}
return installedSkills;
} finally {
if (tempDirToClean) {
await fs.rm(tempDirToClean, { recursive: true, force: true });
}
throw new Error('Invalid path: Directory traversal not allowed.');
}
onLog(`Searching for skills in ${sourcePath}...`);
const skills = await loadSkillsFromDir(sourcePath);
if (skills.length === 0) {
if (tempDirToClean) {
await fs.rm(tempDirToClean, { recursive: true, force: true });
}
throw new Error(
`No valid skills found in ${source}${subpath ? ` at path "${subpath}"` : ''}. Ensure a SKILL.md file exists with valid frontmatter.`,
);
}
const workspaceDir = process.cwd();
const storage = new Storage(workspaceDir);
const targetDir =
scope === 'workspace'
? storage.getProjectSkillsDir()
: Storage.getUserSkillsDir();
await fs.mkdir(targetDir, { recursive: true });
const installedSkills: Array<{ name: string; location: string }> = [];
for (const skill of skills) {
const skillName = skill.name;
const skillDir = path.dirname(skill.location);
const destPath = path.join(targetDir, skillName);
const exists = await fs.stat(destPath).catch(() => null);
if (exists) {
onLog(`Skill "${skillName}" already exists. Overwriting...`);
await fs.rm(destPath, { recursive: true, force: true });
}
await fs.cp(skillDir, destPath, { recursive: true });
installedSkills.push({ name: skillName, location: destPath });
}
if (tempDirToClean) {
await fs.rm(tempDirToClean, { recursive: true, force: true });
}
return installedSkills;
}
/**