fix(api): gate every tool endpoint and stop ZIP streams failing quietly (#646)

Three defects from #645, all of which let the server report something
that was not true.

Tool access was enforced per route, so it drifted. createToolRoute calls
requireToolAccess and the factory tools were fine, but all 45
hand-written routes had to remember the same call and none of them did.
A role without tools:use could run image-to-pdf, svg-to-raster,
erase-object, favicon, qr-generate, upscale, sign-pdf and the rest. The
issue described this as affecting two routes; it was every one of them.

The check now lives in a single preHandler keyed off the tool the router
matched, so it covers sub-paths (/batch, /info, /preview, /analyze,
/inspect) and any route added later without that route opting in. Ids no
tool claims stay unresolved, which keeps an unknown or misfiled tool a
404 rather than telling an unauthorized caller which ids exist.

Resolution reads request.routeOptions.url, the pattern the app itself
registered, rather than parsing request.url a second time. find-my-way
decodes before matching, so an independent parse disagrees with the
router and the router wins: `/api/v1/tools/image/%66avicon` ran favicon
while the gate saw no tool at all. Absolute-form request targets slipped
it the same way. Taking the router's own answer removes the disagreement.

A ZIP stream that failed after the 200 headers were out called
reply.raw.end(). On a chunked response that is indistinguishable from
success, so a client kept an archive with no central directory believing
it whole. Worse, a source stream that errored had no listener: the
request hung until it timed out and the error surfaced as unhandled. A
poisoned-storage probe reproduced both. The socket is destroyed instead,
and every source stream is listened to. svg-to-raster additionally ran
its append loop past the hijack with no try/catch, where a throw leaves
Fastify logging and walking away with the socket neither ended nor
destroyed. pdf-to-image is fixed alongside the other two: it shipped in
#643 with the destroy half but not the listener, so it hung the same way.

A zero-byte upload was dropped during parsing. The client pairs results
with its own file list by index, so every later result shifted onto the
wrong file: one document's output was presented as another's, under
another's name, while the file that actually converted was marked "not
found in batch results". Empty parts now keep their slot and fail in
place with a reason.

Two tests in adversarial-extended.test.ts asserted the old zero-byte
behavior, including a comment that batch "silently skips zero-byte
parts". They now pin the replacement: still rejected, nothing processed,
but the caller is told which files were empty instead of being told it
sent none.

A guard walks the whole catalog and fails if any of the 241 tools answers
anything but 403 for a role without tools:use, so a tool cannot escape
the gate by being registered in a shape nobody thought to sample.

Fixes #645

apps/api/src/index.ts

@@ -53,6 +53,7 @@ import { registerMfa } from "./plugins/mfa.js";
 import { oidcRoutes } from "./plugins/oidc.js";
 import { registerSaml } from "./plugins/saml.js";
 import { registerStatic } from "./plugins/static.js";
+import { toolAccessMiddleware } from "./plugins/tool-access.js";
 import { registerUpload } from "./plugins/upload.js";
 import { adminOpsRoutes } from "./routes/admin-ops.js";
 import { analyticsRoutes } from "./routes/analytics.js";
@@ -456,6 +457,11 @@ await preferencesRoutes(app);
 // Auth middleware (must be registered before routes it protects)
 await authMiddleware(app);
 
+// Tool access gate (after auth so request.user is populated). Covers every
+// per-tool endpoint including the hand-written routes, which each used to
+// have to remember the check themselves.
+await toolAccessMiddleware(app);
+
 // Per-user rate limiting (after auth so request.user is populated)
 await registerPerUserRateLimit(app);
 

apps/api/src/plugins/tool-access.ts

@@ -0,0 +1,63 @@
+import { TOOLS, toolSection } from "@snapotter/shared";
+import type { FastifyInstance } from "fastify";
+import { requireToolAccess } from "../permissions.js";
+
+const TOOLS_PREFIX = "/api/v1/tools/";
+
+/** toolId -> section, so a route resolves without scanning the catalog. */
+const SECTION_BY_TOOL_ID = new Map(TOOLS.map((tool) => [tool.id, toolSection(tool)]));
+
+/**
+ * The tool a request targets, read from the route the router actually matched
+ * rather than from the raw URL.
+ *
+ * This distinction is the whole point. find-my-way percent-decodes before
+ * matching, so `/api/v1/tools/image/%66avicon` runs the favicon handler while
+ * a second, independent parse of the raw string sees no tool at all and waves
+ * it through. Taking the router's own answer means the gate and the router
+ * cannot disagree. `routeUrl` is the registered pattern, so it is already
+ * canonical for static routes; the one parametric tool route
+ * (`:section/:toolId/batch`) fills in from params, which Fastify has likewise
+ * already decoded.
+ *
+ * Null means "not a per-tool endpoint": the catalog listings, and ids no tool
+ * claims. Leaving those alone keeps an unknown or misfiled tool a 404 rather
+ * than telling an unauthorized caller which ids exist.
+ */
+export function toolIdFromRoute(
+  routeUrl: string | undefined,
+  params?: { section?: string; toolId?: string },
+): string | null {
+  if (!routeUrl?.startsWith(TOOLS_PREFIX)) return null;
+  const [patternSection, patternToolId] = routeUrl.slice(TOOLS_PREFIX.length).split("/");
+  if (!patternSection || !patternToolId) return null;
+  const section = patternSection === ":section" ? params?.section : patternSection;
+  const toolId = patternToolId === ":toolId" ? params?.toolId : patternToolId;
+  if (!section || !toolId) return null;
+  return SECTION_BY_TOOL_ID.get(toolId) === section ? toolId : null;
+}
+
+/**
+ * Enforce tool access for every per-tool endpoint from one place.
+ *
+ * createToolRoute gates the tools it builds, but the 45 hand-written routes
+ * each had to remember the same call and none of them did, so a role without
+ * `tools:use` could run image-to-pdf, svg-to-raster, erase-object, favicon and
+ * the rest (issue #645). Per-route calls are exactly what drifted, so the
+ * check keys off the matched route and therefore covers sub-paths (/batch,
+ * /info, /preview, /analyze, /inspect) and any route added later without it
+ * having to opt in.
+ *
+ * Must be registered after the auth middleware, which populates request.user.
+ * The remaining per-route calls stay as defense in depth.
+ */
+export async function toolAccessMiddleware(app: FastifyInstance): Promise<void> {
+  app.addHook("preHandler", async (request, reply) => {
+    const toolId = toolIdFromRoute(
+      request.routeOptions?.url,
+      request.params as { section?: string; toolId?: string } | undefined,
+    );
+    if (!toolId) return;
+    if (!(await requireToolAccess(request, reply, toolId))) return reply;
+  });
+}

apps/api/src/routes/batch.ts

@@ -138,7 +138,11 @@ export async function registerBatchRoutes(app: FastifyInstance): Promise<void> {
                       413,
                     );
                   }
