You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by ka...@apache.org on 2021/03/19 22:02:28 UTC
[airflow] branch v2-0-test updated (0097f91 -> 9854c43)
This is an automated email from the ASF dual-hosted git repository.
kaxilnaik pushed a change to branch v2-0-test
in repository https://gitbox.apache.org/repos/asf/airflow.git.
discard 0097f91 Fix error when running tasks with Sentry integration enabled. (#13929)
discard 831c7ec Webserver: Sanitize string passed to origin param (#14738)
discard 9f33dfb Fix tests for all urllib versions with only '&' as separator (#14710)
new 1592806 Separate out tests to cater of changes in Python 3.8.8 (#14698)
new 44b65e7 Fix tests for all urllib versions with only '&' as separator (#14710)
new b22fa04 Fix tests in tests/www/test_views.py (#14719)
new 9854c43 Webserver: Sanitize string passed to origin param (#14738)
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (0097f91)
\
N -- N -- N refs/heads/v2-0-test (9854c43)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
airflow/sentry.py | 13 +++---------
airflow/utils/session.py | 21 +++++++-----------
tests/utils/test_session.py | 52 ---------------------------------------------
3 files changed, 11 insertions(+), 75 deletions(-)
delete mode 100644 tests/utils/test_session.py
[airflow] 02/04: Fix tests for all urllib versions with only '&' as
separator (#14710)
Posted by ka...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
kaxilnaik pushed a commit to branch v2-0-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 44b65e734f71632fa1158faeb78f4893e6a05f5a
Author: Kaxil Naik <ka...@gmail.com>
AuthorDate: Thu Mar 11 01:39:28 2021 +0000
Fix tests for all urllib versions with only '&' as separator (#14710)
Turns out #14698 did not fix the issue as Master failed again. After
digging a bit more I found that the CVE was fixed in all
Python versions: 3.6.13, 3.7.10 & 3.8.8
The solution in this PR/commit checks the `parse_qsl` behavior with
following tests:
```
❯ docker run -it python:3.8-slim bash
root@41120dfd035e:/# python
Python 3.8.8 (default, Feb 19 2021, 18:07:06)
>>> from urllib.parse import parse_qsl
>>> parse_qsl(";a=b")
[(';a', 'b')]
>>>
```
```
❯ docker run -it python:3.8.7-slim bash
root@68e527725610:/# python
Python 3.8.7 (default, Feb 9 2021, 08:21:15)
>>> from urllib.parse import parse_qsl
>>> parse_qsl(";a=b")
[('a', 'b')]
>>>
```
(cherry picked from commit 7bd9d477dd7c59b8efb7183050de58bcfd6fdd43)
---
tests/www/test_views.py | 88 ++++++++++++-------------------------------------
1 file changed, 21 insertions(+), 67 deletions(-)
diff --git a/tests/www/test_views.py b/tests/www/test_views.py
index 0d50144..ce4478c 100644
--- a/tests/www/test_views.py
+++ b/tests/www/test_views.py
@@ -32,7 +32,7 @@ from datetime import datetime as dt, timedelta
from typing import Any, Dict, Generator, List, NamedTuple
from unittest import mock
from unittest.mock import PropertyMock
-from urllib.parse import quote_plus
+from urllib.parse import parse_qsl, quote_plus
import jinja2
import pytest
@@ -2757,46 +2757,22 @@ class TestTriggerDag(TestBase):
("http://google.com", "/home"),
(
"%2Ftree%3Fdag_id%3Dexample_bash_operator';alert(33)//",
- "/tree?dag_id=example_bash_operator%27&alert%2833%29%2F%2F=",
- ),
- ("%2Ftree%3Fdag_id%3Dexample_bash_operator", "/tree?dag_id=example_bash_operator"),
- ("%2Fgraph%3Fdag_id%3Dexample_bash_operator", "/graph?dag_id=example_bash_operator"),
- ]
- )
- @pytest.mark.skipif(
- sys.version_info < (3, 8, 8),
- reason='Vulnerability was fixed in Python 3.8.8 which changed the query string separator: bpo-42967',
- )
- def test_trigger_dag_form_origin_url_py_lte_387(self, test_origin, expected_origin):
- test_dag_id = "example_bash_operator"
-
- resp = self.client.get(f'trigger?dag_id={test_dag_id}&origin={test_origin}')
- self.check_content_in_response(
- '<button type="button" class="btn" onclick="location.href = \'{}\'; return false">'.format(
- expected_origin
- ),
- resp,
- )
-
- @parameterized.expand(
- [
- ("javascript:alert(1)", "/home"),
- ("http://google.com", "/home"),
- (
- "%2Ftree%3Fdag_id%3Dexample_bash_operator';alert(33)//",
"/tree?dag_id=example_bash_operator%27%3Balert%2833%29%2F%2F",
),
("%2Ftree%3Fdag_id%3Dexample_bash_operator", "/tree?dag_id=example_bash_operator"),
("%2Fgraph%3Fdag_id%3Dexample_bash_operator", "/graph?dag_id=example_bash_operator"),
]
)
- @pytest.mark.skipif(
- sys.version_info > (3, 8, 7),
- reason='Vulnerability was fixed in Python 3.8.8 which changed the query string separator: bpo-42967',
- )
- def test_trigger_dag_form_origin_url_py_gt_387(self, test_origin, expected_origin):
+ def test_trigger_dag_form_origin_url(self, test_origin, expected_origin):
test_dag_id = "example_bash_operator"
+ # https://github.com/python/cpython/pull/24297/files
+ # Check if tests are running with a Python version containing the above fix
+ # where ";" is removed as a separator
+ if parse_qsl(";a=b") != [(';a', 'b')]:
+ expected_url = expected_origin.replace("%3B", "&")
+ expected_url += "="
+
resp = self.client.get(f'trigger?dag_id={test_dag_id}&origin={test_origin}')
self.check_content_in_response(
'<button type="button" class="btn" onclick="location.href = \'{}\'; return false">'.format(
@@ -3329,31 +3305,6 @@ class TestHelperFunctions(TestBase):
(
"http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%test_dag';alert(33)//",
"http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3F"
- "dag_id%25test_dag%27&alert%2833%29%2F%2F=",
- ),
- (
- "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%test_dag",
- "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%25test_dag",
- ),
- ]
- )
- @pytest.mark.skipif(
- sys.version_info < (3, 8, 8),
- reason='Vulnerability was fixed in Python 3.8.8 which changed the query string separator: bpo-42967',
- )
- def test_get_safe_url_py_lte_387(self, test_url, expected_url):
- with mock.patch("airflow.www.views.url_for") as mock_url_for:
- mock_url_for.return_value = "/home"
- with self.app.test_request_context(base_url="http://localhost:8080"):
- assert get_safe_url(test_url) == expected_url
-
- @parameterized.expand(
- [
- ("", "/home"),
- ("http://google.com", "/home"),
- (
- "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%test_dag';alert(33)//",
- "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3F"
"dag_id%25test_dag%27%3Balert%2833%29%2F%2F",
),
(
@@ -3362,15 +3313,18 @@ class TestHelperFunctions(TestBase):
),
]
)
- @pytest.mark.skipif(
- sys.version_info > (3, 8, 7),
- reason='Vulnerability was fixed in Python 3.8.8 which changed the query string separator: bpo-42967',
- )
- def test_get_safe_url_py_gt_387(self, test_url, expected_url):
- with mock.patch("airflow.www.views.url_for") as mock_url_for:
- mock_url_for.return_value = "/home"
- with self.app.test_request_context(base_url="http://localhost:8080"):
- assert get_safe_url(test_url) == expected_url
+ @mock.patch("airflow.www.views.url_for")
+ def test_get_safe_url(self, test_url, expected_url, mock_url_for):
+ # https://github.com/python/cpython/pull/24297/files
+ # Check if tests are running with a Python version containing the above fix
+ # where ";" is removed as a separator
+ if parse_qsl(";a=b") != [(';a', 'b')]:
+ expected_url = expected_url.replace("%3B", "&")
+ expected_url += "="
+
+ mock_url_for.return_value = "/home"
+ with self.app.test_request_context(base_url="http://localhost:8080"):
+ assert get_safe_url(test_url) == expected_url
@parameterized.expand(
[
[airflow] 04/04: Webserver: Sanitize string passed to origin param
(#14738)
Posted by ka...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
kaxilnaik pushed a commit to branch v2-0-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 9854c432b5c88cbfb5e0c9e8462c624ba6eaab02
Author: Kaxil Naik <ka...@gmail.com>
AuthorDate: Fri Mar 12 09:48:59 2021 +0000
Webserver: Sanitize string passed to origin param (#14738)
Follow-up of #12459 & #10334
Since https://github.com/python/cpython/pull/24297/files (bpo-42967)
also removed ';' as query argument separator, we remove query arguments
with semicolons.
(cherry picked from commit 409c249121bd9c8902fc2ba551b21873ab41f953)
---
airflow/www/views.py | 12 +++++++++++-
tests/www/test_views.py | 27 +++++++++------------------
2 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/airflow/www/views.py b/airflow/www/views.py
index 4c272b5..f9f7f5c 100644
--- a/airflow/www/views.py
+++ b/airflow/www/views.py
@@ -129,8 +129,18 @@ def get_safe_url(url):
parsed = urlparse(url)
+ # If the url is relative & it contains semicolon, redirect it to homepage to avoid
+ # potential XSS. (Similar to https://github.com/python/cpython/pull/24297/files (bpo-42967))
+ if parsed.netloc == '' and parsed.scheme == '' and ';' in unquote(url):
+ return url_for('Airflow.index')
+
query = parse_qsl(parsed.query, keep_blank_values=True)
- url = parsed._replace(query=urlencode(query)).geturl()
+
+ # Remove all the query elements containing semicolon
+ # As part of https://github.com/python/cpython/pull/24297/files (bpo-42967)
+ # semicolon was already removed as a separator for query arguments by default
+ sanitized_query = [query_arg for query_arg in query if ';' not in query_arg[1]]
+ url = parsed._replace(query=urlencode(sanitized_query)).geturl()
if parsed.scheme in valid_schemes and parsed.netloc in valid_netlocs:
return url
diff --git a/tests/www/test_views.py b/tests/www/test_views.py
index c8ced89..f67c478 100644
--- a/tests/www/test_views.py
+++ b/tests/www/test_views.py
@@ -32,7 +32,7 @@ from datetime import datetime as dt, timedelta
from typing import Any, Dict, Generator, List, NamedTuple
from unittest import mock
from unittest.mock import PropertyMock
-from urllib.parse import parse_qsl, quote_plus
+from urllib.parse import quote_plus
import jinja2
import pytest
@@ -2755,9 +2755,10 @@ class TestTriggerDag(TestBase):
[
("javascript:alert(1)", "/home"),
("http://google.com", "/home"),
+ ("36539'%3balert(1)%2f%2f166", "/home"),
(
"%2Ftree%3Fdag_id%3Dexample_bash_operator';alert(33)//",
- "/tree?dag_id=example_bash_operator%27%3Balert%2833%29%2F%2F",
+ "/home",
),
("%2Ftree%3Fdag_id%3Dexample_bash_operator", "/tree?dag_id=example_bash_operator"),
("%2Fgraph%3Fdag_id%3Dexample_bash_operator", "/graph?dag_id=example_bash_operator"),
@@ -2766,13 +2767,6 @@ class TestTriggerDag(TestBase):
def test_trigger_dag_form_origin_url(self, test_origin, expected_origin):
test_dag_id = "example_bash_operator"
- # https://github.com/python/cpython/pull/24297/files
- # Check if tests are running with a Python version containing the above fix
- # where ";" is removed as a separator
- if parse_qsl(";a=b") != [(';a', 'b')] and ";" in test_origin:
- expected_origin = expected_origin.replace("%3B", "&")
- expected_origin += "="
-
resp = self.client.get(f'trigger?dag_id={test_dag_id}&origin={test_origin}')
self.check_content_in_response(
'<button type="button" class="btn" onclick="location.href = \'{}\'; return false">'.format(
@@ -3302,10 +3296,14 @@ class TestHelperFunctions(TestBase):
[
("", "/home"),
("http://google.com", "/home"),
+ ("36539'%3balert(1)%2f%2f166", "/home"),
+ (
+ "http://localhost:8080/trigger?dag_id=test&origin=36539%27%3balert(1)%2f%2f166&abc=2",
+ "http://localhost:8080/trigger?dag_id=test&abc=2",
+ ),
(
"http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%test_dag';alert(33)//",
- "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3F"
- "dag_id%25test_dag%27%3Balert%2833%29%2F%2F",
+ "http://localhost:8080/trigger?dag_id=test_dag",
),
(
"http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%test_dag",
@@ -3315,13 +3313,6 @@ class TestHelperFunctions(TestBase):
)
@mock.patch("airflow.www.views.url_for")
def test_get_safe_url(self, test_url, expected_url, mock_url_for):
- # https://github.com/python/cpython/pull/24297/files
- # Check if tests are running with a Python version containing the above fix
- # where ";" is removed as a separator
- if parse_qsl(";a=b") != [(';a', 'b')] and ";" in test_url:
- expected_url = expected_url.replace("%3B", "&")
- expected_url += "="
-
mock_url_for.return_value = "/home"
with self.app.test_request_context(base_url="http://localhost:8080"):
assert get_safe_url(test_url) == expected_url
[airflow] 03/04: Fix tests in tests/www/test_views.py (#14719)
Posted by ka...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
kaxilnaik pushed a commit to branch v2-0-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
commit b22fa04e6ba0b05857e4ce8c220ec18e567a8f47
Author: Kaxil Naik <ka...@gmail.com>
AuthorDate: Thu Mar 11 16:50:19 2021 +0000
Fix tests in tests/www/test_views.py (#14719)
One of tests fixed in (#14710) had an usused variable - `expected_url`,
copy/paste failure. This commit fixes it and adds a condition too to
only replace url if it contains a semi-colon
(cherry picked from commit 52604a3444e4473f5c20f56624624869ea50ef9b)
---
tests/www/test_views.py | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/www/test_views.py b/tests/www/test_views.py
index ce4478c..c8ced89 100644
--- a/tests/www/test_views.py
+++ b/tests/www/test_views.py
@@ -2769,9 +2769,9 @@ class TestTriggerDag(TestBase):
# https://github.com/python/cpython/pull/24297/files
# Check if tests are running with a Python version containing the above fix
# where ";" is removed as a separator
- if parse_qsl(";a=b") != [(';a', 'b')]:
- expected_url = expected_origin.replace("%3B", "&")
- expected_url += "="
+ if parse_qsl(";a=b") != [(';a', 'b')] and ";" in test_origin:
+ expected_origin = expected_origin.replace("%3B", "&")
+ expected_origin += "="
resp = self.client.get(f'trigger?dag_id={test_dag_id}&origin={test_origin}')
self.check_content_in_response(
@@ -3318,7 +3318,7 @@ class TestHelperFunctions(TestBase):
# https://github.com/python/cpython/pull/24297/files
# Check if tests are running with a Python version containing the above fix
# where ";" is removed as a separator
- if parse_qsl(";a=b") != [(';a', 'b')]:
+ if parse_qsl(";a=b") != [(';a', 'b')] and ";" in test_url:
expected_url = expected_url.replace("%3B", "&")
expected_url += "="
[airflow] 01/04: Separate out tests to cater of changes in Python
3.8.8 (#14698)
Posted by ka...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
kaxilnaik pushed a commit to branch v2-0-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 159280656dfd5dcd81e97dade2c5fa5a88a20727
Author: Kaxil Naik <ka...@gmail.com>
AuthorDate: Wed Mar 10 23:15:39 2021 +0000
Separate out tests to cater of changes in Python 3.8.8 (#14698)
https://github.com/python/cpython/pull/24297 change was included in
Python 3.8.8 to fix a vulnerability (bpo-42967)
Depending on which Base Python Image is run in our CI, two of the tests
can fail or succeed.
Our Previous two attempts:
- https://github.com/apache/airflow/commit/061cd236deb22567e4de36af11025f028d787989#
- https://github.com/apache/airflow/commit/49952e79b04da932242ebf3981883e591b467994
We might for a while get different base python version depending on the changes of a PR (whether or not it includes a change to dockerfiler).
a) when you have PR which do not have changes in the Dockerfile, they will use the older python version as base (for example Python 3.8.7)
b) when you have PR that touches the Dockerfile and have setup.py changes in master, it should pull Python 3.8.8 first.
(cherry picked from commit ffe3bd29574d62a0a692cd8f63995856bbff8c0b)
---
tests/www/test_views.py | 72 ++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 66 insertions(+), 6 deletions(-)
diff --git a/tests/www/test_views.py b/tests/www/test_views.py
index d284314..0d50144 100644
--- a/tests/www/test_views.py
+++ b/tests/www/test_views.py
@@ -2763,7 +2763,38 @@ class TestTriggerDag(TestBase):
("%2Fgraph%3Fdag_id%3Dexample_bash_operator", "/graph?dag_id=example_bash_operator"),
]
)
- def test_trigger_dag_form_origin_url(self, test_origin, expected_origin):
+ @pytest.mark.skipif(
+ sys.version_info < (3, 8, 8),
+ reason='Vulnerability was fixed in Python 3.8.8 which changed the query string separator: bpo-42967',
+ )
+ def test_trigger_dag_form_origin_url_py_lte_387(self, test_origin, expected_origin):
+ test_dag_id = "example_bash_operator"
+
+ resp = self.client.get(f'trigger?dag_id={test_dag_id}&origin={test_origin}')
+ self.check_content_in_response(
+ '<button type="button" class="btn" onclick="location.href = \'{}\'; return false">'.format(
+ expected_origin
+ ),
+ resp,
+ )
+
+ @parameterized.expand(
+ [
+ ("javascript:alert(1)", "/home"),
+ ("http://google.com", "/home"),
+ (
+ "%2Ftree%3Fdag_id%3Dexample_bash_operator';alert(33)//",
+ "/tree?dag_id=example_bash_operator%27%3Balert%2833%29%2F%2F",
+ ),
+ ("%2Ftree%3Fdag_id%3Dexample_bash_operator", "/tree?dag_id=example_bash_operator"),
+ ("%2Fgraph%3Fdag_id%3Dexample_bash_operator", "/graph?dag_id=example_bash_operator"),
+ ]
+ )
+ @pytest.mark.skipif(
+ sys.version_info > (3, 8, 7),
+ reason='Vulnerability was fixed in Python 3.8.8 which changed the query string separator: bpo-42967',
+ )
+ def test_trigger_dag_form_origin_url_py_gt_387(self, test_origin, expected_origin):
test_dag_id = "example_bash_operator"
resp = self.client.get(f'trigger?dag_id={test_dag_id}&origin={test_origin}')
@@ -3306,11 +3337,40 @@ class TestHelperFunctions(TestBase):
),
]
)
- @mock.patch("airflow.www.views.url_for")
- def test_get_safe_url(self, test_url, expected_url, mock_url_for):
- mock_url_for.return_value = "/home"
- with self.app.test_request_context(base_url="http://localhost:8080"):
- assert get_safe_url(test_url) == expected_url
+ @pytest.mark.skipif(
+ sys.version_info < (3, 8, 8),
+ reason='Vulnerability was fixed in Python 3.8.8 which changed the query string separator: bpo-42967',
+ )
+ def test_get_safe_url_py_lte_387(self, test_url, expected_url):
+ with mock.patch("airflow.www.views.url_for") as mock_url_for:
+ mock_url_for.return_value = "/home"
+ with self.app.test_request_context(base_url="http://localhost:8080"):
+ assert get_safe_url(test_url) == expected_url
+
+ @parameterized.expand(
+ [
+ ("", "/home"),
+ ("http://google.com", "/home"),
+ (
+ "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%test_dag';alert(33)//",
+ "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3F"
+ "dag_id%25test_dag%27%3Balert%2833%29%2F%2F",
+ ),
+ (
+ "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%test_dag",
+ "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%25test_dag",
+ ),
+ ]
+ )
+ @pytest.mark.skipif(
+ sys.version_info > (3, 8, 7),
+ reason='Vulnerability was fixed in Python 3.8.8 which changed the query string separator: bpo-42967',
+ )
+ def test_get_safe_url_py_gt_387(self, test_url, expected_url):
+ with mock.patch("airflow.www.views.url_for") as mock_url_for:
+ mock_url_for.return_value = "/home"
+ with self.app.test_request_context(base_url="http://localhost:8080"):
+ assert get_safe_url(test_url) == expected_url
@parameterized.expand(
[