Skip to content
Merged
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
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Security

- `InferenceClient` now checks the request `path` with the same rules as the main API client. A path built from untrusted input could inject query parameters or a fragment into a request that carries the inference key, and dot-segment forms such as `..;/` and `..%252f` passed the old check. `path` may now contain only letters, digits, `-._~`, `/` and `:`, and must not contain a `.` or `..` segment. Anything else raises `InferenceClientError` before the request is sent.

### Added

- `InferenceClient.run_sync`, `InferenceClient.run`, `Deployment.run_sync` and `Deployment.run` accept a `params` argument for query string parameters, like the other `InferenceClient` methods.

### Changed

- **Breaking:** `InferenceClient` no longer accepts a query string or fragment in `path`. Pass query data as `params` instead. For example, replace `client.get('v1/models?limit=10')` with `client.get('v1/models', params={'limit': 10})`, and `client.run_sync(data, path='v1/completions?stream=true')` with `client.run_sync(data, path='v1/completions', params={'stream': 'true'})`.
- **Breaking:** `InferenceClient` now accepts only letters, digits, `-._~`, `/` and `:` in `path`. A path with a percent-encoded value (such as `%20` or `%2F`), a space, a non-ASCII character or another character such as `@`, `+`, `,`, `=` or `;` raises `InferenceClientError`. Send such values in the request body or in `params`.

## [1.25.0] - 2026-10-23

### Deprecated
Expand Down
4 changes: 3 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@ All five verbs delegate to a single private `_request`, which is where the url i

The check is `_encode_path_segment` in `verda/http_client/_http_client.py`; `path_params` is the only supported way to put a caller-supplied value into a path. `_add_base_url` re-asserts the same allowlist on the finished path, as a backstop for a call site that skips `path_params`.

`verda.helpers.has_relative_path_segment` strips the query string and decodes escapes and encoded separators. It is for `InferenceClient` only, whose `path` spans several segments and may carry a query string. Do not use it in the http client, and do not re-implement it.
The finished-path check is `verda.helpers.check_request_path`. It rejects `?`, `#`, any character outside an allowlist, and `.`/`..` segments. `HTTPClient._add_base_url` calls it with the default allowlist (unreserved set plus `/`). `InferenceClient._build_url` calls it on the caller's multi-segment `path` with `_ALLOWED_IN_INFERENCE_PATH`, which also allows `:` (TensorFlow Serving's `my_model:predict`). Query data for both clients goes in `params`, never in the path. Do not re-implement the check.

`verda.helpers.has_relative_path_segment` strips the query string and decodes escapes and encoded separators. It is for `InferenceClient`'s `endpoint_base_url` only. Do not use it to check a request path.

## Code style

Expand Down
153 changes: 143 additions & 10 deletions tests/unit_tests/inference_client/test_inference_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,15 @@
# limitations under the License.

import pytest
import responses # https://github.com/getsentry/responses
from responses import matchers

from verda.inference_client import InferenceClient, InferenceClientError

BASE_URL = 'https://inference.example.com/v1/my-deployment'

VERBS = ['get', 'post', 'put', 'delete', 'patch', 'head', 'options']


@pytest.fixture
def inference_client():
Expand All @@ -36,6 +40,10 @@ class TestBuildUrl:
# multi-segment paths are the documented contract and must keep working
('v1/models/predict', f'{BASE_URL}/v1/models/predict'),
('/v1/models/predict', f'{BASE_URL}/v1/models/predict'),
('v1/chat/completions', f'{BASE_URL}/v1/chat/completions'),
('v1/models/', f'{BASE_URL}/v1/models/'),
# TensorFlow Serving names the method after a ':'
('v1/models/my_model:predict', f'{BASE_URL}/v1/models/my_model:predict'),
],
)
def test_ordinary_paths_are_joined_unchanged(self, inference_client, path, expected):
Expand All @@ -49,29 +57,154 @@ def test_ordinary_paths_are_joined_unchanged(self, inference_client, path, expec
'a/../../b',
'./x',
'..',
# requests decodes percent-encoded unreserved characters before sending,
# so these reach the wire as real dot-segments.
'%2e%2e/%2e%2e/v1/victim',
'%2E%2E/other-deployment',
],
)
def test_paths_that_escape_the_deployment_are_rejected(self, inference_client, path):
# endpoint_base_url ends with this deployment's name, so a dot-segment walks
# the request onto a different deployment while still carrying the caller's
# inference key.
with pytest.raises(InferenceClientError, match='relative path segment'):
with pytest.raises(InferenceClientError, match='escape the API base path'):
inference_client._build_url(path)

