BUG-HUNT: [correctness] _sanitise_branch_name() allows git-invalid patterns (leading dot, trailing .lock, double-dot) — certain plan IDs produce branch names git refuses, causing SandboxCreationError #6640

New issue

Open

opened 2026-04-09 22:39:06 +00:00 by HAL9000 · 0 comments

HAL9000 commented

2026-04-09 22:39:06 +00:00

Owner

Bug Report: Correctness — `_sanitise_branch_name()` produces git-invalid branch names for certain inputs

Severity Assessment

Impact: Sandbox creation fails with a cryptic SandboxCreationError for plan IDs that produce git-invalid branch name patterns. The error message does not identify the root cause (invalid branch name), making debugging difficult. Depending on the source of plan IDs, this can be a reliable DoS.
Likelihood: Low-Medium — ULID plan IDs are safe ([0-9A-Z] only), but human-readable plan IDs, test plan IDs, or plan IDs derived from user input can trigger this
Priority: Medium

Location

File: src/cleveragents/infrastructure/sandbox/git_worktree.py
Function: _sanitise_branch_name
Lines: ~40–57

Description

_sanitise_branch_name() strips disallowed characters but does not enforce several git refname rules that apply after character stripping. Git rejects branch names that violate structural rules even if all characters are individually valid.

The sanitiser:

# git_worktree.py  ~L37
_BRANCH_SANITISE_RE: re.Pattern[str] = re.compile(r"[^a-zA-Z0-9/_.\-]+")

def _sanitise_branch_name(raw: str) -> str:
    sanitised = _BRANCH_SANITISE_RE.sub("-", raw).strip("-")
    if not sanitised:
        sanitised = "sandbox"
    return sanitised

Git refname rules violated by the sanitiser's output:

Rule	Trigger input	Resulting branch	Git error
Component must not start with `.`	`.hidden`	`cleveragents/plan-.hidden`	`fatal: '.' is not a valid character in a ref name` (after `/`)
Name must not contain `..`	`foo..bar`	`cleveragents/plan-foo..bar`	`fatal: '..' is not allowed in a ref name`
Name must not end with `.lock`	`end.lock`	`cleveragents/plan-end.lock`	`fatal: refs ending with '.lock' are not allowed`
Name must not end with `.`	`trailing.`	`cleveragents/plan-trailing.`	`fatal: ref ends with '.'`
Name must not contain `@{`	`ref@{0}`	`cleveragents/plan-ref@{0}` — but `@{` passes the sanitiser since `@` is not in the allowed set... wait, `@` IS in `[^a-zA-Z0-9/_.\-]+` so it gets replaced with `-`. Actually `{` also gets replaced. So `@{` is safe.

The most realistic failing cases:

plan_id = ".hidden-plan"  →  branch = "cleveragents/plan-.hidden-plan"
  git error: invalid ref name (component after / starts with .)

plan_id = "v1..0-alpha"   →  branch = "cleveragents/plan-v1..0-alpha"
  git error: '..' not allowed in ref name

plan_id = "my-plan.lock"  →  branch = "cleveragents/plan-my-plan.lock"
  git error: ref name ends with '.lock'

All of these cause git worktree add -b <branch> to fail with a non-zero exit code, which is caught as subprocess.CalledProcessError and re-raised as SandboxCreationError with a message like "Failed to create git worktree for resource ... : fatal: 'cleveragents/plan-.hidden-plan' is not a valid branch name".

Evidence

# Demonstrating the gap:
>>> _sanitise_branch_name(".hidden")
'.hidden'          # leading dot preserved — git will reject this

>>> _sanitise_branch_name("foo..bar")
'foo..bar'         # double dot preserved — git will reject this

>>> _sanitise_branch_name("end.lock")
'end.lock'         # .lock suffix preserved — git will reject this

_BRANCH_SANITISE_RE only strips characters NOT in [a-zA-Z0-9/_.\-]. The dot (.) is explicitly in the allowed set, so double-dots, leading dots, and .lock suffixes all pass through unchanged.

Expected Behavior

_sanitise_branch_name() should produce a branch name that git will always accept, regardless of the input string. All git refname structural rules should be enforced.

Actual Behavior

Character-level sanitisation passes, but structural git refname rules are not checked. Invalid branch names silently propagate to git worktree add which then fails with a cryptic error.

Suggested Fix

Add post-sanitisation structural fixes:

def _sanitise_branch_name(raw: str) -> str:
    # Replace disallowed chars
    sanitised = _BRANCH_SANITISE_RE.sub("-", raw).strip("-")
    if not sanitised:
        sanitised = "sandbox"

    # Enforce git structural rules:
    # 1. Replace ".." sequences
    sanitised = re.sub(r"\.\.+", "-", sanitised)
    # 2. Remove leading dots from each path component
    sanitised = "/".join(
        part.lstrip(".") or "x" for part in sanitised.split("/")
    )
    # 3. Remove trailing dot
    sanitised = sanitised.rstrip(".")
    # 4. Remove .lock suffix
    if sanitised.endswith(".lock"):
        sanitised = sanitised[:-5] or "sandbox"
    # 5. Final fallback
    return sanitised or "sandbox"

TDD Note

After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_<this-issue-number>, and @tdd_expected_fail to prove the bug exists before fixing it.

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: bug-hunter

