Skip to content

Commit e20adf9

Browse files
alexr00Copilot
andcommitted
Add check for PR and issue numbers that can't possibly exist
Co-authored-by: Copilot <copilot@github.com>
1 parent 2f6fda9 commit e20adf9

1 file changed

Lines changed: 69 additions & 0 deletions

File tree

src/github/githubRepository.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,10 @@ export class GitHubRepository extends Disposable {
258258
// eslint-disable-next-line rulesdir/no-any-except-union-method-signature
259259
private _queriesSchema: any;
260260
private _areQueriesLimited: boolean = false;
261+
262+
private static readonly MAX_ITEM_NUMBER_TTL_MS = 10 * 60 * 1000; /* 10 minutes */
263+
private _maxItemNumberCache: { value: number; fetchedAt: number } | undefined;
264+
private _maxItemNumberPromise: Promise<number | undefined> | undefined;
261265
get areQueriesLimited(): boolean { return this._areQueriesLimited; }
262266

263267
private _branchesCache: Map<string, string[]> = new Map();
@@ -1012,6 +1016,62 @@ export class GitHubRepository extends Disposable {
10121016
return this._getMaxItem(false);
10131017
}
10141018

1019+
/**
1020+
* Returns the highest known issue or pull request number for this repository.
1021+
* Issues and pull requests share the same number sequence on GitHub, so the
1022+
* larger of the two latest items is an upper bound for any valid number.
1023+
* Result is cached for a short TTL to avoid extra round-trips.
1024+
*/
1025+
private async getCachedMaxItemNumber(forceRefresh: boolean = false): Promise<number | undefined> {
1026+
const now = Date.now();
1027+
if (!forceRefresh && this._maxItemNumberCache && (now - this._maxItemNumberCache.fetchedAt) < GitHubRepository.MAX_ITEM_NUMBER_TTL_MS) {
1028+
return this._maxItemNumberCache.value;
1029+
}
1030+
if (this._maxItemNumberPromise) {
1031+
return this._maxItemNumberPromise;
1032+
}
1033+
this._maxItemNumberPromise = (async () => {
1034+
const [maxIssue, maxPr] = await Promise.all([this._getMaxItem(true), this._getMaxItem(false)]);
1035+
const max = Math.max(maxIssue ?? 0, maxPr ?? 0);
1036+
if (max > 0) {
1037+
this._maxItemNumberCache = { value: max, fetchedAt: Date.now() };
1038+
return max;
1039+
}
1040+
return undefined;
1041+
})();
1042+
try {
1043+
return await this._maxItemNumberPromise;
1044+
} finally {
1045+
this._maxItemNumberPromise = undefined;
1046+
}
1047+
}
1048+
1049+
/**
1050+
* Returns false when `number` is implausibly higher than the latest known
1051+
* issue/PR number for this repository. Used to short-circuit fetches for
1052+
* numbers that came from arbitrary text (e.g. `#1234567890` in source code)
1053+
* before they hit the network and produce noisy error telemetry.
1054+
*/
1055+
private async isPlausibleItemNumber(number: number): Promise<boolean> {
1056+
if (!Number.isFinite(number) || number <= 0) {
1057+
return false;
1058+
}
1059+
let max = await this.getCachedMaxItemNumber();
1060+
if (max === undefined) {
1061+
// Couldn't determine the max (e.g. network error); allow through.
1062+
return true;
1063+
}
1064+
if (number <= max) {
1065+
return true;
1066+
}
1067+
// Number is above the cached max; refresh once before deciding to handle newly-created items.
1068+
max = await this.getCachedMaxItemNumber(true);
1069+
if (max === undefined) {
1070+
return true;
1071+
}
1072+
return number <= max;
1073+
}
1074+
10151075
async getViewerPermission(): Promise<ViewerPermission> {
10161076
try {
10171077
Logger.debug(`Fetch viewer permission - enter`, this.id);
@@ -1277,6 +1337,11 @@ export class GitHubRepository extends Disposable {
12771337
return this._pullRequestModelsByNumber.get(id)!.model;
12781338
}
12791339

1340+
if (!(await this.isPlausibleItemNumber(id))) {
1341+
Logger.debug(`Skipping pull request fetch for implausible number ${id} (caller: ${callerName})`, this.id);
1342+
return;
1343+
}
1344+
12801345
try {
12811346
const { query, remote, schema } = await this.ensure();
12821347
Logger.debug(`Fetch pull request ${remote.owner}/${remote.repositoryName} ${id} - enter`, this.id);
@@ -1341,6 +1406,10 @@ export class GitHubRepository extends Disposable {
13411406
return cached;
13421407
}
13431408
}
1409+
if (!(await this.isPlausibleItemNumber(id))) {
1410+
Logger.debug(`Skipping issue fetch for implausible number ${id}`, this.id);
1411+
return undefined;
1412+
}
13441413
try {
13451414
Logger.debug(`Fetch issue ${id} - enter`, this.id);
13461415
const { query, remote, schema } = await this.ensure();

0 commit comments

Comments
 (0)