diff --git a/src/github/operations/branch.ts b/src/github/operations/branch.ts index 920eec56..253f7630 100644 --- a/src/github/operations/branch.ts +++ b/src/github/operations/branch.ts @@ -27,14 +27,15 @@ function extractFirstLabel(githubData: FetchDataResult): string | undefined { * This prevents command injection by ensuring only safe characters are used. * * Valid branch names: - * - Start with alphanumeric character (not dash, to prevent option injection) - * - Contain only alphanumeric, forward slash, hyphen, underscore, period, or hash (#) + * - Start with alphanumeric character or @ (not dash, to prevent option injection) + * - Contain only alphanumeric, forward slash, hyphen, underscore, period, hash (#), plus (+), comma (,), or at sign (@) * - Do not start or end with a period * - Do not end with a slash * - Do not contain '..' (path traversal) * - Do not contain '//' (consecutive slashes) * - Do not end with '.lock' * - Do not contain '@{' + * - Are not the single character '@' (HEAD shorthand in git revision syntax) * - Do not contain control characters or special git characters (~^:?*[\]) */ export function validateBranchName(branchName: string): void { @@ -58,18 +59,21 @@ export function validateBranchName(branchName: string): void { ); } - // Strict whitelist pattern: alphanumeric start, then alphanumeric/slash/hyphen/underscore/period/hash/plus/comma. + // Strict whitelist pattern: alphanumeric or @ start, then alphanumeric/slash/hyphen/underscore/period/hash/plus/comma/at-sign. // # 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"). // , 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"). + // @ is valid per git-check-ref-format anywhere in a ref name, including the first character + // (e.g. ticket conventions like "TICKET-123@add-feature" or prefixes like "@hotfix/..."); + // the bare name "@" (HEAD shorthand) and the "@{" sequence (reflog syntax) are rejected below. // 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/_.#+,-]*$/; + 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 (#), plus signs (+), or commas (,).`, + `Invalid branch name: "${branchName}". Branch names must start with an alphanumeric character or '@' and contain only alphanumeric characters, forward slashes, hyphens, underscores, periods, hashes (#), plus signs (+), commas (,), or at signs (@).`, ); } @@ -112,6 +116,15 @@ export function validateBranchName(branchName: string): void { `Invalid branch name: "${branchName}". Branch names cannot contain '@{'`, ); } + + // Per git-check-ref-format, a refname cannot be the single character "@"; "@" also + // resolves to HEAD in git revision syntax, so a bare "@" must never reach git as a + // branch argument where it could be interpreted as a revision instead. + if (branchName === "@") { + throw new Error( + `Invalid branch name: "@". Branch names cannot be the single character '@'.`, + ); + } } /** diff --git a/test/validate-branch-name.test.ts b/test/validate-branch-name.test.ts index 5a9bf680..fe03ce4e 100644 --- a/test/validate-branch-name.test.ts +++ b/test/validate-branch-name.test.ts @@ -64,6 +64,16 @@ describe("validateBranchName", () => { expect(() => validateBranchName("feature/paris,france")).not.toThrow(); expect(() => validateBranchName("fix/issue-1,2,3")).not.toThrow(); }); + + it("should accept branch names containing @ (git-valid, used in team and tooling conventions)", () => { + // Reported in #998: branches like "TICKET-123@add-feature" were rejected, even + // though git check-ref-format and GitHub both accept @ anywhere in a ref name. + // Also common as a leading prefix (e.g. "@hotfix/...") and in agent-generated + // names ("task@sessionid"). Bare "@" and "@{" are still rejected. + expect(() => validateBranchName("TICKET-123@add-feature")).not.toThrow(); + expect(() => validateBranchName("@hotfix/login-timeout")).not.toThrow(); + expect(() => validateBranchName("agent/task@abc123")).not.toThrow(); + }); }); describe("command injection attempts", () => { @@ -137,6 +147,12 @@ describe("validateBranchName", () => { expect(() => validateBranchName("HEAD@{yesterday}")).toThrow(/@{/); }); + it("should reject the single character @", () => { + // Per git-check-ref-format, a refname cannot be the single character "@"; + // "@" also resolves to HEAD in git revision syntax. + expect(() => validateBranchName("@")).toThrow(/single character '@'/); + }); + it("should reject .lock suffix", () => { expect(() => validateBranchName("branch.lock")).toThrow(/\.lock/); expect(() => validateBranchName("feature.lock")).toThrow(/\.lock/);