From b704dd3960e1416ea3185449d626a060e045e705 Mon Sep 17 00:00:00 2001 From: Rishav Naskar <59786899+rishavnaskar@users.noreply.github.com> Date: Fri, 7 Aug 2026 20:29:43 +0530 Subject: [PATCH] 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 --- src/github/operations/branch.ts | 6 ++- test/setup-branch-validation.test.ts | 80 ++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 test/setup-branch-validation.test.ts diff --git a/src/github/operations/branch.ts b/src/github/operations/branch.ts index e095280b..dccde680 100644 --- a/src/github/operations/branch.ts +++ b/src/github/operations/branch.ts @@ -288,6 +288,11 @@ export async function setupBranch( // 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 if (context.inputs.useCommitSigning) { 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 console.log(`Fetching and checking out source branch: ${sourceBranch}`); validateBranchName(sourceBranch); - validateBranchName(newBranch); execGit(["fetch", "origin", sourceBranch, "--depth=1"]); execGit(["checkout", sourceBranch, "--"]); diff --git a/test/setup-branch-validation.test.ts b/test/setup-branch-validation.test.ts new file mode 100644 index 00000000..e3d82480 --- /dev/null +++ b/test/setup-branch-validation.test.ts @@ -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"', + ); + }); + } +});