diff --git a/CHANGELOG.md b/CHANGELOG.md index 14fc3e40..00e14779 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ published version with a date and open a fresh empty `[Unreleased]` above it. ### Added +- `@relayfile/adapter-github/webhook-identity` now exports fail-closed check-run pull-request identity parsing for API and HTML URLs, including repository ownership validation for webhook consumers. - `@relayfile/adapter-gitlab` now creates issues, branches, and merge requests from file-native drafts; accepts or closes/reopens merge requests through canonical sidecars; and publishes schemas, examples, catalog paths, and `LAYOUT.md` guidance for every supported GitLab writeback route. - `@relayfile/adapter-github` now declares and routes pull-request `ready_for_review`, `labeled`, and `unlabeled` webhooks so review flows can wake on draft and policy-label transitions. - `@relayfile/adapter-github` and the core GitHub mapping now declare `check_run` and `issue_comment` webhook keys, so consumers that read the mapping's `webhooks:` block can subscribe to CI check completions and issue/PR conversation comments. diff --git a/packages/github/package.json b/packages/github/package.json index 61ca6b1f..9bcf95e4 100644 --- a/packages/github/package.json +++ b/packages/github/package.json @@ -40,6 +40,11 @@ "types": "./dist/inbound.d.ts", "import": "./dist/inbound.js", "default": "./dist/inbound.js" + }, + "./webhook-identity": { + "types": "./dist/webhook-identity.d.ts", + "import": "./dist/webhook-identity.js", + "default": "./dist/webhook-identity.js" } }, "files": [ diff --git a/packages/github/src/webhook-identity.test.ts b/packages/github/src/webhook-identity.test.ts new file mode 100644 index 00000000..7356aa9f --- /dev/null +++ b/packages/github/src/webhook-identity.test.ts @@ -0,0 +1,100 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; + +import { githubCheckRunPullRequestNumber } from '@relayfile/adapter-github/webhook-identity'; + +const repository = { owner: 'AgentWorkforce', repo: 'cloud' }; + +describe('githubCheckRunPullRequestNumber', () => { + it('accepts a positive safe number when the entry has no URL', () => { + assert.equal(githubCheckRunPullRequestNumber({ number: 42 }, repository), 42); + }); + + it('parses GitHub API and HTML pull-request URLs case-insensitively', () => { + assert.equal( + githubCheckRunPullRequestNumber( + { url: 'https://api.github.com/repos/agentworkforce/CLOUD/pulls/91' }, + repository, + ), + 91, + ); + assert.equal( + githubCheckRunPullRequestNumber( + { html_url: 'https://github.com/AgentWorkforce/cloud/pull/92' }, + repository, + ), + 92, + ); + }); + + it('treats a present URL as authoritative over the numeric field', () => { + assert.equal( + githubCheckRunPullRequestNumber( + { + number: 7, + url: 'https://api.github.com/repos/AgentWorkforce/cloud/pulls/93', + }, + repository, + ), + 93, + ); + for (const entry of [ + { number: 7, url: '' }, + { number: 7, html_url: '' }, + { + number: 7, + url: '', + html_url: 'https://github.com/AgentWorkforce/cloud/pull/93', + }, + { + number: 7, + url: null, + html_url: 'https://github.com/AgentWorkforce/cloud/pull/93', + }, + ]) { + assert.equal(githubCheckRunPullRequestNumber(entry, repository), null); + } + }); + + it('fails closed for foreign repositories and unrecognized hosts', () => { + assert.equal( + githubCheckRunPullRequestNumber( + { + number: 93, + html_url: 'https://github.com/AgentWorkforce/other/pull/93', + }, + repository, + ), + null, + ); + assert.equal( + githubCheckRunPullRequestNumber( + { url: 'https://example.com/AgentWorkforce/cloud/pull/93' }, + repository, + ), + null, + ); + }); + + it('rejects malformed, non-HTTPS, non-canonical, and unsafe identities', () => { + for (const entry of [ + null, + [], + { number: 0 }, + { number: -1 }, + { number: Number.MAX_SAFE_INTEGER + 1 }, + { url: 'not-a-url' }, + { url: 'http://api.github.com/repos/AgentWorkforce/cloud/pulls/1' }, + { url: 'https://api.github.com:444/repos/AgentWorkforce/cloud/pulls/1' }, + { url: 'https://user@api.github.com/repos/AgentWorkforce/cloud/pulls/1' }, + { url: 'https://api.github.com/repos/AgentWorkforce/cloud/issues/1' }, + { url: 'https://api.github.com/repos/AgentWorkforce/cloud/pull/1' }, + { html_url: 'https://github.com/AgentWorkforce/cloud/pulls/1' }, + { url: 'https://api.github.com/repos/AgentWorkforce/cloud/pulls/1/files' }, + { url: 'https://github.com/AgentWorkforce/cloud/pull/0' }, + { url: 'https://github.com/AgentWorkforce/cloud/pull/9007199254740992' }, + ]) { + assert.equal(githubCheckRunPullRequestNumber(entry, repository), null); + } + }); +}); diff --git a/packages/github/src/webhook-identity.ts b/packages/github/src/webhook-identity.ts new file mode 100644 index 00000000..9b4581b3 --- /dev/null +++ b/packages/github/src/webhook-identity.ts @@ -0,0 +1,78 @@ +export type GitHubRepositoryIdentity = { + owner: string; + repo: string; +}; + +type CheckRunPullRequestEntry = { + number?: unknown; + url?: unknown; + html_url?: unknown; +}; + +function positiveSafeInteger(value: unknown): number | null { + return typeof value === 'number' && Number.isSafeInteger(value) && value > 0 + ? value + : null; +} + +function asCheckRunPullRequestEntry(value: unknown): CheckRunPullRequestEntry | null { + return value !== null && typeof value === 'object' && !Array.isArray(value) + ? (value as CheckRunPullRequestEntry) + : null; +} + +/** + * Resolve a pull-request number from one `check_run.pull_requests[]` entry. + * + * GitHub normally supplies both `number` and an API URL, but webhook fixtures + * and older delivery shapes may contain only the API or HTML URL. When a URL + * is present, it is authoritative and must identify the expected repository; + * malformed, foreign-repository, and non-GitHub URLs fail closed. + */ +export function githubCheckRunPullRequestNumber( + value: unknown, + expectedRepository: GitHubRepositoryIdentity, +): number | null { + const entry = asCheckRunPullRequestEntry(value); + if (!entry) return null; + + const hasApiUrl = Object.prototype.hasOwnProperty.call(entry, 'url'); + const hasHtmlUrl = Object.prototype.hasOwnProperty.call(entry, 'html_url'); + if (!hasApiUrl && !hasHtmlUrl) return positiveSafeInteger(entry.number); + + // A supplied URL field is authoritative, including when its value is empty + // or malformed. Prefer the API URL exactly as GitHub's payload does. + const urlValue = hasApiUrl ? entry.url : entry.html_url; + if (typeof urlValue !== 'string' || urlValue.length === 0) return null; + + let parsedUrl: URL; + try { + parsedUrl = new URL(urlValue); + } catch { + return null; + } + + const pathParts = parsedUrl.pathname.split('/').filter(Boolean); + const isApiPath = pathParts[0]?.toLowerCase() === 'repos'; + const ownerIndex = isApiPath ? 1 : 0; + const repoIndex = isApiPath ? 2 : 1; + const kindIndex = isApiPath ? 3 : 2; + const numberIndex = isApiPath ? 4 : 3; + const expectedOrigin = isApiPath ? 'https://api.github.com' : 'https://github.com'; + const pullNumberSegment = pathParts[numberIndex]; + + if ( + parsedUrl.origin.toLowerCase() !== expectedOrigin || + parsedUrl.username !== '' || + parsedUrl.password !== '' || + pathParts.length !== numberIndex + 1 || + pathParts[kindIndex]?.toLowerCase() !== (isApiPath ? 'pulls' : 'pull') || + pathParts[ownerIndex]?.toLowerCase() !== expectedRepository.owner.toLowerCase() || + pathParts[repoIndex]?.toLowerCase() !== expectedRepository.repo.toLowerCase() || + !/^\d+$/.test(pullNumberSegment ?? '') + ) { + return null; + } + + return positiveSafeInteger(Number(pullNumberSegment)); +} diff --git a/packages/github/tsconfig.json b/packages/github/tsconfig.json index 5b8ad354..6bf454ef 100644 --- a/packages/github/tsconfig.json +++ b/packages/github/tsconfig.json @@ -21,6 +21,7 @@ "src/types.ts", "src/resources.ts", "src/writeback.ts", + "src/webhook-identity.ts", "src/webhook/event-map.ts" ], "exclude": [