Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/traceable-commits.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"patchworks": patch
---

Preserve each template commit author on generated update commits and record the GitHub Action automation separately as the committer, so author-mapped preview systems can resolve the source contributor instead of an unowned bot address.
20 changes: 16 additions & 4 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ inputs:
required: false
default: ""
author:
description: "Optional commit author in 'Name <email>' format"
description: "Optional caller-attested commit author override in 'Name <email>' format; defaults to the template commit author"
required: false
default: ""
committer:
description: "Optional commit committer in 'Name <email>' format"
description: "Optional commit committer override in 'Name <email>' format; defaults to the GitHub Actions bot for github.com GITHUB_TOKEN runs"
required: false
default: ""

Expand Down Expand Up @@ -135,6 +135,18 @@ runs:
PATCHWORKS_WORKSPACE: ${{ github.workspace }}
run: node "$GITHUB_ACTION_PATH/action/entrypoint.mjs" collect

- name: Resolve generated commit identity
if: steps.preflight.outputs.skip != 'true' && steps.patchworks.outputs.has_changes == 'true'
id: identity
shell: bash
env:
PATCHWORKS_AUTHOR: ${{ inputs.author }}
PATCHWORKS_COMMITTER: ${{ inputs.committer }}
PATCHWORKS_CUSTOM_TOKEN: ${{ (inputs.token || env.GITHUB_TOKEN || github.token) != github.token }}
PATCHWORKS_SERVER_URL: ${{ github.server_url }}
PATCHWORKS_TEMPLATE_AUTHOR: ${{ steps.patchworks.outputs.template_author }}
run: node "$GITHUB_ACTION_PATH/action/entrypoint.mjs" identity

- name: Check workflow update permissions
if: steps.preflight.outputs.skip != 'true' && steps.patchworks.outputs.has_changes == 'true'
shell: bash
Expand All @@ -149,11 +161,11 @@ runs:
uses: peter-evans/create-pull-request@5f6978faf089d4d20b00c7766989d076bb2fc7f1 # v8.1.1
with:
add-paths: "."
author: ${{ inputs.author || format('{0} <{1}+{0}@users.noreply.github.com>', github.actor, github.actor_id) }}
author: ${{ steps.identity.outputs.author }}
base: ${{ inputs.base || github.event.repository.default_branch }}
branch: ${{ inputs.branch }}
commit-message: ${{ steps.patchworks.outputs.commit_message }}
committer: ${{ inputs.committer || format('{0} <{1}+{0}@users.noreply.github.com>', github.actor, github.actor_id) }}
committer: ${{ steps.identity.outputs.committer }}
title: ${{ steps.patchworks.outputs.pr_title }}
body: ${{ steps.patchworks.outputs.pr_body }}
token: ${{ inputs.token || env.GITHUB_TOKEN || github.token }}
Expand Down
13 changes: 13 additions & 0 deletions action/entrypoint.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
guardWorkflowChanges,
parseWorkflowFiles,
preflight,
resolveGeneratedCommitIdentity,
runUpdate,
writeCommandFile,
writeError,
Expand Down Expand Up @@ -62,6 +63,18 @@ async function main() {
return;
}

case "identity": {
const result = resolveGeneratedCommitIdentity({
authorOverride: process.env.PATCHWORKS_AUTHOR || "",
committerOverride: process.env.PATCHWORKS_COMMITTER || "",
customToken: process.env.PATCHWORKS_CUSTOM_TOKEN === "true",
serverUrl: process.env.PATCHWORKS_SERVER_URL || "https://github.com",
templateAuthor: process.env.PATCHWORKS_TEMPLATE_AUTHOR || "",
});
writeCommandFile(process.env.GITHUB_OUTPUT, result);
return;
}

