mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-26 16:07:53 +00:00
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.
This commit is contained in:
parent
5c80c07dfe
commit
259f51ca4f
@ -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:
|
||||
|
||||
@ -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
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user