From c4a1aa28ad0204becd917a6ae41599483f3a6029 Mon Sep 17 00:00:00 2001 From: RomirJ Date: Wed, 10 Jun 2026 21:54:49 -0700 Subject: [PATCH 1/2] =?UTF-8?q?fix(runtime):=20serve=20hardening=20?= =?UTF-8?q?=E2=80=94=20guard/reset=20auth,=20os=20NameError,=20wrist=20kwa?= =?UTF-8?q?rg?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit §3.2. Three independent runtime bugs in server.py: 1. /guard/reset was unauthenticated — anyone reachable could clear a tripped safety guard, while /act and /config require the api key. Added the same `_auth: None = Depends(_require_api_key)` dependency. 2. NameError: os in create_app's lifespan — the function imports `os as _os`, but the curate block read `os.environ.get("TETHER_CURATE_DRY_RUN")` (bare os), raising NameError (swallowed) whenever a user opted into curate uploads. Fixed to _os.environ. Verified no bare `os.` remains in the fn. 3. image_wrist_b64 kwarg TypeError — the /act path calls server.predict_from_base64_async(image_wrist_b64=...), but the TetherServer base class only accepted (image_b64, instruction, state), so every wrist-camera request to a single-camera model raised TypeError. Pi05DecomposedServer already accepts + uses it. Added the param to the base class's predict_from_base64[_async]; the single-camera base drops the wrist image with a debug log (not silently) — multi-camera VLAs route to the decomposed server that consumes it. Tests: tests/test_runtime_serve_hardening.py (9) — guard_reset auth via route introspection + TestClient 401/200; no-bare-os scan + compile; wrist-kwarg signature + live-call + cross-class checks. All pass. Implemented by a Sonnet subagent against a verified spec; reviewed + the wrist-drop debug log added on review. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/tether/runtime/server.py | 24 ++- tests/test_runtime_serve_hardening.py | 247 ++++++++++++++++++++++++++ 2 files changed, 269 insertions(+), 2 deletions(-) create mode 100644 tests/test_runtime_serve_hardening.py diff --git a/src/tether/runtime/server.py b/src/tether/runtime/server.py index 63e9ea3..14ba17c 100644 --- a/src/tether/runtime/server.py +++ b/src/tether/runtime/server.py @@ -866,6 +866,7 @@ def predict_from_base64( image_b64: str | None = None, instruction: str = "", state: list[float] | None = None, + image_wrist_b64: str | None = None, ) -> dict[str, Any]: """Predict from base64-encoded image (for HTTP API).""" image = None @@ -879,6 +880,16 @@ def predict_from_base64( except Exception as e: return {"error": f"Failed to decode image: {e}"} + # image_wrist_b64 is accepted for API compat: multi-camera VLAs route to + # Pi05DecomposedServer (which consumes it). TetherServer is single-camera + # and cannot, so the wrist image is dropped here — logged so the drop is + # visible, not silent. + if image_wrist_b64: + logger.debug( + "wrist image ignored: %s is single-camera (use a multi-camera " + "VLA / decomposed export to consume image_wrist).", + type(self).__name__, + ) return self.predict(image=image, instruction=instruction, state=state) # --------------------------------------------------------------- @@ -1063,6 +1074,7 @@ async def predict_from_base64_async( image_b64: str | None = None, instruction: str = "", state: list[float] | None = None, + image_wrist_b64: str | None = None, ) -> dict[str, Any]: """Async base64 entrypoint — decodes image, then routes through batching.""" image = None @@ -1075,6 +1087,14 @@ async def predict_from_base64_async( except Exception as e: return {"error": f"Failed to decode image: {e}"} + # image_wrist_b64 accepted for API compat; single-camera base drops it + # (see predict_from_base64). Logged so the drop is visible, not silent. + if image_wrist_b64: + logger.debug( + "wrist image ignored: %s is single-camera (use a multi-camera " + "VLA / decomposed export to consume image_wrist).", + type(self).__name__, + ) return await self.predict_async(image=image, instruction=instruction, state=state) async def run_batch(self, requests: list) -> list[dict[str, Any]]: @@ -2115,7 +2135,7 @@ async def _heartbeat_loop(): if _curate_consent.is_opted_in(): from tether.curate.uploader import Uploader as _CurateUploader _curate_receipt = _curate_consent.load() - _curate_dry_run = os.environ.get("TETHER_CURATE_DRY_RUN", "").lower() in ("1", "true", "yes") + _curate_dry_run = _os.environ.get("TETHER_CURATE_DRY_RUN", "").lower() in ("1", "true", "yes") _curate_uploader = _CurateUploader( contributor_id=_curate_receipt.contributor_id, tier=_curate_receipt.tier, @@ -2686,7 +2706,7 @@ async def guard_status(): }) @app.post("/guard/reset") - async def guard_reset(): + async def guard_reset(_auth: None = Depends(_require_api_key)): g = getattr(server, "_action_guard", None) if g is None: return JSONResponse( diff --git a/tests/test_runtime_serve_hardening.py b/tests/test_runtime_serve_hardening.py new file mode 100644 index 0000000..fc94b41 --- /dev/null +++ b/tests/test_runtime_serve_hardening.py @@ -0,0 +1,247 @@ +"""Tests for the three runtime/server.py hardening fixes. + +Bug #1: /guard/reset was unauthenticated (no _require_api_key dependency). +Bug #2: NameError: `os` inside the lifespan function that only imports `os as _os`. +Bug #3: TypeError from `image_wrist_b64=` kwarg passed to predict_from_base64_async + which didn't accept that parameter. +""" +from __future__ import annotations + +import inspect +import json + +import pytest + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _build_minimal_export_dir(tmp_path): + """Minimal export dir so create_app() can instantiate without a real model.""" + cfg = { + "model_id": "lerobot/smolvla_base", + "model_type": "smolvla", + "target": "desktop", + "action_chunk_size": 50, + "action_dim": 32, + "expert": {"expert_hidden": 720, "action_dim": 32, "num_layers": 16}, + } + (tmp_path / "tether_config.json").write_text(json.dumps(cfg)) + (tmp_path / "model.onnx").write_bytes(b"\x00") + return tmp_path + + +# --------------------------------------------------------------------------- +# Bug #1 — /guard/reset must require api-key auth +# --------------------------------------------------------------------------- + +class TestGuardResetAuth: + """Verify the /guard/reset route has _require_api_key in its dependencies.""" + + def _get_guard_reset_route(self): + """Build the app and find the /guard/reset route object.""" + try: + from fastapi import FastAPI + except ImportError: + pytest.skip("fastapi not installed") + + from tether.runtime.server import create_app + import tempfile, json as _json, pathlib + + with tempfile.TemporaryDirectory() as d: + p = pathlib.Path(d) + cfg = { + "model_id": "lerobot/smolvla_base", + "model_type": "smolvla", + "target": "desktop", + "action_chunk_size": 50, + "action_dim": 32, + "expert": {"expert_hidden": 720, "action_dim": 32, "num_layers": 16}, + } + (p / "tether_config.json").write_text(_json.dumps(cfg)) + (p / "model.onnx").write_bytes(b"\x00") + app = create_app(str(p), device="cpu", api_key="test-key") + + for route in app.routes: + path = getattr(route, "path", None) + methods = getattr(route, "methods", None) or set() + if path == "/guard/reset" and "POST" in methods: + return route + return None + + def test_guard_reset_route_exists(self): + route = self._get_guard_reset_route() + assert route is not None, "/guard/reset POST route not found in app.routes" + + def test_guard_reset_has_api_key_dependency(self): + """The /guard/reset endpoint's dependant should include _require_api_key.""" + try: + from fastapi import FastAPI + except ImportError: + pytest.skip("fastapi not installed") + + route = self._get_guard_reset_route() + assert route is not None, "/guard/reset POST route not found" + + # FastAPI stores dependencies in route.dependant.dependencies + dependant = getattr(route, "dependant", None) + assert dependant is not None, "route has no dependant" + + dep_calls = [ + dep.call.__name__ if callable(dep.call) else str(dep.call) + for dep in dependant.dependencies + ] + assert "_require_api_key" in dep_calls, ( + f"/guard/reset dependencies do not include _require_api_key; " + f"found: {dep_calls}" + ) + + def test_guard_reset_rejects_unauthenticated(self, tmp_path): + """With api_key set, POST /guard/reset without header → 401.""" + try: + from fastapi import FastAPI + from fastapi.testclient import TestClient + except ImportError: + pytest.skip("fastapi not installed") + + # Build a minimal app that mirrors the three guarded endpoints + from fastapi import Depends, Header, HTTPException + from fastapi.responses import JSONResponse + + app = FastAPI() + api_key = "secret-guard-key" + + async def _require_api_key( + x_tether_key: str | None = Header(default=None, alias="X-Tether-Key"), + ) -> None: + if x_tether_key != api_key: + raise HTTPException(status_code=401, detail="bad key") + + @app.post("/guard/reset") + async def guard_reset(_auth: None = Depends(_require_api_key)): + return JSONResponse(content={"reset": True, "was_tripped": False}) + + client = TestClient(app) + # No key → 401 + assert client.post("/guard/reset").status_code == 401 + # Correct key → 200 + assert client.post( + "/guard/reset", + headers={"X-Tether-Key": api_key}, + ).status_code == 200 + + +# --------------------------------------------------------------------------- +# Bug #2 — no bare `os.` inside the lifespan / startup function +# --------------------------------------------------------------------------- + +class TestOsNameInLifespan: + """Verify the lifespan function body has no bare `os.` (would NameError).""" + + def test_no_bare_os_dot_in_server_py(self): + import re + from pathlib import Path + + src = Path( + "/Users/romirjain/Desktop/building projects/fastcrest/tether/" + "src/tether/runtime/server.py" + ).read_text() + + # Match `os.` NOT preceded by underscore (i.e. not `_os.`). + # Exclude comment lines. + bare_os_lines = [] + for i, line in enumerate(src.splitlines(), start=1): + stripped = line.lstrip() + if stripped.startswith("#"): + continue + if re.search(r"(? Date: Thu, 11 Jun 2026 01:16:31 -0700 Subject: [PATCH 2/2] test(runtime): make hardening tests portable The runtime serve hardening test suite used an absolute macOS checkout path for server.py, which made GitHub CI fail on Linux runners. Resolve server.py relative to the test file instead and clean up the test imports so the new test file is ruff-clean. Tests: PYTHONPATH=src /Users/romirjain/Desktop/building\ projects/fastcrest/tether/.venv/bin/python -m pytest tests/test_runtime_serve_hardening.py -p no:cacheprovider Lint: PYTHONPATH=src /Users/romirjain/Desktop/building\ projects/fastcrest/tether/.venv/bin/ruff check tests/test_runtime_serve_hardening.py Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/test_runtime_serve_hardening.py | 35 +++++++++++---------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/tests/test_runtime_serve_hardening.py b/tests/test_runtime_serve_hardening.py index fc94b41..fa5dd68 100644 --- a/tests/test_runtime_serve_hardening.py +++ b/tests/test_runtime_serve_hardening.py @@ -8,7 +8,10 @@ from __future__ import annotations import inspect +import importlib.util import json +import tempfile +from pathlib import Path import pytest @@ -32,6 +35,10 @@ def _build_minimal_export_dir(tmp_path): return tmp_path +def _server_py_path() -> Path: + return Path(__file__).resolve().parents[1] / "src" / "tether" / "runtime" / "server.py" + + # --------------------------------------------------------------------------- # Bug #1 — /guard/reset must require api-key auth # --------------------------------------------------------------------------- @@ -41,16 +48,13 @@ class TestGuardResetAuth: def _get_guard_reset_route(self): """Build the app and find the /guard/reset route object.""" - try: - from fastapi import FastAPI - except ImportError: + if importlib.util.find_spec("fastapi") is None: pytest.skip("fastapi not installed") from tether.runtime.server import create_app - import tempfile, json as _json, pathlib with tempfile.TemporaryDirectory() as d: - p = pathlib.Path(d) + p = Path(d) cfg = { "model_id": "lerobot/smolvla_base", "model_type": "smolvla", @@ -59,7 +63,7 @@ def _get_guard_reset_route(self): "action_dim": 32, "expert": {"expert_hidden": 720, "action_dim": 32, "num_layers": 16}, } - (p / "tether_config.json").write_text(_json.dumps(cfg)) + (p / "tether_config.json").write_text(json.dumps(cfg)) (p / "model.onnx").write_bytes(b"\x00") app = create_app(str(p), device="cpu", api_key="test-key") @@ -76,9 +80,7 @@ def test_guard_reset_route_exists(self): def test_guard_reset_has_api_key_dependency(self): """The /guard/reset endpoint's dependant should include _require_api_key.""" - try: - from fastapi import FastAPI - except ImportError: + if importlib.util.find_spec("fastapi") is None: pytest.skip("fastapi not installed") route = self._get_guard_reset_route() @@ -141,12 +143,8 @@ class TestOsNameInLifespan: def test_no_bare_os_dot_in_server_py(self): import re - from pathlib import Path - src = Path( - "/Users/romirjain/Desktop/building projects/fastcrest/tether/" - "src/tether/runtime/server.py" - ).read_text() + src = _server_py_path().read_text() # Match `os.` NOT preceded by underscore (i.e. not `_os.`). # Exclude comment lines. @@ -166,13 +164,8 @@ def test_no_bare_os_dot_in_server_py(self): def test_py_compile_server(self): """server.py must compile without errors.""" import py_compile - from pathlib import Path - path = str( - Path( - "/Users/romirjain/Desktop/building projects/fastcrest/tether/" - "src/tether/runtime/server.py" - ) - ) + + path = str(_server_py_path()) # raises py_compile.PyCompileError on failure py_compile.compile(path, doraise=True)