Skip to content

Commit 906eb70

Browse files
committed
feat(ci): gate merges on maintainer approval via a silent status check
Adds the OpenShell / Maintainer Approval status check. It passes only when someone listed in MAINTAINERS.md has approved the pull request, and it notifies no one: a required status check is the only native primitive that enforces who must approve without requesting a review from them. The job runs on pull_request_review and merge_group with read-only permissions and posts nothing anywhere; its exit code is the check result. Both MAINTAINERS.md and the decision helper are read from main via a sparse checkout, never from the pull request ref, so a contributor cannot add themselves to the list or rewrite the decision logic and self-approve. Because pull_request_review runs the pull request's own copy of the workflow file, CODEOWNERS now covers the whole enforcement surface -- workflows, actions, CODEOWNERS itself, the zizmor config, MAINTAINERS.md, and the enforcement scripts -- naming the outside collaborators explicitly alongside the team, since org policy keeps them off GitHub teams. The ruleset's native code owner requirement is what blocks a merge that disables the gate, regardless of what the tampered check reports. Also adds a change-alert workflow that comments the approver-set delta on pull requests touching MAINTAINERS.md, and documents both in CONTRIBUTING. Signed-off-by: Jim Meyer <jimeyer@nvidia.com>
1 parent 69a6718 commit 906eb70

5 files changed

Lines changed: 219 additions & 2 deletions

File tree

‎.github/CODEOWNERS‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,35 @@
1-
# Broad ownership — core team reviews everything
1+
## Broad ownership — core team reviews everything
22
* @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
33

