# DB tenancy hardening — lightweight defense-in-depth

- **Date:** 2026-06-22
- **Branch:** `fix/db-rls-hardening`
- **Status:** design approved (lightweight option), spec pending user review
- **Author:** backend agent
- **Related:** follow-up to `fix/db-audit-quickwins` (audit findings B1–B5). This spec
  delivers the deferred B5 + the defense-in-depth "lint/test guard" recommendation.

## Context

Tenant isolation in SEO Insight is enforced **entirely in application code**. The DB
connects as a single Postgres role (`src/lib/db/client.ts`) with no RLS. The canonical
gate is `assertSiteInActiveOrg(siteId)` (and its siblings `assertKeywordInActiveOrg`,
`assertContentItemInActiveOrg` in `src/lib/sites/assert-site.ts`), which verify BOTH
org-ownership AND the staff `site_access` layer. Org-scoped (non-site) surfaces use
`getActiveOrgId()` + an `eq(org_id, …)` filter.

The audit showed this discipline is currently good, but the failure mode is real and
has zero backstop: a single entry point that forgets the helper opens a cross-tenant
gap (findings B2/B3 were exactly this — two route handlers that checked org membership
but skipped the staff `site_access` layer).

## Goal

Add a **filet** so a missed app-code tenancy check cannot silently ship, without the
cost/risk of per-row RLS.

## Non-goals (explicitly deferred / out of scope)

- **No per-row Postgres RLS policies.**
- **No GUC request context** (`set_config` / `SET LOCAL app.current_org`) and no
  wrapping of queries in transactions.
- **No reduced-privilege DB role.** Deferred by decision: without RLS a reduced role
  does not stop cross-tenant `SELECT`s (it only blocks DDL/escalation), so its value
  for *this* goal is marginal and it adds ops friction on the manual-deploy prod.
- No change to the runtime data path, no migration, no perf-affecting change.

These remain candidates for a future, dedicated effort if the product stays on
Postgres long-term (it is slated to move to SurrealDB).

## Component 1 — Static tenancy guard (centerpiece)

A bun test that runs inside the normal `bun run test:all` green gate and statically
inspects the **entry-point surface only**.

### Unit
- **File:** `src/lib/security/tenancy-guard.test.ts`
- **What it does:** fails the test suite if an entry-point file accesses a per-site
  table without a recognised tenancy gate (or an explicit allowlist entry).
- **Depends on:** the source tree (read-only file scan); no DB, no request scope.

### Scope of the scan
Two entry-point classes (where the gate MUST live):
1. Server Actions — any `*.ts` file whose source contains the `"use server"` directive.
2. Route handlers — `src/app/**/route.ts`.

Internal query modules (e.g. `src/lib/keywords/queries.ts`) are **not** scanned: they
are reached only through gated entry points, so scanning them would inflate the
allowlist without adding safety.

### Per-site tables (trigger set)
`sites`, `keywords`, `keyword_themes`, `keyword_targets`, `keyword_exclusions`,
`keyword_shortlist`, `backlinks`, `backlink_outreach`, `domain_backlinks`,
`content_items`, `content_keywords`, `site_audits`, `analytics_jobs`,
`gsc_query_snapshots`, `gsc_page_snapshots`. The list is derived from the Drizzle
schema (the Drizzle export identifiers, e.g. `keywords`, `domainBacklinks`,
`gscPageSnapshots`) and centralised as a constant so it is easy to extend when a new
per-site table is added.

### Rule
For each scanned file that references any per-site-table identifier in a `db.` call,
the file MUST also reference at least one recognised gate:
- `assertSiteInActiveOrg` / `assertKeywordInActiveOrg` / `assertContentItemInActiveOrg`, or
- `getActiveOrgId` (for the org-scoped filter pattern).

Otherwise the test fails, naming the file and the offending table, with a message
pointing at this spec and the assert helpers.

