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

Make CI happy again #2310

Closed

Conversation

matthewhughes934
Copy link
Contributor

@matthewhughes934 matthewhughes934 commented Dec 4, 2024

The motiviation here is to just get CI passing. It started with me just
bumping some dependencies to fix security alerts, but this raised some
further issues. The focus here was to make things pass with the minimum
amount of changes
, I think there's still some more work to bring some
dependencies up-to-date but this is a starting point.

The different types of changes are separated into the different commits
on this branch.

  • Update poetry dev naming

    Following the warning

    The "poetry.dev-dependencies" section is deprecated and will be
    removed in a future version. Use "poetry.group.dev.dependencies"
    instead.

    Update the name and run poetry.lock, this is also because the upcoming
    changes would add the groups = ["dev"] line to updated dependencies
    anyway, so change them all now to avoid noise later

  • Bump some dependencies for security fixes

    Bump jinja

      -> Vulnerability found in jinja2 version 3.1.3
         Vulnerability ID: 71591
         Affected spec: <3.1.4
         ADVISORY: Jinja is an extensible templating engine. The `xmlattr`
         filter in affected versions of Jinja accepts keys containing non-attribute...
         CVE-2024-34064
         For more information, please visit
         https://data.safetycli.com/v/71591/f17
    

    Bump anyio

      -> Vulnerability found in anyio version 4.1.0
         Vulnerability ID: 71199
         Affected spec: <4.4.0
         ADVISORY: Anyio version 4.4.0 addresses a thread race condition in
         `_eventloop.get_asynclib()` that caused crashes when multiple event loops...
         PVE-2024-71199
         For more information, please visit
         https://data.safetycli.com/v/71199/f17
    

    Bump bandit

      -> Vulnerability found in bandit version 1.7.6
         Vulnerability ID: 64484
         Affected spec: <1.7.7
         ADVISORY: Bandit 1.7.7 identifies the str.replace method as a
         potential risk for SQL injection because it can be misused in constructing...
         PVE-2024-64484
         For more information, please visit
         https://data.safetycli.com/v/64484/f17
    

    Bump certifi

      -> Vulnerability found in certifi version 2023.11.17
         Vulnerability ID: 72083
         Affected spec: >=2021.05.30,<2024.07.04
         ADVISORY: Certifi affected versions recognized root certificates from
         GLOBALTRUST. Certifi patch removes these root certificates from the root...
         CVE-2024-39689
         For more information, please visit
         https://data.safetycli.com/v/72083/f17
    

    Bump idna

      -> Vulnerability found in idna version 3.6
         Vulnerability ID: 67895
         Affected spec: <3.7
         ADVISORY: Affected versions of Idna are vulnerable to Denial Of
         Service via the idna.encode(), where a specially crafted argument could...
         CVE-2024-3651
         For more information, please visit
         https://data.safetycli.com/v/67895/f17
    

    Bump requests

      -> Vulnerability found in requests version 2.31.0
         Vulnerability ID: 71064
         Affected spec: <2.32.2
         ADVISORY: Affected versions of Requests, when making requests through
         a Requests `Session`, if the first request is made with `verify=False` to...
         CVE-2024-35195
         For more information, please visit
         https://data.safetycli.com/v/71064/f17
    

    Bump setuptools

      -> Vulnerability found in requests version 2.31.0
         Vulnerability ID: 71064
         Affected spec: <2.32.2
         ADVISORY: Affected versions of Requests, when making requests through
         a Requests `Session`, if the first request is made with `verify=False` to...
         CVE-2024-35195
         For more information, please visit
         https://data.safetycli.com/v/71064/f17
    

    Bump tornado

      -> Vulnerability found in tornado version 6.4
         Vulnerability ID: 71957
         Affected spec: <=6.4.0
         ADVISORY: When Tornado receives a request with two Transfer-Encoding:
         chunked headers, it ignores them both. This enables request smuggling when...
         PVE-2024-71957
         For more information, please visit
         https://data.safetycli.com/v/71957/f17
    
      -> Vulnerability found in tornado version 6.4
         Vulnerability ID: 71956
         Affected spec: <6.4.1
         ADVISORY: Tornado’s curl_httpclient.CurlAsyncHTTPClient class is
         vulnerable to CRLF (carriage return/line feed) injection in the request...
         PVE-2024-71956
         For more information, please visit
         https://data.safetycli.com/v/71956/f17
    

    Bump urllib3

      -> Vulnerability found in urllib3 version 2.1.0
         Vulnerability ID: 71608
         Affected spec: >=2.0.0a1,<=2.2.1
         ADVISORY: Urllib3's ProxyManager ensures that the Proxy-Authorization
         header is correctly directed only to configured proxies. However, when...
         CVE-2024-37891
         For more information, please visit
         https://data.safetycli.com/v/71608/f17
    

    Bump zipp

      -> Vulnerability found in zipp version 3.17.0
         Vulnerability ID: 72132
         Affected spec: <3.19.1
         ADVISORY: A Denial of Service (DoS) vulnerability exists in the
         jaraco/zipp library. The vulnerability is triggered when processing a...
         CVE-2024-5569
         For more information, please visit
         https://data.safetycli.com/v/72132/f17
    

    Bump virutalenv

      -> Vulnerability found in virtualenv version 20.25.0
         Vulnerability ID: 73456
         Affected spec: <20.26.6
         ADVISORY: Affected versions of the virtualenv package are vulnerable to command injection. This vulnerability could allow an attacker to execute arbitrary commands by exploiting improperly quoted string placeholders in activation scripts. The vulnerable functions include
         various shell activation scripts where placeholders like __VIRTUAL_ENV__ are used. The exploitability depends on the ability to control the input to these placeholders. Users are advised to update to the version where a quoting mechanism has been implemented to mitigate this...
         PVE-2024-73456
         For more information, please visit https://data.safetycli.com/v/73456/f17
    
  • Update black

      -> Vulnerability found in black version 23.11.0
         Vulnerability ID: 66742
         Affected spec: <24.3.0
         ADVISORY: Affected versions of Black are vulnerable to Regular
         Expression Denial of Service (ReDoS) via the...
         CVE-2024-21503
         For more information, please visit
         https://data.safetycli.com/v/66742/f17
    

    Also re-run black to pick up any changes from the new version and
    update some unit test that relied on how black formats.

  • Ignore security issue with mkdocs-material

    This requires handling upstream (see linked issue), trying to bump this
    dependency errored with:

      Because mkdocs-material (9.5.32) depends on mkdocs (>=1.6,<2.0)
       and portray (1.8.0) depends on mkdocs (>=1.3.0,<1.4.0), mkdocs-material (9.5.32) is incompatible with portray (1.8.0).
      And because no versions of portray match >1.8.0, mkdocs-material (9.5.32) is incompatible with portray (>=1.8.0).
      So, because isort depends on both portray (>=1.8.0) and mkdocs-material (9.5.32), version solving failed.
    
  • CI: use pip over `pipx for poetry install

    pipx is installed on all the runners by default, but using this means
    pipx is run with the system Python, and not the one installed with
    steup-python. This was noticed when e.g. the MacOS Python 3.9 job
    would report:

      creating virtual environment...
      creating shared libraries...
      upgrading shared libraries...
      installing poetry...
      done! ✨ 🌟 ✨
        installed package poetry 1.3.1, installed using Python 3.13.0
        These apps are now globally available
          - poetry
      Poetry (version 1.3.1)
    

    Python 3.13.0 is the system version pre-installed on these runners[1],
    and a similar pattern was seen on the Ubuntu and Windows runners. An
    alternative would be to add an install step for pipx but this feels
    simpler

    Link: https://github.com/actions/runner-images/blob/de16eefce8361c24c716958843d8c87cb1c25990/images/macos/macos-14-Readme.md [1]

  • Update pip for GitHub runner

    This is to address an error seen on some Python 3.12 runners:

      <-- SNIP -->
      File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/pip/_vendor/pkg_resources/__init__.py", line 2164, in <module>
        register_finder(pkgutil.ImpImporter, find_on_path)
                          ^^^^^^^^^^^^^^^^^^^
      AttributeError: module 'pkgutil' has no attribute 'ImpImporter'. Did you mean: 'zipimporter'?
                      ^^^^^^^^^^^^^^^^^^^
    

    This looks to be the issue[1] fixed in Pip 23.2 so use that verison

    Link: pip's vendored pkg_resources should stop using pkgutil.ImpImporter pypa/pip#11501 [1]

@matthewhughes934
Copy link
Contributor Author

matthewhughes934 commented Dec 4, 2024

well, this raised a bunch more issues. I'll look into the failing CI jobs

EDIT: at least the lint job passes 😅

@matthewhughes-uw
Copy link

matthewhughes-uw commented Dec 5, 2024

Looking at the MacOS failures, I see e.g. for the Python 3.8 job (https://github.com/PyCQA/isort/actions/runs/12178407083/job/33968333999?pr=2310)

PYO3_ENVIRONMENT_SIGNATURE="cpython-3.13-64bit" PYO3_PYTHON="/Users/runner/Library/Caches/pypoetry/virtualenvs/isort-7FOZ_X3_-py3.13/bin/python" PYTHON_SYS_EXECUTABLE="/Users/runner/Library/Caches/pypoetry/virtualenvs/isort-7FOZ_X3_-py3.13/bin/python" "cargo" "rustc" "--features" "pyo3/extension-module" "--message-format" "json-render-diagnostics" "--manifest-path" "/private/var/folders/0w/4z5l9vds32nbkz7l22n8j6s80000gn/T/pip-req-build-mias33_b/Cargo.toml" "--release" "--lib" "--crate-type" "cdylib" "--" "-C" "link-arg=-undefined" "-C" "link-arg=dynamic_lookup" "-C" "link-args=-Wl,-install_name,@rpath/pydantic_core._pydantic_core.cpython-313-darwin.so"`

