Skip to content

Commit 9cc1ab0

Browse files
goda6565goda6565
authored
fix(http/github): support cursor pagination (#45241)
* fix(http/github): support cursor pagination * fix(http/github): harden cursor pagination * fix(http/github): log pagination mode
1 parent 9d0e535 commit 9cc1ab0

3 files changed

Lines changed: 280 additions & 27 deletions

File tree

‎lib/modules/platform/github/index.spec.ts‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5647,6 +5647,103 @@ describe('modules/platform/github/index', () => {
56475647
expect(res[1].security_vulnerability!.package.name).toBe('right-pad');
56485648
expect(res[2].security_vulnerability!.package.name).toBe('center-pad');
56495649
});
5650+
5651+
it('handles cursor pagination correctly', async () => {
5652+
const scope = httpMock.scope(githubApiHost);
5653+
initRepoMock(scope, 'some/repo');
5654+
5655+
scope
5656+
.get(
5657+
'/repos/some/repo/dependabot/alerts?state=open&direction=asc&per_page=100',
5658+
)
5659+
.reply(
5660+
200,
5661+
[
5662+
{
5663+
security_advisory: {
5664+
ghsa_id: 'GHSA-1234-5678-9012',
5665+
summary: 'summary',
5666+
description: 'description',
5667+
identifiers: [{ type: 'type', value: 'value' }],
5668+
references: [],
5669+
severity: 'high',
5670+
},
5671+
security_vulnerability: {
5672+
package: {
5673+
ecosystem: 'npm',
5674+
name: 'left-pad',
5675+
},
5676+
severity: 'high',
5677+
vulnerable_version_range: '0.0.2',
5678+
first_patched_version: { identifier: '0.0.3' },
5679+
},
5680+
dependency: {
5681+
manifest_path: 'bar/foo',
5682+
},
5683+
},
5684+
{
5685+
security_advisory: {
5686+
ghsa_id: 'GHSA-1234-5678-9012',
5687+
summary: 'summary',
5688+
description: 'description',
5689+
identifiers: [{ type: 'type', value: 'value' }],
5690+
references: [],
5691+
severity: 'critical',
5692+
},
5693+
security_vulnerability: {
5694+
package: {
5695+
ecosystem: 'npm',
5696+
name: 'right-pad',
5697+
},
5698+
severity: 'critical',
5699+
vulnerable_version_range: '0.0.1',
5700+
first_patched_version: { identifier: '0.0.2' },
5701+
},
5702+
dependency: {
5703+
manifest_path: 'bar/foo',
5704+
},
5705+
},
5706+
],
5707+
{
5708+
link: `<${githubApiHost}/repos/some/repo/dependabot/alerts?state=open&direction=asc&per_page=100&after=cursor-1>; rel="next"`,
5709+
},
5710+
)
5711+
.get(
5712+
'/repos/some/repo/dependabot/alerts?state=open&direction=asc&per_page=100&after=cursor-1',
5713+
)
5714+
.reply(200, [
5715+
{
5716+
security_advisory: {
5717+
ghsa_id: 'GHSA-1234-5678-9012',
5718+
summary: 'summary',
5719+
description: 'description',
5720+
identifiers: [{ type: 'type', value: 'value' }],
5721+
references: [],
5722+
severity: 'low',
5723+
},
5724+
security_vulnerability: {
5725+
package: {
5726+
ecosystem: 'npm',
5727+
name: 'center-pad',
5728+
},
5729+
severity: 'low',
5730+
vulnerable_version_range: '0.0.3',
5731+
first_patched_version: { identifier: '0.0.4' },
5732+
},
5733+
dependency: {
5734+
manifest_path: 'bar/foo',
5735+
},
5736+
},
5737+
]);
5738+
5739+
await github.initRepo({ repository: 'some/repo' });
5740+
const res = await github.getVulnerabilityAlerts();
5741+
5742+
expect(res).toHaveLength(3);
5743+
expect(res[0].security_vulnerability!.package.name).toBe('left-pad');
5744+
expect(res[1].security_vulnerability!.package.name).toBe('right-pad');
5745+
expect(res[2].security_vulnerability!.package.name).toBe('center-pad');
5746+
});
56505747
});
56515748

