From e86dcd6a2237074ac9e94d8dca2f4d3bd427c1e1 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 16 Aug 2026 13:27:27 -0700 Subject: [PATCH] fix(version-bump): version-path and package-json-path pins cannot escape the repository MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit .gstack/version-path and .gstack/package-json-path are repo-controlled content. A cloned repo pinning '../../victim.json' — or an in-repo symlink pointing outside — turned a routine bump into an arbitrary file overwrite outside the repository. assertRepoContained rejects absolute paths, lexical .. escapes, and symlink escapes (deepest existing ancestor realpath'd, so a not-yet-created VERSION file is checked through its parent). Lockfiles that are symlinks resolving outside the repo are skipped with a warning instead of written through. Six containment tests including the not-over-broad control (subdirectory pins keep working). Co-Authored-By: Claude Fable 5 --- bin/gstack-version-bump | 81 ++++++++++++++++++++++++++---- test/gstack-version-bump.test.ts | 86 ++++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 10 deletions(-) diff --git a/bin/gstack-version-bump b/bin/gstack-version-bump index b30690dcc..cf756800e 100755 --- a/bin/gstack-version-bump +++ b/bin/gstack-version-bump @@ -49,9 +49,9 @@ // (1.67.0.0 → 1.67.0). classify judges drift against the translated form // (accepting the pre-v1.67 1:1 mirror as in-sync until the next write). -import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import { existsSync, readFileSync, realpathSync, writeFileSync } from "node:fs"; import { execFileSync } from "node:child_process"; -import { dirname, join, relative } from "node:path"; +import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; import { extractVersion, isJsonVersionPath, npmVersion, setVersionInJson } from "../lib/version-source"; // 3- or 4-digit (#2501). gstack's own VERSION stays 4-digit MAJOR.MINOR.PATCH. @@ -73,6 +73,39 @@ function argVal(args: string[], flag: string): string | undefined { return i >= 0 && i + 1 < args.length ? args[i + 1] : undefined; } +/** + * Containment guard: `.gstack/version-path` and `.gstack/package-json-path` + * are repo-controlled content. Without this, a cloned repo pinning + * `../../victim.json` — or an in-repo symlink pointing outside — turns a + * routine bump into an arbitrary file overwrite outside the repository. + * Rejects absolute paths, lexical `..` escapes, and symlink escapes (the + * deepest EXISTING ancestor is realpath'd, so a not-yet-created VERSION + * file is still checked through its parent directory). + */ +function assertRepoContained(cwd: string, rel: string, source: string): void { + const root = realpathSync(cwd); + const abs = resolve(root, rel); + const lex = relative(root, abs); + if (isAbsolute(rel) || lex === "" || lex.startsWith("..") || isAbsolute(lex)) { + fail(`${source} ('${rel}') resolves outside the repository. Refusing to read or write it.`, 2); + } + let probe = abs; + while (!existsSync(probe)) { + const parent = dirname(probe); + if (parent === probe) break; + probe = parent; + } + let real: string; + try { + real = realpathSync(probe); + } catch { + return; // vanished between existsSync and realpath — the read/write will fail honestly on its own + } + if (real !== root && !real.startsWith(root + sep)) { + fail(`${source} ('${rel}') resolves through a symlink to outside the repository. Refusing to read or write it.`, 2); + } +} + /** * Resolve the version file's path RELATIVE to the repo root: --version-path, * else .gstack/version-path, else "VERSION". @@ -87,11 +120,18 @@ function argVal(args: string[], flag: string): string | undefined { * Resolving once, here, keeps base and current reads in step. */ function resolveVersionRel(cwd: string, explicit?: string): string { - if (explicit) return explicit.trim(); + if (explicit) { + const rel = explicit.trim(); + assertRepoContained(cwd, rel, "--version-path"); + return rel; + } const pin = join(cwd, ".gstack", "version-path"); if (existsSync(pin)) { const p = readFileSync(pin, "utf-8").split("\n")[0]?.trim() ?? ""; - if (p) return p; + if (p) { + assertRepoContained(cwd, p, ".gstack/version-path"); + return p; + } } return "VERSION"; } @@ -118,11 +158,18 @@ function readVersionFile(p: string, versionRel = "VERSION"): string { * hand, which is exactly the drift this tool exists to prevent. */ function resolvePkgPath(cwd: string, explicit?: string): string { - if (explicit) return join(cwd, explicit); + if (explicit) { + const rel = explicit.trim(); + assertRepoContained(cwd, rel, "--package-json-path"); + return join(cwd, rel); + } const pin = join(cwd, ".gstack", "package-json-path"); if (existsSync(pin)) { const p = readFileSync(pin, "utf-8").split("\n")[0]?.trim() ?? ""; - if (p) return join(cwd, p); + if (p) { + assertRepoContained(cwd, p, ".gstack/package-json-path"); + return join(cwd, p); + } } return join(cwd, "package.json"); } @@ -168,11 +215,25 @@ function writePkgVersion(pkgPath: string, version: string): void { * actually honors, so both names are covered. Returns the names synced. */ const NPM_LOCKFILES = ["package-lock.json", "npm-shrinkwrap.json"]; -function syncNpmLockfiles(dir: string, version: string): string[] { +function syncNpmLockfiles(dir: string, version: string, root: string): string[] { const synced: string[] = []; for (const name of NPM_LOCKFILES) { const lockPath = join(dir, name); if (!existsSync(lockPath)) continue; + // A lockfile that is a symlink out of the repo would make this write an + // arbitrary-file overwrite (same class as the version-path pin escape). + // Skip with a warning — unlike the pins, a weird lockfile shouldn't + // brick the whole bump. + try { + const realRoot = realpathSync(root); + const realLock = realpathSync(lockPath); + if (realLock !== realRoot && !realLock.startsWith(realRoot + sep)) { + process.stderr.write(`WARNING: ${name} resolves outside the repository (symlink); not synced.\n`); + continue; + } + } catch { + continue; + } const parsed = JSON.parse(readFileSync(lockPath, "utf-8")) as Record; parsed.version = version; const packages = parsed.packages as Record> | undefined; @@ -298,7 +359,7 @@ function cmdWrite(args: string[], cwd: string): void { writeFileSync(versionPath, setVersionInJson(readFileSync(versionPath, "utf-8"), jsonV)); manifestWritten = true; // The pinned manifest's OWN lockfiles (beside it) stay in step too. - lockSynced = syncNpmLockfiles(dirname(versionPath), jsonV); + lockSynced = syncNpmLockfiles(dirname(versionPath), jsonV, cwd); } catch { fail( manifestWritten @@ -342,7 +403,7 @@ function cmdWrite(args: string[], cwd: string): void { try { writePkgVersion(pkgPath, manifestV); pkgWritten = true; - lockSynced = syncNpmLockfiles(dirname(pkgPath), manifestV); + lockSynced = syncNpmLockfiles(dirname(pkgPath), manifestV, cwd); } catch { // Accurate recovery per failure point: classify only reads // package.json (never lockfiles), so "re-run and repair" is only true @@ -398,7 +459,7 @@ function cmdRepair(args: string[], cwd: string): void { const manifestV = npmVersion(current); try { writePkgVersion(pkgPath, manifestV); - syncNpmLockfiles(dirname(pkgPath), manifestV); + syncNpmLockfiles(dirname(pkgPath), manifestV, cwd); } catch { fail("drift repair failed — could not update package.json/npm lockfiles.", 3); } diff --git a/test/gstack-version-bump.test.ts b/test/gstack-version-bump.test.ts index 0cd93e0c0..f7ea38828 100644 --- a/test/gstack-version-bump.test.ts +++ b/test/gstack-version-bump.test.ts @@ -491,3 +491,89 @@ describe('npm-valid drift contract (decision 11)', () => { expect(classifyState('1.66.0.0', '1.66.0.0', true, '9.9.9', '1.66.0')).toBe('DRIFT_UNEXPECTED'); }); }); + +describe('path containment: pins and flags cannot escape the repo', () => { + // .gstack/version-path and .gstack/package-json-path are repo-controlled + // content. A cloned repo pinning '../../victim.json' — or an in-repo + // symlink pointing outside — must never turn a bump into an arbitrary + // file overwrite outside the repository. + const outer = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-contain-')); + const dir = path.join(outer, 'repo'); + const victim = path.join(outer, 'victim.json'); + afterAll(() => { try { fs.rmSync(outer, { recursive: true, force: true }); } catch { /* noop */ } }); + + function runFail(args: string[]): { code: number; stderr: string } { + try { + execFileSync('bun', [BIN, ...args], { cwd: dir, stdio: 'pipe' }); + return { code: 0, stderr: '' }; + } catch (e: any) { + return { code: e.status, stderr: (e.stderr || '').toString() }; + } + } + + function resetRepo() { + fs.rmSync(dir, { recursive: true, force: true }); + fs.mkdirSync(path.join(dir, '.gstack'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'VERSION'), '1.0.0.0\n'); + fs.writeFileSync(victim, JSON.stringify({ version: '9.9.9' }, null, 2) + '\n'); + } + + test('a ../ escape in .gstack/version-path fails exit 2 and writes nothing', () => { + resetRepo(); + fs.writeFileSync(path.join(dir, '.gstack', 'version-path'), '../victim.json\n'); + const r = runFail(['write', '--version', '1.1.0.0']); + expect(r.code).toBe(2); + expect(r.stderr).toContain('outside the repository'); + expect(JSON.parse(fs.readFileSync(victim, 'utf-8')).version).toBe('9.9.9'); + }); + + test('an absolute path in .gstack/package-json-path fails exit 2', () => { + resetRepo(); + fs.writeFileSync(path.join(dir, '.gstack', 'package-json-path'), victim + '\n'); + const r = runFail(['write', '--version', '1.1.0.0']); + expect(r.code).toBe(2); + expect(r.stderr).toContain('outside the repository'); + expect(JSON.parse(fs.readFileSync(victim, 'utf-8')).version).toBe('9.9.9'); + }); + + test('an in-repo symlink pointing outside fails exit 2 and never follows', () => { + if (process.platform === 'win32') return; // symlink creation needs privileges there + resetRepo(); + fs.symlinkSync(victim, path.join(dir, 'link.json')); + fs.writeFileSync(path.join(dir, '.gstack', 'version-path'), 'link.json\n'); + const r = runFail(['write', '--version', '1.1.0.0']); + expect(r.code).toBe(2); + expect(r.stderr).toContain('outside the repository'); + expect(JSON.parse(fs.readFileSync(victim, 'utf-8')).version).toBe('9.9.9'); + }); + + test('classify refuses the same escapes (no read outside the repo)', () => { + resetRepo(); + fs.writeFileSync(path.join(dir, '.gstack', 'version-path'), '../victim.json\n'); + const r = runFail(['classify', '--base', 'main']); + expect(r.code).toBe(2); + expect(r.stderr).toContain('outside the repository'); + }); + + test('a lockfile symlinked outside the repo is skipped with a warning, not written', () => { + if (process.platform === 'win32') return; + resetRepo(); + const outerLock = path.join(outer, 'outer-lock.json'); + fs.writeFileSync(outerLock, JSON.stringify({ version: '1.0.0', packages: { '': { version: '1.0.0' } } }, null, 2) + '\n'); + fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name: 'x', version: '1.0.0' }, null, 2) + '\n'); + fs.symlinkSync(outerLock, path.join(dir, 'package-lock.json')); + const res = execFileSync('bun', [BIN, 'write', '--version', '1.1.0.0'], { cwd: dir, stdio: 'pipe' }); + expect(JSON.parse(res.toString()).packageLock).toBe(false); + expect(JSON.parse(fs.readFileSync(outerLock, 'utf-8')).version).toBe('1.0.0'); + }); + + test('legitimate subdirectory pins still work (containment is not over-broad)', () => { + resetRepo(); + fs.mkdirSync(path.join(dir, 'frontend'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'frontend', 'package.json'), JSON.stringify({ name: 'x', version: '1.0.0' }, null, 2) + '\n'); + fs.writeFileSync(path.join(dir, '.gstack', 'version-path'), 'frontend/package.json\n'); + const out = execFileSync('bun', [BIN, 'write', '--version', '1.1.0'], { cwd: dir }).toString(); + expect(JSON.parse(out).wrote).toBe('1.1.0'); + expect(JSON.parse(fs.readFileSync(path.join(dir, 'frontend', 'package.json'), 'utf-8')).version).toBe('1.1.0'); + }); +});