Repository navigation
Add admin-only DELETE /api/owner-expenses by idempotency key #1609
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,25 +4,30 @@ import { NextRequest } from "next/server"; | |
| const mocks = vi.hoisted(() => ({ | ||
| findMany: vi.fn(), | ||
| create: vi.fn(), | ||
| deleteMany: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock("@/lib/prisma", () => ({ | ||
| prisma: { | ||
| externalUsageEvent: { | ||
| findMany: mocks.findMany, | ||
| create: mocks.create, | ||
| deleteMany: mocks.deleteMany, | ||
| }, | ||
| }, | ||
| })); | ||
|
|
||
| let GET: typeof import("../route").GET; | ||
| let POST: typeof import("../route").POST; | ||
| let DELETE: typeof import("../route").DELETE; | ||
| let createSessionToken: typeof import("@/lib/auth").createSessionToken; | ||
|
|
||
| const READ_TOKEN = "r".repeat(64); | ||
|
|
||
| beforeAll(async () => { | ||
| process.env.SESSION_SECRET = "s".repeat(64); | ||
| ({ GET, POST } = await import("../route")); | ||
| ({ GET, POST, DELETE } = await import("../route")); | ||
| ({ createSessionToken } = await import("@/lib/auth")); | ||
| }); | ||
|
|
||
| beforeEach(() => { | ||
|
|
@@ -32,6 +37,8 @@ beforeEach(() => { | |
| delete process.env.OWNER_EXPENSE_TOKEN; | ||
| mocks.findMany.mockReset(); | ||
| mocks.findMany.mockResolvedValue([]); | ||
| mocks.deleteMany.mockReset(); | ||
| mocks.deleteMany.mockResolvedValue({ count: 0 }); | ||
| }); | ||
|
|
||
| function getRequest( | ||
|
|
@@ -155,3 +162,107 @@ describe("GET /api/owner-expenses", () => { | |
| } | ||
| }); | ||
| }); | ||
|
|
||
| describe("DELETE /api/owner-expenses", () => { | ||
| const KEY_A = `owner-recorded-expense:v1:${"a".repeat(64)}`; | ||
| const KEY_B = `owner-recorded-expense:v1:${"b".repeat(64)}`; | ||
|
|
||
| function deleteRequest( | ||
| keys: unknown, | ||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Production hostname exposure in Also found in:
Kody rule violation: Never expose secrets or private infrastructure values; reuse existing fleet env vars Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| method: "DELETE", | ||
| headers: { | ||
| "content-type": "application/json", | ||
| cookie: `dashboard_session=${token}`, | ||
| ...headers, | ||
| }, | ||
| body: JSON.stringify({ idempotencyKeys: keys }), | ||
| }); | ||
| } | ||
|
|
||
| it("401s without a session cookie", async () => { | ||
| const request = new NextRequest( | ||
| "https://usage.jays.services/api/owner-expenses", | ||
| { | ||
| method: "DELETE", | ||
| headers: { "content-type": "application/json" }, | ||
| body: JSON.stringify({ idempotencyKeys: [KEY_A] }), | ||
| } | ||
| ); | ||
| const response = await DELETE(request); | ||
| expect(response.status).toBe(401); | ||
| expect(mocks.deleteMany).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("401s with an owner expense token but no session (no token fallback)", async () => { | ||
| process.env.OWNER_EXPENSE_TOKEN = "t".repeat(64); | ||
| const request = new NextRequest( | ||
| "https://usage.jays.services/api/owner-expenses", | ||
| { | ||
| method: "DELETE", | ||
| headers: { | ||
| "content-type": "application/json", | ||
| "x-owner-expense-token": "t".repeat(64), | ||
| }, | ||
| body: JSON.stringify({ idempotencyKeys: [KEY_A] }), | ||
| } | ||
| ); | ||
| const response = await DELETE(request); | ||
| expect(response.status).toBe(401); | ||
| expect(mocks.deleteMany).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("403s a cross-site cookie request (CSRF guard)", async () => { | ||
| const response = await DELETE( | ||
| deleteRequest([KEY_A], { "sec-fetch-site": "cross-site" }) | ||
| ); | ||
| expect(response.status).toBe(403); | ||
| expect(mocks.deleteMany).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("400s on a malformed idempotency key", async () => { | ||
| const response = await DELETE(deleteRequest(["not-a-key"])); | ||
| expect(response.status).toBe(400); | ||
| expect(mocks.deleteMany).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("400s on an empty key list", async () => { | ||
| const response = await DELETE(deleteRequest([])); | ||
| expect(response.status).toBe(400); | ||
| expect(mocks.deleteMany).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("400s on a non-expense key shape", async () => { | ||
| const response = await DELETE(deleteRequest(["usage-telemetry:v1:abc"])); | ||
| expect(response.status).toBe(400); | ||
| expect(mocks.deleteMany).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("deletes scoped to owner-recorded expenses and reports notFound", async () => { | ||
| mocks.findMany.mockResolvedValue([{ idempotencyKey: KEY_A }]); | ||
| mocks.deleteMany.mockResolvedValue({ count: 1 }); | ||
| const response = await DELETE(deleteRequest([KEY_A, KEY_B])); | ||
| expect(response.status).toBe(200); | ||
| const body = await response.json(); | ||
| expect(body).toEqual({ | ||
| requested: 2, | ||
| deleted: 1, | ||
| notFound: [KEY_B], | ||
| }); | ||
| expect(mocks.deleteMany).toHaveBeenCalledTimes(1); | ||
| const where = mocks.deleteMany.mock.calls[0][0].where; | ||
| expect(where.sourceApp).toBe("owner-recorded-expense"); | ||
| expect(where.idempotencyKey).toEqual({ in: [KEY_A, KEY_B] }); | ||
| }); | ||
|
|
||
| it("dedupes repeated keys", async () => { | ||
| mocks.findMany.mockResolvedValue([{ idempotencyKey: KEY_A }]); | ||
| mocks.deleteMany.mockResolvedValue({ count: 1 }); | ||
| const response = await DELETE(deleteRequest([KEY_A, KEY_A])); | ||
| const body = await response.json(); | ||
| expect(body.requested).toBe(1); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,9 @@ | ||
| import { NextRequest, NextResponse } from "next/server"; | ||
| import { hasValidDashboardSession, shouldEnforceDashboardSession } from "@/lib/auth"; | ||
| import { | ||
| hasValidDashboardSession, | ||
| isCsrfSafeRequest, | ||
| shouldEnforceDashboardSession, | ||
| } from "@/lib/auth"; | ||
| import { readBoundedJsonBody } from "@/lib/bounded-request-body"; | ||
| import { | ||
| isUsageReadAuthorized, | ||
|
|
@@ -150,3 +154,86 @@ export async function GET(request: NextRequest) { | |
| hasMore, | ||
| }); | ||
| } | ||
|
|
||
| const DELETE_MAX_KEYS = 100; | ||
| const OWNER_EXPENSE_KEY_RE = /^owner-recorded-expense:v1:[0-9a-f]{64}$/; | ||
|
|
||
| /** | ||
| * DELETE /api/owner-expenses | ||
| * | ||
| * Admin-only removal of owner-recorded expense rows by idempotency key. | ||
| * Dashboard session cookie required — there is intentionally no | ||
| * OWNER_EXPENSE_TOKEN fallback; deleting ledger rows is destructive and | ||
| * stays behind the admin session. The CSRF guard applies because this is | ||
| * a cookie-authenticated mutator. | ||
| * | ||
| * Two independent 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". | ||
| * | ||
| * Body: { "idempotencyKeys": ["owner-recorded-expense:v1:<64hex>", ...] } | ||
| * Response: { requested, deleted, notFound: [...] } | ||
| */ | ||
| export async function DELETE(request: NextRequest) { | ||
| if (!hasValidDashboardSession(request)) { | ||
| return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); | ||
| } | ||
| if (!isCsrfSafeRequest(request)) { | ||
| return NextResponse.json({ error: "Forbidden" }, { status: 403 }); | ||
| } | ||
|
|
||
| let body: unknown; | ||
| try { | ||
| body = await readBoundedJsonBody(request, { | ||
| label: "Owner expense delete body", | ||
| }); | ||
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : "Invalid request"; | ||
| return NextResponse.json({ error: message }, { status: 400 }); | ||
| } | ||
|
|
||
| const rawKeys = | ||
| body && typeof body === "object" | ||
| ? (body as Record<string, unknown>).idempotencyKeys | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Runtime validation gap in Also found in:
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 LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. HTTP-boundary type assertion in Also found in:
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 LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. HTTP request validation gap in Also found in:
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 LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| : undefined; | ||
| if ( | ||
| !Array.isArray(rawKeys) || | ||
| rawKeys.length === 0 || | ||
| rawKeys.length > DELETE_MAX_KEYS || | ||
| rawKeys.some( | ||
| (key) => typeof key !== "string" || !OWNER_EXPENSE_KEY_RE.test(key) | ||
| ) | ||
| ) { | ||
| 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(rawKeys as string[])]; | ||
|
|
||
| const existing = await prisma.externalUsageEvent.findMany({ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Response inconsistency in 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 LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| where: { | ||
| sourceApp: OWNER_EXPENSE_SOURCE_APP, | ||
| idempotencyKey: { in: keys }, | ||
| }, | ||
| select: { idempotencyKey: true }, | ||
| }); | ||
| const existingKeys = new Set(existing.map((row) => row.idempotencyKey)); | ||
|
|
||
| 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)), | ||
| }); | ||
|
Comment on lines
+227
to
+238
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cache invalidation omission in 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 LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Production hostname exposure in
src/app/api/owner-expenses/__tests__/route.test.ts: the test fixture commits the private production infrastructure originhttps://usage.jays.servicesin public source, even though the value only constructs a request object. Use the synthetic non-production originhttps://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-203Kody rule violation: Keep credentials out of public source and verify UI changes with automated screenshots
Prompt for LLM
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.