Skip to content

Stop passing SUBMODULES_TOKEN to top-level checkout token input - #443

Merged
d-morrison merged 2 commits into
mainfrom
claude/gha-442-i1s8rj
Aug 10, 2026
Merged

Stop passing SUBMODULES_TOKEN to top-level checkout token input#443
d-morrison merged 2 commits into
mainfrom
claude/gha-442-i1s8rj

Conversation

@d-morrison

Copy link
Copy Markdown
Collaborator

Fixes #442.

Problem

antigravity-code-review.yml and gemini-code-review.yml passed
${{ secrets.SUBMODULES_TOKEN || github.token }} as the token for checking
out the caller's own repository. SUBMODULES_TOKEN exists precisely
because a submodule lives under a different owner than the consumer, so a
token scoped to read the submodule has no reason to be able to read the
caller's own repo — and in ucdavis/bcs it could not: the main checkout
failed with fatal: could not read Username for 'https://github.com': terminal prompts disabled on every run (see the issue's observed logs).
The || fallback only fires when SUBMODULES_TOKEN is unset, so a repo
that configures it correctly for its own submodule is exactly the repo that
breaks.

claude-code-review.yml already gets this right: the caller's own repo is
checked out with the runner's default github.token, and SUBMODULES_TOKEN
is used only where a cross-owner submodule fetch actually needs it, via the
shared checkout-submodules composite action.

Grepping the whole repo for the same anti-pattern (token: ${{ secrets. SUBMODULES_TOKEN, anchored so it doesn't also match submodules-token:)
turned up a third site the issue didn't enumerate: gemini.yml (the
@gemini mention-handling agent workflow) carries the identical bug at its
own checkout step. Fixed it too, since it's the same defect, same one-line
mechanism, and the new lint below would otherwise fail on it.

Fix

All three workflows (antigravity-code-review.yml, gemini-code-review.yml,
gemini.yml) now:

  • Check out the caller's own repo with the runner's default token (no
    token: input on the top-level actions/checkout).
  • Authenticate the submodule fetch separately, via the shared
    .github/actions/checkout-submodules composite action (gated on
    inputs.checkout-submodules), matching claude-code-review.yml.

Guard

Per the issue's own "cheaper guard is a lint" suggestion, _selftest.yml
gains a lint-checkout-tokens job that greps every workflow file and fails
if any of them ever passes SUBMODULES_TOKEN to a top-level actions/checkout
token: input again. Verified it currently flags exactly the three sites
above (and only those) before the fix, and is clean after.

Not in scope

The issue's second/third comments raise two related but explicitly separate
concerns:

  • The random agent selector in ai-code-review.yml doesn't fall through to
    another agent when a dispatched one fails partway through (only when it
    can't be dispatched at all). ai-code-review.yml's own header comment
    already documents this as a known, deliberately deferred limitation
    ("tracked separately rather than papered over here"), so no change needed
    here.
  • The Gemini path being empirically confirmed broken (not just "affected by
    inspection") — already resolved by this same token fix, since it shares
    the identical root cause and checkout step.

Verification

  • python3 -c "import yaml; yaml.safe_load(open(f))" on all four edited
    workflow files — clean.
  • grep -rnE '^\s*token:\s*\$\{\{[^}]*SUBMODULES_TOKEN' .github/workflows/*.yml
    — three hits before the fix (the sites above), zero after.
  • NLB_BASE_REF=origin/main python3 check-new-line-breaks/check-new-line-breaks.py
    — clean against the committed diff.
  • Non-ASCII punctuation scan (LC_ALL=C.UTF-8 grep -P) over the committed
    diff's added lines — clean.
  • Diff's deleted lines read individually — all three deletions are exactly
    the submodules:/token: lines being replaced; nothing incidental.

Per CLAUDE.md's "Test changes against a template repo" and "A PR fixing
claude-code-review.yml (or claude.yml) itself can't self-verify before merge"
sections: this PR does not touch claude-code-review.yml/claude.yml
themselves, and doesn't need a template-repo run since it's a workflow-file
change with no new composite-action interface — the new lint-checkout-tokens
selftest job is the closest thing to end-to-end coverage this class of bug
gets. A live test against ucdavis/bcs (where the bug was originally
observed) is the strongest real-world confirmation, but that's outside this
repo's own CI.


Generated by Claude Code

claude added 2 commits August 10, 2026 01:24
antigravity-code-review.yml, gemini-code-review.yml, and gemini.yml all
passed `${{ secrets.SUBMODULES_TOKEN || github.token }}` as the token
for checking out the CALLER's own repo. SUBMODULES_TOKEN authenticates
a cross-owner submodule fetch, so a consumer that sets it correctly for
its own submodule gets a token with no read access to its own repo --
the main checkout then fails before the reviewer runs at all.

claude-code-review.yml already has this right: check out the caller
with the runner's default github.token, and authenticate the submodule
fetch separately through the shared checkout-submodules composite
action. All three workflows now follow that pattern.

Also adds a lint-checkout-tokens selftest job asserting no workflow in
this repo passes SUBMODULES_TOKEN to a top-level actions/checkout
token: input, per the issue's own suggested guard.

Fixes #442
@github-actions

Copy link
Copy Markdown
Contributor

Claude finished reviewView run


Review: #443 — "Stop passing SUBMODULES_TOKEN to top-level checkout token input"

Summary of the change: antigravity-code-review.yml, gemini-code-review.yml, and gemini.yml each had a top-level actions/checkout step passing token: ${{ secrets.SUBMODULES_TOKEN || github.token }} for checking out the caller's own repo — wrong, since SUBMODULES_TOKEN exists only to authenticate a cross-owner submodule fetch. This PR removes submodules:/token: from those top-level checkouts and adds a separate "Checkout submodules" step in each, calling the shared checkout-submodules@v2 composite with submodules-token: ${{ secrets.SUBMODULES_TOKEN }}, matching the pattern already shipped in claude-code-review.yml/claude.yml. It also adds a lint-checkout-tokens selftest job to guard against recurrence, plus a changelog fragment.

Review process: Four independent passes were run — two CLAUDE.md-compliance audits and two Opus bug/security scans — each fetching the diff directly and verifying claims against the live repo tree (reading checkout-submodules/action.yml, re-running the repo-wide grep for the anti-pattern, comparing structurally against claude-code-review.yml's established pattern, and checking the changelog fragment against the originating issue gha#442). All four converged on the same conclusion with no disagreement, so no findings needed step 5's adversarial validation pass.

What was checked and cleared:

  • The fix structurally matches claude-code-review.yml's existing checkout/checkout-submodules split exactly, including preserving each file's original if: guard on the new step.
  • Independent repo-wide greps (workflows, composite actions, examples) confirm exactly the three sites named were affected and all are now fixed; the new lint job's scope (.github/workflows/*.yml only) is correct since no composite action does a top-level self-checkout.
  • checkout-submodules composite input name (submodules-token) and ref (Morrison-Lab/gha/.github/actions/checkout-submodules@v2) are real, not hallucinated, and already an existing @v2-released composite (no first-caller bootstrapping issue).
  • set -euo pipefail + grep inside an if condition is handled correctly (no false failure on the clean/no-match case).
  • Dropping submodules: from the top-level checkout is equivalent to submodules: false (checkout's own default), so no accidental unauthenticated submodule fetch occurs before the dedicated step.
  • The composite's handling of an empty submodules-token (anonymous clone with a ::notice::) means public-submodule consumers who never set the secret aren't regressed; private cross-owner submodules never worked via the old || github.token fallback anyway (wrong token scope).
  • Changelog fragment follows the <slug>.<category>.md convention, its factual claims check out against issue gha#442, and it carries no AI-prose tells.
  • No new workflow_call input was added, so the three-site doc-sync rule doesn't apply here.
  • The pre-existing persist-credentials asymmetry between antigravity-code-review.yml (explicit false) and the two gemini* files (default true) predates this diff and isn't introduced by it; if anything the change narrows what gets persisted (job-scoped token instead of a cross-owner PAT).

No high-signal bugs, security issues, or clear CLAUDE.md violations were found. No inline comments to post.

Verdict

Ready for merge

@github-actions

Copy link
Copy Markdown
Contributor

💰 Cost: $6.2839 (review) — run

@d-morrison
d-morrison merged commit 2726b92 into main Aug 10, 2026
33 checks passed
@d-morrison
d-morrison deleted the claude/gha-442-i1s8rj branch August 10, 2026 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

antigravity/gemini review workflows use SUBMODULES_TOKEN to check out the caller's own repo, which fails for cross-owner consumers

2 participants