Skip to content

Commit 04c10f8

Browse files
fix(skills): stop skill install . cloning into the skills dir itself (#35)
1 parent 76c3047 commit 04c10f8

2 files changed

Lines changed: 32 additions & 3 deletions

File tree

src/skills.mjs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,17 @@ export function claudeSkillsDir() {
1616
/** Derive a skill name from a git URL or path (basename minus `.git`), or use the override. */
1717
export function skillName(source, override) {
1818
const sanitize = (s) => String(s).toLowerCase().replace(/[^a-z0-9._-]/g, "-").replace(/^-+|-+$/g, "");
19-
if (override) return sanitize(override) || "skill";
20-
const base = String(source).replace(/[/\\]+$/, "").split(/[/\\]/).pop() || "skill";
21-
return sanitize(base.replace(/\.git$/i, "")) || "skill";
19+
// `.` and `..` are directory references, not names: path.join would collapse
20+
// them and land the clone on the skills dir itself (or its parent).
21+
const named = (s) => (s === "." || s === ".." ? "" : s);
22+
if (override) return named(sanitize(override)) || "skill";
23+
const raw = String(source).replace(/[/\\]+$/, "");
24+
const base = raw.split(/[/\\]/).pop() || "skill";
25+
const derived = named(sanitize(base.replace(/\.git$/i, "")));
26+
if (derived) return derived;
27+
// A `.` / `..` source means "this directory", so name the skill after the
28+
// directory it resolves to: `skill install .` installs the repo you are in.
29+
return named(sanitize(path.basename(path.resolve(raw)))) || "skill";
2230
}
2331

2432
/**

test/skills.test.mjs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,27 @@ test("skillName derives from a git url or path, or takes an override", () => {
1414
assert.equal(skillName("whatever", "Custom Name"), "custom-name");
1515
});
1616

17+
test("skillName never yields `.` or `..`, which would escape the skills dir", () => {
18+
const here = path.basename(process.cwd());
19+
const parent = path.basename(path.dirname(process.cwd()));
20+
assert.equal(skillName("."), here);
21+
assert.equal(skillName("./"), here);
22+
assert.equal(skillName("a/b/."), "b");
23+
assert.equal(skillName(".."), parent);
24+
assert.equal(skillName("../"), parent);
25+
assert.equal(skillName("whatever", "."), "skill");
26+
assert.equal(skillName("whatever", ".."), "skill");
27+
});
28+
29+
test("the claude clone destination stays inside the skills dir", () => {
30+
const dir = claudeSkillsDir();
31+
for (const source of [".", "./", "..", "../", "a/b/."]) {
32+
const { args } = skillInstallAction("claude", { source, name: skillName(source) });
33+
const dest = args.at(-1);
34+
assert.equal(path.dirname(dest), dir, `${source} escaped to ${dest}`);
35+
}
36+
});
37+
1738
test("skillInstallAction: gemini installs natively, claude clones into its skills dir", () => {
1839
const gemini = skillInstallAction("gemini", { source: "https://x/y", name: "y" });
1940
assert.deepEqual(gemini, { cmd: "gemini", args: ["skills", "install", "https://x/y", "--scope", "user"] });

0 commit comments

Comments
 (0)