From 0c3a5ceca33cba049344887463e778b5192930f9 Mon Sep 17 00:00:00 2001 From: mchwang Date: Thu, 24 Sep 2026 09:35:17 -0700 Subject: [PATCH 1/3] Fix repository-scoped issue trust --- docs/implementation/issue-prioritization.md | 28 +++++--- github/issues.ts | 54 +++++++++++++- test/issue-ranking.test.ts | 1 + test/issues.test.ts | 79 +++++++++++++++++++-- 4 files changed, 146 insertions(+), 16 deletions(-) diff --git a/docs/implementation/issue-prioritization.md b/docs/implementation/issue-prioritization.md index fa74aac..b4b24bd 100644 --- a/docs/implementation/issue-prioritization.md +++ b/docs/implementation/issue-prioritization.md @@ -3,6 +3,12 @@ Baseline: `0ae71a503592de90926063fa563c1f7c715db22b` (`origin/main`, 2026-09-24). +Follow-up #41 was reproduced from validated-head baseline +`1e59b7dfda199c8156c09a9cb9a690e5b55f1b5e`: GitHub documents `MEMBER` as +organization membership, while its repository-collaborator endpoint reports +the identities with access to the repository. The trust policy below records +the corrected repository-scoped boundary. + ## H1 decision: ranking policy The first released policy is deterministic, explainable, and independent of an @@ -26,9 +32,12 @@ Issues sort by descending score, then oldest creation time, then ascending issue number. Every contributing signal is emitted as a user-visible reason. An issue with no contributing signal says that it has no configured priority signals. -An issue is trusted by default only when GitHub reports its author association -as `OWNER`, `MEMBER`, or `COLLABORATOR`. Other issues remain visible but require -an explicit trust decision before queueing. Trust affects eligibility, never the +An issue is trusted by default only when its author appears in GitHub's current, +repository-scoped collaborator list. GitHub's `author_association` remains +validated and visible metadata, but does not grant trust: in particular, +`MEMBER` proves organization membership rather than access to this repository. +Issues from deleted or non-collaborating authors remain visible but require an +explicit trust decision before queueing. Trust affects eligibility, never the score, so an untrusted author cannot improve rank by embedding instructions in issue text. @@ -41,14 +50,17 @@ dedicated read-only gateway with these boundaries: - Codeboost invokes `gh` with literal arguments; issue text is parsed only as data and is never interpolated into a shell command or prompt. -- The gateway fetches open issues, excludes pull requests, follows bounded - pagination, and validates every field used for normalization or ranking. +- The gateway fetches open issues and the repository's current collaborators, + excludes pull requests, follows bounded pagination for both collections, and + validates every field used for normalization, trust, or ranking. If the + configured GitHub identity cannot read collaborators, the refresh is + unavailable rather than falling back to author-association metadata. - A complete successful snapshot includes repository identity and a retrieval timestamp. A subsequent retrieval failure returns an explicit stale snapshot only when a previously validated snapshot exists; otherwise it is unavailable. -- Malformed fields, an exceeded issue/page limit, an unknown author association, - or an incomplete response fail the entire refresh closed. Partial records are - never ranked as if missing values were zero. +- Malformed fields, an exceeded issue or collaborator limit, an unknown author + association, or an incomplete response fail the entire refresh closed. + Partial records are never ranked as if missing values were zero. ## Ownership and staged delivery diff --git a/github/issues.ts b/github/issues.ts index e77ca60..b2754b6 100644 --- a/github/issues.ts +++ b/github/issues.ts @@ -5,6 +5,7 @@ const runFile = promisify(execFile); const PAGE_SIZE = 100; const MAX_PAGES = 10; const MAX_ISSUES = PAGE_SIZE * MAX_PAGES; +const MAX_COLLABORATORS = PAGE_SIZE * MAX_PAGES; const MAX_BODY_LENGTH = 65_536; // Covers one bounded 100-record page, including JSON-escaped bodies, labels and response overhead. export const ISSUE_PAGE_MAX_BYTES = 64 * 1024 * 1024; @@ -24,6 +25,7 @@ export interface RepositoryIssue { readonly comments: number; readonly positiveReactions: number; readonly labels: readonly string[]; + readonly authorLogin: string | null; readonly authorAssociation: IssueAuthorAssociation; readonly trust: 'trusted' | 'requires-approval'; } @@ -62,6 +64,12 @@ function count(value: unknown, field: string): number { return value as number; } +function login(value: unknown, context: 'issue author' | 'collaborator'): string { + if (typeof value !== 'string' || !value || value.length > 100 || /[\s\u0000-\u001f\u007f]/u.test(value)) + throw new Error(`GitHub returned an invalid ${context} login.`); + return value; +} + function repositoryName(value: string): boolean { if (value.length > 201 || !/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(value)) return false; return value.split('/').every(part => part !== '.' && part !== '..' && part.length <= 100); @@ -113,6 +121,10 @@ function normalizeIssue(repository: string, value: unknown): RepositoryIssue | n if (typeof authorAssociation !== 'string' || !associations.has(authorAssociation as IssueAuthorAssociation)) throw new Error('GitHub returned an unknown issue author association.'); const association = authorAssociation as IssueAuthorAssociation; + if (!Object.hasOwn(issue, 'user')) throw new Error('GitHub omitted the issue author.'); + const authorLogin = issue.user === null + ? null + : login(object(issue.user, 'GitHub returned an invalid issue author.').login, 'issue author'); const title = boundedString(issue.title, 'title', 4096); if (!title.trim()) throw new Error('GitHub returned an empty issue title.'); return { @@ -126,8 +138,9 @@ function normalizeIssue(repository: string, value: unknown): RepositoryIssue | n comments: count(issue.comments, 'comment count'), positiveReactions, labels, + authorLogin, authorAssociation: association, - trust: ['OWNER', 'MEMBER', 'COLLABORATOR'].includes(association) ? 'trusted' : 'requires-approval', + trust: 'requires-approval', }; } @@ -147,7 +160,7 @@ export class GhIssueGateway implements IssueGateway { this.now = now; } - async #load(signal: AbortSignal): Promise { + async #loadIssues(signal: AbortSignal): Promise { const issues: RepositoryIssue[] = []; const numbers = new Set(); for (let page = 1; page <= MAX_PAGES; page++) { @@ -172,6 +185,43 @@ export class GhIssueGateway implements IssueGateway { throw new Error(`Issue retrieval exceeded the ${MAX_ISSUES}-record safety limit.`); } + async #loadCollaborators(signal: AbortSignal): Promise> { + const collaborators = new Set(); + for (let page = 1; page <= MAX_PAGES; page++) { + const output = await this.run([ + 'api', '--method', 'GET', '-H', 'Accept: application/vnd.github+json', + `repos/${this.repository}/collaborators`, '-f', 'affiliation=all', + '-f', `per_page=${PAGE_SIZE}`, '-f', `page=${page}`, + ], { signal }); + let decoded: unknown; + try { decoded = JSON.parse(output); } + catch { throw new Error('GitHub returned invalid collaborator JSON.'); } + if (!Array.isArray(decoded)) throw new Error('GitHub returned an invalid collaborator page.'); + if (decoded.length > PAGE_SIZE) throw new Error('GitHub returned an oversized collaborator page.'); + for (const value of decoded) { + const author = object(value, 'GitHub returned an invalid collaborator.'); + const key = login(author.login, 'collaborator').toLocaleLowerCase('en-US'); + if (collaborators.has(key)) throw new Error('GitHub returned a duplicate collaborator.'); + collaborators.add(key); + } + if (decoded.length < PAGE_SIZE) return collaborators; + } + throw new Error(`Collaborator retrieval exceeded the ${MAX_COLLABORATORS}-record safety limit.`); + } + + async #load(signal: AbortSignal): Promise { + const issues = await this.#loadIssues(signal); + if (!issues.some(issue => issue.authorLogin !== null)) return issues; + signal.throwIfAborted(); + const collaborators = await this.#loadCollaborators(signal); + return issues.map(issue => ({ + ...issue, + trust: issue.authorLogin !== null && collaborators.has(issue.authorLogin.toLocaleLowerCase('en-US')) + ? 'trusted' + : 'requires-approval', + })); + } + async fetch(options: { signal?: AbortSignal; timeoutMs?: number } = {}): Promise { const timeoutMs = options.timeoutMs ?? 12_000; if (!Number.isSafeInteger(timeoutMs) || timeoutMs < 1 || timeoutMs > 30_000) throw new Error('Invalid issue retrieval timeout.'); diff --git a/test/issue-ranking.test.ts b/test/issue-ranking.test.ts index 59a7708..ee2aaf5 100644 --- a/test/issue-ranking.test.ts +++ b/test/issue-ranking.test.ts @@ -13,6 +13,7 @@ const issue = (number: number, overrides: Partial = {}): Reposi comments: 0, positiveReactions: 0, labels: [], + authorLogin: 'member', authorAssociation: 'MEMBER', trust: 'trusted', ...overrides, diff --git a/test/issues.test.ts b/test/issues.test.ts index 0b3f67b..164eae2 100644 --- a/test/issues.test.ts +++ b/test/issues.test.ts @@ -10,13 +10,38 @@ const rawIssue = (overrides: Record = {}) => ({ created_at: '2026-01-01T00:00:00Z', updated_at: '2026-01-02T00:00:00Z', comments: 3, + user: { login: 'member' }, author_association: 'MEMBER', labels: [{ name: 'bug' }, { name: 'P1' }], reactions: { '+1': 2, heart: 1, hooray: 0, rocket: 1 }, ...overrides, }); +const isCollaboratorRequest = (args: readonly string[]) => + args.some(argument => argument.toLocaleLowerCase('en-US').endsWith('/collaborators')); +const responses = (issues: readonly unknown[], collaborators: readonly string[] = ['MEMBER']) => + async (args: readonly string[]) => JSON.stringify(isCollaboratorRequest(args) + ? collaborators.map(login => ({ login })) + : issues); + describe('GitHub issue retrieval', () => { + it.each([ + 'OWNER', + 'MEMBER', + 'COLLABORATOR', + 'CONTRIBUTOR', + 'FIRST_TIMER', + 'FIRST_TIME_CONTRIBUTOR', + 'MANNEQUIN', + 'NONE', + ] as const)('classifies the intermediate %s association using current repository membership', async (authorAssociation) => { + const issue = rawIssue({ author_association: authorAssociation }); + const trusted = await new GhIssueGateway('owner/repo', responses([issue])).fetch(); + const outside = await new GhIssueGateway('owner/repo', responses([issue], [])).fetch(); + expect(trusted.issues[0]).toMatchObject({ authorAssociation, authorLogin: 'member', trust: 'trusted' }); + expect(outside.issues[0]).toMatchObject({ authorAssociation, authorLogin: 'member', trust: 'requires-approval' }); + }); + it.each(['./repo', '../repo', 'owner/..'])('rejects unsafe repository identity %s', repository => { expect(() => new GhIssueGateway(repository)).toThrow('GitHub repository'); }); @@ -25,12 +50,15 @@ describe('GitHub issue retrieval', () => { const calls: readonly string[][] = []; const run = vi.fn(async (args: readonly string[]) => { (calls as string[][]).push([...args]); - return JSON.stringify([rawIssue()]); + return responses([rawIssue()])(args); }); const snapshot = await new GhIssueGateway('owner/repo', run, () => new Date('2026-02-01T00:00:00Z')).fetch(); expect(calls).toEqual([[ 'api', '--method', 'GET', '-H', 'Accept: application/vnd.github+json', 'repos/owner/repo/issues', '-f', 'state=open', '-f', 'per_page=100', '-f', 'page=1', + ], [ + 'api', '--method', 'GET', '-H', 'Accept: application/vnd.github+json', + 'repos/owner/repo/collaborators', '-f', 'affiliation=all', '-f', 'per_page=100', '-f', 'page=1', ]]); expect(snapshot).toEqual({ repository: 'owner/repo', @@ -39,23 +67,23 @@ describe('GitHub issue retrieval', () => { repository: 'owner/repo', number: 7, title: 'Fix retries', body: 'Keep issue text as data.', url: 'https://github.com/owner/repo/issues/7', createdAt: '2026-01-01T00:00:00Z', updatedAt: '2026-01-02T00:00:00Z', comments: 3, positiveReactions: 4, - labels: ['bug', 'P1'], authorAssociation: 'MEMBER', trust: 'trusted', + labels: ['bug', 'P1'], authorLogin: 'member', authorAssociation: 'MEMBER', trust: 'trusted', }], }); }); it('excludes pull requests and requires approval for outside authors', async () => { - const run = vi.fn(async () => JSON.stringify([ + const run = vi.fn(responses([ rawIssue({ pull_request: { url: 'https://api.github.com/repos/owner/repo/pulls/7' } }), rawIssue({ number: 8, html_url: 'https://github.com/owner/repo/issues/8', author_association: 'CONTRIBUTOR' }), - ])); + ], [])); const snapshot = await new GhIssueGateway('owner/repo', run).fetch(); expect(snapshot.issues).toHaveLength(1); expect(snapshot.issues[0]).toMatchObject({ number: 8, trust: 'requires-approval' }); }); it('accepts canonical URL casing without relaxing repository identity or URL shape', async () => { - const run = vi.fn(async () => JSON.stringify([ + const run = vi.fn(responses([ rawIssue(), rawIssue({ number: 8, @@ -68,13 +96,20 @@ describe('GitHub issue retrieval', () => { }); it('accepts the maximum bounded issue body', async () => { - const gateway = new GhIssueGateway('owner/repo', async () => JSON.stringify([ + const gateway = new GhIssueGateway('owner/repo', responses([ rawIssue({ body: 'x'.repeat(65_536) }), ])); const snapshot = await gateway.fetch(); expect(snapshot.issues[0]?.body).toHaveLength(65_536); }); + it('requires approval when GitHub explicitly reports a deleted author', async () => { + const snapshot = await new GhIssueGateway('owner/repo', responses([ + rawIssue({ user: null, author_association: 'OWNER' }), + ])).fetch(); + expect(snapshot.issues[0]).toMatchObject({ authorLogin: null, trust: 'requires-approval' }); + }); + it('budgets for a maximum page of JSON-escaped control-character bodies', () => { const body = '\0'.repeat(65_536); const page = Array.from({ length: 100 }, (_, index) => rawIssue({ @@ -92,6 +127,8 @@ describe('GitHub issue retrieval', () => { ['state', { state: 'closed' }], ['pull request marker', { pull_request: {} }], ['author association', { author_association: 'UNKNOWN' }], + ['author object', { user: 42 }], + ['author login', { user: { login: '' } }], ['title', { title: ' ' }], ['body length', { body: 'x'.repeat(65_537) }], ['comments', { comments: -1 }], @@ -104,6 +141,36 @@ describe('GitHub issue retrieval', () => { await expect(gateway.fetch()).rejects.toThrow(/GitHub returned/); }); + it('fails closed when collaborator pagination reaches its bounded limit', async () => { + const gateway = new GhIssueGateway('owner/repo', async args => { + if (!isCollaboratorRequest(args)) return JSON.stringify([rawIssue()]); + const page = Number(args.at(-1)?.split('=')[1]); + return JSON.stringify(Array.from({ length: 100 }, (_, index) => ({ + login: `member-${(page - 1) * 100 + index + 1}`, + }))); + }); + await expect(gateway.fetch()).rejects.toThrow('Collaborator retrieval exceeded the 1000-record safety limit'); + }); + + it('distinguishes an omitted author from an explicitly deleted author', async () => { + const issue: Record = rawIssue(); + delete issue.user; + await expect(new GhIssueGateway('owner/repo', responses([issue])).fetch()) + .rejects.toThrow('omitted the issue author'); + }); + + it.each([ + ['page shape', {}], + ['record shape', [null]], + ['login', [{ login: '' }]], + ['duplicate', [{ login: 'member' }, { login: 'MEMBER' }]], + ])('fails closed on invalid collaborator %s', async (_label, collaboratorPage) => { + const gateway = new GhIssueGateway('owner/repo', async args => JSON.stringify( + isCollaboratorRequest(args) ? collaboratorPage : [rawIssue()], + )); + await expect(gateway.fetch()).rejects.toThrow(/GitHub returned/); + }); + it('rejects duplicate records across pages', async () => { let page = 0; const gateway = new GhIssueGateway('owner/repo', async () => { From cf94c4c0dae9eb0df5e20d1c22c7c13da195b9e9 Mon Sep 17 00:00:00 2001 From: mchwang Date: Thu, 24 Sep 2026 09:41:56 -0700 Subject: [PATCH 2/3] Keep collaborator validation fail closed --- github/issues.ts | 2 +- test/issues.test.ts | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/github/issues.ts b/github/issues.ts index b2754b6..095d816 100644 --- a/github/issues.ts +++ b/github/issues.ts @@ -211,7 +211,7 @@ export class GhIssueGateway implements IssueGateway { async #load(signal: AbortSignal): Promise { const issues = await this.#loadIssues(signal); - if (!issues.some(issue => issue.authorLogin !== null)) return issues; + if (issues.length === 0) return issues; signal.throwIfAborted(); const collaborators = await this.#loadCollaborators(signal); return issues.map(issue => ({ diff --git a/test/issues.test.ts b/test/issues.test.ts index 164eae2..054b93c 100644 --- a/test/issues.test.ts +++ b/test/issues.test.ts @@ -110,6 +110,14 @@ describe('GitHub issue retrieval', () => { expect(snapshot.issues[0]).toMatchObject({ authorLogin: null, trust: 'requires-approval' }); }); + it('fails closed on collaborator-access failure even when every author is deleted', async () => { + const gateway = new GhIssueGateway('owner/repo', async args => { + if (isCollaboratorRequest(args)) throw new Error('Collaborators unavailable.'); + return JSON.stringify([rawIssue({ user: null, author_association: 'OWNER' })]); + }); + await expect(gateway.fetch()).rejects.toThrow('Collaborators unavailable.'); + }); + it('budgets for a maximum page of JSON-escaped control-character bodies', () => { const body = '\0'.repeat(65_536); const page = Array.from({ length: 100 }, (_, index) => rawIssue({ From 9bf0c951b2855fdd9fee7b0cfdb7e4350a0ec2b6 Mon Sep 17 00:00:00 2001 From: mchwang Date: Thu, 24 Sep 2026 09:46:34 -0700 Subject: [PATCH 3/3] Validate collaborator access for empty snapshots --- github/issues.ts | 1 - test/issues.test.ts | 8 ++++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/github/issues.ts b/github/issues.ts index 095d816..a3d3e91 100644 --- a/github/issues.ts +++ b/github/issues.ts @@ -211,7 +211,6 @@ export class GhIssueGateway implements IssueGateway { async #load(signal: AbortSignal): Promise { const issues = await this.#loadIssues(signal); - if (issues.length === 0) return issues; signal.throwIfAborted(); const collaborators = await this.#loadCollaborators(signal); return issues.map(issue => ({ diff --git a/test/issues.test.ts b/test/issues.test.ts index 054b93c..78991af 100644 --- a/test/issues.test.ts +++ b/test/issues.test.ts @@ -118,6 +118,14 @@ describe('GitHub issue retrieval', () => { await expect(gateway.fetch()).rejects.toThrow('Collaborators unavailable.'); }); + it('fails closed on collaborator-access failure even when no issues are open', async () => { + const gateway = new GhIssueGateway('owner/repo', async args => { + if (isCollaboratorRequest(args)) throw new Error('Collaborators unavailable.'); + return '[]'; + }); + await expect(gateway.fetch()).rejects.toThrow('Collaborators unavailable.'); + }); + it('budgets for a maximum page of JSON-escaped control-character bodies', () => { const body = '\0'.repeat(65_536); const page = Array.from({ length: 100 }, (_, index) => rawIssue({