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: CVE fixes, update werkzeug #1967

Open
wants to merge 1 commit into
base: v2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion connexion/apps/flask_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@

import flask
import werkzeug.exceptions
from flask import json, signals
from flask import signals
import json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding this import on this line causes the check for properly sorted imports to fail ("isort"). Still trying to figure it out tho.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok will move that around.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, this is the position that isort accepted for me:

diff --git a/connexion/apps/flask_app.py b/connexion/apps/flask_app.py
index 6a7e61d..ada9c57 100644
--- a/connexion/apps/flask_app.py
+++ b/connexion/apps/flask_app.py
@@ -3,6 +3,7 @@ This module defines a FlaskApp, a Connexion application to wrap a Flask applicat
 """
 
 import datetime
+import json
 import logging
 import pathlib
 from decimal import Decimal
@@ -11,7 +12,6 @@ from types import FunctionType  # NOQA
 import flask
 import werkzeug.exceptions
 from flask import signals
-import json
 
 from ..apis.flask_api import FlaskApi
 from ..exceptions import ProblemException


from ..apis.flask_api import FlaskApi
from ..exceptions import ProblemException
Expand Down
7 changes: 4 additions & 3 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def read_version(package):
'PyYAML>=5.1,<7',
'requests>=2.9.1,<3',
'inflection>=0.3.1,<0.6',
'werkzeug>=1.0,<2.3',
'werkzeug>=1.0,<4.0',
Copy link
Contributor

@chrisinmtown chrisinmtown Aug 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since werkzeug versions below 3.x have security vulnerabilities, I suggest:

'werkzeug>=3.0,<4.0',

That change brings the next challenge:

ERROR: Cannot install -r /Users/me/git/github/connexion_mfmarche/.tox/requirements-min.txt (line 12) and MarkupSafe==0.23 because these package versions have conflicting dependencies.

The conflict is caused by:
    The user requested MarkupSafe==0.23
    werkzeug 3.0.0 depends on MarkupSafe>=2.1.1

'importlib-metadata>=1 ; python_version<"3.8"',
'packaging>=20',
]
Expand All @@ -44,9 +44,10 @@ def read_version(package):

tests_require = [
'decorator>=5,<6',
'pytest>=6,<7',
'pytest>=6,<9',
'pytest-cov>=2,<3',
'testfixtures>=6,<7',
'pytest',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three lines above the pytest dependency is set to pytest>=6,<9, is this unpinned duplicate setting a typo?

*flask_require,
swagger_ui_require
]
Expand Down Expand Up @@ -100,7 +101,7 @@ def readme():
url='https://github.com/zalando/connexion',
keywords='openapi oai swagger rest api oauth flask microservice framework',
license='Apache License Version 2.0',
setup_requires=['flake8'],
setup_requires=['flake8', 'pytest-runner'],
python_requires=">=3.6",
install_requires=install_requires + flask_require,
tests_require=tests_require,
Expand Down
2 changes: 1 addition & 1 deletion tests/api/test_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ def test_get_unicode_request(simple_app):

def test_cookie_param(simple_app):
app_client = simple_app.app.test_client()
app_client.set_cookie("localhost", "test_cookie", "hello")
app_client.set_cookie(domain="localhost", key="test_cookie", value="hello")
response = app_client.get("/v1.0/test-cookie-param")
assert response.status_code == 200
assert response.json == {"cookie_value": "hello"}
2 changes: 1 addition & 1 deletion tests/decorators/test_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def test_get_valid_parameter_with_enum_array_header():
def test_invalid_type(monkeypatch):
logger = MagicMock()
monkeypatch.setattr('connexion.decorators.validation.logger', logger)
result = ParameterValidator.validate_parameter('formdata', 20, {'type': 'string', 'name': 'foo'})
result = ParameterValidator.validate_parameter('formdata', 20, {'name': 'foo', 'type': 'string'})
expected_result = """20 is not of type 'string'

Failed validating 'type' in schema:
Expand Down
2 changes: 1 addition & 1 deletion tests/fakeapi/hello/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ def get_date():


