Gate --pr recheck on trusted author + requester

- Add --requester and --allow-untrusted-author flags to the review CLI,
  plumbed through greenlight-review.yml as the `requester` workflow input
- Add github_client.get_pr_author to resolve a single PR's author for
  the --pr scan path
- Enforce two authz gates in review.run(): reject untrusted requesters
  before any network work, and refuse --pr unless the target PR's author
  is trusted (both case-insensitive, clean exit 0 on refusal)
- Add --allow-untrusted-author as a LOCAL-ONLY bypass of the target-author
  gate, never exposed as a workflow input and never affecting the requester
- Document the @greenlight recheck flow in README/CHEATSHEET; cover all
  gate combinations with new tests

Backs the @greenlight recheck feature: a thin pytorch/pytorch trigger
(deployed separately) dispatches greenlight-review.yml with the PR number
and commenter login. Since --pr names an arbitrary PR (unlike the listing
scan, which is already trusted-author-only), the scan must independently
verify the target PR's author is trusted, or greenlight could be coaxed
into reviewing/approving any PR on request. The core guard: a trusted
requester still cannot bypass the target-author gate. resolve_authorized
(merge-rules work) runs only after both gates pass, so a spammed recheck
from an untrusted login costs nothing. The recheck hint is not yet
advertised in the PR status comment; that lands with the trigger.

Signed-off-by: Jean Schmidt <contato@jschmidt.me>

Author: jeanschmidt

.github/workflows/greenlight-review.yml

@@ -12,6 +12,11 @@ on:
         required: false
         default: ""
         type: string
+      requester:
+        description: "Login that requested this review (@greenlight recheck); must be a trusted author"
+        required: false
+        default: ""
+        type: string
       max:
         description: "Max dispatches this run (empty = no cap)"
         required: false
@@ -86,6 +91,7 @@ jobs:
           CLICKHOUSE_USERNAME: ${{ secrets.CLICKHOUSE_HUD_USER_USERNAME }}
           CLICKHOUSE_PASSWORD: ${{ secrets.CLICKHOUSE_HUD_USER_PASSWORD }}
           IN_PR: ${{ inputs.pr }}
+          IN_REQUESTER: ${{ inputs.requester }}
           IN_MAX: ${{ inputs.max }}
           IN_REF: ${{ inputs.ref }}
           IN_TIMEOUT: ${{ inputs.timeout_minutes }}
@@ -94,6 +100,7 @@ jobs:
           set -euo pipefail
           args=()
           if [ -n "$IN_PR" ]; then args+=(--pr "$IN_PR"); fi
+          if [ -n "$IN_REQUESTER" ]; then args+=(--requester "$IN_REQUESTER"); fi
           if [ -n "$IN_MAX" ]; then args+=(--max "$IN_MAX"); fi
           args+=(--ref "$IN_REF" --timeout-minutes "$IN_TIMEOUT" --log-level "$IN_LOG_LEVEL")
           just review "${args[@]}"

greenlight/CHEATSHEET.md

@@ -39,10 +39,12 @@ pytest, yamllint) into `.venv`.
 ```bash
 just review                          # one scan + dispatch iteration, then exit
 just run <args>                      # pass arbitrary args to the greenlight CLI (review is a shortcut)
-just review --pr 123                 # restrict the scan to PR #123
+just review --pr 123                 # scan only PR #123 (its author must be trusted)
+just review --pr 123 --requester alice  # recheck PR #123 for alice (author and alice must be trusted)
 just review --max 5                  # cap this iteration at 5 dispatches
 just review --ref my-branch          # dispatch the reviewer workflow at this test-infra ref (default main)
 just review --timeout-minutes 60     # re-dispatch an in-flight review after 60 min (default 45)
+just review --pr 123 --allow-untrusted-author  # LOCAL ONLY: skip the --pr author check
 ```
 
 `just review` scans the trusted authors' open PRs and, for each PR that is new or changed
