fix(alt-text): reject unmanaged collections in generate endpoints (#162)

Author: jhb-dev

alt-text/CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Unreleased
 
 - fix: enforce collection access control in the generate and bulk-generate endpoints by running the Local API reads and writes under the requesting user (`overrideAccess: false`)
+- fix: reject requests to the generate and bulk-generate endpoints that target a collection the plugin does not manage with `403`, before any document read or write
 - fix: filter the alt text health report (endpoint and dashboard widget) to the collections the requesting user may read, so the aggregate no longer discloses counts and document IDs for collections their role cannot access
 - feat: `healthCheck` now accepts an access function that gates the health endpoint and hides the dashboard widget, letting the collection-wide report be restricted (e.g. to admins) separately from the generate endpoints
 - fix: respect update access in the admin UI — render the alt text field read-only and hide the single-document and bulk generate buttons for users without update access

alt-text/src/endpoints/bulkGenerateAltTexts.ts

@@ -36,6 +36,18 @@ export const bulkGenerateAltTextsEndpoint =
         return Response.json({ error: 'Plugin config not found' }, { status: 500 })
       }
 
+      // Treat the configured collections as an allowlist. Reject any other
+      // collection before touching the Local API, so the endpoint can only ever
+      // operate on the upload collections the plugin manages.
+      const collectionConfig = pluginConfig.collections.find((entry) => entry.slug === collection)
+
+      if (!collectionConfig) {
+        return Response.json(
+          { error: `Collection "${collection}" is not managed by the alt text plugin.` },
+          { status: 403 },
+        )
+      }
+
       if (!pluginConfig.resolver) {
         return Response.json({ error: 'No alt text resolver configured' }, { status: 500 })
       }
