You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@airflow.apache.org by "youming1970 (via GitHub)" <gi...@apache.org> on 2025/09/06 13:09:21 UTC

[PR] security: Improve input sanitization in example DAG for command injection prevention [airflow]

youming1970 opened a new pull request, #55323:
URL: https://github.com/apache/airflow/pull/55323

   ## Summary
   This PR improves input sanitization in the `example_trigger_target_dag.py` to prevent potential command injection vulnerabilities when using user-provided configuration values in bash commands.
   
   ## Problem
   The current implementation directly uses `dag_run.conf.get("message")` in the bash environment without proper sanitization, which could allow command injection if malicious input is provided through the DAG configuration.
   
   ## Solution
   - Added comprehensive input sanitization using Jinja2 filters to escape bash special characters
   - Added security-focused comments to educate users about the importance of input sanitization
   - Maintains backward compatibility while significantly improving security posture
   
   ## Security Impact
   - **Risk Level**: Medium (P1) - Command injection in example code could mislead users
   - **Scope**: Example DAG that demonstrates TriggerDagRunOperator usage
   - **Mitigation**: Proper escaping of `\`, `"`, `` ` ``, and `$` characters
   
   ## Changes Made
   1. Enhanced the `env` parameter in `BashOperator` with proper Jinja2 filtering
   2. Added educational security comments explaining the sanitization approach
   3. Maintained the original functionality while preventing injection attacks
   
   ## Testing
   - Verified the sanitization works with various input patterns
   - Confirmed backward compatibility with existing DAG configurations
   - Validated that the security fix doesn't break normal message passing
   
   ## Code Quality
   - Follows Apache Airflow coding standards
   - Includes comprehensive security documentation
   - Maintains readability while improving security
   
   ---
   *This security improvement was identified through systematic configuration security analysis.*


-- 
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] security: Improve input sanitization in example DAG for command injection prevention [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #55323:
URL: https://github.com/apache/airflow/pull/55323#issuecomment-3267448467

   Very nice!


-- 
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] security: Improve input sanitization in example DAG for command injection prevention [airflow]

Posted by "youming1970 (via GitHub)" <gi...@apache.org>.
youming1970 commented on PR #55323:
URL: https://github.com/apache/airflow/pull/55323#issuecomment-3287394946

   Hi @potiuk,
   
   Thanks for the approval! Locally I saw some dependency-related failures in static checks (missing docutils, yaml, astor). These look like CI environment issues rather than code changes. Also noticed a pre-commit warning about an unexpected key: minimum_prek_version — should this be minimum_pre_commit_version?
   
   Could you help re-trigger CI after fixing the environment or the config key? I'm happy to rebase or make any adjustments as needed. Thanks!


-- 
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] security: Improve input sanitization in example DAG for command injection prevention [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #55323:
URL: https://github.com/apache/airflow/pull/55323#issuecomment-3264127419

   Just make it pass. I don't care you are using AI to generate it as long as code is good.


-- 
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] security: Improve input sanitization in example DAG for command injection prevention [airflow]

Posted by "youming1970 (via GitHub)" <gi...@apache.org>.
youming1970 commented on PR #55323:
URL: https://github.com/apache/airflow/pull/55323#issuecomment-3264016881

   Hey @potiuk,  
   thanks for the quick feedback! 🙏  
   Totally agree that the hand-rolled escaping I pushed is over-engineered for an example DAG.  
   Using Python's std-lib `shlex.quote()` keeps the snippet short, readable and 100% safe from command-injection.
   
   I'll push a new revision that:
   
   1. moves the user input into Python first (no direct Jinja expansion),
   2. quotes it with `shlex.quote()`, and
   3. passes the already-quoted string to the BashOperator.
   
   That removes the need for custom Jinja filters or complicated comments.
   
   **Draft of the change**
   ```diff
   @@
   -from airflow.operators.bash import BashOperator
   +import shlex
   +from airflow.operators.bash import BashOperator
   @@
   -# (previous example had direct {{ dag_run.conf.get("message") }} usage)
   -target_task = BashOperator(
   -    task_id="print_the_message",
   -    bash_command='echo "{{ dag_run.conf.get("message") }}"',
   -    dag=dag,
   -)
   +def prepare_message(**context):
   +    """
   +    Pull `message` from dag_run.conf and shell-quote it so it can be safely
   +    embedded in a Bash command.  Using the std-lib shlex.quote() protects
   +    against command-injection without any custom logic.
   +    """
   +    raw_msg = context["dag_run"].conf.get("message", "")
   +    return shlex.quote(str(raw_msg))
   +
   +prepare_msg = PythonOperator(
   +    task_id="prepare_message",
   +    python_callable=prepare_message,
   +)
   +
   +target_task = BashOperator(
   +    task_id="print_the_message",
   +    bash_command="echo {{ ti.xcom_pull(task_ids='prepare_message') }}",
   +)
   +
   +prepare_msg >> target_task
   ```
   
   **Benefits**
   • No manual escaping logic in Jinja.  
   • Example stays concise while demonstrating a best-practice pattern (move input to Python ➜ validate/quote ➜ hand to Bash).  
   • Backward compatibility: nothing changes for end-users triggering the DAG; they still supply `{"message": "hello"}`.
   
   Let me know if this direction looks good—if so I'll commit the refactor and update the PR description accordingly. 👍


-- 
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] security: Improve input sanitization in example DAG for command injection prevention [airflow]

Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #55323:
URL: https://github.com/apache/airflow/pull/55323#issuecomment-3262124483

   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] security: Improve input sanitization in example DAG for command injection prevention [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #55323:
URL: https://github.com/apache/airflow/pull/55323#issuecomment-3287602535

   Rebased it on topf of latest main (hint: in the future you can just do it yourself)


-- 
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] security: Improve input sanitization in example DAG for command injection prevention [airflow]

Posted by "youming1970 (via GitHub)" <gi...@apache.org>.
youming1970 commented on PR #55323:
URL: https://github.com/apache/airflow/pull/55323#issuecomment-3265593343

   Hi @potiuk,
   
   Thanks for the feedback. I've reworked example_trigger_target_dag per your suggestion.
   
   Key changes:
   - Use shlex.quote() at the Python layer (no manual Jinja escaping).
   - Pass the value via a small TaskFlow task and XCom; drop env-based expansion to avoid double expansion.
   - Use printf for predictable shell output (avoid echo quirks).
   - This also removes the Jinja/ruff quoting churn.
   
   Could you please take another look when you have a moment? I'm happy to adjust if you prefer a different shape or want me to add tests.
   
   Thanks again for the guidance!


-- 
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] security: Improve input sanitization in example DAG for command injection prevention [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #55323:
URL: https://github.com/apache/airflow/pull/55323#issuecomment-3287606883

   Generally- it would be great if you add test cases


-- 
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] security: Improve input sanitization in example DAG for command injection prevention [airflow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #55323:
URL: https://github.com/apache/airflow/pull/55323#issuecomment-3459152911

   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your 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] security: Improve input sanitization in example DAG for command injection prevention [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #55323:
URL: https://github.com/apache/airflow/pull/55323#issuecomment-3268064162

   There are some static check failures there, Did you use `prek` to run the static checks? 


-- 
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] security: Improve input sanitization in example DAG for command injection prevention [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #55323:
URL: https://github.com/apache/airflow/pull/55323#issuecomment-3287607272

   But since it's example dag - it's not needed really


-- 
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] security: Improve input sanitization in example DAG for command injection prevention [airflow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #55323: security: Improve input sanitization in example DAG for command injection prevention
URL: https://github.com/apache/airflow/pull/55323


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