From bf6d40e0688f27760ff7564735aaf60c260396a0 Mon Sep 17 00:00:00 2001 From: rico <43229430+bugbubug@users.noreply.github.com> Date: Fri, 15 May 2026 06:34:32 +0800 Subject: [PATCH] fix: allow , in branch names (#1310) `validateBranchName` rejects branch names containing a comma, even though `git check-ref-format` permits commas and GitHub itself accepts them. PRs whose head branch contains a `,` fail validation in-process before any git operation, so the action errors out immediately. Branch names with commas show up in real workflows when names are derived from titles, place names, or external identifiers (e.g. "feature/paris,france"). There is no workaround other than renaming the branch, which is often not under the user's control. All git calls in this file use execFileSync with an argv array, so no shell interpretation occurs and `,` carries no injection risk. This is the same reasoning used to add `#` in #1167 and `+` in #1248. - Add `,` to the validateBranchName whitelist regex - Update the surrounding comment and error message to match - Add a test case covering commas in title-derived branch names Fixes #1300 --- src/github/operations/branch.ts | 10 ++++++---- test/validate-branch-name.test.ts | 9 +++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/github/operations/branch.ts b/src/github/operations/branch.ts index 870cd649..920eec56 100644 --- a/src/github/operations/branch.ts +++ b/src/github/operations/branch.ts @@ -58,16 +58,18 @@ export function validateBranchName(branchName: string): void { ); } - // Strict whitelist pattern: alphanumeric start, then alphanumeric/slash/hyphen/underscore/period/hash/plus. + // Strict whitelist pattern: alphanumeric start, then alphanumeric/slash/hyphen/underscore/period/hash/plus/comma. // # is valid per git-check-ref-format and commonly used in branch names like "fix/#123-description". // + is valid per git-check-ref-format and generated by Claude Code's EnterWorktree tool when // converting worktree names containing "/" (e.g. "feat/foo" becomes "worktree-feat+foo"). - // All git calls use execFileSync (not shell interpolation), so neither # nor + carries injection risk. - const validPattern = /^[a-zA-Z0-9][a-zA-Z0-9/_.#+-]*$/; + // , is valid per git-check-ref-format and commonly appears in branch names derived from titles + // or external identifiers (e.g. place names like "feature/paris,france"). + // All git calls use execFileSync (not shell interpolation), so none of these characters carry injection risk. + const validPattern = /^[a-zA-Z0-9][a-zA-Z0-9/_.#+,-]*$/; if (!validPattern.test(branchName)) { throw new Error( - `Invalid branch name: "${branchName}". Branch names must start with an alphanumeric character and contain only alphanumeric characters, forward slashes, hyphens, underscores, periods, hashes (#), or plus signs (+).`, + `Invalid branch name: "${branchName}". Branch names must start with an alphanumeric character and contain only alphanumeric characters, forward slashes, hyphens, underscores, periods, hashes (#), plus signs (+), or commas (,).`, ); } diff --git a/test/validate-branch-name.test.ts b/test/validate-branch-name.test.ts index 9594b48d..5a9bf680 100644 --- a/test/validate-branch-name.test.ts +++ b/test/validate-branch-name.test.ts @@ -55,6 +55,15 @@ describe("validateBranchName", () => { expect(() => validateBranchName("fix+issue-123")).not.toThrow(); expect(() => validateBranchName("feature+new-thing")).not.toThrow(); }); + + it("should accept branch names containing , (git-valid, common in title-derived branches)", () => { + // Reported in #1300: branches like "feature/a,b" were rejected, even though + // git check-ref-format and GitHub both accept commas. Common when branch names + // are derived from titles, place names, or external identifiers. + expect(() => validateBranchName("feature/a,b")).not.toThrow(); + expect(() => validateBranchName("feature/paris,france")).not.toThrow(); + expect(() => validateBranchName("fix/issue-1,2,3")).not.toThrow(); + }); }); describe("command injection attempts", () => {