Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .github/seidroid/ai-review/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ jobs:
with:
uci-ref: v1
# allowed-team: my-org/my-team # default: sei-protocol/sei-core
# allowed-bots: '["dependabot[bot]", "renovate[bot]"]' # exact logins; default: []
# extra-instructions: "Flag added allocations in the hot path."
# prebuild-script: "go mod download" # warm Codex's offline sandbox
# re-review-on-push: true # review again after every PR push
Expand All @@ -72,6 +73,7 @@ jobs:
| `nitpick-label` | `ai: nitpick` | Include nit-level findings only when this label is present. |
| `trigger-phrase` | `@seidroid` | Exact `<trigger-phrase> review` command used to request another review. |
| `allowed-team` | `sei-protocol/sei-core` | Active members may request another review. Empty denies everyone. |
| `allowed-bots` | `'[]'` | JSON array of exact GitHub bot logins allowed to request another review. An empty array denies all bots. |
| `runs-on` | `ubuntu-latest` | Runner label. |
| `claude-model` | `''` | Optional Claude model override. |
| `approve-on-success` | `true` | If true, APPROVE on a clean verdict; else COMMENT. |
Expand Down Expand Up @@ -135,8 +137,10 @@ jobs:
and a read-only token, so the workflow degrades gracefully for forks; do not switch to
`pull_request_target` to "fix" forks.
- An exact `@seidroid review` comment is reserved for the review workflow, so the assistant
ignores it. Re-review requests are accepted only from active members of `allowed-team`;
membership lookup errors fail closed.
ignores it. Human re-review requests are accepted only from active members of
`allowed-team`; bot requests require an exact, case-insensitive login match in
the `allowed-bots` JSON array. Non-allowlisted bots are rejected before a runner starts.
Membership lookup errors and empty allowlists fail closed.

Copy link
Copy Markdown

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.

- The assistant is gated to active members of `allowed-team` (checked before any model
runs) and ignores bot-authored comments. It is read-only unless `allow-write` is set.
- Untrusted PR/comment content is passed to the models as **data**, never interpolated
Expand Down
73 changes: 52 additions & 21 deletions .github/workflows/ai-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Comment thread
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')
Expand Down Expand Up @@ -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 }}
Expand Down Expand Up @@ -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) {
Expand All @@ -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 || "[]");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] JSON.parse is unguarded while the very next lines defensively handle a non-array result — the asymmetry is odd, since a malformed allowed-bots string is the more likely operator error and it throws here, failing the step rather than denying cleanly with a notice. A try/catch defaulting to [] (plus a core.notice) would match the fail-closed style used for the allowed-team branch.

Relatedly, fromJSON(inputs.allowed-bots) in the preflight if: has no fallback at all, so a malformed value errors the expression for any bot-authored comment event.

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));
Expand Down
Loading