From e0cb0fedafaaa355969f58018704e89c084b6b52 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Wed, 3 Jun 2026 17:40:29 -0700 Subject: [PATCH 1/2] fix(asyncEvent): use Map for job listener/retry registries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL (js/unvalidated-dynamic-method-call, high) flags `listener(asyncEvent)` in processEvents because the listener is looked up from a plain object keyed by `asyncEvent.job_id` — a server-supplied value. With a plain object, a job_id that collides with an inherited member (`toString`, `constructor`, etc.) could resolve to an Object.prototype method and be invoked. The existing `hasOwnProperty` guard mitigated this, but the lookup target is still a prototype-bearing object. Switch `listenersByJobId` and `retriesByJobId` from plain objects to `Map`. `Map.get()` only ever returns explicitly-set values, never inherited prototype methods, which removes the dynamic-dispatch hazard entirely and is the canonical fix for this CodeQL rule. The `hasOwnProperty` guard is no longer needed, and `addListener`'s `any` parameter is tightened to `ListenerFn`. Behavior is unchanged. Co-Authored-By: Claude Opus 4.8 --- .../src/middleware/asyncEvent.ts | 37 ++++++++----------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/superset-frontend/src/middleware/asyncEvent.ts b/superset-frontend/src/middleware/asyncEvent.ts index 4fe60d2a4a96..74e8c055fce1 100644 --- a/superset-frontend/src/middleware/asyncEvent.ts +++ b/superset-frontend/src/middleware/asyncEvent.ts @@ -63,17 +63,17 @@ let config: AppConfig; let transport: string; let pollingDelayMs: number; let pollingTimeoutId: number; -let listenersByJobId: Record; -let retriesByJobId: Record; +let listenersByJobId: Map; +let retriesByJobId: Map; let lastReceivedEventId: string | null | undefined; -const addListener = (id: string, fn: any) => { - listenersByJobId[id] = fn; +const addListener = (id: string, fn: ListenerFn) => { + listenersByJobId.set(id, fn); }; const removeListener = (id: string) => { - if (!listenersByJobId[id]) return; - delete listenersByJobId[id]; + if (!listenersByJobId.has(id)) return; + listenersByJobId.delete(id); }; const fetchCachedData = async ( @@ -143,27 +143,22 @@ const setLastId = (asyncEvent: AsyncEvent) => { export const processEvents = async (events: AsyncEvent[]) => { events.forEach((asyncEvent: AsyncEvent) => { const jobId = asyncEvent.job_id; - const listener = Object.prototype.hasOwnProperty.call( - listenersByJobId, - jobId, - ) - ? listenersByJobId[jobId] - : undefined; + const listener = listenersByJobId.get(jobId); if (listener) { listener(asyncEvent); - delete retriesByJobId[jobId]; + retriesByJobId.delete(jobId); } else { // handle race condition where event is received // before listener is registered - if (!retriesByJobId[jobId]) retriesByJobId[jobId] = 0; - retriesByJobId[jobId] += 1; + const retries = (retriesByJobId.get(jobId) ?? 0) + 1; + retriesByJobId.set(jobId, retries); - if (retriesByJobId[jobId] <= MAX_RETRIES) { + if (retries <= MAX_RETRIES) { setTimeout(() => { processEvents([asyncEvent]); - }, RETRY_DELAY * retriesByJobId[jobId]); + }, RETRY_DELAY * retries); } else { - delete retriesByJobId[jobId]; + retriesByJobId.delete(jobId); logging.warn('listener not found for job_id', asyncEvent.job_id); } } @@ -173,7 +168,7 @@ export const processEvents = async (events: AsyncEvent[]) => { const loadEventsFromApi = async () => { const eventArgs = lastReceivedEventId ? { last_id: lastReceivedEventId } : {}; - if (Object.keys(listenersByJobId).length) { + if (listenersByJobId.size) { try { const { result: events } = await fetchEvents(eventArgs); if (events?.length) await processEvents(events); @@ -236,8 +231,8 @@ export const init = (appConfig?: AppConfig) => { if (!isFeatureEnabled(FeatureFlag.GlobalAsyncQueries)) return; if (pollingTimeoutId) clearTimeout(pollingTimeoutId); - listenersByJobId = {}; - retriesByJobId = {}; + listenersByJobId = new Map(); + retriesByJobId = new Map(); lastReceivedEventId = null; config = appConfig || getBootstrapData().common.conf; From ed9caa258b41931d0d1fa227f575faff0047157f Mon Sep 17 00:00:00 2001 From: Evan Date: Thu, 4 Jun 2026 11:23:58 -0700 Subject: [PATCH 2/2] test(asyncEvent): guard reserved job_id keys against prototype pollution Add a regression test covering the motivating CodeQL case where a job_id collides with a built-in Object property (e.g. __proto__, constructor). The Map-based listener/retry registries must route these through without prototype-bearing lookups or pollution. Co-Authored-By: Claude Opus 4.8 --- .../src/middleware/asyncEvent.test.ts | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/superset-frontend/src/middleware/asyncEvent.test.ts b/superset-frontend/src/middleware/asyncEvent.test.ts index 10da96099dd9..0ec0031514ec 100644 --- a/superset-frontend/src/middleware/asyncEvent.test.ts +++ b/superset-frontend/src/middleware/asyncEvent.test.ts @@ -172,6 +172,34 @@ describe('asyncEvent middleware', () => { expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(1); expect(fetchMock.callHistory.calls(CACHED_DATA_ENDPOINT)).toHaveLength(1); }); + + // Regression guard for the motivating CodeQL case: a job_id that collides + // with a built-in Object property (e.g. "__proto__"/"constructor") must be + // routed through the Map-based registries without triggering prototype + // pollution or losing the listener to a prototype-bearing lookup. + test.each(['__proto__', 'constructor', 'prototype', 'hasOwnProperty'])( + 'resolves listeners keyed by reserved job_id "%s"', + async jobId => { + fetchMock.clearHistory().removeRoutes(); + fetchMock.get(EVENTS_ENDPOINT, { + status: 200, + body: { result: [{ ...asyncDoneEvent, job_id: jobId }] }, + }); + fetchMock.get(CACHED_DATA_ENDPOINT, { + status: 200, + body: { result: chartData }, + }); + + const actualResolved = await asyncEvent.waitForAsyncData({ + ...asyncPendingEvent, + job_id: jobId, + }); + expect(actualResolved).toEqual([chartData]); + expect(fetchMock.callHistory.calls(CACHED_DATA_ENDPOINT)).toHaveLength( + 1, + ); + }, + ); }); // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks