fix(batches): let session auth cancel any batch, not just apiKeyId-null ones

src/app/api/v1/_helpers/apiKeyScope.ts

@@ -26,3 +26,21 @@ export async function getApiKeyRequestScope(request: Request): Promise<ApiKeyReq
     isSessionAuth,
   };
 }
+
+/**
+ * Canonical per-record ownership check for API-key-scoped resources (batches,
+ * files, etc.): the operator's own dashboard (session auth) may act on ANY
+ * record, matching the sweep-scoping model in deleteCompletedBatches()
+ * (GHSA-wvxc-jp3v-5mg5 -- session auth is the instance-wide operator, an API
+ * key is scoped to its own records). A record with no owner (null/undefined
+ * api_key_id, e.g. created before ownership tracking existed) is visible to
+ * any caller.
+ */
+export function scopeCheck(
+  scope: Pick<ApiKeyRequestScope, "isSessionAuth" | "apiKeyId">,
+  recordApiKeyId: string | null | undefined
+): boolean {
+  if (scope.isSessionAuth) return true;
+  if (recordApiKeyId === null || recordApiKeyId === undefined) return true;
+  return recordApiKeyId === scope.apiKeyId;
+}

src/app/api/v1/batches/[id]/cancel/route.ts

@@ -1,7 +1,7 @@
 import { CORS_HEADERS, handleCorsOptions } from "@/shared/utils/cors";
 import { getBatch, updateBatch } from "@/lib/db/batches";
 import { NextResponse } from "next/server";
