Add author-permission gating for slash commands#13
Conversation
- GitHubClient.get_user_permission: query collaborator permission level - authorization.py: is_authorized(), check_event_authorization() with configurable min_level and optional denial comment - Export authorization helpers from __init__.py - 16 new tests (all 131 pass)
Keramikus-97
left a comment
Keramikus-97
left a comment
There was a problem hiding this comment.
Devin Review found 2 potential issues.
View 2 additional findings in Devin Review.
- Broaden exception handling in check_event_authorization to catch transport-level errors (httpx.TimeoutException, etc.), not just GitHubAPIError - Update get_user_permission docstring to include maintain and triage levels
Keramikus-97
left a comment
Keramikus-97
left a comment
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
View 3 additional findings in Devin Review.
| async def get_user_permission(self, owner: str, repo: str, username: str) -> str: | ||
| """Return the permission level of *username* on *owner/repo*. | ||
|
|
||
| Returns one of ``"admin"``, ``"maintain"``, ``"write"``, | ||
| ``"triage"``, ``"read"``, or ``"none"``. | ||
| """ | ||
| data = await self._request( | ||
| "GET", f"/repos/{owner}/{repo}/collaborators/{username}/permission" | ||
| ) | ||
| return data.get("permission", "none") |
There was a problem hiding this comment.
🚩 Permission rank map includes levels that may not be returned by the API field being read
The _PERMISSION_RANK dict at src/opencode_github/authorization.py:14-21 defines 6 levels including "maintain" and "triage". The get_user_permission method reads data["permission"] from the GitHub API response (src/opencode_github/github_client.py:146). The GitHub REST API's permission field on the collaborator permission endpoint historically returns only the 4-level model ("admin", "write", "read", "none"), while the more granular "maintain" and "triage" levels are only available via the role_name response field. If this is still the case, then setting min_level="maintain" would incorrectly deny maintain-role users (they'd get "write" from the API, rank 2 < rank 3). The docstring for get_user_permission claims it can return "maintain" and "triage", which would be inaccurate. This needs verification against the current GitHub API behavior for the 2022-11-28 API version.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Currently any GitHub user can trigger the OpenCode agent via
/ocon public repos — the workflow only checkscontains(comment.body, '/oc')with no author validation. This is a security gap since the action runs withcontents: writeandpull-requests: writepermissions.This PR adds permission gating so commands are only processed for users with sufficient repository access.
Key additions:
GitHubClient.get_user_permission(owner, repo, username)→ callsGET /repos/{owner}/{repo}/collaborators/{username}/permission, returns"admin"|"write"|"read"|"none".authorization.pywith three public helpers:permission_rank(level)— maps permission strings to numeric ranks (admin=4 > maintain=3 > write=2 > triage=1 > read=0 > none=-1)is_authorized(permission, min_level="write")— pure comparisoncheck_event_authorization(client, event, min_level, post_denial)— end-to-end check that queries the API, and optionally posts a denial comment when unauthorizedUsage:
On API errors, defaults to deny. Denial comment posting is best-effort (swallows exceptions). 16 new tests, all 131 pass.
Link to Devin session: https://app.devin.ai/sessions/b6df963dc7b44c99b2d49923cde4926a
Requested by: @Keramikus-97