fix(cli): prevent path traversal vulnerabilities during skill install… (#27767)

packages/cli/src/utils/skillUtils.ts

@@ -75,6 +75,18 @@ export function renderSkillActionFeedback(
   return `Skill "${skillName}" ${actionVerb} ${preposition} ${s} settings.`;
 }
 
+function isPathTraversal(relative: string): boolean {
+  return (
+    relative === '..' ||
+    relative.startsWith('..' + path.sep) ||
+    path.isAbsolute(relative)
+  );
+}
+
+function isInvalidSubpath(relative: string): boolean {
+  return relative === '' || isPathTraversal(relative);
+}
+
 /**
  * Central logic for installing a skill from a remote URL or local path.
  */
@@ -132,11 +144,12 @@ export async function installSkill(
     sourcePath = path.resolve(sourcePath);
 
     // 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.');
+    if (tempDirToClean) {
+      const resolvedTemp = path.resolve(tempDirToClean);
+      const relative = path.relative(resolvedTemp, sourcePath);
+      if (isPathTraversal(relative)) {
+        throw new Error('Invalid path: Directory traversal not allowed.');
+      }
     }
 
     onLog(`Searching for skills in ${sourcePath}...`);
@@ -159,16 +172,22 @@ export async function installSkill(
       throw new Error('Skill installation cancelled by user.');
     }
 
-    await fs.mkdir(targetDir, { recursive: true });
+    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.join(targetDir, skillName);
+      const destPath = path.resolve(resolvedTarget, skillName);
 
-      const exists = await fs.stat(destPath).catch(() => null);
+      const relative = path.relative(resolvedTarget, destPath);
+      if (isInvalidSubpath(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. Overwriting...`);
         await fs.rm(destPath, { recursive: true, force: true });
@@ -231,14 +250,20 @@ export async function linkSkill(
     throw new Error('Skill linking cancelled by user.');
   }
 
-  await fs.mkdir(targetDir, { recursive: true });
+  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.join(targetDir, skillName);
+    const destPath = path.resolve(resolvedTarget, skillName);
+
+    const relative = path.relative(resolvedTarget, destPath);
+    if (isInvalidSubpath(relative)) {
+      throw new Error('Invalid skill name: Path traversal detected.');
+    }
 
     const exists = await fs.lstat(destPath).catch(() => null);
     if (exists) {
@@ -275,18 +300,21 @@ export async function uninstallSkill(
       ? 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(targetDir);
+  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(targetDir, name);
+    const skillPath = path.resolve(resolvedTarget, name);
 
     // Security check: ensure the resolved path is within the target directory to prevent path traversal
-    if (!skillPath.startsWith(path.resolve(targetDir))) {
+    const relative = path.relative(resolvedTarget, skillPath);
+    if (isInvalidSubpath(relative)) {
       return null;
     }
 
@@ -300,7 +328,12 @@ export async function uninstallSkill(
     return { location: skillPath };
   }
 
-  const skillDir = path.dirname(skillToUninstall.location);
+  const skillDir = path.resolve(path.dirname(skillToUninstall.location));
+  const relative = path.relative(resolvedTarget, skillDir);
+  if (isInvalidSubpath(relative)) {
+    return null;
+  }
+
   await fs.rm(skillDir, { recursive: true, force: true });
   return { location: skillDir };
 }