case "guard-workflows": {
const result = guardWorkflowChanges({
customToken: process.env.PATCHWORKS_CUSTOM_TOKEN === "true",
Expand Down
92 changes: 92 additions & 0 deletions action/runtime.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import { spawnSync } from "node:child_process";
const WORKFLOW_PREFIX = ".github/workflows/";
const NPM_REGISTRY = "https://registry.npmjs.org/";
const MAX_CONFIG_BYTES = 1024 * 1024;
const GITHUB_ACTIONS_BOT_IDENTITY =
"github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>";
const PATCHWORKS_AUTOMATION_IDENTITY =
"Patchworks automation <automation@patchworks.invalid>";

function cleanApiErrorBody(body) {
try {
Expand Down Expand Up @@ -122,6 +126,84 @@ function reportValue(report, camelCase, snakeCase) {
return report[camelCase] ?? report[snakeCase];
}

function hasControlCharacters(value) {
return [...value].some((character) => {
const codePoint = character.codePointAt(0) ?? 0;
return codePoint < 32 || codePoint === 127;
});
}

function validateIdentityPart(value, label) {
const normalized = typeof value === "string" ? value.trim() : "";
if (
!normalized ||
hasControlCharacters(normalized) ||
normalized.includes("<") ||
normalized.includes(">")
) {
throw new Error(`${label} is not safe for Git commit attribution`);
}
return normalized;
}

export function formatGitIdentity(identity, label = "Git identity") {
if (!identity || typeof identity !== "object" || Array.isArray(identity)) {
throw new Error(`${label} must contain a name and email`);
}
const name = validateIdentityPart(identity.name, `${label} name`);
const email = validateIdentityPart(identity.email, `${label} email`);
return `${name} <${email}>`;
}

export function parseGitIdentity(value, label = "Git identity") {
const normalized = typeof value === "string" ? value.trim() : "";
const match = /^([^<>]+)<([^<>]+)>$/.exec(normalized);
if (!match) {
throw new Error(`${label} must use the 'Name <email>' format`);
}
return {
name: validateIdentityPart(match[1], `${label} name`),
email: validateIdentityPart(match[2], `${label} email`),
};
}

export function resolveGeneratedCommitIdentity({
authorOverride = "",
committerOverride = "",
customToken = false,
serverUrl = "https://github.com",
templateAuthor = "",
}) {
const author = authorOverride.trim() || templateAuthor.trim();
if (!author) {
throw new Error(
"Patchworks could not determine the template commit author. Pass the author input explicitly as 'Name <email>'.",
);
}
const parsedAuthor = parseGitIdentity(
author,
authorOverride.trim() ? "author input" : "template commit author",
);

let committer = committerOverride.trim();
if (!committer) {
const githubDotCom = serverUrl.replace(/\/+$/, "") === "https://github.com";
committer =
githubDotCom && !customToken
? GITHUB_ACTIONS_BOT_IDENTITY
: PATCHWORKS_AUTOMATION_IDENTITY;
}
const parsedCommitter = parseGitIdentity(
committer,
committerOverride.trim() ? "committer input" : "generated committer",
);

return {
author: formatGitIdentity(parsedAuthor, "resolved author"),
committer: formatGitIdentity(parsedCommitter, "resolved committer"),
};
}

function isRejectArtifact(file) {
return (
file.endsWith(".rej") ||
Expand Down Expand Up @@ -627,6 +709,15 @@ export function collectReport({ reportPath, workspace, spawn = spawnSync }) {

const commitMessage = reportValue(report, "commitMessage", "commit_message");
const prTitle = reportValue(report, "prTitle", "pr_title");
const reportedTemplateAuthor = reportValue(
report,
"templateAuthor",
"template_author",
);
const templateAuthor =
reportedTemplateAuthor === null || reportedTemplateAuthor === undefined
? ""
: formatGitIdentity(reportedTemplateAuthor, "template commit author");
if (hasChanges && (typeof commitMessage !== "string" || !commitMessage.trim())) {
throw new Error("Patchworks report is missing commitMessage");
}
Expand Down Expand Up @@ -668,6 +759,7 @@ export function collectReport({ reportPath, workspace, spawn = spawnSync }) {
reject_files: rejectFiles.join("\n"),
reject_files_json: JSON.stringify(rejectFiles),
status,
template_author: templateAuthor,
workflow_changes: workflowFiles.length > 0 ? "true" : "false",
workflow_files: workflowFiles.join("\n"),
workflow_files_json: JSON.stringify(workflowFiles),
Expand Down
141 changes: 140 additions & 1 deletion action/runtime.test.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { spawnSync } from "node:child_process";
import {
mkdirSync,
mkdtempSync,
readFileSync,
rmSync,
writeFileSync,
} from "node:fs";
import { tmpdir } from "node:os";
Expand All @@ -17,6 +19,7 @@ import {
parseWorkflowFiles,
preflight,
readChangedPaths,
resolveGeneratedCommitIdentity,
resolvePackageSpec,
runUpdate,
writeCommandFile,
Expand All @@ -37,6 +40,23 @@ function outputFile() {
return join(directory, "output.txt");
}

function fixtureGit(directory, ...args) {
const env = { ...process.env };
for (const key of Object.keys(env)) {
if (key.startsWith("GIT_")) delete env[key];
}
const result = spawnSync("git", args, {
cwd: directory,
encoding: "utf8",
env,
});
if (result.error) throw result.error;
if (result.status !== 0) {
throw new Error(`git ${args.join(" ")} failed: ${result.stderr}`);
}
return result.stdout.trimEnd();
}

function writeConfig(directory, repository) {
writeFileSync(
join(directory, ".patchworks.json"),
Expand Down Expand Up @@ -783,14 +803,128 @@ describe("action metadata", () => {
const tokenClassification =
"${{ (inputs.token || env.GITHUB_TOKEN || github.token) != github.token }}";

expect(action.split(tokenClassification)).toHaveLength(3);
expect(action.split(tokenClassification)).toHaveLength(4);
expect(action).toContain(
"PATCHWORKS_SERVER_URL: ${{ github.server_url }}",
);
expect(action).toContain(
"PATCHWORKS_TEMPLATE_AUTHOR: ${{ steps.patchworks.outputs.template_author }}",
);
expect(action).toContain("author: ${{ steps.identity.outputs.author }}");
expect(action).toContain(
"committer: ${{ steps.identity.outputs.committer }}",
);
expect(action).not.toContain("github.actor_id");
expect(action).not.toContain("inputs.token != '' ||");
});
});

describe("generated commit identity", () => {
const templateAuthor = "Template Author <author@example.com>";
const actionsBot =
"github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>";
const neutralAutomation =
"Patchworks automation <automation@patchworks.invalid>";

it("preserves the template author and uses GitHub's bot for GITHUB_TOKEN", () => {
expect(
resolveGeneratedCommitIdentity({
customToken: false,
serverUrl: "https://github.com",
templateAuthor,
}),
).toEqual({ author: templateAuthor, committer: actionsBot });
});

it.each([
["a custom token", true, "https://github.com"],
["GitHub Enterprise", false, "https://github.example.com"],
])("uses a neutral automation committer for %s", (_label, customToken, serverUrl) => {
expect(
resolveGeneratedCommitIdentity({
customToken,
serverUrl,
templateAuthor,
}),
).toEqual({ author: templateAuthor, committer: neutralAutomation });
});

it("preserves caller-attested author and committer overrides", () => {
expect(
resolveGeneratedCommitIdentity({
authorOverride: "Downstream Author <downstream@example.com>",
committerOverride: "Template App <app@example.com>",
customToken: true,
serverUrl: "https://github.example.com",
templateAuthor,
}),
).toEqual({
author: "Downstream Author <downstream@example.com>",
committer: "Template App <app@example.com>",
});
});

it("rejects missing or unsafe attribution instead of inventing a user", () => {
expect(() => resolveGeneratedCommitIdentity({})).toThrow(
"could not determine the template commit author",
);
expect(() =>
resolveGeneratedCommitIdentity({
templateAuthor: "Unsafe\nAuthor <author@example.com>",
}),
).toThrow("template commit author name is not safe");
expect(() =>
resolveGeneratedCommitIdentity({
authorOverride: "not an identity",
templateAuthor,
}),
).toThrow("author input must use the 'Name <email>' format");
});

it("writes the resolved author and committer into a disposable Git commit", () => {
const directory = mkdtempSync(join(tmpdir(), "patchworks-identity-fixture-"));
try {
const identity = resolveGeneratedCommitIdentity({
customToken: false,
serverUrl: "https://github.com/",
templateAuthor,
});
const author = /^([^<]+) <([^>]+)>$/.exec(identity.author);
const committer = /^([^<]+) <([^>]+)>$/.exec(identity.committer);
expect(author).not.toBeNull();
expect(committer).not.toBeNull();
if (!author || !committer) throw new Error("Resolved identity did not parse");

fixtureGit(directory, "init", "--initial-branch=main");
fixtureGit(
directory,
"-c",
`author.name=${author[1]}`,
"-c",
`author.email=${author[2]}`,
"-c",
`committer.name=${committer[1]}`,
"-c",
`committer.email=${committer[2]}`,
"-c",
"commit.gpgsign=false",
"-c",
"core.hooksPath=/dev/null",
"commit",
"--allow-empty",
"-m",
"Patchworks generated update",
);

expect(
fixtureGit(directory, "show", "-s", "--format=%an <%ae> / %cn <%ce>"),
).toBe(`${templateAuthor} / ${actionsBot}`);
} finally {
rmSync(directory, { force: true, recursive: true });
}
});
});

describe("git status parsing", () => {
it("parses rename and copy records, de-duplicates paths, and sorts output", () => {
const spawn = vi.fn(() => ({
Expand Down Expand Up @@ -867,6 +1001,10 @@ describe("report collection", () => {
prTitle: "Update template",
rejectFiles: [".patchworks-rejects/bbbbbbb.patch"],
status: "conflicts",
templateAuthor: {
email: "author@example.com",
name: "Template Author",
},
}),
);
const spawn = vi.fn(() => ({
Expand All @@ -883,6 +1021,7 @@ describe("report collection", () => {
reject_files_json:
'[".patchworks-rejects/bbbbbbb.patch","conflict.txt.rej"]',
status: "conflicts",
template_author: "Template Author <author@example.com>",
workflow_changes: "true",
workflow_files_json: '[".github/workflows/ci.yml"]',
});
Expand Down
Loading