Skip to content

Bound the unbounded row loads in verifyStoredReceipts #295

Description

@javiertoledo

Context

Raised while reviewing #264, which adds the reverse pass over audited digests.
Pre-existing and not introduced by that PR — it is the shape of
verifyStoredReceipts as it already stands on main.

Problem

services/api/src/receipt-integrity.ts pulls two unbounded result sets into
memory on every call:

const stored = await db
  .select({ id: runs.id, receipt: runs.receipt })
  .from(runs)
  .where(and(eq(runs.orgId, orgId), isNotNull(runs.receipt), ...));

const audit = await db
  .select({ target: auditEvents.target, payload: auditEvents.payload })
  .from(auditEvents)
  .where(and(eq(auditEvents.orgId, orgId), eq(auditEvents.action, "run.finished")));

Neither has a limit, a window, or pagination. Both grow with every run the
organization has ever completed, and runs.receipt is a JSONB payload, so the
first query's row size is not small either. Every audited digest is then held in
a Map for the lifetime of the call.

checkReceiptIntegrity in services/api/src/doctor.ts calls this org-wide with
no runIds scope, so the readiness doctor pays the full cost. That is the
awkward part: an operator runs the doctor precisely when the platform is already
misbehaving, which is the worst moment for a check to be slow or to pressure
memory.

There is no reachable failure today — this is a scaling concern, not a defect,
and no organization is known to be large enough to feel it. It is filed so the
ceiling is written down rather than discovered.

Proposed change

Any of these resolves it; the last is probably the smallest:

  • Bound the reverse pass with a time window (audited digests since N days) and
    say so in the doctor's message, so the check stays honest about its scope.
  • Paginate both queries and stream the comparison instead of materializing.
  • Do the comparison in SQL — a LEFT JOIN from audited run.finished events to
    runs with receipt IS NULL answers the reverse question without loading
    either set, and an aggregate answers the forward one.

Worth deciding alongside whether checked should keep counting only surviving
receipts; see the note on the message denominator in the review of #264.

Why not in #264

That PR closes a correctness hole in three files and is scoped to it. This is a
different concern on code it merely touches, and the fix is a query redesign
rather than a line change.

Acceptance criteria

  • verifyStoredReceipts no longer loads an unbounded number of rows for an organization with a large run history.
  • The readiness doctor's receipt_integrity check still detects an invalid receipt, an unaudited one, and an audited-but-missing one.
  • If the check becomes windowed, its reported message states the window.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions