From c492920468cf64f479c587ecd88308473e292f2d Mon Sep 17 00:00:00 2001 From: affaan-m <124439313+affaan-m@users.noreply.github.com> Date: Mon, 28 Sep 2026 01:36:41 -0400 Subject: [PATCH] fix(install): bind legacy reads to trusted access flags Preserve contributor history and current-main behavior while resolving the exact reviewed follow-up. Source-PR: https://github.com/affaan-m/ECC/pull/3008 Source-Parent: ef8c2d8b9c91cb16f91e0e97106f294cc14f9e2e Review-Manifest-SHA256: 6189690fc7dd826fda286e46a5d3ec8d50b6049740f74f0dbd4490a36a76854c --- .../lib/install/opencode-legacy-migration.js | 3 +- tests/lib/guarded-write.test.js | 11 +++- .../lib/opencode-consent-legacy-lock.test.js | 65 +++++++++++++++++++ 3 files changed, 76 insertions(+), 3 deletions(-) diff --git a/scripts/lib/install/opencode-legacy-migration.js b/scripts/lib/install/opencode-legacy-migration.js index ea704b6ea..3a9bef66a 100644 --- a/scripts/lib/install/opencode-legacy-migration.js +++ b/scripts/lib/install/opencode-legacy-migration.js @@ -107,7 +107,8 @@ function inspectLegacyOpencodeState(location) { } function hashFileNoFollow(filePath, fileSystem = fs) { - const flags = fileSystem.constants.O_RDONLY | (fileSystem.constants.O_NOFOLLOW || 0); + // 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); try { const before = fileSystem.fstatSync(descriptor, { bigint: true }); diff --git a/tests/lib/guarded-write.test.js b/tests/lib/guarded-write.test.js index 3f3e35cf1..1bbb772db 100644 --- a/tests/lib/guarded-write.test.js +++ b/tests/lib/guarded-write.test.js @@ -117,8 +117,15 @@ test('a parent replacement before native open is refused even when the file iden const parent = path.join(root, 'plugins'); fs.mkdirSync(parent); const file = path.join(parent, 'entry.js'); - fs.writeFileSync(file, 'old activation bytes'); - const fileIdentity = fs.statSync(file, { bigint: true }); + const setupFd = fs.openSync(file, 'wx', 0o600); + let fileIdentity; + try { + fs.writeFileSync(setupFd, 'old activation bytes'); + fileIdentity = fs.fstatSync(setupFd, { bigint: true }); + } finally { + // Close before the parent swap, including on setup failure, for Windows. + fs.closeSync(setupFd); + } const originalOpen = fs.openSync; const originalClose = fs.closeSync; const originalTruncate = fs.ftruncateSync; diff --git a/tests/lib/opencode-consent-legacy-lock.test.js b/tests/lib/opencode-consent-legacy-lock.test.js index a0baced47..6f1d4e502 100644 --- a/tests/lib/opencode-consent-legacy-lock.test.js +++ b/tests/lib/opencode-consent-legacy-lock.test.js @@ -14,6 +14,7 @@ const { withHookConsent } = require('../../scripts/lib/install/hook-consent'); const { repairInstalledStates, buildDoctorReport } = load(require.resolve('../../scripts/lib/install-lifecycle')); const { createInstallState, readInstallState, writeInstallState } = require('../../scripts/lib/install-state'); const { withOpenCodeInstallLocks } = load(require.resolve('../../scripts/lib/install/opencode-install-lock')); +const { removeVerifiedLegacyFile } = require('../../scripts/lib/install/opencode-legacy-migration'); const SOURCE_RELATIVE_PATH = path.join('skills', 'skill-comply', 'SKILL.md'); const SKILL_CONTENT = '---\nname: skill-comply\ndescription: Synthetic migration fixture.\n---\n\n# Inert fixture\n'; @@ -384,6 +385,70 @@ function runTests() { assertBothLocksReleased(value); }); } + for (const boundary of ['existing', 'missing', 'read-error', 'stat-error']) { + test(`legacy hash uses trusted read-only flags at the ${boundary} boundary`, value => { + const setupFd = fs.openSync(value.legacyFile, 'r'); + let stat; + try { stat = fs.fstatSync(setupFd, { bigint: true }); } + finally { fs.closeSync(setupFd); } + const opened = new Set(); + const flagsSeen = []; + const contents = []; + let closes = 0; + let quarantinePath; + const injectedError = new Error(`Synthetic ${boundary}`); + const facade = { + ...fs, + constants: { ...fs.constants, + O_RDONLY: fs.constants.O_RDONLY | fs.constants.O_CREAT | fs.constants.O_TRUNC, + O_NOFOLLOW: 0 }, + openSync(file, flags) { + quarantinePath = file; + flagsSeen.push(flags); + if (boundary === 'missing') fs.unlinkSync(file); + const fd = fs.openSync(file, flags); + opened.add(fd); + return fd; + }, + fstatSync(fd, options) { + if (boundary === 'stat-error') throw injectedError; + return fs.fstatSync(fd, options); + }, + readFileSync(fd) { + if (boundary === 'read-error') throw injectedError; + const content = fs.readFileSync(fd); + contents.push(content.toString('utf8')); + return content; + }, + closeSync(fd) { + assert.ok(opened.has(fd), 'close only an owned hash descriptor'); + fs.closeSync(fd); + opened.delete(fd); + closes++; + }, + }; + try { + const remove = () => removeVerifiedLegacyFile({ destinationPath: value.legacyFile, + stat, digest: sha256(SKILL_CONTENT) }, { targetRoot: value.legacyRoot }, facade); + if (boundary === 'existing') assert.strictEqual(remove(), true); + else if (boundary === 'missing') assert.throws(remove, error => error.code === 'ENOENT'); + else assert.throws(remove, error => error === injectedError); + assert.deepStrictEqual(flagsSeen, + [fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0)]); + 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); + } else { + assert.strictEqual(fs.existsSync(value.legacyFile), false); + } + assert.strictEqual(fs.existsSync(quarantinePath), false); + } finally { + for (const fd of opened) fs.closeSync(fd); + } + }); + } console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); return { passed, failed }; }