You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "TinyGeek (via GitHub)" <gi...@apache.org> on 2026/10/12 08:14:13 UTC
[PR] Exclude stale Dags from active Dag source authorization [airflow]
TinyGeek opened a new pull request, #74576:
URL: https://github.com/apache/airflow/pull/74576
When a stale Dag shares a source file and bundle with an active Dag, the source endpoint can redact code even when the caller can read every active Dag in that file.
Update the per-file authorization check to:
- Exclude stale co-located Dags when the requested Dag is active.
- Retain stale co-located Dags when the requested Dag is stale, since its stored source may still contain them.
- Always include the requested Dag ID so the authorization check cannot be skipped.
Make the existing co-located test fixture explicitly active and add a parametrized regression test for both requested-Dag states. This follows the security boundary raised during review of #73292.
### Tests
- RED: The original implementation redacts an active Dag's source when an unreadable stale sibling exists.
- RED: An intermediate fix exposes a stale requested Dag's source when an unreadable stale sibling exists.
- GREEN: Both regression cases and all 15 tests in `test_dag_sources.py` pass.
- Ruff 0.16.8 format and lint checks, `git diff --check`, and applicable manual prek hooks pass.
- Tests run locally with Python 3.14.4, SQLite, and uv, without Docker.
- Real FAB role-based authorization integration is not tested. The `generate-openapi-spec` and `mypy-airflow-core` prek hooks cannot complete locally because Docker and the C compiler/Kerberos headers are unavailable. GitHub CI is pending.
Fixes #73284
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (OpenAI Codex, GPT-6)
Generated-by: Codex (GPT-6) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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] Exclude stale Dags from active Dag source authorization [airflow]
Posted by "TinyGeek (via GitHub)" <gi...@apache.org>.
TinyGeek commented on code in PR #74576:
URL: https://github.com/apache/airflow/pull/74576#discussion_r4306781269
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dag_sources.py:
##########
@@ -82,21 +82,25 @@ def get_dag_source(
# ``(relative_fileloc, bundle_name)`` -- the same keying
# ``import_error.py`` uses for its equivalent check -- and redact the
# response when any co-located Dag is not in the caller's readable set.
+ # A stale Dag's stored source may still define stale siblings, so keep
+ # those in the check. Always include the requested Dag to avoid skipping
+ # authorization when no other Dags remain.
content = dag_version.dag_code.source_code
dag_model = dag_version.dag_model
if dag_model is not None and dag_model.relative_fileloc:
- file_dag_ids = set(
+ file_dag_ids = {dag_id}
+ file_dag_ids.update(
session.scalars(
select(DagModel.dag_id).where(
DagModel.relative_fileloc == dag_model.relative_fileloc,
DagModel.bundle_name == dag_model.bundle_name,
+ DagModel.is_stale.in_([False, dag_model.is_stale]),
Review Comment:
Thanks for catching this. The follow-up compares the selected `DagVersion` with the latest one. When the selected version is older, stale co-located Dags stay in the file authorization check because the stored source may still define them. An omitted version or an explicitly selected latest version keeps the active Dag behavior: stale siblings are excluded when the requested Dag is active. The requested Dag ID remains in the authorization set in every case.
`DagCode` has no immutable per-version list of co-located Dag IDs, so this change restores the conservative check for the historical case without claiming to reconstruct every historical file membership.
---
Drafted-by: Codex (GPT-6) (no human review before posting)
--
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] Exclude stale Dags from active Dag source authorization [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #74576:
URL: https://github.com/apache/airflow/pull/74576#issuecomment-6120369157
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our [Contributors' Guide](https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:
- Pay attention to the quality of your code (ruff, mypy and type annotations). Our [prek-hooks]( https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-prek-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/airflow-core/docs/howto/custom-operator.rst) Consider adding an example Dag that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/dev/breeze/doc/README.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#coding-style-and-best-practices).
- Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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] Exclude stale Dags from active Dag source authorization [airflow]
Posted by "TinyGeek (via GitHub)" <gi...@apache.org>.
TinyGeek commented on code in PR #74576:
URL: https://github.com/apache/airflow/pull/74576#discussion_r4306781269
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dag_sources.py:
##########
@@ -82,21 +82,25 @@ def get_dag_source(
# ``(relative_fileloc, bundle_name)`` -- the same keying
# ``import_error.py`` uses for its equivalent check -- and redact the
# response when any co-located Dag is not in the caller's readable set.
+ # A stale Dag's stored source may still define stale siblings, so keep
+ # those in the check. Always include the requested Dag to avoid skipping
+ # authorization when no other Dags remain.
content = dag_version.dag_code.source_code
dag_model = dag_version.dag_model
if dag_model is not None and dag_model.relative_fileloc:
- file_dag_ids = set(
+ file_dag_ids = {dag_id}
+ file_dag_ids.update(
session.scalars(
select(DagModel.dag_id).where(
DagModel.relative_fileloc == dag_model.relative_fileloc,
DagModel.bundle_name == dag_model.bundle_name,
+ DagModel.is_stale.in_([False, dag_model.is_stale]),
Review Comment:
Thanks for catching this. I reproduced the regression with two stored source versions: version 1 contains the now-stale sibling, while version 2 does not. Before the fix, `?version_number=1` returned the full version 1 source to a caller allowed to read only the requested Dag.
The follow-up compares the selected `DagVersion` with the latest one. For an older selected version, stale co-located Dags remain in the authorization check. An omitted version or an explicitly selected latest version still excludes stale siblings when the requested Dag is active. The regression test failed before this change and passes afterward; all 16 tests in `test_dag_sources.py` pass locally.
This restores the conservative check for the reported historical case. `DagCode` does not store an immutable per-version list of co-located Dag IDs, so this does not claim to reconstruct every historical file membership.
---
Drafted-by: Codex (GPT-6) (no human review before posting)
--
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] Exclude stale Dags from active Dag source authorization [airflow]
Posted by "Eason09053360 (via GitHub)" <gi...@apache.org>.
Eason09053360 commented on code in PR #74576:
URL: https://github.com/apache/airflow/pull/74576#discussion_r4306483252
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dag_sources.py:
##########
@@ -82,21 +82,25 @@ def get_dag_source(
# ``(relative_fileloc, bundle_name)`` -- the same keying
# ``import_error.py`` uses for its equivalent check -- and redact the
# response when any co-located Dag is not in the caller's readable set.
+ # A stale Dag's stored source may still define stale siblings, so keep
+ # those in the check. Always include the requested Dag to avoid skipping
+ # authorization when no other Dags remain.
content = dag_version.dag_code.source_code
dag_model = dag_version.dag_model
if dag_model is not None and dag_model.relative_fileloc:
- file_dag_ids = set(
+ file_dag_ids = {dag_id}
+ file_dag_ids.update(
session.scalars(
select(DagModel.dag_id).where(
DagModel.relative_fileloc == dag_model.relative_fileloc,
DagModel.bundle_name == dag_model.bundle_name,
+ DagModel.is_stale.in_([False, dag_model.is_stale]),
Review Comment:
Should stale siblings still count for older versions (?version_number=)? An older version's source can still define the now-stale sibling, which main redacts but this now returns in full.
--
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