-
Notifications
You must be signed in to change notification settings - Fork 1
Add option to allow bots reviewing bot code #68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,6 +78,11 @@ on: | |
| required: false | ||
| type: string | ||
| default: 'sei-protocol/sei-core' | ||
| allowed-bots: | ||
| description: "JSON array of GitHub bot logins allowed to request re-reviews. An empty array denies all bots." | ||
| required: false | ||
| type: string | ||
| default: '[]' | ||
| runs-on: | ||
| description: "Runner label." | ||
| required: false | ||
|
|
@@ -120,27 +125,38 @@ jobs: | |
| preflight: | ||
| name: Preflight | ||
| runs-on: ${{ inputs.runs-on }} | ||
| # Comment events are cheap-filtered here, then exact command parsing and authorization | ||
| # happen in `resolve`. Non-command comments never reach a model. | ||
| # Comment events are cheap-filtered here, then exact command parsing and human-team | ||
|
seidroid[bot] marked this conversation as resolved.
|
||
| # or bot-allowlist authorization happen in `resolve`. Non-command comments never | ||
| # reach a model. | ||
| # `allowed-bots` is JSON so `contains` checks array membership rather than substrings. | ||
| if: >- | ||
| ${{ | ||
| github.event_name == 'pull_request' || | ||
| ( | ||
| github.event_name == 'issue_comment' && | ||
| github.event.issue.pull_request && | ||
| github.event.comment.user.type != 'Bot' && | ||
| ( | ||
| github.event.comment.user.type != 'Bot' || | ||
| contains(fromJSON(inputs.allowed-bots), github.event.comment.user.login) | ||
| ) && | ||
| contains(github.event.comment.body, inputs.trigger-phrase) && | ||
| contains(github.event.comment.body, 'review') | ||
| ) || | ||
| ( | ||
| github.event_name == 'pull_request_review_comment' && | ||
| github.event.comment.user.type != 'Bot' && | ||
| ( | ||
| github.event.comment.user.type != 'Bot' || | ||
| contains(fromJSON(inputs.allowed-bots), github.event.comment.user.login) | ||
| ) && | ||
| contains(github.event.comment.body, inputs.trigger-phrase) && | ||
| contains(github.event.comment.body, 'review') | ||
| ) || | ||
| ( | ||
| github.event_name == 'pull_request_review' && | ||
| github.event.review.user.type != 'Bot' && | ||
| ( | ||
| github.event.review.user.type != 'Bot' || | ||
| contains(fromJSON(inputs.allowed-bots), github.event.review.user.login) | ||
| ) && | ||
| github.event.review.body && | ||
| contains(github.event.review.body, inputs.trigger-phrase) && | ||
| contains(github.event.review.body, 'review') | ||
|
|
@@ -203,6 +219,7 @@ jobs: | |
| env: | ||
| TRIGGER_PHRASE: ${{ inputs.trigger-phrase }} | ||
| ALLOWED_TEAM: ${{ inputs.allowed-team }} | ||
| ALLOWED_BOTS: ${{ inputs.allowed-bots }} | ||
| SKIP_REVIEW_LABEL: ${{ inputs.skip-review-label }} | ||
| NITPICK_LABEL: ${{ inputs.nitpick-label }} | ||
| SEIDROID_USER_ID: ${{ vars.PLATFORM_CODE_AGENT_USER_ID }} | ||
|
|
@@ -291,9 +308,11 @@ jobs: | |
| const body = eventName === "pull_request_review" | ||
| ? context.payload.review?.body | ||
| : context.payload.comment?.body; | ||
| const actor = eventName === "pull_request_review" | ||
| ? context.payload.review?.user?.login | ||
| : context.payload.comment?.user?.login; | ||
| const author = eventName === "pull_request_review" | ||
| ? context.payload.review?.user | ||
| : context.payload.comment?.user; | ||
| const actor = author?.login; | ||
| const actorType = author?.type; | ||
| const normalize = value => String(value || "").trim().replace(/\s+/g, " ").toLowerCase(); | ||
| const command = `${normalize(process.env.TRIGGER_PHRASE)} review`; | ||
| if (normalize(body) !== command || !actor) { | ||
|
|
@@ -302,21 +321,33 @@ jobs: | |
| } | ||
|
|
||
| let authorized = false; | ||
| const team = String(process.env.ALLOWED_TEAM || ""); | ||
| const slash = team.indexOf("/"); | ||
| if (slash > 0 && slash < team.length - 1) { | ||
| const org = team.slice(0, slash); | ||
| const team_slug = team.slice(slash + 1); | ||
| try { | ||
| const membership = await github.rest.teams.getMembershipForUserInOrg({ | ||
| org, team_slug, username: actor, | ||
| }); | ||
| authorized = membership.data.state === "active"; | ||
| } catch (error) { | ||
| core.notice(`Could not verify active membership for ${actor} in ${team}; denying request.`); | ||
| if (actorType === "Bot") { | ||
| const configuredBots = JSON.parse(process.env.ALLOWED_BOTS || "[]"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] Relatedly, |
||
| const allowedBots = new Set( | ||
| (Array.isArray(configuredBots) ? configuredBots : []) | ||
| .map(login => String(login).toLowerCase()) | ||
| ); | ||
| authorized = allowedBots.has(actor.toLowerCase()); | ||
| if (!authorized) { | ||
| core.notice(`${actor} is not in allowed-bots; denying request.`); | ||
| } | ||
| } else { | ||
| core.notice("allowed-team is empty or invalid; denying request."); | ||
| const team = String(process.env.ALLOWED_TEAM || ""); | ||
| const slash = team.indexOf("/"); | ||
| if (slash > 0 && slash < team.length - 1) { | ||
| const org = team.slice(0, slash); | ||
| const team_slug = team.slice(slash + 1); | ||
| try { | ||
| const membership = await github.rest.teams.getMembershipForUserInOrg({ | ||
| org, team_slug, username: actor, | ||
| }); | ||
| authorized = membership.data.state === "active"; | ||
| } catch (error) { | ||
| core.notice(`Could not verify active membership for ${actor} in ${team}; denying request.`); | ||
| } | ||
| } else { | ||
| core.notice("allowed-team is empty or invalid; denying request."); | ||
| } | ||
| } | ||
|
|
||
| core.setOutput("should_run", String(authorized)); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[suggestion] This documents the mechanism but not its trust consequence: allowlisting a bot effectively grants re-review triggering to anyone who can make that bot post a comment.
github-actions[bot], for example, can be driven by any workflow in the repo, so allowlisting it delegates the trigger to anyone who can add or modify a workflow. Worth one sentence telling operators to only list bots whose comment bodies are not attacker-controlled.