You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/11/17 10:35:36 UTC
[GitHub] [airflow] bolkedebruin opened a new pull request, #27740: Remove XCom API endpoint full deserialization option
bolkedebruin opened a new pull request, #27740:
URL: https://github.com/apache/airflow/pull/27740
It is possible to fully deserialize XCom values in the webserver by specifying a `deserialize` parameter. This creates a potential security issue as deserialization instantiates arbitrary objects.
Custom XCom backends should implement `orm_deserialize_value` themselves if they want to retain the behavior. However, it is strongly suggested to deserialize on the client side for the above mentioned reasons.
cc @uranusjr @ashb
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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
[GitHub] [airflow] github-actions[bot] closed pull request #27740: Remove XCom API endpoint full deserialization option
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #27740: Remove XCom API endpoint full deserialization option
URL: https://github.com/apache/airflow/pull/27740
--
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
[GitHub] [airflow] bolkedebruin commented on pull request #27740: Remove XCom API endpoint full deserialization option
Posted by GitBox <gi...@apache.org>.
bolkedebruin commented on PR #27740:
URL: https://github.com/apache/airflow/pull/27740#issuecomment-1318514538
You could DOS the webserver with a large pickle without #27540 which is also a security issue :-).
As I mentioned I think the proper way to deserialize over-the-wire is to do this on the client side. After all if using a transport like HTTP we are serializing again.
--
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
[GitHub] [airflow] uranusjr commented on pull request #27740: Remove XCom API endpoint full deserialization option
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #27740:
URL: https://github.com/apache/airflow/pull/27740#issuecomment-1326951283
Sounds to me the most reasonable approach here would be to add a config to allow this feature.
--
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
[GitHub] [airflow] ashb commented on pull request #27740: Remove XCom API endpoint full deserialization option
Posted by GitBox <gi...@apache.org>.
ashb commented on PR #27740:
URL: https://github.com/apache/airflow/pull/27740#issuecomment-1318516007
> As I mentioned I think the proper way to deserialize over-the-wire is to do this on the client side
That implies the client has the credentials to perform that, and doing it on the server almost seems like it was explicitly the reason the OP asked for this feature in https://github.com/apache/airflow/issues/26174
--
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
[GitHub] [airflow] bolkedebruin commented on pull request #27740: Remove XCom API endpoint full deserialization option
Posted by GitBox <gi...@apache.org>.
bolkedebruin commented on PR #27740:
URL: https://github.com/apache/airflow/pull/27740#issuecomment-1318537995
That's not neccessarily true. With JSON serialization all the information to deserialize is available to you. With ``deserialize`` as an option we outsource understanding if it is a risk to deserialize to the client. As the OP implemented a custom XCom backend they have the possibility to choose server side what to deserialize entirely by overriding `orm_deserialize_value`. I don't consider that a best practice though.
--
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
[GitHub] [airflow] github-actions[bot] commented on pull request #27740: Remove XCom API endpoint full deserialization option
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #27740:
URL: https://github.com/apache/airflow/pull/27740#issuecomment-1599761145
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
[GitHub] [airflow] ashb commented on pull request #27740: Remove XCom API endpoint full deserialization option
Posted by GitBox <gi...@apache.org>.
ashb commented on PR #27740:
URL: https://github.com/apache/airflow/pull/27740#issuecomment-1318508283
The changes we are introducing in https://github.com/apache/airflow/pull/27540 are what makes this behaviour a potential security risk now (though tbf this never "worked" well if you stored xcom as pickle as it wouldn't be able to send the "inflated" value over an HTTP transport anyway.
Trying to decide if we just remove that feature entirely, or if we need to keep it for non-pickle+plain JSON cases (which does sound complex, just talking out loud about options)
--
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
[GitHub] [airflow] github-actions[bot] commented on pull request #27740: Remove XCom API endpoint full deserialization option
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #27740:
URL: https://github.com/apache/airflow/pull/27740#issuecomment-1376526315
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
[GitHub] [airflow] github-actions[bot] commented on pull request #27740: Remove XCom API endpoint full deserialization option
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #27740:
URL: https://github.com/apache/airflow/pull/27740#issuecomment-1445512607
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