security: fix path traversal and query injection in resource names

CHANGELOG.md

@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Security
+
+- Fixed a path traversal issue where a resource name or id containing `../` was resolved while the request was prepared, retargeting the call at a different API endpoint under the SDK's own credentials (for example `containers.delete_deployment('../../v1/instances')` issued `DELETE /v1/instances`). This also prevents a name from injecting query parameters, such as overriding the `force` flag of `containers.delete_secret`.
+
+  Caller-supplied path values are no longer interpolated into the request path. `HTTPClient.get/post/put/patch/delete` now accept a keyword-only `path_params` mapping whose values are percent-encoded as a single path segment before substitution, and all service modules pass names and ids that way. This covers `instances.is_available()` and `clusters.is_available()`, where the affected value was the `instance_type`/`cluster_type`. As a backstop, `HTTPClient` refuses to send a request whose path would escape the API base path.
+
+  `InferenceClient` paths are validated too: `path` may still span several segments, but it can no longer walk out of the deployment's base url.
+
 ### Added
 
 - `LongTermService` with `get_cluster_periods()` and `get_instance_periods()` methods
@@ -16,6 +24,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- **Breaking:** a resource name or id containing a relative path segment (`.` or `..` between slashes), an empty value, or `None` now raises `ValueError` instead of being sent. Encoding alone is not sufficient for these: `%2E` is decoded back to `.` before the request is sent, and `%2F` is restored by any intermediary that unescapes encoded slashes before normalising the path.
+- **Breaking:** resource names and ids are now percent-encoded, so a value that was already URL-encoded by the caller is encoded again — `get_deployment_by_name('my%20deployment')` now looks up a deployment literally named `my%20deployment` rather than `my deployment`. Pass the raw name instead.
+- **Breaking:** a `/` inside a resource name is now encoded as `%2F` and stays one path segment, where it previously split the route — `delete_registry_credentials('docker.io/myorg')` sends `DELETE /v1/container-registry-credentials/docker.io%2Fmyorg`. Servers that reject or refuse to decode encoded slashes (nginx and Apache with `AllowEncodedSlashes off`) will not match such a name.
+- **Breaking:** a path value that is not a `str`, `int` or `UUID` now raises `ValueError` rather than being coerced with `str()` into a nonsense path segment.
 - Refactored `Image` model to use `@dataclass` and `@dataclass_json` for consistency with `Instance` and `Volume`
 - License changed from MIT to Apache 2.0
 

CLAUDE.md

@@ -71,6 +71,46 @@ verda/<service>/
 - `__init__.py` files do NOT have the Apache 2.0 license header. All other `.py` files do.
 - Implementation files are prefixed with `_` (e.g., `_instances.py`, `_volumes.py`).
 
+## Making API requests
+
+Service modules call the shared `HTTPClient` (`verda/http_client/`), which exposes `get`, `post`, `put`, `patch`, and `delete`.
+
+**Never interpolate a caller-supplied value into the request path.** Resource names and IDs arrive from application input. A value containing `../` is resolved while the request is prepared and retargets the call at a different API endpoint under the SDK's own credentials; a value containing `?` injects query parameters.
+
+Pass such values as `path_params`. The client percent-encodes each one as exactly one path segment before substituting it:
+
+```python
+# correct
+response = self.client.get(
+    CONTAINER_DEPLOYMENTS_ENDPOINT + '/{deployment_name}/status',
+    path_params={'deployment_name': deployment_name},
+)
+
+# wrong -- the name can escape its path segment
+response = self.client.get(f'{CONTAINER_DEPLOYMENTS_ENDPOINT}/{deployment_name}/status')
+```
+
+- The url is a template: a trusted endpoint constant concatenated with a literal containing `{name}` placeholders. Keep it a plain string, never an f-string, so a value cannot be interpolated by accident.
+- Name each placeholder after the parameter it carries (`{deployment_name}`, `{id}`, `{job_name}`).
+- `path_params` goes last in the call, after any positional `json` body or `params` query dict.
+- Paths with no caller input need no `path_params` (e.g. `self.client.get(INSTANCES_ENDPOINT)`).
+
+Endpoint paths belong in a module-level `<NAME>_ENDPOINT` constant, never an inline string literal — a literal hides the call site from the greps and audits used to check this rule.
+
+The client raises `ValueError` for values it cannot make safe — empty/`None`, and any value with a relative path segment (`.` or `..` between slashes) — and for any template/`path_params` mismatch. Non-string values (`int`, `UUID`) are coerced with `str()`.
+
+Dot-segments are rejected rather than encoded because encoding does not hold end to end: `requests` decodes `%2E` back to `.` before sending, and `%2F` is restored by any intermediary that unescapes encoded slashes before normalising the path. A slash on its own is fine and is encoded (`docker.io/myorg` → `docker.io%2Fmyorg`).
+
+Checking a **finished path** for "would a URL parser resolve this away?" is `verda.helpers.has_relative_path_segment`. It strips any query string, decodes the escapes `requests` decodes, and decodes encoded separators (`%2F`, `%5C`) because an intermediary may unescape them before normalising. Use it — do not re-implement it. `InferenceClient` once carried a second copy that missed `%2E%2E`.
+
+`_encode_path_segment` checks the **raw value** instead, on purpose: that value still has `quote()` ahead of it, which escapes `%` and so makes `%2E%2E` and `..%2F..` inert as literal names. Using the finished-path helper there would reject them for no reason. Different question, different check — keep them apart.
+
+`tests/unit_tests/test_path_traversal.py` enforces its own completeness: it reads the source for methods passing `path_params` and fails if any is absent from its `_call_sites` table.
+
+All five verbs delegate to a single private `_request`, which is where the url is built and validated. Add new verbs by delegating to it, never by calling `requests` directly.
+
+Encoding lives in `_encode_path_segment` in `verda/http_client/_http_client.py`. It is private on purpose: `path_params` is the only supported way to put a caller-supplied value into a path, so service modules never hand-roll encoding. As a backstop, `_add_base_url` refuses to send a request whose path would escape the API base path.
+
 ## Code style
 
 ### Formatting and linting
@@ -143,6 +183,7 @@ Ensure two blank lines between the header and the first top-level `class`/`def`
 - **API error tests:** use `pytest.raises(APIException)` and verify `.code` and `.message`
 - **Request matching:** use `responses.add()` with `matchers.json_params_matcher()` to verify request payloads
 - **Test data:** define constants and mock payloads as module-level variables at top of test file
+- **Path traversal regression:** `tests/unit_tests/test_path_traversal.py` drives every method that takes a resource name or id against hostile values. When adding such a method, add it to the `_call_sites` table there.
 
 ## Git and branching