feat/one-api

STABLE annotation on `get_user_org_ids()` is load-bearing in a non-obvious way

packages/database/drizzle/0069_machine_org_claim.sql:46

STABLE tells the planner it may call the function once per statement and cache the result. The GUC is set by a prior separate `set_config` call in the same transaction, so ordering is correct. However, the STABLE annotation means the planner COULD evaluate the function once and reuse it across multiple RLS-checked rows in a single query. This is the original behaviour inherited from the prior function definition — not introduced by this PR — and is correct in that the GUC doesn't change mid-statement. Worth a brief comment in the migration to avoid future editors marking it VOLATILE 'for safety'.

Post-gate reads use service-role — metrics reads have only one isolation control

apps/platform/src/api/handlers/public-v1/energy-summary.handler.ts:82

After `findSiteInOrg` (dual-controlled), `resolveSiteTariffContext` and `resolveSiteEnergyMetricsShell` use the bare service-role `database`. Tariff/pricing-zone are global reference data (org-agnostic), so service-role is appropriate for them. Metric reads are site-scoped, not directly org-scoped at the Tinybird layer today. This is documented and intentional — but worth tracking: when metric reads land in Postgres, they should be wrapped in `rlsDbFor` too.

Double `current_setting` evaluation in SQL — redundant but no correctness risk

packages/database/drizzle/0069_machine_org_claim.sql:58

`nullif(current_setting('request.jwt.claims', true), '')::json ->> 'batu_org_id'` is evaluated twice (SELECT + WHERE). Within a STABLE function invocation the GUC is stable, so no inconsistency. A CTE or subquery form would evaluate it once and remove the duplication hazard (someone editing one occurrence but not the other).

Invalid UUID in `batu_org_id` claim raises Postgres error rather than returning 0 rows

packages/database/drizzle/0069_machine_org_claim.sql:58

If `batu_org_id` is present but not a valid UUID, `::uuid` cast raises. This surfaces as a 500 rather than a graceful 0-row result. In current trust model this is impossible via normal paths (orgId comes from the DB, signed with our private key). Still, a `EXCEPTION WHEN invalid_text_representation` guard or pre-cast validation would make failure safe.

`rlsDbFor` service case accepts `orgId: string` but ignores it — no type enforcement

apps/platform/src/api/utils/public-v1-rls.ts:58

For `service` actors the function returns bare `database` without using `orgId`. TypeScript forces all callers to pass `orgId: string` even when the service case doesn't use it. Future callers may pass a dummy value rather than wiring the correct early-exit. An overloaded signature or `orgId: string | undefined` with explicit handling would be more self-documenting.

Proxy only intercepts `.transaction()` — bare `.select()` on the wrapper bypasses RLS

packages/database/src/rls.ts:87

Documented behavior: 'only applies row-level security INSIDE .transaction()'. The canary handler correctly wraps in .transaction(). Risk: future handlers copy `rlsDbFor(auth, orgId).select(...)` without the transaction wrapper and silently get service-role reads. Inherited behavior from existing `createRLSDb`. Consider a runtime warning or comment for future callers.

No UUID format validation in JWT verify — malformed org_id causes 500 not 401

packages/api/src/auth/public-jwt.ts:321

`verifyBatuJwt` validates `typeof payload.org_id === 'string'` but not UUID format. A JWT with a malformed `org_id` passes verification, enters `createMachineRLSDb`, and causes the `::uuid` cast in `get_user_org_ids()` to throw — surfacing as a 500. Not a security bypass (rejection still occurs), but the error response is wrong. Add UUID regex validation in `verifyBatuJwt`, returning `PublicJwtClaimsInvalid` (401) for malformed org UUIDs.

`orgId` not validated as a UUID before insertion into the GUC JSON string

packages/database/src/rls.ts:97

`orgId` is typed `string`. `JSON.stringify` safely escapes it (no JSON injection), and the Drizzle `sql` template parameterizes the string (no SQL injection). But a non-UUID string would cause `::uuid` cast to raise a Postgres error (500). Validate UUID format early in `verifyBatuJwt` or in `createMachineRLSDb` itself.

Post-gate metric reads expose a future single-control hazard as RLS coverage expands

apps/platform/src/api/handlers/public-v1/energy-summary.handler.ts:82

Metrics are Tinybird-backed today with no RLS. When ported to Postgres, they will need `rlsDbFor` wrapping here. Track as a convergence follow-up to prevent metrics from silently staying on service-role after the port.

`SET LOCAL ROLE authenticated` fails noisily if role is not granted — not a bypass

packages/database/src/rls.ts:104

If `authenticated` is not granted to the connection user (non-standard Supabase setup, local plain Postgres), this throws inside the transaction and the handler returns a 500. Not a security bypass. Matches the existing `createRLSDb` pattern — not new risk.

GUC trust boundary is correct — external callers cannot set `request.jwt.claims`

packages/database/drizzle/0069_machine_org_claim.sql:33

