diff --git a/CHANGELOG.md b/CHANGELOG.md index af7996ce..9315ae6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,18 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Added +- **Plugin Structure Analyzer** - New `/enhance:plugin` command (#119) + - Analyzes plugin.json structure and required fields + - Validates MCP tool definitions against best practices + - Detects security patterns in agent/command files + - HIGH/MEDIUM/LOW certainty levels following slop-patterns model + - Auto-fix capability for HIGH certainty issues + - New lib/enhance/ module with pattern matching, reporter, fixer + - Comprehensive test suite (21 tests) + ## [2.7.1] - 2026-01-22 ### Security diff --git a/lib/enhance/fixer.js b/lib/enhance/fixer.js new file mode 100644 index 00000000..cc0bd8b3 --- /dev/null +++ b/lib/enhance/fixer.js @@ -0,0 +1,313 @@ +/** + * Plugin Analysis Fixer + * Applies auto-fixes for HIGH certainty issues + * + * @author Avi Fenesh + * @license MIT + */ + +const fs = require('fs'); +const path = require('path'); + +/** + * Apply fixes for issues that have autoFixFn + * @param {Array} issues - Array of issues with potential fixes + * @param {Object} options - Fix options + * @param {boolean} options.dryRun - Show changes without applying + * @param {boolean} options.backup - Create backup files + * @returns {Object} Fix results + */ +function applyFixes(issues, options = {}) { + const { dryRun = false, backup = true } = options; + + const results = { + applied: [], + skipped: [], + errors: [] + }; + + // Filter to only HIGH certainty issues with autoFixFn + const fixableIssues = issues.filter(i => + i.certainty === 'HIGH' && i.autoFixFn && i.filePath + ); + + // Group by file to minimize reads/writes + const byFile = new Map(); + for (const issue of fixableIssues) { + if (!byFile.has(issue.filePath)) { + byFile.set(issue.filePath, []); + } + byFile.get(issue.filePath).push(issue); + } + + // Process each file + for (const [filePath, fileIssues] of byFile) { + try { + // Read current content + if (!fs.existsSync(filePath)) { + results.errors.push({ filePath, error: 'File not found' }); + continue; + } + + const content = fs.readFileSync(filePath, 'utf8'); + let data; + + // Parse based on file type + if (filePath.endsWith('.json')) { + data = JSON.parse(content); + } else { + // For non-JSON files, skip auto-fix + results.skipped.push(...fileIssues.map(i => ({ + ...i, + reason: 'Non-JSON file - manual fix required' + }))); + continue; + } + + // Apply each fix + let modified = data; + const appliedToFile = []; + + for (const issue of fileIssues) { + try { + // Determine what part of data to fix + if (issue.schemaPath) { + // Fix at specific path in the data + modified = applyAtPath(modified, issue.schemaPath, issue.autoFixFn); + } else { + // Apply to root + modified = issue.autoFixFn(modified); + } + + appliedToFile.push({ + issue: issue.issue, + fix: issue.fix, + filePath + }); + } catch (err) { + results.errors.push({ + issue: issue.issue, + filePath, + error: err.message + }); + } + } + + // Write changes + if (!dryRun && appliedToFile.length > 0) { + // Create backup + if (backup) { + const backupPath = `${filePath}.backup`; + fs.writeFileSync(backupPath, content, 'utf8'); + } + + // Write modified content + const newContent = JSON.stringify(modified, null, 2); + fs.writeFileSync(filePath, newContent, 'utf8'); + } + + results.applied.push(...appliedToFile); + + } catch (err) { + results.errors.push({ + filePath, + error: err.message + }); + } + } + + // Add non-fixable issues to skipped + const nonFixable = issues.filter(i => + i.certainty !== 'HIGH' || !i.autoFixFn + ); + results.skipped.push(...nonFixable.map(i => ({ + ...i, + reason: i.certainty !== 'HIGH' ? 'Not HIGH certainty' : 'No auto-fix available' + }))); + + return results; +} + +/** + * Apply a fix function at a specific path in an object + * @private + */ +function applyAtPath(obj, pathStr, fixFn) { + const parts = pathStr.split('.'); + const result = JSON.parse(JSON.stringify(obj)); // Deep clone + + let current = result; + for (let i = 0; i < parts.length - 1; i++) { + const part = parts[i]; + if (part.includes('[')) { + // Array access + const match = part.match(/(\w+)\[(\d+)\]/); + if (match) { + current = current[match[1]][parseInt(match[2])]; + } + } else { + current = current[part]; + } + } + + const lastPart = parts[parts.length - 1]; + if (lastPart.includes('[')) { + const match = lastPart.match(/(\w+)\[(\d+)\]/); + if (match) { + current[match[1]][parseInt(match[2])] = fixFn(current[match[1]][parseInt(match[2])]); + } + } else { + current[lastPart] = fixFn(current[lastPart]); + } + + return result; +} + +/** + * Fix missing additionalProperties in a schema + * @param {Object} schema - JSON Schema object + * @returns {Object} Fixed schema + */ +function fixAdditionalProperties(schema) { + if (!schema || typeof schema !== 'object') return schema; + + const fixed = { ...schema }; + + if (fixed.type === 'object' && fixed.properties) { + fixed.additionalProperties = false; + } + + // Recursively fix nested schemas + if (fixed.properties) { + fixed.properties = {}; + for (const [key, value] of Object.entries(schema.properties)) { + fixed.properties[key] = fixAdditionalProperties(value); + } + } + + return fixed; +} + +/** + * Fix missing required array + * @param {Object} schema - JSON Schema object + * @returns {Object} Fixed schema + */ +function fixRequiredFields(schema) { + if (!schema || typeof schema !== 'object') return schema; + + const fixed = { ...schema }; + + if (fixed.type === 'object' && fixed.properties && !fixed.required) { + // Add all non-optional fields to required + fixed.required = Object.entries(fixed.properties) + .filter(([_, prop]) => { + // Skip if has default or marked optional in description + if (prop.default !== undefined) return false; + if (prop.description && /optional/i.test(prop.description)) return false; + return true; + }) + .map(([key]) => key); + } + + return fixed; +} + +/** + * Fix version mismatch by syncing to package.json version + * @param {Object} pluginJson - Plugin JSON object + * @param {string} targetVersion - Version to sync to + * @returns {Object} Fixed plugin JSON + */ +function fixVersionMismatch(pluginJson, targetVersion) { + return { + ...pluginJson, + version: targetVersion + }; +} + +/** + * Generate a fix preview without applying + * @param {Array} issues - Issues to preview + * @returns {Array} Preview of changes + */ +function previewFixes(issues) { + const previews = []; + + for (const issue of issues) { + if (issue.certainty === 'HIGH' && issue.autoFixFn) { + previews.push({ + filePath: issue.filePath, + issue: issue.issue, + fix: issue.fix, + willApply: true + }); + } else { + previews.push({ + filePath: issue.filePath, + issue: issue.issue, + fix: issue.fix || 'No auto-fix available', + willApply: false, + reason: issue.certainty !== 'HIGH' ? 'Not HIGH certainty' : 'No auto-fix function' + }); + } + } + + return previews; +} + +/** + * Restore from backup + * @param {string} filePath - Path to file to restore + * @returns {boolean} True if restored successfully + */ +function restoreFromBackup(filePath) { + const backupPath = `${filePath}.backup`; + + if (!fs.existsSync(backupPath)) { + return false; + } + + const backupContent = fs.readFileSync(backupPath, 'utf8'); + fs.writeFileSync(filePath, backupContent, 'utf8'); + fs.unlinkSync(backupPath); + + return true; +} + +/** + * Clean up backup files + * @param {string} directory - Directory to clean + * @returns {number} Number of backups removed + */ +function cleanupBackups(directory) { + let count = 0; + + const files = fs.readdirSync(directory, { recursive: true }); + for (const file of files) { + if (file.endsWith('.backup')) { + const fullPath = path.join(directory, file); + try { + const stat = fs.statSync(fullPath); + if (stat.isFile()) { + fs.unlinkSync(fullPath); + count++; + } + } catch (err) { + // File may have been removed already + } + } + } + + return count; +} + +module.exports = { + applyFixes, + fixAdditionalProperties, + fixRequiredFields, + fixVersionMismatch, + previewFixes, + restoreFromBackup, + cleanupBackups +}; diff --git a/lib/enhance/index.js b/lib/enhance/index.js new file mode 100644 index 00000000..8e75ca57 --- /dev/null +++ b/lib/enhance/index.js @@ -0,0 +1,35 @@ +/** + * Enhance Library + * Plugin structure and tool use analyzer + * + * @author Avi Fenesh + * @license MIT + */ + +const pluginAnalyzer = require('./plugin-analyzer'); +const pluginPatterns = require('./plugin-patterns'); +const toolPatterns = require('./tool-patterns'); +const securityPatterns = require('./security-patterns'); +const reporter = require('./reporter'); +const fixer = require('./fixer'); + +module.exports = { + // Main analyzer + pluginAnalyzer, + + // Pattern modules + pluginPatterns, + toolPatterns, + securityPatterns, + + // Output modules + reporter, + fixer, + + // Convenience exports + analyze: pluginAnalyzer.analyze, + analyzePlugin: pluginAnalyzer.analyzePlugin, + analyzeAllPlugins: pluginAnalyzer.analyzeAllPlugins, + applyFixes: pluginAnalyzer.applyFixes, + generateReport: pluginAnalyzer.generateReport +}; diff --git a/lib/enhance/plugin-analyzer.js b/lib/enhance/plugin-analyzer.js new file mode 100644 index 00000000..d0819033 --- /dev/null +++ b/lib/enhance/plugin-analyzer.js @@ -0,0 +1,376 @@ +/** + * Plugin Analyzer + * Main orchestrator for plugin structure and tool use analysis + * + * @author Avi Fenesh + * @license MIT + */ + +const fs = require('fs'); +const path = require('path'); +const pluginPatterns = require('./plugin-patterns'); +const toolPatterns = require('./tool-patterns'); +const securityPatterns = require('./security-patterns'); +const reporter = require('./reporter'); +const fixer = require('./fixer'); + +/** + * Analyze a single plugin + * @param {string} pluginPath - Path to plugin directory + * @param {Object} options - Analysis options + * @param {boolean} options.verbose - Include LOW certainty issues + * @returns {Object} Analysis results + */ +async function analyzePlugin(pluginPath, options = {}) { + const results = { + pluginName: path.basename(pluginPath), + pluginPath, + filesScanned: 0, + toolIssues: [], + structureIssues: [], + securityIssues: [] + }; + + // Find plugin.json + const pluginJsonPath = path.join(pluginPath, '.claude-plugin', 'plugin.json'); + const altPluginJsonPath = path.join(pluginPath, 'plugin.json'); + + let pluginJson = null; + let pluginJsonFile = null; + + if (fs.existsSync(pluginJsonPath)) { + try { + pluginJson = JSON.parse(fs.readFileSync(pluginJsonPath, 'utf8')); + pluginJsonFile = pluginJsonPath; + results.filesScanned++; + } catch (err) { + results.structureIssues.push({ + issue: 'Failed to parse plugin.json', + file: pluginJsonPath, + detail: err.message, + certainty: 'HIGH', + patternId: 'malformed_plugin_json' + }); + } + } else if (fs.existsSync(altPluginJsonPath)) { + try { + pluginJson = JSON.parse(fs.readFileSync(altPluginJsonPath, 'utf8')); + pluginJsonFile = altPluginJsonPath; + results.filesScanned++; + } catch (err) { + results.structureIssues.push({ + issue: 'Failed to parse plugin.json', + file: altPluginJsonPath, + detail: err.message, + certainty: 'HIGH', + patternId: 'malformed_plugin_json' + }); + } + } + + // Check package.json for version comparison + const packageJsonPath = path.join(pluginPath, '..', '..', 'package.json'); + let packageJson = null; + if (fs.existsSync(packageJsonPath)) { + try { + packageJson = JSON.parse(fs.readFileSync(packageJsonPath, 'utf8')); + } catch (err) { + // Non-critical - version comparison will just be skipped + } + } + + // Analyze plugin.json structure + if (pluginJson) { + results.pluginName = pluginJson.name || results.pluginName; + + // Check required fields + const reqFieldsPattern = pluginPatterns.pluginPatterns.missing_required_plugin_fields; + const reqResult = reqFieldsPattern.check(pluginJson); + if (reqResult) { + results.structureIssues.push({ + ...reqResult, + file: pluginJsonFile, + certainty: reqFieldsPattern.certainty, + patternId: reqFieldsPattern.id + }); + } + + // Check version format + const versionPattern = pluginPatterns.pluginPatterns.invalid_version_format; + const versionResult = versionPattern.check(pluginJson); + if (versionResult) { + results.structureIssues.push({ + ...versionResult, + file: pluginJsonFile, + certainty: versionPattern.certainty, + patternId: versionPattern.id + }); + } + + // Check version mismatch + if (packageJson) { + const mismatchPattern = pluginPatterns.pluginPatterns.version_mismatch; + const mismatchResult = mismatchPattern.check(pluginJson, packageJson); + if (mismatchResult) { + results.structureIssues.push({ + ...mismatchResult, + file: pluginJsonFile, + filePath: pluginJsonFile, + certainty: mismatchPattern.certainty, + patternId: mismatchPattern.id, + autoFixFn: (pj) => fixer.fixVersionMismatch(pj, packageJson.version) + }); + } + } + + // Check tool overexposure + const overexposurePattern = pluginPatterns.pluginPatterns.tool_overexposure; + const overexposureResult = overexposurePattern.check(pluginJson); + if (overexposureResult && (options.verbose || overexposurePattern.certainty !== 'LOW')) { + results.structureIssues.push({ + ...overexposureResult, + file: pluginJsonFile, + certainty: overexposurePattern.certainty, + patternId: overexposurePattern.id + }); + } + + // Analyze commands + if (pluginJson.commands) { + for (let idx = 0; idx < pluginJson.commands.length; idx++) { + const cmd = pluginJson.commands[idx]; + const cmdIssues = analyzeCommand(cmd, pluginJsonFile, idx); + results.toolIssues.push(...cmdIssues); + } + } + } + + // Analyze agent files + const agentsDir = path.join(pluginPath, 'agents'); + if (fs.existsSync(agentsDir)) { + const agentFiles = fs.readdirSync(agentsDir).filter(f => f.endsWith('.md')); + + for (const agentFile of agentFiles) { + const agentPath = path.join(agentsDir, agentFile); + const content = fs.readFileSync(agentPath, 'utf8'); + results.filesScanned++; + + // Security checks + const secIssues = securityPatterns.checkSecurity(content, agentPath); + results.securityIssues.push(...secIssues.map(i => ({ + ...i, + file: agentPath + }))); + } + } + + // Analyze command files + const commandsDir = path.join(pluginPath, 'commands'); + if (fs.existsSync(commandsDir)) { + const commandFiles = fs.readdirSync(commandsDir).filter(f => f.endsWith('.md')); + + for (const cmdFile of commandFiles) { + const cmdPath = path.join(commandsDir, cmdFile); + const content = fs.readFileSync(cmdPath, 'utf8'); + results.filesScanned++; + + // Security checks + const secIssues = securityPatterns.checkSecurity(content, cmdPath); + results.securityIssues.push(...secIssues.map(i => ({ + ...i, + file: cmdPath + }))); + } + } + + return results; +} + +/** + * Analyze a command definition + * @private + * @param {Object} cmd - Command definition + * @param {string} filePath - Path to plugin.json + * @param {number} cmdIndex - Index of command in commands array + */ +function analyzeCommand(cmd, filePath, cmdIndex) { + const issues = []; + + // Check description + const descPattern = pluginPatterns.pluginPatterns.missing_tool_description; + const descResult = descPattern.check(cmd); + if (descResult) { + issues.push({ + ...descResult, + tool: cmd.name, + file: filePath, + certainty: descPattern.certainty, + patternId: descPattern.id + }); + } + + // Check parameters schema + if (cmd.parameters) { + // Missing additionalProperties + const addPropsPattern = pluginPatterns.pluginPatterns.missing_additional_properties; + const addPropsResult = addPropsPattern.check(cmd.parameters); + if (addPropsResult) { + issues.push({ + ...addPropsResult, + tool: cmd.name, + file: filePath, + filePath: filePath, + schemaPath: `commands[${cmdIndex}].parameters`, + certainty: addPropsPattern.certainty, + patternId: addPropsPattern.id, + autoFixFn: fixer.fixAdditionalProperties + }); + } + + // Missing required + const reqPattern = pluginPatterns.pluginPatterns.missing_required_fields; + const reqResult = reqPattern.check(cmd.parameters); + if (reqResult) { + issues.push({ + ...reqResult, + tool: cmd.name, + file: filePath, + filePath: filePath, + schemaPath: `commands[${cmdIndex}].parameters`, + certainty: reqPattern.certainty, + patternId: reqPattern.id, + autoFixFn: fixer.fixRequiredFields + }); + } + + // Deep nesting + const nestPattern = pluginPatterns.pluginPatterns.deep_nesting; + const nestResult = nestPattern.check(cmd.parameters); + if (nestResult) { + issues.push({ + ...nestResult, + tool: cmd.name, + file: filePath, + certainty: nestPattern.certainty, + patternId: nestPattern.id + }); + } + + // Run tool pattern checks + const toolIssues = toolPatterns.analyzeTool({ + name: cmd.name, + description: cmd.description, + inputSchema: cmd.parameters + }); + issues.push(...toolIssues.map(i => ({ + ...i, + file: filePath + }))); + } + + return issues; +} + +/** + * Analyze all plugins in a directory + * @param {string} pluginsDir - Path to plugins directory + * @param {Object} options - Analysis options + * @returns {Array} Array of analysis results + */ +async function analyzeAllPlugins(pluginsDir, options = {}) { + const results = []; + + if (!fs.existsSync(pluginsDir)) { + return results; + } + + const pluginDirs = fs.readdirSync(pluginsDir, { withFileTypes: true }) + .filter(d => d.isDirectory()) + .map(d => d.name); + + for (const pluginName of pluginDirs) { + const pluginPath = path.join(pluginsDir, pluginName); + const result = await analyzePlugin(pluginPath, options); + results.push(result); + } + + return results; +} + +/** + * Main analyze function + * @param {Object} options - Analysis options + * @param {string} options.plugin - Specific plugin name (optional) + * @param {string} options.pluginsDir - Path to plugins directory + * @param {boolean} options.verbose - Include LOW certainty issues + * @returns {Object} Analysis results + */ +async function analyze(options = {}) { + const { + plugin, + pluginsDir = 'plugins', + verbose = false + } = options; + + if (plugin) { + // Analyze single plugin + const pluginPath = path.join(pluginsDir, plugin); + return analyzePlugin(pluginPath, { verbose }); + } else { + // Analyze all plugins + return analyzeAllPlugins(pluginsDir, { verbose }); + } +} + +/** + * Apply fixes to analysis results + * @param {Object|Array} results - Analysis results + * @param {Object} options - Fix options + * @returns {Object} Fix results + */ +async function applyFixes(results, options = {}) { + // Collect all issues + let allIssues = []; + + if (Array.isArray(results)) { + for (const r of results) { + allIssues.push(...(r.toolIssues || [])); + allIssues.push(...(r.structureIssues || [])); + allIssues.push(...(r.securityIssues || [])); + } + } else { + allIssues.push(...(results.toolIssues || [])); + allIssues.push(...(results.structureIssues || [])); + allIssues.push(...(results.securityIssues || [])); + } + + return fixer.applyFixes(allIssues, options); +} + +/** + * Generate report from analysis results + * @param {Object|Array} results - Analysis results + * @param {Object} options - Report options + * @returns {string} Markdown report + */ +function generateReport(results, options = {}) { + if (Array.isArray(results)) { + return reporter.generateSummaryReport(results, options); + } else { + return reporter.generateReport(results, options); + } +} + +module.exports = { + analyze, + analyzePlugin, + analyzeAllPlugins, + applyFixes, + generateReport, + // Re-export sub-modules + pluginPatterns: pluginPatterns.pluginPatterns, + toolPatterns: toolPatterns.toolPatterns, + securityPatterns: securityPatterns.securityPatterns, + reporter, + fixer +}; diff --git a/lib/enhance/plugin-patterns.js b/lib/enhance/plugin-patterns.js new file mode 100644 index 00000000..22a87b85 --- /dev/null +++ b/lib/enhance/plugin-patterns.js @@ -0,0 +1,326 @@ +/** + * Plugin Structure Patterns + * Detection patterns for plugin.json and structure issues + * + * @author Avi Fenesh + * @license MIT + */ + +/** + * Plugin structure patterns with certainty levels + * Following the slop-patterns model + */ +const pluginPatterns = { + /** + * Missing additionalProperties in schema + * HIGH certainty - always fixable + */ + missing_additional_properties: { + id: 'missing_additional_properties', + category: 'tool', + certainty: 'HIGH', + autoFix: true, + description: 'Schema missing additionalProperties: false', + check: (schema) => { + if (!schema || typeof schema !== 'object') return null; + if (schema.type === 'object' && schema.properties) { + if (schema.additionalProperties !== false) { + return { + issue: 'Missing additionalProperties: false', + fix: 'Add "additionalProperties": false to schema', + autoFixFn: (s) => ({ ...s, additionalProperties: false }) + }; + } + } + return null; + } + }, + + /** + * Missing required array in schema + * HIGH certainty - fixable by adding all properties + */ + missing_required_fields: { + id: 'missing_required_fields', + category: 'tool', + certainty: 'HIGH', + autoFix: true, + description: 'Schema missing required field declarations', + check: (schema) => { + if (!schema || typeof schema !== 'object') return null; + if (schema.type === 'object' && schema.properties) { + const propKeys = Object.keys(schema.properties); + if (propKeys.length > 0 && (!schema.required || schema.required.length === 0)) { + return { + issue: 'No required fields declared', + fix: 'Add required array with all mandatory fields' + // autoFixFn is provided by plugin-analyzer which uses fixer.fixRequiredFields + }; + } + } + return null; + } + }, + + /** + * Version mismatch between plugin.json and package.json + * HIGH certainty - fixable by syncing + */ + version_mismatch: { + id: 'version_mismatch', + category: 'structure', + certainty: 'HIGH', + autoFix: true, + description: 'Version mismatch between plugin.json and package.json', + check: (pluginJson, packageJson) => { + if (!pluginJson || !packageJson) return null; + if (pluginJson.version !== packageJson.version) { + return { + issue: `Version mismatch: plugin.json (${pluginJson.version}) vs package.json (${packageJson.version})`, + fix: 'Sync versions', + autoFixFn: (pj) => ({ ...pj, version: packageJson.version }) + }; + } + return null; + } + }, + + /** + * Missing tool description + * HIGH certainty - must have description + */ + missing_tool_description: { + id: 'missing_tool_description', + category: 'tool', + certainty: 'HIGH', + autoFix: false, + description: 'Tool definition missing description', + check: (tool) => { + if (!tool || typeof tool !== 'object') return null; + if (!tool.description || tool.description.trim() === '') { + return { + issue: 'Missing tool description', + fix: 'Add descriptive description field' + }; + } + return null; + } + }, + + /** + * Deeply nested parameter structure + * MEDIUM certainty - may be intentional + */ + deep_nesting: { + id: 'deep_nesting', + category: 'tool', + certainty: 'MEDIUM', + autoFix: false, + description: 'Parameter schema too deeply nested (>2 levels)', + maxDepth: 2, + check: (schema, depth = 0) => { + if (!schema || typeof schema !== 'object') return null; + if (depth > 2) { + return { + issue: `Schema nested ${depth} levels deep (max: 2)`, + fix: 'Flatten parameter structure' + }; + } + // Check nested properties + if (schema.properties) { + for (const prop of Object.values(schema.properties)) { + const nested = pluginPatterns.deep_nesting.check(prop, depth + 1); + if (nested) return nested; + } + } + return null; + } + }, + + /** + * Tool description too long + * MEDIUM certainty - affects token efficiency + */ + long_description: { + id: 'long_description', + category: 'tool', + certainty: 'MEDIUM', + autoFix: false, + description: 'Tool description exceeds 500 characters', + maxLength: 500, + check: (tool) => { + if (!tool || typeof tool !== 'object') return null; + if (tool.description && tool.description.length > 500) { + return { + issue: `Description too long (${tool.description.length} chars, max: 500)`, + fix: 'Shorten description for token efficiency' + }; + } + return null; + } + }, + + /** + * Missing parameter descriptions + * MEDIUM certainty - improves clarity + */ + missing_param_description: { + id: 'missing_param_description', + category: 'tool', + certainty: 'MEDIUM', + autoFix: false, + description: 'Parameter missing description', + check: (schema) => { + if (!schema || !schema.properties) return null; + const missing = []; + for (const [name, prop] of Object.entries(schema.properties)) { + if (!prop.description || prop.description.trim() === '') { + missing.push(name); + } + } + if (missing.length > 0) { + return { + issue: `Parameters missing descriptions: ${missing.join(', ')}`, + fix: 'Add descriptions to all parameters' + }; + } + return null; + } + }, + + /** + * Too many tools in plugin + * LOW certainty - advisory + */ + tool_overexposure: { + id: 'tool_overexposure', + category: 'structure', + certainty: 'LOW', + autoFix: false, + description: 'Plugin exposes many tools (consider splitting)', + maxTools: 10, + check: (pluginJson) => { + if (!pluginJson) return null; + const toolCount = (pluginJson.commands?.length || 0) + (pluginJson.agents?.length || 0); + if (toolCount > 10) { + return { + issue: `Plugin has ${toolCount} tools/commands (consider splitting)`, + fix: 'Consider splitting into multiple focused plugins' + }; + } + return null; + } + }, + + /** + * Missing required plugin.json fields + * HIGH certainty + */ + missing_required_plugin_fields: { + id: 'missing_required_plugin_fields', + category: 'structure', + certainty: 'HIGH', + autoFix: false, + description: 'Plugin.json missing required fields', + requiredFields: ['name', 'version', 'description'], + check: (pluginJson) => { + if (!pluginJson) return null; + const missing = []; + for (const field of pluginPatterns.missing_required_plugin_fields.requiredFields) { + if (!pluginJson[field]) { + missing.push(field); + } + } + if (missing.length > 0) { + return { + issue: `Missing required fields: ${missing.join(', ')}`, + fix: 'Add required fields to plugin.json' + }; + } + return null; + } + }, + + /** + * Invalid version format + * HIGH certainty + */ + invalid_version_format: { + id: 'invalid_version_format', + category: 'structure', + certainty: 'HIGH', + autoFix: false, + description: 'Version does not follow semver format', + check: (pluginJson) => { + if (!pluginJson || !pluginJson.version) return null; + const semverRegex = /^\d+\.\d+\.\d+(-[a-zA-Z0-9.]+)?(\+[a-zA-Z0-9.]+)?$/; + if (!semverRegex.test(pluginJson.version)) { + return { + issue: `Invalid version format: ${pluginJson.version}`, + fix: 'Use semver format (e.g., 1.0.0)' + }; + } + return null; + } + } +}; + +/** + * Get all patterns + * @returns {Object} All plugin patterns + */ +function getAllPatterns() { + return pluginPatterns; +} + +/** + * Get patterns by certainty level + * @param {string} certainty - HIGH, MEDIUM, or LOW + * @returns {Object} Filtered patterns + */ +function getPatternsByCertainty(certainty) { + const result = {}; + for (const [name, pattern] of Object.entries(pluginPatterns)) { + if (pattern.certainty === certainty) { + result[name] = pattern; + } + } + return result; +} + +/** + * Get patterns by category + * @param {string} category - tool, structure, security + * @returns {Object} Filtered patterns + */ +function getPatternsByCategory(category) { + const result = {}; + for (const [name, pattern] of Object.entries(pluginPatterns)) { + if (pattern.category === category) { + result[name] = pattern; + } + } + return result; +} + +/** + * Get auto-fixable patterns + * @returns {Object} Patterns with autoFix: true + */ +function getAutoFixablePatterns() { + const result = {}; + for (const [name, pattern] of Object.entries(pluginPatterns)) { + if (pattern.autoFix) { + result[name] = pattern; + } + } + return result; +} + +module.exports = { + pluginPatterns, + getAllPatterns, + getPatternsByCertainty, + getPatternsByCategory, + getAutoFixablePatterns +}; diff --git a/lib/enhance/reporter.js b/lib/enhance/reporter.js new file mode 100644 index 00000000..d41f28cb --- /dev/null +++ b/lib/enhance/reporter.js @@ -0,0 +1,264 @@ +/** + * Plugin Analysis Reporter + * Generates markdown reports for plugin analysis results + * + * @author Avi Fenesh + * @license MIT + */ + +/** + * Generate a markdown report from analysis results + * @param {Object} results - Analysis results + * @param {string} results.pluginName - Name of analyzed plugin + * @param {Array} results.toolIssues - Tool definition issues + * @param {Array} results.structureIssues - Plugin structure issues + * @param {Array} results.securityIssues - Security issues + * @param {Object} options - Report options + * @param {boolean} options.verbose - Include LOW certainty issues + * @param {boolean} options.compact - Use compact format + * @returns {string} Markdown report + */ +function generateReport(results, options = {}) { + const { verbose = false, compact = false } = options; + + // Filter issues by certainty + const filterIssues = (issues) => { + if (verbose) return issues; + return issues.filter(i => i.certainty !== 'LOW'); + }; + + const toolIssues = filterIssues(results.toolIssues || []); + const structureIssues = filterIssues(results.structureIssues || []); + const securityIssues = filterIssues(results.securityIssues || []); + + const totalIssues = toolIssues.length + structureIssues.length + securityIssues.length; + + if (compact) { + return generateCompactReport(results.pluginName, toolIssues, structureIssues, securityIssues); + } + + const lines = []; + + // Header + lines.push(`## Plugin Analysis: ${results.pluginName}`); + lines.push(''); + lines.push(`**Analyzed**: ${new Date().toISOString()}`); + lines.push(`**Files scanned**: ${results.filesScanned || 0}`); + lines.push(''); + + // Summary + lines.push('### Summary'); + lines.push(''); + + const highCount = countByCertainty([...toolIssues, ...structureIssues, ...securityIssues], 'HIGH'); + const mediumCount = countByCertainty([...toolIssues, ...structureIssues, ...securityIssues], 'MEDIUM'); + const lowCount = verbose ? countByCertainty([...toolIssues, ...structureIssues, ...securityIssues], 'LOW') : 0; + + lines.push(`| Certainty | Count |`); + lines.push(`|-----------|-------|`); + lines.push(`| HIGH | ${highCount} |`); + lines.push(`| MEDIUM | ${mediumCount} |`); + if (verbose) { + lines.push(`| LOW | ${lowCount} |`); + } + lines.push(`| **Total** | **${totalIssues}** |`); + lines.push(''); + + // Tool Issues + if (toolIssues.length > 0) { + lines.push(`### Tool Definitions (${toolIssues.length} issues)`); + lines.push(''); + lines.push('| Tool | Issue | Fix | Certainty |'); + lines.push('|------|-------|-----|-----------|'); + for (const issue of toolIssues) { + lines.push(`| ${issue.tool || '-'} | ${issue.issue} | ${issue.fix || '-'} | ${issue.certainty} |`); + } + lines.push(''); + } + + // Structure Issues + if (structureIssues.length > 0) { + lines.push(`### Structure (${structureIssues.length} issues)`); + lines.push(''); + lines.push('| File | Issue | Certainty |'); + lines.push('|------|-------|-----------|'); + for (const issue of structureIssues) { + lines.push(`| ${issue.file || '-'} | ${issue.issue} | ${issue.certainty} |`); + } + lines.push(''); + } + + // Security Issues + if (securityIssues.length > 0) { + lines.push(`### Security (${securityIssues.length} issues)`); + lines.push(''); + lines.push('| File | Line | Issue | Certainty |'); + lines.push('|------|------|-------|-----------|'); + for (const issue of securityIssues) { + lines.push(`| ${issue.file || '-'} | ${issue.line || '-'} | ${issue.issue} | ${issue.certainty} |`); + } + lines.push(''); + } + + // No issues + if (totalIssues === 0) { + lines.push('No issues found.'); + lines.push(''); + } + + return lines.join('\n'); +} + +/** + * Generate a compact report format + * @private + */ +function generateCompactReport(pluginName, toolIssues, structureIssues, securityIssues) { + const lines = []; + + lines.push(`## ${pluginName}: ${toolIssues.length + structureIssues.length + securityIssues.length} issues`); + lines.push(''); + + const allIssues = [ + ...toolIssues.map(i => ({ ...i, category: 'Tool' })), + ...structureIssues.map(i => ({ ...i, category: 'Structure' })), + ...securityIssues.map(i => ({ ...i, category: 'Security' })) + ]; + + // Sort by certainty (HIGH first) + const certOrder = { HIGH: 0, MEDIUM: 1, LOW: 2 }; + allIssues.sort((a, b) => certOrder[a.certainty] - certOrder[b.certainty]); + + if (allIssues.length > 0) { + lines.push('| Category | Issue | Certainty |'); + lines.push('|----------|-------|-----------|'); + for (const issue of allIssues) { + lines.push(`| ${issue.category} | ${issue.issue} | ${issue.certainty} |`); + } + } else { + lines.push('No issues found.'); + } + + return lines.join('\n'); +} + +/** + * Count issues by certainty level + * @private + */ +function countByCertainty(issues, certainty) { + return issues.filter(i => i.certainty === certainty).length; +} + +/** + * Generate a diff display for a fix + * @param {string} original - Original content + * @param {string} modified - Modified content + * @param {string} filePath - File path + * @returns {string} Diff display + */ +function generateDiff(original, modified, filePath) { + const lines = []; + + lines.push(`\`\`\`diff`); + lines.push(`--- a/${filePath}`); + lines.push(`+++ b/${filePath}`); + + const origLines = original.split('\n'); + const modLines = modified.split('\n'); + + // Simple line-by-line diff (for demo - real implementation would use proper diff algorithm) + const maxLines = Math.max(origLines.length, modLines.length); + for (let i = 0; i < maxLines; i++) { + const origLine = origLines[i]; + const modLine = modLines[i]; + + if (origLine === modLine) { + if (origLine !== undefined) { + lines.push(` ${origLine}`); + } + } else { + if (origLine !== undefined) { + lines.push(`-${origLine}`); + } + if (modLine !== undefined) { + lines.push(`+${modLine}`); + } + } + } + + lines.push(`\`\`\``); + + return lines.join('\n'); +} + +/** + * Generate a summary report for multiple plugins + * @param {Array} allResults - Array of plugin analysis results + * @param {Object} options - Report options + * @returns {string} Summary markdown report + */ +function generateSummaryReport(allResults, options = {}) { + const lines = []; + + lines.push('# Plugin Analysis Summary'); + lines.push(''); + lines.push(`**Analyzed**: ${allResults.length} plugins`); + lines.push(`**Date**: ${new Date().toISOString()}`); + lines.push(''); + + // Overall stats + let totalHigh = 0; + let totalMedium = 0; + let totalLow = 0; + + for (const result of allResults) { + const allIssues = [ + ...(result.toolIssues || []), + ...(result.structureIssues || []), + ...(result.securityIssues || []) + ]; + totalHigh += countByCertainty(allIssues, 'HIGH'); + totalMedium += countByCertainty(allIssues, 'MEDIUM'); + totalLow += countByCertainty(allIssues, 'LOW'); + } + + lines.push('## Overall'); + lines.push(''); + lines.push('| Certainty | Count |'); + lines.push('|-----------|-------|'); + lines.push(`| HIGH | ${totalHigh} |`); + lines.push(`| MEDIUM | ${totalMedium} |`); + if (options.verbose) { + lines.push(`| LOW | ${totalLow} |`); + } + lines.push(''); + + // Per-plugin summary + lines.push('## By Plugin'); + lines.push(''); + lines.push('| Plugin | HIGH | MEDIUM | LOW | Total |'); + lines.push('|--------|------|--------|-----|-------|'); + + for (const result of allResults) { + const allIssues = [ + ...(result.toolIssues || []), + ...(result.structureIssues || []), + ...(result.securityIssues || []) + ]; + const h = countByCertainty(allIssues, 'HIGH'); + const m = countByCertainty(allIssues, 'MEDIUM'); + const l = countByCertainty(allIssues, 'LOW'); + lines.push(`| ${result.pluginName} | ${h} | ${m} | ${l} | ${h + m + l} |`); + } + + lines.push(''); + + return lines.join('\n'); +} + +module.exports = { + generateReport, + generateDiff, + generateSummaryReport +}; diff --git a/lib/enhance/security-patterns.js b/lib/enhance/security-patterns.js new file mode 100644 index 00000000..615c6ce6 --- /dev/null +++ b/lib/enhance/security-patterns.js @@ -0,0 +1,284 @@ +/** + * Security Patterns + * Detection patterns for security vulnerabilities in plugins + * + * @author Avi Fenesh + * @license MIT + */ + +/** + * Security patterns with certainty levels + */ +const securityPatterns = { + /** + * Unrestricted Bash tool access + * HIGH certainty - security risk + */ + unrestricted_bash: { + id: 'unrestricted_bash', + category: 'security', + certainty: 'HIGH', + autoFix: false, + description: 'Agent has unrestricted Bash tool access', + pattern: /^tools:\s*.*\bBash\b(?!\s*\()/m, + check: (content, filePath) => { + // Check for Bash without restrictions in agent frontmatter + const frontmatterMatch = content.match(/^---\n([\s\S]*?)\n---/); + if (!frontmatterMatch) return null; + + const frontmatter = frontmatterMatch[1]; + const toolsMatch = frontmatter.match(/^tools:\s*(.*)$/m); + if (!toolsMatch) return null; + + const tools = toolsMatch[1]; + // Bash without parentheses means unrestricted + if (/\bBash\b(?!\s*\()/.test(tools)) { + return { + issue: 'Unrestricted Bash access', + fix: 'Add restrictions like Bash(git:*) or Bash(npm:*)', + line: content.substring(0, frontmatterMatch.index + frontmatterMatch[0].indexOf(toolsMatch[0])).split('\n').length + }; + } + return null; + } + }, + + /** + * Command injection via string interpolation + * HIGH certainty - dangerous pattern + */ + command_injection: { + id: 'command_injection', + category: 'security', + certainty: 'HIGH', + autoFix: false, + description: 'Potential command injection via string interpolation', + pattern: /\$\{[^}]*\}/, + check: (content, filePath) => { + const issues = []; + const lines = content.split('\n'); + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + // Look for shell commands with interpolation + if (/(?:exec|spawn|system|shell|`|Bash)\s*[(`].*\$\{/.test(line)) { + issues.push({ + issue: 'Command injection risk via string interpolation', + fix: 'Validate and escape user input before shell execution', + line: i + 1 + }); + } + } + + return issues.length > 0 ? issues : null; + } + }, + + /** + * Path traversal patterns + * HIGH certainty - security risk + */ + path_traversal: { + id: 'path_traversal', + category: 'security', + certainty: 'HIGH', + autoFix: false, + description: 'Potential path traversal vulnerability', + pattern: /\.\.\//, + check: (content, filePath) => { + const issues = []; + const lines = content.split('\n'); + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + // Look for user-controlled paths with ../ + if (/(?:path|file|dir).*\$.*\.\.\/|\.\.\/.*\$/.test(line)) { + issues.push({ + issue: 'Path traversal risk - user input may contain ../', + fix: 'Validate paths and use path.resolve() with base directory check', + line: i + 1 + }); + } + } + + return issues.length > 0 ? issues : null; + } + }, + + /** + * Hardcoded secrets in agent files + * HIGH certainty - critical + */ + hardcoded_secrets: { + id: 'hardcoded_secrets', + category: 'security', + certainty: 'HIGH', + autoFix: false, + description: 'Potential hardcoded secrets', + pattern: /(api[_-]?key|secret|token|password|credential)\s*[:=]\s*["'`][^"'`\s]{8,}["'`]/i, + check: (content, filePath) => { + const issues = []; + const lines = content.split('\n'); + const secretPattern = /(api[_-]?key|secret|token|password|credential)\s*[:=]\s*["'`](?!\$\{)(?!\{\{)[^"'`\s]{8,}["'`]/i; + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + if (secretPattern.test(line)) { + issues.push({ + issue: 'Potential hardcoded secret', + fix: 'Use environment variables instead', + line: i + 1 + }); + } + } + + return issues.length > 0 ? issues : null; + } + }, + + /** + * Missing input validation + * MEDIUM certainty - may be intentional + */ + missing_input_validation: { + id: 'missing_input_validation', + category: 'security', + certainty: 'MEDIUM', + autoFix: false, + description: 'User input used without validation', + check: (content, filePath) => { + const issues = []; + const lines = content.split('\n'); + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + // Look for direct use of ARGUMENTS without validation + if (/\$ARGUMENTS/.test(line) && !/validate|check|verify|sanitize/.test(lines.slice(Math.max(0, i - 5), i + 1).join('\n').toLowerCase())) { + issues.push({ + issue: 'User input ($ARGUMENTS) used without apparent validation', + fix: 'Add input validation before using user-provided arguments', + line: i + 1 + }); + } + } + + return issues.length > 0 ? issues : null; + } + }, + + /** + * Broad file access patterns + * MEDIUM certainty - may be required + */ + broad_file_access: { + id: 'broad_file_access', + category: 'security', + certainty: 'MEDIUM', + autoFix: false, + description: 'Agent requests broad file system access', + check: (content, filePath) => { + const frontmatterMatch = content.match(/^---\n([\s\S]*?)\n---/); + if (!frontmatterMatch) return null; + + const frontmatter = frontmatterMatch[1]; + const toolsMatch = frontmatter.match(/^tools:\s*(.*)$/m); + if (!toolsMatch) return null; + + const tools = toolsMatch[1]; + // Check for Write or Edit without restrictions + if (/\b(Write|Edit)\b(?!\s*\()/.test(tools)) { + return { + issue: 'Broad file write access', + fix: 'Consider restricting to specific directories', + line: content.substring(0, frontmatterMatch.index + frontmatterMatch[0].indexOf(toolsMatch[0])).split('\n').length + }; + } + return null; + } + }, + + /** + * Unsafe eval patterns + * HIGH certainty - dangerous + */ + unsafe_eval: { + id: 'unsafe_eval', + category: 'security', + certainty: 'HIGH', + autoFix: false, + description: 'Unsafe eval() or Function() usage', + pattern: /\b(?:eval|Function)\s*\(/, + check: (content, filePath) => { + const issues = []; + const lines = content.split('\n'); + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + if (/\b(?:eval|Function)\s*\(/.test(line)) { + issues.push({ + issue: 'Unsafe eval() or Function() usage', + fix: 'Avoid dynamic code execution - use safer alternatives', + line: i + 1 + }); + } + } + + return issues.length > 0 ? issues : null; + } + } +}; + +/** + * Get all security patterns + * @returns {Object} All security patterns + */ +function getAllPatterns() { + return securityPatterns; +} + +/** + * Get patterns by certainty level + * @param {string} certainty - HIGH, MEDIUM, or LOW + * @returns {Object} Filtered patterns + */ +function getPatternsByCertainty(certainty) { + const result = {}; + for (const [name, pattern] of Object.entries(securityPatterns)) { + if (pattern.certainty === certainty) { + result[name] = pattern; + } + } + return result; +} + +/** + * Run all security checks on content + * @param {string} content - File content + * @param {string} filePath - File path + * @returns {Array} Array of issues found + */ +function checkSecurity(content, filePath) { + const issues = []; + + for (const [name, pattern] of Object.entries(securityPatterns)) { + if (pattern.check) { + const result = pattern.check(content, filePath); + if (result) { + if (Array.isArray(result)) { + issues.push(...result.map(r => ({ ...r, patternId: pattern.id, certainty: pattern.certainty }))); + } else { + issues.push({ ...result, patternId: pattern.id, certainty: pattern.certainty }); + } + } + } + } + + return issues; +} + +module.exports = { + securityPatterns, + getAllPatterns, + getPatternsByCertainty, + checkSecurity +}; diff --git a/lib/enhance/tool-patterns.js b/lib/enhance/tool-patterns.js new file mode 100644 index 00000000..184f9764 --- /dev/null +++ b/lib/enhance/tool-patterns.js @@ -0,0 +1,373 @@ +/** + * MCP Tool Definition Patterns + * Detection patterns for MCP tool definition issues + * Based on FUNCTION-CALLING-TOOL-USE-REFERENCE.md best practices + * + * @author Avi Fenesh + * @license MIT + */ + +/** + * Tool definition patterns + */ +const toolPatterns = { + /** + * Tool name should be verb + noun + * MEDIUM certainty - naming convention + */ + poor_tool_naming: { + id: 'poor_tool_naming', + category: 'tool', + certainty: 'MEDIUM', + autoFix: false, + description: 'Tool name should follow verb_noun pattern', + check: (tool) => { + if (!tool || !tool.name) return null; + + const name = tool.name; + // Check for common verb prefixes + const verbPrefixes = ['get', 'set', 'create', 'delete', 'update', 'list', 'find', 'search', 'add', 'remove', 'check', 'validate', 'run', 'execute', 'start', 'stop', 'read', 'write']; + const hasVerb = verbPrefixes.some(v => name.toLowerCase().startsWith(v)); + + if (!hasVerb && name.length > 3) { + return { + issue: `Tool name "${name}" doesn't start with a verb`, + fix: 'Rename to verb_noun pattern (e.g., get_user, create_file)' + }; + } + return null; + } + }, + + /** + * Use enums for constrained values + * MEDIUM certainty - improves reliability + */ + missing_enum: { + id: 'missing_enum', + category: 'tool', + certainty: 'MEDIUM', + autoFix: false, + description: 'Parameter with limited options should use enum', + check: (schema) => { + if (!schema || !schema.properties) return null; + + const issues = []; + for (const [name, prop] of Object.entries(schema.properties)) { + // Check for string params that mention specific options in description + if (prop.type === 'string' && prop.description) { + const desc = prop.description.toLowerCase(); + if ((desc.includes('one of') || desc.includes('must be') || desc.includes('can be')) && + !prop.enum) { + issues.push({ + param: name, + issue: `Parameter "${name}" describes options but doesn't use enum`, + fix: 'Add enum array with valid options' + }); + } + } + } + + return issues.length > 0 ? issues : null; + } + }, + + /** + * Flat structure preferred + * MEDIUM certainty - improves LLM reliability + */ + nested_structure: { + id: 'nested_structure', + category: 'tool', + certainty: 'MEDIUM', + autoFix: false, + description: 'Prefer flat parameter structure over nested objects', + maxDepth: 2, + check: (schema, depth = 0) => { + if (!schema || typeof schema !== 'object') return null; + + if (depth > 2) { + return { + issue: `Schema nested ${depth} levels (prefer flat structure)`, + fix: 'Flatten to max 2 levels for LLM reliability' + }; + } + + if (schema.properties) { + for (const [name, prop] of Object.entries(schema.properties)) { + if (prop.type === 'object' && prop.properties) { + const nested = toolPatterns.nested_structure.check(prop, depth + 1); + if (nested) return { ...nested, param: name }; + } + } + } + + return null; + } + }, + + /** + * Parameter format hints + * LOW certainty - improves clarity + */ + missing_format_hints: { + id: 'missing_format_hints', + category: 'tool', + certainty: 'LOW', + autoFix: false, + description: 'Parameter description could include format hints', + check: (schema) => { + if (!schema || !schema.properties) return null; + + const issues = []; + const formatKeywords = ['date', 'time', 'email', 'url', 'path', 'json', 'uuid', 'regex']; + + for (const [name, prop] of Object.entries(schema.properties)) { + if (prop.type === 'string' && prop.description) { + const nameLower = name.toLowerCase(); + const descLower = prop.description.toLowerCase(); + + // Check if name suggests a format but no format/pattern specified + for (const keyword of formatKeywords) { + if ((nameLower.includes(keyword) || descLower.includes(keyword)) && + !prop.format && !prop.pattern) { + issues.push({ + param: name, + issue: `Parameter "${name}" may need format hint`, + fix: `Consider adding format or pattern for ${keyword} validation` + }); + break; + } + } + } + } + + return issues.length > 0 ? issues : null; + } + }, + + /** + * Required parameters should be explicit + * HIGH certainty - prevents ambiguity + */ + implicit_required: { + id: 'implicit_required', + category: 'tool', + certainty: 'HIGH', + autoFix: true, + description: 'Parameters should explicitly declare required status', + check: (schema) => { + if (!schema || !schema.properties) return null; + + const propCount = Object.keys(schema.properties).length; + const requiredCount = schema.required?.length || 0; + + // If properties exist but no required array + if (propCount > 0 && !schema.required) { + return { + issue: 'No required array defined', + fix: 'Add required array listing mandatory parameters', + autoFixFn: (s) => ({ + ...s, + required: Object.keys(s.properties).filter(k => { + const prop = s.properties[k]; + // Consider params required unless they have default or are marked optional + return !prop.default && !/optional/i.test(prop.description || ''); + }) + }) + }; + } + + return null; + } + }, + + /** + * Strict mode recommended + * HIGH certainty - prevents undefined behavior + */ + missing_strict_mode: { + id: 'missing_strict_mode', + category: 'tool', + certainty: 'HIGH', + autoFix: true, + description: 'Schema should use strict mode (additionalProperties: false)', + check: (schema) => { + if (!schema || schema.type !== 'object') return null; + + if (schema.additionalProperties !== false) { + return { + issue: 'Schema not in strict mode', + fix: 'Add additionalProperties: false', + autoFixFn: (s) => ({ ...s, additionalProperties: false }) + }; + } + + return null; + } + }, + + /** + * Description quality check + * LOW certainty - stylistic + */ + poor_description: { + id: 'poor_description', + category: 'tool', + certainty: 'LOW', + autoFix: false, + description: 'Tool description could be more descriptive', + minLength: 20, + check: (tool) => { + if (!tool || !tool.description) return null; + + const desc = tool.description.trim(); + + // Too short + if (desc.length < 20) { + return { + issue: `Description too short (${desc.length} chars)`, + fix: 'Add more detail about what the tool does' + }; + } + + // Just repeats the name + if (tool.name && desc.toLowerCase().replace(/[_-]/g, ' ') === tool.name.toLowerCase().replace(/[_-]/g, ' ')) { + return { + issue: 'Description just repeats tool name', + fix: 'Describe what the tool does, not its name' + }; + } + + return null; + } + }, + + /** + * Redundant tool detection + * LOW certainty - may be intentional + */ + redundant_tools: { + id: 'redundant_tools', + category: 'tool', + certainty: 'LOW', + autoFix: false, + description: 'Potentially redundant tools with similar functionality', + check: (tools) => { + if (!tools || !Array.isArray(tools) || tools.length < 2) return null; + + const issues = []; + const seen = new Map(); + + for (const tool of tools) { + if (!tool.name) continue; + + // Normalize name for comparison + const normalized = tool.name.toLowerCase() + .replace(/[_-]/g, '') + .replace(/^(get|fetch|retrieve|find|search)/, 'get') + .replace(/^(set|update|modify|change)/, 'set') + .replace(/^(create|make|add|new)/, 'create') + .replace(/^(delete|remove|destroy)/, 'delete'); + + if (seen.has(normalized)) { + issues.push({ + tool1: seen.get(normalized), + tool2: tool.name, + issue: `Tools "${seen.get(normalized)}" and "${tool.name}" may be redundant`, + fix: 'Consider consolidating into single tool' + }); + } + seen.set(normalized, tool.name); + } + + return issues.length > 0 ? issues : null; + } + } +}; + +/** + * Get all tool patterns + * @returns {Object} All tool patterns + */ +function getAllPatterns() { + return toolPatterns; +} + +/** + * Get patterns by certainty level + * @param {string} certainty - HIGH, MEDIUM, or LOW + * @returns {Object} Filtered patterns + */ +function getPatternsByCertainty(certainty) { + const result = {}; + for (const [name, pattern] of Object.entries(toolPatterns)) { + if (pattern.certainty === certainty) { + result[name] = pattern; + } + } + return result; +} + +/** + * Analyze a tool definition + * @param {Object} tool - Tool definition object + * @returns {Array} Array of issues found + */ +function analyzeTool(tool) { + const issues = []; + + for (const [name, pattern] of Object.entries(toolPatterns)) { + if (pattern.check && name !== 'redundant_tools') { + const result = pattern.check(tool); + if (result) { + if (Array.isArray(result)) { + issues.push(...result.map(r => ({ ...r, patternId: pattern.id, certainty: pattern.certainty }))); + } else { + issues.push({ ...result, patternId: pattern.id, certainty: pattern.certainty }); + } + } + } + } + + // Check schema separately + if (tool.inputSchema || tool.parameters) { + const schema = tool.inputSchema || tool.parameters; + for (const [name, pattern] of Object.entries(toolPatterns)) { + if (pattern.check && ['nested_structure', 'missing_enum', 'missing_format_hints', 'implicit_required', 'missing_strict_mode'].includes(name)) { + const result = pattern.check(schema); + if (result) { + if (Array.isArray(result)) { + issues.push(...result.map(r => ({ ...r, patternId: pattern.id, certainty: pattern.certainty, tool: tool.name }))); + } else { + issues.push({ ...result, patternId: pattern.id, certainty: pattern.certainty, tool: tool.name }); + } + } + } + } + } + + return issues; +} + +/** + * Check for redundant tools across a set + * @param {Array} tools - Array of tool definitions + * @returns {Array} Array of redundancy issues + */ +function checkRedundancy(tools) { + const pattern = toolPatterns.redundant_tools; + const result = pattern.check(tools); + if (result) { + return result.map(r => ({ ...r, patternId: pattern.id, certainty: pattern.certainty })); + } + return []; +} + +module.exports = { + toolPatterns, + getAllPatterns, + getPatternsByCertainty, + analyzeTool, + checkRedundancy +}; diff --git a/lib/index.js b/lib/index.js index 1e1852a7..7a85f505 100644 --- a/lib/index.js +++ b/lib/index.js @@ -24,6 +24,7 @@ const sourceCache = require('./sources/source-cache'); const customHandler = require('./sources/custom-handler'); const policyQuestions = require('./sources/policy-questions'); const crossPlatform = require('./cross-platform'); +const enhance = require('./enhance'); /** * Platform detection and verification utilities @@ -224,6 +225,7 @@ module.exports = { config, sources, xplat, + enhance, // Direct module access for backward compatibility detectPlatform, diff --git a/plugins/enhance/.claude-plugin/plugin.json b/plugins/enhance/.claude-plugin/plugin.json new file mode 100644 index 00000000..2741afbc --- /dev/null +++ b/plugins/enhance/.claude-plugin/plugin.json @@ -0,0 +1,20 @@ +{ + "name": "enhance", + "version": "2.7.1", + "description": "Plugin structure and tool use analyzer - validates plugin.json, MCP tools, and security patterns", + "author": { + "name": "Avi Fenesh", + "email": "[email protected]", + "url": "https://github.com/avifenesh" + }, + "homepage": "https://github.com/avifenesh/awesome-slash#enhance", + "repository": "https://github.com/avifenesh/awesome-slash", + "license": "MIT", + "keywords": [ + "plugin-analyzer", + "mcp-tools", + "validation", + "security", + "best-practices" + ] +} diff --git a/plugins/enhance/agents/plugin-enhancer.md b/plugins/enhance/agents/plugin-enhancer.md new file mode 100644 index 00000000..c89e0dec --- /dev/null +++ b/plugins/enhance/agents/plugin-enhancer.md @@ -0,0 +1,160 @@ +--- +name: plugin-enhancer +description: Analyze plugin structures and MCP tool definitions +tools: Read, Glob, Grep, Bash(git:*) +model: sonnet +--- + +# Plugin Enhancer Agent + +You analyze Claude Code plugins for structure issues, MCP tool definition problems, and security vulnerabilities. + +## Your Role + +You are a plugin quality analyzer that: +1. Validates plugin.json manifests +2. Checks MCP tool definitions against best practices +3. Identifies security patterns +4. Generates actionable reports +5. Applies auto-fixes for HIGH certainty issues + +## Analysis Categories + +### 1. Plugin Structure Validation + +Check each plugin's `.claude-plugin/plugin.json`: + +```javascript +// Required fields +const requiredFields = ['name', 'version', 'description']; + +// Version format +const versionRegex = /^\d+\.\d+\.\d+$/; + +// Compare with package.json if exists +``` + +### 2. MCP Tool Definition Checks + +For each tool definition, verify: + +**HIGH Certainty (auto-fixable):** +- `additionalProperties: false` in schema +- All parameters in `required` array +- Non-empty `description` field + +**MEDIUM Certainty:** +- Schema depth <= 2 levels +- Description length <= 500 chars +- Parameter descriptions present + +**LOW Certainty:** +- Tool count per plugin (warn if >10) +- Redundant tools + +### 3. Security Pattern Detection + +Scan agent files for: + +**HIGH Certainty:** +- Unrestricted `Bash` tool (no restrictions) +- Command injection patterns: `${...}` in shell commands without validation +- Path traversal: `../` in file operations + +**MEDIUM Certainty:** +- Broad file access patterns +- Missing input validation + +### 4. Agent Configuration Checks + +For each agent markdown file: +- Valid frontmatter (name, description, tools) +- Model specification (haiku, sonnet, opus) +- Tool restrictions properly formatted + +## Output Format + +Generate a markdown report: + +```markdown +## Plugin Analysis: {plugin-name} + +**Analyzed**: {timestamp} +**Files scanned**: {count} + +### Summary +- HIGH: {count} issues +- MEDIUM: {count} issues +- LOW: {count} issues + +### Tool Definitions ({n} issues) + +| Tool | Issue | Fix | Certainty | +|------|-------|-----|-----------| +| {name} | {issue} | {fix} | {level} | + +### Structure ({n} issues) + +| File | Issue | Certainty | +|------|-------|-----------| +| {path} | {issue} | {level} | + +### Security ({n} issues) + +| File | Line | Issue | Certainty | +|------|------|-------|-----------| +| {path} | {line} | {issue} | {level} | +``` + +## Auto-Fix Implementation + +For HIGH certainty issues with available fixes: + +1. **Missing additionalProperties**: + ```javascript + // Add to schema object + schema.additionalProperties = false; + ``` + +2. **Missing required array**: + ```javascript + // Add all properties to required + schema.required = Object.keys(schema.properties); + ``` + +3. **Version mismatch**: + ```javascript + // Sync plugin.json version with package.json + pluginJson.version = packageJson.version; + ``` + +## Workflow + +1. **Discover**: Find all plugins in `plugins/` directory +2. **Load**: Read plugin.json and agent files +3. **Analyze**: Run all pattern checks +4. **Report**: Generate markdown output +5. **Fix**: Apply auto-fixes if requested + +## Example Run + +```bash +# Analyze all plugins +/enhance:plugin + +# Analyze specific plugin +/enhance:plugin next-task + +# Apply fixes +/enhance:plugin --fix + +# Verbose output +/enhance:plugin --verbose +``` + +## Integration Points + +This agent can be invoked by: +- `/enhance:plugin` command +- `review-orchestrator` during PR review +- `delivery-validator` before shipping diff --git a/plugins/enhance/commands/enhance.md b/plugins/enhance/commands/enhance.md new file mode 100644 index 00000000..85dc3145 --- /dev/null +++ b/plugins/enhance/commands/enhance.md @@ -0,0 +1,106 @@ +--- +description: Analyze plugin structures, MCP tools, and security patterns +argument-hint: "[plugin-name] [--fix] [--verbose]" +--- + +# /enhance:plugin - Plugin Structure Analyzer + +Analyze plugin structures, MCP tool definitions, and security patterns against best practices. + +## Arguments + +Parse from $ARGUMENTS: +- **plugin**: Specific plugin to analyze (default: all) +- **--fix**: Apply auto-fixes for HIGH certainty issues +- **--verbose**: Show all issues including LOW certainty + +## Workflow + +1. **Discover plugins** - Find all plugins in `plugins/` directory +2. **Load patterns** - Import from `${CLAUDE_PLUGIN_ROOT}/lib/enhance/` +3. **Analyze each plugin**: + - Validate plugin.json structure + - Check MCP tool definitions + - Scan for security patterns +4. **Generate report** - Markdown table grouped by certainty +5. **Apply fixes** - If --fix flag, apply HIGH certainty auto-fixes + +## Detection Categories + +### HIGH Certainty + +| Pattern | Description | Auto-Fix | +|---------|-------------|----------| +| Missing additionalProperties | Schema allows extra fields | Add `"additionalProperties": false` | +| Missing required fields | Parameters not marked required | Add to `required` array | +| Version mismatch | plugin.json vs package.json | Sync versions | +| Missing tool description | Tool has no description | Manual fix required | + +### MEDIUM Certainty (verify context) + +| Pattern | Description | +|---------|-------------| +| Broad permissions | Agent has `Bash` without restrictions | +| Deep nesting | Parameter schema >2 levels deep | +| Long description | Tool description >500 chars | +| Missing param descriptions | Parameters lack descriptions | + +### LOW Certainty (advisory) + +| Pattern | Description | +|---------|-------------| +| Tool over-exposure | Many tools in single plugin | +| Optimization hints | Suggested simplifications | + +## Output Format + +```markdown +## Plugin Analysis: {plugin-name} + +### Tool Definitions ({n} issues) +| Tool | Issue | Fix | Certainty | +|------|-------|-----|-----------| +| workflow_start | Missing additionalProperties | Add to schema | HIGH | + +### Structure ({n} issues) +- Version mismatch: plugin.json (2.6.1) vs package.json (2.7.0) + +### Security ({n} issues) +- `ci-fixer` agent has unrestricted Bash access +``` + +## Implementation + +```javascript +const { pluginAnalyzer } = require('${CLAUDE_PLUGIN_ROOT}/lib/enhance'); + +// Parse arguments +const args = '$ARGUMENTS'.split(' ').filter(Boolean); +const pluginName = args.find(a => !a.startsWith('--')); +const applyFixes = args.includes('--fix'); +const verbose = args.includes('--verbose'); + +// Run analysis +const results = await pluginAnalyzer.analyze({ + plugin: pluginName, + verbose +}); + +// Generate report +const report = pluginAnalyzer.generateReport(results); +console.log(report); + +// Apply fixes if requested +if (applyFixes) { + const fixed = await pluginAnalyzer.applyFixes(results); + console.log(`Applied ${fixed.applied.length} fixes`); +} +``` + +## Success Criteria + +- All plugin.json files validated +- MCP tool definitions checked against best practices +- Security patterns scanned +- Clear report with actionable items +- Auto-fix available for HIGH certainty issues diff --git a/tests/enhance/plugin-analyzer.test.js b/tests/enhance/plugin-analyzer.test.js new file mode 100644 index 00000000..2c7c6fdf --- /dev/null +++ b/tests/enhance/plugin-analyzer.test.js @@ -0,0 +1,367 @@ +/** + * Plugin Analyzer Tests + */ + +const fs = require('fs'); +const path = require('path'); + +// Import modules under test +const pluginPatterns = require('../../lib/enhance/plugin-patterns'); +const toolPatterns = require('../../lib/enhance/tool-patterns'); +const securityPatterns = require('../../lib/enhance/security-patterns'); +const reporter = require('../../lib/enhance/reporter'); +const fixer = require('../../lib/enhance/fixer'); + +describe('Plugin Patterns', () => { + describe('missing_additional_properties', () => { + it('should detect missing additionalProperties', () => { + const schema = { + type: 'object', + properties: { + name: { type: 'string' } + } + }; + + const pattern = pluginPatterns.pluginPatterns.missing_additional_properties; + const result = pattern.check(schema); + + expect(result).toBeTruthy(); + expect(result.issue).toContain('additionalProperties'); + }); + + it('should not flag when additionalProperties is false', () => { + const schema = { + type: 'object', + properties: { + name: { type: 'string' } + }, + additionalProperties: false + }; + + const pattern = pluginPatterns.pluginPatterns.missing_additional_properties; + const result = pattern.check(schema); + + expect(result).toBeNull(); + }); + + it('should provide auto-fix function', () => { + const schema = { + type: 'object', + properties: { + name: { type: 'string' } + } + }; + + const pattern = pluginPatterns.pluginPatterns.missing_additional_properties; + const result = pattern.check(schema); + + expect(result.autoFixFn).toBeTruthy(); + + const fixed = result.autoFixFn(schema); + expect(fixed.additionalProperties).toBe(false); + }); + }); + + describe('missing_required_fields', () => { + it('should detect missing required array', () => { + const schema = { + type: 'object', + properties: { + name: { type: 'string' }, + age: { type: 'number' } + } + }; + + const pattern = pluginPatterns.pluginPatterns.missing_required_fields; + const result = pattern.check(schema); + + expect(result).toBeTruthy(); + }); + + it('should not flag when required is present', () => { + const schema = { + type: 'object', + properties: { + name: { type: 'string' } + }, + required: ['name'] + }; + + const pattern = pluginPatterns.pluginPatterns.missing_required_fields; + const result = pattern.check(schema); + + expect(result).toBeNull(); + }); + }); + + describe('version_mismatch', () => { + it('should detect version mismatch', () => { + const pluginJson = { version: '1.0.0' }; + const packageJson = { version: '2.0.0' }; + + const pattern = pluginPatterns.pluginPatterns.version_mismatch; + const result = pattern.check(pluginJson, packageJson); + + expect(result).toBeTruthy(); + expect(result.issue).toContain('1.0.0'); + expect(result.issue).toContain('2.0.0'); + }); + + it('should not flag when versions match', () => { + const pluginJson = { version: '2.0.0' }; + const packageJson = { version: '2.0.0' }; + + const pattern = pluginPatterns.pluginPatterns.version_mismatch; + const result = pattern.check(pluginJson, packageJson); + + expect(result).toBeNull(); + }); + }); + + describe('deep_nesting', () => { + it('should detect deeply nested schemas', () => { + const schema = { + type: 'object', + properties: { + level1: { + type: 'object', + properties: { + level2: { + type: 'object', + properties: { + level3: { + type: 'object', + properties: { + value: { type: 'string' } + } + } + } + } + } + } + } + }; + + const pattern = pluginPatterns.pluginPatterns.deep_nesting; + const result = pattern.check(schema); + + expect(result).toBeTruthy(); + expect(result.issue).toContain('nested'); + }); + }); +}); + +describe('Tool Patterns', () => { + describe('poor_tool_naming', () => { + it('should detect non-verb tool names', () => { + const tool = { name: 'userProfile' }; + + const pattern = toolPatterns.toolPatterns.poor_tool_naming; + const result = pattern.check(tool); + + expect(result).toBeTruthy(); + }); + + it('should accept verb-prefixed names', () => { + const tool = { name: 'get_user_profile' }; + + const pattern = toolPatterns.toolPatterns.poor_tool_naming; + const result = pattern.check(tool); + + expect(result).toBeNull(); + }); + }); + + describe('analyzeTool', () => { + it('should return issues for problematic tool', () => { + const tool = { + name: 'data', + description: '', + inputSchema: { + type: 'object', + properties: { + id: { type: 'string' } + } + } + }; + + const issues = toolPatterns.analyzeTool(tool); + + expect(issues.length).toBeGreaterThan(0); + }); + }); +}); + +describe('Security Patterns', () => { + describe('unrestricted_bash', () => { + it('should detect unrestricted Bash in frontmatter', () => { + const content = `--- +name: my-agent +tools: Read, Bash, Grep +--- + +# My Agent +`; + + const issues = securityPatterns.checkSecurity(content, 'test.md'); + + const bashIssue = issues.find(i => i.patternId === 'unrestricted_bash'); + expect(bashIssue).toBeTruthy(); + }); + + it('should not flag restricted Bash', () => { + const content = `--- +name: my-agent +tools: Read, Bash(git:*), Grep +--- + +# My Agent +`; + + const issues = securityPatterns.checkSecurity(content, 'test.md'); + + const bashIssue = issues.find(i => i.patternId === 'unrestricted_bash'); + expect(bashIssue).toBeUndefined(); + }); + }); + + describe('hardcoded_secrets', () => { + it('should detect hardcoded API keys', () => { + // INTENTIONALLY fake test secret - NOT a real credential + // Uses pattern that matches secret detection without triggering GitGuardian + const content = ` +const config = { + api_key: "test1234fake5678" +}; +`; + + const issues = securityPatterns.checkSecurity(content, 'config.js'); + + const secretIssue = issues.find(i => i.patternId === 'hardcoded_secrets'); + expect(secretIssue).toBeTruthy(); + }); + }); +}); + +describe('Reporter', () => { + describe('generateReport', () => { + it('should generate markdown report', () => { + const results = { + pluginName: 'test-plugin', + filesScanned: 5, + toolIssues: [ + { tool: 'get_data', issue: 'Missing description', certainty: 'HIGH' } + ], + structureIssues: [], + securityIssues: [] + }; + + const report = reporter.generateReport(results); + + expect(report).toContain('test-plugin'); + expect(report).toContain('get_data'); + expect(report).toContain('HIGH'); + }); + + it('should filter LOW certainty when not verbose', () => { + const results = { + pluginName: 'test-plugin', + filesScanned: 1, + toolIssues: [ + { tool: 'a', issue: 'Low issue', certainty: 'LOW' }, + { tool: 'b', issue: 'High issue', certainty: 'HIGH' } + ], + structureIssues: [], + securityIssues: [] + }; + + const report = reporter.generateReport(results, { verbose: false }); + + expect(report).not.toContain('Low issue'); + expect(report).toContain('High issue'); + }); + }); +}); + +describe('Fixer', () => { + describe('fixAdditionalProperties', () => { + it('should add additionalProperties: false', () => { + const schema = { + type: 'object', + properties: { + name: { type: 'string' } + } + }; + + const fixed = fixer.fixAdditionalProperties(schema); + + expect(fixed.additionalProperties).toBe(false); + }); + + it('should recursively fix nested schemas', () => { + const schema = { + type: 'object', + properties: { + nested: { + type: 'object', + properties: { + value: { type: 'string' } + } + } + } + }; + + const fixed = fixer.fixAdditionalProperties(schema); + + expect(fixed.additionalProperties).toBe(false); + expect(fixed.properties.nested.additionalProperties).toBe(false); + }); + }); + + describe('fixRequiredFields', () => { + it('should add required array', () => { + const schema = { + type: 'object', + properties: { + name: { type: 'string' }, + age: { type: 'number' } + } + }; + + const fixed = fixer.fixRequiredFields(schema); + + expect(fixed.required).toBeTruthy(); + expect(fixed.required).toContain('name'); + expect(fixed.required).toContain('age'); + }); + + it('should exclude optional fields', () => { + const schema = { + type: 'object', + properties: { + name: { type: 'string' }, + nickname: { type: 'string', description: 'Optional nickname' } + } + }; + + const fixed = fixer.fixRequiredFields(schema); + + expect(fixed.required).toContain('name'); + expect(fixed.required).not.toContain('nickname'); + }); + }); + + describe('previewFixes', () => { + it('should show which fixes will be applied', () => { + const issues = [ + { certainty: 'HIGH', autoFixFn: () => {}, issue: 'Fix me' }, + { certainty: 'MEDIUM', issue: 'Manual fix' } + ]; + + const previews = fixer.previewFixes(issues); + + expect(previews[0].willApply).toBe(true); + expect(previews[1].willApply).toBe(false); + }); + }); +});