mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-16 01:45:29 +02:00
fix(version-bump): version-path and package-json-path pins cannot escape the repository
.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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
6bcd2ddfa4
commit
e86dcd6a22
+71
-10
@@ -49,9 +49,9 @@
|
|||||||
// (1.67.0.0 → 1.67.0). classify judges drift against the translated form
|
// (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).
|
// (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 { 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";
|
import { extractVersion, isJsonVersionPath, npmVersion, setVersionInJson } from "../lib/version-source";
|
||||||
|
|
||||||
// 3- or 4-digit (#2501). gstack's own VERSION stays 4-digit MAJOR.MINOR.PATCH.
|
// 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;
|
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,
|
* Resolve the version file's path RELATIVE to the repo root: --version-path,
|
||||||
* else .gstack/version-path, else "VERSION".
|
* 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.
|
* Resolving once, here, keeps base and current reads in step.
|
||||||
*/
|
*/
|
||||||
function resolveVersionRel(cwd: string, explicit?: string): string {
|
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");
|
const pin = join(cwd, ".gstack", "version-path");
|
||||||
if (existsSync(pin)) {
|
if (existsSync(pin)) {
|
||||||
const p = readFileSync(pin, "utf-8").split("\n")[0]?.trim() ?? "";
|
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";
|
return "VERSION";
|
||||||
}
|
}
|
||||||
@@ -118,11 +158,18 @@ function readVersionFile(p: string, versionRel = "VERSION"): string {
|
|||||||
* hand, which is exactly the drift this tool exists to prevent.
|
* hand, which is exactly the drift this tool exists to prevent.
|
||||||
*/
|
*/
|
||||||
function resolvePkgPath(cwd: string, explicit?: string): string {
|
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");
|
const pin = join(cwd, ".gstack", "package-json-path");
|
||||||
if (existsSync(pin)) {
|
if (existsSync(pin)) {
|
||||||
const p = readFileSync(pin, "utf-8").split("\n")[0]?.trim() ?? "";
|
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");
|
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.
|
* actually honors, so both names are covered. Returns the names synced.
|
||||||
*/
|
*/
|
||||||
const NPM_LOCKFILES = ["package-lock.json", "npm-shrinkwrap.json"];
|
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[] = [];
|
const synced: string[] = [];
|
||||||
for (const name of NPM_LOCKFILES) {
|
for (const name of NPM_LOCKFILES) {
|
||||||
const lockPath = join(dir, name);
|
const lockPath = join(dir, name);
|
||||||
if (!existsSync(lockPath)) continue;
|
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<string, unknown>;
|
const parsed = JSON.parse(readFileSync(lockPath, "utf-8")) as Record<string, unknown>;
|
||||||
parsed.version = version;
|
parsed.version = version;
|
||||||
const packages = parsed.packages as Record<string, Record<string, unknown>> | undefined;
|
const packages = parsed.packages as Record<string, Record<string, unknown>> | undefined;
|
||||||
@@ -298,7 +359,7 @@ function cmdWrite(args: string[], cwd: string): void {
|
|||||||
writeFileSync(versionPath, setVersionInJson(readFileSync(versionPath, "utf-8"), jsonV));
|
writeFileSync(versionPath, setVersionInJson(readFileSync(versionPath, "utf-8"), jsonV));
|
||||||
manifestWritten = true;
|
manifestWritten = true;
|
||||||
// The pinned manifest's OWN lockfiles (beside it) stay in step too.
|
// The pinned manifest's OWN lockfiles (beside it) stay in step too.
|
||||||
lockSynced = syncNpmLockfiles(dirname(versionPath), jsonV);
|
lockSynced = syncNpmLockfiles(dirname(versionPath), jsonV, cwd);
|
||||||
} catch {
|
} catch {
|
||||||
fail(
|
fail(
|
||||||
manifestWritten
|
manifestWritten
|
||||||
@@ -342,7 +403,7 @@ function cmdWrite(args: string[], cwd: string): void {
|
|||||||
try {
|
try {
|
||||||
writePkgVersion(pkgPath, manifestV);
|
writePkgVersion(pkgPath, manifestV);
|
||||||
pkgWritten = true;
|
pkgWritten = true;
|
||||||
lockSynced = syncNpmLockfiles(dirname(pkgPath), manifestV);
|
lockSynced = syncNpmLockfiles(dirname(pkgPath), manifestV, cwd);
|
||||||
} catch {
|
} catch {
|
||||||
// Accurate recovery per failure point: classify only reads
|
// Accurate recovery per failure point: classify only reads
|
||||||
// package.json (never lockfiles), so "re-run and repair" is only true
|
// 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);
|
const manifestV = npmVersion(current);
|
||||||
try {
|
try {
|
||||||
writePkgVersion(pkgPath, manifestV);
|
writePkgVersion(pkgPath, manifestV);
|
||||||
syncNpmLockfiles(dirname(pkgPath), manifestV);
|
syncNpmLockfiles(dirname(pkgPath), manifestV, cwd);
|
||||||
} catch {
|
} catch {
|
||||||
fail("drift repair failed — could not update package.json/npm lockfiles.", 3);
|
fail("drift repair failed — could not update package.json/npm lockfiles.", 3);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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');
|
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');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user