`request.jwt.claims` is a Postgres GUC, not an HTTP header. Setting it requires executing SQL on our backend connection. The only code paths that can call `set_config` are `createRLSDb` and `createMachineRLSDb`. External callers have no SQL execution path. Trust boundary is sound.

`UNION` vs `UNION ALL` — dedup provides no security benefit here

packages/database/drizzle/0069_machine_org_claim.sql:53

The two UNION branches are structurally disjoint: branch 1 uses `auth.uid()` → profile chain; branch 2 reads `batu_org_id`. A human session never has `batu_org_id` set (the `rlsDbFor` switch routes them to `createRLSDb`). `UNION ALL` would be identical in effect. No security concern.

`get_user_admin_org_ids()` correctly NOT widened — `api_keys` isolation maintained

packages/database/drizzle/0069_machine_org_claim.sql:27

The 19 policies guarding credential management call `get_user_admin_org_ids()`, which is unchanged. The integration test empirically confirms a machine actor sees zero rows from `api_keys` even with a valid org claim. One leaked `bk_` credential cannot enumerate its org's others.

Blast radius of widening is bounded by the additive property

packages/database/drizzle/0069_machine_org_claim.sql:53

All 88 policies reading `get_user_org_ids()` inherit the change at once. The additive property (no claim = no org rows) means all existing human sessions are unaffected. A bug in the new branch could only affect machine callers, not human sessions gaining escalated access. The worst case is cross-org exposure for machine credentials — serious but structurally limited.

`rls-checklist.md` not updated to document the machine-caller path

.claude/rules/rls-checklist.md

The checklist currently documents only the human-auth (`createRLSDb`) path. `createMachineRLSDb` is a parallel entry point with the same transaction-binding requirement but a different claim field (`batu_org_id` not `sub`). Without updating the checklist, a future implementer following it will either add an unnecessary `sub` claim to machine transactions or flag correct machine code as a violation.

`db as never` cast in test vs `db as unknown as Database` established pattern

apps/platform/src/__tests__/integration/rls-machine-claim.test.ts:84

`createMachineRLSDb(db as never, orgId)` uses an escape-hatch cast. All other call sites in the repo use `db as unknown as Database`. Align for consistency — the test double cast is not harmful but will confuse readers.

Redundant `database` module singleton in `public-v1-rls.ts`

apps/platform/src/api/utils/public-v1-rls.ts:45

Both `public-v1-rls.ts` and `energy-summary.handler.ts` independently declare `const database = db as unknown as Database`. Two module-level casts of the same singleton. Would be resolved if `rlsDbFor` accepted `db` as a parameter (see improvement finding).

`rlsDbFor` location in `apps/platform/src/api/utils/` is architecturally correct

apps/platform/src/api/utils/public-v1-rls.ts:1

Moving it to `packages/database` would create a dependency on `@batu/api` from the database package, violating the stated rule. Current location is correct.

Comment density is high but justified for a security-critical boundary

packages/database/src/rls.ts:63

The multi-paragraph JSDoc on `createMachineRLSDb` and the large file-level comment in `public-v1-rls.ts` document non-obvious security reasoning (why `service` is not wrapped, why org-claim approach was chosen, explicit BAT-291 rejection). The information is non-redundant with code. Defensible for this risk level.

Proxy pattern and `config?: any` in `createMachineRLSDb` exactly mirrors `createRLSDb`

packages/database/src/rls.ts:87

The Proxy shape, eslint-disable comment, and Reflect.get fallback are byte-for-byte consistent with the pre-existing `createRLSDb`. The `any` is pre-existing and required to match Drizzle's transaction config overload signature. Not a regression.

Vacuous pass when DB unavailable — `if (!available) return` makes tests report green

apps/platform/src/__tests__/integration/rls-machine-claim.test.ts:54

Every `it` block starts with `if (!available) return;`. Vitest counts a test that returns early as PASSED, not skipped. On a CI runner where the DB is unavailable, every security-critical assertion silently passes. Use `it.skipIf(!available)` or a top-level `describe.skipIf` so the skip is visible in the test report rather than masquerading as green.

CI gate fires only on DB-file changes — handler-only changes bypass the RLS test

.github/workflows/pr-checks.yml:310

The step runs only when `steps.db-changes.outputs.changed == 'true'`. A change to `energy-summary.handler.ts`, `rlsDbFor`, or any auth-path handler without touching a migration file skips the machine-claim RLS test entirely. The step should also fire on changes to `packages/database/src/**` and `apps/platform/src/api/**`.

'Claim does not leak' test validates the wrong invariant — service-role always sees all rows

apps/platform/src/__tests__/integration/rls-machine-claim.test.ts:116

After `readJobsAs(ORG_A)` the test queries via bare `db.execute` (service-role) and expects count=2. Service-role always bypasses RLS and sees all rows — this proves nothing about claim leakage. The meaningful test is: a subsequent `readJobsAs(ORG_B)` should still return `[JOB_B]` (not `[JOB_A, JOB_B]`), or a non-service-role connection should see zero rows from the seeded set.