It looks like this job is actually running most commands with Python 3.13? The poetry install step reports

creating virtual environment...
creating shared libraries...
upgrading shared libraries...
installing poetry...
done! ✨ 🌟 ✨
  installed package poetry 1.3.1, installed using Python 3.13.0
  These apps are now globally available
    - poetry
Poetry (version 1.3.1)

EDIT: further looking, it looks like all the MacOS jobs are run on Python 3.13, but the Ubuntu ones are 3.10.12 and Windows on 3.9.13

so it looks like it's using the system Python and not the one installed with the setup-python action 🤔

@matthewhughes-uw
Copy link

matthewhughes-uw commented Dec 5, 2024

There's currently an inconsistent failure with error

 File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-wgph2mrz\overlay\Lib\site-packages\setuptools\__init__.py", line 22, in <module>

  import _distutils_hack.override  # noqa: F401

  ModuleNotFoundError: No module named '_distutils_hack.override'

I say inconsistent because I saw it:

I'll address this in the future (I expect it will go away with some dependency bumps, if it shows up again)

@matthewhughes934 matthewhughes934 force-pushed the security-updates branch 3 times, most recently from 7cc8ed2 to e1fe80d Compare December 5, 2024 18:10
Copy link

codecov bot commented Dec 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.15%. Comparing base (7de1829) to head (e1fe80d).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2310   +/-   ##
=======================================
  Coverage   99.15%   99.15%           
