Skip to content

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

Merged
choo121600 merged 1 commit into
apache:mainfrom
potiuk:restrict-owner-link-and-extra-link-href-to-safe-schemes
May 12, 2026
Merged

Restrict owner-link and extra-link href to safe schemes (http, https, mailto, relative)#66741
choo121600 merged 1 commit into
apache:mainfrom
potiuk:restrict-owner-link-and-extra-link-href-to-safe-schemes

Conversation

@potiuk
Copy link
Copy Markdown
Member

@potiuk potiuk commented May 12, 2026

Summary

Bring DAG owner_links and operator extra-link rendering in line with the scheme-allowlist policy already applied to markdown links (react-markdown's default urlTransform) and log / XCom linkification (which is https?://-only).

Before this change, both surfaces rendered DAG-author-supplied URLs (dag.owner_links set in the DAG file; operator extra-link URLs commonly read from task-pushed XCom) directly as <a href={url} target="_blank"> with no scheme filter. target="_blank" blocks javascript: navigation on every modern browser, so the application itself provided no defense in depth on the rare legacy / embedded webview that still navigates javascript: in a new tab.

After this change, both surfaces consult a shared getSafeExternalUrl(url) helper that passes through http: / https: / mailto: / relative URLs and returns undefined for anything else. Unsafe URLs are silently skipped on the extra-links page; the owner-link surface falls back to the existing in-app /dags?owners=X filter link.

Files changed

  • airflow-core/src/airflow/ui/src/utils/links.ts — new getSafeExternalUrl helper.
  • airflow-core/src/airflow/ui/src/utils/links.test.ts — new vitest cases for the allow / reject matrix.
  • airflow-core/src/airflow/ui/src/pages/TaskInstance/ExtraLinks.tsx — wrap extra-link url with the helper; skip rendering when undefined.
  • airflow-core/src/airflow/ui/src/pages/DagsList/DagOwners.tsx — wrap owner_links[owner] with the helper; fall back to the filter-link when unsafe.

Test plan

  • pnpm vitest run src/utils/links.test.ts — 34 / 34 tests pass (8 allow + 9 reject cases for getSafeExternalUrl, plus the existing tests)
  • pnpm lint — eslint clean, tsc --p tsconfig.app.json clean
  • prek run --from-ref upstream/main --stage pre-commit — clean

Migration

DAG-author-supplied owner_links entries using non-allowlisted schemes (javascript:, data:, file:, etc.) are silently skipped on the UI and fall back to the in-app owner-filter link; same for operator extra-links carrying such schemes. No DAG-level API surface changes.

Was generative AI tooling used to co-author this PR?
  • Yes — Claude Opus 4.7 (1M context)

Generated-by: Claude Opus 4.7 (1M context) following the guidelines at https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions

… mailto, relative)

Bring DAG `owner_links` and operator extra-link rendering in line with the
scheme-allowlist policy already used by markdown links (react-markdown's
default `urlTransform`) and log / XCom linkification (which is
`https?://`-only).

Adds a `getSafeExternalUrl` helper that passes through `http:`, `https:`,
`mailto:`, and same-origin / relative URLs and returns `undefined` for any
other scheme. Both `DagOwners.tsx` (owner-link rendering on the DAGs list
and DAG header) and `ExtraLinks.tsx` (operator extra-link buttons on the
task-instance page) now consult the helper and skip / downgrade to the
filter-link fallback when the href would be unsafe.

Generated-by: Claude Opus 4.7 (1M context) following the guidelines at
https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions
@boring-cyborg boring-cyborg Bot added the area:UI Related to UI/UX. For Frontend Developers. label May 12, 2026
Copy link
Copy Markdown
Member

@choo121600 choo121600 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm :)

@choo121600 choo121600 merged commit e248670 into apache:main May 12, 2026
63 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:UI Related to UI/UX. For Frontend Developers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants