feat: support optional auth and guarded anonymous suggestions - #228
Open
damienriehl wants to merge 12 commits into
Open
damienriehl wants to merge 12 commits into
damienriehl wants to merge 12 commits into
Conversation
…odes - 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)
…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)
/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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018eQ2CoAwpGv9Q8vBhuGGtH
- 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
…e 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
- 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)
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) <noreply@anthropic.com>
…int tests - 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
…mous branch reaper, honeypot de-disclosure - 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
…icOS 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zx7Bt1W5Jhazi3T68v9BT
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Sep 7, 2026
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Issue links
Closes #83.
Supersedes #27.
Source and scope
de91234755fafefda6e709608021e48779e4c277(upstream-queue/t1-api-replay-20260907, also on alea-institute/ontokit-api)d2c31aea(upstream-queue/t1-api-synthesis, based on CatholicOS APIdevata21b7d5c)devatff8b300f(2026-09-06); rebase applied all 11 commits with no conflictst8u9v0w1x2y3now parents on this chain's head47cc27515626(its previous parent was an ALEA-chain id that does not exist here)Verification (2026-09-07, on this branch, fresh
uv sync --group devagainst this base's lockfile)uv run pytest tests/with real PostgreSQL 17 (pgvector) and Redis: 1,580 passeduv run ruff check ontokit/,uv run ruff format --check ontokit/: cleanuv run mypy ontokit/: no issues in 110 source filesuv run pyright ontokit/: 0 errorst8u9v0w1x2y3; on a scratch databaseupgrade headapplied94afeba9ab5c -> 47cc27515626 -> t8u9v0w1x2y3and landed the foursuggestion_sessionscolumns, anddowngrade -1reverted cleanlyNo self-merge is requested.
🤖 Generated with Claude Code
https://claude.ai/code/session_013zx7Bt1W5Jhazi3T68v9BT