From 01a4b24dc111bd442c14a2a2436d6bc8f6a3c8fa Mon Sep 17 00:00:00 2001 From: jav1er8 <93607250+jav1er8@users.noreply.github.com> Date: Fri, 23 Oct 2026 19:21:20 +0200 Subject: [PATCH] fix(cloud): stop --wait from polling or succeeding after --wait-timeout `_poll()` only checked the deadline after a non-terminal response, so after sleeping up to the deadline it always started one more GET, and a final status from that late request was returned as success. Each request could also run for the full request timeout (120 s by default) regardless of the remaining wait budget. - Check the deadline before every poll, including the one after sleeping. - Cap each poll's request timeout at the remaining budget via a new `timeout` argument on `http.request()`, which can only shorten the configured request timeout. - Treat a response that arrives after the deadline as a timeout even when its status is final. The existing timeout error path keeps the operation ID in the error so the scan can still be inspected. Fixes #1564 --- strix/interface/cloud/http.py | 9 ++- strix/interface/cloud/runner.py | 27 ++++++-- tests/test_cloud_cli_runtime.py | 110 ++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 7 deletions(-) diff --git a/strix/interface/cloud/http.py b/strix/interface/cloud/http.py index 1531f0b58..68f84b09f 100644 --- a/strix/interface/cloud/http.py +++ b/strix/interface/cloud/http.py @@ -171,7 +171,14 @@ def request( body: dict[str, Any] | None = None, stream: bool = False, idempotency_key: str | None = None, + timeout: float | None = None, ) -> requests.Response: + """Send one platform API request. + + ``timeout`` can only shorten the configured request timeout, so a caller + with a smaller remaining budget (``--wait``) cannot be held past it by a + stalled connection. + """ url = f"{app_url()}/api/v1{path}" headers = { "Authorization": f"Bearer {api_token(token)}", @@ -193,7 +200,7 @@ def request( } or None, json=body, - timeout=_timeout_s, + timeout=_timeout_s if timeout is None else min(_timeout_s, timeout), stream=stream, allow_redirects=False, ) diff --git a/strix/interface/cloud/runner.py b/strix/interface/cloud/runner.py index f35cc4366..4fbc77db0 100644 --- a/strix/interface/cloud/runner.py +++ b/strix/interface/cloud/runner.py @@ -1111,10 +1111,21 @@ def _poll( as_json: bool, wait_timeout: float, ) -> Any: - """Poll a GET path until its status is final. Returns the last response.""" + """Poll a GET path until its status is final. Returns the last response. + + ``wait_timeout`` bounds the whole wait: no request starts once the deadline + has passed, each request's timeout is capped at the remaining budget, and a + response that only arrives after the deadline is reported as a timeout even + when it is final, so ``--wait`` never succeeds late. + """ deadline = time.monotonic() + wait_timeout while True: - current: Any = http.check(http.request("GET", path, token=token)) + remaining = deadline - time.monotonic() + if remaining <= 0: + raise _wait_timeout_error(wait_timeout) + current: Any = http.check(http.request("GET", path, token=token, timeout=remaining)) + if time.monotonic() > deadline: + raise _wait_timeout_error(wait_timeout) fields = cast("dict[str, Any]", current) if isinstance(current, dict) else {} status = str(fields.get("status", "")) if status.lower() in _TERMINAL_STATUSES: @@ -1125,8 +1136,12 @@ def _poll( ) remaining = deadline - time.monotonic() if remaining <= 0: - raise http.CloudError( - f"wait timed out after {wait_timeout:g} seconds; the remote operation is still " - "running. Re-run its get command to check the status." - ) + raise _wait_timeout_error(wait_timeout) time.sleep(min(_WAIT_POLL_S, remaining)) + + +def _wait_timeout_error(wait_timeout: float) -> http.CloudError: + return http.CloudError( + f"wait timed out after {wait_timeout:g} seconds; the remote operation is still " + "running. Re-run its get command to check the status." + ) diff --git a/tests/test_cloud_cli_runtime.py b/tests/test_cloud_cli_runtime.py index 4e2a5e5c6..0a24870ae 100644 --- a/tests/test_cloud_cli_runtime.py +++ b/tests/test_cloud_cli_runtime.py @@ -742,6 +742,116 @@ def test_wait_timeout_is_bounded(monkeypatch: pytest.MonkeyPatch, capsys: Any) - assert "wait timed out" in json.loads(capsys.readouterr().out)["error"] +class _FakeClock: + def __init__(self) -> None: + self.now = 0.0 + + def monotonic(self) -> float: + return self.now + + def sleep(self, seconds: float) -> None: + self.now += seconds + + +def _start_scan_and_wait(wait_timeout: str) -> int: + return cloud.run_cloud( + [ + "scans", + "start", + "--domain-ids", + "domain-1", + "--wait", + "--wait-timeout", + wait_timeout, + "--json", + ] + ) + + +def test_wait_starts_no_poll_after_the_deadline( + monkeypatch: pytest.MonkeyPatch, capsys: Any +) -> None: + clock = _FakeClock() + poll_starts: list[float] = [] + + def response(method: str, _path: str, **_kwargs: Any) -> FakeResponse: + if method == "POST": + return FakeResponse({"id": "scan-1", "status": "pending"}) + poll_starts.append(clock.now) + clock.now += 0.1 + return FakeResponse({"id": "scan-1", "status": "running"}) + + monkeypatch.setattr(http, "request", response) + monkeypatch.setattr(runner, "time", clock) + + assert _start_scan_and_wait("1") == 1 + assert poll_starts == [0.0] + payload = json.loads(capsys.readouterr().out) + assert "wait timed out" in payload["error"] + assert payload["operation_id"] == "scan-1" + + +def test_wait_reports_a_final_status_received_after_the_deadline_as_a_timeout( + monkeypatch: pytest.MonkeyPatch, capsys: Any +) -> None: + clock = _FakeClock() + + def response(method: str, _path: str, **_kwargs: Any) -> FakeResponse: + if method == "POST": + return FakeResponse({"id": "scan-1", "status": "pending"}) + clock.now += 30.0 + return FakeResponse({"id": "scan-1", "status": "completed"}) + + monkeypatch.setattr(http, "request", response) + monkeypatch.setattr(runner, "time", clock) + + assert _start_scan_and_wait("1") == 1 + payload = json.loads(capsys.readouterr().out) + assert "wait timed out" in payload["error"] + assert payload["operation_id"] == "scan-1" + + +def test_wait_caps_each_poll_request_at_the_remaining_budget( + monkeypatch: pytest.MonkeyPatch, capsys: Any +) -> None: + clock = _FakeClock() + poll_timeouts: list[float | None] = [] + + def response(method: str, _path: str, **kwargs: Any) -> FakeResponse: + if method == "POST": + return FakeResponse({"id": "scan-1", "status": "pending"}) + poll_timeouts.append(kwargs.get("timeout")) + status = "running" if len(poll_timeouts) == 1 else "completed" + return FakeResponse({"id": "scan-1", "status": status}) + + monkeypatch.setattr(http, "request", response) + monkeypatch.setattr(runner, "time", clock) + + assert _start_scan_and_wait("20") == 0 + assert poll_timeouts == [20.0, 5.0] + assert json.loads(capsys.readouterr().out)["status"] == "completed" + + +def test_request_timeout_override_only_shortens_the_configured_timeout( + monkeypatch: pytest.MonkeyPatch, +) -> None: + timeouts: list[float] = [] + + def fake_request(*_args: Any, **kwargs: Any) -> FakeResponse: + timeouts.append(kwargs["timeout"]) + return FakeResponse({}) + + monkeypatch.setattr(http, "_timeout_s", 120.0) + monkeypatch.setattr(http, "api_token", lambda _token=None: "token") + monkeypatch.setattr(http.requests, "request", fake_request) + + http.request("GET", "/scans/scan-1") + http.request("GET", "/scans/scan-1", timeout=5.0) + http.request("GET", "/scans/scan-1", timeout=600.0) + + assert timeouts == [120.0, 5.0, 120.0] + + def test_wait_interruption_returns_130_with_remote_operation_id( monkeypatch: pytest.MonkeyPatch, capsys: Any ) -> None: