[pull] master from apache:master

UPDATING.md

@@ -31,6 +31,7 @@ The unqualified `DatabaseRestApi.oauth2` StatsD counter has been replaced with
 `DatabaseRestApi.oauth2.error`. Update monitoring rules and dashboards that consume
 the old counter to use the outcome-specific replacements.
 
+- [42930](https://github.com/apache/superset/pull/42930): Dataset import data-URI fetches no longer honor an HTTP(S) proxy when `DATASET_IMPORT_ALLOW_INTERNAL_DATA_URLS` is `False` (the default): the connection is now made directly to the destination so the peer-address check validates the real target instead of a proxy's. Deployments that require an egress proxy to reach legitimate external data URLs for dataset import should set `DATASET_IMPORT_ALLOW_INTERNAL_DATA_URLS = True` or otherwise ensure those URLs resolve without one.
 - [42935](https://github.com/apache/superset/pull/42935): The MCP service now refuses to start (`MCPAuthConfigError`) when `MCP_JWT_ISSUER` trusts more than one issuer and no `MCP_USER_RESOLVER` is configured, instead of only logging a warning. This was already a documented misconfiguration (the default resolver isn't issuer-scoped, so distinct trusted issuers minting the same username/email would resolve to the same Superset user); deployments trusting multiple issuers must configure an `MCP_USER_RESOLVER` that derives its identity from the token's `iss` claim before upgrading. Single-issuer deployments are unaffected.
 - [42393](https://github.com/apache/superset/pull/42393): Exported dataset YAML now carries a `uuid` for each metric and column so that custom folder assignments (which reference metrics/columns by UUID) survive an import into another workspace. This affects any export bundle that contains datasets, not just a dataset export: chart, dashboard, database and full-asset exports all embed the same dataset YAML, so a dashboard exported from this release also fails to import into an older one even though no dataset was exported directly. As with `folders` and `currency_code_column`, the affected `datasets/` files fail schema validation (`Unknown field: uuid`) when imported into Superset releases that predate this change; regenerate or hand-edit exports for older targets in mixed-version fleets.
 - [42300](https://github.com/apache/superset/pull/42300): Timeseries charts (line/area/bar) with a Y-axis bound in effect — either an explicit `yAxisBounds` or one derived from `truncateYAxis` — now clamp out-of-range data points to that bound instead of letting ECharts drop the point (and the line segments around it) entirely. Any existing chart with a configured Y-axis bound and data outside it will look different after upgrading: a gap becomes a point pinned to the boundary. The clamp also rewrites the value ECharts reads for that point's tooltip and data label, so the displayed value is the bound rather than the true observation.

superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.test.tsx

@@ -16,9 +16,13 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { render, waitFor } from '../../../../spec/helpers/testing-library';
 import type { EChartsCoreOption } from 'echarts/core';
-import Echart, { isReportScreenshotMode } from './Echart';
+import { render, waitFor } from '../../../../spec/helpers/testing-library';
+import Echart, {
+  ECHARTS_HOST_CLASS,
+  ECHARTS_RENDER_FINISHED_CLASS,
+  isReportScreenshotMode,
+} from './Echart';
 import type { EchartsProps } from '../types';
 
 type Handler = (params: unknown) => void;
@@ -272,3 +276,31 @@ test('keeps animation enabled when not in report screenshot mode', async () => {
   const lastOptions = mockChart.setOption.mock.calls.at(-1)?.[0];
   expect(lastOptions.animation).not.toBe(false);
 });
+
+test('tags the ECharts canvas host with the readiness-gate class', async () => {
+  const { container } = render(renderEchart(), {
+    initialState,
+    useRedux: true,
+  });
+  await waitFor(() => expect(mockChart.setOption).toHaveBeenCalled());
+  expect(container.querySelector(`.${ECHARTS_HOST_CLASS}`)).not.toBeNull();
+});
+
+test('marks the host painted only on the ECharts `finished` event', async () => {
+  const { container } = render(renderEchart(), {
+    initialState,
+    useRedux: true,
+  });
+  await waitFor(() => expect(mockChart.setOption).toHaveBeenCalled());
+
+  const host = container.querySelector(`.${ECHARTS_HOST_CLASS}`) as HTMLElement;
+  expect(host).not.toBeNull();
+
+  // `setOption` ran during mount, which clears the marker; `finished` has not
+  // fired yet, so the host must NOT be flagged as painted.
+  expect(host).not.toHaveClass(ECHARTS_RENDER_FINISHED_CLASS);
+
+  // Simulate ECharts completing its draw -> the host is flagged painted.
+  trigger('finished');
+  expect(host).toHaveClass(ECHARTS_RENDER_FINISHED_CLASS);
+});

superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx

@@ -138,6 +138,15 @@ export function isReportScreenshotMode(): boolean {
   }
 }
 
+// Report-screenshot readiness contract (see superset/utils/screenshot_utils.py).
+// `echarts-host` marks the canvas host element; `echarts-render-finished` is
+// toggled OFF before each setOption and ON in the ECharts `finished` event --
+// the only signal that the canvas is fully painted (chartStatus/onRenderSuccess
+// both fire pre-paint). The readiness gate treats a host that lacks
+// `echarts-render-finished` as not-yet-painted so it never captures a blank chart.
+export const ECHARTS_HOST_CLASS = 'echarts-host';
+export const ECHARTS_RENDER_FINISHED_CLASS = 'echarts-render-finished';
+
 function Echart(
   {
     width,
@@ -201,6 +210,11 @@ function Echart(
           width,
           height,
         });
+        // Paint marker for the report-screenshot readiness gate. `finished`
+        // is the only event that guarantees the canvas is fully drawn.
+        chartRef.current.on('finished', () => {
+          divRef.current?.classList.add(ECHARTS_RENDER_FINISHED_CLASS);
+        });
       }
       // did mount
       handleSizeChange({ width, height });
@@ -321,6 +335,9 @@ function Echart(
             }
           )?.dataZoom
         : undefined;
+      // Clear the paint marker before (re)drawing; the `finished` handler
+      // re-adds it once the new frame is fully rendered.
+      divRef.current?.classList.remove(ECHARTS_RENDER_FINISHED_CLASS);
       chartRef.current?.setOption(themedEchartOptions, {
         notMerge,
         replaceMerge: notMerge ? undefined : ['series'],
@@ -412,7 +429,14 @@ function Echart(
     handleSizeChange({ width, height });
   }, [width, height, handleSizeChange]);
 
-  return <Styles ref={divRef} height={height} width={width} />;
+  return (
+    <Styles
+      ref={divRef}
+      className={ECHARTS_HOST_CLASS}
+      height={height}
+      width={width}
+    />
+  );
 }
 
 export default forwardRef(Echart);

superset/commands/database/oauth2.py

@@ -21,7 +21,7 @@
 from typing import cast
 from uuid import UUID
 
-from superset import db
+from superset import db, security_manager
 from superset.commands.base import BaseCommand
 from superset.commands.database.exceptions import DatabaseNotFoundError
 from superset.daos.database import DatabaseUserOAuth2TokensDAO
@@ -31,6 +31,7 @@
 from superset.key_value.types import JsonKeyValueCodec, KeyValueResource
 from superset.models.core import Database, DatabaseUserOAuth2Tokens
 from superset.superset_typing import OAuth2State
+from superset.utils.core import get_user_id
 from superset.utils.decorators import on_error, transaction
 from superset.utils.oauth2 import decode_oauth2_state
 
@@ -121,6 +122,14 @@ def validate(self) -> None:
 
         self._state = decode_oauth2_state(self._parameters["state"])
 
+        # Bind the callback to the current session: require an authenticated,
+        # non-guest user whose id matches the one carried in the state.
+        user_id = get_user_id()
+        if user_id is None or security_manager.is_guest_user():
+            raise OAuth2Error("The OAuth2 callback requires an authenticated user")
+        if user_id != self._state["user_id"]:
+            raise OAuth2Error("The OAuth2 state belongs to a different user")
+
         if database := DatabaseUserOAuth2TokensDAO.get_database(
             self._state["database_id"]
         ):

superset/commands/dataset/importers/v1/utils.py

@@ -15,9 +15,12 @@
 # specific language governing permissions and limitations
 # under the License.
 import gzip
+import ipaddress
 import logging
 import os
 import re
+import socket
+from http.client import HTTPConnection, HTTPResponse, HTTPSConnection
 from typing import Any
 from urllib import request
 from urllib.parse import urljoin, urlparse
@@ -47,7 +50,7 @@
 from superset.sql.parse import Table
 from superset.utils import json
 from superset.utils.core import get_user
-from superset.utils.network import is_safe_host
+from superset.utils.network import is_safe_host, is_safe_ip
 
 logger = logging.getLogger(__name__)
 
@@ -76,6 +79,47 @@ def redirect_request(
         return super().redirect_request(req, fp, code, msg, headers, newurl)
 
 
+def _raise_for_unsafe_peer(sock: socket.socket) -> None:
+    """
+    Validate that an established connection's actual peer is publicly
+    routable, so the address reached matches the policy applied to the host.
+    """
+    peer = sock.getpeername()[0]
+    if not is_safe_ip(ipaddress.ip_address(peer)):
+        raise DatasetForbiddenDataURI()
+
+
+class _PeerValidatingHTTPConnection(HTTPConnection):
+    """HTTP connection that validates the peer address on connect."""
+
+    def connect(self) -> None:
+        super().connect()
+        _raise_for_unsafe_peer(self.sock)
+
+
+class _PeerValidatingHTTPSConnection(HTTPSConnection):
+    """HTTPS connection that validates the peer address after the handshake."""
+
+    def connect(self) -> None:
+        super().connect()
+        _raise_for_unsafe_peer(self.sock)
+
+
+class _PeerValidatingHTTPHandler(request.HTTPHandler):
+    """Opens HTTP connections through the peer-validating connection class."""
+
+    def http_open(self, req: request.Request) -> HTTPResponse:
+        return self.do_open(_PeerValidatingHTTPConnection, req)
+
+
+class _PeerValidatingHTTPSHandler(request.HTTPSHandler):
+    """Opens HTTPS connections through the peer-validating connection class."""
+
+    def https_open(self, req: request.Request) -> HTTPResponse:
+        context = self._context  # type: ignore[attr-defined]
+        return self.do_open(_PeerValidatingHTTPSConnection, req, context=context)
+
+
 CHUNKSIZE = 512
 VARCHAR = re.compile(r"VARCHAR\((\d+)\)", re.IGNORECASE)
 
@@ -581,7 +625,17 @@ def load_data(data_uri: str, dataset: SqlaTable, database: Database) -> None:
 
     validate_data_uri(data_uri)
     logger.info("Downloading data from %s", data_uri)
-    opener = request.build_opener(_ValidatingRedirectHandler)
+    handlers: list[request.BaseHandler | type[request.BaseHandler]] = [
+        _ValidatingRedirectHandler
+    ]
+    if not app.config["DATASET_IMPORT_ALLOW_INTERNAL_DATA_URLS"]:
+        # Also enforce the policy at the socket layer: re-check the peer of
+        # every connection, including each redirect hop. Disable proxies so the
+        # connection is made directly to the destination and the peer check
+        # validates the destination address rather than a proxy's.
+        handlers.append(request.ProxyHandler({}))
+        handlers.extend([_PeerValidatingHTTPHandler, _PeerValidatingHTTPSHandler])
+    opener = request.build_opener(*handlers)
     data = opener.open(data_uri)  # pylint: disable=consider-using-with  # noqa: S310
     if data_uri.endswith(".gz"):
         data = gzip.open(data)

superset/commands/report/execute.py

@@ -137,6 +137,29 @@ def resolve_executor_user(model: ReportSchedule) -> tuple["User", str]:
     return user, username
 
 
+def _should_build_execution_context(model: ReportSchedule) -> bool:
+    """
+    Whether an execution should run under a :class:`ReportExecutionContext`.
+
+    Reports always do — their behavior is unchanged. Alerts join them only when
+    they deliver a rendered PNG/PDF screenshot to recipients, which happens when
+    ``ALERTS_ATTACH_REPORTS`` is enabled. Delivered screenshots must fail closed:
+    the context selects the fail-closed readiness predicate and disables
+    partial-tile fallback, so a blank or incomplete capture raises instead of
+    being delivered.
+
+    CSV/text alerts, alerts without the attach flag, the non-delivered
+    query-context capture, and UI thumbnails are deliberately excluded and keep
+    their lenient capture contract.
+    """
+    if model.type == ReportScheduleType.REPORT:
+        return True
+    return model.report_format in (
+        ReportDataFormat.PNG,
+        ReportDataFormat.PDF,
+    ) and feature_flag_manager.is_feature_enabled("ALERTS_ATTACH_REPORTS")
+
+
 def log_report_delivery_phase(
     report_context: ReportExecutionContext | None,
     recipient_type: ReportRecipientType | None,
@@ -1972,13 +1995,13 @@ def next(self) -> None:  # noqa: C901
 
         try:
             self.send()
-        except Exception as ex:  # pylint: disable=broad-except
-            if self._handle_retry_or_error(str(ex), ex):
+        except Exception as first_ex:  # pylint: disable=broad-except
+            if self._handle_retry_or_error(str(first_ex), first_ex):
                 return  # retry scheduled — exit cleanly
 
             try:
                 self.update_report_schedule_and_log(
-                    ReportState.ERROR, error_message=str(ex)
+                    ReportState.ERROR, error_message=str(first_ex)
                 )
             except (ReportScheduleUnexpectedError, SQLAlchemyError) as logging_ex:
                 # Logging failed (likely StaleDataError), but we still want to
@@ -1991,7 +2014,45 @@ def next(self) -> None:  # noqa: C901
                     exc_info=True,
                 )
                 # Re-raise the original exception, not the logging failure
-                raise ex from logging_ex
+                raise first_ex from logging_ex
+
+            # A delivery failure from the Success/Grace path must notify the
+            # owner just like the first-run path (ReportNotTriggeredErrorState).
+            # Without this, a schedule whose previous run succeeded would fail
+            # silently — e.g. once a screenshot capture starts failing closed.
+            # The error grace period still throttles repeated notifications.
+            if not self.is_in_error_grace_period():
+                second_error_message = REPORT_SCHEDULE_ERROR_NOTIFICATION_MARKER
+                try:
+                    self.send_error(
+                        f"Error occurred for {self._report_schedule.type}:"
+                        f" {self._report_schedule.name}",
+                        str(first_ex),
+                    )
+                except SupersetErrorsException as second_ex:
+                    second_error_message = ";".join(
+                        [error.message for error in second_ex.errors]
+                    )
+                except ReportScheduleUnexpectedError:
+                    # send_error failed due to logging issue; log and continue
+                    # to raise the original error
+                    logger.warning(
+                        "Failed to send error notification due to database issue",
+                        exc_info=True,
+                    )
+                except Exception as second_ex:  # pylint: disable=broad-except
+                    second_error_message = str(second_ex)
+                finally:
+                    try:
+                        self.update_report_schedule_and_log(
+                            ReportState.ERROR, error_message=second_error_message
+                        )
+                    except ReportScheduleUnexpectedError:
+                        # Logging failed again; log it but don't hide first_ex
+                        logger.warning(
+                            "Failed to log final error state due to database issue",
+                            exc_info=True,
+                        )
             raise
 
         # send() succeeded — clear retry state and log success. Any execution
@@ -2058,13 +2119,18 @@ def run(self) -> None:
             if not self._model:
                 raise ReportScheduleExecuteUnexpectedError()
 
-            if self._model.type == ReportScheduleType.REPORT:
+            # Reports always run under an execution context; alerts join them
+            # only when they deliver a rendered screenshot, so a blank/partial
+            # capture fails closed instead of being delivered. Ownership and
+            # terminal-error persistence remain report-only recovery semantics.
+            if _should_build_execution_context(self._model):
                 # An invocation that enters on WORKING is a duplicate or stale
                 # recovery, not the owner that created the active row. Its state
                 # handler may terminalize a stale execution, but the command
                 # boundary must never infer ownership from a replayed UUID.
                 owns_report_working_state = (
-                    self._model.last_state != ReportState.WORKING
+                    self._model.type == ReportScheduleType.REPORT
+                    and self._model.last_state != ReportState.WORKING
                 )
                 total_seconds = resolve_report_execution_budget_seconds(
                     app.config,