fix(semantic-pr): harden pr-title gate against head-branch tampering via pull_request_target

[kyle-sexton]

The tampering vector

The pr-title caller (pr-title.yml) triggered on on: pull_request, which reads the workflow definition from the PR's head branch. A contributor could edit pr-title.yml on their own branch — loosen the guard, drop the trigger, or weaken the inputs — and that tampered definition would run on their PR, silently disabling the Conventional-Commits title gate on the very PR meant to be gated.

The fix (three edits, one PR, backward-compatible)

1. semantic-pr.yml — change the reusable's event-name guard from == 'pull_request' to != 'merge_group'. The step now runs on both pull_request and pull_request_target, skipping only the merge queue. This is load-bearing: an == 'pull_request' guard would skip the step under pull_request_target, the job would pass vacuously, and the gate would be silently disabled.
2. pr-title.yml — switch the dogfood caller's trigger to pull_request_target, so the gate runs the base-branch definition. A head-branch edit to this file can no longer bypass the gate on its own PR.
3. README.md — update the documented reference caller block + prose rationale to match.

Why it's safe

semantic-pr performs no checkout and runs no head code — it reads PR title metadata only. pull_request_target grants it no code-execution surface over untrusted head content, so the usual pull_request_target risk does not apply here.

Backward-compatible

Existing consumers keep validating after they bump the pinned SHA, and can migrate their own caller trigger to pull_request_target independently on their own schedule. Check job names are unchanged, so the required context stays pr-title / pr-title — no ruleset changes needed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LzW7mBrnnXK3f1amgXzf1n

Closes #68

---

Note

Low Risk
Workflow guard and documentation only; semantic-pr still validates PR title metadata with no checkout of head code.

Overview
Hardens the reusable semantic-pr gate so callers can run on pull_request_target without the validation step being skipped and the check passing vacuously.

The reusable workflow’s title-validation step now runs when github.event_name is pull_request or pull_request_target, still skipping only merge-queue runs where the title was already checked. Inline comments document why an explicit allowlist is required (not a bare != 'merge_group') so non-PR triggers do not error.

README updates the canonical consumer pattern: adopt pull_request_target (base-branch workflow definition), add a concurrency group keyed on PR number (with github.ref fallback for merge_group), and explain why the in-repo dogfood pr-title.yml intentionally remains on pull_request until a separate self-flip.

^{Reviewed by Cursor Bugbot for commit 6dd19d7. Bugbot is set up for automated code reviews on this repo. Configure here.}

[claude]

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

---

Code Review

• Gather context and understand the request
• Read changed files and diff
• Review semantic-pr.yml changes
• Review pr-title.yml changes
• Review README.md changes
• Assess security argument against CLAUDE.md ground rules
• Post findings

---

Summary

The vulnerability being fixed is real and the overall approach is sound. However, there are two issues — one that will cause functional breakage in practice, and one that is a guard-precision concern.

---

Bug: Concurrency group collapses across all open PRs under pull_request_target

Severity: High — functional regression

pr-title.yml line 27:

concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: true

Under pull_request (the old trigger), github.ref is refs/pull/{number}/merge — unique per PR. Under pull_request_target (the new trigger), github.ref is the base branch ref — e.g., refs/heads/main — shared by every open PR targeting that branch.

Effect: editing the title of PR #2 while PR #1's check is still running cancels PR #1's check. On a busy repo with several open PRs, this means the pr-title / pr-title check will be in a permanently cancelled state for all but the most-recently-touched PR — which is exactly the gate we're trying to enforce.

Fix: key the group on the PR number, falling back to github.ref so the merge-queue event still groups correctly:

concurrency:
  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
  cancel-in-progress: true

Fix this →

---

Issue: != 'merge_group' guard is too broad

Severity: Medium — subtle footgun for consumers

semantic-pr.yml line 88:

if: ${{ github.event_name != 'merge_group' }}

The PR's comment correctly explains why this guard can't be == 'pull_request' (it would silently pass under pull_request_target). But != 'merge_group' will also pass for any other event a caller might include — workflow_dispatch, schedule, push, etc. In those cases github.event.pull_request is null, and amannn/action-semantic-pull-request will error in an unexpected way.

An explicit allowlist is both clearer in intent and safer:

