Merge pull request #24 from dmno-dev/fix/security-hardening

Harden security across publishing, auth, and input validation

Author: theoephraim

.bumpy/security-hardening.md

@@ -0,0 +1,5 @@
+---
+'@varlock/bumpy': patch
+---
+
+Security hardening: ephemeral git token auth, custom command gating via allowCustomCommands, bump file input validation, structured tarball path parsing, changelog formatter path traversal fix, and force-push safeguard

docs/cli.md

@@ -84,7 +84,7 @@ bumpy publish --filter "@myorg/*"
 
 **How bumpy detects unpublished packages:**
 
-1. Custom `checkPublished` command (if configured per-package)
+1. Custom `checkPublished` command (if configured per-package — see [`allowCustomCommands`](./configuration.md#custom-commands-and-allowcustomcommands))
 2. Git tags (for packages with `skipNpmPublish` or custom `publishCommand`)
 3. npm registry query (default)
 

docs/configuration.md

@@ -21,6 +21,7 @@ Bumpy is configured via `.bumpy/_config.json`, created by `bumpy init`. Per-pack
 | `publish`                    | `object`                               | see below                        | Publishing pipeline config                                                          |
 | `gitUser`                    | `{ name, email }`                      | bumpy-bot                        | Git identity for CI commits                                                         |
 | `versionPr`                  | `{ title, branch, preamble }`          | see below                        | Customize the version PR                                                            |
+| `allowCustomCommands`        | `boolean \| string[]`                  | `false`                          | Allow per-package custom commands from `package.json` (see below)                   |
 | `packages`                   | `object`                               | `{}`                             | Per-package config overrides (keyed by package name)                                |
 
 ### Dependency bump rules
@@ -87,8 +88,48 @@ Per-package settings can be defined in two places:
 | `dependencyBumpRules` | `object`                   | Per-package override for dependency propagation rules                   |
 | `cascadeTo`           | `object`                   | Explicit cascade targets — glob pattern mapped to `{ trigger, bumpAs }` |
 
+### Custom commands and `allowCustomCommands`
+
+The `publishCommand`, `buildCommand`, and `checkPublished` fields run shell commands during publishing. Because these execute with CI credentials, bumpy distinguishes between two trust levels:
+
+- **Root config** (`.bumpy/_config.json` → `packages`): always trusted — repo admins control this file.
+- **Per-package config** (`package.json` → `"bumpy"`): requires opt-in via `allowCustomCommands` in the root config.
+
+By default, custom commands defined in `package.json` are **ignored** with a warning. To enable them, set `allowCustomCommands` in `.bumpy/_config.json`:
+
+```json
+{
+  "allowCustomCommands": true
+}
+```
+
+Or restrict to specific packages/globs:
+
+```json
+{
+  "allowCustomCommands": ["@myorg/vscode-extension", "@myorg/deploy-*"]
+}
+```
+
+This prevents a contributor from introducing arbitrary shell commands via a package's `package.json` without the root config explicitly allowing it.
+
 ### Example: custom publish for a VSCode extension
 
+In `.bumpy/_config.json` (recommended — no `allowCustomCommands` needed):
+
+```json
+{
+  "packages": {
+    "my-vscode-extension": {
+      "publishCommand": "vsce publish",
+      "skipNpmPublish": true
+    }
+  }
+}
+```
+
+Or in the package's `package.json` (requires `allowCustomCommands`):
+
 ```json
 {
   "name": "my-vscode-extension",
@@ -137,6 +178,7 @@ See the [Changelog Formatters](./changelog-formatters.md) docs for full details
       "publishCommand": "vsce publish",
       "skipNpmPublish": true
     }
-  }
+  },
+  "allowCustomCommands": ["@myorg/deploy-*"]
 }
 ```

packages/bumpy/src/commands/ci.ts

@@ -229,20 +229,28 @@ async function autoPublish(rootDir: string, config: BumpyConfig, tag?: string):
  * but PR workflows won't be triggered automatically.
  */
 function pushWithToken(rootDir: string, branch: string): void {
+  // Guard against misconfigured versionPr.branch pointing at the base branch
+  const baseBranch = tryRunArgs(['git', 'rev-parse', '--abbrev-ref', 'HEAD'], { cwd: rootDir });
+  if (branch === baseBranch || branch === 'main' || branch === 'master') {
+    throw new Error(`Refusing to force-push to "${branch}" — this looks like a base branch, not a version PR branch`);
+  }
+
   const token = process.env.BUMPY_GH_TOKEN;
   const repo = process.env.GITHUB_REPOSITORY; // e.g. "owner/repo"
   const server = process.env.GITHUB_SERVER_URL || 'https://github.com';
 
   if (token && repo) {
-    const authedUrl = `${server.replace('://', `://x-access-token:${token}@`)}/${repo}.git`;
-    const originalUrl = tryRunArgs(['git', 'remote', 'get-url', 'origin'], { cwd: rootDir });
+    // Use an ephemeral `-c` flag to inject auth so the token never touches .git/config.
+    // GitHub accepts HTTP basic auth with "x-access-token" as the username.
+    const basicAuth = Buffer.from(`x-access-token:${token}`).toString('base64');
+    const extraHeaderKey = `http.${server}/.extraheader`;
+    const authHeader = `Authorization: basic ${basicAuth}`;
 
     // `actions/checkout@v6` persists the default GITHUB_TOKEN in two ways:
     //   1. Direct http.<server>/.extraheader config
     //   2. includeIf.gitdir entries pointing to a credentials config file
     //      that also sets http.<server>/.extraheader
     // Both must be cleared for our custom token to be used.
-    const extraHeaderKey = `http.${server}/.extraheader`;
     const savedHeader = tryRunArgs(['git', 'config', '--local', extraHeaderKey], { cwd: rootDir });
 
     // Collect includeIf entries that point to credential config files
@@ -266,13 +274,12 @@ function pushWithToken(rootDir: string, branch: string): void {
       for (const entry of savedIncludeIfs) {
         tryRunArgs(['git', 'config', '--local', '--unset', entry.key], { cwd: rootDir });
       }
-      runArgs(['git', 'remote', 'set-url', 'origin', authedUrl], { cwd: rootDir });
-      runArgs(['git', 'push', '-u', 'origin', branch, '--force'], { cwd: rootDir });
+      // Pass auth via ephemeral -c flag — never written to .git/config
+      runArgs(['git', '-c', `${extraHeaderKey}=${authHeader}`, 'push', '-u', 'origin', branch, '--force'], {
+        cwd: rootDir,
+      });
     } finally {
-      // Restore original URL, extraheader, and includeIf entries
-      if (originalUrl) {
-        runArgs(['git', 'remote', 'set-url', 'origin', originalUrl], { cwd: rootDir });
-      }
+      // Restore extraheader and includeIf entries cleared above
       if (savedHeader) {
         runArgs(['git', 'config', '--local', extraHeaderKey, savedHeader], { cwd: rootDir });
       }

packages/bumpy/src/core/bump-file.ts

@@ -4,6 +4,24 @@ import { readText, writeText, listFiles, removeFile } from '../utils/fs.ts';
 import { getBumpyDir } from './config.ts';
 import { tryRunArgs } from '../utils/shell.ts';
 import type { BumpFile, BumpFileRelease, BumpFileReleaseCascade, BumpType, BumpTypeWithNone } from '../types.ts';
+import { log } from '../utils/logger.ts';
+
+const VALID_BUMP_TYPES = new Set<string>(['major', 'minor', 'patch', 'none']);
+
+/**
+ * Reject package names that contain characters which could cause injection
+ * when used in git tags, markdown, URLs, or shell-quoted strings.
+ * Intentionally permissive — we don't enforce npm naming rules because
+ * bumpy may be used with other registries or non-JS packages.
+ */
+function validatePackageName(name: string): boolean {
+  if (!name || name.length > 214) return false;
+  // disallow control chars, HTML/shell metacharacters, whitespace
+  if (/[\u0000-\u001f\u007f<>"'`&;|$(){}[\]\\!#%\s]/.test(name)) return false;
+  // must not start with - (could be interpreted as a CLI flag)
+  if (name.startsWith('-')) return false;
+  return true;
+}
 
 /** Read all bump files from .bumpy/ directory, sorted by git creation order */
 export async function readBumpFiles(rootDir: string): Promise<BumpFile[]> {
@@ -81,12 +99,25 @@ export function parseBumpFile(content: string, id: string): BumpFile | null {
 
   const releases: BumpFileRelease[] = [];
   for (const [name, value] of Object.entries(parsed)) {
+    if (!validatePackageName(name)) {
+      log.warn(`Skipping invalid package name in bump file "${id}": ${name}`);
+      continue;
+    }
+
     if (typeof value === 'string') {
+      if (!VALID_BUMP_TYPES.has(value)) {
+        log.warn(`Skipping unknown bump type "${value}" for ${name} in bump file "${id}"`);
+        continue;
+      }
       // Simple format: "pkg-name": minor
       releases.push({ name, type: value as BumpTypeWithNone });
     } else if (value && typeof value === 'object') {
       // Nested format: "pkg-name": { bump: minor, cascade: { ... } }
       const obj = value as { bump: BumpTypeWithNone; cascade?: Record<string, BumpType> };
+      if (!VALID_BUMP_TYPES.has(obj.bump)) {
+        log.warn(`Skipping unknown bump type "${obj.bump}" for ${name} in bump file "${id}"`);
+        continue;
+      }
       const release: BumpFileReleaseCascade = {
         name,
         type: obj.bump,

packages/bumpy/src/core/changelog.ts

@@ -1,4 +1,5 @@
-import { resolve } from 'node:path';
+import { resolve, relative } from 'node:path';
+import { realpathSync } from 'node:fs';
 import { log } from '../utils/logger.ts';
 import type { BumpFile, PlannedRelease, BumpyConfig } from '../types.ts';
 
@@ -95,9 +96,15 @@ export async function loadFormatter(changelog: BumpyConfig['changelog'], rootDir
     try {
       let modulePath: string;
       if (name.startsWith('.')) {
-        // Relative path — resolve and verify it stays within the project root
+        // Relative path — resolve symlinks and verify it stays within the project root
         modulePath = resolve(rootDir, name);
-        if (!modulePath.startsWith(rootDir + '/')) {
+        try {
+          modulePath = realpathSync(modulePath);
+        } catch {
+          // File doesn't exist yet — use the resolved path as-is
+        }
+        const rel = relative(realpathSync(rootDir), modulePath);
+        if (rel.startsWith('..') || resolve('/', rel) === resolve('/')) {
           throw new Error(`Changelog formatter path "${name}" resolves outside the project root`);
         }
       } else {

packages/bumpy/src/core/config.ts

@@ -57,6 +57,22 @@ export async function loadPackageConfig(
     // ignore
   }
 
+  // Block custom commands from per-package config unless the root explicitly allows them.
+  // Commands defined in the root config's `packages` map are always trusted.
+  const CUSTOM_CMD_KEYS = ['buildCommand', 'publishCommand', 'checkPublished'] as const;
+  const disallowedKeys = CUSTOM_CMD_KEYS.filter((k) => pkgJsonConfig[k] != null);
+  if (disallowedKeys.length > 0 && !isCustomCommandAllowed(pkgName, rootConfig)) {
+    const fields = disallowedKeys.map((k) => `"${k}"`).join(', ');
+    throw new Error(
+      `Package "${pkgName}" defines custom command(s) (${fields}) in its package.json "bumpy" config, ` +
+        'but the root config does not allow this.\n' +
+        'Custom commands execute shell commands during publishing and must be explicitly enabled.\n\n' +
+        'To fix this, either:\n' +
+        '  1. Move the command(s) to .bumpy/_config.json under "packages" (always trusted)\n' +
+        `  2. Add "allowCustomCommands": true (or ["${pkgName}"]) to .bumpy/_config.json`,
+    );
+  }
+
   // Merge: root packages map → package.json["bumpy"] (later wins)
   return mergePackageConfig(rootPkgConfig, pkgJsonConfig);
 }
@@ -124,6 +140,16 @@ function mergePackageConfig(...configs: PackageConfig[]): PackageConfig {
   return result;
 }
 
+/** Check if a package is allowed to define custom commands via package.json */
+function isCustomCommandAllowed(pkgName: string, config: BumpyConfig): boolean {
+  const { allowCustomCommands } = config;
+  if (allowCustomCommands === true) return true;
+  if (Array.isArray(allowCustomCommands)) {
+    return allowCustomCommands.some((pattern) => matchGlob(pkgName, pattern));
+  }
+  return false;
+}
+
 export function getBumpyDir(rootDir: string): string {
   return resolve(rootDir, BUMPY_DIR);
 }

packages/bumpy/src/core/publish-pipeline.ts

@@ -246,8 +246,7 @@ async function packThenPublish(
 
   // Pack and capture the tarball filename
   const packOutput = await runArgsAsync(packArgs, { cwd: pkg.dir });
-  // Pack commands output the tarball filename on the last line
-  const tarball = parseTarballPath(packOutput, pkg.dir);
+  const tarball = parseTarballPath(packOutput, pkg.dir, packManager);
 
   try {
     // Publish the tarball
@@ -281,14 +280,14 @@ async function npmPublishDirect(
 function getPackArgs(pm: PackageManager): string[] {
   switch (pm) {
     case 'pnpm':
-      return ['pnpm', 'pack'];
+      return ['pnpm', 'pack', '--json'];
     case 'bun':
       return ['bun', 'pm', 'pack'];
     case 'yarn':
       return ['yarn', 'pack'];
     case 'npm':
     default:
-      return ['npm', 'pack'];
+      return ['npm', 'pack', '--json'];
   }
 }
 
@@ -332,20 +331,32 @@ function buildPublishArgs(
 
 /**
  * Parse the tarball path from pack command output.
- * Each PM has different output formats:
- *   npm/pnpm: tarball filename on the last line
- *   bun:      tarball filename mid-output, summary lines after
- *   yarn:     'success Wrote tarball to "/path/to/foo.tgz".'
+ * npm/pnpm use --json for structured output; bun/yarn fall back to regex parsing.
  */
-function parseTarballPath(output: string, cwd: string): string {
-  // Extract any .tgz path — handles both bare filenames and quoted paths (yarn)
+function parseTarballPath(output: string, cwd: string, pm: PackageManager): string {
+  // npm and pnpm support --json which gives us a deterministic filename
+  if (pm === 'npm' || pm === 'pnpm') {
+    try {
+      const parsed = JSON.parse(output);
+      // npm returns an array, pnpm returns an object or array
+      const entry = Array.isArray(parsed) ? parsed[0] : parsed;
+      if (entry?.filename) {
+        return resolve(cwd, entry.filename);
+      }
+    } catch {
+      // JSON parse failed — fall through to regex
+    }
+  }
+
+  // Fallback for bun/yarn or if JSON parsing failed:
+  // extract any .tgz path — handles both bare filenames and quoted paths (yarn)
   const tgzMatch = output.match(/(?:^|["'\s])([^\s"']*\.tgz)/m);
   if (tgzMatch) {
     const tarball = tgzMatch[1]!;
     return tarball.startsWith('/') ? tarball : resolve(cwd, tarball);
   }
 
-  // Fallback: last non-empty line
+  // Last resort: last non-empty line
   const lines = output.trim().split('\n').filter(Boolean);
   const lastLine = lines[lines.length - 1]?.trim() || '';
   return lastLine.startsWith('/') ? lastLine : resolve(cwd, lastLine);

packages/bumpy/src/types.ts

@@ -70,6 +70,16 @@ export interface BumpyConfig {
   updateInternalDependencies: 'patch' | 'minor' | 'out-of-range';
   dependencyBumpRules: Partial<Record<DepType, DependencyBumpRule | false>>;
   privatePackages: { version: boolean; tag: boolean };
+  /**
+   * Allow per-package custom commands (buildCommand, publishCommand, checkPublished)
+   * defined in package.json "bumpy" fields.
+   * Commands defined in the root config's `packages` map are always trusted.
+   *
+   * true = allow all packages to define custom commands
+   * string[] = allow only matching package names/globs
+   * false = only root-config commands are allowed (default)
+   */
+  allowCustomCommands: boolean | string[];
   packages: Record<string, PackageConfig>;
   publish: PublishConfig;
   /**
@@ -125,6 +135,7 @@ export const DEFAULT_CONFIG: BumpyConfig = {
   updateInternalDependencies: 'out-of-range',
   dependencyBumpRules: {},
   privatePackages: { version: false, tag: false },
+  allowCustomCommands: false,
   packages: {},
   publish: { ...DEFAULT_PUBLISH_CONFIG },
   aggregateRelease: false,