Bellal Mohamed 360be9c8fc
fix: allow @ in branch names (valid per git-check-ref-format) (#1411)
`validateBranchName` rejects branch names containing `@`, even though
`git check-ref-format` permits `@` and GitHub itself accepts such
branches. PRs whose head or base branch contains an `@` fail validation
in-process before any git operation, so the action errors out
immediately.

Branch names with `@` show up in real workflows: ticket conventions
like "TICKET-123@add-feature" (#998), leading-prefix conventions like
"@hotfix/...", and agent tooling that appends "@<sessionid>" (#1305).
There is no workaround other than renaming the branch, which is often
not under the user's control.

Branch names are never passed through a shell (git calls use
execFileSync argv arrays), so `@` carries no injection risk. This is
the same reasoning used to add `#` in #1167, `+` in #1248, and `,` in
#1310. The bare name "@" (HEAD shorthand in git revision syntax) and
the "@{" reflog sequence are still rejected.

- Add `@` to the validateBranchName whitelist regex, including the
  leading position (the leading-character rule blocks option injection
  via `-`, which `@` cannot cause)
- Reject the bare name "@" with a dedicated check
- Update the surrounding comment, JSDoc, and error message to match
- Add test cases for @-containing names and bare "@"

Fixes #998

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-06-22 14:42:36 -07:00

336 lines
12 KiB
TypeScript

#!/usr/bin/env bun
/**
* Setup the appropriate branch based on the event type:
* - For PRs: Checkout the PR branch
* - For Issues: Create a new branch
*/
import { $ } from "bun";
import { execFileSync } from "child_process";
import type { ParsedGitHubContext } from "../context";
import type { GitHubPullRequest } from "../types";
import type { Octokits } from "../api/client";
import type { FetchDataResult } from "../data/fetcher";
import { generateBranchName } from "../../utils/branch-template";
/**
* Extracts the first label from GitHub data, or returns undefined if no labels exist
*/
function extractFirstLabel(githubData: FetchDataResult): string | undefined {
const labels = githubData.contextData.labels?.nodes;
return labels && labels.length > 0 ? labels[0]?.name : undefined;
}
/**
* Validates a git branch name against a strict whitelist pattern.
* This prevents command injection by ensuring only safe characters are used.
*
* Valid branch names:
* - 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 {
// Check for empty or whitespace-only names
if (!branchName || branchName.trim().length === 0) {
throw new Error("Branch name cannot be empty");
}
// Check for leading dash (prevents option injection like --help, -x)
if (branchName.startsWith("-")) {
throw new Error(
`Invalid branch name: "${branchName}". Branch names cannot start with a dash.`,
);
}
// Check for control characters and special git characters (~^:?*[\])
// eslint-disable-next-line no-control-regex
if (/[\x00-\x1F\x7F ~^:?*[\]\\]/.test(branchName)) {
throw new Error(
`Invalid branch name: "${branchName}". Branch names cannot contain control characters, spaces, or special git characters (~^:?*[\\]).`,
);
}
// 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/_.#+,@-]*$/;
if (!validPattern.test(branchName)) {
throw new Error(
`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 (@).`,
);
}
// Check for leading/trailing periods
if (branchName.startsWith(".") || branchName.endsWith(".")) {
throw new Error(
`Invalid branch name: "${branchName}". Branch names cannot start or end with a period.`,
);
}
// Check for trailing slash
if (branchName.endsWith("/")) {
throw new Error(
`Invalid branch name: "${branchName}". Branch names cannot end with a slash.`,
);
}
// Check for consecutive slashes
if (branchName.includes("//")) {
throw new Error(
`Invalid branch name: "${branchName}". Branch names cannot contain consecutive slashes.`,
);
}
// Additional git-specific validations
if (branchName.includes("..")) {
throw new Error(
`Invalid branch name: "${branchName}". Branch names cannot contain '..'`,
);
}
if (branchName.endsWith(".lock")) {
throw new Error(
`Invalid branch name: "${branchName}". Branch names cannot end with '.lock'`,
);
}
if (branchName.includes("@{")) {
throw new Error(
`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 '@'.`,
);
}
}
/**
* Executes a git command safely using execFileSync to avoid shell interpolation.
*
* Security: execFileSync passes arguments directly to the git binary without
* invoking a shell, preventing command injection attacks where malicious input
* could be interpreted as shell commands (e.g., branch names containing `;`, `|`, `&&`).
*
* @param args - Git command arguments (e.g., ["checkout", "branch-name"])
*/
function execGit(args: string[]): void {
execFileSync("git", args, { stdio: "inherit", env: process.env });
}
export type BranchInfo = {
baseBranch: string;
claudeBranch?: string;
currentBranch: string;
};
export async function setupBranch(
octokits: Octokits,
githubData: FetchDataResult,
context: ParsedGitHubContext,
): Promise<BranchInfo> {
const { owner, repo } = context.repository;
const entityNumber = context.entityNumber;
const { baseBranch, branchPrefix, branchNameTemplate } = context.inputs;
const isPR = context.isPR;
if (isPR) {
const prData = githubData.contextData as GitHubPullRequest;
const prState = prData.state;
// Check if PR is closed or merged
if (prState === "CLOSED" || prState === "MERGED") {
console.log(
`PR #${entityNumber} is ${prState}, creating new branch from source...`,
);
// Fall through to create a new branch like we do for issues
} else {
// Handle open PR: Checkout the PR branch
console.log("This is an open PR, checking out PR branch...");
const branchName = prData.headRefName;
// Determine optimal fetch depth based on PR commit count, with a minimum of 20
const commitCount = prData.commits.totalCount;
const fetchDepth = Math.max(commitCount, 20);
console.log(
`PR #${entityNumber}: ${commitCount} commits, using fetch depth ${fetchDepth}`,
);
// Validate branch names before use to prevent command injection
validateBranchName(branchName);
// For cross-repository (fork) PRs, fetch via the pull ref since the
// branch only exists on the fork's remote, not on origin.
if (prData.isCrossRepository) {
console.log(
`PR #${entityNumber} is from a fork, fetching via refs/pull/${entityNumber}/head...`,
);
execGit([
"fetch",
"origin",
`--depth=${fetchDepth}`,
`pull/${entityNumber}/head:${branchName}`,
]);
} else {
// Execute git commands to checkout PR branch (dynamic depth based on PR size)
// Using execFileSync instead of shell template literals for security
execGit(["fetch", "origin", `--depth=${fetchDepth}`, branchName]);
}
execGit(["checkout", branchName, "--"]);
console.log(`Successfully checked out PR branch for PR #${entityNumber}`);
// For open PRs, we need to get the base branch of the PR
const baseBranch = prData.baseRefName;
validateBranchName(baseBranch);
return {
baseBranch,
currentBranch: branchName,
};
}
}
// Determine source branch - use baseBranch if provided, otherwise fetch default
let sourceBranch: string;
if (baseBranch) {
// Use provided base branch for source
sourceBranch = baseBranch;
} else {
// No base branch provided, fetch the default branch to use as source
const repoResponse = await octokits.rest.repos.get({
owner,
repo,
});
sourceBranch = repoResponse.data.default_branch;
}
// Generate branch name for either an issue or closed/merged PR
const entityType = isPR ? "pr" : "issue";
// Get the SHA of the source branch to use in template
let sourceSHA: string | undefined;
try {
// Get the SHA of the source branch to verify it exists
const sourceBranchRef = await octokits.rest.git.getRef({
owner,
repo,
ref: `heads/${sourceBranch}`,
});
sourceSHA = sourceBranchRef.data.object.sha;
console.log(`Source branch SHA: ${sourceSHA}`);
// Extract first label from GitHub data
const firstLabel = extractFirstLabel(githubData);
// Extract title from GitHub data
const title = githubData.contextData.title;
// Generate branch name using template or default format
let newBranch = generateBranchName(
branchNameTemplate,
branchPrefix,
entityType,
entityNumber,
sourceSHA,
firstLabel,
title,
);
// Check if generated branch already exists on remote
try {
await $`git ls-remote --exit-code origin refs/heads/${newBranch}`.quiet();
// If we get here, branch exists (exit code 0)
console.log(
`Branch '${newBranch}' already exists, falling back to default format`,
);
newBranch = generateBranchName(
undefined, // Force default template
branchPrefix,
entityType,
entityNumber,
sourceSHA,
firstLabel,
title,
);
} catch {
// Branch doesn't exist (non-zero exit code), continue with generated name
}
// For commit signing, defer branch creation to the file ops server
if (context.inputs.useCommitSigning) {
console.log(
`Branch name generated: ${newBranch} (will be created by file ops server on first commit)`,
);
// Ensure we're on the source branch
console.log(`Fetching and checking out source branch: ${sourceBranch}`);
validateBranchName(sourceBranch);
execGit(["fetch", "origin", sourceBranch, "--depth=1"]);
execGit(["checkout", sourceBranch, "--"]);
return {
baseBranch: sourceBranch,
claudeBranch: newBranch,
currentBranch: sourceBranch, // Stay on source branch for now
};
}
// For non-signing case, create and checkout the branch locally only
console.log(
`Creating local branch ${newBranch} for ${entityType} #${entityNumber} from source branch: ${sourceBranch}...`,
);
// 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, "--"]);
// Create and checkout the new branch from the source branch
execGit(["checkout", "-b", newBranch]);
console.log(
`Successfully created and checked out local branch: ${newBranch}`,
);
return {
baseBranch: sourceBranch,
claudeBranch: newBranch,
currentBranch: newBranch,
};
} catch (error) {
console.error("Error in branch setup:", error);
process.exit(1);
}
}