Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/bump-callers/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ forward automatically instead of silently drifting commits behind.
| [`bump-cursor-review-callers.yml`](../workflows/bump-cursor-review-callers.yml) | `cursor-review.yml` or `cursor-review/**` | `CURSOR_REVIEW_CALLERS` | non-empty (hard-fails if empty) |
| [`bump-agents-md-callers.yml`](../workflows/bump-agents-md-callers.yml) | `agents-md-integrity.yml` or `agents-md-integrity/**` | `AGENTS_MD_CALLERS` | empty `[]` (grows as callers land) |
| [`bump-pr-size-callers.yml`](../workflows/bump-pr-size-callers.yml) | `pr-size.yml` or `scripts/check-pr-size/**` | `PR_SIZE_CALLERS` | empty `[]` (grows as callers land) |
| [`bump-assign-reviewers-callers.yml`](../workflows/bump-assign-reviewers-callers.yml) | `assign-reviewers.yml` | `ASSIGN_REVIEWERS_CALLERS` | empty `[]` (grows as callers land) |

They stay as thin entrypoints rather than one matrix because their triggers
differ: a `cursor-review.yml` change must not spuriously bump agents-md or
Expand Down
92 changes: 92 additions & 0 deletions .github/workflows/bump-assign-reviewers-callers.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
name: Bump assign-reviewers callers

# When the assign-reviewers reusable workflow is updated on main, open a
# SHA-bump PR in every repo that pins a caller against it. PRs are opened by
# Cloud Code Bot so they are easy to filter and merge.
#
# This is a thin entrypoint over the shared bumper at
# .github/bump-callers/bump-callers.sh — the same script drives the
# cursor-review, agents-md-integrity, AND pr-size caller fleets
# (bump-cursor-review-callers.yml, bump-agents-md-callers.yml,
# bump-pr-size-callers.yml). Keeping ONE implementation is deliberate: a forked
# copy is how other shared machinery in the org has drifted. The entrypoints
# differ only in their path-filter trigger and the parameters passed below.
# (They stay separate rather than a single matrix because their triggers differ
# — an assign-reviewers.yml change must not spuriously bump the other fleets'
# callers, and vice versa.)
#
# The caller list is NOT hardcoded here. This repo is PUBLIC (workflow file and
# Actions run logs are both publicly viewable), and most callers are private, so
# their names must never appear in this file or its logs. The list lives in the
# repo-level Actions variable `ASSIGN_REVIEWERS_CALLERS` (config, not a
# credential — a variable, not a secret, since secrets are write-only via the
# API) as a JSON array of {"repo","file","label"} objects, same shape as
# CURSOR_REVIEW_CALLERS. Every repo name is `::add-mask::`ed out of the (public)
# run logs before it is ever echoed. The variable is seeded EMPTY (`[]`) — an
# empty list is a clean no-op here (ALLOW_EMPTY below); the rollout's per-repo
# caller tickets add entries as callers land (today only Comfy-Org/cloud pins a

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium — This comment hard-codes the caller repo name Comfy-Org/cloud, permanently disclosing a private consumer in this public repo's source history — the exact leak the surrounding design (variable-stored, ::add-mask::ed caller list) exists to prevent. Drop the repo name from the comment. Raised by 1 of 8 reviewers (gpt-5.6-sol-max adversarial).

# caller).
#
# Update flow — adding/removing a caller needs NO public commit:
# gh variable set ASSIGN_REVIEWERS_CALLERS --repo Comfy-Org/github-workflows \
# --body "$(jq -c . callers.json)"
# Keep the canonical callers.json in a PRIVATE infra/ops repo so variable edits
# have a reviewed source of truth; the org audit log records each edit. (The
# specific home repo is intentionally not named here — this file is public.)

on:
workflow_dispatch: {} # allow on-demand runs (e.g. to re-bump callers)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highworkflow_dispatch here accepts any branch or tag ref, and the dispatched ref supplies both this workflow definition and the checked-out bump-callers.sh. A collaborator can dispatch a branch with a modified bumper and have it run with the org-scoped Cloud Code Bot token, bypassing main-branch review; add a job guard that fails unless github.ref == 'refs/heads/main'. Raised by 1 of 8 reviewers (gpt-5.6-sol-max adversarial).

push:
branches: [main]
paths:
# assign-reviewers.yml is entirely self-contained — its selection logic
# runs inline via actions/github-script, with no external prompt/script
# asset dir (unlike cursor-review/** or scripts/check-pr-size/**). So the
# workflow file itself is the only path that can change its behavior.
- .github/workflows/assign-reviewers.yml

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low — The push path filter also fires when assign-reviewers.yml is deleted or renamed; the bumper would then open PRs pinning callers to a commit where the referenced reusable workflow no longer exists, breaking those callers. Raised by 1 of 8 reviewers (gpt-5.6-sol-max edge-case).


permissions:
contents: read

# Serialize runs of this fleet. The bumper now pushes to a STABLE branch
# (ci/bump-assign-reviewers) shared across runs, so two overlapping runs (a
# rapid second main push, or a push racing a manual re-run) would force-reset
# that branch and race the PR update — an older run finishing last could leave
# the committed diff pinned to a stale SHA. cancel-in-progress: false lets the
# running bump finish; GitHub keeps only the newest pending run, so the latest
# SHA always wins (BE-3882).
concurrency:
group: bump-assign-reviewers-callers
cancel-in-progress: false

