feat(mosaic): add secure skill registration CLI (#826)

packages/mosaic/src/commands/skill.spec.ts

@@ -0,0 +1,421 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { Command } from 'commander';
+import { spawnSync } from 'node:child_process';
+import { fileURLToPath } from 'node:url';
+import {
+  existsSync,
+  lstatSync,
+  mkdirSync,
+  mkdtempSync,
+  readlinkSync,
+  rmSync,
+  symlinkSync,
+  writeFileSync,
+} from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import {
+  listSkills,
+  registerSkill,
+  registerSkillCommand,
+  syncClaudeSkills,
+  unregisterSkill,
+  type SkillPaths,
+} from './skill.js';
+
+const LEGACY_SYNC_SCRIPT = fileURLToPath(
+  new URL('../../framework/tools/_scripts/mosaic-sync-skills', import.meta.url),
+);
+
+describe('Claude skill bridge', () => {
+  let root: string;
+  let paths: SkillPaths;
+
+  beforeEach(() => {
+    root = mkdtempSync(join(tmpdir(), 'mosaic-skill-cli-'));
+    paths = {
+      mosaicSkillsDir: join(root, '.config', 'mosaic', 'skills'),
+      claudeSkillsDir: join(root, '.claude', 'skills'),
+    };
+    mkdirSync(paths.mosaicSkillsDir, { recursive: true });
+  });
+
+  afterEach(() => {
+    rmSync(root, { recursive: true, force: true });
+  });
+
+  function createSkill(name: string): string {
+    const skillDir = join(paths.mosaicSkillsDir, name);
+    mkdirSync(skillDir, { recursive: true });
+    writeFileSync(join(skillDir, 'SKILL.md'), `# ${name}\n`);
+    return skillDir;
+  }
+
+  function expectCorrectLink(name: string): void {
+    const linkPath = join(paths.claudeSkillsDir, name);
+    expect(lstatSync(linkPath).isSymbolicLink()).toBe(true);
+    expect(readlinkSync(linkPath)).toBe(join(paths.mosaicSkillsDir, name));
+  }
+
+  describe('name validation', () => {
+    const invalidNames = [
+      '../../etc',
+      '/abs/path',
+      'a/b',
+      String.raw`a\b`,
+      '-rf',
+      '..',
+      'safe.',
+      'space name',
+      'line\nbreak',
+      'escape\u001B[31m',
+    ];
+
+    for (const name of invalidNames) {
+      it(`rejects ${JSON.stringify(name)} before register can escape its roots`, () => {
+        expect(() => registerSkill(name, paths)).toThrow(/invalid skill name/i);
+        expect(existsSync(paths.claudeSkillsDir)).toBe(false);
+      });
+
+      it(`rejects ${JSON.stringify(name)} before unregister can escape its roots`, () => {
+        expect(() => unregisterSkill(name, paths)).toThrow(/invalid skill name/i);
+        expect(existsSync(paths.claudeSkillsDir)).toBe(false);
+      });
+    }
+  });
+
+  describe('CLI validation errors', () => {
+    let previousExitCode: number | string | null | undefined;
+
+    beforeEach(() => {
+      previousExitCode = process.exitCode;
+      process.exitCode = undefined;
+    });
+
+    afterEach(() => {
+      process.exitCode = previousExitCode;
+    });
+
+    it.each(['register', 'unregister'])(
+      'reports invalid %s names on stderr and sets a nonzero exit status',
+      async (subcommand) => {
+        const error = vi.spyOn(console, 'error').mockImplementation(() => undefined);
+        const program = new Command().exitOverride();
+        registerSkillCommand(program, paths);
+
+        await program.parseAsync(['node', 'mosaic', 'skill', subcommand, '../../etc']);
+
+        expect(error).toHaveBeenCalledWith(expect.stringMatching(/invalid skill name/i));
+        expect(process.exitCode).toBe(1);
+        expect(existsSync(paths.claudeSkillsDir)).toBe(false);
+        error.mockRestore();
+      },
+    );
+  });
+
+  describe('CLI status output', () => {
+    let previousExitCode: number | string | null | undefined;
+
+    beforeEach(() => {
+      previousExitCode = process.exitCode;
+      process.exitCode = undefined;
+    });
+
+    afterEach(() => {
+      process.exitCode = previousExitCode;
+    });
+
+    async function run(...args: string[]): Promise<void> {
+      const program = new Command().exitOverride();
+      registerSkillCommand(program, paths);
+      await program.parseAsync(['node', 'mosaic', 'skill', ...args]);
+    }
+
+    it('reports register repair/idempotency and unregister idempotency statuses', async () => {
+      const log = vi.spyOn(console, 'log').mockImplementation(() => undefined);
+      createSkill('status-skill');
+
+      await run('register', 'status-skill');
+      await run('register', 'status-skill');
+      rmSync(join(paths.claudeSkillsDir, 'status-skill'));
+      symlinkSync(
+        join(paths.mosaicSkillsDir, 'retired'),
+        join(paths.claudeSkillsDir, 'status-skill'),
+      );
+      await run('register', 'status-skill');
+      await run('unregister', 'status-skill');
+      await run('unregister', 'status-skill');
+
+      expect(log.mock.calls.flat()).toEqual([
+        'status-skill: registered',
+        'status-skill: already registered',
+        'status-skill: repaired dangling registration',
+        'status-skill: unregistered',
+        'status-skill: already unregistered',
+      ]);
+      log.mockRestore();
+    });
+
+    it('reports empty and populated skill lists', async () => {
+      const log = vi.spyOn(console, 'log').mockImplementation(() => undefined);
+
+      await run('list');
+      createSkill('listed');
+      await run('list');
+
+      expect(log).toHaveBeenCalledWith('No Mosaic or Claude Code skills found.');
+      expect(log).toHaveBeenCalledWith(expect.stringMatching(/^unregistered\s+listed$/));
+      log.mockRestore();
+    });
+  });
+
+  describe('registerSkill', () => {
+    it('creates the exact canonical symlink and is idempotent', () => {
+      createSkill('new-skill');
+
+      expect(registerSkill('new-skill', paths).status).toBe('registered');
+      expectCorrectLink('new-skill');
+
+      expect(registerSkill('new-skill', paths).status).toBe('already-registered');
+      expectCorrectLink('new-skill');
+    });
+
+    it('repairs a dangling Mosaic-owned symlink', () => {
+      createSkill('new-skill');
+      mkdirSync(paths.claudeSkillsDir, { recursive: true });
+      symlinkSync(
+        join(paths.mosaicSkillsDir, 'retired-skill'),
+        join(paths.claudeSkillsDir, 'new-skill'),
+      );
+
+      expect(registerSkill('new-skill', paths).status).toBe('repaired');
+      expectCorrectLink('new-skill');
+    });
+
+    it.each(['file', 'directory', 'symlink'] as const)(
+      'refuses to clobber a foreign %s at the target',
+      (kind) => {
+        createSkill('protected');
+        mkdirSync(paths.claudeSkillsDir, { recursive: true });
+        const target = join(paths.claudeSkillsDir, 'protected');
+        const foreign = join(root, 'foreign');
+
+        if (kind === 'file') writeFileSync(target, 'keep me\n');
+        if (kind === 'directory') mkdirSync(target);
+        if (kind === 'symlink') {
+          writeFileSync(foreign, 'keep me\n');
+          symlinkSync(foreign, target);
+        }
+
+        expect(() => registerSkill('protected', paths)).toThrow(/foreign|refus/i);
+        if (kind === 'file') expect(lstatSync(target).isFile()).toBe(true);
+        if (kind === 'directory') expect(lstatSync(target).isDirectory()).toBe(true);
+        if (kind === 'symlink') expect(readlinkSync(target)).toBe(foreign);
+      },
+    );
+
+    it('refuses a symlinked Claude skills ancestor instead of writing outside the bridge root', () => {
+      createSkill('protected');
+      const externalClaude = join(root, 'external-claude');
+      mkdirSync(externalClaude);
+      symlinkSync(externalClaude, join(root, '.claude'));
+
+      expect(() => registerSkill('protected', paths)).toThrow(
+        /symlink.*ancestor|ancestor.*symlink/i,
+      );
+      expect(existsSync(join(externalClaude, 'skills', 'protected'))).toBe(false);
+    });
+
+    it('refuses a symlinked canonical skills root instead of registering an external source', () => {
+      rmSync(paths.mosaicSkillsDir, { recursive: true });
+      const externalSkills = join(root, 'external-skills');
+      mkdirSync(join(externalSkills, 'protected'), { recursive: true });
+      symlinkSync(externalSkills, paths.mosaicSkillsDir);
+
+      expect(() => registerSkill('protected', paths)).toThrow(
+        /symlink.*ancestor|ancestor.*symlink/i,
+      );
+      expect(existsSync(paths.claudeSkillsDir)).toBe(false);
+    });
+
+    it('refuses a dangling foreign symlink rather than treating it as repairable', () => {
+      createSkill('protected');
+      mkdirSync(paths.claudeSkillsDir, { recursive: true });
+      const foreignMissing = join(root, 'foreign-missing');
+      const target = join(paths.claudeSkillsDir, 'protected');
+      symlinkSync(foreignMissing, target);
+
+      expect(() => registerSkill('protected', paths)).toThrow(/foreign|refus/i);
+      expect(readlinkSync(target)).toBe(foreignMissing);
+    });
+  });
+
+  describe('unregisterSkill', () => {
+    it('removes a Mosaic-owned symlink and is idempotent when absent', () => {
+      createSkill('removable');
+      registerSkill('removable', paths);
+
+      expect(unregisterSkill('removable', paths).status).toBe('unregistered');
+      expect(existsSync(join(paths.claudeSkillsDir, 'removable'))).toBe(false);
+
+      expect(unregisterSkill('removable', paths).status).toBe('already-unregistered');
+    });
+
+    it('refuses to remove a misdirected Mosaic-root symlink', () => {
+      createSkill('other');
+      mkdirSync(paths.claudeSkillsDir, { recursive: true });
+      const requested = join(paths.claudeSkillsDir, 'requested');
+      symlinkSync(join(paths.mosaicSkillsDir, 'other'), requested);
+
+      expect(() => unregisterSkill('requested', paths)).toThrow(/misdirected/i);
+      expect(readlinkSync(requested)).toBe(join(paths.mosaicSkillsDir, 'other'));
+    });
+
+    it.each(['file', 'directory', 'symlink'] as const)('refuses to remove a foreign %s', (kind) => {
+      mkdirSync(paths.claudeSkillsDir, { recursive: true });
+      const target = join(paths.claudeSkillsDir, 'protected');
+      const foreign = join(root, 'foreign');
+
+      if (kind === 'file') writeFileSync(target, 'keep me\n');
+      if (kind === 'directory') mkdirSync(target);
+      if (kind === 'symlink') {
+        writeFileSync(foreign, 'keep me\n');
+        symlinkSync(foreign, target);
+      }
+
+      expect(() => unregisterSkill('protected', paths)).toThrow(/foreign|refus/i);
+      expect(lstatSync(target)).toBeDefined();
+      if (kind === 'symlink') expect(readlinkSync(target)).toBe(foreign);
+    });
+  });
+
+  describe('listSkills', () => {
+    it('flags registered, unregistered, Mosaic-owned dangling, and foreign entries', () => {
+      createSkill('registered');
+      createSkill('unregistered');
+      registerSkill('registered', paths);
+      symlinkSync(join(paths.mosaicSkillsDir, 'retired'), join(paths.claudeSkillsDir, 'dangling'));
+      writeFileSync(join(paths.claudeSkillsDir, 'foreign-file'), 'keep me\n');
+      symlinkSync(join(root, 'missing-foreign'), join(paths.claudeSkillsDir, 'foreign-link'));
+
+      expect(listSkills(paths)).toEqual([
+        expect.objectContaining({ name: 'dangling', status: 'dangling' }),
+        expect.objectContaining({ name: 'foreign-file', status: 'foreign' }),
+        expect.objectContaining({ name: 'foreign-link', status: 'foreign-dangling' }),
+        expect.objectContaining({ name: 'registered', status: 'registered' }),
+        expect.objectContaining({ name: 'unregistered', status: 'unregistered' }),
+      ]);
+    });
+  });
+
+  describe('install linker compatibility', () => {
+    it('preserves foreign-name links into Mosaic home but outside canonical skills', () => {
+      createSkill('missing');
+      mkdirSync(paths.claudeSkillsDir, { recursive: true });
+      const mosaicHome = join(root, '.config', 'mosaic');
+      const liveForeignTarget = join(mosaicHome, 'foreign-non-skill-target');
+      mkdirSync(liveForeignTarget);
+      const liveForeignLink = join(paths.claudeSkillsDir, 'foreign-tool');
+      const danglingForeignLink = join(paths.claudeSkillsDir, 'unresolvable-foreign');
+      symlinkSync(liveForeignTarget, liveForeignLink);
+      symlinkSync(join(mosaicHome, 'foreign-missing'), danglingForeignLink);
+
+      const result = spawnSync('bash', [LEGACY_SYNC_SCRIPT, '--link-only'], {
+        encoding: 'utf8',
+        env: { ...process.env, HOME: root, MOSAIC_HOME: mosaicHome },
+      });
+
+      expect(result.status, result.stderr).toBe(0);
+      expect(readlinkSync(liveForeignLink)).toBe(liveForeignTarget);
+      expect(readlinkSync(danglingForeignLink)).toBe(join(mosaicHome, 'foreign-missing'));
+      expect(readlinkSync(join(paths.claudeSkillsDir, 'missing'))).toBe(
+        join(paths.mosaicSkillsDir, 'missing'),
+      );
+    });
+
+    it('preserves live and dangling foreign Claude symlinks while linking missing skills', () => {
+      createSkill('dangling-foreign');
+      createSkill('live-foreign');
+      createSkill('missing');
+      mkdirSync(paths.claudeSkillsDir, { recursive: true });
+      const external = join(root, 'external');
+      mkdirSync(external);
+      const liveLink = join(paths.claudeSkillsDir, 'live-foreign');
+      const danglingLink = join(paths.claudeSkillsDir, 'dangling-foreign');
+      symlinkSync(external, liveLink);
+      symlinkSync(join(root, 'external-missing'), danglingLink);
+
+      const result = spawnSync('bash', [LEGACY_SYNC_SCRIPT, '--link-only'], {
+        encoding: 'utf8',
+        env: { ...process.env, HOME: root, MOSAIC_HOME: join(root, '.config', 'mosaic') },
+      });
+
+      expect(result.status, result.stderr).toBe(0);
+      expect(readlinkSync(liveLink)).toBe(external);
+      expect(readlinkSync(danglingLink)).toBe(join(root, 'external-missing'));
+      expect(readlinkSync(join(paths.claudeSkillsDir, 'missing'))).toBe(
+        join(paths.mosaicSkillsDir, 'missing'),
+      );
+    });
+  });
+
+  describe('syncClaudeSkills', () => {
+    it('generically creates every missing canonical link and repairs managed broken links', () => {
+      createSkill('added-after-setup');
+      createSkill('another-new-skill');
+      mkdirSync(paths.claudeSkillsDir, { recursive: true });
+      symlinkSync(
+        join(paths.mosaicSkillsDir, 'retired'),
+        join(paths.claudeSkillsDir, 'added-after-setup'),
+      );
+
+      const result = syncClaudeSkills(paths);
+
+      expect(result).toEqual({
+        registered: ['another-new-skill'],
+        repaired: ['added-after-setup'],
+        unchanged: [],
+        conflicts: [],
+      });
+      expectCorrectLink('added-after-setup');
+      expectCorrectLink('another-new-skill');
+    });
+
+    it('escapes an invalid filesystem-derived name in conflict output', () => {
+      createSkill('line\nbreak');
+
+      const result = syncClaudeSkills(paths);
+
+      expect(result.registered).toEqual([]);
+      expect(result.conflicts).toEqual([
+        expect.objectContaining({
+          name: '"line\\nbreak"',
+          reason: expect.stringMatching(/invalid/i),
+        }),
+      ]);
+      expect(existsSync(paths.claudeSkillsDir)).toBe(false);
+    });
+
+    it('continues syncing other skills without clobbering foreign entries', () => {
+      createSkill('blocked');
+      createSkill('link-me');
+      mkdirSync(paths.claudeSkillsDir, { recursive: true });
+      const blocked = join(paths.claudeSkillsDir, 'blocked');
+      writeFileSync(blocked, 'keep me\n');
+
+      const result = syncClaudeSkills(paths);
+
+      expect(result.registered).toEqual(['link-me']);
+      expect(result.conflicts).toEqual([
+        expect.objectContaining({
+          name: 'blocked',
+          reason: expect.stringMatching(/foreign|refus/i),
+        }),
+      ]);
+      expect(readlinkSync(join(paths.claudeSkillsDir, 'link-me'))).toBe(
+        join(paths.mosaicSkillsDir, 'link-me'),
+      );
+      expect(lstatSync(blocked).isFile()).toBe(true);
+    });
+  });
+});