Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/skills.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}

/**
Expand Down
21 changes: 21 additions & 0 deletions test/skills.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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"] });
Expand Down
Loading