You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "1fanwang (via GitHub)" <gi...@apache.org> on 2026/05/04 01:27:24 UTC
[PR] Strip CR/LF from MySQL URL query values before forwarding to my.cnf [airflow]
1fanwang opened a new pull request, #66325:
URL: https://github.com/apache/airflow/pull/66325
## Why
`_build_mysql_cnf` (introduced in #65575) forwards an allowlist of SQLAlchemy URL query keys (`ssl_ca`, `ssl_cert`, `ssl_key`, `ssl_cipher`, `ssl_mode`, `charset`) into the `[client]` section of the temporary my.cnf written by `airflow db shell`. The values are written without sanitization.
SQLAlchemy's URL parser percent-decodes query values, so a `sql_alchemy_conn` containing `%0A` or `%0D` in any of those values would inject extra option lines into the `[client]` section. Example:
```
sql_alchemy_conn = mysql://root@h:3306/d?ssl_ca=/etc/ssl/ca.crt%0Amalicious=evil
```
renders as:
```
[client]
...
ssl-ca = /etc/ssl/ca.crt
malicious = evil
```
The password field already escapes via `_quote_mysql_password_for_cnf`; this is the asymmetric defense-in-depth gap on the rest.
## Fix
Strip CR/LF in a small `_strip_newlines` helper applied to each forwarded value. Adds a regression test exercising both `%0A` and `%0D` in `ssl_ca`.
## Threat model
`sql_alchemy_conn` is set by the airflow admin via `airflow.cfg` or env. An admin who can write a malicious URL already has process-level control. The case this defends against is templated/secret-managed configs where a `%0A` in an upstream value silently turns into an option-injection through the `db shell` CLI path. Same intent as the existing password escape.
## Tests
```
pytest airflow-core/tests/unit/cli/commands/test_db_command.py -v -k "build_mysql_cnf"
... 7 passed
```
--
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] Strip CR/LF from MySQL URL query values before forwarding to my.cnf [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #66325:
URL: https://github.com/apache/airflow/pull/66325
--
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] Strip CR/LF from MySQL URL query values before forwarding to my.cnf [airflow]
Posted by "1fanwang (via GitHub)" <gi...@apache.org>.
1fanwang commented on PR #66325:
URL: https://github.com/apache/airflow/pull/66325#issuecomment-4381238654
> @1fanwang A few things need addressing before review — see our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
>
> * ❌ **mypy (type checking)** — Failing: `CI image checks / Static checks` (`mypy-airflow-core` reports `airflow-core/src/airflow/cli/commands/db_command.py:290: Argument 1 to "_strip_newlines" has incompatible type "tuple[str, ...] | str"; expected "str"`). See [docs](https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst).
>
> No rush.
>
> _Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._
Thanks @potiuk for the pointer, lemme update
--
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] Strip CR/LF from MySQL URL query values before forwarding to my.cnf [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #66325:
URL: https://github.com/apache/airflow/pull/66325#issuecomment-4381157615
@1fanwang A few things need addressing before review — see our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
- :x: **mypy (type checking)** — Failing: `CI image checks / Static checks` (`mypy-airflow-core` reports `airflow-core/src/airflow/cli/commands/db_command.py:290: Argument 1 to "_strip_newlines" has incompatible type "tuple[str, ...] | str"; expected "str"`). See [docs](https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst).
No rush.
---
_Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._
--
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