From c29fe1ac5e2272c445fda02c7061e181200e8413 Mon Sep 17 00:00:00 2001 From: Michael Wu Date: Sat, 15 Aug 2026 14:15:24 +0900 Subject: [PATCH 1/6] feat: allow GitHub team trigger filters --- .../.paseo/workflows/github.yml | 2 +- examples/single-repo-team-bot/README.md | 9 +- src/config/compiler.test.ts | 39 ++++++ src/config/compiler.ts | 19 ++- src/providers/github/index.ts | 3 + src/triggers/github/match.test.ts | 125 ++++++++++++++---- src/triggers/github/match.ts | 118 ++++++++++++----- src/triggers/github/provider.test.ts | 60 ++++++++- src/triggers/github/provider.ts | 13 +- src/triggers/github/team-membership.test.ts | 70 ++++++++++ src/triggers/github/team-membership.ts | 76 +++++++++++ src/workflows/reactions.test.ts | 8 ++ 12 files changed, 474 insertions(+), 68 deletions(-) create mode 100644 src/triggers/github/team-membership.test.ts create mode 100644 src/triggers/github/team-membership.ts diff --git a/examples/single-repo-team-bot/.paseo/workflows/github.yml b/examples/single-repo-team-bot/.paseo/workflows/github.yml index 1e413791..29fa146d 100644 --- a/examples/single-repo-team-bot/.paseo/workflows/github.yml +++ b/examples/single-repo-team-bot/.paseo/workflows/github.yml @@ -3,7 +3,7 @@ on: github.issue_comment max_runtime: 2h filters: repo: your-org/your-repo - from_users: [your-github-login] + from_teams: [your-org/your-team] contains: "@your-bot" steps: - id: classify diff --git a/examples/single-repo-team-bot/README.md b/examples/single-repo-team-bot/README.md index 21c8d149..6cbb15b7 100644 --- a/examples/single-repo-team-bot/README.md +++ b/examples/single-repo-team-bot/README.md @@ -15,11 +15,14 @@ Copy `.paseo` to your repository root, then replace these values: | `your-github-connection` | GitHub connection slug in Hub | | `YOUR_DISCORD_USER_ID` | Discord user allowed to trigger runs | | `YOUR_SLACK_USER_ID` | Slack user allowed to trigger runs | -| `your-github-login` | GitHub user allowed to trigger runs | +| `your-org/your-team` | GitHub team allowed to trigger runs | | `@your-bot` | Mention that starts the GitHub workflow | -Connect Discord, Slack, and GitHub to the project before enabling their triggers. Keep the user -allowlists narrow; wildcards are not supported. +Connect Discord, Slack, and GitHub to the project before enabling their triggers. GitHub team +filters use `organization/team-slug` and require the GitHub App's organization **Members** +permission with read access. If membership cannot be checked, Hub does not start a run. A GitHub +`from_users` allowlist can be combined with `from_teams`; either grants access. Keep allowlists +narrow; wildcards are not supported. ## Deploy diff --git a/src/config/compiler.test.ts b/src/config/compiler.test.ts index 0512f362..8b4d88ac 100644 --- a/src/config/compiler.test.ts +++ b/src/config/compiler.test.ts @@ -286,6 +286,45 @@ describe("workflow compiler", () => { }); }); + it("supports GitHub team trigger allowlists and rejects invalid team filter uses", () => { + const raw = configuration(); + const trigger = raw.triggers[0]!; + const githubTrigger = { + ...trigger, + on: "github.issue_comment", + filters: { from_teams: ["getpaseo/maintainers"] }, + }; + + assert.deepEqual(compileHubConfig({ ...raw, triggers: [githubTrigger] }).triggers[0]?.filters, { + from_teams: ["getpaseo/maintainers"], + }); + assert.throws( + () => + compileHubConfig({ + ...raw, + triggers: [{ ...trigger, on: "slack.mention", filters: githubTrigger.filters }], + }), + /filters\.from_teams only for GitHub events/iu, + ); + assert.throws( + () => + compileHubConfig({ + ...raw, + triggers: [ + { + ...githubTrigger, + filters: { from_teams: ["maintainers"] }, + }, + ], + }), + /organization\/team-slug/iu, + ); + assert.throws( + () => compileHubConfig({ ...raw, triggers: [{ ...trigger, on: "github.issue_comment" }] }), + /filters\.from_users or filters\.from_teams/iu, + ); + }); + it("requires explicit repositories for non-GitHub authority", () => { const raw = configuration(); const step = raw.triggers[0]!.steps[0]!; diff --git a/src/config/compiler.ts b/src/config/compiler.ts index 7049ea51..94dc5b19 100644 --- a/src/config/compiler.ts +++ b/src/config/compiler.ts @@ -29,6 +29,7 @@ const EVENT_NAME = /^[a-z][a-z0-9_-]*\.[a-z][a-z0-9_-]*$/u; const DURATION = /^([1-9][0-9]*)(ms|s|m|h)$/u; const MAX_DURATION_MS = 24 * 60 * 60_000; const INPUT_NAME = /^[a-z][a-z0-9_-]*$/u; +const GITHUB_TEAM_REFERENCE = /^[^/\s]+\/[^/\s]+$/u; const DYNAMIC_INPUT_REFERENCE = /^\$\{\{\s*paseo\.inputs\.([a-z][a-z0-9_-]*)\s*\}\}$/u; const EXPRESSION_START = "${{"; const EXPRESSION_END = "}}"; @@ -89,6 +90,9 @@ const AuthoredTriggerFilterSchema = z workspace: z.string().min(1).optional(), channels: z.array(z.string().min(1)).optional(), from_users: z.array(z.string().min(1)).optional(), + from_teams: z + .array(z.string().regex(GITHUB_TEAM_REFERENCE, "must be formatted as organization/team-slug")) + .optional(), inputs: z.record(z.string(), InputValueSchema).optional(), connection: z .string() @@ -243,9 +247,10 @@ export interface CompiledStep { export type CompiledSteps = readonly CompiledStep[]; export type CompiledTriggerFilter = Readonly< - Omit & { + Omit & { channels?: readonly string[] | undefined; from_users?: readonly string[] | undefined; + from_teams?: readonly string[] | undefined; inputs?: Readonly> | undefined; connectionId?: string | undefined; resourceId?: string | undefined; @@ -1312,10 +1317,18 @@ function validateAuthoredIds(config: AuthoredHubConfig): void { } function validateTriggerLaunchSecurity(trigger: CompiledTrigger): void { + const fromTeams = trigger.filters?.from_teams ?? []; + if (fromTeams.length > 0 && !trigger.on.startsWith("github.")) { + throw new Error(`trigger ${trigger.name} may use filters.from_teams only for GitHub events`); + } if (trigger.on === "manual.run") return; - if ((trigger.filters?.from_users?.length ?? 0) === 0) { + const fromUsers = trigger.filters?.from_users ?? []; + if (fromUsers.length === 0 && fromTeams.length === 0) { + const allowlist = trigger.on.startsWith("github.") + ? "filters.from_users or filters.from_teams" + : "filters.from_users"; throw new Error( - `trigger ${trigger.name} requires a non-empty filters.from_users allowlist for externally sourced events`, + `trigger ${trigger.name} requires a non-empty ${allowlist} allowlist for externally sourced events`, ); } } diff --git a/src/providers/github/index.ts b/src/providers/github/index.ts index 7166e27e..3aa323be 100644 --- a/src/providers/github/index.ts +++ b/src/providers/github/index.ts @@ -24,6 +24,7 @@ import { createGitHubTriggerProvider, type GitHubReactionClient, } from "../../triggers/github/provider.js"; +import { createGitHubTeamMembershipClient } from "../../triggers/github/team-membership.js"; import { createWebhookSource } from "../../triggers/github/webhook.js"; import { createGitHubConnectionClient, @@ -81,6 +82,7 @@ export function createGitHubRegistration( let configurationForProject: ((projectId: string) => ProjectConfigurationStore) | undefined; const appAuth = options.appAuth ?? createGitHubAuth(); + const teamMemberships = createGitHubTeamMembershipClient(appAuth); const client = options.connectionClient ?? createGitHubConnectionClient({ @@ -207,6 +209,7 @@ export function createGitHubRegistration( return createGitHubTriggerProvider({ configurationStoreForProject, reactions, + teamMemberships, }); }, ], diff --git a/src/triggers/github/match.test.ts b/src/triggers/github/match.test.ts index ee15c3c1..692232f5 100644 --- a/src/triggers/github/match.test.ts +++ b/src/triggers/github/match.test.ts @@ -3,32 +3,36 @@ import { describe, it } from "vitest"; import { compileHubConfig } from "../../config/index.js"; import type { NormalizedGitHubEvent } from "../../auth/github-events.js"; import { matchTriggers } from "./match.js"; +import type { GitHubTeamMembershipClient } from "./team-membership.js"; describe("GitHub trigger matching", () => { - it("matches the compiled one-step trigger by repository, text, and actor", () => { + it("matches the compiled one-step trigger by repository, text, and actor", async () => { const config = configFor({ repo: "boudra/faro", contains: "@paseo", from_users: ["boudra"] }); - const matches = matchTriggers(config, createEvent()); + const matches = await matchTriggers(config, createEvent(), { teamMemberships: denyTeams }); assert.equal(matches.length, 1); assert.equal(matches[0]?.trigger.name, "github-comment"); }); - it("matches pattern-less comments while preserving the provider allowlist", () => { + it("matches pattern-less comments while preserving the provider allowlist", async () => { const config = configFor({ repo: "boudra/faro", from_users: ["boudra"] }); assert.equal( - matchTriggers( - config, - createEvent({ - payload: { - comment: { id: 123, body: "please explain", user: { login: "boudra" } }, - sender: { login: "boudra" }, - }, - }), + ( + await matchTriggers( + config, + createEvent({ + payload: { + comment: { id: 123, body: "please explain", user: { login: "boudra" } }, + sender: { login: "boudra" }, + }, + }), + { teamMemberships: denyTeams }, + ) ).length, 1, ); assert.deepEqual( - matchTriggers( + await matchTriggers( config, createEvent({ payload: { @@ -36,28 +40,37 @@ describe("GitHub trigger matching", () => { sender: { login: "stranger" }, }, }), + { teamMemberships: denyTeams }, ), [], ); }); - it("keeps repository and pattern filters literal", () => { + it("keeps repository and pattern filters literal", async () => { const config = configFor({ repo: "boudra/faro", pattern: "@paseo", from_users: ["boudra"] }); assert.equal( - matchTriggers( - config, - createEvent({ - payload: { - comment: { id: 123, body: "@paseo please explain", user: { login: "boudra" } }, - sender: { login: "boudra" }, - }, - }), + ( + await matchTriggers( + config, + createEvent({ + payload: { + comment: { id: 123, body: "@paseo please explain", user: { login: "boudra" } }, + sender: { login: "boudra" }, + }, + }), + { teamMemberships: denyTeams }, + ) ).length, 1, ); - assert.deepEqual(matchTriggers(config, createEvent({ repo: "elsewhere/repo" })), []); assert.deepEqual( - matchTriggers( + await matchTriggers(config, createEvent({ repo: "elsewhere/repo" }), { + teamMemberships: denyTeams, + }), + [], + ); + assert.deepEqual( + await matchTriggers( config, createEvent({ payload: { @@ -65,12 +78,13 @@ describe("GitHub trigger matching", () => { sender: { login: "boudra" }, }, }), + { teamMemberships: denyTeams }, ), [], ); }); - it("applies the same security filters to pull-request review comments", () => { + it("applies the same security filters to pull-request review comments", async () => { const config = compileHubConfig({ environments: [{ name: "runner", kind: "daemon", daemon: "runner", cwd: "/repo" }], triggers: [ @@ -101,10 +115,71 @@ describe("GitHub trigger matching", () => { pull_request: { head: { ref: "topic" } }, }, }; - assert.equal(matchTriggers(config, event).length, 1); + assert.equal((await matchTriggers(config, event, { teamMemberships: denyTeams })).length, 1); + }); + + it("allows an active GitHub team member and fails closed when membership is unavailable", async () => { + const config = configFor({ + repo: "boudra/faro", + contains: "@paseo", + from_teams: ["boudra/maintainers"], + }); + const activeTeams = new TestTeamMemberships(true); + + assert.equal( + ( + await matchTriggers(config, createEvent(), { + teamMemberships: activeTeams, + }) + ).length, + 1, + ); + assert.deepEqual(activeTeams.checks, [ + { + installationId: 42, + organization: "boudra", + teamSlug: "maintainers", + username: "boudra", + }, + ]); + assert.deepEqual( + await matchTriggers(config, createEvent(), { teamMemberships: denyTeams }), + [], + ); + }); + + it("treats user and team allowlists as alternatives", async () => { + const config = configFor({ + repo: "boudra/faro", + contains: "@paseo", + from_users: ["boudra"], + from_teams: ["boudra/maintainers"], + }); + const memberships = new TestTeamMemberships(false); + + assert.equal( + (await matchTriggers(config, createEvent(), { teamMemberships: memberships })).length, + 1, + ); + assert.deepEqual(memberships.checks, []); }); }); +const denyTeams: GitHubTeamMembershipClient = { + isActiveMember: async () => false, +}; + +class TestTeamMemberships implements GitHubTeamMembershipClient { + readonly checks: Array[0]> = []; + + constructor(private readonly active: boolean) {} + + async isActiveMember(input: Parameters[0]) { + this.checks.push(input); + return this.active; + } +} + function configFor(filters: Record) { return compileHubConfig({ environments: [{ name: "runner", kind: "daemon", daemon: "runner", cwd: "/repo" }], diff --git a/src/triggers/github/match.ts b/src/triggers/github/match.ts index 8d4cd91a..fa3b70e0 100644 --- a/src/triggers/github/match.ts +++ b/src/triggers/github/match.ts @@ -9,6 +9,7 @@ import { PullRequestReviewPayloadSchema, } from "../../auth/github-events.js"; import type { NormalizedGitHubEvent } from "../../auth/github-events.js"; +import type { GitHubTeamMembershipClient } from "./team-membership.js"; type MatchedTriggerDefinition = Pick; @@ -17,6 +18,11 @@ export interface MatchedTriggerEvent { trigger: MatchedTriggerDefinition; } +export interface GitHubTriggerMatchOptions { + connectionId?: string | null; + teamMemberships: GitHubTeamMembershipClient; +} + export function readGitHubInvocationMessage(event: NormalizedGitHubEvent): string { return getFilterText(event); } @@ -42,16 +48,20 @@ export function readGitHubMention( return candidate !== undefined && message.includes(candidate) ? candidate : undefined; } -export function matchTriggers( +export async function matchTriggers( config: { triggers: readonly MatchedTriggerDefinition[] }, event: NormalizedGitHubEvent, - connectionId?: string | null, -): MatchedTriggerEvent[] { + options: GitHubTriggerMatchOptions, +): Promise { const on = `github.${event.type}`; const matches: MatchedTriggerEvent[] = []; + const membershipChecks = new Map>(); for (const trigger of config.triggers) { - if (trigger.on !== on || !matchesFilter(event, trigger.filters, connectionId)) { + if ( + trigger.on !== on || + !(await matchesFilter(event, trigger.filters, options, membershipChecks)) + ) { continue; } @@ -61,49 +71,82 @@ export function matchTriggers( return matches; } -function matchesFilter( +async function matchesFilter( event: NormalizedGitHubEvent, filter: TriggerFilter | undefined, - connectionId?: string | null, -): boolean { - if (filter === undefined) { - return false; - } + options: GitHubTriggerMatchOptions, + membershipChecks: Map>, +): Promise { + if (!matchesStaticFilter(event, filter, options.connectionId)) return false; + if (filter === undefined) return false; - if (filter.from_users === undefined || filter.from_users.length === 0) { - return false; - } + const actor = getEventActor(event); + if (filter.from_users?.includes(actor) === true) return true; + if (actor.length === 0 || filter.from_teams === undefined) return false; + + return matchesTeamFilter( + event, + actor, + filter.from_teams, + options.teamMemberships, + membershipChecks, + ); +} - if (filter.connectionId !== undefined && filter.connectionId !== connectionId) { +function matchesStaticFilter( + event: NormalizedGitHubEvent, + filter: TriggerFilter | undefined, + connectionId: string | null | undefined, +): boolean { + if (filter === undefined) return false; + if ((filter.from_users?.length ?? 0) === 0 && (filter.from_teams?.length ?? 0) === 0) { return false; } + if (filter.connectionId !== undefined && filter.connectionId !== connectionId) return false; const repo = filter["repo"]; - if (typeof repo === "string" && repo !== event.repo) { - return false; - } + if (typeof repo === "string" && repo !== event.repo) return false; const resourceId = filter["resourceId"]; - if (typeof resourceId === "string" && resourceId !== String(event.repositoryId)) { - return false; - } + if (typeof resourceId === "string" && resourceId !== String(event.repositoryId)) return false; + const text = getFilterText(event); const pattern = readStringFilter(filter, "pattern"); - if (pattern !== undefined && !getFilterText(event).startsWith(pattern)) { - return false; - } + if (pattern !== undefined && !text.startsWith(pattern)) return false; const contains = readStringFilter(filter, "contains"); - if (contains !== undefined && !getFilterText(event).includes(contains)) { - return false; - } + return contains === undefined || text.includes(contains); +} - const actor = getEventActor(event); - if (!filter.from_users.includes(actor)) { - return false; +async function matchesTeamFilter( + event: NormalizedGitHubEvent, + actor: string, + references: readonly string[], + teamMemberships: GitHubTeamMembershipClient, + membershipChecks: Map>, +): Promise { + for (const reference of references) { + const team = parseTeamReference(reference); + if (team === undefined) continue; + const key = [event.installationId, team.organization, team.slug, actor].join("\u0000"); + let membership = membershipChecks.get(key); + if (membership === undefined) { + membership = teamMemberships.isActiveMember({ + installationId: event.installationId, + organization: team.organization, + teamSlug: team.slug, + username: actor, + }); + membershipChecks.set(key, membership); + } + try { + if (await membership) return true; + } catch { + // Membership checks are authorization decisions. An unavailable client denies the trigger. + } } - return true; + return false; } function readStringFilter(filter: TriggerFilter, key: "pattern" | "contains"): string | undefined { @@ -111,6 +154,21 @@ function readStringFilter(filter: TriggerFilter, key: "pattern" | "contains"): s return typeof value === "string" && value.length > 0 ? value : undefined; } +function parseTeamReference(value: string): { organization: string; slug: string } | undefined { + const parts = value.split("/"); + const organization = parts.length === 2 ? parts[0] : undefined; + const slug = parts.length === 2 ? parts[1] : undefined; + if ( + organization === undefined || + organization.length === 0 || + slug === undefined || + slug.length === 0 + ) { + return undefined; + } + return { organization, slug }; +} + function getFilterText(event: NormalizedGitHubEvent): string { switch (event.type) { case "issue_comment": { diff --git a/src/triggers/github/provider.test.ts b/src/triggers/github/provider.test.ts index af3aaa47..a6ccaf25 100644 --- a/src/triggers/github/provider.test.ts +++ b/src/triggers/github/provider.test.ts @@ -6,6 +6,7 @@ import { createActiveProjectConfiguration } from "../../test-utils/project-confi import { createDurableWorkflowHandler } from "../../workflows/engine.js"; import type { GitHubReactionClient } from "./provider.js"; import { createGitHubTriggerProvider } from "./provider.js"; +import type { GitHubTeamMembershipClient } from "./team-membership.js"; import type { NormalizedGitHubEvent } from "../../auth/github-events.js"; import { isAcceptedTriggerProviderMatch } from "../index.js"; import { createUnlimitedEntitlementsService } from "../../entitlements/test-utils.js"; @@ -73,6 +74,50 @@ describe("GitHub Phase 1 trigger provider", () => { assert.equal(wrongActor, "trigger_filters_rejected"); }); + it("allows active GitHub team members and fails closed when their membership cannot be checked", async () => { + const base = githubConfiguration(); + const trigger = base.triggers[0]!; + const teamFilterBase = { + repo: trigger.filters.repo, + contains: trigger.filters.contains, + }; + const { project, revision, store } = await activeConfiguration({ + ...base, + triggers: [ + { + ...trigger, + filters: { + ...teamFilterBase, + from_teams: ["boudra/maintainers"], + }, + }, + ], + }); + const activeTeams = new TestTeamMemberships(true); + const activeProvider = createProvider(store, new TestReactions(), activeTeams); + + const active = await activeProvider.match( + external(project.id, revision.id, createEvent({ actor: "maintainer" })), + ); + assert.ok(Array.isArray(active)); + assert.equal(active.length, 1); + assert.deepEqual(activeTeams.checks, [ + { + installationId: 42, + organization: "boudra", + teamSlug: "maintainers", + username: "maintainer", + }, + ]); + + const denied = await createProvider( + store, + new TestReactions(), + new TestTeamMemberships(false), + ).match(external(project.id, revision.id, createEvent({ actor: "maintainer" }))); + assert.equal(denied, "trigger_filters_rejected"); + }); + it("exposes safe issue and pull-request item context without the raw webhook", async () => { const { project, revision, store } = await activeConfiguration(); const provider = createProvider(store, new TestReactions()); @@ -256,14 +301,16 @@ describe("GitHub Phase 1 trigger provider", () => { function createProvider( store: Awaited>["store"], reactions: TestReactions, + teamMemberships: GitHubTeamMembershipClient = new TestTeamMemberships(), ) { return createGitHubTriggerProvider({ configurationStoreForProject: () => store, reactions, + teamMemberships, }); } -async function activeConfiguration(rawConfiguration = githubConfiguration()) { +async function activeConfiguration(rawConfiguration: unknown = githubConfiguration()) { return createActiveProjectConfiguration(createMemoryDatabase(), rawConfiguration); } @@ -398,3 +445,14 @@ class TestReactions implements GitHubReactionClient { this.deleted.push(input); } } + +class TestTeamMemberships implements GitHubTeamMembershipClient { + readonly checks: Array[0]> = []; + + constructor(private readonly active = false) {} + + async isActiveMember(input: Parameters[0]) { + this.checks.push(input); + return this.active; + } +} diff --git a/src/triggers/github/provider.ts b/src/triggers/github/provider.ts index 1b025e6f..79afe998 100644 --- a/src/triggers/github/provider.ts +++ b/src/triggers/github/provider.ts @@ -13,6 +13,7 @@ import { readGitHubInvocationParserMessage, readGitHubMention, } from "./match.js"; +import type { GitHubTeamMembershipClient } from "./team-membership.js"; import { matchesInputFilters, parseInvocation } from "../invocation.js"; import { IssueCommentPayloadSchema, @@ -129,6 +130,7 @@ interface GitHubReactionState { export function createGitHubTriggerProvider(options: { configurationStoreForProject: (projectId: string) => ProjectConfigurationStore; reactions: GitHubReactionClient; + teamMemberships: GitHubTeamMembershipClient; }): TriggerProvider<"github", GitHubTriggerContext> { return { name: "github", @@ -151,11 +153,12 @@ export function createGitHubTriggerProvider(options: { return "no_trigger_for_source"; const matches: TriggerProviderMatch[] = []; - for (const match of matchTriggers( - stored.configuration, - event, - externalTrigger.connectionId, - )) { + for (const match of await matchTriggers(stored.configuration, event, { + teamMemberships: options.teamMemberships, + ...(externalTrigger.connectionId === undefined + ? {} + : { connectionId: externalTrigger.connectionId }), + })) { const compiledTrigger = stored.configuration.triggers.find( (candidate) => candidate.name === match.trigger.name, ); diff --git a/src/triggers/github/team-membership.test.ts b/src/triggers/github/team-membership.test.ts new file mode 100644 index 00000000..a9371268 --- /dev/null +++ b/src/triggers/github/team-membership.test.ts @@ -0,0 +1,70 @@ +import assert from "node:assert/strict"; +import { describe, it } from "vitest"; +import { + createGitHubTeamMembershipClient, + type GitHubTeamMembershipAuth, +} from "./team-membership.js"; + +describe("GitHub team membership client", () => { + it("accepts only active memberships", async () => { + const harness = createHarness({ state: "active" }); + const client = createGitHubTeamMembershipClient(harness.auth); + + assert.equal( + await client.isActiveMember({ + installationId: 42, + organization: "getpaseo", + teamSlug: "maintainers", + username: "michael", + }), + true, + ); + assert.deepEqual(harness.requests, [ + { + route: "GET /orgs/{org}/teams/{team_slug}/memberships/{username}", + parameters: { + org: "getpaseo", + team_slug: "maintainers", + username: "michael", + headers: { "x-github-api-version": "2026-03-10" }, + }, + }, + ]); + }); + + it.each([ + ["pending", { state: "pending" }], + ["missing", httpError(404)], + ["permission denied", httpError(403)], + ])("fails closed for a %s team membership result", async (_name, result) => { + const client = createGitHubTeamMembershipClient(createHarness(result).auth); + + assert.equal( + await client.isActiveMember({ + installationId: 42, + organization: "getpaseo", + teamSlug: "maintainers", + username: "michael", + }), + false, + ); + }); +}); + +function createHarness(result: unknown) { + const requests: Array<{ route: string; parameters: Record }> = []; + const auth = { + createInstallationOctokit: async () => ({ + request: async (route: string, parameters: Record) => { + requests.push({ route, parameters }); + if (result instanceof Error) throw result; + return { data: result }; + }, + }), + } satisfies GitHubTeamMembershipAuth; + return { auth, requests }; +} + +function httpError(status: number): Error & { status: number } { + return Object.assign(new Error(`GitHub returned ${status}`), { status }); +} diff --git a/src/triggers/github/team-membership.ts b/src/triggers/github/team-membership.ts new file mode 100644 index 00000000..11f19b20 --- /dev/null +++ b/src/triggers/github/team-membership.ts @@ -0,0 +1,76 @@ +import { GITHUB_APP_PERMISSION_VOCABULARY } from "../../config/github-authority.js"; +import { logger } from "../../logger.js"; +import { z } from "zod"; + +const TeamMembershipSchema = z.object({ state: z.enum(["active", "pending"]) }).passthrough(); + +export interface GitHubTeamMembershipClient { + isActiveMember(input: GitHubTeamMembershipCheck): Promise; +} + +export interface GitHubTeamMembershipCheck { + installationId: number; + organization: string; + teamSlug: string; + username: string; +} + +export interface GitHubTeamMembershipAuth { + createInstallationOctokit(installationId: number): Promise; +} + +interface GitHubTeamMembershipOctokit { + request( + route: "GET /orgs/{org}/teams/{team_slug}/memberships/{username}", + parameters: { + org: string; + team_slug: string; + username: string; + headers: { "x-github-api-version": string }; + }, + ): Promise<{ data: unknown }>; +} + +export function createGitHubTeamMembershipClient( + auth: GitHubTeamMembershipAuth, +): GitHubTeamMembershipClient { + return { + async isActiveMember(input) { + try { + const octokit = await auth.createInstallationOctokit(input.installationId); + const response = await octokit.request( + "GET /orgs/{org}/teams/{team_slug}/memberships/{username}", + { + org: input.organization, + team_slug: input.teamSlug, + username: input.username, + headers: { + "x-github-api-version": GITHUB_APP_PERMISSION_VOCABULARY.apiVersion, + }, + }, + ); + return TeamMembershipSchema.parse(response.data).state === "active"; + } catch (error) { + if (hasHttpStatus(error, 404)) return false; + logger.warn( + { + err: error, + installationId: input.installationId, + organization: input.organization, + teamSlug: input.teamSlug, + username: input.username, + }, + hasHttpStatus(error, 403) + ? "GitHub team trigger filter access was denied; denying trigger" + : "GitHub team trigger filter check failed; denying trigger", + ); + return false; + } + }, + }; +} + +function hasHttpStatus(error: unknown, status: number): boolean { + if (error === null || typeof error !== "object") return false; + return Reflect.get(error, "status") === status; +} diff --git a/src/workflows/reactions.test.ts b/src/workflows/reactions.test.ts index 6f4e2e9e..7b29f4ae 100644 --- a/src/workflows/reactions.test.ts +++ b/src/workflows/reactions.test.ts @@ -20,6 +20,7 @@ import type { DiscordBotClient } from "../triggers/discord/bot.js"; import { createDiscordTriggerProvider } from "../triggers/discord/provider.js"; import type { GitHubReactionClient } from "../triggers/github/provider.js"; import { createGitHubTriggerProvider } from "../triggers/github/provider.js"; +import type { GitHubTeamMembershipClient } from "../triggers/github/team-membership.js"; import type { SlackBotClient } from "../triggers/slack/client.js"; import { createSlackTriggerProvider } from "../triggers/slack/provider.js"; import type { TriggerProvider } from "../triggers/index.js"; @@ -207,6 +208,7 @@ function createGitHubReactionFixture() { configurationStoreForProject: () => new ProjectConfigurationStore(createMemoryDatabase(), "unused"), reactions, + teamMemberships: new DenyingGitHubTeamMemberships(), }); return { provider, @@ -225,6 +227,12 @@ function createGitHubReactionFixture() { }; } +class DenyingGitHubTeamMemberships implements GitHubTeamMembershipClient { + async isActiveMember(): Promise { + return false; + } +} + type WorkflowOutcome = "step_failure" | "timeout" | "prelaunch_failure"; interface WorkflowScenarioOptions { From 390fda6631304201093d64ff09b99062973f7dd1 Mon Sep 17 00:00:00 2001 From: Michael Wu Date: Sat, 15 Aug 2026 14:15:24 +0900 Subject: [PATCH 2/6] feat: allow GitHub team trigger filters --- .../.paseo/workflows/github.yml | 2 +- examples/single-repo-team-bot/README.md | 9 +- src/config/compiler.test.ts | 39 ++++++ src/config/compiler.ts | 19 ++- src/providers/github/index.ts | 3 + src/triggers/github/match.test.ts | 125 ++++++++++++++---- src/triggers/github/match.ts | 118 ++++++++++++----- src/triggers/github/provider.test.ts | 60 ++++++++- src/triggers/github/provider.ts | 13 +- src/triggers/github/team-membership.test.ts | 70 ++++++++++ src/triggers/github/team-membership.ts | 76 +++++++++++ src/workflows/reactions.test.ts | 8 ++ 12 files changed, 474 insertions(+), 68 deletions(-) create mode 100644 src/triggers/github/team-membership.test.ts create mode 100644 src/triggers/github/team-membership.ts diff --git a/examples/single-repo-team-bot/.paseo/workflows/github.yml b/examples/single-repo-team-bot/.paseo/workflows/github.yml index 1e413791..29fa146d 100644 --- a/examples/single-repo-team-bot/.paseo/workflows/github.yml +++ b/examples/single-repo-team-bot/.paseo/workflows/github.yml @@ -3,7 +3,7 @@ on: github.issue_comment max_runtime: 2h filters: repo: your-org/your-repo - from_users: [your-github-login] + from_teams: [your-org/your-team] contains: "@your-bot" steps: - id: classify diff --git a/examples/single-repo-team-bot/README.md b/examples/single-repo-team-bot/README.md index 21c8d149..6cbb15b7 100644 --- a/examples/single-repo-team-bot/README.md +++ b/examples/single-repo-team-bot/README.md @@ -15,11 +15,14 @@ Copy `.paseo` to your repository root, then replace these values: | `your-github-connection` | GitHub connection slug in Hub | | `YOUR_DISCORD_USER_ID` | Discord user allowed to trigger runs | | `YOUR_SLACK_USER_ID` | Slack user allowed to trigger runs | -| `your-github-login` | GitHub user allowed to trigger runs | +| `your-org/your-team` | GitHub team allowed to trigger runs | | `@your-bot` | Mention that starts the GitHub workflow | -Connect Discord, Slack, and GitHub to the project before enabling their triggers. Keep the user -allowlists narrow; wildcards are not supported. +Connect Discord, Slack, and GitHub to the project before enabling their triggers. GitHub team +filters use `organization/team-slug` and require the GitHub App's organization **Members** +permission with read access. If membership cannot be checked, Hub does not start a run. A GitHub +`from_users` allowlist can be combined with `from_teams`; either grants access. Keep allowlists +narrow; wildcards are not supported. ## Deploy diff --git a/src/config/compiler.test.ts b/src/config/compiler.test.ts index 0512f362..8b4d88ac 100644 --- a/src/config/compiler.test.ts +++ b/src/config/compiler.test.ts @@ -286,6 +286,45 @@ describe("workflow compiler", () => { }); }); + it("supports GitHub team trigger allowlists and rejects invalid team filter uses", () => { + const raw = configuration(); + const trigger = raw.triggers[0]!; + const githubTrigger = { + ...trigger, + on: "github.issue_comment", + filters: { from_teams: ["getpaseo/maintainers"] }, + }; + + assert.deepEqual(compileHubConfig({ ...raw, triggers: [githubTrigger] }).triggers[0]?.filters, { + from_teams: ["getpaseo/maintainers"], + }); + assert.throws( + () => + compileHubConfig({ + ...raw, + triggers: [{ ...trigger, on: "slack.mention", filters: githubTrigger.filters }], + }), + /filters\.from_teams only for GitHub events/iu, + ); + assert.throws( + () => + compileHubConfig({ + ...raw, + triggers: [ + { + ...githubTrigger, + filters: { from_teams: ["maintainers"] }, + }, + ], + }), + /organization\/team-slug/iu, + ); + assert.throws( + () => compileHubConfig({ ...raw, triggers: [{ ...trigger, on: "github.issue_comment" }] }), + /filters\.from_users or filters\.from_teams/iu, + ); + }); + it("requires explicit repositories for non-GitHub authority", () => { const raw = configuration(); const step = raw.triggers[0]!.steps[0]!; diff --git a/src/config/compiler.ts b/src/config/compiler.ts index 7049ea51..94dc5b19 100644 --- a/src/config/compiler.ts +++ b/src/config/compiler.ts @@ -29,6 +29,7 @@ const EVENT_NAME = /^[a-z][a-z0-9_-]*\.[a-z][a-z0-9_-]*$/u; const DURATION = /^([1-9][0-9]*)(ms|s|m|h)$/u; const MAX_DURATION_MS = 24 * 60 * 60_000; const INPUT_NAME = /^[a-z][a-z0-9_-]*$/u; +const GITHUB_TEAM_REFERENCE = /^[^/\s]+\/[^/\s]+$/u; const DYNAMIC_INPUT_REFERENCE = /^\$\{\{\s*paseo\.inputs\.([a-z][a-z0-9_-]*)\s*\}\}$/u; const EXPRESSION_START = "${{"; const EXPRESSION_END = "}}"; @@ -89,6 +90,9 @@ const AuthoredTriggerFilterSchema = z workspace: z.string().min(1).optional(), channels: z.array(z.string().min(1)).optional(), from_users: z.array(z.string().min(1)).optional(), + from_teams: z + .array(z.string().regex(GITHUB_TEAM_REFERENCE, "must be formatted as organization/team-slug")) + .optional(), inputs: z.record(z.string(), InputValueSchema).optional(), connection: z .string() @@ -243,9 +247,10 @@ export interface CompiledStep { export type CompiledSteps = readonly CompiledStep[]; export type CompiledTriggerFilter = Readonly< - Omit & { + Omit & { channels?: readonly string[] | undefined; from_users?: readonly string[] | undefined; + from_teams?: readonly string[] | undefined; inputs?: Readonly> | undefined; connectionId?: string | undefined; resourceId?: string | undefined; @@ -1312,10 +1317,18 @@ function validateAuthoredIds(config: AuthoredHubConfig): void { } function validateTriggerLaunchSecurity(trigger: CompiledTrigger): void { + const fromTeams = trigger.filters?.from_teams ?? []; + if (fromTeams.length > 0 && !trigger.on.startsWith("github.")) { + throw new Error(`trigger ${trigger.name} may use filters.from_teams only for GitHub events`); + } if (trigger.on === "manual.run") return; - if ((trigger.filters?.from_users?.length ?? 0) === 0) { + const fromUsers = trigger.filters?.from_users ?? []; + if (fromUsers.length === 0 && fromTeams.length === 0) { + const allowlist = trigger.on.startsWith("github.") + ? "filters.from_users or filters.from_teams" + : "filters.from_users"; throw new Error( - `trigger ${trigger.name} requires a non-empty filters.from_users allowlist for externally sourced events`, + `trigger ${trigger.name} requires a non-empty ${allowlist} allowlist for externally sourced events`, ); } } diff --git a/src/providers/github/index.ts b/src/providers/github/index.ts index 0f38af8d..59a36430 100644 --- a/src/providers/github/index.ts +++ b/src/providers/github/index.ts @@ -24,6 +24,7 @@ import { createGitHubTriggerProvider, type GitHubReactionClient, } from "../../triggers/github/provider.js"; +import { createGitHubTeamMembershipClient } from "../../triggers/github/team-membership.js"; import { createWebhookSource } from "../../triggers/github/webhook.js"; import { createGitHubConnectionClient, @@ -86,6 +87,7 @@ export function createGitHubRegistration( const appAuth = options.appAuth ?? createGitHubAuth({ appId: configuration.appId, privateKey: configuration.privateKey }); + const teamMemberships = createGitHubTeamMembershipClient(appAuth); const client = options.connectionClient ?? createGitHubConnectionClient({ @@ -224,6 +226,7 @@ export function createGitHubRegistration( return createGitHubTriggerProvider({ configurationStoreForProject, reactions, + teamMemberships, }); }, ], diff --git a/src/triggers/github/match.test.ts b/src/triggers/github/match.test.ts index ee15c3c1..692232f5 100644 --- a/src/triggers/github/match.test.ts +++ b/src/triggers/github/match.test.ts @@ -3,32 +3,36 @@ import { describe, it } from "vitest"; import { compileHubConfig } from "../../config/index.js"; import type { NormalizedGitHubEvent } from "../../auth/github-events.js"; import { matchTriggers } from "./match.js"; +import type { GitHubTeamMembershipClient } from "./team-membership.js"; describe("GitHub trigger matching", () => { - it("matches the compiled one-step trigger by repository, text, and actor", () => { + it("matches the compiled one-step trigger by repository, text, and actor", async () => { const config = configFor({ repo: "boudra/faro", contains: "@paseo", from_users: ["boudra"] }); - const matches = matchTriggers(config, createEvent()); + const matches = await matchTriggers(config, createEvent(), { teamMemberships: denyTeams }); assert.equal(matches.length, 1); assert.equal(matches[0]?.trigger.name, "github-comment"); }); - it("matches pattern-less comments while preserving the provider allowlist", () => { + it("matches pattern-less comments while preserving the provider allowlist", async () => { const config = configFor({ repo: "boudra/faro", from_users: ["boudra"] }); assert.equal( - matchTriggers( - config, - createEvent({ - payload: { - comment: { id: 123, body: "please explain", user: { login: "boudra" } }, - sender: { login: "boudra" }, - }, - }), + ( + await matchTriggers( + config, + createEvent({ + payload: { + comment: { id: 123, body: "please explain", user: { login: "boudra" } }, + sender: { login: "boudra" }, + }, + }), + { teamMemberships: denyTeams }, + ) ).length, 1, ); assert.deepEqual( - matchTriggers( + await matchTriggers( config, createEvent({ payload: { @@ -36,28 +40,37 @@ describe("GitHub trigger matching", () => { sender: { login: "stranger" }, }, }), + { teamMemberships: denyTeams }, ), [], ); }); - it("keeps repository and pattern filters literal", () => { + it("keeps repository and pattern filters literal", async () => { const config = configFor({ repo: "boudra/faro", pattern: "@paseo", from_users: ["boudra"] }); assert.equal( - matchTriggers( - config, - createEvent({ - payload: { - comment: { id: 123, body: "@paseo please explain", user: { login: "boudra" } }, - sender: { login: "boudra" }, - }, - }), + ( + await matchTriggers( + config, + createEvent({ + payload: { + comment: { id: 123, body: "@paseo please explain", user: { login: "boudra" } }, + sender: { login: "boudra" }, + }, + }), + { teamMemberships: denyTeams }, + ) ).length, 1, ); - assert.deepEqual(matchTriggers(config, createEvent({ repo: "elsewhere/repo" })), []); assert.deepEqual( - matchTriggers( + await matchTriggers(config, createEvent({ repo: "elsewhere/repo" }), { + teamMemberships: denyTeams, + }), + [], + ); + assert.deepEqual( + await matchTriggers( config, createEvent({ payload: { @@ -65,12 +78,13 @@ describe("GitHub trigger matching", () => { sender: { login: "boudra" }, }, }), + { teamMemberships: denyTeams }, ), [], ); }); - it("applies the same security filters to pull-request review comments", () => { + it("applies the same security filters to pull-request review comments", async () => { const config = compileHubConfig({ environments: [{ name: "runner", kind: "daemon", daemon: "runner", cwd: "/repo" }], triggers: [ @@ -101,10 +115,71 @@ describe("GitHub trigger matching", () => { pull_request: { head: { ref: "topic" } }, }, }; - assert.equal(matchTriggers(config, event).length, 1); + assert.equal((await matchTriggers(config, event, { teamMemberships: denyTeams })).length, 1); + }); + + it("allows an active GitHub team member and fails closed when membership is unavailable", async () => { + const config = configFor({ + repo: "boudra/faro", + contains: "@paseo", + from_teams: ["boudra/maintainers"], + }); + const activeTeams = new TestTeamMemberships(true); + + assert.equal( + ( + await matchTriggers(config, createEvent(), { + teamMemberships: activeTeams, + }) + ).length, + 1, + ); + assert.deepEqual(activeTeams.checks, [ + { + installationId: 42, + organization: "boudra", + teamSlug: "maintainers", + username: "boudra", + }, + ]); + assert.deepEqual( + await matchTriggers(config, createEvent(), { teamMemberships: denyTeams }), + [], + ); + }); + + it("treats user and team allowlists as alternatives", async () => { + const config = configFor({ + repo: "boudra/faro", + contains: "@paseo", + from_users: ["boudra"], + from_teams: ["boudra/maintainers"], + }); + const memberships = new TestTeamMemberships(false); + + assert.equal( + (await matchTriggers(config, createEvent(), { teamMemberships: memberships })).length, + 1, + ); + assert.deepEqual(memberships.checks, []); }); }); +const denyTeams: GitHubTeamMembershipClient = { + isActiveMember: async () => false, +}; + +class TestTeamMemberships implements GitHubTeamMembershipClient { + readonly checks: Array[0]> = []; + + constructor(private readonly active: boolean) {} + + async isActiveMember(input: Parameters[0]) { + this.checks.push(input); + return this.active; + } +} + function configFor(filters: Record) { return compileHubConfig({ environments: [{ name: "runner", kind: "daemon", daemon: "runner", cwd: "/repo" }], diff --git a/src/triggers/github/match.ts b/src/triggers/github/match.ts index 8d4cd91a..fa3b70e0 100644 --- a/src/triggers/github/match.ts +++ b/src/triggers/github/match.ts @@ -9,6 +9,7 @@ import { PullRequestReviewPayloadSchema, } from "../../auth/github-events.js"; import type { NormalizedGitHubEvent } from "../../auth/github-events.js"; +import type { GitHubTeamMembershipClient } from "./team-membership.js"; type MatchedTriggerDefinition = Pick; @@ -17,6 +18,11 @@ export interface MatchedTriggerEvent { trigger: MatchedTriggerDefinition; } +export interface GitHubTriggerMatchOptions { + connectionId?: string | null; + teamMemberships: GitHubTeamMembershipClient; +} + export function readGitHubInvocationMessage(event: NormalizedGitHubEvent): string { return getFilterText(event); } @@ -42,16 +48,20 @@ export function readGitHubMention( return candidate !== undefined && message.includes(candidate) ? candidate : undefined; } -export function matchTriggers( +export async function matchTriggers( config: { triggers: readonly MatchedTriggerDefinition[] }, event: NormalizedGitHubEvent, - connectionId?: string | null, -): MatchedTriggerEvent[] { + options: GitHubTriggerMatchOptions, +): Promise { const on = `github.${event.type}`; const matches: MatchedTriggerEvent[] = []; + const membershipChecks = new Map>(); for (const trigger of config.triggers) { - if (trigger.on !== on || !matchesFilter(event, trigger.filters, connectionId)) { + if ( + trigger.on !== on || + !(await matchesFilter(event, trigger.filters, options, membershipChecks)) + ) { continue; } @@ -61,49 +71,82 @@ export function matchTriggers( return matches; } -function matchesFilter( +async function matchesFilter( event: NormalizedGitHubEvent, filter: TriggerFilter | undefined, - connectionId?: string | null, -): boolean { - if (filter === undefined) { - return false; - } + options: GitHubTriggerMatchOptions, + membershipChecks: Map>, +): Promise { + if (!matchesStaticFilter(event, filter, options.connectionId)) return false; + if (filter === undefined) return false; - if (filter.from_users === undefined || filter.from_users.length === 0) { - return false; - } + const actor = getEventActor(event); + if (filter.from_users?.includes(actor) === true) return true; + if (actor.length === 0 || filter.from_teams === undefined) return false; + + return matchesTeamFilter( + event, + actor, + filter.from_teams, + options.teamMemberships, + membershipChecks, + ); +} - if (filter.connectionId !== undefined && filter.connectionId !== connectionId) { +function matchesStaticFilter( + event: NormalizedGitHubEvent, + filter: TriggerFilter | undefined, + connectionId: string | null | undefined, +): boolean { + if (filter === undefined) return false; + if ((filter.from_users?.length ?? 0) === 0 && (filter.from_teams?.length ?? 0) === 0) { return false; } + if (filter.connectionId !== undefined && filter.connectionId !== connectionId) return false; const repo = filter["repo"]; - if (typeof repo === "string" && repo !== event.repo) { - return false; - } + if (typeof repo === "string" && repo !== event.repo) return false; const resourceId = filter["resourceId"]; - if (typeof resourceId === "string" && resourceId !== String(event.repositoryId)) { - return false; - } + if (typeof resourceId === "string" && resourceId !== String(event.repositoryId)) return false; + const text = getFilterText(event); const pattern = readStringFilter(filter, "pattern"); - if (pattern !== undefined && !getFilterText(event).startsWith(pattern)) { - return false; - } + if (pattern !== undefined && !text.startsWith(pattern)) return false; const contains = readStringFilter(filter, "contains"); - if (contains !== undefined && !getFilterText(event).includes(contains)) { - return false; - } + return contains === undefined || text.includes(contains); +} - const actor = getEventActor(event); - if (!filter.from_users.includes(actor)) { - return false; +async function matchesTeamFilter( + event: NormalizedGitHubEvent, + actor: string, + references: readonly string[], + teamMemberships: GitHubTeamMembershipClient, + membershipChecks: Map>, +): Promise { + for (const reference of references) { + const team = parseTeamReference(reference); + if (team === undefined) continue; + const key = [event.installationId, team.organization, team.slug, actor].join("\u0000"); + let membership = membershipChecks.get(key); + if (membership === undefined) { + membership = teamMemberships.isActiveMember({ + installationId: event.installationId, + organization: team.organization, + teamSlug: team.slug, + username: actor, + }); + membershipChecks.set(key, membership); + } + try { + if (await membership) return true; + } catch { + // Membership checks are authorization decisions. An unavailable client denies the trigger. + } } - return true; + return false; } function readStringFilter(filter: TriggerFilter, key: "pattern" | "contains"): string | undefined { @@ -111,6 +154,21 @@ function readStringFilter(filter: TriggerFilter, key: "pattern" | "contains"): s return typeof value === "string" && value.length > 0 ? value : undefined; } +function parseTeamReference(value: string): { organization: string; slug: string } | undefined { + const parts = value.split("/"); + const organization = parts.length === 2 ? parts[0] : undefined; + const slug = parts.length === 2 ? parts[1] : undefined; + if ( + organization === undefined || + organization.length === 0 || + slug === undefined || + slug.length === 0 + ) { + return undefined; + } + return { organization, slug }; +} + function getFilterText(event: NormalizedGitHubEvent): string { switch (event.type) { case "issue_comment": { diff --git a/src/triggers/github/provider.test.ts b/src/triggers/github/provider.test.ts index af3aaa47..a6ccaf25 100644 --- a/src/triggers/github/provider.test.ts +++ b/src/triggers/github/provider.test.ts @@ -6,6 +6,7 @@ import { createActiveProjectConfiguration } from "../../test-utils/project-confi import { createDurableWorkflowHandler } from "../../workflows/engine.js"; import type { GitHubReactionClient } from "./provider.js"; import { createGitHubTriggerProvider } from "./provider.js"; +import type { GitHubTeamMembershipClient } from "./team-membership.js"; import type { NormalizedGitHubEvent } from "../../auth/github-events.js"; import { isAcceptedTriggerProviderMatch } from "../index.js"; import { createUnlimitedEntitlementsService } from "../../entitlements/test-utils.js"; @@ -73,6 +74,50 @@ describe("GitHub Phase 1 trigger provider", () => { assert.equal(wrongActor, "trigger_filters_rejected"); }); + it("allows active GitHub team members and fails closed when their membership cannot be checked", async () => { + const base = githubConfiguration(); + const trigger = base.triggers[0]!; + const teamFilterBase = { + repo: trigger.filters.repo, + contains: trigger.filters.contains, + }; + const { project, revision, store } = await activeConfiguration({ + ...base, + triggers: [ + { + ...trigger, + filters: { + ...teamFilterBase, + from_teams: ["boudra/maintainers"], + }, + }, + ], + }); + const activeTeams = new TestTeamMemberships(true); + const activeProvider = createProvider(store, new TestReactions(), activeTeams); + + const active = await activeProvider.match( + external(project.id, revision.id, createEvent({ actor: "maintainer" })), + ); + assert.ok(Array.isArray(active)); + assert.equal(active.length, 1); + assert.deepEqual(activeTeams.checks, [ + { + installationId: 42, + organization: "boudra", + teamSlug: "maintainers", + username: "maintainer", + }, + ]); + + const denied = await createProvider( + store, + new TestReactions(), + new TestTeamMemberships(false), + ).match(external(project.id, revision.id, createEvent({ actor: "maintainer" }))); + assert.equal(denied, "trigger_filters_rejected"); + }); + it("exposes safe issue and pull-request item context without the raw webhook", async () => { const { project, revision, store } = await activeConfiguration(); const provider = createProvider(store, new TestReactions()); @@ -256,14 +301,16 @@ describe("GitHub Phase 1 trigger provider", () => { function createProvider( store: Awaited>["store"], reactions: TestReactions, + teamMemberships: GitHubTeamMembershipClient = new TestTeamMemberships(), ) { return createGitHubTriggerProvider({ configurationStoreForProject: () => store, reactions, + teamMemberships, }); } -async function activeConfiguration(rawConfiguration = githubConfiguration()) { +async function activeConfiguration(rawConfiguration: unknown = githubConfiguration()) { return createActiveProjectConfiguration(createMemoryDatabase(), rawConfiguration); } @@ -398,3 +445,14 @@ class TestReactions implements GitHubReactionClient { this.deleted.push(input); } } + +class TestTeamMemberships implements GitHubTeamMembershipClient { + readonly checks: Array[0]> = []; + + constructor(private readonly active = false) {} + + async isActiveMember(input: Parameters[0]) { + this.checks.push(input); + return this.active; + } +} diff --git a/src/triggers/github/provider.ts b/src/triggers/github/provider.ts index 9e0ff58c..10605e20 100644 --- a/src/triggers/github/provider.ts +++ b/src/triggers/github/provider.ts @@ -13,6 +13,7 @@ import { readGitHubInvocationParserMessage, readGitHubMention, } from "./match.js"; +import type { GitHubTeamMembershipClient } from "./team-membership.js"; import { matchesInputFilters, parseInvocation } from "../invocation.js"; import { IssueCommentPayloadSchema, @@ -129,6 +130,7 @@ interface GitHubReactionState { export function createGitHubTriggerProvider(options: { configurationStoreForProject: (projectId: string) => ProjectConfigurationStore; reactions: GitHubReactionClient; + teamMemberships: GitHubTeamMembershipClient; }): TriggerProvider<"github", GitHubTriggerContext> { return { name: "github", @@ -151,11 +153,12 @@ export function createGitHubTriggerProvider(options: { return "no_trigger_for_source"; const matches: TriggerProviderMatch[] = []; - for (const match of matchTriggers( - stored.configuration, - event, - externalTrigger.connectionId, - )) { + for (const match of await matchTriggers(stored.configuration, event, { + teamMemberships: options.teamMemberships, + ...(externalTrigger.connectionId === undefined + ? {} + : { connectionId: externalTrigger.connectionId }), + })) { const compiledTrigger = stored.configuration.triggers.find( (candidate) => candidate.name === match.trigger.name, ); diff --git a/src/triggers/github/team-membership.test.ts b/src/triggers/github/team-membership.test.ts new file mode 100644 index 00000000..a9371268 --- /dev/null +++ b/src/triggers/github/team-membership.test.ts @@ -0,0 +1,70 @@ +import assert from "node:assert/strict"; +import { describe, it } from "vitest"; +import { + createGitHubTeamMembershipClient, + type GitHubTeamMembershipAuth, +} from "./team-membership.js"; + +describe("GitHub team membership client", () => { + it("accepts only active memberships", async () => { + const harness = createHarness({ state: "active" }); + const client = createGitHubTeamMembershipClient(harness.auth); + + assert.equal( + await client.isActiveMember({ + installationId: 42, + organization: "getpaseo", + teamSlug: "maintainers", + username: "michael", + }), + true, + ); + assert.deepEqual(harness.requests, [ + { + route: "GET /orgs/{org}/teams/{team_slug}/memberships/{username}", + parameters: { + org: "getpaseo", + team_slug: "maintainers", + username: "michael", + headers: { "x-github-api-version": "2026-03-10" }, + }, + }, + ]); + }); + + it.each([ + ["pending", { state: "pending" }], + ["missing", httpError(404)], + ["permission denied", httpError(403)], + ])("fails closed for a %s team membership result", async (_name, result) => { + const client = createGitHubTeamMembershipClient(createHarness(result).auth); + + assert.equal( + await client.isActiveMember({ + installationId: 42, + organization: "getpaseo", + teamSlug: "maintainers", + username: "michael", + }), + false, + ); + }); +}); + +function createHarness(result: unknown) { + const requests: Array<{ route: string; parameters: Record }> = []; + const auth = { + createInstallationOctokit: async () => ({ + request: async (route: string, parameters: Record) => { + requests.push({ route, parameters }); + if (result instanceof Error) throw result; + return { data: result }; + }, + }), + } satisfies GitHubTeamMembershipAuth; + return { auth, requests }; +} + +function httpError(status: number): Error & { status: number } { + return Object.assign(new Error(`GitHub returned ${status}`), { status }); +} diff --git a/src/triggers/github/team-membership.ts b/src/triggers/github/team-membership.ts new file mode 100644 index 00000000..11f19b20 --- /dev/null +++ b/src/triggers/github/team-membership.ts @@ -0,0 +1,76 @@ +import { GITHUB_APP_PERMISSION_VOCABULARY } from "../../config/github-authority.js"; +import { logger } from "../../logger.js"; +import { z } from "zod"; + +const TeamMembershipSchema = z.object({ state: z.enum(["active", "pending"]) }).passthrough(); + +export interface GitHubTeamMembershipClient { + isActiveMember(input: GitHubTeamMembershipCheck): Promise; +} + +export interface GitHubTeamMembershipCheck { + installationId: number; + organization: string; + teamSlug: string; + username: string; +} + +export interface GitHubTeamMembershipAuth { + createInstallationOctokit(installationId: number): Promise; +} + +interface GitHubTeamMembershipOctokit { + request( + route: "GET /orgs/{org}/teams/{team_slug}/memberships/{username}", + parameters: { + org: string; + team_slug: string; + username: string; + headers: { "x-github-api-version": string }; + }, + ): Promise<{ data: unknown }>; +} + +export function createGitHubTeamMembershipClient( + auth: GitHubTeamMembershipAuth, +): GitHubTeamMembershipClient { + return { + async isActiveMember(input) { + try { + const octokit = await auth.createInstallationOctokit(input.installationId); + const response = await octokit.request( + "GET /orgs/{org}/teams/{team_slug}/memberships/{username}", + { + org: input.organization, + team_slug: input.teamSlug, + username: input.username, + headers: { + "x-github-api-version": GITHUB_APP_PERMISSION_VOCABULARY.apiVersion, + }, + }, + ); + return TeamMembershipSchema.parse(response.data).state === "active"; + } catch (error) { + if (hasHttpStatus(error, 404)) return false; + logger.warn( + { + err: error, + installationId: input.installationId, + organization: input.organization, + teamSlug: input.teamSlug, + username: input.username, + }, + hasHttpStatus(error, 403) + ? "GitHub team trigger filter access was denied; denying trigger" + : "GitHub team trigger filter check failed; denying trigger", + ); + return false; + } + }, + }; +} + +function hasHttpStatus(error: unknown, status: number): boolean { + if (error === null || typeof error !== "object") return false; + return Reflect.get(error, "status") === status; +} diff --git a/src/workflows/reactions.test.ts b/src/workflows/reactions.test.ts index 6f4e2e9e..7b29f4ae 100644 --- a/src/workflows/reactions.test.ts +++ b/src/workflows/reactions.test.ts @@ -20,6 +20,7 @@ import type { DiscordBotClient } from "../triggers/discord/bot.js"; import { createDiscordTriggerProvider } from "../triggers/discord/provider.js"; import type { GitHubReactionClient } from "../triggers/github/provider.js"; import { createGitHubTriggerProvider } from "../triggers/github/provider.js"; +import type { GitHubTeamMembershipClient } from "../triggers/github/team-membership.js"; import type { SlackBotClient } from "../triggers/slack/client.js"; import { createSlackTriggerProvider } from "../triggers/slack/provider.js"; import type { TriggerProvider } from "../triggers/index.js"; @@ -207,6 +208,7 @@ function createGitHubReactionFixture() { configurationStoreForProject: () => new ProjectConfigurationStore(createMemoryDatabase(), "unused"), reactions, + teamMemberships: new DenyingGitHubTeamMemberships(), }); return { provider, @@ -225,6 +227,12 @@ function createGitHubReactionFixture() { }; } +class DenyingGitHubTeamMemberships implements GitHubTeamMembershipClient { + async isActiveMember(): Promise { + return false; + } +} + type WorkflowOutcome = "step_failure" | "timeout" | "prelaunch_failure"; interface WorkflowScenarioOptions { From 730f2a92a8163963ed20b77b1c411fb2e8b3dade Mon Sep 17 00:00:00 2001 From: Michael Wu Date: Fri, 21 Aug 2026 09:24:31 +0900 Subject: [PATCH 3/6] docs: include GitHub team membership permission --- src/provider-applications/guides.test.ts | 1 + src/provider-applications/guides.ts | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/src/provider-applications/guides.test.ts b/src/provider-applications/guides.test.ts index 84e7a1ac..247bd4cd 100644 --- a/src/provider-applications/guides.test.ts +++ b/src/provider-applications/guides.test.ts @@ -192,6 +192,7 @@ test("GitHub renders permissions as a mapping and events as a list, never as pro { name: "Issues", access: "Read and write" }, { name: "Pull requests", access: "Read and write" }, { name: "Metadata", access: "Read-only" }, + { name: "Members", access: "Read-only" }, ]); assert.deepEqual( steps.flatMap((step) => step.events ?? []), diff --git a/src/provider-applications/guides.ts b/src/provider-applications/guides.ts index 0be556b0..6fde464d 100644 --- a/src/provider-applications/guides.ts +++ b/src/provider-applications/guides.ts @@ -264,6 +264,14 @@ export const GITHUB_GUIDE: ProviderGuide = { "Push", ], }, + { + segments: [ + { kind: "text", value: "If a workflow uses a GitHub team allowlist, under " }, + { kind: "term", value: "Organization permissions" }, + { kind: "text", value: ", grant:" }, + ], + permissions: [{ name: "Members", access: "Read-only" }], + }, ], fields: [ { From a9ef51cef61d1f999637d66e972e6a83fe0a8be8 Mon Sep 17 00:00:00 2001 From: Michael Wu Date: Thu, 27 Aug 2026 09:35:49 +0900 Subject: [PATCH 4/6] fix(github): classify push actors for team filters --- src/triggers/github/classification.ts | 10 ++++++++++ src/triggers/github/match.test.ts | 21 +++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/triggers/github/classification.ts b/src/triggers/github/classification.ts index e315676a..ab2a0161 100644 --- a/src/triggers/github/classification.ts +++ b/src/triggers/github/classification.ts @@ -4,6 +4,7 @@ import { PullRequestPayloadSchema, PullRequestReviewCommentPayloadSchema, PullRequestReviewPayloadSchema, + PushPayloadSchema, } from "../../auth/github-events.js"; import type { NormalizedGitHubEvent } from "../../auth/github-events.js"; @@ -52,9 +53,18 @@ export function classifyGitHubEvent(event: NormalizedGitHubEvent): GitHubClassif if (event.type === "issue_comment") return classifyIssueComment(event); if (event.type === "pull_request_review") return classifyReview(event); if (event.type === "pull_request_review_comment") return classifyReviewComment(event); + if (event.type === "push") return classifyPush(event); return emptyClassification(); } +function classifyPush(event: NormalizedGitHubEvent): GitHubClassifiedEvent { + const payload = PushPayloadSchema.parse(event.payload); + return { + ...emptyClassification(), + actor: payload.sender?.login ?? "", + }; +} + function classifyIssue(event: NormalizedGitHubEvent): GitHubClassifiedEvent { const payload = IssuesPayloadSchema.parse(event.payload); const item = payload.issue === undefined ? null : itemFor("issue", payload.issue); diff --git a/src/triggers/github/match.test.ts b/src/triggers/github/match.test.ts index f2be24b9..6e26fc36 100644 --- a/src/triggers/github/match.test.ts +++ b/src/triggers/github/match.test.ts @@ -426,6 +426,27 @@ describe("GitHub trigger matching", () => { ); }); + it("uses the push payload sender for team authorization", async () => { + const base = configFor({ from_teams: ["boudra/maintainers"] }); + const trigger = base.triggers[0]!; + const config = { ...base, triggers: [{ ...trigger, on: "github.push" }] }; + const memberships = new TestTeamMemberships(true); + const event = eventFor("push", { + after: "commit-sha", + ref: "refs/heads/main", + }); + + assert.equal((await matchTriggers(config, event, { teamMemberships: memberships })).length, 1); + assert.deepEqual(memberships.checks, [ + { + installationId: 42, + organization: "boudra", + teamSlug: "maintainers", + username: "boudra", + }, + ]); + }); + it("treats direct-user and team allowlists as alternatives", async () => { const config = configFor({ repo: "boudra/faro", From 01faf2031501102c708ae4ac1d03d325c7e487fd Mon Sep 17 00:00:00 2001 From: Michael Wu Date: Sat, 29 Aug 2026 10:15:53 +0900 Subject: [PATCH 5/6] fix: fail closed for malformed GitHub pushes --- src/triggers/github/classification.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/triggers/github/classification.ts b/src/triggers/github/classification.ts index ab2a0161..159f0a44 100644 --- a/src/triggers/github/classification.ts +++ b/src/triggers/github/classification.ts @@ -58,10 +58,11 @@ export function classifyGitHubEvent(event: NormalizedGitHubEvent): GitHubClassif } function classifyPush(event: NormalizedGitHubEvent): GitHubClassifiedEvent { - const payload = PushPayloadSchema.parse(event.payload); + const parsed = PushPayloadSchema.safeParse(event.payload); + if (!parsed.success) return emptyClassification(); return { ...emptyClassification(), - actor: payload.sender?.login ?? "", + actor: parsed.data.sender?.login ?? "", }; } From 3509dc763acfbee906f565a8bcbb98d45d3a2df6 Mon Sep 17 00:00:00 2001 From: Michael Wu Date: Tue, 29 Sep 2026 05:17:00 +0900 Subject: [PATCH 6/6] test: provide team memberships in GitHub provider fixture --- src/triggers/github/provider.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/triggers/github/provider.test.ts b/src/triggers/github/provider.test.ts index 726ee8c4..e1bb98b1 100644 --- a/src/triggers/github/provider.test.ts +++ b/src/triggers/github/provider.test.ts @@ -30,6 +30,7 @@ describe("GitHub Phase 1 trigger provider", () => { const provider = createGitHubTriggerProvider({ configurationStoreForProject: () => store, reactions, + teamMemberships: new TestTeamMemberships(), billingUrlForOrganization: async () => "https://hub.paseo.sh/o/acme/settings/billing", comments: { createIssueComment: async (input) => {