@@ -63,6 +65,13 @@ scanner lets a running review finish (or time out and record a verdict) before i
 re-dispatches, rather than cancelling and restarting one that is still running; lower
 `--timeout-minutes` in the deployment if you need a stuck review reclaimed sooner.
 
+`@greenlight recheck` on a `pytorch/pytorch` PR (via the separately deployed trigger) dispatches
+`greenlight-review.yml` with the PR number and commenter as `--pr N --requester <login>`. The scan
+is the sole authorizer: `--pr` refuses unless PR N's author is trusted, and `--requester` refuses
+unless the commenter is trusted too (case-insensitive; a refusal is a clean exit 0). The local-only
+`--allow-untrusted-author` skips the target-author check for iteration and is never a workflow
+input. The command is not yet advertised in the PR status comment.
+
 Daemon mode loops the phase on an interval:
 
 ```bash

greenlight/README.md

@@ -37,10 +37,12 @@ just run review                      # scan + dispatch once, then exit
 just review                          # convenience alias for `just run review`
 just run review --loop               # scan + dispatch forever as a daemon
 just run review --loop --interval 30 # daemon, 30s between iterations
-just run review --pr 123             # restrict the scan to PR #123
+just run review --pr 123             # scan only PR #123 (its author must be trusted)
+just run review --pr 123 --requester alice  # recheck PR #123 for alice (author and alice must be trusted)
 just run review --max 5              # cap this iteration at 5 dispatches
 just run review --ref my-branch      # dispatch the reviewer workflow at this test-infra ref (default main)
 just run review --timeout-minutes 60 # re-dispatch an in-flight review after 60 min (default 45)
+just run review --pr 123 --allow-untrusted-author  # LOCAL ONLY: skip the --pr author check
 ```
 
 `review` requires `PYTORCH_GREENLIGHT_GITHUB_TOKEN`, and any scan that finds at least one
@@ -53,6 +55,31 @@ record a verdict) before it re-dispatches, rather than cancelling and restarting
 is still running. Lower `--timeout-minutes` in the deployment if you need a stuck review
 reclaimed sooner.
 
+### Rechecking a PR (`@greenlight recheck`)
+
+A trusted author can re-trigger a review by commenting `@greenlight recheck` on a
+`pytorch/pytorch` PR. A thin `pytorch/pytorch` workflow (deployed separately) dispatches
+`greenlight-review.yml` with the PR number and the commenter's login, and the scan re-checks
+that one PR through the `--pr` path.
+
+The scan is the single source of authorization and enforces two gates against the trusted-author
+set (`review.TRUSTED_AUTHORS`), matched case-insensitively:
+
+- **Target-author gate** — `--pr N` looks up PR `N`'s author and refuses (no fingerprint, no
+  dispatch, no review) unless that author is trusted. Unlike the listing scan, `--pr` names an
+  arbitrary PR, so this gate is what stops an arbitrary PR from being reviewed or approved on request.
+- **Requester gate** — `--requester <login>`, when given, additionally requires `<login>` to be
+  trusted; an untrusted requester is refused before any network work, and the requester is logged
+  for audit.
+
+A refusal is a clean no-op (exit 0), not a failure. `--allow-untrusted-author` is a **local-only**
+flag that skips the target-author gate for iteration; it is deliberately not exposed as a
+`greenlight-review.yml` input, is unreachable from the comment/dispatch path, and never affects the
+requester gate.
+
+The user-facing `@greenlight recheck` hint is not yet advertised in the PR status comment; that
+is added once the `pytorch/pytorch` trigger is deployed.
+
 ### Recording a verdict
 
 A privileged CI job records a review verdict with `verdict`. It runs once (never a

greenlight/src/greenlight/cli.py

@@ -46,7 +46,10 @@ def build_parser() -> argparse.ArgumentParser:
         ),
     )
     review_parser.add_argument(
-        "--pr", type=int, default=None, help="scan only this PR number, bypassing the trusted-author listing"
+        "--pr",
+        type=int,
+        default=None,
+        help="scan only this PR number (skips the listing; the PR's author must still be trusted)",
     )
     review_parser.add_argument(
         "--max",
@@ -66,6 +69,16 @@ def build_parser() -> argparse.ArgumentParser:
         action="store_true",
         help="re-dispatch even if already reviewed; requires --pr, not allowed with --loop",
     )
