diff --git a/.bumpy/security-audit.md b/.bumpy/security-audit.md new file mode 100644 index 0000000..6e250f9 --- /dev/null +++ b/.bumpy/security-audit.md @@ -0,0 +1,12 @@ +--- +'@varlock/bumpy': patch +--- + +Security hardening: eliminate shell injection vulnerabilities across all CLI commands + +- Replace shell string interpolation with `execFile`-based argument arrays (`runArgs`/`runArgsAsync`) throughout the codebase, preventing command injection via branch names, PR numbers, config values, package names, and registry URLs +- Add input validation for git branch names and PR numbers from environment variables +- Remove broken `escapeShell` function in favor of shell-free execution +- Use `sq()` single-quote escaping for template substitutions in user-defined publish commands +- Restrict dynamic changelog formatter imports to paths within the project root +- Reduce changeset filename collisions by using three-word random names diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 86ffe33..d8fb77a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -10,7 +10,8 @@ jobs: - run: bun install - run: cd packages/bumpy && bunx varlock load # need ENV types for tsdown config file - run: bun run check - - run: git config --global user.name "CI" && git config --global user.email "ci@test" + # publish tests create temp git repos with commits, which requires a git identity + - run: git config --global user.name "CI" && git config --global user.email "ci@example.com" - run: bun run test bumpy-check: diff --git a/.gitignore b/.gitignore index 571fe2d..aa6d230 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,5 @@ env.d.ts *.tsbuildinfo ignore + +.claude/worktrees \ No newline at end of file diff --git a/packages/bumpy/src/commands/check.ts b/packages/bumpy/src/commands/check.ts index e64fc3e..eb120ac 100644 --- a/packages/bumpy/src/commands/check.ts +++ b/packages/bumpy/src/commands/check.ts @@ -3,7 +3,7 @@ import { log, colorize } from '../utils/logger.ts'; import { loadConfig } from '../core/config.ts'; import { discoverWorkspace } from '../core/workspace.ts'; import { readChangesets } from '../core/changeset.ts'; -import { tryRun } from '../utils/shell.ts'; +import { tryRunArgs } from '../utils/shell.ts'; import type { WorkspacePackage } from '../types.ts'; /** @@ -61,9 +61,9 @@ export async function checkCommand(rootDir: string): Promise { /** Get files changed on this branch compared to the base branch */ function getChangedFiles(rootDir: string, baseBranch: string): string[] { // Try merge-base first (works on branches) - const mergeBase = tryRun(`git merge-base HEAD origin/${baseBranch}`, { cwd: rootDir }); + const mergeBase = tryRunArgs(['git', 'merge-base', 'HEAD', `origin/${baseBranch}`], { cwd: rootDir }); const ref = mergeBase || `origin/${baseBranch}`; - const diff = tryRun(`git diff --name-only ${ref}`, { cwd: rootDir }); + const diff = tryRunArgs(['git', 'diff', '--name-only', ref], { cwd: rootDir }); if (!diff) return []; return diff.split('\n').filter(Boolean); } diff --git a/packages/bumpy/src/commands/ci.ts b/packages/bumpy/src/commands/ci.ts index 4cccea3..35b66b6 100644 --- a/packages/bumpy/src/commands/ci.ts +++ b/packages/bumpy/src/commands/ci.ts @@ -4,16 +4,34 @@ import { discoverWorkspace } from '../core/workspace.ts'; import { DependencyGraph } from '../core/dep-graph.ts'; import { readChangesets } from '../core/changeset.ts'; import { assembleReleasePlan } from '../core/release-plan.ts'; -import { run, tryRun, runAsync } from '../utils/shell.ts'; +import { runArgs, runArgsAsync, tryRunArgs } from '../utils/shell.ts'; import type { BumpyConfig, ReleasePlan, PlannedRelease } from '../types.ts'; +// ---- Validation helpers ---- + +/** Validate a git branch name to prevent injection */ +function validateBranchName(name: string): string { + if (!/^[a-zA-Z0-9_./-]+$/.test(name)) { + throw new Error(`Invalid branch name: ${name}`); + } + return name; +} + +/** Validate a PR number is numeric */ +function validatePrNumber(pr: string): string { + if (!/^\d+$/.test(pr)) { + throw new Error(`Invalid PR number: ${pr}`); + } + return pr; +} + /** Configure git identity for CI commits if not already set */ function ensureGitIdentity(rootDir: string, config: BumpyConfig): void { - const name = tryRun('git config user.name', { cwd: rootDir }); + const name = tryRunArgs(['git', 'config', 'user.name'], { cwd: rootDir }); if (!name) { const { name: gitName, email: gitEmail } = config.gitUser; - run(`git config user.name "${gitName}"`, { cwd: rootDir }); - run(`git config user.email "${gitEmail}"`, { cwd: rootDir }); + runArgs(['git', 'config', 'user.name', gitName], { cwd: rootDir }); + runArgs(['git', 'config', 'user.email', gitEmail], { cwd: rootDir }); log.dim(` Using git identity: ${gitName} <${gitEmail}>`); } } @@ -119,11 +137,11 @@ async function autoPublish(rootDir: string, config: BumpyConfig, tag?: string): // Commit the version changes log.step('Committing version changes...'); - run('git add -A', { cwd: rootDir }); - const status = tryRun('git status --porcelain', { cwd: rootDir }); + runArgs(['git', 'add', '-A'], { cwd: rootDir }); + const status = tryRunArgs(['git', 'status', '--porcelain'], { cwd: rootDir }); if (status) { - run('git commit -m "Version packages"', { cwd: rootDir }); - run('git push', { cwd: rootDir }); + runArgs(['git', 'commit', '-m', 'Version packages'], { cwd: rootDir }); + runArgs(['git', 'push'], { cwd: rootDir }); } log.step('Running bumpy publish...'); @@ -139,21 +157,25 @@ async function createVersionPr( config: BumpyConfig, branchName?: string, ): Promise { - const branch = branchName || config.versionPr.branch; - const baseBranch = tryRun('git rev-parse --abbrev-ref HEAD', { cwd: rootDir }) || 'main'; + const branch = validateBranchName(branchName || config.versionPr.branch); + const baseBranch = validateBranchName( + tryRunArgs(['git', 'rev-parse', '--abbrev-ref', 'HEAD'], { cwd: rootDir }) || 'main', + ); // Check if a version PR already exists - const existingPr = tryRun(`gh pr list --head "${branch}" --json number --jq ".[0].number"`, { cwd: rootDir }); + const existingPr = tryRunArgs(['gh', 'pr', 'list', '--head', branch, '--json', 'number', '--jq', '.[0].number'], { + cwd: rootDir, + }); // Create or update the branch log.step(`Creating branch ${branch}...`); - const branchExists = tryRun(`git rev-parse --verify ${branch}`, { cwd: rootDir }) !== null; + const branchExists = tryRunArgs(['git', 'rev-parse', '--verify', branch], { cwd: rootDir }) !== null; if (branchExists) { - run(`git checkout ${branch}`, { cwd: rootDir }); - run(`git reset --hard ${baseBranch}`, { cwd: rootDir }); + runArgs(['git', 'checkout', branch], { cwd: rootDir }); + runArgs(['git', 'reset', '--hard', baseBranch], { cwd: rootDir }); } else { - run(`git checkout -b ${branch}`, { cwd: rootDir }); + runArgs(['git', 'checkout', '-b', branch], { cwd: rootDir }); } // Run bumpy version @@ -162,40 +184,41 @@ async function createVersionPr( await versionCommand(rootDir); // Commit and push - run('git add -A', { cwd: rootDir }); - const status = tryRun('git status --porcelain', { cwd: rootDir }); + runArgs(['git', 'add', '-A'], { cwd: rootDir }); + const status = tryRunArgs(['git', 'status', '--porcelain'], { cwd: rootDir }); if (!status) { log.info('No version changes to commit.'); - run(`git checkout ${baseBranch}`, { cwd: rootDir }); + runArgs(['git', 'checkout', baseBranch], { cwd: rootDir }); return; } const commitMsg = ['Version packages', '', ...plan.releases.map((r) => `${r.name}@${r.newVersion}`)].join('\n'); - run('git commit -F -', { cwd: rootDir, input: commitMsg }); - run(`git push -u origin ${branch} --force`, { cwd: rootDir }); + runArgs(['git', 'commit', '-F', '-'], { cwd: rootDir, input: commitMsg }); + runArgs(['git', 'push', '-u', 'origin', branch, '--force'], { cwd: rootDir }); // Create or update PR const prBody = formatVersionPrBody(plan, config.versionPr.preamble); if (existingPr) { - log.step(`Updating existing PR #${existingPr}...`); - await runAsync(`gh pr edit ${existingPr} --title "${config.versionPr.title}" --body-file -`, { + const validPr = validatePrNumber(existingPr); + log.step(`Updating existing PR #${validPr}...`); + await runArgsAsync(['gh', 'pr', 'edit', validPr, '--title', config.versionPr.title, '--body-file', '-'], { cwd: rootDir, input: prBody, }); - log.success(`Updated PR #${existingPr}`); + log.success(`Updated PR #${validPr}`); } else { log.step('Creating version PR...'); const prTitle = config.versionPr.title; - const result = await runAsync( - `gh pr create --title "${prTitle}" --body-file - --base "${baseBranch}" --head "${branch}"`, + const result = await runArgsAsync( + ['gh', 'pr', 'create', '--title', prTitle, '--body-file', '-', '--base', baseBranch, '--head', branch], { cwd: rootDir, input: prBody }, ); log.success(`Created PR: ${result}`); } // Switch back to the base branch - run(`git checkout ${baseBranch}`, { cwd: rootDir }); + runArgs(['git', 'checkout', baseBranch], { cwd: rootDir }); } // ---- PR comment helpers ---- @@ -277,23 +300,27 @@ function formatVersionPrBody(plan: ReleasePlan, preamble: string): string { const COMMENT_MARKER = ''; async function postOrUpdatePrComment(prNumber: string, body: string, rootDir: string): Promise { + const validPr = validatePrNumber(prNumber); const markedBody = `${COMMENT_MARKER}\n${body}`; try { - // Find existing bumpy comment - const existingComment = tryRun( - `gh pr view ${prNumber} --json comments --jq '.comments[] | select(.body | startswith("${COMMENT_MARKER}")) | .id' | head -1`, - { cwd: rootDir }, - ); + // Find existing bumpy comment using gh with jq + const jqFilter = `.comments[] | select(.body | startswith("${COMMENT_MARKER}")) | .id`; + const existingComment = tryRunArgs(['gh', 'pr', 'view', validPr, '--json', 'comments', '--jq', jqFilter], { + cwd: rootDir, + }); + + // Take the first result if multiple + const commentId = existingComment?.split('\n')[0]?.trim(); - if (existingComment) { - await runAsync(`gh api repos/{owner}/{repo}/issues/comments/${existingComment} -X PATCH -f body=@-`, { - cwd: rootDir, - input: markedBody, - }); + if (commentId) { + await runArgsAsync( + ['gh', 'api', `repos/{owner}/{repo}/issues/comments/${commentId}`, '-X', 'PATCH', '-f', 'body=@-'], + { cwd: rootDir, input: markedBody }, + ); log.dim(' Updated PR comment'); } else { - await runAsync(`gh pr comment ${prNumber} --body-file -`, { cwd: rootDir, input: markedBody }); + await runArgsAsync(['gh', 'pr', 'comment', validPr, '--body-file', '-'], { cwd: rootDir, input: markedBody }); log.dim(' Posted PR comment'); } } catch (err) { @@ -308,6 +335,11 @@ function detectPrNumber(): string | null { const match = process.env.GITHUB_REF?.match(/refs\/pull\/(\d+)\//); if (match) return match[1]!; } - // Also check for explicit env var - return process.env.BUMPY_PR_NUMBER || process.env.PR_NUMBER || null; + // Also check for explicit env var — validate it's numeric + const envPr = process.env.BUMPY_PR_NUMBER || process.env.PR_NUMBER || null; + if (envPr && !/^\d+$/.test(envPr)) { + log.warn(`Ignoring invalid PR number from environment: ${envPr}`); + return null; + } + return envPr; } diff --git a/packages/bumpy/src/commands/generate.ts b/packages/bumpy/src/commands/generate.ts index 9e1730f..8a3c696 100644 --- a/packages/bumpy/src/commands/generate.ts +++ b/packages/bumpy/src/commands/generate.ts @@ -1,5 +1,5 @@ import { log, colorize } from '../utils/logger.ts'; -import { tryRun } from '../utils/shell.ts'; +import { tryRunArgs } from '../utils/shell.ts'; import { loadConfig } from '../core/config.ts'; import { discoverPackages } from '../core/workspace.ts'; import { writeChangeset } from '../core/changeset.ts'; @@ -50,7 +50,7 @@ export async function generateCommand(rootDir: string, opts: GenerateOptions): P log.step(`Scanning commits from ${colorize(from, 'cyan')}...`); // Get commits since ref - const rawLog = tryRun(`git log ${from}..HEAD --format="%H%n%s%n%b%n---END---"`, { cwd: rootDir }); + const rawLog = tryRunArgs(['git', 'log', `${from}..HEAD`, '--format=%H%n%s%n%b%n---END---'], { cwd: rootDir }); if (!rawLog) { log.info('No commits found since ' + from); @@ -239,9 +239,8 @@ function bumpPriority(type: BumpType): number { /** Find the most recent version tag in the repo */ function findLastVersionTag(rootDir: string): string | null { // Look for tags matching common patterns: v1.2.3, pkg@1.2.3, etc. - const tag = tryRun( - 'git describe --tags --abbrev=0 --match "v*" 2>/dev/null || git describe --tags --abbrev=0 --match "*@*" 2>/dev/null', - { cwd: rootDir }, - ); + const tag = + tryRunArgs(['git', 'describe', '--tags', '--abbrev=0', '--match', 'v*'], { cwd: rootDir }) || + tryRunArgs(['git', 'describe', '--tags', '--abbrev=0', '--match', '*@*'], { cwd: rootDir }); return tag || null; } diff --git a/packages/bumpy/src/commands/publish.ts b/packages/bumpy/src/commands/publish.ts index b94794b..4b79c7a 100644 --- a/packages/bumpy/src/commands/publish.ts +++ b/packages/bumpy/src/commands/publish.ts @@ -158,10 +158,9 @@ async function findUnpublishedPackages( } async function checkIfPublished(name: string, version: string, pkgConfig?: PackageConfig): Promise { - const { runAsync } = await import('../utils/shell.ts'); - const { tryRun } = await import('../utils/shell.ts'); + const { runAsync, runArgsAsync, tryRunArgs } = await import('../utils/shell.ts'); - // 1. Custom check command + // 1. Custom check command (user-defined, runs in shell by design) if (pkgConfig?.checkPublished) { try { const result = await runAsync(pkgConfig.checkPublished); @@ -174,13 +173,14 @@ async function checkIfPublished(name: string, version: string, pkgConfig?: Packa // 2. Non-npm packages — check git tags if (pkgConfig?.skipNpmPublish || pkgConfig?.publishCommand) { const tag = `${name}@${version}`; - return tryRun(`git tag -l "${tag}"`) === tag; + return tryRunArgs(['git', 'tag', '-l', tag]) === tag; } // 3. Default — check npm registry try { - const regFlag = pkgConfig?.registry ? `--registry ${pkgConfig.registry}` : ''; - const result = await runAsync(`npm info "${name}@${version}" version ${regFlag}`.trim()); + const args = ['npm', 'info', `${name}@${version}`, 'version']; + if (pkgConfig?.registry) args.push('--registry', pkgConfig.registry); + const result = await runArgsAsync(args); return result === version; } catch { return false; diff --git a/packages/bumpy/src/commands/version.ts b/packages/bumpy/src/commands/version.ts index 742fd57..580b875 100644 --- a/packages/bumpy/src/commands/version.ts +++ b/packages/bumpy/src/commands/version.ts @@ -5,7 +5,7 @@ import { DependencyGraph } from '../core/dep-graph.ts'; import { readChangesets } from '../core/changeset.ts'; import { assembleReleasePlan } from '../core/release-plan.ts'; import { applyReleasePlan } from '../core/apply-release-plan.ts'; -import { run, tryRun } from '../utils/shell.ts'; +import { runArgs, tryRunArgs } from '../utils/shell.ts'; import { detectWorkspaces } from '../utils/package-manager.ts'; export async function versionCommand(rootDir: string): Promise { @@ -46,18 +46,18 @@ export async function versionCommand(rootDir: string): Promise { if (config.commit) { try { // Stage version changes, changelogs, deleted changesets, and lockfile - run('git add -A .bumpy/', { cwd: rootDir }); + runArgs(['git', 'add', '-A', '.bumpy/'], { cwd: rootDir }); for (const r of plan.releases) { const pkg = packages.get(r.name)!; - run(`git add "${pkg.relativeDir}/package.json"`, { cwd: rootDir }); - run(`git add "${pkg.relativeDir}/CHANGELOG.md"`, { cwd: rootDir }); + runArgs(['git', 'add', '--', `${pkg.relativeDir}/package.json`], { cwd: rootDir }); + runArgs(['git', 'add', '--', `${pkg.relativeDir}/CHANGELOG.md`], { cwd: rootDir }); } // Stage lockfile if it changed for (const lockfile of ['bun.lock', 'bun.lockb', 'pnpm-lock.yaml', 'yarn.lock', 'package-lock.json']) { - tryRun(`git add "${lockfile}"`, { cwd: rootDir }); + tryRunArgs(['git', 'add', '--', lockfile], { cwd: rootDir }); } const msg = ['Version packages', '', ...plan.releases.map((r) => `${r.name}@${r.newVersion}`)].join('\n'); - run('git commit -F -', { cwd: rootDir, input: msg }); + runArgs(['git', 'commit', '-F', '-'], { cwd: rootDir, input: msg }); log.success('Created git commit'); } catch (e) { log.warn(`Git commit failed: ${e}`); @@ -68,26 +68,26 @@ export async function versionCommand(rootDir: string): Promise { /** Run the package manager's install to update the lockfile */ async function updateLockfile(rootDir: string): Promise { const { packageManager } = await detectWorkspaces(rootDir); - const installCmd = getInstallCommand(packageManager); + const installArgs = getInstallArgs(packageManager); - log.step(`Updating lockfile (${installCmd})...`); + log.step(`Updating lockfile (${installArgs.join(' ')})...`); try { - run(installCmd, { cwd: rootDir }); + runArgs(installArgs, { cwd: rootDir }); log.dim(' Lockfile updated'); } catch (err) { log.warn(` Lockfile update failed: ${err instanceof Error ? err.message : err}`); } } -function getInstallCommand(pm: string): string { +function getInstallArgs(pm: string): string[] { switch (pm) { case 'pnpm': - return 'pnpm install --lockfile-only'; + return ['pnpm', 'install', '--lockfile-only']; case 'bun': - return 'bun install'; + return ['bun', 'install']; case 'yarn': - return 'yarn install --mode update-lockfile'; + return ['yarn', 'install', '--mode', 'update-lockfile']; default: - return 'npm install --package-lock-only'; + return ['npm', 'install', '--package-lock-only']; } } diff --git a/packages/bumpy/src/core/changelog-github.ts b/packages/bumpy/src/core/changelog-github.ts index f721219..6c05bd6 100644 --- a/packages/bumpy/src/core/changelog-github.ts +++ b/packages/bumpy/src/core/changelog-github.ts @@ -1,4 +1,4 @@ -import { tryRun } from '../utils/shell.ts'; +import { tryRunArgs } from '../utils/shell.ts'; import type { ChangelogContext, ChangelogFormatter } from './changelog.ts'; interface GithubOptions { @@ -73,19 +73,37 @@ interface PrInfo { async function findPrForChangeset(changesetId: string, repo?: string): Promise { try { // Find the commit that added this changeset file - const commitHash = tryRun( - `git log --diff-filter=A --format="%H" -- ".bumpy/${changesetId}.md" ".changeset/${changesetId}.md"`, - ); + const commitHash = tryRunArgs([ + 'git', + 'log', + '--diff-filter=A', + '--format=%H', + '--', + `.bumpy/${changesetId}.md`, + `.changeset/${changesetId}.md`, + ]); if (!commitHash) return null; const hash = commitHash.split('\n')[0]!.trim(); if (!hash) return null; // Look up the PR for this commit - const repoFlag = repo ? `--repo ${repo}` : ''; - const prJson = tryRun( - `gh pr list --search "${hash}" --state merged --json number,url,author --jq ".[0]" ${repoFlag}`, - ); + const ghArgs = [ + 'gh', + 'pr', + 'list', + '--search', + hash, + '--state', + 'merged', + '--json', + 'number,url,author', + '--jq', + '.[0]', + ]; + if (repo) ghArgs.push('--repo', repo); + + const prJson = tryRunArgs(ghArgs); if (!prJson) return null; const pr = JSON.parse(prJson); diff --git a/packages/bumpy/src/core/changelog.ts b/packages/bumpy/src/core/changelog.ts index 94ce9c6..9ca1a76 100644 --- a/packages/bumpy/src/core/changelog.ts +++ b/packages/bumpy/src/core/changelog.ts @@ -93,7 +93,17 @@ export async function loadFormatter(changelog: BumpyConfig['changelog'], rootDir // Custom module if (typeof name === 'string') { try { - const modulePath = name.startsWith('.') ? resolve(rootDir, name) : name; + let modulePath: string; + if (name.startsWith('.')) { + // Relative path — resolve and verify it stays within the project root + modulePath = resolve(rootDir, name); + if (!modulePath.startsWith(rootDir + '/')) { + throw new Error(`Changelog formatter path "${name}" resolves outside the project root`); + } + } else { + // Bare module specifier (e.g. npm package name) + modulePath = name; + } const mod = await import(modulePath); // Support: export default fn, export const changelogFormatter = fn, or module is fn const exported = mod.default || mod.changelogFormatter; diff --git a/packages/bumpy/src/core/git.ts b/packages/bumpy/src/core/git.ts index 76e67ad..94f8aca 100644 --- a/packages/bumpy/src/core/git.ts +++ b/packages/bumpy/src/core/git.ts @@ -1,43 +1,43 @@ -import { run, tryRun } from '../utils/shell.ts'; +import { runArgs, tryRunArgs } from '../utils/shell.ts'; /** Create a git tag */ export function createTag(tag: string, opts?: { cwd?: string }): void { - run(`git tag ${tag}`, opts); + runArgs(['git', 'tag', tag], opts); } /** Push commits and tags to remote */ export function pushWithTags(opts?: { cwd?: string }): void { - run('git push --follow-tags', opts); + runArgs(['git', 'push', '--follow-tags'], opts); } /** Check if there are uncommitted changes */ export function hasUncommittedChanges(opts?: { cwd?: string }): boolean { - const result = tryRun('git status --porcelain', opts); + const result = tryRunArgs(['git', 'status', '--porcelain'], opts); return result !== null && result.length > 0; } /** Get the current branch name */ export function getCurrentBranch(opts?: { cwd?: string }): string | null { - return tryRun('git rev-parse --abbrev-ref HEAD', opts); + return tryRunArgs(['git', 'rev-parse', '--abbrev-ref', 'HEAD'], opts); } /** Stage files and create a commit */ export function commitFiles(files: string[], message: string, opts?: { cwd?: string }): void { for (const file of files) { // Use -- to prevent filenames from being interpreted as flags - run(`git add -- "${file}"`, opts); + runArgs(['git', 'add', '--', file], opts); } - run('git commit -F -', { ...opts, input: message }); + runArgs(['git', 'commit', '-F', '-'], { ...opts, input: message }); } /** Check if a tag already exists */ export function tagExists(tag: string, opts?: { cwd?: string }): boolean { - return tryRun(`git tag -l "${tag}"`, opts) === tag; + return tryRunArgs(['git', 'tag', '-l', tag], opts) === tag; } /** Get all tags matching a pattern */ export function listTags(pattern: string, opts?: { cwd?: string }): string[] { - const result = tryRun(`git tag -l "${pattern}"`, opts); + const result = tryRunArgs(['git', 'tag', '-l', pattern], opts); if (!result) return []; return result.split('\n').filter(Boolean); } diff --git a/packages/bumpy/src/core/github-release.ts b/packages/bumpy/src/core/github-release.ts index 34222ec..157c5c7 100644 --- a/packages/bumpy/src/core/github-release.ts +++ b/packages/bumpy/src/core/github-release.ts @@ -1,4 +1,4 @@ -import { tryRun, runAsync } from '../utils/shell.ts'; +import { tryRunArgs, runArgsAsync } from '../utils/shell.ts'; import { log } from '../utils/logger.ts'; import type { PlannedRelease, Changeset } from '../types.ts'; @@ -30,7 +30,7 @@ export async function createIndividualReleases( } try { - await runAsync(`gh release create "${tag}" --title "${escapeShell(title)}" --notes "${escapeShell(body)}"`, { + await runArgsAsync(['gh', 'release', 'create', tag, '--title', title, '--notes', body], { cwd: rootDir, }); log.dim(` Created GitHub release: ${title}`); @@ -70,9 +70,9 @@ export async function createAggregateRelease( try { // Create the tag if it doesn't exist - tryRun(`git tag "${tag}"`, { cwd: rootDir }); + tryRunArgs(['git', 'tag', tag], { cwd: rootDir }); - await runAsync(`gh release create "${tag}" --title "${escapeShell(title)}" --notes "${escapeShell(body)}"`, { + await runArgsAsync(['gh', 'release', 'create', tag, '--title', title, '--notes', body], { cwd: rootDir, }); log.success(`Created aggregate GitHub release: ${title}`); @@ -136,9 +136,5 @@ function buildAggregateBody(releases: PlannedRelease[], changesets: Changeset[]) } function isGhAvailable(): boolean { - return tryRun('gh --version') !== null; -} - -function escapeShell(str: string): string { - return str.replace(/"/g, '\\"').replace(/\n/g, '\\n'); + return tryRunArgs(['gh', '--version']) !== null; } diff --git a/packages/bumpy/src/core/publish-pipeline.ts b/packages/bumpy/src/core/publish-pipeline.ts index ac0aa8c..442d6ff 100644 --- a/packages/bumpy/src/core/publish-pipeline.ts +++ b/packages/bumpy/src/core/publish-pipeline.ts @@ -2,8 +2,7 @@ import { resolve } from 'node:path'; import { existsSync, readFileSync, writeFileSync, appendFileSync } from 'node:fs'; import { unlink } from 'node:fs/promises'; import { readJson, writeJson } from '../utils/fs.ts'; -import { runAsync } from '../utils/shell.ts'; -import { tryRun } from '../utils/shell.ts'; +import { runAsync, runArgsAsync, tryRunArgs, sq } from '../utils/shell.ts'; import { log, colorize } from '../utils/logger.ts'; import { createTag, tagExists } from './git.ts'; import { DependencyGraph } from './dep-graph.ts'; @@ -72,7 +71,7 @@ function setupNpmAuth(rootDir: string, publishManager: string): void { // Scenario 1: OIDC trusted publishing const oidcProvider = detectOidcProvider(); if (oidcProvider) { - const npmVersion = tryRun('npm --version'); + const npmVersion = tryRunArgs(['npm', '--version']); if (npmVersion) { const [major, minor, patch] = npmVersion.split('.').map(Number); const meetsMinVersion = major! > 11 || (major === 11 && (minor! > 5 || (minor === 5 && patch! >= 1))); @@ -187,7 +186,10 @@ export async function publishPackages( : [pkgConfig.publishCommand]; for (const cmd of commands) { - const expanded = cmd.replace(/\{\{version\}\}/g, release.newVersion).replace(/\{\{name\}\}/g, release.name); + // Shell-quote substituted values to prevent injection via package names/versions + const expanded = cmd + .replace(/\{\{version\}\}/g, sq(release.newVersion)) + .replace(/\{\{name\}\}/g, sq(release.name)); log.dim(` Running: ${expanded}`); if (!opts.dryRun) { await runAsync(expanded, { cwd: pkg.dir }); @@ -233,25 +235,25 @@ async function packThenPublish( packManager: PackageManager, opts: PublishOptions, ): Promise { - const packCmd = getPackCommand(packManager); - log.dim(` Packing with: ${packCmd}`); + const packArgs = getPackArgs(packManager); + log.dim(` Packing with: ${packArgs.join(' ')}`); if (opts.dryRun) { - const publishCmd = buildPublishCommand(pkg, pkgConfig, config, opts, ''); - log.dim(` Would publish with: ${publishCmd}`); + const publishArgs = buildPublishArgs(pkg, pkgConfig, config, opts, ''); + log.dim(` Would publish with: ${publishArgs.join(' ')}`); return; } // Pack and capture the tarball filename - const packOutput = await runAsync(packCmd, { cwd: pkg.dir }); + const packOutput = await runArgsAsync(packArgs, { cwd: pkg.dir }); // Pack commands output the tarball filename on the last line const tarball = parseTarballPath(packOutput, pkg.dir); try { // Publish the tarball - const publishCmd = buildPublishCommand(pkg, pkgConfig, config, opts, tarball); - log.dim(` Publishing: ${publishCmd}`); - await runAsync(publishCmd, { cwd: pkg.dir }); + const publishArgs = buildPublishArgs(pkg, pkgConfig, config, opts, tarball); + log.dim(` Publishing: ${publishArgs.join(' ')}`); + await runArgsAsync(publishArgs, { cwd: pkg.dir }); } finally { // Clean up tarball try { @@ -269,63 +271,63 @@ async function npmPublishDirect( config: BumpyConfig, opts: PublishOptions, ): Promise { - const cmd = buildPublishCommand(pkg, pkgConfig, config, opts); - log.dim(` Running: ${cmd}`); + const args = buildPublishArgs(pkg, pkgConfig, config, opts); + log.dim(` Running: ${args.join(' ')}`); if (!opts.dryRun) { - await runAsync(cmd, { cwd: pkg.dir }); + await runArgsAsync(args, { cwd: pkg.dir }); } } -function getPackCommand(pm: PackageManager): string { +function getPackArgs(pm: PackageManager): string[] { switch (pm) { case 'pnpm': - return 'pnpm pack'; + return ['pnpm', 'pack']; case 'bun': - return 'bun pm pack'; + return ['bun', 'pm', 'pack']; case 'yarn': - return 'yarn pack'; + return ['yarn', 'pack']; case 'npm': default: - return 'npm pack'; + return ['npm', 'pack']; } } -function buildPublishCommand( +function buildPublishArgs( pkg: WorkspacePackage, pkgConfig: WorkspacePackage['bumpy'] & {}, config: BumpyConfig, opts: PublishOptions, tarball?: string, -): string { +): string[] { const publishManager = config.publish.publishManager; - const parts: string[] = []; + const args: string[] = []; // Base command if (publishManager === 'yarn') { - parts.push('yarn npm publish'); + args.push('yarn', 'npm', 'publish'); } else { - parts.push(`${publishManager} publish`); + args.push(publishManager, 'publish'); } // Tarball path (if pack-then-publish) - if (tarball) parts.push(tarball); + if (tarball) args.push(tarball); // Access const access = pkgConfig?.access || config.access; - parts.push(`--access ${access}`); + args.push('--access', access); // Registry - if (pkgConfig?.registry) parts.push(`--registry ${pkgConfig.registry}`); + if (pkgConfig?.registry) args.push('--registry', pkgConfig.registry); // Dist tag - if (opts.tag) parts.push(`--tag ${opts.tag}`); + if (opts.tag) args.push('--tag', opts.tag); // Extra user-configured args (e.g., --provenance) if (config.publish.publishArgs.length > 0) { - parts.push(...config.publish.publishArgs); + args.push(...config.publish.publishArgs); } - return parts.join(' '); + return args; } /** diff --git a/packages/bumpy/src/utils/names.ts b/packages/bumpy/src/utils/names.ts index d3d7cb6..33c1d3b 100644 --- a/packages/bumpy/src/utils/names.ts +++ b/packages/bumpy/src/utils/names.ts @@ -1,8 +1,7 @@ /** Generate a random adjective-noun name for changeset files */ export function randomName(): string { - const adj = ADJECTIVES[Math.floor(Math.random() * ADJECTIVES.length)]!; - const noun = NOUNS[Math.floor(Math.random() * NOUNS.length)]!; - return `${adj}-${noun}`; + const pick = (arr: T[]) => arr[Math.floor(Math.random() * arr.length)]!; + return `${pick(ADJECTIVES)}-${pick(ADJECTIVES)}-${pick(NOUNS)}`; } /** Sanitize a user-provided name into a valid filename slug */ diff --git a/packages/bumpy/src/utils/shell.ts b/packages/bumpy/src/utils/shell.ts index 8ebd092..9a01b49 100644 --- a/packages/bumpy/src/utils/shell.ts +++ b/packages/bumpy/src/utils/shell.ts @@ -1,4 +1,15 @@ -import { execSync, exec } from 'node:child_process'; +import { execSync, execFileSync, exec, execFile } from 'node:child_process'; + +/** + * Escape a value for safe interpolation inside a single-quoted shell string. + * Works by ending the current single-quote, inserting an escaped single-quote, + * and re-opening the single-quote: "it's" → 'it'\''s' + */ +export function sq(value: string): string { + return "'" + value.replace(/'/g, "'\\''") + "'"; +} + +// ---- String-based commands (for static/trusted command strings only) ---- export function run(cmd: string, opts?: { cwd?: string; input?: string }): string { return execSync(cmd, { @@ -32,3 +43,43 @@ export function tryRun(cmd: string, opts?: { cwd?: string }): string | null { return null; } } + +// ---- Array-based commands (shell-injection safe) ---- + +/** Run a command with an argument array — bypasses the shell entirely */ +export function runArgs(args: string[], opts?: { cwd?: string; input?: string }): string { + const [cmd, ...rest] = args; + return execFileSync(cmd!, rest, { + cwd: opts?.cwd, + input: opts?.input, + encoding: 'utf-8', + stdio: [opts?.input ? 'pipe' : 'pipe', 'pipe', 'pipe'], + }).trim(); +} + +/** Async version of runArgs */ +export function runArgsAsync(args: string[], opts?: { cwd?: string; input?: string }): Promise { + const [cmd, ...rest] = args; + return new Promise((resolve, reject) => { + const child = execFile(cmd!, rest, { cwd: opts?.cwd, encoding: 'utf-8' }, (err, stdout, stderr) => { + if (err) { + reject(new Error(`Command failed: ${args.join(' ')}\n${stderr}`)); + } else { + resolve(stdout.trim()); + } + }); + if (opts?.input) { + child.stdin?.write(opts.input); + child.stdin?.end(); + } + }); +} + +/** tryRun equivalent for argument arrays */ +export function tryRunArgs(args: string[], opts?: { cwd?: string }): string | null { + try { + return runArgs(args, opts); + } catch { + return null; + } +}