Skip to content

Verification reads a commit's Actions workflow runs, not its check runs - #914

Closed
mkreyman wants to merge 3 commits into
masterfrom
fix/913-verification-actions-runs
Closed

mkreyman wants to merge 3 commits into
masterfrom
fix/913-verification-actions-runs

Conversation

@mkreyman

Copy link
Copy Markdown
Owner

Closes #913.

Why

Story verification read commits/:sha/check-runs, which needs the Checks permission. GitHub grants Checks to Apps only, so the fine-grained GITHUB_TOKEN now set on production cannot hold it. Every verification CI lookup on a private repository returned 403, measured on infra and home_care_billing.

What

  • get_status/2 now reads actions/runs?head_sha=, which needs actions: read, a permission the token already holds.
  • The summary had two latent defects, fixed here:
    • An empty run list read as success, because Enum.all?/2 of nothing is true. A commit whose CI had not started was verified.
    • A cancelled, timed-out or skipped run read as in_progress indefinitely. It now fails.
  • Only the newest run of each workflow per event counts, so a green re-run supersedes a failure.
  • A Req.Test seam (:verification_github_req_plug in config/test.exs, the same shape as the pull-request source's seam) makes the endpoint, the head_sha parameter and the response key testable.
  • Docs: in FLY_SECRETS.md, the GITHUB_TOKEN row now asks for actions read, not checks read. The chokepoint-scan justification is reworded.

Checks

  • Commit gate: 11948 tests, 0 failures.
  • 8 mutations run with bin/mutate.sh, each exit 0 (caught): the empty-list guard, the not-completed guard, the newest-run dedupe, the per-event dedupe, skipped counted as a pass, the check-runs endpoint, a dropped head_sha, and the check_runs body key.

The review gate is running.

…ns (#913)

GitHub offers the Checks permission to Apps only, so the fine-grained GITHUB_TOKEN now on
production cannot hold it, and every story-verification CI lookup on a private repository
403'd (measured on infra and home_care_billing). get_status now reads
actions/runs?head_sha=, which needs actions: read.

The summary had two latent defects, fixed with it:
- an empty run list read as SUCCESS (Enum.all? of nothing), so a commit whose CI had not
  started was verified;
- a cancelled, timed-out or skipped run read as in_progress for ever.
Only the newest run of each workflow per event counts, so a green re-run supersedes a
failure.

A Req.Test seam (config :verification_github_req_plug) lets the endpoint, the head_sha
parameter and the response key be tested.

Mutations, each run with bin/mutate.sh against github_actions_test.exs, all exit 0:
empty-list guard, not-completed guard, newest-run dedupe, per-event dedupe, skipped-as-pass,
check-runs endpoint, dropped head_sha, check_runs body key.

FLY_SECRETS.md: GITHUB_TOKEN needs actions (not checks) read.
…-run reader

Most findings had one cause: verification got its own, weaker reader of a commit's workflow
runs instead of the merge gate's hardened one. GitHubPullRequestSource.commit_ci_runs/2 now
reads them with the same request, shape check and truncation refusal as check_evidence/3,
keeps only push and pull_request runs of the exact commit, and returns the newest run per
workflow, event and branch (a failure on any branch the commit was pushed to counts).
GitHubActions keeps only the verdict, and its request plumbing, Req.Test seam and egress
allowlist entry are gone.

Verdict: no runs, or only skipped/neutral runs, is no evidence (the worker falls back to
local re-execution) instead of an unbounded wait; a cancelled run is no evidence rather
than a failure; any other non-success fails and carries that run's URL.

Docs: the FLY_SECRETS GITHUB_TOKEN row no longer says actions read is thread-only; a
CHANGELOG entry tells operators the token needs actions read; egress-guard.md and the
allowlist describe the shared reader.

Mutations, each with bin/mutate.sh, all exit 0: empty guard, not-completed guard, skipped
and neutral exclusion, cancelled as no evidence, cancelled not counted as failure, failure
URL, event filter, sha filter, per-branch grouping, newest-by-id, truncation refusal, and
the sha passed through get_status.
…d the worker keeps the reason

Round 1's shared reader asked for every run of the commit and filtered by event only after
the truncation check, so a busy schedule history could make a commit unverifiable; it also
counted tag pushes (release workflows) as CI and let a newer cancelled run hide an older
success. commit_ci_runs now reads push and pull_request separately, drops runs of a pushed
TAG (one ref read per distinct pushed name, bounded), and keeps per workflow, event and
branch the newest run that was not cancelled. newest_per_workflow and the verification
path share one newest_per rule.

The verdict treats a run waiting on an environment approval as no evidence rather than an
endless wait. The worker snoozes out a rate limit instead of cloning, records why CI gave
no verdict (ci_unavailable_reason) and the failing run's url. It also casts the story and
tenant ids in its schemaless query: the uncast string crashed every run that had a commit
before it reached CI. The repo URL parser keeps dots in a repository name.

Docs: the module's call list and ceiling, the allowlist justification's attribution, and
the CHANGELOG entry.

Mutations, each with bin/mutate.sh, all exit 0 (23): empty guard, not-completed guard,
waiting not in progress, waiting as no evidence, skipped/neutral exclusion, cancelled as no
evidence, cancelled not a failure, failure URL, dotted repo name, sha passed through,
sha filter, local event filter, event query parameter, per-branch grouping, newest by id,
prefer uncancelled, tag drop, tag-drop wiring, truncation refusal, worker URL, rate-limit
snooze, recorded reason, UUID cast.
@mkreyman

Copy link
Copy Markdown
Owner Author

Superseded by #915: the third review round still found material defects, so the change was rewritten per the review-round rule (see #915's body).

@mkreyman mkreyman closed this Sep 28, 2026
mkreyman added a commit that referenced this pull request Sep 28, 2026
…thout evidence, bound the wait

- Only runs the commit's own push or pull_request triggered count; a deploy, schedule or
  dispatch run on the same commit neither passes nor fails it.
- Only success passes. skipped, neutral and cancelled are no evidence either way (a
  skipped run ran no tests; cancel-in-progress cancels a superseded commit's run). No
  counting run is {:error, :no_workflow_runs}, not in progress.
- The newest run per workflow is the highest id. A body of another shape is
  unreadable_workflow_runs, not a crash. Req's transient retry is back.
- The worker logs the reason a lookup gave no verdict (a token without actions: read
  would otherwise be a silent local run), and ends a run whose CI is still unfinished
  past verification_max_run_age_seconds as error ci_unfinished instead of snoozing for
  ever. The clock is inserted_at: every snooze restarts the run and resets started_at.
- Found while testing that: the worker's project lookup passed string UUIDs to a
  schemaless query, which Postgrex refused, so no started run ever reached the CI read.
  The ids are now cast.
- The egress allowlist justification, docs/egress-guard.md and the GITHUB_TOKEN row
  describe the Actions read; the row no longer implies a pr-mode deployment can skip
  actions: read.

Declined: reusing Loopctl.Delivery.CiEvidence. It judges named required checks against
a thread's intake source and branch; verification holds only a repo URL and a sha, and
wiring it to the intake source is what grew #914/#915.

Mutations (bin/mutate.sh, all exit 0): endpoint back to check-runs; event filter
removed; no-evidence drop removed; cancelled counted; empty read as in progress; oldest
run chosen; truncation never refusing; shape check removed; the UUID cast removed (all
three worker CI tests fail as on master); the wait bound removed; the reason log
removed.
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.

Story verification CI lookup 403s on private repos: check-runs needs checks:read, which fine-grained PATs cannot have

1 participant