+    review_parser.add_argument(
+        "--requester",
+        default=None,
+        help="login that requested this review (@greenlight recheck); must be a trusted author or the run refuses",
+    )
+    review_parser.add_argument(
+        "--allow-untrusted-author",
+        action="store_true",
+        help="LOCAL USE ONLY: skip the --pr target-author trusted check (never exposed as a workflow input)",
+    )
 
     verdict_parser = subparsers.add_parser(
         "verdict",
@@ -203,6 +216,8 @@ def main(argv: Sequence[str] | None = None) -> int:
         ref=args.ref,
         timeout_minutes=args.timeout_minutes,
         force=args.force,
+        requester=args.requester,
+        allow_untrusted_author=args.allow_untrusted_author,
         resolve_authorized=authorized_cache.get,
     )
     return _dispatch(config, run, loop=args.loop, lock_path=lock_path)

greenlight/src/greenlight/github_client.py

@@ -80,6 +80,16 @@ def get_reviews(self) -> Iterable[_PRReview]: ...
     class _ScanRepo(Protocol):
         def get_pull(self, number: int) -> _FingerprintPR: ...
 
+    class _AuthorPR(Protocol):
+        @property
+        def user(self) -> _PRUser | None: ...
+
+    class _AuthorRepo(Protocol):
+        def get_pull(self, number: int) -> _AuthorPR: ...
+
+    class _AuthorClient(Protocol):
+        def get_repo(self, full_name_or_id: str) -> _AuthorRepo: ...
+
     class _VerdictReview(Protocol):
         @property
         def id(self) -> int: ...
