diff --git a/backend/fluksio/mcp/server.py b/backend/fluksio/mcp/server.py index 94432ce..5c7b5a8 100644 --- a/backend/fluksio/mcp/server.py +++ b/backend/fluksio/mcp/server.py @@ -15,12 +15,13 @@ from __future__ import annotations import logging from contextvars import ContextVar from typing import Any -from urllib.parse import quote import httpx from mcp.server.fastmcp import FastMCP from mcp.server.lowlevel.server import request_ctx +from fluksio.sdk.client import seg as _seg + logger = logging.getLogger(__name__) mcp = FastMCP("fluksio") @@ -86,17 +87,6 @@ async def forward_caller_auth(request: httpx.Request) -> None: request.headers["authorization"] = authorization -def _seg(value: str) -> str: - """*value* as one path segment, and nothing more. - - httpx resolves ``..`` before sending, so a name put into a path as written - could climb out of the route it was meant for: ``save_flow("../ai/config")`` - was ``PUT /ai/config``. ``quote`` covers the slashes but leaves dots alone, - hence encoding those too; the server decodes both. - """ - return quote(value, safe="").replace(".", "%2E") - - async def _call(method: str, path: str, **kwargs: Any) -> Any: """One REST call, with failures handed back as data rather than raised. diff --git a/backend/fluksio/sdk/client.py b/backend/fluksio/sdk/client.py index db7ad9d..0ef0ded 100644 --- a/backend/fluksio/sdk/client.py +++ b/backend/fluksio/sdk/client.py @@ -15,6 +15,7 @@ import uuid from collections.abc import Iterable from pathlib import Path from typing import Any +from urllib.parse import quote from fluksio.sdk import MARKER, Flow, SyncError @@ -27,6 +28,7 @@ __all__ = [ "find_data_dir", "ignore_self", "login", + "seg", "sync", "write_config", ] @@ -126,6 +128,17 @@ def _stored() -> dict[str, str]: return {} +def seg(value: str) -> str: + """*value* as one path segment, and nothing more. + + httpx resolves ``..`` before sending, so a name put into a path as written + could climb out of the route it was meant for: ``get_flow("../ai/config")`` + was ``GET /ai/config``. ``quote`` covers the slashes but leaves dots alone, + hence encoding those too; the server decodes both. + """ + return quote(value, safe="").replace(".", "%2E") + + class ApiError(Exception): """The engine refused, with what it said.""" @@ -226,7 +239,7 @@ class Client: def get_flow(self, name: str) -> dict[str, Any] | None: """The stored flow, draft included, or ``None`` if there is none.""" try: - result: dict[str, Any] = self._call("GET", f"/flows/{name}") + result: dict[str, Any] = self._call("GET", f"/flows/{seg(name)}") except ApiError as exc: if exc.status == 404: return None @@ -235,7 +248,7 @@ class Client: def put_flow(self, document: dict[str, Any]) -> dict[str, Any]: result: dict[str, Any] = self._call( - "PUT", f"/flows/{document['name']}", json=document + "PUT", f"/flows/{seg(document['name'])}", json=document ) return result @@ -249,18 +262,20 @@ class Client: somebody edited: the engine answers a new-node template for both, and only one of them is work worth refusing to overwrite. """ - result: dict[str, Any] = self._call("GET", f"/flows/{flow}/nodes/{node}/source") + result: dict[str, Any] = self._call( + "GET", f"/flows/{seg(flow)}/nodes/{seg(node)}/source" + ) return result or {} def put_source(self, flow: str, node: str, code: str) -> dict[str, Any]: result: dict[str, Any] = self._call( - "PUT", f"/flows/{flow}/nodes/{node}/source", json={"code": code} + "PUT", f"/flows/{seg(flow)}/nodes/{seg(node)}/source", json={"code": code} ) return result def publish(self, name: str, version: int) -> dict[str, Any]: result: dict[str, Any] = self._call( - "POST", f"/flows/{name}/publish", json={"version": version} + "POST", f"/flows/{seg(name)}/publish", json={"version": version} ) return result @@ -345,7 +360,7 @@ class Client: """ row = self._call( "POST", - f"/runs/flows/{flow}", + f"/runs/flows/{seg(flow)}", idempotent=True, json={ "params": params or {}, @@ -371,7 +386,7 @@ class Client: keyed = [{**entry, "idempotency_key": uuid.uuid4().hex} for entry in entries] rows = self._call( "POST", - f"/runs/flows/{flow}/sweep", + f"/runs/flows/{seg(flow)}/sweep", idempotent=True, json={"runs": keyed, "no_cache": no_cache}, ) @@ -397,7 +412,7 @@ class Client: return answers def run(self, run_id: str) -> dict[str, Any]: - result: dict[str, Any] = self._call("GET", f"/runs/{run_id}") + result: dict[str, Any] = self._call("GET", f"/runs/{seg(run_id)}") return result def runs(self, flow: str = "", limit: int = 20, **filters: Any) -> Any: @@ -410,7 +425,7 @@ class Client: query: dict[str, Any] = {"stride": stride} if name: query["name"] = name - return self._call("GET", f"/runs/{run_id}/metrics", params=query) + return self._call("GET", f"/runs/{seg(run_id)}/metrics", params=query) def metric_names( self, ids: Iterable[str] = (), flow: str = "", **filters: Any @@ -512,14 +527,14 @@ class Client: def cancel(self, run_id: str) -> Any: # Cancelling a cancelled run is cancelled. - return self._call("POST", f"/runs/{run_id}/cancel", idempotent=True) + return self._call("POST", f"/runs/{seg(run_id)}/cancel", idempotent=True) def retry(self, run_id: str) -> Any: """Submit the same run again. Not idempotent: it makes a run.""" - return self._call("POST", f"/runs/{run_id}/retry") + return self._call("POST", f"/runs/{seg(run_id)}/retry") def download(self, digest: str) -> bytes: - response = self._request("GET", f"/artifacts/{digest}", idempotent=True) + response = self._request("GET", f"/artifacts/{seg(digest)}", idempotent=True) if response.status_code >= 400: raise ApiError(response.status_code, _detail(response)) return bytes(response.content) diff --git a/backend/tests/sdk/test_client.py b/backend/tests/sdk/test_client.py index bab6c18..8092f7b 100644 --- a/backend/tests/sdk/test_client.py +++ b/backend/tests/sdk/test_client.py @@ -181,3 +181,23 @@ def test_a_history_is_imported_a_page_at_a_time(monkeypatch): assert [one["external_id"] for one in answers] == [ e["external_id"] for e in entries ] + + +def test_a_name_cannot_climb_out_of_its_route(monkeypatch): + """httpx resolves ``..``, so ``get_flow("../x")`` was ``GET /api/v1/x``.""" + seen = [] + + def handler(request): + seen.append(request.url.raw_path.decode()) + return httpx.Response(200, json={}) + + client = a_client(handler, monkeypatch) + client.get_flow("../x") + client.put_source("..", "../x", "code") + client.run("..") + + assert seen == [ + "/api/v1/flows/%2E%2E%2Fx", + "/api/v1/flows/%2E%2E/nodes/%2E%2E%2Fx/source", + "/api/v1/runs/%2E%2E", + ]