From 367d137f9140cf7e55bb0b5aa435f508dcd89356 Mon Sep 17 00:00:00 2001 From: Noor Fatima Date: Wed, 30 Sep 2026 00:53:56 +0500 Subject: [PATCH 1/2] fix(runtime): handle null and malformed GraphQL data in Caido loginAsGuest (#1104) - Safely validate nested GraphQL response structure when logging in as guest in both caido_bootstrap.py and caido_api.py. - Prevent crashes when 'data' is null, non-dict, or lacks expected loginAsGuest/token fields. - Retry on session.exec exceptions and handle non-zero exit codes with None or byte stderrs. - Reject whitespace-only tokens. - Add comprehensive regression and edge-case unit tests. --- strix/runtime/caido_bootstrap.py | 72 ++++++--- strix/tools/proxy/caido_api.py | 29 +++- tests/test_caido_bootstrap.py | 259 ++++++++++++++++++++++++++++++- 3 files changed, 329 insertions(+), 31 deletions(-) diff --git a/strix/runtime/caido_bootstrap.py b/strix/runtime/caido_bootstrap.py index a9c7c82ae..ff663c404 100644 --- a/strix/runtime/caido_bootstrap.py +++ b/strix/runtime/caido_bootstrap.py @@ -13,7 +13,7 @@ import asyncio import contextlib import json import logging -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any if TYPE_CHECKING: @@ -29,6 +29,32 @@ _LOGIN_AS_GUEST_BODY = ( ) +def _extract_access_token(payload: Any) -> str | None: + if not isinstance(payload, dict): + return None + data = payload.get("data") + if not isinstance(data, dict): + return None + login = data.get("loginAsGuest") + if not isinstance(login, dict): + return None + token_obj = login.get("token") + if not isinstance(token_obj, dict): + return None + token = token_obj.get("accessToken") + if isinstance(token, str) and token.strip(): + return token.strip() + return None + + +def _format_stderr(raw_stderr: Any, max_len: int = 200) -> str: + if isinstance(raw_stderr, (bytes, bytearray)): + return raw_stderr.decode("utf-8", errors="replace")[:max_len] + if raw_stderr is not None: + return str(raw_stderr)[:max_len] + return "" + + async def _login_as_guest( session: BaseSandboxSession, *, @@ -43,34 +69,36 @@ async def _login_as_guest( """ last_err: str | None = None for i in range(1, attempts + 1): - result = await session.exec( - "curl", - "-fsS", - "-X", - "POST", - "-H", - "Content-Type: application/json", - "-d", - _LOGIN_AS_GUEST_BODY, - f"{container_url}/graphql", - timeout=15, - ) + try: + result = await session.exec( + "curl", + "-fsS", + "-X", + "POST", + "-H", + "Content-Type: application/json", + "-d", + _LOGIN_AS_GUEST_BODY, + f"{container_url}/graphql", + timeout=15, + ) + except Exception as exc: # noqa: BLE001 + last_err = f"session.exec failed: {exc}" + logger.debug("loginAsGuest attempt %d/%d failed: %s", i, attempts, last_err) + await asyncio.sleep(min(2.0 * i, 8.0)) + continue + if result.ok(): try: payload = json.loads(result.stdout) - token = ( - payload.get("data", {}) - .get("loginAsGuest", {}) - .get("token", {}) - .get("accessToken") - ) + token = _extract_access_token(payload) if token: - return str(token) + return token last_err = f"loginAsGuest returned no token: {payload}" - except json.JSONDecodeError as exc: + except (json.JSONDecodeError, AttributeError, TypeError) as exc: last_err = f"unparseable response: {exc}: {result.stdout!r}" else: - stderr = result.stderr.decode("utf-8", errors="replace")[:200] + stderr = _format_stderr(result.stderr) last_err = f"curl exit {result.exit_code}: {stderr}" logger.debug("loginAsGuest attempt %d/%d failed: %s", i, attempts, last_err) await asyncio.sleep(min(2.0 * i, 8.0)) diff --git a/strix/tools/proxy/caido_api.py b/strix/tools/proxy/caido_api.py index af81b5008..2f3145020 100644 --- a/strix/tools/proxy/caido_api.py +++ b/strix/tools/proxy/caido_api.py @@ -6,6 +6,7 @@ import asyncio import json import os import time +import urllib.error import urllib.request from typing import TYPE_CHECKING, Any, Literal from urllib.parse import parse_qs, urlencode, urlparse, urlunparse @@ -75,9 +76,31 @@ def _login_as_guest() -> str: headers={"Content-Type": "application/json"}, method="POST", ) - with urllib.request.urlopen(req, timeout=10) as resp: # noqa: S310 # nosec B310 - payload = json.loads(resp.read()) - return str(payload["data"]["loginAsGuest"]["token"]["accessToken"]) + try: + with urllib.request.urlopen(req, timeout=10) as resp: # noqa: S310 # nosec B310 + raw_resp = resp.read() + except urllib.error.HTTPError as err: + raw_resp = err.read() + except urllib.error.URLError as err: + raise RuntimeError(f"Failed to connect to Caido at {_graphql_url()}: {err}") from err + + try: + payload = json.loads(raw_resp) + except (json.JSONDecodeError, TypeError) as err: + raise RuntimeError(f"Unparseable response from Caido: {raw_resp!r}") from err + + token = None + if isinstance(payload, dict): + data = payload.get("data") + if isinstance(data, dict): + login = data.get("loginAsGuest") + if isinstance(login, dict): + token_obj = login.get("token") + if isinstance(token_obj, dict): + token = token_obj.get("accessToken") + if not token or not isinstance(token, str) or not token.strip(): + raise RuntimeError(f"loginAsGuest returned no token: {payload}") + return token.strip() async def _new_client() -> Client: diff --git a/tests/test_caido_bootstrap.py b/tests/test_caido_bootstrap.py index 98e42a94d..10da9a1bd 100644 --- a/tests/test_caido_bootstrap.py +++ b/tests/test_caido_bootstrap.py @@ -8,24 +8,33 @@ client exists but no caller will ever see it to close it. from __future__ import annotations import asyncio +import io import sys import types +import urllib.error +import urllib.request from typing import Any +from unittest.mock import MagicMock import pytest -from strix.runtime.caido_bootstrap import bootstrap_caido +from strix.runtime.caido_bootstrap import _login_as_guest, bootstrap_caido +from strix.tools.proxy import caido_api class _FakeExecResult: - stderr = b"" - exit_code = 0 - - def __init__(self, stdout: str) -> None: + def __init__( + self, + stdout: str, + stderr: Any = b"", + exit_code: int = 0, + ) -> None: self.stdout = stdout + self.stderr = stderr + self.exit_code = exit_code def ok(self) -> bool: - return True + return self.exit_code == 0 class _FakeSession: @@ -79,3 +88,241 @@ async def test_cancellation_during_connect_closes_the_client( async def test_failed_connect_closes_the_client(monkeypatch: pytest.MonkeyPatch) -> None: client = await _bootstrap_expecting(monkeypatch, RuntimeError("no listener")) assert client.closed + + +class _MultiResultSession: + def __init__(self, responses: list[str]) -> None: + self._responses = list(responses) + self.call_count = 0 + + async def exec(self, *_args: Any, **_kwargs: Any) -> _FakeExecResult: + self.call_count += 1 + resp = self._responses.pop(0) if self._responses else "{}" + return _FakeExecResult(resp) + + +async def _instant_sleep(_s: float) -> None: + pass + + +async def test_login_as_guest_retries_when_data_is_null( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(asyncio, "sleep", _instant_sleep) + session = _MultiResultSession([ + '{"data": null, "errors": [{"message": "Could not acquire a connection to the database"}]}', + '{"data":{"loginAsGuest":{"token":{"accessToken":"retry_ok_token"}}}}', + ]) + token = await _login_as_guest( + session, # type: ignore[arg-type] + container_url="http://container", + attempts=3, + ) + assert token == "retry_ok_token" # noqa: S105 + assert session.call_count == 2 + + +async def test_login_as_guest_retries_when_login_as_guest_is_null( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(asyncio, "sleep", _instant_sleep) + session = _MultiResultSession([ + '{"data": {"loginAsGuest": null}}', + '{"data":{"loginAsGuest":{"token":{"accessToken":"retry_ok_token"}}}}', + ]) + token = await _login_as_guest( + session, # type: ignore[arg-type] + container_url="http://container", + attempts=3, + ) + assert token == "retry_ok_token" # noqa: S105 + assert session.call_count == 2 + + +async def test_login_as_guest_retries_when_token_is_null( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(asyncio, "sleep", _instant_sleep) + session = _MultiResultSession([ + '{"data": {"loginAsGuest": {"token": null}}}', + '{"data":{"loginAsGuest":{"token":{"accessToken":"retry_ok_token"}}}}', + ]) + token = await _login_as_guest( + session, # type: ignore[arg-type] + container_url="http://container", + attempts=3, + ) + assert token == "retry_ok_token" # noqa: S105 + assert session.call_count == 2 + + +async def test_login_as_guest_retries_when_data_is_non_dict( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(asyncio, "sleep", _instant_sleep) + session = _MultiResultSession([ + '{"data": "unexpected_string"}', + '{"data": [1, 2, 3]}', + '{"data":{"loginAsGuest":{"token":{"accessToken":"retry_ok_token"}}}}', + ]) + token = await _login_as_guest( + session, # type: ignore[arg-type] + container_url="http://container", + attempts=4, + ) + assert token == "retry_ok_token" # noqa: S105 + assert session.call_count == 3 + + +async def test_login_as_guest_fails_when_all_attempts_return_null_data( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(asyncio, "sleep", _instant_sleep) + session = _MultiResultSession([ + '{"data": null, "errors": [{"message": "busy"}]}', + '{"data": null, "errors": [{"message": "busy"}]}', + ]) + with pytest.raises(RuntimeError, match="loginAsGuest failed after 2 attempts"): + await _login_as_guest( + session, # type: ignore[arg-type] + container_url="http://container", + attempts=2, + ) + assert session.call_count == 2 + + +def test_caido_api_login_as_guest_null_data(monkeypatch: pytest.MonkeyPatch) -> None: + mock_resp = MagicMock() + mock_resp.read.return_value = b'{"data": null, "errors": [{"message": "busy"}]}' + mock_resp.__enter__.return_value = mock_resp + mock_resp.__exit__.return_value = None + + monkeypatch.setattr(urllib.request, "urlopen", lambda *_a, **_k: mock_resp) + + with pytest.raises(RuntimeError, match="loginAsGuest returned no token"): + caido_api._login_as_guest() + + +def test_caido_api_login_as_guest_malformed_data(monkeypatch: pytest.MonkeyPatch) -> None: + mock_resp = MagicMock() + mock_resp.read.return_value = b'{"data": "invalid_string_instead_of_dict"}' + mock_resp.__enter__.return_value = mock_resp + mock_resp.__exit__.return_value = None + + monkeypatch.setattr(urllib.request, "urlopen", lambda *_a, **_k: mock_resp) + + with pytest.raises(RuntimeError, match="loginAsGuest returned no token"): + caido_api._login_as_guest() + + +def test_caido_api_login_as_guest_success(monkeypatch: pytest.MonkeyPatch) -> None: + mock_resp = MagicMock() + mock_resp.read.return_value = ( + b'{"data": {"loginAsGuest": {"token": {"accessToken": "host_token_123"}}}}' + ) + mock_resp.__enter__.return_value = mock_resp + mock_resp.__exit__.return_value = None + + monkeypatch.setattr(urllib.request, "urlopen", lambda *_a, **_k: mock_resp) + + assert caido_api._login_as_guest() == "host_token_123" + + +async def test_login_as_guest_retries_when_session_exec_raises_exception( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(asyncio, "sleep", _instant_sleep) + + class _ExceptionThenSuccessSession: + def __init__(self) -> None: + self.call_count = 0 + + async def exec(self, *_args: Any, **_kwargs: Any) -> _FakeExecResult: + self.call_count += 1 + if self.call_count == 1: + raise TimeoutError("container exec timed out") + return _FakeExecResult( + '{"data":{"loginAsGuest":{"token":{"accessToken":"retry_after_exec_err"}}}}' + ) + + session = _ExceptionThenSuccessSession() + token = await _login_as_guest( + session, # type: ignore[arg-type] + container_url="http://container", + attempts=3, + ) + assert token == "retry_after_exec_err" # noqa: S105 + assert session.call_count == 2 + + +async def test_login_as_guest_handles_none_stderr_on_curl_failure( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(asyncio, "sleep", _instant_sleep) + + class _NoneStderrThenSuccessSession: + def __init__(self) -> None: + self.call_count = 0 + + async def exec(self, *_args: Any, **_kwargs: Any) -> _FakeExecResult: + self.call_count += 1 + if self.call_count == 1: + return _FakeExecResult("", stderr=None, exit_code=7) + return _FakeExecResult( + '{"data":{"loginAsGuest":{"token":{"accessToken":"retry_after_none_stderr"}}}}' + ) + + session = _NoneStderrThenSuccessSession() + token = await _login_as_guest( + session, # type: ignore[arg-type] + container_url="http://container", + attempts=3, + ) + assert token == "retry_after_none_stderr" # noqa: S105 + assert session.call_count == 2 + + +async def test_login_as_guest_rejects_whitespace_token( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(asyncio, "sleep", _instant_sleep) + session = _MultiResultSession([ + '{"data":{"loginAsGuest":{"token":{"accessToken":" "}}}}', + '{"data":{"loginAsGuest":{"token":{"accessToken":"valid_token"}}}}', + ]) + token = await _login_as_guest( + session, # type: ignore[arg-type] + container_url="http://container", + attempts=3, + ) + assert token == "valid_token" # noqa: S105 + assert session.call_count == 2 + + +def test_caido_api_login_as_guest_http_500_with_graphql_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + err_body = b'{"errors": [{"message": "database locked"}]}' + http_err = urllib.error.HTTPError( + url="http://127.0.0.1:8080/graphql", + code=500, + msg="Internal Server Error", + hdrs={}, # type: ignore[arg-type] + fp=io.BytesIO(err_body), + ) + + monkeypatch.setattr(urllib.request, "urlopen", MagicMock(side_effect=http_err)) + + with pytest.raises(RuntimeError, match="database locked"): + caido_api._login_as_guest() + + +def test_caido_api_login_as_guest_url_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + url_err = urllib.error.URLError("Connection refused") + monkeypatch.setattr(urllib.request, "urlopen", MagicMock(side_effect=url_err)) + + with pytest.raises(RuntimeError, match="Failed to connect to Caido"): + caido_api._login_as_guest() + From 1cfa32b114a15fa6eed9aac80a01422843e3ce17 Mon Sep 17 00:00:00 2001 From: Noor Fatima Date: Wed, 30 Sep 2026 01:28:17 +0500 Subject: [PATCH 2/2] fix(runtime): retain HTTP error status and fail promptly on permanent transport failures - Preserve HTTP status codes alongside error details when Caido returns an HTTPError in caido_api._login_as_guest. - Distinguish permanent transport and execution failures (ExecTransportError, WorkspaceStopError, exit 126/127) from transient Caido readiness probes in caido_bootstrap._login_as_guest. - Add unit tests verifying prompt failure on permanent errors and HTTP status retention. --- strix/runtime/caido_bootstrap.py | 46 +++++++++++++- strix/tools/proxy/caido_api.py | 7 ++- tests/test_caido_bootstrap.py | 102 ++++++++++++++++++++++++++++++- 3 files changed, 152 insertions(+), 3 deletions(-) diff --git a/strix/runtime/caido_bootstrap.py b/strix/runtime/caido_bootstrap.py index ff663c404..fb88a1552 100644 --- a/strix/runtime/caido_bootstrap.py +++ b/strix/runtime/caido_bootstrap.py @@ -28,6 +28,42 @@ _LOGIN_AS_GUEST_BODY = ( '{"query":"mutation LoginAsGuest { loginAsGuest { token { accessToken } } }"}' ) +_PERMANENT_EXIT_CODES = frozenset({ + 2, # curl: failed to initialize / bad command-line syntax + 3, # curl: malformed URL + 126, # command invoked cannot execute (permission denied) + 127, # command not found (curl is not installed in the sandbox image) +}) + + +def _is_permanent_transport_error(exc: BaseException) -> bool: + """Return True if the sandbox transport has failed permanently and cannot recover.""" + if isinstance(exc, asyncio.CancelledError): + return True + + if getattr(exc, "retryable", None) is False: + return True + + try: + from agents.sandbox.errors import ExecTransportError, WorkspaceStopError + + if isinstance(exc, WorkspaceStopError): + return True + if isinstance(exc, ExecTransportError) and not exc.context.get("retry_safe", False): + return True + except ImportError: + pass + + try: + from docker import errors as docker_errors # type: ignore[import-untyped, unused-ignore] + + if isinstance(exc, (docker_errors.NotFound, docker_errors.APIError)): + return True + except ImportError: + pass + + return False + def _extract_access_token(payload: Any) -> str | None: if not isinstance(payload, dict): @@ -82,7 +118,11 @@ async def _login_as_guest( f"{container_url}/graphql", timeout=15, ) - except Exception as exc: # noqa: BLE001 + except Exception as exc: + if _is_permanent_transport_error(exc): + raise RuntimeError( + f"loginAsGuest failed permanently: session.exec failed: {exc}" + ) from exc last_err = f"session.exec failed: {exc}" logger.debug("loginAsGuest attempt %d/%d failed: %s", i, attempts, last_err) await asyncio.sleep(min(2.0 * i, 8.0)) @@ -99,6 +139,10 @@ async def _login_as_guest( last_err = f"unparseable response: {exc}: {result.stdout!r}" else: stderr = _format_stderr(result.stderr) + if result.exit_code in _PERMANENT_EXIT_CODES: + raise RuntimeError( + f"loginAsGuest failed permanently: curl exit {result.exit_code}: {stderr}" + ) last_err = f"curl exit {result.exit_code}: {stderr}" logger.debug("loginAsGuest attempt %d/%d failed: %s", i, attempts, last_err) await asyncio.sleep(min(2.0 * i, 8.0)) diff --git a/strix/tools/proxy/caido_api.py b/strix/tools/proxy/caido_api.py index 2f3145020..392477925 100644 --- a/strix/tools/proxy/caido_api.py +++ b/strix/tools/proxy/caido_api.py @@ -80,7 +80,12 @@ def _login_as_guest() -> str: with urllib.request.urlopen(req, timeout=10) as resp: # noqa: S310 # nosec B310 raw_resp = resp.read() except urllib.error.HTTPError as err: - raw_resp = err.read() + body = err.read() + try: + payload = json.loads(body) + except (json.JSONDecodeError, TypeError): + payload = body.decode("utf-8", errors="replace")[:200].strip() or err.reason + raise RuntimeError(f"Caido loginAsGuest returned HTTP {err.code}: {payload}") from err except urllib.error.URLError as err: raise RuntimeError(f"Failed to connect to Caido at {_graphql_url()}: {err}") from err diff --git a/tests/test_caido_bootstrap.py b/tests/test_caido_bootstrap.py index 10da9a1bd..0128ab062 100644 --- a/tests/test_caido_bootstrap.py +++ b/tests/test_caido_bootstrap.py @@ -13,10 +13,12 @@ import sys import types import urllib.error import urllib.request +from pathlib import Path from typing import Any from unittest.mock import MagicMock import pytest +from agents.sandbox.errors import ExecTransportError, WorkspaceStopError from strix.runtime.caido_bootstrap import _login_as_guest, bootstrap_caido from strix.tools.proxy import caido_api @@ -313,7 +315,24 @@ def test_caido_api_login_as_guest_http_500_with_graphql_error( monkeypatch.setattr(urllib.request, "urlopen", MagicMock(side_effect=http_err)) - with pytest.raises(RuntimeError, match="database locked"): + with pytest.raises(RuntimeError, match=r"HTTP 500.*database locked"): + caido_api._login_as_guest() + + +def test_caido_api_login_as_guest_http_401_non_json_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + http_err = urllib.error.HTTPError( + url="http://127.0.0.1:8080/graphql", + code=401, + msg="Unauthorized", + hdrs={}, # type: ignore[arg-type] + fp=io.BytesIO(b"Unauthorized access"), + ) + + monkeypatch.setattr(urllib.request, "urlopen", MagicMock(side_effect=http_err)) + + with pytest.raises(RuntimeError, match=r"HTTP 401.*Unauthorized access"): caido_api._login_as_guest() @@ -326,3 +345,84 @@ def test_caido_api_login_as_guest_url_error( with pytest.raises(RuntimeError, match="Failed to connect to Caido"): caido_api._login_as_guest() + +async def test_login_as_guest_fails_promptly_on_permanent_transport_error() -> None: + class _PermanentTransportSession: + def __init__(self) -> None: + self.call_count = 0 + + async def exec(self, *_args: Any, **_kwargs: Any) -> _FakeExecResult: + self.call_count += 1 + raise ExecTransportError(command=["curl"], message="Container gone", retryable=False) + + session = _PermanentTransportSession() + with pytest.raises( + RuntimeError, match=r"loginAsGuest failed permanently.*session.exec failed" + ): + await _login_as_guest( + session, # type: ignore[arg-type] + container_url="http://container", + attempts=10, + ) + assert session.call_count == 1 + + +async def test_login_as_guest_fails_promptly_on_workspace_stop_error() -> None: + class _StoppedSession: + def __init__(self) -> None: + self.call_count = 0 + + async def exec(self, *_args: Any, **_kwargs: Any) -> _FakeExecResult: + self.call_count += 1 + raise WorkspaceStopError(path=Path("/workspace")) + + session = _StoppedSession() + with pytest.raises( + RuntimeError, match=r"loginAsGuest failed permanently.*session.exec failed" + ): + await _login_as_guest( + session, # type: ignore[arg-type] + container_url="http://container", + attempts=10, + ) + assert session.call_count == 1 + + +async def test_login_as_guest_fails_promptly_when_curl_not_found() -> None: + class _CurlNotFoundSession: + def __init__(self) -> None: + self.call_count = 0 + + async def exec(self, *_args: Any, **_kwargs: Any) -> _FakeExecResult: + self.call_count += 1 + return _FakeExecResult("", stderr=b"/bin/sh: curl: not found", exit_code=127) + + session = _CurlNotFoundSession() + with pytest.raises(RuntimeError, match="loginAsGuest failed permanently: curl exit 127"): + await _login_as_guest( + session, # type: ignore[arg-type] + container_url="http://container", + attempts=10, + ) + assert session.call_count == 1 + + +async def test_login_as_guest_fails_promptly_when_curl_permission_denied() -> None: + class _CurlPermissionDeniedSession: + def __init__(self) -> None: + self.call_count = 0 + + async def exec(self, *_args: Any, **_kwargs: Any) -> _FakeExecResult: + self.call_count += 1 + return _FakeExecResult("", stderr=b"/bin/sh: curl: Permission denied", exit_code=126) + + session = _CurlPermissionDeniedSession() + with pytest.raises(RuntimeError, match="loginAsGuest failed permanently: curl exit 126"): + await _login_as_guest( + session, # type: ignore[arg-type] + container_url="http://container", + attempts=10, + ) + assert session.call_count == 1 + +