diff --git a/src/application/ClientConfigurationPublication.ts b/src/application/ClientConfigurationPublication.ts index 230f50d81b..d46c4725e5 100644 --- a/src/application/ClientConfigurationPublication.ts +++ b/src/application/ClientConfigurationPublication.ts @@ -1,16 +1,9 @@ import { constants, type Stats } from "node:fs"; -import { - link, - lstat, - mkdtemp, - open, - realpath, - rename, - type FileHandle, -} from "node:fs/promises"; +import { link, lstat, mkdtemp, open, realpath, rename } from "node:fs/promises"; import { basename, dirname, join, resolve, sep } from "node:path"; import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; +import { fileHandleMatchesBytes } from "../filesystem/fileHandleMatchesBytes.js"; import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import { OwnedStagedOutput } from "../filesystem/OwnedStagedOutput.js"; import { @@ -160,7 +153,10 @@ export class ClientConfigurationPublication { !before.isFile() || named.isSymbolicLink() || !sameRegularFileState(before, named) || - !(await this.#hasPublishedBytes(handle)) || + !(await fileHandleMatchesBytes( + handle, + Buffer.from(this.contents, "utf8"), + )) || !sameRegularFileState(before, await handle.stat()) || !sameRegularFileState(before, await lstat(this.path)) ) @@ -185,29 +181,6 @@ export class ClientConfigurationPublication { if (!sameRegularFileState(await handle.stat(), await lstat(this.path))) throw new RegularFileChangedError(this.path); } - - async #hasPublishedBytes(handle: FileHandle): Promise { - const expected = Buffer.from(this.contents, "utf8"); - if ((await handle.stat()).size !== expected.length) return false; - const chunk = Buffer.allocUnsafe(Math.min(64 * 1024, expected.length)); - for (let position = 0; position < expected.length;) { - const { bytesRead } = await handle.read( - chunk, - 0, - Math.min(chunk.length, expected.length - position), - position, - ); - if ( - bytesRead === 0 || - !chunk - .subarray(0, bytesRead) - .equals(expected.subarray(position, position + bytesRead)) - ) - return false; - position += bytesRead; - } - return true; - } } const assertRequestedConfigurationPath = async ( diff --git a/src/application/SkillFilePublication.ts b/src/application/SkillFilePublication.ts index 0cc4977381..b07a1dff6e 100644 --- a/src/application/SkillFilePublication.ts +++ b/src/application/SkillFilePublication.ts @@ -3,6 +3,7 @@ import { link, lstat, mkdtemp, open, realpath, rename } from "node:fs/promises"; import { basename, dirname, join } from "node:path"; import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; +import { fileHandleMatchesBytes } from "../filesystem/fileHandleMatchesBytes.js"; import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import { OwnedStagedOutput } from "../filesystem/OwnedStagedOutput.js"; import { @@ -163,7 +164,7 @@ export class SkillFilePublication { ) throw new RegularFileChangedError(this.path); await this.#assertNamed(before); - if (!(await this.#matches(this.bytes))) + if (!(await fileHandleMatchesBytes(this.owner.handle, this.bytes))) throw new RegularFileChangedError(this.path); if (!sameRegularFileState(before, await this.owner.handle.stat())) throw new RegularFileChangedError(this.path); @@ -203,7 +204,7 @@ export class SkillFilePublication { await this.owner.handle.chmod(original.mode); await this.owner.handle.sync(); await this.#assertNamed(await this.owner.handle.stat()); - if (!(await this.#matches(original.bytes))) + if (!(await fileHandleMatchesBytes(this.owner.handle, original.bytes))) throw new RegularFileChangedError(this.path); this.#restored = true; } @@ -222,28 +223,6 @@ export class SkillFilePublication { throw new RegularFileChangedError(this.path); } - async #matches(expected: Buffer): Promise { - if ((await this.owner.handle.stat()).size !== expected.length) return false; - const chunk = Buffer.allocUnsafe(Math.min(64 * 1024, expected.length)); - for (let position = 0; position < expected.length;) { - const { bytesRead } = await this.owner.handle.read( - chunk, - 0, - Math.min(chunk.length, expected.length - position), - position, - ); - if ( - bytesRead === 0 || - !chunk - .subarray(0, bytesRead) - .equals(expected.subarray(position, position + bytesRead)) - ) - return false; - position += bytesRead; - } - return true; - } - async #assertSelected(): Promise { const current = await optionalState(this.path); if ( diff --git a/src/filesystem/fileHandleMatchesBytes.ts b/src/filesystem/fileHandleMatchesBytes.ts new file mode 100644 index 0000000000..3883e944d1 --- /dev/null +++ b/src/filesystem/fileHandleMatchesBytes.ts @@ -0,0 +1,27 @@ +import type { FileHandle } from "node:fs/promises"; + +/** Compare descriptor bytes without taking ownership or validating pathname identity. */ +export const fileHandleMatchesBytes = async ( + handle: Pick, + expected: Buffer, +): Promise => { + if ((await handle.stat()).size !== expected.length) return false; + const chunk = Buffer.allocUnsafe(Math.min(64 * 1024, expected.length)); + for (let position = 0; position < expected.length;) { + const { bytesRead } = await handle.read( + chunk, + 0, + Math.min(chunk.length, expected.length - position), + position, + ); + if ( + bytesRead === 0 || + !chunk + .subarray(0, bytesRead) + .equals(expected.subarray(position, position + bytesRead)) + ) + return false; + position += bytesRead; + } + return true; +}; diff --git a/tests/boundary/filesystem/setupSkillPublication.test.ts b/tests/boundary/filesystem/setupSkillPublication.test.ts index 336d88e1b2..a25797eb39 100644 --- a/tests/boundary/filesystem/setupSkillPublication.test.ts +++ b/tests/boundary/filesystem/setupSkillPublication.test.ts @@ -123,6 +123,7 @@ describe("skill publication ownership", () => { describe("skill publication recovery", () => { it.each([ + Buffer.alloc(0), Buffer.from([0xff, 0xfe, 0x61, 0x00]), Buffer.from("\ufefforiginal BOM instructions\r\n"), ])(