if: ${{ github.event_name == 'pull_request' || github.event_name == 'pull_request_target' }}

This is self-documenting (the comment says "runs on pull_request AND pull_request_target"), immune to unexpected caller triggers, and makes future reasoning about "what events reach this step" trivial.

Fix this →

---

Security assessment — CLAUDE.md alignment

The CLAUDE.md pull_request_target prohibition is scoped to the claude-review reusable workflow (AI agent + org credential). semantic-pr is categorically different:

• No checkout of head code
• No secrets — only the auto-generated GITHUB_TOKEN
• permissions: pull-requests: read declared at both workflow and job level
• The called action (amannn/action-semantic-pull-request, SHA-pinned) only reads github.event.pull_request.title from the event payload

The classic pull_request_target attack vector (attacker-controlled code runs with a privileged token) does not apply here. The permissions declaration narrows the token even for non-fork PRs where pull_request_target would otherwise carry a broader write token. The security reasoning in the PR description is correct.

---

What's right

• The underlying vulnerability is real: a pull_request-triggered caller reads its own definition from the head branch, so a contributor could weaken or drop the gate on their own PR. Switching to pull_request_target closes this.
• SHA-pinned action (amannn/action-semantic-pull-request@48f256284b…)
• Backward-compatible: existing pull_request callers continue to work; consumers can migrate independently.
• The in-code comments explaining the guard rationale are high-quality and load-bearing.
• README example matches the new canonical caller pattern.

---

The concurrency group bug is the one issue that needs to be fixed before merging — it undermines the gate reliability in exactly the multi-PR scenario this PR is hardening against.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c595429c45

ℹ️ 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".

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit c595429. Configure here.}

[claude]

Claude finished @kyle-sexton's task in 1m 56s —— View job

---

Code Review

• Gather context and understand the request
• Review the latest diff (including ec167ec concurrency fix)
• Verify concurrency group fix is correct
• Assess the != 'merge_group' guard issue from prior review
• Review README changes
• Post findings

---

Concurrency fix (ec167ec) — correct

The fix in pr-title.yml line 32 is sound:

group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}

Under pull_request_target, pull_request.number is the PR number — unique per open PR, so runs don't interfere with each other. Under merge_group, pull_request.number is empty (the event has no PR context), so it falls back to github.ref, which is the queue's unique ref. Both paths are correct. The explanatory comment in the file is accurate and will help future readers.

The README reference block was updated consistently in the same PR. ✅

---

Open: != 'merge_group' guard is still too broad

semantic-pr.yml line 88:

if: ${{ github.event_name != 'merge_group' }}

This was flagged in the previous review and is still unresolved. The guard passes for any event that isn't merge_group — including workflow_dispatch, push, schedule, or any other trigger a consumer might add to their caller. In those cases github.event.pull_request is null, and amannn/action-semantic-pull-request will error with an opaque failure message rather than a clean skip.

The fix is a one-line change to an explicit allowlist, which is also self-documenting:

if: ${{ github.event_name == 'pull_request' || github.event_name == 'pull_request_target' }}

This matches the intent stated in the comment directly above it ("Runs on both pull_request AND pull_request_target, skipping only the merge queue"), is immune to unexpected caller triggers, and makes the allowed surface trivially auditable.

One note: the ignore-labels input documentation already hints at consumers adding labeled/unlabeled to their triggers. Those are activity types on pull_request/pull_request_target, so github.event_name remains pull_request or pull_request_target for them — the allowlist handles that correctly.

Fix this →

---

Security assessment — CLAUDE.md alignment

CLAUDE.md's pull_request_target prohibition is explicitly scoped to the claude-review reusable workflow (AI agent + org credential). semantic-pr is categorically different:

• No checkout of head code
• No secrets — only auto-generated GITHUB_TOKEN
• permissions: pull-requests: read declared at both workflow and job level
• amannn/action-semantic-pull-request (SHA-pinned) reads only github.event.pull_request.title from the event payload

The classic attack surface (pull_request_target runs attacker-controlled code with a privileged token) does not exist here. The permissions declaration narrows the token even for non-fork PRs where pull_request_target would otherwise carry write access. The security reasoning in the PR description is correct.

---

What's right