def get_uuid():
return {'value': uuid.UUID(hex='e7ff66d0-3ec2-4c4e-bed0-6e4723c24c51')}
return {'value': str(uuid.UUID(hex='e7ff66d0-3ec2-4c4e-bed0-6e4723c24c51'))}


def test_optional_headers():
Expand Down
8 changes: 7 additions & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,20 @@ envlist =
{py37}-{min,pypi,dev}
{py38}-{min,pypi,dev}
{py39}-{min,pypi,dev}
{py310}-{min,pypi,dev}
isort-check
Copy link
Contributor

@chrisinmtown chrisinmtown Aug 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should add py311 also, please. I tried that, and that brings new problems. In Py3.11 the collections module had some breaking (?) changes

ImportError while loading conftest '/Users/me/git/github/connexion_mfmarche/tests/conftest.py'.
/Users/me/git/github/connexion-mfmarche/tests/conftest.py:7: in <module>
    ???
connexion/__init__.py:14: in <module>
    from .apis import AbstractAPI  # NOQA
connexion/apis/__init__.py:16: in <module>
    from .abstract import AbstractAPI  # NOQA
connexion/apis/abstract.py:14: in <module>
    from ..exceptions import ResolverError
connexion/exceptions.py:7: in <module>
    from jsonschema.exceptions import ValidationError
.tox/py311-min/lib/python3.11/site-packages/jsonschema/__init__.py:18: in <module>
    from jsonschema.validators import (
.tox/py311-min/lib/python3.11/site-packages/jsonschema/validators.py:8: in <module>
    import requests
.tox/py311-min/lib/python3.11/site-packages/requests/__init__.py:58: in <module>
    from . import utils
.tox/py311-min/lib/python3.11/site-packages/requests/utils.py:30: in <module>
    from .cookies import RequestsCookieJar, cookiejar_from_dict
.tox/py311-min/lib/python3.11/site-packages/requests/cookies.py:164: in <module>
    class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping):
E   AttributeError: module 'collections' has no attribute 'MutableMapping'

https://stackoverflow.com/questions/70943244/attributeerror-module-collections-has-no-attribute-mutablemapping

isort-check-examples
isort-check-tests
flake8
mypy
pytest

[gh-actions]
python =
3.7: py37-min,py37-pypi
3.8: py38-min,py38-pypi
3.9: py39-min,py39-pypi,flake8,isort-check,isort-check-examples,isort-check-tests,mypy
3.10: py310-min,py310-pypi,flake8,isort-check,isort-check-examples,isort-check-tests,mypy

[testenv]
setenv=PYTHONPATH = {toxinidir}:{toxinidir}
Expand All @@ -31,7 +34,10 @@ commands=
pypi: pip install --upgrade -r {toxworkdir}/requirements-pypi.txt
dev: requirements-builder --level=dev --extras aiohttp --req=requirements-devel.txt -o {toxworkdir}/requirements-dev.txt setup.py
dev: pip install --upgrade -r {toxworkdir}/requirements-dev.txt
python setup.py test
python setup.py pytest {posargs}
Copy link
Contributor

@chrisinmtown chrisinmtown Aug 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a /lot/ going on in the tox setup/run commands that I don't understand, like dynamically creating requirements files such as requirements-min.txt. The tests get quite far but still fail with the old line (just test). The change from test to pytest changes the behavior significantly, and the tests still fail. From asking the cloud, I believe the invocation python setup.py is deprecated and has to be changed at this point bcos of this warning:

/Users/me/git/github/connexion_mfmarche/.tox/py311-min/lib/python3.11/site-packages/setuptools/__init__.py:81: _DeprecatedInstaller: setuptools.installer and fetch_build_eggs are deprecated.
!!

        ********************************************************************************
        Requirements should be satisfied by a PEP 517 installer.
        If you are using pip, you can try `pip install --use-pep517`.
        ********************************************************************************

!!
  dist.fetch_build_eggs(dist.setup_requires)


[testenv:pytest]
commands=python -m pytest

[testenv:flake8]
deps=
Expand Down