Skip to content

Commit e65d50d

Browse files
alexr00Copilot
andauthored
Add check for PR and issue numbers that can't possibly exist (#8704)
* Add check for PR and issue numbers that can't possibly exist Co-authored-by: Copilot <copilot@github.com> * CCR feedback Co-authored-by: Copilot <copilot@github.com> --------- Co-authored-by: Copilot <copilot@github.com>
1 parent 2f6fda9 commit e65d50d

1 file changed

Lines changed: 70 additions & 0 deletions

File tree

src/github/githubRepository.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,11 @@ 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 = 60 * 60 * 1000; /* 1 hour */
263+
private static readonly MAX_ITEM_NUMBER_BUFFER = 50;
264+
private _maxItemNumberCache: { value: number; fetchedAt: number } | undefined;
265+
private _maxItemNumberPromise: Promise<number | undefined> | undefined;
261266
get areQueriesLimited(): boolean { return this._areQueriesLimited; }
262267

263268
private _branchesCache: Map<string, string[]> = new Map();
@@ -1012,6 +1017,62 @@ export class GitHubRepository extends Disposable {
10121017
return this._getMaxItem(false);
10131018
}
10141019

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

1341+
if (!(await this.isPlausibleItemNumber(id))) {
1342+
Logger.debug(`Skipping pull request fetch for implausible number ${id} (caller: ${callerName})`, this.id);
1343+
return;
1344+
}
1345+
12801346
try {
12811347
const { query, remote, schema } = await this.ensure();
12821348
Logger.debug(`Fetch pull request ${remote.owner}/${remote.repositoryName} ${id} - enter`, this.id);
@@ -1341,6 +1407,10 @@ export class GitHubRepository extends Disposable {
13411407
return cached;
13421408
}
13431409
}
1410+
if (!(await this.isPlausibleItemNumber(id))) {
1411+
Logger.debug(`Skipping issue fetch for implausible number ${id}`, this.id);
1412+
return undefined;
1413+
}
13441414
try {
13451415
Logger.debug(`Fetch issue ${id} - enter`, this.id);
13461416
const { query, remote, schema } = await this.ensure();

0 commit comments

Comments
 (0)