-import { getApiKeyRequestScope } from "@/app/api/v1/_helpers/apiKeyScope";
+import { getApiKeyRequestScope, scopeCheck } from "@/app/api/v1/_helpers/apiKeyScope";
 import { formatBatchResponse } from "../../formatBatchResponse";
 
 export async function OPTIONS() {
@@ -11,12 +11,11 @@ export async function OPTIONS() {
 export async function POST(request: Request, { params }: { params: Promise<{ id: string }> }) {
   const scope = await getApiKeyRequestScope(request);
   if (scope.rejection) return scope.rejection;
-  const apiKeyId = scope.apiKeyId;
 
   const { id } = await params;
   const batch = getBatch(id);
 
-  if (!batch || (batch.apiKeyId !== null && batch.apiKeyId !== apiKeyId)) {
+  if (!batch || !scopeCheck(scope, batch.apiKeyId)) {
     return NextResponse.json(
       { error: { message: "Batch not found", type: "invalid_request_error" } },
       { status: 404, headers: CORS_HEADERS }

src/app/api/v1/batches/[id]/route.ts

@@ -1,22 +1,13 @@
 import { CORS_HEADERS, handleCorsOptions } from "@/shared/utils/cors";
 import { getBatch, deleteBatch } from "@/lib/db/batches";
 import { NextResponse } from "next/server";
-import { getApiKeyRequestScope } from "@/app/api/v1/_helpers/apiKeyScope";
+import { getApiKeyRequestScope, scopeCheck } from "@/app/api/v1/_helpers/apiKeyScope";
 import { formatBatchResponse } from "../formatBatchResponse";
 
 export async function OPTIONS() {
   return handleCorsOptions();
 }
 
-function scopeCheck(
-  scope: { isSessionAuth: boolean; apiKeyId: string | null },
-  recordApiKeyId: string | null | undefined
-): boolean {
-  if (scope.isSessionAuth) return true;
-  if (recordApiKeyId === null || recordApiKeyId === undefined) return true;
-  return recordApiKeyId === scope.apiKeyId;
-}
-
 export async function GET(request: Request, { params }: { params: Promise<{ id: string }> }) {
   const scope = await getApiKeyRequestScope(request);
   if (scope.rejection) return scope.rejection;

tests/unit/batch-cancel-session-auth-scope.test.ts

@@ -0,0 +1,109 @@
+/**
+ * `POST /api/v1/batches/[id]/cancel` rejected the dashboard's own
+ * session-authenticated caller as "Batch not found" (404) for any batch
+ * owned by a non-null api_key_id -- which in practice is every batch created
+ * through the default `env-key`, i.e. every real batch on the instance.
+ * Cancelling from the dashboard silently did nothing.
+ *
+ * Root cause: the route carried its own inline ownership check
+ * (`batch.apiKeyId !== null && batch.apiKeyId !== apiKeyId`) instead of the
+ * canonical `scopeCheck()` that two sibling implementations already get
+ * right: `batches/[id]/route.ts` (GET/DELETE) and `deleteCompletedBatches()`
+ * (GHSA-wvxc-jp3v-5mg5) both treat session auth as the instance-wide
+ * operator, able to act on any record regardless of which API key owns it.
+ * The inline check never granted that exemption, so a session-authenticated
+ * caller (`apiKeyId === null`) was treated as a mismatched key the instant
+ * `batch.apiKeyId` was non-null.
+ *
+ * This test proves the fix at the ownership-decision boundary (`scopeCheck`,
+ * now shared via `_helpers/apiKeyScope.ts`) against a batch shaped exactly
+ * like the two that were actually stuck in production (`api_key_id: "env-key"`),
+ * and proves the route source no longer contains the buggy inline check.
+ *
+ * Run with:
+ *   node --import tsx/esm --test tests/unit/batch-cancel-session-auth-scope.test.ts
+ */
+
+import { describe, it } from "node:test";
+import assert from "node:assert/strict";
+import { createFile } from "@/lib/db/files";
+import { createBatch } from "@/lib/db/batches";
+import { scopeCheck } from "@/app/api/v1/_helpers/apiKeyScope";
+
+function seedBatch(apiKeyId: string | null, status: "validating" | "in_progress", tag: string) {
+  const file = createFile({
+    bytes: 10,
+    filename: `cancel-scope-${tag}.jsonl`,
+    purpose: "batch",
+    content: Buffer.from("{}"),
+  });
+  return createBatch({
+    endpoint: "/v1/chat/completions",
+    completionWindow: "24h",
+    inputFileId: file.id,
+    status,
+    apiKeyId,
+  });
+}
+
+describe("cancel route ownership scoping", () => {
+  it("session auth (dashboard) may cancel a batch owned by an API key", () => {
+    const batch = seedBatch("env-key", "in_progress", "a1");
+
+    // Exactly the check cancel/route.ts now runs: `!scopeCheck(scope, batch.apiKeyId)`
+    const allowed = scopeCheck({ isSessionAuth: true, apiKeyId: null }, batch.apiKeyId);
+
+    assert.equal(allowed, true, "the operator's dashboard must be able to cancel any batch");
+  });
+
+  it("an unrelated API key may not cancel someone else's batch", () => {
+    const batch = seedBatch("env-key", "validating", "a2");
+
+    const allowed = scopeCheck({ isSessionAuth: false, apiKeyId: "other-key" }, batch.apiKeyId);
+
+    assert.equal(allowed, false, "a foreign API key must not be able to cancel this batch");
+  });
+
+  it("the owning API key may cancel its own batch", () => {
+    const batch = seedBatch("key-owns-this", "validating", "a3");
+
+    const allowed = scopeCheck({ isSessionAuth: false, apiKeyId: "key-owns-this" }, batch.apiKeyId);
+
+    assert.equal(allowed, true, "the owning API key must be able to cancel its own batch");
+  });
+
+  it("the original buggy inline check would have rejected the session-auth caller", () => {
+    const batch = seedBatch("env-key", "in_progress", "a4");
+
+    // This is the exact predicate cancel/route.ts used to run before the fix.
+    const apiKeyId: string | null = null; // session auth
+    const rejectedByOldCheck = !batch || (batch.apiKeyId !== null && batch.apiKeyId !== apiKeyId);
+
+    assert.equal(
+      rejectedByOldCheck,
+      true,
+      "documents the regression: the old inline check 404'd every dashboard cancel"
+    );
+  });
+});
+
+describe("the route uses the shared scopeCheck instead of its old inline predicate", () => {
+  it("cancel/route.ts no longer carries the buggy apiKeyId !== null inline check", async () => {
+    const { readFileSync } = await import("node:fs");
+    const { fileURLToPath } = await import("node:url");
+    const src = readFileSync(
+      fileURLToPath(
+        new URL("../../src/app/api/v1/batches/[id]/cancel/route.ts", import.meta.url)
+      ),
+      "utf8"
+    );
+    assert.ok(
+      !/batch\.apiKeyId\s*!==\s*null\s*&&\s*batch\.apiKeyId\s*!==\s*apiKeyId/.test(src),
+      "the route still carries the old inline ownership check that 404s session auth"
+    );
+    assert.ok(
+      /scopeCheck\(\s*scope\s*,\s*batch\.apiKeyId\s*\)/.test(src),
+      "the route must delegate ownership to the shared scopeCheck helper"
+    );
+  });
+});