You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "bbovenzi (via GitHub)" <gi...@apache.org> on 2025/07/15 17:09:40 UTC

[PR] Add react apps plugins to the UI [airflow]

bbovenzi opened a new pull request, #53377:
URL: https://github.com/apache/airflow/pull/53377

   Dynamically render react_apps plugins in the nav bar and in dag/run/task tabs just like iframe plugins.
   
   Next up, adding react plugins into the dashboard
   
   <img width="747" height="498" alt="Screenshot 2025-07-15 at 1 02 40 PM" src="https://github.com/user-attachments/assets/96db0b6d-77c3-4528-8a58-d416aa82db4b" />
   
   
   
   ---
   **^ Add meaningful description above**
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
   In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add react apps plugins to the UI [airflow]

Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #53377:
URL: https://github.com/apache/airflow/pull/53377#discussion_r2210659295


##########
airflow-core/src/airflow/ui/src/pages/PluginView.tsx:
##########
@@ -0,0 +1,91 @@
+/*!
+ * 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 { Box } from "@chakra-ui/react";
+import { useTranslation } from "react-i18next";
+import { useParams } from "react-router-dom";
+
+import { usePluginServiceGetPlugins } from "openapi/queries";
+import { ProgressBar } from "src/components/ui";
+
+import { ErrorPage } from "./Error";
+import { Iframe } from "./Iframe";
+import { ReactPlugin } from "./ReactPlugin";
+
+export const PluginView = () => {
+  const { t: translate } = useTranslation();
+  const { page } = useParams();
+  const { data: pluginData, isLoading } = usePluginServiceGetPlugins();
+
+  // Check for external_views first
+  const externalView =
+    page === "legacy-fab-views"
+      ? {
+          destination: "nav" as const,
+          href: "/pluginsv2/",
+          name: translate("nav.legacyFabViews"),
+          url_route: "legacy-fab-views",
+        }
+      : pluginData?.plugins
+          .flatMap((plugin) => plugin.external_views)
+          .find((view) => (view.url_route ?? view.name.toLowerCase().replace(" ", "-")) === page);
+
+  // Check for react_apps if no external view found
+  const reactApp = pluginData?.plugins
+    .flatMap((plugin) => plugin.react_apps)
+    .find((view) => (view.url_route ?? view.name.toLowerCase().replace(" ", "-")) === page);
+
+  if (isLoading) {
+    return (
+      <Box flexGrow={1}>
+        <ProgressBar />
+      </Box>
+    );
+  }
+
+  // If external view is found, render Iframe component
+  if (externalView) {
+    return (
+      <Box
+        flexGrow={1}
+        height="100%"
+        m={-2} // Compensate for parent padding
+        minHeight={0}
+      >
+        <Iframe externalView={externalView} sandbox="allow-scripts allow-forms" />

Review Comment:
   It's quite risky to do so but yeah, we're relying on plugin managers being trustworthy



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add react apps plugins to the UI [airflow]

Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #53377:
URL: https://github.com/apache/airflow/pull/53377#discussion_r2210654360


##########
airflow-core/src/airflow/ui/src/utils/sanitizeUrl.ts:
##########
@@ -0,0 +1,84 @@
+/*!
+ * 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.
+ */
+
+type UrlType = "bundle" | "iframe";
+
+/**
+ * Sanitizes URLs for safe use in iframe src attributes or dynamic imports
+ * Prevents XSS by validating protocol and removing dangerous content
+ */
+export const sanitizeUrl = (url: string, type: UrlType): string => {
+  try {
+    // Parse the URL to validate its structure
+    const urlObj = new URL(url);
+
+    // Only allow http, https, and relative URLs
+    if (urlObj.protocol !== "http:" && urlObj.protocol !== "https:" && urlObj.protocol !== "") {
+      throw new Error("Invalid protocol");

Review Comment:
   I'm not sure either. Maybe we can keep that code around and put it back if requested.



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add react apps plugins to the UI [airflow]

Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #53377:
URL: https://github.com/apache/airflow/pull/53377#discussion_r2208085380


##########
airflow-core/src/airflow/ui/src/pages/Iframe.tsx:
##########
@@ -69,23 +49,16 @@
   }
 
   return (
-    <Box
-      flexGrow={1}
-      height="100%"
-      m={-2} // Compensate for parent padding
-      minHeight={0}
-    >
-      <iframe
-        sandbox={sandbox}
-        src={src}
-        style={{
-          border: "none",
-          display: "block",
-          height: "100%",
-          width: "100%",
-        }}
-        title={iframeView.name}
-      />
-    </Box>
+    <iframe
+      sandbox={sandbox}
+      src={src}

Review Comment:
   ## Client-side cross-site scripting
   
   Cross-site scripting vulnerability due to [user-provided value](1).
   
   [Show more details](https://github.com/apache/airflow/security/code-scanning/502)



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add react apps plugins to the UI [airflow]

Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi merged PR #53377:
URL: https://github.com/apache/airflow/pull/53377


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add react apps plugins to the UI [airflow]

Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #53377:
URL: https://github.com/apache/airflow/pull/53377#discussion_r2210057628


##########
airflow-core/src/airflow/ui/src/hooks/usePluginTabs.tsx:
##########
@@ -36,9 +36,11 @@ export const usePluginTabs = (destination: string): Array<TabPlugin> => {
   // Get external views with the specified destination and ensure they have url_route
   const externalViews =
     pluginData?.plugins
-      .flatMap((plugin) => plugin.external_views)
-      .filter((view: ExternalViewResponse) => view.destination === destination && Boolean(view.url_route)) ??
-    [];
+      .flatMap((plugin) => [...plugin.external_views, ...plugin.react_apps])
+      .filter(
+        (view: ExternalViewResponse | ReactAppResponse) =>

Review Comment:
   Maybe also a rename to emphasize the fact that this is an API type. 
   ```suggestion
           (view: NavItemReponse) =>
   ```



##########
airflow-core/src/airflow/ui/src/pages/PluginView.tsx:
##########
@@ -0,0 +1,91 @@
+/*!
+ * 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 { Box } from "@chakra-ui/react";
+import { useTranslation } from "react-i18next";
+import { useParams } from "react-router-dom";
+
+import { usePluginServiceGetPlugins } from "openapi/queries";
+import { ProgressBar } from "src/components/ui";
+
+import { ErrorPage } from "./Error";
+import { Iframe } from "./Iframe";
+import { ReactPlugin } from "./ReactPlugin";
+
+export const PluginView = () => {
+  const { t: translate } = useTranslation();
+  const { page } = useParams();
+  const { data: pluginData, isLoading } = usePluginServiceGetPlugins();
+
+  // Check for external_views first
+  const externalView =
+    page === "legacy-fab-views"
+      ? {
+          destination: "nav" as const,
+          href: "/pluginsv2/",
+          name: translate("nav.legacyFabViews"),
+          url_route: "legacy-fab-views",
+        }
+      : pluginData?.plugins
+          .flatMap((plugin) => plugin.external_views)
+          .find((view) => (view.url_route ?? view.name.toLowerCase().replace(" ", "-")) === page);
+
+  // Check for react_apps if no external view found
+  const reactApp = pluginData?.plugins
+    .flatMap((plugin) => plugin.react_apps)
+    .find((view) => (view.url_route ?? view.name.toLowerCase().replace(" ", "-")) === page);
+
+  if (isLoading) {
+    return (
+      <Box flexGrow={1}>
+        <ProgressBar />
+      </Box>
+    );
+  }
+
+  // If external view is found, render Iframe component
+  if (externalView) {
+    return (
+      <Box
+        flexGrow={1}
+        height="100%"
+        m={-2} // Compensate for parent padding
+        minHeight={0}
+      >
+        <Iframe externalView={externalView} sandbox="allow-scripts allow-forms" />

Review Comment:
   We don't have 'same-origin' here, meaning that the embedded Iframe can't be from airflow domain itself, is that expected ? I don't think it is.



##########
airflow-core/src/airflow/ui/src/pages/PluginView.tsx:
##########


Review Comment:
   Maybe `ExternalViewIframe` or anything else `PluginView` is too generic.



##########
airflow-core/src/airflow/ui/src/router.tsx:
##########
@@ -60,13 +60,13 @@ import { XCom } from "src/pages/XCom";
 
 import { client } from "./queryClient";
 
-const iframeRoute = {
+const pluginRoute = {
   // The following iframe sandbox setting is intentionally less restrictive.
   // This is considered safe because the framed content originates from the Plugins,
   // which is part of the deployment of Airflow and trusted as per our security policy.
   // https://airflow.apache.org/docs/apache-airflow/stable/security/security_model.html
   // They are not user provided plugins.

Review Comment:
   This comment is not relevant anymore and should be removed or moved to the appropriate place if we come back to a allow-same-origin + allow-script sandbox somewhere.



##########
airflow-core/src/airflow/ui/src/utils/sanitizeUrl.ts:
##########
@@ -0,0 +1,84 @@
+/*!
+ * 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.
+ */
+
+type UrlType = "bundle" | "iframe";
+
+/**
+ * Sanitizes URLs for safe use in iframe src attributes or dynamic imports
+ * Prevents XSS by validating protocol and removing dangerous content
+ */
+export const sanitizeUrl = (url: string, type: UrlType): string => {
+  try {
+    // Parse the URL to validate its structure
+    const urlObj = new URL(url);
+
+    // Only allow http, https, and relative URLs
+    if (urlObj.protocol !== "http:" && urlObj.protocol !== "https:" && urlObj.protocol !== "") {
+      throw new Error("Invalid protocol");

Review Comment:
   Interesting. One of the hypothesis I had was that, none of the external views or react apps are user provided values. 
   
   Those are made by plugins developer and deployed by deployment manager, so I figured we could trust such values.
   
   WDYT, is that needed in some scenarios ?



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add react apps plugins to the UI [airflow]

Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #53377:
URL: https://github.com/apache/airflow/pull/53377#discussion_r2208576921


##########
airflow-core/src/airflow/ui/src/pages/Iframe.tsx:
##########
@@ -68,24 +49,20 @@
     }
   }
 
+  // Sanitize the URL for safe use in iframe
+  const sanitizedSrc = sanitizeIframeUrl(src);
+
   return (
-    <Box
-      flexGrow={1}
-      height="100%"
-      m={-2} // Compensate for parent padding
-      minHeight={0}
-    >
-      <iframe
-        sandbox={sandbox}
-        src={src}
-        style={{
-          border: "none",
-          display: "block",
-          height: "100%",
-          width: "100%",
-        }}
-        title={iframeView.name}
-      />
-    </Box>
+    <iframe
+      sandbox={sandbox}
+      src={sanitizedSrc}

Review Comment:
   ## Client-side cross-site scripting
   
   Cross-site scripting vulnerability due to [user-provided value](1).
   
   [Show more details](https://github.com/apache/airflow/security/code-scanning/503)



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add react apps plugins to the UI [airflow]

Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #53377:
URL: https://github.com/apache/airflow/pull/53377#discussion_r2210589018


##########
airflow-core/src/airflow/ui/src/utils/sanitizeUrl.ts:
##########
@@ -0,0 +1,84 @@
+/*!
+ * 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.
+ */
+
+type UrlType = "bundle" | "iframe";
+
+/**
+ * Sanitizes URLs for safe use in iframe src attributes or dynamic imports
+ * Prevents XSS by validating protocol and removing dangerous content
+ */
+export const sanitizeUrl = (url: string, type: UrlType): string => {
+  try {
+    // Parse the URL to validate its structure
+    const urlObj = new URL(url);
+
+    // Only allow http, https, and relative URLs
+    if (urlObj.protocol !== "http:" && urlObj.protocol !== "https:" && urlObj.protocol !== "") {
+      throw new Error("Invalid protocol");

Review Comment:
   Yeah, I was back and forth on this. I can remove for now



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org