rusackas opened a new pull request, #41163:
URL: https://github.com/apache/superset/pull/41163
### SUMMARY
Resolves code-scanning alert #2534 (`js/unvalidated-dynamic-method-call`,
HIGH) at `superset-frontend/src/middleware/asyncEvent.ts:148`.
**The alert:** In `processEvents()` a listener is resolved by `job_id`
(which arrives in server polling responses and WebSocket message payloads) and
then invoked. CodeQL flags the invocation as an unvalidated dynamic method call
because the data-flow it tracks runs `job_id` (untrusted) -> key lookup -> a
value that is then called as a function.
**Risk assessment — effectively a false positive, but worth a real guard:**
The lookup is done through a `Map` (`listenersByJobId.get(jobId)`), not
plain-object property access, so a malicious `job_id` such as `__proto__`,
`constructor`, or `hasOwnProperty` cannot return an inherited prototype method
— `Map.get` only ever returns values explicitly stored via `addListener`.
Prototype pollution / arbitrary-method invocation is therefore structurally
impossible here, and there is already a committed regression test exercising
exactly those reserved keys. The metadata/results backend that produces these
events also sits inside the operator trust boundary per `SECURITY.md`. CodeQL
cannot model `Map` semantics, so it heuristically flags the call.
**The fix:** Rather than suppress, this replaces the truthy `if (listener)`
check with an explicit `typeof listener === 'function'` guard before the
dynamic dispatch. This confirms the retrieved value really is a registered
listener function before calling it, which is both defensible hardening and
satisfies the validate-before-invoke contract the scanner expects. A short
comment documents why the `Map`-based lookup is safe.
```diff
- if (listener) {
+ // `jobId` originates from server/WebSocket payloads, so the listener is
+ // resolved exclusively through a Map (never plain-object property
access,
+ // which would expose the prototype chain), and we confirm the retrieved
+ // value is a registered function before dispatching the event to it.
+ if (typeof listener === 'function') {
```
### TESTING INSTRUCTIONS
Covered by the existing suite at
`superset-frontend/src/middleware/asyncEvent.test.ts`, which includes a
regression test that drives reserved `job_id` values (`__proto__`,
`constructor`, `prototype`, `hasOwnProperty`) through the registries and
asserts the listener still resolves. `tsc --noEmit`, `oxlint`, and `prettier`
were run against the file with no new findings. The `typeof` guard is
behaviorally identical for every legitimate (function) listener.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
