fix(bmad-module): harden source/remove/update paths and channel input

Address automated review findings on PR #2482:
- source.mjs: validate URL-derived subdir with safePathInsideRoot so a
  ../ subdir can't copy out of the shared clone cache; run cleanup() if
  the terminal copyDir throws so the temp working dir never leaks.
- install.mjs: reject unknown --channel values (e.g. a 'stabl' typo)
  instead of silently treating them as the 'next' default.
- remove.mjs / update.mjs: containment-check manifest/CLI-derived paths
  before destructive fs.rm / atomic swap, reusing safePathInsideRoot.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

src/core-skills/bmad-module/scripts/install.mjs

@@ -206,7 +206,15 @@ export async function runInstall(opts) {
 // /tree/<ref> parsed from the source) pins; --channel stable resolves the latest
 // non-prerelease GitHub tag, falling back to next (with a warning) when there are
 // no tags, the URL isn't a GitHub repo, or the tags API is unreachable.
+const VALID_CHANNELS = new Set(['stable', 'pinned', 'next']);
+
 export async function resolveCloneTarget(descriptor, opts) {
+  // Reject typo'd channels up front (e.g. `--channel stabl`) so they error
+  // instead of silently falling through the branches below to the `next` default.
+  if (opts.channel && !VALID_CHANNELS.has(opts.channel)) {
+    throw new BmadModuleError(EXIT.USAGE, `unknown --channel "${opts.channel}" (expected: stable, pinned, next)`);
+  }
+
   if (descriptor.kind !== 'git') {
     return { ref: null, channel: null, version: null };
   }

src/core-skills/bmad-module/scripts/lib/source.mjs

@@ -1,7 +1,7 @@
 import fs from 'node:fs/promises';
 import path from 'node:path';
 import os from 'node:os';
-import { copyDir } from './fs-safe.mjs';
+import { copyDir, safePathInsideRoot } from './fs-safe.mjs';
 import { ensureCachedRepo } from './cache.mjs';
 import { EXIT, BmadModuleError } from './exit.mjs';
 
@@ -210,7 +210,12 @@ export async function materializeSource(descriptor, opts = {}) {
       await cleanup();
       throw new BmadModuleError(EXIT.USAGE, `local source not a directory: ${descriptor.path}`);
     }
-    await copyDir(descriptor.path, dir, STAGE_IGNORE);
+    try {
+      await copyDir(descriptor.path, dir, STAGE_IGNORE);
+    } catch (e) {
+      await cleanup();
+      throw e;
+    }
     return { dir, sha: null, ref: null, cleanup };
   }
 
@@ -224,13 +229,28 @@ export async function materializeSource(descriptor, opts = {}) {
     throw e;
   }
 
-  const moduleRoot = descriptor.subdir ? path.join(cached.repoDir, descriptor.subdir) : cached.repoDir;
+  // A subdir parsed from the source URL (/tree/<ref>/<subdir> or ?path=) is
+  // untrusted: reject `..`/absolute/symlink escapes so it can't copy out of the
+  // shared clone cache. safePathInsideRoot returns null on any escape.
+  let moduleRoot = cached.repoDir;
+  if (descriptor.subdir) {
+    moduleRoot = safePathInsideRoot(cached.repoDir, descriptor.subdir);
+    if (!moduleRoot) {
+      await cleanup();
+      throw new BmadModuleError(EXIT.PATH_TRAVERSAL, `subdirectory "${descriptor.subdir}" escapes the repository root`);
+    }
+  }
   const rootStat = await fs.stat(moduleRoot).catch(() => null);
   if (!rootStat || !rootStat.isDirectory()) {
     await cleanup();
     throw new BmadModuleError(EXIT.USAGE, `subdirectory "${descriptor.subdir}" not found in ${descriptor.displayName}`);
   }
 
-  await copyDir(moduleRoot, dir, STAGE_IGNORE);
+  try {
+    await copyDir(moduleRoot, dir, STAGE_IGNORE);
+  } catch (e) {
+    await cleanup();
+    throw e;
+  }
   return { dir, sha: cached.sha, ref: cached.ref, cleanup };
 }

src/core-skills/bmad-module/scripts/remove.mjs

