diff --git a/.changeset/reject-dot-segment-path-parameters.md b/.changeset/reject-dot-segment-path-parameters.md new file mode 100644 index 00000000..f1432e3e --- /dev/null +++ b/.changeset/reject-dot-segment-path-parameters.md @@ -0,0 +1,5 @@ +--- +'@fingerprint/python-sdk': patch +--- + +Reject `.` and `..` as event and visitor IDs. `get_event`, `update_event`, and `delete_visitor_data` now raise the new `InvalidArgumentError` without sending a request. diff --git a/docs/FingerprintApi.md b/docs/FingerprintApi.md index 785d4333..2e428eac 100644 --- a/docs/FingerprintApi.md +++ b/docs/FingerprintApi.md @@ -86,6 +86,11 @@ Name | Type | Description | Notes ------------- | ------------- | ------------- | ------------- **visitor_id** | **str**| The [visitor ID](https://docs.fingerprint.com/reference/js-agent-v4-get-function#visitor_id) you want to delete. | +> [!WARNING] +> #### Invalid values +> +> `visitor_id` must not be `.` or `..`. When you use them, the call raises `InvalidArgumentError` without sending a request. + ### Return type void (empty response body) @@ -164,6 +169,11 @@ Name | Type | Description | Notes **event_id** | **str**| The unique [identifier](https://docs.fingerprint.com/reference/js-agent-v4-get-function#event_id) of each identification request (`requestId` can be used in its place). | **ruleset_id** | **str**| The ID of the ruleset to evaluate against the event, producing the action to take for this event. The resulting action is returned in the `rule_action` attribute of the response. | [optional] +> [!WARNING] +> #### Invalid values +> +> `event_id` must not be `.` or `..`. When you use them, the call raises `InvalidArgumentError` without sending a request. + ### Return type [**Event**](Event.md) @@ -461,6 +471,11 @@ Name | Type | Description | Notes **event_id** | **str**| The unique event [identifier](https://docs.fingerprint.com/reference/js-agent-v4-get-function#event_id). | **event_update** | [**EventUpdate**](EventUpdate.md)| | +> [!WARNING] +> #### Invalid values +> +> `event_id` must not be `.` or `..`. When you use them, the call raises `InvalidArgumentError` without sending a request. + ### Return type void (empty response body) diff --git a/fingerprint_server_sdk/__init__.py b/fingerprint_server_sdk/__init__.py index afa630bd..ec609c87 100644 --- a/fingerprint_server_sdk/__init__.py +++ b/fingerprint_server_sdk/__init__.py @@ -37,6 +37,7 @@ 'TooManyRequestsException', 'ServiceException', 'GatewayTimeoutException', + 'InvalidArgumentError', 'WebhookValidation', 'DecryptionKey', 'DecryptionAlgorithm', @@ -128,6 +129,7 @@ from fingerprint_server_sdk.exceptions import UnprocessableEntityException from fingerprint_server_sdk.exceptions import TooManyRequestsException from fingerprint_server_sdk.exceptions import GatewayTimeoutException +from fingerprint_server_sdk.exceptions import InvalidArgumentError # import models into sdk package from fingerprint_server_sdk.models.bot_info import BotInfo diff --git a/fingerprint_server_sdk/api_client.py b/fingerprint_server_sdk/api_client.py index 7469b4e1..1099e79c 100644 --- a/fingerprint_server_sdk/api_client.py +++ b/fingerprint_server_sdk/api_client.py @@ -37,6 +37,7 @@ from fingerprint_server_sdk.exceptions import ( ApiException, ApiValueError, + InvalidArgumentError, ) RequestSerialized = tuple[str, str, dict[str, Any], Optional[Any], Any] @@ -169,9 +170,10 @@ def param_serialize( ) for k, v in path_params_tuples: # specified safe chars, encode everything - resource_path = resource_path.replace( - '{' + k + '}', quote(str(v), safe=config.safe_chars_for_path_param) - ) + encoded = quote(str(v), safe=config.safe_chars_for_path_param) + if encoded in ('.', '..'): + raise InvalidArgumentError(k, str(v)) + resource_path = resource_path.replace('{' + k + '}', encoded) # post parameters post_params_result: Any = None diff --git a/fingerprint_server_sdk/exceptions.py b/fingerprint_server_sdk/exceptions.py index 1361a649..db2acc65 100644 --- a/fingerprint_server_sdk/exceptions.py +++ b/fingerprint_server_sdk/exceptions.py @@ -236,6 +236,15 @@ class GatewayTimeoutException(ServiceException): pass +class InvalidArgumentError(ApiValueError): + """Exception when ``.`` or ``..`` used as resource identifier.""" + + def __init__(self, argument: str, value: str) -> None: + self.argument = argument + self.value = value + super().__init__(f'invalid value {value!r} for {argument}: not a valid identifier') + + def render_path(path_to_item: list[Any]) -> str: """Returns a string representation of a path""" result = '' diff --git a/template/__init__package.mustache b/template/__init__package.mustache index 430c1e57..435894cc 100644 --- a/template/__init__package.mustache +++ b/template/__init__package.mustache @@ -26,6 +26,7 @@ __all__ = [ "TooManyRequestsException", "ServiceException", "GatewayTimeoutException", + "InvalidArgumentError", "WebhookValidation", "DecryptionKey", "DecryptionAlgorithm", diff --git a/template/api_client.mustache b/template/api_client.mustache index 04c90402..b6110c46 100644 --- a/template/api_client.mustache +++ b/template/api_client.mustache @@ -28,6 +28,7 @@ from {{packageName}}.configuration import Configuration from {{packageName}}.exceptions import ( ApiException, ApiValueError, + InvalidArgumentError, ) RequestSerialized = tuple[str, str, dict[str, Any], Optional[Any], Any] @@ -176,9 +177,10 @@ class ApiClient: ) for k, v in path_params_tuples: # specified safe chars, encode everything - resource_path = resource_path.replace( - '{' + k + '}', quote(str(v), safe=config.safe_chars_for_path_param) - ) + encoded = quote(str(v), safe=config.safe_chars_for_path_param) + if encoded in ('.', '..'): + raise InvalidArgumentError(k, str(v)) + resource_path = resource_path.replace('{' + k + '}', encoded) # post parameters post_params_result: Any = None diff --git a/template/api_doc.mustache b/template/api_doc.mustache index 3f3d9dd2..d3e7a903 100644 --- a/template/api_doc.mustache +++ b/template/api_doc.mustache @@ -32,6 +32,13 @@ Name | Type | Description | Notes {{#allParams}} **{{paramName}}** | {{#isFile}}**{{dataType}}**{{/isFile}}{{^isFile}}{{#isPrimitiveType}}**{{dataType}}**{{/isPrimitiveType}}{{^isPrimitiveType}}[**{{dataType}}**]({{baseType}}.md){{/isPrimitiveType}}{{/isFile}}| {{{description}}} | {{^required}}[optional] {{/required}}{{#defaultValue}}[default to {{.}}]{{/defaultValue}} {{/allParams}} +{{#pathParams}} +> [!WARNING] +> #### Invalid values +> +> `{{paramName}}` must not be `.` or `..`. When you use them, the call raises `InvalidArgumentError` without sending a request. + +{{/pathParams}} ### Return type {{#returnType}}{{#returnTypeIsPrimitive}}**{{{returnType}}}**{{/returnTypeIsPrimitive}}{{^returnTypeIsPrimitive}}[**{{{returnType}}}**]({{returnBaseType}}.md){{/returnTypeIsPrimitive}}{{/returnType}}{{^returnType}}void (empty response body){{/returnType}} diff --git a/template/exceptions.mustache b/template/exceptions.mustache index 9431f0c8..3407fc13 100644 --- a/template/exceptions.mustache +++ b/template/exceptions.mustache @@ -222,6 +222,15 @@ class GatewayTimeoutException(ServiceException): pass +class InvalidArgumentError(ApiValueError): + """Exception when ``.`` or ``..`` used as resource identifier.""" + + def __init__(self, argument: str, value: str) -> None: + self.argument = argument + self.value = value + super().__init__(f"invalid value {value!r} for {argument}: not a valid identifier") + + def render_path(path_to_item: list[Any]) -> str: """Returns a string representation of a path""" result = "" diff --git a/template/exports_package.mustache b/template/exports_package.mustache index 33afda40..ac07250f 100644 --- a/template/exports_package.mustache +++ b/template/exports_package.mustache @@ -20,6 +20,7 @@ from {{packageName}}.exceptions import ConflictException from {{packageName}}.exceptions import UnprocessableEntityException from {{packageName}}.exceptions import TooManyRequestsException from {{packageName}}.exceptions import GatewayTimeoutException +from {{packageName}}.exceptions import InvalidArgumentError {{#hasHttpSignatureMethods}} from {{packageName}}.signing import HttpSigningConfiguration {{/hasHttpSignatureMethods}} diff --git a/test/test_path_params.py b/test/test_path_params.py new file mode 100644 index 00000000..e52d4c12 --- /dev/null +++ b/test/test_path_params.py @@ -0,0 +1,185 @@ +"""Tests for how path parameter values reach the Server API.""" + +import threading +import unittest +from dataclasses import dataclass +from http.server import BaseHTTPRequestHandler, HTTPServer +from typing import Callable, Optional + +from fingerprint_server_sdk import ( + Configuration, + EventUpdate, + InvalidArgumentError, +) +from fingerprint_server_sdk.api.fingerprint_api import FingerprintApi + +API_KEY = '' + +RESPONSE_BODY = b'{"event_id": "1708102555327.NLOjmg", "timestamp": 1708102555327}' + + +@dataclass(frozen=True) +class PathParamOperation: + """An operation that takes an ID as a URL path parameter.""" + + name: str + argument: str + prefix: str + call: Callable[[FingerprintApi, str], object] + + +OPERATIONS = ( + PathParamOperation( + name='get_event', + argument='event_id', + prefix='/events/', + call=lambda api, event_id: api.get_event(event_id), + ), + PathParamOperation( + name='update_event', + argument='event_id', + prefix='/events/', + call=lambda api, event_id: api.update_event(event_id, EventUpdate(suspect=True)), + ), + PathParamOperation( + name='delete_visitor_data', + argument='visitor_id', + prefix='/visitors/', + call=lambda api, visitor_id: api.delete_visitor_data(visitor_id), + ), +) + + +class StubServer: + """A local HTTP server recording the request target it was last asked for.""" + + def __init__(self) -> None: + self.request_target: Optional[str] = None + server = self + + class Handler(BaseHTTPRequestHandler): + def _handle(self) -> None: + server.request_target = self.path.split('?', 1)[0] + + self.send_response(200) + self.send_header('Content-Type', 'application/json') + self.send_header('Content-Length', str(len(RESPONSE_BODY))) + self.end_headers() + self.wfile.write(RESPONSE_BODY) + + do_GET = _handle + do_PATCH = _handle + do_DELETE = _handle + + def log_message(self, *args: object) -> None: + """Silence the default logging.""" + + self._server = HTTPServer(('127.0.0.1', 0), Handler) + self._thread = threading.Thread(target=self._server.serve_forever, daemon=True) + + def start(self) -> None: + self._thread.start() + + def stop(self) -> None: + self._server.shutdown() + self._server.server_close() + self._thread.join() + + def reset(self) -> None: + self.request_target = None + + @property + def base_path(self) -> str: + host, port = self._server.server_address[:2] + return f'http://{host!s}:{port!s}/base' + + +class TestPathParams(unittest.TestCase): + """Test path parameter handling for every operation that takes an ID in the path.""" + + server: StubServer + + @classmethod + def setUpClass(cls) -> None: + cls.server = StubServer() + cls.server.start() + + @classmethod + def tearDownClass(cls) -> None: + cls.server.stop() + + def setUp(self) -> None: + self.server.reset() + self.api = FingerprintApi(Configuration(api_key=API_KEY, host=self.server.base_path)) + + def call(self, operation: PathParamOperation, value: str) -> Optional[str]: + """Run operation with value and return the request target the server received.""" + self.server.reset() + operation.call(self.api, value) + return self.server.request_target + + def test_value_is_encoded_into_a_single_path_segment(self) -> None: + """A value travels as one opaque segment, so it cannot inject path structure.""" + cases = ( + ('path', '../events', '..%2Fevents'), + ('nested path', '../../base/events', '..%2F..%2Fbase%2Fevents'), + ('leading slash', '/events/123', '%2Fevents%2F123'), + ('absolute url', 'https://test.com/events', 'https%3A%2F%2Ftest.com%2Fevents'), + ('protocol relative url', '//test.com', '%2F%2Ftest.com'), + ('query injection', '123?limit=1', '123%3Flimit%3D1'), + ('fragment injection', '123#fragment', '123%23fragment'), + ('whitespace', 'hello world', 'hello%20world'), + ('non ascii', 'é', '%C3%A9'), + ('pre-encoded', '..%2Fevents', '..%252Fevents'), + ) + + for operation in OPERATIONS: + for name, value, encoded in cases: + with self.subTest(operation=operation.name, case=name): + request_target = self.call(operation, value) + + self.assertEqual(f'/base{operation.prefix}{encoded}', request_target) + + def test_dot_in_value_is_not_touched(self) -> None: + """Test dots in the value stay literal.""" + cases = ( + ('event id', '1708102555327.NLOjmg'), + ('three dots', '...'), + ('leading dot', '.leading'), + ('trailing dot', 'trailing.'), + ) + + for operation in OPERATIONS: + for name, value in cases: + with self.subTest(operation=operation.name, case=name): + request_target = self.call(operation, value) + + self.assertEqual(f'/base{operation.prefix}{value}', request_target) + + def test_dot_segment_is_rejected_without_sending_a_request(self) -> None: + """`.` and `..` are refused""" + for operation in OPERATIONS: + for value in ('.', '..'): + with self.subTest(operation=operation.name, value=value): + self.server.reset() + + with self.assertRaises(InvalidArgumentError) as context: + operation.call(self.api, value) + + self.assertIsNone(self.server.request_target) + self.assertEqual(operation.argument, context.exception.argument) + self.assertEqual(value, context.exception.value) + self.assertIn(operation.argument, str(context.exception)) + + def test_empty_value_does_not_address_the_collection(self) -> None: + """An empty ID leaves a trailing slash""" + for operation in OPERATIONS: + with self.subTest(operation=operation.name): + request_target = self.call(operation, '') + + self.assertEqual(f'/base{operation.prefix}', request_target) + self.assertNotEqual(f'/base{operation.prefix.rstrip("/")}', request_target) + + +if __name__ == '__main__': + unittest.main()