Maxim
adaeda2dc6
Round-2 review fixes for the GITHUB_OUTPUT helper and the release scripts
that emit through it.
emitGithubOutputs (scripts/release/lib/github-output.ts):
- Replace the key newline/CR check with a full GitHub-Actions-safe charset
check: /^[A-Za-z_][A-Za-z0-9_-]*$/. A key containing "=" or whitespace
would silently corrupt the key=value line; rejecting up-front is
strictly safer. Value validation (single-line) is unchanged — "=" in
values is legal because GitHub splits on the first "=".
- Update the docblock accordingly.
prerelease.ts:
- Remove the dead `?? getCurrentVersion(scope)` fallback. The empty-list
guard above makes packages[0] guaranteed, and the fallback would have
masked a package.json missing its version field by emitting a version
divergent from what the loop publishes. Fail loudly with an explicit
exit instead.
- Drop the now-unused getCurrentVersion import.
- Add a comment above the dry-run emitGithubOutputs call explaining that
emitting in dry-run is safe — the publish workflow gates publish + the
verify guard on inputs.dry-run != true, so the dry-run emission only
serves local/e2e contract verification.
publish-release.ts:
- Hoist getPackagesForScope + empty-list guard above the prerelease-suffix
and registry checks. A misconfigured scope now fails with the clear
"no packages found" error instead of a misleading "not greater than
published" one. Loop is unchanged.
github-output.test.ts:
- Loosen the key-newline assertion from the JSON.stringify-coupled
/bad\\nkey/ to the stable /alphanumeric/ phrase from the new message.
- Add tests: "=" in key throws, space in key throws, empty key throws,
and "=" in value is accepted and written verbatim (note=a=b).
- Move vi.restoreAllMocks() to the top of afterEach so spies cannot leak
into env restore + rmSync cleanup.
Call sites audited:
- emitGithubOutputs: only ever called with {version, scope} (prerelease,
publish-release) — all valid under the new charset.
- publishVersion derivation: only used inside prerelease.ts main().
- getCurrentVersion: still imported by publish-release.ts, bump-prerelease.ts,
prepare-release.ts; only the prerelease.ts import was removed.
- getPackagesForScope hoist in publish-release.ts: `packages` was only
read inside the publish loop below; nothing earlier depended on it.
94 lines
2.7 KiB
TypeScript
94 lines
2.7 KiB
TypeScript
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
|
import fs from "fs";
|
|
import path from "path";
|
|
import os from "os";
|
|
import { emitGithubOutputs } from "./github-output.js";
|
|
|
|
let tmpDir: string;
|
|
let outputFile: string;
|
|
const originalGithubOutput = process.env.GITHUB_OUTPUT;
|
|
|
|
beforeEach(() => {
|
|
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-output-"));
|
|
outputFile = path.join(tmpDir, "output");
|
|
fs.writeFileSync(outputFile, "");
|
|
process.env.GITHUB_OUTPUT = outputFile;
|
|
});
|
|
|
|
afterEach(() => {
|
|
// Restore spies first so they cannot leak into the env/fs cleanup below.
|
|
vi.restoreAllMocks();
|
|
if (originalGithubOutput === undefined) {
|
|
delete process.env.GITHUB_OUTPUT;
|
|
} else {
|
|
process.env.GITHUB_OUTPUT = originalGithubOutput;
|
|
}
|
|
fs.rmSync(tmpDir, { recursive: true, force: true });
|
|
});
|
|
|
|
describe("emitGithubOutputs", () => {
|
|
it("appends key=value lines to the GITHUB_OUTPUT file", () => {
|
|
emitGithubOutputs({ version: "1.2.3-canary.42", scope: "monorepo" });
|
|
|
|
expect(fs.readFileSync(outputFile, "utf8")).toBe(
|
|
"version=1.2.3-canary.42\nscope=monorepo\n",
|
|
);
|
|
});
|
|
|
|
it("appends without truncating prior outputs", () => {
|
|
fs.writeFileSync(outputFile, "earlier=value\n");
|
|
|
|
emitGithubOutputs({ version: "1.2.3" });
|
|
|
|
expect(fs.readFileSync(outputFile, "utf8")).toBe(
|
|
"earlier=value\nversion=1.2.3\n",
|
|
);
|
|
});
|
|
|
|
it("is a no-op when GITHUB_OUTPUT is unset", () => {
|
|
delete process.env.GITHUB_OUTPUT;
|
|
const appendSpy = vi.spyOn(fs, "appendFileSync");
|
|
|
|
expect(() => emitGithubOutputs({ version: "1.2.3" })).not.toThrow();
|
|
expect(appendSpy).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("throws when a value contains a newline, naming the offending key", () => {
|
|
expect(() =>
|
|
emitGithubOutputs({ version: "1.2.3\nmalicious=evil" }),
|
|
).toThrow(/version/);
|
|
});
|
|
|
|
it("throws when a value contains a carriage return", () => {
|
|
expect(() => emitGithubOutputs({ version: "1.2.3\r" })).toThrow(/version/);
|
|
});
|
|
|
|
it("throws when a key contains a newline", () => {
|
|
expect(() => emitGithubOutputs({ "bad\nkey": "value" })).toThrow(
|
|
/alphanumeric/,
|
|
);
|
|
});
|
|
|
|
it("throws when a key contains '='", () => {
|
|
expect(() => emitGithubOutputs({ "bad=key": "value" })).toThrow(
|
|
/alphanumeric/,
|
|
);
|
|
});
|
|
|
|
it("throws when a key contains a space", () => {
|
|
expect(() => emitGithubOutputs({ "bad key": "value" })).toThrow(
|
|
/alphanumeric/,
|
|
);
|
|
});
|
|
|
|
it("throws when a key is empty", () => {
|
|
expect(() => emitGithubOutputs({ "": "value" })).toThrow(/alphanumeric/);
|
|
});
|
|
|
|
it("accepts a value containing '=' and writes it verbatim", () => {
|
|
emitGithubOutputs({ note: "a=b" });
|
|
|
|
expect(fs.readFileSync(outputFile, "utf8")).toBe("note=a=b\n");
|
|
});
|
|
});
|