Copilot commented on code in PR #40625:
URL: https://github.com/apache/superset/pull/40625#discussion_r3337752147
##########
superset-websocket/src/config.ts:
##########
@@ -114,6 +116,7 @@ function applyEnvOverrides(config: ConfigType): ConfigType {
(config.redisStreamReadBlockMs = toNumber(val)),
JWT_SECRET: val => (config.jwtSecret = val),
JWT_COOKIE_NAME: val => (config.jwtCookieName = val),
+ ALLOWED_ORIGINS: val => (config.allowedOrigins = toStringArray(val)),
SOCKET_RESPONSE_TIMEOUT_MS: val =>
Review Comment:
`ALLOWED_ORIGINS` is documented as comma-separated, but `toStringArray()`
does not trim whitespace or drop empty entries. A common value like
`"https://a.example, https://b.example"` will include a leading space and fail
origin matching unexpectedly. Consider trimming and filtering when parsing this
env var.
##########
superset-websocket/src/index.ts:
##########
@@ -378,6 +378,31 @@ export const httpRequest = (
}
};
+/**
+ * Validates the `Origin` header of a WebSocket upgrade request against the
+ * configured `allowedOrigins` list, mitigating Cross-Site WebSocket Hijacking.
+ *
+ * When `allowedOrigins` is empty the check is skipped (preserving existing
+ * behavior); a single `'*'` entry explicitly allows any origin. Otherwise the
+ * request's `Origin` must exactly match one of the configured origins.
+ */
+export const isOriginAllowed = (request: http.IncomingMessage): boolean => {
+ const { allowedOrigins } = opts;
+
+ if (!allowedOrigins || allowedOrigins.length === 0) {
+ return true;
+ }
+ if (allowedOrigins.includes('*')) {
+ return true;
+ }
+
+ const origin = request.headers.origin;
+ if (!origin) {
+ return false;
+ }
+ return allowedOrigins.includes(origin);
Review Comment:
`request.headers.origin` is typed as `string | string[] | undefined` in
Node. Passing it directly to `allowedOrigins.includes(...)` causes a TypeScript
type error and can behave incorrectly if multiple Origin headers are present.
Normalize the header to a single string (or reject non-string values) before
comparing.
--
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]