56525749
describe('getJsonFile()', () => {

‎lib/util/http/github.spec.ts‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,104 @@ describe('util/http/github', () => {
113113
expect(res.body).toEqual(['a', 'b', 'c', 'd', 'e']);
114114
});
115115

116+
it('paginates cursor links', async () => {
117+
const url = '/some-url?per_page=2';
118+
httpMock
119+
.scope(githubApiHost)
120+
.get(url)
121+
.reply(200, ['a', 'b'], {
122+
link: `<${url}&after=cursor-1>; rel="next"`,
123+
})
124+
.get(`${url}&after=cursor-1`)
125+
.reply(200, ['c', 'd'], {
126+
link: `<${url}&after=cursor-2>; rel="next", <${url}&before=cursor-1>; rel="prev"`,
127+
})
128+
.get(`${url}&after=cursor-2`)
129+
.reply(200, ['e']);
130+
const res = await githubApi.getJsonUnchecked(url, { paginate: true });
131+
expect(res.body).toEqual(['a', 'b', 'c', 'd', 'e']);
132+
});
133+
134+
it('limits cursor pagination', async () => {
135+
const url = '/some-url?per_page=2';
136+
httpMock
137+
.scope(githubApiHost)
138+
.get(url)
139+
.reply(200, ['a', 'b'], {
140+
link: `<${url}&after=cursor-1>; rel="next"`,
141+
})
142+
.get(`${url}&after=cursor-1`)
143+
.reply(200, ['c', 'd'], {
144+
link: `<${url}&after=cursor-2>; rel="next"`,
145+
});
146+
const res = await githubApi.getJsonUnchecked(url, {
147+
paginate: true,
148+
pageLimit: 2,
149+
});
150+
expect(res.body).toEqual(['a', 'b', 'c', 'd']);
151+
});
152+
153+
it('paginates all cursor links', async () => {
154+
const url = '/some-url?per_page=2';
155+
httpMock
156+
.scope(githubApiHost)
157+
.get(url)
158+
.reply(200, ['a', 'b'], {
159+
link: `<${url}&after=cursor-1>; rel="next"`,
160+
})
161+
.get(`${url}&after=cursor-1`)
162+
.reply(200, ['c', 'd'], {
163+
link: `<${url}&after=cursor-2>; rel="next"`,
164+
})
165+
.get(`${url}&after=cursor-2`)
166+
.reply(200, ['e']);
167+
const res = await githubApi.getJsonUnchecked(url, {
168+
paginate: 'all',
169+
pageLimit: 2,
170+
});
171+
expect(res.body).toEqual(['a', 'b', 'c', 'd', 'e']);
172+
});
173+
174+
it('limits full cursor pagination', async () => {
175+
const url = '/some-url?per_page=1';
176+
httpMock
177+
.scope(githubApiHost)
178+
.get(url)
179+
.times(100)
180+
.reply(200, ['a'], {
181+
link: `<${url}>; rel="next"`,
182+
});
183+
const res = await githubApi.getJsonUnchecked(url, { paginate: 'all' });
184+
expect(res.body).toHaveLength(100);
185+
expect(logger.logger.warn).toHaveBeenCalledWith(
186+
{ maxPages: 100 },
187+
'GitHub cursor pagination limit reached',
188+
);
189+
});
190+
191+
it('does not follow cursor pagination links to a different origin', async () => {
192+
const url = '/some-url?per_page=2';
193+
httpMock
194+
.scope(githubApiHost)
195+
.get(url)
196+
.reply(200, ['a', 'b'], {
197+
link: `<${url}&after=cursor-1>; rel="next"`,
198+
})
199+
.get(`${url}&after=cursor-1`)
200+
.reply(200, ['c', 'd'], {
201+
link: '<https://attacker.example.com/some-url?after=cursor-2>; rel="next"',
202+
});
203+
const res = await githubApi.getJsonUnchecked(url, { paginate: true });
204+
expect(res.body).toEqual(['a', 'b', 'c', 'd']);
205+
expect(logger.logger.once.warn).toHaveBeenCalledWith(
206+
{
207+
requestHost: 'api.github.com',
208+
paginationHost: 'attacker.example.com',
209+
},
210+
'Ignoring cross-origin GitHub pagination link. Set RENOVATE_X_REBASE_PAGINATION_LINKS if this is a self-hosted instance that returns a different host in pagination links.',
211+
);
212+
});
213+
116214
it('uses paginationField', async () => {
117215
const url = '/some-url';
118216
httpMock

‎lib/util/http/github.ts‎

Lines changed: 85 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import type {
4040
} from './types.ts';
4141

4242
const githubBaseUrl = 'https://api.github.com/';
43+
const MAX_PAGINATION_PAGES = 100;
4344
let baseUrl = githubBaseUrl;
4445
export function setBaseUrl(url: string): void {
4546
baseUrl = url;
@@ -333,6 +334,20 @@ function replaceUrlBase(url: URL, baseUrl: string): URL {
333334
return new URL(relativeUrl, baseUrl);
334335
}
335336

337+
function resolvePaginationUrl(
338+
url: string,
339+
baseUrl: string | undefined,
340+
rebasePaginationLinks: boolean,
341+
): URL {
342+
const parsedUrl = new URL(url, baseUrl);
343+
const rebasePagination =
344+
!!baseUrl &&
345+
rebasePaginationLinks &&
346+
// Preserve github.com URLs for use cases like release notes
347+
parsedUrl.origin !== 'https://api.github.com';
348+
return rebasePagination ? replaceUrlBase(parsedUrl, baseUrl) : parsedUrl;
349+
}
350+
336351
export class GithubHttp extends HttpBase<GithubHttpOptions> {
337352
protected override get baseUrl(): string | undefined {
338353
return baseUrl;
@@ -423,36 +438,79 @@ export class GithubHttp extends HttpBase<GithubHttpOptions> {
423438
const linkHeader = parseLinkHeader(result?.headers?.link);
424439
const next = linkHeader?.next;
425440
const env = getEnv();
426-
if (next?.url && linkHeader?.last?.page) {
427-
let lastPage = parseInt(linkHeader.last.page, 10);
428-
// v8 ignore else -- TODO: add test #40625
429-
if (!env.RENOVATE_PAGINATE_ALL && httpOptions.paginate !== 'all') {
430-
lastPage = Math.min(pageLimit, lastPage);
431-
}
441+
if (next?.url) {
432442
const baseUrl = httpOptions.baseUrl ?? this.baseUrl;
433-
const parsedUrl = new URL(next.url, baseUrl);
434-
const rebasePagination =
435-
!!baseUrl &&
436-
!!env.RENOVATE_X_REBASE_PAGINATION_LINKS &&
437-
// Preserve github.com URLs for use cases like release notes
438-
parsedUrl.origin !== 'https://api.github.com';
439-
const firstPageUrl = rebasePagination
440-
? replaceUrlBase(parsedUrl, baseUrl)
441-
: parsedUrl;
443+
const rebasePaginationLinks = !!env.RENOVATE_X_REBASE_PAGINATION_LINKS;
444+
const firstPageUrl = resolvePaginationUrl(
445+
next.url,
446+
baseUrl,
447+
rebasePaginationLinks,
448+
);
442449
// Don't follow a cross-origin request, unless we've been explicitly requested to do so with `RENOVATE_X_REBASE_PAGINATION_LINKS`
443450
if (firstPageUrl.origin === resolvedUrl.origin) {
444-
const queue = [...range(2, lastPage)].map(
445-
(pageNumber) => (): Promise<HttpResponse<T>> => {
446-
// copy before modifying searchParams
447-
const nextUrl = parseUrl(firstPageUrl.toString())!;
448-
nextUrl.searchParams.set('page', String(pageNumber));
449-
return super.requestJsonUnsafe<T>(method, {
450-
...opts,
451-
url: nextUrl,
452-
});
453-
},
454-
);
455-
const pages = await p.all(queue);
451+
let pages: HttpResponse<T>[];
452+
if (linkHeader?.last?.page) {
453+
logger.debug('Using GitHub offset-based pagination');
454+
let lastPage = parseInt(linkHeader.last.page, 10);
455+
// v8 ignore else -- TODO: add test #40625
456+
if (!env.RENOVATE_PAGINATE_ALL && httpOptions.paginate !== 'all') {
457+
lastPage = Math.min(pageLimit, lastPage);
458+
}
459+
const queue = [...range(2, lastPage)].map(
460+
(pageNumber) => (): Promise<HttpResponse<T>> => {
461+
// copy before modifying searchParams
462+
const nextUrl = parseUrl(firstPageUrl.toString())!;
463+
nextUrl.searchParams.set('page', String(pageNumber));
464+
return super.requestJsonUnsafe<T>(method, {
465+
...opts,
466+
url: nextUrl,
467+
});
468+
},
469+
);
470+
pages = await p.all(queue);
471+
} else {
472+
logger.debug('Using GitHub cursor-based pagination');
473+
pages = [];
474+
const paginateAll =
475+
!!env.RENOVATE_PAGINATE_ALL || httpOptions.paginate === 'all';
476+
const cursorPageLimit = paginateAll
477+
? MAX_PAGINATION_PAGES
478+
: pageLimit;
479+
let nextUrl: URL | null = firstPageUrl;
480+
let pageNumber = 2;
481+
for (; nextUrl && pageNumber <= cursorPageLimit; pageNumber += 1) {
482+
if (nextUrl.origin !== resolvedUrl.origin) {
483+
logger.once.warn(
484+
{
485+
requestHost: resolvedUrl.host,
486+
paginationHost: nextUrl.host,
487+
},
488+
'Ignoring cross-origin GitHub pagination link. Set RENOVATE_X_REBASE_PAGINATION_LINKS if this is a self-hosted instance that returns a different host in pagination links.',
489+
);
490+
break;
491+
}
492+
const nextPage: HttpResponse<T> =
493+
await super.requestJsonUnsafe<T>(method, {
494+
...opts,
495+
url: nextUrl,
496+
});
497+
pages.push(nextPage);
498+
const nextLink = parseLinkHeader(nextPage.headers.link)?.next;
499+
nextUrl = nextLink?.url
500+
? resolvePaginationUrl(
501+
nextLink.url,
502+
baseUrl,
503+
rebasePaginationLinks,
504+
)
505+
: null;
506+
}
507+
if (paginateAll && nextUrl && pageNumber > cursorPageLimit) {
508+
logger.warn(
509+
{ maxPages: MAX_PAGINATION_PAGES },
510+
'GitHub cursor pagination limit reached',
511+
);
512+
}
513+
}
456514
// v8 ignore else -- TODO: add test #40625
457515
if (httpOptions.paginationField && isPlainObject(result.body)) {
458516
const paginatedResult = result.body[httpOptions.paginationField];

0 commit comments

Comments
 (0)