diff --git a/__tests__/bundle/ownersGroups.test.ts b/__tests__/bundle/ownersGroups.test.ts new file mode 100644 index 00000000..c1a07f60 --- /dev/null +++ b/__tests__/bundle/ownersGroups.test.ts @@ -0,0 +1,121 @@ +import type { FakeGithub } from './fakeGithub' +import { Buffer } from 'node:buffer' +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' + +import pullReqOpenedEvent from '../fixtures/pullReq/pullReqOpenedEvent.json' +import { prCommentEvent } from '../utils/ownersData' +import { start } from './fakeGithub' +import { comment, helpersFor, repo, token } from './helpers' +import { runBundle } from './runBundle' + +vi.setConfig({ testTimeout: 30_000 }) + +// `@org/team` entries of OWNERS files (src/utils/owners.ts: expandGroups, ownersInclude), driven through +// dist/index.js: the team is read once per run however many files and roles list it, a login listed +// directly never reads a team, and a group of another org or an unreadable team warns once and admits nobody +describe('dist/index.js OWNERS groups', () => { + const teamRead = `GET /orgs/Codertocat/teams/reviewers/members?per_page=100` + const rootOwnersRead = `GET ${repo}/contents/OWNERS` + const labelsRead = `GET ${repo}/labels?per_page=100` + const labelsPost = `POST ${repo}/issues/1/labels` + const commentPost = `POST ${repo}/issues/1/comments` + let gh: FakeGithub + const { calls, expectCommandThenConfig, routeOwners } = helpersFor(() => gh) + + beforeAll(async () => { + gh = await start() + }) + beforeEach(() => gh.mergeQueueFallback({ pullRequestId: 'PR_none', headOid: pullReqOpenedEvent.pull_request.head.sha, enabled: false })) + afterEach(() => gh.reset()) + afterAll(() => gh.close()) + + function routeRootOwners(contents: string) { + gh.route('GET', `${repo}/contents/OWNERS`, { + status: 200, + body: { encoding: 'base64', content: Buffer.from(contents).toString('base64') }, + }) + } + + function routeTeam(members: string[]) { + gh.route('GET', '/orgs/Codertocat/teams/reviewers/members', { status: 200, body: members.map(login => ({ login })) }) + } + + function routeLabelWrite() { + gh.route('GET', `${repo}/labels`, { status: 200, body: [{ name: 'lgtm' }] }) + gh.route('POST', `${repo}/issues/1/labels`, { status: 200, body: [] }) + } + + function lgtm(payload: Record) { + return runBundle({ eventName: 'issue_comment', payload, inputs: { ...token, 'prow-commands': '/lgtm' }, apiUrl: gh.url }) + } + + function warnings(stdout: string) { + return stdout.split('\n').filter(line => line.startsWith('::warning::')) + } + + it('on a pull request, a reviewer listed only through a team is authorized; the team is read once, other-org and malformed entries warn once', async () => { + routeOwners( + { + 'OWNERS': 'approvers:\n- \'@bob\'\nreviewers:\n- \'@other/team\'\n', + 'sdk/OWNERS': 'reviewers:\n- \'@codertocat/reviewers\'\n', + }, + ['sdk/a.go', 'sdk/b.go'], + { user: { login: 'some-author' } }, + ) + routeTeam(['Alice', 'carol']) + gh.route('POST', `${repo}/statuses/headsha`, { status: 201, body: {} }) + routeLabelWrite() + + const result = await lgtm(prCommentEvent('/lgtm', 'alice')) + + expect(result.status, result.stdout).toBe(0) + expect(result.errors).toEqual([]) + expect(gh.requestsMatching('POST', /\/issues\/1\/labels$/).map(r => r.body)).toEqual([{ labels: ['lgtm'] }]) + expect(gh.requestsMatching('GET', /\/orgs\/Codertocat\/members\/|\/collaborators\//)).toEqual([]) + expect(calls().filter(c => c === teamRead)).toHaveLength(1) + expect(warnings(result.stdout)).toEqual([ + '::warning::OWNERS: ignoring @bob: a group is written @/', + '::warning::OWNERS: ignoring group @other/team: only groups of codertocat are supported', + ]) + }) + + it('on an issue, a commenter listed through a team in the root OWNERS file is authorized by the team read', async () => { + routeRootOwners('reviewers:\n- \'@codertocat/reviewers\'\n') + routeTeam(['codertocat']) + routeLabelWrite() + + const result = await lgtm(comment('/lgtm', 'some-author')) + + expect(result.status, result.stdout).toBe(0) + expect(result.errors).toEqual([]) + expect(gh.requestsMatching('POST', /\/issues\/1\/labels$/).map(r => r.body)).toEqual([{ labels: ['lgtm'] }]) + expectCommandThenConfig([rootOwnersRead, teamRead, labelsRead, labelsPost]) + }) + + it('on an issue, a commenter listed directly next to a team is authorized without reading the team', async () => { + routeRootOwners('reviewers:\n- codertocat\n- \'@codertocat/reviewers\'\n') + routeLabelWrite() + + const result = await lgtm(comment('/lgtm', 'some-author')) + + expect(result.status, result.stdout).toBe(0) + expect(result.errors).toEqual([]) + expect(gh.requestsMatching('GET', /\/teams\//)).toEqual([]) + expectCommandThenConfig([rootOwnersRead, labelsRead, labelsPost]) + }) + + it('a team the token cannot read admits nobody: refused with one warning', async () => { + routeRootOwners('reviewers:\n- \'@codertocat/reviewers\'\n') + gh.route('GET', '/orgs/Codertocat/teams/reviewers/members', { status: 403, body: { message: 'Resource not accessible by integration' } }) + gh.route('POST', `${repo}/issues/1/comments`, { status: 201, body: {} }) + + const result = await lgtm(comment('/lgtm', 'some-author')) + + expect(result.status, result.stdout).toBe(1) + expect(result.errors.some(e => e.includes('Codertocat is not included in the reviewers role in the OWNERS file'))).toBe(true) + expect(gh.requestsMatching('POST', /\/issues\/1\/labels$/)).toEqual([]) + expect(warnings(result.stdout)).toHaveLength(1) + expect(warnings(result.stdout)[0]).toMatch(/^::warning::OWNERS: could not read the members of @codertocat\/reviewers: /) + expectCommandThenConfig([rootOwnersRead, teamRead, commentPost]) + }) +}) diff --git a/__tests__/utils/owners.test.ts b/__tests__/utils/owners.test.ts index 58e3f5ae..c3c1a241 100644 --- a/__tests__/utils/owners.test.ts +++ b/__tests__/utils/owners.test.ts @@ -1,10 +1,14 @@ import * as core from '@actions/core' -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' import { effectiveOwners, + expandGroups, + expandOwnersSet, ownersDir, + ownersInclude, parseOwners, + resetOwnersCaches, } from '../../src/utils/owners' describe('parseOwners', () => { @@ -186,3 +190,119 @@ describe('effectiveOwners', () => { expect(effectiveOwners('README.md', new Map())).toBeUndefined() }) }) + +describe('expandGroups', () => { + const context = { repo: { owner: 'MyOrg', repo: 'r' } } as never + + function client(listed: Record) { + const paginate = vi.fn(async (_fn: unknown, params: { team_slug: string }) => { + const members = listed[params.team_slug] + if (members === undefined) { + throw new Error('404') + } + return members.map(login => ({ login })) + }) + return { paginate, teams: { listMembersInOrg: vi.fn() } } as never + } + + it('replaces same-org groups with their members and keeps plain logins', async () => { + const result = await expandGroups(client({ maintainers: ['Bob', 'carol'] }), context, ['alice', '@myorg/maintainers']) + + expect([...result].sort()).toEqual(['alice', 'bob', 'carol']) + }) + + beforeEach(() => resetOwnersCaches()) + + it('ignores groups of another org and groups it cannot read', async () => { + const result = await expandGroups(client({}), context, ['alice', '@other/team', '@myorg/missing']) + + expect([...result]).toEqual(['alice']) + expect(core.warning).toHaveBeenCalledTimes(2) + }) + + it('drops an @ entry that is not @org/team-slug, with a warning', async () => { + const result = await expandGroups(client({}), context, ['alice', '@bob', '@myorg/a/b']) + + expect([...result]).toEqual(['alice']) + expect(core.warning).toHaveBeenCalledTimes(2) + expect(core.warning).toHaveBeenCalledWith('OWNERS: ignoring @bob: a group is written @/') + }) + + it('warns once per entry per run, however many files list it', async () => { + const octokit = client({}) + await expandGroups(octokit, context, ['@other/team', '@myorg/missing']) + await expandGroups(octokit, context, ['@other/team', '@myorg/missing']) + await expandGroups(octokit, context, ['@other/team']) + + expect(core.warning).toHaveBeenCalledTimes(2) + expect((octokit as { paginate: ReturnType }).paginate).toHaveBeenCalledTimes(1) + }) + + it('reads each team once per run', async () => { + const octokit = client({ maintainers: ['bob'] }) + const [a, b] = await Promise.all([ + expandGroups(octokit, context, ['@myorg/maintainers']), + expandGroups(octokit, context, ['@myorg/maintainers', 'alice']), + ]) + + expect([...a]).toEqual(['bob']) + expect([...b].sort()).toEqual(['alice', 'bob']) + expect((octokit as { paginate: ReturnType }).paginate).toHaveBeenCalledTimes(1) + }) +}) + +describe('expandOwnersSet', () => { + const context = { repo: { owner: 'myorg', repo: 'r' } } as never + const octokit = { + paginate: vi.fn(async () => [{ login: 'Bob' }]), + teams: { listMembersInOrg: vi.fn() }, + } as never + + it('expands both roles and keeps labels and sources', async () => { + const set = await expandOwnersSet(octokit, context, { + approvers: new Set(['@myorg/maintainers']), + reviewers: new Set(['alice', '@myorg/maintainers']), + labels: new Set(['area/sdk']), + sources: ['sdk/OWNERS'], + }) + + expect(set).toEqual({ + approvers: new Set(['bob']), + reviewers: new Set(['alice', 'bob']), + labels: new Set(['area/sdk']), + sources: ['sdk/OWNERS'], + }) + }) + + it('passes undefined through', async () => { + expect(await expandOwnersSet(octokit, context, undefined)).toBeUndefined() + }) +}) + +describe('ownersInclude', () => { + const context = { repo: { owner: 'myorg', repo: 'r' } } as never + + function client(members: string[]) { + return { + paginate: vi.fn(async () => members.map(login => ({ login }))), + teams: { listMembersInOrg: vi.fn() }, + } + } + + beforeEach(() => resetOwnersCaches()) + + it('answers a direct listing without reading any team', async () => { + const octokit = client(['bob']) + + expect(await ownersInclude(octokit as never, context, ['alice', '@myorg/maintainers'], 'alice')).toBe(true) + expect(octokit.paginate).not.toHaveBeenCalled() + }) + + it('reads the teams when the login is not listed directly', async () => { + const octokit = client(['bob']) + + expect(await ownersInclude(octokit as never, context, ['alice', '@myorg/maintainers'], 'bob')).toBe(true) + expect(await ownersInclude(octokit as never, context, ['alice', '@myorg/maintainers'], 'carol')).toBe(false) + expect(octokit.paginate).toHaveBeenCalledTimes(1) + }) +}) diff --git a/dist/index.js b/dist/index.js index 4319e8ef..67156fa9 100644 --- a/dist/index.js +++ b/dist/index.js @@ -41870,6 +41870,100 @@ function stringList(path, doc, key, what) { } return value; } +/** + * Parse an `@org/team-slug` entry of an approvers or reviewers list. + * + * @param entry - a lowercased OWNERS entry + * @returns undefined when the entry is a plain login + */ +function parseGroup(entry) { + const match = /^@([^/\s]+)\/([^/\s]+)$/.exec(entry); + return match === null ? undefined : { org: match[1], slug: match[2] }; +} +const groupCache = new Map(); +// entries already warned about: the same OWNERS entry is expanded once per changed file per role +const warnedEntries = new Set(); +function warnOnce(entry, message) { + if (!warnedEntries.has(entry)) { + warnedEntries.add(entry); + warning(message); + } +} +/** + * Replace the `@org/team` entries of a list of OWNERS entries with the logins of + * the team's members (transitive, as the teams API returns them). A group of + * another organization, one that cannot be read, or a malformed `@…` entry + * contributes nobody and is warned about once per run. + * + * @param octokit - a hydrated github client + * @param context - the github actions event context + * @param entries - lowercased OWNERS entries + */ +async function expandGroups(octokit, context, entries) { + const result = new Set(); + const owner = context.repo.owner.toLowerCase(); + const groups = new Set(); + for (const entry of entries) { + const group = parseGroup(entry); + if (group !== undefined) { + if (group.org === owner) { + groups.add(group.slug); + } + else { + warnOnce(entry, `OWNERS: ignoring group ${entry}: only groups of ${owner} are supported`); + } + } + else if (entry.startsWith('@')) { + warnOnce(entry, `OWNERS: ignoring ${entry}: a group is written @/`); + } + else { + result.add(entry); + } + } + const members = await Promise.all([...groups].map(slug => memoized(groupCache, `${owner}/${slug}`, async () => { + try { + const listed = await octokit.paginate(octokit.teams.listMembersInOrg, { + org: context.repo.owner, + team_slug: slug, + per_page: 100, + }); + return listed.map(member => member.login.toLowerCase()); + } + catch (e) { + warning(`OWNERS: could not read the members of @${owner}/${slug}: ${e}`); + return []; + } + }))); + members.flat().forEach(login => result.add(login)); + return result; +} +/** + * Whether a list of OWNERS entries names a login, directly or through one of + * its groups. A direct listing answers without reading any team. + * + * @param octokit - a hydrated github client + * @param context - the github actions event context + * @param entries - lowercased OWNERS entries + * @param login - a lowercased login + */ +async function ownersInclude(octokit, context, entries, login) { + const list = [...entries]; + if (list.includes(login)) { + return true; + } + return (await expandGroups(octokit, context, list)).has(login); +} +/** an OwnersSet whose approvers and reviewers hold logins only */ +async function expandOwnersSet(octokit, context, set) { + if (set === undefined) { + return undefined; + } + const [approvers, reviewers] = await Promise.all([ + expandGroups(octokit, context, set.approvers), + expandGroups(octokit, context, set.reviewers), + ]); + return { ...set, approvers, reviewers }; +} /** * The directory that contains a path: 'sdk/OWNERS' is 'sdk', 'OWNERS' is '' * @@ -42035,6 +42129,8 @@ function branchHasOwners(octokit, context, branch) { function resetOwnersCaches() { hasOwnersCache.clear(); treeCache.clear(); + groupCache.clear(); + warnedEntries.clear(); } function memoized(cache, key, compute) { let pending = cache.get(key); @@ -42138,7 +42234,7 @@ async function pullRequestOwners_load(octokit, context, pullNumber) { return pull.base.sha; }); const tree = await loadOwnersTree(octokit, context, baseSha, files); - const perFile = new Map(files.map(file => [file, effectiveOwners(file, tree.owners)])); + const perFile = new Map(await Promise.all(files.map(async (file) => [file, await expandOwnersSet(octokit, context, effectiveOwners(file, tree.owners))]))); return { number: pullNumber, baseSha, @@ -42420,7 +42516,7 @@ async function assertRootOwner(octokit, context, role, username) { return false; } const owners = parseOwners('OWNERS', contents); - if (!owners[role].includes(username.toLowerCase())) { + if (!(await ownersInclude(octokit, context, owners[role], username.toLowerCase()))) { throw new Error(`${username} is not included in the ${role} role in the OWNERS file`); } return true; @@ -42465,7 +42561,7 @@ async function rootOwnersIncludes(octokit, context, user) { } const owners = parseOwners('OWNERS', contents); const login = user.toLowerCase(); - return owners.reviewers.includes(login) || owners.approvers.includes(login); + return ownersInclude(octokit, context, [...owners.reviewers, ...owners.approvers], login); } /** * Retrieve the contents of the OWNERS file at the root of the repository. diff --git a/docs/commands.md b/docs/commands.md index b18bb8e4..db500324 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -181,6 +181,20 @@ An `OWNERS` file may be placed in any directory. It applies to the files in that Setting `options.no_parent_owners: true` in an `OWNERS` file stops the walk there, so only that file (and any below it) applies and the parents' approvers and reviewers are not inherited. +### GitHub groups + +`approvers` and `reviewers` may list a GitHub team of the repository's own organization as `@/` (quote it in YAML, since a plain `@` cannot start a scalar): + +```yaml +approvers: + - user1 + - '@myorg/maintainers' +``` + +The team's members (child teams included, as the teams API returns them) are looked up once per team per run and treated as if listed individually. A team of another organization is ignored, and so is an `@…` entry that is not `@/`; both are warned about once per run. Groups are matched when OWNERS are resolved, so changes to the team apply on the next event. + +Reading a team's members needs a token with the organization's *Members: read* permission (classic PAT: `read:org`; GitHub App: *Organization permissions → Members: read*). **The workflow's default `GITHUB_TOKEN` does not have it**: with that token every group warns `could not read the members of @org/team` and contributes nobody, so a user listed only through a group is refused. Pass such a token via the `token` secret ([installing](./installing.md#inputs-and-secrets)). A login listed directly is matched without reading any team. + ### Which files decide the outcome On a pull request the changed files are listed (for renames both the old and the new path count) and their OWNERS are read from the **current tip of the pull request's base branch**, never from the pull request itself: the head branch is never consulted, so a pull request cannot grant itself approvers by editing an `OWNERS` file, and a pull request opened before the OWNERS files existed picks them up without a rebase (GitHub's `base.sha` is a snapshot from the PR's last update; the bot reads the branch instead, one `GET /branches/{base}` per branch per run). The `[APPROVALNOTIFIER]` links point at that tip. diff --git a/docs/installing.md b/docs/installing.md index a0a15f0b..a28eb374 100644 --- a/docs/installing.md +++ b/docs/installing.md @@ -185,7 +185,7 @@ Input | Default | Meaning Secret | Default | Meaning --- | --- | --- -`token` | the caller's `github.token` | A token that can read a private `/.project`, a bot user's token so merges trigger `push` workflows ([events](./events.md#check_suite-and-status)), or one with the `workflows` scope so fork pull requests whose merge involves workflow files can be merged ([automatic merging](./automatic-merging.md#fork-pull-requests-and-workflow-files)). +`token` | the caller's `github.token` | A token that can read a private `/.project`, a bot user's token so merges trigger `push` workflows ([events](./events.md#check_suite-and-status)), one with the `workflows` scope so fork pull requests whose merge involves workflow files can be merged ([automatic merging](./automatic-merging.md#fork-pull-requests-and-workflow-files)), or one with the organization's *Members: read* permission so OWNERS files may list teams ([commands](./commands.md#github-groups)). `cat-api-key` | — | The [thecatapi.com](https://thecatapi.com) key for [`/meow`](./commands.md#enabling-meow). ```yaml diff --git a/docs/prow-for-your-org.md b/docs/prow-for-your-org.md index fa37cfe1..ed3ec55d 100644 --- a/docs/prow-for-your-org.md +++ b/docs/prow-for-your-org.md @@ -188,7 +188,10 @@ options: Those are all the keys supported: `approvers`, `reviewers`, `labels`, `options.no_parent_owners`. `OWNERS_ALIASES`, `filters` and `emeritus_approvers` are not -supported. [commands](./commands.md#owners) has the full format and the failure modes. +supported; in place of aliases, `approvers` and `reviewers` may list a GitHub team of your own +organization as `'@/'`, which needs a `token` that can read the organization's +members ([commands](./commands.md#github-groups)). [commands](./commands.md#owners) has the full +format and the failure modes. What changes the moment a pull request's base branch (your default branch, for most) carries any OWNERS file, open pull requests included: @@ -231,6 +234,7 @@ lines. | Branch protection or rulesets require reviews/checks the `GITHUB_TOKEN` cannot satisfy | `mergeable_state` is `blocked`; the event path skips the merge and logs the state | Lower the requirement, or let a human's review satisfy it ([automatic merging](./automatic-merging.md#event-driven-merging)) | | A merge made with `GITHUB_TOKEN` fires no `push` workflows | post-merge automation does not run | pass a PAT or GitHub App token via the `token` secret ([installing](./installing.md#inputs-and-secrets)) | | `/.project` is private | a consumer's `GITHUB_TOKEN` cannot read it; the org tier silently falls through to `/.github` | keep the org config in `.github`, or pass a `token` that can read `.project` ([installing](./installing.md#the-project-tier)) | +| An OWNERS file lists a team (`'@org/team'`) and its members are refused | `GITHUB_TOKEN` has no organization permissions, so the team cannot be read: one `could not read the members of @org/team` warning, and the group contributes nobody ([commands](./commands.md#github-groups)) | pass a `token` with the organization's *Members: read* permission (PAT `read:org`), or list the logins directly | | Fork PRs | the template uses `pull_request_target`, so forks get labels and merges; safe because nothing checks out PR code ([events](./events.md#pull_request_target-and-the-reusable-workflow)) | nothing | | A fork PR passes the gate but will not merge: `403 Resource not accessible by integration` | the merge involves workflow files the fork's branch lacks (or changes); `GITHUB_TOKEN` cannot get the `workflows` permission. The bot comments once and skips the PR ([fork pull requests and workflow files](./automatic-merging.md#fork-pull-requests-and-workflow-files)) | the author rebases onto the base branch; or pass a `token` with the `workflows` scope | | zizmor (`dangerous-triggers`) or a hash-pin org policy rejects `pull_request_target` | `pull_request` gives fork PRs a read-only token | use the [`pull_request` template](./installing.md#without-pull_request_target): fork PRs are handled by the scheduled `sweep` within minutes, comments stay instant | diff --git a/src/utils/auth.ts b/src/utils/auth.ts index db993139..f5d5d1f9 100644 --- a/src/utils/auth.ts +++ b/src/utils/auth.ts @@ -7,7 +7,7 @@ import { Buffer } from 'node:buffer' import * as core from '@actions/core' import { defaultAuthorization, loadProwConfig, resolveAuthorization } from './config' -import { parseOwners } from './owners' +import { ownersInclude, parseOwners } from './owners' import { loadPullRequestOwners } from './pullRequestOwners' function getErrorDetails(error: unknown): { status: unknown, message: string } { @@ -356,7 +356,7 @@ async function assertRootOwner( } const owners = parseOwners('OWNERS', contents) - if (!owners[role].includes(username.toLowerCase())) { + if (!(await ownersInclude(octokit, context, owners[role], username.toLowerCase()))) { throw new Error( `${username} is not included in the ${role} role in the OWNERS file`, ) @@ -420,7 +420,7 @@ async function rootOwnersIncludes( const owners = parseOwners('OWNERS', contents) const login = user.toLowerCase() - return owners.reviewers.includes(login) || owners.approvers.includes(login) + return ownersInclude(octokit, context, [...owners.reviewers, ...owners.approvers], login) } /** diff --git a/src/utils/owners.ts b/src/utils/owners.ts index 52c8306a..35a3ad19 100644 --- a/src/utils/owners.ts +++ b/src/utils/owners.ts @@ -89,6 +89,121 @@ function stringList( return value } +/** + * Parse an `@org/team-slug` entry of an approvers or reviewers list. + * + * @param entry - a lowercased OWNERS entry + * @returns undefined when the entry is a plain login + */ +export function parseGroup(entry: string): { org: string, slug: string } | undefined { + const match = /^@([^/\s]+)\/([^/\s]+)$/.exec(entry) + return match === null ? undefined : { org: match[1]!, slug: match[2]! } +} + +const groupCache = new Map>() +// entries already warned about: the same OWNERS entry is expanded once per changed file per role +const warnedEntries = new Set() + +function warnOnce(entry: string, message: string): void { + if (!warnedEntries.has(entry)) { + warnedEntries.add(entry) + core.warning(message) + } +} + +/** + * Replace the `@org/team` entries of a list of OWNERS entries with the logins of + * the team's members (transitive, as the teams API returns them). A group of + * another organization, one that cannot be read, or a malformed `@…` entry + * contributes nobody and is warned about once per run. + * + * @param octokit - a hydrated github client + * @param context - the github actions event context + * @param entries - lowercased OWNERS entries + */ +export async function expandGroups( + octokit: Octokit, + context: Context, + entries: Iterable, +): Promise> { + const result = new Set() + const owner = context.repo.owner.toLowerCase() + const groups = new Set() + for (const entry of entries) { + const group = parseGroup(entry) + if (group !== undefined) { + if (group.org === owner) { + groups.add(group.slug) + } + else { + warnOnce(entry, `OWNERS: ignoring group ${entry}: only groups of ${owner} are supported`) + } + } + else if (entry.startsWith('@')) { + warnOnce(entry, `OWNERS: ignoring ${entry}: a group is written @/`) + } + else { + result.add(entry) + } + } + + const members = await Promise.all([...groups].map(slug => + memoized(groupCache, `${owner}/${slug}`, async () => { + try { + const listed = await octokit.paginate(octokit.teams.listMembersInOrg, { + org: context.repo.owner, + team_slug: slug, + per_page: 100, + }) + return listed.map(member => member.login.toLowerCase()) + } + catch (e) { + core.warning(`OWNERS: could not read the members of @${owner}/${slug}: ${e}`) + return [] + } + }))) + members.flat().forEach(login => result.add(login)) + return result +} + +/** + * Whether a list of OWNERS entries names a login, directly or through one of + * its groups. A direct listing answers without reading any team. + * + * @param octokit - a hydrated github client + * @param context - the github actions event context + * @param entries - lowercased OWNERS entries + * @param login - a lowercased login + */ +export async function ownersInclude( + octokit: Octokit, + context: Context, + entries: Iterable, + login: string, +): Promise { + const list = [...entries] + if (list.includes(login)) { + return true + } + return (await expandGroups(octokit, context, list)).has(login) +} + +/** an OwnersSet whose approvers and reviewers hold logins only */ +export async function expandOwnersSet( + octokit: Octokit, + context: Context, + set: OwnersSet | undefined, +): Promise { + if (set === undefined) { + return undefined + } + const [approvers, reviewers] = await Promise.all([ + expandGroups(octokit, context, set.approvers), + expandGroups(octokit, context, set.reviewers), + ]) + return { ...set, approvers, reviewers } +} + /** * The directory that contains a path: 'sdk/OWNERS' is 'sdk', 'OWNERS' is '' * @@ -301,6 +416,8 @@ export function branchHasOwners(octokit: Octokit, context: Context, branch: stri export function resetOwnersCaches(): void { hasOwnersCache.clear() treeCache.clear() + groupCache.clear() + warnedEntries.clear() } function memoized(cache: Map>, key: string, compute: () => Promise): Promise { diff --git a/src/utils/pullRequestOwners.ts b/src/utils/pullRequestOwners.ts index b4dbfc9b..a50c2cc7 100644 --- a/src/utils/pullRequestOwners.ts +++ b/src/utils/pullRequestOwners.ts @@ -4,7 +4,7 @@ import type { OwnersSet, OwnersTree } from './owners' import * as core from '@actions/core' -import { effectiveOwners, loadOwnersTree } from './owners' +import { effectiveOwners, expandOwnersSet, loadOwnersTree } from './owners' /** a pull request and the OWNERS covering its changed files; every login is lowercased */ export interface PullRequestOwners { @@ -83,7 +83,8 @@ async function load( return pull.base.sha }) const tree = await loadOwnersTree(octokit, context, baseSha, files) - const perFile = new Map(files.map(file => [file, effectiveOwners(file, tree.owners)])) + const perFile = new Map(await Promise.all(files.map(async file => + [file, await expandOwnersSet(octokit, context, effectiveOwners(file, tree.owners))] as const))) return { number: pullNumber,