=======================================
  Files          39       39           
  Lines        3091     3091           
  Branches      748      785   +37     
=======================================
  Hits         3065     3065           
  Misses         15       15           
  Partials       11       11           

@matthewhughes934 matthewhughes934 changed the title Security updates to fix CI Make CI happy again Dec 5, 2024
This requires handling upstream (see linked issue), trying to bump this
dependency errored with:

    Because mkdocs-material (9.5.32) depends on mkdocs (>=1.6,<2.0)
     and portray (1.8.0) depends on mkdocs (>=1.3.0,<1.4.0), mkdocs-material (9.5.32) is incompatible with portray (1.8.0).
    And because no versions of portray match >1.8.0, mkdocs-material (9.5.32) is incompatible with portray (>=1.8.0).
    So, because isort depends on both portray (>=1.8.0) and mkdocs-material (9.5.32), version solving failed.
Bump `jinja`

    -> Vulnerability found in jinja2 version 3.1.3
       Vulnerability ID: 71591
       Affected spec: <3.1.4
       ADVISORY: Jinja is an extensible templating engine. The `xmlattr`
       filter in affected versions of Jinja accepts keys containing non-attribute...
       CVE-2024-34064
       For more information, please visit
       https://data.safetycli.com/v/71591/f17

Bump `anyio`

    -> Vulnerability found in anyio version 4.1.0
       Vulnerability ID: 71199
       Affected spec: <4.4.0
       ADVISORY: Anyio version 4.4.0 addresses a thread race condition in
       `_eventloop.get_asynclib()` that caused crashes when multiple event loops...
       PVE-2024-71199
       For more information, please visit
       https://data.safetycli.com/v/71199/f17

