fix: validate image order and alt text updates

apps/api/src/modules/inventory/domain/inventory-image.js

@@ -42,4 +42,17 @@ export function validateImageSignature(data, mimeType) {
   }
 
   return true;
-}
+}
+export function validateImageSortOrder(sortOrder) {
+  if (!Number.isInteger(sortOrder) || sortOrder < 0 || sortOrder > 2147483647) {
+    throw createApplicationError('IMAGE_ORDER_INVALID', 'Image order must be an integer between 0 and 2147483647.', 400);
+  }
+}
+
+export function normalizeImageAltText(altText) {
+  if (altText === null) return null;
+  if (typeof altText !== 'string' || altText.trim().length > 1000) {
+    throw createApplicationError('IMAGE_ALT_TEXT_INVALID', 'Alt text must be text up to 1,000 characters or null.', 400);
+  }
+  return altText.trim();
+}

apps/api/src/modules/inventory/routes/admin-image-routes.js

@@ -1,5 +1,5 @@
 import { toAdminInventoryImageDto } from '@pokemon-inventory/contracts/image';
-import { createInventoryImageSchema, moveInventoryImageSchema } from '@pokemon-inventory/validation/image';
+import { createInventoryImageSchema, moveInventoryImageSchema, updateImageOrderSchema, updateImageAltTextSchema } from '@pokemon-inventory/validation/image';
 import { randomUUID } from 'node:crypto';
 
 import { createApplicationError } from '../../../shared/errors/application-error.js';
@@ -110,11 +110,13 @@ export async function registerAdminImageRoutes(app, options) {
     return { data: toAdminInventoryImageDto(image) };
   });
 
