diff --git a/app.py b/app.py index ff2ae55..8e15a59 100644 --- a/app.py +++ b/app.py @@ -804,8 +804,18 @@ def cleanup_stale_sessions(): @app.before_request def authorize_request(): """Check authorization before processing any request.""" - # Skip auth for health check, setup status, and Socket.IO (has own auth via connect event) - if request.path in ("/health", "/api/setup-status", "/api/pat-status", "/api/configure-pat", "/api/app-state") or request.path.startswith("/socket.io"): + # Auth-exempt: + # /health — minimal liveness probe, returns no sensitive data + # /api/configure-pat — owner-gates itself in-handler (cannot use the + # before_request gate; needed during bootstrap before + # app_owner is resolved). See configure_pat() guard. + # /socket.io/* — has own auth gate via the 'connect' WS event + # + # Previously exempt but now owner-gated (closed unauth info-disclosure + # surface): /api/setup-status, /api/pat-status, /api/app-state. All three + # are only polled by the frontend, which loads from "/" (auth'd) so already + # has SSO cookies — no functional regression. + if request.path in ("/health", "/api/configure-pat") or request.path.startswith("/socket.io"): return None authorized, user = check_authorization() @@ -905,17 +915,11 @@ def attach_session(): @app.route("/health") def health(): - with sessions_lock: - session_count = len(sessions) - with setup_lock: - current_setup_status = setup_state["status"] - return jsonify({ - "status": "healthy", - "version": APP_VERSION, - "setup_status": current_setup_status, - "active_sessions": session_count, - "session_timeout_seconds": SESSION_TIMEOUT_SECONDS - }) + # Minimal liveness response — no version, session count, or setup state + # exposed to unauthenticated callers. Version-targeted exploit selection + # is the avoided footgun; richer detail is available via authenticated + # endpoints (/api/version, /api/sessions, /api/setup-status). + return jsonify({"status": "healthy"}) @app.route("/api/version") diff --git a/tests/test_auth_enforcement.py b/tests/test_auth_enforcement.py index 42030c3..9c83773 100644 --- a/tests/test_auth_enforcement.py +++ b/tests/test_auth_enforcement.py @@ -271,3 +271,98 @@ def test_get_request_user_lowercases(self): assert result == "user@example.com", ( f"get_request_user() should lowercase, got '{result}'" ) + + +# --------------------------------------------------------------------------- +# 3. Info-disclosure endpoints — auth-gated or trimmed +# --------------------------------------------------------------------------- + +class TestInfoDisclosureEndpoints: + """These endpoints were previously reachable without auth and leaked + info about the app's state. They are now either owner-gated (setup-status, + pat-status, app-state) or minimally informative (health). + """ + + def _post_or_get(self, app_module, method, path, headers): + client = _make_client(app_module) + with mock.patch.object(app_module, "_is_databricks_apps", return_value=True): + if method == "GET": + return client.get(path, headers=headers) + return client.post(path, headers=headers) + + # -- /api/setup-status now requires auth -- + + def test_setup_status_denied_for_non_owner(self): + app_module = _get_app_module() + original = app_module.app_owner + try: + app_module.app_owner = "owner@databricks.com" + resp = self._post_or_get(app_module, "GET", "/api/setup-status", + {"X-Forwarded-Email": "intruder@evil.com"}) + assert resp.status_code == 403, ( + f"GET /api/setup-status should 403 for non-owner, got {resp.status_code}" + ) + finally: + app_module.app_owner = original + + def test_setup_status_allowed_for_owner(self): + app_module = _get_app_module() + original = app_module.app_owner + try: + app_module.app_owner = "owner@databricks.com" + resp = self._post_or_get(app_module, "GET", "/api/setup-status", + {"X-Forwarded-Email": "owner@databricks.com"}) + assert resp.status_code == 200, ( + f"Owner should see setup-status, got {resp.status_code}" + ) + finally: + app_module.app_owner = original + + # -- /api/pat-status now requires auth -- + + def test_pat_status_denied_for_non_owner(self): + app_module = _get_app_module() + original = app_module.app_owner + try: + app_module.app_owner = "owner@databricks.com" + resp = self._post_or_get(app_module, "GET", "/api/pat-status", + {"X-Forwarded-Email": "intruder@evil.com"}) + assert resp.status_code == 403, ( + f"GET /api/pat-status should 403 for non-owner, got {resp.status_code}" + ) + finally: + app_module.app_owner = original + + # -- /api/app-state now requires auth -- + + def test_app_state_denied_for_non_owner(self): + app_module = _get_app_module() + original = app_module.app_owner + try: + app_module.app_owner = "owner@databricks.com" + resp = self._post_or_get(app_module, "GET", "/api/app-state", + {"X-Forwarded-Email": "intruder@evil.com"}) + assert resp.status_code == 403, ( + f"GET /api/app-state should 403 for non-owner, got {resp.status_code}" + ) + finally: + app_module.app_owner = original + + # -- /health stays unauth but returns ONLY {"status": "healthy"} -- + + def test_health_minimal_response_no_version(self): + """Unauthenticated /health must NOT leak version, session counts, or + setup state — those enable version-targeted exploit selection.""" + app_module = _get_app_module() + client = _make_client(app_module) + # No SSO header; health stays unauth-exempt + resp = client.get("/health") + assert resp.status_code == 200 + body = resp.get_json() + assert body == {"status": "healthy"}, ( + f"/health should return only status, got keys: {list(body.keys())}" + ) + # Explicit anti-leak assertions + assert "version" not in body, "/health must not expose version" + assert "setup_status" not in body, "/health must not expose setup_status" + assert "active_sessions" not in body, "/health must not expose session count" diff --git a/tests/test_session_linger.py b/tests/test_session_linger.py index 0b3f2aa..b77274f 100644 --- a/tests/test_session_linger.py +++ b/tests/test_session_linger.py @@ -175,15 +175,16 @@ def test_warning_at_20_hours(self): # --------------------------------------------------------------------------- -# 5. /api/status reports 86400 to the frontend +# 5. Session timeout is configured to 24h # --------------------------------------------------------------------------- -class TestStatusEndpoint: +class TestSessionTimeoutConfig: + """The session-linger contract is that idle sessions get reaped after 24h. + We assert this on the constant directly. /health used to expose this + value to unauthenticated callers but no longer does (it leaked the app's + timeout posture to anyone who could reach the URL); the value is part of + the app's internal lifecycle config, not its public API.""" - def test_health_reports_24h_timeout(self): + def test_session_timeout_is_24h(self): app_module = _get_app() - client = app_module.app.test_client() - with mock.patch.object(app_module, "check_authorization", return_value=(True, "test-user")): - resp = client.get("/health") - body = resp.get_json() - assert body["session_timeout_seconds"] == 86400 + assert app_module.SESSION_TIMEOUT_SECONDS == 86400