Skip to content

Fix zipped DAG template loading with safe path handling#60999

Open
soooojinlee wants to merge 19 commits intoapache:mainfrom
soooojinlee:fix/59310-zipped-dag-template-path
Open

Fix zipped DAG template loading with safe path handling#60999
soooojinlee wants to merge 19 commits intoapache:mainfrom
soooojinlee:fix/59310-zipped-dag-template-path

Conversation

@soooojinlee
Copy link

@soooojinlee soooojinlee commented Jan 24, 2026

Switch DAG templating to a zip-aware loader that preserves searchpath order and applies the same path traversal rules as FileSystemLoader. Add tests for mixed paths, ordering, and zip traversal blocking.

Summary

  • Add a zip-aware Jinja2 loader to resolve templates from zipped DAGs.
  • Resolve relative template_searchpath against DAG folder for zipped DAGs (e.g., ["templates"] → "{zip}/templates")
  • Preserve searchpath ordering and apply FileSystemLoader-equivalent path validation to zip paths.
  • Consolidate zipped DAG template tests into task-sdk DAG tests (mixed paths, ordering, traversal blocking).

Test plan

  • uv run pytest task-sdk/tests/task_sdk/definitions/test_dag.py -v --tb=short
  • prek --all-files (local failures: Docker not running for OpenAPI spec hooks; FAB assets yarn install failed with 502)
  • Manual testing with local Airflow instance (6 edge cases for zipped DAGs - all passed)

Docs

  • N/A

Breaking changes

  • N/A

Related


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)
    Generated-by: Claude Opus 4.5

  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborg
Copy link

boring-cyborg bot commented Jan 24, 2026

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 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 Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment 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 for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • 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

@soooojinlee soooojinlee force-pushed the fix/59310-zipped-dag-template-path branch from 872e812 to e8e3fba Compare January 24, 2026 10:48
Copy link
Contributor

@SameerMesiah97 SameerMesiah97 left a comment

Choose a reason for hiding this comment

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

I have left a few comments.

Copy link
Contributor

@kalluripradeep kalluripradeep left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this issue! The zip-aware loader is a good solution to #59310.

I agree with @SameerMesiah97's feedback:

  1. Removing the operator-level override check - This does seem like it could be a breaking change. Could you clarify why this was removed or restore it if it was unintentional?

  2. Docstring length - The suggested shorter version would be clearer for IDE tooltips while keeping the details in comments.

Additional observations:

The test coverage looks comprehensive with mixed paths, ordering, and traversal blocking. Good work on the security aspect (path traversal prevention).

One question: Have you tested this with real-world zipped DAGs to ensure backward compatibility with existing deployments?

Looking forward to the updates! 👍

soooojinlee added a commit to soooojinlee/airflow that referenced this pull request Jan 27, 2026
- Restore _should_render_native method that was unintentionally removed
- Update _render to use _should_render_native for operator-level
  render_template_as_native_obj override support
- Simplify ZipAwareFileSystemLoader docstring per reviewer suggestion
- Move implementation details and issue reference to code comments
- Use Python naming convention 'up_to_date' instead of 'uptodate'
@soooojinlee soooojinlee force-pushed the fix/59310-zipped-dag-template-path branch from c3cfe34 to 06e5ec8 Compare January 27, 2026 03:57
@soooojinlee
Copy link
Author

Verified the fix by running a local Airflow instance with zipped DAGs.

  test_edge_cases.zip/                                                              
  ├── test_edge_cases.py          # DAG definition file (6 DAGs)                    
  ├── root_template.sh            # Template at ZIP root level                      
  ├── templates/                                                                    
  │   ├── run.sh                  # Basic template                                  
  │   ├── query.sql                                                                 
  │   └── nested/                                                                   
  │       └── deep.sh             # Nested directory template                       
  └── sql/                                                                          
      └── query.sh                # Separate directory template    

Test Cases (All Passed ✅)
DAG ID: edge_case_1_relative_searchpath
Configuration: template_searchpath=["templates"]
Test Purpose: Verify relative path resolves to {zip}/templates
────────────────────────────────────────
DAG ID: edge_case_2_multiple_searchpaths
Configuration: template_searchpath=["templates", "sql"]
Test Purpose: Verify multiple relative paths all resolve correctly
────────────────────────────────────────
DAG ID: edge_case_3_nested_searchpath
Configuration: template_searchpath=["templates/nested"]
Test Purpose: Verify nested relative path support
────────────────────────────────────────
DAG ID: edge_case_4_direct_reference
Configuration: bash_command="templates/run.sh"
Test Purpose: Direct path reference without template_searchpath
────────────────────────────────────────
DAG ID: edge_case_5_root_template
Configuration: bash_command="root_template.sh"
Test Purpose: Load template from ZIP root level
────────────────────────────────────────
DAG ID: edge_case_6_empty_searchpath
Configuration: template_searchpath=[]
Test Purpose: Empty searchpath uses DAG folder only

image

soooojinlee and others added 8 commits February 7, 2026 22:53
Switch DAG templating to a zip-aware loader that preserves searchpath order and applies the same path traversal rules as FileSystemLoader. Add tests for mixed paths, ordering, and zip traversal blocking.
- Restore _should_render_native method that was unintentionally removed
- Update _render to use _should_render_native for operator-level
  render_template_as_native_obj override support
- Simplify ZipAwareFileSystemLoader docstring per reviewer suggestion
- Move implementation details and issue reference to code comments
- Use Python naming convention 'up_to_date' instead of 'uptodate'
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@soooojinlee soooojinlee force-pushed the fix/59310-zipped-dag-template-path branch from 276d69c to dba2c2e Compare February 7, 2026 15:49
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@soooojinlee soooojinlee requested a review from jscheffl February 8, 2026 03:17
@eladkal eladkal added this to the Airflow 3.1.9 milestone Mar 4, 2026
@eladkal eladkal added the type:bug-fix Changelog: Bug Fixes label Mar 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:task-sdk type:bug-fix Changelog: Bug Fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Airflow can not open path in zipped DAG folder

6 participants