Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BigQueryCheckOperator does not support parameterized queries #40556

Closed
2 tasks done
aldenstpage opened this issue Jul 2, 2024 · 3 comments · Fixed by #40558
Closed
2 tasks done

BigQueryCheckOperator does not support parameterized queries #40556

aldenstpage opened this issue Jul 2, 2024 · 3 comments · Fixed by #40558
Labels
area:providers good first issue kind:feature Feature Requests provider:google Google (including GCP) related issues

Comments

@aldenstpage
Copy link
Contributor

aldenstpage commented Jul 2, 2024

Apache Airflow Provider(s)

google

Versions of Apache Airflow Providers

apache-airflow-providers-google==8.10.0

Apache Airflow version

2.7.3

Operating System

Debian 12

Deployment

Google Cloud Composer

Deployment details

No response

What happened

BigQueryCheckOperator doesn't support parameterized queries.

What you think should happen instead

BigQueryCheckOperator should support query parameters.

How to reproduce

This is a missing feature, so reproduction is not applicable.

Anything else

The various official BigQuery operators have inconsistent support for parameterized queries. Some, like BigQueryInsertJobOperator, have built-in support; others, like BigQueryCheckOperator, don't appear to support query parameters at all. The Google Cloud blog actually uses string formatting for passing parameters to demonstrate BigQueryCheckOperator.

SQL injection stubbornly remains in the OWASP top 10 vulnerabilities, as it has for decades. The way that we prevent SQL injection is to use query parameters.

Even though BigQueryCheckOperator is a read-only job that returns a boolean, meaning the potential attack surface is small to non-existent, it is less than ideal to have to use string formatting to introduce parameters for a number of reasons:

  1. I want to build good habits in my team. "Never use string formatting to introduce parameters" sets a better precedent than "never use string formatting to introduce parameters except in BigQueryCheckOperator". Creating exceptions to the rule is normalization of deviance. It opens the door to introducing remote-exploitable injection vulnerabilities in places where it actually matters.
  2. Security scanner tools will (correctly) flag these as potential SQL injection vulnerabilities. Scanners cannot have the context to understand that these are generally not exploitable.
  3. A user of the library shouldn't have to decide whether to use query parameters or not; they should just be consistently available.

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@aldenstpage aldenstpage added area:providers kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet labels Jul 2, 2024
Copy link

boring-cyborg bot commented Jul 2, 2024

Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.

@dosubot dosubot bot added kind:feature Feature Requests provider:google Google (including GCP) related issues labels Jul 2, 2024
@potiuk potiuk added good first issue and removed kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet labels Jul 2, 2024
@potiuk
Copy link
Member

potiuk commented Jul 2, 2024

Feel free to implement it @aldenstpage

@aldenstpage
Copy link
Contributor Author

Thanks, @potiuk. I've submitted a PR. #40558

potiuk pushed a commit that referenced this issue Jul 3, 2024
…40558)

* Add support for query parameters to BigQueryCheckOperator (#40556)

Remove unnecessary space

* Add a unit test for BigQueryCheckOperator query params; fix missing 'self' reference

* Fix lint (#40558)

---------

Co-authored-by: Alden S. Page <alden.page@doubleverify.com>
romsharon98 pushed a commit to romsharon98/airflow that referenced this issue Jul 26, 2024
…) (apache#40558)

* Add support for query parameters to BigQueryCheckOperator (apache#40556)

Remove unnecessary space

* Add a unit test for BigQueryCheckOperator query params; fix missing 'self' reference

* Fix lint (apache#40558)

---------

Co-authored-by: Alden S. Page <alden.page@doubleverify.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers good first issue kind:feature Feature Requests provider:google Google (including GCP) related issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants