From 749097561123928925500673f4bb2376944b9b90 Mon Sep 17 00:00:00 2001
From: clawedassistant26 <307253840+clawedassistant26@users.noreply.github.com>
Date: Sat, 25 Jul 2026 22:17:43 +0000
Subject: [PATCH] fix(skills): stop `skill install .` cloning into the skills
 dir itself

skillName() derives the name from the last path segment, so a `.` or `..`
source yields the literal name "." / "..". path.join then collapses it and
the claude clone target becomes ~/.claude/skills itself (or ~/.claude).

On a fresh machine, where ~/.claude/skills does not exist yet, the clone
succeeds: the skills root becomes a checkout of that one skill and the
command still reports installed. Where skills already exist, git fails with
an unexplained "destination path already exists" and the install is dead.

Resolve `.` / `..` sources against the filesystem so the skill is named
after the directory it points at, and reject them as an explicit --name.
---
 src/skills.mjs       | 14 +++++++++++---
 test/skills.test.mjs | 21 +++++++++++++++++++++
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/src/skills.mjs b/src/skills.mjs
index f8fc430..4b5905e 100644
--- a/src/skills.mjs
+++ b/src/skills.mjs
@@ -16,9 +16,17 @@ export function claudeSkillsDir() {
 /** Derive a skill name from a git URL or path (basename minus `.git`), or use the override. */
 export function skillName(source, override) {
   const sanitize = (s) => String(s).toLowerCase().replace(/[^a-z0-9._-]/g, "-").replace(/^-+|-+$/g, "");
-  if (override) return sanitize(override) || "skill";
-  const base = String(source).replace(/[/\\]+$/, "").split(/[/\\]/).pop() || "skill";
-  return sanitize(base.replace(/\.git$/i, "")) || "skill";
+  // `.` and `..` are directory references, not names: path.join would collapse
+  // them and land the clone on the skills dir itself (or its parent).
+  const named = (s) => (s === "." || s === ".." ? "" : s);
+  if (override) return named(sanitize(override)) || "skill";
+  const raw = String(source).replace(/[/\\]+$/, "");
+  const base = raw.split(/[/\\]/).pop() || "skill";
+  const derived = named(sanitize(base.replace(/\.git$/i, "")));
+  if (derived) return derived;
+  // A `.` / `..` source means "this directory", so name the skill after the
+  // directory it resolves to: `skill install .` installs the repo you are in.
+  return named(sanitize(path.basename(path.resolve(raw)))) || "skill";
 }
 
 /**
diff --git a/test/skills.test.mjs b/test/skills.test.mjs
index c00f7c9..c92c5a5 100644
--- a/test/skills.test.mjs
+++ b/test/skills.test.mjs
@@ -14,6 +14,27 @@ test("skillName derives from a git url or path, or takes an override", () => {
   assert.equal(skillName("whatever", "Custom Name"), "custom-name");
 });
 
+test("skillName never yields `.` or `..`, which would escape the skills dir", () => {
+  const here = path.basename(process.cwd());
+  const parent = path.basename(path.dirname(process.cwd()));
+  assert.equal(skillName("."), here);
+  assert.equal(skillName("./"), here);
+  assert.equal(skillName("a/b/."), "b");
+  assert.equal(skillName(".."), parent);
+  assert.equal(skillName("../"), parent);
+  assert.equal(skillName("whatever", "."), "skill");
+  assert.equal(skillName("whatever", ".."), "skill");
+});
+
+test("the claude clone destination stays inside the skills dir", () => {
+  const dir = claudeSkillsDir();
+  for (const source of [".", "./", "..", "../", "a/b/."]) {
+    const { args } = skillInstallAction("claude", { source, name: skillName(source) });
+    const dest = args.at(-1);
+    assert.equal(path.dirname(dest), dir, `${source} escaped to ${dest}`);
+  }
+});
+
 test("skillInstallAction: gemini installs natively, claude clones into its skills dir", () => {
   const gemini = skillInstallAction("gemini", { source: "https://x/y", name: "y" });
   assert.deepEqual(gemini, { cmd: "gemini", args: ["skills", "install", "https://x/y", "--scope", "user"] });