diff --git a/src/artifacts/ArtifactReader.ts b/src/artifacts/ArtifactReader.ts index 78723823a..1278d815d 100644 --- a/src/artifacts/ArtifactReader.ts +++ b/src/artifacts/ArtifactReader.ts @@ -31,6 +31,37 @@ export interface ArtifactEntry { }; } +/** Snapshot mutable entry metadata before the producer later validates open(). */ +export const copyArtifactEntry = (entry: ArtifactEntry): ArtifactEntry => ({ + ...entry, + limitations: [...entry.limitations], + ...(entry.sourceIdentity === undefined + ? {} + : { sourceIdentity: { ...entry.sourceIdentity } }), +}); + +/** Compare every portable and adapter-owned fact supplied with an entry. */ +export const sameArtifactEntry = ( + entry: ArtifactEntry, + produced: ArtifactEntry, +): boolean => + entry.path === produced.path && + entry.kind === produced.kind && + entry.declaredSize === produced.declaredSize && + entry.compressedSize === produced.compressedSize && + entry.executable === produced.executable && + entry.encrypted === produced.encrypted && + entry.byteOffset === produced.byteOffset && + entry.declaredSha256 === produced.declaredSha256 && + entry.unpacked === produced.unpacked && + entry.adapterKey === produced.adapterKey && + entry.limitations.length === produced.limitations.length && + entry.limitations.every( + (limitation, index) => limitation === produced.limitations[index], + ) && + entry.sourceIdentity?.device === produced.sourceIdentity?.device && + entry.sourceIdentity?.inode === produced.sourceIdentity?.inode; + /** Read-only adapter over one directory, archive, or virtual container. */ export interface ArtifactReader { readonly format: "directory" | ZipPackageFormat | "asar" | "file"; diff --git a/src/artifacts/AsarArtifactReader.ts b/src/artifacts/AsarArtifactReader.ts index 1408bce8a..60c298e97 100644 --- a/src/artifacts/AsarArtifactReader.ts +++ b/src/artifacts/AsarArtifactReader.ts @@ -23,6 +23,8 @@ import { listPackage, statFile, uncache } from "@electron/asar"; import { ArtifactReaderFailure, + copyArtifactEntry, + sameArtifactEntry, type ArtifactEntry, type ArtifactReader, } from "./ArtifactReader.js"; @@ -490,35 +492,6 @@ const asarUnpackedEntryFailure = ( return asarFailure(archivePath, `read ${entry.path}`, cause); }; -const copyArtifactEntry = (entry: ArtifactEntry): ArtifactEntry => ({ - ...entry, - limitations: [...entry.limitations], - ...(entry.sourceIdentity === undefined - ? {} - : { sourceIdentity: { ...entry.sourceIdentity } }), -}); - -const sameArtifactEntry = ( - entry: ArtifactEntry, - produced: ArtifactEntry, -): boolean => - entry.path === produced.path && - entry.kind === produced.kind && - entry.declaredSize === produced.declaredSize && - entry.compressedSize === produced.compressedSize && - entry.executable === produced.executable && - entry.encrypted === produced.encrypted && - entry.byteOffset === produced.byteOffset && - entry.declaredSha256 === produced.declaredSha256 && - entry.unpacked === produced.unpacked && - entry.adapterKey === produced.adapterKey && - entry.limitations.length === produced.limitations.length && - entry.limitations.every( - (limitation, index) => limitation === produced.limitations[index], - ) && - entry.sourceIdentity?.device === produced.sourceIdentity?.device && - entry.sourceIdentity?.inode === produced.sourceIdentity?.inode; - const isAsarFileMetadata = ( metadata: ReturnType, ): metadata is AsarFileMetadata => diff --git a/src/artifacts/MachOSliceArtifactReader.ts b/src/artifacts/MachOSliceArtifactReader.ts index f306025c8..814099296 100644 --- a/src/artifacts/MachOSliceArtifactReader.ts +++ b/src/artifacts/MachOSliceArtifactReader.ts @@ -15,6 +15,8 @@ import type { MachoSlice } from "../domain/apple/dylibResolution.js"; import { readMachoImage, type ReadAt } from "./apple/MachoLoadCommandReader.js"; import { ArtifactReaderFailure, + copyArtifactEntry, + sameArtifactEntry, type ArtifactEntry, type ArtifactReader, } from "./ArtifactReader.js"; @@ -31,6 +33,7 @@ export class MachOSliceArtifactReader implements ArtifactReader { #command: ArtifactCommand | undefined; #source: StableRegularFileDescriptor | undefined; #pendingSource: Promise | undefined; + readonly #entries = new Map(); readonly #ownsSource: boolean; #closed = false; #closePromise: Promise | undefined; @@ -48,6 +51,7 @@ export class MachOSliceArtifactReader implements ArtifactReader { } async *entries(signal?: AbortSignal): AsyncIterable { + this.#entries.clear(); const source = await this.#ensureSource(); await this.#verifySource(source); const captured = await this.runner.run( @@ -127,7 +131,7 @@ export class MachOSliceArtifactReader implements ArtifactReader { "integrity", `lipo reported an out-of-bounds Mach-O slice: ${architecture.name}`, ); - yield { + const entry: ArtifactEntry = { path: `slices/${architecture.name}`, kind: "slice", declaredSize: sliceSize, @@ -163,6 +167,8 @@ export class MachOSliceArtifactReader implements ArtifactReader { ], adapterKey: `${String(offset)}:${String(sliceSize)}`, }; + this.#entries.set(entry.adapterKey, copyArtifactEntry(entry)); + yield entry; } } @@ -171,14 +177,19 @@ export class MachOSliceArtifactReader implements ArtifactReader { return Promise.reject( new ArtifactReaderFailure("cancelled", "Mach-O slice read cancelled"), ); - const [offsetText, sizeText] = entry.adapterKey.split(":"); - const offset = parseSliceKeyInteger(offsetText); - const size = parseSliceKeyInteger(sizeText); + const produced = this.#entries.get(entry.adapterKey); + if (produced === undefined || !sameArtifactEntry(entry, produced)) + throw new ArtifactReaderFailure( + "integrity", + "Mach-O slice entry metadata does not match this reader's inventory", + ); + const offset = produced.byteOffset; + const size = produced.declaredSize; if ( - !Number.isSafeInteger(offset) || - !Number.isSafeInteger(size) || offset === null || size === null || + !Number.isSafeInteger(offset) || + !Number.isSafeInteger(size) || offset < 0 || size <= 0 ) @@ -357,9 +368,3 @@ const lipoMatchesSlice = ( const lipoNameMatches = (reported: string, observed: string): boolean => reported === observed || (observed === "arm64e" && /^arm64e\.[A-Za-z0-9_]+$/u.test(reported)); - -const parseSliceKeyInteger = (value: string | undefined): number | null => { - if (value === undefined || !/^\d+$/u.test(value)) return null; - const parsed = Number(value); - return Number.isSafeInteger(parsed) ? parsed : null; -}; diff --git a/src/artifacts/ZipArtifactReader.ts b/src/artifacts/ZipArtifactReader.ts index 17637ca45..ca3988a70 100644 --- a/src/artifacts/ZipArtifactReader.ts +++ b/src/artifacts/ZipArtifactReader.ts @@ -6,6 +6,8 @@ import { Reader, ZipReader, type Entry, type FileEntry } from "@zip.js/zip.js"; import { ArtifactReaderFailure, + copyArtifactEntry, + sameArtifactEntry, type ArtifactEntry, type ArtifactReader, } from "./ArtifactReader.js"; @@ -97,7 +99,10 @@ export class ZipArtifactReader implements ArtifactReader { readonly format: ZipPackageFormat; readonly #source: NodeFileReader; readonly #reader: ZipReader; - readonly #entries = new Map(); + readonly #entries = new Map< + string, + { readonly source: Entry; readonly metadata: ArtifactEntry } + >(); /** Optionally bound each metadata read before allocating its backing buffer. */ constructor( @@ -120,39 +125,38 @@ export class ZipArtifactReader implements ArtifactReader { async *entries(signal?: AbortSignal): AsyncIterable { for await (const entry of this.#reader.getEntriesGenerator()) { abortIfNeeded(signal); - this.#entries.set(entry.filename, entry); - const symlink = isSymlink(entry); - yield { - path: entry.filename, - kind: entry.directory ? "directory" : symlink ? "symlink" : "file", - declaredSize: entry.directory ? null : entry.uncompressedSize, - compressedSize: entry.directory ? null : entry.compressedSize, - executable: entry.executable, - encrypted: entry.encrypted, - byteOffset: null, - declaredSha256: null, - unpacked: false, - limitations: symlink ? ["Archive symlink target was not read."] : [], - adapterKey: entry.filename, - }; + const metadata = projectZipEntry(entry); + this.#entries.set(entry.filename, { + source: entry, + metadata: copyArtifactEntry(metadata), + }); + yield metadata; } } open(entry: ArtifactEntry, signal?: AbortSignal): Promise { abortIfNeeded(signal); const stored = this.#entries.get(entry.adapterKey); - if (stored === undefined || stored.directory || isSymlink(stored)) + if (stored === undefined || !sameArtifactEntry(entry, stored.metadata)) + return Promise.reject( + new ArtifactReaderFailure( + "integrity", + "ZIP entry metadata does not match this reader's inventory", + ), + ); + const source = stored.source; + if (source.directory || isSymlink(source)) return Promise.reject( new ArtifactReaderFailure("format", "ZIP entry is not a regular file"), ); - if (stored.encrypted) + if (source.encrypted) return Promise.reject( new ArtifactReaderFailure( "unavailable", "Encrypted ZIP entry is unsupported", ), ); - return Promise.resolve(extractStream(stored, signal)); + return Promise.resolve(extractStream(source, signal)); } async close(): Promise { @@ -166,6 +170,23 @@ export class ZipArtifactReader implements ArtifactReader { } } +const projectZipEntry = (entry: Entry): ArtifactEntry => { + const symlink = isSymlink(entry); + return { + path: entry.filename, + kind: entry.directory ? "directory" : symlink ? "symlink" : "file", + declaredSize: entry.directory ? null : entry.uncompressedSize, + compressedSize: entry.directory ? null : entry.compressedSize, + executable: entry.executable, + encrypted: entry.encrypted, + byteOffset: null, + declaredSha256: null, + unpacked: false, + limitations: symlink ? ["Archive symlink target was not read."] : [], + adapterKey: entry.filename, + }; +}; + const extractStream = (entry: FileEntry, signal?: AbortSignal): Readable => { const output = new PassThrough(); const controller = new AbortController(); diff --git a/tests/boundary/filesystem/artifactZipIntegrity.test.ts b/tests/boundary/filesystem/artifactZipIntegrity.test.ts index a3fcb81db..eafeda0fe 100644 --- a/tests/boundary/filesystem/artifactZipIntegrity.test.ts +++ b/tests/boundary/filesystem/artifactZipIntegrity.test.ts @@ -35,6 +35,48 @@ it("bounds a requested metadata read without rejecting the underlying valid ZIP" } }); +it("binds ZIP opens to the emitted entry metadata", async () => { + const root = await createTestTempDirectory("rea-zip-entry-identity-"); + const path = join(root, "entries.zip"); + const writer = new ZipWriter(new Uint8ArrayWriter()); + await writer.add("first.txt", new TextReader("first bytes")); + await writer.add("second.txt", new TextReader("second bytes")); + await writeFile(path, await writer.close()); + + const reader = new ZipArtifactReader(path, "zip"); + try { + const entries = []; + for await (const entry of reader.entries()) entries.push(entry); + const first = entries[0]; + const second = entries[1]; + if (first === undefined || second === undefined) + throw new Error("Expected both ZIP entries"); + + await expect( + reader.open({ ...second, adapterKey: first.adapterKey }), + ).rejects.toMatchObject({ reason: "integrity" }); + await expect( + reader.open({ ...second, path: first.path }), + ).rejects.toMatchObject({ reason: "integrity" }); + await expect( + reader.open({ ...second, declaredSize: first.declaredSize }), + ).rejects.toMatchObject({ reason: "integrity" }); + const originalKey = second.adapterKey; + Reflect.set(second, "adapterKey", first.adapterKey); + await expect(reader.open(second)).rejects.toMatchObject({ + reason: "integrity", + }); + Reflect.set(second, "adapterKey", originalKey); + + const stream = await reader.open({ ...second }); + const chunks: Buffer[] = []; + for await (const chunk of stream) chunks.push(Buffer.from(chunk)); + expect(Buffer.concat(chunks).toString()).toBe("second bytes"); + } finally { + await reader.close(); + } +}); + it("rejects overlapping ZIP member data ranges through artifact inventory", async () => { const root = await createTestTempDirectory("rea-zip-overlap-"); const path = join(root, "overlap.zip"); diff --git a/tests/boundary/providers/native/artifactProvider.machOSlices.test.ts b/tests/boundary/providers/native/artifactProvider.machOSlices.test.ts index f46453cc2..8de8212bf 100644 --- a/tests/boundary/providers/native/artifactProvider.machOSlices.test.ts +++ b/tests/boundary/providers/native/artifactProvider.machOSlices.test.ts @@ -83,9 +83,10 @@ describe("artifact Mach-O slices", () => { const entries = []; for await (const entry of reader.entries()) entries.push(entry); expect(entries).toHaveLength(2); + const firstEntry = entries[0]; const secondEntry = entries[1]; - expect(secondEntry).toBeDefined(); - if (secondEntry === undefined) return; + if (firstEntry === undefined || secondEntry === undefined) + throw new Error("Expected both universal Mach-O slice entries"); expect(secondEntry).toMatchObject({ path: "slices/arm64", byteOffset: 8192, @@ -95,8 +96,26 @@ describe("artifact Mach-O slices", () => { await expect( reader.open({ ...secondEntry, adapterKey }), ).rejects.toMatchObject({ reason: "integrity" }); + await expect( + reader.open({ ...secondEntry, adapterKey: firstEntry.adapterKey }), + ).rejects.toMatchObject({ reason: "integrity" }); + await expect( + reader.open({ ...secondEntry, path: firstEntry.path }), + ).rejects.toMatchObject({ reason: "integrity" }); + await expect( + reader.open({ ...secondEntry, byteOffset: firstEntry.byteOffset }), + ).rejects.toMatchObject({ reason: "integrity" }); + await expect( + reader.open({ ...secondEntry, declaredSize: arm.length + 1 }), + ).rejects.toMatchObject({ reason: "integrity" }); + const originalKey = secondEntry.adapterKey; + Reflect.set(secondEntry, "adapterKey", firstEntry.adapterKey); + await expect(reader.open(secondEntry)).rejects.toMatchObject({ + reason: "integrity", + }); + Reflect.set(secondEntry, "adapterKey", originalKey); const chunks: Buffer[] = []; - const stream = await reader.open(secondEntry); + const stream = await reader.open({ ...secondEntry }); for await (const chunk of stream) chunks.push(Buffer.from(chunk)); expect(Buffer.concat(chunks)).toEqual(Buffer.from(arm)); expect(reader.provenance()).toEqual([ diff --git a/src/artifacts/MachOSliceArtifactReader.test.ts b/tests/boundary/providers/native/machoSliceReader.test.ts similarity index 69% rename from src/artifacts/MachOSliceArtifactReader.test.ts rename to tests/boundary/providers/native/machoSliceReader.test.ts index 8aec8fffc..f67b48388 100644 --- a/src/artifacts/MachOSliceArtifactReader.test.ts +++ b/tests/boundary/providers/native/machoSliceReader.test.ts @@ -1,8 +1,8 @@ import { readFile } from "node:fs/promises"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, onTestFinished } from "vitest"; -import { MachOSliceArtifactReader } from "./MachOSliceArtifactReader.js"; +import { MachOSliceArtifactReader } from "../../../../src/artifacts/MachOSliceArtifactReader.js"; /** * Read a 32-bit FAT table directly. The host's /bin/ls changes between macOS @@ -21,15 +21,18 @@ const fatSlices = async ( describe("Mach-O slice reader with system lipo", () => { it.skipIf(process.platform !== "darwin")( - "checks real /bin/ls lipo CPU symbols and ptrauth capabilities against the FAT table", + "checks real /bin/ls slice ranges against its FAT table", async () => { const reader = new MachOSliceArtifactReader("/bin/ls", {}); + onTestFinished(() => reader.close()); const entries = []; for await (const entry of reader.entries()) entries.push(entry); - expect(entries.map(({ path }) => path)).toEqual([ - "slices/x86_64", - "slices/arm64e", - ]); + expect(new Set(entries.map(({ path }) => path)).size).toBe( + entries.length, + ); + expect(entries.every(({ path }) => path.startsWith("slices/"))).toBe( + true, + ); expect( entries.map(({ byteOffset, declaredSize }) => [ byteOffset,