Skip to content

fix: block path traversal via ".." in dag_id and run_id - #63296

Merged
potiuk merged 4 commits into
apache:mainfrom
YoannAbriel:fix/issue-63295
Mar 24, 2026
Merged

fix: block path traversal via ".." in dag_id and run_id#63296
potiuk merged 4 commits into
apache:mainfrom
YoannAbriel:fix/issue-63295

Conversation

@YoannAbriel

Copy link
Copy Markdown
Contributor

Problem

validate_key() and validate_run_id() both allow .. in their values. Since dag_id and run_id are used in log file paths (e.g. dag_id=.../run_id=.../), a crafted value containing .. could theoretically traverse outside the intended log directory.

Root Cause

KEY_REGEX (^[\w.-]+$) matches .. as valid consecutive dots. validate_run_id uses a configurable allowed_run_id_pattern that could also permit ... Neither function checks for path traversal sequences.

Fix

Added an explicit .. check in both validate_key() (raises AirflowException) and validate_run_id() (raises ValueError) before any other validation. Added corresponding unit tests for both functions.

Closes: #63295


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

Generated-by: Claude Code following the guidelines


  • 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.

@potiukpotiuk 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.

This is extremely risky change if we do not have a mechanism to allow those.

If we merge thius change, suddenly all the Dags of someone who has .. in their name will fail and they will have to manually convert their Dags. People migh have weird conventions for names and

@potiuk

Copy link
Copy Markdown
Member

cc: @ferruzzi -> did you think how to handle the case if someone already uses .. ? I do not think this "Weak" possible issue is worth the risk ? WDYT?

@ferruzzi

Copy link
Copy Markdown
Contributor

I'm not honestly sure how big of a risk it is, in the end, but it did seem like a reasonable concern. IMHO, we can have a flag to allow it, set to false. That way we're "safe by default" and the user has to explicitly allow it? It's still breaking, but with some allowance??

@YoannAbriel

Copy link
Copy Markdown
ContributorAuthor

Good point — will add a config flag (default: block) so existing users with .. in IDs aren't broken. Will update the PR.

@ferruzzi

Copy link
Copy Markdown
Contributor

Since this is a breaking change, it will also need a newsfragment

@YoannAbriel
YoannAbrielforce-pushed the fix/issue-63295 branch 6 times, most recently from 4897b94 to 1e0378dCompareMarch 16, 2026 16:08

@ferruzziferruzzi 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.

Still pending the config flag to enable/disable the feature, but nice progress. Ping me when you are ready for a re-review.

Comment threadairflow-core/newsfragments/63296.significant.rst
@YoannAbriel
YoannAbrielforce-pushed the fix/issue-63295 branch 4 times, most recently from 507874d to 1c42375CompareMarch 23, 2026 13:09
Comment threadairflow-core/src/airflow/config_templates/config.yml Outdated
Comment threadairflow-core/src/airflow/config_templates/config.yml Outdated
validate_key() and validate_run_id() allow ".." which can be used
for path traversal when these values end up in log file paths.
Reject any key or run_id containing ".." early in validation.
Closes: apache#63295
… in create_dagrun
- Add [core] allow_dotdot_in_ids config flag (default: False) so existing
users with '..' in their IDs can opt out of the blocking behavior
- Add '..' check in SerializedDAG.create_dagrun() before the regex check,
fixing the test_dag_run_id_rejects_path_traversal failure where
create_dagrun raised a regex mismatch error before the traversal check
- Collapse multi-line raise ValueError in dagrun.py to fix static check
- Make all '..' checks conditional on allow_dotdot_in_ids config

@ferruzziferruzzi 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.

I wonder if we might want to make and import a constant like DOUBLE_DOT - ".." rather than littering magic strings around, but I'm fine with it as-is... that may be overkill.

@ferruzzi
ferruzzi requested a review from potiukMarch 23, 2026 21:42
@potiuk

Copy link
Copy Markdown
Member

I wonder if we might want to make and import a constant like DOUBLE_DOT - ".." rather than littering magic strings around, but I'm fine with it as-is... that may be overkill.

Yeah.. It's longer than ".." and DOUBLE_DOT constant is unlikely to change to something else (like "..." to justify DRY).