@@ -167,6 +177,18 @@ def list_open_prs_by_authors(client: _RepoClient, repo: str, authors: Iterable[s
     return sorted(prs, key=lambda p: p.number)
 
 
+def get_pr_author(client: _AuthorClient, repo: str, number: int) -> str | None:
+    """Return the login of a single PR's author, or None if it has no resolvable user.
+
+    Used by the ``--pr`` scan path to gate on the target PR's author: unlike the listing path
+    (already filtered to trusted authors), ``--pr`` names an arbitrary PR, so the caller must
+    verify its author before fingerprinting or dispatching a review.
+    """
+    pr = client.get_repo(repo).get_pull(number)
+    user = pr.user
+    return user.login if user is not None else None
+
+
 def _actor_login(
     user: _PRActor | None, self_login: str | None, authorized_logins: frozenset[str] | None = None
 ) -> str | None:

greenlight/src/greenlight/review.py

@@ -46,6 +46,15 @@
     "jeanschmidt",  # Jean Schmidt
 }
 
+# Case-insensitive membership for the two authz gates (target-PR author and recheck requester);
+# GitHub logins are case-insensitive, so gate on the lowercased login against this derived set.
+_TRUSTED_LOWER: frozenset[str] = frozenset(author.lower() for author in TRUSTED_AUTHORS)
+
+
+def _is_trusted(login: str | None) -> bool:
+    return login is not None and login.lower() in _TRUSTED_LOWER
+
+
 _FINGERPRINT_WORKERS = 8
 
 # Never-reviewed candidates sort ahead of every recorded one; this stands in for their
@@ -64,6 +73,10 @@ def _default_fetch(client: Github) -> list[OpenPR]:
     return github_client.list_open_prs_by_authors(client, TARGET_REPO, TRUSTED_AUTHORS)
 
 
+def _default_fetch_author(client: Github, pr_number: int) -> str | None:
+    return github_client.get_pr_author(client, TARGET_REPO, pr_number)
+
+
 def _default_fingerprint(client: Github, pr_number: int, authorized_logins: frozenset[str]) -> tuple[str, str]:
     return github_client.fingerprint_pr(client, TARGET_REPO, pr_number, authorized_logins=authorized_logins)
 
@@ -289,8 +302,11 @@ def run(
     ref: str = DEFAULT_DISPATCH_REF,
     timeout_minutes: int = DEFAULT_TIMEOUT_MINUTES,
     force: bool = False,
+    requester: str | None = None,
+    allow_untrusted_author: bool = False,
     build_github: Callable[[str], Github] = github_client.build_client,
     fetch: Callable[[Github], list[OpenPR]] = _default_fetch,
+    fetch_author: Callable[[Github, int], str | None] = _default_fetch_author,
     fingerprint: Callable[[Github, int, frozenset[str]], tuple[str, str]] = _default_fingerprint,
     read_state: Callable[[str, Sequence[int]], dict[int, PRState]] = state.read_latest_states,
     dispatch: Callable[[Github, int, str, str, str], None] = dispatch_module.dispatch_review,
@@ -302,13 +318,29 @@ def run(
     token = config.github_token
     if not token:
         raise ValueError("PYTORCH_GREENLIGHT_GITHUB_TOKEN is required to query GitHub")
-    # Resolved once per scan and never caught here: a cold failure must fail the scan (one-shot
-    # exits non-zero, daemon backs off) rather than silently revert to hashing all human comments.
-    authorized_logins = resolve_authorized()
-    logger.info("filtering fingerprint comments to %d merge-authorized login(s)", len(authorized_logins))
+    # Requester gate (recheck path): an untrusted requester is rejected before any network work,
+    # so a spammed @greenlight recheck from an untrusted login costs nothing. A policy refusal is
+    # not a failure -- return cleanly rather than raising (no non-zero exit, no daemon backoff).
+    if requester is not None:
+        if not _is_trusted(requester):
+            logger.warning("refusing review: requester %r is not a trusted author", requester)
+            return
+        logger.info("review requested by trusted author %s", requester)
     with contextlib.ExitStack() as clients:
         client = build_github(token)
         clients.callback(_close_client, client)
+        # Target-author gate: the listing path is already trusted-only, but --pr names an arbitrary
+        # PR, so its author MUST be trusted too or greenlight would review/approve any PR on request.
+        # allow_untrusted_author bypasses ONLY this check (local iteration; never a workflow input).
+        if pr is not None and not allow_untrusted_author:
+            author = fetch_author(client, pr)
+            if not _is_trusted(author):
+                logger.warning("refusing --pr %d: author %r is not a trusted author", pr, author)
+                return
+        # Resolved once per scan and never caught here: a cold failure must fail the scan (one-shot
+        # exits non-zero, daemon backs off) rather than silently revert to hashing all human comments.
+        authorized_logins = resolve_authorized()
+        logger.info("filtering fingerprint comments to %d merge-authorized login(s)", len(authorized_logins))
         pr_numbers, updated_at_by_number = _candidate_numbers(client, pr=pr, fetch=fetch)
         states = read_state(TARGET_REPO, pr_numbers)
         evaluated_at = now()

greenlight/tests/test_cli.py

@@ -107,6 +107,8 @@ def fake_run_forever(config, *, run):
         "ref": "main",
         "timeout_minutes": 45,
         "force": False,
+        "requester": None,
+        "allow_untrusted_author": False,
     }
 
 
@@ -504,6 +506,15 @@ def test_review_parser_scan_flag_defaults():
     assert args.ref == DEFAULT_DISPATCH_REF
     assert args.timeout_minutes == DEFAULT_TIMEOUT_MINUTES
     assert args.force is False
+    assert args.requester is None
+    assert args.allow_untrusted_author is False
+
+
+def test_review_parser_parses_requester_and_allow_untrusted_author():
+    parser = cli.build_parser()
+    args = parser.parse_args(["review", "--pr", "5", "--requester", "albanD", "--allow-untrusted-author"])
+    assert args.requester == "albanD"
+    assert args.allow_untrusted_author is True
 
 
 def test_main_review_binds_scan_flags_into_run(monkeypatch):
@@ -523,6 +534,8 @@ def test_main_review_binds_scan_flags_into_run(monkeypatch):
         "ref": "release/2.9",
         "timeout_minutes": 60,
         "force": False,
+        "requester": None,
+        "allow_untrusted_author": False,
     }
 
 
@@ -541,6 +554,8 @@ def test_main_review_defaults_bind_into_run(monkeypatch):
         "ref": DEFAULT_DISPATCH_REF,
         "timeout_minutes": DEFAULT_TIMEOUT_MINUTES,
         "force": False,
+        "requester": None,
+        "allow_untrusted_author": False,
     }
 
 
