[upstream-queue] feat: anonymous suggestions (PR-7/8, 7a) — drain complete#10
Open
damienriehl wants to merge 31 commits into
Open
[upstream-queue] feat: anonymous suggestions (PR-7/8, 7a) — drain complete#10damienriehl wants to merge 31 commits into
damienriehl wants to merge 31 commits into
Conversation
- Add ProjectLLMConfig model with provider, encrypted key, model_tier, base_url, budget fields - Add LLMAuditLog model with token counts, cost_estimate_usd, is_byo_key flag - Add can_self_merge_structural boolean to ProjectMember model - Add LLMProviderType enum (13 providers) and LLM Pydantic schemas in schemas/llm.py - Add Alembic migration u9v0w1x2y3a4 — applies cleanly against live DB - Export new models from ontokit/models/__init__.py
… validator - Port 13-provider registry from folio-enrich with get_provider() factory - OpenAICompatProvider handles 9 providers via OpenAI SDK base_url param - AnthropicProvider, GoogleProvider (httpx REST), CohereProvider, GitHubModelsProvider - All chat() methods return (text, input_tokens, output_tokens) for audit logging - crypto.py: Fernet encrypt_secret/decrypt_secret using same pattern as embedding_service.py - pricing.py: LiteLLM pricing fetch with 7-day module-level cache, graceful stale fallback - ssrf.py: validate_base_url() blocks private IPs, metadata endpoint, non-https for cloud - Add openai, anthropic, google-generativeai, cohere to pyproject.toml dependencies
…dule Carved from phase-11 commit 9d1d7ca (mixed PR-3/PR-4). Includes the PR-3 routes only — config GET/PUT, test-connection, usage, and public providers/known-models — and registers them. The PR-4 routes (GET /llm/status and the can_self_merge_structural member-flags PATCH) are dropped here and ship with cost-controls (PR-4). audit.py (from PR-4 commit 119e501) is pulled in as the shared backing for GET /llm/usage; budget/rate_limiter/role_gates stay in PR-4. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
…type fixes - crypto.py: _get_fernet() now hard-fails in production (and warns in dev) when SECRET_KEY is the shipped 'change-me-in-production' default. Because this key encrypts user/BYO provider API keys, a publicly-known secret would make them trivially decryptable — equivalent to plaintext. (Addresses the session-4 hardcoded-secret class of finding for the BYOK lineage.) - registry.py: fix latent UnboundLocalError — the unknown-provider handler interpolated an unbound 'provider_type'; use 'name'. Also raise ... from None. - ruff/mypy: StrEnum for LLMProviderType (UP042); dict[str, ...] type args; provider _client: Any annotations; ssrf addr str(); B904 raise-from. Net-new files must pass CI's authoritative mypy + ruff. - tests: real secrets tests (crypto round-trip, response never exposes key, write-only api_key, production default-secret block, audit metadata-only) replacing the phase-11 skipped stubs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
Exercises the two no-auth PR-3 routes through the real FastAPI app (route registration + response_model serialization), asserts no secret material in the public catalogue, and confirms project-scoped /llm/config is auth-gated. This is the PR-3 surface verifiable without an authenticated, seeded project. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
…e TZ) Kieran-python review (no BLOCKER): - H1: secret-key guard now hard-fails in ANY deployed env (staging + production), not just production. Staging is shared/reachable and may hold real tenant keys; a log warning there is not a control. Uses "not settings.is_development". - H2: SSRF was write-time only (TOCTOU / DNS-rebinding). Re-validate base_url in test-connection immediately before the outbound call (the only PR-3 outbound path); harden metadata detection to catch IPv4-mapped IPv6 (::ffff:169.254...) and AWS IPv6 IMDS (fd00:ec2::254), and unwrap mapped IPv6 in private-IP checks. Full connect-time IP pinning + redirect disabling + the PR-5 chat path are noted as follow-ups. - M1: usage aggregation compared a timestamptz column against a naive UTC wall-clock (NOW AT TIME ZONE UTC), shifting month/day boundaries under a non-UTC DB session. Now uses tz-aware date_trunc(field, now, 'UTC') and now - interval, so totals + budget% are correct regardless of session TimeZone. - L1: a base_url-only update on an existing local (Ollama) config is no longer rejected -- effective provider falls back to the stored row. Tests: staging block (parametrized), SSRF metadata/private-IP detection incl. IPv6 forms. ruff + mypy clean; full suite 1547 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
Two queued follow-ups from the PR-3 /ce:review: SSRF connect-time re-validation (kills the resolve-then-connect TOCTOU): - ssrf.resolve_and_validate() resolves + validates every address at call time - SSRFProtectedTransport re-validates the target host on every request, immediately before the socket opens, so a base_url that passed validation at config-save can't later DNS-rebind to a private/metadata IP - secure_async_client() wires that transport and disables redirect following (a 3xx to a private IP can't bypass the guard) - wired into every provider that dials a project-controlled base_url: cohere/google/github_models (raw httpx) + openai_compat (openai SDK http_client); local providers pass allow_private=True (metadata still blocked) Fernet key rotation via MultiFernet: - crypto._get_fernet() now builds a MultiFernet: current SECRET_KEY encrypts, retired keys in SECRET_KEY_PREVIOUS decrypt only -> zero-downtime rotation - rotate_secret() re-encrypts stored ciphertext onto the current key - the shipped insecure default is never trusted as a rotation key - new config.secret_key_previous (comma-separated) +13 tests (SSRF resolver/rebinding/redirects; rotation round-trip/drop/migrate). Full suite 1575 green; ruff + mypy-strict clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018eQ2CoAwpGv9Q8vBhuGGtH
… services + status/member-flags routes
Reintroduces the PR-4 pieces deferred from PR-3:
- services/llm/{budget,rate_limiter,role_gates}.py carved from the phase-11
lineage (119e501); __init__.py re-exports restored
- routes/llm.py: GET /projects/{id}/llm/status (member-readable, advisory —
dispatch re-checks in PR-5) + PATCH /projects/{id}/members/{uid}/flags
(owner/admin-only ROLE-03 toggle), hand-inserted from 9d1d7ca so PR-3's
hardening (TOCTOU SSRF re-validation, effective_provider fallback) stays
- MemberFlagsUpdate/Response schemas moved to schemas/llm.py (lineage had
them inline in the routes file with an aliased BaseModel import)
- Activates the inert PR-3 carry-alongs: LLMStatusResponse schema and the
can_self_merge_structural column/migration
Deviations from lineage (hardening folded in):
- budget.py time windows now tz-aware func.date_trunc(..., 'UTC') — mirrors
the PR-3 review fix already applied to audit.py (naive-UTC text() SQL
mis-windows spend against timestamptz)
- rate_limiter.py: UTC day key (was server-local date.today()) so the rate
window aligns with budget/audit windows; EXPIRE now NX+unconditional,
repairing keys left TTL-less by a crash between INCR and EXPIRE (which
permanently rate-limited the user once over cap)
- /llm/status: no-access roles report daily_remaining=0 (lineage reported
null, which the schema documents as 'uncapped')
- member-flags PATCH logs privilege changes (actor/target/value, metadata
only) — it grants structural self-merge and previously left no trace
- mypy strict: BudgetStatus TypedDict + typed role descriptors; dead lineage
imports/_MemberFlagsUpdate class dropped
Net-new tests (lineage had only skip-stubs): rate-limit matrix incl. NX TTL
+ documented fail-open pin; budget matrix incl. BYO-exclusion asserted
against compiled SQL + daily-before-monthly ordering; role-gate matrix incl.
anonymous override + RATE_LIMITS consistency; status/member-flags route
tests (auth boundaries 401/403/404/422, viewer=0, owner=null, exhaustion,
local-provider, no-key-leak). 55 tests; full suite 1617.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
…ining invariant, query consolidation + tests) Completes the /ce:review pass on the cost-control layer (was session-capped mid-review). Two reviewers (kieran-python + security-sentinel) — SHIP, no BLOCKER/HIGH. Service/route hardening (budget/rate_limiter/llm routes): - budget.py: get_budget_status consolidated 3 queries -> 1 round-trip (FILTER'd SUMs over month/day/week, LEAST(month_start, week_ago) outer bound) since it backs the member-polled /llm/status; project_id typed str -> uuid.UUID; tz-aware date_trunc(..., 'UTC') windows. - budget.py: budget_consumed_pct now on the 0-100 scale to match LLMUsageResponse / get_llm_usage (both *100) — the bare-fraction was a latent 100x mis-render for the first consumer of the snapshot. [MEDIUM] - rate_limiter.py: typed RateLimitRedis Protocol (drops the object + type:ignore); _REDIS_INFRA_ERRORS narrows the fail-open catch so a mis-wired client raises instead of silently disabling metering; EXPIRE NX repairs TTL-less keys. - llm.py: explicit `role is not None` superadmin fallback (empty-string role cannot escalate to admin); unknown provider -> unconfigured, not 500; spend telemetry gated (no-access roles see caps/booleans, not spend/burn); daily_remaining is now an unconditional static per-role lookup so a no-access role reports 0 even on an unconfigured project (the null-reads-as-uncapped invariant must not hinge on config). [LOW] - role_gates.py: hoisted the per-role descriptor map to a module-level constant (was rebuilt on every gate call). [LOW] - models/__init__.py: import ordering (isort). Tests reconciled + strengthened (no orphaned red): - Updated the budget-status and status-route test doubles to the single consolidated-query shape (result.one().monthly/.daily/.week). - Added a compiled-SQL pin for get_budget_status (BYO exclusion, UTC date_trunc, LEAST outer bound, 3 FILTER aggregates). [LOW] - Added a viewer-on-unconfigured-project regression (daily_remaining == 0). - Full suite 1619 passed; mypy strict 0; ruff 0 on all PR-4 files. Non-blocking follow-up noted on the PR: alert on the rate-limiter fail-open WARNING so a sustained Redis outage is visible (the fail-open is bounded — not yet wired into enforcement here; PR-5 wires it, backstopped by the non-fail-open DB budget layer). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
Both limiter fail-open paths (check_rate_limit, get_remaining_calls) now emit ONE stable, greppable signal — FAIL_OPEN_EVENT=llm_rate_limiter_fail_open — with structured extra fields (event/operation/project_id/user_id) and the triggering error, so ops can alert on a single marker instead of two ad-hoc log lines. Parity verified: budget layer fails CLOSED (non-fail-open backstop); route-level fail-open alerting lands with PR-5. +2 tests assert the marker fires on both paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018eQ2CoAwpGv9Q8vBhuGGtH
…ss-branch search) Minimal phase-12 chain needed by the suggestion generation pipeline: - DuplicateRejection model + duplicate_check schemas (from 8226d9e) - StructuralSimilarityService — folio-python Jaccard, fail-soft to 0.0 (from 9a97712) - EmbeddingService.semantic_search_all_branches + SemanticSearchResultWithBranch schema (from e4f8a71), ported to the CAST(:param AS vector) bindparam style this branch already uses (::vector after a bindparam is silently dropped by SQLAlchemy text()) - DuplicateCheckService with 40/40/20 exact/semantic/structural scoring (from a884e27); reviewer-flow hunks (suggestion.py reject-linking fields, suggestion_service.py) dropped — they belong to the PR-6 review flow - HNSW index + duplicate_rejections migration (from 11d1d3a); also repairs the orphaned u9v0w1x2y3a4 down_revision (t8u9v0w1x2y3 was never extracted) to chain onto 47cc27515626 so alembic history is a single linear head again Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
…nts/annotations/edges) Extracted from the phase-13 lineage (029d309, 220c88c, 4b54f96, c6d3cb7, 3399045, 481284a + final-state tests from 3b1874a/1fa317e): - schemas/generation.py: request/response contract, Provenance literal, GeneratedSuggestion with confidence + provenance + embedded validation and duplicate results - services/validation_service.py: VALID-01..06 rules, mint_iri, detect_project_namespace - services/context_assembler.py: ~2-4K-token ontology context assembly (quality_filter.py from the same lineage commit dropped — PR-6 reviewer concern, nothing in the generate path imports it) - services/llm/prompts/: 5 prompt templates + typed PROMPT_BUILDERS dispatch - services/suggestion_generation_service.py: context -> LLM -> parse -> validate -> dedup pipeline; confidence normalization (clamp 0-1, /100 for 0-100 scales); dedup failures fail-soft to verdict=pass - api/routes/generation.py: POST generate-suggestions + validate-entity; generate enforces role gate (403), rate limit (429, Redis fail-open), budget (402) BEFORE any LLM call — closes the PR-4 follow-up where the rate limiter shipped without enforcement - tests: generation schemas, entity validation, suggestion generation (GEN-01..09), context assembler; quality_filter-only tests trimmed Typing tightened for this branch's strict mypy gate (dict type args, typed PROMPT_BUILDERS, cast on JSON parse, isinstance guard on confidence). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
Per-suggestion provenance now carries model + prompt identity, not just the audit log (D-08: metadata only — raw prompt text is never persisted): - GeneratedSuggestion gains model: str | None and prompt_template: str | None - SuggestionGenerationService.generate() accepts model_id and stamps model=model_id, prompt_template=<PROMPT_BUILDERS key> on every suggestion - generate-suggestions route threads config.model into the pipeline - tests pin: provenance='llm-proposed', confidence in [0,1], model id, and prompt_template key on every generated suggestion; model stays None when no model_id is threaded (user-written paths) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
…22/502 + success shape) Route-level coverage for the enforcement boundary, which the extracted lineage left thin: - 401/403 unauthenticated + non-member + viewer role-gate - 429 rate-limit exceeded — pins that check_rate_limit fires BEFORE any provider construction or budget check (the PR-4 'shipped but not wired' follow-up), plus the documented Redis fail-open path - 402 budget exhausted / daily cap, provider never constructed - 400 missing LLM config, 422 batch_size bounds, 404 unknown project and unknown class_iri, 502 provider auth error - success shape pins provenance/model/prompt_template on the wire and that config.model is threaded into the pipeline as model_id Also fixes the endpoints' user dependency: the lineage's 'Annotated[RequiredUser, Depends()]' double-wrap breaks on this branch's FastAPI (CurrentUser got re-interpreted as query params -> every request 422'd); switched to the bare 'user: RequiredUser' style the rest of this repo's routes use. Also isort-fixes the u9v0w1x2y3a4 migration touched by the chain repair. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
The generation route wires rate-limit enforcement, but its route-level fail-open path (Redis pool absent -> _get_redis returns None -> rate check skipped) was silent, so ops could not alert on unmetered LLM traffic. Now both fail-open layers log at WARNING: _get_redis narrows to Import/AttributeError and warns on an absent pool, and the route logs the bypass with user/project context when it skips the check. Behaviour is unchanged (still fail-open; DB budget cap in step 5 remains the non-fail-open backstop) -- only observability is added. Extends the redis-unavailable route test to assert the WARNING fires. Closes the PR-4 rate-limiter fail-open review follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
…provenance/robustness
BLOCKER: edges and annotations returned empty, validation-failing stubs and
dropped all type-specific payload — the pipeline parsed a single generic
raw["label"] that the edge/annotation prompts never emit. Replace with a
per-type dispatch (_parse_typed):
- edges: parse target_label/target_iri/relationship_type; flag non-controlled
relationship_type and null target with GEN-05 validation errors instead of
silently dropping; label = target_label.
- annotations: parse property_iri/value/lang; carry the focus class IRI;
display label falls back to value.
- parents (HIGH): link an existing parent by its IRI instead of minting a
duplicate node; correct IS-A direction (web links class_iri -> parent).
- siblings: parent to the focus class's shared parents, not to the class
itself; children unchanged.
Move the type-specific fields (property_iri/value/lang/target_iri/
relationship_type) onto the flat base GeneratedSuggestion so list[...] response
serialization carries them and the shape matches the web contract.
HIGH: refuse generation with 400 when config.model is empty (every suggestion
must carry a resolvable model id for provenance — D-08).
HIGH: rewrite the edge/annotation/parent tests with real prompt-output shapes
and assert the payload round-trips (the old fixtures injected a "label" key,
masking the blocker).
MEDIUM: unexpected pipeline exceptions now surface as 500 instead of being
masked as an empty-200; only transient TimeoutError/ConnectionError fail soft.
MEDIUM: NaN/inf confidence -> None (clamping had promoted NaN to 1.0).
LOW: 502 returns a generic client message (provider detail logged server-side
only, not echoed); dedup-unavailable logs an alertable distinct WARNING;
provider param is honestly typed (Optional + runtime guard, no type: ignore).
Full pytest 1688 passed; ruff + mypy(strict) clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
The generation route's Redis-pool-absent bypass now emits the same FAIL_OPEN_EVENT=llm_rate_limiter_fail_open signal (with structured extra) as the limiter-internal fail-open paths, so all three degraded paths alert as one event. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018eQ2CoAwpGv9Q8vBhuGGtH
Duplicate detection always searches across ALL branches (DEDUP-08), so the request's branch field was silently ignored. Remove it (nothing sends it — the duplicate-check route lands in PR-6 with no branch consumer) and document why no per-request branch scope exists. The candidate's found-on branch is still reported via DuplicateCandidate.branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018eQ2CoAwpGv9Q8vBhuGGtH
- New route: POST /projects/{project_id}/duplicate-check (DuplicateCheckResponse)
- Router registered in __init__.py alongside validation, llm, and other routes
- Replace 7 Wave 0 skip stubs with real async unit tests covering DEDUP-04/05/06/07/08 + D-01/D-09/D-13
- Tests mock semantic_search_all_branches + compute_similarity to control scores
- All 7 tests pass; full unit suite: 162 passed, 14 skipped
…ntity_type - Route had NO auth dependency in the lineage — any caller could run duplicate checks (label + embedding similarity reads) against private projects. Now mirrors /search/semantic: OptionalUser + project_service.get (public readable by anyone incl. anonymous; private requires membership; 403/404 before any scoring work). - Re-added entity_type to DuplicateCheckService.check() — PR-5's carve dropped it while DuplicateCheckRequest still advertises the field and the route forwards it (would have raised TypeError at runtime). Accepted-but-unused, documented as type-agnostic scoring. - New tests/unit/test_duplicate_check_route.py (5 endpoint tests: private-denied-before-check, 404, anonymous-public-read, request wiring incl. entity_type, 422) + ruff fixes in the lineage test file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
…w MEDIUM visibility) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q
Update the branch-ignored contract pin to assert the field is gone from the request model (removed on PR-5) and that a stray branch key in the body is ignored and never forwarded to the all-branches search. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018eQ2CoAwpGv9Q8vBhuGGtH
…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
- 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
damienriehl
force-pushed
the
upstream-queue/anonymous-suggestions
branch
from
July 11, 2026 02:56
5385e22 to
f9d78c4
Compare
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
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.
PR-7 (7a / api) — anonymous suggestions
[upstream-queue]— baseupstream-queue/reviewer-tools(PR-6), UNMERGED. Part of the alea-fork upstream queue; do not merge (dev stays a pure upstream mirror). Pairs with web PR-7 (7b): alea-institute/ontokit-web#10. Completes the 8-PR drain (PR-0…PR-7).What
Credit-based suggesting for logged-out users when
AUTH_MODE != required(phase 10 of the llm-helper lineage):POST /projects/{id}/suggestions/anonymous/sessions— create session (no auth; 5 sessions/IP/hour rate limit; real git branch) → returns a session-scoped HMAC anonymous token (24h TTL,anon:-prefixed to prevent confusion with beacon tokens).PUT .../sessions/{sid}/save,POST .../sessions/{sid}/submit,POST .../sessions/{sid}/discard— authenticated viaX-Anonymous-Token; the service binds token↔session (403 mismatch) and requiresis_anonymous.POST .../anonymous/beacon— sendBeacon flush via token query param.websitefield (schema alias) triggers silent fake success without touching the service — bot filter.t8u9v0w1x2y3(4 nullable columns onsuggestion_sessions), down_revision repointed onto PR-5's headv9w0x1y2z3a4(the lineage pointed at a dev revision that already has a different child — would have forked Alembic heads). Single head verified.Stack note
The bottom 3 commits (
69aaff0,a943409,5ffc5d1) are PR-2's AUTH_MODE stack (alea api#5, already reviewed there) —auth_modedoesn't exist on the PR-1..6 chain, and these endpoints gate on it. Merge PR-2 first (or rebase this once it lands); the PR-7 delta is the 5 commits above them.Hardening applied during extraction (beyond the lineage)
data.session_idwherebeacon_saveexpects a beacon token —verify_beacon_token(session_id)always returnedNone, so the endpoint 401'd on every request (fails closed; fully dead). Newbeacon_save_anonymousbinds the route-verified anonymous token to the payload session (403 mismatch), re-checksis_anonymous(an anonymous token must never flush an authenticated user's session), and shares the branch-locked flush (_beacon_flush) with the authenticated path.create_anonymous_sessionnow requiresproject.is_public— the lineage let anonymous users create suggestion branches/PRs against private projects (IP rate limiting was the only guard).test_suggestion_service.pyfactory reconciled to the new model columns (5 orphaned-red tests fixed).Verification
alembic headssingle head; all 5 routes register in the OpenAPI schema at runtime; OpenAPI scan pins no honeypot disclosure./ce:review outcome
(security-sentinel, resource-abuse-focused) — FIX-FIRST → all HIGH/actionable-MEDIUM FIXED (
4f14a51); token security + session binding PASSreap_stale_anonymous_sessions(atomic claim; discard + force branch delete at 24h = token TTL; includes zero-change sessions), wired into the worker cron; the authed sweep now excludes anonymous sessions by design.anonymous_tokenunit tests → newtest_anonymous_token.pyincl. BOTH directions of anonymous↔beacon cross-token rejection (reviewer hand-verified theanon:prefix separation is structurally airtight; now regression-pinned).is_anonymous, 403 mismatch), the beacon fix (no regression to the authed beacon path), migration safety (server_default backfill, single head), privacy (client_ip never serialized; reviewer-only submitter credit).is_publicre-check on session lifecycle (visibility flip mid-session); PR title/body sanitization for anonymous-controlled strings;contentmax_length;request.client.hostproxy-topology note (no XFF trust — correct, but document--proxy-headersfor LB deploys); rate-limit TOCTOU (approximate under concurrency).🤖 Generated with Claude Code
https://claude.ai/code/session_01ELAutiUDrKdGf2vZAwgt9Q