fix(api): enforce job ownership on cancel endpoint (#599)

The job cancel endpoint authenticated the caller but never checked that the job belonged to them, so any authenticated user could cancel another user's job by ID. Load the job's owner and allow cancellation only for the owner or a caller with files:all; return 404 for missing and non-owned jobs alike. Extract the route into a shared registerJobRoutes() so the ownership check is covered by tests.

Reported by Alpesh Bhagwatkar.

apps/api/src/index.ts

@@ -67,6 +67,7 @@ import { feedbackRoutes } from "./routes/feedback.js";
 import { registerFetchUrlsRoute } from "./routes/fetch-urls.js";
 import { filePreviewRoutes } from "./routes/file-preview.js";
 import { fileRoutes } from "./routes/files.js";
+import { registerJobRoutes } from "./routes/jobs.js";
 import { registerMemeTemplates } from "./routes/meme-templates.js";
 import { registerPipelineRoutes } from "./routes/pipeline.js";
 import { preferencesRoutes } from "./routes/preferences.js";
@@ -706,24 +707,8 @@ app.get("/api/v1/readyz", async (_request, reply) => {
   return reply.code(ok ? 200 : 503).send({ ok, postgres, redis, disk: diskOk, s3: s3Ok });
 });
 
-// Cancel a job (authenticated)
+// Cancel a job (authenticated; owner or files:all only)
-app.post(
+registerJobRoutes(app);
-  "/api/v1/jobs/:jobId/cancel",
-  { config: { rateLimit: { max: 300, timeWindow: "1 minute" } } },
-  async (
-    request: import("fastify").FastifyRequest<{ Params: { jobId: string } }>,
-    reply: import("fastify").FastifyReply,
-  ) => {
-    const { requireAuth } = await import("./plugins/auth.js");
-    const user = requireAuth(request, reply);
-    if (!user) return;
-
-    const { requestCancel } = await import("./jobs/cancel.js");
-    const { jobId } = request.params;
-    const canceled = await requestCancel(jobId);
-    return reply.send({ canceled });
-  },
-);
 
 // Serve SPA in production
 if (process.env.NODE_ENV === "production") {

apps/api/src/routes/jobs.ts

@@ -0,0 +1,46 @@
+/**
+ * HTTP control routes for jobs.
+ *
+ * POST /api/v1/jobs/:jobId/cancel
+ *
+ * Cancellation is authorized at this boundary. Authentication alone is not
+ * enough: a caller may cancel a job only if they own it (jobs.user_id ===
+ * their id) or hold `files:all` (admins and editors, the same "act across all
+ * users" permission that governs the file library). Without this check any
+ * authenticated user could cancel any other user's job by guessing its id.
+ *
+ * Missing and non-owned jobs both return 404 so a caller can't use the response
+ * to learn which job ids exist. This mirrors the ownership handling in the
+ * user-files routes.
+ */
+import { eq } from "drizzle-orm";
+import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify";
+import { db, schema } from "../db/index.js";
+import { requestCancel } from "../jobs/cancel.js";
+import { hasEffectivePermission } from "../permissions.js";
+import { requireAuth } from "../plugins/auth.js";
+
+export function registerJobRoutes(app: FastifyInstance): void {
+  app.post(
+    "/api/v1/jobs/:jobId/cancel",
+    { config: { rateLimit: { max: 300, timeWindow: "1 minute" } } },
+    async (request: FastifyRequest<{ Params: { jobId: string } }>, reply: FastifyReply) => {
+      const user = requireAuth(request, reply);
+      if (!user) return;
+
+      const { jobId } = request.params;
+
+      const [job] = await db
+        .select({ userId: schema.jobs.userId })
+        .from(schema.jobs)
+        .where(eq(schema.jobs.id, jobId));
+
+      if (!job || (job.userId !== user.id && !(await hasEffectivePermission(user, "files:all")))) {
+        return reply.status(404).send({ error: "Job not found" });
+      }
+
+      const canceled = await requestCancel(jobId);
+      return reply.send({ canceled });
+    },
+  );
+}

tests/integration/platform/progress-transport.test.ts

@@ -16,11 +16,12 @@ import { eq } from "drizzle-orm";
 import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
 import { db, schema } from "../../../apps/api/src/db/index.js";
 import { sharedRedis } from "../../../apps/api/src/jobs/connection.js";
+import { getQueue } from "../../../apps/api/src/jobs/queues.js";
 import {
   publishEphemeral,
   updateSingleFileProgress,
 } from "../../../apps/api/src/routes/progress.js";
-import { buildTestApp, loginAsAdmin, type TestApp } from "../test-server.js";
+import { buildTestApp, createUserAndLogin, loginAsAdmin, type TestApp } from "../test-server.js";
 
 let testApp: TestApp;
 let app: TestApp["app"];
@@ -251,9 +252,47 @@ describe("Redis progress transport", () => {
   });
 });
 
