Skip to content

Scope UI next_run_assets endpoint to assets the caller may read - #72862

Merged
pierrejeambrun merged 1 commit into
apache:mainfrom
henry3260:fix-next-run-assets-filter
Oct 5, 2026
Merged

pierrejeambrun merged 1 commit into
apache:mainfrom
henry3260:fix-next-run-assets-filter

Conversation

@henry3260

@henry3260 henry3260 commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Why

GET /ui/next_run_assets/{dag_id} now redacts asset_expression through ReadableAssetsFilterDep (#72864), but the query behind the events list is still filtered by dag_id alone. The class-level requires_access_asset("GET") check passes AssetDetails(id=None) to the auth manager, because the route has no asset_id path parameter, so it only confirms the caller may read assets in general.

A caller with read access to a Dag and generic asset read permission therefore still receives the id, name, uri, asset_inactive flag and, for partitioned Dags, the received_keys / required_keys of upstream assets that a fine-grained auth manager hides from them — the names the expression just redacted come straight back in events. The sibling GET /ui/assets list endpoint already scopes its results through ReadableAssetsFilterDep; this endpoint never wired it into the query.

What

  • airflow-core/src/airflow/api_fastapi/core_api/routes/ui/assets.py: apply the already-injected ReadableAssetsFilterDep to the asset query before execution, so the events list and the partitioned enrichment only cover assets the caller may read.
Note on caller-scoped counts

Unlike the asset_expression redaction in #72864, which keeps a hidden slot per unreadable asset to preserve the schedule shape, this drops unreadable rows from events entirely — a hidden asset's id / name / uri are required fields on NextRunAssetEventResponse and cannot be returned at all without reintroducing the leak.


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Claude Fable 5.1)

@henry3260

Copy link
Copy Markdown
Contributor Author

Note: the asset_expression field still echoes dag_model.asset_expression unfiltered, as does the public DagDetailsResponse. That is a broader, pre-existing exposure and is out of scope here.

@rjgoyln rjgoyln left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me!

Two notes below: one on the shape this gives the events list, and one on the new test's mock assertion.

Just my thoughts, feel free to resolve them.


Drafted-by: Claude Code (Opus 5); reviewed by @rjgoyln before posting

Comment thread airflow-core/src/airflow/api_fastapi/core_api/routes/ui/assets.py
Comment thread airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_assets.py Outdated

@pierrejeambrun pierrejeambrun left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yep, needs a UI follow up to still show 1 of 2 without disclosing more information.

@pierrejeambrun
pierrejeambrun merged commit 8239997 into apache:main Oct 5, 2026
79 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants