mirror of
https://github.com/usestrix/strix.git
synced 2026-08-21 18:52:47 +02:00
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 <yoni@usestrix.com>
This commit is contained in:
co-authored by
yoni
parent
7b412ac3b1
commit
000a243861
+39
-9
@@ -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")
|
||||
|
||||
+10
-5
@@ -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))
|
||||
|
||||
+18
-9
@@ -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()
|
||||
|
||||
@@ -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] = {}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user