Validate skill listing prices strictly

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);
+    },
+  );
+});

packages/cli/src/commands/skills.ts

@@ -111,6 +111,20 @@ async function saveManifest(path: string, manifest: SkillManifest): Promise<void
   await writeFile(path, `${JSON.stringify(manifest, null, 2)}\n`, 'utf8');
 }
 
+function parsePriceSats(raw: string): number {
+  const value = raw.trim();
+  if (!/^\d+$/.test(value)) {
+    throw new Error(`--price must be a non-negative integer in sats. Got: ${JSON.stringify(raw)}`);
+  }
+
+  const price = Number(value);
+  if (!Number.isSafeInteger(price)) {
+    throw new Error(`--price must be a safe non-negative integer in sats. Got: ${JSON.stringify(raw)}`);
+  }
+
+  return price;
+}
+
 function normalizeText(text: string): string {
   const normalized = text.replace(/\r\n/g, '\n').replace(/\r/g, '\n');
   return normalized.endsWith('\n') ? normalized : `${normalized}\n`;
@@ -357,10 +371,11 @@ skillsCmd
   .option('--price <sats>', 'Price in sats; 0 = free', '0')
   .option('--source-url <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);

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');
+  });
 });

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<Config>({
   id: 'pkg-flatpak',
   kind: 'package-manager',

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');
+  });
 });

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<Config>({
   id: 'pkg-snap',
   kind: 'package-manager',

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');
+  });
 });

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<Config>({
   id: 'pkg-winget',
   kind: 'package-manager',