@@ -134,9 +146,11 @@ async function generateAndUpdateAltText({
   const mimeType =
     'mimeType' in imageDoc && typeof imageDoc.mimeType === 'string' ? imageDoc.mimeType : undefined
 
-  const collectionConfig = pluginConfig.collections.find((entry) => entry.slug === collection)
+  // The handler validates `collection` against the configured collections before
+  // reaching this helper, so a matching entry is guaranteed.
+  const collectionConfig = pluginConfig.collections.find((entry) => entry.slug === collection)!
 
-  if (mimeType && collectionConfig && !matchesMimeType(mimeType, collectionConfig.mimeTypes)) {
+  if (mimeType && !matchesMimeType(mimeType, collectionConfig.mimeTypes)) {
     throw new Error(
       `Alt text is not tracked for files of type "${mimeType}" in the "${collection}" collection. Tracked types: ${collectionConfig.mimeTypes.join(', ')}.`,
     )

alt-text/src/endpoints/generateAltText.ts

@@ -28,6 +28,26 @@ export const generateAltTextEndpoint =
 
       const { id, collection, locale, update } = generateAltTextRequestSchema.parse(data)
 
+      const pluginConfig = req.payload.config.custom?.altTextPluginConfig as
+        | AltTextPluginConfig
+        | undefined
+
+      if (!pluginConfig) {
+        return Response.json({ error: 'Plugin config not found' }, { status: 500 })
+      }
+
+      // Treat the configured collections as an allowlist. Reject any other
+      // collection before touching the Local API, so the endpoint can only ever
+      // operate on the upload collections the plugin manages.
+      const collectionConfig = pluginConfig.collections.find((entry) => entry.slug === collection)
+
+      if (!collectionConfig) {
+        return Response.json(
+          { error: `Collection "${collection}" is not managed by the alt text plugin.` },
+          { status: 403 },
+        )
+      }
+
       const imageDoc = await req.payload.findByID({
         id,
         collection,
@@ -42,14 +62,6 @@ export const generateAltTextEndpoint =
         return Response.json({ error: 'Image not found' }, { status: 404 })
       }
 
-      const pluginConfig = req.payload.config.custom?.altTextPluginConfig as
-        | AltTextPluginConfig
-        | undefined
-
-      if (!pluginConfig) {
-        return Response.json({ error: 'Plugin config not found' }, { status: 500 })
-      }
-
       if (!pluginConfig.getImageThumbnail) {
         return Response.json(
           { error: 'getImageThumbnail function not configured' },
@@ -66,9 +78,7 @@ export const generateAltTextEndpoint =
           ? imageDoc.mimeType
           : undefined
 
-      const collectionConfig = pluginConfig.collections.find((entry) => entry.slug === collection)
-
-      if (mimeType && collectionConfig && !matchesMimeType(mimeType, collectionConfig.mimeTypes)) {
+      if (mimeType && !matchesMimeType(mimeType, collectionConfig.mimeTypes)) {
         return Response.json(
           {
             error: `Alt text is not tracked for files of type "${mimeType}" in the "${collection}" collection. Tracked types: ${collectionConfig.mimeTypes.join(', ')}.`,

alt-text/test/endpointCollectionAllowlist.test.ts

@@ -0,0 +1,129 @@
+import assert from 'node:assert/strict'
+
+import { describe, test } from 'vitest'
+
+import type { PayloadRequest } from 'payload'
+
+import type { AltTextPluginConfig } from '../src/types/AltTextPluginConfig.ts'
+
+import { bulkGenerateAltTextsEndpoint } from '../src/endpoints/bulkGenerateAltTexts.ts'
+import { generateAltTextEndpoint } from '../src/endpoints/generateAltText.ts'
+
+/**
+ * The endpoints must treat the configured `collections` as an allowlist. A
+ * request naming a collection the plugin does not manage (e.g. `users`) must be
+ * rejected with `403` before any Local API call — otherwise the endpoints can be
+ * pointed at any collection in the app, widening the blast radius of the
+ * access-control surface to the entire data model.
+ */
+
+const user = { id: 'low-priv-user', email: 'user@example.com', role: 'user' }
+
+type LocalApiCall = Record<string, unknown>
+
+function buildPluginConfig(): AltTextPluginConfig {
+  return {
+    access: ({ req }) => !!req.user,
+    collections: [{ slug: 'media', mimeTypes: ['image/*'] }],
+    enabled: true,
+    getImageThumbnail: () => 'https://example.com/thumb.png',
+    healthCheck: true,
+    healthCheckAccess: ({ req }) => !!req.user,
+    locale: 'en',
+    locales: [],
+    maxBulkGenerateConcurrency: 1,
+    resolver: {
+      key: 'mock',
+      resolve: async () => ({
+        success: true,
+        result: { altText: 'generated alt', keywords: ['a', 'b'] },
+      }),
+      resolveBulk: async () => ({
+        success: true,
+        results: { en: { altText: 'generated alt', keywords: ['a', 'b'] } },
+      }),
+    },
+  }
+}
+
+function buildRequest(body: unknown): {
+  findByIDCalls: LocalApiCall[]
+  req: PayloadRequest
+  updateCalls: LocalApiCall[]
+} {
+  const findByIDCalls: LocalApiCall[] = []
+  const updateCalls: LocalApiCall[] = []
+
+  const req = {
+    json: async () => body,
+    payload: {
+      config: { custom: { altTextPluginConfig: buildPluginConfig() } },
+      findByID: async (args: LocalApiCall) => {
+        findByIDCalls.push(args)
+        return { id: args.id, filename: 'photo.png', mimeType: 'image/png' }
+      },
+      update: async (args: LocalApiCall) => {
+        updateCalls.push(args)
+        return { id: args.id }
+      },
+    },
+    user,
+  } as unknown as PayloadRequest
+
+  return { findByIDCalls, req, updateCalls }
+}
+
+describe('generate endpoint collection allowlist', () => {
+  test('rejects a collection the plugin does not manage with 403 and no Local API call', async () => {
+    const { findByIDCalls, req, updateCalls } = buildRequest({
+      id: 'doc-1',
+      collection: 'users',
+      locale: 'en',
+      update: true,
+    })
+
+    const response = await generateAltTextEndpoint(buildPluginConfig().access)(req)
+
+    assert.equal(response.status, 403)
+    assert.equal(findByIDCalls.length, 0)
+    assert.equal(updateCalls.length, 0)
+  })
+
+  test('allows a configured collection', async () => {
+    const { findByIDCalls, req } = buildRequest({
+      id: 'doc-1',
+      collection: 'media',
+      locale: 'en',
+      update: false,
+    })
+
+    const response = await generateAltTextEndpoint(buildPluginConfig().access)(req)
+
+    assert.equal(response.status, 200)
+    assert.equal(findByIDCalls.length, 1)
+  })
+})
+
+describe('bulk generate endpoint collection allowlist', () => {
+  test('rejects a collection the plugin does not manage with 403 and no Local API call', async () => {
+    const { findByIDCalls, req, updateCalls } = buildRequest({
+      collection: 'users',
+      ids: ['doc-1', 'doc-2'],
+    })
+
+    const response = await bulkGenerateAltTextsEndpoint(buildPluginConfig().access)(req)
+
+    assert.equal(response.status, 403)
+    assert.equal(findByIDCalls.length, 0)
+    assert.equal(updateCalls.length, 0)
+  })
+
+  test('allows a configured collection', async () => {
+    const { findByIDCalls, req } = buildRequest({ collection: 'media', ids: ['doc-1'] })
+
+    const response = await bulkGenerateAltTextsEndpoint(buildPluginConfig().access)(req)
+
+    assert.equal(response.status, 200)
+    assert.equal(findByIDCalls.length, 1)
+  })
+})