diff --git a/tools/installer/modules/custom-module-manager.js b/tools/installer/modules/custom-module-manager.js index 5908ecc92..5e5766ee2 100644 --- a/tools/installer/modules/custom-module-manager.js +++ b/tools/installer/modules/custom-module-manager.js @@ -104,6 +104,7 @@ class CustomModuleManager { * @param {string} repoUrl - GitHub repository URL * @param {Object} [options] - Clone options * @param {boolean} [options.silent] - Suppress spinner output + * @param {boolean} [options.skipInstall] - Skip npm install (for browsing before user confirms) * @returns {string} Path to the cloned repository */ async cloneRepo(repoUrl, options = {}) { @@ -159,9 +160,9 @@ class CustomModuleManager { } } - // Install dependencies if package.json exists + // Install dependencies if package.json exists (skip during browsing/analysis) const packageJsonPath = path.join(repoCacheDir, 'package.json'); - if (await fs.pathExists(packageJsonPath)) { + if (!options.skipInstall && (await fs.pathExists(packageJsonPath))) { const installSpinner = await createSpinner(); installSpinner.start(`Installing dependencies for ${owner}/${repo}...`); try { diff --git a/tools/installer/modules/official-modules.js b/tools/installer/modules/official-modules.js index 8da6e3d1c..a203dd85d 100644 --- a/tools/installer/modules/official-modules.js +++ b/tools/installer/modules/official-modules.js @@ -326,6 +326,11 @@ class OfficialModules { if (fileTrackingCallback) fileTrackingCallback(helpTarget); } + // Create directories declared in module.yaml (strategies 1-4 may have these) + if (!options.skipModuleInstaller) { + await this.createModuleDirectories(resolved.code, bmadDir, options); + } + // Update manifest const { Manifest } = require('../core/manifest'); const manifestObj = new Manifest(); diff --git a/tools/installer/modules/plugin-resolver.js b/tools/installer/modules/plugin-resolver.js index 8606da21a..9fbf325a2 100644 --- a/tools/installer/modules/plugin-resolver.js +++ b/tools/installer/modules/plugin-resolver.js @@ -33,11 +33,16 @@ class PluginResolver { return []; } - // Resolve skill paths to absolute and filter out non-existent + // Resolve skill paths to absolute, constrain to repo root, filter non-existent + const repoRoot = path.resolve(repoPath); const skillPaths = []; for (const rel of skillRelPaths) { const normalized = rel.replace(/^\.\//, ''); - const abs = path.join(repoPath, normalized); + const abs = path.resolve(repoPath, normalized); + // Guard against path traversal (.. segments, absolute paths in marketplace.json) + if (!abs.startsWith(repoRoot + path.sep) && abs !== repoRoot) { + continue; + } if (await fs.pathExists(abs)) { skillPaths.push(abs); } @@ -384,7 +389,7 @@ class PluginResolver { _escapeCSVField(value) { if (!value) return ''; if (value.includes(',') || value.includes('"') || value.includes('\n')) { - return `"${value.replace(/"/g, '""')}"`; + return `"${value.replaceAll('"', '""')}"`; } return value; } diff --git a/tools/installer/ui.js b/tools/installer/ui.js index b93cd02ce..75b704f64 100644 --- a/tools/installer/ui.js +++ b/tools/installer/ui.js @@ -863,11 +863,11 @@ class UI { 'UNVERIFIED MODULE: This module has not been reviewed by the BMad team.\n' + ' Only install modules from sources you trust.', ); - // Clone the repo so we can resolve plugin structures + // Clone the repo so we can resolve plugin structures (skip npm install until user confirms) s.start('Cloning repository...'); let repoPath; try { - repoPath = await customMgr.cloneRepo(url.trim()); + repoPath = await customMgr.cloneRepo(url.trim(), { skipInstall: true }); s.stop('Repository cloned'); } catch (cloneError) { s.error('Failed to clone repository');