fix: harden installer migration paths
This commit is contained in:
@@ -7199,9 +7199,35 @@ function resolveInstallRelativePath(baseDir, relPath) {
|
|||||||
if (fullPath !== root && !fullPath.startsWith(root + path.sep)) {
|
if (fullPath !== root && !fullPath.startsWith(root + path.sep)) {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
if (hasExistingSymlinkBetween(root, fullPath)) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
return { relPath: normalized, fullPath };
|
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
|
* Write file manifest after installation for future modification detection
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -56,10 +56,14 @@ function normalizeRelPath(relPath) {
|
|||||||
throw new Error('migration action relPath must be a non-empty string');
|
throw new Error('migration action relPath must be a non-empty string');
|
||||||
}
|
}
|
||||||
const normalized = relPath.replace(/\\/g, '/');
|
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}`);
|
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) {
|
function classifyArtifact(configDir, relPath, manifest) {
|
||||||
|
|||||||
@@ -22,6 +22,7 @@ const { describe, test, beforeEach, afterEach, before } = require('node:test');
|
|||||||
const assert = require('node:assert/strict');
|
const assert = require('node:assert/strict');
|
||||||
const fs = require('fs');
|
const fs = require('fs');
|
||||||
const path = require('path');
|
const path = require('path');
|
||||||
|
const crypto = require('crypto');
|
||||||
const { execFileSync } = require('child_process');
|
const { execFileSync } = require('child_process');
|
||||||
const { createTempDir, cleanup } = require('./helpers.cjs');
|
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', () => {
|
describe('manifest path safety', () => {
|
||||||
let tmpDir;
|
let tmpDir;
|
||||||
|
let outside;
|
||||||
|
|
||||||
beforeEach(() => { tmpDir = createTempDir('gsd-manifest-path-safety-'); });
|
beforeEach(() => {
|
||||||
afterEach(() => { cleanup(tmpDir); });
|
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', () => {
|
test('saveLocalPatches ignores manifest entries that escape the install root', () => {
|
||||||
const origMode = process.env.GSD_TEST_MODE;
|
const origMode = process.env.GSD_TEST_MODE;
|
||||||
@@ -236,7 +244,6 @@ describe('manifest path safety', () => {
|
|||||||
else process.env.GSD_TEST_MODE = origMode;
|
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(outside, 'outside user data\n', 'utf8');
|
||||||
fs.writeFileSync(
|
fs.writeFileSync(
|
||||||
path.join(tmpDir, MANIFEST_NAME),
|
path.join(tmpDir, MANIFEST_NAME),
|
||||||
@@ -254,6 +261,46 @@ describe('manifest path safety', () => {
|
|||||||
|
|
||||||
assert.deepEqual(modified, []);
|
assert.deepEqual(modified, []);
|
||||||
assert.equal(fs.readFileSync(outside, 'utf8'), 'outside user data\n');
|
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);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -407,6 +407,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', () => {
|
test('runs discovered installer migrations against manifest-managed legacy orphan files', () => {
|
||||||
const configDir = createTempInstall();
|
const configDir = createTempInstall();
|
||||||
try {
|
try {
|
||||||
|
|||||||
Reference in New Issue
Block a user