`rlsDbFor` routing dispatch table is not tested

apps/platform/src/__tests__/integration/rls-machine-claim.test.ts:84

The test calls `createMachineRLSDb` directly, bypassing `rlsDbFor`. A regression in the `rlsDbFor` switch (e.g., machine branch not reached, wrong actorType check) would go undetected. At minimum one test should call `rlsDbFor({ actorType: 'machine', ... }, ORG_A)` and assert per-org isolation.

No test for `createMachineRLSDb` with null/undefined orgId

apps/platform/src/__tests__/integration/rls-machine-claim.test.ts

If `orgId` is `undefined` at runtime (e.g., credential lookup returns no org), `JSON.stringify({ batu_org_id: undefined })` omits the key — which makes the WHERE clause false and returns zero rows (safe). But this should be asserted, not assumed. Add a test that an empty/missing orgId yields 0 rows rather than unintended broad access.

Migration guard can give false positive on substring match

apps/platform/src/__tests__/integration/rls-machine-claim.test.ts:22

`position('batu_org_id' in pg_get_functiondef(...)) > 0` matches any occurrence of the string `batu_org_id` in the function source, including comments or dead code. A more reliable guard: inject a synthetic claim via `set_config` and assert the function returns a non-empty result, or check for the exact SQL clause rather than just the field name.

`api_keys` test may pass vacuously if table is empty in test DB

apps/platform/src/__tests__/integration/rls-machine-claim.test.ts:105

Asserts count=0 via machine RLS, but if `api_keys` is empty in the test DB, the test passes regardless of whether RLS is applied. Seed one `api_keys` row for `ORG_A` in `beforeAll` and confirm it's visible via service-role `db`, then assert 0 via `createMachineRLSDb` — that proves RLS is actually filtering.

Seed cleanup ordering is correct but fragile — no cascade awareness

apps/platform/src/__tests__/integration/rls-machine-claim.test.ts:46

`cfe_jobs` deleted before `organizations` — correct for FK constraints. If a new FK referencing `cfe_jobs` is added, teardown silently fails. Comment the ordering rationale or use explicit FK-safe patterns.

Both human and machine claims set simultaneously is not tested

apps/platform/src/__tests__/integration/rls-machine-claim.test.ts

A JWT with both `sub` and `batu_org_id` set is not exercised. The expected behavior (UNION of both orgs? or machine branch only?) should be specified and tested, even just to assert the current outcome is intentional.

DRY: `createRLSDb` and `createMachineRLSDb` are identical except for the claims object

packages/database/src/rls.ts:87

The two functions share identical Proxy boilerplate (SET LOCAL ROLE, set_config ordering). A private `createRlsDbWithClaims(db, claims)` would centralize the security-sensitive ordering. Fixes applied to one currently must be manually applied to the other. Becomes more pressing if a third variant (e.g., multi-org service key) is added.

`rlsDbFor` closes over a module-level singleton — untestable without real infrastructure

apps/platform/src/api/utils/public-v1-rls.ts:45

The function closes over `const database = db as unknown as Database` at module level rather than accepting a `db` parameter. This makes unit testing impossible without real Drizzle infrastructure. Signature `rlsDbFor(db: Database, auth: AuthContext, orgId: string): Database` would match how `createRLSDb` is typed and make the dispatch table testable in isolation.

Handler wraps only `findSiteInOrg` in RLS transaction — inconsistency creates copy-paste hazard

apps/platform/src/api/handlers/public-v1/energy-summary.handler.ts:73

The `rlsDbFor(...).transaction()` scope covers only the org gate. Subsequent reads on bare `database` are intentional (global ref data + site already confirmed), but a future editor adding a read between lines 73–95 may not notice the two different connection spellings. Add per-call-site comments (`// global ref data, org-agnostic`) distinguishing why each post-gate read is safe on service-role.

`config?: any` could use Drizzle's `PgTransactionConfig` type

packages/database/src/rls.ts:39

Both RLS wrappers use `config?: any` forwarded to `target.transaction`. The drizzle-orm postgres-js driver exposes `PgTransactionConfig`. Low priority since `config` is never passed in practice, but this was a missed opportunity to type it in both wrappers at once.

`orgId` as plain `string` — not branded as `OrganizationId`

packages/database/src/rls.ts:87

The docstring explicitly calls out 'this value IS the tenant boundary; a wrong one is a cross-tenant read', but the type is `string`. A branded `OrganizationId` type would make that invariant structurally enforced. Consider as part of a broader branding pass.

Comment density in `public-v1-rls.ts` is high but appropriate

apps/platform/src/api/utils/public-v1-rls.ts:1

70 lines total: ~40 lines comment, ~15 lines impl. The prose carries non-obvious security reasoning and explicit BAT-291 rejection rationale. Information is non-redundant with code. Defensible for a security-critical dispatch table.