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

Fix failing get_safe_url tests for latest Python 3.8 and 3.9 #31766

Merged
merged 1 commit into from Jun 7, 2023

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Jun 7, 2023

The latest release of Python 3.8 and 3.9 have been just released that contain the fix to a security vulnerability backported to those versions:

python/cpython#102153

Release notes:

The fix improved sanitizing of the URLs and until Python 3.10 and 3.11 get released, we need to add the sanitization ourselves to pass tests on all versions.

In order to improve security of airflow users and make the tests work regardless whether the users have latest Python versions released, we add extra sanitisation step to the URL to apply the standard WHATWG specification.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
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.

@boring-cyborg boring-cyborg bot added the area:webserver Webserver related Issues label Jun 7, 2023
@potiuk potiuk requested a review from jedcunningham June 7, 2023 14:08
@potiuk
Copy link
Member Author

potiuk commented Jun 7, 2023

Example failing build: https://github.com/apache/airflow/actions/runs/5199718582/jobs/9378018845

=================================== FAILURES ===================================
_ test_get_safe_url[ javascript:alert(1)-http://localhost:8080/ javascript:alert(1)] _

mock_url_for = <MagicMock name='url_for' id='139816089007056'>
app = <Flask 'airflow.www.app'>, test_url = ' javascript:alert(1)'
expected_url = 'http://localhost:8080/ javascript:alert(1)'

    @pytest.mark.parametrize(
        "test_url, expected_url",
        [
            ("", "/home"),
            ("javascript:alert(1)", "/home"),
            (" javascript:alert(1)", "http://localhost:8080/ javascript:alert(1)"),
            ("http://google.com/", "/home"),
            ("google.com", "http://localhost:8080/google.com"),
            ("\\/google.com", "http://localhost:8080/\\/google.com"),
            ("//google.com", "/home"),
            ("\\/\\/google.com", "http://localhost:8080/\\/\\/google.com"),
            ("36539'%3balert(1)%2f%2f166", "/home"),
            (
                "http://localhost:8080/trigger?dag_id=test&origin=36539%27%3balert(1)%2f%2f166&abc=2",
                "/home",
            ),
            (
                "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%test_dag';alert(33)//",
                "/home",
            ),
            (
                "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%3Dtest_dag",
                "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%3Dtest_dag",
            ),
        ],
    )
    @mock.patch("airflow.www.views.url_for")
    def test_get_safe_url(mock_url_for, app, test_url, expected_url):
        mock_url_for.return_value = "/home"
        with app.test_request_context(base_url="http://localhost:8080"):
>           assert get_safe_url(test_url) == expected_url
E           AssertionError: assert '/home' == 'http://local...ript:alert(1)/'
E             - http://localhost:8080/ javascript:alert(1)
E             + /home

The latest release of Python 3.8 and 3.9 have been just released
that contain the fix to a security vulnerability backported to
those versions:

python/cpython#102153

Release notes:
* https://www.python.org/downloads/release/python-3817/
* https://www.python.org/downloads/release/python-3917/

The fix improved sanitizing of the URLs and until Python 3.10 and
3.11 get released, we need to add the sanitization ourselves to
pass tests on all versions.

In order to improve security of airflow users and make the tests
work regardless whether the users have latest Python versions
released, we add extra sanitisation step to the URL to apply
the standard WHATWG specification.
@potiuk potiuk force-pushed the fix-failing-3-8-3-9-tests branch from d565771 to cb8b4da Compare June 7, 2023 14:19
@potiuk potiuk merged commit 87c5c9f into apache:main Jun 7, 2023
42 checks passed
@potiuk potiuk deleted the fix-failing-3-8-3-9-tests branch June 7, 2023 14:55
@eladkal eladkal added this to the Airflow 2.6.2 milestone Jun 8, 2023
@eladkal eladkal added the type:bug-fix Changelog: Bug Fixes label Jun 8, 2023
eladkal pushed a commit that referenced this pull request Jun 8, 2023
The latest release of Python 3.8 and 3.9 have been just released
that contain the fix to a security vulnerability backported to
those versions:

python/cpython#102153

Release notes:
* https://www.python.org/downloads/release/python-3817/
* https://www.python.org/downloads/release/python-3917/

The fix improved sanitizing of the URLs and until Python 3.10 and
3.11 get released, we need to add the sanitization ourselves to
pass tests on all versions.

In order to improve security of airflow users and make the tests
work regardless whether the users have latest Python versions
released, we add extra sanitisation step to the URL to apply
the standard WHATWG specification.

(cherry picked from commit 87c5c9f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:webserver Webserver related Issues type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants