From 82595b003bf862a51816f504767fe043fe57c2e4 Mon Sep 17 00:00:00 2001 From: Vlada Dusek Date: Fri, 30 Oct 2026 08:40:54 +0100 Subject: [PATCH] fix: reject non-positive `expires_in` in signed public URLs --- src/apify_client/_resource_clients/dataset.py | 10 +- .../_resource_clients/key_value_store.py | 10 +- tests/unit/test_url_generation.py | 91 +++++++++++++++++++ 3 files changed, 107 insertions(+), 4 deletions(-) diff --git a/src/apify_client/_resource_clients/dataset.py b/src/apify_client/_resource_clients/dataset.py index 7df6cccd..69749799 100644 --- a/src/apify_client/_resource_clients/dataset.py +++ b/src/apify_client/_resource_clients/dataset.py @@ -605,12 +605,15 @@ def create_items_public_url( skip_hidden: If True, then hidden fields are skipped from the output. flatten: A list of fields that should be flattened. view: Name of the dataset view to be used. - expires_in: How long the signed URL should be valid. + expires_in: How long the signed URL should be valid. Must be positive. timeout: Timeout for the API HTTP request. Returns: The public dataset items URL. """ + if expires_in is not None and expires_in.total_seconds() <= 0: + raise ValueError(f'expires_in must be positive, got {expires_in}') + dataset = self.get(timeout=timeout) request_params = self._build_params( @@ -1197,12 +1200,15 @@ async def create_items_public_url( skip_hidden: If True, then hidden fields are skipped from the output. flatten: A list of fields that should be flattened. view: Name of the dataset view to be used. - expires_in: How long the signed URL should be valid. + expires_in: How long the signed URL should be valid. Must be positive. timeout: Timeout for the API HTTP request. Returns: The public dataset items URL. """ + if expires_in is not None and expires_in.total_seconds() <= 0: + raise ValueError(f'expires_in must be positive, got {expires_in}') + dataset = await self.get(timeout=timeout) request_params = self._build_params( diff --git a/src/apify_client/_resource_clients/key_value_store.py b/src/apify_client/_resource_clients/key_value_store.py index aa8fe41b..c4a548e9 100644 --- a/src/apify_client/_resource_clients/key_value_store.py +++ b/src/apify_client/_resource_clients/key_value_store.py @@ -476,12 +476,15 @@ def create_keys_public_url( exclusive_start_key: All keys up to this one (including) are skipped from the result. collection: The name of the collection in store schema to list keys from. prefix: The prefix of the keys to be listed. - expires_in: How long the signed URL should be valid from the time it is generated. + expires_in: How long the signed URL should be valid from the time it is generated. Must be positive. timeout: Timeout for the API HTTP request. Returns: The public key-value store keys URL. """ + if expires_in is not None and expires_in.total_seconds() <= 0: + raise ValueError(f'expires_in must be positive, got {expires_in}') + metadata = self.get(timeout=timeout) request_params = self._build_params( @@ -916,12 +919,15 @@ async def create_keys_public_url( exclusive_start_key: All keys up to this one (including) are skipped from the result. collection: The name of the collection in store schema to list keys from. prefix: The prefix of the keys to be listed. - expires_in: How long the signed URL should be valid from the time it is generated. + expires_in: How long the signed URL should be valid from the time it is generated. Must be positive. timeout: Timeout for the API HTTP request. Returns: The public key-value store keys URL. """ + if expires_in is not None and expires_in.total_seconds() <= 0: + raise ValueError(f'expires_in must be positive, got {expires_in}') + metadata = await self.get(timeout=timeout) request_params = self._build_params( diff --git a/tests/unit/test_url_generation.py b/tests/unit/test_url_generation.py index fc003118..7a9ec6a6 100644 --- a/tests/unit/test_url_generation.py +++ b/tests/unit/test_url_generation.py @@ -1,6 +1,8 @@ from __future__ import annotations import json +from base64 import urlsafe_b64decode +from datetime import timedelta from unittest import mock from unittest.mock import Mock from urllib.parse import parse_qs, urlparse @@ -308,3 +310,92 @@ async def test_api_url_version_appended_once_async(api_url: str, expected_base_u assert call_mock.call_args.kwargs['url'] == f'{expected_base_url}/key-value-stores/{MOCKED_KVS_ID}' assert public_url == f'{expected_base_url}/key-value-stores/{MOCKED_KVS_ID}/records/some_key' + + +parametrized_non_positive_expires_in = pytest.mark.parametrize( + 'expires_in', + [ + pytest.param(timedelta(0), id='zero'), + pytest.param(timedelta(seconds=-1), id='negative'), + ], +) + + +@parametrized_non_positive_expires_in +def test_dataset_public_url_rejects_non_positive_expires_in_sync(expires_in: timedelta) -> None: + """A non-positive `expires_in` raises `ValueError` before any request is sent, with sync client.""" + client = ApifyClient(token='dummy-token') + + dataset = client.dataset('someID') + + with ( + mock.patch.object(client._http_client, 'call') as call_mock, + pytest.raises(ValueError, match='expires_in must be positive'), + ): + dataset.create_items_public_url(expires_in=expires_in) + + call_mock.assert_not_called() + + +@parametrized_non_positive_expires_in +async def test_dataset_public_url_rejects_non_positive_expires_in_async(expires_in: timedelta) -> None: + """A non-positive `expires_in` raises `ValueError` before any request is sent, with async client.""" + client = ApifyClientAsync(token='dummy-token') + + dataset = client.dataset('someID') + + with ( + mock.patch.object(client._http_client, 'call') as call_mock, + pytest.raises(ValueError, match='expires_in must be positive'), + ): + await dataset.create_items_public_url(expires_in=expires_in) + + call_mock.assert_not_called() + + +@parametrized_non_positive_expires_in +def test_kvs_public_url_rejects_non_positive_expires_in_sync(expires_in: timedelta) -> None: + """A non-positive `expires_in` raises `ValueError` before any request is sent, with sync client.""" + client = ApifyClient(token='dummy-token') + + kvs = client.key_value_store(MOCKED_KVS_ID) + + with ( + mock.patch.object(client._http_client, 'call') as call_mock, + pytest.raises(ValueError, match='expires_in must be positive'), + ): + kvs.create_keys_public_url(expires_in=expires_in) + + call_mock.assert_not_called() + + +@parametrized_non_positive_expires_in +async def test_kvs_public_url_rejects_non_positive_expires_in_async(expires_in: timedelta) -> None: + """A non-positive `expires_in` raises `ValueError` before any request is sent, with async client.""" + client = ApifyClientAsync(token='dummy-token') + + kvs = client.key_value_store(MOCKED_KVS_ID) + + with ( + mock.patch.object(client._http_client, 'call') as call_mock, + pytest.raises(ValueError, match='expires_in must be positive'), + ): + await kvs.create_keys_public_url(expires_in=expires_in) + + call_mock.assert_not_called() + + +def test_dataset_public_url_signs_positive_expires_in() -> None: + """A positive `expires_in` produces a signature with a non-zero expiry.""" + client = ApifyClient(token='dummy-token') + dataset = client.dataset('someID') + + mock_response = Mock() + mock_response.json.return_value = json.loads(MOCKED_DATASET_RESPONSE) + + with mock.patch.object(client._http_client, 'call', return_value=mock_response): + public_url = dataset.create_items_public_url(expires_in=timedelta(seconds=1)) + + signature = parse_qs(urlparse(public_url).query)['signature'][0] + _, expires_at, _ = urlsafe_b64decode(signature).decode().split('.') + assert int(expires_at) > 0