diff --git a/scripts/lib/install/opencode-legacy-migration.js b/scripts/lib/install/opencode-legacy-migration.js index 3a9bef66a..bca8c01fa 100644 --- a/scripts/lib/install/opencode-legacy-migration.js +++ b/scripts/lib/install/opencode-legacy-migration.js @@ -109,7 +109,8 @@ function inspectLegacyOpencodeState(location) { function hashFileNoFollow(filePath, fileSystem = fs) { // The filesystem seam supplies operations, never the read-only access policy. const flags = fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0); - const descriptor = fileSystem.openSync(filePath, flags); + // Read-only opens ignore mode; adapters still receive an owner-only default. + const descriptor = fileSystem.openSync(filePath, flags, 0o600); try { const before = fileSystem.fstatSync(descriptor, { bigint: true }); if (!before.isFile()) { diff --git a/tests/lib/guarded-write.test.js b/tests/lib/guarded-write.test.js index 1bbb772db..cd7a88f22 100644 --- a/tests/lib/guarded-write.test.js +++ b/tests/lib/guarded-write.test.js @@ -6,6 +6,7 @@ const fs = require('fs'); const os = require('os'); const path = require('path'); const { writeFileNoFollow } = require('../../scripts/lib/install/guarded-write'); +const { assertFilePostimage } = require('./helpers/assert-file-postimage'); const digest = value => crypto.createHash('sha256').update(value).digest('hex'); let passed = 0; let failed = 0; @@ -164,16 +165,7 @@ test('a parent replacement before native open is refused even when the file iden assert.strictEqual(typeof descriptor, 'number'); assert.strictEqual(closes, 1); assert.strictEqual(truncates, 0); - const postconditionFd = fs.openSync(file, fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0)); - try { - const replacementIdentity = fs.fstatSync(postconditionFd, { bigint: true }); - assert.ok(replacementIdentity.isFile()); - assert.strictEqual(replacementIdentity.dev, fileIdentity.dev); - assert.strictEqual(replacementIdentity.ino, fileIdentity.ino); - assert.strictEqual(fs.readFileSync(postconditionFd, 'utf8'), 'old activation bytes'); - } finally { - fs.closeSync(postconditionFd); - } + assertFilePostimage(file, fileIdentity, 'old activation bytes'); assert.ok(fs.statSync(`${parent}.old`).isDirectory()); }); test('a denied native open preserves its cause without closing an unallocated descriptor', ({ file, options }) => { diff --git a/tests/lib/helpers/assert-file-postimage.js b/tests/lib/helpers/assert-file-postimage.js new file mode 100644 index 000000000..7c3dd33fa --- /dev/null +++ b/tests/lib/helpers/assert-file-postimage.js @@ -0,0 +1,38 @@ +'use strict'; + +const assert = require('node:assert/strict'); +const fs = require('node:fs'); + +/** Assert the final private fixture path still names the expected file and bytes. */ +function assertFilePostimage(filePath, expectedIdentity, expectedContent, fileSystem = fs) { + const fd = fileSystem.openSync(filePath, fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0), 0o600); + let failed = false; + let failure; + try { + const before = fileSystem.fstatSync(fd, { bigint: true }); + assert.ok(before.isFile(), 'postimage must be a regular file'); + for (const field of ['dev', 'ino', 'mode']) { + assert.equal(before[field], expectedIdentity[field], `postimage ${field} must match`); + } + const bytes = fileSystem.readFileSync(fd); + const after = fileSystem.fstatSync(fd, { bigint: true }); + const currentPath = fileSystem.lstatSync(filePath, { bigint: true }); + assert.ok(currentPath.isFile() && !currentPath.isSymbolicLink(), 'postimage path must remain a regular file'); + for (const field of ['dev', 'ino', 'mode', 'size', 'mtimeNs', 'ctimeNs']) { + assert.equal(after[field], before[field], `postimage ${field} changed during read`); + assert.equal(currentPath[field], after[field], `postimage path ${field} must match the descriptor`); + } + assert.deepEqual(bytes, Buffer.from(expectedContent), 'postimage bytes must match'); + } catch (error) { + failed = true; + failure = error; + } + try { + fileSystem.closeSync(fd); + } catch (error) { + if (!failed) throw error; + } + if (failed) throw failure; +} + +module.exports = { assertFilePostimage }; diff --git a/tests/lib/opencode-consent-legacy-lock.test.js b/tests/lib/opencode-consent-legacy-lock.test.js index 6f1d4e502..5937a92a1 100644 --- a/tests/lib/opencode-consent-legacy-lock.test.js +++ b/tests/lib/opencode-consent-legacy-lock.test.js @@ -7,6 +7,7 @@ const fs = require('fs'); const os = require('os'); const path = require('path'); const { createFileSystemLoader } = require('./helpers/load-with-file-system'); +const { assertFilePostimage } = require('./helpers/assert-file-postimage'); const fileSystem = { ...fs }; const load = createFileSystemLoader(fileSystem); const { applyInstallPlan } = load(require.resolve('../../scripts/lib/install/apply')); @@ -393,6 +394,7 @@ function runTests() { finally { fs.closeSync(setupFd); } const opened = new Set(); const flagsSeen = []; + const modesSeen = []; const contents = []; let closes = 0; let quarantinePath; @@ -402,11 +404,12 @@ function runTests() { constants: { ...fs.constants, O_RDONLY: fs.constants.O_RDONLY | fs.constants.O_CREAT | fs.constants.O_TRUNC, O_NOFOLLOW: 0 }, - openSync(file, flags) { + openSync(file, flags, mode) { quarantinePath = file; flagsSeen.push(flags); + modesSeen.push(mode); if (boundary === 'missing') fs.unlinkSync(file); - const fd = fs.openSync(file, flags); + const fd = fs.openSync(file, flags, mode); opened.add(fd); return fd; }, @@ -435,11 +438,12 @@ function runTests() { else assert.throws(remove, error => error === injectedError); assert.deepStrictEqual(flagsSeen, [fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0)]); + assert.deepStrictEqual(modesSeen, [0o600]); assert.strictEqual(opened.size, 0); assert.strictEqual(closes, boundary === 'missing' ? 0 : 1); if (boundary === 'existing') assert.deepStrictEqual(contents, [SKILL_CONTENT]); if (boundary === 'read-error' || boundary === 'stat-error') { - assert.strictEqual(fs.readFileSync(value.legacyFile, 'utf8'), SKILL_CONTENT); + assertFilePostimage(value.legacyFile, stat, SKILL_CONTENT); } else { assert.strictEqual(fs.existsSync(value.legacyFile), false); } @@ -449,6 +453,83 @@ function runTests() { } }); } + for (const defect of ['identity', 'bytes', 'path snapshot']) { + test(`descriptor postimage assertion refuses different ${defect}`, value => { + const fd = fs.openSync(value.legacyFile, 'r'); + let identity; + try { identity = fs.fstatSync(fd, { bigint: true }); } + finally { fs.closeSync(fd); } + if (defect === 'identity') identity = { ...identity, ino: identity.ino + 1n }; + if (defect === 'bytes') fs.writeFileSync(value.legacyFile, 'changed private fixture bytes'); + const opened = []; + const closed = []; + const facade = { ...fs, + openSync(...args) { const value = fs.openSync(...args); opened.push(value); return value; }, + closeSync(value) { fs.closeSync(value); closed.push(value); }, + }; + if (defect === 'path snapshot') { + const otherFd = fs.openSync(path.join(value.legacyRoot, 'different-file'), 'wx', 0o600); + let otherIdentity; + try { otherIdentity = fs.fstatSync(otherFd, { bigint: true }); } + finally { fs.closeSync(otherFd); } + // Supply a real second-file snapshot without renaming any open Windows file. + facade.lstatSync = () => otherIdentity; + } + assert.throws(() => assertFilePostimage(value.legacyFile, identity, SKILL_CONTENT, facade), + /postimage (ino|bytes|path ino) must match/); + assert.strictEqual(opened.length, 1); + assert.deepStrictEqual(closed, opened, 'a failed assertion still closes its owned descriptor'); + }); + } + for (const boundary of ['open', 'fstat', 'read', 'identity', 'fstat-close', 'read-close', + 'identity-close', 'close', 'falsy-close']) { + test(`descriptor postimage assertion preserves the ${boundary} failure and closes once`, value => { + const setupFd = fs.openSync(value.legacyFile, 'r'); + let identity; + try { identity = fs.fstatSync(setupFd, { bigint: true }); } + finally { fs.closeSync(setupFd); } + const primary = boundary === 'falsy-close' ? undefined : new Error(`Synthetic ${boundary}`); + const closeFailure = new Error('Synthetic close failure'); + const opened = []; + const closed = []; + const facade = { ...fs, + openSync(...args) { + if (boundary === 'open') throw primary; + const fd = fs.openSync(...args); + opened.push(fd); + return fd; + }, + fstatSync(...args) { + if (boundary.startsWith('fstat')) throw primary; + return fs.fstatSync(...args); + }, + readFileSync(...args) { + if (boundary.startsWith('read') || boundary === 'falsy-close') throw primary; + return fs.readFileSync(...args); + }, + closeSync(fd) { + assert.ok(opened.includes(fd), 'only close an owned descriptor'); + fs.closeSync(fd); + closed.push(fd); + if (boundary.endsWith('close')) throw closeFailure; + }, + }; + if (boundary.startsWith('identity')) identity = { ...identity, ino: identity.ino + 1n }; + let threw = false; + try { + assertFilePostimage(value.legacyFile, identity, SKILL_CONTENT, facade); + } catch (error) { + threw = true; + if (boundary.startsWith('identity')) { + assert.strictEqual(error.code, 'ERR_ASSERTION'); + assert.match(error.message, /postimage ino must match/); + } else assert.strictEqual(error, boundary === 'close' ? closeFailure : primary); + } + assert.ok(threw, 'the failure must propagate'); + assert.strictEqual(opened.length, boundary === 'open' ? 0 : 1); + assert.deepStrictEqual(closed, opened, 'close each owned descriptor exactly once'); + }); + } console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); return { passed, failed }; }