mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-06-24 18:27:01 -07:00
fix(cli): prevent path traversal false positives on safe skill names
This commit is contained in:
Om Patel
@@ -1,343 +0,0 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { SettingScope } from '../config/settings.js';
|
||||
import type { SkillActionResult } from './skillSettings.js';
|
||||
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';
|
||||
import * as path from 'node:path';
|
||||
import * as os from 'node:os';
|
||||
|
||||
/**
|
||||
* Shared logic for building the core skill action message while allowing the
|
||||
* caller to control how each scope and its path are rendered (e.g., bolding or
|
||||
* dimming).
|
||||
*
|
||||
* This function ONLY returns the description of what happened. It is up to the
|
||||
* caller to append any interface-specific guidance (like "Use /skills reload"
|
||||
* or "Restart required").
|
||||
*/
|
||||
export function renderSkillActionFeedback(
|
||||
result: SkillActionResult,
|
||||
formatScope: (label: string, path: string) => string,
|
||||
): string {
|
||||
const { skillName, action, status, error } = result;
|
||||
|
||||
if (status === 'error') {
|
||||
return (
|
||||
error ||
|
||||
`An error occurred while attempting to ${action} skill "${skillName}".`
|
||||
);
|
||||
}
|
||||
|
||||
if (status === 'no-op') {
|
||||
return `Skill "${skillName}" is already ${action === 'enable' ? 'enabled' : 'disabled'}.`;
|
||||
}
|
||||
|
||||
const isEnable = action === 'enable';
|
||||
const actionVerb = isEnable ? 'enabled' : 'disabled';
|
||||
const preposition = isEnable
|
||||
? 'by removing it from the disabled list in'
|
||||
: 'by adding it to the disabled list in';
|
||||
|
||||
const formatScopeItem = (s: { scope: SettingScope; path: string }) => {
|
||||
const label =
|
||||
s.scope === SettingScope.Workspace ? 'workspace' : s.scope.toLowerCase();
|
||||
return formatScope(label, s.path);
|
||||
};
|
||||
|
||||
const totalAffectedScopes = [
|
||||
...result.modifiedScopes,
|
||||
...result.alreadyInStateScopes,
|
||||
];
|
||||
|
||||
if (totalAffectedScopes.length === 2) {
|
||||
const s1 = formatScopeItem(totalAffectedScopes[0]);
|
||||
const s2 = formatScopeItem(totalAffectedScopes[1]);
|
||||
|
||||
if (isEnable) {
|
||||
return `Skill "${skillName}" ${actionVerb} ${preposition} ${s1} and ${s2} settings.`;
|
||||
} else {
|
||||
return `Skill "${skillName}" is now disabled in both ${s1} and ${s2} settings.`;
|
||||
}
|
||||
}
|
||||
|
||||
const s = formatScopeItem(totalAffectedScopes[0]);
|
||||
return `Skill "${skillName}" ${actionVerb} ${preposition} ${s} settings.`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Central logic for installing a skill from a remote URL or local path.
|
||||
*/
|
||||
export async function installSkill(
|
||||
source: string,
|
||||
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;
|
||||
|
||||
const isGitUrl =
|
||||
source.startsWith('git@') ||
|
||||
source.startsWith('http://') ||
|
||||
source.startsWith('https://');
|
||||
|
||||
const isSkillFile = source.toLowerCase().endsWith('.skill');
|
||||
|
||||
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(`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);
|
||||
}
|
||||
|
||||
sourcePath = path.resolve(sourcePath);
|
||||
|
||||
// Quick security check to prevent directory traversal out of temp dir when cloning
|
||||
if (tempDirToClean) {
|
||||
const resolvedTemp = path.resolve(tempDirToClean);
|
||||
const relative = path.relative(resolvedTemp, sourcePath);
|
||||
if (relative.startsWith('..') || path.isAbsolute(relative)) {
|
||||
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.');
|
||||
}
|
||||
|
||||
const resolvedTarget = path.resolve(targetDir);
|
||||
await fs.mkdir(resolvedTarget, { 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.resolve(resolvedTarget, skillName);
|
||||
|
||||
const relative = path.relative(resolvedTarget, destPath);
|
||||
if (
|
||||
relative.startsWith('..') ||
|
||||
path.isAbsolute(relative) ||
|
||||
relative === ''
|
||||
) {
|
||||
throw new Error('Invalid skill name: Path traversal detected.');
|
||||
}
|
||||
|
||||
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 });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Central logic for linking a skill from a local path via symlink.
|
||||
*/
|
||||
export async function linkSkill(
|
||||
source: string,
|
||||
scope: 'user' | 'workspace',
|
||||
onLog: (msg: string) => void,
|
||||
requestConsent: (
|
||||
skills: SkillDefinition[],
|
||||
targetDir: string,
|
||||
) => Promise<boolean> = () => Promise.resolve(true),
|
||||
): Promise<Array<{ name: string; location: string }>> {
|
||||
const sourcePath = path.resolve(source);
|
||||
|
||||
onLog(`Searching for skills in ${sourcePath}...`);
|
||||
const skills = await loadSkillsFromDir(sourcePath);
|
||||
|
||||
if (skills.length === 0) {
|
||||
throw new Error(
|
||||
`No valid skills found in "${sourcePath}". Ensure a SKILL.md file exists with valid frontmatter.`,
|
||||
);
|
||||
}
|
||||
|
||||
// Check for internal name collisions
|
||||
const seenNames = new Map<string, string>();
|
||||
for (const skill of skills) {
|
||||
if (seenNames.has(skill.name)) {
|
||||
throw new Error(
|
||||
`Duplicate skill name "${skill.name}" found at multiple locations:\n - ${seenNames.get(skill.name)}\n - ${skill.location}`,
|
||||
);
|
||||
}
|
||||
seenNames.set(skill.name, skill.location);
|
||||
}
|
||||
|
||||
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 linking cancelled by user.');
|
||||
}
|
||||
|
||||
const resolvedTarget = path.resolve(targetDir);
|
||||
await fs.mkdir(resolvedTarget, { recursive: true });
|
||||
|
||||
const linkedSkills: Array<{ name: string; location: string }> = [];
|
||||
|
||||
for (const skill of skills) {
|
||||
const skillName = skill.name;
|
||||
const skillSourceDir = path.dirname(skill.location);
|
||||
const destPath = path.resolve(resolvedTarget, skillName);
|
||||
|
||||
const relative = path.relative(resolvedTarget, destPath);
|
||||
if (
|
||||
relative.startsWith('..') ||
|
||||
path.isAbsolute(relative) ||
|
||||
relative === ''
|
||||
) {
|
||||
throw new Error('Invalid skill name: Path traversal detected.');
|
||||
}
|
||||
|
||||
const exists = await fs.lstat(destPath).catch(() => null);
|
||||
if (exists) {
|
||||
onLog(
|
||||
`Skill "${skillName}" already exists at destination. Overwriting...`,
|
||||
);
|
||||
await fs.rm(destPath, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
// Use 'junction' on Windows to avoid EPERM errors — junctions don't
|
||||
// require elevated privileges or Developer Mode (fixes #24816)
|
||||
await fs.symlink(
|
||||
skillSourceDir,
|
||||
destPath,
|
||||
process.platform === 'win32' ? 'junction' : 'dir',
|
||||
);
|
||||
linkedSkills.push({ name: skillName, location: destPath });
|
||||
}
|
||||
|
||||
return linkedSkills;
|
||||
}
|
||||
|
||||
/**
|
||||
* Central logic for uninstalling a skill by name.
|
||||
*/
|
||||
export async function uninstallSkill(
|
||||
name: string,
|
||||
scope: 'user' | 'workspace',
|
||||
): Promise<{ location: string } | null> {
|
||||
const workspaceDir = process.cwd();
|
||||
const storage = new Storage(workspaceDir);
|
||||
const targetDir =
|
||||
scope === 'workspace'
|
||||
? storage.getProjectSkillsDir()
|
||||
: Storage.getUserSkillsDir();
|
||||
|
||||
const resolvedTarget = path.resolve(targetDir);
|
||||
|
||||
// Load all skills in the target directory to find the one with the matching name
|
||||
const discoveredSkills = await loadSkillsFromDir(resolvedTarget);
|
||||
const skillToUninstall = discoveredSkills.find((s) => s.name === name);
|
||||
|
||||
if (!skillToUninstall) {
|
||||
// Fallback: Check if a directory with the given name exists.
|
||||
// This maintains backward compatibility for cases where the metadata might be missing or corrupted
|
||||
// but the directory name matches the user's request.
|
||||
const skillPath = path.resolve(resolvedTarget, name);
|
||||
|
||||
// Security check: ensure the resolved path is within the target directory to prevent path traversal
|
||||
const relative = path.relative(resolvedTarget, skillPath);
|
||||
if (
|
||||
relative.startsWith('..') ||
|
||||
path.isAbsolute(relative) ||
|
||||
relative === ''
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const exists = await fs.lstat(skillPath).catch(() => null);
|
||||
|
||||
if (!exists) {
|
||||
return null;
|
||||
}
|
||||
|
||||
await fs.rm(skillPath, { recursive: true, force: true });
|
||||
return { location: skillPath };
|
||||
}
|
||||
|
||||
const skillDir = path.resolve(path.dirname(skillToUninstall.location));
|
||||
const relative = path.relative(resolvedTarget, skillDir);
|
||||
if (
|
||||
relative.startsWith('..') ||
|
||||
path.isAbsolute(relative) ||
|
||||
relative === ''
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
await fs.rm(skillDir, { recursive: true, force: true });
|
||||
return { location: skillDir };
|
||||
}
|
||||
Reference in New Issue
Block a user