Harden GitHub handle extraction and verify account exists before inviting

src/nf_core_bot/commands/github/add_member_shortcut.py

@@ -26,12 +26,13 @@
 
 logger = logging.getLogger(__name__)
 
-# Pattern to extract the GitHub handle from workflow messages.
-# Matches lines like:
-#   *Which is your GitHub handle?*\nMuteebaAzhar
-#   Which is your GitHub handle?\nMuteebaAzhar
+# Pattern to capture the *answer* to the GitHub-handle question in workflow
+# messages. We grab the whole rest of the answer line (not just the first
+# token) because people often write a sentence rather than a bare handle, e.g.
+#   *Which is your GitHub handle?*\nMy GitHub username is @octocat.
+# The answer is then mined for a real handle by _handle_from_answer().
 _GITHUB_HANDLE_RE = re.compile(
-    r"(?:\*)?(?:Which is your GitHub handle\??|GitHub handle\??)(?:\*)?[\n\r]+\s*(\S+)",
+    r"(?:\*)?(?:Which is your GitHub handle\??|GitHub handle\??)(?:\*)?[\n\r]+[ \t]*([^\n\r]+)",
     re.IGNORECASE,
 )
 
@@ -40,11 +41,37 @@
 _WORKFLOW_REQUESTER_RE = re.compile(r"By\s+<@(U[A-Z0-9]+)(?:\|[^>]*)?>", re.IGNORECASE)
 
 
+def _handle_from_answer(answer: str) -> str | None:
+    """Mine a single GitHub handle out of a free-text answer.
+
+    People rarely answer "Which is your GitHub handle?" with a bare username —
+    they write things like "My GitHub username is @octocat." Picking the first
+    whitespace-delimited token in that case invites the wrong account (e.g.
+    ``My`` → github.com/my). So we look for an unambiguous signal instead: an
+    ``@mention`` or a ``github.com/<user>`` URL token, falling back to a lone
+    token when the whole answer is just one word. Each candidate is handed to
+    ``normalise_github_username`` for stripping and validation.
+
+    Multi-word prose with no ``@`` or URL is ambiguous — we refuse to guess and
+    return ``None`` so the caller falls back to the explicit slash command.
+    """
+    tokens = answer.split()
+
+    for token in tokens:
+        if token.startswith("@") or "github.com/" in token.lower():
+            return normalise_github_username(token)
+
+    if len(tokens) == 1:
+        return normalise_github_username(tokens[0])
+
+    return None
+
+
 def _extract_github_handle_from_text(text: str) -> str | None:
     """Try to extract and validate a GitHub handle from workflow message text."""
     match = _GITHUB_HANDLE_RE.search(text)
     if match:
-        return normalise_github_username(match.group(1))
+        return _handle_from_answer(match.group(1))
     return None
 
 

src/nf_core_bot/commands/github/invite_flow.py

@@ -5,7 +5,7 @@
 import logging
 from typing import TYPE_CHECKING
 
-from nf_core_bot.checks.github import add_to_team, check_org_membership, invite_to_org
+from nf_core_bot.checks.github import add_to_team, check_org_membership, check_user_exists, invite_to_org
 
 if TYPE_CHECKING:
     from collections.abc import Awaitable, Callable
