From 3793ca57d0332e1e182f3a79a0d120ce6598c422 Mon Sep 17 00:00:00 2001 From: Moshe Abramovitch Date: Mon, 13 Jul 2026 13:38:14 -0500 Subject: [PATCH 1/2] Add reviewed-external-contributor allowlist to verify-authors External contributions were mechanically blocked: verify-authors only accepts @nvidia.com or github-noreply emails, so a reviewed and approved external PR could not pass CI without the contributor rewriting their commits to a noreply address, which hides the real provenance instead of recording it. This adds .github/external-contributors.yml. After a maintainer runs the contribution review, they record the contributor's email scoped to specific PR numbers. verify-authors reads the allowlist from the BASE branch only, so a PR cannot allowlist itself, and accepts listed emails for their listed PRs. Seeds the list with the reviewed one-line docs fix in PR #76. Signed-off-by: Moshe Abramovitch --- .github/external-contributors.yml | 31 ++++++++++++++++++++ .github/workflows/verify-authors.yml | 42 ++++++++++++++++++++++++++-- 2 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 .github/external-contributors.yml diff --git a/.github/external-contributors.yml b/.github/external-contributors.yml new file mode 100644 index 00000000..993a16cd --- /dev/null +++ b/.github/external-contributors.yml @@ -0,0 +1,31 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright (c) 2026 NVIDIA Corporation. All rights reserved. +# +# Reviewed external contributors. +# +# The verify-authors check requires an @nvidia.com or github-noreply +# email on every commit. External (non-NVIDIA) contributions are +# accepted only after a maintainer-run IP review of the contribution. +# Once a contribution passes review, a maintainer records the +# contributor here, and the authors check accepts their email for the +# listed pull requests only. +# +# This file is always read from the target (base) branch by CI, so a +# pull request cannot allowlist itself. +# +# Entry fields: +# email commit author/committer email to accept +# github contributor's GitHub login +# prs pull request numbers this entry applies to (required — +# entries are scoped per contribution, never blanket) +# reviewed_by maintainer who ran the contribution review +# review_date date the review was completed (YYYY-MM-DD) +# notes one-line description of the contribution + +external_contributors: + - email: wenyuchiou12@gmail.com + github: WenyuChiou + prs: [76] + reviewed_by: mosheabr + review_date: "2026-07-13" + notes: One-line CONTRIBUTING.md anchor fix (dead README link). Review passed on all criteria. diff --git a/.github/workflows/verify-authors.yml b/.github/workflows/verify-authors.yml index 12546259..bd40ecb9 100644 --- a/.github/workflows/verify-authors.yml +++ b/.github/workflows/verify-authors.yml @@ -13,6 +13,13 @@ # # The automated/sync-skills branch is exempt — same reason as DCO: # it's the bot mirror, not a contributor. +# +# Reviewed external contributions are accepted via +# .github/external-contributors.yml: a maintainer runs the contribution +# review, records the contributor's email scoped to specific PR +# numbers, and this check then accepts that email for those PRs only. +# The allowlist is read from the BASE branch, never the PR head, so a +# pull request cannot allowlist itself. name: Verify Authors @@ -38,6 +45,7 @@ jobs: BASE_SHA: ${{ github.event.pull_request.base.sha }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} HEAD_REF: ${{ github.event.pull_request.head.ref }} + PR_NUMBER: ${{ github.event.pull_request.number }} run: | set -euo pipefail @@ -54,12 +62,40 @@ jobs: # NVIDIA-org GitHub user) allowed='@(nvidia\.com|users\.noreply\.github\.com)$' + # Reviewed-external allowlist, read from the BASE branch so a + # PR cannot allowlist itself. Produces "email" lines for + # entries whose prs list contains this PR's number. + reviewed_externals=$(git show "$BASE_SHA:.github/external-contributors.yml" 2>/dev/null \ + | python3 -c ' + import os, sys, yaml + try: + doc = yaml.safe_load(sys.stdin) or {} + except yaml.YAMLError: + sys.exit(0) # unparsable allowlist grants nothing + pr = int(os.environ["PR_NUMBER"]) + for entry in doc.get("external_contributors") or []: + email = str(entry.get("email", "")).strip().lower() + prs = entry.get("prs") or [] + if email and isinstance(prs, list) and pr in prs: + print(email) + ' || true) + + is_reviewed_external() { + local email_lc + email_lc=$(echo "$1" | tr '[:upper:]' '[:lower:]') + [ -n "$reviewed_externals" ] && echo "$reviewed_externals" | grep -qxF "$email_lc" + } + violations=() while read -r sha; do author_email=$(git show --format='%ae' -s "$sha") committer_email=$(git show --format='%ce' -s "$sha") for email in "$author_email" "$committer_email"; do if ! echo "$email" | grep -qE "$allowed"; then + if is_reviewed_external "$email"; then + echo "Accepting reviewed external contributor email $email for PR #$PR_NUMBER (see .github/external-contributors.yml)." + continue + fi subject=$(git show --format='%s' -s "$sha") violations+=("${sha:0:7} $email $subject") fi @@ -86,8 +122,10 @@ jobs: echo " origin/main && git push --force-with-lease" echo "" echo "If you are not an NVIDIA employee, please contact a CODEOWNER." - echo "External contributions to catalog content require explicit OSRB" - echo "review before merge and cannot be self-served." + echo "External contributions are accepted after a maintainer-run" + echo "review; once cleared, a maintainer records your email in" + echo ".github/external-contributors.yml scoped to your PR and this" + echo "check will accept it. This cannot be self-served." exit 1 fi From 5c2088f222e880a94dfc47376de062723ba84838 Mon Sep 17 00:00:00 2001 From: Moshe Abramovitch Date: Tue, 14 Jul 2026 10:33:33 -0500 Subject: [PATCH 2/2] Address review: fail loudly on parser issues, read allowlist from base tip Per review feedback on #343: - Preflight the YAML parser and make an unparsable allowlist fail the job with a visible error, instead of silently granting nothing. - Read the allowlist from the tip of the base branch rather than the event's base SHA, so a re-run picks up entries merged after the PR was opened. Same security property: still base-branch content only. - Note in the allowlist header that per-entry flow is a deliberate v1. Signed-off-by: Moshe Abramovitch --- .github/external-contributors.yml | 9 +++++-- .github/workflows/verify-authors.yml | 35 ++++++++++++++++++++-------- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/.github/external-contributors.yml b/.github/external-contributors.yml index 993a16cd..1fd757bd 100644 --- a/.github/external-contributors.yml +++ b/.github/external-contributors.yml @@ -10,8 +10,13 @@ # contributor here, and the authors check accepts their email for the # listed pull requests only. # -# This file is always read from the target (base) branch by CI, so a -# pull request cannot allowlist itself. +# This file is always read from the tip of the target (base) branch by +# CI, so a pull request cannot allowlist itself. After merging a new +# entry, re-run the authors check on the contributor's PR to pick it up. +# +# This per-entry flow is a deliberate v1 while external contributions +# are rare. If volume grows, revisit with a lower-toil mechanism (e.g., +# a maintainer-applied label) without weakening the review requirement. # # Entry fields: # email commit author/committer email to accept diff --git a/.github/workflows/verify-authors.yml b/.github/workflows/verify-authors.yml index bd40ecb9..7b69e49e 100644 --- a/.github/workflows/verify-authors.yml +++ b/.github/workflows/verify-authors.yml @@ -18,8 +18,9 @@ # .github/external-contributors.yml: a maintainer runs the contribution # review, records the contributor's email scoped to specific PR # numbers, and this check then accepts that email for those PRs only. -# The allowlist is read from the BASE branch, never the PR head, so a -# pull request cannot allowlist itself. +# The allowlist is read from the tip of the BASE branch, never the PR +# head, so a pull request cannot allowlist itself — and a re-run picks +# up entries merged after the PR was opened. name: Verify Authors @@ -43,6 +44,7 @@ jobs: - name: Verify all commit author emails are NVIDIA-affiliated env: BASE_SHA: ${{ github.event.pull_request.base.sha }} + BASE_REF: ${{ github.event.pull_request.base.ref }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} HEAD_REF: ${{ github.event.pull_request.head.ref }} PR_NUMBER: ${{ github.event.pull_request.number }} @@ -62,23 +64,36 @@ jobs: # NVIDIA-org GitHub user) allowed='@(nvidia\.com|users\.noreply\.github\.com)$' - # Reviewed-external allowlist, read from the BASE branch so a - # PR cannot allowlist itself. Produces "email" lines for - # entries whose prs list contains this PR's number. - reviewed_externals=$(git show "$BASE_SHA:.github/external-contributors.yml" 2>/dev/null \ - | python3 -c ' + # The allowlist logic depends on the YAML parser. Fail loudly + # if it is unavailable — otherwise reviewed externals would be + # silently rejected with no clue why. + python3 -c 'import yaml' + + # Reviewed-external allowlist, read from the TIP of the base + # branch so a PR cannot allowlist itself, and so a re-run + # picks up entries merged after the PR was opened. Produces + # "email" lines for entries whose prs list contains this PR's + # number. An unparsable allowlist fails the job loudly. + allowlist_ref="origin/$BASE_REF" + if git cat-file -e "$allowlist_ref:.github/external-contributors.yml" 2>/dev/null; then + reviewed_externals=$(git show "$allowlist_ref:.github/external-contributors.yml" \ + | python3 -c ' import os, sys, yaml try: doc = yaml.safe_load(sys.stdin) or {} - except yaml.YAMLError: - sys.exit(0) # unparsable allowlist grants nothing + except yaml.YAMLError as exc: + print(f"::error::.github/external-contributors.yml on the base branch is unparsable: {exc}", file=sys.stderr) + sys.exit(1) pr = int(os.environ["PR_NUMBER"]) for entry in doc.get("external_contributors") or []: email = str(entry.get("email", "")).strip().lower() prs = entry.get("prs") or [] if email and isinstance(prs, list) and pr in prs: print(email) - ' || true) + ') + else + reviewed_externals="" + fi is_reviewed_external() { local email_lc