Motir

Type · code · Parent · MOTIR-3413 (discovery epic; no dependency edge into the finding card, so the epic is the container per log-bug.md's edge test) · Discovered in · MOTIR-3707 · Resolution · open

The §4.1 work-item cap's warm-pool TOCTOU guard is inert. entitlementsService.assertWithinWorkItemCap (lib/services/entitlementsService.ts:73-84) opens with

await organizationRepository.lockByIdForUpdate(organizationId, tx);

and lockByIdForUpdate (lib/repositories/organizationRepository.ts:81-86) is

const rows = await tx.$queryRaw<Array<{ id: string }>>`
  SELECT "id" FROM "organization" WHERE "id" = ${id} FOR UPDATE`;
return rows.length > 0;

Under withWorkspaceContext that statement matches ZERO rows, so it locks nothing, every racer falls through together, and the count → compare → create guard is a plain read-then-write. The boolean it returns says exactly this, and no caller reads it.

Measured, not inferred — a probe run inside withWorkspaceContext on a migrated test DB

{ "locked": false, "visibleOrgRows": 1,
  "gucs": [{ "ws": "cmtbmsnok0004gdnxnrjpm0b6", "role": "motir_app", "iso": "read committed" }] }

The org row is readable (visibleOrgRows: 1, admitted by organization_membership_visible) and is not lockable (locked: false).

Why. Postgres applies the UPDATE policy's USING clause to a SELECT … FOR UPDATE, because locking a row for update implies update permission — and rows failing it are filtered out silently rather than raising. organization's UPDATE policy, read out of pg_policy in the same transaction:

withWorkspaceContext binds app.user_id, app.workspace_id and app.project_id — never app.organization_id (lib/workspaces/context.ts:107-119). So the READ is armed by the membership policy and the LOCK, which needs the UPDATE arm, is not. relrowsecurity and relforcerowsecurity are both true, so the owner-bypass that hides this from a fixture does not apply to motir_app.

This is [[cant-lock-an-empty-set]] (FOR UPDATE over zero rows serializes nobody) crossed with [[bound-read-needs-an-arm]] (a correctly-bound statement against a table with no arm for THAT context returns nothing and raises nothing). Both scanners in tests/rls/ ask is it bound, never is it admitted, so neither reports this.

The overage reproduces — two transactions, real concurrency

tests/entitlementsService.test.ts's race test seeds 249 items against the 250 cap and races two withWorkspaceContext transactions. As written, Promise.allSettled alone does not overlap them — the first reaches its count, its create AND its COMMIT before the second counts, so the second legitimately sees 250 and rejects. That is why the test passes: it does not exercise the lock at all. Deleting lockByIdForUpdate from the service leaves it GREEN (measured).

Add a barrier holding both transactions open past their GUC binding until both have arrived, and unmodified product code reports:

AssertionError: census: seeded=249 finalCount=251 fulfilled=2 rejected=0 rejections=[]
  — 251 means the org-row FOR UPDATE did not serialize: expected 251 to be 250

finalCount=251 on a 250 cap, with both creates fulfilled. Removing the lock produces the identical result — the two are indistinguishable because the lock was never doing anything.

And the three CI reds MOTIR-3707 was filed about are consistent with this, not with a fixture shortfall: each failed with fulfilled length 2 on shard Vitest (3/3) (runs 32628202745, 32999646685, 33075123375). Under an inert lock a loaded shard that happens to interleave the two transactions produces exactly that. Not proven — none of those runs captured finalCount, which is what MOTIR-3707 adds — but it is now the leading reading, and it means those reds were a real defect surfacing, not flake.

Blast radius

assertWithinWorkItemCap is the pattern every §4 count-cap follows, and the service's own header comment states the contract this breaks ("every count-cap LOCKS THE ORG ROW FOR UPDATE first … the second racer blocks until the first commits"). Audit assertWithinProjectCap / assertWithinWorkspaceCap / the org-creation and storage caps in the same file: any of them that locks the org row from a workspace-bound context has the same hole. Cloud-only (isCloudBilling() gates every method), so a self-hosted build is unaffected.

Fix direction

Three moves, and the first two are not alternatives — do both:

1. Make the guard FAIL LOUD instead of silently inert. lockByIdForUpdate already returns whether it locked anything; assertWithinWorkItemCap must treat false as an error rather than proceeding unserialized. A cap that cannot serialize must refuse, not admit.
2. Give the lock a context that can take it. Either arm the organization UPDATE policy for the workspace-bound context, or bind app.organization_id on the cap path (withOrgServiceWriteContext already exists and is what the unbound tierForOrg helper uses for exactly this reason), or lock a row the workspace context genuinely owns. Whichever is chosen, prove it with the probe above returning locked: true.
3. Sweep the sibling caps in lib/services/entitlementsService.ts for the same shape.

Acceptance criteria

• A probe of organizationRepository.lockByIdForUpdate executed inside withWorkspaceContext returns true, and the pull-request body quotes it.
• assertWithinWorkItemCap raises rather than proceeding when the org-row lock matches no row; a test covers that arm directly.
• tests/entitlementsService.test.ts's race test is strengthened so the two withWorkspaceContext transactions genuinely overlap (a barrier releasing both after each has bound its GUCs), it PASSES on the fixed code, and it FAILS with finalCount=251 when lockByIdForUpdate is deleted — quote both outputs in the pull-request body. This lands here rather than in MOTIR-3707 because landing it before the fix turns main red.
• Every other cap in lib/services/entitlementsService.ts that locks the org row is audited in the same pass; the pull-request body names each one and says whether it had the hole.
• pnpm vitest run tests/entitlementsService.test.ts passes on the changed files and the pull request's Vitest shard is green.

Context refs

• lib/services/entitlementsService.ts:73-84 — assertWithinWorkItemCap, and the header comment stating the contract
• lib/repositories/organizationRepository.ts:81-86 — lockByIdForUpdate, and the boolean nobody reads
• lib/workspaces/context.ts:102-119 — withWorkspaceContext, which binds three GUCs and not app.organization_id
• tests/entitlementsService.test.ts — the race test, and MOTIR-3707's census that makes its red legible
• tests/rls/singletonReadScan.ts · tests/rls/callSiteScan.ts · tests/rls/systemContextScan.ts — the three scanners that cannot see this class