Skip to content

Commit 91671f7

Browse files
committed
fix(apps): cascade application deletes and make release transitions atomic
Application ids are client-supplied, and deleting an app removed only the parent row. Versions, budgets, invocations and pinned graphs had no ownership column and no cascading foreign key, so a user who knew a deleted app's id could recreate it under their own account: the ownership check passed against the new parent row while the child reads returned the previous owner's orphaned releases and usage. Child tables now carry real foreign keys with ON DELETE CASCADE, and delete runs as one transaction that erases children explicitly -- foreign key enforcement is per connection, so the cascade is declared and performed rather than assumed. application_versions and application_invocations gained a user_id stamped from the parent, and the reads that take only an application id can now filter on it; the tRPC router and the websocket runner pass the caller's id. Ids are validated and claimed through an insert that fails on reuse instead of an upsert, so a released id cannot be taken over. A migration deletes orphans, backfills user_id, and rebuilds the tables with the constraints. Publishing read MAX(version), cleared the released flag and inserted the new row as separate statements, with nothing in the schema forbidding a duplicate version or a second released row -- concurrent publishes could produce either, leaving the released lookup to pick arbitrarily. Each transition is now one transaction, (application_id, version) is unique, and the lookup orders by version. Rollback verifies the target exists before clearing anything, so a bad version number no longer leaves the app with nothing released. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Su7dXD6fnXtKKbdCcM2vS
1 parent 0adee54 commit 91671f7

17 files changed

Lines changed: 1328 additions & 121 deletions

packages/models/src/application-budget.ts

Lines changed: 55 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
applicationBudgets,
2020
applicationInvocations
2121
} from "./schema/application-budgets.js";
22+
import { applications } from "./schema/applications.js";
2223

2324
export type BudgetPeriod = "day" | "month" | "total";
2425

@@ -53,6 +54,8 @@ export type BudgetDecision =
5354
export interface InvocationRecord {
5455
id: string;
5556
applicationId: string;
57+
/** Owner of the app when the run was recorded; null on pre-existing rows. */
58+
userId: string | null;
5659
version: number | null;
5760
invocationId: string;
5861
operationId: string;
@@ -75,6 +78,7 @@ const toBudget = (row: Record<string, unknown>): ApplicationBudget => ({
7578
const toRecord = (row: Record<string, unknown>): InvocationRecord => ({
7679
id: String(row.id),
7780
applicationId: String(row.application_id),
81+
userId: row.user_id == null ? null : String(row.user_id),
7882
version: row.version == null ? null : Number(row.version),
7983
invocationId: String(row.invocation_id),
8084
operationId: String(row.operation_id ?? ""),
@@ -85,6 +89,24 @@ const toRecord = (row: Record<string, unknown>): InvocationRecord => ({
8589
settledAt: row.settled_at == null ? null : String(row.settled_at)
8690
});
8791

92+
/**
93+
* Who a ledger row belongs to. The ledger is read by application id, and ids
94+
* are client-supplied, so each row records the owner of the app that produced
95+
* it instead of leaving that to whatever row holds the id later. Callers that
96+
* already know the user pass it; the rest read it off the parent.
97+
*/
98+
async function ownerOfApplication(
99+
applicationId: string
100+
): Promise<string | null> {
101+
const db = getDb();
102+
const rows = await db
103+
.select({ user_id: applications.user_id })
104+
.from(applications)
105+
.where(eq(applications.id, applicationId))
106+
.limit(1);
107+
return (rows[0]?.user_id as string | undefined) ?? null;
108+
}
109+
88110
/** Start of the window a period covers, or null for "total". */
89111
export const periodStart = (period: BudgetPeriod, now: Date): string | null => {
90112
if (period === "total") return null;
@@ -220,26 +242,15 @@ export async function checkApplicationBudget(
220242
}
221243

222244
/** Record a run against the app. Also the release telemetry row. */
223-
export async function recordInvocation(input: {
224-
applicationId: string;
225-
version?: number | null;
226-
invocationId: string;
227-
operationId?: string;
228-
estimatedUsd?: number;
229-
}): Promise<InvocationRecord> {
245+
export async function recordInvocation(
246+
input: ReserveInput
247+
): Promise<InvocationRecord> {
230248
const db = getDb();
249+
const userId =
250+
input.userId ?? (await ownerOfApplication(input.applicationId));
231251
const rows = await db
232252
.insert(applicationInvocations)
233-
.values({
234-
id: createTimeOrderedUuid(),
235-
application_id: input.applicationId,
236-
version: input.version ?? null,
237-
invocation_id: input.invocationId,
238-
operation_id: input.operationId ?? "",
239-
estimated_usd: input.estimatedUsd ?? 0,
240-
status: "running",
241-
created_at: new Date().toISOString()
242-
})
253+
.values(invocationRow(input, userId))
243254
.returning();
244255
return toRecord(rows[0] as Record<string, unknown>);
245256
}
@@ -296,9 +307,10 @@ export type Reservation =
296307
};
297308

298309
/** Values every reserved row carries, independent of which driver writes it. */
299-
const invocationRow = (input: ReserveInput) => ({
310+
const invocationRow = (input: ReserveInput, userId: string | null) => ({
300311
id: createTimeOrderedUuid(),
301312
application_id: input.applicationId,
313+
user_id: userId,
302314
version: input.version ?? null,
303315
invocation_id: input.invocationId,
304316
operation_id: input.operationId ?? "",
@@ -326,6 +338,8 @@ const overBudget = (
326338

327339
export interface ReserveInput {
328340
applicationId: string;
341+
/** Owner the run is booked against. Read off the app when omitted. */
342+
userId?: string | null;
329343
version?: number | null;
330344
invocationId: string;
331345
operationId?: string;
@@ -349,11 +363,15 @@ export async function reserveInvocation(
349363
): Promise<Reservation> {
350364
const estimatedUsd = input.estimatedUsd ?? 0;
351365
const db = getDb();
366+
// Resolved before the transaction: the SQLite branch runs synchronously and
367+
// cannot await a lookup of its own.
368+
const userId =
369+
input.userId ?? (await ownerOfApplication(input.applicationId));
352370

353371
// No budget row means unmetered, so there is nothing to serialize on.
354372
const configured = await getApplicationBudget(input.applicationId);
355373
if (!configured) {
356-
const record = await recordInvocation(input);
374+
const record = await recordInvocation({ ...input, userId });
357375
return {
358376
allowed: true,
359377
record,
@@ -389,7 +407,7 @@ export async function reserveInvocation(
389407
if (!budgetRow) {
390408
const orphan = tx
391409
.insert(applicationInvocations)
392-
.values(invocationRow(input))
410+
.values(invocationRow(input, userId))
393411
.returning()
394412
.get();
395413
return unmetered(toRecord(orphan as Record<string, unknown>));
@@ -418,7 +436,7 @@ export async function reserveInvocation(
418436
if (refused) return { allowed: false, ...refused, usage, budget };
419437
const row = tx
420438
.insert(applicationInvocations)
421-
.values(invocationRow(input))
439+
.values(invocationRow(input, userId))
422440
.returning()
423441
.get();
424442
return {
@@ -442,7 +460,7 @@ export async function reserveInvocation(
442460
if (!budgetRow) {
443461
const [orphan] = await tx
444462
.insert(applicationInvocations)
445-
.values(invocationRow(input))
463+
.values(invocationRow(input, userId))
446464
.returning();
447465
return unmetered(toRecord(orphan as Record<string, unknown>));
448466
}
@@ -469,7 +487,7 @@ export async function reserveInvocation(
469487
if (refused) return { allowed: false, ...refused, usage, budget };
470488
const [row] = await tx
471489
.insert(applicationInvocations)
472-
.values(invocationRow(input))
490+
.values(invocationRow(input, userId))
473491
.returning();
474492
return {
475493
allowed: true,
@@ -479,16 +497,27 @@ export async function reserveInvocation(
479497
});
480498
}
481499

482-
/** Recent runs of an application, newest first. */
500+
/**
501+
* Recent runs of an application, newest first. `userId` scopes the read to the
502+
* owner the rows were written for; rows predating the column stay visible.
503+
*/
483504
export async function listInvocations(
484505
applicationId: string,
485-
limit = 50
506+
limit = 50,
507+
userId?: string
486508
): Promise<InvocationRecord[]> {
487509
const db = getDb();
510+
const scope =
511+
userId === undefined
512+
? eq(applicationInvocations.application_id, applicationId)
513+
: and(
514+
eq(applicationInvocations.application_id, applicationId),
515+
sql`(${applicationInvocations.user_id} IS NULL OR ${applicationInvocations.user_id} = ${userId})`
516+
);
488517
const rows = await db
489518
.select()
490519
.from(applicationInvocations)
491-
.where(eq(applicationInvocations.application_id, applicationId))
520+
.where(scope)
492521
.orderBy(sql`${applicationInvocations.created_at} desc`)
493522
.limit(limit);
494523
return rows.map((r: Record<string, unknown>) => toRecord(r));

0 commit comments

Comments
 (0)