From 83752d4e1ca0e63a4aff463ceeb67f5625c9ecef Mon Sep 17 00:00:00 2001 From: William Valentin Date: Sun, 15 Feb 2026 11:06:02 -0800 Subject: [PATCH] feat(skills): scan manifest spec and warn on missing permissions --- src/skills/loader.ts | 21 ++----- src/skills/scanner.ts | 125 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 128 insertions(+), 18 deletions(-) diff --git a/src/skills/loader.ts b/src/skills/loader.ts index 85eb7a1..b8d4be1 100644 --- a/src/skills/loader.ts +++ b/src/skills/loader.ts @@ -215,30 +215,20 @@ export function loadSkill(directory: string, tier: SkillTier): Skill | null { }; let manifest: SkillManifest = inferManifest(); - const manifestReasons: string[] = []; if (existsSync(manifestPath)) { // Parse manifest.json try { const raw = JSON.parse(readFileSync(manifestPath, 'utf-8')); - // Validate required fields - if (!raw.name || !raw.description || !raw.version) { - manifestReasons.push('manifest.missing_required_fields: manifest.json missing required fields (name, description, version)'); - } else if (!hasValidInstallers(raw)) { - manifestReasons.push('manifest.invalid_installers: manifest.json has invalid installers specification'); - } else if (!hasValidPermissions(raw)) { - manifestReasons.push('manifest.invalid_permissions: manifest.json has invalid permissions specification'); - } else { + if (raw && typeof raw === 'object' && raw.name && raw.description && raw.version) { manifest = { - ...raw, + ...(raw as SkillManifest), tier, // Override tier from argument }; } } catch (error) { - manifestReasons.push( - `manifest.invalid_json: Failed to parse manifest.json (${error instanceof Error ? error.message : 'Unknown error'})`, - ); + // Scanner will capture this and mark the skill unavailable. } } else { manifest = inferManifest(); @@ -251,15 +241,12 @@ export function loadSkill(directory: string, tier: SkillTier): Skill | null { if (!scan.ok) { unavailableReasons.push(...scanReasons); } - if (manifestReasons.length > 0) { - unavailableReasons.push(...manifestReasons); - } const skill: Skill = { manifest, instructions, directory: absDir, - available: available && scan.ok && manifestReasons.length === 0, + available: available && scan.ok, }; if (!skill.available) { diff --git a/src/skills/scanner.ts b/src/skills/scanner.ts index 9e0cb33..53af3ba 100644 --- a/src/skills/scanner.ts +++ b/src/skills/scanner.ts @@ -69,6 +69,111 @@ function scanPromptInjectionMarkers(text: string, filePath: string): SkillScanIs return issues; } +function isStringArray(value: unknown): value is string[] { + return Array.isArray(value) && value.every((item) => typeof item === 'string'); +} + +function isNumberArray(value: unknown): value is number[] { + return Array.isArray(value) && value.every((item) => typeof item === 'number' && Number.isFinite(item)); +} + +function hasValidInstallers(manifest: unknown): boolean { + if (!manifest || typeof manifest !== 'object') { + return false; + } + const candidate = manifest as { installers?: unknown }; + if (candidate.installers === undefined) { + return true; + } + if (!Array.isArray(candidate.installers)) { + return false; + } + return candidate.installers.every((installer) => { + if (!installer || typeof installer !== 'object') { + return false; + } + const typedInstaller = installer as { type?: unknown; packages?: unknown; url?: unknown; destination?: unknown }; + if (typedInstaller.type === 'brew' || typedInstaller.type === 'node' || typedInstaller.type === 'go') { + return isStringArray(typedInstaller.packages); + } + if (typedInstaller.type === 'download') { + if (typeof typedInstaller.url !== 'string') { + return false; + } + if (typedInstaller.destination !== undefined && typeof typedInstaller.destination !== 'string') { + return false; + } + return true; + } + return false; + }); +} + +function hasValidPermissions(manifest: unknown): boolean { + if (!manifest || typeof manifest !== 'object') { + return false; + } + const candidate = manifest as { permissions?: unknown }; + if (candidate.permissions === undefined) { + return true; + } + if (!candidate.permissions || typeof candidate.permissions !== 'object') { + return false; + } + const perms = candidate.permissions as { + tool_groups?: unknown; + tools?: unknown; + fs?: unknown; + net?: unknown; + secrets?: unknown; + execution_environment?: unknown; + }; + if (perms.tool_groups !== undefined && !isStringArray(perms.tool_groups)) { + return false; + } + if (perms.tools !== undefined && !isStringArray(perms.tools)) { + return false; + } + if (perms.fs !== undefined) { + if (!perms.fs || typeof perms.fs !== 'object') { + return false; + } + const fsPerms = perms.fs as { read?: unknown; write?: unknown }; + if (fsPerms.read !== undefined && !isStringArray(fsPerms.read)) { + return false; + } + if (fsPerms.write !== undefined && !isStringArray(fsPerms.write)) { + return false; + } + } + if (perms.net !== undefined) { + if (!Array.isArray(perms.net)) { + return false; + } + for (const entry of perms.net) { + if (!entry || typeof entry !== 'object') { + return false; + } + const e = entry as { host?: unknown; ports?: unknown }; + if (typeof e.host !== 'string' || e.host.trim().length === 0) { + return false; + } + if (e.ports !== undefined && !isNumberArray(e.ports)) { + return false; + } + } + } + if (perms.secrets !== undefined && !isStringArray(perms.secrets)) { + return false; + } + if (perms.execution_environment !== undefined) { + if (perms.execution_environment !== 'sandbox' && perms.execution_environment !== 'host') { + return false; + } + } + return true; +} + function safeJsonParse(raw: string): unknown { return JSON.parse(raw) as unknown; } @@ -143,7 +248,25 @@ export function scanSkillDirectory(directory: string, opts: SkillScanOptions = { issues.push({ severity: 'error', code: 'content.binary', message: 'manifest.json appears to be binary', path: manifestPath }); } else { const text = buf.toString('utf-8'); - safeJsonParse(text); + const raw = safeJsonParse(text) as any; + if (!raw || typeof raw !== 'object') { + issues.push({ severity: 'error', code: 'manifest.missing_required_fields', message: 'manifest.json must be an object with required fields', path: manifestPath }); + } else { + if (!raw.name || !raw.description || !raw.version) { + issues.push({ severity: 'error', code: 'manifest.missing_required_fields', message: 'manifest.json missing required fields (name, description, version)', path: manifestPath }); + } + if (!hasValidInstallers(raw)) { + issues.push({ severity: 'error', code: 'manifest.invalid_installers', message: 'manifest.json has invalid installers specification', path: manifestPath }); + } + if (!hasValidPermissions(raw)) { + issues.push({ severity: 'error', code: 'manifest.invalid_permissions', message: 'manifest.json has invalid permissions specification', path: manifestPath }); + } + + // Soft guidance: permissions should exist for routable skills. + if (!raw.permissions) { + issues.push({ severity: 'warn', code: 'manifest.permissions_missing', message: 'manifest.json has no permissions block (skill will not be routable)' , path: manifestPath }); + } + } } } catch { issues.push({ severity: 'error', code: 'manifest.invalid_json', message: 'manifest.json must be valid JSON', path: manifestPath });