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
This commit is contained in:
rico 2026-05-15 06:34:32 +08:00 committed by GitHub
parent 86eb26bf01
commit bf6d40e068
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 15 additions and 4 deletions

View File

@ -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 (,).`,
);
}

View File

@ -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", () => {