From 19c18ac2f8ff5094c5721608c7dc8239100b059c Mon Sep 17 00:00:00 2001 From: Jax Liu Date: Fri, 4 Sep 2026 21:41:39 +0800 Subject: [PATCH] fix(genbi): harden context loader package verification --- .github/workflows/genbi-ci.yml | 4 + apps/context-loader/lib/verified.mjs | 73 +++++++++++++++--- apps/context-loader/scripts/installer.mjs | 22 +++++- apps/context-loader/scripts/postinstall.mjs | 4 +- apps/context-loader/test/installer.test.mjs | 39 +++++++++- apps/genbi/harness/compile/context-loader.ts | 76 ++++++++++++++++--- .../genbi/test/compile-context-loader.test.ts | 41 +++++++++- apps/genbi/test/packaging-contract.test.ts | 2 + 8 files changed, 231 insertions(+), 30 deletions(-) diff --git a/.github/workflows/genbi-ci.yml b/.github/workflows/genbi-ci.yml index 91cbea73a..a2eb98a37 100644 --- a/.github/workflows/genbi-ci.yml +++ b/.github/workflows/genbi-ci.yml @@ -74,6 +74,10 @@ jobs: working-directory: . run: pnpm install --frozen-lockfile + - name: Test context-loader installer contract + working-directory: . + run: pnpm --dir apps/context-loader test + - name: Typecheck run: pnpm run typecheck diff --git a/apps/context-loader/lib/verified.mjs b/apps/context-loader/lib/verified.mjs index 845e14105..35cf146d7 100644 --- a/apps/context-loader/lib/verified.mjs +++ b/apps/context-loader/lib/verified.mjs @@ -1,5 +1,5 @@ import { createHash } from "node:crypto"; -import { existsSync, readFileSync } from "node:fs"; +import { lstatSync, readFileSync, realpathSync, statSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; @@ -28,25 +28,31 @@ export function packageRootFrom(moduleUrl = import.meta.url) { } export function readVerifiedState(root = packageRootFrom()) { - const packageJson = readJson(path.join(root, "package.json"), "package.json"); + const verifiedRoot = verifiedPackageRoot(root); + const packageJson = readPackageJson(verifiedRoot); if (packageJson.name !== PACKAGE_NAME || typeof packageJson.version !== "string") { throw new ContextLoaderPackageError("invalid-package", "package.json does not identify this package"); } - const state = readJson(path.join(root, STATE_FILE), STATE_FILE); + const manifest = readManifest(verifiedRoot, packageJson.version); + const state = readJson(verifiedRegularFile(verifiedRoot, STATE_FILE), STATE_FILE); const target = targetFor(); - if (state.package !== PACKAGE_NAME || state.version !== packageJson.version || state.target !== target) { - throw new ContextLoaderPackageError("stale-state", "verification record does not match package version or platform"); + const artifact = manifest.artifacts?.[target]; + if (!isArtifact(artifact)) { + throw new ContextLoaderPackageError("invalid-manifest", "artifact manifest has no safe row for this platform"); } - if (!isSha256(state.binarySha256)) { - throw new ContextLoaderPackageError("invalid-state", "verification record has no binary SHA-256"); + if ( + state.package !== PACKAGE_NAME || + state.version !== packageJson.version || + state.target !== target || + state.archiveSha256 !== artifact.archiveSha256 || + state.binarySha256 !== artifact.binarySha256 + ) { + throw new ContextLoaderPackageError("stale-state", "verification record does not match package version or platform"); } if (state.binaryPath !== CANONICAL_BINARY) { throw new ContextLoaderPackageError("invalid-state", "verification record does not name the canonical package-local binary"); } - const binary = path.resolve(root, state.binaryPath); - if (!binary.startsWith(`${root}${path.sep}`) || !existsSync(binary)) { - throw new ContextLoaderPackageError("missing-binary", "verified package-local binary is absent"); - } + const binary = verifiedRegularFile(verifiedRoot, state.binaryPath); if (sha256File(binary) !== state.binarySha256) { throw new ContextLoaderPackageError("binary-digest-mismatch", "verified package-local binary was modified"); } @@ -68,3 +74,48 @@ export function readJson(file, label) { export function isSha256(value) { return typeof value === "string" && /^[a-f0-9]{64}$/u.test(value); } + +function verifiedPackageRoot(root) { + try { + if (!lstatSync(root).isDirectory()) throw new Error("not a directory"); + return realpathSync(root); + } catch { + throw new ContextLoaderPackageError("invalid-package", "package root is missing or is not a real directory"); + } +} + +function readPackageJson(root) { + return readJson(verifiedRegularFile(root, "package.json"), "package.json"); +} + +function readManifest(root, version) { + const manifest = readJson(verifiedRegularFile(root, "artifacts.json"), "artifacts.json"); + if (manifest.schema !== 1 || manifest.package !== PACKAGE_NAME || manifest.version !== version || typeof manifest.artifacts !== "object" || manifest.artifacts === null) { + throw new ContextLoaderPackageError("invalid-manifest", "artifact manifest does not match package identity"); + } + return manifest; +} + +function isArtifact(artifact) { + return typeof artifact === "object" && artifact !== null && + typeof artifact.url === "string" && artifact.url.startsWith("https://") && + isSha256(artifact.archiveSha256) && isSha256(artifact.binarySha256) && + typeof artifact.binaryPath === "string" && + !artifact.binaryPath.includes("..") && !path.isAbsolute(artifact.binaryPath); +} + +function verifiedRegularFile(root, relativePath) { + const lexicalRoot = path.resolve(root); + const lexicalFile = path.resolve(lexicalRoot, relativePath); + if (!lexicalFile.startsWith(`${lexicalRoot}${path.sep}`)) { + throw new ContextLoaderPackageError("invalid-state", "verification record escapes the package root"); + } + try { + if (!lstatSync(lexicalFile).isFile()) throw new Error("not a regular file"); + const physicalFile = realpathSync(lexicalFile); + if (!physicalFile.startsWith(`${root}${path.sep}`) || !statSync(physicalFile).isFile()) throw new Error("not contained"); + return physicalFile; + } catch { + throw new ContextLoaderPackageError("missing-binary", "verified package-local file is absent, linked, or outside the package root"); + } +} diff --git a/apps/context-loader/scripts/installer.mjs b/apps/context-loader/scripts/installer.mjs index 329479b3c..6a0def74d 100644 --- a/apps/context-loader/scripts/installer.mjs +++ b/apps/context-loader/scripts/installer.mjs @@ -1,5 +1,5 @@ import { createHash } from "node:crypto"; -import { chmod, mkdir, readFile, rename, rm, writeFile } from "node:fs/promises"; +import { access, chmod, mkdir, readFile, rename, rm, writeFile } from "node:fs/promises"; import path from "node:path"; import { gunzipSync } from "node:zlib"; import { CANONICAL_BINARY, ContextLoaderPackageError, isSha256, readJson, sha256File, targetFor } from "../lib/verified.mjs"; @@ -45,6 +45,26 @@ export async function installContextLoader({ packageRoot, fetchImpl = fetch, pla } } +/** + * The checked-out source package carries an intentionally empty, pre-release + * artifact template. Only that exact repository layout skips postinstall; + * packed packages always download and verify their tagged artifact. + */ +export async function isRepositorySourcePackage(packageRoot) { + const repositoryRoot = path.resolve(packageRoot, "..", ".."); + if (path.resolve(packageRoot) !== path.join(repositoryRoot, "apps", "context-loader")) return false; + try { + await Promise.all([ + access(path.join(repositoryRoot, ".git")), + access(path.join(repositoryRoot, "pnpm-workspace.yaml")), + access(path.join(repositoryRoot, "core", "wren", "pyproject.toml")), + ]); + return true; + } catch { + return false; + } +} + function validateManifest(packageJson, manifest, artifact, target) { if (manifest.schema !== 1 || manifest.package !== packageJson.name || manifest.version !== packageJson.version) { throw new ContextLoaderPackageError("invalid-manifest", "manifest does not match package identity"); diff --git a/apps/context-loader/scripts/postinstall.mjs b/apps/context-loader/scripts/postinstall.mjs index cb697bdf3..ac05d2c5a 100644 --- a/apps/context-loader/scripts/postinstall.mjs +++ b/apps/context-loader/scripts/postinstall.mjs @@ -1,8 +1,10 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; -import { installContextLoader } from "./installer.mjs"; +import { installContextLoader, isRepositorySourcePackage } from "./installer.mjs"; const packageRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +if (await isRepositorySourcePackage(packageRoot)) process.exit(0); + try { await installContextLoader({ packageRoot }); } catch (error) { diff --git a/apps/context-loader/test/installer.test.mjs b/apps/context-loader/test/installer.test.mjs index 2ae67dc4c..8c7766469 100644 --- a/apps/context-loader/test/installer.test.mjs +++ b/apps/context-loader/test/installer.test.mjs @@ -1,11 +1,11 @@ import assert from "node:assert/strict"; import { createHash } from "node:crypto"; -import { mkdtemp, readFile, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, realpath, rm, symlink, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import test from "node:test"; import { gzipSync } from "node:zlib"; -import { installContextLoader } from "../scripts/installer.mjs"; +import { installContextLoader, isRepositorySourcePackage } from "../scripts/installer.mjs"; import { readVerifiedState } from "../lib/verified.mjs"; const sha256 = (content) => createHash("sha256").update(content).digest("hex"); @@ -50,7 +50,7 @@ test("installs a digest-verified darwin-arm64 binary atomically and records cano assert.equal(first.reused, false); assert.deepEqual(await readFile(first.binary), artifact); const state = readVerifiedState(root); - assert.equal(state.binary, first.binary); + assert.equal(state.binary, await realpath(first.binary)); assert.match(state.identity, /package:@wrenai\/context-loader@0\.1\.0:darwin-arm64:[a-f0-9]{64}/); const second = await installContextLoader({ packageRoot: root, fetchImpl: async () => { throw new Error("offline"); }, platform: "darwin", arch: "arm64" }); assert.equal(second.reused, true); @@ -76,3 +76,36 @@ test("rejects stale or altered installed state at runtime", async () => { await writeFile(statePath, JSON.stringify({ ...state, version: "0.1.1" })); assert.throws(() => readVerifiedState(root), /stale-state/); }); + +test("rejects final-file and ancestor-directory symlinks at runtime", async () => { + const finalLink = await fixture(); + const installed = await installContextLoader({ packageRoot: finalLink.root, fetchImpl: fetchArchive(finalLink.archive), platform: "darwin", arch: "arm64" }); + const external = path.join(await mkdtemp(path.join(os.tmpdir(), "context-loader-external-")), "wren-context-loader"); + await writeFile(external, await readFile(installed.binary), { mode: 0o755 }); + await rm(installed.binary); + await symlink(external, installed.binary); + assert.throws(() => readVerifiedState(finalLink.root), /linked|outside/); + + const ancestorLink = await fixture(); + const installedAncestor = await installContextLoader({ packageRoot: ancestorLink.root, fetchImpl: fetchArchive(ancestorLink.archive), platform: "darwin", arch: "arm64" }); + const externalDir = await mkdtemp(path.join(os.tmpdir(), "context-loader-external-dir-")); + await writeFile(path.join(externalDir, "wren-context-loader"), await readFile(installedAncestor.binary), { mode: 0o755 }); + await rm(path.join(ancestorLink.root, "bin"), { recursive: true }); + await symlink(externalDir, path.join(ancestorLink.root, "bin")); + assert.throws(() => readVerifiedState(ancestorLink.root), /linked|outside/); +}); + +test("skips only the verifiable source-workspace layout, never an equivalent packed copy", async () => { + const repositoryRoot = await mkdtemp(path.join(os.tmpdir(), "context-loader-source-")); + const sourcePackage = path.join(repositoryRoot, "apps", "context-loader"); + await mkdir(path.join(repositoryRoot, "core", "wren"), { recursive: true }); + await mkdir(sourcePackage, { recursive: true }); + await writeFile(path.join(repositoryRoot, ".git"), "gitdir: fixture\n"); + await writeFile(path.join(repositoryRoot, "pnpm-workspace.yaml"), "packages: []\n"); + await writeFile(path.join(repositoryRoot, "core", "wren", "pyproject.toml"), "[project]\nname = \"wren\"\n"); + assert.equal(await isRepositorySourcePackage(sourcePackage), true); + + const packedCopy = path.join(repositoryRoot, "node_modules", "@wrenai", "context-loader"); + await mkdir(packedCopy, { recursive: true }); + assert.equal(await isRepositorySourcePackage(packedCopy), false); +}); diff --git a/apps/genbi/harness/compile/context-loader.ts b/apps/genbi/harness/compile/context-loader.ts index e52ec4c81..a2d3f053b 100644 --- a/apps/genbi/harness/compile/context-loader.ts +++ b/apps/genbi/harness/compile/context-loader.ts @@ -1,5 +1,5 @@ import { execFile } from "node:child_process"; -import { existsSync, readFileSync } from "node:fs"; +import { existsSync, lstatSync, readFileSync, realpathSync, statSync } from "node:fs"; import { createHash } from "node:crypto"; import { fileURLToPath } from "node:url"; import path from "node:path"; @@ -97,37 +97,91 @@ function resolveVerifiedPackageBinary(): ResolvedContextLoader | undefined { /** Validates a package-local installation record without resolving it from the host environment. */ export function readVerifiedContextLoaderPackage(packageRoot: string, target = targetForCurrentPlatform()): ResolvedContextLoader | undefined { try { - const packageJson = readJson(path.join(packageRoot, "package.json")); - const state = readJson(path.join(packageRoot, "install-state.json")); - const bin = path.resolve(packageRoot, CANONICAL_PACKAGE_BINARY); + const root = physicalPackageRoot(packageRoot); + if (root === undefined) return undefined; + const packageJson = readVerifiedJson(root, "package.json"); + const manifest = readVerifiedJson(root, "artifacts.json"); + const state = readVerifiedJson(root, "install-state.json"); + if (packageJson === undefined || manifest === undefined || state === undefined) return undefined; + const artifacts = typeof manifest.artifacts === "object" && manifest.artifacts !== null ? manifest.artifacts as Record : undefined; + const artifact = artifacts?.[target]; + const bin = physicalRegularFile(root, CANONICAL_PACKAGE_BINARY); if ( packageJson.name !== CONTEXT_LOADER_PACKAGE || typeof packageJson.version !== "string" || + manifest.schema !== 1 || + manifest.package !== CONTEXT_LOADER_PACKAGE || + manifest.version !== packageJson.version || + !isArtifact(artifact) || state.package !== CONTEXT_LOADER_PACKAGE || state.version !== packageJson.version || state.target !== target || state.binaryPath !== CANONICAL_PACKAGE_BINARY || - !isSha256(state.binarySha256) || - !bin.startsWith(`${packageRoot}${path.sep}`) || - !existsSync(bin) || - createHash("sha256").update(readFileSync(bin)).digest("hex") !== state.binarySha256 + state.archiveSha256 !== artifact.archiveSha256 || + state.binarySha256 !== artifact.binarySha256 || + bin === undefined || + createHash("sha256").update(readFileSync(bin)).digest("hex") !== artifact.binarySha256 ) { return undefined; } - return { bin, kind: "package", verifiedIdentity: `package:${CONTEXT_LOADER_PACKAGE}@${packageJson.version}:${target}:${state.binarySha256}` }; + return { bin, kind: "package", verifiedIdentity: `package:${CONTEXT_LOADER_PACKAGE}@${packageJson.version}:${target}:${artifact.binarySha256}` }; } catch { return undefined; } } -function readJson(file: string): Record { - return JSON.parse(readFileSync(file, "utf8")) as Record; +function readVerifiedJson(root: string, relativePath: string): Record | undefined { + const file = physicalRegularFile(root, relativePath); + if (file === undefined) return undefined; + try { + const value = JSON.parse(readFileSync(file, "utf8")); + return typeof value === "object" && value !== null ? value as Record : undefined; + } catch { + return undefined; + } } function isSha256(value: unknown): value is string { return typeof value === "string" && /^[a-f0-9]{64}$/u.test(value); } +function isArtifact(value: unknown): value is { archiveSha256: string; binarySha256: string; binaryPath: string; url: string } { + if (typeof value !== "object" || value === null) return false; + const artifact = value as Record; + return ( + typeof artifact.url === "string" && + artifact.url.startsWith("https://") && + isSha256(artifact.archiveSha256) && + isSha256(artifact.binarySha256) && + typeof artifact.binaryPath === "string" && + !artifact.binaryPath.includes("..") && + !path.isAbsolute(artifact.binaryPath) + ); +} + +/** Returns a package-owned physical root; package-root symlinks are never trusted. */ +function physicalPackageRoot(packageRoot: string): string | undefined { + try { + if (!lstatSync(packageRoot).isDirectory()) return undefined; + return realpathSync(packageRoot); + } catch { + return undefined; + } +} + +/** Refuses final-file links and any ancestor link that resolves outside the physical package root. */ +function physicalRegularFile(root: string, relativePath: string): string | undefined { + const lexical = path.resolve(root, relativePath); + if (!lexical.startsWith(`${root}${path.sep}`)) return undefined; + try { + if (!lstatSync(lexical).isFile()) return undefined; + const physical = realpathSync(lexical); + return physical.startsWith(`${root}${path.sep}`) && statSync(physical).isFile() ? physical : undefined; + } catch { + return undefined; + } +} + function targetForCurrentPlatform(): string { return `${process.platform}-${process.arch}`; } diff --git a/apps/genbi/test/compile-context-loader.test.ts b/apps/genbi/test/compile-context-loader.test.ts index c202bafae..b696a6aea 100644 --- a/apps/genbi/test/compile-context-loader.test.ts +++ b/apps/genbi/test/compile-context-loader.test.ts @@ -1,4 +1,4 @@ -import { mkdir, mkdtemp, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, realpath, rm, symlink, writeFile } from "node:fs/promises"; import { createHash } from "node:crypto"; import os from "node:os"; import path from "node:path"; @@ -47,10 +47,13 @@ async function makeVerifiedPackage(marker: string, version = "0.1.0"): Promise { const a = readVerifiedContextLoaderPackage(aRoot); const b = readVerifiedContextLoaderPackage(bRoot); expect(a?.kind).toBe("package"); - expect(a?.bin).toBe(path.join(aRoot, "bin", "wren-context-loader")); + expect(a?.bin).toBe(await realpath(path.join(aRoot, "bin", "wren-context-loader"))); expect(a).toBeDefined(); expect(b).toBeDefined(); expect(getContextLoaderIdentity(a!)).not.toBe(getContextLoaderIdentity(b!)); @@ -127,6 +130,38 @@ describe("verified package context loader", () => { await writeFile(path.join(stale, "install-state.json"), JSON.stringify({ package: "@wrenai/context-loader", version: "0.0.0", target: `${process.platform}-${process.arch}`, binaryPath: path.join("bin", "wren-context-loader"), binarySha256: "0".repeat(64) })); expect(readVerifiedContextLoaderPackage(stale)).toBeUndefined(); }); + + it("requires the packaged artifact manifest to attest the record and binary, not merely a self-reported state hash", async () => { + const missingManifest = await makeVerifiedPackage("missing-manifest"); + await rm(path.join(missingManifest, "artifacts.json")); + expect(readVerifiedContextLoaderPackage(missingManifest)).toBeUndefined(); + + const forgedState = await makeVerifiedPackage("trusted"); + const forgedBytes = Buffer.from("#!/bin/sh\n# forged\n"); + const forgedDigest = createHash("sha256").update(forgedBytes).digest("hex"); + await writeFile(path.join(forgedState, "bin", "wren-context-loader"), forgedBytes, { mode: 0o755 }); + const state = JSON.parse(await readFile(path.join(forgedState, "install-state.json"), "utf8")); + await writeFile(path.join(forgedState, "install-state.json"), JSON.stringify({ ...state, binarySha256: forgedDigest })); + expect(readVerifiedContextLoaderPackage(forgedState)).toBeUndefined(); + }); + + it("rejects final-file and ancestor-directory symlinks even when their bytes match the declared digest", async () => { + const finalLink = await makeVerifiedPackage("final-link"); + const finalTarget = path.join(await mkdtemp(path.join(os.tmpdir(), "wren-harness-loader-external-")), "wren-context-loader"); + const finalBytes = await readFile(path.join(finalLink, "bin", "wren-context-loader")); + await writeFile(finalTarget, finalBytes, { mode: 0o755 }); + await rm(path.join(finalLink, "bin", "wren-context-loader")); + await symlink(finalTarget, path.join(finalLink, "bin", "wren-context-loader")); + expect(readVerifiedContextLoaderPackage(finalLink)).toBeUndefined(); + + const ancestorLink = await makeVerifiedPackage("ancestor-link"); + const externalDir = await mkdtemp(path.join(os.tmpdir(), "wren-harness-loader-external-dir-")); + const ancestorBytes = await readFile(path.join(ancestorLink, "bin", "wren-context-loader")); + await writeFile(path.join(externalDir, "wren-context-loader"), ancestorBytes, { mode: 0o755 }); + await rm(path.join(ancestorLink, "bin"), { recursive: true }); + await symlink(externalDir, path.join(ancestorLink, "bin")); + expect(readVerifiedContextLoaderPackage(ancestorLink)).toBeUndefined(); + }); }); /** diff --git a/apps/genbi/test/packaging-contract.test.ts b/apps/genbi/test/packaging-contract.test.ts index 3fd3f76f4..1d72d0cf4 100644 --- a/apps/genbi/test/packaging-contract.test.ts +++ b/apps/genbi/test/packaging-contract.test.ts @@ -23,6 +23,8 @@ describe("installed-package and vendor-contract CI wiring", () => { expect(workflow).toContain("macOS packed-install context-loader acceptance"); expect(workflow).toContain("runs-on: macos-14"); expect(workflow).toContain("pnpm run check:installed-package"); + expect(workflow).toContain("Test context-loader installer contract"); + expect(workflow).toContain("pnpm --dir apps/context-loader test"); }); it("keeps the deterministic vendor-contract entry point explicit", () => {