Bump `bandit`

    -> Vulnerability found in bandit version 1.7.6
       Vulnerability ID: 64484
       Affected spec: <1.7.7
       ADVISORY: Bandit 1.7.7 identifies the str.replace method as a
       potential risk for SQL injection because it can be misused in constructing...
       PVE-2024-64484
       For more information, please visit
       https://data.safetycli.com/v/64484/f17

Bump `certifi`

    -> Vulnerability found in certifi version 2023.11.17
       Vulnerability ID: 72083
       Affected spec: >=2021.05.30,<2024.07.04
       ADVISORY: Certifi affected versions recognized root certificates from
       GLOBALTRUST. Certifi patch removes these root certificates from the root...
       CVE-2024-39689
       For more information, please visit
       https://data.safetycli.com/v/72083/f17

Bump `idna`

    -> Vulnerability found in idna version 3.6
       Vulnerability ID: 67895
       Affected spec: <3.7
       ADVISORY: Affected versions of Idna are vulnerable to Denial Of
       Service via the idna.encode(), where a specially crafted argument could...
       CVE-2024-3651
       For more information, please visit
       https://data.safetycli.com/v/67895/f17

Bump `requests`

    -> Vulnerability found in requests version 2.31.0
       Vulnerability ID: 71064
       Affected spec: <2.32.2
       ADVISORY: Affected versions of Requests, when making requests through
       a Requests `Session`, if the first request is made with `verify=False` to...
       CVE-2024-35195
       For more information, please visit
       https://data.safetycli.com/v/71064/f17

Bump `setuptools`

    -> Vulnerability found in requests version 2.31.0
       Vulnerability ID: 71064
       Affected spec: <2.32.2
       ADVISORY: Affected versions of Requests, when making requests through
       a Requests `Session`, if the first request is made with `verify=False` to...
       CVE-2024-35195
       For more information, please visit
       https://data.safetycli.com/v/71064/f17

Bump `tornado`

    -> Vulnerability found in tornado version 6.4
       Vulnerability ID: 71957
       Affected spec: <=6.4.0
       ADVISORY: When Tornado receives a request with two Transfer-Encoding:
       chunked headers, it ignores them both. This enables request smuggling when...
       PVE-2024-71957
       For more information, please visit
       https://data.safetycli.com/v/71957/f17

    -> Vulnerability found in tornado version 6.4
       Vulnerability ID: 71956
       Affected spec: <6.4.1
       ADVISORY: Tornado’s curl_httpclient.CurlAsyncHTTPClient class is
       vulnerable to CRLF (carriage return/line feed) injection in the request...
       PVE-2024-71956
       For more information, please visit
       https://data.safetycli.com/v/71956/f17

