diff --git a/airflow-core/src/airflow/ui/src/pages/DagsList/DagOwners.tsx b/airflow-core/src/airflow/ui/src/pages/DagsList/DagOwners.tsx index 5eb12f3d1eade..e4102eb22de6c 100644 --- a/airflow-core/src/airflow/ui/src/pages/DagsList/DagOwners.tsx +++ b/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 = []; 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; diff --git a/airflow-core/src/airflow/ui/src/pages/TaskInstance/ExtraLinks.tsx b/airflow-core/src/airflow/ui/src/pages/TaskInstance/ExtraLinks.tsx index b2d72bae13092..f0b13b84d801e 100644 --- a/airflow-core/src/airflow/ui/src/pages/TaskInstance/ExtraLinks.tsx +++ b/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 ( diff --git a/airflow-core/src/airflow/ui/src/utils/links.test.ts b/airflow-core/src/airflow/ui/src/utils/links.test.ts index 542f508a962a2..a23711ded545d 100644 --- a/airflow-core/src/airflow/ui/src/utils/links.test.ts +++ b/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,"], + ["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(); + }); + }); +}); diff --git a/airflow-core/src/airflow/ui/src/utils/links.ts b/airflow-core/src/airflow/ui/src/utils/links.ts index 6770f2e7cbbf1..403f280171015 100644 --- a/airflow-core/src/airflow/ui/src/utils/links.ts +++ b/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;