@@ -579,6 +594,28 @@ def test_main_review_force_binds_into_run(monkeypatch):
         "ref": DEFAULT_DISPATCH_REF,
         "timeout_minutes": DEFAULT_TIMEOUT_MINUTES,
         "force": True,
+        "requester": None,
+        "allow_untrusted_author": False,
+    }
+
+
+def test_main_review_binds_requester_and_override_into_run(monkeypatch):
+    review_mock = Mock()
+    monkeypatch.setattr(review, "run", review_mock)
+    monkeypatch.setattr(cli, "single_instance_lock", _noop_lock)
+    monkeypatch.setattr(cli, "configure_logging", Mock())
+
+    rc = cli.main(["review", "--pr", "5", "--requester", "albanD", "--allow-untrusted-author"])
+
+    assert rc == EXIT_OK
+    assert _pop_resolve_authorized(review_mock.call_args.kwargs) == {
+        "pr": 5,
+        "max_dispatches": None,
+        "ref": DEFAULT_DISPATCH_REF,
+        "timeout_minutes": DEFAULT_TIMEOUT_MINUTES,
+        "force": False,
+        "requester": "albanD",
+        "allow_untrusted_author": True,
     }
 
 

greenlight/tests/test_github_client.py

@@ -263,6 +263,48 @@ def test_list_open_prs_by_authors_propagates_errors():
         github_client.list_open_prs_by_authors(client, "pytorch/pytorch", ["alice"])
 
 
+class _FakeAuthorPull:
+    def __init__(self, user: _FakeUser | None) -> None:
+        self.user = user
+
+
+class _FakeAuthorRepo:
+    def __init__(self, pull: _FakeAuthorPull) -> None:
+        self._pull = pull
+        self.get_pull_numbers: list[int] = []
+
+    def get_pull(self, number: int) -> _FakeAuthorPull:
+        self.get_pull_numbers.append(number)
+        return self._pull
+
+
+class _FakeAuthorClient:
+    def __init__(self, repo: _FakeAuthorRepo) -> None:
+        self._repo = repo
+        self.get_repo_names: list[str] = []
+
+    def get_repo(self, full_name_or_id: str) -> _FakeAuthorRepo:
+        self.get_repo_names.append(full_name_or_id)
+        return self._repo
+
+
+def test_get_pr_author_returns_login():
+    repo = _FakeAuthorRepo(_FakeAuthorPull(_FakeUser("albanD")))
+    client = _FakeAuthorClient(repo)
+
+    author = github_client.get_pr_author(client, "pytorch/pytorch", 42)
+
+    assert author == "albanD"
+    assert client.get_repo_names == ["pytorch/pytorch"]
+    assert repo.get_pull_numbers == [42]
+
+
+def test_get_pr_author_returns_none_when_user_missing():
+    client = _FakeAuthorClient(_FakeAuthorRepo(_FakeAuthorPull(None)))
+
+    assert github_client.get_pr_author(client, "pytorch/pytorch", 7) is None
+
+
 def test_build_client_returns_github_instance_with_per_page_100():
     client = github_client.build_client("x")