From 9bc7ed53296f3d98c3fbbd8f6991a04693702c3b Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Wed, 29 Jul 2026 10:54:46 -0500 Subject: [PATCH 1/2] ci: reject @mentions in pull request descriptions Flag GitHub @username mentions in PR bodies so they are not copied into merge commits and re-notify people on merge, rebase, or backport. Use a dedicated Python helper with unit tests, preserve the existing required-check name, and allow complete email addresses. --- .../check_pr_description_mentions.py | 139 ++++++++++++++ .github/workflows/semantic-pull-request.yml | 18 +- .../test_check_pr_description_mentions.py | 180 ++++++++++++++++++ 3 files changed, 336 insertions(+), 1 deletion(-) create mode 100755 .github/workflows/check_pr_description_mentions.py create mode 100755 .github/workflows/test_check_pr_description_mentions.py diff --git a/.github/workflows/check_pr_description_mentions.py b/.github/workflows/check_pr_description_mentions.py new file mode 100755 index 000000000000..50387636925d --- /dev/null +++ b/.github/workflows/check_pr_description_mentions.py @@ -0,0 +1,139 @@ +#!/usr/bin/env python3 +# Copyright (c) 2026 The Dash Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +""" +Reject GitHub @username mentions in pull request descriptions. + +Mentions are copied into merge commits and re-notify people on merge, +rebase, or backport. Email addresses are allowed; empty descriptions pass. + +Usage: + PR_BODY='...' python3 .github/workflows/check_pr_description_mentions.py + python3 .github/workflows/check_pr_description_mentions.py --body-file path + printf '%s' '...' | python3 .github/workflows/check_pr_description_mentions.py --stdin +""" + +from __future__ import annotations + +import argparse +import os +import re +import sys +from typing import List, Optional, Sequence, Tuple + + +# Match complete, conventional dot-atom email addresses with a dotted domain. +# Email spans are excluded from the independent GitHub @username scan below. +EMAIL_LOCAL_ATOM = r"[A-Za-z0-9!#$%&'*+/=?^_`{|}~-]+" +EMAIL_DOMAIN_LABEL = r"[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?" +EMAIL_RE = re.compile( + rf"{EMAIL_LOCAL_ATOM}(?:\.{EMAIL_LOCAL_ATOM})*" + rf"@{EMAIL_DOMAIN_LABEL}(?:\.{EMAIL_DOMAIN_LABEL})+" +) +EMAIL_LOCAL_SPECIALS = frozenset("!#$%&'*+/=?^_`{|}~-") + +# GitHub @username: @ + 1-39 characters (alphanumeric or internal hyphens). +MENTION_RE = re.compile(r"@[A-Za-z0-9](?:[A-Za-z0-9-]{0,37}[A-Za-z0-9])?\b") + +ERROR_MESSAGE = """\ +::error::PR description contains GitHub @mentions. +Do not put @username mentions in PR descriptions. +They are copied into merge commits and notify people again +whenever the PR is merged, rebased, or backported. +Refer to people by name or GitHub URL without the leading @.\ +""" + + +def find_email_spans(line: str) -> List[Tuple[int, int]]: + """Return spans for complete email addresses in a line.""" + spans: List[Tuple[int, int]] = [] + for match in EMAIL_RE.finditer(line): + start, end = match.span() + if start > 0: + previous = line[start - 1] + if previous.isalnum() or previous == "." or previous in EMAIL_LOCAL_SPECIALS: + continue + if end < len(line) and ( + line[end].isalnum() or line[end] in "-_" + ): + continue + spans.append((start, end)) + return spans + + +def find_mentions(body: str) -> List[Tuple[int, str, str]]: + """Return (1-based line number, line text, match text) for each @mention.""" + matches: List[Tuple[int, str, str]] = [] + for line_no, line in enumerate(body.splitlines(), start=1): + email_spans = find_email_spans(line) + for match in MENTION_RE.finditer(line): + if any(start <= match.start() < end for start, end in email_spans): + continue + matches.append((line_no, line, match.group(0))) + return matches + + +def check_body(body: Optional[str]) -> int: + """Validate PR body. Return 0 on pass, 1 when @mentions are present.""" + if body is None or body == "": + print("PR description is empty; no @mentions to check.") + return 0 + + matches = find_mentions(body) + if not matches: + print("No @mentions found in PR description.") + return 0 + + for line_no, line, mention in matches: + print(f"{line_no}:{mention}: {line}") + + print("", file=sys.stderr) + print(ERROR_MESSAGE, file=sys.stderr) + return 1 + + +def parse_args(argv: Sequence[str]) -> argparse.Namespace: + parser = argparse.ArgumentParser( + description="Reject GitHub @username mentions in pull request descriptions." + ) + source = parser.add_mutually_exclusive_group() + source.add_argument( + "--body-file", + metavar="PATH", + help="Read the PR body from PATH instead of the PR_BODY environment variable", + ) + source.add_argument( + "--stdin", + action="store_true", + help="Read the PR body from stdin instead of the PR_BODY environment variable", + ) + return parser.parse_args(argv) + + +def read_body(args: argparse.Namespace) -> Optional[str]: + if args.body_file is not None: + with open(args.body_file, encoding="utf-8") as handle: + return handle.read() + if args.stdin: + return sys.stdin.read() + # PR_BODY may be unset (treated as empty) or set to "" / multi-line text. + # Never shell-interpolate untrusted body content; workflows pass it via env. + if "PR_BODY" not in os.environ: + return None + return os.environ["PR_BODY"] + + +def main(argv: Sequence[str] | None = None) -> int: + args = parse_args(argv if argv is not None else sys.argv[1:]) + try: + body = read_body(args) + except OSError as exc: + print(f"error: failed to read PR body: {exc}", file=sys.stderr) + return 1 + return check_body(body) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.github/workflows/semantic-pull-request.yml b/.github/workflows/semantic-pull-request.yml index 191a34f37381..e9360c567f52 100644 --- a/.github/workflows/semantic-pull-request.yml +++ b/.github/workflows/semantic-pull-request.yml @@ -1,4 +1,4 @@ -name: "Check PR title format" +name: "Check PR title and description" on: pull_request_target: @@ -50,3 +50,19 @@ jobs: utils wallet zmq + + # Checkout the base branch (pull_request_target default). Safe: no untrusted + # PR code is executed. The helper script and this workflow land together on + # merge, so enforcement starts after merge — same as the workflow YAML itself. + - name: Checkout + uses: actions/checkout@v6 + with: + persist-credentials: false + + # Reject @username mentions in the PR description. Mentions are copied + # into merge commits and re-notify people on merge/rebase/backport. + # Body is passed via env (not shell interpolation) to avoid injection. + - name: Check PR description for @mentions + env: + PR_BODY: ${{ github.event.pull_request.body }} + run: python3 .github/workflows/check_pr_description_mentions.py diff --git a/.github/workflows/test_check_pr_description_mentions.py b/.github/workflows/test_check_pr_description_mentions.py new file mode 100755 index 000000000000..13aee0fd4319 --- /dev/null +++ b/.github/workflows/test_check_pr_description_mentions.py @@ -0,0 +1,180 @@ +#!/usr/bin/env python3 +# Copyright (c) 2026 The Dash Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +import importlib.util +import io +import os +import tempfile +import unittest +from contextlib import redirect_stderr, redirect_stdout +from pathlib import Path +from unittest import mock + + +SCRIPT_PATH = Path(__file__).with_name("check_pr_description_mentions.py") +SPEC = importlib.util.spec_from_file_location("check_pr_description_mentions", SCRIPT_PATH) +assert SPEC is not None and SPEC.loader is not None +check_pr_description_mentions = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(check_pr_description_mentions) + + +class TestFindMentions(unittest.TestCase): + def test_empty_body_has_no_mentions(self): + self.assertEqual([], check_pr_description_mentions.find_mentions("")) + + def test_plain_text_without_at_has_no_mentions(self): + body = "Explain the change and why.\n\n## Checklist\n- [x] tests" + self.assertEqual([], check_pr_description_mentions.find_mentions(body)) + + def test_email_addresses_are_allowed(self): + body = ( + "Contact alice@example.com, bob.smith+ci@dash.org, " + "or dev!@example.com for details." + ) + self.assertEqual([], check_pr_description_mentions.find_mentions(body)) + + def test_email_with_punctuation_boundaries_is_allowed(self): + body = "Contact ; backup: (first.last@example.co.uk)." + self.assertEqual([], check_pr_description_mentions.find_mentions(body)) + + def test_single_username_mention_is_found(self): + matches = check_pr_description_mentions.find_mentions("Thanks @knst for the review.") + self.assertEqual([(1, "Thanks @knst for the review.", "@knst")], matches) + + def test_mention_after_period_is_found(self): + matches = check_pr_description_mentions.find_mentions("Thanks.@knst") + self.assertEqual([(1, "Thanks.@knst", "@knst")], matches) + + def test_mention_after_plus_is_found(self): + matches = check_pr_description_mentions.find_mentions("cc +@knst") + self.assertEqual([(1, "cc +@knst", "@knst")], matches) + + def test_mention_at_start_of_line(self): + matches = check_pr_description_mentions.find_mentions("@PastaClaw requested this.") + self.assertEqual([(1, "@PastaClaw requested this.", "@PastaClaw")], matches) + + def test_multiple_mentions_across_lines(self): + body = "Ping @alice\nand also @bob-user later." + matches = check_pr_description_mentions.find_mentions(body) + self.assertEqual( + [ + (1, "Ping @alice", "@alice"), + (2, "and also @bob-user later.", "@bob-user"), + ], + matches, + ) + + def test_single_character_username(self): + matches = check_pr_description_mentions.find_mentions("ask @a please") + self.assertEqual([(1, "ask @a please", "@a")], matches) + + def test_username_with_internal_hyphen(self): + matches = check_pr_description_mentions.find_mentions("cc @some-user-name") + self.assertEqual([(1, "cc @some-user-name", "@some-user-name")], matches) + + def test_trailing_punctuation_does_not_break_match(self): + matches = check_pr_description_mentions.find_mentions("See @reviewer.") + self.assertEqual([(1, "See @reviewer.", "@reviewer")], matches) + + def test_github_url_without_at_is_allowed(self): + body = "Discussed with https://github.com/knst in review." + self.assertEqual([], check_pr_description_mentions.find_mentions(body)) + + def test_email_and_mention_mixed(self): + body = "Email dev@example.com and ping @maintainer" + matches = check_pr_description_mentions.find_mentions(body) + self.assertEqual( + [(1, "Email dev@example.com and ping @maintainer", "@maintainer")], + matches, + ) + + def test_mention_adjacent_to_email_is_found(self): + body = "Email dev@example.com.@maintainer" + matches = check_pr_description_mentions.find_mentions(body) + self.assertEqual( + [(1, "Email dev@example.com.@maintainer", "@maintainer")], + matches, + ) + + def test_incomplete_email_is_not_exempted(self): + body = "This is not a complete email: dev!@example" + matches = check_pr_description_mentions.find_mentions(body) + self.assertEqual( + [(1, "This is not a complete email: dev!@example", "@example")], + matches, + ) + + +class TestCheckBody(unittest.TestCase): + def test_none_and_empty_pass(self): + for body in (None, ""): + with self.subTest(body=body): + stdout = io.StringIO() + with redirect_stdout(stdout): + code = check_pr_description_mentions.check_body(body) + self.assertEqual(0, code) + self.assertIn("empty", stdout.getvalue().lower()) + + def test_clean_body_passes(self): + stdout = io.StringIO() + with redirect_stdout(stdout): + code = check_pr_description_mentions.check_body( + "No people tagged.\nContact team@example.com if needed." + ) + self.assertEqual(0, code) + self.assertIn("No @mentions found", stdout.getvalue()) + + def test_body_with_mention_fails_and_prints_guidance(self): + stdout = io.StringIO() + stderr = io.StringIO() + with redirect_stdout(stdout), redirect_stderr(stderr): + code = check_pr_description_mentions.check_body("cc @someone") + self.assertEqual(1, code) + self.assertIn("@someone", stdout.getvalue()) + self.assertIn("::error::PR description contains GitHub @mentions.", stderr.getvalue()) + self.assertIn("merge commits", stderr.getvalue()) + + +class TestMain(unittest.TestCase): + def test_main_reads_pr_body_env(self): + with mock.patch.dict(os.environ, {"PR_BODY": "hello @user"}, clear=False): + stderr = io.StringIO() + with redirect_stdout(io.StringIO()), redirect_stderr(stderr): + code = check_pr_description_mentions.main([]) + self.assertEqual(1, code) + self.assertIn("@mentions", stderr.getvalue()) + + def test_main_missing_pr_body_env_treated_as_empty(self): + env = {k: v for k, v in os.environ.items() if k != "PR_BODY"} + with mock.patch.dict(os.environ, env, clear=True): + stdout = io.StringIO() + with redirect_stdout(stdout): + code = check_pr_description_mentions.main([]) + self.assertEqual(0, code) + self.assertIn("empty", stdout.getvalue().lower()) + + def test_main_body_file(self): + with tempfile.NamedTemporaryFile("w", encoding="utf-8", delete=False) as handle: + handle.write("safe body with email only: a@b.co\n") + path = handle.name + try: + stdout = io.StringIO() + with redirect_stdout(stdout): + code = check_pr_description_mentions.main(["--body-file", path]) + self.assertEqual(0, code) + self.assertIn("No @mentions found", stdout.getvalue()) + finally: + os.unlink(path) + + def test_main_stdin(self): + stdin = io.StringIO("@bad\n") + with mock.patch.object(check_pr_description_mentions.sys, "stdin", stdin): + with redirect_stdout(io.StringIO()), redirect_stderr(io.StringIO()): + code = check_pr_description_mentions.main(["--stdin"]) + self.assertEqual(1, code) + + +if __name__ == "__main__": + unittest.main() From 9cde0fafa013dd2848896d7d6611fc823ef39b41 Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Thu, 30 Jul 2026 13:22:58 -0500 Subject: [PATCH 2/2] ci: drop unit tests for PR description mention check Reviewers asked to keep this check simple; the helper itself is enough without a dedicated CI unit-test file. --- .../check_pr_description_mentions.py | 2 +- .../test_check_pr_description_mentions.py | 180 ------------------ 2 files changed, 1 insertion(+), 181 deletions(-) delete mode 100755 .github/workflows/test_check_pr_description_mentions.py diff --git a/.github/workflows/check_pr_description_mentions.py b/.github/workflows/check_pr_description_mentions.py index 50387636925d..e398344736e2 100755 --- a/.github/workflows/check_pr_description_mentions.py +++ b/.github/workflows/check_pr_description_mentions.py @@ -129,7 +129,7 @@ def main(argv: Sequence[str] | None = None) -> int: args = parse_args(argv if argv is not None else sys.argv[1:]) try: body = read_body(args) - except OSError as exc: + except (OSError, UnicodeDecodeError) as exc: print(f"error: failed to read PR body: {exc}", file=sys.stderr) return 1 return check_body(body) diff --git a/.github/workflows/test_check_pr_description_mentions.py b/.github/workflows/test_check_pr_description_mentions.py deleted file mode 100755 index 13aee0fd4319..000000000000 --- a/.github/workflows/test_check_pr_description_mentions.py +++ /dev/null @@ -1,180 +0,0 @@ -#!/usr/bin/env python3 -# Copyright (c) 2026 The Dash Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. - -import importlib.util -import io -import os -import tempfile -import unittest -from contextlib import redirect_stderr, redirect_stdout -from pathlib import Path -from unittest import mock - - -SCRIPT_PATH = Path(__file__).with_name("check_pr_description_mentions.py") -SPEC = importlib.util.spec_from_file_location("check_pr_description_mentions", SCRIPT_PATH) -assert SPEC is not None and SPEC.loader is not None -check_pr_description_mentions = importlib.util.module_from_spec(SPEC) -SPEC.loader.exec_module(check_pr_description_mentions) - - -class TestFindMentions(unittest.TestCase): - def test_empty_body_has_no_mentions(self): - self.assertEqual([], check_pr_description_mentions.find_mentions("")) - - def test_plain_text_without_at_has_no_mentions(self): - body = "Explain the change and why.\n\n## Checklist\n- [x] tests" - self.assertEqual([], check_pr_description_mentions.find_mentions(body)) - - def test_email_addresses_are_allowed(self): - body = ( - "Contact alice@example.com, bob.smith+ci@dash.org, " - "or dev!@example.com for details." - ) - self.assertEqual([], check_pr_description_mentions.find_mentions(body)) - - def test_email_with_punctuation_boundaries_is_allowed(self): - body = "Contact ; backup: (first.last@example.co.uk)." - self.assertEqual([], check_pr_description_mentions.find_mentions(body)) - - def test_single_username_mention_is_found(self): - matches = check_pr_description_mentions.find_mentions("Thanks @knst for the review.") - self.assertEqual([(1, "Thanks @knst for the review.", "@knst")], matches) - - def test_mention_after_period_is_found(self): - matches = check_pr_description_mentions.find_mentions("Thanks.@knst") - self.assertEqual([(1, "Thanks.@knst", "@knst")], matches) - - def test_mention_after_plus_is_found(self): - matches = check_pr_description_mentions.find_mentions("cc +@knst") - self.assertEqual([(1, "cc +@knst", "@knst")], matches) - - def test_mention_at_start_of_line(self): - matches = check_pr_description_mentions.find_mentions("@PastaClaw requested this.") - self.assertEqual([(1, "@PastaClaw requested this.", "@PastaClaw")], matches) - - def test_multiple_mentions_across_lines(self): - body = "Ping @alice\nand also @bob-user later." - matches = check_pr_description_mentions.find_mentions(body) - self.assertEqual( - [ - (1, "Ping @alice", "@alice"), - (2, "and also @bob-user later.", "@bob-user"), - ], - matches, - ) - - def test_single_character_username(self): - matches = check_pr_description_mentions.find_mentions("ask @a please") - self.assertEqual([(1, "ask @a please", "@a")], matches) - - def test_username_with_internal_hyphen(self): - matches = check_pr_description_mentions.find_mentions("cc @some-user-name") - self.assertEqual([(1, "cc @some-user-name", "@some-user-name")], matches) - - def test_trailing_punctuation_does_not_break_match(self): - matches = check_pr_description_mentions.find_mentions("See @reviewer.") - self.assertEqual([(1, "See @reviewer.", "@reviewer")], matches) - - def test_github_url_without_at_is_allowed(self): - body = "Discussed with https://github.com/knst in review." - self.assertEqual([], check_pr_description_mentions.find_mentions(body)) - - def test_email_and_mention_mixed(self): - body = "Email dev@example.com and ping @maintainer" - matches = check_pr_description_mentions.find_mentions(body) - self.assertEqual( - [(1, "Email dev@example.com and ping @maintainer", "@maintainer")], - matches, - ) - - def test_mention_adjacent_to_email_is_found(self): - body = "Email dev@example.com.@maintainer" - matches = check_pr_description_mentions.find_mentions(body) - self.assertEqual( - [(1, "Email dev@example.com.@maintainer", "@maintainer")], - matches, - ) - - def test_incomplete_email_is_not_exempted(self): - body = "This is not a complete email: dev!@example" - matches = check_pr_description_mentions.find_mentions(body) - self.assertEqual( - [(1, "This is not a complete email: dev!@example", "@example")], - matches, - ) - - -class TestCheckBody(unittest.TestCase): - def test_none_and_empty_pass(self): - for body in (None, ""): - with self.subTest(body=body): - stdout = io.StringIO() - with redirect_stdout(stdout): - code = check_pr_description_mentions.check_body(body) - self.assertEqual(0, code) - self.assertIn("empty", stdout.getvalue().lower()) - - def test_clean_body_passes(self): - stdout = io.StringIO() - with redirect_stdout(stdout): - code = check_pr_description_mentions.check_body( - "No people tagged.\nContact team@example.com if needed." - ) - self.assertEqual(0, code) - self.assertIn("No @mentions found", stdout.getvalue()) - - def test_body_with_mention_fails_and_prints_guidance(self): - stdout = io.StringIO() - stderr = io.StringIO() - with redirect_stdout(stdout), redirect_stderr(stderr): - code = check_pr_description_mentions.check_body("cc @someone") - self.assertEqual(1, code) - self.assertIn("@someone", stdout.getvalue()) - self.assertIn("::error::PR description contains GitHub @mentions.", stderr.getvalue()) - self.assertIn("merge commits", stderr.getvalue()) - - -class TestMain(unittest.TestCase): - def test_main_reads_pr_body_env(self): - with mock.patch.dict(os.environ, {"PR_BODY": "hello @user"}, clear=False): - stderr = io.StringIO() - with redirect_stdout(io.StringIO()), redirect_stderr(stderr): - code = check_pr_description_mentions.main([]) - self.assertEqual(1, code) - self.assertIn("@mentions", stderr.getvalue()) - - def test_main_missing_pr_body_env_treated_as_empty(self): - env = {k: v for k, v in os.environ.items() if k != "PR_BODY"} - with mock.patch.dict(os.environ, env, clear=True): - stdout = io.StringIO() - with redirect_stdout(stdout): - code = check_pr_description_mentions.main([]) - self.assertEqual(0, code) - self.assertIn("empty", stdout.getvalue().lower()) - - def test_main_body_file(self): - with tempfile.NamedTemporaryFile("w", encoding="utf-8", delete=False) as handle: - handle.write("safe body with email only: a@b.co\n") - path = handle.name - try: - stdout = io.StringIO() - with redirect_stdout(stdout): - code = check_pr_description_mentions.main(["--body-file", path]) - self.assertEqual(0, code) - self.assertIn("No @mentions found", stdout.getvalue()) - finally: - os.unlink(path) - - def test_main_stdin(self): - stdin = io.StringIO("@bad\n") - with mock.patch.object(check_pr_description_mentions.sys, "stdin", stdin): - with redirect_stdout(io.StringIO()), redirect_stderr(io.StringIO()): - code = check_pr_description_mentions.main(["--stdin"]) - self.assertEqual(1, code) - - -if __name__ == "__main__": - unittest.main()