From 1caffc5f266283517ddf9e3d9365e1de2041e62d Mon Sep 17 00:00:00 2001 From: ekklesia Date: Thu, 8 Oct 2026 17:51:22 -0500 Subject: [PATCH] fix: hand npm the release artifact's absolute path, so publishing finds it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first real 0.2.0 publish failed on its first package with ENOENT: "Publishing ekklesia-contracts-0.2.0.tgz failed: npm error code ENOENT". The publish workflow passes a relative release path, --release "$(find package-train …)", which is package-train/ekklesia-packages-0.2.0. The tool validated the artifacts relative to the job's working directory, but ran npm in its own temporary home (cwd /tmp/ekklesia-publish-XXXXXX) with the same relative path, package-train/ekklesia-packages-0.2.0/ekklesia-contracts-0.2.0.tgz, which there names nothing. The disposable-registry suite always passed an absolute path, so it never saw this. - loadReleaseTrain resolves the release directory once; every artifact path is absolute from validation on, and npm is given only that exact file's absolute path (anything else is refused at the npm boundary). - Just before each upload the file is checked against the validated bytes once more; the tool logs the absolute path it publishes, and a failure names it with npm's path/syscall lines (npm's temporary home masked, the token never shown). - Regression tests: unit tests with a recording npm (relative nested release from another cwd; npm's argv, cwd, and the file's existence; a changed artifact refused before npm runs), which fail on the previous tool; and a disposable-Gitea test that runs the CLI exactly as the workflow does, from the Actions artifact layout, round-tripping all nine packages byte for byte. Co-Authored-By: Claude Opus 5.5 --- docs/development/releases.md | 7 +- scripts/publish-package-train.ts | 40 ++++++-- scripts/test-package-publication.sh | 2 +- tests/package-publication.integration.ts | 60 +++++++++++- tests/package-publication.test.ts | 115 ++++++++++++++++++++++- 5 files changed, 207 insertions(+), 17 deletions(-) diff --git a/docs/development/releases.md b/docs/development/releases.md index 2faaca1..46e4a47 100644 --- a/docs/development/releases.md +++ b/docs/development/releases.md @@ -243,9 +243,10 @@ never builds, packs, or edits a package, and it never deletes, unpublishes, or o name, size, SHA-256, integrity, `SHA256SUMS` line, and the `package.json` inside it must match. 2. **Publishes in dependency order.** Each package comes after the train packages it depends on, then alphabetically. For each package: - - **absent:** `npm publish ` under the staging dist-tag `ekklesia-staging`. npm sends the - file's own bytes and runs no script for a file; the registry checks them against their - integrity and stores them as they are; + - **absent:** `npm publish ` under the staging dist-tag `ekklesia-staging`, given the + artifact's absolute path (npm runs in a temporary directory of its own), after the file is + checked once more against the manifest. npm sends the file's own bytes and runs no script for a + file; the registry checks them against their integrity and stores them as they are; - **present with the same bytes:** accepted as already published, and verified; - **present with other bytes:** a hard stop. Nothing is deleted or overwritten, nothing after it is published, and the version is burned ([Immutability](#immutability)). diff --git a/scripts/publish-package-train.ts b/scripts/publish-package-train.ts index 4711ff4..7cdb428 100644 --- a/scripts/publish-package-train.ts +++ b/scripts/publish-package-train.ts @@ -109,6 +109,7 @@ export function tarballPackageJson(tarball: Uint8Array): Record export interface LocalPackage { readonly entry: TrainPackage; + /** The artifact's absolute path: the exact file validated against the release manifest. */ readonly path: string; readonly data: Buffer; readonly packageJson: Readonly>; @@ -129,9 +130,12 @@ export async function loadReleaseTrain( releaseDirectory: string, expected: { readonly version: string; readonly revision?: string }, ): Promise { + // Resolved once, here: every artifact path from now on is absolute, so the file npm is handed + // is the file validated below, whatever directory npm itself runs in. + const directory = path.resolve(releaseDirectory); const files = new Map(); - for (const file of await readdir(releaseDirectory)) { - files.set(file, await readFile(path.join(releaseDirectory, file))); + for (const file of await readdir(directory)) { + files.set(file, await readFile(path.join(directory, file))); } const manifestBytes = files.get(PACKAGE_TRAIN_MANIFEST); if (manifestBytes === undefined) { @@ -156,7 +160,7 @@ export async function loadReleaseTrain( data, entry, packageJson: tarballPackageJson(data), - path: path.join(releaseDirectory, entry.file), + path: path.join(directory, entry.file), }; }), }; @@ -525,9 +529,9 @@ async function npmSession(registry: string, token: string, npm: string): Promise const stderr = String((error as { stderr?: unknown }).stderr ?? ""); const summary = stderr .split("\n") - .filter((line) => /^npm error (?:code|\d{3}|[A-Z])/u.test(line)) + .filter((line) => /^npm error (?:code|path|syscall|errno|enoent|\d{3}|[A-Z])/u.test(line)) .filter((line) => !line.includes("complete log")) - .slice(0, 3) + .slice(0, 5) .join(" / ") .replaceAll(token, "***") .replaceAll(home, ""); @@ -541,13 +545,22 @@ async function npmSession(registry: string, token: string, npm: string): Promise addTag: (name, version, tag) => run(["dist-tag", "add", `${name}@${version}`, tag], `Moving ${tag} to ${name}@${version}`), dispose: () => rm(home, { force: true, recursive: true }), - publish: (file, tag) => - run( + publish: async (file, tag) => { + // npm runs in its own temporary home, so a relative path would name a file there, not the + // release artifact. Only the artifact's absolute path is ever passed. + if (!path.isAbsolute(file)) { + throw new PublicationError( + "PUBLISH_FAILED", + `Refusing to publish ${file}: npm is only ever given an artifact's absolute path.`, + ); + } + await run( // A file spec runs no lifecycle script and sends the file's own bytes; --ignore-scripts // says so explicitly. --access public is what a scoped package needs on a public registry. ["publish", file, "--tag", tag, "--access", "public", "--ignore-scripts"], - `Publishing ${path.basename(file)}`, - ), + `Publishing ${path.basename(file)} from ${file}`, + ); + }, }; } @@ -677,6 +690,15 @@ export async function publishPackageTrain(options: PublishOptions): Promise undefined); + if (current === undefined || !current.equals(local.data)) { + throw new PublicationError( + "INVALID_RELEASE", + `${local.path} is missing or no longer the artifact validated against the release manifest.`, + ); + } + log(`${name}@${version}: publishing ${local.path} (sha256 ${local.entry.sha256})`); try { await npm.publish(local.path, stagingTag); outcome = "published"; diff --git a/scripts/test-package-publication.sh b/scripts/test-package-publication.sh index b39db52..0a42846 100755 --- a/scripts/test-package-publication.sh +++ b/scripts/test-package-publication.sh @@ -110,7 +110,7 @@ for user in publisher outsider; do password_variable="${user}_password" api POST /admin/users "{\"username\":\"$user\",\"password\":\"${!password_variable}\",\"email\":\"$user@example.invalid\",\"must_change_password\":false}" >/dev/null done -owners=(River_of_Glory_Church ekklesia-burned-version ekklesia-tag-retry) +owners=(River_of_Glory_Church ekklesia-burned-version ekklesia-tag-retry ekklesia-artifact-layout) for owner in "${owners[@]}"; do api POST /orgs "{\"username\":\"$owner\",\"visibility\":\"public\"}" >/dev/null team="$(api POST "/orgs/$owner/teams" '{"name":"package-publishers","includes_all_repositories":false,"can_create_org_repo":false,"units":["repo.packages"],"units_map":{"repo.packages":"write"}}' | diff --git a/tests/package-publication.integration.ts b/tests/package-publication.integration.ts index d44c71a..994cf95 100644 --- a/tests/package-publication.integration.ts +++ b/tests/package-publication.integration.ts @@ -1,5 +1,5 @@ import { execFile } from "node:child_process"; -import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { cp, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { promisify } from "node:util"; @@ -28,8 +28,8 @@ const origin = setting("EKKLESIA_TEST_REGISTRY_ORIGIN"); const releaseDirectory = setting("EKKLESIA_TEST_RELEASE"); const publishToken = setting("EKKLESIA_TEST_PUBLISH_TOKEN"); const outsiderToken = setting("EKKLESIA_TEST_OUTSIDER_TOKEN"); -const [mainOwner, burnedOwner, tagOwner] = setting("EKKLESIA_TEST_OWNERS").split(" "); -if (!origin || !releaseDirectory || !publishToken || !outsiderToken || !tagOwner) { +const [mainOwner, burnedOwner, tagOwner, layoutOwner] = setting("EKKLESIA_TEST_OWNERS").split(" "); +if (!origin || !releaseDirectory || !publishToken || !outsiderToken || !layoutOwner) { throw new Error("Run this suite with scripts/test-package-publication.sh."); } const registryOf = (owner: string | undefined) => @@ -303,3 +303,57 @@ describe("moving the final tag, interrupted and resumed", () => { } }); }); + +describe("the publish workflow's own invocation", () => { + it("publishes from the Actions artifact layout, given a relative release path from another directory", async () => { + // Exactly as .github/workflows/publish-packages.yml runs it: the artifact unpacked under + // `package-train/` in the job's workspace, and `--release "$(find package-train …)"`. + const workspace = await mkdtemp(path.join(tmpdir(), "ekklesia-publish-workspace-")); + scratch.push(workspace); + const releaseName = path.basename(releaseDirectory); + await mkdir(path.join(workspace, "package-train"), { recursive: true }); + await cp(releaseDirectory, path.join(workspace, "package-train", releaseName), { + recursive: true, + }); + const record = path.join(workspace, "publication-record.json"); + const { stdout } = await execFileAsync( + process.execPath, + [ + path.resolve("scripts/publish-package-train.ts"), + "--release", + `package-train/${releaseName}`, + "--registry", + registryOf(layoutOwner), + "--version", + train.manifest.version, + "--revision", + train.manifest.source.revision, + "--manifest-sha256", + train.manifestSha256, + "--record", + record, + "--require-public-read", + "--allow-http", + ], + { + cwd: workspace, + env: { [PUBLISH_TOKEN_VARIABLE]: publishToken, PATH: process.env.PATH ?? "" }, + }, + ); + expect(stdout).toContain( + `publishing ${path.join(workspace, "package-train", releaseName, local("@ekklesia/contracts").entry.file)}`, + ); + expect(stdout).not.toContain(publishToken); + const published = JSON.parse(await readFile(record, "utf8")) as PublicationRecord; + expect(published.packages.map((entry) => entry.outcome)).toEqual(Array(9).fill("published")); + for (const name of order) { + const version = (await anonymousPackument(layoutOwner, name))?.versions[ + train.manifest.version + ]; + expect( + (await anonymousBytes(version?.dist.tarball ?? "")).equals(local(name).data), + name, + ).toBe(true); + } + }); +}); diff --git a/tests/package-publication.test.ts b/tests/package-publication.test.ts index 988bead..1bffe56 100644 --- a/tests/package-publication.test.ts +++ b/tests/package-publication.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { chmod, cp, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; @@ -462,6 +462,119 @@ describe("the registry and its credentials", () => { }); }); +describe("the artifact npm is given", () => { + /** + * The release where the Actions artifact puts it, `/package-train/`, named + * relatively from the current directory, which is not the release directory: exactly how the + * publish workflow called the tool when npm was handed a path relative to its own temporary home. + */ + async function nestedRelease(): Promise<{ relative: string; absolute: string }> { + const workspace = await mkdtemp(path.join(tmpdir(), "ekklesia-workspace-")); + scratch.push(workspace); + const absolute = path.join(workspace, "package-train", `ekklesia-packages-${VERSION}`); + await mkdir(path.dirname(absolute), { recursive: true }); + await cp(await releaseDirectory(), absolute, { recursive: true }); + const relative = path.relative(process.cwd(), absolute); + expect(path.isAbsolute(relative)).toBe(false); + expect(process.cwd()).not.toBe(absolute); + return { absolute, relative }; + } + + /** An npm that records what it was given, and where it ran, then fails as a registry would. */ + async function recordingNpm(): Promise<{ + npm: string; + calls: () => Promise<{ argv: string[]; cwd: string; exists: boolean }[]>; + }> { + const directory = await mkdtemp(path.join(tmpdir(), "ekklesia-fake-npm-")); + scratch.push(directory); + const log = path.join(directory, "calls.jsonl"); + const npm = path.join(directory, "npm"); + await writeFile( + npm, + `#!/usr/bin/env node +const { appendFileSync, existsSync } = require("node:fs"); +const argv = process.argv.slice(2); +appendFileSync(${JSON.stringify(log)}, JSON.stringify({ argv, cwd: process.cwd(), exists: existsSync(argv[1] ?? "") }) + "\\n"); +process.stderr.write("npm error code E503\\nnpm error 503 Service Unavailable\\n"); +process.exit(1); +`, + ); + await chmod(npm, 0o755); + return { + calls: async () => + (await readFile(log, "utf8").catch(() => "")) + .split("\n") + .filter(Boolean) + .map((line) => JSON.parse(line)), + npm, + }; + } + + const notFound = (async () => new Response("{}", { status: 404 })) as typeof fetch; + + it("resolves a relative, nested release to absolute artifact paths when it loads the train", async () => { + const { absolute, relative } = await nestedRelease(); + const train = await loadReleaseTrain(relative, { revision: REVISION, version: VERSION }); + for (const local of train.packages) { + expect(local.path).toBe(path.join(absolute, local.entry.file)); + expect(path.isAbsolute(local.path)).toBe(true); + expect((await readFile(local.path)).equals(local.data)).toBe(true); + } + }); + + it("hands npm the exact validated tarball by its absolute path, from npm's own directory, and says which", async () => { + const { absolute, relative } = await nestedRelease(); + const { calls, npm } = await recordingNpm(); + const error = await publishPackageTrain({ + fetch: notFound, + npm, + registry: REGISTRY, + releaseDirectory: relative, + revision: REVISION, + token: "publish-token-value", + version: VERSION, + }).catch((caught: unknown) => caught); + expect(error).toMatchObject({ code: "PUBLISH_FAILED" }); + const first = tarballName("@ekklesia/contracts", VERSION); + const expected = path.join(absolute, first); + // The failure names the exact file npm was given, and never the token. + expect((error as Error).message).toContain(expected); + expect((error as Error).message).toContain("E503"); + expect((error as Error).message).not.toContain("publish-token-value"); + + const [call] = await calls(); + expect(call?.argv.slice(0, 2)).toEqual(["publish", expected]); + expect(call?.exists).toBe(true); + // npm ran in a directory of its own, where a relative path would have named nothing. + expect(call?.cwd).not.toBe(absolute); + expect(path.isAbsolute(call?.cwd ?? "")).toBe(true); + expect(JSON.stringify(call)).not.toContain("publish-token-value"); + }); + + it("refuses to publish an artifact that changed after it was validated, before npm runs", async () => { + const { absolute, relative } = await nestedRelease(); + const { calls, npm } = await recordingNpm(); + const file = path.join(absolute, tarballName("@ekklesia/contracts", VERSION)); + await expect( + publishPackageTrain({ + fetch: notFound, + hooks: { + beforePublish: async (_name, index) => { + if (index === 0) await writeFile(file, "changed after validation"); + }, + }, + npm, + registry: REGISTRY, + releaseDirectory: relative, + revision: REVISION, + token: "publish-token-value", + version: VERSION, + }), + ).rejects.toMatchObject({ code: "INVALID_RELEASE" }); + expect(await calls()).toEqual([]); + }); +}); + describe("what the publisher may do to a registry", () => { it("never deletes, unpublishes, or overwrites: it only publishes new versions and moves a tag", async () => { // Code only: the tool's comments may say what it never does. -- 2.54.0