Fix race condition in content exclusion that causes intermittent bypass

Concurrent isIgnored() calls could cache stale 'not ignored' results
when a content exclusion rule fetch was in progress. This happened
because:

1. After yielding at getRepositoryFetchUrls, a concurrent caller could
   have already started a fetch and seeded the cache with empty patterns.
   The current caller would skip the fetch (patterns already seeded,
   lastRuleFetch just set) and match against empty rules.

2. The glob result cache was only cleared at the start of the fetch, so
   any 'false' entries written during the fetch window persisted until
   the next 30-minute refresh.

Fix:
- Re-check _contentExclusionFetchPromise after the getRepositoryFetchUrls
  yield point and wait for any in-progress fetch before pattern matching.
- Clear both glob and regex result caches at the end of the fetch to
  invalidate stale entries written by concurrent callers.

Fixes github/Copilot-Controls#650

Author: ulugbekna

extensions/copilot/src/platform/ignore/node/remoteContentExclusion.ts

@@ -124,6 +124,11 @@ export class RemoteContentExclusion implements IDisposable {
 			this._logService.trace(`Fetching content exclusions, due to ${this.shouldFetchContentExclusionRules(repoMetadata) ? 'repository change' : 'stale cache'}.`);
 			this._lastRuleFetch = Date.now();
 			await raceCancellationError(this.makeContentExclusionRequest(), token);
+		} else if (this._contentExclusionFetchPromise) {
+			// A concurrent caller may have started a fetch while we were awaiting
+			// getRepositoryFetchUrls above. Wait for it to complete so we match
+			// against the actual rules instead of the empty seed entries.
+			await raceCancellationError(this._contentExclusionFetchPromise, token);
 		}
 
 		const minimatchConfig = {
@@ -259,7 +264,9 @@ export class RemoteContentExclusion implements IDisposable {
 	 * Not recommended to call directly and instead use {@link makeContentExclusionRequest} as that ensures only one call is pending at any time
 	 */
 	private async _contentExclusionRequest(): Promise<void> {
-		// Clear the result cache as new rules will come and therefore it is no longer valid
+		// Clear the result cache as new rules will come and therefore it is no longer valid.
+		// Note: we clear again at the end of this method to invalidate any stale entries
+		// written by concurrent isIgnored() calls that ran while the fetch was in flight.
 		this._ignoreGlobResultCache.clear();
 		const startTime = Date.now();
 		const capiClientService = this._capiClientService;
@@ -300,6 +307,10 @@ export class RemoteContentExclusion implements IDisposable {
 			await updateRulesForRepos(batch);
 		}
 		this._lastRuleFetch = Date.now();
+		// Clear result caches again to invalidate any stale entries written by concurrent
+		// isIgnored() calls that matched against the empty seed entries during the fetch.
+		this._ignoreGlobResultCache.clear();
+		this._ignoreRegexResultCache.clear();
 		this._logService.info(`Fetched content exclusion rules in ${Date.now() - startTime}ms`);
 
 		// Log the fetched rules to the request logger for debugging visibility

extensions/copilot/src/platform/ignore/node/test/remoteContentExclusion.spec.ts

@@ -5,6 +5,7 @@
 
 import { beforeEach, describe, expect, suite, test, vi } from 'vitest';
 import { CancellationToken } from '../../../../util/vs/base/common/cancellation';
+import { Barrier } from '../../../../util/vs/base/common/async';
 import { URI } from '../../../../util/vs/base/common/uri';
 import { IAuthenticationService } from '../../../authentication/common/authentication';
 import { ICAPIClientService } from '../../../endpoint/common/capiClient';
@@ -232,4 +233,176 @@ suite('RemoteContentExclusion', () => {
 			expect(mockGitService.getRepositoryFetchUrlsCallCount).toBe(0);
 		});
 	});
+
+	describe('concurrent isIgnored calls', () => {
+		test('concurrent call must not cache false while rules are being fetched', async () => {
+			// Reproduces the exact race condition from github/Copilot-Controls#650:
+			//
+			// Timeline without fix:
+			//   1. Call A and Call B both enter isIgnored(), both yield at getRepositoryFetchUrls
+			//   2. Call A resumes first: shouldFetchContentExclusionRules() seeds empty patterns
+			//      in _contentExclusionCache, sets _lastRuleFetch, starts CAPI fetch — yields
+			//   3. Call B resumes: shouldFetchContentExclusionRules() returns false (already seeded),
+			//      stale-time check passes (_lastRuleFetch just set), skips fetch entirely.
+			//      Matches against EMPTY patterns → caches false → returns false. BUG!
+			//   4. CAPI fetch completes with real rules, but Call B's stale false persists.
+			//
+			// With fix: Call B sees _contentExclusionFetchPromise is set and waits for it.
+
+			const repoRoot = '/workspace/my-repo';
+
+			// Per-call barriers so we can control exactly when each call's
+			// getRepositoryFetchUrls resolves.
+			const gitBarrierA = new Barrier();
+			const gitBarrierB = new Barrier();
+			let gitCallIndex = 0;
+			mockGitService.getRepositoryFetchUrls = vi.fn().mockImplementation(() => {
+				const barrier = gitCallIndex === 0 ? gitBarrierA : gitBarrierB;
+				gitCallIndex++;
+				return barrier.wait().then(() => ({
+					rootUri: URI.file(repoRoot),
+					remoteFetchUrls: ['https://github.com/org/repo.git']
+				}));
+			});
+
+			// CAPI fetch is gated by its own barrier so it doesn't resolve
+			// until we explicitly release it — after Call B has had a chance
+			// to reach the pattern matching code.
+			const capiBarrier = new Barrier();
+			mockCAPIClientService.setMockResponse({
+				ok: true,
+				json: () => capiBarrier.wait().then(() => [{
+					rules: [{ paths: ['**/keyword/**'], source: { name: 'org', type: 'Organization' } }],
+					last_updated_at: Date.now()
+				}]),
+			} as any);
+
+			const fileA = URI.file('/workspace/my-repo/keyword/keyword.py');
+			const fileB = URI.file('/workspace/my-repo/keyword/extra.py');
+
+			// Start both calls — both will block at getRepositoryFetchUrls
+			const resultA = remoteContentExclusion.isIgnored(fileA, CancellationToken.None);
+			const resultB = remoteContentExclusion.isIgnored(fileB, CancellationToken.None);
+
+			// Step 2: Let Call A resume first. It will call shouldFetchContentExclusionRules()
+			// (seeding empty patterns), then start the CAPI fetch which blocks on capiBarrier.
+			gitBarrierA.open();
+			// Flush microtasks so Call A progresses through shouldFetchContentExclusionRules
+			// and into makeContentExclusionRequest before Call B gets to run.
+			await flushMicrotasks();
+
+			// Step 3: Now let Call B resume. Without the fix, it would skip the
+			// fetch and match against empty patterns. With the fix, it sees the
+			// in-progress _contentExclusionFetchPromise and waits.
+			gitBarrierB.open();
+			await flushMicrotasks();
+
+			// Step 4: Release the CAPI response so real rules load.
+			capiBarrier.open();
+
+			// Both files are inside keyword/ and must be excluded.
+			expect(await resultA).toBe(true);
+			expect(await resultB).toBe(true);
+		});
+
+		test('post-fetch cache clear invalidates stale entries from concurrent callers', async () => {
+			// Tests the second part of the fix: clearing _ignoreGlobResultCache
+			// at the end of _contentExclusionRequest().
+			//
+			// If a concurrent call somehow wrote a false entry during the fetch,
+			// the post-fetch clear ensures the very next call re-evaluates against
+			// the real rules instead of returning the stale cached false.
+
+			const repoRoot = '/workspace/my-repo';
+
+			// Call A resolves immediately, Call B is gated by a barrier.
+			const gitBarrierB = new Barrier();
+			let gitCallIndex = 0;
+			mockGitService.getRepositoryFetchUrls = vi.fn().mockImplementation(() => {
+				const immediate = gitCallIndex === 0;
+				gitCallIndex++;
+				if (immediate) {
+					return Promise.resolve({
+						rootUri: URI.file(repoRoot),
+						remoteFetchUrls: ['https://github.com/org/repo.git']
+					});
+				}
+				return gitBarrierB.wait().then(() => ({
+					rootUri: URI.file(repoRoot),
+					remoteFetchUrls: ['https://github.com/org/repo.git']
+				}));
+			});
+
+			// CAPI responds with rules after a barrier
+			const capiBarrier = new Barrier();
+			let capiCallCount = 0;
+			mockCAPIClientService.setMockResponse({
+				ok: true,
+				json: () => {
+					capiCallCount++;
+					return capiBarrier.wait().then(() => [{
+						rules: [{ paths: ['**/keyword/**'], source: { name: 'org', type: 'Organization' } }],
+						last_updated_at: Date.now()
+					}]);
+				},
+			} as any);
+
+			const fileA = URI.file('/workspace/my-repo/keyword/keyword.py');
+			const fileB = URI.file('/workspace/my-repo/keyword/extra.py');
+			const fileC = URI.file('/workspace/my-repo/keyword/third.py');
+
+			// Call A starts — its git resolves immediately, triggers CAPI fetch, blocks on capiBarrier
+			const resultA = remoteContentExclusion.isIgnored(fileA, CancellationToken.None);
+			await flushMicrotasks();
+
+			// Call B starts — blocks at gitBarrierB
+			const resultB = remoteContentExclusion.isIgnored(fileB, CancellationToken.None);
+
+			// Release Call B's git barrier. It will now enter the shouldFetch/else-if
+			// path. With the fix, it waits on the CAPI fetch.
+			gitBarrierB.open();
+			await flushMicrotasks();
+
+			// Release CAPI — rules load, post-fetch cache clear runs
+			capiBarrier.open();
+
+			expect(await resultA).toBe(true);
+			expect(await resultB).toBe(true);
+
+			// A third sequential call should also correctly exclude (post-fetch
+			// cache clear wiped any stale entries, so this re-evaluates with real rules)
+			const resultC = await remoteContentExclusion.isIgnored(fileC, CancellationToken.None);
+			expect(resultC).toBe(true);
+		});
+
+		test('should correctly exclude files when rules arrive after concurrent calls start', async () => {
+			// Similar to above but with the non-git-file path (no repository)
+			mockGitService.setRepositoryFetchUrls(undefined);
+
+			// Use a barrier to control when the CAPI request resolves
+			const capiBarrier = new Barrier();
+			mockCAPIClientService.setMockResponse({
+				ok: true,
+				json: () => capiBarrier.wait().then(() => [{
+					rules: [{ paths: ['**/secret/**'], source: { name: 'org', type: 'Organization' } }],
+					last_updated_at: Date.now()
+				}]),
+			} as any);
+
+			const secretFile = URI.file('/project/secret/config.py');
+
+			// Start isIgnored — it will trigger a fetch that blocks on the capiBarrier
+			const resultPromise = remoteContentExclusion.isIgnored(secretFile, CancellationToken.None);
+
+			// Release CAPI response so rules load
+			capiBarrier.open();
+
+			expect(await resultPromise).toBe(true);
+		});
+	});
 });
+
+/** Flush pending microtasks by yielding to the event loop. */
+function flushMicrotasks(): Promise<void> {
+	return new Promise(resolve => setTimeout(resolve, 0));
+}