@@ -2,7 +2,7 @@ import fs from 'node:fs/promises';
 import path from 'node:path';
 import { EXIT, BmadModuleError } from './lib/exit.mjs';
 import { findBmadDir } from './lib/bmad-dir.mjs';
-import { pruneEmptyDirs } from './lib/fs-safe.mjs';
+import { pruneEmptyDirs, safePathInsideRoot } from './lib/fs-safe.mjs';
 import {
   readManifestYaml,
   removeModuleFromManifest,
@@ -53,11 +53,21 @@ export async function runRemove(opts) {
     process.stderr.write(`[bmad-module] warning: failed to update config for removal of ${code}: ${e.message}\n`);
   }
 
+  // Resolve the module root with a containment check, so a traversal-tainted
+  // code (e.g. from a hand-edited manifest) can never delete outside _bmad/.
+  const moduleRoot = safePathInsideRoot(bmadDir, code);
+  if (!moduleRoot) {
+    throw new BmadModuleError(EXIT.PATH_TRAVERSAL, `module code "${code}" escapes _bmad/`);
+  }
+
   // Delete each file tracked in files-manifest.csv; prune empty dirs after.
   const fileEntries = await readFileEntriesForModule(bmadDir, code);
-  const moduleRoot = path.join(bmadDir, code);
   for (const fe of fileEntries) {
-    const abs = path.join(bmadDir, fe.path);
+    const abs = safePathInsideRoot(bmadDir, fe.path);
+    if (!abs) {
+      process.stderr.write(`[bmad-module] warn: skipping files-manifest path that escapes _bmad/: ${fe.path}\n`);
+      continue;
+    }
     try {
       await fs.rm(abs, { force: true });
       await pruneEmptyDirs(path.dirname(abs), moduleRoot);
@@ -72,8 +82,8 @@ export async function runRemove(opts) {
 
   // Optionally purge custom overrides.
   if (opts.purge) {
-    const customDir = path.join(bmadDir, 'custom', code);
-    await fs.rm(customDir, { recursive: true, force: true });
+    const customDir = safePathInsideRoot(path.join(bmadDir, 'custom'), code);
+    if (customDir) await fs.rm(customDir, { recursive: true, force: true });
   }
 
   // Drop manifest rows.

src/core-skills/bmad-module/scripts/update.mjs

@@ -4,7 +4,7 @@ import { findBmadDir } from './lib/bmad-dir.mjs';
 import { parseSource, materializeSource } from './lib/source.mjs';
 import { readAndValidateManifest } from './lib/plugin-json.mjs';
 import { readUserIgnores, buildIgnoreMatcher, buildCopyPlan, rewriteManifestPaths, validateDeclaredPaths } from './lib/install-plan.mjs';
-import { stageCopyPlan, atomicSwapDir, sha256File, pruneEmptyDirs } from './lib/fs-safe.mjs';
+import { stageCopyPlan, atomicSwapDir, sha256File, pruneEmptyDirs, safePathInsideRoot } from './lib/fs-safe.mjs';
 import {
   readManifestYaml,
   addModuleToManifest,
@@ -88,7 +88,8 @@ async function updateOne(bmadDir, projectDir, entry, opts) {
     const oldEntries = await readFileEntriesForModule(bmadDir, code);
     const modified = [];
     for (const fe of oldEntries) {
-      const abs = path.join(bmadDir, fe.path);
+      const abs = safePathInsideRoot(bmadDir, fe.path);
+      if (!abs) continue; // an entry that escapes _bmad/ is not a tracked file
       const current = await sha256File(abs);
       if (current === null) continue;
       if (fe.hash && current !== fe.hash) modified.push(fe.path);
@@ -113,7 +114,10 @@ async function updateOne(bmadDir, projectDir, entry, opts) {
     await stageCopyPlan(materialized.dir, stagedDir, plan, {
       '.claude-plugin/plugin.json': rewrittenManifestJson,
     });
-    const targetDir = path.join(bmadDir, code);
+    const targetDir = safePathInsideRoot(bmadDir, code);
+    if (!targetDir) {
+      throw new BmadModuleError(EXIT.PATH_TRAVERSAL, `module code "${code}" escapes _bmad/`);
+    }
     try {
       await atomicSwapDir(stagedDir, targetDir);
     } catch (e) {