From 259f51ca4fdd5eff118aa1d834a0e463e52a3d23 Mon Sep 17 00:00:00 2001 From: Aari Date: Thu, 16 Jul 2026 09:12:29 +0800 Subject: [PATCH] fix(github): match allow_authors logins case-insensitively (#4218) * fix(github): match allow_authors logins case-insensitively GitHub logins are case-insensitive, and the sibling gates in this module already treat them that way. allow_authors used a bare string membership test, so a YAML casing mismatch silently dropped owner webhooks that should have bypassed require_mention. * test(github): parametrize allow_authors case-fold over both directions Cover the reverse cfg "alice" / payload "Alice" direction plus the all-caps and exact-case rows from the PR's E2E matrix. Each casing-differs row is red on the pre-fix source; the exact-case row stays green both ways, pinning that case-folding is a superset of the old exact match. --- backend/app/gateway/github/triggers.py | 6 ++- backend/tests/test_github_triggers.py | 53 ++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/backend/app/gateway/github/triggers.py b/backend/app/gateway/github/triggers.py index c89799ed5..2c1e3ab0c 100644 --- a/backend/app/gateway/github/triggers.py +++ b/backend/app/gateway/github/triggers.py @@ -180,9 +180,13 @@ def event_should_fire( # allow_authors bypasses require_mention entirely. Useful so a repo # owner can talk to the bot without typing the handle every time. + # Match is case-insensitive — GitHub logins are, and the sibling gates + # in this module already are (``_mentions`` uses re.IGNORECASE; the + # self-event check lowercases both sides). A bare ``in`` membership + # test would drop an owner whose YAML casing differs from the payload. if trigger.allow_authors: author = _author_login(event, payload) - if author and author in trigger.allow_authors: + if author and author.lower() in {a.lower() for a in trigger.allow_authors}: return True, f"allow_authors:{author}" if trigger.require_mention: diff --git a/backend/tests/test_github_triggers.py b/backend/tests/test_github_triggers.py index aca712845..efd1046db 100644 --- a/backend/tests/test_github_triggers.py +++ b/backend/tests/test_github_triggers.py @@ -2,6 +2,8 @@ from __future__ import annotations +import pytest + from app.gateway.github.triggers import ( DEFAULT_TRIGGERS, _resolved_trigger, @@ -162,6 +164,57 @@ def test_allow_authors_does_not_help_other_users() -> None: assert fire is False +@pytest.mark.parametrize( + ("allow_authors", "author"), + [ + (["Alice"], "alice"), # config upper / payload lower + (["alice"], "Alice"), # reverse: config lower / payload upper + (["ALICE"], "alice"), # all-caps config + (["Alice"], "Alice"), # exact case still fires after folding + ], +) +def test_allow_authors_match_is_case_insensitive(allow_authors: list[str], author: str) -> None: + """GitHub logins are case-insensitive; allow_authors must match that. + + Sibling gates already ignore case: ``_mentions`` documents + ``Match is case-insensitive; GitHub itself is.``, and the self-event + check lowercases both sides. A bare ``in`` membership test rejects an + owner whose YAML casing differs from the payload login, so + ``require_mention`` still applies and the webhook is silently dropped. + + ``.lower()`` is symmetric, so both fold directions and an all-caps + config must fire; the exact-case pair pins that folding stays a + superset of the old exact match. + """ + trigger = _resolve( + "issue_comment", + GitHubTriggerConfig(require_mention=True, allow_authors=allow_authors), + ) + fire, reason = event_should_fire( + "issue_comment", + _comment_payload("no handle here", author=author), + trigger, + BOT, + ) + assert fire is True + assert "allow_authors" in reason + + +def test_allow_authors_case_insensitive_still_rejects_other_users() -> None: + """Case-folding the allowlist must not open the gate for non-members.""" + trigger = _resolve( + "issue_comment", + GitHubTriggerConfig(require_mention=True, allow_authors=["Alice"]), + ) + fire, _ = event_should_fire( + "issue_comment", + _comment_payload("no handle", author="bob"), + trigger, + BOT, + ) + assert fire is False + + # --------------------------------------------------------------------------- # Override: mention_login replaces default login # ---------------------------------------------------------------------------