fix(source-control): validate git ref syntax in worktree-create.sh

[kyle-sexton]

This was generated by AI during an autonomous work-loop execution session.

Closes #1016

Summary

#1016 batched two P2 follow-ups deferred from PR #898's review. Only one of them is still live — the other landed inside #898 itself, which the reproduction below establishes before any code was changed.

Site 2 — git ref-syntax validation (the actual fix)

worktree-create.sh validates --name against the EnterWorktree schema (a character class of letters, digits, dots, underscores, and dashes per /-separated segment, plus a 64-char cap) and documents that a name failing validation is refused with usage exit 2. A header comment asserted that class was "a strict subset of what git refs allow, so a validated name is always a creatable branch."

That assertion is false. These names satisfy the class yet git rejects them as refs:

Name	Why git rejects it
feat/foo..bar	contains ..
feat/., .foo	component starts/ends with .
foo.	ends with .
foo.lock, feat/x.lock	reserved .lock suffix
HEAD	reserved name
-lead	leading dash

Reproduced against a throwaway fixture repo on the pre-fix helper — every one of them fell through to git worktree add and returned environment exit 4, the code reserved for "not a git repo, or git worktree add failed". Since the caller's correction flow keys on exit 2, an invalid name was indistinguishable from a broken environment.

Fix: follow the schema check with git check-ref-format --branch, per the disposition's own fix direction. Three details are load-bearing:

• Both streams are discarded. On success --branch echoes the name to stdout — leaving it would corrupt the helper's "created worktree path is the SOLE stdout line" output contract, which /worktree create parses to feed EnterWorktree(path:). A regression test asserts stdout is still exactly the path.
• No repository is required. Git documents --branch as repository-scoped because of its @{-n} previous-branch expansion; @, {, and } are already outside the character class, so that path is unreachable and the check is correct run from anywhere. Verified empirically outside any repo.
• Option-shaped names are safe. git check-ref-format --branch --help (and -h, --normalize, --branch) exit 128 with "not a valid branch name" — git parses no further options after --branch, so no name is mistaken for a flag and nothing opens a pager.

No over-rejection. Ran a matrix over 24 names with a fresh fixture repo per name (an earlier shared-repo matrix produced false discrepancies from cross-case contamination): for every name the character class admits, check-ref-format --branch and git worktree add -b agree exactly. feat/-lead, foo-, head, _x, a, and feat/scope.v2_final-1 all still create.

The false "strict subset" comment is corrected, and the exit-code header, --help text, and skills/worktree/context/create.md's name-validation list are brought in line. Those two files are the only tracked surfaces restating the rules — checked, not assumed.

Site 1 — bare-repo root detection (no code change; already fixed)

Per the Bug Investigation Rule this was reproduced first, and it does not reproduce. A root under a bare clone exits 3 today with "worktree target is inside a git directory".

Git history explains why: 81627ac ("reject worktree roots inside a git directory") landed inside PR #898, after the bare-repo thread was dispositioned. It added the --is-inside-git-dir probe, which returns true for both a .git directory and a bare repository — exactly the fix direction #1016 asks for. That commit's own resolution comment says so: "This closes the .git-directory case here and, via the same --is-inside-git-dir mechanism, the sibling bare-clone gap deferred to #657 — #657 can drop the bare-repo item." The #657 → #1016 batch conversion carried the stale line forward anyway.

worktree-create.test.sh already covers it (root inside a bare clone refuses exit 3), so there is no coverage gap to close either. Nothing to do beyond recording the finding.

Note for reviewers

The --name %q sanitizes to an empty slug guard is now unreachable: an empty slug requires a name of only - and /, which must start with / (fails the character class) or - (fails check-ref-format). Left in place deliberately — it is cheap defense-in-depth whose reachability depends on check ordering, and removing it is outside this item's scope.

Test plan

