Repository navigation
Add admin-only DELETE /api/owner-expenses by idempotency key - #1609
Conversation
Adds DELETE /api/owner-expenses for removing duplicate owner-recorded expense rows from the ledger. Dashboard session cookie required with no token fallback, plus the cookie-mutator CSRF guard. Two safety pins keep a key from ever deleting a non-expense row: the key format is validated against the owner-expense idempotency shape, and the delete WHERE clause is pinned to sourceApp owner-recorded-expense. Returns requested/deleted/ notFound counts. Needed to purge 11 double-posted receipt rows inflating budget math by $1,553.86.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
![kody-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
| headers: Record<string, string> = {} | ||
| ): NextRequest { | ||
| const token = createSessionToken(); | ||
| return new NextRequest("https://usage.jays.services/api/owner-expenses", { |
There was a problem hiding this comment.
Production hostname exposure in src/app/api/owner-expenses/__tests__/route.test.ts: the test fixture commits the private production infrastructure origin https://usage.jays.services in public source, even though the value only constructs a request object. Use the synthetic non-production origin https://owner-expenses.test.
Also found in:
src/app/api/owner-expenses/__tests__/route.test.ts:188-188src/app/api/owner-expenses/__tests__/route.test.ts:203-203
Kody rule violation: Keep credentials out of public source and verify UI changes with automated screenshots
Prompt for LLM
File src/app/api/owner-expenses/__tests__/route.test.ts:
Line 175:
Production hostname exposure in `src/app/api/owner-expenses/__tests__/route.test.ts`: the test fixture commits the private production infrastructure origin `https://usage.jays.services` in public source, even though the value only constructs a request object. Use the synthetic non-production origin `https://owner-expenses.test`.
**Also found in:**
- `src/app/api/owner-expenses/__tests__/route.test.ts:188-188`
- `src/app/api/owner-expenses/__tests__/route.test.ts:203-203`
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| headers: Record<string, string> = {} | ||
| ): NextRequest { | ||
| const token = createSessionToken(); | ||
| return new NextRequest("https://usage.jays.services/api/owner-expenses", { |
There was a problem hiding this comment.
Production hostname exposure in src/app/api/owner-expenses/__tests__/route.test.ts: the test fixture commits the private production infrastructure origin https://usage.jays.services in public source, even though the value only constructs a request object. Use the synthetic non-production origin https://owner-expenses.test.
Also found in:
src/app/api/owner-expenses/__tests__/route.test.ts:188-188src/app/api/owner-expenses/__tests__/route.test.ts:203-203
Kody rule violation: Never expose secrets or private infrastructure values; reuse existing fleet env vars
Prompt for LLM
File src/app/api/owner-expenses/__tests__/route.test.ts:
Line 175:
Production hostname exposure in `src/app/api/owner-expenses/__tests__/route.test.ts`: the test fixture commits the private production infrastructure origin `https://usage.jays.services` in public source, even though the value only constructs a request object. Use the synthetic non-production origin `https://owner-expenses.test`.
**Also found in:**
- `src/app/api/owner-expenses/__tests__/route.test.ts:188-188`
- `src/app/api/owner-expenses/__tests__/route.test.ts:203-203`
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| const rawKeys = | ||
| body && typeof body === "object" | ||
| ? (body as Record<string, unknown>).idempotencyKeys |
There was a problem hiding this comment.
Runtime validation gap in src/app/api/owner-expenses/route.ts: the HTTP-boundary assertion (body as Record<string, unknown>).idempotencyKeys trusts untyped input before DELETE uses it for deletion. Add import { z } from "zod";, define a strict OwnerExpenseDeleteSchema before DELETE, replace the extraction and manual guard with safeParse, and use only parsed.data.idempotencyKeys for deletion.
Also found in:
src/app/api/owner-expenses/route.ts:216-216
Kody rule violation: Validate every inbound payload with Zod before it reaches Prisma
import { z } from "zod";
const OwnerExpenseDeleteSchema = z
.object({
idempotencyKeys: z
.array(z.string().regex(OWNER_EXPENSE_KEY_RE))
.min(1)
.max(DELETE_MAX_KEYS),
})
.strict();
const parsed = OwnerExpenseDeleteSchema.safeParse(body);
if (!parsed.success) {
return NextResponse.json(
{
error:
"idempotencyKeys must be a non-empty array of at most 100 owner-recorded-expense idempotency keys",
},
{ status: 400 },
);
}
const keys = [...new Set(parsed.data.idempotencyKeys)];Prompt for LLM
File src/app/api/owner-expenses/route.ts:
Line 198:
Runtime validation gap in `src/app/api/owner-expenses/route.ts`: the HTTP-boundary assertion `(body as Record<string, unknown>).idempotencyKeys` trusts untyped input before `DELETE` uses it for deletion. Add `import { z } from "zod";`, define a strict `OwnerExpenseDeleteSchema` before `DELETE`, replace the extraction and manual guard with `safeParse`, and use only `parsed.data.idempotencyKeys` for deletion.
**Also found in:**
- `src/app/api/owner-expenses/route.ts:216-216`
Suggested Code:
import { z } from "zod";
const OwnerExpenseDeleteSchema = z
.object({
idempotencyKeys: z
.array(z.string().regex(OWNER_EXPENSE_KEY_RE))
.min(1)
.max(DELETE_MAX_KEYS),
})
.strict();
const parsed = OwnerExpenseDeleteSchema.safeParse(body);
if (!parsed.success) {
return NextResponse.json(
{
error:
"idempotencyKeys must be a non-empty array of at most 100 owner-recorded-expense idempotency keys",
},
{ status: 400 },
);
}
const keys = [...new Set(parsed.data.idempotencyKeys)];
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| const rawKeys = | ||
| body && typeof body === "object" | ||
| ? (body as Record<string, unknown>).idempotencyKeys |
There was a problem hiding this comment.
HTTP-boundary type assertion in src/app/api/owner-expenses/route.ts: (body as Record<string, unknown>).idempotencyKeys derives the DELETE key-array type from an asserted object shape without runtime validation. Replace it with a strict Zod schema and safeParse, deriving the key-array type from the schema rather than trusting the asserted shape.
Also found in:
src/app/api/owner-expenses/route.ts:216-216
Kody rule violation: Validate all boundary data with Zod schemas instead of type assertions
import { z } from "zod";
const OwnerExpenseDeleteSchema = z
.object({
idempotencyKeys: z
.array(z.string().regex(OWNER_EXPENSE_KEY_RE))
.min(1)
.max(DELETE_MAX_KEYS),
})
.strict();
const parsed = OwnerExpenseDeleteSchema.safeParse(body);
if (!parsed.success) {
return NextResponse.json(
{
error:
"idempotencyKeys must be a non-empty array of at most 100 owner-recorded-expense idempotency keys",
},
{ status: 400 },
);
}
const keys = [...new Set(parsed.data.idempotencyKeys)];Prompt for LLM
File src/app/api/owner-expenses/route.ts:
Line 198:
HTTP-boundary type assertion in `src/app/api/owner-expenses/route.ts`: `(body as Record<string, unknown>).idempotencyKeys` derives the `DELETE` key-array type from an asserted object shape without runtime validation. Replace it with a strict Zod schema and `safeParse`, deriving the key-array type from the schema rather than trusting the asserted shape.
**Also found in:**
- `src/app/api/owner-expenses/route.ts:216-216`
Suggested Code:
import { z } from "zod";
const OwnerExpenseDeleteSchema = z
.object({
idempotencyKeys: z
.array(z.string().regex(OWNER_EXPENSE_KEY_RE))
.min(1)
.max(DELETE_MAX_KEYS),
})
.strict();
const parsed = OwnerExpenseDeleteSchema.safeParse(body);
if (!parsed.success) {
return NextResponse.json(
{
error:
"idempotencyKeys must be a non-empty array of at most 100 owner-recorded-expense idempotency keys",
},
{ status: 400 },
);
}
const keys = [...new Set(parsed.data.idempotencyKeys)];
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| const rawKeys = | ||
| body && typeof body === "object" | ||
| ? (body as Record<string, unknown>).idempotencyKeys |
There was a problem hiding this comment.
HTTP request validation gap in src/app/api/owner-expenses/route.ts: the handler accesses idempotencyKeys without validating the entire request body. Validate the full HTTP request body with a strict Zod schema, return a 4xx response on failure, and use only the parsed result downstream.
Also found in:
src/app/api/owner-expenses/route.ts:216-216
Kody rule violation: Validate every untrusted input with a zod schema at the trust boundary
import { z } from "zod";
const OwnerExpenseDeleteSchema = z
.object({
idempotencyKeys: z
.array(z.string().regex(OWNER_EXPENSE_KEY_RE))
.min(1)
.max(DELETE_MAX_KEYS),
})
.strict();
const parsed = OwnerExpenseDeleteSchema.safeParse(body);
if (!parsed.success) {
return NextResponse.json(
{
error:
"idempotencyKeys must be a non-empty array of at most 100 owner-recorded-expense idempotency keys",
},
{ status: 400 },
);
}
const keys = [...new Set(parsed.data.idempotencyKeys)];Prompt for LLM
File src/app/api/owner-expenses/route.ts:
Line 198:
HTTP request validation gap in `src/app/api/owner-expenses/route.ts`: the handler accesses `idempotencyKeys` without validating the entire request body. Validate the full HTTP request body with a strict Zod schema, return a 4xx response on failure, and use only the parsed result downstream.
**Also found in:**
- `src/app/api/owner-expenses/route.ts:216-216`
Suggested Code:
import { z } from "zod";
const OwnerExpenseDeleteSchema = z
.object({
idempotencyKeys: z
.array(z.string().regex(OWNER_EXPENSE_KEY_RE))
.min(1)
.max(DELETE_MAX_KEYS),
})
.strict();
const parsed = OwnerExpenseDeleteSchema.safeParse(body);
if (!parsed.success) {
return NextResponse.json(
{
error:
"idempotencyKeys must be a non-empty array of at most 100 owner-recorded-expense idempotency keys",
},
{ status: 400 },
);
}
const keys = [...new Set(parsed.data.idempotencyKeys)];
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const deleted = await prisma.externalUsageEvent.deleteMany({ | ||
| where: { | ||
| sourceApp: OWNER_EXPENSE_SOURCE_APP, | ||
| idempotencyKey: { in: keys }, | ||
| }, | ||
| }); | ||
|
|
||
| return NextResponse.json({ | ||
| requested: keys.length, | ||
| deleted: deleted.count, | ||
| notFound: keys.filter((key) => !existingKeys.has(key)), | ||
| }); |
There was a problem hiding this comment.
Cache invalidation omission in src/app/api/owner-expenses/route.ts: the DELETE handler removes ExternalUsageEvent rows without calling bustBudgetStatusCache(), so GET /api/providers can keep serving provider spend and budget totals that include deleted expenses until the memoized budgetStatusSwrCache SWR entry expires after 60 seconds. Owner-expense rows carry costUsd and occurredAt and feed the month-to-date computeBudgetStatusUncached aggregate through sumMonthToDateExternalCostByProvider (src/lib/budget-status.ts:892), which group-bys all rows without a sourceApp exclusion; import bustBudgetStatusCache from @/lib/budget-status and call it when deleted.count > 0 to match recordOwnerExpense → bustBudgetStatusCache() after the same-table write (src/lib/owner-expense.ts:177) and preserve the invalidation invariant used by projects, providers, budget-controls, workspace-copy, and ensure-fleet-projects.
const deleted = await prisma.externalUsageEvent.deleteMany({
where: {
sourceApp: OWNER_EXPENSE_SOURCE_APP,
idempotencyKey: { in: keys },
},
});
// Same invariant the insert path upholds (see lib/owner-expense.ts recordOwnerExpense):
// owner-expense rows feed sumMonthToDateExternalCostByProvider, which is memoized by the
// budget-status SWR cache, so a delete must invalidate it too.
if (deleted.count > 0) bustBudgetStatusCache();
return NextResponse.json({
requested: keys.length,
deleted: deleted.count,
notFound: keys.filter((key) => !existingKeys.has(key)),
});Prompt for LLM
File src/app/api/owner-expenses/route.ts:
Line 227 to 238:
Cache invalidation omission in `src/app/api/owner-expenses/route.ts`: the `DELETE` handler removes `ExternalUsageEvent` rows without calling `bustBudgetStatusCache()`, so `GET /api/providers` can keep serving provider spend and budget totals that include deleted expenses until the memoized `budgetStatusSwrCache` SWR entry expires after 60 seconds. Owner-expense rows carry `costUsd` and `occurredAt` and feed the month-to-date `computeBudgetStatusUncached` aggregate through `sumMonthToDateExternalCostByProvider` (`src/lib/budget-status.ts:892`), which group-bys all rows without a `sourceApp` exclusion; import `bustBudgetStatusCache` from `@/lib/budget-status` and call it when `deleted.count > 0` to match `recordOwnerExpense` → `bustBudgetStatusCache()` after the same-table write (`src/lib/owner-expense.ts:177`) and preserve the invalidation invariant used by projects, providers, budget-controls, workspace-copy, and ensure-fleet-projects.
Suggested Code:
const deleted = await prisma.externalUsageEvent.deleteMany({
where: {
sourceApp: OWNER_EXPENSE_SOURCE_APP,
idempotencyKey: { in: keys },
},
});
// Same invariant the insert path upholds (see lib/owner-expense.ts recordOwnerExpense):
// owner-expense rows feed sumMonthToDateExternalCostByProvider, which is memoized by the
// budget-status SWR cache, so a delete must invalidate it too.
if (deleted.count > 0) bustBudgetStatusCache();
return NextResponse.json({
requested: keys.length,
deleted: deleted.count,
notFound: keys.filter((key) => !existingKeys.has(key)),
});
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
| const keys = [...new Set(rawKeys as string[])]; | ||
|
|
||
| const existing = await prisma.externalUsageEvent.findMany({ |
There was a problem hiding this comment.
Response inconsistency in src/app/api/owner-expenses/route.ts: notFound uses the pre-delete findMany snapshot at line 218 before deleteMany at line 227, so a same-key row inserted or replayed between the statements by a concurrent POST /api/owner-expenses from the iOS client, or by a second concurrent DELETE, can be removed while still reported in notFound; deleted.count can also exceed existing.size, leaving { requested, deleted, notFound } internally inconsistent and giving callers incorrect reconciliation data. Derive notFound from post-delete state or run both statements in a single prisma.$transaction instead of relying on the pre-delete read.
const deleted = await prisma.externalUsageEvent.deleteMany({
where: {
sourceApp: OWNER_EXPENSE_SOURCE_APP,
idempotencyKey: { in: keys },
},
});
// Derive notFound from post-delete state instead of a pre-delete snapshot, so a row
// inserted between two reads can never be reported as not found while being deleted.
const survivors = await prisma.externalUsageEvent.findMany({
where: {
sourceApp: OWNER_EXPENSE_SOURCE_APP,
idempotencyKey: { in: keys },
},
select: { idempotencyKey: true },
});
const survivorKeys = new Set(survivors.map((row) => row.idempotencyKey));Prompt for LLM
File src/app/api/owner-expenses/route.ts:
Line 218:
Response inconsistency in `src/app/api/owner-expenses/route.ts`: `notFound` uses the pre-delete `findMany` snapshot at line 218 before `deleteMany` at line 227, so a same-key row inserted or replayed between the statements by a concurrent `POST /api/owner-expenses` from the iOS client, or by a second concurrent `DELETE`, can be removed while still reported in `notFound`; `deleted.count` can also exceed `existing.size`, leaving `{ requested, deleted, notFound }` internally inconsistent and giving callers incorrect reconciliation data. Derive `notFound` from post-delete state or run both statements in a single `prisma.$transaction` instead of relying on the pre-delete read.
Suggested Code:
const deleted = await prisma.externalUsageEvent.deleteMany({
where: {
sourceApp: OWNER_EXPENSE_SOURCE_APP,
idempotencyKey: { in: keys },
},
});
// Derive notFound from post-delete state instead of a pre-delete snapshot, so a row
// inserted between two reads can never be reported as not found while being deleted.
const survivors = await prisma.externalUsageEvent.findMany({
where: {
sourceApp: OWNER_EXPENSE_SOURCE_APP,
idempotencyKey: { in: keys },
},
select: { idempotencyKey: true },
});
const survivorKeys = new Set(survivors.map((row) => row.idempotencyKey));
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
1 participant
Summary
Adds an admin-only
DELETE /api/owner-expensesendpoint that removes owner-recorded expense rows by idempotency key. Deletion is destructive, so it is gated behind the dashboard session cookie only — there is intentionally noOWNER_EXPENSE_TOKENfallback — and is protected by the CSRF guard since it is a cookie-authenticated mutator.Changes
src/app/api/owner-expenses/route.tsDELETEhandler on the owner-expenses route.401otherwise. Requests presenting only anx-owner-expense-tokenare rejected. Cross-site cookie requests are rejected with403via the CSRF check.{ "idempotencyKeys": ["owner-recorded-expense:v1:<64 hex>", ...] }, read through the bounded JSON body helper.400when the key list is missing, not an array, empty, longer than 100 entries, or contains any key that does not match the owner-expense idempotency format. Duplicate keys are collapsed before processing.sourceApp: "owner-recorded-expense"in thewhereclause, so a valid-shaped key can never remove a non-expense row.{ requested, deleted, notFound: [...] }, wherenotFoundlists requested keys that did not exist.src/app/api/owner-expenses/__tests__/route.test.tsdeleteManymock and imports theDELETEexport andcreateSessionToken.DELETE /api/owner-expensessuite covering:401without a session cookie,401with an owner-expense token but no session,403for a cross-site cookie request,400for malformed keys, an empty key list, and a non-expense key shape (each asserting no delete is attempted); plus scoping assertions on thesourceApp/idempotencyKeyfilter, reporting ofnotFound, and deduplication of repeated keys.Review Findings
The automated review raised 7 findings (2 critical, 5 high) that are not addressed by the code in this PR:
DELETEhandler reaches into the request body with a type assertion rather than parsing it with Zod before it reaches Prisma (3 findings, same location).bustBudgetStatusCache(), leaving budget figures stale after a purge.notFoundis derived from a pre-delete read, so a row created between thefindManyanddeleteManywould be deleted yet reported as not found.