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.
This commit is contained in:
SnapOtter
2026-07-21 15:09:00 +08:00
committed by GitHub
parent a6bce6825a
commit 577d74bdb1
4 changed files with 194 additions and 58 deletions
@@ -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(200);
const body = JSON.parse(res.body);
expect(body.canceled).toBe(false);
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);
expect(JSON.parse(res.body).canceled).toBe(true);
// 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);
}
});
});