feat(claude-lanes): admit listed bot pushers through an allowed-bots input by kyle-sexton · Pull Request #665 · melodic-software/ci-workflows

kyle-sexton · 2026-10-04T01:19:02Z

Closes #637.

Problem

Since #622, both Claude review lanes skip any event whose github.actor ends in [bot]. That checks who pushed, not who authored the PR. So a cursor[bot] push to a PR a human opened is never reviewed.

Changes

Both claude-review.yml and claude-security-review.yml now have:

A new workflow_call input, allowed-bots, with an empty default, so current callers see no change.
A new job gate. On a pull_request event, the PR must come from this repo, must not be a draft, and its author must not be a bot. A bot pusher runs the job only if it appears in allowed-bots.

${{ (github.event_name != 'pull_request'
    || (github.event.pull_request.head.repo.full_name == github.repository
        && github.event.pull_request.draft == false
        && !endsWith(github.event.pull_request.user.login, '[bot]')))
    && (!endsWith(github.actor, '[bot]')
        || contains(format(',{0},', inputs.allowed-bots), format(',{0},', github.actor))) }}

The same list is passed to claude-code-action as allowed_bots. Without it, checkHumanActor throws for any bot actor not on the list.
Updated the SECURITY MODEL headers and the README Claude lanes contract (inputs table and skips).

Notes:

The comma-wrapped contains avoids partial-name matches. An empty input or * can't admit anything, because ,*, never contains ,cursor[bot],.
Following the issue, I left out an author_association clause.

Testing

claude-lane-job-gates.test.cjs evaluates the gate for four combinations:

Pusher PR author Result

human human runs

listed bot human runs

unlisted bot human skips

anyone dependabot[bot] skips

