Skip to content

[upstream-queue] feat: cost controls & role gating (PR-4/8)#7

Open
damienriehl wants to merge 10 commits into
devfrom
upstream-queue/cost-controls
Open

[upstream-queue] feat: cost controls & role gating (PR-4/8)#7
damienriehl wants to merge 10 commits into
devfrom
upstream-queue/cost-controls

Conversation

@damienriehl

@damienriehl damienriehl commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown

PR-4/8 — Cost controls & role gating (api)

Part of the LLM node-expansion 8-PR drain. Base dev, do not merge — staged in the alea fork for later retarget to CatholicOS. Backend for ontokit-web [upstream-queue] PR-4 (web #7).

Stack dependency: stacked on PR-3 (#6 upstream-queue/llm-config, unmerged). Its bottom 5 commits are PR-3's; the PR-4 delta is the top two — 96a57ce (feat) + f8a14d3 (harden). Merge/retarget PR-3 first (or rebase this onto the merged base); the diff-vs-dev shown here therefore also includes PR-3's changes.

What this delivers

Reintroduces the cost-control layer deferred out of PR-3 and wires the two PR-4 routes:

  • GET /projects/{id}/llm/status — member-readable advisory snapshot: configured, provider, budget_exhausted, gated monthly_spent_usd/burn_rate_daily_usd, monthly_budget_usd, daily_remaining (static per-role cap; live Redis count arrives with the PR-5 dispatch layer).
  • PATCH /projects/{id}/members/{target_user_id}/flags — owner/admin-only grant of can_self_merge_structural (ROLE-03); logs the change (metadata only).
  • services/llm/budget.py — monthly/daily spend aggregation + check_budget (daily sub-cap before monthly, Open Question 3); BYO-key calls excluded (D-17).
  • services/llm/rate_limiter.py — per-role daily call caps (editor 500 / suggester 100; COST-03/04); fails open on Redis infra error only.
  • services/llm/role_gates.py — check_llm_access + per-role capability descriptors (ROLE-01…05).
  • Activates the inert PR-3 carry-alongs: LLMStatusResponse schema + the can_self_merge_structural member column.

/ce:review outcome (kieran-python + security-sentinel) — SHIP, no BLOCKER/HIGH

The previous session was capped mid-review; this pass was re-run fresh on the full PR-4 delta and all findings resolved:

  • security-sentinel — CLEAN across authorization, IDOR/tenant isolation, information disclosure, audit, and injection. Only owner/admin/superadmin reach the member-flags PATCH (editor/suggester/viewer/non-member → 401/403); target member is project-scoped (cross-project → 404); no key material in /llm/status; privilege change is audit-logged metadata-only.
  • kieran-python — 1 MEDIUM + 3 LOW, all fixed:
    • [MEDIUM] budget-pct scale: get_budget_status.budget_consumed_pct returned a bare fraction (0.8) while the sibling get_llm_usage / LLMUsageResponse use 0–100 (*100) — a latent 100× mis-render for the first consumer. Now *100 on both paths.
    • [LOW] daily-remaining invariant: daily_remaining was gated on configured, so a no-access role (viewer) on an unconfigured project reported null (reads as "uncapped"). Now an unconditional static per-role lookup → viewer always 0. Regression test added.
    • [LOW] consolidated-query coverage: added a compiled-SQL pin (BYO exclusion, UTC date_trunc, LEAST(month_start, week_ago) outer bound, 3 FILTER aggregates) — the correctness-sensitive rewrite the mocked tests couldn't see.
    • [LOW] descriptor allocation: hoisted the per-role descriptor map to a module-level constant (was rebuilt per gate call).
  • Non-blocking follow-up: check_rate_limit fails open on Redis infra error. It's bounded (not yet wired into enforcement here; PR-5 wires it, backstopped by the non-fail-open DB budget layer) and correctly narrowed via _REDIS_INFRA_ERRORS (a mis-wired client still raises). Recommend alerting on the fail-open WARNING so a sustained Redis outage is visible.

Hardening folded into the harden commit

tz-aware UTC budget windows; get_budget_status consolidated 3 queries → 1 round-trip (member-polled endpoint); EXPIRE NX repairs TTL-less keys left by a crash between INCR and EXPIRE; explicit role is not None superadmin fallback (empty-string role can't escalate); unknown provider string → unconfigured (not 500); spend telemetry gated for no-access roles.

Verification

  • Full pytest 1619 passed; mypy strict 0; ruff 0 on all PR-4 files. PR-4 routes register in the OpenAPI schema.
  • Authed E2E deferred (member/owner-authed routes; shares the L4.4 OIDC/seed gate). FOLIO DEV confirmed serving (web 200, api/health 200) — the deployed build is dev, not this unmerged branch.
  • Note: one pre-existing ruff ARG001 remains in the PR-3 file tests/unit/test_llm_config.py (pytest-fixture false-positive, unmodified by PR-4) — belongs on PR-3 ([upstream-queue] feat: multi-provider LLM config + BYOK (PR-3/8) #6), not fixed here to keep the slice clean.

🤖 Generated with Claude Code

damienriehl and others added 10 commits July 7, 2026 08:56
@damienriehl
feat(11-01): DB models, schemas, and Alembic migration for LLM config
e81b70c 
- 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
@damienriehl
feat(11-01): LLM provider registry, crypto helpers, pricing, and SSRF…
f09b3c4 
… 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
@damienriehl @claude
feat(pr-3): LLM config/test-connection/usage routes + shared audit mo…
45e3e0f 
…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
@damienriehl @claude
harden(pr-3): refuse default secret for API-key encryption + CI lint/…
2a340bd 
…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
@damienriehl @claude
test(pr-3): public LLM catalogue endpoint tests (providers/known-models)
ecda536 
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
@damienriehl @claude
harden(pr-3): apply /ce:review findings (secret-key scope, SSRF, usag…
2b9ba9c 
…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
@damienriehl @claude
harden(pr-3): SSRF connect-time pinning + Fernet MultiFernet rotation
b8dc77e 
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
@damienriehl @claude
feat(pr-4): cost controls & role gating — budget/rate-limit/role-gate…
b404457 
… 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
@damienriehl @claude
harden(pr-4): apply /ce:review findings (budget-pct scale, daily-rema…
212c487 
…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
@damienriehl @claude
harden(pr-4): actionable, unified rate-limiter fail-open alert
3df1cb3 
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
@damienriehl
damienriehl force-pushed the upstream-queue/cost-controls branch from f8a14d3 to 3df1cb3 Compare July 11, 2026 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@damienriehl