• Root vulnerability is real and the fix approach is correct: switching to pull_request_target ensures the caller definition always comes from the base branch, closing the head-branch tampering vector. ✅
• Concurrency collapse regression (high severity from prior review) is fixed. ✅
• SHA-pinned action (amannn/action-semantic-pull-request@48f256284b…). ✅
• Backward-compatible: existing pull_request callers continue to work; check name pr-title / pr-title is unchanged. ✅
• In-code comments are accurate, load-bearing, and well-placed. ✅
• README consumer example is updated consistently. ✅

---

Summary

One open issue remains from the prior review: the != 'merge_group' guard in semantic-pr.yml is broader than needed and will cause opaque failures if a consumer adds an unexpected trigger. It's a one-line fix to an explicit allowlist. Not a blocker by itself (the action errors rather than silently passing), but worth addressing before merge since this file is the stable reusable API that all consumers depend on.

[claude]

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

---

Code Review

• Gather context and understand the request
• Read the full diff (origin/main...HEAD)
• Review semantic-pr.yml changes
• Review README.md changes
• Assess open issue from prior reviews (!= 'merge_group' guard)
• Verify pr-title.yml disposition at HEAD
• Post findings

---

What changed in 2217a6a

The latest commit refined the approach from the prior two commits. At HEAD, pr-title.yml is now unchanged from main — the dogfood caller was reverted to pull_request and its original concurrency group. The PR's actual diff against main is therefore only two files:

• semantic-pr.yml: step guard changed from == 'pull_request' to != 'merge_group'
• README.md: consumer snippet updated to use pull_request_target + the per-PR concurrency key

This is a cleaner design than the intermediate commits. The reusable supports both callers; each consumer independently decides whether to use pull_request (simpler) or pull_request_target (tamper-resistant). This repo's own dogfood caller stays on pull_request — appropriate since only maintainers commit here.

---

Remaining open issue: != 'merge_group' guard is still too broad

semantic-pr.yml line 88:

if: ${{ github.event_name != 'merge_group' }}

This was flagged in both prior reviews and remains unresolved. The guard passes for any event that isn't merge_group — workflow_dispatch, push, schedule, or any trigger a consumer adds. On those events github.event.pull_request is null and amannn/action-semantic-pull-request throws an opaque error rather than cleanly skipping.

The comment directly above the guard states the intent:

"Runs on both pull_request AND pull_request_target, skipping only the merge queue."

An explicit allowlist matches that intent precisely and is self-documenting:

if: ${{ github.event_name == 'pull_request' || github.event_name == 'pull_request_target' }}

Note: the ignore-labels input documentation correctly mentions adding labeled/unlabeled to the caller's triggers — those are activity types on pull_request/pull_request_target, so github.event_name stays pull_request or pull_request_target for them. The allowlist handles this correctly.

The comment's argument that != 'merge_group' is "load-bearing" (vs == 'pull_request') is correct — but that argument rules out option 1 (== 'pull_request'), not option 3 (the explicit allowlist). An explicit allowlist is strictly better than the negation guard: same valid-event behaviour, no surface area for misconfigured triggers.

Fix this →

---

Security assessment — CLAUDE.md alignment

CLAUDE.md's pull_request_target prohibition is explicitly scoped to the claude-review reusable workflow (AI agent + org credential). semantic-pr is categorically different:

• No checkout of head code
• No secrets — only the auto-generated GITHUB_TOKEN
• permissions: pull-requests: read declared at both workflow and job level
• amannn/action-semantic-pull-request (SHA-pinned at 48f256284b) reads only github.event.pull_request.title from the event payload

The README consumer example correctly documents pull_request_target with pull-requests: read only — the narrowest possible grant. The classic attack vector (pull_request_target running attacker-controlled code with a privileged token) does not apply here. Security posture is sound.

---

What's right

• Root vulnerability (head-branch tampering of pull_request caller) correctly identified and the reusable is hardened to support the fix. ✅
• Dogfood caller pr-title.yml is unchanged from main — concurrency regression from intermediate commits is fully reverted. ✅
• SHA-pinned action (amannn/action-semantic-pull-request@48f256284b…). ✅
• Backward-compatible: existing pull_request callers continue to work unchanged. ✅
• In-code comment on line 77–87 is accurate, load-bearing, and well-written. ✅
• README consumer example includes the correct pull_request_target + per-PR concurrency pattern. ✅
• No CLAUDE.md ground rules violated. ✅

