diff --git a/actions/setup/js/push_signed_commits.cjs b/actions/setup/js/push_signed_commits.cjs index ce652f233be..802feddda69 100644 --- a/actions/setup/js/push_signed_commits.cjs +++ b/actions/setup/js/push_signed_commits.cjs @@ -1,10 +1,6 @@ // @ts-check /// -/** @type {typeof import("fs")} */ -const fs = require("fs"); -/** @type {typeof import("path")} */ -const path = require("path"); const { ERR_API } = require("./error_codes.cjs"); /** @@ -80,6 +76,33 @@ function unquoteCPath(s) { return Buffer.from(bytes).toString("utf8"); } +/** + * Read a blob from a specific commit as a base64-encoded string using + * `git show :`. The raw bytes emitted by git are collected via + * the `exec.exec` stdout listener so that binary files are not corrupted by + * any UTF-8 decoding layer (unlike `exec.getExecOutput` which always passes + * stdout through a `StringDecoder('utf8')`). + * + * @param {string} sha - Commit SHA to read the blob from + * @param {string} filePath - Repo-relative path of the file + * @param {string} cwd - Working directory of the local git checkout + * @returns {Promise} Base64-encoded file contents + */ +async function readBlobAsBase64(sha, filePath, cwd) { + /** @type {Buffer[]} */ + const chunks = []; + await exec.exec("git", ["show", `${sha}:${filePath}`], { + cwd, + silent: true, + listeners: { + stdout: (/** @type {Buffer} */ data) => { + chunks.push(data); + }, + }, + }); + return Buffer.concat(chunks).toString("base64"); +} + /** * @fileoverview Signed Commit Push Helper * @@ -180,8 +203,7 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c if (dstMode === "100755") { core.warning(`pushSignedCommits: executable bit on ${renamedPath} will be lost in signed commit (GitHub GraphQL does not support mode 100755)`); } - const content = fs.readFileSync(path.join(cwd, renamedPath)); - additions.push({ path: renamedPath, contents: content.toString("base64") }); + additions.push({ path: renamedPath, contents: await readBlobAsBase64(sha, renamedPath, cwd) }); } else if (status && status.startsWith("C")) { // Copy: source path is kept (no deletion), only the destination path is added const copiedPath = unquoteCPath(paths[1]); @@ -196,8 +218,7 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c if (dstMode === "100755") { core.warning(`pushSignedCommits: executable bit on ${copiedPath} will be lost in signed commit (GitHub GraphQL does not support mode 100755)`); } - const content = fs.readFileSync(path.join(cwd, copiedPath)); - additions.push({ path: copiedPath, contents: content.toString("base64") }); + additions.push({ path: copiedPath, contents: await readBlobAsBase64(sha, copiedPath, cwd) }); } else { // Added or Modified if (dstMode === "120000") { @@ -207,8 +228,7 @@ async function pushSignedCommits({ githubClient, owner, repo, branch, baseRef, c if (dstMode === "100755") { core.warning(`pushSignedCommits: executable bit on ${filePath} will be lost in signed commit (GitHub GraphQL does not support mode 100755)`); } - const content = fs.readFileSync(path.join(cwd, filePath)); - additions.push({ path: filePath, contents: content.toString("base64") }); + additions.push({ path: filePath, contents: await readBlobAsBase64(sha, filePath, cwd) }); } } diff --git a/actions/setup/js/push_signed_commits.test.cjs b/actions/setup/js/push_signed_commits.test.cjs index d3c9e6f5084..bcc3f5caf18 100644 --- a/actions/setup/js/push_signed_commits.test.cjs +++ b/actions/setup/js/push_signed_commits.test.cjs @@ -186,17 +186,24 @@ function makeRealExec(cwd) { /** * @param {string} program * @param {string[]} args - * @param {{ cwd?: string, env?: NodeJS.ProcessEnv }} [opts] + * @param {{ cwd?: string, env?: NodeJS.ProcessEnv, silent?: boolean, listeners?: { stdout?: (data: Buffer) => void } }} [opts] */ exec: async (program, args, opts = {}) => { + const stdoutListener = opts.listeners?.stdout; const result = spawnSync(program, args, { - encoding: "utf8", + // Use raw Buffer encoding when a stdout listener is provided so binary + // content is not corrupted by UTF-8 decoding. + encoding: stdoutListener ? null : "utf8", cwd: opts.cwd ?? cwd, env: opts.env ?? { ...process.env, GIT_CONFIG_NOSYSTEM: "1", HOME: os.tmpdir() }, }); if (result.error) throw result.error; if (result.status !== 0) { - throw new Error(`${program} ${args.join(" ")} failed:\n${result.stderr}`); + const stderr = Buffer.isBuffer(result.stderr) ? result.stderr.toString("utf8") : (result.stderr ?? ""); + throw new Error(`${program} ${args.join(" ")} failed:\n${stderr}`); + } + if (stdoutListener && result.stdout) { + stdoutListener(Buffer.isBuffer(result.stdout) ? result.stdout : Buffer.from(result.stdout)); } return result.status ?? 0; }, @@ -324,6 +331,47 @@ describe("push_signed_commits integration tests", () => { expect(headlines).toEqual(["Add file-a.txt", "Add file-b.txt"]); }); + it("each commit in a series should carry its own file content, not the working-tree tip", async () => { + // Regression test for the bug where fs.readFileSync always read from the + // working tree (HEAD), so intermediate commits A and B would contain the + // content of C when A→B→C were replayed. + execGit(["checkout", "-b", "versioned-branch"], { cwd: workDir }); + + fs.writeFileSync(path.join(workDir, "data.txt"), "version A\n"); + execGit(["add", "data.txt"], { cwd: workDir }); + execGit(["commit", "-m", "Version A"], { cwd: workDir }); + + fs.writeFileSync(path.join(workDir, "data.txt"), "version B\n"); + execGit(["add", "data.txt"], { cwd: workDir }); + execGit(["commit", "-m", "Version B"], { cwd: workDir }); + + fs.writeFileSync(path.join(workDir, "data.txt"), "version C\n"); + execGit(["add", "data.txt"], { cwd: workDir }); + execGit(["commit", "-m", "Version C"], { cwd: workDir }); + + execGit(["push", "-u", "origin", "versioned-branch"], { cwd: workDir }); + + global.exec = makeRealExec(workDir); + const githubClient = makeMockGithubClient(); + + await pushSignedCommits({ + githubClient, + owner: "test-owner", + repo: "test-repo", + branch: "versioned-branch", + baseRef: "origin/main", + cwd: workDir, + }); + + expect(githubClient.graphql).toHaveBeenCalledTimes(3); + const calls = githubClient.graphql.mock.calls.map(c => c[1].input); + + // Each commit must carry its own version of data.txt, not the working-tree tip (C) + expect(Buffer.from(calls[0].fileChanges.additions[0].contents, "base64").toString()).toBe("version A\n"); + expect(Buffer.from(calls[1].fileChanges.additions[0].contents, "base64").toString()).toBe("version B\n"); + expect(Buffer.from(calls[2].fileChanges.additions[0].contents, "base64").toString()).toBe("version C\n"); + }); + it("should include deletions when files are removed in a commit", async () => { execGit(["checkout", "-b", "delete-branch"], { cwd: workDir });