golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge remote-tracking branch 'origin/main' into codex/installer-migrations-phase-two

Browse Source 
# Conflicts:
#	bin/install.js
#	get-shit-done/bin/lib/installer-migrations.cjs
This commit is contained in:
Tom Boucher
2026-05-11 14:57:34 -04:00
parent 9889d0a9aa 68cd2449f8
commit 6a93b14186
4 changed files with 232 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
bin/install.js
View File

@@ -7129,9 +7129,35 @@ function resolveInstallRelativePath(baseDir, relPath) {
if (fullPath !== root && !fullPath.startsWith(root + path.sep)) {
return null;
}
if (hasExistingSymlinkBetween(root, fullPath)) {
return null;
}
return { relPath: normalized, fullPath };
}
function hasExistingSymlinkBetween(root, fullPath) {
const resolvedRoot = path.resolve(root);
const resolvedFullPath = path.resolve(fullPath);
if (resolvedFullPath !== resolvedRoot && !resolvedFullPath.startsWith(resolvedRoot + path.sep)) {
return true;
}
let cursor = resolvedRoot;
if (fs.existsSync(cursor) && fs.lstatSync(cursor).isSymbolicLink()) {
return true;
}
const relative = path.relative(resolvedRoot, resolvedFullPath);
for (const segment of relative.split(path.sep)) {
if (!segment) continue;
cursor = path.join(cursor, segment);
if (!fs.existsSync(cursor)) return false;
if (fs.lstatSync(cursor).isSymbolicLink()) return true;
}
return false;
}
/**
* Write file manifest after installation for future modification detection
*/
@@ -7538,6 +7564,13 @@ function install(isGlobal, runtime = 'claude') {
// Track installation failures
const failures = [];
let installerMigrationResult = null;
const rollbackInstallerMigrations = () => {
if (!installerMigrationResult || typeof installerMigrationResult.rollback !== 'function') return;
const rollback = installerMigrationResult.rollback;
installerMigrationResult = null;
rollback();
};
// Save any locally modified GSD files before they get wiped.
// The pristine context lets saveLocalPatches populate gsd-pristine/ via
@@ -7550,6 +7583,9 @@ function install(isGlobal, runtime = 'claude') {
isGlobal,
});
// Run manifest-backed cleanup migrations before package materialization.
installerMigrationResult = runInstallerMigrations({ configDir: targetDir });
// #3245 — Codex idempotent rollback. Capture pre-install state of ALL
// directories and files GSD will mutate so that any post-install validation
// failure (config.toml schema check, write failure, etc.) can revert the
@@ -7580,13 +7616,6 @@ function install(isGlobal, runtime = 'claude') {
// Map<filename, Buffer> — content snapshot of each pre-existing gsd-* agent file.
const codexPreInstallAgentContents = new Map();
let codexPreInstallVersionBytes = null;
let installerMigrationResult = null;
const rollbackInstallerMigrations = () => {
if (!installerMigrationResult || typeof installerMigrationResult.rollback !== 'function') return;
const rollback = installerMigrationResult.rollback;
installerMigrationResult = null;
rollback();
};
if (isCodex && !isMinimalMode(installMode)) {
const _preSkillsDir = path.join(targetDir, 'skills');
if (fs.existsSync(_preSkillsDir)) {
@@ -8357,6 +8386,7 @@ function install(isGlobal, runtime = 'claude') {
// #3245 CR finding 2 — any throw in the pre-config install operations (skills copy,
// agents copy, VERSION write, manifest write, etc.) triggers the Codex pre-config
// rollback so the caller is never left in a partially-installed state.
rollbackInstallerMigrations();
if (_codexPreConfigRollback) {
_codexPreConfigRollback();
}

64
get-shit-done/bin/lib/installer-migrations.cjs
View File

@@ -72,10 +72,14 @@ function normalizeRelPath(relPath) {
throw new Error('migration action relPath must be a non-empty string');
}
const normalized = relPath.replace(/\\/g, '/');
if (normalized.startsWith('/') || normalized.includes('../') || normalized === '..') {
if (path.isAbsolute(normalized) || path.win32.isAbsolute(normalized)) {
throw new Error(`migration action relPath must stay inside configDir: ${relPath}`);
}
return normalized;
const segments = normalized.split('/');
if (segments.some((segment) => segment === '' || segment === '.' || segment === '..')) {
throw new Error(`migration action relPath must stay inside configDir: ${relPath}`);
}
return segments.join('/');
}
function classifyArtifact(configDir, relPath, manifest) {
@@ -460,7 +464,13 @@ function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().t
return {
appliedMigrationIds: journal.appliedMigrationIds,
journalRelPath,
rollback: () => rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollbackRoot, previousInstallStateBytes }),
rollback: () => rollbackAppliedMigrationResult({
configDir,
journal,
journalPath,
rollbackRoot,
previousInstallStateBytes,
}),
};
} catch (error) {
const rollbackFailures = [];
@@ -487,6 +497,54 @@ function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().t
}
}
function rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollbackRoot, previousInstallStateBytes }) {
const failures = [];
for (const action of [...journal.actions].reverse()) {
if (!action.rollbackRelPath) continue;
const rollbackPath = path.join(configDir, action.rollbackRelPath);
const dest = path.join(configDir, action.relPath);
try {
if (fs.existsSync(rollbackPath)) {
fs.mkdirSync(path.dirname(dest), { recursive: true });
fs.copyFileSync(rollbackPath, dest);
}
} catch (error) {
failures.push({ relPath: action.relPath, error: error.message });
}
if (action.backupRelPath) {
try {
fs.rmSync(path.join(configDir, action.backupRelPath), { force: true });
} catch {
// backup cleanup is best-effort; preserve restore failures above
}
}
}
try {
if (previousInstallStateBytes === null) {
fs.rmSync(path.join(configDir, INSTALL_STATE_NAME), { force: true });
} else {
fs.mkdirSync(configDir, { recursive: true });
fs.writeFileSync(path.join(configDir, INSTALL_STATE_NAME), previousInstallStateBytes);
}
} catch (error) {
failures.push({ relPath: INSTALL_STATE_NAME, error: error.message });
}
try {
fs.rmSync(journalPath, { force: true });
fs.rmSync(rollbackRoot, { recursive: true, force: true });
} catch {
// journal cleanup is best-effort; the rollback above is the safety-critical part
}
if (failures.length > 0) {
const error = new Error('migration rollback incomplete');
error.rollbackFailures = failures;
throw error;
}
}
function runInstallerMigrations({
configDir,
runtime = null,

55
tests/bug-2771-user-profile-manifest.test.cjs
View File

@@ -22,6 +22,7 @@ const { describe, test, beforeEach, afterEach, before } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const path = require('path');
const crypto = require('crypto');
const { execFileSync } = require('child_process');
const { createTempDir, cleanup } = require('./helpers.cjs');
@@ -220,9 +221,16 @@ describe('#2771: USER_OWNED_ARTIFACTS is a single source of truth', () => {
describe('manifest path safety', () => {
let tmpDir;
let outside;
beforeEach(() => { tmpDir = createTempDir('gsd-manifest-path-safety-'); });
afterEach(() => { cleanup(tmpDir); });
beforeEach(() => {
tmpDir = createTempDir('gsd-manifest-path-safety-');
outside = path.join(tmpDir, '..', `outside-managed-file-${path.basename(tmpDir)}.txt`);
});
afterEach(() => {
if (outside) fs.rmSync(outside, { recursive: true, force: true });
cleanup(tmpDir);
});
test('saveLocalPatches ignores manifest entries that escape the install root', () => {
const origMode = process.env.GSD_TEST_MODE;
@@ -236,7 +244,6 @@ describe('manifest path safety', () => {
else process.env.GSD_TEST_MODE = origMode;
}
const outside = path.join(tmpDir, '..', 'outside-managed-file.txt');
fs.writeFileSync(outside, 'outside user data\n', 'utf8');
fs.writeFileSync(
path.join(tmpDir, MANIFEST_NAME),
@@ -254,6 +261,46 @@ describe('manifest path safety', () => {
assert.deepEqual(modified, []);
assert.equal(fs.readFileSync(outside, 'utf8'), 'outside user data\n');
assert.equal(fs.existsSync(path.join(tmpDir, PATCHES_DIR_NAME, '..', 'outside-managed-file.txt')), false);
assert.equal(fs.existsSync(path.join(tmpDir, PATCHES_DIR_NAME, '..', path.basename(outside))), false);
});
test('saveLocalPatches does not follow symlinked patch directories outside the install root', () => {
const origMode = process.env.GSD_TEST_MODE;
process.env.GSD_TEST_MODE = '1';
let mod;
try {
delete require.cache[require.resolve(INSTALL_SCRIPT)];
mod = require(INSTALL_SCRIPT);
} finally {
if (origMode === undefined) delete process.env.GSD_TEST_MODE;
else process.env.GSD_TEST_MODE = origMode;
}
const hookPath = path.join(tmpDir, 'hooks', 'managed.js');
fs.mkdirSync(path.dirname(hookPath), { recursive: true });
fs.writeFileSync(hookPath, 'user edited hook\n', 'utf8');
fs.writeFileSync(
path.join(tmpDir, MANIFEST_NAME),
JSON.stringify({
version: 'legacy',
timestamp: '2026-05-11T00:00:00.000Z',
files: {
'hooks/managed.js': crypto.createHash('sha256').update('managed hook\n').digest('hex'),
},
}, null, 2),
'utf8'
);
fs.mkdirSync(outside, { recursive: true });
try {
fs.symlinkSync(outside, path.join(tmpDir, PATCHES_DIR_NAME), 'dir');
} catch {
return;
}
const modified = mod.saveLocalPatches(tmpDir);
assert.deepEqual(modified, []);
assert.equal(fs.existsSync(path.join(outside, 'hooks', 'managed.js')), false);
});
});

83
tests/installer-migrations.test.cjs
View File

@@ -243,6 +243,58 @@ test('applies an unblocked plan with a journal and install-state update', () =>
}
});
test('rollback handle restores files and install state after a successful apply', () => {
const configDir = createTempInstall();
try {
writeFile(configDir, 'hooks/old-hook.js', 'managed hook\n');
writeManifest(configDir, {
'hooks/old-hook.js': sha256('managed hook\n'),
});
writeInstallState(configDir, {
schemaVersion: 1,
appliedMigrations: [
{
id: 'already-applied',
appliedAt: '2026-05-10T00:00:00.000Z',
journal: 'gsd-migration-journal/prior.json',
},
],
});
const plan = planInstallerMigrations({
configDir,
migrations: [
{
id: '2026-05-11-remove-old-hook',
description: 'Remove retired hook',
plan: () => [
{
type: 'remove-managed',
relPath: 'hooks/old-hook.js',
reason: 'retired hook',
},
],
},
],
now: () => '2026-05-11T00:00:00.000Z',
});
const result = applyInstallerMigrationPlan({
configDir,
plan,
now: () => '2026-05-11T00:00:01.000Z',
});
result.rollback();
assert.equal(fs.readFileSync(path.join(configDir, 'hooks/old-hook.js'), 'utf8'), 'managed hook\n');
assert.deepEqual(readInstallState(configDir).appliedMigrations.map((entry) => entry.id), ['already-applied']);
assert.equal(fs.existsSync(path.join(configDir, result.journalRelPath)), false);
} finally {
cleanup(configDir);
}
});
test('rolls back touched files and leaves state unchanged when apply fails', () => {
const configDir = createTempInstall();
try {
@@ -583,6 +635,37 @@ test('rejects migration actions that escape the install root', () => {
}
});
test('rejects migration actions that normalize to the install root', () => {
const configDir = createTempInstall();
try {
writeManifest(configDir, {});
for (const relPath of ['.', 'hooks/..']) {
assert.throws(
() => planInstallerMigrations({
configDir,
migrations: [
{
id: `2026-05-11-bad-path-${relPath.replace(/[^a-z0-9]/gi, '-')}`,
description: 'Bad path',
plan: () => [
{
type: 'remove-managed',
relPath,
reason: 'bad path',
},
],
},
],
}),
/relPath must stay inside configDir/
);
}
} finally {
cleanup(configDir);
}
});
test('runs discovered installer migrations against manifest-managed legacy orphan files', () => {
const configDir = createTempInstall();
try {