fix(branch): validate generated branch name under commit signing (#1582)

The non-signing path validated newBranch before checkout, but the
use_commit_signing path passed it straight to the file ops server, so an
invalid branch_name_template surfaced only as a 422 "Reference name is
not valid" on the first commit.

Validate once after the name is resolved so both paths fail early with
the same message.

Fixes #1573
This commit is contained in:
Rishav Naskar
2026-08-07 07:59:43 -07:00
committed by GitHub
parent ecf573bd65
commit b704dd3960
2 changed files with 85 additions and 1 deletions
+5 -1
View File
@@ -288,6 +288,11 @@ export async function setupBranch(
// Branch doesn't exist (non-zero exit code), continue with generated name // Branch doesn't exist (non-zero exit code), continue with generated name
} }
// Validate before either path uses the name. The signing path hands it to
// the file ops server rather than to git, so without this an invalid
// template only surfaces as a 422 on the first commit.
validateBranchName(newBranch);
// For commit signing, defer branch creation to the file ops server // For commit signing, defer branch creation to the file ops server
if (context.inputs.useCommitSigning) { if (context.inputs.useCommitSigning) {
console.log( console.log(
@@ -315,7 +320,6 @@ export async function setupBranch(
// Fetch and checkout the source branch first to ensure we branch from the correct base // Fetch and checkout the source branch first to ensure we branch from the correct base
console.log(`Fetching and checking out source branch: ${sourceBranch}`); console.log(`Fetching and checking out source branch: ${sourceBranch}`);
validateBranchName(sourceBranch); validateBranchName(sourceBranch);
validateBranchName(newBranch);
execGit(["fetch", "origin", sourceBranch, "--depth=1"]); execGit(["fetch", "origin", sourceBranch, "--depth=1"]);
execGit(["checkout", sourceBranch, "--"]); execGit(["checkout", sourceBranch, "--"]);
+80
View File
@@ -0,0 +1,80 @@
import { describe, expect, test, beforeEach, afterEach } from "bun:test";
import { mkdtempSync, rmSync } from "fs";
import { join } from "path";
import { setupBranch } from "../src/github/operations/branch";
import { createMockContext } from "./mockContext";
const octokits = {
rest: {
repos: { get: async () => ({ data: { default_branch: "main" } }) },
git: { getRef: async () => ({ data: { object: { sha: "abc1234" } } }) },
},
} as any;
const githubData = {
contextData: { title: "Add feature", labels: { nodes: [] } },
} as any;
// ':' is rejected by validateBranchName. The signing path used to skip that
// check and only fail on the file ops server's first commit (a 422).
const INVALID_TEMPLATE = "{{prefix}}release:{{entityNumber}}";
const loggedErrors: string[] = [];
describe("setupBranch generated branch name validation", () => {
let originalCwd: string;
let tempDir: string;
let exitCode: number | undefined;
let originalExit: typeof process.exit;
let originalError: typeof console.error;
beforeEach(() => {
originalCwd = process.cwd();
// Not a git repo, so the remote existence probe fails and setupBranch
// continues with the generated name.
tempDir = mkdtempSync(join("/tmp", "setup-branch-"));
process.chdir(tempDir);
exitCode = undefined;
loggedErrors.length = 0;
originalExit = process.exit;
originalError = console.error;
console.error = (...args: unknown[]) => {
loggedErrors.push(args.map(String).join(" "));
};
process.exit = ((code?: number) => {
exitCode = code;
throw new Error("process.exit called");
}) as typeof process.exit;
});
afterEach(() => {
process.exit = originalExit;
console.error = originalError;
process.chdir(originalCwd);
rmSync(tempDir, { recursive: true, force: true });
});
for (const useCommitSigning of [true, false]) {
test(`rejects an invalid generated branch name with use_commit_signing: ${useCommitSigning}`, async () => {
const context = createMockContext({
isPR: false,
entityNumber: 42,
inputs: {
useCommitSigning,
branchPrefix: "claude/",
branchNameTemplate: INVALID_TEMPLATE,
},
});
await expect(setupBranch(octokits, githubData, context)).rejects.toThrow(
"process.exit called",
);
expect(exitCode).toBe(1);
// Must fail on the name itself, not on a later git or API call.
expect(loggedErrors.join("\n")).toContain(
'Invalid branch name: "claude/release:42"',
);
});
}
});