diff --git a/src/core-skills/bmad-module/scripts/install.mjs b/src/core-skills/bmad-module/scripts/install.mjs index 696d9cf5e..82570d46b 100644 --- a/src/core-skills/bmad-module/scripts/install.mjs +++ b/src/core-skills/bmad-module/scripts/install.mjs @@ -206,7 +206,15 @@ export async function runInstall(opts) { // /tree/ 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 }; } diff --git a/src/core-skills/bmad-module/scripts/lib/source.mjs b/src/core-skills/bmad-module/scripts/lib/source.mjs index 772d1a5d9..6a4b143d3 100644 --- a/src/core-skills/bmad-module/scripts/lib/source.mjs +++ b/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// 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 }; } diff --git a/src/core-skills/bmad-module/scripts/remove.mjs b/src/core-skills/bmad-module/scripts/remove.mjs index d9a544a2e..db18e6d8e 100644 --- a/src/core-skills/bmad-module/scripts/remove.mjs +++ b/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. diff --git a/src/core-skills/bmad-module/scripts/update.mjs b/src/core-skills/bmad-module/scripts/update.mjs index 9c23ec876..ebf42f251 100644 --- a/src/core-skills/bmad-module/scripts/update.mjs +++ b/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) {