fix(cli): harden skill name handling during install (#2597)

* fix(cli): validate skill names to prevent path traversal on install

Adds boundary validation and containment checks so a remote SKILL.md
with a malicious name field (e.g. `name: ..`) cannot escape the skills
root during `ctx7 skills install`. Previously, the value flowed from
parseSkillFrontmatter to installSkillFiles unchecked, and the existing
traversal guard only verified files stayed inside the attacker-chosen
directory rather than the real skills root, enabling arbitrary file
writes outside `.claude/skills` (e.g. `.claude/settings.json` for
hook-driven RCE). symlinkSkill had the same trust issue and could
`rm(recursive: true)` arbitrary directories.

* chore: add changeset for skill name validation

* chore: soften changeset wording

Author: fahreddinozcan

.changeset/validate-skill-names.md

@@ -0,0 +1,5 @@
+---
+"ctx7": patch
+---
+
+Harden skill name handling during `ctx7 skills install` and `ctx7 skills remove`. Skill names from remote `SKILL.md` files are now restricted to a safe character set, and the install sinks assert the target directory is a direct child of the skills root before writing.

packages/cli/src/__tests__/installer.test.ts

@@ -0,0 +1,159 @@
+import { afterEach, beforeEach, describe, expect, test } from "vitest";
+import { mkdir, readFile, writeFile, rm, access } from "fs/promises";
+import { join } from "path";
+import { tmpdir } from "os";
+
+import { installSkillFiles, symlinkSkill } from "../utils/installer.js";
+import { isSafeSkillName, assertSkillNameInRoot } from "../utils/skill-name.js";
+
+let tempDir: string;
+
+async function exists(path: string): Promise<boolean> {
+  try {
+    await access(path);
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+beforeEach(async () => {
+  tempDir = join(tmpdir(), `ctx7-installer-test-${Date.now()}-${Math.random()}`);
+  await mkdir(tempDir, { recursive: true });
+});
+
+afterEach(async () => {
+  await rm(tempDir, { recursive: true, force: true });
+});
+
+describe("isSafeSkillName", () => {
+  test("accepts typical skill names", () => {
+    for (const name of [
+      "pdf",
+      "find-docs",
+      "skill_one",
+      "skill.v2",
+      "a1",
+      "abc123",
+      "PDF",
+      "Find-Docs",
+      "MySkill",
+    ]) {
+      expect(isSafeSkillName(name)).toBe(true);
+    }
+  });
+
+  test("rejects path traversal and separators", () => {
+    for (const name of [
+      "..",
+      ".",
+      "../evil",
+      "..\\evil",
+      "a/b",
+      "a\\b",
+      "/abs",
+      "C:\\drive",
+      "with space",
+      "",
+      ".hidden",
+      "name\0",
+      "name\nwith-newline",
+    ]) {
+      expect(isSafeSkillName(name)).toBe(false);
+    }
+  });
+
+  test("rejects names longer than 128 chars", () => {
+    expect(isSafeSkillName("a".repeat(128))).toBe(true);
+    expect(isSafeSkillName("a".repeat(129))).toBe(false);
+  });
+});
+
+describe("assertSkillNameInRoot", () => {
+  test("returns resolved path for safe name", () => {
+    const result = assertSkillNameInRoot("/tmp/skills", "pdf");
+    expect(result).toBe("/tmp/skills/pdf");
+  });
+
+  test("throws on traversal", () => {
+    expect(() => assertSkillNameInRoot("/tmp/skills", "..")).toThrow();
+    expect(() => assertSkillNameInRoot("/tmp/skills", "../evil")).toThrow();
+  });
+});
+
+describe("installSkillFiles", () => {
+  test("writes files inside the skill directory", async () => {
+    const skillsRoot = join(tempDir, "skills");
+    await mkdir(skillsRoot, { recursive: true });
+
+    await installSkillFiles("good", [{ path: "SKILL.md", content: "hello" }], skillsRoot);
+
+    const written = await readFile(join(skillsRoot, "good", "SKILL.md"), "utf8");
+    expect(written).toBe("hello");
+  });
+
+  test("rejects skill name '..' and does not write outside skills root", async () => {
+    const skillsRoot = join(tempDir, ".claude", "skills");
+    await mkdir(skillsRoot, { recursive: true });
+
+    const settingsPath = join(tempDir, ".claude", "settings.json");
+
+    await expect(
+      installSkillFiles("..", [{ path: "settings.json", content: '{"hooks":{}}' }], skillsRoot)
+    ).rejects.toThrow();
+
+    expect(await exists(settingsPath)).toBe(false);
+  });
+
+  test("rejects skill names with path separators", async () => {
+    const skillsRoot = join(tempDir, "skills");
+    await mkdir(skillsRoot, { recursive: true });
+
+    for (const bad of ["../evil", "a/b", "..\\evil"]) {
+      await expect(
+        installSkillFiles(bad, [{ path: "SKILL.md", content: "x" }], skillsRoot)
+      ).rejects.toThrow();
+    }
+  });
+
+  test("still rejects traversal in file.path", async () => {
+    const skillsRoot = join(tempDir, "skills");
+    await mkdir(skillsRoot, { recursive: true });
+
+    await expect(
+      installSkillFiles("good", [{ path: "../escape.txt", content: "x" }], skillsRoot)
+    ).rejects.toThrow(/outside/);
+  });
+});
+
+describe("symlinkSkill", () => {
+  test("creates symlink under skills root for safe name", async () => {
+    const skillsRoot = join(tempDir, "linked");
+    await mkdir(skillsRoot, { recursive: true });
+
+    const source = join(tempDir, "source");
+    await mkdir(source, { recursive: true });
+    await writeFile(join(source, "marker"), "ok");
+
+    await symlinkSkill("good", source, skillsRoot);
+
+    const linkedMarker = await readFile(join(skillsRoot, "good", "marker"), "utf8");
+    expect(linkedMarker).toBe("ok");
+  });
+
+  test("does not rm parent when skill name is '..'", async () => {
+    const skillsRoot = join(tempDir, ".cursor", "skills");
+    await mkdir(skillsRoot, { recursive: true });
+
+    const sentinel = join(tempDir, ".cursor", "keep.txt");
+    await writeFile(sentinel, "sentinel");
+
+    const source = join(tempDir, "source");
+    await mkdir(source, { recursive: true });
+
+    await expect(symlinkSkill("..", source, skillsRoot)).rejects.toThrow();
+
+    expect(await exists(sentinel)).toBe(true);
+    expect(await exists(join(tempDir, ".cursor"))).toBe(true);
+  });
+});

packages/cli/src/commands/skill.ts

@@ -30,6 +30,7 @@ import {
   getTrustLabel,
 } from "../utils/prompts.js";
 import { installSkillFiles, symlinkSkill } from "../utils/installer.js";
+import { assertSkillNameInRoot } from "../utils/skill-name.js";
 import { listSkillsFromGitHub, getSkillFromGitHub } from "../utils/github.js";
 import { trackEvent } from "../utils/tracking.js";
 import { registerGenerateCommand } from "./generate.js";
@@ -708,7 +709,13 @@ async function removeCommand(name: string, options: RemoveOptions): Promise<void
   }
 
   const skillsDir = getTargetDirFromSelection(target.ide, target.scope);
-  const skillPath = join(skillsDir, name);
+  let skillPath: string;
+  try {
+    skillPath = assertSkillNameInRoot(skillsDir, name);
+  } catch {
+    log.error(`Invalid skill name: ${name}`);
+    return;
+  }
 
   try {
     await rm(skillPath, { recursive: true });

packages/cli/src/utils/github.ts

@@ -1,5 +1,6 @@
 import { execSync } from "node:child_process";
 import type { SkillFile, Skill } from "../types.js";
+import { isSafeSkillName } from "./skill-name.js";
 
 const GITHUB_API = "https://api.github.com";
 const GITHUB_RAW = "https://raw.githubusercontent.com";
@@ -100,10 +101,8 @@ function parseSkillFrontmatter(content: string): { name: string; description: st
 
   const nameMatch = frontmatter.match(/^name:\s*(.+)$/m);
   if (!nameMatch) return null;
-  const name = nameMatch[1]
-    .trim()
-    .replace(/^["']|["']$/g, "")
-    .toLowerCase();
+  const name = nameMatch[1].trim().replace(/^["']|["']$/g, "");
+  if (!isSafeSkillName(name)) return null;
 
   let description = "";
   const multiLineMatch = frontmatter.match(/^description:\s*([|>])-?\s*$/m);
@@ -230,7 +229,7 @@ export async function getSkillFromGitHub(
 ): Promise<GitHubSkillsResult & { skill?: Skill & { project: string } }> {
   const result = await listSkillsFromGitHub(project);
   if (result.status !== "ok") return result;
-  const skill = result.skills.find((s) => s.name === skillName.toLowerCase());
+  const skill = result.skills.find((s) => s.name.toLowerCase() === skillName.toLowerCase());
   return { ...result, skill };
 }
 

packages/cli/src/utils/installer.ts

@@ -1,14 +1,15 @@
 import { mkdir, writeFile, rm, symlink, lstat } from "fs/promises";
-import { join, resolve, dirname } from "path";
+import { resolve, dirname } from "path";
 
 import type { SkillFile } from "../types.js";
+import { assertSkillNameInRoot } from "./skill-name.js";
 
 export async function installSkillFiles(
   skillName: string,
   files: SkillFile[],
-  targetDir: string
+  skillsRoot: string
 ): Promise<void> {
-  const skillDir = resolve(targetDir, skillName);
+  const skillDir = assertSkillNameInRoot(skillsRoot, skillName);
 
   for (const file of files) {
     const filePath = resolve(skillDir, file.path);
@@ -32,9 +33,9 @@ export async function installSkillFiles(
 export async function symlinkSkill(
   skillName: string,
   sourcePath: string,
-  targetDir: string
+  skillsRoot: string
 ): Promise<void> {
-  const targetPath = join(targetDir, skillName);
+  const targetPath = assertSkillNameInRoot(skillsRoot, skillName);
 
   try {
     const stats = await lstat(targetPath);
@@ -43,6 +44,6 @@ export async function symlinkSkill(
     }
   } catch {}
 
-  await mkdir(targetDir, { recursive: true });
+  await mkdir(skillsRoot, { recursive: true });
   await symlink(sourcePath, targetPath);
 }

packages/cli/src/utils/skill-name.ts

@@ -0,0 +1,24 @@
+import { resolve, dirname, basename } from "path";
+
+const SAFE_NAME = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/;
+
+export function isSafeSkillName(name: string): boolean {
+  if (typeof name !== "string") return false;
+  if (name.length === 0 || name.length > 128) return false;
+  if (name === "." || name === "..") return false;
+  if (name.includes("\0")) return false;
+  if (!SAFE_NAME.test(name)) return false;
+  return true;
+}
+
+export function assertSkillNameInRoot(skillsRoot: string, skillName: string): string {
+  if (!isSafeSkillName(skillName)) {
+    throw new Error(`Unsafe skill name: ${JSON.stringify(skillName)}`);
+  }
+  const root = resolve(skillsRoot);
+  const target = resolve(root, skillName);
+  if (dirname(target) !== root || basename(target) !== skillName) {
+    throw new Error(`Skill name "${skillName}" escapes the skills root`);
+  }
+  return target;
+}