Bump `urllib3`

    -> Vulnerability found in urllib3 version 2.1.0
       Vulnerability ID: 71608
       Affected spec: >=2.0.0a1,<=2.2.1
       ADVISORY: Urllib3's ProxyManager ensures that the Proxy-Authorization
       header is correctly directed only to configured proxies. However, when...
       CVE-2024-37891
       For more information, please visit
       https://data.safetycli.com/v/71608/f17

Bump `zipp`

    -> Vulnerability found in zipp version 3.17.0
       Vulnerability ID: 72132
       Affected spec: <3.19.1
       ADVISORY: A Denial of Service (DoS) vulnerability exists in the
       jaraco/zipp library. The vulnerability is triggered when processing a...
       CVE-2024-5569
       For more information, please visit
       https://data.safetycli.com/v/72132/f17

Bump `virutalenv`

    -> Vulnerability found in virtualenv version 20.25.0
       Vulnerability ID: 73456
       Affected spec: <20.26.6
       ADVISORY: Affected versions of the virtualenv package are vulnerable to command injection. This vulnerability could allow an attacker to execute arbitrary commands by exploiting improperly quoted string placeholders in activation scripts. The vulnerable functions include
       various shell activation scripts where placeholders like __VIRTUAL_ENV__ are used. The exploitability depends on the ability to control the input to these placeholders. Users are advised to update to the version where a quoting mechanism has been implemented to mitigate this...
       PVE-2024-73456
       For more information, please visit https://data.safetycli.com/v/73456/f17
    -> Vulnerability found in black version 23.11.0
       Vulnerability ID: 66742
       Affected spec: <24.3.0
       ADVISORY: Affected versions of Black are vulnerable to Regular
       Expression Denial of Service (ReDoS) via the...
       CVE-2024-21503
       For more information, please visit
       https://data.safetycli.com/v/66742/f17

Also re-run `black` to pick up any changes from the new version and
update some unit test that relied on how black formats.
`pipx` is installed on all the runners by default, but using this means
`pipx` is run with the system Python, and not the one installed with
`steup-python`. This was noticed when e.g. the MacOS Python 3.9 job
would report:

    creating virtual environment...
    creating shared libraries...
    upgrading shared libraries...
    installing poetry...
    done! ✨ 🌟 ✨
      installed package poetry 1.3.1, installed using Python 3.13.0
      These apps are now globally available
        - poetry
    Poetry (version 1.3.1)

Python 3.13.0 is the system version pre-installed on these runners[1],
and a similar pattern was seen on the Ubuntu and Windows runners. An
alternative would be to add an install step for `pipx` but this feels
simpler

Link: https://github.com/actions/runner-images/blob/de16eefce8361c24c716958843d8c87cb1c25990/images/macos/macos-14-Readme.md [1]
This is to address an error seen on some Python 3.12 runners:

    <-- SNIP -->
    File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/pip/_vendor/pkg_resources/__init__.py", line 2164, in <module>
      register_finder(pkgutil.ImpImporter, find_on_path)
                        ^^^^^^^^^^^^^^^^^^^
    AttributeError: module 'pkgutil' has no attribute 'ImpImporter'. Did you mean: 'zipimporter'?
                    ^^^^^^^^^^^^^^^^^^^

This looks to be the issue[1] fixed in Pip 23.2 so use that verison

Link: pypa/pip#11501 [1]
@matthewhughes934 matthewhughes934 deleted the security-updates branch December 8, 2024 10:42
@matthewhughes934
Copy link
Contributor Author

I accidentally deleted the source branch while cleaning up branches on my fork, re-created this PR: #2311

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants