diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bf60b45..46b0427 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,12 +31,12 @@ jobs: - name: Run type check run: npm run typecheck - - name: Run tests - run: npm test - - name: Build run: npm run build + - name: Run tests + run: npm test + - name: Test CLI run: | npm link diff --git a/package-lock.json b/package-lock.json index 9cab60f..43b3331 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "openskills", - "version": "1.0.0", + "version": "1.2.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "openskills", - "version": "1.0.0", + "version": "1.2.1", "license": "Apache-2.0", "dependencies": { "@inquirer/prompts": "^7.9.0", @@ -24,7 +24,7 @@ "vitest": "^4.0.3" }, "engines": { - "node": ">=18.0.0" + "node": ">=20.6.0" } }, "node_modules/@esbuild/aix-ppc64": { @@ -1287,6 +1287,7 @@ "integrity": "sha512-QoiaXANRkSXK6p0Duvt56W208du4P9Uye9hWLWgGMDTEoKPhuenzNcC4vGUmrNkiOKTlIrBoyNQYNpSwfEZXSg==", "devOptional": true, "license": "MIT", + "peer": true, "dependencies": { "undici-types": "~7.16.0" } @@ -1685,6 +1686,7 @@ "dev": true, "hasInstallScript": true, "license": "MIT", + "peer": true, "bin": { "esbuild": "bin/esbuild" }, @@ -2203,6 +2205,7 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -2252,6 +2255,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "nanoid": "^3.3.11", "picocolors": "^1.1.1", @@ -2780,6 +2784,7 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -2808,6 +2813,7 @@ "integrity": "sha512-ZWyE8YXEXqJrrSLvYgrRP7p62OziLW7xI5HYGWFzOvupfAlrLvURSzv/FyGyy0eidogEM3ujU+kUG1zuHgb6Ug==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.5.0", diff --git a/src/cli.ts b/src/cli.ts index 84ecd3a..4804a6f 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -54,6 +54,7 @@ program .command('sync') .description('Update AGENTS.md with installed skills (interactive, pre-selects current state)') .option('-y, --yes', 'Skip interactive selection, sync all skills') + .option('-o, --output ', 'Output file path (default: AGENTS.md)') .action(syncAgentsMd); program diff --git a/src/commands/install.ts b/src/commands/install.ts index b91e736..dc35b18 100644 --- a/src/commands/install.ts +++ b/src/commands/install.ts @@ -1,5 +1,5 @@ import { readFileSync, readdirSync, existsSync, mkdirSync, rmSync, cpSync, statSync } from 'fs'; -import { join, basename } from 'path'; +import { join, basename, resolve } from 'path'; import { homedir } from 'os'; import { execSync } from 'child_process'; import chalk from 'chalk'; @@ -11,7 +11,42 @@ import { ANTHROPIC_MARKETPLACE_SKILLS } from '../utils/marketplace-skills.js'; import type { InstallOptions } from '../types.js'; /** - * Install skill from GitHub or Git URL + * Check if source is a local path + */ +function isLocalPath(source: string): boolean { + return ( + source.startsWith('/') || + source.startsWith('./') || + source.startsWith('../') || + source.startsWith('~/') + ); +} + +/** + * Check if source is a git URL (SSH, git://, or HTTPS) + */ +function isGitUrl(source: string): boolean { + return ( + source.startsWith('git@') || + source.startsWith('git://') || + source.startsWith('http://') || + source.startsWith('https://') || + source.endsWith('.git') + ); +} + +/** + * Expand ~ to home directory + */ +function expandPath(source: string): string { + if (source.startsWith('~/')) { + return join(homedir(), source.slice(2)); + } + return resolve(source); +} + +/** + * Install skill from local path, GitHub, or Git URL */ export async function installSkill(source: string, options: InstallOptions): Promise { const folder = options.universal ? '.agent/skills' : '.claude/skills'; @@ -27,60 +62,149 @@ export async function installSkill(source: string, options: InstallOptions): Pro console.log(`Installing from: ${chalk.cyan(source)}`); console.log(`Location: ${location}\n`); - // Parse source + // Handle local path installation + if (isLocalPath(source)) { + const localPath = expandPath(source); + await installFromLocal(localPath, targetDir, options); + printPostInstallHints(isProject); + return; + } + + // Parse git source let repoUrl: string; - let skillSubpath: string; + let skillSubpath: string = ''; - if (source.startsWith('http://') || source.startsWith('https://')) { + if (isGitUrl(source)) { + // Full git URL (SSH, HTTPS, git://) repoUrl = source; - skillSubpath = ''; } else { + // GitHub shorthand: owner/repo or owner/repo/skill-path const parts = source.split('/'); if (parts.length === 2) { repoUrl = `https://github.com/${source}`; - skillSubpath = ''; } else if (parts.length > 2) { repoUrl = `https://github.com/${parts[0]}/${parts[1]}`; skillSubpath = parts.slice(2).join('/'); } else { console.error(chalk.red('Error: Invalid source format')); - console.error('Expected: owner/repo or owner/repo/skill-name'); + console.error('Expected: owner/repo, owner/repo/skill-name, git URL, or local path'); process.exit(1); } } - // Create unique temp directory per invocation + // Clone and install from git const tempDir = join(homedir(), `.openskills-temp-${Date.now()}`); mkdirSync(tempDir, { recursive: true }); try { - // Clone repository with spinner const spinner = ora('Cloning repository...').start(); - execSync(`git clone --depth 1 --quiet "${repoUrl}" "${tempDir}/repo"`, { - stdio: 'ignore', - }); - spinner.succeed('Repository cloned'); + try { + execSync(`git clone --depth 1 --quiet "${repoUrl}" "${tempDir}/repo"`, { + stdio: 'pipe', + }); + spinner.succeed('Repository cloned'); + } catch (error) { + spinner.fail('Failed to clone repository'); + const err = error as { stderr?: Buffer }; + if (err.stderr) { + console.error(chalk.dim(err.stderr.toString().trim())); + } + console.error(chalk.yellow('\nTip: For private repos, ensure git SSH keys or credentials are configured')); + process.exit(1); + } const repoDir = join(tempDir, 'repo'); if (skillSubpath) { - // Specific skill path provided - install directly - await installSpecificSkill(repoDir, skillSubpath, targetDir, isProject); + await installSpecificSkill(repoDir, skillSubpath, targetDir, isProject, options); } else { - // Find all skills in repo await installFromRepo(repoDir, targetDir, options); } } finally { - // Cleanup rmSync(tempDir, { recursive: true, force: true }); } + printPostInstallHints(isProject); +} + +/** + * Print post-install hints + */ +function printPostInstallHints(isProject: boolean): void { console.log(`\n${chalk.dim('Read skill:')} ${chalk.cyan('openskills read ')}`); if (isProject) { console.log(`${chalk.dim('Sync to AGENTS.md:')} ${chalk.cyan('openskills sync')}`); } } +/** + * Install from local path (directory containing skills or single skill) + */ +async function installFromLocal(localPath: string, targetDir: string, options: InstallOptions): Promise { + if (!existsSync(localPath)) { + console.error(chalk.red(`Error: Path does not exist: ${localPath}`)); + process.exit(1); + } + + const stats = statSync(localPath); + if (!stats.isDirectory()) { + console.error(chalk.red('Error: Path must be a directory')); + process.exit(1); + } + + // Check if this is a single skill (has SKILL.md) or a directory of skills + const skillMdPath = join(localPath, 'SKILL.md'); + if (existsSync(skillMdPath)) { + // Single skill directory + const isProject = targetDir.includes(process.cwd()); + await installSingleLocalSkill(localPath, targetDir, isProject, options); + } else { + // Directory containing multiple skills + await installFromRepo(localPath, targetDir, options); + } +} + +/** + * Install a single local skill directory + */ +async function installSingleLocalSkill( + skillDir: string, + targetDir: string, + isProject: boolean, + options: InstallOptions +): Promise { + const skillMdPath = join(skillDir, 'SKILL.md'); + const content = readFileSync(skillMdPath, 'utf-8'); + + if (!hasValidFrontmatter(content)) { + console.error(chalk.red('Error: Invalid SKILL.md (missing YAML frontmatter)')); + process.exit(1); + } + + const skillName = basename(skillDir); + const targetPath = join(targetDir, skillName); + + const shouldInstall = await warnIfConflict(skillName, targetPath, isProject, options.yes); + if (!shouldInstall) { + console.log(chalk.yellow(`Skipped: ${skillName}`)); + return; + } + + mkdirSync(targetDir, { recursive: true }); + // Security: ensure target path stays within target directory + const resolvedTargetPath = resolve(targetPath); + const resolvedTargetDir = resolve(targetDir); + if (!resolvedTargetPath.startsWith(resolvedTargetDir + '/')) { + console.error(chalk.red(`Security error: Installation path outside target directory`)); + process.exit(1); + } + + cpSync(skillDir, targetPath, { recursive: true, dereference: true }); + + console.log(chalk.green(`✅ Installed: ${skillName}`)); + console.log(` Location: ${targetPath}`); +} + /** * Install specific skill from subpath (no interaction needed) */ @@ -88,7 +212,8 @@ async function installSpecificSkill( repoDir: string, skillSubpath: string, targetDir: string, - isProject: boolean + isProject: boolean, + options: InstallOptions ): Promise { const skillDir = join(repoDir, skillSubpath); const skillMdPath = join(skillDir, 'SKILL.md'); @@ -109,14 +234,21 @@ async function installSpecificSkill( const targetPath = join(targetDir, skillName); // Warn about potential conflicts - const shouldInstall = await warnIfConflict(skillName, targetPath, isProject); + const shouldInstall = await warnIfConflict(skillName, targetPath, isProject, options.yes); if (!shouldInstall) { console.log(chalk.yellow(`Skipped: ${skillName}`)); return; } mkdirSync(targetDir, { recursive: true }); - cpSync(skillDir, targetPath, { recursive: true }); + // Security: ensure target path stays within target directory + const resolvedTargetPath = resolve(targetPath); + const resolvedTargetDir = resolve(targetDir); + if (!resolvedTargetPath.startsWith(resolvedTargetDir + '/')) { + console.error(chalk.red(`Security error: Installation path outside target directory`)); + process.exit(1); + } + cpSync(skillDir, targetPath, { recursive: true, dereference: true }); console.log(chalk.green(`✅ Installed: ${skillName}`)); console.log(` Location: ${targetPath}`); @@ -228,14 +360,21 @@ async function installFromRepo( for (const info of skillsToInstall) { // Warn about conflicts - const shouldInstall = await warnIfConflict(info.skillName, info.targetPath, isProject); + const shouldInstall = await warnIfConflict(info.skillName, info.targetPath, isProject, options.yes); if (!shouldInstall) { console.log(chalk.yellow(`Skipped: ${info.skillName}`)); continue; // Skip this skill, continue with next } mkdirSync(targetDir, { recursive: true }); - cpSync(info.skillDir, info.targetPath, { recursive: true }); + // Security: ensure target path stays within target directory + const resolvedTargetPath = resolve(info.targetPath); + const resolvedTargetDir = resolve(targetDir); + if (!resolvedTargetPath.startsWith(resolvedTargetDir + '/')) { + console.error(chalk.red(`Security error: Installation path outside target directory`)); + continue; + } + cpSync(info.skillDir, info.targetPath, { recursive: true, dereference: true }); console.log(chalk.green(`✅ Installed: ${info.skillName}`)); installedCount++; @@ -248,9 +387,14 @@ async function installFromRepo( * Warn if installing could conflict with Claude Code marketplace * Returns true if should proceed, false if should skip */ -async function warnIfConflict(skillName: string, targetPath: string, isProject: boolean): Promise { +async function warnIfConflict(skillName: string, targetPath: string, isProject: boolean, skipPrompt = false): Promise { // Check if overwriting existing skill if (existsSync(targetPath)) { + if (skipPrompt) { + // Auto-overwrite in non-interactive mode + console.log(chalk.dim(`Overwriting: ${skillName}`)); + return true; + } try { const shouldOverwrite = await confirm({ message: chalk.yellow(`Skill '${skillName}' already exists. Overwrite?`), diff --git a/src/commands/sync.ts b/src/commands/sync.ts index c72648f..0a296bb 100644 --- a/src/commands/sync.ts +++ b/src/commands/sync.ts @@ -1,4 +1,5 @@ -import { existsSync, readFileSync, writeFileSync } from 'fs'; +import { existsSync, readFileSync, writeFileSync, mkdirSync } from 'fs'; +import { dirname, basename } from 'path'; import chalk from 'chalk'; import { checkbox } from '@inquirer/prompts'; import { ExitPromptError } from '@inquirer/core'; @@ -8,15 +9,30 @@ import type { Skill } from '../types.js'; export interface SyncOptions { yes?: boolean; + output?: string; } /** - * Sync installed skills to AGENTS.md + * Sync installed skills to a markdown file */ export async function syncAgentsMd(options: SyncOptions = {}): Promise { - if (!existsSync('AGENTS.md')) { - console.log(chalk.yellow('No AGENTS.md to update')); - return; + const outputPath = options.output || 'AGENTS.md'; + const outputName = basename(outputPath); + + // Validate output file is markdown + if (!outputPath.endsWith('.md')) { + console.error(chalk.red('Error: Output file must be a markdown file (.md)')); + process.exit(1); + } + + // Create file if it doesn't exist + if (!existsSync(outputPath)) { + const dir = dirname(outputPath); + if (dir && dir !== '.' && !existsSync(dir)) { + mkdirSync(dir, { recursive: true }); + } + writeFileSync(outputPath, `# ${outputName.replace('.md', '')}\n\n`); + console.log(chalk.dim(`Created ${outputPath}`)); } let skills = findAllSkills(); @@ -30,8 +46,8 @@ export async function syncAgentsMd(options: SyncOptions = {}): Promise { // Interactive mode by default (unless -y flag) if (!options.yes) { try { - // Parse what's currently in AGENTS.md - const content = readFileSync('AGENTS.md', 'utf-8'); + // Parse what's currently in output file + const content = readFileSync(outputPath, 'utf-8'); const currentSkills = parseCurrentSkills(content); // Sort: project first @@ -46,22 +62,22 @@ export async function syncAgentsMd(options: SyncOptions = {}): Promise { name: `${chalk.bold(skill.name.padEnd(25))} ${skill.location === 'project' ? chalk.blue('(project)') : chalk.dim('(global)')}`, value: skill.name, description: skill.description.slice(0, 70), - // Pre-select if currently in AGENTS.md, otherwise default to project skills + // Pre-select if currently in file, otherwise default to project skills checked: currentSkills.includes(skill.name) || (currentSkills.length === 0 && skill.location === 'project'), })); const selected = await checkbox({ - message: 'Select skills to sync to AGENTS.md', + message: `Select skills to sync to ${outputName}`, choices, pageSize: 15, }); if (selected.length === 0) { // User unchecked everything - remove skills section - const content = readFileSync('AGENTS.md', 'utf-8'); + const content = readFileSync(outputPath, 'utf-8'); const updated = removeSkillsSection(content); - writeFileSync('AGENTS.md', updated); - console.log(chalk.green('✅ Removed all skills from AGENTS.md')); + writeFileSync(outputPath, updated); + console.log(chalk.green(`✅ Removed all skills from ${outputName}`)); return; } @@ -77,17 +93,17 @@ export async function syncAgentsMd(options: SyncOptions = {}): Promise { } const xml = generateSkillsXml(skills); - const content = readFileSync('AGENTS.md', 'utf-8'); + const content = readFileSync(outputPath, 'utf-8'); const updated = replaceSkillsSection(content, xml); - writeFileSync('AGENTS.md', updated); + writeFileSync(outputPath, updated); const hadMarkers = content.includes(''); if (hadMarkers) { - console.log(chalk.green(`✅ Synced ${skills.length} skill(s) to AGENTS.md`)); + console.log(chalk.green(`✅ Synced ${skills.length} skill(s) to ${outputName}`)); } else { - console.log(chalk.green(`✅ Added skills section to AGENTS.md (${skills.length} skill(s))`)); + console.log(chalk.green(`✅ Added skills section to ${outputName} (${skills.length} skill(s))`)); } } diff --git a/src/utils/skills.ts b/src/utils/skills.ts index 141aeb0..d3c8253 100644 --- a/src/utils/skills.ts +++ b/src/utils/skills.ts @@ -1,9 +1,29 @@ -import { readFileSync, readdirSync, existsSync } from 'fs'; +import { readFileSync, readdirSync, existsSync, statSync, Dirent } from 'fs'; import { join } from 'path'; import { getSearchDirs } from './dirs.js'; import { extractYamlField } from './yaml.js'; import type { Skill, SkillLocation } from '../types.js'; +/** + * Check if a directory entry is a directory or a symlink pointing to a directory + */ +function isDirectoryOrSymlinkToDirectory(entry: Dirent, parentDir: string): boolean { + if (entry.isDirectory()) { + return true; + } + if (entry.isSymbolicLink()) { + try { + const fullPath = join(parentDir, entry.name); + const stats = statSync(fullPath); // statSync follows symlinks + return stats.isDirectory(); + } catch { + // Broken symlink or permission error + return false; + } + } + return false; +} + /** * Find all installed skills across directories */ @@ -18,7 +38,7 @@ export function findAllSkills(): Skill[] { const entries = readdirSync(dir, { withFileTypes: true }); for (const entry of entries) { - if (entry.isDirectory()) { + if (isDirectoryOrSymlinkToDirectory(entry, dir)) { // Deduplicate: only add if we haven't seen this skill name yet if (seen.has(entry.name)) continue; diff --git a/src/utils/yaml.ts b/src/utils/yaml.ts index e60d0a8..dffc2bc 100644 --- a/src/utils/yaml.ts +++ b/src/utils/yaml.ts @@ -2,7 +2,7 @@ * Extract field from YAML frontmatter */ export function extractYamlField(content: string, field: string): string { - const match = content.match(new RegExp(`^${field}:\\s*(.+)$`, 'm')); + const match = content.match(new RegExp(`^${field}:\\s*(.+?)$`, 'm')); return match ? match[1].trim() : ''; } diff --git a/tests/commands/install.test.ts b/tests/commands/install.test.ts new file mode 100644 index 0000000..586b9e5 --- /dev/null +++ b/tests/commands/install.test.ts @@ -0,0 +1,212 @@ +import { describe, it, expect } from 'vitest'; +import { resolve, join } from 'path'; +import { homedir } from 'os'; + +// We need to test the helper functions, but they're not exported +// So we'll test them indirectly or create a test module +// For now, let's test the logic patterns directly + +describe('install.ts helper functions', () => { + describe('isLocalPath detection', () => { + // Replicate the logic from isLocalPath() + const isLocalPath = (source: string): boolean => { + return ( + source.startsWith('/') || + source.startsWith('./') || + source.startsWith('../') || + source.startsWith('~/') + ); + }; + + it('should detect absolute paths starting with /', () => { + expect(isLocalPath('/absolute/path/to/skill')).toBe(true); + expect(isLocalPath('/Users/test/skills')).toBe(true); + }); + + it('should detect relative paths starting with ./', () => { + expect(isLocalPath('./relative/path')).toBe(true); + expect(isLocalPath('./skill')).toBe(true); + }); + + it('should detect parent relative paths starting with ../', () => { + expect(isLocalPath('../parent/path')).toBe(true); + expect(isLocalPath('../../../deep/path')).toBe(true); + }); + + it('should detect home directory paths starting with ~/', () => { + expect(isLocalPath('~/skills/my-skill')).toBe(true); + expect(isLocalPath('~/.claude/skills')).toBe(true); + }); + + it('should NOT detect GitHub shorthand as local path', () => { + expect(isLocalPath('owner/repo')).toBe(false); + expect(isLocalPath('anthropics/skills')).toBe(false); + expect(isLocalPath('owner/repo/skill-path')).toBe(false); + }); + + it('should NOT detect git URLs as local path', () => { + expect(isLocalPath('git@github.com:owner/repo.git')).toBe(false); + expect(isLocalPath('https://github.com/owner/repo')).toBe(false); + expect(isLocalPath('http://github.com/owner/repo')).toBe(false); + }); + + it('should NOT detect plain names as local path', () => { + expect(isLocalPath('skill-name')).toBe(false); + expect(isLocalPath('my-skill')).toBe(false); + }); + }); + + describe('isGitUrl detection', () => { + // Replicate the logic from isGitUrl() + const isGitUrl = (source: string): boolean => { + return ( + source.startsWith('git@') || + source.startsWith('git://') || + source.startsWith('http://') || + source.startsWith('https://') || + source.endsWith('.git') + ); + }; + + it('should detect SSH git URLs', () => { + expect(isGitUrl('git@github.com:owner/repo.git')).toBe(true); + expect(isGitUrl('git@gitlab.com:group/project.git')).toBe(true); + expect(isGitUrl('git@bitbucket.org:team/repo.git')).toBe(true); + }); + + it('should detect git:// protocol URLs', () => { + expect(isGitUrl('git://github.com/owner/repo.git')).toBe(true); + }); + + it('should detect HTTPS URLs', () => { + expect(isGitUrl('https://github.com/owner/repo')).toBe(true); + expect(isGitUrl('https://github.com/owner/repo.git')).toBe(true); + expect(isGitUrl('https://gitlab.com/group/project')).toBe(true); + }); + + it('should detect HTTP URLs', () => { + expect(isGitUrl('http://github.com/owner/repo')).toBe(true); + }); + + it('should detect URLs ending in .git', () => { + expect(isGitUrl('custom-host.com/repo.git')).toBe(true); + expect(isGitUrl('anything.git')).toBe(true); + }); + + it('should NOT detect GitHub shorthand as git URL', () => { + expect(isGitUrl('owner/repo')).toBe(false); + expect(isGitUrl('anthropics/skills')).toBe(false); + }); + + it('should NOT detect local paths as git URL', () => { + expect(isGitUrl('/absolute/path')).toBe(false); + expect(isGitUrl('./relative/path')).toBe(false); + expect(isGitUrl('~/home/path')).toBe(false); + }); + }); + + describe('expandPath tilde expansion', () => { + // Replicate the logic from expandPath() + const expandPath = (source: string): string => { + if (source.startsWith('~/')) { + return join(homedir(), source.slice(2)); + } + return resolve(source); + }; + + it('should expand ~ to home directory', () => { + const expanded = expandPath('~/skills/test'); + expect(expanded).toBe(join(homedir(), 'skills/test')); + }); + + it('should expand ~/.claude/skills correctly', () => { + const expanded = expandPath('~/.claude/skills'); + expect(expanded).toBe(join(homedir(), '.claude/skills')); + }); + + it('should resolve relative paths', () => { + const expanded = expandPath('./relative'); + expect(expanded).toBe(resolve('./relative')); + }); + + it('should keep absolute paths as-is (resolved)', () => { + const expanded = expandPath('/absolute/path'); + expect(expanded).toBe('/absolute/path'); + }); + }); + + describe('path traversal security', () => { + // Test the security check logic + const isPathSafe = (targetPath: string, targetDir: string): boolean => { + const resolvedTargetPath = resolve(targetPath); + const resolvedTargetDir = resolve(targetDir); + return resolvedTargetPath.startsWith(resolvedTargetDir + '/'); + }; + + it('should allow normal skill paths within target directory', () => { + expect(isPathSafe('/home/user/.claude/skills/my-skill', '/home/user/.claude/skills')).toBe(true); + }); + + it('should block path traversal attempts with ../', () => { + expect(isPathSafe('/home/user/.claude/skills/../../../etc/passwd', '/home/user/.claude/skills')).toBe(false); + }); + + it('should block paths outside target directory', () => { + expect(isPathSafe('/etc/passwd', '/home/user/.claude/skills')).toBe(false); + }); + + it('should block paths that are prefix but not subdirectory', () => { + // /home/user/.claude/skills-evil should NOT be allowed when target is /home/user/.claude/skills + expect(isPathSafe('/home/user/.claude/skills-evil', '/home/user/.claude/skills')).toBe(false); + }); + + it('should allow nested subdirectories', () => { + expect(isPathSafe('/home/user/.claude/skills/category/my-skill', '/home/user/.claude/skills')).toBe(true); + }); + }); +}); + +describe('GitHub shorthand parsing', () => { + // Test the parsing logic for owner/repo and owner/repo/path + const parseGitHubShorthand = (source: string): { repoUrl: string; skillSubpath: string } | null => { + const parts = source.split('/'); + if (parts.length === 2) { + return { + repoUrl: `https://github.com/${source}`, + skillSubpath: '', + }; + } else if (parts.length > 2) { + return { + repoUrl: `https://github.com/${parts[0]}/${parts[1]}`, + skillSubpath: parts.slice(2).join('/'), + }; + } + return null; + }; + + it('should parse owner/repo format', () => { + const result = parseGitHubShorthand('anthropics/skills'); + expect(result).not.toBeNull(); + expect(result?.repoUrl).toBe('https://github.com/anthropics/skills'); + expect(result?.skillSubpath).toBe(''); + }); + + it('should parse owner/repo/skill-path format', () => { + const result = parseGitHubShorthand('anthropics/skills/document-skills/pdf'); + expect(result).not.toBeNull(); + expect(result?.repoUrl).toBe('https://github.com/anthropics/skills'); + expect(result?.skillSubpath).toBe('document-skills/pdf'); + }); + + it('should parse owner/repo/nested/path format', () => { + const result = parseGitHubShorthand('owner/repo/deep/nested/skill'); + expect(result).not.toBeNull(); + expect(result?.repoUrl).toBe('https://github.com/owner/repo'); + expect(result?.skillSubpath).toBe('deep/nested/skill'); + }); + + it('should return null for single part', () => { + const result = parseGitHubShorthand('single'); + expect(result).toBeNull(); + }); +}); diff --git a/tests/commands/sync.test.ts b/tests/commands/sync.test.ts new file mode 100644 index 0000000..6a2d179 --- /dev/null +++ b/tests/commands/sync.test.ts @@ -0,0 +1,256 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdirSync, writeFileSync, readFileSync, rmSync, existsSync } from 'fs'; +import { join, dirname, basename } from 'path'; +import { tmpdir } from 'os'; + +// Test the sync utility functions directly +import { + generateSkillsXml, + replaceSkillsSection, + parseCurrentSkills, + removeSkillsSection, +} from '../../src/utils/agents-md.js'; +import type { Skill } from '../../src/types.js'; + +const testId = Math.random().toString(36).slice(2); +const testTempDir = join(tmpdir(), `openskills-sync-test-${testId}`); + +describe('sync utilities (agents-md.ts)', () => { + describe('generateSkillsXml', () => { + it('should generate valid XML for skills', () => { + const skills: Skill[] = [ + { name: 'pdf', description: 'PDF manipulation', location: 'project', path: '/path/to/pdf' }, + { name: 'xlsx', description: 'Spreadsheet editing', location: 'global', path: '/path/to/xlsx' }, + ]; + + const xml = generateSkillsXml(skills); + + expect(xml).toContain(''); + expect(xml).toContain('pdf'); + expect(xml).toContain('PDF manipulation'); + expect(xml).toContain('project'); + expect(xml).toContain('xlsx'); + expect(xml).toContain('Spreadsheet editing'); + expect(xml).toContain('global'); + expect(xml).toContain(''); + }); + + it('should include usage instructions', () => { + const skills: Skill[] = [ + { name: 'test', description: 'Test skill', location: 'project', path: '/path' }, + ]; + + const xml = generateSkillsXml(skills); + + expect(xml).toContain(''); + expect(xml).toContain('openskills read'); + expect(xml).toContain(''); + }); + + it('should generate empty skills section for empty array', () => { + const xml = generateSkillsXml([]); + + expect(xml).toContain(''); + expect(xml).toContain(''); + }); + }); + + describe('parseCurrentSkills', () => { + it('should parse skill names from existing content', () => { + const content = ` +# AGENTS.md + + + + +pdf +PDF tools + + +xlsx +Excel tools + + + +`; + + const skills = parseCurrentSkills(content); + + expect(skills).toContain('pdf'); + expect(skills).toContain('xlsx'); + expect(skills).toHaveLength(2); + }); + + it('should return empty array for content without skills', () => { + const content = '# AGENTS.md\n\nNo skills here.'; + + const skills = parseCurrentSkills(content); + + expect(skills).toHaveLength(0); + }); + + it('should handle malformed XML gracefully', () => { + const content = 'broken'; + + const skills = parseCurrentSkills(content); + + expect(Array.isArray(skills)).toBe(true); + }); + }); + + describe('replaceSkillsSection', () => { + it('should replace existing skills_system section', () => { + const content = `# AGENTS.md + + +OLD CONTENT + + +Other content`; + + const newSection = 'NEW CONTENT'; + const result = replaceSkillsSection(content, newSection); + + expect(result).toContain('NEW CONTENT'); + expect(result).not.toContain('OLD CONTENT'); + expect(result).toContain('Other content'); + }); + + it('should replace HTML comment markers', () => { + const content = `# AGENTS.md + + +OLD SKILLS + + +Footer`; + + const newSection = 'NEW SKILLS'; + const result = replaceSkillsSection(content, newSection); + + expect(result).toContain('NEW SKILLS'); + expect(result).not.toContain('OLD SKILLS'); + }); + + it('should append to end if no markers found', () => { + const content = '# AGENTS.md\n\nSome content.'; + const newSection = 'SKILLS'; + + const result = replaceSkillsSection(content, newSection); + + expect(result).toContain('Some content.'); + expect(result).toContain('SKILLS'); + }); + }); + + describe('removeSkillsSection', () => { + it('should remove skills_system section', () => { + const content = `# AGENTS.md + + +Skills content + + +Footer`; + + const result = removeSkillsSection(content); + + expect(result).not.toContain('Skills content'); + expect(result).toContain('Footer'); + }); + + it('should handle content without skills section', () => { + const content = '# AGENTS.md\n\nNo skills.'; + + const result = removeSkillsSection(content); + + expect(result).toBe(content); + }); + }); +}); + +describe('sync --output flag logic', () => { + beforeEach(() => { + mkdirSync(testTempDir, { recursive: true }); + }); + + afterEach(() => { + rmSync(testTempDir, { recursive: true, force: true }); + }); + + describe('output path validation', () => { + it('should accept .md files', () => { + const validPaths = [ + 'AGENTS.md', + 'custom.md', + '.ruler/AGENTS.md', + 'docs/rules.md', + ]; + + for (const path of validPaths) { + expect(path.endsWith('.md')).toBe(true); + } + }); + + it('should reject non-.md files', () => { + const invalidPaths = [ + 'AGENTS.txt', + 'rules.yaml', + 'config.json', + 'noextension', + ]; + + for (const path of invalidPaths) { + expect(path.endsWith('.md')).toBe(false); + } + }); + }); + + describe('auto-create file behavior', () => { + it('should create file with heading if not exists', () => { + const outputPath = join(testTempDir, 'NEW-FILE.md'); + + // Simulate the auto-create logic + if (!existsSync(outputPath)) { + const outputName = basename(outputPath); + writeFileSync(outputPath, `# ${outputName.replace('.md', '')}\n\n`); + } + + expect(existsSync(outputPath)).toBe(true); + const content = readFileSync(outputPath, 'utf-8'); + expect(content).toBe('# NEW-FILE\n\n'); + }); + + it('should create nested directories if needed', () => { + const outputPath = join(testTempDir, 'nested', 'deep', 'AGENTS.md'); + const dir = dirname(outputPath); + + // Simulate the directory creation logic + if (!existsSync(dir)) { + mkdirSync(dir, { recursive: true }); + } + writeFileSync(outputPath, '# AGENTS\n\n'); + + expect(existsSync(outputPath)).toBe(true); + expect(existsSync(dir)).toBe(true); + }); + + it('should preserve existing file content', () => { + const outputPath = join(testTempDir, 'existing.md'); + const existingContent = '# Existing Content\n\nImportant stuff here.'; + writeFileSync(outputPath, existingContent); + + // Read existing content + const content = readFileSync(outputPath, 'utf-8'); + + expect(content).toBe(existingContent); + }); + }); + + describe('default output path', () => { + it('should default to AGENTS.md', () => { + const defaultPath = 'AGENTS.md'; + expect(defaultPath).toBe('AGENTS.md'); + }); + }); +}); diff --git a/tests/integration/e2e.test.ts b/tests/integration/e2e.test.ts new file mode 100644 index 0000000..b4d328d --- /dev/null +++ b/tests/integration/e2e.test.ts @@ -0,0 +1,268 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdirSync, writeFileSync, readFileSync, rmSync, existsSync, symlinkSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { execSync } from 'child_process'; + +const testId = Math.random().toString(36).slice(2); +const testTempDir = join(tmpdir(), `openskills-e2e-${testId}`); +const cliPath = join(process.cwd(), 'dist', 'cli.js'); + +// Helper to run CLI commands +function runCli(args: string, cwd?: string): { stdout: string; stderr: string; exitCode: number } { + try { + const stdout = execSync(`node ${cliPath} ${args}`, { + cwd: cwd || testTempDir, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + }); + return { stdout, stderr: '', exitCode: 0 }; + } catch (error: unknown) { + const err = error as { stdout?: string; stderr?: string; status?: number }; + return { + stdout: err.stdout || '', + stderr: err.stderr || '', + exitCode: err.status || 1, + }; + } +} + +// Helper to create a valid skill +function createTestSkill(dir: string, name: string, description: string = 'Test skill'): void { + const skillDir = join(dir, name); + mkdirSync(skillDir, { recursive: true }); + writeFileSync( + join(skillDir, 'SKILL.md'), + `--- +name: ${name} +description: ${description} +--- + +# ${name} + +Instructions for ${name}. +` + ); +} + +describe('End-to-end CLI tests', () => { + beforeEach(() => { + mkdirSync(testTempDir, { recursive: true }); + }); + + afterEach(() => { + rmSync(testTempDir, { recursive: true, force: true }); + }); + + describe('openskills list', () => { + it('should list installed skills', () => { + // Create a project skills directory with a skill + const skillsDir = join(testTempDir, '.claude', 'skills'); + createTestSkill(skillsDir, 'test-skill', 'A test skill'); + + const result = runCli('list'); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('test-skill'); + }); + + it('should show summary with skill counts', () => { + // Create empty project skills directory - global skills may still exist + mkdirSync(join(testTempDir, '.claude', 'skills'), { recursive: true }); + + const result = runCli('list'); + + // Should show a summary line with counts + expect(result.stdout).toMatch(/Summary:|No skills installed/); + }); + }); + + describe('openskills read', () => { + it('should read skill content', () => { + const skillsDir = join(testTempDir, '.claude', 'skills'); + createTestSkill(skillsDir, 'readable-skill', 'Readable skill description'); + + const result = runCli('read readable-skill'); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('readable-skill'); + expect(result.stdout).toContain('Instructions for readable-skill'); + }); + + it('should error for non-existent skill', () => { + const result = runCli('read non-existent-skill'); + + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('not found'); + }); + }); + + describe('openskills sync', () => { + it('should sync skills to AGENTS.md', () => { + // Create skill and AGENTS.md + const skillsDir = join(testTempDir, '.claude', 'skills'); + createTestSkill(skillsDir, 'sync-skill', 'Skill to sync'); + writeFileSync(join(testTempDir, 'AGENTS.md'), '# AGENTS\n'); + + const result = runCli('sync -y'); + + expect(result.exitCode).toBe(0); + + const agentsMd = readFileSync(join(testTempDir, 'AGENTS.md'), 'utf-8'); + expect(agentsMd).toContain('sync-skill'); + expect(agentsMd).toContain(' { + const skillsDir = join(testTempDir, '.claude', 'skills'); + createTestSkill(skillsDir, 'output-skill', 'Test output'); + + const outputPath = join(testTempDir, 'custom-output.md'); + const result = runCli(`sync -y --output ${outputPath}`); + + expect(result.exitCode).toBe(0); + expect(existsSync(outputPath)).toBe(true); + + const content = readFileSync(outputPath, 'utf-8'); + expect(content).toContain('output-skill'); + }); + + it('should create nested directories with --output flag', () => { + const skillsDir = join(testTempDir, '.claude', 'skills'); + createTestSkill(skillsDir, 'nested-skill', 'Test nested'); + + const outputPath = join(testTempDir, '.ruler', 'deep', 'AGENTS.md'); + const result = runCli(`sync -y --output ${outputPath}`); + + expect(result.exitCode).toBe(0); + expect(existsSync(outputPath)).toBe(true); + }); + + it('should reject non-.md output files', () => { + const skillsDir = join(testTempDir, '.claude', 'skills'); + createTestSkill(skillsDir, 'any-skill', 'Test'); + + const result = runCli(`sync -y --output ${join(testTempDir, 'invalid.txt')}`); + + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('.md'); + }); + }); + + describe('openskills install (local paths)', () => { + it('should install from absolute local path', () => { + // Create a source skill + const sourceDir = join(testTempDir, 'source-skills'); + createTestSkill(sourceDir, 'local-skill', 'Local skill'); + + // Install to project + const result = runCli(`install ${join(sourceDir, 'local-skill')} -y`); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('Installed'); + + // Verify skill was copied + const installedPath = join(testTempDir, '.claude', 'skills', 'local-skill', 'SKILL.md'); + expect(existsSync(installedPath)).toBe(true); + }); + + it('should install directory of skills from local path', () => { + // Create multiple source skills + const sourceDir = join(testTempDir, 'multi-skills'); + createTestSkill(sourceDir, 'skill-one', 'First skill'); + createTestSkill(sourceDir, 'skill-two', 'Second skill'); + + const result = runCli(`install ${sourceDir} -y`); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('skill-one'); + expect(result.stdout).toContain('skill-two'); + }); + + it('should error for non-existent local path', () => { + const result = runCli(`install /non/existent/path -y`); + + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('does not exist'); + }); + }); + + describe('openskills remove', () => { + it('should remove installed skill', () => { + const skillsDir = join(testTempDir, '.claude', 'skills'); + createTestSkill(skillsDir, 'removable-skill', 'To be removed'); + + const result = runCli('remove removable-skill'); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('Removed'); + expect(existsSync(join(skillsDir, 'removable-skill'))).toBe(false); + }); + + it('should error for non-existent skill', () => { + const result = runCli('remove ghost-skill'); + + expect(result.exitCode).toBe(1); + }); + }); + + describe('symlinked skills', () => { + it('should list symlinked skills', () => { + // Create a skill in a separate location + const actualSkillDir = join(testTempDir, 'actual-skills'); + createTestSkill(actualSkillDir, 'symlinked-skill', 'Symlinked skill'); + + // Create symlink in skills directory + const skillsDir = join(testTempDir, '.claude', 'skills'); + mkdirSync(skillsDir, { recursive: true }); + symlinkSync( + join(actualSkillDir, 'symlinked-skill'), + join(skillsDir, 'symlinked-skill') + ); + + const result = runCli('list'); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('symlinked-skill'); + }); + + it('should read symlinked skill content', () => { + const actualSkillDir = join(testTempDir, 'actual-skills'); + createTestSkill(actualSkillDir, 'linked-readable', 'Linked readable'); + + const skillsDir = join(testTempDir, '.claude', 'skills'); + mkdirSync(skillsDir, { recursive: true }); + symlinkSync( + join(actualSkillDir, 'linked-readable'), + join(skillsDir, 'linked-readable') + ); + + const result = runCli('read linked-readable'); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('linked-readable'); + }); + }); + + describe('--yes flag behavior', () => { + it('should auto-overwrite with -y flag', () => { + // Create initial skill + const skillsDir = join(testTempDir, '.claude', 'skills'); + createTestSkill(skillsDir, 'overwrite-skill', 'Original'); + + // Create source skill to install + const sourceDir = join(testTempDir, 'source'); + createTestSkill(sourceDir, 'overwrite-skill', 'Updated'); + + // Install with -y should overwrite + const result = runCli(`install ${join(sourceDir, 'overwrite-skill')} -y`); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('Overwriting'); + + // Verify content was updated + const content = readFileSync(join(skillsDir, 'overwrite-skill', 'SKILL.md'), 'utf-8'); + expect(content).toContain('Updated'); + }); + }); +}); diff --git a/tests/utils/skills.test.ts b/tests/utils/skills.test.ts new file mode 100644 index 0000000..b460361 --- /dev/null +++ b/tests/utils/skills.test.ts @@ -0,0 +1,212 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { mkdirSync, writeFileSync, symlinkSync, rmSync, existsSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { findAllSkills, findSkill } from '../../src/utils/skills.js'; +import * as dirsModule from '../../src/utils/dirs.js'; + +// Create unique temp directories for each test run +const testId = Math.random().toString(36).slice(2); +const testTempDir = join(tmpdir(), `openskills-test-${testId}`); +const testProjectSkillsDir = join(testTempDir, 'project', '.claude', 'skills'); +const testGlobalSkillsDir = join(testTempDir, 'global', '.claude', 'skills'); +const testSymlinkTargetDir = join(testTempDir, 'symlink-targets'); + +// Helper to create a skill directory with SKILL.md +function createSkill(baseDir: string, skillName: string, description: string = 'Test skill'): void { + const skillDir = join(baseDir, skillName); + mkdirSync(skillDir, { recursive: true }); + writeFileSync( + join(skillDir, 'SKILL.md'), + `--- +name: ${skillName} +description: ${description} +--- + +# ${skillName} + +This is a test skill.` + ); +} + +// Helper to create a symlinked skill +function createSymlinkedSkill( + skillsDir: string, + skillName: string, + description: string = 'Symlinked test skill' +): void { + // Create the actual skill in the symlink target directory + const actualSkillDir = join(testSymlinkTargetDir, skillName); + mkdirSync(actualSkillDir, { recursive: true }); + writeFileSync( + join(actualSkillDir, 'SKILL.md'), + `--- +name: ${skillName} +description: ${description} +--- + +# ${skillName} + +This is a symlinked test skill.` + ); + + // Create symlink in the skills directory + mkdirSync(skillsDir, { recursive: true }); + symlinkSync(actualSkillDir, join(skillsDir, skillName), 'dir'); +} + +// Helper to create a broken symlink +function createBrokenSymlink(skillsDir: string, skillName: string): void { + mkdirSync(skillsDir, { recursive: true }); + const nonExistentTarget = join(testTempDir, 'non-existent', skillName); + symlinkSync(nonExistentTarget, join(skillsDir, skillName), 'dir'); +} + +describe('skills.ts', () => { + beforeEach(() => { + // Create test directories + mkdirSync(testProjectSkillsDir, { recursive: true }); + mkdirSync(testGlobalSkillsDir, { recursive: true }); + mkdirSync(testSymlinkTargetDir, { recursive: true }); + + // Mock getSearchDirs to return our test directories + vi.spyOn(dirsModule, 'getSearchDirs').mockReturnValue([ + testProjectSkillsDir, + testGlobalSkillsDir, + ]); + }); + + afterEach(() => { + // Cleanup test directories + rmSync(testTempDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + describe('findAllSkills', () => { + it('should find regular directory skills', () => { + createSkill(testProjectSkillsDir, 'regular-skill', 'A regular skill'); + + const skills = findAllSkills(); + + expect(skills).toHaveLength(1); + expect(skills[0].name).toBe('regular-skill'); + expect(skills[0].description).toBe('A regular skill'); + }); + + it('should find symlinked skill directories', () => { + createSymlinkedSkill(testGlobalSkillsDir, 'symlinked-skill', 'A symlinked skill'); + + const skills = findAllSkills(); + + expect(skills).toHaveLength(1); + expect(skills[0].name).toBe('symlinked-skill'); + expect(skills[0].description).toBe('A symlinked skill'); + }); + + it('should find both regular and symlinked skills', () => { + createSkill(testProjectSkillsDir, 'regular-skill', 'Regular'); + createSymlinkedSkill(testGlobalSkillsDir, 'symlinked-skill', 'Symlinked'); + + const skills = findAllSkills(); + + expect(skills).toHaveLength(2); + const names = skills.map(s => s.name); + expect(names).toContain('regular-skill'); + expect(names).toContain('symlinked-skill'); + }); + + it('should skip broken symlinks gracefully', () => { + createSkill(testProjectSkillsDir, 'good-skill', 'Good skill'); + createBrokenSymlink(testGlobalSkillsDir, 'broken-symlink'); + + const skills = findAllSkills(); + + expect(skills).toHaveLength(1); + expect(skills[0].name).toBe('good-skill'); + }); + + it('should deduplicate skills with same name (project takes priority)', () => { + createSkill(testProjectSkillsDir, 'duplicate-skill', 'Project version'); + createSkill(testGlobalSkillsDir, 'duplicate-skill', 'Global version'); + + const skills = findAllSkills(); + + expect(skills).toHaveLength(1); + expect(skills[0].name).toBe('duplicate-skill'); + expect(skills[0].description).toBe('Project version'); + }); + + it('should skip directories without SKILL.md', () => { + const noSkillDir = join(testProjectSkillsDir, 'not-a-skill'); + mkdirSync(noSkillDir, { recursive: true }); + writeFileSync(join(noSkillDir, 'README.md'), '# Not a skill'); + + const skills = findAllSkills(); + + expect(skills).toHaveLength(0); + }); + + it('should skip files (not directories)', () => { + writeFileSync(join(testProjectSkillsDir, 'file.txt'), 'Just a file'); + createSkill(testProjectSkillsDir, 'actual-skill', 'Real skill'); + + const skills = findAllSkills(); + + expect(skills).toHaveLength(1); + expect(skills[0].name).toBe('actual-skill'); + }); + + it('should handle empty skills directories', () => { + const skills = findAllSkills(); + expect(skills).toHaveLength(0); + }); + + it('should handle non-existent directories gracefully', () => { + vi.spyOn(dirsModule, 'getSearchDirs').mockReturnValue([ + '/non/existent/path', + testProjectSkillsDir, + ]); + createSkill(testProjectSkillsDir, 'skill', 'Test'); + + const skills = findAllSkills(); + + expect(skills).toHaveLength(1); + }); + }); + + describe('findSkill', () => { + it('should find a skill by name', () => { + createSkill(testProjectSkillsDir, 'my-skill', 'My skill description'); + + const skill = findSkill('my-skill'); + + expect(skill).not.toBeNull(); + expect(skill?.path).toContain('my-skill/SKILL.md'); + expect(skill?.baseDir).toContain('my-skill'); + }); + + it('should find symlinked skills by name', () => { + createSymlinkedSkill(testGlobalSkillsDir, 'linked-skill', 'Linked description'); + + const skill = findSkill('linked-skill'); + + expect(skill).not.toBeNull(); + expect(skill?.path).toContain('linked-skill/SKILL.md'); + }); + + it('should return null for non-existent skill', () => { + const skill = findSkill('non-existent'); + expect(skill).toBeNull(); + }); + + it('should return project skill over global with same name', () => { + createSkill(testProjectSkillsDir, 'shared-skill', 'Project'); + createSkill(testGlobalSkillsDir, 'shared-skill', 'Global'); + + const skill = findSkill('shared-skill'); + + expect(skill).not.toBeNull(); + expect(skill?.source).toBe(testProjectSkillsDir); + }); + }); +}); diff --git a/tests/utils/yaml.test.ts b/tests/utils/yaml.test.ts index 258db2c..35e9574 100644 --- a/tests/utils/yaml.test.ts +++ b/tests/utils/yaml.test.ts @@ -30,6 +30,55 @@ description: First line expect(extractYamlField(content, 'description')).toBe('First line'); }); + + // Security tests for non-greedy regex + it('should use non-greedy matching (security)', () => { + // With greedy regex (.+), this could match across lines incorrectly + const content = `--- +name: skill-name +description: Short desc +other: value +---`; + + // Non-greedy should stop at end of line + const name = extractYamlField(content, 'name'); + expect(name).toBe('skill-name'); + expect(name).not.toContain('description'); + }); + + it('should not be vulnerable to ReDoS patterns', () => { + // Test that extraction completes quickly even with potentially problematic input + const start = Date.now(); + const content = `--- +name: ${'a'.repeat(1000)} +---`; + + extractYamlField(content, 'name'); + const elapsed = Date.now() - start; + + // Should complete in under 100ms even with long input + expect(elapsed).toBeLessThan(100); + }); + + it('should handle special characters in values', () => { + const content = `--- +name: skill-with-special_chars.v2 +description: Contains "quotes" and 'apostrophes' +---`; + + expect(extractYamlField(content, 'name')).toBe('skill-with-special_chars.v2'); + }); + + it('should handle colons in values', () => { + const content = `--- +name: my-skill +description: URL: https://example.com +---`; + + // Should capture the full value including the colon + const desc = extractYamlField(content, 'description'); + expect(desc).toContain('URL:'); + }); }); describe('hasValidFrontmatter', () => {