• plugins/source-control/scripts/worktree-create.test.sh — 81 assertions pass, 0 fail (was 60). Because this script is load-bearing for every worktree the work-loop provisions, the whole suite was run, not just the new cases: normal creation, slug/path computation, base-ref fresh/head, .worktreeinclude copying, and all existing containment guards are unchanged and green.
• New cases: 8 git-invalid names each asserted exit 2, not 4, with the message naming the branch grammar; plus a valid-name case asserting creation still succeeds and that stdout remains the sole path line.
• Pre-fix reproduction and post-fix verification run through the same harness against both helper revisions.
• shellcheck clean on both scripts (repo .shellcheckrc).
• scripts/check-changelog-parity.sh --check and --check-bump origin/main pass (version 0.26.3 → 0.26.4 with a matching entry).
• Full suite, shellcheck, both changelog-parity gates, skill-portability, and markdownlint were re-run after merging main in — all green on the merged head (78/78 assertions).
• scripts/check-skill-portability.sh origin/main, scripts/check-silent-skips.sh, and markdownlint-cli2 on both changed markdown files pass.

Review round 1 — CWD discovery regression (fixed in a286444)

Codex caught a regression the ref check itself introduced, and it was the real-world shape, not a corner case. git check-ref-format --branch takes a branchname-shorthand and so performs repository discovery. Run unscoped it inherited the caller's CWD, so from a directory whose .git names a gitdir that no longer exists — a stale checkout, exactly what this plugin's own worktree cleanup handles — git exits 128 and a valid name was rejected with exit 2, creating nothing. The documented invocation in SKILL.md / context/create.md omits --repo-dir, so the CWD is the default.

Reproduced end-to-end before fixing: identical invocation exited 2 from the stale dir and 0 from a neutral one.

Fixed by scoping the check — with a refinement on the suggestion: -C "$repo_dir" alone would still misreport when repo_dir is the broken default, so the grammar check now runs after the exit-4 repository probe and uses -C "$toplevel". A healthy repository is then guaranteed, so a non-zero exit can only mean the name.

Ordering consequence, stated plainly: exits 3 (root unconfigured) and 4 (not a repository) can now precede an invalid-name exit 2. This matches the file's existing shape rather than introducing a new one — the pre-existing --base-ref and empty-slug exit-2 checks already sat after both. The character-class and length checks still run first, before any git call. --help, the exit-code header, and create.md say so.

Two regression tests added: a valid name from a stale-.git CWD exits 0 and prints the path; an invalid name from that same CWD still exits 2. Suite is now 81/81.

Conflict resolution (2026-07-25)

main bumped source-control to 0.26.3 while this PR was open. Both sides landed the same version literal, so git auto-merged plugin.json to a single 0.26.3 with no conflict reported — a silent collision that would have shipped two different releases under one version. Caught by re-reading the merged file; this branch is retargeted to 0.26.4, with main's 0.26.3 CHANGELOG section preserved verbatim below this PR's entry.

Integrated as a merge commit rather than a rebase. The rebase was done and verified first, but publishing it needed a force-push that this environment declines, so the content-equivalent merge was used instead — the merged tree is byte-identical to the verified rebased tree. Squash merge collapses the merge commit at land time.

No semantic overlap with main's incoming worktree changes: those add external-root convention prose and touch neither name validation nor the exit-code contract.

Related

