fix(db): enforce tenant ownership in session_pins and billing spend queries (#796) by devin-ai-integration[bot] · Pull Request #806 · workweave/router

devin-ai-integration · 2026-07-21T13:10:06Z

Summary

Convention-compliant rewrite of #800 (by @rohith500). The production fix is preserved verbatim; only the test surface moved to satisfy the AGENTS.md hard rule.

Closes the four SQL ownership gaps from #796 where tenant isolation relied on the app layer always passing same-tenant IDs rather than on the SQL itself (same class as the already-fixed SoftDeleteModelRouterAPIKey in #536):

session_pins: added AND installation_id = @installation_id::uuid to GetSessionPin, UpdateSessionPinUsage, IncrementSessionPinUpstreamErrors, ResetSessionPinUpstreamErrors; UpsertSessionPin's ON CONFLICT DO UPDATE gained WHERE router.session_pins.installation_id = EXCLUDED.installation_id so a mismatched caller no-ops instead of overwriting another tenant's pin. installationID threaded through the sessionpin.Store interface and every call site. SweepExpiredSessionPins left untouched (global GC by expires_at).
DebitOrgCredits key_spend / user_month_spend CTEs: both now join api_key_id / router_user_id through to model_router_installations.external_id and require it to match the organization_id being debited before bumping the counter; a mismatch no-ops just that CTE (org debit + ledger still proceed).
GetUserMonthlySpendAndLimit: has_override / override_limit_usd_micros / spent_usd_micros subqueries now join through model_router_users to model_router_installations.external_id and require a match; org_default_limit_usd_micros stays unjoined (org-scoped only).

All four: mismatch = silent miss/no-op, matching how each query already handles a missing row. Hardening fix, not a response to an active exploit (confirmed in #796 that none are reachable via a current authenticated /v1/* request).

Why a rewrite

#800 added internal/postgres/tenant_isolation_796_test.go, a live-Postgres DB-backed test in internal/. Per root AGENTS.md ("Tests"): "No DB-backed integration tests in internal/. If need real Postgres, docker compose stack is runtime fixture; write scripts under scripts/ rather than *_test.go." The four regression scenarios were ported verbatim (real SessionPinRepo / BillingRepo, no mocks) into scripts/tenant_isolation_check/, gated on ROUTER_TEST_DATABASE_URL, following the existing scripts/feedback_integration_check/ pattern. go test ./... no longer touches Postgres.

Testing

make precommit (fmt + vet + build + test) clean.
make generate produces no diff under internal/sqlc.
go build ./scripts/tenant_isolation_check clean.

Co-authored-by: rohith500 rohith500@users.noreply.github.com

Link to Devin session: https://app.devin.ai/sessions/055a0e376d2b4318b42c4991fb14625f
Requested by: @steventohme

devin-ai-integration · 2026-07-21T13:10:09Z

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

Disable automatic comment, CI, and merge conflict monitoring

greptile-apps · 2026-07-21T13:10:10Z

PR author is not in the allowed authors list.

…r_month_spend, and GetUserMonthlySpendAndLimit (#796) Signed-off-by: N Rohith Reddy <rohithreddy2202@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>

claude · 2026-07-31T05:01:24Z

Claude finished @devin-ai-integration[bot]'s task —— View job

Get diff and analyze comment blocks
Flag verbose comment blocks
Post review

Posted 3 comment-length suggestions on the PR (advisory, won't block merge):

Lines 26–30 (package-level comment): 5 lines → 2. The "Uses real Postgres … no mocks" tail restates what the code obviously shows.
Lines 206–209 (function doc): 4 lines → 2. "Write-side twin" phrasing + repetition of the cross-tenant clause collapses cleanly.
Lines 272–275 (function doc): 4 lines → 3. Collapsed without losing the "silent miss" invariant note.

cursor

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 40998c2. Configure here.}

cursor · 2026-07-31T05:02:39Z

+	require.NotNil(t, limit)
+	require.Equal(t, orgADefault, *limit,
+		"org default limit for A must still resolve on a mismatched user read")
+}


DB tests violate AGENTS.md rule

Medium Severity

This adds a live-Postgres integration suite under internal/postgres/, gated on ROUTER_TEST_DATABASE_URL with a DATABASE_URL fallback. That violates the AGENTS.md Tests hard rule against DB-backed *_test.go in internal/ (use scripts/ instead, as feedback_integration_check does). go test ./... can then touch and mutate a real database when DATABASE_URL is set.

^{Reviewed by Cursor Bugbot for commit 40998c2. Configure here.}

workweave-bot

Advisory only — comment-length nits. Won't block merge.

workweave-bot · 2026-07-31T05:02:56Z

+func testDatabaseURL(t *testing.T) string {
+	t.Helper()
+	dsn := os.Getenv("ROUTER_TEST_DATABASE_URL")
+	if dsn == "" {
+		dsn = os.Getenv("DATABASE_URL")


Suggested change

func testDatabaseURL(t *testing.T) string {

t.Helper()

dsn := os.Getenv("ROUTER_TEST_DATABASE_URL")

if dsn == "" {

dsn = os.Getenv("DATABASE_URL")

// #796 regressions: SQL must enforce tenant ownership rather than relying

// solely on the app layer passing same-tenant IDs. Gated on ROUTER_TEST_DATABASE_URL.

Was 5 lines; the "Uses real Postgres … no mocks" half restates what the code already shows.

workweave-bot · 2026-07-31T05:02:56Z

+	ctx := context.Background()
+
+	suffix := time.Now().UnixNano()
+	orgA := fmt.Sprintf("796-usr-a-%d", suffix%1_000_000_000_000)


Suggested change

ctx := context.Background()

suffix := time.Now().UnixNano()

orgA := fmt.Sprintf("796-usr-a-%d", suffix%1_000_000_000_000)

// TestDebitOrgCredits_UserMonthSpendRequiresOrgOwnership reproduces #796 finding 3

// (write side): user_month_spend must not bump a foreign user's counter across tenants.

Was 4 lines; collapsed without losing meaning.

workweave-bot · 2026-07-31T05:02:56Z

+	ctx := context.Background()
+
+	suffix := time.Now().UnixNano()
+	orgA := fmt.Sprintf("796-rd-a-%d", suffix%1_000_000_000_000)


Suggested change

ctx := context.Background()

suffix := time.Now().UnixNano()

orgA := fmt.Sprintf("796-rd-a-%d", suffix%1_000_000_000_000)

// TestGetUserMonthlySpendAndLimit_RequiresUserOrgOwnership reproduces #796 finding 3

// (read side): spend/limit subqueries must not return a foreign user's figures; mismatch

// = silent miss, org default still resolves.

Was 4 lines; collapsed without losing the "silent miss" invariant note.

devin-ai-integration Bot mentioned this pull request Jul 21, 2026

fix(db): enforce tenant ownership in session_pins and billing spend queries (#796) #800

Open

test: reproduce tenant-isolation gaps in session_pins, key_spend, use…

40998c2

…r_month_spend, and GetUserMonthlySpendAndLimit (#796) Signed-off-by: N Rohith Reddy <rohithreddy2202@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>

steventohme force-pushed the devin/tenant-sql-ownership branch from 9f97369 to 40998c2 Compare July 31, 2026 05:01

cursor Bot reviewed Jul 31, 2026

View reviewed changes

workweave-bot reviewed Jul 31, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix(db): enforce tenant ownership in session_pins and billing spend queries (#796) - #806

fix(db): enforce tenant ownership in session_pins and billing spend queries (#796)#806
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/tenant-sql-ownership

devin-ai-integration Bot commented Jul 21, 2026

Uh oh!

devin-ai-integration Bot commented Jul 21, 2026

Uh oh!

greptile-apps Bot commented Jul 21, 2026

Uh oh!

claude Bot commented Jul 31, 2026 •

edited

Loading

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Jul 31, 2026

Uh oh!

workweave-bot left a comment

Uh oh!

workweave-bot Jul 31, 2026

Uh oh!

workweave-bot Jul 31, 2026

Uh oh!

workweave-bot Jul 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

devin-ai-integration Bot commented Jul 21, 2026

Summary

Why a rewrite

Testing

Uh oh!

devin-ai-integration Bot commented Jul 21, 2026

🤖 Devin AI Engineer

Uh oh!

greptile-apps Bot commented Jul 21, 2026

Uh oh!

claude Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 31, 2026

Choose a reason for hiding this comment

DB tests violate AGENTS.md rule

Uh oh!

workweave-bot left a comment

Choose a reason for hiding this comment

Uh oh!

workweave-bot Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

workweave-bot Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

workweave-bot Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

claude Bot commented Jul 31, 2026 •

edited

Loading