From 6fdcb9aed60ec6ef3dd92c24ee05cd5b83336e51 Mon Sep 17 00:00:00 2001 From: quality Date: Sat, 3 Oct 2026 16:06:12 -0400 Subject: [PATCH 1/2] test(bundle): drive root-OWNERS authorization on an issue and the OWNERS-based /lgtm refusal through dist/index.js Signed-off-by: quality --- __tests__/bundle/ownersAuth.test.ts | 92 +++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 __tests__/bundle/ownersAuth.test.ts diff --git a/__tests__/bundle/ownersAuth.test.ts b/__tests__/bundle/ownersAuth.test.ts new file mode 100644 index 00000000..9bb3b82c --- /dev/null +++ b/__tests__/bundle/ownersAuth.test.ts @@ -0,0 +1,92 @@ +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 { blobSha, prCommentEvent } from '../utils/ownersFixtures' +import { start } from './fakeGithub' +import { comment, helpersFor, ownersProbe, ownersReads, queueRead, repo, token } from './helpers' +import { runBundle } from './runBundle' + +vi.setConfig({ testTimeout: 30_000 }) + +// the OWNERS-based authorization of a reviewer, driven through dist/index.js: on an issue the root OWNERS file +// of the default branch decides (the membership fallback in bundle.test.ts never reads it), and on a pull +// request the OWNERS covering the changed files refuse a commenter they do not list; the post-command +// needs-* re-check reads the configuration after the command either way +describe('dist/index.js /lgtm authorized by OWNERS files', () => { + const rootOwnersRead = `GET ${repo}/contents/OWNERS` + const commentPost = `POST ${repo}/issues/1/comments` + const labelsRead = `GET ${repo}/labels?per_page=100` + const labelsPost = `POST ${repo}/issues/1/labels` + let gh: FakeGithub + const { 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 lgtm(payload: Record) { + return runBundle({ eventName: 'issue_comment', payload, inputs: { ...token, 'prow-commands': '/lgtm' }, apiUrl: gh.url }) + } + + function comments() { + return gh.requestsMatching('POST', /\/issues\/1\/comments$/).map(r => (r.body as { body: string }).body) + } + + it('on an issue, a reviewer listed in the root OWNERS file is authorized by that file alone: no membership read', async () => { + routeRootOwners('reviewers:\n- codertocat\n') + gh.route('GET', `${repo}/labels`, { status: 200, body: [{ name: 'lgtm' }] }) + gh.route('POST', `${repo}/issues/1/labels`, { status: 200, body: [] }) + + 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, labelsRead, labelsPost]) + }) + + it('on an issue, a commenter the root OWNERS file does not list is refused by name, with no membership fallback', async () => { + routeRootOwners('reviewers:\n- alice\n') + 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(comments()).toEqual(['Cannot apply the lgtm label because Error: Codertocat is not included in the reviewers role in the OWNERS file']) + expect(gh.requestsMatching('POST', /\/issues\/1\/labels$/)).toEqual([]) + expectCommandThenConfig([rootOwnersRead, commentPost]) + }) + + it('on a pull request, a commenter no covering OWNERS file lists is refused: the root file is never read', async () => { + routeOwners({ 'OWNERS': 'reviewers:\n- alice\n', 'sdk/OWNERS': 'approvers:\n- bob\n' }, ['sdk/file.go'], { user: { login: 'some-author' } }) + gh.route('POST', `${repo}/issues/1/comments`, { status: 201, body: {} }) + + const result = await lgtm(prCommentEvent('/lgtm', 'stranger')) + + expect(result.status, result.stdout).toBe(1) + expect(result.errors.some(e => e.includes('stranger is not a reviewer or approver for any changed file'))).toBe(true) + expect(comments()).toEqual(['Cannot apply the lgtm label because Error: stranger is not a reviewer or approver for any changed file']) + expect(gh.requestsMatching('POST', /\/issues\/1\/labels$/)).toEqual([]) + expect(gh.requestsMatching('GET', /\/contents\/OWNERS$/)).toEqual([]) + expect(gh.requestsMatching('GET', /\/orgs\/|\/collaborators\//)).toEqual([]) + // the owners reads, the two OWNERS blobs, the refusal, then the config reads and tide's gate on the pull request + expectCommandThenConfig( + [`GET ${repo}/git/blobs/${blobSha('OWNERS')}`, `GET ${repo}/git/blobs/${blobSha('sdk/OWNERS')}`, commentPost], + [`GET ${repo}/pulls/1`, ownersProbe, queueRead], + ownersReads, + ) + }) +}) From ec40611fcf173d4d4763386584d7e09ebe3fc480 Mon Sep 17 00:00:00 2001 From: quality Date: Wed, 7 Oct 2026 22:12:05 -0400 Subject: [PATCH 2/2] test(bundle): accept the two OWNERS blob reads in any order and check the refusal reason only Signed-off-by: quality --- __tests__/bundle/ownersAuth.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/__tests__/bundle/ownersAuth.test.ts b/__tests__/bundle/ownersAuth.test.ts index 9bb3b82c..5edca205 100644 --- a/__tests__/bundle/ownersAuth.test.ts +++ b/__tests__/bundle/ownersAuth.test.ts @@ -16,6 +16,8 @@ vi.setConfig({ testTimeout: 30_000 }) // needs-* re-check reads the configuration after the command either way describe('dist/index.js /lgtm authorized by OWNERS files', () => { const rootOwnersRead = `GET ${repo}/contents/OWNERS` + const blobOWNERS = `GET ${repo}/git/blobs/${blobSha('OWNERS')}` + const blobSdkOWNERS = `GET ${repo}/git/blobs/${blobSha('sdk/OWNERS')}` const commentPost = `POST ${repo}/issues/1/comments` const labelsRead = `GET ${repo}/labels?per_page=100` const labelsPost = `POST ${repo}/issues/1/labels` @@ -65,7 +67,7 @@ describe('dist/index.js /lgtm authorized by OWNERS files', () => { 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(comments()).toEqual(['Cannot apply the lgtm label because Error: Codertocat is not included in the reviewers role in the OWNERS file']) + expect(comments().some(c => c.includes('Codertocat is not included in the reviewers role in the OWNERS file'))).toBe(true) expect(gh.requestsMatching('POST', /\/issues\/1\/labels$/)).toEqual([]) expectCommandThenConfig([rootOwnersRead, commentPost]) }) @@ -78,15 +80,15 @@ describe('dist/index.js /lgtm authorized by OWNERS files', () => { expect(result.status, result.stdout).toBe(1) expect(result.errors.some(e => e.includes('stranger is not a reviewer or approver for any changed file'))).toBe(true) - expect(comments()).toEqual(['Cannot apply the lgtm label because Error: stranger is not a reviewer or approver for any changed file']) + expect(comments().some(c => c.includes('stranger is not a reviewer or approver for any changed file'))).toBe(true) expect(gh.requestsMatching('POST', /\/issues\/1\/labels$/)).toEqual([]) expect(gh.requestsMatching('GET', /\/contents\/OWNERS$/)).toEqual([]) expect(gh.requestsMatching('GET', /\/orgs\/|\/collaborators\//)).toEqual([]) // the owners reads, the two OWNERS blobs, the refusal, then the config reads and tide's gate on the pull request expectCommandThenConfig( - [`GET ${repo}/git/blobs/${blobSha('OWNERS')}`, `GET ${repo}/git/blobs/${blobSha('sdk/OWNERS')}`, commentPost], + [commentPost], [`GET ${repo}/pulls/1`, ownersProbe, queueRead], - ownersReads, + [...ownersReads, blobOWNERS, blobSdkOWNERS], ) }) })