fix(bmad-module): prevent atomicSwapDir data loss + harden ide-sync/wds cleanup paths

- atomicSwapDir: back the existing target up to a sibling before swapping the
  staged dir in, and roll back on a failed rename so an interrupted update can
  no longer leave neither the old nor the new install.
- ide-sync cleanup: only remove _bmad/ skill source dirs when EVERY IDE synced
  successfully (so failed targets remain retryable), and containment-check each
  CSV-derived path before fs.remove so a malformed row can't escape _bmad/.
- module-dirs: validate untrusted wds_folders entries for traversal/absolute
  escape before mkdir.
- Regenerate the vendored ide-sync bundle (vendor:check clean).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
pbean 2026-06-20 15:57:38 -07:00
parent 02806ba4cd
commit 08c73796a6
4 changed files with 45 additions and 9 deletions

View File

@ -99,7 +99,9 @@ export async function stageCopyPlan(srcRoot, destDir, plan, extras = {}) {
export async function atomicSwapDir(stagedDir, targetDir) {
const parent = path.dirname(targetDir);
await fsp.mkdir(parent, { recursive: true });
const sibling = path.join(parent, `.${path.basename(targetDir)}.bmad-tmp-${crypto.randomBytes(6).toString('hex')}`);
const suffix = crypto.randomBytes(6).toString('hex');
const sibling = path.join(parent, `.${path.basename(targetDir)}.bmad-tmp-${suffix}`);
const backup = path.join(parent, `.${path.basename(targetDir)}.bmad-old-${suffix}`);
try {
try {
await fsp.rename(stagedDir, sibling);
@ -108,10 +110,23 @@ export async function atomicSwapDir(stagedDir, targetDir) {
await copyDir(stagedDir, sibling);
await fsp.rm(stagedDir, { recursive: true, force: true });
}
await fsp.rm(targetDir, { recursive: true, force: true });
// Move any existing target aside as a backup rather than deleting it
// up front, so a failed swap can be rolled back to the old install.
const hadTarget = await fsp
.stat(targetDir)
.then(() => true)
.catch(() => false);
if (hadTarget) await fsp.rename(targetDir, backup);
try {
await fsp.rename(sibling, targetDir);
} catch (e) {
if (hadTarget) await fsp.rename(backup, targetDir).catch(() => {});
throw e;
}
if (hadTarget) await fsp.rm(backup, { recursive: true, force: true });
} catch (e) {
await fsp.rm(sibling, { recursive: true, force: true });
await fsp.rm(backup, { recursive: true, force: true });
throw e;
}
}

View File

@ -108,7 +108,14 @@ export async function createModuleDirectories(bmadDir, code, moduleConfig = {},
// WDS subfolders under design_artifacts.
if (configKey === 'design_artifacts' && wdsFolders.length) {
for (const sub of wdsFolders) {
const subPath = path.join(fullPath, sub);
if (typeof sub !== 'string' || sub === '') continue;
const subPath = path.normalize(path.join(fullPath, sub));
// `sub` is untrusted module content; reject traversal/absolute escapes
// out of the design_artifacts directory.
if (subPath !== normalizedNewAbs && !subPath.startsWith(normalizedNewAbs + path.sep)) {
warn(`wds_folders entry escapes design_artifacts, skipping: ${sub}`);
continue;
}
if (!(await pathExists(subPath))) {
await fs.mkdir(subPath, { recursive: true });
createdWdsFolders.push(sub);

View File

@ -10233,7 +10233,8 @@ var require_ide_sync = __commonJS({
verbose,
silent
});
if (cleanup) await cleanupBmadSkillDirs(bmadDir);
const allSucceeded = results.every((r) => r && r.success);
if (cleanup && allSucceeded) await cleanupBmadSkillDirs(bmadDir);
return { skipped: false, results };
}
async function cleanupBmadSkillDirs(bmadDir) {
@ -10244,10 +10245,14 @@ var require_ide_sync = __commonJS({
const records = csv.parse(csvContent, { columns: true, skip_empty_lines: true });
const bmadFolderName = path.basename(bmadDir);
const bmadPrefix = bmadFolderName + "/";
const bmadRoot = path.resolve(bmadDir);
for (const record of records) {
if (!record.path) continue;
const relativePath = record.path.startsWith(bmadPrefix) ? record.path.slice(bmadPrefix.length) : record.path;
const sourceDir = path.dirname(path.join(bmadDir, relativePath));
const skillFilePath = path.resolve(bmadDir, relativePath);
if (skillFilePath !== bmadRoot && !skillFilePath.startsWith(bmadRoot + path.sep)) continue;
const sourceDir = path.dirname(skillFilePath);
if (sourceDir === bmadRoot) continue;
if (await fs.pathExists(sourceDir)) {
await fs.remove(sourceDir);
await removeEmptyParents(path.dirname(sourceDir), bmadDir);

View File

@ -54,8 +54,11 @@ async function syncIdes({ projectRoot, bmadDir, ides, previousSkillIds = [], ver
});
// Mirror Installer._cleanupSkillDirs: skills are self-contained in IDE dirs,
// so _bmad/ only needs module-level files.
if (cleanup) await cleanupBmadSkillDirs(bmadDir);
// so _bmad/ only needs module-level files. Only clean up when every IDE
// synced successfully — otherwise the source skill dirs are still needed to
// retry the failed targets.
const allSucceeded = results.every((r) => r && r.success);
if (cleanup && allSucceeded) await cleanupBmadSkillDirs(bmadDir);
return { skipped: false, results };
}
@ -77,10 +80,16 @@ async function cleanupBmadSkillDirs(bmadDir) {
const bmadFolderName = path.basename(bmadDir);
const bmadPrefix = bmadFolderName + '/';
const bmadRoot = path.resolve(bmadDir);
for (const record of records) {
if (!record.path) continue;
const relativePath = record.path.startsWith(bmadPrefix) ? record.path.slice(bmadPrefix.length) : record.path;
const sourceDir = path.dirname(path.join(bmadDir, relativePath));
const skillFilePath = path.resolve(bmadDir, relativePath);
// Containment guard: a malformed CSV row (absolute path or `../`) must not
// let cleanup escape _bmad/ and remove arbitrary directories.
if (skillFilePath !== bmadRoot && !skillFilePath.startsWith(bmadRoot + path.sep)) continue;
const sourceDir = path.dirname(skillFilePath);
if (sourceDir === bmadRoot) continue;
if (await fs.pathExists(sourceDir)) {
await fs.remove(sourceDir);
await removeEmptyParents(path.dirname(sourceDir), bmadDir);