Skip to content

fix: allow , in branch names#1310

Open
bugbubug wants to merge 1 commit into
anthropics:mainfrom
bugbubug:fix/allow-comma-in-branch-names
Open

fix: allow , in branch names#1310
bugbubug wants to merge 1 commit into
anthropics:mainfrom
bugbubug:fix/allow-comma-in-branch-names

Conversation

@bugbubug
Copy link
Copy Markdown

Problem

validateBranchName in src/github/operations/branch.ts rejects branch names containing a comma (,), even though git check-ref-format permits commas and GitHub itself accepts them when creating the branch. When the action runs on a PR whose head branch contains a ,, validation throws before any git operation and the action fails immediately:

Error: Action failed with error: Invalid branch name: "<branch-containing-comma>".
Branch names must start with an alphanumeric character and contain only alphanumeric
characters, forward slashes, hyphens, underscores, periods, hashes (#), or plus signs (+).

This shows up in real workflows when branch 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.

Fixes #1300.

Root cause

The whitelist regex at src/github/operations/branch.ts:66 is:

const validPattern = /^[a-zA-Z0-9][a-zA-Z0-9/_.#+-]*$/;

, is not in the character class, so any ref containing one is rejected regardless of position.

The whitelist exists for injection safety, but every git call in this file uses execFileSync with an argv array (no shell, no interpolation), so , carries no injection risk under that execution model. This is the same reasoning used to add # in #1167 and + in #1248.

Fix

Add , to the whitelist: /^[a-zA-Z0-9][a-zA-Z0-9/_.#+,-]*$/. Update the surrounding comment and the user-facing error message to match.

Testing

  • 1 new test case covering commas in title-derived branch names (feature/a,b, feature/paris,france, fix/issue-1,2,3)
  • All existing rejection cases still pass (commas embedded in shell-injection payloads, control characters, etc. continue to be rejected by the other rules)
  • bun test: 666 pass, 0 fail
  • bun run typecheck: clean
  • bun run format:check: clean

`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 anthropics#1167 and `+` in anthropics#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 anthropics#1300
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

validateBranchName() in src/github/operations/branch.ts rejects branch names containing a comma (,)

1 participant