[pull] master from apache:master

UPDATING.md

@@ -28,6 +28,12 @@ assists people when migrating to a new version.
 
 YDB SQL parsing now relies on the dedicated [`ydb-sqlglot-plugin`](https://pypi.org/project/ydb-sqlglot-plugin/) dialect, which registers itself with sqlglot automatically. YDB users must install this plugin (e.g., via `pip install "apache-superset[ydb]"`) to avoid a `ValueError` when Superset parses YDB queries.
 
+### Embedded dashboards enforce configured Allowed Domains for postMessage
+
+The embedded dashboard page now validates the origin of incoming `postMessage` events against the dashboard's configured **Allowed Domains**. The server-rendered embedded page exposes the configured domains in its bootstrap payload, and the frontend rejects message events whose origin is not in that list.
+
+Enforcement only applies when the Allowed Domains list is non-empty. If the list is empty (the default), any origin is accepted, so there is no behavior change for embeds that did not configure Allowed Domains.
+
 ### Dataset import validates catalog against the target connection
 
 Importing a dataset now validates the `catalog` field against the target database connection. When the connection has multi-catalog disabled (`allow_multi_catalog` off) and the dataset's catalog is not the connection's default catalog, the import fails instead of silently persisting the non-default catalog. This matches the validation already enforced on the dataset update path and prevents imported datasets from querying an unintended database.

superset-frontend/plugins/plugin-chart-ag-grid-table/src/stateConversion.ts

@@ -216,7 +216,13 @@ function convertFilterToSQL(
     if (conditions.length === 0) return null;
     if (conditions.length === 1) return conditions[0];
 
-    return `(${conditions.join(` ${filter.operator} `)})`;
+    // The join operator is interpolated into raw SQL, so only allow the two
+    // boolean connectors AG Grid produces.
+    const joinOperator = String(filter.operator).toUpperCase();
+    if (joinOperator !== 'AND' && joinOperator !== 'OR') {
+      return null;
+    }
+    return `(${conditions.join(` ${joinOperator} `)})`;
   }
 
   // Handle blank/notBlank operators for all filter types
@@ -263,20 +269,26 @@ function convertFilterToSQL(
     filter.filter !== undefined &&
     filter.type
   ) {
+    // Number values are interpolated into the clause without quoting, so they
+    // must be finite numbers. Coerce and skip the filter if it is not numeric.
+    const numericValue = Number(filter.filter);
+    if (!Number.isFinite(numericValue)) {
+      return null;
+    }
     // Map number filter types to SQL operators
     switch (filter.type) {
       case FILTER_OPERATORS.EQUALS:
-        return `${colId} ${SQL_OPERATORS.EQUALS} ${filter.filter}`;
+        return `${colId} ${SQL_OPERATORS.EQUALS} ${numericValue}`;
       case FILTER_OPERATORS.NOT_EQUAL:
-        return `${colId} ${SQL_OPERATORS.NOT_EQUALS} ${filter.filter}`;
+        return `${colId} ${SQL_OPERATORS.NOT_EQUALS} ${numericValue}`;
       case FILTER_OPERATORS.LESS_THAN:
-        return `${colId} ${SQL_OPERATORS.LESS_THAN} ${filter.filter}`;
+        return `${colId} ${SQL_OPERATORS.LESS_THAN} ${numericValue}`;
       case FILTER_OPERATORS.LESS_THAN_OR_EQUAL:
-        return `${colId} ${SQL_OPERATORS.LESS_THAN_OR_EQUAL} ${filter.filter}`;
+        return `${colId} ${SQL_OPERATORS.LESS_THAN_OR_EQUAL} ${numericValue}`;
       case FILTER_OPERATORS.GREATER_THAN:
-        return `${colId} ${SQL_OPERATORS.GREATER_THAN} ${filter.filter}`;
+        return `${colId} ${SQL_OPERATORS.GREATER_THAN} ${numericValue}`;
       case FILTER_OPERATORS.GREATER_THAN_OR_EQUAL:
-        return `${colId} ${SQL_OPERATORS.GREATER_THAN_OR_EQUAL} ${filter.filter}`;
+        return `${colId} ${SQL_OPERATORS.GREATER_THAN_OR_EQUAL} ${numericValue}`;
       default:
         return null;
     }
@@ -314,7 +326,9 @@ export function convertFilterModel(
     return undefined;
   }
 
-  const sqlClauses: Record<string, string> = {};
+  // Null-prototype object: column ids come from the (user-influenced) filter
+  // model and are used as keys here, so avoid prototype-chain keys.
+  const sqlClauses: Record<string, string> = Object.create(null);
 
   Object.entries(filterModel).forEach(([colId, filter]) => {
     const sqlClause = convertFilterToSQL(colId, filter);

superset-frontend/plugins/plugin-chart-ag-grid-table/test/stateConversion.test.ts

@@ -0,0 +1,73 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { convertFilterModel } from '../src/stateConversion';
+
+describe('convertFilterModel', () => {
+  test('emits a clause for a valid numeric comparison filter', () => {
+    const result = convertFilterModel({
+      revenue: { filterType: 'number', type: 'greaterThan', filter: 100 },
+    } as any);
+
+    expect(result?.sqlClauses?.revenue).toBe('revenue > 100');
+  });
+
+  test('drops a number filter whose value is not numeric', () => {
+    const result = convertFilterModel({
+      revenue: { filterType: 'number', type: 'equals', filter: '1 OR 1=1' },
+    } as any);
+
+    expect(result).toBeUndefined();
+  });
+
+  test('joins conditions with a valid boolean operator', () => {
+    const result = convertFilterModel({
+      revenue: {
+        filterType: 'number',
+        operator: 'AND',
+        condition1: { filterType: 'number', type: 'greaterThan', filter: 1 },
+        condition2: { filterType: 'number', type: 'lessThan', filter: 9 },
+      },
+    } as any);
+
+    expect(result?.sqlClauses?.revenue).toBe('(revenue > 1 AND revenue < 9)');
+  });
+
+  test('drops a compound filter whose join operator is not AND/OR', () => {
+    const result = convertFilterModel({
+      revenue: {
+        filterType: 'number',
+        operator: 'AND 1=1) OR (1=1',
+        condition1: { filterType: 'number', type: 'greaterThan', filter: 1 },
+        condition2: { filterType: 'number', type: 'lessThan', filter: 9 },
+      },
+    } as any);
+
+    expect(result).toBeUndefined();
+  });
+
+  test('stores clauses on a null-prototype map (prototype-safe column ids)', () => {
+    const result = convertFilterModel({
+      constructor: { filterType: 'number', type: 'equals', filter: 5 },
+    } as any);
+
+    // The map has no prototype, so a column id like "constructor" is just data.
+    expect(Object.getPrototypeOf(result?.sqlClauses)).toBeNull();
+    expect(result?.sqlClauses?.constructor).toBe('constructor = 5');
+  });
+});

superset-frontend/src/embedded/index.tsx

@@ -48,6 +48,7 @@ import {
 } from './EmbeddedContextProviders';
 import { embeddedApi } from './api';
 import { getDataMaskChangeTrigger } from './utils';
+import { validateMessageEvent } from './originValidation';
 
 setupPlugins();
 setupCodeOverrides({ embedded: true });
@@ -125,8 +126,6 @@ const EmbeddedApp = () => (
 
 const appMountPoint = document.getElementById('app')!;
 
-const MESSAGE_TYPE = '__embedded_comms__';
-
 function showFailureMessage(message: string) {
   appMountPoint.innerHTML = message;
 }
@@ -139,17 +138,6 @@ if (!window.parent || window.parent === window) {
   );
 }
 
-// if the page is embedded in an origin that hasn't
-// been authorized by the curator, we forbid access entirely.
-// todo: check the referrer on the route serving this page instead
-// const ALLOW_ORIGINS = ['http://127.0.0.1:9001', 'http://localhost:9001'];
-// const parentOrigin = new URL(document.referrer).origin;
-// if (!ALLOW_ORIGINS.includes(parentOrigin)) {
-//   throw new Error(
-//     `[superset] iframe parent ${parentOrigin} is not in the list of allowed origins`,
-//   );
-// }
-
 let displayedUnauthorizedToast = false;
 let root: Root | null = null;
 let started = false;
@@ -225,21 +213,9 @@ function setupGuestClient(guestToken: string) {
   });
 }
 
-function validateMessageEvent(event: MessageEvent) {
-  // if (!ALLOW_ORIGINS.includes(event.origin)) {
-  //   throw new Error('Message origin is not in the allowed list');
-  // }
-
-  if (typeof event.data !== 'object' || event.data.type !== MESSAGE_TYPE) {
-    throw new Error(`Message type does not match type used for embedded comms`);
-  }
-}
-
 window.addEventListener('message', function embeddedPageInitializer(event) {
-  try {
-    validateMessageEvent(event);
-  } catch (err) {
-    log('ignoring message unrelated to embedded comms', err, event);
+  if (!validateMessageEvent(event, bootstrapData.embedded?.allowed_domains)) {
+    log('ignoring message unrelated to embedded comms', event);
     return;
   }
 

superset-frontend/src/embedded/originValidation.test.ts

@@ -0,0 +1,112 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+  MESSAGE_TYPE,
+  isMessageOriginAllowed,
+  validateMessageEvent,
+} from './originValidation';
+
+const makeEvent = (origin: string, data: unknown): MessageEvent =>
+  ({ origin, data }) as MessageEvent;
+
+const validData = { type: MESSAGE_TYPE, handshake: 'port transfer' };
+
+test('isMessageOriginAllowed allows any origin when the list is undefined', () => {
+  expect(isMessageOriginAllowed('https://anywhere.example.com')).toBe(true);
+});
+
+test('isMessageOriginAllowed allows any origin when the list is empty', () => {
+  expect(isMessageOriginAllowed('https://anywhere.example.com', [])).toBe(true);
+});
+
+test('isMessageOriginAllowed allows an origin that is in the list', () => {
+  expect(
+    isMessageOriginAllowed('https://allowed.example.com', [
+      'https://allowed.example.com',
+    ]),
+  ).toBe(true);
+});
+
+test('isMessageOriginAllowed matches a listed domain with a trailing slash', () => {
+  expect(
+    isMessageOriginAllowed('https://allowed.example.com', [
+      'https://allowed.example.com/',
+    ]),
+  ).toBe(true);
+});
+
+test('isMessageOriginAllowed matches a listed domain that includes a path', () => {
+  expect(
+    isMessageOriginAllowed('https://allowed.example.com', [
+      'https://allowed.example.com/embed',
+    ]),
+  ).toBe(true);
+});
+
+test('isMessageOriginAllowed rejects an origin that is not in the list and warns', () => {
+  const warn = jest.spyOn(console, 'warn').mockImplementation(() => {});
+  expect(
+    isMessageOriginAllowed('https://evil.example.com', [
+      'https://allowed.example.com',
+    ]),
+  ).toBe(false);
+  expect(warn).toHaveBeenCalledTimes(1);
+  warn.mockRestore();
+});
+
+test('validateMessageEvent accepts a valid embedded message from any origin when unrestricted', () => {
+  const event = makeEvent('https://anywhere.example.com', validData);
+  expect(validateMessageEvent(event)).toBe(true);
+  expect(validateMessageEvent(event, [])).toBe(true);
+});
+
+test('validateMessageEvent accepts a valid embedded message from a listed origin', () => {
+  const event = makeEvent('https://allowed.example.com', validData);
+  expect(validateMessageEvent(event, ['https://allowed.example.com'])).toBe(
+    true,
+  );
+});
+
+test('validateMessageEvent rejects a message from an origin not in a non-empty list', () => {
+  const warn = jest.spyOn(console, 'warn').mockImplementation(() => {});
+  const event = makeEvent('https://evil.example.com', validData);
+  expect(validateMessageEvent(event, ['https://allowed.example.com'])).toBe(
+    false,
+  );
+  warn.mockRestore();
+});
+
+test('validateMessageEvent rejects a message whose data type does not match', () => {
+  const event = makeEvent('https://allowed.example.com', {
+    type: 'something-else',
+  });
+  expect(validateMessageEvent(event, ['https://allowed.example.com'])).toBe(
+    false,
+  );
+});
+
+test('validateMessageEvent rejects a message whose data is not an object', () => {
+  const event = makeEvent('https://allowed.example.com', 'not-an-object');
+  expect(validateMessageEvent(event)).toBe(false);
+});
+
+test('validateMessageEvent rejects a message whose data is null', () => {
+  const event = makeEvent('https://allowed.example.com', null);
+  expect(validateMessageEvent(event)).toBe(false);
+});