jobs:
bump:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v6

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Highactions/checkout is pinned to the mutable @​v6 tag instead of a full commit SHA, violating this repo's SHA-pin-everything policy (AGENTS.md) and inconsistent with the SHA-pinned create-github-app-token on line 72 of the same job. If that tag were retargeted it would run attacker-controlled checkout code in the very job that mints an org-scoped Cloud Code Bot token, and the bare tag also fails the pinact/zizmor pin-validation. Raised by 5 of 8 reviewers (claude-opus-4-8-thinking-max adversarial, gemini-3.1-pro adversarial, gpt-5.6-sol-max adversarial, kimi-k2.7-code adversarial, gemini-3.1-pro edge-case).

with:
persist-credentials: false

- name: Generate Cloud Code Bot token
uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1.12.0
id: token
with:
app-id: ${{ vars.APP_ID }}
private-key: ${{ secrets.CLOUD_CODE_BOT_PRIVATE_KEY }}
owner: Comfy-Org

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium — The token is requested with only owner: Comfy-Org and no repositories: scope, so it grants every repository and permission in the App's org installation rather than just the caller repos the bumper touches. Scope it to the specific caller repositories to limit blast radius if the bumper or its token is misdirected or compromised. Raised by 1 of 8 reviewers (gpt-5.6-sol-max adversarial).


- name: Bump SHA in caller repos
env:
GH_TOKEN: ${{ steps.token.outputs.token }}
NEW_SHA: ${{ github.sha }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 MediumNEW_SHA uses github.sha unconditionally, but that is not guaranteed to be the current main tip: a workflow_dispatch on a branch, a re-run of an old push (which keeps its original SHA), or an out-of-order concurrency run can each supply a stale/unmerged commit and pin callers to code that isn't on main — contradicting the 'latest SHA always wins' claim in the concurrency comment. Resolve the current main tip at run time (e.g. git rev-parse origin/main) instead of trusting github.sha. Raised by 3 of 8 reviewers (gpt-5.6-sol-max edge-case, kimi-k2.7-code edge-case, gpt-5.6-sol-max adversarial).

VAR_NAME: ASSIGN_REVIEWERS_CALLERS
TAG: assign-reviewers
WORKFLOW_FILE: assign-reviewers.yml
# Seeded empty; an empty list is a clean no-op, not an error.
ALLOW_EMPTY: "true"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 LowALLOW_EMPTY: "true" is correct for the seeded-empty phase, but once callers are added it turns an accidentally-cleared or missing ASSIGN_REVIEWERS_CALLERS variable into a silent no-op, so SHA bumps would quietly stop with no failure signal. Consider requiring non-empty once the fleet has live callers, or at least warning when the list is empty. Raised by 1 of 8 reviewers (gpt-5.6-sol-max adversarial).

# JSON array of {"repo","file","label"} — see the header comment for the
# update flow. Kept in a variable (not the file) so private caller names
# never land in this public repo or its logs.
CALLERS_JSON: ${{ vars.ASSIGN_REVIEWERS_CALLERS }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 HighCALLERS_JSON is populated from a non-secret Actions variable and set as step-level env, so the runner prints its value in the step's env: preamble before bump-callers.sh can ::add-mask:: the names, leaking the entire private caller list into this public repo's logs once the variable is populated. Register the masks in an earlier step, or feed the list via a file/stdin instead of a logged env var. Raised by 1 of 8 reviewers (gpt-5.6-sol-max adversarial).

run: bash .github/bump-callers/bump-callers.sh
5 changes: 4 additions & 1 deletion .github/workflows/test-bump-callers.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ name: Test bump-callers script

# Runs the functional tests + shellcheck for the shared caller-bump script
# (.github/bump-callers/bump-callers.sh). That one script drives the SHA-bump
# fan-out for the cursor-review, agents-md-integrity, AND pr-size caller fleets,
# fan-out for the cursor-review, agents-md-integrity, pr-size, AND
# assign-reviewers caller fleets,
# so a regression here silently breaks every consumer repo's pin auto-bump (or,
# worse, leaks a private caller name into this public repo's run logs) — cheap
# to guard with a unit run on change. Mirrors test-cursor-review-scripts.yml /
Expand All @@ -15,6 +16,7 @@ on:
- '.github/workflows/bump-cursor-review-callers.yml'
- '.github/workflows/bump-agents-md-callers.yml'
- '.github/workflows/bump-pr-size-callers.yml'
- '.github/workflows/bump-assign-reviewers-callers.yml'
- '.github/workflows/test-bump-callers.yml'
push:
branches: [main]
Expand All @@ -23,6 +25,7 @@ on:
- '.github/workflows/bump-cursor-review-callers.yml'
- '.github/workflows/bump-agents-md-callers.yml'
- '.github/workflows/bump-pr-size-callers.yml'
- '.github/workflows/bump-assign-reviewers-callers.yml'
- '.github/workflows/test-bump-callers.yml'

permissions:
Expand Down
4 changes: 2 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ tests — run the matching command above for whatever you touched.
- `assign-prs-to-author.yml` — assigns unassigned open PRs to their author.
- `detect-unreviewed-merge.yml` — SOC 2: flags PRs merged without approval.
- `bump-cursor-review-callers.yml` / `bump-agents-md-callers.yml` /
`bump-pr-size-callers.yml` — thin entrypoints over `bump-callers.sh` that fan
SHA bumps out to consumers.
`bump-pr-size-callers.yml` / `bump-assign-reviewers-callers.yml` — thin
entrypoints over `bump-callers.sh` that fan SHA bumps out to consumers.

## Conventions & gotchas

Expand Down
Loading