From b640785f566fc355dd5209f82cf8e6c8aac18e1a Mon Sep 17 00:00:00 2001 From: david ruiz Date: Mon, 26 Oct 2026 12:02:47 +0100 Subject: [PATCH 1/3] Python Security updates --- .github/workflows/build-main.yml | 8 +++- .github/workflows/build-pull-request.yml | 8 +++- .github/workflows/build-release.yml | 11 +++-- checkout_sdk/oauth_credentials.py | 5 +- requirements-ci.txt | 7 +++ requirements-dev.txt | 8 +--- tests/oauth_credentials_test.py | 58 ++++++++++++++++++++++++ 7 files changed, 85 insertions(+), 20 deletions(-) create mode 100644 requirements-ci.txt create mode 100644 tests/oauth_credentials_test.py diff --git a/.github/workflows/build-main.yml b/.github/workflows/build-main.yml index eb574151..27c5b560 100644 --- a/.github/workflows/build-main.yml +++ b/.github/workflows/build-main.yml @@ -23,10 +23,14 @@ jobs: uses: actions/setup-python@v2 with: python-version: ${{ matrix.python-version }} + # --only-binary :all: installs wheels only, so pip never runs a downloaded package's + # setup.py in a job that holds repository secrets. requirements-ci.txt is + # requirements-dev.txt without pre-commit, a local git hook tool CI never runs whose + # PyYAML dependency ships no PyPy wheels. - name: Upgrade pip and virtualenv to latest run: | - python -m pip install --upgrade pip - pip install -r requirements-dev.txt + python -m pip install --only-binary :all: --upgrade pip + pip install --only-binary :all: -r requirements-ci.txt - name: Lint with flake8 run: flake8 . --count --max-complexity=10 --max-line-length=127 --statistics - name: Run unit tests diff --git a/.github/workflows/build-pull-request.yml b/.github/workflows/build-pull-request.yml index f7cb943f..a7c46845 100644 --- a/.github/workflows/build-pull-request.yml +++ b/.github/workflows/build-pull-request.yml @@ -22,10 +22,14 @@ jobs: uses: actions/setup-python@v2 with: python-version: ${{ matrix.python-version }} + # --only-binary :all: installs wheels only, so pip never runs a downloaded package's + # setup.py in a job that holds repository secrets. requirements-ci.txt is + # requirements-dev.txt without pre-commit, a local git hook tool CI never runs whose + # PyYAML dependency ships no PyPy wheels. - name: Upgrade pip and virtualenv to latest run: | - python -m pip install --upgrade pip - pip install -r requirements-dev.txt + python -m pip install --only-binary :all: --upgrade pip + pip install --only-binary :all: -r requirements-ci.txt - name: Lint with flake8 run: flake8 . --count --max-complexity=10 --max-line-length=127 --statistics - name: Run PyLint diff --git a/.github/workflows/build-release.yml b/.github/workflows/build-release.yml index 056bf630..30184e5c 100644 --- a/.github/workflows/build-release.yml +++ b/.github/workflows/build-release.yml @@ -16,15 +16,16 @@ jobs: uses: actions/setup-python@v2 with: python-version: '3.10' + # Wheels only, as in build-main.yml: this job also holds the PyPI credentials. - id: install-tooling run: | - python -m pip install --upgrade pip - pip install wheel - pip install twine - pip install build + python -m pip install --only-binary :all: --upgrade pip + pip install --only-binary :all: wheel + pip install --only-binary :all: twine + pip install --only-binary :all: build - id: install-project-dependencies run: | - pip install -r requirements-dev.txt + pip install --only-binary :all: -r requirements-ci.txt - id: testing env: CHECKOUT_PROCESSING_CHANNEL_ID: ${{ secrets.IT_CHECKOUT_PROCESSING_CHANNEL_ID }} diff --git a/checkout_sdk/oauth_credentials.py b/checkout_sdk/oauth_credentials.py index 1f3bb5ce..ae610411 100644 --- a/checkout_sdk/oauth_credentials.py +++ b/checkout_sdk/oauth_credentials.py @@ -4,7 +4,6 @@ from abc import ABCMeta from datetime import datetime, timedelta -import urllib3 from requests import HTTPError, Session, ConnectionError from checkout_sdk.authorization_type import AuthorizationType @@ -69,11 +68,9 @@ def get_access_token(self): 'scope': str.join(' ', self.__scopes) } - urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) - try: response = self.__http_client.request(method='POST', url=self.__authorization_uri, - data=data, verify=False, allow_redirects=False, + data=data, allow_redirects=False, auth=(self.__client_id, self.__client_secret)) response.raise_for_status() except HTTPError as err: diff --git a/requirements-ci.txt b/requirements-ci.txt new file mode 100644 index 00000000..ebbffd09 --- /dev/null +++ b/requirements-ci.txt @@ -0,0 +1,7 @@ +requests>=2.27.1 +pytest>=6.1.2 +pytest-mock>=3.6.1 +flake8>=4.0.1 +python-dateutil>=2.8.2 +pylint>=2.12.2 +Deprecated>=1.2.14 diff --git a/requirements-dev.txt b/requirements-dev.txt index c1cc2ee3..fc509e14 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,8 +1,2 @@ -requests>=2.27.1 -pytest>=6.1.2 -pytest-mock>=3.6.1 -flake8>=4.0.1 -python-dateutil>=2.8.2 +-r requirements-ci.txt pre-commit>=2.17.0 -pylint>=2.12.2 -Deprecated>=1.2.14 \ No newline at end of file diff --git a/tests/oauth_credentials_test.py b/tests/oauth_credentials_test.py new file mode 100644 index 00000000..0c190a53 --- /dev/null +++ b/tests/oauth_credentials_test.py @@ -0,0 +1,58 @@ +import json + +import pytest +from requests import Response, Session +from requests.adapters import HTTPAdapter + +from checkout_sdk.environment import Environment +from checkout_sdk.oauth_credentials import OAuthSdkCredentials + + +def token_response(request): + response = Response() + response.status_code = 200 + response.request = request + response.url = request.url + response._content = json.dumps( + {'access_token': 'access_token', 'token_type': 'Bearer', 'expires_in': 3600}).encode('utf-8') + return response + + +class TestOAuthSdkCredentials: + """The token request carries the client id and secret, so it must never go out with TLS + certificate verification turned off. The SDK leaves verification to the http_client the + caller passed in, and to requests' defaults, instead of overriding it per request. + + Only the transport is stubbed: a real Session merges its settings into the request, so the + test sees the same verify value the TLS handshake would. + """ + + @pytest.fixture(autouse=True) + def no_ca_bundle_from_environment(self, monkeypatch): + # requests reads these when verification is on, which would make the asserted value + # depend on the machine running the tests. + monkeypatch.delenv('REQUESTS_CA_BUNDLE', raising=False) + monkeypatch.delenv('CURL_CA_BUNDLE', raising=False) + + @staticmethod + def request_token(mocker, http_client: Session): + send = mocker.patch.object(HTTPAdapter, 'send', autospec=True, + side_effect=lambda adapter, request, **kwargs: token_response(request)) + credentials = OAuthSdkCredentials(http_client, Environment.sandbox(), 'client_id', 'client_secret', + None, ['vault']) + assert credentials.get_access_token().token == 'access_token' + return send + + def test_should_verify_the_server_certificate_when_requesting_the_token(self, mocker): + send = self.request_token(mocker, Session()) + + assert send.call_count == 1 + assert send.call_args.kwargs['verify'] is True + + def test_should_use_the_ca_bundle_configured_on_the_http_client(self, mocker): + http_client = Session() + http_client.verify = '/path/to/ca-bundle.pem' + + send = self.request_token(mocker, http_client) + + assert send.call_args.kwargs['verify'] == '/path/to/ca-bundle.pem' From 982d17dfc9436a78095f3a10f872b6623b51ffb3 Mon Sep 17 00:00:00 2001 From: david ruiz Date: Mon, 26 Oct 2026 12:33:17 +0100 Subject: [PATCH 2/3] Pin exact versions for pip --- .github/workflows/build-main.yml | 7 ++++++- .github/workflows/build-pull-request.yml | 7 ++++++- .github/workflows/build-release.yml | 9 +++++---- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build-main.yml b/.github/workflows/build-main.yml index 27c5b560..6603fa26 100644 --- a/.github/workflows/build-main.yml +++ b/.github/workflows/build-main.yml @@ -29,7 +29,12 @@ jobs: # PyYAML dependency ships no PyPy wheels. - name: Upgrade pip and virtualenv to latest run: | - python -m pip install --only-binary :all: --upgrade pip + # Exact pins (Sonar S8544). 25.0.1 is the last pip that supports Python 3.8. + if [ "${{ matrix.python-version }}" = "pypy-3.8" ]; then + python -m pip install --only-binary :all: pip==25.0.1 + else + python -m pip install --only-binary :all: pip==26.2.2 + fi pip install --only-binary :all: -r requirements-ci.txt - name: Lint with flake8 run: flake8 . --count --max-complexity=10 --max-line-length=127 --statistics diff --git a/.github/workflows/build-pull-request.yml b/.github/workflows/build-pull-request.yml index a7c46845..4806b7a6 100644 --- a/.github/workflows/build-pull-request.yml +++ b/.github/workflows/build-pull-request.yml @@ -28,7 +28,12 @@ jobs: # PyYAML dependency ships no PyPy wheels. - name: Upgrade pip and virtualenv to latest run: | - python -m pip install --only-binary :all: --upgrade pip + # Exact pins (Sonar S8544). 25.0.1 is the last pip that supports Python 3.8. + if [ "${{ matrix.python-version }}" = "pypy-3.8" ]; then + python -m pip install --only-binary :all: pip==25.0.1 + else + python -m pip install --only-binary :all: pip==26.2.2 + fi pip install --only-binary :all: -r requirements-ci.txt - name: Lint with flake8 run: flake8 . --count --max-complexity=10 --max-line-length=127 --statistics diff --git a/.github/workflows/build-release.yml b/.github/workflows/build-release.yml index 30184e5c..0fc19be3 100644 --- a/.github/workflows/build-release.yml +++ b/.github/workflows/build-release.yml @@ -17,12 +17,13 @@ jobs: with: python-version: '3.10' # Wheels only, as in build-main.yml: this job also holds the PyPI credentials. + # The tooling is pinned to exact versions (Sonar S8544). - id: install-tooling run: | - python -m pip install --only-binary :all: --upgrade pip - pip install --only-binary :all: wheel - pip install --only-binary :all: twine - pip install --only-binary :all: build + python -m pip install --only-binary :all: pip==26.2.2 + pip install --only-binary :all: wheel==0.48.0 + pip install --only-binary :all: twine==7.0.0 + pip install --only-binary :all: build==1.6.1 - id: install-project-dependencies run: | pip install --only-binary :all: -r requirements-ci.txt From 6dd57483279b29e97bb44429f1311b8afa6a7180 Mon Sep 17 00:00:00 2001 From: david ruiz Date: Mon, 26 Oct 2026 14:36:22 +0100 Subject: [PATCH 3/3] api_client invoke file close improvement + tests --- checkout_sdk/api_client.py | 6 +++ tests/api_client_file_upload_test.py | 73 ++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 tests/api_client_file_upload_test.py diff --git a/checkout_sdk/api_client.py b/checkout_sdk/api_client.py index 64e605dd..7b2a7c63 100644 --- a/checkout_sdk/api_client.py +++ b/checkout_sdk/api_client.py @@ -102,6 +102,7 @@ def invoke(self, base_uri = self._base_uri + path + files = None try: json_body, params_dict, files = self._prepare_request_payload( request_headers, body, params, file_request, multipart_file) @@ -122,6 +123,11 @@ def invoke(self, except OSError as err: error = err.strerror raise CheckoutException(error) from err + finally: + # get_file_request opens the upload; requests reads it but never closes it. + if files is not None: + for _, file, _ in files.values(): + file.close() http_metadata = map_to_http_metadata(response) if response.text: diff --git a/tests/api_client_file_upload_test.py b/tests/api_client_file_upload_test.py new file mode 100644 index 00000000..52aad134 --- /dev/null +++ b/tests/api_client_file_upload_test.py @@ -0,0 +1,73 @@ +from unittest.mock import MagicMock + +import pytest +from requests import ConnectionError, HTTPError + +from checkout_sdk.api_client import ApiClient +from checkout_sdk.exception import CheckoutApiException, CheckoutException +from checkout_sdk.files.files import FileRequest + + +# Exercises the REAL ApiClient upload path (invoke -> get_file_request -> http_client.request). +# get_file_request opens the file and hands the open handle to requests, which reads it but never +# closes it, so invoke has to close it whatever the outcome of the request. +def _build(mock_sdk_configuration, request_side_effect=None, status=200): + api_client = ApiClient(configuration=mock_sdk_configuration, + base_uri=mock_sdk_configuration.environment.base_uri) + response = MagicMock() + response.status_code = status + response.text = '{"id": "file_123"}' + response.headers = {'Content-Type': 'application/json'} + response.json.return_value = {'id': 'file_123'} + if status >= 400: + response.raise_for_status.side_effect = HTTPError(response=response) + http_client = MagicMock() + http_client.request.return_value = response + http_client.request.side_effect = request_side_effect + api_client._http_client = http_client + + authorization = MagicMock() + authorization.get_authorization_header.return_value = 'Bearer test' + return api_client, http_client, authorization + + +def _file_request(tmp_path): + path = tmp_path / 'evidence.pdf' + path.write_bytes(b'%PDF-1.4') + file_request = FileRequest() + file_request.file = str(path) + file_request.purpose = 'dispute_evidence' + return file_request + + +def _sent_file(http_client): + files = http_client.request.call_args.kwargs['files'] + return files['file'][1] + + +class TestApiClientFileUpload: + + def test_should_close_the_uploaded_file_after_a_successful_request(self, mock_sdk_configuration, tmp_path): + api_client, http_client, authorization = _build(mock_sdk_configuration) + + response = api_client.submit_file('files', authorization, _file_request(tmp_path)) + + assert response.id == 'file_123' + assert _sent_file(http_client).closed + + def test_should_close_the_uploaded_file_when_the_api_returns_an_error(self, mock_sdk_configuration, tmp_path): + api_client, http_client, authorization = _build(mock_sdk_configuration, status=422) + + with pytest.raises(CheckoutApiException): + api_client.submit_file('files', authorization, _file_request(tmp_path)) + + assert _sent_file(http_client).closed + + def test_should_close_the_uploaded_file_when_the_connection_fails(self, mock_sdk_configuration, tmp_path): + api_client, http_client, authorization = _build(mock_sdk_configuration, + request_side_effect=ConnectionError('unreachable')) + + with pytest.raises(CheckoutException): + api_client.submit_file('files', authorization, _file_request(tmp_path)) + + assert _sent_file(http_client).closed