### Allowlist
An explicit, commented `ALLOWLIST` map (`relative path → reason`) for legitimate
exceptions. A file only passes the guard while flagged if it is on the allowlist.
Seeded during implementation by running the guard and triaging each hit:
- valid non-assert gate (e.g. the `gsc-token` worker route's Bearer `WORKER_API_KEY`
  check; super-admin/god-mode files gated by `assertSuperAdmin`) → add with a one-line
  reason;
- the assert helpers' own module → add with a reason;
- a genuine gap → **fix it** (add the proper assert) rather than allowlist it.

Adding a new allowlist entry is a deliberate, reviewable act — that is the point.

### Honest limitation
This is a heuristic source scan, not a proof. It catches "the gate is entirely
missing" (the B2/B3 class), not subtly-wrong scoping (e.g. asserting the wrong id).
It is a backstop, not a replacement for the asserts.

## Component 2 — B5: `currentUserCanAccessSite` fail-closed, done cleanly

`src/lib/sites/access.ts` `currentUserCanAccessSite` currently fails **open**: when no
caller resolves it returns `true`. Not exploitable in production (a real HTTP request
always resolves a caller), but it is a fail-open branch in a security primitive.

### Change
Flip the default to **fail-closed** (`if (!c) return false`).

### Test seam (avoids the `mock.module` footgun)
The quickwins branch showed that flipping this naively reds the DB suites that exercise
the real assert path, and that papering over it with `mock.module("./access", …)` leaks
process-globally across sibling test files (documented project footgun). So the flip
ships with a clean injection seam:
- Add a test-only caller override in `access.ts`, guarded by `process.env.NODE_ENV ===
  "test"`: a module-level settable hook (e.g. `__setTestCaller(caller | null)`) that
  `currentCaller()` consults first when set. No effect outside tests.
- The DB suites that exercise the real path (`assert-site.test.ts`,
  `assert-entities.test.ts`, `spec/audit/actions.test.ts`, and the site-mutation tests)
  call `__setTestCaller({ userId, orgId, role: "owner" })` in `beforeEach` and clear it
  in `afterEach` — explicit dependency injection instead of module mocking.
- Remove the now-unnecessary partial `mock.module("./access", …)` from
  `sites/actions.test.ts` and `billing/enforcement.test.ts` where the seam replaces it
  (or keep a minimal complete mock only where the seam doesn't reach). The success
  criterion is: no partial `./access` mock remains that could leak.

### Consistency note
`getSitesForActiveOrg` (same file) also has a fail-open `if (!c || isFullAccess(...))`
branch. It returns the full org site list when no caller resolves. It is a UI-list
read, lower-risk; flipping it is in scope **only if** it does not cascade into broad
test churn — otherwise note it and leave it. Decide during implementation based on the
gate result; do not force it.

## Testing & integration

- Everything rides the existing `bun run test:all` gate (per-directory processes).
- Acceptance: `test:all` green (0 fail), `tsc --noEmit` clean, touched files biome-clean.
- Commits (conventional):
  - `test(security): static tenancy guard for entry-point queries`
  - `fix(security): currentUserCanAccessSite fail-closed + test-caller seam`

## Risks & mitigations

- **Guard false positives** → the allowlist + a clear failure message; seeded by
  triage, not guesswork.
- **Guard false negatives** (subtly-wrong scoping) → accepted; documented as a backstop,
  not a proof. The asserts remain the primary control.
- **B5 flip re-reds the suites** → mitigated by the DI seam (Component 2), validated by
  running the full gate before commit.
- **New per-site table added later without updating the trigger set** → the trigger set
  lives next to the schema with a comment; a follow-up could derive it programmatically
  from the Drizzle schema exports.

## Acceptance criteria

1. A deliberately-introduced entry point that queries a per-site table without a gate
   makes `tenancy-guard.test.ts` fail.
2. `currentUserCanAccessSite` returns `false` when no caller resolves; the real-path DB
   suites pass via the injected test caller.
3. No partial `mock.module("./access", …)` remains that can leak across files.
4. Full green gate + clean typecheck; no migration, no runtime/perf change.