@@ -62,7 +62,29 @@ async def invite_and_greet(
 
     Returns ``True`` on success.
     """
-    # ── 0. Skip if already an org member ─────────────────────────────
+    # ── 0a. Verify the account actually exists ───────────────────────
+    # Guards against typos and prose-derived handles inviting the wrong
+    # person (e.g. a stray word that happens to be a real username).
+    try:
+        exists = await check_user_exists(github_username)
+    except Exception as e:
+        logger.exception("Network error checking GitHub user %s", github_username)
+        msg = (
+            f"Failed to reach the GitHub API while checking whether `{github_username}` exists "
+            f"(`{type(e).__name__}: {e}`). Please try again later."
+        )
+        await _reply_or_dm(reply, client, caller_user_id, msg)
+        return False
+
+    if not exists.ok:
+        msg = (
+            f"No GitHub user `{github_username}` found — double-check the handle and try again "
+            f"with `/nf-core github add <github-username>`."
+        )
+        await _reply_or_dm(reply, client, caller_user_id, msg)
+        return False
+
+    # ── 0b. Skip if already an org member ────────────────────────────
     # Calling invite_to_org on an existing member with role=member would
     # downgrade admins/owners, so bail out early.
     try:

tests/conftest.py

@@ -4,12 +4,15 @@
 
 import os
 from typing import TYPE_CHECKING, Any
+from unittest.mock import AsyncMock, patch
 
 import pytest
 from moto import mock_aws
 
+from nf_core_bot.checks.github import GitHubResult
+
 if TYPE_CHECKING:
-    from collections.abc import AsyncIterator
+    from collections.abc import AsyncIterator, Iterator
 
 
 @pytest.fixture(autouse=True)
@@ -40,3 +43,17 @@ async def ddb_table() -> AsyncIterator[Any]:
         db_client.init(table_name="test-table", endpoint_url=None, region="us-east-1")
         yield db_client.get_table()
         db_client._table = None
+
+
+@pytest.fixture(autouse=True)
+def _user_exists() -> Iterator[AsyncMock]:
+    """Default the GitHub user-existence check to "exists" for every test.
+
+    The invite flow calls ``check_user_exists`` before inviting; tests that
+    care about the "user not found" path override this mock's return value.
+    """
+    with patch(
+        "nf_core_bot.commands.github.invite_flow.check_user_exists",
+        return_value=GitHubResult(ok=True, message="exists"),
+    ) as mock:
+        yield mock

tests/test_add_member.py

@@ -103,6 +103,38 @@ async def test_url_as_username_normalised(self, _perm: AsyncMock) -> None:
             mock_org.assert_awaited_once_with("octocat")
 
 
+# ── Nonexistent GitHub account ───────────────────────────────────────
+
+
+class TestUserDoesNotExist:
+    @patch("nf_core_bot.commands.github.add_member.is_core_team", return_value=True)
+    @patch("nf_core_bot.commands.github.invite_flow.check_org_membership")
+    @patch("nf_core_bot.commands.github.invite_flow.invite_to_org")
+    async def test_missing_user_not_invited(
+        self,
+        mock_org: AsyncMock,
+        mock_check: AsyncMock,
+        _perm: AsyncMock,
+        _user_exists: AsyncMock,
+    ) -> None:
+        """A handle that resolves to no GitHub account must not be invited."""
+        _user_exists.return_value = GitHubResult(ok=False, message="does not exist")
+
+        ack = AsyncMock()
+        respond = AsyncMock()
+        client = make_slack_client()
+
+        await handle_add_member(ack, respond, client, "U_ADMIN", _command(), ["my"])
+
+        _user_exists.assert_awaited_once_with("my")
+        # Bail before touching membership or invite.
+        mock_check.assert_not_awaited()
+        mock_org.assert_not_awaited()
+
+        all_texts = channel_messages(client) + dm_messages(client)
+        assert any("No GitHub user `my` found" in t for t in all_texts)
+
+
 # ── Slack mention argument ───────────────────────────────────────────
 
 

tests/test_add_member_shortcut.py

@@ -234,6 +234,25 @@ def test_empty_answer_returns_none(self) -> None:
         text = "Which is your GitHub handle?\n"
         assert _extract_github_handle_from_text(text) is None
 
+    def test_prose_answer_with_at_mention(self) -> None:
+        # First token is a stray word ("My") — pick the @-mentioned handle.
+        text = "Which is your GitHub handle?\nMy GitHub username is @Harshita-sriv."
+        assert _extract_github_handle_from_text(text) == "Harshita-sriv"
+
+    def test_prose_answer_with_bare_username_rejected(self) -> None:
+        # No @-mention or URL to disambiguate — refuse to guess rather than
+        # invite the first word.
+        text = "Which is your GitHub handle?\nMy username is octocat"
+        assert _extract_github_handle_from_text(text) is None
+
+    def test_prose_answer_with_url(self) -> None:
+        text = "Which is your GitHub handle?\nIt's https://github.com/octocat thanks!"
+        assert _extract_github_handle_from_text(text) == "octocat"
+
+    def test_at_mention_only(self) -> None:
+        text = "*Which is your GitHub handle?*\n@octocat"
+        assert _extract_github_handle_from_text(text) == "octocat"
+
 
 class TestExtractRequester:
     """Unit tests for _extract_requester_from_text."""
@@ -294,6 +313,32 @@ async def test_workflow_message_no_handle_shows_error(self, _perm: AsyncMock) ->
         text = client.chat_postEphemeral.call_args.kwargs["text"]
         assert "Couldn't find a GitHub username" in text
 
+    @patch("nf_core_bot.commands.github.add_member_shortcut.is_core_team", return_value=True)
+    @patch("nf_core_bot.commands.github.invite_flow.check_org_membership")
+    @patch("nf_core_bot.commands.github.invite_flow.invite_to_org")
+    async def test_workflow_message_nonexistent_user_not_invited(
+        self,
+        mock_org: AsyncMock,
+        mock_check: AsyncMock,
+        _perm: AsyncMock,
+        _user_exists: AsyncMock,
+    ) -> None:
+        """An extracted handle for a nonexistent account must not be invited."""
+        _user_exists.return_value = GitHubResult(ok=False, message="does not exist")
+
+        ack = AsyncMock()
+        client = make_slack_client()
+
+        sc = _workflow_shortcut("*Which is your GitHub handle?*\n@ghost-account-xyz")
+        await handle_add_member_shortcut(ack, sc, client)
+
+        _user_exists.assert_awaited_once_with("ghost-account-xyz")
+        mock_check.assert_not_awaited()
+        mock_org.assert_not_awaited()
+
+        all_texts = channel_messages(client) + dm_messages(client)
+        assert any("No GitHub user `ghost-account-xyz` found" in t for t in all_texts)
+
 
 # ── GitHub API error handling ────────────────────────────────────────