fix(cli): address code review findings for uninstall command

- Add path traversal guard in uninstallOutputFolder (resolve + startsWith)
- Thread silent flag through to cleanupCopilotInstructions
- Trim text input before path.resolve in directory prompt
- DRY uninstall() by delegating to extracted helper methods
- Validate projectDir existence before probing for BMAD
- Use fs.rmdir instead of fs.remove in removeEmptyParents (race safety)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Davor Racić 2026-02-14 17:44:31 +01:00
parent fc3c3d873c
commit 54876a6af3
4 changed files with 21 additions and 21 deletions

View File

@ -43,12 +43,17 @@ module.exports = {
},
});
projectDir = path.resolve(customDir);
projectDir = path.resolve(customDir.trim());
} else {
projectDir = process.cwd();
}
}
if (!(await fs.pathExists(projectDir))) {
await prompts.log.error(`Directory does not exist: ${projectDir}`);
process.exit(1);
}
const { bmadDir } = await installer.findBmadDir(projectDir);
if (!(await fs.pathExists(bmadDir))) {

View File

@ -1552,30 +1552,18 @@ class Installer {
// 2. IDE CLEANUP (before _bmad/ deletion so configs are accessible)
if (options.removeIdeConfigs !== false) {
await this.ideManager.ensureInitialized();
const cleanupOptions = { isUninstall: true };
const ideList = existingInstall.ides || [];
if (ideList.length > 0) {
await this.ideManager.cleanupByList(projectDir, ideList, cleanupOptions);
} else {
await this.ideManager.cleanup(projectDir, cleanupOptions);
}
await this.uninstallIdeConfigs(projectDir, existingInstall, { silent: options.silent });
removed.ideConfigs = true;
}
// 3. OUTPUT FOLDER (only if explicitly requested)
if (options.removeOutputFolder === true && outputFolder) {
const outputPath = path.join(projectDir, outputFolder);
if (await fs.pathExists(outputPath)) {
await fs.remove(outputPath);
removed.outputFolder = true;
}
removed.outputFolder = await this.uninstallOutputFolder(projectDir, outputFolder);
}
// 4. BMAD DIRECTORY (last, after everything that needs it)
if (options.removeModules !== false) {
await fs.remove(bmadDir);
removed.modules = true;
removed.modules = await this.uninstallModules(projectDir);
}
return { success: true, removed, version: existingInstall.version };
@ -1606,7 +1594,11 @@ class Installer {
*/
async uninstallOutputFolder(projectDir, outputFolder) {
if (!outputFolder) return false;
const outputPath = path.join(projectDir, outputFolder);
const resolvedProject = path.resolve(projectDir);
const outputPath = path.resolve(resolvedProject, outputFolder);
if (!outputPath.startsWith(resolvedProject + path.sep)) {
return false;
}
if (await fs.pathExists(outputPath)) {
await fs.remove(outputPath);
return true;

View File

@ -548,7 +548,7 @@ LOAD and execute from: {project-root}/{{bmadFolderName}}/{{path}}
if (!(await fs.pathExists(fullPath))) break;
const remaining = await fs.readdir(fullPath);
if (remaining.length > 0) break;
await fs.remove(fullPath);
await fs.rmdir(fullPath);
} catch {
break;
}

View File

@ -640,7 +640,7 @@ Type \`/bmad-\` in Copilot Chat to see all available BMAD workflows and agent ac
// handles marker-based replacement in a single read-modify-write pass,
// which correctly preserves user content outside the markers.
if (options.isUninstall) {
await this.cleanupCopilotInstructions(projectDir);
await this.cleanupCopilotInstructions(projectDir, options);
}
}
@ -649,8 +649,9 @@ Type \`/bmad-\` in Copilot Chat to see all available BMAD workflows and agent ac
* If file becomes empty after stripping, delete it.
* If a .bak backup exists and the main file was deleted, restore the backup.
* @param {string} projectDir - Project directory
* @param {Object} [options] - Options (e.g. { silent: true })
*/
async cleanupCopilotInstructions(projectDir) {
async cleanupCopilotInstructions(projectDir, options = {}) {
const instructionsPath = path.join(projectDir, this.githubDir, 'copilot-instructions.md');
const backupPath = `${instructionsPath}.bak`;
@ -680,7 +681,9 @@ Type \`/bmad-\` in Copilot Chat to see all available BMAD workflows and agent ac
// If backup exists, restore it
if (await fs.pathExists(backupPath)) {
await fs.rename(backupPath, instructionsPath);
await prompts.log.message(' Restored copilot-instructions.md from backup');
if (!options.silent) {
await prompts.log.message(' Restored copilot-instructions.md from backup');
}
}
} else {
// Write cleaned content back (preserve original whitespace)