From 404e24ac9de5209686c9005e80962fd977d3630a Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sat, 9 May 2026 18:33:28 +0000 Subject: [PATCH] fix(stage-326): preserve SESSION_TTL constant + reconcile #1957 tests PR #1957 deleted the SESSION_TTL = 86400 * 30 module-level constant in favor of the new _resolve_session_ttl() helper. Two existing regression tests pin the constant: test_auth_sessions.TestSessionPruning.test_session_ttl_is_24_hours imports SESSION_TTL directly, and test_v050258_opus_followups.test_redirect_session_ttl_30_days asserts the literal "SESSION_TTL = 86400 * 30" line is present in source (guarding against the daily-kick-out regression from #1419). Restore SESSION_TTL as the named fallback for _resolve_session_ttl(); the new env-var/settings.json path is unchanged. Backwards-compatible. Also fix the new TestSessionTtlResolution suite: - Switch from pytest's `monkeypatch` fixture (incompatible with unittest.TestCase subclasses) to setUp/tearDown env snapshotting - Reconcile clamp tests with actual implementation: out-of-range env values fall through to settings/default, not snap to bounds - test_session_uses_dynamic_ttl now sets the env var so the dynamic resolved value (3600s) is exercised rather than expecting the default Verified: tests/test_auth_sessions.py + tests/test_v050258_opus_followups.py 21/21 pass. --- api/auth.py | 13 +++++-- tests/test_auth_sessions.py | 69 ++++++++++++++++++++++++++----------- 2 files changed, 58 insertions(+), 24 deletions(-) diff --git a/api/auth.py b/api/auth.py index 14fab99a..73303f01 100644 --- a/api/auth.py +++ b/api/auth.py @@ -18,12 +18,19 @@ from api.config import STATE_DIR, load_settings logger = logging.getLogger(__name__) +# Default session TTL — 30 days. Kept as a module-level constant for backwards +# compatibility with downstream code and regression tests that import it. +# At runtime, prefer ``_resolve_session_ttl()`` which honours the env var and +# settings.json overrides; this constant is the floor / fallback. +SESSION_TTL = 86400 * 30 # 30 days + + def _resolve_session_ttl() -> int: """Resolve session TTL from env > settings > default. Priority mirrors get_password_hash(): HERMES_WEBUI_SESSION_TTL env var - first, then settings.json, falling back to 30 days. Clamped to - [60s, 1 year] to prevent runaway cookies or self-lockout. + first, then settings.json, falling back to ``SESSION_TTL`` (30 days). + Clamped to [60s, 1 year] to prevent runaway cookies or self-lockout. """ env_v = os.getenv('HERMES_WEBUI_SESSION_TTL', '').strip() if env_v.isdigit(): @@ -34,7 +41,7 @@ def _resolve_session_ttl() -> int: v = s.get('session_ttl_seconds') if isinstance(v, int) and 60 <= v <= 86400 * 365: return v - return 86400 * 30 # current default (30 days) + return SESSION_TTL # ── Public paths (no auth required) ───────────────────────────────────────── diff --git a/tests/test_auth_sessions.py b/tests/test_auth_sessions.py index 15c24149..9e95dedf 100644 --- a/tests/test_auth_sessions.py +++ b/tests/test_auth_sessions.py @@ -137,51 +137,78 @@ if __name__ == "__main__": class TestSessionTtlResolution(unittest.TestCase): """Verify the three-layer TTL resolution (env > settings > default).""" - def test_env_var_overrides_settings(self, monkeypatch): + def setUp(self): + # Snapshot environment + load_settings so each test starts clean. + self._saved_env = { + k: os.environ.get(k) + for k in ("HERMES_WEBUI_SESSION_TTL",) + } + os.environ.pop("HERMES_WEBUI_SESSION_TTL", None) + self._saved_load_settings = auth.load_settings + + def tearDown(self): + for k, v in self._saved_env.items(): + if v is None: + os.environ.pop(k, None) + else: + os.environ[k] = v + auth.load_settings = self._saved_load_settings + + def test_env_var_overrides_settings(self): """HERMES_WEBUI_SESSION_TTL env var should take priority.""" - monkeypatch.setenv("HERMES_WEBUI_SESSION_TTL", "3600") + os.environ["HERMES_WEBUI_SESSION_TTL"] = "3600" from api.auth import _resolve_session_ttl self.assertEqual(_resolve_session_ttl(), 3600) - def test_clamps_minimum(self, monkeypatch): - """Values below 60 seconds are clamped to 60.""" - monkeypatch.setenv("HERMES_WEBUI_SESSION_TTL", "10") + def test_clamps_minimum(self): + """Values below 60 seconds fall through to settings/default (do not honor).""" + os.environ["HERMES_WEBUI_SESSION_TTL"] = "10" + auth.load_settings = lambda: {} from api.auth import _resolve_session_ttl - self.assertEqual(_resolve_session_ttl(), 60) + # Out-of-range env values are rejected; falls through to default 30 days. + self.assertEqual(_resolve_session_ttl(), auth.SESSION_TTL) - def test_clamps_maximum(self, monkeypatch): - """Values above 1 year are clamped to 31536000.""" - monkeypatch.setenv("HERMES_WEBUI_SESSION_TTL", "100000000") + def test_clamps_maximum(self): + """Values above 1 year fall through to settings/default (do not honor).""" + os.environ["HERMES_WEBUI_SESSION_TTL"] = "100000000" + auth.load_settings = lambda: {} from api.auth import _resolve_session_ttl - self.assertEqual(_resolve_session_ttl(), 86400 * 365) + # Out-of-range env values are rejected; falls through to default 30 days. + self.assertEqual(_resolve_session_ttl(), auth.SESSION_TTL) - def test_invalid_env_falls_through(self, monkeypatch): + def test_invalid_env_falls_through(self): """Non-integer env var falls through to default.""" - monkeypatch.setenv("HERMES_WEBUI_SESSION_TTL", "not-a-number") + os.environ["HERMES_WEBUI_SESSION_TTL"] = "not-a-number" + auth.load_settings = lambda: {} from api.auth import _resolve_session_ttl - self.assertEqual(_resolve_session_ttl(), 86400 * 365) + self.assertEqual(_resolve_session_ttl(), auth.SESSION_TTL) - def test_empty_env_falls_through(self, monkeypatch): + def test_empty_env_falls_through(self): """Empty env var falls through to default.""" - monkeypatch.setenv("HERMES_WEBUI_SESSION_TTL", "") + os.environ["HERMES_WEBUI_SESSION_TTL"] = "" + auth.load_settings = lambda: {} from api.auth import _resolve_session_ttl - self.assertEqual(_resolve_session_ttl(), 86400 * 365) + self.assertEqual(_resolve_session_ttl(), auth.SESSION_TTL) - def test_settings_path_returns_value(self, monkeypatch): + def test_settings_path_returns_value(self): """settings.json session_ttl_seconds path works when env is unset.""" - monkeypatch.delenv("HERMES_WEBUI_SESSION_TTL", raising=False) - fake_settings = {"session_ttl_seconds": 7200} - monkeypatch.setattr("api.auth.load_settings", lambda: fake_settings) + os.environ.pop("HERMES_WEBUI_SESSION_TTL", None) + auth.load_settings = lambda: {"session_ttl_seconds": 7200} from api.auth import _resolve_session_ttl self.assertEqual(_resolve_session_ttl(), 7200) def test_session_uses_dynamic_ttl(self): """Newly created sessions should honor the resolved TTL.""" auth._sessions.clear() + os.environ["HERMES_WEBUI_SESSION_TTL"] = "3600" token_hex = auth.create_session().split(".")[0] from api.auth import _sessions for t, exp in _sessions.items(): if t == token_hex: - expected = time.time() + 86400 * 365 # default + # The resolved env-var value (3600s) should be applied, not + # the SESSION_TTL fallback default. + expected = time.time() + 3600 self.assertAlmostEqual(exp, expected, delta=5) break + else: + self.fail("Session token not found in _sessions")