diff --git a/scripts/build-release-changelog.ts b/scripts/build-release-changelog.ts index c7868db572..7a7a07ad68 100644 --- a/scripts/build-release-changelog.ts +++ b/scripts/build-release-changelog.ts @@ -430,18 +430,30 @@ async function generateGitHubNotes( ]); } +const GIT_OBJECT_ID_PATTERN = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/; + export function parseGitLog(raw: string): Array> { + if (raw === "") return []; + if (!raw.endsWith("\0")) { + throw new Error("git log produced a malformed release commit record"); + } + + const fields = raw.split("\0"); + fields.pop(); + if (fields.length % 3 !== 0) { + throw new Error("git log produced a malformed release commit record"); + } + const commits: Array> = []; - for (const record of raw.split("\x1e")) { - if (!record.trim()) continue; - const [sha, subject, ...bodyParts] = record.replace(/^\n+/, "").split("\x1f"); - if (!sha?.trim() || !subject?.trim()) { + for (let index = 0; index < fields.length; index += 3) { + const [sha, subject, body] = fields.slice(index, index + 3); + if (!sha || !GIT_OBJECT_ID_PATTERN.test(sha) || !subject?.trim() || body === undefined) { throw new Error("git log produced a malformed release commit record"); } commits.push({ - sha: sha.trim(), + sha, subject: subject.trim(), - body: bodyParts.join("\x1f").trim(), + body: body.trim(), }); } return commits; @@ -464,7 +476,8 @@ async function releaseCommits( "log", "--first-parent", "--reverse", - "--format=%H%x1f%s%x1f%B%x1e", + "-z", + "--format=%H%x00%s%x00%B", range, ]); return parseGitLog(raw); diff --git a/tests/build-release-changelog.test.ts b/tests/build-release-changelog.test.ts index 7baf966d91..cfa9faa14c 100644 --- a/tests/build-release-changelog.test.ts +++ b/tests/build-release-changelog.test.ts @@ -1,4 +1,7 @@ import { describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { buildReleaseNotes, categoryForTitle, @@ -136,10 +139,10 @@ describe("commit helpers", () => { describe("release metadata parsers", () => { test("parses multiline git-log records and trailing separators", () => { const raw = [ - `${sha("a")}\x1ffix(core): first change\x1ffix(core): first change\n\nline one\nline two\x1e`, - `${sha("b")}\x1ffeat(api): second change\x1ffeat(api): second change\x1e`, + sha("a"), "fix(core): first change", "fix(core): first change\n\nline one\nline two", + sha("b"), "feat(api): second change", "feat(api): second change", "", - ].join("\n"); + ].join("\0"); expect(parseGitLog(raw)).toEqual([ { @@ -156,12 +159,104 @@ describe("release metadata parsers", () => { }); test("fails closed on malformed git-log records", () => { - expect(() => parseGitLog(`\x1ffix(core): missing sha\x1fbody\x1e`)).toThrow( + expect(parseGitLog("")).toEqual([]); + expect(() => parseGitLog(`\0fix(core): missing sha\0body\0`)).toThrow( "malformed release commit record", ); - expect(() => parseGitLog(`${sha("a")}\x1f\x1fbody\x1e`)).toThrow( + expect(() => parseGitLog(`${sha("a")}\0\0body\0`)).toThrow( "malformed release commit record", ); + expect(() => parseGitLog(`${sha("a")}\0fix(core): missing body\0`)).toThrow( + "malformed release commit record", + ); + expect(() => parseGitLog(`${sha("a")}\0fix(core): truncated\0body`)).toThrow( + "malformed release commit record", + ); + expect(() => parseGitLog(`not-a-sha\0fix(core): invalid sha\0body\0`)).toThrow( + "malformed release commit record", + ); + expect(() => parseGitLog(`${sha("a")} \0fix(core): padded sha\0body\0`)).toThrow( + "malformed release commit record", + ); + }); + + test("accepts full SHA-1 and SHA-256 object ids", () => { + const sha256 = "b".repeat(64); + const raw = [ + sha("a"), "fix(core): sha-1", "sha-1 body", + sha256, "fix(core): sha-256", "sha-256 body", + "", + ].join("\0"); + + expect(parseGitLog(raw).map(item => item.sha)).toEqual([sha("a"), sha256]); + }); + + test("preserves control bytes in commit subjects and bodies", () => { + const subject = "release: v1.2.3\x1ffix: visible change"; + const body = `${subject}\n\nrecord separator: \x1e`; + + expect(parseGitLog(`${sha("a")}\0${subject}\0${body}\0`)).toEqual([{ + sha: sha("a"), + subject, + body, + }]); + }); + + test("parses actual NUL-delimited git-log output without control-byte collisions", () => { + const repo = mkdtempSync(join(tmpdir(), "ocx-release-log-")); + const gitText = (args: string[]): string => { + const result = Bun.spawnSync(["git", "-C", repo, ...args], { + stdout: "pipe", + stderr: "pipe", + }); + if (result.exitCode !== 0) { + throw new Error(result.stderr.toString().trim() || `git ${args[0]} failed`); + } + return result.stdout.toString(); + }; + + try { + gitText(["init", "--quiet"]); + const tree = gitText(["write-tree"]).trim(); + const firstSubject = "fix: first \x1f"; + const firstBody = `${firstSubject}\n\nbody \x1e`; + const firstMessage = join(repo, "first-message.txt"); + writeFileSync(firstMessage, `${firstBody}\n`, "utf8"); + const first = gitText([ + "-c", "user.name=OpenCodex Test", + "-c", "user.email=test@example.test", + "commit-tree", tree, + "-F", firstMessage, + ]).trim(); + + const secondMessage = join(repo, "second-message.txt"); + writeFileSync(secondMessage, "feat: second\n", "utf8"); + const second = gitText([ + "-c", "user.name=OpenCodex Test", + "-c", "user.email=test@example.test", + "commit-tree", tree, + "-p", first, + "-F", secondMessage, + ]).trim(); + + const raw = gitText([ + "log", + "--first-parent", + "--reverse", + "-z", + "--format=%H%x00%s%x00%B", + second, + ]); + const fields = raw.split("\0"); + expect(fields.pop()).toBe(""); + expect(fields).toHaveLength(6); + expect(parseGitLog(raw)).toEqual([ + { sha: first, subject: firstSubject, body: firstBody }, + { sha: second, subject: "feat: second", body: "feat: second" }, + ]); + } finally { + rmSync(repo, { recursive: true, force: true }); + } }); test("normalizes associated pull metadata safely", () => {