---

Summary

One open issue persists across all three reviews: the != 'merge_group' guard in semantic-pr.yml passes for unexpected triggers and will cause opaque action failures. A one-line fix to an explicit allowlist closes the gap and directly matches what the comment documents as intent. Not a silent-pass risk — the action errors rather than vacuously succeeding — but worth addressing before this becomes the stable reusable API all consumers pin to.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2217a6a2a7

ℹ️ 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 1m 27s —— View job

---

Code Review

• Gather context and understand the request
• Read current diff (origin/main...HEAD)
• Review semantic-pr.yml — verify != 'merge_group' guard fix
• Review README.md changes
• Check CLAUDE.md alignment
• Post findings

---

All prior findings resolved ✅

The latest commit (6dd19d7) closes the last open issue from the three-round review. Here's a full accounting of the current state.

---

semantic-pr.yml — guard fix is correct

Line 91:

if: ${{ github.event_name == 'pull_request' || github.event_name == 'pull_request_target' }}

This is exactly the explicit allowlist requested across all three prior reviews. Behaviour by event:

Event	Result
pull_request	step runs, gate enforced
pull_request_target	step runs, gate enforced (base-branch definition, tamper-resistant)
merge_group	step skipped, job succeeds → required check reports for the queue
workflow_dispatch, push, schedule, etc.	step skipped, job succeeds → no opaque action failure on missing PR context

The ignore-labels input documentation mentions adding labeled/unlabeled to the caller's triggers — those are activity types on pull_request/pull_request_target, so github.event_name stays pull_request or pull_request_target for them. The allowlist handles that correctly.

The comment block above the step (lines 77–90) is accurate, thorough, and explains the reasoning for all three non-obvious constraints. ✅

---

README.md — canonical block is correct

The consumer block now documents:

• pull_request_target trigger (tamper-resistant, base-branch definition)
• Per-PR concurrency group: github.event.pull_request.number || github.ref (unique per PR under pull_request_target; falls back to the queue ref under merge_group)
• pull-requests: read only — narrowest possible grant

The prose following the block explains the bootstrap constraint (why the in-repo dogfood caller deliberately stays on pull_request) and directly addresses the Codex P2 concern about pointing consumers at the stale caller. The README now explicitly says "Adopt the canonical block below (not the in-repo .github/workflows/pr-title.yml...)". ✅

---

pr-title.yml — unchanged from main, intentionally

The dogfood caller is back to pull_request with ${{ github.workflow }}-${{ github.ref }} concurrency. Under pull_request, github.ref is refs/pull/{number}/merge — unique per PR — so the concurrency group is correct for both the PR and merge_group paths. The bootstrap rationale (flipping to pull_request_target while the required check reads the still-pull_request base definition would deadlock the self-flip) is documented in the README. ✅

---

Resolution summary

Finding	Status
HIGH: Concurrency group collapses across PRs under pull_request_target	✅ Fixed (ec167ec), then cleanly resolved by reverting dogfood to pull_request (2217a6a) — no regression
MEDIUM: != 'merge_group' guard passes for unexpected triggers	✅ Fixed in 6dd19d7 — explicit allowlist
Codex P2: README pointing consumers at stale dogfood caller	✅ Addressed in 2217a6a/6dd19d7 — README now directs consumers to canonical block and explains dogfood divergence

---

Security assessment — CLAUDE.md alignment

CLAUDE.md's pull_request_target prohibition is explicitly scoped to the claude-review reusable workflow (AI agent + org credential). semantic-pr is categorically different: no checkout of head code, no secrets, permissions: pull-requests: read at both workflow and job level, and amannn/action-semantic-pull-request (SHA-pinned at 48f256284b) reads only the PR title from the event payload. The classic attack vector does not apply. ✅

---

Ready to merge

No open findings. The root vulnerability is correctly closed, prior findings are fully resolved, the implementation is backward-compatible, and the in-code comments and README documentation are accurate and load-bearing.

[cursor]

Risk: medium. Cursor Bugbot passed on the latest sync (6dd19d7); all prior review threads are resolved and no new Bugbot findings were reported. Approved; no reviewers assigned.

  

_{Sent by Cursor Approval Agent: Pull Request Router and Approver}