From 000a243861bdac87bcc44abbd9e93ee37c064687 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 21 Jul 2026 00:22:19 +0000 Subject: [PATCH] Close two follow-up viewer authz gaps (session-gate history reads, fail-closed expiry) (#819) * Session-gate historical run reads; fail closed on missing expiry * Accept epoch-seconds relay expiry so valid verification is not misread * Reject OTP verify responses lacking a usable expiry --------- Co-authored-by: yoni --- strix/viewer/auth.py | 48 +++++++++++++++++++++++++++++++-------- strix/viewer/server.py | 15 ++++++++---- tests/test_viewer.py | 27 ++++++++++++++-------- tests/test_viewer_auth.py | 41 +++++++++++++++++++++++++++++---- 4 files changed, 103 insertions(+), 28 deletions(-) diff --git a/strix/viewer/auth.py b/strix/viewer/auth.py index 5a51c37a..1c7c5fef 100644 --- a/strix/viewer/auth.py +++ b/strix/viewer/auth.py @@ -58,15 +58,25 @@ def read_auth() -> dict[str, Any] | None: return data -def _expiry(record: dict[str, Any]) -> datetime | None: - """Parse ``verified_at`` (the relay's ``expires_at``) into an aware UTC datetime. +def parse_expiry(raw: object) -> datetime | None: + """Parse a relay ``expires_at`` value into an aware UTC datetime. - Returns None when it is absent or unparseable, in which case expiry cannot be - enforced locally (the relay still rejects an expired token on report send). + Accepts both ISO 8601 strings and epoch seconds (as a number or numeric + string) so a valid relay expiry is not misread as missing. Returns None only + when it is genuinely absent or unparseable; both the local gate (see + ``is_verified``) and OTP verification (see ``otp_verify``) fail closed on such + values, matching the relay, which rejects a token with no valid expiry. """ - raw = record.get("verified_at") + if isinstance(raw, bool): + return None + if isinstance(raw, int | float): + return _from_epoch(raw) if not isinstance(raw, str) or not raw: return None + try: + return _from_epoch(float(raw)) + except ValueError: + pass try: parsed = datetime.fromisoformat(raw.replace("Z", "+00:00")) except ValueError: @@ -74,18 +84,33 @@ def _expiry(record: dict[str, Any]) -> datetime | None: return parsed if parsed.tzinfo else parsed.replace(tzinfo=UTC) +def _expiry(record: dict[str, Any]) -> datetime | None: + """The stored ``verified_at`` parsed to a datetime, or None if unusable.""" + return parse_expiry(record.get("verified_at")) + + +def _from_epoch(seconds: float) -> datetime | None: + """Epoch seconds → aware UTC datetime, or None if out of range.""" + try: + return datetime.fromtimestamp(seconds, tz=UTC) + except (OverflowError, OSError, ValueError): + return None + + def is_verified() -> bool: - """True when a usable, unexpired email + token record exists locally. + """True when a usable email + token record with a valid future expiry exists. The expiry returned by OTP verification is enforced here so history stops - unlocking once the token lapses, keeping the local gate in step with the - relay (which rejects an expired token on report send). + unlocking once the token lapses. It fails closed: a record whose expiry is + absent, blank, or unparseable requires re-verification rather than unlocking + forever, keeping the local gate in step with the relay (which rejects an + expired token on report send). """ record = read_auth() if record is None: return False expiry = _expiry(record) - return expiry is None or expiry > datetime.now(UTC) + return expiry is not None and expiry > datetime.now(UTC) def write_auth(email: str, token: str, verified_at: str) -> None: @@ -171,6 +196,11 @@ def otp_verify(email: str, code: str) -> dict[str, Any]: timeout=_OTP_TIMEOUT, ) if status == 200 and isinstance(data.get("token"), str): + # A token with no usable expiry cannot unlock history locally (the gate + # fails closed), so treat such a response as a failed verification rather + # than reporting success and then leaving the user stuck unverified. + if parse_expiry(data.get("expires_at")) is None: + raise RelayError("unavailable") return data if status == 403: raise RelayError("invalid_code") diff --git a/strix/viewer/server.py b/strix/viewer/server.py index 4d177fab..535bc002 100644 --- a/strix/viewer/server.py +++ b/strix/viewer/server.py @@ -255,12 +255,17 @@ def _make_handler(state: _ViewerState) -> type[BaseHTTPRequestHandler]: return # The launched run is always viewable. Any *other* run's data is part - # of the gated history, so it requires the same email verification as - # the /api/runs list — otherwise knowing a run name would leak its + # of the gated history: it needs this process's session capability + # (so merely reaching an exposed --host port is not enough) *and* + # email verification -- otherwise knowing a run name would leak its # metadata, vulnerabilities, report, and transcript. - if run_dir.resolve() != state.run_dir.resolve() and not auth.is_verified(): - self._send_json(HTTPStatus.UNAUTHORIZED, {"error": "unverified"}) - return + if run_dir.resolve() != state.run_dir.resolve(): + if not self._has_session(): + self._send_json(HTTPStatus.FORBIDDEN, {"error": "forbidden"}) + return + if not auth.is_verified(): + self._send_json(HTTPStatus.UNAUTHORIZED, {"error": "unverified"}) + return if path == "/api/run": self._send_json(HTTPStatus.OK, read_run_summary(run_dir)) diff --git a/tests/test_viewer.py b/tests/test_viewer.py index fb629e1d..8510fd7c 100644 --- a/tests/test_viewer.py +++ b/tests/test_viewer.py @@ -213,9 +213,11 @@ def _session_cookie(url: str, token: str) -> str: return raw.split(";", 1)[0] -def _get_status(url: str) -> int: +def _get_status(url: str, *, cookie: str | None = None) -> int: + headers = {"Cookie": cookie} if cookie else {} + req = urllib.request.Request(url, headers=headers) # noqa: S310 - localhost test server try: - with urllib.request.urlopen(url) as resp: # noqa: S310 - localhost test server + with urllib.request.urlopen(req) as resp: # noqa: S310 return int(resp.status) except urllib.error.HTTPError as exc: return int(exc.code) @@ -378,19 +380,26 @@ def test_historical_run_data_requires_verification( verified = {"value": False} monkeypatch.setattr("strix.viewer.auth.is_verified", lambda: verified["value"]) - httpd, url, _ = serve(launched, open_browser=False) + httpd, url, token = serve(launched, open_browser=False) try: - # The launched run is always viewable, no verification required. + # The launched run is always viewable, no verification and no cookie. status, _, _ = _get(f"{url}/api/run") assert status == 200 - # A different run's data is gated behind verification. - assert _get_status(f"{url}/api/run?run=other") == 401 + cookie = _session_cookie(url, token) - # Once verified, the historical run resolves. + # A different run needs the session capability first: a cookie-less + # caller is forbidden even once the machine is verified. verified["value"] = True - status, _, _ = _get(f"{url}/api/run?run=other") - assert status == 200 + assert _get_status(f"{url}/api/run?run=other") == 403 + + # With the cookie but not verified, the history gate returns 401. + verified["value"] = False + assert _get_status(f"{url}/api/run?run=other", cookie=cookie) == 401 + + # With both the cookie and verification, the historical run resolves. + verified["value"] = True + assert _get_status(f"{url}/api/run?run=other", cookie=cookie) == 200 finally: httpd.shutdown() httpd.server_close() diff --git a/tests/test_viewer_auth.py b/tests/test_viewer_auth.py index b4f99393..049f80a2 100644 --- a/tests/test_viewer_auth.py +++ b/tests/test_viewer_auth.py @@ -56,13 +56,33 @@ def test_is_verified_enforces_expiry() -> None: assert auth.is_verified() is True -def test_is_verified_when_expiry_absent_or_unparseable() -> None: - # No/blank expiry: cannot enforce locally, so treat as valid. +def test_is_verified_fails_closed_when_expiry_absent_or_unparseable() -> None: + # No/blank expiry: fail closed rather than unlocking history forever. auth.write_auth(email="a@b.com", token="t", verified_at="") - assert auth.is_verified() is True + assert auth.read_auth() is not None + assert auth.is_verified() is False - # Garbage expiry is ignored rather than locking the user out. + # Garbage expiry likewise requires re-verification. auth.write_auth(email="a@b.com", token="t", verified_at="not-a-date") + assert auth.is_verified() is False + + +def test_is_verified_accepts_epoch_expiry() -> None: + # A relay expiry expressed as epoch seconds must not be misread as missing. + future = (datetime.now(UTC) + timedelta(hours=1)).timestamp() + past = (datetime.now(UTC) - timedelta(hours=1)).timestamp() + + # As a numeric string (how write_auth persists it). + auth.write_auth(email="a@b.com", token="t", verified_at=str(future)) + assert auth.is_verified() is True + auth.write_auth(email="a@b.com", token="t", verified_at=str(past)) + assert auth.is_verified() is False + + # As a raw JSON number, if a record is written that way. + auth.AUTH_PATH.parent.mkdir(parents=True, exist_ok=True) + auth.AUTH_PATH.write_text( + f'{{"email": "a@b.com", "token": "t", "verified_at": {future}}}', encoding="utf-8" + ) assert auth.is_verified() is True @@ -102,7 +122,8 @@ def test_otp_start_maps_errors(monkeypatch: pytest.MonkeyPatch) -> None: def test_otp_verify_success_and_invalid(monkeypatch: pytest.MonkeyPatch) -> None: - _stub_post(monkeypatch, 200, {"token": "t", "email": "a@b.com", "expires_at": "later"}) + expires = _iso(timedelta(hours=1)) + _stub_post(monkeypatch, 200, {"token": "t", "email": "a@b.com", "expires_at": expires}) result = auth.otp_verify("a@b.com", "123456") assert result["token"] == "t" @@ -112,6 +133,16 @@ def test_otp_verify_success_and_invalid(monkeypatch: pytest.MonkeyPatch) -> None assert exc.value.code == "invalid_code" +def test_otp_verify_rejects_token_without_usable_expiry(monkeypatch: pytest.MonkeyPatch) -> None: + # A 200 with a token but no valid expiry must not be reported as success, + # otherwise the caller would store a record that immediately reads unverified. + for expires in (None, "", "later"): + _stub_post(monkeypatch, 200, {"token": "t", "email": "a@b.com", "expires_at": expires}) + with pytest.raises(auth.RelayError) as exc: + auth.otp_verify("a@b.com", "123456") + assert exc.value.code == "unavailable" + + def test_report_send_never_includes_password(monkeypatch: pytest.MonkeyPatch) -> None: captured: dict[str, Any] = {}