diff --git a/packages/cli/src/commands/skills.test.ts b/packages/cli/src/commands/skills.test.ts index 0f23e4ca..83847e7a 100644 --- a/packages/cli/src/commands/skills.test.ts +++ b/packages/cli/src/commands/skills.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; import { @@ -151,3 +151,77 @@ describe('skills marketplaces --json', () => { } }); }); + +describe('skills new command', () => { + let tempDir: string; + let stdout: string[]; + let stderr: string[]; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), 'sh1pt-skills-new-')); + stdout = []; + stderr = []; + vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + stdout.push(args.map(String).join(' ')); + }); + vi.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { + stderr.push(args.map(String).join(' ')); + }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + rmSync(tempDir, { recursive: true, force: true }); + }); + + function writeSkillFile() { + const skillDir = join(tempDir, 'skill'); + mkdirSync(skillDir, { recursive: true }); + const skillFile = join(skillDir, 'SKILL.md'); + writeFileSync(skillFile, [ + '---', + 'name: invoice-helper', + 'description: Helps prepare invoices', + '---', + '', + '# Invoice Helper', + '', + ].join('\n')); + return skillFile; + } + + async function runNew(price: string) { + const newCmd = skillsCmd.commands.find((c) => c.name() === 'new')!; + const skillFile = writeSkillFile(); + const out = join(tempDir, 'sh1pt.skill.json'); + await newCmd.parseAsync([ + '--skill-file', skillFile, + '--out', out, + '--price', price, + ], { from: 'user' }); + return out; + } + + it('writes valid integer prices to the manifest and marketplace command', async () => { + const out = await runNew('100'); + const manifest = JSON.parse(readFileSync(out, 'utf8')); + + expect(manifest.price).toBe(100); + expect(manifest.marketplaces.ugig.command).toContain('--price 100'); + expect(stdout.join('\n')).toContain('wrote'); + }); + + it.each(['-5', '1.9', '5abc', '1e2', '0x10', '+5', `${Number.MAX_SAFE_INTEGER + 1}`])( + 'rejects invalid listing price %s before writing a manifest', + async (price) => { + const exit = vi.spyOn(process, 'exit').mockImplementation(((code?: string | number | null) => { + throw new Error(`process.exit(${code})`); + }) as never); + + await expect(runNew(price)).rejects.toThrow('process.exit(1)'); + expect(stderr.join('\n')).toContain('--price'); + expect(existsSync(join(tempDir, 'sh1pt.skill.json'))).toBe(false); + expect(exit).toHaveBeenCalledWith(1); + }, + ); +}); diff --git a/packages/cli/src/commands/skills.ts b/packages/cli/src/commands/skills.ts index f7b463d5..b4203b2a 100644 --- a/packages/cli/src/commands/skills.ts +++ b/packages/cli/src/commands/skills.ts @@ -111,6 +111,20 @@ async function saveManifest(path: string, manifest: SkillManifest): Promise', 'Price in sats; 0 = free', '0') .option('--source-url ', 'Public raw SKILL.md or repo URL') .action(async (opts: { skillFile: string; out: string; name?: string; title?: string; description?: string; tagline?: string; category: string; tags: string; price: string; sourceUrl?: string }) => { - // Validate price before writing any files. - const parsedPrice = Number.parseInt(opts.price, 10); - if (!Number.isInteger(parsedPrice) || parsedPrice < 0 || opts.price.includes('.') || Number.isNaN(parsedPrice)) { - console.error(kleur.red(`--price must be a non-negative integer (sats). Got: ${JSON.stringify(opts.price)}`)); + let price: number; + try { + price = parsePriceSats(opts.price); + } catch (error) { + console.error(kleur.red(error instanceof Error ? error.message : String(error))); process.exit(1); } @@ -369,17 +384,18 @@ skillsCmd const name = slugify(opts.name ?? inferred.name ?? basename(dirname(skillFile))); const title = opts.title ?? inferred.title ?? name; const description = opts.description ?? inferred.description ?? `Agent skill: ${title}`; + const tags = opts.tags.split(',').map(t => t.trim()).filter(Boolean).slice(0, 10); const manifest: SkillManifest = { name, title, description, tagline: opts.tagline, category: opts.category, - tags: opts.tags.split(',').map(t => t.trim()).filter(Boolean).slice(0, 10), - price: parsedPrice, + tags, + price, skillFile, sourceUrl: opts.sourceUrl, - marketplaces: Object.fromEntries(MARKETPLACES.map(mp => [mp.id, { enabled: true, status: 'pending', command: 'command' in mp && mp.command ? mp.command({ name, title, description, tagline: opts.tagline, category: opts.category, tags: opts.tags.split(',').map(t => t.trim()).filter(Boolean).slice(0, 10), price: parsedPrice, skillFile, sourceUrl: opts.sourceUrl, marketplaces: {} }) : undefined, note: 'note' in mp ? mp.note : undefined }])) as SkillManifest['marketplaces'], + marketplaces: Object.fromEntries(MARKETPLACES.map(mp => [mp.id, { enabled: true, status: 'pending', command: 'command' in mp && mp.command ? mp.command({ name, title, description, tagline: opts.tagline, category: opts.category, tags, price, skillFile, sourceUrl: opts.sourceUrl, marketplaces: {} }) : undefined, note: 'note' in mp ? mp.note : undefined }])) as SkillManifest['marketplaces'], }; await mkdir(dirname(resolve(opts.out)), { recursive: true }); await saveManifest(opts.out, manifest); diff --git a/packages/targets/pkg-flatpak/src/index.test.ts b/packages/targets/pkg-flatpak/src/index.test.ts index 79d7e324..594339e6 100644 --- a/packages/targets/pkg-flatpak/src/index.test.ts +++ b/packages/targets/pkg-flatpak/src/index.test.ts @@ -62,4 +62,28 @@ describe('Flatpak manifest generation', () => { appId: 'com.example.MyApp', })).resolves.toEqual({ id: 'dry-run' }); }); + + it('rejects invalid app IDs before manifest generation', async () => { + const outDir = await mkdtemp(join(tmpdir(), 'sh1pt-flatpak-')); + tempDirs.push(outDir); + const ctx = fakeBuildContext({ + outDir, + projectDir: '/repo/myapp', + version: '1.2.3', + channel: 'stable', + }) as any; + + for (const appId of ['../escape', 'com.example', 'com.example.App-', 'com.123.App']) { + await expect(adapter.build(ctx, { appId })).rejects.toThrow('appId'); + } + }); + + it('rejects invalid app IDs before shipping', async () => { + await expect(adapter.ship(fakeShipContext({ + version: '1.2.3', + dryRun: true, + }) as any, { + appId: '../escape', + })).rejects.toThrow('appId'); + }); }); diff --git a/packages/targets/pkg-flatpak/src/index.ts b/packages/targets/pkg-flatpak/src/index.ts index 97a928ba..4b1b26ab 100644 --- a/packages/targets/pkg-flatpak/src/index.ts +++ b/packages/targets/pkg-flatpak/src/index.ts @@ -30,7 +30,7 @@ function renderList(values: string[], indent: string): string[] { /** * Validate a Flatpak application ID. * Must be a reverse-DNS string with at least 3 dot-separated segments, - * each non-empty and containing only alphanumeric characters or hyphens/underscores. + * each starting with a letter and containing alphanumeric characters, hyphens, or underscores. * Must not contain path traversal characters. */ function validateAppId(appId: string): void { @@ -49,7 +49,7 @@ function validateAppId(appId: string): void { if (!seg) { throw new Error(`pkg-flatpak: invalid appId "${appId}" — segments must be non-empty`); } - if (!/^[A-Za-z0-9_-]+$/.test(seg)) { + if (!/^[A-Za-z][A-Za-z0-9_]*(?:-[A-Za-z0-9_]+)*$/.test(seg)) { throw new Error(`pkg-flatpak: invalid appId "${appId}" — segment "${seg}" contains invalid characters`); } } @@ -107,26 +107,6 @@ function renderFlatpakManifest(ctx: { projectDir: string; version: string; chann return lines.join('\n'); } -/** - * Validate a Flatpak app ID (reverse-DNS format): at least 3 dot-separated segments, - * each containing alphanumeric or hyphens. e.g. "com.example.MyApp" - */ -function validateAppId(appId: string): void { - if (!appId) throw new Error('pkg-flatpak: appId is required'); - const segments = appId.split('.'); - if (segments.length < 3) { - throw new Error(`pkg-flatpak: invalid appId "${appId}". Must have at least 3 reverse-DNS segments (e.g. "com.example.MyApp").`); - } - if (appId.includes('..') || appId.includes('/') || appId.includes('\\')) { - throw new Error(`pkg-flatpak: appId "${appId}" contains path traversal characters.`); - } - for (const seg of segments) { - if (!seg || !/^[A-Za-z0-9_-]+$/.test(seg)) { - throw new Error(`pkg-flatpak: invalid segment "${seg}" in appId "${appId}".`); - } - } -} - export default defineTarget({ id: 'pkg-flatpak', kind: 'package-manager', diff --git a/packages/targets/pkg-snap/src/index.test.ts b/packages/targets/pkg-snap/src/index.test.ts index 9c1c4c52..b05d54cc 100644 --- a/packages/targets/pkg-snap/src/index.test.ts +++ b/packages/targets/pkg-snap/src/index.test.ts @@ -1,5 +1,5 @@ import { fakeBuildContext, fakeShipContext, smokeTest } from '@profullstack/sh1pt-core/testing'; -import { mkdtemp, readFile, rm } from 'node:fs/promises'; +import { mkdir, mkdtemp, readFile, readdir, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; @@ -60,4 +60,46 @@ describe('snapcraft manifest generation', () => { snapName: 'myapp', })).resolves.toEqual({ id: 'dry-run' }); }); + + it('rejects invalid snap names before generating manifests', async () => { + const outDir = await mkdtemp(join(tmpdir(), 'sh1pt-snap-')); + tempDirs.push(outDir); + + const ctx = fakeBuildContext({ + outDir, + projectDir: '/repo/myapp', + version: '1.2.3', + channel: 'stable', + }) as any; + + for (const snapName of ['Bad: Name', '-myapp', 'myapp-', 'my--app', '1234', 'a'.repeat(41)]) { + await expect(adapter.build(ctx, { snapName })).rejects.toThrow('snapName'); + } + }); + + it('rejects invalid snap names before touching the output directory', async () => { + const outDir = await mkdtemp(join(tmpdir(), 'sh1pt-snap-')); + tempDirs.push(outDir); + await mkdir(outDir, { recursive: true }); + + await expect(adapter.build(fakeBuildContext({ + outDir, + projectDir: '/repo/myapp', + version: '1.2.3', + channel: 'stable', + }) as any, { + snapName: 'my--app', + })).rejects.toThrow('snapName'); + + await expect(readdir(outDir)).resolves.toEqual([]); + }); + + it('rejects invalid snap names before shipping', async () => { + await expect(adapter.ship(fakeShipContext({ + version: '1.2.3', + dryRun: true, + }) as any, { + snapName: 'Bad: Name', + })).rejects.toThrow('snapName'); + }); }); diff --git a/packages/targets/pkg-snap/src/index.ts b/packages/targets/pkg-snap/src/index.ts index 5504dfb8..c294756b 100644 --- a/packages/targets/pkg-snap/src/index.ts +++ b/packages/targets/pkg-snap/src/index.ts @@ -48,6 +48,9 @@ function validateSnapName(snapName: string): void { if (snapName.startsWith('-') || snapName.endsWith('-')) { throw new Error(`pkg-snap: snapName "${snapName}" must not start or end with a hyphen`); } + if (snapName.includes('--')) { + throw new Error(`pkg-snap: snapName "${snapName}" must not contain consecutive hyphens`); + } if (!/^[a-z0-9-]+$/.test(snapName)) { throw new Error(`pkg-snap: snapName "${snapName}" must contain only lowercase letters, digits, and hyphens`); } @@ -57,6 +60,7 @@ function validateSnapName(snapName: string): void { } function renderSnapcraftYaml(ctx: { projectDir: string; version: string; channel: string }, config: Config): string { + validateSnapName(config.snapName); const grade = config.grade ?? (ctx.channel === 'stable' ? 'stable' : 'devel'); const confinement = config.confinement ?? 'strict'; const base = config.base ?? 'core22'; @@ -102,15 +106,6 @@ function renderSnapcraftYaml(ctx: { projectDir: string; version: string; channel return lines.join('\n'); } -/** Validate a snap package name: lowercase, alphanumeric, hyphens only (no leading/trailing hyphen). */ -function validateSnapName(name: string): void { - if (!name || !/^[a-z0-9]([a-z0-9-]*[a-z0-9])?$/.test(name)) { - throw new Error( - `pkg-snap: invalid snapName "${name}". Snap names must be lowercase alphanumeric with optional hyphens (no leading/trailing hyphen, no uppercase, no underscore).`, - ); - } -} - export default defineTarget({ id: 'pkg-snap', kind: 'package-manager', diff --git a/packages/targets/pkg-winget/src/index.test.ts b/packages/targets/pkg-winget/src/index.test.ts index 5f3292b2..d534e429 100644 --- a/packages/targets/pkg-winget/src/index.test.ts +++ b/packages/targets/pkg-winget/src/index.test.ts @@ -82,4 +82,40 @@ describe('winget manifest generation', () => { ], })).resolves.toEqual({ id: 'dry-run' }); }); + + it('rejects invalid package IDs before manifest generation', async () => { + const outDir = await mkdtemp(join(tmpdir(), 'sh1pt-winget-')); + tempDirs.push(outDir); + const ctx = fakeBuildContext({ + outDir, + version: '1.2.3', + }) as any; + const installers = [ + { + architecture: 'x64' as const, + url: 'https://downloads.example.com/my-tool-1.2.3-x64.exe', + sha256: 'c'.repeat(64), + }, + ]; + + for (const packageId of ['NoDot', '.Acme.MyTool', 'Acme..MyTool', 'Acme/MyTool']) { + await expect(adapter.build(ctx, { packageId, installers })).rejects.toThrow('packageId'); + } + }); + + it('rejects invalid package IDs before shipping', async () => { + await expect(adapter.ship(fakeShipContext({ + version: '1.2.3', + dryRun: true, + }) as any, { + packageId: 'Acme/MyTool', + installers: [ + { + architecture: 'x64', + url: 'https://downloads.example.com/my-tool-1.2.3-x64.exe', + sha256: 'c'.repeat(64), + }, + ], + })).rejects.toThrow('packageId'); + }); }); diff --git a/packages/targets/pkg-winget/src/index.ts b/packages/targets/pkg-winget/src/index.ts index 2893cc29..9c4d16b3 100644 --- a/packages/targets/pkg-winget/src/index.ts +++ b/packages/targets/pkg-winget/src/index.ts @@ -124,28 +124,6 @@ function renderLocaleManifest(config: Config, version: string): string { return lines.join('\n'); } -/** - * Validate a winget package identifier: must contain at least one dot, - * each segment must be non-empty alphanumeric+hyphens/underscores/dots. - * e.g. "Microsoft.WindowsTerminal" - */ -function validatePackageId(packageId: string): void { - if (!packageId) throw new Error('pkg-winget: packageId is required'); - const segments = packageId.split('.'); - if (segments.length < 2) { - throw new Error(`pkg-winget: invalid packageId "${packageId}". Must be "Publisher.AppName" format with at least one dot.`); - } - for (const seg of segments) { - if (!seg) throw new Error(`pkg-winget: empty segment in packageId "${packageId}".`); - if (!/^[A-Za-z0-9_\-]+$/.test(seg)) { - throw new Error(`pkg-winget: invalid segment "${seg}" in packageId "${packageId}". Segments must be alphanumeric with hyphens or underscores.`); - } - } - if (packageId.includes('..') || packageId.includes('/') || packageId.includes('\\')) { - throw new Error(`pkg-winget: packageId "${packageId}" contains path traversal characters.`); - } -} - export default defineTarget({ id: 'pkg-winget', kind: 'package-manager',