Restrict owner-link and extra-link href to safe schemes (http, https, mailto, relative)

airflow-core/src/airflow/ui/src/pages/DagsList/DagOwners.tsx

@@ -21,6 +21,7 @@ import { useTranslation } from "react-i18next";
 import { Link as RouterLink } from "react-router-dom";
 
 import { LimitedItemsList } from "src/components/LimitedItemsList";
+import { getSafeExternalUrl } from "src/utils/links";
 
 const DEFAULT_OWNERS: Array<string> = [];
 const MAX_OWNERS = 3;
@@ -34,7 +35,8 @@ export const DagOwners = ({
 }) => {
   const { t: translate } = useTranslation("dags");
   const items = owners.map((owner) => {
-    const ownerLink = ownerLinks?.[owner];
+    const rawOwnerLink = ownerLinks?.[owner];
+    const ownerLink = rawOwnerLink === undefined ? undefined : getSafeExternalUrl(rawOwnerLink);
     const ownerFilterLink = `/dags?owners=${owner}`;
     const hasOwnerLink = ownerLink !== undefined;
 

airflow-core/src/airflow/ui/src/pages/TaskInstance/ExtraLinks.tsx

@@ -22,6 +22,7 @@ import { useParams, useSearchParams } from "react-router-dom";
 
 import { useTaskInstanceServiceGetExtraLinks } from "openapi/queries";
 import { SearchParamsKeys } from "src/constants/searchParams";
+import { getSafeExternalUrl } from "src/utils/links";
 
 type ExtraLinksProps = {
   readonly refetchInterval: number | false;
@@ -67,11 +68,17 @@ export const ExtraLinks = ({ refetchInterval }: ExtraLinksProps) => {
             return undefined;
           }
 
-          const target = getTarget(url);
+          const safeUrl = getSafeExternalUrl(url);
+
+          if (safeUrl === undefined) {
+            return undefined;
+          }
+
+          const target = getTarget(safeUrl);
 
           return (
             <Button asChild colorPalette="brand" key={key} variant="surface">
-              <a href={url} rel="noopener noreferrer" target={target}>
+              <a href={safeUrl} rel="noopener noreferrer" target={target}>
                 {key}
               </a>
             </Button>

airflow-core/src/airflow/ui/src/utils/links.test.ts

@@ -20,7 +20,12 @@ import { describe, it, expect } from "vitest";
 
 import type { TaskInstanceResponse } from "openapi/requests/types.gen";
 
-import { buildTaskInstanceUrl, getTaskInstanceAdditionalPath, getTaskInstanceLink } from "./links";
+import {
+  buildTaskInstanceUrl,
+  getSafeExternalUrl,
+  getTaskInstanceAdditionalPath,
+  getTaskInstanceLink,
+} from "./links";
 
 describe("getTaskInstanceLink", () => {
   const testCases = [
@@ -284,3 +289,40 @@ describe("buildTaskInstanceUrl", () => {
     ).toBe("/dags/new_dag/runs/new_run/tasks/new_task/mapped");
   });
 });
+
+describe("getSafeExternalUrl", () => {
+  describe("allows", () => {
+    const safeCases = [
+      ["http URL", "http://example.com/path"],
+      ["https URL", "https://example.com/path?q=1#anchor"],
+      ["mailto URL", "mailto:ops@example.com"],
+      ["relative absolute path", "/dags/my_dag"],
+      ["relative same-document fragment", "#section"],
+      ["relative query string", "?owner=me"],
+      ["same-origin absolute URL", `${globalThis.location.origin}/dags`],
+      ["URL with surrounding whitespace", "  https://example.com/x  "],
+    ];
+
+    it.each(safeCases)("passes through a %s", (_description, input) => {
+      expect(getSafeExternalUrl(input)).toBe(input.trim());
+    });
+  });
+
+  describe("rejects", () => {
+    const unsafeCases = [
+      ["javascript: URL", "javascript:alert(1)"],
+      ["javascript: URL with mixed case", "JavaScript:alert(1)"],
+      ["javascript: URL with leading whitespace", " javascript:alert(1)"],
+      ["data: URL", "data:text/html,<script>alert(1)</script>"],
+      ["file: URL", "file:///etc/passwd"],
+      ["vbscript: URL", "vbscript:msgbox(1)"],
+      ["ftp: URL", "ftp://example.com/file"],
+      ["empty string", ""],
+      ["whitespace-only string", "   "],
+    ];
+
+    it.each(unsafeCases)("returns undefined for a %s", (_description, input) => {
+      expect(getSafeExternalUrl(input)).toBeUndefined();
+    });
+  });
+});

airflow-core/src/airflow/ui/src/utils/links.ts

@@ -73,6 +73,55 @@ export const getTaskInstanceAdditionalPath = (pathname: string): string => {
   return "";
 };
 
+const SAFE_EXTERNAL_URL_SCHEMES = new Set(["http:", "https:", "mailto:"]);
+
+/**
+ * Pass-through filter for href values that originate outside the application —
+ * for example DAG-author-supplied `owner_links`, or operator extra-link URLs
+ * read from task-pushed XCom.
+ *
+ * Returns the URL unchanged when it is either a same-origin / relative path or
+ * uses one of the allow-listed schemes (`http:`, `https:`, `mailto:`).
+ * Returns `undefined` for any other scheme (`javascript:`, `data:`, `file:`,
+ * `vbscript:`, etc.) and for unparsable input, matching the scheme-allowlist
+ * policy already applied to markdown links via react-markdown's default
+ * `urlTransform` and to log / XCom linkification (which is `https?://`-only).
+ *
+ * Callers should fall back to plain text or skip rendering when this returns
+ * `undefined`.
+ */
+export const getSafeExternalUrl = (url: string): string | undefined => {
+  const trimmed = url.trim();
+
+  if (trimmed === "") {
+    return undefined;
+  }
+
+  let parsed: URL;
+
+  try {
+    parsed = new URL(trimmed, globalThis.location.origin);
+  } catch {
+    return undefined;
+  }
+
+  // Same-origin URL (relative input, or absolute pointing at our own origin).
+  // We have to compare against `location.origin` rather than just looking at
+  // the protocol because `new URL("/foo", origin)` produces a URL whose
+  // protocol is the origin's protocol (typically `http(s):`), so a
+  // protocol-only check would let through any non-allow-listed scheme that
+  // happens to share the origin's protocol shape.
+  if (parsed.origin === globalThis.location.origin) {
+    return trimmed;
+  }
+
+  if (SAFE_EXTERNAL_URL_SCHEMES.has(parsed.protocol)) {
+    return trimmed;
+  }
+
+  return undefined;
+};
+
 export const buildTaskInstanceUrl = (params: {
   currentPathname: string;
   dagId: string;