From 42440d0cc74c76108c7c830b1f4c39c9b47afa33 Mon Sep 17 00:00:00 2001 From: Justin Ling Date: Fri, 14 Aug 2026 18:39:52 +0800 Subject: [PATCH 1/2] fix(install): reject option-like skill names --- src/catalog/io.ts | 6 +++ src/catalog/types.ts | 8 ++++ src/commands/install.ts | 4 ++ src/skills/cli.ts | 4 ++ test/viblib.test.ts | 90 ++++++++++++++++++++++++++++++++++++++++- 5 files changed, 111 insertions(+), 1 deletion(-) diff --git a/src/catalog/io.ts b/src/catalog/io.ts index 0475d63..68a38bf 100644 --- a/src/catalog/io.ts +++ b/src/catalog/io.ts @@ -7,6 +7,7 @@ import { CATALOG_VERSION, type Catalog, type CatalogSkill, + assertSafeSkillName, emptyCatalog, normalizeCategory, normalizeInstallName, @@ -69,6 +70,11 @@ function validateSkill(value: unknown, key: string): CatalogSkill { ) { invalid(`${field}.skill`, "expected a non-empty string."); } + try { + assertSafeSkillName(value.skill); + } catch { + invalid(`${field}.skill`, "is not a valid install name."); + } if ( typeof value.source !== "string" || !value.source.trim() || diff --git a/src/catalog/types.ts b/src/catalog/types.ts index 973b507..c021816 100644 --- a/src/catalog/types.ts +++ b/src/catalog/types.ts @@ -19,6 +19,14 @@ const MAX_INSTALL_NAME_LENGTH = 255, INVALID_CATEGORY_RUN = /[^a-z0-9]+/g, CATEGORY_EDGE = /^-+|-+$/g; +export function assertSafeSkillName(value: string): void { + if (value.startsWith("-")) { + throw new ViblibError( + "Option-like skill names cannot be passed to skills@1.5.22." + ); + } +} + export function normalizeInstallName(value: string): string { const normalized = value .toLowerCase() diff --git a/src/commands/install.ts b/src/commands/install.ts index 574e724..d7bac21 100644 --- a/src/commands/install.ts +++ b/src/commands/install.ts @@ -3,6 +3,7 @@ import prompts from "prompts"; import { loadCatalog } from "../catalog/io.js"; import { type CatalogSkill, + assertSafeSkillName, normalizeCategory, normalizeInstallName, } from "../catalog/types.js"; @@ -131,6 +132,9 @@ export async function applyCatalogInstall( runner?: SkillsRunner; } ): Promise<{ failed: string[]; installed: number }> { + for (const entry of entries) { + assertSafeSkillName(entry.skill); + } const grouped = new Map(); for (const entry of entries) { grouped.set(entry.source, [...(grouped.get(entry.source) ?? []), entry]); diff --git a/src/skills/cli.ts b/src/skills/cli.ts index a74469a..7073090 100644 --- a/src/skills/cli.ts +++ b/src/skills/cli.ts @@ -3,6 +3,7 @@ import { createRequire } from "node:module"; import path from "node:path"; import { stripVTControlCharacters } from "node:util"; +import { assertSafeSkillName } from "../catalog/types.js"; import { ViblibError } from "../util/errors.js"; import { sourceForCatalog } from "./source.js"; import type { InstalledSkill, SkillScope } from "./types.js"; @@ -164,6 +165,9 @@ export async function discoverSkills( { exitCode: 2 } ); } + for (const skill of skills) { + assertSafeSkillName(skill.name); + } return { skills, source: await sourceForCatalog(source, options.cwd), diff --git a/test/viblib.test.ts b/test/viblib.test.ts index aee0231..1dd869d 100644 --- a/test/viblib.test.ts +++ b/test/viblib.test.ts @@ -15,7 +15,11 @@ import { } from "../src/catalog/types.js"; import { createProgram } from "../src/cli.js"; import { runAdd } from "../src/commands/add.js"; -import { runInstall, selectCatalogSkills } from "../src/commands/install.js"; +import { + applyCatalogInstall, + runInstall, + selectCatalogSkills, +} from "../src/commands/install.js"; import { runSync } from "../src/commands/sync.js"; import { runUninstall } from "../src/commands/uninstall.js"; import { @@ -145,6 +149,33 @@ describe("catalog storage", () => { }) ).toThrow('skills."my-skill".categories[0]'); }); + + it("rejects option-like skill names without exposing them", () => { + const optionLikeName = "-unsafe-skill"; + let thrown: unknown = undefined; + try { + validateCatalog({ + skills: { + "unsafe-skill": { + categories: ["work"], + skill: optionLikeName, + source: "owner/repo", + }, + }, + version: 1, + }); + } catch (error) { + thrown = error; + } + expect(thrown).toBeInstanceOf(ViblibError); + expect(thrown).toMatchObject({ + message: + 'Invalid catalog at skills."unsafe-skill".skill: is not a valid install name.', + }); + expect(thrown).toMatchObject({ + message: expect.not.stringContaining(optionLikeName), + }); + }); }); describe("catalog commands", () => { @@ -226,6 +257,16 @@ describe("pinned skills adapter", () => { discoverSkills("owner/repo", { runner: async () => success("changed") }) ).rejects.toThrow("skills@1.5.22"); }); + + it("rejects option-like discovery names", async () => { + const output = DISCOVERY_OUTPUT.replace("Foo Skill", "-unsafe-skill"); + await expect( + discoverSkills("owner/repo", { runner: async () => success(output) }) + ).rejects.toThrow(ViblibError); + await expect( + discoverSkills("owner/repo", { runner: async () => success(output) }) + ).rejects.toThrow("Option-like skill names"); + }); }); describe("install and uninstall", () => { @@ -284,6 +325,53 @@ describe("install and uninstall", () => { ]); }); + it("rejects option-like names before calling the installer", async () => { + const calls: string[][] = [], + install = () => + applyCatalogInstall( + [ + { + categories: [], + skill: "-unsafe-skill", + source: "owner/repo", + }, + ], + { + runner: async (args) => { + calls.push(args); + return success(); + }, + scope: "project", + } + ); + await expect(install()).rejects.toThrow(ViblibError); + await expect(install()).rejects.toThrow("Option-like skill names"); + expect(calls).toEqual([]); + }); + + it("keeps normal multiword names intact with the option-like guard", async () => { + const calls: string[][] = []; + await applyCatalogInstall( + [ + { + categories: [], + skill: "Foo Skill", + source: "owner/repo", + }, + ], + { + runner: async (args) => { + calls.push(args); + return success(); + }, + scope: "project", + } + ); + expect(calls).toEqual([ + ["add", "owner/repo", "--skill", "Foo Skill", "--yes"], + ]); + }); + it("expands uninstall --all to catalog names and all agents", async () => { const calls: string[][] = []; await runUninstall({ From e7128fa7f92575183f5679e82bfc77719a3d8afe Mon Sep 17 00:00:00 2001 From: Justin Ling Date: Fri, 14 Aug 2026 18:45:34 +0800 Subject: [PATCH 2/2] docs: mark option-like skill name plan as done --- plans/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plans/README.md b/plans/README.md index 6b214e7..b8af95b 100644 --- a/plans/README.md +++ b/plans/README.md @@ -12,7 +12,7 @@ modified while creating them. | Plan | Title | Priority | Effort | Depends on | Status | | -------------------------------------------------- | ------------------------------------------------------- | -------: | -----: | ---------- | ------ | -| [001](001-reject-option-like-skill-names.md) | Reject option-like skill names before invoking `skills` | P1 | S | — | TODO | +| [001](001-reject-option-like-skill-names.md) | Reject option-like skill names before invoking `skills` | P1 | S | — | DONE | | [002](002-compare-sync-source-subpaths.md) | Detect source-subpath drift during sync | P1 | M | — | TODO | | [003](003-block-credential-bearing-sources.md) | Block credential-bearing source references | P1 | S | — | TODO | | [004](004-serialize-catalog-updates.md) | Serialize catalog updates | P1 | M | — | TODO |