-  app.patch('/images/:id/order', async (request) => {
+  app.patch('/images/:id/order', async (request, reply) => {
     const admin = await authorizeAdminMutation(request);
+    const result = updateImageOrderSchema.safeParse(request.body);
+    if (!result.success) return validationError(reply, result);
     const image = await inventoryImageService.updateOrder({
       imageId: request.params.id,
-      sortOrder: request.body?.sortOrder,
+      sortOrder: result.data.sortOrder,
       sellerId: admin.sellerId,
     });
     return { data: toAdminInventoryImageDto(image) };
@@ -129,11 +131,13 @@ export async function registerAdminImageRoutes(app, options) {
     return { data: toAdminInventoryImageDto(image) };
   });
 
-  app.patch('/images/:id/alt-text', async (request) => {
+  app.patch('/images/:id/alt-text', async (request, reply) => {
     const admin = await authorizeAdminMutation(request);
+    const result = updateImageAltTextSchema.safeParse(request.body);
+    if (!result.success) return validationError(reply, result);
     const image = await inventoryImageService.updateAltText({
       imageId: request.params.id,
-      altText: request.body?.altText ?? null,
+      altText: result.data.altText,
       sellerId: admin.sellerId,
     });
     return { data: toAdminInventoryImageDto(image) };

apps/api/src/modules/inventory/services/inventory-image-service.js

@@ -1,6 +1,6 @@
 import { validateImageMove } from '../domain/image-order.js';
 import { createApplicationError } from '../../../shared/errors/application-error.js';
-import { validateImageSignature, validateInventoryImageMetadata } from '../domain/inventory-image.js';
+import { normalizeImageAltText, validateImageSortOrder, validateImageSignature, validateInventoryImageMetadata } from '../domain/inventory-image.js';
 
 export function createInventoryImageService({ inventoryImageRepository, imageDerivativeJobRepository, imageStorage }) {
   return {
@@ -49,6 +49,7 @@ export function createInventoryImageService({ inventoryImageRepository, imageDer
     },
 
     async updateOrder({ imageId, sortOrder, sellerId }) {
+      validateImageSortOrder(sortOrder);
       return requireOwnedImage(inventoryImageRepository.updateOrderForSeller(imageId, sortOrder, sellerId));
     },
 
@@ -57,7 +58,7 @@ export function createInventoryImageService({ inventoryImageRepository, imageDer
     },
 
     async updateAltText({ imageId, altText, sellerId }) {
-      return requireOwnedImage(inventoryImageRepository.updateAltTextForSeller(imageId, altText, sellerId));
+      return requireOwnedImage(inventoryImageRepository.updateAltTextForSeller(imageId, normalizeImageAltText(altText), sellerId));
     },
 
     async delete({ imageId, sellerId }) {

apps/api/tests/application/inventory-image-management.test.js

@@ -48,3 +48,19 @@ test('moves an image through the owned atomic repository operation', async () =>
   await assert.rejects(() => service.move({ imageId: 'missing', direction: 'down', sellerId: 'seller-1' }), { code: 'IMAGE_NOT_FOUND' });
   await assert.rejects(() => service.move({ imageId: 'owned', direction: 'left', sellerId: 'seller-1' }), { code: 'IMAGE_MOVE_INVALID' });
 });
+
+test('validates image updates before persistence and normalizes valid alt text', async () => {
+  const { service, calls } = createService();
+  for (const sortOrder of [undefined, null, -1, 0.5, '1', NaN, Infinity, 2147483648]) {
+    await assert.rejects(() => service.updateOrder({ imageId: 'image-1', sortOrder, sellerId: 'seller-1' }), { code: 'IMAGE_ORDER_INVALID' });
+  }
+  for (const altText of [undefined, 123, {}, [], 'x'.repeat(1001)]) {
+    await assert.rejects(() => service.updateAltText({ imageId: 'image-1', altText, sellerId: 'seller-1' }), { code: 'IMAGE_ALT_TEXT_INVALID' });
+  }
+  assert.deepEqual(calls, []);
+  await service.updateOrder({ imageId: 'image-1', sortOrder: 2147483647, sellerId: 'seller-1' });
+  for (const [input, expected] of [['  Front  ', 'Front'], ['   ', ''], [null, null], ['x'.repeat(1000), 'x'.repeat(1000)]]) {
+    const result = await service.updateAltText({ imageId: 'image-1', altText: input, sellerId: 'seller-1' });
+    assert.equal(result.altText, expected);
+  }
+});

apps/api/tests/http/image-update-validation.test.js

@@ -0,0 +1,51 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+import { buildApp } from '../../src/app/build-app.js';
+import { createInventoryImageService } from '../../src/modules/inventory/services/inventory-image-service.js';
+
+test('rejects malformed image updates without writes and accepts normalized boundary values', async (context) => {
+  const writes = [];
+  const app = buildApp({
+    authenticateAdmin: async () => ({ sellerId: 'seller-1' }),
+    inventoryImageService: createInventoryImageService({ inventoryImageRepository: {
+      async updateOrderForSeller(id, sortOrder, sellerId) { writes.push({ id, sortOrder, sellerId }); return { id, sortOrder }; },
+      async updateAltTextForSeller(id, altText, sellerId) { writes.push({ id, altText, sellerId }); return { id, altText }; },
+    } }),
+  });
+  context.after(() => app.close());
+  const patch = (path, payload) => app.inject({ method: 'PATCH', url: `/api/v1/admin/images/image-1/${path}`, payload });
+  for (const payload of [{}, { sortOrder: -1 }, { sortOrder: 0.2 }, { sortOrder: '1' }, { sortOrder: null }, { sortOrder: 2147483648 }, { sortOrder: 0, sellerId: 'other' }]) {
+    const response = await patch('order', payload);
+    assert.equal(response.statusCode, 400);
+    assert.equal(response.json().error.code, 'VALIDATION_ERROR');
+  }
+  for (const payload of [{}, { altText: 1 }, { altText: {} }, { altText: [] }, { altText: 'x'.repeat(1001) }, { altText: 'Front', private: true }]) {
+    assert.equal((await patch('alt-text', payload)).statusCode, 400);
+  }
+  assert.deepEqual(writes, []);
+  for (const sortOrder of [0, 2147483647]) assert.equal((await patch('order', { sortOrder })).json().data.sortOrder, sortOrder);
+  for (const [altText, expected] of [[' Front ', 'Front'], ['   ', ''], [null, null], ['x'.repeat(1000), 'x'.repeat(1000)]]) {
+    const response = await patch('alt-text', { altText });
+    assert.equal(response.statusCode, 200);
+    assert.equal(response.json().data.altText, expected);
+  }
+  assert.ok(writes.every(({ sellerId }) => sellerId === 'seller-1'));
+});
+
+test('image update validation does not bypass authentication or CSRF', async (context) => {
+  for (const signedIn of [false, true]) {
+    const app = buildApp({
+      authenticateAdmin: async () => signedIn ? { sellerId: 'seller-1' } : null,
+      authorizeUnsafeRequest: async () => { throw Object.assign(new Error('Forbidden'), { statusCode: 403 }); },
+      inventoryImageService: {
+        async updateOrder() { assert.fail('blocked request reached order service'); },
+        async updateAltText() { assert.fail('blocked request reached alt-text service'); },
+      },
+    });
+    context.after(() => app.close());
+    for (const path of ['order', 'alt-text']) {
+      const response = await app.inject({ method: 'PATCH', url: `/api/v1/admin/images/image-1/${path}`, payload: {} });
+      assert.equal(response.statusCode, signedIn ? 403 : 401);
+    }
+  }
+});

apps/web/src/components/organisms/AdminImageManager.stories.js

@@ -55,3 +55,33 @@ export const DisabledAfterRefreshFailure = {
     }
   },
 };
+
+export const AltTextLimit = {
+  args: { images: [{ ...images[0], altText: 'card '.repeat(200) }] },
+  play: async ({ canvasElement }) => {
+    const canvas = within(canvasElement);
+    await userEvent.click(canvas.getByRole('button', { name: 'Edit alt text' }));
+    const input = canvas.getByLabelText('Alt text');
+    await expect(input).toHaveAttribute('maxlength', '1000');
+    await expect(input).toHaveAccessibleDescription(/up to 1,000 characters/);
+    await userEvent.type(input, 'extra');
+    await expect(input).toHaveValue('card '.repeat(200));
+    await userEvent.clear(input);
+    await userEvent.type(input, '  Front of card  ');
+    await userEvent.click(canvas.getByRole('button', { name: 'Save alt text' }));
+    await expect(input).toHaveValue('Front of card');
+  },
+};
+export const OversizedExistingAltText = {
+  args: { images: [{ ...images[0], altText: 'card '.repeat(201) }] },
+  play: async ({ canvasElement }) => {
+    const canvas = within(canvasElement);
+    await userEvent.click(canvas.getByRole('button', { name: 'Edit alt text' }));
+    await expect(canvas.getByRole('alert')).toHaveTextContent('Alt text must be 1,000 characters or fewer.');
+    await expect(canvas.getByLabelText('Alt text')).toHaveAttribute('aria-invalid', 'true');
+    await expect(canvas.getByRole('button', { name: 'Save alt text' })).toBeDisabled();
+    await userEvent.clear(canvas.getByLabelText('Alt text'));
+    await expect(canvas.getByRole('button', { name: 'Save alt text' })).toBeEnabled();
+    await expect(canvas.queryByRole('alert')).toBeNull();
+  },
+};

apps/web/src/components/organisms/AdminImageManager.vue

@@ -23,7 +23,10 @@ function beginEditAltText(image) {
 }
 
 function saveAltText(imageId) {
-  emit('alt-text', { id: imageId, altText: altTextDrafts[imageId] ?? '' });
+  const altText = (altTextDrafts[imageId] ?? '').trim();
+  if (props.isSaving || props.disabled || altText.length > 1000) return;
+  altTextDrafts[imageId] = altText;
+  emit('alt-text', { id: imageId, altText });
 }
 
 watch(() => props.images, (images) => {
@@ -85,11 +88,14 @@ function handleDrop(event) {
 
         <div class="admin-image-manager__row">
           <label class="fixture-label" :for="`admin-alt-text-${image.id}`">Alt text</label>
-          <input :id="`admin-alt-text-${image.id}`" v-model="altTextDrafts[image.id]" class="fixture-input" :disabled="isSaving || disabled || editingImageId !== image.id" />
+          <input :id="`admin-alt-text-${image.id}`" v-model="altTextDrafts[image.id]" class="fixture-input" maxlength="1000" :aria-describedby="`admin-alt-help-${image.id}${(altTextDrafts[image.id] ?? '').trim().length > 1000 ? ` admin-alt-error-${image.id}` : ''}`" :aria-invalid="(altTextDrafts[image.id] ?? '').trim().length > 1000" :disabled="isSaving || disabled || editingImageId !== image.id" />
           <button v-if="editingImageId !== image.id" class="button button--secondary" type="button" :disabled="isSaving || disabled" @click="beginEditAltText(image)">Edit alt text</button>
-          <button v-else class="button button--primary" type="button" :disabled="isSaving || disabled" @click="saveAltText(image.id)">Save alt text</button>
+          <button v-else class="button button--primary" type="button" :disabled="isSaving || disabled || (altTextDrafts[image.id] ?? '').trim().length > 1000" @click="saveAltText(image.id)">Save alt text</button>
         </div>
 
+        <p :id="`admin-alt-help-${image.id}`">Describe the image in up to 1,000 characters. Clear the field to remove its alt text.</p>
+        <p :id="`admin-alt-error-${image.id}`" v-if="(altTextDrafts[image.id] ?? '').trim().length > 1000" class="state state--error" role="alert">Alt text must be 1,000 characters or fewer.</p>
+
         <div class="admin-image-manager__row">
           <button class="button button--secondary" type="button" :disabled="isSaving || disabled || image.isPrimary" @click="emit('primary', image.id)">Make primary</button>
           <button class="button button--secondary" type="button" :disabled="isSaving || disabled || index === 0" @click="emit('move', { id: image.id, direction: 'up' })">Move up</button>