It also checks that allowed_bots is wired to the same input.
node --test .github/scripts/*.test.cjs .github/actions/claude-lane-outcome/*.test.cjs passes 219 of 219.
actionlint 1.7.12 reports nothing on both workflows.

Follow-ups outside this repo (acceptance criteria)

The melodic-software/standards runner-policy contract must allow callers to pass allowed-bots at the new pin.
A live check after release: a cursor[bot] push to a human-owned same-repo PR runs both lanes, and a Dependabot PR still skips.

This PR and #664 edit the same two workflow files in different places. They merge together without conflicts.

…input Gate review jobs on the PR author rather than the pusher, and pass the same allowed-bots list through to claude-code-action so cursor[bot] pushes on human-owned PRs can be reviewed. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>

chatgpt-codex-connector · 2026-10-04T01:19:11Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-04T01:21:41.127661Z	`28b86ab`	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

claude · 2026-10-04T01:19:17Z

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

Security review — no issues found

Gather context (PR diff, changed workflows)
Review job-gate expression logic in both workflows for bypasses
Review allowed-bots / allowed_bots wiring for injection or trust issues
Review README changes
Post findings

Scope: .github/workflows/claude-review.yml, .github/workflows/claude-security-review.yml, .github/scripts/claude-lane-job-gates.test.cjs, README.md (diffed against origin/main).

Instruction-surface check: the SECURITY MODEL header in both workflows narrows a standing control — "every bot actor" skips becomes "bot pushers skip unless listed in allowed-bots." The docs/conventions/instruction-exception-register/README.md file this lane checks such changes against isn't present in this checkout, so per the skill's fallback I reasoned from consequence instead: the relaxation is bounded by checks the diff leaves untouched — same-repo only (fork PRs still never reach secrets), not draft, and the PR author still must not be a bot (no exception for that field). allowed-bots itself is a workflow_call input set by the calling workflow's own YAML, not by anything in the PR/issue body, so it isn't attacker-influenced. Since a same-repo PR requires the author to already have write access to the base repo, admitting a maintainer-named bot as a pusher on that author's own branch doesn't cross a trust boundary the old blanket skip was protecting. I did not treat this as a finding, but flagging the register was unavailable as the skill requires.

Gate logic (claude-review.yml:159-166, claude-security-review.yml:152-159):

claude-review.yml#L159-L166
The contains(format(',{0},', inputs.allowed-bots), format(',{0},', github.actor)) comma-wrap correctly prevents partial-name matches (,bot, can't match inside ,notabot,), and GitHub login charset rules mean neither side can contain a literal comma, so no list-boundary spoofing.
The github.event_name != 'pull_request' escape valve that lets draft/fork/author checks pass through for pull_request_target/workflow_run is pre-existing (unchanged by this diff) and is still caught by the separate "Reject privileged triggers" hard-fail step later in the same job — not a new bypass.
allowed_bots is passed to claude-code-action only via with: (action input), never interpolated into a run: shell block, so no command-injection surface from this value.

No CRITICAL, IMPORTANT, or SUGGESTION findings. GitHub Actions hardening concerns (trigger types, permission scopes, pin freshness) are zizmor's lane and out of scope here; this review covered only the logic/trust-boundary reasoning static analysis can't do.

claude · 2026-10-04T01:19:19Z

Claude finished @kyle-sexton's task in 4m 25s —— View job

Claude code review

Gather context (diff, CLAUDE.md/AGENTS.md/REVIEW.md rules, changed files)
Understand the request (CI code-review lane invocation)
Review claude-review.yml / claude-security-review.yml gate logic
Review claude-lane-job-gates.test.cjs coverage
Review README.md changes
Adversarially verify candidate findings
Post findings and final summary

Scope reviewed: claude-review.yml, claude-security-review.yml, claude-lane-job-gates.test.cjs, README.md (the 4 files in this PR), against origin/main.

What the change does: restructures the review/security-review job if: gate so a bot pusher (github.actor) can run when listed in the new allowed-bots input, while a bot PR author (github.event.pull_request.user.login) is always skipped — correctly fixing the cursor[bot]-push-to-human-PR gap from #622/#637. The restructured boolean is logically equivalent to the old gate for the pre-existing draft/fork/pull_request_target checks, and the four-case test matrix matches the PR's own description.

Finding

Posted as two inline comments (same root cause, duplicated in both workflows):

Summary: contains(format(',{0},', inputs.allowed-bots), format(',{0},', github.actor)) only matches an entry that sits between two bare commas. With allowed-bots: 'cursor[bot], dependabot[bot]' (space after the comma — a natural way to write the list), the wrapped list is ,cursor[bot], dependabot[bot],; dependabot[bot] wraps to ,dependabot[bot],, but the character before dependabot in the list is a space, not a comma, so it never matches. The first entry is admitted, every later entry with a space before it is silently dropped — no error, just a bot that should run but doesn't. Fails closed, so not a security issue, but it's an untested footgun: claude-lane-job-gates.test.cjs only ever exercises a single-entry list. Suggested fix: document "no spaces around commas" explicitly, or make the match whitespace-tolerant, and add a 2+-entry test case (with and without a space after the comma).

Everything else — the gate restructuring itself, the allowed_bots wiring into claude-code-action, the test matrix, and the README table/prose — checked out.
· branch

github-actions · 2026-10-04T01:20:31Z

PR body contract — issue linkage

This PR body does not yet satisfy the issue-linkage contract:

Missing a "## Summary" section. Describe what this PR changes and why, in a sentence or two.
Missing a "## Fix" section. State the concrete change and how it addresses the problem.
Missing a "## Verification" section. Record concrete evidence the change works (commands, gates, output).
Missing a "## Related" section. List related PRs, ADRs, or decision-log entries this PR does not close.

Edit the body and this comment updates itself on the next run.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 28b86ab302

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

chatgpt-codex-connector · 2026-10-04T01:21:38Z

+          && (!endsWith(github.actor, '[bot]')
+              || contains(format(',{0},', inputs.allowed-bots), format(',{0},', github.actor))) }}


Normalize list entries before matching allowed bots

When a caller uses conventional comma-separated formatting such as cursor[bot], renovate[bot], the wrapped input becomes ,cursor[bot], renovate[bot],, which does not contain ,renovate[bot],; pushes by that explicitly listed bot therefore skip the job. The same expression appears in both review workflows, so normalize or validate whitespace before applying the gate.

Useful? React with 👍 / 👎.

github-actions · 2026-10-04T01:21:59Z

claude-security-review has reviewed this pull request through 28b86ab; a later push is reviewed from there.

claude · 2026-10-04T01:23:44Z

+              && github.event.pull_request.draft == false
+              && !endsWith(github.event.pull_request.user.login, '[bot]')))
+          && (!endsWith(github.actor, '[bot]')
+              || contains(format(',{0},', inputs.allowed-bots), format(',{0},', github.actor))) }}


Correctness: every allowed-bots entry after the first is silently dropped when the list has a space after the comma.

contains(format(',{0},', inputs.allowed-bots), format(',{0},', github.actor)) only matches when the wrapped actor sits between two bare commas. Given allowed-bots: 'cursor[bot], dependabot[bot]' — a natural way to write a comma-separated list — the wrapped list is ,cursor[bot], dependabot[bot],. Looking up dependabot[bot] wraps to ,dependabot[bot],, but the character immediately before dependabot in the list is a space, not a comma, so that substring never appears. cursor[bot] (the first entry) is admitted; dependabot[bot] is quietly skipped even though it's in the list, with no error anywhere.

This fails closed (worst case the job just skips a bot that should have run), so it isn't a security issue, but it's a silent footgun, and the new test suite (claude-lane-job-gates.test.cjs) only ever exercises a single-entry allowed-bots list, so this never gets caught.

Fix: either document that entries must have no surrounding whitespace (cursor[bot],dependabot[bot]), or make the match whitespace-tolerant, and add a test with 2+ entries — with and without a space after the comma — asserting both are admitted.

Same construct, same bug, at claude-security-review.yml:160.

claude · 2026-10-04T01:23:52Z

+              && github.event.pull_request.draft == false
+              && !endsWith(github.event.pull_request.user.login, '[bot]')))
+          && (!endsWith(github.actor, '[bot]')
+              || contains(format(',{0},', inputs.allowed-bots), format(',{0},', github.actor))) }}


Same comma-wrap matching bug as claude-review.yml:167: contains(format(',{0},', inputs.allowed-bots), format(',{0},', github.actor)) requires a bare comma immediately before the wrapped actor name. With allowed-bots: 'cursor[bot], dependabot[bot]' the list wraps to ,cursor[bot], dependabot[bot], — the space after the first comma means ,dependabot[bot], never appears as a substring, so dependabot[bot] is silently never admitted even though it's listed. Only the first entry (or any entry with no space after its preceding comma) reliably matches.

No test in claude-lane-job-gates.test.cjs covers a multi-entry allowed-bots list, so this is untested. Either document "no spaces around commas" explicitly, or make the match whitespace-tolerant, and add coverage for 2+ entries.

github-actions · 2026-10-04T01:24:13Z

claude-review has reviewed this pull request through 28b86ab; a later push is reviewed from there.

…669) No related issue: Phase 4 of the GitHub Actions conventions program (tracking: melodic-software/standards#672) ## Summary Renames this repository's workflows and actions to the org naming convention (`<stage>-<function>[-<modifier>]`, `name:` = file stem), as recorded in standards `components/github-actions-conventions/rename-map.json`. It also folds the three `-self` dogfood callers into thin jobs in `pr-require-checks.yml` (formerly `ci.yml`). This is the content of the v0.34.0 restructure release; consumers keep working on their SHA pins until they repin. ## Fix - `d1b5dc0`: `git mv` of 45 files to their mapped paths, `job_renames` applied (including `needs:` and `needs.<id>.result`), and reusables reference their own actions with `$/.github/actions/<x>` instead of a pinned full path. - `a724617`: deletes `claude-review-self.yml`, `claude-security-review-self.yml` and `issue-triage-label-self.yml`; adds jobs `pr-review`, `pr-review-security` and `intake-label-needs-triage`, each pinned to the v0.33.0 SHA of its old path with the same permissions and the one secret the `-self` file passed. It adds the `ready_for_review` and `issues: [opened, reopened]` triggers and the event guards, keeps the three folded jobs out of `ci-status`, and deletes `composites-head`, because `$/` from a `./` call already runs HEAD's composites. - `6c7ba42`: the two standards-synced composites (`comment-hygiene` and `machine-specific-paths`) are reached through `$/`. Their directories keep their old names until the standards sync destinations move. - `b2f99f9`: the folded review jobs run only on opened, synchronize, reopened and ready_for_review, which is what the `-self` files ran on. - `ci-status` keeps its job id and stays the only required check. ## Verification - Fresh-context verifier: 8 of 9 criteria pass. The ninth fails on one pre-existing naming-lint warning (job id `plan` in `maintenance-sync-standards.yml`). It is non-blocking, and the job id is unchanged from main. - `actionlint`: only the `$/ ... ref is missing` errors, which standards#675 waives through the synced config. - `node --test .github/scripts/*.test.cjs`: 192/192 pass. - naming-lint enforcing: only the two sync-managed directories remain, and they move with the standards sync-destination change. - The github-iac governance-verify draft (github-iac#598) passes against this branch. ## Related - Prerequisites, all merged: standards#675 (actionlint `$/`), #676 (lockstep drift), #677 (repin path mapping), #678 (runner-policy `$/`), and the Dependabot ignores in six repos. - Lands in lockstep with github-iac#598, followed by a self-repin PR and the v0.34.0 release. - Open PRs #663, #664 and #665 touch renamed paths and need a rebase after this merges. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

kyle-sexton · 2026-10-07T05:14:30Z

Closing: stale agent PR built on pre-rename paths (v0.34.0 renamed the workflows); superseded by the conventions migration.

github-actions Bot added the needs-issue-linkage label Oct 4, 2026

chatgpt-codex-connector Bot reviewed Oct 4, 2026

View reviewed changes

claude Bot reviewed Oct 4, 2026

View reviewed changes

kyle-sexton mentioned this pull request Oct 4, 2026

refactor: rename workflows and actions to the org naming convention #669

Merged

kyle-sexton closed this Oct 7, 2026

kyle-sexton deleted the cursor/review-lanes-allowed-bots-dd48 branch October 7, 2026 05:14

Navigation Menu

Repository navigation

feat(claude-lanes): admit listed bot pushers through an allowed-bots input - #665

feat(claude-lanes): admit listed bot pushers through an allowed-bots input#665
kyle-sexton wants to merge 1 commit into
mainfrom
cursor/review-lanes-allowed-bots-dd48

kyle-sexton commented Oct 4, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 4, 2026 •

edited

Loading

Uh oh!

claude Bot commented Oct 4, 2026 •

edited

Loading

Uh oh!

claude Bot commented Oct 4, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 4, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Oct 4, 2026

Uh oh!

github-actions Bot commented Oct 4, 2026

Uh oh!

claude Bot Oct 4, 2026

Uh oh!

claude Bot Oct 4, 2026

Uh oh!

github-actions Bot commented Oct 4, 2026

Uh oh!

kyle-sexton commented Oct 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Pusher	PR author	Result
human	human	runs
listed bot	human	runs
unlisted bot	human	skips
anyone	`dependabot[bot]`	skips

		&& (!endsWith(github.actor, '[bot]')
		\|\| contains(format(',{0},', inputs.allowed-bots), format(',{0},', github.actor))) }}

Conversation

kyle-sexton commented Oct 4, 2026

Problem

Changes

Testing

Follow-ups outside this repo (acceptance criteria)

Uh oh!

chatgpt-codex-connector Bot commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

claude Bot commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security review — no issues found

Uh oh!

claude Bot commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Claude code review

Finding

Uh oh!

github-actions Bot commented Oct 4, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Oct 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 4, 2026

Uh oh!

claude Bot Oct 4, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Oct 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 4, 2026

Uh oh!

kyle-sexton commented Oct 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chatgpt-codex-connector Bot commented Oct 4, 2026 •

edited

Loading

claude Bot commented Oct 4, 2026 •

edited

Loading

claude Bot commented Oct 4, 2026 •

edited

Loading