From 0c9d1fe55200a3f6521f824331fa6a2f12177e4a Mon Sep 17 00:00:00 2001 From: openhands Date: Sat, 23 May 2026 13:33:32 +0000 Subject: [PATCH] fix: restrict github repo monitor triggers by login Co-authored-by: openhands --- skills/github-repo-monitor/README.md | 5 +- skills/github-repo-monitor/SKILL.md | 41 ++++++++++++---- skills/github-repo-monitor/scripts/main.py | 48 ++++++++++++++++++- skills/github-repo-monitor/tests/test_main.py | 35 ++++++++++++++ 4 files changed, 115 insertions(+), 14 deletions(-) diff --git a/skills/github-repo-monitor/README.md b/skills/github-repo-monitor/README.md index fce46fe..bb91777 100644 --- a/skills/github-repo-monitor/README.md +++ b/skills/github-repo-monitor/README.md @@ -36,8 +36,8 @@ Just tell OpenHands: > *"Set up a GitHub repository monitor for `owner/repo`"* -The skill will walk through token verification, event-type selection, cron -schedule, and automation creation automatically. +The skill will walk through token verification, allowed-login selection, +event-type selection, cron schedule, and automation creation automatically. ## Requirements @@ -52,6 +52,7 @@ schedule, and automation creation automatically. |--------|---------|-------------| | Repository | (required) | `owner/repo` format | | Trigger phrase | `@OpenHands` | Case-insensitive string to watch for in comments | +| Allowed GitHub logins | token owner only | Who may trigger conversations; use explicit logins or `*` for any non-bot commenter | | Event types | `issue_comment` | `issue_comment`, `pr_review_comment`, or both | | Cron schedule | `* * * * *` | Every minute; any valid 5-field cron expression | diff --git a/skills/github-repo-monitor/SKILL.md b/skills/github-repo-monitor/SKILL.md index 9ee83c6..7de2e8f 100644 --- a/skills/github-repo-monitor/SKILL.md +++ b/skills/github-repo-monitor/SKILL.md @@ -118,7 +118,25 @@ Accepted values: any non-empty string unlikely to appear by accident. Record as `TRIGGER_PHRASE`. Default: `"@openhands"`. -### Step 4 - Collect event types +### Step 4 - Collect allowed GitHub logins + +Ask the user: *"Which GitHub users may trigger this automation? +Press Enter to allow only the authenticated `GITHUB_TOKEN` owner. +You may also provide comma-separated GitHub logins, or `*` to allow any +non-bot commenter on the monitored repository."* + +Map the answer to `ALLOWED_GITHUB_LOGINS`: + +| User answer | `ALLOWED_GITHUB_LOGINS` value | +|---|---| +| Empty/default | `[""]` | +| `enyst,tofarr` | `["enyst", "tofarr"]` | +| `*` | `["*"]` | + +Default to token-owner-only unless the user explicitly chooses a broader +allowlist. Record as `ALLOWED_GITHUB_LOGINS`. + +### Step 5 - Collect event types Ask the user: *"Which event types should be monitored? Choose one or more:* @@ -135,7 +153,7 @@ Map the choice to the `EVENT_TYPES` list: | 2 | `["pr_review_comment"]` | | 3 | `["issue_comment", "pr_review_comment"]` | -### Step 5 - Collect cron schedule +### Step 6 - Collect cron schedule Ask the user: *"How often should the automation poll GitHub? (Press Enter for the default: every minute. @@ -147,9 +165,9 @@ Default: `* * * * *` (every minute). Record as `CRON_SCHEDULE`. -### Step 6 - Generate the automation script +### Step 7 - Generate the automation script -Read `scripts/main.py` from this skill's directory. Apply exactly four +Read `scripts/main.py` from this skill's directory. Apply exactly five constant substitutions near the top of the file: | Placeholder | Replace with | @@ -157,6 +175,7 @@ constant substitutions near the top of the file: | `REPO = "owner/repo"` | `REPO = "{owner_repo}"` | | `TRIGGER_PHRASE = "@openhands"` | `TRIGGER_PHRASE = "{trigger_phrase_lower}"` | | `EVENT_TYPES = ["issue_comment"]` | `EVENT_TYPES = {event_types_list}` | +| `ALLOWED_GITHUB_LOGINS = [""]` | `ALLOWED_GITHUB_LOGINS = {allowed_logins_list}` | | `DEFAULT_OPENHANDS_URL = "http://localhost:8000"` | `DEFAULT_OPENHANDS_URL = "{url}"` (keep default if the user has no preference) | Write the customised script to a temporary build directory: @@ -172,7 +191,7 @@ python3 -m py_compile /tmp/github-monitor-build/main.py && echo "Syntax OK" Fix any syntax errors before proceeding. -### Step 7 - Package and upload +### Step 8 - Package and upload ```bash tar -czf /tmp/github-monitor.tar.gz -C /tmp/github-monitor-build . @@ -189,7 +208,7 @@ TARBALL_PATH=$(curl -s -X POST \ echo "Uploaded: $TARBALL_PATH" ``` -### Step 8 - Create the automation +### Step 9 - Create the automation ```bash curl -s -X POST "${OPENHANDS_HOST}/api/automation/v1" \ @@ -206,7 +225,7 @@ curl -s -X POST "${OPENHANDS_HOST}/api/automation/v1" \ Record the returned `id`. -### Step 9 - Confirm +### Step 10 - Confirm Tell the user: @@ -216,12 +235,13 @@ Tell the user: > - Repository: `{owner}/{repo}` > - Trigger phrase: `{phrase}` > - Event types: `{event_types}` +> - Allowed GitHub logins: `{allowed_logins}` > - Polling schedule: `{cron_schedule}` > - State file: `~/.openhands/workspaces/automation-state/github_poller_{id}.json` > -> Post a comment containing `{phrase}` on any issue or PR in `{owner}/{repo}` -> to test it. OpenHands will acknowledge with a comment and a link to the -> new conversation. +> From an allowed GitHub login, post a comment containing `{phrase}` on any +> issue or PR in `{owner}/{repo}` to test it. OpenHands will acknowledge with +> a comment and a link to the new conversation. --- @@ -237,6 +257,7 @@ Each cron run executes `main.py`, which: 4. **Processes matching comments** in chronological order: - Skips bot accounts (login ending in `[bot]`) to avoid feedback loops. - Skips already-processed comment IDs. + - Skips comments from logins outside `ALLOWED_GITHUB_LOGINS`. - Checks body for the trigger phrase (case-insensitive). - Extracts the issue/PR number from the comment URL. 5. **For each trigger comment**, per issue/PR: diff --git a/skills/github-repo-monitor/scripts/main.py b/skills/github-repo-monitor/scripts/main.py index c5125e0..c0548e8 100644 --- a/skills/github-repo-monitor/scripts/main.py +++ b/skills/github-repo-monitor/scripts/main.py @@ -40,6 +40,9 @@ REPO = "owner/repo" # e.g. "microsoft/vscode" TRIGGER_PHRASE = "@openhands" # case-insensitive EVENT_TYPES = ["issue_comment"] # e.g. ["issue_comment", "pr_review_comment"] +# Who may trigger conversations. Default is the authenticated GITHUB_TOKEN owner. +# Use ["*"] to allow any non-bot commenter, or explicit logins like ["octocat"]. +ALLOWED_GITHUB_LOGINS = [""] DEFAULT_OPENHANDS_URL = "http://localhost:8000" # Context: number of recent issue/PR comments to include in the initial prompt. @@ -420,13 +423,44 @@ def conversation_final_response(agent_url: str, api_key: str, conv_id: str) -> s # ── Comment filtering helpers ───────────────────────────────────────────────── +def _comment_author_login(comment: dict) -> str: + """Return the GitHub login for the comment author, if present.""" + user = comment.get("user") or {} + return (user.get("login") or "").strip() + + def _is_bot_comment(comment: dict) -> bool: """Return True if the comment was posted by a bot account.""" user = comment.get("user") or {} - login = user.get("login", "") + login = _comment_author_login(comment) return login.endswith("[bot]") or user.get("type") == "Bot" +def _allowed_login_set(token_owner_login: str) -> set[str]: + """Resolve configured login allowlist, including the token-owner sentinel.""" + token_owner = token_owner_login.strip().lower() + allowed: set[str] = set() + for login in ALLOWED_GITHUB_LOGINS: + normalized = login.strip().lower() + if not normalized: + continue + if normalized == "": + if token_owner: + allowed.add(token_owner) + continue + allowed.add(normalized) + return allowed + + +def _is_allowed_comment_author(comment: dict, token_owner_login: str) -> bool: + """Return True if this comment author is allowed to trigger conversations.""" + author = _comment_author_login(comment).lower() + if not author: + return False + allowed = _allowed_login_set(token_owner_login) + return "*" in allowed or author in allowed + + def _has_trigger(comment: dict, phrase: str) -> bool: """Return True if the comment body contains *phrase* (case-insensitive).""" body = (comment.get("body") or "").strip() @@ -685,7 +719,7 @@ def main() -> None: api_key = _get_env_key() github_token = _resolve_github_token() - _verify_token_and_repo(github_token, REPO) + token_owner_login = _verify_token_and_repo(github_token, REPO) try: openhands_url = get_secret("OPENHANDS_URL").rstrip("/") or DEFAULT_OPENHANDS_URL @@ -736,6 +770,16 @@ def main() -> None: processed_set.add(comment_id) continue + if not _is_allowed_comment_author(comment, token_owner_login): + if _has_trigger(comment, TRIGGER_PHRASE): + author = _comment_author_login(comment) or "unknown" + print( + f" Skipping trigger comment {comment_id} " + f"from unauthorized user @{author}" + ) + processed_set.add(comment_id) + continue + if not _has_trigger(comment, TRIGGER_PHRASE): processed_set.add(comment_id) continue diff --git a/skills/github-repo-monitor/tests/test_main.py b/skills/github-repo-monitor/tests/test_main.py index f5b4c20..d5f5677 100644 --- a/skills/github-repo-monitor/tests/test_main.py +++ b/skills/github-repo-monitor/tests/test_main.py @@ -127,6 +127,41 @@ def test_login_containing_but_not_ending_with_bot(self): )) +# ── Author authorization tests ──────────────────────────────────────────────── + +class TestAllowedCommentAuthor(unittest.TestCase): + + def setUp(self): + self._original_allowed = main.ALLOWED_GITHUB_LOGINS + + def tearDown(self): + main.ALLOWED_GITHUB_LOGINS = self._original_allowed + + def test_default_token_owner_allows_owner(self): + main.ALLOWED_GITHUB_LOGINS = [""] + comment = _make_comment(login="OctoCat") + self.assertTrue(main._is_allowed_comment_author(comment, "octocat")) + + def test_default_token_owner_rejects_other_user(self): + main.ALLOWED_GITHUB_LOGINS = [""] + comment = _make_comment(login="enyst") + self.assertFalse(main._is_allowed_comment_author(comment, "tofarr")) + + def test_explicit_allowlist_is_case_insensitive(self): + main.ALLOWED_GITHUB_LOGINS = ["Enyst", "tofarr"] + comment = _make_comment(login="enyst") + self.assertTrue(main._is_allowed_comment_author(comment, "someone-else")) + + def test_wildcard_allows_any_commenter(self): + main.ALLOWED_GITHUB_LOGINS = ["*"] + comment = _make_comment(login="anyone") + self.assertTrue(main._is_allowed_comment_author(comment, "octocat")) + + def test_missing_author_is_rejected(self): + main.ALLOWED_GITHUB_LOGINS = ["*"] + self.assertFalse(main._is_allowed_comment_author({}, "octocat")) + + # ── Trigger phrase tests ─────────────────────────────────────────────────────── class TestHasTrigger(unittest.TestCase):