4-
# Vouch list — maintainers only (bot commits bypass, but manual edits need review)
4+
## Vouch list — maintainers only (bot commits bypass, but manual edits need review)
55
.github/VOUCHED.td @NVIDIA/openshell-codeowners
6+
7+
## Merge enforcement — these paths decide who can merge, so they notify the full
8+
# list of maintainers to raise scrutiny on changes here.
9+
#
10+
# PR checks run local to the branch, so unless the check specifically avoids
11+
# using branch-local files, a PR could change its own gates and appear to pass
12+
# them. Covering the required checks here closes that gap.
13+
#
14+
# The whole workflows directory is covered, not just the approval gate, because
15+
# any workflow can claim a required check context or ask for a broader token.
16+
#
17+
# Normally, we'd use a GitHub team to cover all maintainers, but org-level
18+
# policies prevent outside collaborators from being on a GH team at this time
19+
# so we spell them out explicitly.
20+
21+
# Core check mechanisms
22+
/.github/workflows/ @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
23+
/.github/actions/ @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
24+
/.github/CODEOWNERS @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
25+
/.github/zizmor.yml @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
26+
27+
# `MAINTAINERS.md` maintainer approval checks
28+
/MAINTAINERS.md @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
29+
/tasks/scripts/check_maintainer_approval.py @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
30+
/tasks/scripts/alert_maintainer_change.py @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
31+
/tasks/scripts/check_maintainer_approval_test.py @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
32+
/tasks/scripts/alert_maintainer_change_test.py @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
33+
34+
# Auditable SBOM check
35+
/tasks/scripts/verify-image-sbom.sh @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
4+
name: Maintainer Approval
5+
6+
on:
7+
merge_group:
8+
types: [checks_requested]
9+
pull_request_review:
10+
types: [submitted, dismissed]
11+
12+
permissions:
13+
contents: read
14+
pull-requests: read
15+
16+
concurrency:
17+
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
18+
cancel-in-progress: true
19+
20+
jobs:
21+
maintainer-approval:
22+
# This name is the required status check context. Changing it silently
23+
# breaks the ruleset entry that gates merges on this job.
24+
name: OpenShell / Maintainer Approval
25+
if: github.repository_owner == 'NVIDIA'
26+
runs-on: ubuntu-latest
27+
timeout-minutes: 5
28+
steps:
29+
# Check out the default branch, never the pull request head. Both the
30+
# maintainer list and the decision helper must come from main: reading
31+
# either from the pull request ref would let a contributor add themselves
32+
# to the list, or rewrite the decision logic, and self-approve.
33+
- name: Check out the maintainer list and helper
34+
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
35+
with:
36+
ref: main
37+
sparse-checkout: |
38+
MAINTAINERS.md
39+
tasks/scripts/check_maintainer_approval.py
40+
sparse-checkout-cone-mode: false
41+
persist-credentials: false
42+
43+
- name: Require an approving review from a maintainer
44+
env:
45+
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
46+
GH_REPO: ${{ github.repository }}
47+
PR_NUMBER: ${{ github.event.pull_request.number }}
48+
# merge_group carries no pull request in its payload, but the queue
49+
# branch names the entry: gh-readonly-queue/main/pr-3027-<base sha>.
50+
MERGE_GROUP_REF: ${{ github.ref_name }}
51+
shell: bash
52+
run: |
53+
set -euo pipefail
54+
55+
if [ -z "${PR_NUMBER:-}" ]; then
56+
if [[ "$MERGE_GROUP_REF" =~ /pr-([0-9]+)-[0-9a-f]+$ ]]; then
57+
PR_NUMBER="${BASH_REMATCH[1]}"
58+
else
59+
# Fail closed: an unrecognised ref must never satisfy the gate.
60+
echo "::error::No pull request resolved from '$MERGE_GROUP_REF'."
61+
exit 1
62+
fi
63+
fi
64+
65+
gh api --paginate "repos/$GH_REPO/pulls/$PR_NUMBER/reviews" --jq '.[]' \
66+
| jq -s '.' > reviews.json
67+
68+
# Exits non-zero when no maintainer's latest decisive review is an
69+
# approval, and when MAINTAINERS.md yields no handles. That exit code
70+
# is the check result; nothing is posted anywhere.
71+
python3 tasks/scripts/check_maintainer_approval.py \
72+
--maintainers MAINTAINERS.md \
73+
--reviews reviews.json
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
4+
name: Maintainers Change Alert
5+
6+
on:
7+
pull_request_target:
8+
types: [opened, reopened, synchronize]
9+
paths:
10+
- MAINTAINERS.md
11+
12+
permissions:
13+
contents: read
14+
pull-requests: write
15+
16+
concurrency:
17+
group: ${{ github.workflow }}-${{ github.event.pull_request.number }}
18+
cancel-in-progress: true
19+
20+
jobs:
21+
describe-change:
22+
name: Comment on the approver set change if MAINTAINERS.md has changed
23+
if: github.repository_owner == 'NVIDIA'
24+
runs-on: ubuntu-latest
25+
timeout-minutes: 10
26+
steps:
27+
# Default branch only. The helper must be the reviewed version, not
28+
# whatever the pull request happens to contain.
29+
- name: Check out the change-alert helper
30+
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
31+
with:
32+
ref: main
33+
sparse-checkout: tasks/scripts/alert_maintainer_change.py
34+
sparse-checkout-cone-mode: false
35+
persist-credentials: false
36+
37+
- name: Post the maintainer delta
38+
env:
39+
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
40+
GH_REPO: ${{ github.repository }}
41+
PR_NUMBER: ${{ github.event.pull_request.number }}
42+
BASE_SHA: ${{ github.event.pull_request.base.sha }}
43+
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
44+
# Single source of truth for the comment marker: the script emits
45+
# it as the first line of the body, the lookup below matches on it.
46+
COMMENT_MARKER: "<!-- maintainer-approval-delta -->"
47+
shell: bash
48+
run: |
49+
set -euo pipefail
50+
51+
# Fetching file contents is reading data, not executing it. The head
52+
# revision is never checked out or run.
53+
#
54+
# A 404 means the file genuinely does not exist at that revision — a
55+
# pull request that adds or deletes MAINTAINERS.md — and yields an
56+
# empty side of the comparison. Every other failure is fatal: an empty
57+
# file from a rate limit or a 5xx would render as "every maintainer
58+
# was just added" or "nothing changed", both of which mislead the
59+
# reviewer about who can merge code.
60+
fetch_maintainers() {
61+
local ref="$1" out="$2" err
62+
err="$(mktemp)"
63+
if gh api -H "Accept: application/vnd.github.raw" \
64+
"repos/$GH_REPO/contents/MAINTAINERS.md?ref=$ref" > "$out" 2>"$err"; then
65+
rm -f "$err"
66+
return 0
67+
fi
68+
if grep -q 'HTTP 404' "$err"; then
69+
rm -f "$err"
70+
: > "$out"
71+
return 0
72+
fi
73+
echo "::error::Could not fetch MAINTAINERS.md at $ref"
74+
cat "$err" >&2
75+
rm -f "$err"
76+
return 1
77+
}
78+
79+
fetch_maintainers "$BASE_SHA" before.md
80+
fetch_maintainers "$HEAD_SHA" after.md
81+
82+
# An unparseable result exits non-zero, but the comment explaining
83+
# why still has to be posted before this job fails.
84+
status=0
85+
python3 tasks/scripts/alert_maintainer_change.py \
86+
--before before.md --after after.md > body.md || status=$?
87+
88+
# No output means the approver set did not change. A comment saying
89+
# so is noise, so post nothing.
90+
if [ -s body.md ]; then
91+
cat body.md >> "$GITHUB_STEP_SUMMARY"
92+
93+
# Update the existing comment rather than stacking one per push.
94+
COMMENT_ID=$(gh api --paginate "repos/$GH_REPO/issues/$PR_NUMBER/comments" \
95+
--jq '.[] | select(.body | startswith($ENV.COMMENT_MARKER)) | .id' \
96+
| head -n 1)
97+
98+
if [ -n "$COMMENT_ID" ]; then
99+
gh api --method PATCH "repos/$GH_REPO/issues/comments/$COMMENT_ID" \
100+
-F "body=@body.md" >/dev/null
101+
else
102+
gh api --method POST "repos/$GH_REPO/issues/$PR_NUMBER/comments" \
103+
-F "body=@body.md" >/dev/null
104+
fi
105+
fi
106+
107+
exit "$status"

