From 1f7c0cb7e8909a852ae38aff6710999c8012a13f Mon Sep 17 00:00:00 2001 From: damienriehl Date: Fri, 3 Apr 2026 11:19:33 -0500 Subject: [PATCH 01/12] feat(08-01): add AUTH_MODE config and anonymous user for three auth modes - Add auth_mode field (required/optional/disabled) to Settings class in config.py - Add ANONYMOUS_USER constant to auth.py (id=anonymous, roles=[viewer]) - Add early returns in get_current_user, get_current_user_optional, get_current_user_with_token for disabled mode - Optional mode: RequiredUser still 401s (write protection), OptionalUser returns None (browse works) --- ontokit/core/auth.py | 20 ++++++++++++++++++++ ontokit/core/config.py | 3 +++ 2 files changed, 23 insertions(+) diff --git a/ontokit/core/auth.py b/ontokit/core/auth.py index e5ee2975..058d639d 100644 --- a/ontokit/core/auth.py +++ b/ontokit/core/auth.py @@ -68,6 +68,15 @@ def is_superadmin(self) -> bool: return self.id in settings.superadmin_ids +# Anonymous user returned when auth is disabled +ANONYMOUS_USER = CurrentUser( + id="anonymous", + email=None, + name="Anonymous", + username="anonymous", + roles=["viewer"], +) + # Cache for JWKS (JSON Web Key Set) with TTL _jwks_cache: dict[str, Any] | None = None _jwks_cache_time: float = 0.0 @@ -259,6 +268,10 @@ async def get_current_user( Raises 401 if not authenticated. """ + if settings.auth_mode == "disabled": + return ANONYMOUS_USER + # "optional" mode: still require auth for RequiredUser (401 if no credentials) + # "required" mode: existing behavior (401 if no credentials) if credentials is None: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, @@ -300,6 +313,10 @@ async def get_current_user_optional( Useful for endpoints that work differently for authenticated vs anonymous users. """ + if settings.auth_mode == "disabled": + return ANONYMOUS_USER + # "optional" mode: existing behavior — returns None if no credentials, real user if valid token + # "required" mode: existing behavior if credentials is None: return None @@ -318,6 +335,9 @@ async def get_current_user_with_token( Raises 401 if not authenticated. Returns tuple of (CurrentUser, access_token). """ + if settings.auth_mode == "disabled": + return ANONYMOUS_USER, "anonymous" + # "optional" and "required" modes: existing behavior (401 if no credentials) if credentials is None: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, diff --git a/ontokit/core/config.py b/ontokit/core/config.py index 6e8c2b92..3706de82 100644 --- a/ontokit/core/config.py +++ b/ontokit/core/config.py @@ -80,6 +80,9 @@ def zitadel_jwks_base_url(self) -> str: frontend_url: str = "" # e.g. http://localhost:3000 revalidation_secret: str = "" # shared secret for sitemap revalidation + # Auth mode: "required" (default), "optional" (browse without login, sign in for editing), "disabled" (no auth) + auth_mode: str = "required" + # Superadmin - comma-separated list of user IDs with full system access superadmin_user_ids: str = "" From 1263c705469d6b1e54b80ee67e8107aee45b43d3 Mon Sep 17 00:00:00 2001 From: damienriehl Date: Fri, 3 Apr 2026 11:20:16 -0500 Subject: [PATCH 02/12] test(08-01): add tests for all three auth modes (required, optional, disabled) - Test ANONYMOUS_USER properties (id, roles, type, not superadmin) - Test disabled mode: all three auth functions return ANONYMOUS_USER - Test required mode: get_current_user raises 401, get_current_user_optional returns None - Test optional mode: RequiredUser raises 401 (write protection), OptionalUser returns None (browse works) --- tests/unit/test_auth_disabled.py | 127 +++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 tests/unit/test_auth_disabled.py diff --git a/tests/unit/test_auth_disabled.py b/tests/unit/test_auth_disabled.py new file mode 100644 index 00000000..14cebdf0 --- /dev/null +++ b/tests/unit/test_auth_disabled.py @@ -0,0 +1,127 @@ +"""Tests for the three auth modes: required, optional, disabled.""" + +from unittest.mock import patch + +import pytest +from fastapi import HTTPException + +from ontokit.core.auth import ( + ANONYMOUS_USER, + CurrentUser, + get_current_user, + get_current_user_optional, + get_current_user_with_token, +) + + +# --------------------------------------------------------------------------- +# ANONYMOUS_USER constant +# --------------------------------------------------------------------------- + + +class TestAnonymousUser: + """Tests for the ANONYMOUS_USER constant.""" + + def test_anonymous_user_id(self) -> None: + """ANONYMOUS_USER has id='anonymous'.""" + assert ANONYMOUS_USER.id == "anonymous" + + def test_anonymous_user_roles(self) -> None: + """ANONYMOUS_USER has roles=['viewer'].""" + assert ANONYMOUS_USER.roles == ["viewer"] + + @patch("ontokit.core.auth.settings") + def test_anonymous_user_is_not_superadmin(self, mock_settings) -> None: # noqa: ANN001 + """ANONYMOUS_USER is never a superadmin.""" + mock_settings.superadmin_ids = set() + assert ANONYMOUS_USER.is_superadmin is False + + def test_anonymous_user_is_current_user_instance(self) -> None: + """ANONYMOUS_USER is an instance of CurrentUser.""" + assert isinstance(ANONYMOUS_USER, CurrentUser) + + +# --------------------------------------------------------------------------- +# AUTH_MODE=disabled +# --------------------------------------------------------------------------- + + +class TestAuthModeDisabled: + """Tests for AUTH_MODE=disabled — all functions return ANONYMOUS_USER.""" + + @pytest.mark.asyncio + @patch("ontokit.core.auth.settings") + async def test_disabled_get_current_user_returns_anonymous(self, mock_settings) -> None: # noqa: ANN001 + """In disabled mode, get_current_user returns ANONYMOUS_USER (no credentials needed).""" + mock_settings.auth_mode = "disabled" + result = await get_current_user(credentials=None) + assert result is ANONYMOUS_USER + + @pytest.mark.asyncio + @patch("ontokit.core.auth.settings") + async def test_disabled_get_current_user_optional_returns_anonymous(self, mock_settings) -> None: # noqa: ANN001 + """In disabled mode, get_current_user_optional returns ANONYMOUS_USER (not None).""" + mock_settings.auth_mode = "disabled" + result = await get_current_user_optional(credentials=None) + assert result is ANONYMOUS_USER + + @pytest.mark.asyncio + @patch("ontokit.core.auth.settings") + async def test_disabled_get_current_user_with_token_returns_anonymous(self, mock_settings) -> None: # noqa: ANN001 + """In disabled mode, get_current_user_with_token returns (ANONYMOUS_USER, 'anonymous').""" + mock_settings.auth_mode = "disabled" + user, token = await get_current_user_with_token(credentials=None) + assert user is ANONYMOUS_USER + assert token == "anonymous" + + +# --------------------------------------------------------------------------- +# AUTH_MODE=required (default) +# --------------------------------------------------------------------------- + + +class TestAuthModeRequired: + """Tests for AUTH_MODE=required — existing behavior, 401 without credentials.""" + + @pytest.mark.asyncio + @patch("ontokit.core.auth.settings") + async def test_required_get_current_user_raises_401_without_credentials(self, mock_settings) -> None: # noqa: ANN001 + """In required mode, get_current_user raises 401 when no credentials provided.""" + mock_settings.auth_mode = "required" + with pytest.raises(HTTPException) as exc_info: + await get_current_user(credentials=None) + assert exc_info.value.status_code == 401 + + @pytest.mark.asyncio + @patch("ontokit.core.auth.settings") + async def test_required_get_current_user_optional_returns_none_without_credentials(self, mock_settings) -> None: # noqa: ANN001 + """In required mode, get_current_user_optional returns None when no credentials provided.""" + mock_settings.auth_mode = "required" + result = await get_current_user_optional(credentials=None) + assert result is None + + +# --------------------------------------------------------------------------- +# AUTH_MODE=optional +# --------------------------------------------------------------------------- + + +class TestAuthModeOptional: + """Tests for AUTH_MODE=optional — GET endpoints work anonymously, writes require auth.""" + + @pytest.mark.asyncio + @patch("ontokit.core.auth.settings") + async def test_optional_get_current_user_raises_401_without_credentials(self, mock_settings) -> None: # noqa: ANN001 + """In optional mode, get_current_user (RequiredUser) raises 401 without credentials (write protection).""" + mock_settings.auth_mode = "optional" + with pytest.raises(HTTPException) as exc_info: + await get_current_user(credentials=None) + assert exc_info.value.status_code == 401 + + @pytest.mark.asyncio + @patch("ontokit.core.auth.settings") + async def test_optional_get_current_user_optional_returns_none_without_credentials(self, mock_settings) -> None: # noqa: ANN001 + """In optional mode, get_current_user_optional returns None without credentials (browse works).""" + mock_settings.auth_mode = "optional" + result = await get_current_user_optional(credentials=None) + assert result is None From 053de7f7642c4c5d78545707b0be0b9a7b3ac305 Mon Sep 17 00:00:00 2001 From: damienriehl Date: Tue, 7 Jul 2026 08:31:33 -0500 Subject: [PATCH 03/12] harden(auth): Literal auth_mode + disabled-ignores-credentials test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /ce:review (PR-2) follow-ups: - config.py: auth_mode str -> Literal[required|optional|disabled] so pydantic-settings rejects config typos at startup (fail-fast) instead of silently falling through to required behavior. - test_auth_disabled.py: add test_disabled_ignores_valid_credentials proving disabled mode ignores even a valid Bearer token (everyone anonymous viewer). WS-auth parity (authenticate_ws does not consult auth_mode) is intentionally NOT changed here — it is new work beyond the extracted phase-08 slice; noted on the PR for a follow-up. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q --- ontokit/core/config.py | 4 +++- tests/unit/test_auth_disabled.py | 16 +++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/ontokit/core/config.py b/ontokit/core/config.py index 3706de82..533ac94e 100644 --- a/ontokit/core/config.py +++ b/ontokit/core/config.py @@ -81,7 +81,9 @@ def zitadel_jwks_base_url(self) -> str: revalidation_secret: str = "" # shared secret for sitemap revalidation # Auth mode: "required" (default), "optional" (browse without login, sign in for editing), "disabled" (no auth) - auth_mode: str = "required" + # Literal (not bare str) so pydantic-settings rejects typos at startup instead of + # silently falling through to required behavior (/ce:review MEDIUM, PR-2). + auth_mode: Literal["required", "optional", "disabled"] = "required" # Superadmin - comma-separated list of user IDs with full system access superadmin_user_ids: str = "" diff --git a/tests/unit/test_auth_disabled.py b/tests/unit/test_auth_disabled.py index 14cebdf0..d8512280 100644 --- a/tests/unit/test_auth_disabled.py +++ b/tests/unit/test_auth_disabled.py @@ -4,6 +4,7 @@ import pytest from fastapi import HTTPException +from fastapi.security import HTTPAuthorizationCredentials from ontokit.core.auth import ( ANONYMOUS_USER, @@ -13,7 +14,6 @@ get_current_user_with_token, ) - # --------------------------------------------------------------------------- # ANONYMOUS_USER constant # --------------------------------------------------------------------------- @@ -74,6 +74,20 @@ async def test_disabled_get_current_user_with_token_returns_anonymous(self, mock assert user is ANONYMOUS_USER assert token == "anonymous" + @pytest.mark.asyncio + @patch("ontokit.core.auth.settings") + async def test_disabled_ignores_valid_credentials(self, mock_settings) -> None: # noqa: ANN001 + """In disabled mode, even a present/valid Bearer token is ignored — everyone is + anonymous viewer, no privilege differentiation (the disabled early-return fires + before any token validation). Documents the /ce:review LOW finding for PR-2.""" + mock_settings.auth_mode = "disabled" + creds = HTTPAuthorizationCredentials(scheme="Bearer", credentials="a.valid.jwt") + assert await get_current_user(credentials=creds) is ANONYMOUS_USER + assert await get_current_user_optional(credentials=creds) is ANONYMOUS_USER + user, token = await get_current_user_with_token(credentials=creds) + assert user is ANONYMOUS_USER + assert token == "anonymous" + # --------------------------------------------------------------------------- # AUTH_MODE=required (default) From fbf7ed714547585891ed8f38de26662d6dacfea6 Mon Sep 17 00:00:00 2001 From: damienriehl Date: Fri, 10 Jul 2026 21:39:43 -0500 Subject: [PATCH 04/12] harden(auth): WebSocket auth honors auth_mode (HTTP parity) authenticate_ws hard-required a token regardless of settings.auth_mode, so a disabled/optional deployment admitted anonymous callers on its HTTP API but rejected them at the WebSocket handshake. Resolve caller identity per auth_mode mirroring core.auth: - disabled -> ANONYMOUS_USER, no token required - optional -> absent/invalid token downgrades to anonymous (get_current_user_optional) - required -> valid token mandatory (unchanged) Project-access check still gates private projects for anonymous callers. +7 parity tests. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018eQ2CoAwpGv9Q8vBhuGGtH --- ontokit/api/utils/ws_auth.py | 126 ++++++++++++++++++++---------- tests/unit/test_ws_auth.py | 144 +++++++++++++++++++++++++++++++++++ 2 files changed, 229 insertions(+), 41 deletions(-) diff --git a/ontokit/api/utils/ws_auth.py b/ontokit/api/utils/ws_auth.py index 46e11848..aa787afb 100644 --- a/ontokit/api/utils/ws_auth.py +++ b/ontokit/api/utils/ws_auth.py @@ -5,13 +5,41 @@ from fastapi import HTTPException, WebSocket -from ontokit.core.auth import CurrentUser, fetch_userinfo, validate_token +from ontokit.core.auth import ( + ANONYMOUS_USER, + CurrentUser, + fetch_userinfo, + validate_token, +) +from ontokit.core.config import settings from ontokit.core.database import async_session_maker from ontokit.services.project_service import ProjectService logger = logging.getLogger(__name__) +async def _build_user_from_token(token: str) -> CurrentUser: + """Validate a JWT and assemble the ``CurrentUser``. + + Mirrors the user-assembly half of ``core.auth.get_current_user`` (userinfo + backfill for missing name/email) so WebSocket callers get the same identity + an HTTP request with the same token would. + """ + payload = await validate_token(token) + name = payload.name + email = payload.email + username = payload.preferred_username + if not name or not email: + userinfo = await fetch_userinfo(token) + if userinfo: + name = name or userinfo.get("name") or userinfo.get("preferred_username") + email = email or userinfo.get("email") + username = username or userinfo.get("preferred_username") + return CurrentUser( + id=payload.sub, email=email, name=name, username=username, roles=payload.roles + ) + + async def authenticate_ws( websocket: WebSocket, project_id: UUID, @@ -19,54 +47,70 @@ async def authenticate_ws( ) -> bool: """Authenticate a WebSocket connection and verify project access. - Validates the JWT token, builds a ``CurrentUser``, and checks project - access via ``ProjectService.get``. Returns ``True`` if the caller - should proceed (auth + access succeeded). Returns ``False`` after - closing the WebSocket with an appropriate code when auth or access - fails. + Resolves the caller identity **honoring ``settings.auth_mode``**, then checks + project access via ``ProjectService.get``. Returns ``True`` if the caller + should proceed, or ``False`` after closing the WebSocket with an appropriate + code when auth or access fails. - HTTP 401/403/404 from the auth/service layer are translated to - WebSocket close codes: + ``auth_mode`` parity with the HTTP dependencies in ``ontokit.core.auth`` + (``get_current_user`` / ``get_current_user_optional``) — without this a + ``disabled`` or ``optional`` deployment would admit anonymous callers on its + HTTP API but still reject them at the WebSocket handshake: - * **4001** – missing or invalid token - * **4003** – authenticated but access denied + * ``disabled`` — everyone is the shared :data:`ANONYMOUS_USER`; **no token + required** (matches ``get_current_user``'s disabled branch). + * ``optional`` — an absent *or* invalid token downgrades to + :data:`ANONYMOUS_USER` (matches ``get_current_user_optional``); the + project-access check below still gates private projects. + * ``required`` — a valid token is mandatory (original behavior). + + HTTP 401/403/404 from the auth/service layer are translated to WebSocket + close codes: + + * **4001** – missing or invalid token (``required`` mode only) + * **4003** – authenticated (or anonymous) but access denied * **4004** – project not found - Unexpected server errors are closed with **1011** (internal error) - and logged. + Unexpected server errors are closed with **1011** (internal error) and logged. - The WebSocket is accepted before any error close so that the client - receives a proper close frame rather than a raw HTTP 403. + The WebSocket is accepted before any error close so that the client receives + a proper close frame rather than a raw HTTP 403. """ await websocket.accept() - # --- Token required --- - if not token: - await websocket.close(code=4001, reason="Authentication required") - return False - - # --- Validate JWT --- - try: - payload = await validate_token(token) - name = payload.name - email = payload.email - username = payload.preferred_username - if not name or not email: - userinfo = await fetch_userinfo(token) - if userinfo: - name = name or userinfo.get("name") or userinfo.get("preferred_username") - email = email or userinfo.get("email") - username = username or userinfo.get("preferred_username") - user = CurrentUser( - id=payload.sub, email=email, name=name, username=username, roles=payload.roles - ) - except HTTPException: - await websocket.close(code=4001, reason="Invalid or expired token") - return False - except Exception: - logger.exception("Unexpected error during WebSocket token validation") - await websocket.close(code=1011, reason="Internal server error") - return False + # --- Resolve caller identity per auth_mode (parity with core.auth) --- + user: CurrentUser + if settings.auth_mode == "disabled": + # Auth fully disabled — the shared anonymous identity, no token needed. + user = ANONYMOUS_USER + elif settings.auth_mode == "optional": + # Mirror get_current_user_optional: absent/invalid token → anonymous. + # The project-access check still enforces private-project boundaries. + if not token: + user = ANONYMOUS_USER + else: + try: + user = await _build_user_from_token(token) + except HTTPException: + user = ANONYMOUS_USER + except Exception: + logger.exception("Unexpected error during WebSocket token validation") + await websocket.close(code=1011, reason="Internal server error") + return False + else: + # "required" mode: a valid token is mandatory. + if not token: + await websocket.close(code=4001, reason="Authentication required") + return False + try: + user = await _build_user_from_token(token) + except HTTPException: + await websocket.close(code=4001, reason="Invalid or expired token") + return False + except Exception: + logger.exception("Unexpected error during WebSocket token validation") + await websocket.close(code=1011, reason="Internal server error") + return False # --- Verify project access --- try: diff --git a/tests/unit/test_ws_auth.py b/tests/unit/test_ws_auth.py index bd17208b..fc403ff1 100644 --- a/tests/unit/test_ws_auth.py +++ b/tests/unit/test_ws_auth.py @@ -157,3 +157,147 @@ async def test_success_returns_true(self) -> None: assert result is True ws.accept.assert_awaited_once() ws.close.assert_not_awaited() + + +def _ok_project_ctx() -> Mock: + """A patched async_session_maker whose ProjectService.get succeeds.""" + mock_ctx = AsyncMock() + mock_ctx.__aenter__ = AsyncMock(return_value=AsyncMock()) + mock_ctx.__aexit__ = AsyncMock(return_value=False) + return mock_ctx + + +class TestAuthenticateWsAuthModeParity: + """WebSocket auth must honor settings.auth_mode exactly like core.auth HTTP deps. + + Without this parity a ``disabled``/``optional`` deployment admits anonymous + callers on its HTTP API but rejects them at the WebSocket handshake. + """ + + @pytest.mark.asyncio + async def test_disabled_mode_no_token_succeeds_anonymous(self, monkeypatch) -> None: + """auth_mode=disabled → no token required; proceeds as ANONYMOUS_USER.""" + monkeypatch.setattr("ontokit.api.utils.ws_auth.settings.auth_mode", "disabled") + ws = AsyncMock(spec=WebSocket) + mock_svc = AsyncMock() + mock_svc.get.return_value = Mock() + + validate = AsyncMock() + with ( + patch("ontokit.api.utils.ws_auth.validate_token", validate), + patch("ontokit.api.utils.ws_auth.async_session_maker", Mock(return_value=_ok_project_ctx())), + patch("ontokit.api.utils.ws_auth.ProjectService", return_value=mock_svc), + ): + result = await authenticate_ws(ws, PROJECT_UUID, token=None) + + assert result is True + ws.close.assert_not_awaited() + # No token was validated — the disabled branch never touches the JWT path. + validate.assert_not_awaited() + # The anonymous identity was the one passed to the access check. + passed_user = mock_svc.get.await_args.args[1] + assert passed_user.id == "anonymous" + + @pytest.mark.asyncio + async def test_disabled_mode_still_enforces_project_access(self, monkeypatch) -> None: + """Anonymous access to a private project is still denied (4003).""" + monkeypatch.setattr("ontokit.api.utils.ws_auth.settings.auth_mode", "disabled") + ws = AsyncMock(spec=WebSocket) + mock_svc = AsyncMock() + mock_svc.get.side_effect = HTTPException(status_code=403, detail="Forbidden") + + with ( + patch("ontokit.api.utils.ws_auth.async_session_maker", Mock(return_value=_ok_project_ctx())), + patch("ontokit.api.utils.ws_auth.ProjectService", return_value=mock_svc), + ): + result = await authenticate_ws(ws, PROJECT_UUID, token=None) + + assert result is False + ws.close.assert_awaited_once_with(code=4003, reason="Access denied") + + @pytest.mark.asyncio + async def test_optional_mode_no_token_downgrades_to_anonymous(self, monkeypatch) -> None: + """auth_mode=optional → absent token proceeds as anonymous (public project).""" + monkeypatch.setattr("ontokit.api.utils.ws_auth.settings.auth_mode", "optional") + ws = AsyncMock(spec=WebSocket) + mock_svc = AsyncMock() + mock_svc.get.return_value = Mock() + + with ( + patch("ontokit.api.utils.ws_auth.async_session_maker", Mock(return_value=_ok_project_ctx())), + patch("ontokit.api.utils.ws_auth.ProjectService", return_value=mock_svc), + ): + result = await authenticate_ws(ws, PROJECT_UUID, token=None) + + assert result is True + ws.close.assert_not_awaited() + assert mock_svc.get.await_args.args[1].id == "anonymous" + + @pytest.mark.asyncio + async def test_optional_mode_invalid_token_downgrades_to_anonymous(self, monkeypatch) -> None: + """auth_mode=optional → invalid token silently downgrades (mirrors OptionalUser).""" + monkeypatch.setattr("ontokit.api.utils.ws_auth.settings.auth_mode", "optional") + ws = AsyncMock(spec=WebSocket) + mock_svc = AsyncMock() + mock_svc.get.return_value = Mock() + + with ( + patch( + "ontokit.api.utils.ws_auth.validate_token", + AsyncMock(side_effect=HTTPException(status_code=401)), + ), + patch("ontokit.api.utils.ws_auth.fetch_userinfo", AsyncMock(return_value=None)), + patch("ontokit.api.utils.ws_auth.async_session_maker", Mock(return_value=_ok_project_ctx())), + patch("ontokit.api.utils.ws_auth.ProjectService", return_value=mock_svc), + ): + result = await authenticate_ws(ws, PROJECT_UUID, token="bad") + + assert result is True + ws.close.assert_not_awaited() + assert mock_svc.get.await_args.args[1].id == "anonymous" + + @pytest.mark.asyncio + async def test_optional_mode_valid_token_uses_real_identity(self, monkeypatch) -> None: + """auth_mode=optional → a valid token yields the real authenticated user.""" + monkeypatch.setattr("ontokit.api.utils.ws_auth.settings.auth_mode", "optional") + ws = AsyncMock(spec=WebSocket) + mock_svc = AsyncMock() + mock_svc.get.return_value = Mock() + + with ( + patch( + "ontokit.api.utils.ws_auth.validate_token", + AsyncMock(return_value=_fake_token_payload()), + ), + patch("ontokit.api.utils.ws_auth.fetch_userinfo", AsyncMock(return_value=None)), + patch("ontokit.api.utils.ws_auth.async_session_maker", Mock(return_value=_ok_project_ctx())), + patch("ontokit.api.utils.ws_auth.ProjectService", return_value=mock_svc), + ): + result = await authenticate_ws(ws, PROJECT_UUID, token="tok") + + assert result is True + ws.close.assert_not_awaited() + assert mock_svc.get.await_args.args[1].id == "user-1" + + @pytest.mark.asyncio + async def test_optional_mode_token_unexpected_error_closes_1011(self, monkeypatch) -> None: + """auth_mode=optional → an infra error during validation still 1011s (not a downgrade).""" + monkeypatch.setattr("ontokit.api.utils.ws_auth.settings.auth_mode", "optional") + ws = AsyncMock(spec=WebSocket) + with patch( + "ontokit.api.utils.ws_auth.validate_token", + AsyncMock(side_effect=RuntimeError("network error")), + ): + result = await authenticate_ws(ws, PROJECT_UUID, token="tok") + + assert result is False + ws.close.assert_awaited_once_with(code=1011, reason="Internal server error") + + @pytest.mark.asyncio + async def test_required_mode_no_token_still_closes_4001(self, monkeypatch) -> None: + """auth_mode=required (explicit) → no token still hard-closes 4001.""" + monkeypatch.setattr("ontokit.api.utils.ws_auth.settings.auth_mode", "required") + ws = AsyncMock(spec=WebSocket) + result = await authenticate_ws(ws, PROJECT_UUID, token=None) + assert result is False + ws.close.assert_awaited_once_with(code=4001, reason="Authentication required") From 6ae621c87ce8a23cb136ee25e512d8a46a4e373b Mon Sep 17 00:00:00 2001 From: damienriehl Date: Fri, 3 Apr 2026 17:45:33 -0500 Subject: [PATCH 05/12] feat(10-01): anonymous token module, extended model, and schemas - Add ontokit/core/anonymous_token.py with HMAC-signed create/verify functions (24h TTL, anon: prefix to prevent token type confusion with beacon tokens) - Extend SuggestionSession model with is_anonymous, submitter_name, submitter_email, and client_ip columns - Add ontokit/schemas/anonymous_suggestion.py with AnonymousSessionCreateResponse, AnonymousSubmitRequest (honeypot field aliased as 'website'), and AnonymousSubmitResponse --- ontokit/core/anonymous_token.py | 85 +++++++++++++++++++++++++ ontokit/models/suggestion_session.py | 8 ++- ontokit/schemas/anonymous_suggestion.py | 48 ++++++++++++++ 3 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 ontokit/core/anonymous_token.py create mode 100644 ontokit/schemas/anonymous_suggestion.py diff --git a/ontokit/core/anonymous_token.py b/ontokit/core/anonymous_token.py new file mode 100644 index 00000000..9f8f0ea9 --- /dev/null +++ b/ontokit/core/anonymous_token.py @@ -0,0 +1,85 @@ +"""HMAC-based anonymous session token for unauthenticated suggestion workflows. + +Anonymous tokens are long-lived, session-scoped tokens that allow +unauthenticated users to save and submit suggestion sessions without +needing a Bearer token. They are used when AUTH_MODE is "optional" or "disabled". +""" + +import base64 +import hashlib +import hmac +import json +import time + +from ontokit.core.config import settings + +_INSECURE_DEFAULTS = {"change-me-in-production", ""} +_MIN_SECRET_LENGTH = 16 + +# Prefix added to HMAC input to prevent token type confusion with beacon tokens +_HMAC_PREFIX = "anon:" + + +def _check_secret_key() -> None: + """Raise if secret_key is an insecure placeholder or too short.""" + key = settings.secret_key + if key in _INSECURE_DEFAULTS or len(key) < _MIN_SECRET_LENGTH: + raise RuntimeError( + "SECRET_KEY is not configured securely. " + "Set a strong, random SECRET_KEY (>= 16 characters) before using anonymous tokens." + ) + + +def create_anonymous_token(session_id: str, ttl: int = 86400) -> str: + """Create an HMAC-signed anonymous session token. + + Args: + session_id: The suggestion session ID to scope the token to. + ttl: Time-to-live in seconds (default 24 hours). + + Returns: + Base64url-encoded token string. + """ + _check_secret_key() + if ttl <= 0: + raise ValueError("ttl must be a positive number of seconds") + payload = json.dumps({"sid": session_id, "exp": int(time.time()) + ttl}) + # Prepend "anon:" to differentiate from beacon tokens using the same secret + sig = hmac.new( + settings.secret_key.encode(), (_HMAC_PREFIX + payload).encode(), hashlib.sha256 + ).hexdigest() + return base64.urlsafe_b64encode(f"{payload}|{sig}".encode()).decode() + + +def verify_anonymous_token(token: str) -> str | None: + """Verify an anonymous session token and return the session_id if valid. + + Args: + token: The base64url-encoded token string. + + Returns: + The session_id if the token is valid and not expired, None otherwise. + """ + _check_secret_key() + try: + decoded = base64.urlsafe_b64decode(token.encode()).decode() + payload_str, sig = decoded.rsplit("|", 1) + expected = hmac.new( + settings.secret_key.encode(), + (_HMAC_PREFIX + payload_str).encode(), + hashlib.sha256, + ).hexdigest() + if not hmac.compare_digest(sig, expected): + return None + payload = json.loads(payload_str) + if not isinstance(payload, dict): + return None + exp = payload.get("exp") + sid = payload.get("sid") + if not isinstance(exp, (int, float)) or not isinstance(sid, str): + return None + if time.time() > exp: + return None + return sid + except Exception: + return None diff --git a/ontokit/models/suggestion_session.py b/ontokit/models/suggestion_session.py index 696832e3..9ff83abe 100644 --- a/ontokit/models/suggestion_session.py +++ b/ontokit/models/suggestion_session.py @@ -5,7 +5,7 @@ from enum import StrEnum from typing import TYPE_CHECKING -from sqlalchemy import DateTime, ForeignKey, Integer, String, Text, UniqueConstraint, func +from sqlalchemy import Boolean, DateTime, ForeignKey, Integer, String, Text, UniqueConstraint, func from sqlalchemy.orm import Mapped, mapped_column, relationship if TYPE_CHECKING: @@ -54,6 +54,12 @@ class SuggestionSession(Base): # Auth beacon_token: Mapped[str] = mapped_column(String(500), nullable=False) + # Anonymous session fields + is_anonymous: Mapped[bool] = mapped_column(Boolean, default=False, server_default="false") + submitter_name: Mapped[str | None] = mapped_column(String(255), nullable=True) + submitter_email: Mapped[str | None] = mapped_column(String(255), nullable=True) + client_ip: Mapped[str | None] = mapped_column(String(45), nullable=True) + # PR link (set after submit) pr_number: Mapped[int | None] = mapped_column(Integer, nullable=True) pr_id: Mapped[uuid.UUID | None] = mapped_column( diff --git a/ontokit/schemas/anonymous_suggestion.py b/ontokit/schemas/anonymous_suggestion.py new file mode 100644 index 00000000..1b0bf68d --- /dev/null +++ b/ontokit/schemas/anonymous_suggestion.py @@ -0,0 +1,48 @@ +"""Schemas for anonymous suggestion session endpoints.""" + +from datetime import datetime + +from pydantic import BaseModel, Field + + +class AnonymousSessionCreateResponse(BaseModel): + """Response when creating an anonymous suggestion session.""" + + session_id: str + branch: str + created_at: datetime + anonymous_token: str + + class Config: + from_attributes = True + + +class AnonymousSubmitRequest(BaseModel): + """Request body for submitting an anonymous suggestion session. + + Includes optional credit fields and a honeypot field. + Bots filling the honeypot (aliased as 'website') trigger a silent fake success. + """ + + summary: str | None = Field(default=None, description="Optional summary of the changes") + submitter_name: str | None = Field( + default=None, description="Optional name to credit with the suggestion" + ) + submitter_email: str | None = Field( + default=None, description="Optional email to associate with the suggestion" + ) + honeypot: str | None = Field( + default=None, + alias="website", + description="Honeypot field — must be empty; bots fill this automatically", + ) + + model_config = {"populate_by_name": True} + + +class AnonymousSubmitResponse(BaseModel): + """Response after submitting an anonymous suggestion session.""" + + pr_number: int + pr_url: str | None = None + status: str From a962ad51f3620960d0267b6eaf3d706357a3d2a6 Mon Sep 17 00:00:00 2001 From: damienriehl Date: Fri, 3 Apr 2026 17:47:01 -0500 Subject: [PATCH 06/12] feat(10-01): anonymous suggestion endpoints, service methods, and rate limiting - Add ontokit/api/routes/anonymous_suggestions.py with create, save, submit, discard, and beacon endpoints - All endpoints gate on AUTH_MODE != 'required' (return 403 otherwise) - Rate limit: create endpoint checks for >= 5 anonymous sessions from same IP in last hour, returns 429 - Save/submit/discard authenticate via X-Anonymous-Token header using verify_anonymous_token() - Submit endpoint silently returns fake success for honeypot-filled bot requests - Add create_anonymous_session, save_anonymous, submit_anonymous, discard_anonymous methods to SuggestionService - Register anonymous_suggestions router in ontokit/api/routes/__init__.py under /projects prefix --- ontokit/api/routes/__init__.py | 4 + ontokit/api/routes/anonymous_suggestions.py | 176 +++++++++++++ ontokit/services/suggestion_service.py | 258 ++++++++++++++++++++ 3 files changed, 438 insertions(+) create mode 100644 ontokit/api/routes/anonymous_suggestions.py diff --git a/ontokit/api/routes/__init__.py b/ontokit/api/routes/__init__.py index 6e70e596..aaf3a57c 100644 --- a/ontokit/api/routes/__init__.py +++ b/ontokit/api/routes/__init__.py @@ -4,6 +4,7 @@ from ontokit.api.routes import ( analytics, + anonymous_suggestions, auth, classes, embeddings, @@ -39,6 +40,9 @@ router.include_router(classes.router, tags=["Classes"]) router.include_router(properties.router, tags=["Properties"]) router.include_router(suggestions.router, prefix="/projects", tags=["Suggestions"]) +router.include_router( + anonymous_suggestions.router, prefix="/projects", tags=["anonymous-suggestions"] +) router.include_router(remote_sync.router, prefix="/projects", tags=["Sync from Remote"]) router.include_router(notifications.router, prefix="/notifications", tags=["Notifications"]) router.include_router(search.router, prefix="/search", tags=["Search"]) diff --git a/ontokit/api/routes/anonymous_suggestions.py b/ontokit/api/routes/anonymous_suggestions.py new file mode 100644 index 00000000..10255621 --- /dev/null +++ b/ontokit/api/routes/anonymous_suggestions.py @@ -0,0 +1,176 @@ +"""Anonymous suggestion session endpoints. + +Provides create/save/submit/discard/beacon endpoints for unauthenticated users. +All endpoints are gated on AUTH_MODE != "required". +""" + +from typing import Annotated +from uuid import UUID + +from fastapi import APIRouter, Depends, Header, Query, Request, status +from fastapi.responses import Response +from sqlalchemy.ext.asyncio import AsyncSession + +from ontokit.core.anonymous_token import verify_anonymous_token +from ontokit.core.config import settings +from ontokit.core.database import get_db +from ontokit.schemas.anonymous_suggestion import ( + AnonymousSessionCreateResponse, + AnonymousSubmitRequest, + AnonymousSubmitResponse, +) +from ontokit.schemas.suggestion import ( + SuggestionBeaconRequest, + SuggestionSaveRequest, + SuggestionSaveResponse, +) +from ontokit.services.suggestion_service import SuggestionService, get_suggestion_service + +router = APIRouter() + + +def _require_anonymous_mode() -> None: + """Raise 403 if anonymous suggestions are not enabled.""" + from fastapi import HTTPException + + if settings.auth_mode == "required": + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Anonymous suggestions not available", + ) + + +def _verify_anon_token(x_anonymous_token: str) -> str: + """Verify the X-Anonymous-Token header and return the session_id. + + Raises 401 if the token is missing, invalid, or expired. + """ + from fastapi import HTTPException + + verified = verify_anonymous_token(x_anonymous_token) + if verified is None: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Invalid or expired anonymous token", + ) + return verified + + +def get_service(db: Annotated[AsyncSession, Depends(get_db)]) -> SuggestionService: + """Dependency to get suggestion service with database session.""" + return get_suggestion_service(db) + + +@router.post( + "/{project_id}/suggestions/anonymous/sessions", + response_model=AnonymousSessionCreateResponse, + status_code=status.HTTP_201_CREATED, +) +async def create_anonymous_session( + project_id: UUID, + request: Request, + service: Annotated[SuggestionService, Depends(get_service)], +) -> AnonymousSessionCreateResponse: + """Create a new anonymous suggestion session. + + No authentication required. Rate-limited to 5 sessions per IP per hour. + Only available when AUTH_MODE is not "required". + """ + _require_anonymous_mode() + client_ip = request.client.host if request.client else "unknown" + return await service.create_anonymous_session(project_id, client_ip) + + +@router.put( + "/{project_id}/suggestions/anonymous/sessions/{session_id}/save", + response_model=SuggestionSaveResponse, +) +async def save_anonymous_session( + project_id: UUID, + session_id: str, + data: SuggestionSaveRequest, + service: Annotated[SuggestionService, Depends(get_service)], + x_anonymous_token: Annotated[str, Header()], +) -> SuggestionSaveResponse: + """Save content to an anonymous suggestion session. + + Authenticated via X-Anonymous-Token header. + """ + _require_anonymous_mode() + verified_session_id = _verify_anon_token(x_anonymous_token) + return await service.save_anonymous(project_id, session_id, data, verified_session_id) + + +@router.post( + "/{project_id}/suggestions/anonymous/sessions/{session_id}/submit", + response_model=AnonymousSubmitResponse, +) +async def submit_anonymous_session( + project_id: UUID, + session_id: str, + data: AnonymousSubmitRequest, + service: Annotated[SuggestionService, Depends(get_service)], + x_anonymous_token: Annotated[str, Header()], +) -> AnonymousSubmitResponse: + """Submit an anonymous suggestion session as a pull request. + + Authenticated via X-Anonymous-Token header. + Honeypot field ('website') triggers silent fake success for bot detection. + """ + _require_anonymous_mode() + verified_session_id = _verify_anon_token(x_anonymous_token) + + # Honeypot check: bots fill the 'website' field, humans leave it blank + if data.honeypot is not None and data.honeypot != "": + # Silent fake success — do not create anything + return AnonymousSubmitResponse(pr_number=0, pr_url=None, status="submitted") + + return await service.submit_anonymous(project_id, session_id, data, verified_session_id) + + +@router.post( + "/{project_id}/suggestions/anonymous/sessions/{session_id}/discard", + status_code=status.HTTP_204_NO_CONTENT, +) +async def discard_anonymous_session( + project_id: UUID, + session_id: str, + service: Annotated[SuggestionService, Depends(get_service)], + x_anonymous_token: Annotated[str, Header()], +) -> Response: + """Discard an anonymous suggestion session and delete its branch. + + Authenticated via X-Anonymous-Token header. + """ + _require_anonymous_mode() + verified_session_id = _verify_anon_token(x_anonymous_token) + await service.discard_anonymous(project_id, session_id, verified_session_id) + return Response(status_code=status.HTTP_204_NO_CONTENT) + + +@router.post( + "/{project_id}/suggestions/anonymous/beacon", + status_code=status.HTTP_204_NO_CONTENT, +) +async def anonymous_beacon_save( + project_id: UUID, + data: SuggestionBeaconRequest, + service: Annotated[SuggestionService, Depends(get_service)], + token: str = Query(..., description="Anonymous session token for authentication"), +) -> Response: + """Handle a sendBeacon flush for anonymous sessions. + + Authenticated via 'token' query parameter (same pattern as authenticated beacon). + """ + _require_anonymous_mode() + verified_session_id = verify_anonymous_token(token) + if verified_session_id is None: + from fastapi import HTTPException + + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Invalid or expired anonymous token", + ) + # Delegate to the existing beacon_save (session lookup is by session_id, no user check) + await service.beacon_save(project_id, data, data.session_id) + return Response(status_code=status.HTTP_204_NO_CONTENT) diff --git a/ontokit/services/suggestion_service.py b/ontokit/services/suggestion_service.py index 36bd64e7..0640f976 100644 --- a/ontokit/services/suggestion_service.py +++ b/ontokit/services/suggestion_service.py @@ -10,6 +10,8 @@ from typing import TYPE_CHECKING from uuid import UUID +from ontokit.core.anonymous_token import create_anonymous_token + if TYPE_CHECKING: from ontokit.models.pull_request import PullRequest @@ -21,6 +23,11 @@ from ontokit.core.auth import CurrentUser from ontokit.core.beacon_token import create_beacon_token, verify_beacon_token +from ontokit.schemas.anonymous_suggestion import ( + AnonymousSessionCreateResponse, + AnonymousSubmitRequest, + AnonymousSubmitResponse, +) from ontokit.git import GitRepositoryService, get_git_service from ontokit.models.project import Project from ontokit.models.suggestion_session import SuggestionSession, SuggestionSessionStatus @@ -812,6 +819,257 @@ async def beacon_save( session.last_activity = datetime.now(UTC) await self.db.commit() + # --- Anonymous session methods --- + + async def create_anonymous_session( + self, project_id: UUID, client_ip: str + ) -> AnonymousSessionCreateResponse: + """Create an anonymous suggestion session with rate limiting. + + Checks that fewer than 5 anonymous sessions have been created from + the same IP address in the last hour before creating a new one. + """ + from sqlalchemy import func as sa_func + + # Verify project exists + await self._get_project(project_id) + + # Rate limit check: max 5 anonymous sessions per IP per hour + cutoff = datetime.now(UTC) - timedelta(hours=1) + rate_result = await self.db.execute( + select(sa_func.count(SuggestionSession.id)).where( + SuggestionSession.project_id == project_id, + SuggestionSession.is_anonymous.is_(True), + SuggestionSession.client_ip == client_ip, + SuggestionSession.created_at > cutoff, + ) + ) + session_count = rate_result.scalar() or 0 + if session_count >= 5: + raise HTTPException( + status_code=status.HTTP_429_TOO_MANY_REQUESTS, + detail="Rate limit exceeded. Try again later.", + ) + + # Generate identifiers + session_id = f"s_{secrets.token_hex(8)}" + branch = f"suggest/anonymous/{session_id}" + anonymous_token = create_anonymous_token(session_id) + beacon_token = create_beacon_token(session_id) + anon_user_id = f"anonymous-{secrets.token_hex(6)}" + + # Create the git branch + try: + self.git_service.create_branch(project_id, branch) + except Exception as e: + logger.error(f"Failed to create anonymous suggestion branch: {e}") + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Failed to create suggestion branch", + ) from e + + # Create the database record + db_session = SuggestionSession( + project_id=project_id, + user_id=anon_user_id, + user_name="Anonymous", + user_email=None, + session_id=session_id, + branch=branch, + beacon_token=beacon_token, + is_anonymous=True, + client_ip=client_ip, + ) + try: + self.db.add(db_session) + await self.db.commit() + except Exception: + await self.db.rollback() + try: + self.git_service.delete_branch(project_id, branch, force=True) + except Exception: + logger.warning(f"Failed to clean up orphaned anonymous branch {branch}") + raise + + try: + await self.db.refresh(db_session) + except Exception: + logger.warning("Anonymous session %s committed but refresh failed", session_id) + re_result = await self.db.execute( + select(SuggestionSession).where( + SuggestionSession.project_id == project_id, + SuggestionSession.session_id == session_id, + ) + ) + db_session = re_result.scalar_one() + + return AnonymousSessionCreateResponse( + session_id=db_session.session_id, + branch=db_session.branch, + created_at=db_session.created_at, + anonymous_token=anonymous_token, + ) + + async def save_anonymous( + self, + project_id: UUID, + session_id: str, + data: SuggestionSaveRequest, + verified_session_id: str, + ) -> SuggestionSaveResponse: + """Save content to an anonymous suggestion session branch.""" + session = await self._get_session(project_id, session_id) + + # Verify the token belongs to this session + if verified_session_id != session.session_id: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Token does not match session", + ) + if not session.is_anonymous: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Session is not an anonymous session", + ) + if session.status != SuggestionSessionStatus.ACTIVE.value: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"Session is {session.status}, cannot save", + ) + + project = await self._get_project(project_id) + filename = self._get_git_ontology_path(project) + + async with _branch_locks[session.branch]: + commit_message = f"Update {data.entity_label}" + try: + commit_info = self.git_service.commit_to_branch( # type: ignore[attr-defined] + project_id=project_id, + branch_name=session.branch, + ontology_content=data.content.encode("utf-8"), + filename=filename, + message=commit_message, + author_name=session.user_name or "Anonymous", + author_email=session.user_email or "anonymous@ontokit.dev", + ) + except Exception as e: + logger.error(f"Failed to save anonymous suggestion: {e}") + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Failed to save suggestion to branch", + ) from e + + session.changes_count += 1 + self._update_entities_modified(session, data.entity_label) + session.last_activity = datetime.now(UTC) + try: + await self.db.commit() + except Exception as e: + await self.db.rollback() + logger.error( + "Failed to update anonymous session metadata: session=%s error=%s", + session.session_id, + e, + ) + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Saved to branch but failed to update session metadata", + ) from e + + return SuggestionSaveResponse( + commit_hash=commit_info.hash, + branch=session.branch, + changes_count=session.changes_count, + ) + + async def submit_anonymous( + self, + project_id: UUID, + session_id: str, + data: AnonymousSubmitRequest, + verified_session_id: str, + ) -> AnonymousSubmitResponse: + """Submit an anonymous suggestion session as a pull request.""" + session = await self._get_session(project_id, session_id) + + if verified_session_id != session.session_id: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Token does not match session", + ) + if not session.is_anonymous: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Session is not an anonymous session", + ) + if session.status != SuggestionSessionStatus.ACTIVE.value: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"Session is {session.status}, cannot submit", + ) + if session.changes_count == 0: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="No changes to submit", + ) + + # Store optional credit info + if data.submitter_name or data.submitter_email: + session.submitter_name = data.submitter_name + session.submitter_email = data.submitter_email + # Update user_name so the PR description shows the provided credit name + session.user_name = data.submitter_name or "Anonymous" + + mock_user = CurrentUser( + id=session.user_id, + name=session.user_name or "Anonymous", + email=session.submitter_email, + ) + + result = await self._create_pr_for_session( + project_id, session, mock_user, data.summary, "submitted" + ) + + return AnonymousSubmitResponse( + pr_number=result.pr_number, + pr_url=result.pr_url, + status=result.status, + ) + + async def discard_anonymous( + self, + project_id: UUID, + session_id: str, + verified_session_id: str, + ) -> None: + """Discard an anonymous suggestion session and delete its branch.""" + session = await self._get_session(project_id, session_id) + + if verified_session_id != session.session_id: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Token does not match session", + ) + if not session.is_anonymous: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Session is not an anonymous session", + ) + if session.status != SuggestionSessionStatus.ACTIVE.value: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"Session is {session.status}, cannot discard", + ) + + try: + self.git_service.delete_branch(project_id, session.branch, force=True) + except Exception as e: + logger.warning(f"Failed to delete anonymous suggestion branch {session.branch}: {e}") + + session.status = SuggestionSessionStatus.DISCARDED.value + session.last_activity = datetime.now(UTC) + await self.db.commit() + async def auto_submit_stale_sessions(self) -> int: """Auto-create PRs for stale suggestion sessions. From 3d1cbcba543783aab1b950e3bfffe8f9e3ffe3b0 Mon Sep 17 00:00:00 2001 From: damienriehl Date: Fri, 3 Apr 2026 17:47:38 -0500 Subject: [PATCH 07/12] feat(10-01): update review summaries to show anonymous submitter info - Add is_anonymous field to SuggestionSessionSummary schema - _build_summary: use submitter_name/email (credit info) over user_name/email for anonymous sessions - _create_pr_for_session: show 'Submitted anonymously' or 'Submitted by {name}' for anonymous sessions - Existing authenticated session summaries are unchanged (backward compatible) --- ontokit/schemas/suggestion.py | 1 + ontokit/services/suggestion_service.py | 23 +++++++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/ontokit/schemas/suggestion.py b/ontokit/schemas/suggestion.py index 62d83ba5..414a8dd1 100644 --- a/ontokit/schemas/suggestion.py +++ b/ontokit/schemas/suggestion.py @@ -72,6 +72,7 @@ class SuggestionSessionSummary(BaseModel): reviewed_at: datetime | None = None revision: int | None = None summary: str | None = None + is_anonymous: bool = False model_config = ConfigDict(from_attributes=True) diff --git a/ontokit/services/suggestion_service.py b/ontokit/services/suggestion_service.py index 0640f976..cc60a771 100644 --- a/ontokit/services/suggestion_service.py +++ b/ontokit/services/suggestion_service.py @@ -395,7 +395,15 @@ async def _create_pr_for_session( body_parts.append(f"\n**Entities modified** ({session.changes_count} changes):") for entity in entities: body_parts.append(f"- {entity}") - body_parts.append(f"\n*Submitted by {session.user_name or session.user_id}*") + is_anonymous = getattr(session, "is_anonymous", False) + if is_anonymous: + submitter_name = getattr(session, "submitter_name", None) + if submitter_name: + body_parts.append(f"\n*Submitted by {submitter_name}*") + else: + body_parts.append("\n*Submitted anonymously*") + else: + body_parts.append(f"\n*Submitted by {session.user_name or session.user_id}*") description = "\n".join(body_parts) # Check for an existing PR on this branch (idempotency on retry) @@ -537,7 +545,17 @@ async def _build_summary(self, s: SuggestionSession) -> SuggestionSessionSummary pr_url = pr.github_pr_url if hasattr(pr, "github_pr_url") else None github_pr_url = pr_url - submitter = SuggestionUser(id=s.user_id, name=s.user_name, email=s.user_email) + # For anonymous sessions, prefer submitter_name/email (credit info collected at submit) + # over the generic user_name/email set at session creation. + is_anonymous = getattr(s, "is_anonymous", False) + if is_anonymous: + submitter_name = getattr(s, "submitter_name", None) or s.user_name or "Anonymous" + submitter_email = getattr(s, "submitter_email", None) or s.user_email + else: + submitter_name = s.user_name + submitter_email = s.user_email + + submitter = SuggestionUser(id=s.user_id, name=submitter_name, email=submitter_email) reviewer = None if s.reviewer_id: reviewer = SuggestionUser( @@ -560,6 +578,7 @@ async def _build_summary(self, s: SuggestionSession) -> SuggestionSessionSummary reviewed_at=s.reviewed_at, revision=s.revision, summary=s.summary, + is_anonymous=is_anonymous, ) def _can_review(self, role: str | None, user: CurrentUser) -> bool: From 7ee86c8b069b8d9bb9ff1238b1c31bcf5598cc48 Mon Sep 17 00:00:00 2001 From: damienriehl Date: Fri, 3 Apr 2026 19:09:35 -0500 Subject: [PATCH 08/12] feat: add alembic migration for anonymous suggestion fields Adds is_anonymous, submitter_name, submitter_email, client_ip columns to suggestion_sessions table. These were added to the model in plan 10-01 but the migration was missing. Co-Authored-By: Claude Opus 4.6 (1M context) --- ...0w1x2y3_add_anonymous_suggestion_fields.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 alembic/versions/t8u9v0w1x2y3_add_anonymous_suggestion_fields.py diff --git a/alembic/versions/t8u9v0w1x2y3_add_anonymous_suggestion_fields.py b/alembic/versions/t8u9v0w1x2y3_add_anonymous_suggestion_fields.py new file mode 100644 index 00000000..c9b5be3a --- /dev/null +++ b/alembic/versions/t8u9v0w1x2y3_add_anonymous_suggestion_fields.py @@ -0,0 +1,29 @@ +"""Add anonymous suggestion fields to suggestion_sessions. + +Revision ID: t8u9v0w1x2y3 +Revises: s7t8u9v0w1x2 +Create Date: 2026-04-03 +""" + +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = "t8u9v0w1x2y3" +down_revision = "v9w0x1y2z3a4" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column("suggestion_sessions", sa.Column("is_anonymous", sa.Boolean(), server_default="false", nullable=False)) + op.add_column("suggestion_sessions", sa.Column("submitter_name", sa.String(), nullable=True)) + op.add_column("suggestion_sessions", sa.Column("submitter_email", sa.String(), nullable=True)) + op.add_column("suggestion_sessions", sa.Column("client_ip", sa.String(), nullable=True)) + + +def downgrade() -> None: + op.drop_column("suggestion_sessions", "client_ip") + op.drop_column("suggestion_sessions", "submitter_email") + op.drop_column("suggestion_sessions", "submitter_name") + op.drop_column("suggestion_sessions", "is_anonymous") From 79209715ed30a79b5cdbbf71660d87c16de14026 Mon Sep 17 00:00:00 2001 From: damienriehl Date: Tue, 7 Jul 2026 19:08:31 -0500 Subject: [PATCH 09/12] harden(pr-7): fix dead anonymous beacon + public-project gate + endpoint tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - BLOCKER-class functional fix: the lineage's anonymous beacon route passed data.session_id where beacon_save expects a BEACON token — verify_beacon_token (session_id) always returned None, so the endpoint 401'd on every request (fails closed; dead code). New SuggestionService.beacon_save_anonymous binds the route-verified X-Anonymous-Token session to the payload session (403 on mismatch), re-checks is_anonymous (an anonymous token must never flush an authenticated session), and shares the branch-locked flush via _beacon_flush. - Security gate: create_anonymous_session now requires project.is_public — anonymous users could previously create suggestion branches/PRs against PRIVATE projects (rate limiting was the only guard). - Tests (lineage shipped none): 11 new — AUTH_MODE 403 sweep, garbage-token 401s, verified-session forwarding (save + beacon), honeypot silent fake success without service call, service-level private-project/mismatch/ authenticated-session guards. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q --- ontokit/api/routes/anonymous_suggestions.py | 15 +- ontokit/services/suggestion_service.py | 52 ++++- .../unit/test_anonymous_suggestions_routes.py | 204 ++++++++++++++++++ tests/unit/test_suggestion_service.py | 5 + 4 files changed, 261 insertions(+), 15 deletions(-) create mode 100644 tests/unit/test_anonymous_suggestions_routes.py diff --git a/ontokit/api/routes/anonymous_suggestions.py b/ontokit/api/routes/anonymous_suggestions.py index 10255621..17ef987f 100644 --- a/ontokit/api/routes/anonymous_suggestions.py +++ b/ontokit/api/routes/anonymous_suggestions.py @@ -163,14 +163,9 @@ async def anonymous_beacon_save( Authenticated via 'token' query parameter (same pattern as authenticated beacon). """ _require_anonymous_mode() - verified_session_id = verify_anonymous_token(token) - if verified_session_id is None: - from fastapi import HTTPException - - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Invalid or expired anonymous token", - ) - # Delegate to the existing beacon_save (session lookup is by session_id, no user check) - await service.beacon_save(project_id, data, data.session_id) + verified_session_id = _verify_anon_token(token) + # beacon_save_anonymous binds the verified token to the payload session and + # re-checks is_anonymous (the lineage version passed data.session_id where a + # BEACON token was expected — the endpoint always 401'd; fixed in PR-7). + await service.beacon_save_anonymous(project_id, data, verified_session_id) return Response(status_code=status.HTTP_204_NO_CONTENT) diff --git a/ontokit/services/suggestion_service.py b/ontokit/services/suggestion_service.py index cc60a771..d397004c 100644 --- a/ontokit/services/suggestion_service.py +++ b/ontokit/services/suggestion_service.py @@ -23,14 +23,14 @@ from ontokit.core.auth import CurrentUser from ontokit.core.beacon_token import create_beacon_token, verify_beacon_token +from ontokit.git import GitRepositoryService, get_git_service +from ontokit.models.project import Project +from ontokit.models.suggestion_session import SuggestionSession, SuggestionSessionStatus from ontokit.schemas.anonymous_suggestion import ( AnonymousSessionCreateResponse, AnonymousSubmitRequest, AnonymousSubmitResponse, ) -from ontokit.git import GitRepositoryService, get_git_service -from ontokit.models.project import Project -from ontokit.models.suggestion_session import SuggestionSession, SuggestionSessionStatus from ontokit.schemas.pull_request import PRCreate from ontokit.schemas.suggestion import ( SuggestionBeaconRequest, @@ -814,6 +814,42 @@ async def beacon_save( ) await self._verify_project_access(project_id, session_user) + await self._beacon_flush(project_id, session, data) + + async def beacon_save_anonymous( + self, project_id: UUID, data: SuggestionBeaconRequest, verified_session_id: str + ) -> None: + """Handle a beacon save for an ANONYMOUS session. + + The caller (route) has already verified the X-Anonymous-Token; this + method binds it to the payload's session and re-checks the session is + actually anonymous (an anonymous token must never flush an + authenticated user's session). No user-identity access re-check: the + session's project is public by construction (create-time gate). + """ + if verified_session_id != data.session_id: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Token does not match session", + ) + + session = await self._get_session(project_id, data.session_id) + + if not session.is_anonymous: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Session is not an anonymous session", + ) + + if session.status != SuggestionSessionStatus.ACTIVE.value: + return # Silently ignore saves to non-active sessions + + await self._beacon_flush(project_id, session, data) + + async def _beacon_flush( + self, project_id: UUID, session: SuggestionSession, data: SuggestionBeaconRequest + ) -> None: + """Commit a beacon payload to the session branch (fire-and-forget).""" project = await self._get_project(project_id) filename = self._get_git_ontology_path(project) @@ -850,8 +886,14 @@ async def create_anonymous_session( """ from sqlalchemy import func as sa_func - # Verify project exists - await self._get_project(project_id) + # Verify project exists AND is public — anonymous users must never be + # able to create suggestion branches/PRs against a private project. + project = await self._get_project(project_id) + if not project.is_public: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Anonymous suggestions are only available on public projects", + ) # Rate limit check: max 5 anonymous sessions per IP per hour cutoff = datetime.now(UTC) - timedelta(hours=1) diff --git a/tests/unit/test_anonymous_suggestions_routes.py b/tests/unit/test_anonymous_suggestions_routes.py new file mode 100644 index 00000000..c4a45918 --- /dev/null +++ b/tests/unit/test_anonymous_suggestions_routes.py @@ -0,0 +1,204 @@ +"""Endpoint + guard tests for the anonymous suggestion routes (PR-7). + +The lineage shipped these endpoints without tests. Coverage here pins: +- the AUTH_MODE gate (403 everywhere when auth is required), +- X-Anonymous-Token verification (401 on garbage), +- token→session binding at the routes that delegate it to the service, +- the honeypot silent-fake-success path (no service call), +- the PR-7 beacon fix (route passes the VERIFIED session id to + beacon_save_anonymous — the lineage passed data.session_id where a beacon + token was expected, so the endpoint always 401'd), +- the service-level public-project gate and anonymous-beacon binding. +""" + +from __future__ import annotations + +from unittest.mock import AsyncMock, MagicMock, patch +from uuid import uuid4 + +import pytest +from fastapi import HTTPException +from fastapi.testclient import TestClient + +from ontokit.core.anonymous_token import create_anonymous_token +from ontokit.schemas.suggestion import SuggestionBeaconRequest + +PROJECT_ID = "11111111-1111-1111-1111-111111111111" +BASE = f"/api/v1/projects/{PROJECT_ID}/suggestions/anonymous" + +SAVE_BODY = { + "content": "ex:Foo a owl:Class .", + "entity_iri": "http://example.org/ont#Foo", + "entity_label": "Foo", +} +SUBMIT_BODY = {"summary": "adds Foo"} + + +# ── AUTH_MODE gate ──────────────────────────────────────────────────────────── + + +def test_all_endpoints_403_when_auth_mode_required(client: TestClient) -> None: + with patch( + "ontokit.api.routes.anonymous_suggestions.settings" + ) as settings_mock: + settings_mock.auth_mode = "required" + token = "irrelevant" + responses = [ + client.post(f"{BASE}/sessions"), + client.put(f"{BASE}/sessions/s_x/save", json=SAVE_BODY, headers={"X-Anonymous-Token": token}), + client.post(f"{BASE}/sessions/s_x/submit", json=SUBMIT_BODY, headers={"X-Anonymous-Token": token}), + client.post(f"{BASE}/sessions/s_x/discard", headers={"X-Anonymous-Token": token}), + client.post(f"{BASE}/beacon?token={token}", json={"session_id": "s_x", "content": "x"}), + ] + assert [r.status_code for r in responses] == [403] * 5 + + +# ── Token verification ──────────────────────────────────────────────────────── + + +@pytest.mark.parametrize( + "method,url_suffix,kwargs", + [ + ("put", "/sessions/s_x/save", {"json": SAVE_BODY}), + ("post", "/sessions/s_x/submit", {"json": SUBMIT_BODY}), + ("post", "/sessions/s_x/discard", {}), + ], +) +def test_garbage_token_401(client: TestClient, method: str, url_suffix: str, kwargs: dict) -> None: + with patch("ontokit.api.routes.anonymous_suggestions.settings") as settings_mock: + settings_mock.auth_mode = "optional" + resp = getattr(client, method)( + f"{BASE}{url_suffix}", headers={"X-Anonymous-Token": "not-a-token"}, **kwargs + ) + assert resp.status_code == 401 + + +def test_beacon_garbage_token_401(client: TestClient) -> None: + with patch("ontokit.api.routes.anonymous_suggestions.settings") as settings_mock: + settings_mock.auth_mode = "optional" + resp = client.post( + f"{BASE}/beacon?token=not-a-token", + json={"session_id": "s_x", "content": "x"}, + ) + assert resp.status_code == 401 + + +# ── Verified-session forwarding (PR-7 beacon fix) ───────────────────────────── + + +def test_beacon_forwards_verified_session_id(client: TestClient) -> None: + token = create_anonymous_token("s_abc") + service = MagicMock() + service.beacon_save_anonymous = AsyncMock(return_value=None) + with ( + patch("ontokit.api.routes.anonymous_suggestions.settings") as settings_mock, + patch( + "ontokit.api.routes.anonymous_suggestions.get_suggestion_service", + return_value=service, + ), + ): + settings_mock.auth_mode = "optional" + resp = client.post( + f"{BASE}/beacon?token={token}", + json={"session_id": "s_abc", "content": "x"}, + ) + assert resp.status_code == 204 + service.beacon_save_anonymous.assert_awaited_once() + # third positional arg / kwarg is the VERIFIED session id from the token + call = service.beacon_save_anonymous.await_args + forwarded = call.args[2] if len(call.args) > 2 else call.kwargs.get("verified_session_id") + assert forwarded == "s_abc" + + +def test_save_forwards_verified_session_id(client: TestClient) -> None: + token = create_anonymous_token("s_abc") + service = MagicMock() + service.save_anonymous = AsyncMock( + return_value={"commit_hash": "deadbeef", "branch": "b", "changes_count": 1} + ) + with ( + patch("ontokit.api.routes.anonymous_suggestions.settings") as settings_mock, + patch( + "ontokit.api.routes.anonymous_suggestions.get_suggestion_service", + return_value=service, + ), + ): + settings_mock.auth_mode = "optional" + resp = client.put( + f"{BASE}/sessions/s_other/save", + json=SAVE_BODY, + headers={"X-Anonymous-Token": token}, + ) + assert resp.status_code == 200 + args = service.save_anonymous.await_args.args + assert args[-1] == "s_abc" # verified id travels separately from the path id + + +# ── Honeypot ────────────────────────────────────────────────────────────────── + + +def test_submit_honeypot_fake_success_without_service_call(client: TestClient) -> None: + token = create_anonymous_token("s_abc") + service = MagicMock() + service.submit_anonymous = AsyncMock() + with ( + patch("ontokit.api.routes.anonymous_suggestions.settings") as settings_mock, + patch( + "ontokit.api.routes.anonymous_suggestions.get_suggestion_service", + return_value=service, + ), + ): + settings_mock.auth_mode = "optional" + resp = client.post( + f"{BASE}/sessions/s_abc/submit", + json={**SUBMIT_BODY, "website": "http://spam.example"}, + headers={"X-Anonymous-Token": token}, + ) + assert resp.status_code == 200 + assert resp.json()["pr_number"] == 0 + service.submit_anonymous.assert_not_awaited() + + +# ── Service-level guards (PR-7 hardening) ───────────────────────────────────── + + +@pytest.mark.asyncio +async def test_create_anonymous_session_rejects_private_project() -> None: + from ontokit.services.suggestion_service import SuggestionService + + service = SuggestionService.__new__(SuggestionService) + private_project = MagicMock() + private_project.is_public = False + service._get_project = AsyncMock(return_value=private_project) # type: ignore[method-assign] + + with pytest.raises(HTTPException) as exc: + await service.create_anonymous_session(uuid4(), "1.2.3.4") + assert exc.value.status_code == 403 + + +@pytest.mark.asyncio +async def test_beacon_save_anonymous_rejects_session_mismatch() -> None: + from ontokit.services.suggestion_service import SuggestionService + + service = SuggestionService.__new__(SuggestionService) + data = SuggestionBeaconRequest(session_id="s_other", content="x") + + with pytest.raises(HTTPException) as exc: + await service.beacon_save_anonymous(uuid4(), data, "s_abc") + assert exc.value.status_code == 403 + + +@pytest.mark.asyncio +async def test_beacon_save_anonymous_rejects_authenticated_session() -> None: + """An anonymous token must never flush an authenticated user's session.""" + from ontokit.services.suggestion_service import SuggestionService + + service = SuggestionService.__new__(SuggestionService) + authed_session = MagicMock() + authed_session.is_anonymous = False + service._get_session = AsyncMock(return_value=authed_session) # type: ignore[method-assign] + data = SuggestionBeaconRequest(session_id="s_abc", content="x") + + with pytest.raises(HTTPException) as exc: + await service.beacon_save_anonymous(uuid4(), data, "s_abc") + assert exc.value.status_code == 403 diff --git a/tests/unit/test_suggestion_service.py b/tests/unit/test_suggestion_service.py index 719c3d14..86d41304 100644 --- a/tests/unit/test_suggestion_service.py +++ b/tests/unit/test_suggestion_service.py @@ -77,6 +77,11 @@ def _make_session( session.reviewed_at = None session.revision = 1 session.summary = None + # Anonymous-suggestion columns (PR-7): authenticated session defaults + session.is_anonymous = False + session.submitter_name = None + session.submitter_email = None + session.client_ip = None session.created_at = datetime.now(UTC) session.last_activity = last_activity or datetime.now(UTC) return session From f7c97c67de8461d6e052c0a8b9787ea9d57714d5 Mon Sep 17 00:00:00 2001 From: damienriehl Date: Tue, 7 Jul 2026 19:20:00 -0500 Subject: [PATCH 10/12] =?UTF-8?q?harden(pr-7):=20apply=20/ce:review=20find?= =?UTF-8?q?ings=20=E2=80=94=20global=20IP=20rate=20limit,=20anonymous=20br?= =?UTF-8?q?anch=20reaper,=20honeypot=20de-disclosure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - HIGH: 5-sessions/hour rate limit was scoped per (project_id, IP) — one IP could mint 5 sessions+branches on EVERY public project per hour. Now global per-IP (the docstring's original intent). - HIGH: anonymous git branches were never garbage-collected — sessions with changes_count==0 matched no sweep at all, and the authed sweep's access re-check always fails for the anonymous pseudo-user, discarding WITHOUT branch delete (orphaned-branch leak = unbounded unauthenticated git-storage abuse). New reap_stale_anonymous_sessions (atomic claim, discard + force branch delete at 24h = token TTL, includes zero-change sessions), wired into the worker cron next to the authed sweep, which now excludes anonymous sessions by design. - MEDIUM: honeypot semantics were disclosed verbatim in the public OpenAPI schema (field description, model docstring, route docstring) — now reads as an ordinary optional website URL; mechanism documented in code comments only. OpenAPI scan pins no leak. - MEDIUM (tests): new test_anonymous_token.py — TTL expiry, tamper, malformed, insecure-secret guard, and BOTH directions of anonymous<->beacon cross-token rejection (same-secret confusion). Reaper behavioral tests (branch delete + concurrent-claim skip). Worker test reconciled. Follow-ups noted on the PR (not blocking): is_public re-check on session lifecycle, PR title/body sanitization, content max_length, proxy-topology note for request.client.host, rate-limit TOCTOU. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q --- ontokit/api/routes/anonymous_suggestions.py | 1 - ontokit/schemas/anonymous_suggestion.py | 11 ++- ontokit/services/suggestion_service.py | 69 +++++++++++++++- ontokit/worker.py | 10 ++- .../unit/test_anonymous_suggestions_routes.py | 74 +++++++++++++++++ tests/unit/test_anonymous_token.py | 80 +++++++++++++++++++ tests/unit/test_worker.py | 2 + 7 files changed, 236 insertions(+), 11 deletions(-) create mode 100644 tests/unit/test_anonymous_token.py diff --git a/ontokit/api/routes/anonymous_suggestions.py b/ontokit/api/routes/anonymous_suggestions.py index 17ef987f..f7b1089c 100644 --- a/ontokit/api/routes/anonymous_suggestions.py +++ b/ontokit/api/routes/anonymous_suggestions.py @@ -115,7 +115,6 @@ async def submit_anonymous_session( """Submit an anonymous suggestion session as a pull request. Authenticated via X-Anonymous-Token header. - Honeypot field ('website') triggers silent fake success for bot detection. """ _require_anonymous_mode() verified_session_id = _verify_anon_token(x_anonymous_token) diff --git a/ontokit/schemas/anonymous_suggestion.py b/ontokit/schemas/anonymous_suggestion.py index 1b0bf68d..479fb690 100644 --- a/ontokit/schemas/anonymous_suggestion.py +++ b/ontokit/schemas/anonymous_suggestion.py @@ -18,11 +18,7 @@ class Config: class AnonymousSubmitRequest(BaseModel): - """Request body for submitting an anonymous suggestion session. - - Includes optional credit fields and a honeypot field. - Bots filling the honeypot (aliased as 'website') trigger a silent fake success. - """ + """Request body for submitting an anonymous suggestion session.""" summary: str | None = Field(default=None, description="Optional summary of the changes") submitter_name: str | None = Field( @@ -31,10 +27,13 @@ class AnonymousSubmitRequest(BaseModel): submitter_email: str | None = Field( default=None, description="Optional email to associate with the suggestion" ) + # Spam-control field. Deliberately documented as an ordinary optional + # website URL: the real semantics (filled -> silent fake success) must not + # appear in the public OpenAPI schema, which bots can read. honeypot: str | None = Field( default=None, alias="website", - description="Honeypot field — must be empty; bots fill this automatically", + description="Optional website URL", ) model_config = {"populate_by_name": True} diff --git a/ontokit/services/suggestion_service.py b/ontokit/services/suggestion_service.py index d397004c..b81be5b6 100644 --- a/ontokit/services/suggestion_service.py +++ b/ontokit/services/suggestion_service.py @@ -895,11 +895,12 @@ async def create_anonymous_session( detail="Anonymous suggestions are only available on public projects", ) - # Rate limit check: max 5 anonymous sessions per IP per hour + # Rate limit check: max 5 anonymous sessions per IP per hour — GLOBAL + # across projects on purpose. Scoping by project would let one IP mint + # 5 sessions (and git branches) on every public project per hour. cutoff = datetime.now(UTC) - timedelta(hours=1) rate_result = await self.db.execute( select(sa_func.count(SuggestionSession.id)).where( - SuggestionSession.project_id == project_id, SuggestionSession.is_anonymous.is_(True), SuggestionSession.client_ip == client_ip, SuggestionSession.created_at > cutoff, @@ -1143,6 +1144,11 @@ async def auto_submit_stale_sessions(self) -> int: SuggestionSession.status == SuggestionSessionStatus.ACTIVE.value, SuggestionSession.changes_count > 0, SuggestionSession.last_activity < cutoff, + # Anonymous sessions are handled by reap_stale_anonymous_sessions: + # their pseudo-user is never a project member, so the access + # re-check below would always discard them WITHOUT deleting the + # branch (orphaned-branch leak). + SuggestionSession.is_anonymous.is_(False), ) ) stale_sessions = result.scalars().all() @@ -1159,6 +1165,7 @@ async def auto_submit_stale_sessions(self) -> int: SuggestionSession.status == SuggestionSessionStatus.ACTIVE.value, SuggestionSession.changes_count > 0, SuggestionSession.last_activity < cutoff, + SuggestionSession.is_anonymous.is_(False), ) .values(status=SuggestionSessionStatus.AUTO_SUBMITTED.value) ) @@ -1213,6 +1220,64 @@ async def auto_submit_stale_sessions(self) -> int: return count + async def reap_stale_anonymous_sessions(self, ttl_hours: int = 24) -> int: + """Discard stale ANONYMOUS sessions and delete their git branches. + + Anonymous tokens expire after 24h, so past the TTL the session is + unreachable by its creator anyway. Without this reaper every abandoned + anonymous session leaves an orphaned git branch forever (the authed + sweep can't handle them: the anonymous pseudo-user is never a project + member). Includes sessions with changes_count == 0 — those were never + matched by any sweep at all. + + Returns the number of sessions reaped. + """ + cutoff = datetime.now(UTC) - timedelta(hours=ttl_hours) + + result = await self.db.execute( + select(SuggestionSession).where( + SuggestionSession.status == SuggestionSessionStatus.ACTIVE.value, + SuggestionSession.is_anonymous.is_(True), + SuggestionSession.last_activity < cutoff, + ) + ) + stale = result.scalars().all() + + count = 0 + for session in stale: + # Atomic claim (same pattern as auto_submit_stale_sessions) + claim_result = await self.db.execute( + update(SuggestionSession) + .where( + SuggestionSession.id == session.id, + SuggestionSession.status == SuggestionSessionStatus.ACTIVE.value, + SuggestionSession.is_anonymous.is_(True), + SuggestionSession.last_activity < cutoff, + ) + .values(status=SuggestionSessionStatus.DISCARDED.value) + ) + if claim_result.rowcount != 1: # type: ignore[attr-defined] + continue + await self.db.commit() + + try: + self.git_service.delete_branch(session.project_id, session.branch, force=True) + except Exception as e: + # Branch may already be gone; log and keep the discard. + logger.warning( + "Reaped anonymous session %s but branch delete failed: %s", + session.session_id, + e, + ) + count += 1 + logger.info( + "Reaped stale anonymous session %s (changes_count=%s)", + session.session_id, + session.changes_count, + ) + + return count + def get_suggestion_service(db: AsyncSession) -> SuggestionService: """Factory function for dependency injection.""" diff --git a/ontokit/worker.py b/ontokit/worker.py index d36ec7fe..d4c0d17f 100644 --- a/ontokit/worker.py +++ b/ontokit/worker.py @@ -631,9 +631,15 @@ async def auto_submit_stale_suggestions(ctx: dict[str, Any]) -> dict[str, Any]: service = SuggestionService(db) count = await service.auto_submit_stale_sessions() + # Anonymous sessions get a dedicated reaper (discard + branch delete at + # token TTL) — the authed sweep excludes them by design (PR-7). + reaped = await service.reap_stale_anonymous_sessions() - logger.info(f"Auto-submit complete: {count} stale suggestion sessions submitted") - return {"auto_submitted": count} + logger.info( + f"Auto-submit complete: {count} stale suggestion sessions submitted, " + f"{reaped} stale anonymous sessions reaped" + ) + return {"auto_submitted": count, "anonymous_reaped": reaped} except Exception as e: logger.exception(f"Auto-submit stale suggestions failed: {e}") diff --git a/tests/unit/test_anonymous_suggestions_routes.py b/tests/unit/test_anonymous_suggestions_routes.py index c4a45918..8498e9cc 100644 --- a/tests/unit/test_anonymous_suggestions_routes.py +++ b/tests/unit/test_anonymous_suggestions_routes.py @@ -202,3 +202,77 @@ async def test_beacon_save_anonymous_rejects_authenticated_session() -> None: with pytest.raises(HTTPException) as exc: await service.beacon_save_anonymous(uuid4(), data, "s_abc") assert exc.value.status_code == 403 + + +@pytest.mark.asyncio +async def test_reap_deletes_branch_and_discards() -> None: + """The anonymous reaper must delete the git branch (orphaned-branch leak fix).""" + from ontokit.services.suggestion_service import SuggestionService + + service = SuggestionService.__new__(SuggestionService) + stale = MagicMock() + stale.id = uuid4() + stale.project_id = uuid4() + stale.session_id = "s_stale" + stale.branch = "suggest/anonymous/s_stale" + stale.changes_count = 0 + + select_result = MagicMock() + select_result.scalars.return_value.all.return_value = [stale] + claim_result = MagicMock() + claim_result.rowcount = 1 + + db = MagicMock() + db.execute = AsyncMock(side_effect=[select_result, claim_result]) + db.commit = AsyncMock() + service.db = db + service.git_service = MagicMock() + + count = await service.reap_stale_anonymous_sessions() + + assert count == 1 + service.git_service.delete_branch.assert_called_once_with( + stale.project_id, stale.branch, force=True + ) + + +@pytest.mark.asyncio +async def test_reap_skips_sessions_claimed_by_another_worker() -> None: + from ontokit.services.suggestion_service import SuggestionService + + service = SuggestionService.__new__(SuggestionService) + stale = MagicMock() + stale.id = uuid4() + + select_result = MagicMock() + select_result.scalars.return_value.all.return_value = [stale] + claim_result = MagicMock() + claim_result.rowcount = 0 # another worker won the claim + + db = MagicMock() + db.execute = AsyncMock(side_effect=[select_result, claim_result]) + db.commit = AsyncMock() + service.db = db + service.git_service = MagicMock() + + count = await service.reap_stale_anonymous_sessions() + + assert count == 0 + service.git_service.delete_branch.assert_not_called() + + +def test_openapi_does_not_disclose_honeypot_semantics() -> None: + """The honeypot only works if the public schema doesn't explain it.""" + import json + + from ontokit.main import app + + schema = app.openapi() + request_schema = schema["components"]["schemas"]["AnonymousSubmitRequest"] + blob = json.dumps(request_schema) + json.dumps( + {p: ops for p, ops in schema["paths"].items() if "anonymous" in p} + ) + for needle in ("honeypot", "Honeypot", "bots", "bot detection", "fake success"): + assert needle not in blob, f"OpenAPI leaks honeypot semantics via {needle!r}" + # the field itself must still be present under its innocuous alias + assert "website" in request_schema["properties"] diff --git a/tests/unit/test_anonymous_token.py b/tests/unit/test_anonymous_token.py new file mode 100644 index 00000000..6271a8d5 --- /dev/null +++ b/tests/unit/test_anonymous_token.py @@ -0,0 +1,80 @@ +"""Unit tests for ontokit/core/anonymous_token.py (PR-7). + +Pins the invariants /ce:review called out as untested: +- TTL expiry and tamper rejection, +- malformed-token handling (uniform None, no exceptions), +- the insecure-SECRET_KEY guard, +- CROSS-TOKEN REJECTION between anonymous and beacon tokens — both are HMACs + under the same secret; the `anon:` prefix separation must hold in BOTH + directions or a beacon token could act as a session credential (and vice + versa). +""" + +from __future__ import annotations + +import time +from unittest.mock import patch + +import pytest + +from ontokit.core.anonymous_token import create_anonymous_token, verify_anonymous_token +from ontokit.core.beacon_token import create_beacon_token, verify_beacon_token + + +def test_round_trip() -> None: + token = create_anonymous_token("s_abc123") + assert verify_anonymous_token(token) == "s_abc123" + + +def test_expired_token_rejected() -> None: + token = create_anonymous_token("s_abc123", ttl=1) + with patch("ontokit.core.anonymous_token.time") as time_mock: + time_mock.time.return_value = time.time() + 10 + assert verify_anonymous_token(token) is None + + +def test_zero_or_negative_ttl_rejected_at_creation() -> None: + with pytest.raises(ValueError): + create_anonymous_token("s_abc123", ttl=0) + with pytest.raises(ValueError): + create_anonymous_token("s_abc123", ttl=-5) + + +def test_tampered_token_rejected() -> None: + token = create_anonymous_token("s_abc123") + # flip a character in the middle of the token + mid = len(token) // 2 + flipped = token[:mid] + ("A" if token[mid] != "A" else "B") + token[mid + 1 :] + assert verify_anonymous_token(flipped) is None + + +@pytest.mark.parametrize( + "garbage", + ["", "not-base64!!!", "aGVsbG8=", "e30=", "a.b.c"], +) +def test_malformed_tokens_return_none(garbage: str) -> None: + assert verify_anonymous_token(garbage) is None + + +def test_insecure_secret_key_guard() -> None: + with patch("ontokit.core.anonymous_token.settings") as settings_mock: + settings_mock.secret_key = "change-me-in-production" + with pytest.raises(RuntimeError): + create_anonymous_token("s_abc123") + with patch("ontokit.core.anonymous_token.settings") as settings_mock: + settings_mock.secret_key = "short" + with pytest.raises(RuntimeError): + create_anonymous_token("s_abc123") + + +# ── Cross-token confusion (anonymous vs beacon, same secret) ───────────────── + + +def test_beacon_token_never_verifies_as_anonymous_token() -> None: + beacon = create_beacon_token("s_abc123") + assert verify_anonymous_token(beacon) is None + + +def test_anonymous_token_never_verifies_as_beacon_token() -> None: + anon = create_anonymous_token("s_abc123") + assert verify_beacon_token(anon) is None diff --git a/tests/unit/test_worker.py b/tests/unit/test_worker.py index 94ae52cc..dbdfaa71 100644 --- a/tests/unit/test_worker.py +++ b/tests/unit/test_worker.py @@ -965,10 +965,12 @@ async def test_auto_submit_success(self, mock_ctx: dict[str, Any]) -> None: with patch("ontokit.services.suggestion_service.SuggestionService") as mock_cls: mock_svc = mock_cls.return_value mock_svc.auto_submit_stale_sessions = AsyncMock(return_value=3) + mock_svc.reap_stale_anonymous_sessions = AsyncMock(return_value=2) result = await auto_submit_stale_suggestions(mock_ctx) assert result["auto_submitted"] == 3 + assert result["anonymous_reaped"] == 2 @pytest.mark.asyncio async def test_auto_submit_failure_reraises(self, mock_ctx: dict[str, Any]) -> None: From 67dbbc77ac83f141a803f943ceb5c44ac127359f Mon Sep 17 00:00:00 2001 From: damienriehl Date: Thu, 20 Aug 2026 21:00:54 -0500 Subject: [PATCH 11/12] test(auth): isolate anonymous token secret --- tests/unit/test_anonymous_suggestions_routes.py | 7 +++++++ tests/unit/test_anonymous_token.py | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/tests/unit/test_anonymous_suggestions_routes.py b/tests/unit/test_anonymous_suggestions_routes.py index 8498e9cc..f74cfee2 100644 --- a/tests/unit/test_anonymous_suggestions_routes.py +++ b/tests/unit/test_anonymous_suggestions_routes.py @@ -34,6 +34,13 @@ SUBMIT_BODY = {"summary": "adds Foo"} +@pytest.fixture(autouse=True) +def _secure_test_secret(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep route tests independent of the intentionally insecure config default.""" + test_settings = type("Settings", (), {"secret_key": "test-secret-key-long-enough"})() + monkeypatch.setattr("ontokit.core.anonymous_token.settings", test_settings) + + # ── AUTH_MODE gate ──────────────────────────────────────────────────────────── diff --git a/tests/unit/test_anonymous_token.py b/tests/unit/test_anonymous_token.py index 6271a8d5..296f5c92 100644 --- a/tests/unit/test_anonymous_token.py +++ b/tests/unit/test_anonymous_token.py @@ -21,6 +21,14 @@ from ontokit.core.beacon_token import create_beacon_token, verify_beacon_token +@pytest.fixture(autouse=True) +def _secure_test_secret(monkeypatch: pytest.MonkeyPatch) -> None: + """Exercise token behavior with an explicit non-production test key.""" + test_settings = type("Settings", (), {"secret_key": "test-secret-key-long-enough"})() + monkeypatch.setattr("ontokit.core.anonymous_token.settings", test_settings) + monkeypatch.setattr("ontokit.core.beacon_token.settings", test_settings) + + def test_round_trip() -> None: token = create_anonymous_token("s_abc123") assert verify_anonymous_token(token) == "s_abc123" From de91234755fafefda6e709608021e48779e4c277 Mon Sep 17 00:00:00 2001 From: damienriehl Date: Mon, 7 Sep 2026 09:11:40 -0500 Subject: [PATCH 12/12] fix(alembic): parent the anonymous-suggestion migration on the CatholicOS head The T1 series declared down_revision v9w0x1y2z3a4, an ALEA-chain id that does not exist upstream, so `alembic heads` raised KeyError. Re-parent on 47cc27515626 (add subject_type to lint_issues), the single CatholicOS dev head. Verified on a scratch database: upgrade head applies both steps and lands the four suggestion_sessions columns; downgrade -1 reverts cleanly. Replay gates on this branch: ruff check/format, mypy, pyright clean; 1,580 tests green against real PostgreSQL and Redis; single Alembic head. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013zx7Bt1W5Jhazi3T68v9BT --- .../versions/t8u9v0w1x2y3_add_anonymous_suggestion_fields.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/alembic/versions/t8u9v0w1x2y3_add_anonymous_suggestion_fields.py b/alembic/versions/t8u9v0w1x2y3_add_anonymous_suggestion_fields.py index c9b5be3a..379a9210 100644 --- a/alembic/versions/t8u9v0w1x2y3_add_anonymous_suggestion_fields.py +++ b/alembic/versions/t8u9v0w1x2y3_add_anonymous_suggestion_fields.py @@ -1,7 +1,7 @@ """Add anonymous suggestion fields to suggestion_sessions. Revision ID: t8u9v0w1x2y3 -Revises: s7t8u9v0w1x2 +Revises: 47cc27515626 Create Date: 2026-04-03 """ @@ -10,7 +10,7 @@ # revision identifiers, used by Alembic. revision = "t8u9v0w1x2y3" -down_revision = "v9w0x1y2z3a4" +down_revision = "47cc27515626" branch_labels = None depends_on = None