fix(cli): harden wizard package normalization

Preserve requested npm tags when the wizard strips source prefixes. Validate package names before probing node_modules so traversal inputs cannot escape the package directory.

Author: harlan-zw

src/cache/internal/version.ts

@@ -11,6 +11,14 @@ const VALID_PKG_NAME = /^(?:@[a-z0-9][-a-z0-9._]*\/)?[a-z0-9][-a-z0-9._]*$/
 /** Validate version string (semver-ish, no path separators) */
 const VALID_VERSION = /^[a-z0-9][-\w.+]*$/i
 
+export function isValidCachePackageName(name: string): boolean {
+  return VALID_PKG_NAME.test(name)
+}
+
+export function isValidCacheVersion(version: string): boolean {
+  return VALID_VERSION.test(version)
+}
+
 /**
  * Get exact version key for cache keying
  */
@@ -30,9 +38,9 @@ export function getCacheKey(name: string, version: string): string {
  * Validates name/version to prevent path traversal.
  */
 export function getCacheDir(name: string, version: string): string {
-  if (!VALID_PKG_NAME.test(name))
+  if (!isValidCachePackageName(name))
     throw new Error(`Invalid package name: ${name}`)
-  if (!VALID_VERSION.test(version))
+  if (!isValidCacheVersion(version))
     throw new Error(`Invalid version: ${version}`)
 
   const dir = resolve(REFERENCES_DIR, getCacheKey(name, version))

src/cli.ts

@@ -21,7 +21,7 @@ import { runWizard } from './commands/wizard.ts'
 import { timedSpinner } from './core/formatting.ts'
 import { getProjectState, hasCompletedWizard, isOutdated, readConfig, semverGt } from './core/index.ts'
 import { readPackageJsonSafe } from './core/package-json.ts'
-import { parseSkillInput } from './core/prefix.ts'
+import { parseNpmPackageInputs } from './core/prefix.ts'
 import { COMMA_OR_WHITESPACE_RE, VERSION_RANGE_PREFIX_RE } from './core/regex.ts'
 import { iterateSkills } from './core/skills.ts'
 import { fetchLatestVersion, fetchNpmRegistryMeta } from './sources/index.ts'
@@ -60,19 +60,6 @@ function deprecatedForwarder(
 
 // ── Subcommands (lazy-loaded) ──
 
-function toPackageNames(tokens: string[]): string[] | null {
-  const names: string[] = []
-  for (const token of tokens) {
-    const source = parseSkillInput(token)
-    if (source.type !== 'npm' && source.type !== 'bare') {
-      p.log.error(`${token} is not an npm package. Install it with \`skilld add ${token}\`.`)
-      return null
-    }
-    names.push(source.package)
-  }
-  return names
-}
-
 const SUBCOMMAND_NAMES = ['add', 'eject', 'update', 'info', 'list', 'config', 'remove', 'install', 'uninstall', 'search', 'cache', 'validate', 'assemble', 'setup', 'prepare', 'author', 'publish', 'upload', 'login', 'logout', 'whoami', 'pull']
 
 // ── Main command ──
@@ -304,10 +291,12 @@ const main = defineCommand({
             p.log.warn('No packages entered')
             continue
           }
-          const names = toPackageNames(input.split(COMMA_OR_WHITESPACE_RE).map(s => s.trim()).filter(Boolean))
-          if (!names)
+          const parsed = parseNpmPackageInputs(input.split(COMMA_OR_WHITESPACE_RE).map(s => s.trim()).filter(Boolean))
+          if (parsed._tag === 'Err') {
+            p.log.error(`${parsed.input} is not an npm package. Install it with \`skilld add ${parsed.input}\`.`)
             continue
-          selected = names
+          }
+          selected = parsed.packageSpecs
           if (selected.length === 0) {
             p.log.warn('No valid packages entered')
             continue
@@ -555,10 +544,12 @@ const main = defineCommand({
               }))
               if (!input)
                 return
-              const names = toPackageNames(input.split(COMMA_OR_WHITESPACE_RE).map(s => s.trim()).filter(Boolean))
-              if (!names)
+              const parsed = parseNpmPackageInputs(input.split(COMMA_OR_WHITESPACE_RE).map(s => s.trim()).filter(Boolean))
+              if (parsed._tag === 'Err') {
+                p.log.error(`${parsed.input} is not an npm package. Install it with \`skilld add ${parsed.input}\`.`)
                 return
-              selected = names
+              }
+              selected = parsed.packageSpecs
               if (selected.length === 0)
                 return
             }

src/core/prefix.ts

@@ -16,6 +16,7 @@ import type { GitSkillSource } from '../sources/git-skills.ts'
 import { parseGitSkillInput } from '../sources/git-skills.ts'
 
 const STATIC_REGEX_1 = /^[\w.-]+\/[\w.-]+/
+const EXPLICIT_NON_NPM_PREFIX_RE = /^(?:crate|gh|github):/
 
 export type SkillSource
   = | { type: 'npm', package: string, tag?: string }
@@ -25,6 +26,25 @@ export type SkillSource
     | { type: 'collection', handle: string, name: string }
     | { type: 'bare', package: string, tag?: string }
 
+export type NpmPackageInputResult
+  = | { _tag: 'Ok', packageSpecs: string[] }
+    | { _tag: 'Err', input: string }
+
+export function parseNpmPackageInputs(inputs: string[]): NpmPackageInputResult {
+  const packageSpecs: string[] = []
+
+  for (const input of inputs) {
+    const source = parseSkillInput(input)
+    const isMalformedExplicitSource = source.type === 'bare' && EXPLICIT_NON_NPM_PREFIX_RE.test(input)
+    if ((source.type !== 'npm' && source.type !== 'bare') || isMalformedExplicitSource || !source.package)
+      return { _tag: 'Err', input }
+
+    packageSpecs.push(source.tag ? `${source.package}@${source.tag}` : source.package)
+  }
+
+  return { _tag: 'Ok', packageSpecs }
+}
+
 /**
  * Parse a single CLI input token into a typed SkillSource.
  *

src/core/prepare.ts

@@ -9,7 +9,7 @@
 import type { SkillInfo } from './lockfile.ts'
 import { existsSync, lstatSync, mkdirSync, readdirSync, rmSync, symlinkSync, unlinkSync } from 'node:fs'
 import { basename, join } from 'pathe'
-import { getCacheDir } from '../cache/internal/version.ts'
+import { getCacheDir, isValidCachePackageName, isValidCacheVersion } from '../cache/internal/version.ts'
 import { readPackageJsonSafe } from './package-json.ts'
 import { README_FILENAME_RE } from './regex.ts'
 
@@ -24,21 +24,17 @@ function toStorageName(name: string): string {
 
 /** Resolve package directory: node_modules first, then global cache */
 export function resolvePkgDir(name: string, cwd: string, version?: string): string | null {
-  if (!name)
+  if (!isValidCachePackageName(name))
     return null
 
   const nodeModulesPath = join(cwd, 'node_modules', name)
   if (existsSync(nodeModulesPath))
     return nodeModulesPath
 
   if (version) {
-    let cachedPkgDir: string
-    try {
-      cachedPkgDir = join(getCacheDir(name, version), 'pkg')
-    }
-    catch {
+    if (!isValidCacheVersion(version))
       return null
-    }
+    const cachedPkgDir = join(getCacheDir(name, version), 'pkg')
     if (existsSync(join(cachedPkgDir, 'package.json')))
       return cachedPkgDir
   }

test/unit/pkg-dir-probe.test.ts

@@ -1,11 +1,39 @@
-import { describe, expect, it } from 'vitest'
+import { mkdirSync, mkdtempSync, rmSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'pathe'
+import { afterEach, describe, expect, it } from 'vitest'
 import { getShippedSkills, resolvePkgDir } from '../../src/core/prepare.ts'
 
 describe('package dir probing', () => {
-  it.each(['npm:vue', 'gh:owner/repo', '../escape', ''])('returns null for %j', (name) => {
+  const fixtureDirs: string[] = []
+
+  afterEach(() => {
+    for (const dir of fixtureDirs)
+      rmSync(dir, { recursive: true, force: true })
+    fixtureDirs.length = 0
+  })
+
+  it.each(['npm:vue', 'gh:owner/repo', ''])('returns null for %j', (name) => {
     expect(resolvePkgDir(name, process.cwd(), '1.0.0')).toBeNull()
   })
 
+  it('rejects traversal when the escaped directory exists', () => {
+    const cwd = mkdtempSync(join(tmpdir(), 'skilld-pkg-probe-'))
+    fixtureDirs.push(cwd)
+    mkdirSync(join(cwd, 'escape'))
+
+    expect(resolvePkgDir('../escape', cwd, '1.0.0')).toBeNull()
+  })
+
+  it('returns an installed package before validating the cache version', () => {
+    const cwd = mkdtempSync(join(tmpdir(), 'skilld-pkg-probe-'))
+    fixtureDirs.push(cwd)
+    const packageDir = join(cwd, 'node_modules', 'vue')
+    mkdirSync(packageDir, { recursive: true })
+
+    expect(resolvePkgDir('vue', cwd, '../invalid')).toBe(packageDir)
+  })
+
   it.each(['npm:vue', '../escape'])('reports no shipped skills for %j', (name) => {
     expect(getShippedSkills(name, process.cwd(), '1.0.0')).toEqual([])
   })

test/unit/prefix.test.ts

@@ -1,7 +1,20 @@
 import { describe, expect, it } from 'vitest'
-import { parseSkillInput, resolveSkillName } from '../../src/core/prefix'
+import { parseNpmPackageInputs, parseSkillInput, resolveSkillName } from '../../src/core/prefix'
 
 describe('prefix parser', () => {
+  describe('wizard npm inputs', () => {
+    it('normalizes prefixes without dropping npm tags', () => {
+      expect(parseNpmPackageInputs(['npm:vue@beta', '@nuxt/ui@3.0.0', 'pinia'])).toEqual({
+        _tag: 'Ok',
+        packageSpecs: ['vue@beta', '@nuxt/ui@3.0.0', 'pinia'],
+      })
+    })
+
+    it.each(['gh:owner/repo', 'gh:not-a-repo', 'crate:serde', '@curator'])('rejects non-npm input %s', (input) => {
+      expect(parseNpmPackageInputs([input])).toEqual({ _tag: 'Err', input })
+    })
+  })
+
   describe('npm: prefix', () => {
     it('parses simple package name', () => {
       expect(parseSkillInput('npm:vue')).toEqual({

test/unit/prepare-restore.test.ts

@@ -12,9 +12,13 @@ vi.mock('node:fs', async () => {
   }
 })
 
-vi.mock('../../src/cache/internal/version', () => ({
-  getCacheDir: (name: string, version: string) => `/home/.skilld/references/${name}@${version}`,
-}))
+vi.mock('../../src/cache/internal/version', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('../../src/cache/internal/version')>()
+  return {
+    ...actual,
+    getCacheDir: (name: string, version: string) => `/home/.skilld/references/${name}@${version}`,
+  }
+})
 
 describe('restorePkgSymlink', () => {
   beforeEach(() => vi.resetAllMocks())