From f0d175dacefba65f40ea8b4198a9a93083f34d3e Mon Sep 17 00:00:00 2001 From: Ysopwnable <161784452+N0zoM1z0@users.noreply.github.com> Date: Sat, 17 Oct 2026 18:35:31 +0800 Subject: [PATCH] fix(verifiers): read packed bridge without extraction Parse only the exact regular packed Python bridge member without extracting into the caller-owned private runtime root. Preserve required Ghidra admission, typed callers, native lease ownership and cleanup on success or failure. Validation: 263 focused real tar/Python/ownership and CI selection checks, 42 official WABT packaged CLI/MCP cases, full check:ci/check:fast/actionlint, real-package alternating measurements and exact-head hosted CI passed. The required Windows lane exercises the retained native lease without skipping missing authority. Fixes #4134. Co-authored-by: abdooo2235 --- .github/workflows/ci.yml | 1 + scripts/ci/scopes.mjs | 2 + scripts/verify-package-pack.mjs | 2 +- scripts/verify-packed-bridge.d.mts | 5 + scripts/verify-packed-bridge.mjs | 20 ++- .../boundary/filesystem/ciChangeScope.test.ts | 33 ++++ .../filesystem/ghidraBridgeSources.test.ts | 2 +- .../process/packedBridgeVerification.test.ts | 154 ++++++++++++++++++ .../process-global/curatedWindowsLane.test.ts | 6 + 9 files changed, 216 insertions(+), 9 deletions(-) create mode 100644 scripts/verify-packed-bridge.d.mts create mode 100644 tests/boundary/process/packedBridgeVerification.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 60459cded..b034d8198 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -753,6 +753,7 @@ jobs: tests/boundary/filesystem/webScriptManifestPaths.test.ts tests/boundary/process/ProcessEvidence.host.test.ts tests/boundary/process/packageNpmInvocation.test.ts + tests/boundary/process/packedBridgeVerification.test.ts tests/boundary/process/windowsOutputCapture.test.ts tests/acceptance/webModuleCli.test.ts tests/acceptance/webSourceLocationCli.test.ts diff --git a/scripts/ci/scopes.mjs b/scripts/ci/scopes.mjs index fef84802e..7c4697667 100644 --- a/scripts/ci/scopes.mjs +++ b/scripts/ci/scopes.mjs @@ -118,6 +118,7 @@ export const SCOPES = { "scripts/verify/inspector/**", "scripts/verify-package.mjs", "scripts/verify-package-*", + "scripts/verify-packed-bridge.*", "scripts/verify-windows-*.mjs", "scripts/build-windows-native.mjs", "scripts/windows-capabilities.mjs", @@ -145,6 +146,7 @@ export const SCOPES = { "tests/boundary/filesystem/webScriptManifestPaths.test.ts", "tests/boundary/process/ProcessEvidence.host.test.ts", "tests/boundary/process/packageNpmInvocation.test.ts", + "tests/boundary/process/packedBridgeVerification.test.ts", "tests/boundary/process/windowsOutputCapture.test.ts", "tests/acceptance/webModuleCli.test.ts", "tests/acceptance/webSourceLocationCli.test.ts", diff --git a/scripts/verify-package-pack.mjs b/scripts/verify-package-pack.mjs index bd33bdda1..c1e4edb00 100644 --- a/scripts/verify-package-pack.mjs +++ b/scripts/verify-package-pack.mjs @@ -66,6 +66,6 @@ export async function verifyPackagePack({ root, workspace }) { ) { throw new Error("package contained generated Python bytecode"); } - await verifyPackedBridge({ workspace, tarball, packedFiles }); + await verifyPackedBridge({ tarball, packedFiles }); return { tarball }; } diff --git a/scripts/verify-packed-bridge.d.mts b/scripts/verify-packed-bridge.d.mts new file mode 100644 index 000000000..f13622710 --- /dev/null +++ b/scripts/verify-packed-bridge.d.mts @@ -0,0 +1,5 @@ +/** Validate required shipped bridges and parse packed Python without extraction. */ +export function verifyPackedBridge(input: { + readonly tarball: string; + readonly packedFiles: readonly string[]; +}): Promise; diff --git a/scripts/verify-packed-bridge.mjs b/scripts/verify-packed-bridge.mjs index 19f61cae6..5f4116874 100644 --- a/scripts/verify-packed-bridge.mjs +++ b/scripts/verify-packed-bridge.mjs @@ -1,5 +1,4 @@ import { execFile } from "node:child_process"; -import { join } from "node:path"; import { promisify } from "node:util"; import { GHIDRA_BRIDGE_SOURCES } from "../src/ghidra/GhidraBridgeSources.ts"; @@ -8,7 +7,7 @@ const packedHopperBridge = "package/bridge/hopper_bridge.py"; const packedCutterBridge = "package/bridge/cutter/rea_bridge.py"; /** Check shipped bridge files; real-provider lanes verify runtime behavior. */ -export async function verifyPackedBridge({ workspace, tarball, packedFiles }) { +export async function verifyPackedBridge({ tarball, packedFiles }) { for (const path of [ packedHopperBridge, packedCutterBridge, @@ -18,12 +17,19 @@ export async function verifyPackedBridge({ workspace, tarball, packedFiles }) { ]) { if (!packedFiles.includes(path)) throw new Error(`package omitted ${path}`); } - await exec("tar", ["-xf", tarball, "-C", workspace]); - // Parse the actual packed Python source without loading Hopper globals or - // generating bytecode. This check has no dependency on test-only fixtures. + // Read the packed source without extracting into the caller's leased root. await exec("python3", [ "-c", - "import ast, pathlib, sys; p = pathlib.Path(sys.argv[1]); ast.parse(p.read_text(encoding='utf-8'), filename=str(p))", - join(workspace, packedHopperBridge), + [ + "import ast, sys, tarfile", + "with tarfile.open(sys.argv[1]) as archive:", + " member = archive.getmember(sys.argv[2])", + " if not member.isfile():", + " raise ValueError(f'{member.name} is not a regular file')", + " with archive.extractfile(member) as source:", + " ast.parse(source.read().decode('utf-8'), filename=sys.argv[2])", + ].join("\n"), + tarball, + packedHopperBridge, ]); } diff --git a/tests/boundary/filesystem/ciChangeScope.test.ts b/tests/boundary/filesystem/ciChangeScope.test.ts index 3da350c26..cb8519f70 100644 --- a/tests/boundary/filesystem/ciChangeScope.test.ts +++ b/tests/boundary/filesystem/ciChangeScope.test.ts @@ -724,6 +724,39 @@ it.each([ "tests/boundary/filesystem/windowsNpxRegistration.test.ts", { code: true, package: true, runtime: true, apple: false }, ], + [ + "scripts/verify-packed-bridge.mjs", + { + code: true, + package: true, + runtime: true, + apple: false, + evm: false, + browser: false, + }, + ], + [ + "scripts/verify-packed-bridge.d.mts", + { + code: true, + package: true, + runtime: true, + apple: false, + evm: false, + browser: false, + }, + ], + [ + "tests/boundary/process/packedBridgeVerification.test.ts", + { + code: true, + package: true, + runtime: true, + apple: false, + evm: false, + browser: false, + }, + ], [ "src/process/ProcessOwnership.test.ts", { code: true, package: true, apple: false, evm: false, browser: false }, diff --git a/tests/boundary/filesystem/ghidraBridgeSources.test.ts b/tests/boundary/filesystem/ghidraBridgeSources.test.ts index baf584fba..e0c4571e6 100644 --- a/tests/boundary/filesystem/ghidraBridgeSources.test.ts +++ b/tests/boundary/filesystem/ghidraBridgeSources.test.ts @@ -210,7 +210,7 @@ it("rejects an omitted auxiliary source at the actual package verification bound "--input-type=module", "-e", `import { verifyPackedBridge } from "./scripts/verify-packed-bridge.mjs"; - await verifyPackedBridge({ workspace: ".", tarball: "unused-because-admission-must-fail", packedFiles: JSON.parse(process.argv[1]) });`, + await verifyPackedBridge({ tarball: "unused-because-admission-must-fail", packedFiles: JSON.parse(process.argv[1]) });`, JSON.stringify(packedFiles), ]), ).rejects.toMatchObject({ diff --git a/tests/boundary/process/packedBridgeVerification.test.ts b/tests/boundary/process/packedBridgeVerification.test.ts new file mode 100644 index 000000000..df9d5ba3b --- /dev/null +++ b/tests/boundary/process/packedBridgeVerification.test.ts @@ -0,0 +1,154 @@ +import { execFile } from "node:child_process"; +import { + access, + copyFile, + mkdir, + readFile, + readdir, + writeFile, +} from "node:fs/promises"; +import { dirname, join } from "node:path"; +import { promisify } from "node:util"; +import { expect, it } from "vitest"; + +import { verifyPackedBridge } from "../../../scripts/verify-packed-bridge.mjs"; +import { + PrivateRuntimeRoot, + privateRuntimeRootCapability, +} from "../../../src/process/PrivateRuntimeRoot.js"; +import { GHIDRA_BRIDGE_SOURCES } from "../../../src/ghidra/GhidraBridgeSources.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +const exec = promisify(execFile); +const packedFiles = [ + "package/bridge/hopper_bridge.py", + "package/bridge/cutter/rea_bridge.py", + ...Object.values(GHIDRA_BRIDGE_SOURCES).map( + (name) => `package/bridge/ghidra/${name}`, + ), +]; +const hopper = packedFiles[0]!; +const validSource = + '# Unicode source: 雨\nraise RuntimeError("Packed source must never execute")\n'; + +async function fixture(source: string = validSource, omitHopper = false) { + const directory = await createTestTempDirectory("rea-packed-bridge-"); + const staging = join(directory, "source"); + const workspace = join(directory, "workspace"); + await mkdir(workspace); + for (const member of packedFiles) { + if (omitHopper && member === hopper) continue; + const path = join(staging, member); + await mkdir(dirname(path), { recursive: true }); + await writeFile(path, member === hopper ? source : "fixture\n"); + } + const tarball = join(directory, "packed bridges.tgz"); + await exec("tar", ["-czf", tarball, "-C", staging, "package"]); + return { workspace, tarball, packedFiles }; +} + +it("parses packed Python without executing it or extracting archive members", async () => { + const input = await fixture(); + const sentinel = join(input.workspace, "caller-owned.txt"); + await writeFile(sentinel, "preserved\n"); + await verifyPackedBridge(input); + expect(await readdir(input.workspace)).toEqual(["caller-owned.txt"]); + expect(await readFile(sentinel, "utf8")).toBe("preserved\n"); +}); + +it.each(packedFiles)( + "rejects an unadvertised required member: %s", + async (member) => { + const input = await fixture(); + await expect( + verifyPackedBridge({ + ...input, + packedFiles: packedFiles.filter((path) => path !== member), + }), + ).rejects.toThrow(`package omitted ${member}`); + expect(await readdir(input.workspace)).toEqual([]); + }, +); + +it("rejects a missing actual Python member even when the listing advertises it", async () => { + const input = await fixture(validSource, true); + await expect(verifyPackedBridge(input)).rejects.toMatchObject({ + stderr: expect.stringContaining(hopper), + }); + expect(await readdir(input.workspace)).toEqual([]); +}); + +it("reports invalid packed Python syntax with the member filename", async () => { + const input = await fixture("def broken(:\n"); + await expect(verifyPackedBridge(input)).rejects.toMatchObject({ + stderr: expect.stringMatching(/hopper_bridge\.py[\s\S]*SyntaxError/u), + }); + expect(await readdir(input.workspace)).toEqual([]); +}); + +it.each(["valid", "invalid"] as const)( + "parses %s source under an owned private root and preserves owner cleanup", + async (kind) => { + expect(privateRuntimeRootCapability().available).toBe(true); + const input = await fixture( + kind === "valid" ? validSource : "def broken(:\n", + ); + const root = await PrivateRuntimeRoot.create({ + prefix: "rea-packed-bridge-lease-", + }); + try { + const pack = join(root.path, "pack"); + await mkdir(pack); + const tarball = join(pack, "packed bridges.tgz"); + await copyFile(input.tarball, tarball); + const parsed = verifyPackedBridge({ tarball, packedFiles }); + if (kind === "valid") await parsed; + else + await expect(parsed).rejects.toMatchObject({ + stderr: expect.stringContaining("SyntaxError"), + }); + expect(await readdir(root.path)).toEqual(["pack"]); + expect(await readFile(tarball)).toEqual(await readFile(input.tarball)); + } finally { + await root.close(); + } + await expect(access(root.path)).rejects.toMatchObject({ code: "ENOENT" }); + }, +); + +it.each(["symlink", "hardlink", "directory"])( + "rejects a %s Python member without following it or extracting files", + async (kind) => { + const input = await fixture(); + await exec("python3", [ + "-c", + [ + "import io, sys, tarfile", + "with tarfile.open(sys.argv[1], 'w:gz') as archive:", + " target = tarfile.TarInfo('package/bridge/target.py')", + " target.size = 5", + " archive.addfile(target, io.BytesIO(b'pass\\n'))", + " member = tarfile.TarInfo(sys.argv[2])", + " member.type = {'symlink': tarfile.SYMTYPE, 'hardlink': tarfile.LNKTYPE, 'directory': tarfile.DIRTYPE}[sys.argv[3]]", + " member.linkname = 'target.py' if sys.argv[3] == 'symlink' else target.name", + " archive.addfile(member)", + ].join("\n"), + input.tarball, + hopper, + kind, + ]); + await expect(verifyPackedBridge(input)).rejects.toMatchObject({ + stderr: expect.stringContaining(`${hopper} is not a regular file`), + }); + expect(await readdir(input.workspace)).toEqual([]); + }, +); + +it("rejects malformed archives without writing to the caller's workspace", async () => { + const input = await fixture(); + await writeFile(input.tarball, "not a tar archive\n"); + await expect(verifyPackedBridge(input)).rejects.toMatchObject({ + stderr: expect.stringContaining("ReadError"), + }); + expect(await readdir(input.workspace)).toEqual([]); +}); diff --git a/tests/process-global/curatedWindowsLane.test.ts b/tests/process-global/curatedWindowsLane.test.ts index 52510e055..c486b7a24 100644 --- a/tests/process-global/curatedWindowsLane.test.ts +++ b/tests/process-global/curatedWindowsLane.test.ts @@ -83,6 +83,12 @@ describe("curated Windows test lane", () => { ); }); + it("keeps packed bridge verification under the retained native runtime lease in the lane", async () => { + expect(await explicitPaths("windows")).toContain( + "tests/boundary/process/packedBridgeVerification.test.ts", + ); + }); + it("keeps Windows runner cancellation coverage in the lane", async () => { expect(await explicitPaths("windows")).toEqual( expect.arrayContaining([