• Follow-up batch from PR feat(source-control): route /worktree create through a shared worktree-creation helper (#399 Phase A) #898 review threads (codex P2, dispositioned 2026-07-22).
• 81627ac — the commit that already closed Site 1.
• Folded from the lane sweep: triage — week of 7/20 (rolling paper-cut batch) #657 sweep line.

[claude]

Claude finished @kyle-sexton's task in 2m 19s —— View job

---

PR Review — fix(source-control): validate git ref syntax in worktree-create.sh

• Gather context and read PR diff
• Review worktree-create.sh changes
• Review worktree-create.test.sh changes
• Review create.md and documentation changes
• Review plugin.json and CHANGELOG.md
• Post findings

---

Summary

This is a well-executed, focused fix. The core problem — the character-class gate not being a subset of git's ref grammar, so names like feat/foo..bar or HEAD fell through to git worktree add as an opaque environment exit 4 — is correctly identified and correctly resolved. Findings below, ranked by severity.

---

Findings

No blocking issues. The fix is correct, the tests cover the key contract points, and the documentation is accurate. A few observations worth recording:

1. The stdout-purity test is load-bearing and should stay — good that it's here

worktree-create.test.sh line 288–290

out=$(bash "$HELPER" --name "feat/refok" --root "$root" --repo-dir "$repo" 2>/dev/null)
assert_exit "ref-valid name still creates (exit 0)" 0 "$?"
assert_eq "ref check leaves stdout as the sole path line" "$root/acme-widget-feat-refok" "$out"

This is the right test. git check-ref-format --branch echoes the name to stdout on success (exits 0), so a forgotten >/dev/null on the check would silently corrupt the output contract in a way no other assertion in the suite would catch. This test makes the suppression verifiable rather than trusted-on-faith. Worth keeping prominent.

2. The @{-n} argument in the comment is sound — minor suggestion

worktree-create.sh lines 115–127

The argument is correct: @, {, } are outside the character class, so @{-n} expansion cannot reach the --branch path. One thing left implicit: the git check-ref-format man page calls --branch "repository-scoped" specifically because of the reflog shorthand, not because the format rules themselves require a repo. Since the expansion is blocked by the class, the "correct run from anywhere" claim holds. This is fine as-is; the comment is already detailed enough that a future reader can reconstruct the reasoning. Mentioning "reflog shorthand, not format rules" somewhere in the comment could make the distinction slightly crisper, but it is not required.

3. The feat/. case — verify it enters the test loop

worktree-create.test.sh line 279

for badref in 'feat/foo..bar' 'feat/.' 'foo.lock' 'feat/x.lock' '.foo' 'foo.' 'HEAD' '-lead'; do

feat/. passes the character class because the trailing segment is a single ., which matches [A-Za-z0-9._-]+. Confirmed: git check-ref-format --branch 'feat/.' rejects it (ref components may not be . or ..). The case is correctly in the loop. No issue — just confirming the non-obvious ones are actually exercised by the class/check combination as described.

4. Empty-slug guard is now unreachable — acknowledged, appropriate to leave

worktree-create.sh lines 274–277

The PR description notes this explicitly. The guard requires a name of only - and /, but leading - is now caught by check-ref-format and leading / by the character class, so neither can reach the slug stage. Leaving it in as defense-in-depth is a reasonable call — the check is cheap and its reachability depends on the ordering of the upstream guards.

5. create.md line 24 — long single-line bullet

create.md line 24

The added bullet is a single long line. It renders fine in Markdown, but the content is accurate and appropriately placed. Not a blocker.

---

Verdict

LGTM. The fix is correct: git check-ref-format --branch is the right oracle, both streams are suppressed for the right reasons, and the test matrix covers exit-code semantics plus the stdout-purity invariant. The documentation and CHANGELOG entries accurately describe the change, and the version bump (0.26.2 → 0.26.3) is appropriate for a patch fix.

[claude]

Claude finished @kyle-sexton's task in 3m 7s —— View job

---

Security Review — PR #1333

• Load tools
• Fetch PR diff
• Read changed files
• Analyze for security vulnerabilities
• Post findings

---

Review: fix(source-control): validate git ref syntax in worktree-create.sh

Scope: worktree-create.sh (primary), worktree-create.test.sh, plugin.json, create.md, CHANGELOG.md.

---

No CRITICAL or IMPORTANT vulnerabilities found.

The central addition is well-implemented:

if ! git check-ref-format --branch "$name" >/dev/null 2>&1; then

• $name is double-quoted — no word-splitting or glob expansion risk. ✓
• Both stdout and stderr are discarded — stdout suppression is load-bearing (git echoes the name on success, which would corrupt the sole-stdout-line output contract). ✓
• $name has already been validated for length and character class before this call, so the argument set is well-bounded. ✓
• All other uses of $name in the script are similarly double-quoted. ✓

---

SUGGESTION — Confidence: MEDIUM

Reliance on undocumented git option-parsing behavior for leading-hyphen names

The character-class regex at line 110 (^[A-Za-z0-9._-]+…) permits names that begin with hyphens — e.g. -lead, --normalize, --branch. These are then passed directly to:

git check-ref-format --branch "$name"   # line 128

The PR correctly notes that "git parses no further options after --branch" and empirically verified that --help, -h, --normalize, and --branch each exit 128 as invalid branch names rather than being treated as flags. That verification is reassuring.

However, this protection relies on git's implementation-internal behavior — specifically that parse_options stops option processing once --branch (a mode-flag in OPT_CMDMODE style) has been consumed. This is not formally documented in git check-ref-format's manpage. A future git release that reorganizes option parsing could theoretically change this, though the risk is low in practice.

Consequence if the assumption breaks: git check-ref-format --branch "--normalize" exits 0 instead of 128, allowing an option-shaped but git-invalid name through this check. The name would still fail downstream at git worktree add -b "$name" (exit 4), but the caller's correction flow keys on exit 2, so the error becomes less actionable rather than exploitable.

Mitigation (defense-in-depth): The safest fix is to tighten the character-class regex to disallow a leading hyphen, since git branch names starting with - are already unconditionally invalid per check-ref-format's own rules. Changing the first segment anchor from [A-Za-z0-9._-]+ to [A-Za-z0-9._][A-Za-z0-9._-]* eliminates the dependency entirely, keeps the valid-name set identical (no currently-accepted names start with -), and removes the need for the @{-n} carve-out comment. Fix this →

---

Other observations (no security impact)

• git -C "$repo_dir" rev-parse and git -C "$toplevel" worktree add pass all user-derived values double-quoted. No injection surface introduced.
• parse_owner_repo handles remote URLs via pure string operations; the URL never reaches a shell invocation.
• --exclude-from="$include_file" at line 418 uses =-assignment form, so a $include_file value that looks like a flag cannot be misinterpreted.
• printf '%s' / '%q' usage is correct throughout: %q is display-only and nothing it produces is subsequently eval'd.
• The >/dev/null 2>&1 ordering on line 128 is correct (>/dev/null redirects stdout, then 2>&1 duplicates that into stderr; the common mistake of reversing them doesn't apply here since both go to /dev/null).

[claude]

Claude finished @kyle-sexton's task in 3m 18s —— View job

---

PR Review — fix(source-control): validate git ref syntax in worktree-create.sh

• Load tools
• Read PR diff
• Review changed files
• Post findings

---

Summary

This is a well-scoped, correctly implemented fix. The core problem — names like feat/foo..bar, foo.lock, .foo, HEAD, and -lead satisfying the character-class check yet being rejected by git as branch names, then surfacing as opaque environment-exit-4 instead of actionable usage-exit-2 — is correctly identified and resolved.

No blocking issues found. One suggestion from the prior security review remains open; recorded below for tracking.

---

Findings

🟡 Nit — regex still permits leading hyphens per segment; prior security-review suggestion unaddressed

worktree-create.sh line 110

if [[ ! "$name" =~ ^[A-Za-z0-9._-]+(/[A-Za-z0-9._-]+)*$ ]]; then

The character-class regex still allows segments beginning with - (e.g., -lead, --normalize, --branch). These are caught downstream by git check-ref-format --branch at line 128, which the test suite verifies. However, the protection relies on git not re-interpreting a leading-dash argument as a flag after --branch — correct empirically and consistent with git's option-parsing, but not explicitly guaranteed in git's manpage.

The prior security review (SUGGESTION, MEDIUM confidence) recommended tightening the first-character anchor from [A-Za-z0-9._-]+ to [A-Za-z0-9._][A-Za-z0-9._-]* per segment. This would eliminate the undocumented-behavior dependency entirely, since leading-dash names are already unconditionally invalid git branch names — no currently-accepted name would be newly refused. Apply it →

Not a blocker — the current code is correct and the test coverage is solid. Carrying the suggestion forward for a follow-up.

---

REVIEW.md checks

Criterion	Result
Injection ($name to shell/git)	$name is double-quoted at all call sites, including git check-ref-format --branch "$name". No injection surface. ✓
Secrets / credentials	None added. ✓
Atomicity	The ref check exits before any worktree is created; creation delegates entirely to git worktree add. ✓
Authorization / tenant scope	Not applicable (local shell helper). ✓

Code observations (no action required)

• stdout-purity test (worktree-create.test.sh line 288–290): correctly asserts that the sole stdout line is still the worktree path after the check-ref-format call. A forgotten >/dev/null on line 128 would silently corrupt the EnterWorktree(path:) contract in a way no other assertion would catch — this test makes the suppression verifiable.
• Version bump (0.26.3 → 0.26.4): appropriate — 0.26.3 landed in main before this PR, requiring the bump to avoid a conflict.
• create.md bullet (line 24): accurate and correctly placed.

---

Verdict

LGTM. The fix is correct: git check-ref-format --branch is the right oracle, both streams are suppressed for the right reasons, and the test matrix covers the exit-code contract, the error-message content, and the stdout-purity invariant. No blocking issues; one open suggestion from the prior security review documented above.

[claude]

Claude finished @kyle-sexton's task in 2m 11s —— View job

---

Security Review — PR #1333

• Load tools
• Fetch PR diff
• Read changed files
• Analyze for security vulnerabilities
• Post findings

---

Review: fix(source-control): validate git ref syntax in worktree-create.sh

This re-review was triggered by the merge-from-main synchronize event (d2d82de). The merge added only a version bump (0.26.3 → 0.26.4 in plugin.json) and the matching CHANGELOG entry — the core fix in worktree-create.sh is identical to the prior reviewed commit.

Scope: worktree-create.sh (primary), worktree-create.test.sh, plugin.json, create.md, CHANGELOG.md.

---

No CRITICAL or IMPORTANT vulnerabilities found.

The central addition is correctly implemented:

if ! git check-ref-format --branch "$name" >/dev/null 2>&1; then

• $name is double-quoted — no word-splitting or glob expansion risk. ✓
• Both stdout and stderr are discarded — stdout suppression is load-bearing (git echoes the name on success, which would corrupt the sole-stdout-line output contract). ✓
• $name has already been validated for length and character class before this call. ✓
• All other uses of $name downstream (git -C "$toplevel" worktree add -b "$name" …, printf '%s' "$name") are double-quoted throughout. ✓
• The merge commit added no new shell execution paths or argument surfaces. ✓

---

SUGGESTION — Confidence: MEDIUM (carryover from prior review; unaddressed)

Reliance on undocumented git option-parsing behavior for leading-hyphen segment names

The character-class regex at line 110 (^[A-Za-z0-9._-]+…) permits segments that begin with a hyphen (e.g. -lead, --normalize, --branch). These are then passed as the ref argument to:

git check-ref-format --branch "$name"   # line 128

The PR correctly notes that "git parses no further options after --branch" and empirically verified that option-shaped names like --help, -h, --normalize, and --branch each exit 128 as invalid branch names rather than being treated as flags. That is the correct outcome — these names are properly rejected at exit 2.

However, this protection depends on git's implementation-internal behavior (that parse_options stops processing options once a OPT_CMDMODE-style flag like --branch is consumed). It is not formally guaranteed by the git check-ref-format manpage. A future git refactor of option parsing could theoretically change this, though the practical risk is very low.

Consequence if the assumption breaks: A name like --normalize exits 0 instead of 128, passes this check, and then fails downstream at git worktree add -b "$name" with exit 4 — a less actionable error, not an exploitable one.

Mitigation (defense-in-depth): Anchor each segment to disallow a leading hyphen, since git branch names starting with - are unconditionally invalid:

Change line 110 from:

if [[ ! "$name" =~ ^[A-Za-z0-9._-]+(/[A-Za-z0-9._-]+)*$ ]]; then

to:

if [[ ! "$name" =~ ^[A-Za-z0-9._][A-Za-z0-9._-]*(/[A-Za-z0-9._][A-Za-z0-9._-]*)*$ ]]; then

This eliminates the dependency, keeps the accepted name set identical (no currently-valid branch name starts with -), and removes the need for the option-parsing caveat in the comment. Fix this →

---

Other observations (no security impact)

• The merge commit changed only plugin.json (version 0.26.3 → 0.26.4) and CHANGELOG.md — no new shell execution paths or argument surfaces introduced.
• The test file's for badref in … loop iterates over a fixed literal array; no user input reaches bash "$HELPER".
• All git -C "$toplevel" invocations pass $toplevel (derived from rev-parse --show-toplevel) double-quoted. ✓
• --exclude-from="$include_file" uses =-assignment form, immune to flag misinterpretation. ✓
• $rel from the .worktreeinclude copy is NUL-delimited (-z/read -d ''), preventing path manipulation via embedded newlines. ✓

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d2d82def69

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 2m 0s —— View job

---

PR Review — fix(source-control): validate git ref syntax in worktree-create.sh

• Load tools
• Read PR diff and changed files
• Review worktree-create.sh changes
• Review worktree-create.test.sh changes
• Review create.md and documentation changes
• Post findings

---

Summary

This is a well-scoped, correctly implemented fix. The CWD discovery regression caught in round 1 is resolved in a286444 and the test suite now covers it explicitly. No blocking issues.

---

The core fix — correct

worktree-create.sh line 184

if ! git -C "$toplevel" check-ref-format --branch "$name" >/dev/null 2>&1; then

Three load-bearing details are all correct:

• -C "$toplevel" scoping. The check runs after rev-parse --show-toplevel succeeds (line 159), guaranteeing $toplevel is a healthy repository root. A non-zero exit from check-ref-format therefore means the name, nothing else. This closes the stale-CWD regression.
• >/dev/null 2>&1. --branch echoes the name to stdout on success; discarding it preserves the "created path is the sole stdout line" contract that EnterWorktree(path:) relies on. The order is correct — stdout to /dev/null first, then 2>&1 dupes stderr there.
• Placement. The character-class and length checks still run first (no git calls), then exits 3/4, then the ref grammar check. This matches the existing ordering convention for the pre-existing --base-ref and empty-slug exit-2 checks, so the exit-code contract is consistent.

---

Stale-CWD regression tests — well done

worktree-create.test.sh lines 296–305

stale="$(mktemp -d "$TEST_TMPDIR/staleXXXXXX")"
printf 'gitdir: %s/definitely-not-here\n' "$TEST_TMPDIR" > "$stale/.git"

Creating the stale fixture inline is the right approach — it makes the regression reproducible without external state. The three assertions (valid name exits 0, path on stdout, invalid name still exits 2) together cover the regression and preserve the existing behavior invariants. This is the hard part of the fix and it's done properly.

---

Stdout-purity test — load-bearing, correctly placed

worktree-create.test.sh lines 287–290

out=$(bash "$HELPER" --name "feat/refok" --root "$root" --repo-dir "$repo" 2>/dev/null)
assert_exit "ref-valid name still creates (exit 0)" 0 "$?"
assert_eq "ref check leaves stdout as the sole path line" "$root/acme-widget-feat-refok" "$out"

A forgotten >/dev/null on line 184 would silently corrupt the EnterWorktree contract with no other assertion in the suite catching it. This test makes the suppression verifiable. Worth keeping prominent.

---

Open suggestion — leading-hyphen regex (carryover; still unaddressed)

worktree-create.sh line 112

if [[ ! "$name" =~ ^[A-Za-z0-9._-]+(/[A-Za-z0-9._-]+)*$ ]]; then

Segments may still start with - (e.g., -lead, --normalize). These are correctly caught by check-ref-format --branch downstream — the badref loop includes -lead and the test passes. The comment at line 182 explicitly states the protection mechanism: "git parses no further options after --branch."

That said, this protection relies on an implementation-internal detail of git's option parsing (that OPT_CMDMODE-style flags consume the option-parser before any remaining arguments). It is correct empirically and consistent with git's design, but is not formally documented in the git check-ref-format manpage.

The prior security review's SUGGESTION remains valid: anchoring each segment to [A-Za-z0-9._][A-Za-z0-9._-]* would eliminate this dependency entirely, since leading-hyphen names are unconditionally invalid git branch names. No currently-accepted name would be refused. Apply it →

Not a blocker for this PR — the current implementation is correct and tested. Logging it for a follow-up.

---

Minor observations (no action required)

• create.md line 24 — The added bullet is a long single line. It renders correctly in Markdown and the content is accurate. Not a blocker.
• Version bump 0.26.5 → 0.26.6 — appropriate; 0.26.5 landed in main before this PR closed.
• CHANGELOG entry — detailed and accurate; the stdout-purity and CWD scoping rationale are both recorded.
• Empty-slug guard (lines 274–277) — acknowledged as unreachable in the PR description. Cheap defense-in-depth; correct to leave it.

---

Verdict

LGTM. The fix is correct: git check-ref-format --branch is the right oracle, both streams are suppressed for the right reasons, the CWD regression is fixed and tested, and the stdout-purity invariant is explicitly asserted. One open defense-in-depth suggestion from the prior security review is documented above for a follow-up.

[claude]

Claude finished @kyle-sexton's task in 2m 34s —— View job

---

Security Review — PR #1333

• Load tools
• Fetch PR diff
• Read changed files
• Analyze for security vulnerabilities
• Post findings

---

Review: fix(source-control): validate git ref syntax in worktree-create.sh

Scope: worktree-create.sh (primary), worktree-create.test.sh, plugin.json, create.md, CHANGELOG.md.

---

No CRITICAL or IMPORTANT vulnerabilities found.

The central addition is correctly implemented:

if ! git -C "$toplevel" check-ref-format --branch "$name" >/dev/null 2>&1; then

• $name is double-quoted throughout — no word-splitting or glob expansion risk. ✓
• -C "$toplevel" is double-quoted; $toplevel derives from git rev-parse --show-toplevel, not raw user input. ✓
• Both stdout and stderr are discarded — stdout suppression is load-bearing (git echoes the name on success, which would corrupt the sole-stdout-line output contract). ✓
• The stale-CWD regression (commit a286444) is correctly fixed by scoping the check to -C "$toplevel" after the exit-4 repository probe, guaranteeing a healthy repo at call-time. ✓
• git worktree add -b "$name" at line 402 is safe even for leading-hyphen names: -b is a value-consuming flag, so its argument is consumed as the branch name and not re-parsed as a flag by git. ✓

---

SUGGESTION — Confidence: MEDIUM (third consecutive carry-forward; unaddressed)

Segment-leading hyphen names reach check-ref-format --branch as option-shaped arguments

worktree-create.sh line 112

if [[ ! "$name" =~ ^[A-Za-z0-9._-]+(/[A-Za-z0-9._-]+)*$ ]]; then

The character-class regex still allows segments beginning with - (e.g. -lead, --normalize, --branch). These reach:

git -C "$toplevel" check-ref-format --branch "$name"   # line 184

The PR correctly notes that git parses no further options after --branch and empirically verified that option-shaped names exit 128 as invalid branch names. That protection is correct and tests confirm it. However, it depends on git's implementation-internal behaviour (that parse_options stops after consuming the OPT_CMDMODE-style --branch flag) — a guarantee absent from the git check-ref-format manpage.

Consequence if the assumption breaks: an option-shaped name like --normalize exits 0 instead of 128, passes this check, and fails downstream at git worktree add -b "$name" (exit 4) rather than exit 2 — a less actionable error, not an exploitable one.

Mitigation: anchor each segment's first character to exclude a leading hyphen, since leading-hyphen git branch names are already unconditionally invalid. Change line 112 from:

if [[ ! "$name" =~ ^[A-Za-z0-9._-]+(/[A-Za-z0-9._-]+)*$ ]]; then

to:

if [[ ! "$name" =~ ^[A-Za-z0-9._][A-Za-z0-9._-]*(/[A-Za-z0-9._][A-Za-z0-9._-]*)*$ ]]; then

This eliminates the undocumented-behaviour dependency entirely, keeps the accepted name set identical (no currently-valid branch name starts with -), and removes the need for the option-parsing caveat in the comment block above line 184. Fix this →

---

Other observations (no security impact)

• The >/dev/null 2>&1 ordering at line 184 is correct: stdout redirects first, then 2>&1 duplicates that — the common reversal mistake (2>&1 >/dev/null, which leaves stderr to the terminal) is not present. ✓
• --exclude-from="$include_file" at line 430 uses =-assignment form, immune to flag misinterpretation. ✓
• $rel from the .worktreeinclude loop is NUL-delimited (-z / read -d ''), preventing path manipulation via embedded newlines. ✓
• parse_owner_repo handles remote URLs via pure string operations — the URL never reaches a shell execution context. ✓
• The merge commit (912df57) added only changelog and version-bump changes; no new shell execution paths or argument surfaces were introduced. ✓