feat/one-api

SavingsConfig findByPublicId has no org-id predicate — single control when configPublicId is provided

apps/platform/src/api/handlers/public-v1/savings.handler.ts:125

findByPublicId(tx, configPublicId) queries only by publicId. Protection is: (a) RLS via readInOrg, and (b) a post-query guard checking config.siteId === site.siteId && config.orgId === orgId. The public-v1-rls.ts docstring promises 'two independent controls — the query keeps its orgId argument'. This route breaks that invariant: if the RLS policy were reverted, only the post-query guard would remain. findEnabledBySite is scoped by siteId and is fine; only the findByPublicId branch is missing the orgId predicate.

loadMonitoringHealthByRpu runs on service-role connection — single control for this path

apps/platform/src/api/handlers/public-v1/monitoring.handler.ts:119

After the RLS-wrapped getAccessibleContractPublicIds + listMonitoringStatusByContractPublicIds pair, loadMonitoringHealthByRpu(database, orgId, ...) runs on the bare service-role connection. The contractPublicIds argument is already narrowed by the RLS transaction, so no cross-org data is reachable — but it is application-level scoping only, not a two-control path. Noted departure from the two-controls principle stated in the docstring.

jobsDetailHandler: app-level org check is structurally unreachable — RLS returns null first

apps/platform/src/api/handlers/public-v1/jobs.handler.ts:267

findByPublicId has no org_id SQL predicate; it queries by publicId only. The org boundary is enforced by RLS (cfe_jobs_select_member). Under readInOrg, a cross-org job returns undefined → JobNotFound before the job.orgId !== orgId check fires. The check is correct defence-in-depth but structurally unreachable. Not a bug — both paths produce an identical 404 body — but worth a comment for future auditors: `// Structural defence-in-depth: RLS returns null for cross-org jobs before this line is reached`.

readInOrg async handling confirmed correct — Drizzle transaction wrapper properly awaits callback

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

rlsDbFor(auth, orgId).transaction(read) — the Proxy's transaction intercept wraps fn(tx) in an inner async callback and returns its result. Drizzle awaits the callback. No missing await, no fire-and-forget. Correct.

monitoringRemoveHandler write path correctly stays on service-role — no authenticated write policy

apps/platform/src/api/handlers/public-v1/monitoring.handler.ts

The deactivation shells (deactivateMonitoringShell, deactivatePaymentMonitoringShell) use database (service-role). monitoring_subscriptions has no authenticated write policy; running under RLS would deny the write (same rationale as BAT-291). Pattern correct.

zipFilesHandler: no cross-org data leak in the fused read+Lambda path

apps/platform/src/api/handlers/public-v1/files.handler.ts

generatePublicFilesZip calls getAccessibleContractPublicIds before any data fetch. No cross-org data is reachable before the Lambda invocation. The exclusion from readInOrg is correctly documented.

webhooksList positive-path test correctly validates service-role behavior — appropriate exception

apps/platform/src/__tests__/integration/public-v1-idor.test.ts:631

The test fires webhooksList as org B and asserts orgB.webhookPublicId is present, proving service-role access returns data for the owning org. Comment correctly explains that this route must NOT be RLS-wrapped (admin-gated table). Test caught the original regression when it was first wrapped and returned empty. Correct design.

Webhook list/delete accessible to non-admin machine credentials (known, unresolved disagreement)

apps/platform/src/api/handlers/public-v1/webhooks.handler.ts:156

webhooksListHandler reads through service-role with orgId filter, bypassing the DB policy webhook_endpoints_select_admin (gates on get_user_admin_org_ids()). Any machine credential for org X can list/delete org X's webhooks regardless of privilege level. The handler comment explicitly acknowledges this as an unresolved disagreement. An attacker with a low-privilege leaked bk_ key can enumerate webhook URLs and event configurations for its org. No new regression — this gap predates this commit — but documenting the planned fix (widen get_user_admin_org_ids for machine callers at webhook endpoints, or add an admin-role gate at the handler layer) would move it forward.

ZIP route is single-control — no DB-level isolation fallback (documented gap)

apps/platform/src/api/handlers/public-v1/files.handler.ts:149

zipFilesHandler is deliberately excluded from readInOrg (transaction would hold a Postgres connection open across a Lambda round-trip). Application-layer orgId + getAccessibleContractPublicIds is the sole gate. The comment documents the fix (split read from Lambda invocation so the read can be wrapped). No exploitable cross-org path exists today. The gap is real and documented — the right next step is splitting the orchestrator.

readInOrg transactions have no statement_timeout — potential pool exhaustion under adversarial load

packages/database/src/rls.ts:40

