Skip to content

Commit bb4e344

Browse files
committed
ci: grant checks/statuses, fail closed on an absent rollup, widen push paths
Four fixes from the ninth review on #844. The first is a fail-open hole. Grant checks: read and statuses: read. statusCheckRollup is the gate both the workflow and the merge script depend on, and it is built from Checks resources and commit statuses — neither of which a permissions block naming only contents, pull-requests and actions grants. Either GraphQL errors, making the workflow a permanent no-op that looks healthy, or it returns the field empty, which `[]?` would have swallowed as "all checks green" and merged past a red Commit Lint. That is the exact check #799 shows going red on a Dependabot pull request. Belt and braces on the same hole: the script now refuses outright when statusCheckRollup is not an array, so neither an absent permission nor a head with no checks yet can read as success. It used to merge in that case; it now skips and lets the sweep look again. The drift guard read only the plural `directories`. The singular `directory` is equally valid and is the canonical form for one directory, so an entry using it contributed nothing and its manifest was reported as unclaimed — validate failing on correct config, with a message telling the author to add an entry already in the file. Both spellings are read now. The drift guard also asserts the two keys the grouping actually rests on. `applies-to: security-updates` is load-bearing because plain groups batch version updates only, and those are off via the limit; a 48th entry copy-pasted without it would pass every check here while its advisories reverted to one pull request each. A missing or non-zero open-pull-requests-limit is the mirror image. With 47 near-identical entries these are exactly the copy-paste omissions worth machine checking. Widen the push trigger to every input the selector calls shared. It listed only src/**, so layer/**, Cargo.toml and Cargo.lock were verified against the examples on no event at all — and layer/bootstrap is the code path all eight zip examples boot, since build-layer copies it into the artifact each of them injects. The comment claiming those changes are covered on push to main is now true. Pull request triggers are unchanged, so source pull requests still do not fan out to eighteen jobs. Twenty merge-script paths re-run with expected exit codes, zero mismatches, plus the drift guard against a singular-directory entry, a missing applies-to, a non-zero limit, and the clean tree.
1 parent b21954d commit bb4e344

4 files changed

Lines changed: 55 additions & 9 deletions

File tree

.github/scripts/check-example-config.sh

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,38 @@ for path in tracked:
7979
found.add((ecosystem, directory))
8080
8181
configured = set()
82+
problems = []
8283
config = yaml.safe_load(open(dependabot_path))
8384
for update in config["updates"]:
84-
for directory in update.get("directories", []):
85-
configured.add((update["package-ecosystem"], directory))
86-
87-
problems = []
85+
ecosystem = update["package-ecosystem"]
86+
87+
# Both spellings are valid Dependabot config. This file uses the plural throughout,
88+
# but the singular is the canonical form for one directory and is what someone
89+
# adding an entry is likely to reach for — reading only the plural would report
90+
# their manifest as unclaimed and tell them to add an entry that is already there.
91+
directories = list(update.get("directories") or [])
92+
if "directory" in update:
93+
directories.append(update["directory"])
94+
for directory in directories:
95+
configured.add((ecosystem, directory))
96+
97+
where = f"{ecosystem} {directories}"
98+
99+
# Both keys below are load-bearing, and an entry copy-pasted without either looks
100+
# correct here while silently reverting that example to what this file exists to
101+
# prevent. Plain `groups` batches version updates only, so without
102+
# `applies-to: security-updates` the grouping does not apply to the advisories that
103+
# are the whole point.
104+
groups = update.get("groups") or {}
105+
if not any(g.get("applies-to") == "security-updates" for g in groups.values()):
106+
problems.append(f"{where}: needs a group with `applies-to: security-updates`, "
107+
"or its security updates arrive one pull request per advisory.")
108+
109+
# And a missing or non-zero limit turns routine version bumps back on for that one
110+
# example.
111+
if update.get("open-pull-requests-limit") != 0:
112+
problems.append(f"{where}: needs `open-pull-requests-limit: 0`, or version "
113+
"updates come back on for it.")
88114
89115
unclaimed = sorted(found - configured)
90116
if unclaimed:

.github/scripts/dependabot-automerge.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,12 @@ head_sha=$(jq -r '.headRefOid' <<<"$pr_json")
8888
# yet — a real few-second window every time Dependabot force-pushes a rebase — and
8989
# iterating null aborts jq. The sweep's pre-filter already tolerates it, which is what
9090
# makes the combination reachable: it would pass such a pull request straight to here.
91+
# An absent rollup must never read as "everything passed": that is the shape a missing
92+
# checks/statuses permission would produce, and `[]?` alone would swallow it and merge.
93+
if [[ "$(jq -r '.statusCheckRollup | type' <<<"$pr_json")" != "array" ]]; then
94+
skip "no check rollup available for $head_sha (yet)."
95+
fi
96+
9197
not_green=$(jq -r '
9298
.statusCheckRollup[]?
9399
| select((.workflowName // "") != "Dependabot Auto-merge")

.github/workflows/dependabot-automerge.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,14 @@ permissions:
5252
# permissions block sets every scope not named here to none, so without this the
5353
# Actions API returns 403. Same reason commitlint-comment.yaml declares it.
5454
actions: read
55+
# statusCheckRollup is the gate both this file and dependabot-automerge.sh depend on,
56+
# and it is made of Checks resources (CheckRun nodes) and commit statuses
57+
# (StatusContext nodes). Without these two the query either errors — making the whole
58+
# workflow a no-op that looks healthy — or comes back empty, which would read as "all
59+
# checks green" and merge past a red Commit Lint. The script also refuses an absent
60+
# rollup outright, so neither failure can be mistaken for success.
61+
checks: read
62+
statuses: read
5563

5664
jobs:
5765
merge:

.github/workflows/examples.yaml

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,19 @@ on:
1818
push:
1919
branches:
2020
- main
21+
# Every input select-examples.sh treats as shared, not just src/. layer/bootstrap in
22+
# particular is the code path all eight zip examples boot — build-layer copies it
23+
# into the artifact each of them injects — so leaving it out meant a regression there
24+
# was verified by nothing at all.
2125
paths:
2226
- "src/**"
23-
# Note that src/, layer/ and Cargo.* are deliberately NOT pull request triggers,
24-
# even though select-examples.sh treats them as shared: adding them would run all
25-
# eighteen matrix entries on every source pull request. Adapter changes are verified
26-
# against the examples on push to main (above), and the selector's shared-path rule
27-
# still applies to a pull request that touches both source and examples.
27+
- "layer/**"
28+
- "Cargo.toml"
29+
- "Cargo.lock"
30+
# Those same paths are deliberately NOT pull request triggers: adding them would run
31+
# all eighteen matrix entries on every source pull request. Adapter changes are
32+
# verified against the examples on push to main (above), and the selector's shared-path
33+
# rule still applies to a pull request that touches both source and examples.
2834
workflow_dispatch:
2935

3036
permissions:

0 commit comments

Comments
 (0)