SQL queries for session_pins, DebitOrgCredits key_spend, and GetUserMonthlySpendAndLimit lack tenant-ownership predicates

[rohith500]

Verification

Independently verified twice before filing: once by an independent Cursor investigation pass, and once by manual live SQL reproduction on a local Postgres instance (all three findings, with real observed query output; test data fully cleaned up afterward).

Summary

Three tables (session_pins, model_router_api_keys via DebitOrgCredits's
key_spend CTE, model_router_user_monthly_spend via
GetUserMonthlySpendAndLimit) have SQL that doesn't enforce tenant
ownership at the query level — the installation_id/organization_id
scoping that keeps one tenant's data isolated from another's is currently
guaranteed only by the application layer always passing trusted,
same-tenant IDs, not by the SQL itself. Same pattern precedent as #536
(closed, fixed for SoftDeleteModelRouterAPIKey), found here on three
additional call sites.

Not currently exploitable via any authenticated /v1/* request —
investigated exhaustively, see below. Filing as a hardening/defense-in-depth
issue, not an active vulnerability report.

Finding 1: session_pins — installation_id stored but never enforced

db/queries/session_pins.sql: GetSessionPin, UpsertSessionPin, and the
Update*/Increment*/Reset* variants all scope exclusively by
session_key + role. UpsertSessionPin's ON CONFLICT clause updates
pinned_model/pinned_provider/etc. but does not update
installation_id — the column's own comment says this is deliberate
("re-binding a session to a different installation would indicate a bug,
not a legitimate state"), but nothing anywhere actually checks that
assumption holds. internal/proxy/turnloop.go's loadPin never compares
pin.InstallationID against the requesting installation.

Live SQL reproduction, run twice independently (once during investigation,
once manually on a fresh local Postgres, both cleaned up afterward):

Step	installation_id	pinned_model	turn_count
after install A's insert	A	model-from-A	1
after install B upserts same (session_key, role)	still A	model-from-B	2

If two installations ever produced the same session_key, install B's
request would silently overwrite install A's pinned model/provider, while
the row's installation_id stays stuck on A — a false signal to anything
downstream that trusts that column.

Finding 2: DebitOrgCredits's key_spend CTE

UPDATE router.model_router_api_keys
SET spent_usd_micros = spent_usd_micros - @delta_usd_micros::bigint
WHERE id = sqlc.narg('api_key_id')::uuid
  AND EXISTS (SELECT 1 FROM updated)

No join confirming the key belongs to the organization_id being debited.
Live (verified manually): debiting org A $0.50 with org B's api_key_id
correctly drops org A's balance from $1000.00 to $999.50, but also bumps
org B's key's spent_usd_micros from 0 to $0.50 — could trip org B's
per-key spend cap from an unrelated org's activity, if the IDs were ever
mismatched.

Finding 3: GetUserMonthlySpendAndLimit

db/queries/spend_limits.sql: looks up user spend/override by
router_user_id alone; organization_id is only used to resolve the org's
default limit, not to scope the user lookup itself. Live (verified
manually): querying with org A's ID and org B's user UUID returns org B's
real spend ($0.424242) paired with org A's default limit ($1.111111) — a
genuine cross-tenant spend read if a foreign user ID were ever injectable.

Why none of these are reachable today

Investigated exhaustively across every entry point, not just the normal
request flow:

• /v1/messages, /v1/chat/completions, responses, Gemini: WithAuth
  sets APIKeyID from context whenever VerifyAPIKey succeeds (cache hit
  or DB lookup), and a successful verification never returns a nil key —
  so apiKeyID is always a real UUID on any path that touches pins or
  billing debits.
• Agent-shadow evaluation (feat: add isolated agent shadow routing #787): still runs through WithAuth normally;
  its special routing path (runAgentShadowEvaluationRoute) deliberately
  never sets SessionKey and skips pin I/O entirely, so it can't produce a
  pin collision.
• Hard-pinned turn types (Probe/TitleGen/Classifier/Compaction): also leave
  SessionKey zero and skip pin writes.
• Admin-cookie paths and /v1/route don't touch pins or the debit path at
  all.
• No path found anywhere that supplies a foreign or attacker-influenced
  api_key_id/router_user_id to the billing queries — both always come
  from the authenticated request's own context.
• A same-session_key collision between two different real API keys would
  require a SHA-256 (truncated to 128 bits) collision — not a realistic
  attack surface.

Duplicate check

#536 (closed) fixed the same class of gap for
SoftDeleteModelRouterAPIKey specifically — doesn't cover any of these
three. No open issue tracks this pattern.

Suggested fix

For each: add the missing tenant-scoping predicate to the WHERE clause
(or an explicit ownership check after the read, matching whichever pattern
this codebase already uses elsewhere for similar cases) so the SQL itself
enforces the invariant the application currently only assumes. For
session_pins specifically, loadPin should also reject (not silently
serve) a pin whose installation_id doesn't match the requesting
installation, rather than relying solely on prevention at write time.

Severity

Defense-in-depth / hardening, not an active cross-tenant breach — no
demonstrated live exploit path under current auth wiring. Worth fixing so
these columns/checks aren't a false signal, and so the invariant doesn't
become exploitable if some future change (a new entry point, a debug
header, a refactor) ever loosens how these IDs get populated.