createRLSDb and createMachineRLSDb do not apply SET LOCAL statement_timeout inside the transaction. client.ts recommends per-transaction SET LOCAL as the correct place. Under concurrent API load, slow queries could hold connections open for the full idle timeout (20s). Bounded by Vercel's serverless function timeout in practice, but adding SET LOCAL statement_timeout = '10s' after the SET LOCAL ROLE call would close the gap defensively.

TOCTOU across three savings transactions — theoretical, no exploitable path

apps/platform/src/api/handlers/public-v1/savings.handler.ts:100

Site resolved in tx1, config in tx2, report in tx3. Org transfer of a site between tx1 and tx2 is the theoretical attack surface; the application guard config.orgId !== orgId catches it. Site org transfer is not a supported operation. Merging into one readInOrg transaction eliminates the concern entirely and is the cleaner fix.

rlsDbFor used outside .transaction() silently falls through to service-role — potential footgun

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

The docstring warns that bare rlsDbFor(auth, orgId).select(...) passes straight through to the admin connection. All current call sites use readInOrg, which enforces the transaction. Consider making rlsDbFor unexported and requiring callers to go through readInOrg.

readInOrg JSDoc is a 44-line multi-paragraph block — explicit convention violation

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

CLAUDE.md: 'Never write multi-paragraph docstrings or multi-line comment blocks — one short line max.' The function body runs 44 lines (lines 47–91) covering why RLS exists, four NOT-FOR categories, and a general rule. The same content already lives in .claude/rules/one-api.md. Reduce to: `// Org-bound RLS read — see one-api.md § 'Reads now run under it too' for exclusion rules.`

readInOrg tx parameter typed as opaque Parameters<...> chain instead of Transaction

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

`read: (tx: Parameters<Parameters<Database['transaction']>[0]>[0]) => Promise<T>` resolves to `Transaction` from @batu/shared-kernel — the same type used in every *.queries.ts file as `DbOrTx`. Replace with `import type { Transaction } from '@batu/shared-kernel'` and `read: (tx: Transaction) => Promise<T>`. The existing createRLSDb and createMachineRLSDb callbacks already use Transaction directly.

savingsReportGetHandler opens three separate readInOrg transactions for one logical read

apps/platform/src/api/handlers/public-v1/savings.handler.ts:115

Lines 115, 123, and 133 each open a separate org-bound transaction (site → config → report). Three Postgres round-trips for data that is logically a single snapshot read. The monitoring handler explicitly batches its two-query pair ('One transaction for the pair — a single logical read'). These three reads have a sequential dependency chain (site → config.siteId → report.configId), so they CAN be batched: one readInOrg that resolves site, then config if site found, then report if config found. Also applies to savingsConfigsListHandler (two transactions, lines 211 + 215).

filesList skip comment claims rls-machine-claim.test.ts covers bill_files — it doesn't

apps/platform/src/__tests__/integration/public-v1-idor.test.ts:648

Comment: 'Its READ is covered by the bill_files policy exercise in rls-machine-claim.test.ts.' That file tests only cfe_jobs and api_keys — bill_files are absent. The claim is false, so the justification for skipping the positive-path assertion is ungrounded. filesListHandler calls listPublicFiles inside readInOrg; if the bill_files SELECT policy were too narrow for machine callers, the handler silently returns 200 with an empty list and no test catches it.

savingsReportGetHandler positive path missing — three readInOrg calls, none verified to return data

apps/platform/src/__tests__/integration/public-v1-idor.test.ts:611

savingsReportGetHandler (savings.handler.ts) issues three separate readInOrg calls: findSiteInOrg, config lookup, report lookup. A misconfigured savings_configs or savings_reports SELECT policy returns null silently → handler returns 404. No positive-path test fires this as org B (the owner) to confirm data comes back. This is exactly the failure mode the positive-path suite was introduced to catch.

savingsConfigsListHandler positive path missing — broken RLS returns 200 with empty array

apps/platform/src/__tests__/integration/public-v1-idor.test.ts:611

savingsConfigsListHandler calls readInOrg twice (findSiteInOrg → listBySite). A policy that is too narrow for machine callers on savings_configs returns listBySite as an empty array with 200 — indistinguishable from no data. No positive-path assertion covers this route.

monitoringList/paymentStatusList positive-path asserts 200 only — not discriminating against silent empty result

apps/platform/src/__tests__/integration/public-v1-idor.test.ts:637

The comment says 'An empty result is legitimate here' but org B has seeded data (orgB.contractPublicId). An over-narrow RLS policy on utility_contracts or monitoring_subscriptions would also return 200 + empty array. The test cannot distinguish a correct result from a broken RLS. Since orgB.rpu is seeded, asserting the response contains it would give the test real discriminating power.