‎.github/zizmor.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ rules:
88
# head code. Keep each suppression scoped to its reviewed trigger block.
99
- dco.yml:3
1010
- e2e-label-help.yml:13
11+
- maintainers-change-alert.yml:6
1112
- release-canary.yml:3
1213
- required-ci-gates.yml:3
1314
- vouch-check.yml:3

‎CONTRIBUTING.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,12 @@ Do not start substantial issue-backed work until a maintainer has accepted the i
6868

6969
Use agents and the repository skills as needed to understand the affected code, evaluate tradeoffs, implement the smallest coherent change, and verify it. The pull request should explain what changed and how it was tested; it should not substitute an agent transcript for the contributor's understanding.
7070

71+
Every pull request must be approved by someone listed in [MAINTAINERS.md](MAINTAINERS.md) before it can merge. This is enforced by the `OpenShell / Maintainer Approval` status check, which turns green once one of those reviewers approves. Reviews from other contributors are welcome and count toward the general approval requirement, but they do not satisfy this check.
72+
73+
Pull requests that touch the enforcement machinery itself — anything under `.github/workflows/`, `.github/actions/`, `MAINTAINERS.md`, `.github/CODEOWNERS`, or the enforcement scripts in `tasks/scripts/` — additionally require a code owner's approval, listed in [.github/CODEOWNERS](.github/CODEOWNERS). The status check reads its inputs from `main`, but a workflow triggered by a review runs the pull request's own copy of the workflow file, so GitHub's native code owner requirement is what keeps a change from disabling the gate that would have blocked it.
74+
75+
Maintainers are not requested automatically. If your pull request has been idle, ask for a reviewer in the pull request or in the CNCF Slack channel rather than waiting.
76+
7177
## Agent Skills
7278

7379
OpenShell keeps skills for using the product separate from skills for developing the repository.

0 commit comments

Comments
 (0)