-// ── Cancel route auth ──────────────────────────────────────────
+// ── Cancel route auth + ownership ──────────────────────────────
+//
+// Regression guard for the IDOR where any authenticated user could cancel any
+// other user's job by ID. A caller may cancel a job only if they own it
+// (jobs.user_id === their id) or hold `files:all` (admins/editors). Missing and
+// non-owned jobs both return 404 so ownership can't be probed by job ID.
 
 describe("Cancel route auth", () => {
+  let ownerToken: string;
+  let ownerId: string;
+  let attackerToken: string;
+
+  beforeAll(async () => {
+    const owner = await createUserAndLogin(app, "cancel-owner");
+    const attacker = await createUserAndLogin(app, "cancel-attacker");
+    ownerToken = owner.token;
+    ownerId = owner.userId;
+    attackerToken = attacker.token;
+  }, 30_000);
+
+  /** Seed a durable job row owned by `ownerId`, plus a long-delayed BullMQ job
+   *  (workers never promote a delayed job within the test window) so we can
+   *  observe whether a cancel actually reached the queue. */
+  async function seedOwnedJob(): Promise<string> {
+    const jobId = `cancel-${randomUUID()}`;
+    await db.insert(schema.jobs).values({
+      id: jobId,
+      userId: ownerId,
+      type: "single",
+      status: "queued",
+    });
+    await getQueue("image").add("noop", {} as never, { jobId, delay: 600_000 });
+    return jobId;
+  }
+
+  async function cleanupJob(jobId: string): Promise<void> {
+    const job = await getQueue("image").getJob(jobId);
+    if (job) await job.remove().catch(() => {});
+    await db.delete(schema.jobs).where(eq(schema.jobs.id, jobId));
+  }
+
   it("rejects unauthenticated cancel with 401", async () => {
     const jobId = randomUUID();
 
@@ -268,7 +307,7 @@ describe("Cancel route auth", () => {
     expect(body.error).toContain("Authentication required");
   });
 
-  it("returns canceled:false for an unknown job when authenticated", async () => {
+  it("returns 404 for an unknown job (existence is not probeable)", async () => {
     const jobId = randomUUID();
 
     const res = await app.inject({
@@ -279,8 +318,67 @@ describe("Cancel route auth", () => {
       },
     });
 
+    expect(res.statusCode).toBe(404);
+  });
+
+  it("blocks a non-owner from canceling another user's job (404, job untouched)", async () => {
+    const jobId = await seedOwnedJob();
+    try {
+      const res = await app.inject({
+        method: "POST",
+        url: `/api/v1/jobs/${jobId}/cancel`,
+        headers: { authorization: `Bearer ${attackerToken}` },
+      });
+
+      // The attacker gets the same 404 as a nonexistent job...
+      expect(res.statusCode).toBe(404);
+
+      // ...and the victim's job is entirely untouched: the DB row stays queued
+      // and the queued BullMQ job survives.
+      const [row] = await db.select().from(schema.jobs).where(eq(schema.jobs.id, jobId));
+      expect(row?.status).toBe("queued");
+      expect(await getQueue("image").getJob(jobId)).toBeTruthy();
+    } finally {
+      await cleanupJob(jobId);
+    }
+  });
+
+  it("lets the owner cancel their own job", async () => {
+    const jobId = await seedOwnedJob();
+    try {
+      const res = await app.inject({
+        method: "POST",
+        url: `/api/v1/jobs/${jobId}/cancel`,
+        headers: { authorization: `Bearer ${ownerToken}` },
+      });
+
       expect(res.statusCode).toBe(200);
-    const body = JSON.parse(res.body);
+      expect(JSON.parse(res.body).canceled).toBe(true);
-    expect(body.canceled).toBe(false);
+
+      // The cancel actually took effect: row marked canceled, queue job removed.
+      const [row] = await db.select().from(schema.jobs).where(eq(schema.jobs.id, jobId));
+      expect(row?.status).toBe("canceled");
+      expect(await getQueue("image").getJob(jobId)).toBeFalsy();
+    } finally {
+      await cleanupJob(jobId);
+    }
+  });
+
+  it("lets an admin (files:all) cancel another user's job", async () => {
+    const jobId = await seedOwnedJob();
+    try {
+      const res = await app.inject({
+        method: "POST",
+        url: `/api/v1/jobs/${jobId}/cancel`,
+        headers: { authorization: `Bearer ${adminToken}` },
+      });
+
+      expect(res.statusCode).toBe(200);
+      expect(JSON.parse(res.body).canceled).toBe(true);
+      const [row] = await db.select().from(schema.jobs).where(eq(schema.jobs.id, jobId));
+      expect(row?.status).toBe("canceled");
+    } finally {
+      await cleanupJob(jobId);
+    }
   });
 });

tests/integration/test-server.ts

@@ -30,11 +30,7 @@ import { afterAll } from "vitest";
 import { env } from "../../apps/api/src/config.js";
 import { db, schema } from "../../apps/api/src/db/index.js";
 import { runMigrations } from "../../apps/api/src/db/migrate.js";
-import {
+import { startCancelListener, stopCancelListener } from "../../apps/api/src/jobs/cancel.js";
-  requestCancel,
-  startCancelListener,
-  stopCancelListener,
-} from "../../apps/api/src/jobs/cancel.js";
 import { pingRedis } from "../../apps/api/src/jobs/connection.js";
 import { closeQueueEvents, warmQueueEvents } from "../../apps/api/src/jobs/enqueue.js";
 import { closeWorkers, startWorkers } from "../../apps/api/src/jobs/worker.js";
@@ -45,7 +41,6 @@ import {
   ensureBuiltinRoles,
   ensureDefaultAdmin,
   ensureDefaultTeam,
-  requireAuth,
 } from "../../apps/api/src/plugins/auth.js";
 import { registerIpAllowlist } from "../../apps/api/src/plugins/ip-allowlist.js";
 import { registerMfa } from "../../apps/api/src/plugins/mfa.js";
@@ -63,6 +58,7 @@ import { registerEnterpriseRoutes } from "../../apps/api/src/routes/enterprise/i
 import { feedbackRoutes } from "../../apps/api/src/routes/feedback.js";
 import { registerFetchUrlsRoute } from "../../apps/api/src/routes/fetch-urls.js";
 import { fileRoutes } from "../../apps/api/src/routes/files.js";
+import { registerJobRoutes } from "../../apps/api/src/routes/jobs.js";
 import { registerMemeTemplates } from "../../apps/api/src/routes/meme-templates.js";
 import { registerPipelineRoutes } from "../../apps/api/src/routes/pipeline.js";
 import { preferencesRoutes } from "../../apps/api/src/routes/preferences.js";
@@ -210,6 +206,10 @@ export async function buildTestApp(): Promise<TestApp> {
   // Progress SSE routes
   await registerProgressRoutes(app);
 
+  // Job control routes (cancel) -- shares the production handler so the
+  // ownership check is exercised by integration tests, not a divergent copy.
+  registerJobRoutes(app);
+
   // API key management routes
   await apiKeyRoutes(app);
 
@@ -306,22 +306,6 @@ export async function buildTestApp(): Promise<TestApp> {
     return reply.code(ok ? 200 : 503).send({ ok, postgres, redis });
   });
 
-  // Cancel a job (authenticated)
-  app.post(
-    "/api/v1/jobs/:jobId/cancel",
-    async (
-      request: import("fastify").FastifyRequest<{ Params: { jobId: string } }>,
-      reply: import("fastify").FastifyReply,
-    ) => {
-      const user = requireAuth(request, reply);
-      if (!user) return;
-
-      const { jobId } = request.params;
-      const canceled = await requestCancel(jobId);
-      return reply.send({ canceled });
-    },
-  );
-
   // Run pre-ready hooks (test files register extra routes here)
   for (const hook of preReadyHooks) {
     await hook(app);
@@ -364,15 +348,21 @@ export async function loginAsAdmin(app: ReturnType<typeof Fastify>): Promise<str
 }
 
 /**
- * Create (idempotently) a non-admin `user`-role account and return its session
+ * Create (idempotently) a non-admin account with the given role and return its
- * token. The user is created through the real admin-gated register route
+ * session token together with its user id. The account is created through the
- * (`POST /api/auth/register`, permission `users:manage`), so the helper proves
+ * real admin-gated register route (`POST /api/auth/register`, permission
- * the route shape rather than poking the DB directly. The register route sets
+ * `users:manage`), so the helper proves the route shape rather than poking the
- * `mustChangePassword: true`, which the auth middleware would otherwise turn
+ * DB directly. The register route sets `mustChangePassword: true`, which the
- * into a 403 on every non-auth API call (SKIP_MUST_CHANGE_PASSWORD defaults to
+ * auth middleware would otherwise turn into a 403 on every non-auth API call
- * false in tests), so we clear that flag the same way the admin seed does.
+ * (SKIP_MUST_CHANGE_PASSWORD defaults to false in tests), so we clear that flag
+ * the same way the admin seed does.
  */
-export async function loginAsUser(app: ReturnType<typeof Fastify>): Promise<string> {
+export async function createUserAndLogin(
+  app: ReturnType<typeof Fastify>,
+  username: string,
+  role = "user",
+  password = "Userpass1",
+): Promise<{ token: string; userId: string }> {
   const adminToken = await loginAsAdmin(app);
 
   // Create the user. A 409 means a prior call already created it -- tolerate it.
@@ -380,7 +370,7 @@ export async function loginAsUser(app: ReturnType<typeof Fastify>): Promise<stri
     method: "POST",
     url: "/api/auth/register",
     headers: { authorization: `Bearer ${adminToken}` },
-    payload: { username: "plainuser", password: "Userpass1", role: "user" },
+    payload: { username, password, role },
   });
   if (create.statusCode !== 201 && create.statusCode !== 409) {
     throw new Error(`User create failed (${create.statusCode}): ${create.body}`);
@@ -391,18 +381,35 @@ export async function loginAsUser(app: ReturnType<typeof Fastify>): Promise<stri
   await db
     .update(schema.users)
     .set({ mustChangePassword: false })
-    .where(eq(schema.users.username, "plainuser"));
+    .where(eq(schema.users.username, username));
+
+  const [row] = await db
+    .select({ id: schema.users.id })
+    .from(schema.users)
+    .where(eq(schema.users.username, username));
+  if (!row) {
+    throw new Error(`User ${username} not found after create`);
+  }
 
   const res = await app.inject({
     method: "POST",
     url: "/api/auth/login",
-    payload: { username: "plainuser", password: "Userpass1" },
+    payload: { username, password },
   });
   const body = JSON.parse(res.body);
   if (!body.token) {
     throw new Error(`User login failed: ${res.body}`);
   }
-  return body.token as string;
+  return { token: body.token as string, userId: row.id };
+}
+
+/**
+ * Log in as a default non-admin `user`-role account, returning its session
+ * token. Thin wrapper over {@link createUserAndLogin} kept for existing callers.
+ */
+export async function loginAsUser(app: ReturnType<typeof Fastify>): Promise<string> {
+  const { token } = await createUserAndLogin(app, "plainuser");
+  return token;
 }
 
 /**