## Bug Report: Correctness — `_sanitise_branch_name()` produces git-invalid branch names for certain inputs ### Severity Assessment - **Impact**: Sandbox creation fails with a cryptic `SandboxCreationError` for plan IDs that produce git-invalid branch name patterns. The error message does not identify the root cause (invalid branch name), making debugging difficult. Depending on the source of plan IDs, this can be a reliable DoS. - **Likelihood**: Low-Medium — ULID plan IDs are safe (`[0-9A-Z]` only), but human-readable plan IDs, test plan IDs, or plan IDs derived from user input can trigger this - **Priority**: Medium ### Location - **File**: `src/cleveragents/infrastructure/sandbox/git_worktree.py` - **Function**: `_sanitise_branch_name` - **Lines**: ~40–57 ### Description `_sanitise_branch_name()` strips disallowed characters but does **not** enforce several git refname rules that apply after character stripping. Git rejects branch names that violate structural rules even if all characters are individually valid. The sanitiser: ```python # git_worktree.py ~L37 _BRANCH_SANITISE_RE: re.Pattern[str] = re.compile(r"[^a-zA-Z0-9/_.\-]+") def _sanitise_branch_name(raw: str) -> str: sanitised = _BRANCH_SANITISE_RE.sub("-", raw).strip("-") if not sanitised: sanitised = "sandbox" return sanitised ``` **Git refname rules violated by the sanitiser's output:** | Rule | Trigger input | Resulting branch | Git error | |------|--------------|-----------------|-----------| | Component must not start with `.` | `.hidden` | `cleveragents/plan-.hidden` | `fatal: '.' is not a valid character in a ref name` (after `/`) | | Name must not contain `..` | `foo..bar` | `cleveragents/plan-foo..bar` | `fatal: '..' is not allowed in a ref name` | | Name must not end with `.lock` | `end.lock` | `cleveragents/plan-end.lock` | `fatal: refs ending with '.lock' are not allowed` | | Name must not end with `.` | `trailing.` | `cleveragents/plan-trailing.` | `fatal: ref ends with '.'` | | Name must not contain `@{` | `ref@{0}` | `cleveragents/plan-ref@{0}` — but `@{` passes the sanitiser since `@` is not in the allowed set... wait, `@` IS in `[^a-zA-Z0-9/_.\-]+` so it gets replaced with `-`. Actually `{` also gets replaced. So `@{` is safe. | The most realistic failing cases: ``` plan_id = ".hidden-plan" → branch = "cleveragents/plan-.hidden-plan" git error: invalid ref name (component after / starts with .) plan_id = "v1..0-alpha" → branch = "cleveragents/plan-v1..0-alpha" git error: '..' not allowed in ref name plan_id = "my-plan.lock" → branch = "cleveragents/plan-my-plan.lock" git error: ref name ends with '.lock' ``` All of these cause `git worktree add -b <branch>` to fail with a non-zero exit code, which is caught as `subprocess.CalledProcessError` and re-raised as `SandboxCreationError` with a message like `"Failed to create git worktree for resource ... : fatal: 'cleveragents/plan-.hidden-plan' is not a valid branch name"`. ### Evidence ```python # Demonstrating the gap: >>> _sanitise_branch_name(".hidden") '.hidden' # leading dot preserved — git will reject this >>> _sanitise_branch_name("foo..bar") 'foo..bar' # double dot preserved — git will reject this >>> _sanitise_branch_name("end.lock") 'end.lock' # .lock suffix preserved — git will reject this ``` `_BRANCH_SANITISE_RE` only strips characters NOT in `[a-zA-Z0-9/_.\-]`. The dot (`.`) is explicitly in the allowed set, so double-dots, leading dots, and `.lock` suffixes all pass through unchanged. ### Expected Behavior `_sanitise_branch_name()` should produce a branch name that git will always accept, regardless of the input string. All git refname structural rules should be enforced. ### Actual Behavior Character-level sanitisation passes, but structural git refname rules are not checked. Invalid branch names silently propagate to `git worktree add` which then fails with a cryptic error. ### Suggested Fix Add post-sanitisation structural fixes: ```python def _sanitise_branch_name(raw: str) -> str: # Replace disallowed chars sanitised = _BRANCH_SANITISE_RE.sub("-", raw).strip("-") if not sanitised: sanitised = "sandbox" # Enforce git structural rules: # 1. Replace ".." sequences sanitised = re.sub(r"\.\.+", "-", sanitised) # 2. Remove leading dots from each path component sanitised = "/".join( part.lstrip(".") or "x" for part in sanitised.split("/") ) # 3. Remove trailing dot sanitised = sanitised.rstrip(".") # 4. Remove .lock suffix if sanitised.endswith(".lock"): sanitised = sanitised[:-5] or "sandbox" # 5. Final fallback return sanitised or "sandbox" ``` ### Category correctness / boundary ### TDD Note After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: `@tdd_issue`, `@tdd_issue_<this-issue-number>`, and `@tdd_expected_fail` to prove the bug exists before fixing it. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: bug-hunter

HAL9000 added the

labels

2026-04-09 22:45:20 +00:00

HAL9000 added this to the v3.2.0 milestone

2026-04-09 22:47:14 +00:00

HAL9000 added