@pytest.mark.parametrize(
'path',
['predict?filter=a/../b', 'predict#a/../b', 'predict?q=..'],
['chat?x=1', 'v1#frag', 'predict?filter=a/../b', 'predict?q=..'],
)
def test_dot_segments_inside_a_query_string_are_not_path_traversal(
def test_a_query_string_or_fragment_in_the_path_is_rejected(self, inference_client, path):
# Query data is passed as `params`, so a '?' or '#' here can only come from a
# value interpolated into the path, injecting parameters into a request that
# carries the inference key.
with pytest.raises(InferenceClientError, match='query string or fragment'):
inference_client._build_url(path)

@pytest.mark.parametrize(
'path',
[
# requests decodes percent-encoded unreserved characters before sending,
# so these reach the wire as real dot-segments.
'%2e%2e/x',
'%2e%2e/%2e%2e/v1/victim',
'%2E%2E/other-deployment',
# a decoding check misses these, but a normalising proxy may not
'..%252fx',
'..%2Fx',
'..;/x',
'a\\b',
'a;b',
'v1/my model',
],
)
def test_an_escape_or_a_character_outside_the_allowlist_is_rejected(
self, inference_client, path
):
# Only the path is resolved by a URL parser; a value inside the query string
# is not, so rejecting it would break callers passing an inline query.
assert inference_client._build_url(path) == f'{BASE_URL}/{path}'
with pytest.raises(InferenceClientError, match='unencoded value'):
inference_client._build_url(path)


class TestRequestPaths:
@pytest.mark.parametrize('verb', VERBS)
@responses.activate
def test_params_are_sent_as_the_query_string(self, inference_client, verb):
# arrange
responses.add(
verb.upper(),
f'{BASE_URL}/v1/models',
match=[matchers.query_param_matcher({'filter': 'a/../b'})],
)

# act
response = getattr(inference_client, verb)('v1/models', params={'filter': 'a/../b'})

# assert
assert response.status_code == 200

@pytest.mark.parametrize('verb', VERBS)
@responses.activate
def test_a_rejected_path_sends_no_request(self, inference_client, verb):
with pytest.raises(InferenceClientError, match='query string or fragment'):
getattr(inference_client, verb)('chat?x=1')

assert len(responses.calls) == 0

@responses.activate
def test_run_sync_posts_to_the_path(self, inference_client):
# arrange
responses.add(responses.POST, f'{BASE_URL}/v1/chat/completions', json={'ok': True})

# act
response = inference_client.run_sync({'prompt': 'hi'}, path='v1/chat/completions')

# assert
assert response.status_code == 200

@responses.activate
def test_run_posts_to_the_path(self, inference_client):
# arrange
responses.add(responses.POST, f'{BASE_URL}/v1/chat/completions', json={'Id': 'abc'})

# act
execution = inference_client.run({'prompt': 'hi'}, path='v1/chat/completions')

# assert
assert execution.id == 'abc'

@responses.activate
def test_run_sync_sends_params_as_the_query_string(self, inference_client):
# arrange
responses.add(
responses.POST,
f'{BASE_URL}/v1/completions',
json={'ok': True},
match=[matchers.query_param_matcher({'stream': 'true'})],
)

# act
response = inference_client.run_sync(
{'prompt': 'hi'}, path='v1/completions', params={'stream': 'true'}
)

# assert
assert response.status_code == 200

@pytest.mark.parametrize('no_response', [False, True])
@responses.activate
def test_run_sends_params_as_the_query_string(self, inference_client, no_response):
# arrange
responses.add(
responses.POST,
f'{BASE_URL}/v1/completions',
json={'Id': 'abc'},
match=[matchers.query_param_matcher({'stream': 'true'})],
)

# act
inference_client.run(
{'prompt': 'hi'},
path='v1/completions',
no_response=no_response,
params={'stream': 'true'},
)

# assert
assert len(responses.calls) == 1

@responses.activate
def test_health_gets_the_health_path(self, inference_client):
# arrange
responses.add(responses.GET, f'{BASE_URL}/health')

# act
response = inference_client.health()

# assert
assert response.status_code == 200

@responses.activate
def test_run_sync_rejects_a_query_string_in_the_path(self, inference_client):
with pytest.raises(InferenceClientError, match='query string or fragment'):
inference_client.run_sync({'prompt': 'hi'}, path='v1/completions?stream=true')

assert len(responses.calls) == 0


class TestEndpointBaseUrl:
Expand Down
10 changes: 8 additions & 2 deletions verda/containers/_containers.py
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,7 @@ def run_sync(
headers: dict[str, str] | None = None,
http_method: str = 'POST',
stream: bool = False,
params: dict[str, Any] | None = None,
) -> InferenceResponse:
"""Runs a synchronous inference request.

Expand All @@ -501,6 +502,7 @@ def run_sync(
headers: Optional headers to include in the request.
http_method: The HTTP method to use for the request.
stream: Whether to stream the response.
params: Optional query string parameters.

Returns:
InferenceResponse: The response from the inference request.
Expand All @@ -510,7 +512,7 @@ def run_sync(
"""
self._validate_inference_client()
return self._inference_client.run_sync(
data, path, timeout_seconds, headers, http_method, stream
data, path, timeout_seconds, headers, http_method, stream, params=params
)

def run(
Expand All @@ -521,6 +523,7 @@ def run(
headers: dict[str, str] | None = None,
http_method: str = 'POST',
stream: bool = False,
params: dict[str, Any] | None = None,
):
"""Runs an asynchronous inference request.

Expand All @@ -531,6 +534,7 @@ def run(
headers: Optional headers to include in the request.
http_method: The HTTP method to use for the request.
stream: Whether to stream the response.
params: Optional query string parameters.

Returns:
The response from the inference request.
Expand All @@ -539,7 +543,9 @@ def run(
ValueError: If the inference client is not initialized.
"""
self._validate_inference_client()
return self._inference_client.run(data, path, timeout_seconds, headers, http_method, stream)
return self._inference_client.run(
data, path, timeout_seconds, headers, http_method, stream, params=params
)

def health(self):
"""Checks the health of the deployed application.
Expand Down
36 changes: 36 additions & 0 deletions verda/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
# retargeted at a different endpoint.
_RELATIVE_SEGMENTS = frozenset({'.', '..'})

# The RFC 3986 unreserved set plus the '/' between path segments.
_ALLOWED_IN_PATH = re.compile(r'^[A-Za-z0-9._~/-]*$')

# A percent-escape of an unreserved character (RFC 3986, section 2.3). `requests`
# decodes these while preparing a request, so '%2E' becomes '.' before it is sent.
_ESCAPED_UNRESERVED = re.compile(r'%([0-9A-Fa-f]{2})')
Expand Down Expand Up @@ -80,6 +83,39 @@ def has_relative_path_segment(path: str) -> bool:
return bool(_RELATIVE_SEGMENTS.intersection(decoded.split('/')))


def check_request_path(path: str, allowed: re.Pattern[str] = _ALLOWED_IN_PATH) -> None:
r"""Check a finished request path cannot be retargeted or carry a query string.

Rejects rather than decodes: ``requests`` decodes ``%2E`` back to ``.`` before
sending, and an intermediary that unescapes ``%2F`` or treats ``\`` as a separator
before normalising the path restores a traversal. Refusing ``%`` and ``\``
outright also covers forms a decoder would miss, such as the double-encoded
``..%252f``, and the allowlist refuses matrix parameters such as ``..;``.

Args:
path: A url path, relative to a trusted base url.
allowed: Pattern the whole path must match. Defaults to the unreserved set
plus ``/``.

Raises:
ValueError: If the path contains a query string or fragment, a character the
pattern does not allow, or a relative path segment.
"""
# Query data is passed separately as `params`.
if '?' in path or '#' in path:
raise ValueError(f'request path must not contain a query string or fragment: {path!r}')

# Catches '%', which requests may decode into a separator or a dot, and '\',
# which some servers normalise to '/'.
if not allowed.match(path):
raise ValueError(f'request path must not contain an unencoded value: {path!r}')

if _RELATIVE_SEGMENTS.intersection(path.split('/')):
raise ValueError(
f'refusing to send a request whose path would escape the API base path: {path!r}'
)


def stringify_class_object_properties(class_object: type) -> str:
"""Generates a json string representation of a class object's properties and values.

Expand Down
19 changes: 2 additions & 17 deletions verda/http_client/_http_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

from verda._version import __version__
from verda.exceptions import APIException
from verda.helpers import _RELATIVE_SEGMENTS
from verda.helpers import _RELATIVE_SEGMENTS, check_request_path

# A `{name}` placeholder in a relative url template.
_PLACEHOLDER = re.compile(r'\{(\w+)\}')
Expand All @@ -30,9 +30,6 @@
# position is a slug, an id or a machine type: 'my-deployment', '1A100.22V', a UUID.
_UNSAFE_IN_SEGMENT = re.compile(r'[^A-Za-z0-9._~-]')

# The unreserved set plus the '/' between the segments of an endpoint constant.
_ALLOWED_IN_PATH = re.compile(r'^[A-Za-z0-9._~/-]*$')


def _encode_path_segment(value: object) -> str:
"""Check a caller-supplied value is safe as a single URL path segment.
Expand Down Expand Up @@ -421,22 +418,10 @@ def _add_base_url(self, url: str) -> str:
Raises:
ValueError: If the path could escape the API base path.
"""
# Query data is passed separately as `params`.
if '?' in url or '#' in url:
raise ValueError(f'request path must not contain a query string or fragment: {url!r}')

# Catches '%', which requests may decode into a separator or a dot, and '\',
# which some servers normalise to '/'.
if not _ALLOWED_IN_PATH.match(url):
raise ValueError(f'request path must not contain an unencoded value: {url!r}')
check_request_path(url)

# An empty segment collapses a delete-one call onto the collection endpoint.
if any(segment == '' for segment in url.split('/')[1:]):
raise ValueError(f'request path must not contain an empty path segment: {url!r}')

if _RELATIVE_SEGMENTS.intersection(url.split('/')):
raise ValueError(
f'refusing to send a request whose path would escape the API base path: {url!r}'
)

return self._base_url + url
Loading
Loading