@potiuk
potiuk merged commit 4f2493d into apache:mainMar 24, 2026
146 checks passed
@YoannAbriel
YoannAbriel deleted the fix/issue-63295 branch March 24, 2026 07:05
vatsrahul1001 pushed a commit that referenced this pull request Mar 25, 2026
* fix: block path traversal via ".." in dag_id and run_id
validate_key() and validate_run_id() allow ".." which can be used
for path traversal when these values end up in log file paths.
Reject any key or run_id containing ".." early in validation.
Closes: #63295
* Add newsfragment for breaking change
* fix: add config flag allow_dotdot_in_ids and fix path traversal check in create_dagrun
- Add [core] allow_dotdot_in_ids config flag (default: False) so existing
users with '..' in their IDs can opt out of the blocking behavior
- Add '..' check in SerializedDAG.create_dagrun() before the regex check,
fixing the test_dag_run_id_rejects_path_traversal failure where
create_dagrun raised a regex mismatch error before the traversal check
- Collapse multi-line raise ValueError in dagrun.py to fix static check
- Make all '..' checks conditional on allow_dotdot_in_ids config
* refactor: rename allow_dotdot_in_ids to allow_double_dot_in_ids, bump version_added to 3.3.0
aaron-y-chen pushed a commit to aaron-y-chen/airflow that referenced this pull request Mar 30, 2026
* fix: block path traversal via ".." in dag_id and run_id
validate_key() and validate_run_id() allow ".." which can be used
for path traversal when these values end up in log file paths.
Reject any key or run_id containing ".." early in validation.
Closes: apache#63295
* Add newsfragment for breaking change
* fix: add config flag allow_dotdot_in_ids and fix path traversal check in create_dagrun
- Add [core] allow_dotdot_in_ids config flag (default: False) so existing
users with '..' in their IDs can opt out of the blocking behavior
- Add '..' check in SerializedDAG.create_dagrun() before the regex check,
fixing the test_dag_run_id_rejects_path_traversal failure where
create_dagrun raised a regex mismatch error before the traversal check
- Collapse multi-line raise ValueError in dagrun.py to fix static check
- Make all '..' checks conditional on allow_dotdot_in_ids config
* refactor: rename allow_dotdot_in_ids to allow_double_dot_in_ids, bump version_added to 3.3.0
Suraj-kumar00 pushed a commit to Suraj-kumar00/airflow that referenced this pull request Apr 7, 2026
* fix: block path traversal via ".." in dag_id and run_id
validate_key() and validate_run_id() allow ".." which can be used
for path traversal when these values end up in log file paths.
Reject any key or run_id containing ".." early in validation.
Closes: apache#63295
* Add newsfragment for breaking change
* fix: add config flag allow_dotdot_in_ids and fix path traversal check in create_dagrun
- Add [core] allow_dotdot_in_ids config flag (default: False) so existing
users with '..' in their IDs can opt out of the blocking behavior
- Add '..' check in SerializedDAG.create_dagrun() before the regex check,
fixing the test_dag_run_id_rejects_path_traversal failure where
create_dagrun raised a regex mismatch error before the traversal check
- Collapse multi-line raise ValueError in dagrun.py to fix static check
- Make all '..' checks conditional on allow_dotdot_in_ids config
* refactor: rename allow_dotdot_in_ids to allow_double_dot_in_ids, bump version_added to 3.3.0
abhijeets25012-tech pushed a commit to abhijeets25012-tech/airflow that referenced this pull request Apr 9, 2026
* fix: block path traversal via ".." in dag_id and run_id
validate_key() and validate_run_id() allow ".." which can be used
for path traversal when these values end up in log file paths.
Reject any key or run_id containing ".." early in validation.
Closes: apache#63295
* Add newsfragment for breaking change
* fix: add config flag allow_dotdot_in_ids and fix path traversal check in create_dagrun
- Add [core] allow_dotdot_in_ids config flag (default: False) so existing
users with '..' in their IDs can opt out of the blocking behavior
- Add '..' check in SerializedDAG.create_dagrun() before the regex check,
fixing the test_dag_run_id_rejects_path_traversal failure where
create_dagrun raised a regex mismatch error before the traversal check
- Collapse multi-line raise ValueError in dagrun.py to fix static check
- Make all '..' checks conditional on allow_dotdot_in_ids config
* refactor: rename allow_dotdot_in_ids to allow_double_dot_in_ids, bump version_added to 3.3.0
@vatsrahul1001vatsrahul1001 added this to the Airflow 3.3.0 milestone Jun 15, 2026
Andrushika added a commit to Andrushika/airflow that referenced this pull request Jul 28, 2026
apache#63296 added a check that blocks `..` in IDs to prevent path traversal in
airflow.utils.helpers.validate_key, gated by [core] allow_double_dot_in_ids.
Dag and task authoring now use the Task SDK validate_key, which never got that
check, so IDs like a..b are accepted even when the flag is off. Add the same
check to the Task SDK validator so the two validators agree.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DAG names should be filtered a little stronger

4 participants

@YoannAbriel@potiuk@ferruzzi@vatsrahul1001