-                  if (file.size > 0) files.push({ kind: "path", ...file });
+                  // Empty parts keep their slot: the client maps results back
+                  // onto its own file list by index, so dropping one here
+                  // would label a converted file with a different file's name
+                  // (issue #645). They fail in place in the loop below.
+                  files.push({ kind: "path", ...file });
                 } else {
                   const chunks: Buffer[] = [];
                   for await (const chunk of part.file) {
@@ -154,7 +158,6 @@ export async function registerBatchRoutes(app: FastifyInstance): Promise<void> {
                     chunks.push(chunk);
                   }
                   const buffer = Buffer.concat(chunks);
-                  if (buffer.length > 0) {
                   files.push({
                     kind: "buffer",
                     buffer,
@@ -162,7 +165,6 @@ export async function registerBatchRoutes(app: FastifyInstance): Promise<void> {
                     size: buffer.length,
                   });
                 }
-                }
                 filePartIndex++;
               } else if (part.fieldname === "settings") {
                 settingsRaw = part.value as string;
@@ -285,6 +287,18 @@ export async function registerBatchRoutes(app: FastifyInstance): Promise<void> {
             const childId = `${parentId}-f${flowChildIndex}`;
             let key = `uploads/${childId}/${processFilename}`;
 
+            // Reported against the slot it arrived in, so every later result
+            // stays paired with the file it came from.
+            if (file.size === 0) {
+              preFailures.push({
+                originalIndex: i,
+                filename: file.filename,
+                error: "File is empty",
+              });
+              if (file.kind === "path") await rm(file.path, { force: true }).catch(() => {});
+              continue;
+            }
+
             if (file.kind === "path") {
               try {
                 await storeValidatedOcrPdf(file, key, {
@@ -573,12 +587,20 @@ export async function registerBatchRoutes(app: FastifyInstance): Promise<void> {
 
           const archive = archiver("zip", { zlib: { level: 5 } });
 
-          archive.on("error", (err) => {
-            request.log.error({ err }, "Archiver error during batch processing");
-            if (!reply.raw.writableEnded) {
-              reply.raw.end();
-            }
-          });
+          // The 200 headers are already out, so nothing here can change the
+          // status. Destroy the socket rather than end() it: a clean end on a
+          // chunked response is indistinguishable from success, so the client
+          // would keep a ZIP with no central directory believing it complete.
+          let streamFailed = false;
+          const failStream = (err: Error, msg: string) => {
+            if (streamFailed) return;
+            streamFailed = true;
+            request.log.error({ err, jobId: parentId }, msg);
+            archive.abort();
+            reply.raw.destroy(err);
+          };
+
+          archive.on("error", (err) => failStream(err, "Archiver error during batch processing"));
 
           archive.pipe(reply.raw);
 
@@ -587,16 +609,22 @@ export async function registerBatchRoutes(app: FastifyInstance): Promise<void> {
             for (const entry of successEntries) {
               if (!entry.outputRef) continue;
               const stream = await getObjectStream(entry.outputRef);
+              // A backend that resolves the stream and only then fails (local
+              // createReadStream emitting ENOENT) never rejects here, so the
+              // catch below cannot see it. Without this listener that error is
+              // unhandled and the request hangs instead of terminating.
+              stream.on("error", (err: Error) =>
+                failStream(err, "Object stream error during batch processing"),
+              );
               archive.append(stream, { name: entry.filename });
             }
 
             await archive.finalize();
           } catch (err) {
-            request.log.error({ err }, "Failed to stream ZIP entries during batch processing");
-            archive.abort();
-            if (!reply.raw.writableEnded) {
-              reply.raw.end();
-            }
+            failStream(
+              err instanceof Error ? err : new Error(String(err)),
+              "Failed to stream ZIP entries during batch processing",
+            );
           }
         } finally {
           request.raw.removeListener("aborted", abortIngress);

apps/api/src/routes/tools/pdf-to-image.ts

@@ -591,7 +591,10 @@ export function registerPdfToImageRoute(
       // Destroy the socket rather than end() it: a clean end on a chunked
       // response is indistinguishable from success, and the client would keep
       // a truncated ZIP believing it complete.
+      let streamFailed = false;
       const failStream = (err: Error, msg: string) => {
+        if (streamFailed) return;
+        streamFailed = true;
         request.log.error({ err, jobId }, msg);
         archive.abort();
         reply.raw.destroy(err);
@@ -603,7 +606,14 @@ export function registerPdfToImageRoute(
       try {
         for (const entry of results) {
           if (!entry) continue;
-          archive.append(await getObjectStream(entry.key), { name: entry.filename });
+          const stream = await getObjectStream(entry.key);
+          // The local backend resolves the stream and only then emits ENOENT,
+          // so the catch below never sees it. Without this listener the error
+          // is unhandled and the request hangs instead of terminating.
+          stream.on("error", (err: Error) =>
+            failStream(err, "Object stream error during PDF batch processing"),
+          );
+          archive.append(stream, { name: entry.filename });
         }
         await archive.finalize();
       } catch (err) {

apps/api/src/routes/tools/svg-to-raster.ts

@@ -130,13 +130,14 @@ export function registerSvgToRasterRoute(
           for await (const chunk of part.file) {
             chunks.push(chunk);
           }
-          const buf = Buffer.concat(chunks);
-          if (buf.length > 0) {
+          // Empty parts keep their slot: the client maps results back onto its
+          // own file list by index, so dropping one would label a converted
+          // file with a different file's name (issue #645). The per-file loop
+          // below fails it in place.
           files.push({
-              buffer: buf,
+            buffer: Buffer.concat(chunks),
             filename: sanitizeFilename(part.filename ?? "output"),
           });
-          }
         } else if (part.fieldname === "settings") {
           settingsRaw = part.value as string;
         } else if (part.fieldname === "clientJobId") {
@@ -165,7 +166,13 @@ export function registerSvgToRasterRoute(
 
     if (opts.accept) {
       const accept = opts.accept;
-      const invalid = files.find((file) => !matchesAccept(file.filename, accept));
+      // Empty parts are exempt: they now keep their slot rather than being
+      // dropped (issue #645), and a nameless one would otherwise fail this
+      // whole-request check and take every valid file down with it. They fail
+      // on their own below, where the reason reaches the file it belongs to.
+      const invalid = files.find(
+        (file) => file.buffer.length > 0 && !matchesAccept(file.filename, accept),
+      );
       if (invalid) {
         return reply.status(400).send({
           error: "File is not a valid SVG. This tool only accepts SVG files.",
@@ -337,22 +344,41 @@ export function registerSvgToRasterRoute(
 
     const archive = archiver("zip", { zlib: { level: 5 } });
 
-    archive.on("error", (err) => {
-      request.log.error({ err }, "Archiver error during SVG batch processing");
-      if (!reply.raw.writableEnded) {
-        reply.raw.end();
-      }
-    });
+    // Headers are already out, so this cannot change the status. Destroying
+    // the socket is the only way to tell the client the archive is incomplete:
+    // ending it cleanly reads as success and hands back a ZIP with no central
+    // directory. Entries here are in-memory buffers, so unlike the other batch
+    // routes there is no storage read to fail, but an archiver fault still has
+    // to reach the client.
+    let streamFailed = false;
+    const failStream = (err: Error, msg: string) => {
+      if (streamFailed) return;
+      streamFailed = true;
+      request.log.error({ err, jobId }, msg);
+      archive.abort();
+      reply.raw.destroy(err);
+    };
+
+    archive.on("error", (err) => failStream(err, "Archiver error during SVG batch processing"));
 
     archive.pipe(reply.raw);
 
+    // A throw past this point cannot be answered: the reply is hijacked, so
+    // Fastify logs and walks away, leaving the socket neither ended nor
+    // destroyed and the client waiting forever.
+    try {
       for (const result of results) {
         if (result) {
           archive.append(result.buffer, { name: result.filename });
         }
       }
-
       await archive.finalize();
+    } catch (err) {
+      failStream(
+        err instanceof Error ? err : new Error(String(err)),
+        "Failed to finalize ZIP during SVG batch processing",
+      );
+    }
   });
 
   // --- Single-file endpoint ---

tests/integration/platform/batch.test.ts

@@ -214,6 +214,32 @@ describe("Filename deduplication in batch", () => {
 
 // ── X-File-Results header ───────────────────────────────────────
 describe("X-File-Results header", () => {
+  it("keeps index alignment when an upload is empty (issue #645)", async () => {
+    // The client maps results back onto its own file list by index, so
+    // dropping a zero-byte part would shift every later index and label a
+    // converted file with a different file's name.
+    const { body, contentType } = createMultipartPayload([
+      { name: "file", filename: "empty.png", contentType: "image/png", content: Buffer.alloc(0) },
+      { name: "file", filename: "real.png", contentType: "image/png", content: PNG },
+      { name: "settings", content: JSON.stringify({ width: 80 }) },
+    ]);
+
+    const res = await app.inject({
+      method: "POST",
+      url: "/api/v1/tools/image/resize/batch",
+      headers: { "content-type": contentType, authorization: `Bearer ${adminToken}` },
+      body,
+    });
+
+    expect(res.statusCode).toBe(200);
+    const fileResults = JSON.parse(decodeURIComponent(res.headers["x-file-results"] as string));
+    expect(fileResults["0"]).toBeUndefined();
+    expect(fileResults["1"]).toMatch(/real/);
+
+    const zip = new AdmZip(res.rawPayload);
+    expect(zip.getEntries()).toHaveLength(1);
+  });
+
   it("maps file indices to output filenames", async () => {
     const { body, contentType } = createMultipartPayload([
       { name: "file", filename: "first.png", contentType: "image/png", content: PNG },

tests/integration/platform/tool-access-gate.test.ts

@@ -0,0 +1,191 @@
+/**
+ * Every tool endpoint enforces tool access, not just the factory-built ones
+ * (issue #645).
+ *
+ * createToolRoute calls requireToolAccess, so factory tools were gated. The
+ * 45 hand-written routes each called nothing, so a role without `tools:use`
+ * could run erase-object, favicon, qr-generate, image-to-pdf, svg-to-raster
+ * and the rest. Per-route calls are what drifted, so the gate now lives in
+ * one preHandler keyed off the tool id in the URL.
+ */
+import { apiToolPath, TOOLS } from "@snapotter/shared";
+import { afterAll, beforeAll, describe, expect, it } from "vitest";
+import { fixtures, readFixture } from "../../fixtures/index.js";
+import {
+  buildTestApp,
+  createMultipartPayload,
+  createUserAndLogin,
+  loginAsAdmin,
+  type TestApp,
+} from "../test-server.js";
+
+const PNG = readFixture(fixtures.image.base.png200);
+const SVG = readFixture(fixtures.image.base.svg100);
+const PDF = readFixture(fixtures.document.pdf3);
+
+let testApp: TestApp;
+let app: TestApp["app"];
+let adminToken: string;
+/** Session for a role that deliberately lacks tools:use. */
+let noToolsToken: string;
+
+beforeAll(async () => {
+  testApp = await buildTestApp();
+  app = testApp.app;
+  adminToken = await loginAsAdmin(app);
+
+  await app.inject({
+    method: "POST",
+    url: "/api/v1/roles",
+    headers: { authorization: `Bearer ${adminToken}` },
+    payload: {
+      name: "notools645",
+      description: "Can browse but not run tools",
+      permissions: ["files:own"],
+    },
+  });
+  noToolsToken = (await createUserAndLogin(app, "notools645user", "notools645")).token;
+}, 30_000);
+
+afterAll(async () => {
+  await testApp.cleanup();
+}, 10_000);
+
+function post(
+  url: string,
+  token: string | null,
+  file: { name: string; type: string; buf: Buffer },
+) {
+  const { body, contentType } = createMultipartPayload([
+    { name: "file", filename: file.name, contentType: file.type, content: file.buf },
+    { name: "settings", content: JSON.stringify({}) },
+  ]);
+  return app.inject({
+    method: "POST",
+    url,
+    body,
+    headers: {
+      "content-type": contentType,
+      ...(token ? { authorization: `Bearer ${token}` } : {}),
+    },
+  });
+}
+
+const IMG = { name: "a.png", type: "image/png", buf: PNG };
+const VEC = { name: "a.svg", type: "image/svg+xml", buf: SVG };
+const DOC = { name: "a.pdf", type: "application/pdf", buf: PDF };
+
+/**
+ * A spread of endpoints across every registration style: the factory, the
+ * three custom ZIP routes and their presets, plain hand-written routes, an AI
+ * route, and the generic batch route.
+ */
+const GATED: Array<[string, string, typeof IMG]> = [
+  ["factory tool", "/api/v1/tools/image/resize", IMG],
+  ["generic batch", "/api/v1/tools/image/resize/batch", IMG],
+  ["image-to-pdf base", "/api/v1/tools/image/image-to-pdf", IMG],
+  ["image-to-pdf preset", "/api/v1/tools/image/jpg-to-pdf", IMG],
+  ["svg-to-raster base", "/api/v1/tools/image/svg-to-raster", VEC],
+  ["svg-to-raster batch", "/api/v1/tools/image/svg-to-raster/batch", VEC],
+  ["svg-to-raster preset", "/api/v1/tools/image/svg-to-png", VEC],
+  ["pdf-to-image preset", "/api/v1/tools/pdf/pdf-to-jpg", DOC],
+  ["pdf-to-image info", "/api/v1/tools/pdf/pdf-to-jpg/info", DOC],
+  ["hand-written: favicon", "/api/v1/tools/image/favicon", IMG],
+  ["hand-written: qr-generate", "/api/v1/tools/image/qr-generate", IMG],
+  ["hand-written: collage", "/api/v1/tools/image/collage", IMG],
+  ["hand-written: split", "/api/v1/tools/image/split", IMG],
+  ["hand-written: stitch", "/api/v1/tools/image/stitch", IMG],
+  ["hand-written: image-to-base64", "/api/v1/tools/image/image-to-base64", IMG],
+  ["hand-written: info", "/api/v1/tools/image/info", IMG],
+  ["hand-written: compare", "/api/v1/tools/image/compare", IMG],
+  ["hand-written: vectorize", "/api/v1/tools/image/vectorize", IMG],
+  ["hand-written: strip-metadata inspect", "/api/v1/tools/image/strip-metadata/inspect", IMG],
+  ["ai route: upscale", "/api/v1/tools/image/upscale", IMG],
+  ["ai route: erase-object", "/api/v1/tools/image/erase-object", IMG],
+  ["ai route: remove-background", "/api/v1/tools/image/remove-background", IMG],
+  ["sign-pdf", "/api/v1/tools/pdf/sign-pdf", DOC],
+];
+
+describe("tool access gate (issue #645)", () => {
+  it.each(GATED)("%s returns 403 for a role without tools:use", async (_label, url, file) => {
+    const res = await post(url, noToolsToken, file);
+    expect(res.statusCode, `${url} -> ${res.body.slice(0, 200)}`).toBe(403);
+  });
+
+  it.each(GATED)("%s returns 401 with no session", async (_label, url, file) => {
+    const res = await post(url, null, file);
+    expect(res.statusCode, `${url} -> ${res.body.slice(0, 200)}`).toBe(401);
+  });
+
+  it.each(GATED)("%s does not 403 an admin", async (_label, url, file) => {
+    const res = await post(url, adminToken, file);
+    expect(res.statusCode, `${url} -> ${res.body.slice(0, 200)}`).not.toBe(403);
+  });
+
+  /**
+   * The sampled list above is readable but partial. This walks the whole
+   * catalog so a tool cannot escape the gate by being registered in a shape
+   * nobody thought to sample, for instance inside an encapsulated plugin,
+   * which is the way a global hook would silently stop applying.
+   */
+  it("refuses every tool in the catalog for a role without tools:use", async () => {
+    const reachable: string[] = [];
+    for (const tool of TOOLS) {
+      const res = await post(apiToolPath(tool.id), noToolsToken, IMG);
+      if (res.statusCode !== 403) reachable.push(`${tool.id} -> ${res.statusCode}`);
+    }
+    expect(reachable, `tools not gated: ${reachable.join(", ")}`).toEqual([]);
+  }, 120_000);
+
+  it("still lets a tools:use-less role read the popular-tools listing", async () => {
+    // A listing under the same prefix is not a tool run. `popular` has one
+    // path segment, so it must not be mistaken for a section/toolId pair.
+    const res = await app.inject({
+      method: "GET",
+      url: "/api/v1/tools/popular",
+      headers: { authorization: `Bearer ${noToolsToken}` },
+    });
+    expect(res.statusCode).toBe(200);
+  });
+
+  it("leaves an unknown tool id as a 404, not a 403", async () => {
+    const res = await post("/api/v1/tools/image/not-a-real-tool", noToolsToken, IMG);
+    expect(res.statusCode).toBe(404);
+  });
+
+  it("leaves a real tool under the wrong section as a 404", async () => {
+    const res = await post("/api/v1/tools/video/resize", noToolsToken, IMG);
+    expect(res.statusCode).toBe(404);
+  });
+
+  /**
+   * The gate reads the tool id out of the URL, so any URL form the router
+   * still resolves to a tool handler but the gate reads differently would be
+   * a way straight past it. Each of these must be refused or unrouted; the
+   * one answer that must never appear is a 2xx.
+   */
+  it.each([
+    // Probe hand-written routes specifically. A factory tool like resize
+    // keeps its own requireToolAccess call, so it answers 403 whether or not
+    // the gate resolved the URL, and would hide a gate that failed open.
+    ["percent-encoded hand-written id", "/api/v1/tools/image/%66avicon"],
+    ["percent-encoded ai route", "/api/v1/tools/image/%75pscale"],
+    ["fully percent-encoded hand-written id", "/api/v1/tools/image/%73%70%6c%69%74"],
+    ["percent-encoded tool id", "/api/v1/tools/image/%72esize"],
+    ["percent-encoded section", "/api/v1/tools/%69mage/resize"],
+    ["fully percent-encoded id", "/api/v1/tools/image/%72%65%73%69%7a%65"],
+    ["encoded separator", "/api/v1/tools/image%2fresize"],
+    ["uppercase tool id", "/api/v1/tools/image/RESIZE"],
+    ["double slash before section", "/api/v1/tools//image/resize"],
+    ["dot segment", "/api/v1/tools/image/./resize"],
+    ["parent segment", "/api/v1/tools/pdf/../image/resize"],
+    ["trailing dot", "/api/v1/tools/image/resize."],
+    ["query smuggling", "/api/v1/tools/image/resize?x=/api/v1/tools/image/other"],
+  ])("never processes a tool via %s", async (_label, url) => {
+    const res = await post(url, noToolsToken, IMG);
+    expect(
+      res.statusCode,
+      `${url} reached a handler with ${res.statusCode}: ${res.body.slice(0, 200)}`,
+    ).toBeGreaterThanOrEqual(400);
+  });
+});

tests/integration/platform/zip-stream-failure.test.ts

@@ -0,0 +1,185 @@
+/**
+ * What every ZIP-streaming route does when object storage fails AFTER the 200
+ * headers are already on the wire (issue #645).
+ *
+ * The response is hijacked and chunked by then, so the status cannot change.
+ * Ending the response cleanly is therefore indistinguishable from success and
+ * hands the client a ZIP with no central directory; worse, a source stream
+ * that errors with no listener leaves the request hanging and raises an
+ * unhandled error. The routes must instead destroy the connection so the
+ * client sees a transport failure it can act on.
+ */
+import AdmZip from "adm-zip";
+import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
+import { fixtures, readFixture } from "../../fixtures/index.js";
+import {
+  buildTestApp,
+  createMultipartPayload,
+  loginAsAdmin,
+  type TestApp,
+} from "../test-server.js";
+
+/** Prefixes whose getObjectStream should fail once a consumer starts reading. */
+const storageMock = vi.hoisted(() => ({ poison: new Set<string>() }));
+
+vi.mock("../../../apps/api/src/lib/object-storage.js", async (importOriginal) => {
+  const actual =
+    await importOriginal<typeof import("../../../apps/api/src/lib/object-storage.js")>();
+  const { Readable } = await import("node:stream");
+  return {
+    ...actual,
+    getObjectStream: async (key: string) => {
+      for (const prefix of storageMock.poison) {
+        if (key.startsWith(prefix)) {
+          // Fail on read, not on open: that is how a real backend behaves
+          // (createReadStream resolves, then emits ENOENT), and it is the case
+          // the route's try/catch cannot see.
+          return new Readable({
+            read() {
+              this.destroy(new Error(`test poison: stream error for ${key}`));
+            },
+          });
+        }
+      }
+      return actual.getObjectStream(key);
+    },
+  };
+});
+
+const PNG = readFixture(fixtures.image.base.png200);
+const JPG = readFixture(fixtures.image.base.jpg100);
+const SVG = readFixture(fixtures.image.base.svg100);
+const PDF_3PAGE = readFixture(fixtures.document.pdf3);
+const PDF_2PAGE = readFixture(fixtures.document.pdf2);
+
+let testApp: TestApp;
+let token: string;
+
+beforeAll(async () => {
+  testApp = await buildTestApp();
+  token = await loginAsAdmin(testApp.app);
+}, 30_000);
+
+afterAll(async () => {
+  await testApp.cleanup();
+}, 10_000);
+
+/**
+ * Run a request whose ZIP entries all come from poisoned storage and report
+ * how it ended. A route that handles this correctly must terminate promptly
+ * and must not hand back something that parses as a complete archive.
+ */
+async function runWithPoisonedOutputs(
+  url: string,
+  parts: Array<{ name: string; filename?: string; contentType?: string; content: Buffer | string }>,
+): Promise<{ settledWithin: boolean; deliveredCompleteZip: boolean }> {
+  const { body, contentType } = createMultipartPayload(parts);
+  storageMock.poison.add("outputs/");
+
+  // Race a real deadline rather than leaning on the test timeout: a hang is
+  // the failure mode being tested, and "the suite timed out" is a much weaker
+  // signal than a named assertion.
+  const HANG_BUDGET_MS = 10_000;
+  let timer: NodeJS.Timeout | undefined;
+  const deadline = new Promise<"timeout">((resolve) => {
+    timer = setTimeout(() => resolve("timeout"), HANG_BUDGET_MS);
+  });
+
+  try {
+    const request = testApp.app
+      .inject({
+        method: "POST",
+        url,
+        headers: { authorization: `Bearer ${token}`, "content-type": contentType },
+        body,
+      })
+      // A destroyed connection surfaces as a rejected request, which is
+      // exactly the signal the client should get.
+      .then(
+        (res) => res,
+        () => null,
+      );
+
+    const outcome = await Promise.race([request, deadline]);
+    if (outcome === "timeout") return { settledWithin: false, deliveredCompleteZip: false };
+    if (outcome === null) return { settledWithin: true, deliveredCompleteZip: false };
+
+    try {
+      new AdmZip(outcome.rawPayload).getEntries();
+      return { settledWithin: true, deliveredCompleteZip: true };
+    } catch {
+      return { settledWithin: true, deliveredCompleteZip: false };
+    }
+  } finally {
+    if (timer) clearTimeout(timer);
+    storageMock.poison.clear();
+  }
+}
+
+describe("ZIP streaming failure after headers are sent (issue #645)", () => {
+  // Healthy path first: a hung request from a later case would otherwise
+  // starve it and make an unrelated timeout look like a regression.
+  it("returns a complete ZIP when storage is healthy", async () => {
+    const { body, contentType } = createMultipartPayload([
+      { name: "file", filename: "a.png", contentType: "image/png", content: PNG },
+      { name: "file", filename: "b.jpg", contentType: "image/jpeg", content: JPG },
+      { name: "settings", content: JSON.stringify({ width: 50 }) },
+    ]);
+    const res = await testApp.app.inject({
+      method: "POST",
+      url: "/api/v1/tools/image/resize/batch",
+      headers: { authorization: `Bearer ${token}`, "content-type": contentType },
+      body,
+    });
+
+    expect(res.statusCode).toBe(200);
+    expect(new AdmZip(res.rawPayload).getEntries()).toHaveLength(2);
+  }, 60_000);
+
+  it("svg-to-raster still returns a complete ZIP (it never reads storage mid-stream)", async () => {
+    // Kept as a guard rather than a failure case: svg-to-raster appends
+    // in-memory buffers, so it has no post-header storage read to poison.
+    const { body, contentType } = createMultipartPayload([
+      { name: "file", filename: "a.svg", contentType: "image/svg+xml", content: SVG },
+      { name: "file", filename: "b.svg", contentType: "image/svg+xml", content: SVG },
+      { name: "settings", content: JSON.stringify({ outputFormat: "png" }) },
+    ]);
+    storageMock.poison.add("outputs/");
+    try {
+      const res = await testApp.app.inject({
+        method: "POST",
+        url: "/api/v1/tools/image/svg-to-raster/batch",
+        headers: { authorization: `Bearer ${token}`, "content-type": contentType },
+        body,
+      });
+      expect(res.statusCode).toBe(200);
+      expect(new AdmZip(res.rawPayload).getEntries()).toHaveLength(2);
+    } finally {
+      storageMock.poison.clear();
+    }
+  }, 60_000);
+
+  it("generic /batch terminates instead of hanging, and delivers no complete ZIP", async () => {
+    const result = await runWithPoisonedOutputs("/api/v1/tools/image/resize/batch", [
+      { name: "file", filename: "a.png", contentType: "image/png", content: PNG },
+      { name: "file", filename: "b.jpg", contentType: "image/jpeg", content: JPG },
+      { name: "settings", content: JSON.stringify({ width: 50 }) },
+      { name: "clientJobId", content: "zipfail-batch" },
+    ]);
+
+    expect(result.settledWithin, "request hung instead of terminating").toBe(true);
+    expect(result.deliveredCompleteZip).toBe(false);
+  }, 60_000);
+
+  it("pdf-to-image /batch terminates instead of hanging", async () => {
+    const result = await runWithPoisonedOutputs("/api/v1/tools/pdf/pdf-to-jpg/batch", [
+      { name: "file", filename: "a.pdf", contentType: "application/pdf", content: PDF_3PAGE },
+      { name: "file", filename: "b.pdf", contentType: "application/pdf", content: PDF_2PAGE },
+      { name: "settings", content: JSON.stringify({ dpi: 72 }) },
+      { name: "clientJobId", content: "zipfail-pdf" },
+    ]);
+
+    expect(result.settledWithin, "request hung instead of terminating").toBe(true);
+    expect(result.deliveredCompleteZip).toBe(false);
+  }, 60_000);
+});

tests/integration/security/adversarial-extended.test.ts

@@ -726,8 +726,11 @@ describe("Batch edge cases — extended", () => {
     expect(json.errors.length).toBe(2);
   });
 
-  it("handles batch with zero-byte files (skipped as empty)", async () => {
-    // Batch processing silently skips zero-byte parts (buffer.length === 0)
+  it("rejects a zero-byte file and names it (issue #645)", async () => {
+    // Zero-byte parts used to be dropped during parsing, which answered "No
+    // files provided" for a request that did provide one and, in a mixed
+    // batch, shifted every later result onto the wrong file. They now keep
+    // their slot and fail in place with a reason.
     const res = await postBatch("resize", [
       {
         name: "file",
@@ -738,10 +741,12 @@ describe("Batch edge cases — extended", () => {
       { name: "settings", content: JSON.stringify({ width: 50 }) },
     ]);
 
-    // The zero-byte file is skipped, resulting in 0 valid files
-    expect(res.statusCode).toBe(400);
+    expect(res.statusCode).toBe(422);
     const json = JSON.parse(res.body);
-    expect(json.error).toMatch(/no file/i);
+    expect(json.error).toMatch(/all files failed/i);
+    expect(json.errors).toHaveLength(1);
+    expect(json.errors[0].filename).toBe("empty.png");
+    expect(json.errors[0].error).toMatch(/empty/i);
   });
 });
 
@@ -1273,7 +1278,7 @@ describe("Settings boundary values", () => {
 // ═══════════════════════════════════════════════════════════════════════════
 // MULTI-FORMAT ZERO-BYTE BATCH
 // ═══════════════════════════════════════════════════════════════════════════
-describe("Batch with only zero-byte files", () => {
+describe("Batch where every file is zero-byte", () => {
   it("rejects batch where all files are zero-byte", async () => {
     const res = await postBatch("resize", [
       {
@@ -1291,10 +1296,15 @@ describe("Batch with only zero-byte files", () => {
       { name: "settings", content: JSON.stringify({ width: 50 }) },
     ]);
 
-    // Zero-byte files are skipped during parsing, so 0 valid files
-    expect(res.statusCode).toBe(400);
+    // Still rejected outright, nothing processed, but the caller is now told
+    // which files were empty instead of being told it sent none (issue #645).
+    expect(res.statusCode).toBe(422);
     const json = JSON.parse(res.body);
-    expect(json.error).toMatch(/no file/i);
+    expect(json.error).toMatch(/all files failed/i);
+    expect(json.errors.map((e: { filename: string }) => e.filename)).toEqual([
+      "empty1.png",
+      "empty2.png",
+    ]);
   });
 });
 

tests/integration/test-server.ts

@@ -47,6 +47,7 @@ import { registerMfa } from "../../apps/api/src/plugins/mfa.js";
 import { oidcRoutes } from "../../apps/api/src/plugins/oidc.js";
 import { registerPerUserRateLimit } from "../../apps/api/src/plugins/per-user-rate-limit.js";
 import { registerSaml } from "../../apps/api/src/plugins/saml.js";
+import { toolAccessMiddleware } from "../../apps/api/src/plugins/tool-access.js";
 import { registerUpload } from "../../apps/api/src/plugins/upload.js";
 import { adminOpsRoutes } from "../../apps/api/src/routes/admin-ops.js";
 import { analyticsRoutes } from "../../apps/api/src/routes/analytics.js";
@@ -155,6 +156,9 @@ export async function buildTestApp(): Promise<TestApp> {
   // Auth middleware (must be registered before routes)
   await authMiddleware(app);
 
+  // Tool access gate (mirrors index.ts: after auth, before routes)
+  await toolAccessMiddleware(app);
+
   // Per-user rate limiting (after auth so request.user is populated)
   try {
     await registerPerUserRateLimit(app);

tests/integration/tools/image/svg-to-raster.test.ts

@@ -129,11 +129,9 @@ describe("svg-to-raster", () => {
     expect(meta.height).toBeLessThanOrEqual(50);
   });
 
-  it.each([
-    "png",
-    "jpg",
-    "webp",
-  ] as const)("converts to output format: %s", async (outputFormat) => {
+  it.each(["png", "jpg", "webp"] as const)(
+    "converts to output format: %s",
+    async (outputFormat) => {
       const { body, contentType } = createMultipartPayload([
         { name: "file", filename: "test.svg", contentType: "image/svg+xml", content: SVG },
         { name: "settings", content: JSON.stringify({ outputFormat }) },
@@ -156,7 +154,8 @@ describe("svg-to-raster", () => {
       const meta = await sharp(Buffer.from(dlRes.rawPayload)).metadata();
       const expectedFormat = outputFormat === "jpg" ? "jpeg" : outputFormat;
       expect(meta.format).toBe(expectedFormat);
-  });
+    },
+  );
 
   it("respects DPI setting", async () => {
     const { body, contentType } = createMultipartPayload([
@@ -572,6 +571,33 @@ describe("svg-to-raster", () => {
   // ── Batch endpoint coverage ────────────────────────────────────────
 
   describe("batch", () => {
+    it("keeps index alignment when an upload is empty (issue #645)", async () => {
+      // The client pairs results with its own file list by index, so a
+      // dropped zero-byte part would pin this SVG's output to the empty slot.
+      const { body, contentType } = createMultipartPayload([
+        {
+          name: "file",
+          filename: "empty.svg",
+          contentType: "image/svg+xml",
+          content: Buffer.alloc(0),
+        },
+        { name: "file", filename: "real.svg", contentType: "image/svg+xml", content: SVG },
+        { name: "settings", content: JSON.stringify({ outputFormat: "png" }) },
+      ]);
+
+      const res = await app.inject({
+        method: "POST",
+        url: "/api/v1/tools/image/svg-to-raster/batch",
+        headers: { authorization: `Bearer ${adminToken}`, "content-type": contentType },
+        body,
+      });
+
+      expect(res.statusCode).toBe(200);
+      const fileResults = JSON.parse(decodeURIComponent(res.headers["x-file-results"] as string));
+      expect(fileResults["0"]).toBeUndefined();
+      expect(fileResults["1"]).toMatch(/real/);
+    });
+
     it("converts multiple SVGs in a single batch request", async () => {
       const { body, contentType } = createMultipartPayload([
         { name: "file", filename: "a.svg", contentType: "image/svg+xml", content: SVG },

tests/unit/api/tool-access-url.test.ts

@@ -0,0 +1,68 @@
+/**
+ * Route resolution for the tool access gate (issue #645).
+ *
+ * The gate protects whatever tool this resolves, so getting it wrong either
+ * opens a tool up or 403s something that was never a tool run. It reads the
+ * matched route pattern rather than the raw URL on purpose: find-my-way
+ * decodes before matching, so a second parse of the raw string disagrees with
+ * the router and fails open on `/api/v1/tools/image/%66avicon`.
+ */
+import { apiToolPath, TOOLS, toolSection } from "@snapotter/shared";
+import { describe, expect, it } from "vitest";
+import { toolIdFromRoute } from "../../../apps/api/src/plugins/tool-access.js";
+
+const BATCH_PATTERN = "/api/v1/tools/:section/:toolId/batch";
+
+describe("toolIdFromRoute", () => {
+  it("resolves every tool in the catalog from its own registered path", () => {
+    const unresolved = TOOLS.filter(
+      (tool) => toolIdFromRoute(apiToolPath(tool.id)) !== tool.id,
+    ).map((tool) => tool.id);
+    expect(unresolved, `tools whose own path does not resolve: ${unresolved.join(", ")}`).toEqual(
+      [],
+    );
+  });
+
+  it.each(["batch", "info", "preview", "analyze", "inspect", "generate", "effects"])(
+    "resolves the /%s sub-path to its parent tool",
+    (suffix) => {
+      expect(toolIdFromRoute(`/api/v1/tools/image/resize/${suffix}`)).toBe("resize");
+    },
+  );
+
+  it("resolves the parametric batch route from the router's decoded params", () => {
+    expect(toolIdFromRoute(BATCH_PATTERN, { section: "image", toolId: "resize" })).toBe("resize");
+  });
+
+  it("returns null when the parametric route names an unknown tool", () => {
+    expect(toolIdFromRoute(BATCH_PATTERN, { section: "image", toolId: "nope" })).toBeNull();
+  });
+
+  it("returns null when the parametric route's section does not match the tool", () => {
+    expect(toolIdFromRoute(BATCH_PATTERN, { section: "video", toolId: "resize" })).toBeNull();
+  });
+
+  it("returns null when the parametric route has no params", () => {
+    expect(toolIdFromRoute(BATCH_PATTERN)).toBeNull();
+  });
+
+  it.each([
+    ["no matched route", undefined],
+    ["the catalog root", "/api/v1/tools/"],
+    ["a single-segment listing", "/api/v1/tools/popular"],
+    ["an unrelated route", "/api/v1/jobs/:jobId/progress"],
+    ["a path that merely looks similar", "/api/v1/toolsomething/image/resize"],
+  ])("returns null for %s", (_label, routeUrl) => {
+    expect(toolIdFromRoute(routeUrl)).toBeNull();
+  });
+
+  it("returns null for an unknown tool id so the route can still 404", () => {
+    expect(toolIdFromRoute("/api/v1/tools/image/not-a-real-tool")).toBeNull();
+  });
+
+  it("returns null when a real tool is registered under the wrong section", () => {
+    const image = TOOLS.find((t) => toolSection(t) === "image");
+    if (!image) throw new Error("no image-section tool in the catalog");
+    expect(toolIdFromRoute(`/api/v1/tools/video/${image.id}`)).toBeNull();
+  });
+});