jobsDetailHandler positive path missing — readInOrg wraps findByPublicId, no data-return assertion

apps/platform/src/__tests__/integration/public-v1-idor.test.ts:611

jobsDetailHandler wraps jobQueries.findByPublicId in readInOrg. A broken cfe_jobs SELECT policy for machine callers silently returns null → identical 404 to 'not found'. No positive-path test creates a job for org B and asserts GET /v1/jobs/:id returns it. rls-machine-claim.test.ts covers the policy at DB level, but not the handler wiring.

filesDetailHandler positive path missing — readInOrg wraps getPublicFile, no positive assertion

apps/platform/src/__tests__/integration/public-v1-idor.test.ts:611

filesDetailHandler wraps getPublicFile in readInOrg. The AWS-credentials block applies to S3 presigning, not the RLS-wrapped DB query. A positive-path test can assert the handler returns a non-500 for org B's bfl_ id even without S3 creds (it would get a presigning error, not an RLS-empty 404). Currently no such test exists.

jobsCreate idempotency read-back (JobAlreadyExists path) not tested — INFRA_DEPENDENT routes skip it

apps/platform/src/__tests__/integration/public-v1-idor.test.ts:362

The readInOrg at jobs.handler.ts line 151 (idempotency read-back on the JobAlreadyExists dedup branch) is in infrastructure-dependent code (SFN/SQS). INFRA_DEPENDENT routes tolerate 500s, so a broken idempotency wiring would not be detected. Low-probability given rls-machine-claim pins the policy, but the branch is an untested code path.

monitoringRemoveHandler positive path missing — readInOrg lookup on remove path untested

apps/platform/src/__tests__/integration/public-v1-idor.test.ts:611

monitoringRemoveHandler uses readInOrg to fetch accessible contracts before deactivating. If the RLS lookup silently returns empty for the owning org, the handler returns RPU_NOT_FOUND for the owner. No positive-path probe fires DELETE /v1/monitoring/:rpu as org B against org B's own RPU.

No unit tests for readInOrg — actorType branch dispatch covered only via integration

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

The three actorType branches (machine → createMachineRLSDb, user → createRLSDb, service → bare db) are only exercised by integration tests. Logic is a simple switch — integration coverage is sufficient. Worth noting for future refactors.

savingsConfigsListHandler uses two readInOrg calls where one suffices

apps/platform/src/api/handlers/public-v1/savings.handler.ts:211

findSiteInOrg (line 211) and listBySite (line 215) are two separate transactions with a sequential dependency. One readInOrg returning { site, configs } would halve the round-trips and match the monitoring handler's documented pattern. Currently inconsistent with the 'one transaction for a logical pair' convention.

savingsConfigCreateHandler connection split undocumented — unlike monitoringRemoveHandler

apps/platform/src/api/handlers/public-v1/savings.handler.ts:168

The site gate uses readInOrg, then createSavingsConfigShell uses bare database (service-role). This split is correct (no authenticated write policy on savings_configs). Unlike monitoringRemoveHandler which has an explicit comment explaining the split ('The DEACTIVATION below stays on the service-role connection: those shells write subscription + outbox, and monitoring_subscriptions grants no authenticated write policy'), the savings create handler has no explanation. Readers comparing the two handlers may assume the service-role usage is accidental.

monitoringRemoveHandler: deactivatePaymentMonitoringShell result silently dropped without explanation

apps/platform/src/api/handlers/public-v1/monitoring.handler.ts:202

deactivatePaymentMonitoringShell returns Result<{deactivated: boolean}, never>. The never error type means it structurally cannot fail, so dropping the result is correct. However, a plain `await` with no assignment looks like a copy-paste oversight compared to deactivateMonitoringShell which checks `!r.ok`. Add `void await` or a one-line comment: `// Result<_, never> — structurally infallible, result dropped`.

Positive-path suite covers 5 of 16 converted read sites — savings and files routes have no positive coverage

apps/platform/src/__tests__/integration/public-v1-idor.test.ts:611

billsList/Detail, webhooksList, monitoringList, paymentStatusList are covered. savingsReportGet, savingsConfigsList, filesDetail, monitoringRemove are not. The filesList exclusion is documented; the savings routes have no note.

Pattern convention for batched reads is documented in monitoring/payment-status but absent from savings

apps/platform/src/api/handlers/public-v1/savings.handler.ts

monitoring.handler.ts and payment-status.handler.ts carry the comment 'One transaction for the pair — a single logical read, one org-bound snapshot.' The savings handlers that do NOT batch (savingsReportGetHandler — three calls) lack any note explaining why the dependency chain prevents batching. Adding this as a rule in api-patterns.md ('batch related reads into one readInOrg; if sequential calls appear, explain why') would make the asymmetry intentional.