From c6b8bffc1f9d66c6a8bcebc00bb6ac9dcfde875c Mon Sep 17 00:00:00 2001 From: mrboxs Date: Thu, 12 Nov 2026 02:31:59 +0700 Subject: [PATCH 1/2] feat: upload Markdown images to S3-compatible storage --- apps/web/src/components/block-note/editor.tsx | 7 + .../src/components/block-note/upload-image.ts | 68 ++++++ .../src/components/public-content/image.tsx | 31 +++ .../src/components/public-content/index.tsx | 4 + apps/web/src/features/about/editor.tsx | 2 + apps/web/src/features/admin-blogs/editor.tsx | 2 + .../src/features/admin-collections/editor.tsx | 2 + .../components/create-project-form.tsx | 2 + .../components/edit-project-form.tsx | 3 + .../components/member-project-editor.tsx | 3 + apps/web/tests/block-note-import.test.tsx | 14 ++ .../tests/browser/content-images.browser.tsx | 141 +++++++++++ apps/web/tests/content-image-upload.test.ts | 128 ++++++++++ apps/web/tests/public-content.test.tsx | 27 +++ docs/development.md | 34 +++ packages/api/src/contracts/index.ts | 6 +- packages/api/src/contracts/upload.ts | 35 +++ packages/api/src/routers/index.ts | 6 +- packages/api/src/routers/upload.ts | 83 ++++++- packages/api/src/s3.ts | 2 + packages/api/src/storage.ts | 17 +- .../api/tests/content-image-upload.test.ts | 219 ++++++++++++++++++ .../api/tests/s3-presigned-upload.test.ts | 40 ++++ packages/shared/src/schemas/upload.ts | 29 +++ 24 files changed, 900 insertions(+), 5 deletions(-) create mode 100644 apps/web/src/components/block-note/upload-image.ts create mode 100644 apps/web/src/components/public-content/image.tsx create mode 100644 apps/web/tests/browser/content-images.browser.tsx create mode 100644 apps/web/tests/content-image-upload.test.ts create mode 100644 packages/api/tests/content-image-upload.test.ts create mode 100644 packages/api/tests/s3-presigned-upload.test.ts diff --git a/apps/web/src/components/block-note/editor.tsx b/apps/web/src/components/block-note/editor.tsx index e1a8099c..12309aab 100644 --- a/apps/web/src/components/block-note/editor.tsx +++ b/apps/web/src/components/block-note/editor.tsx @@ -68,8 +68,10 @@ import type { ReadmeImportMode, ReadmeImportPreview, } from '#/components/block-note/schema' +import { uploadEditorImage } from '#/components/block-note/upload-image' interface Props { + projectId?: string value?: string onChange?: (value: string) => void onBlur?: () => void @@ -78,6 +80,7 @@ interface Props { } export interface BlockNoteEditorHandle { + isUploading?: () => boolean // Runs the scheduled markdown export immediately. Call before reading // the field value (e.g. form submit) so the latest edits are included. // Safe outside BlockNote's change callback, where React is idle. @@ -95,6 +98,7 @@ export interface BlockNoteEditorHandle { } export default function BlockNoteEditor({ + projectId, value, onChange, onBlur, @@ -102,9 +106,11 @@ export default function BlockNoteEditor({ ref, }: Props) { const { resolvedTheme } = useTheme() + const pendingUploads = useRef(0) const editor = useCreateBlockNote({ schema: blockNoteSchema, extensions: contentExtensions(), + uploadFile: (file) => uploadEditorImage(file, projectId, pendingUploads), }) // Initial content loads once: re-running on every parent value change @@ -166,6 +172,7 @@ export default function BlockNoteEditor({ onChangeRef.current?.(editor.blocksToMarkdownLossy(editor.document)) } return { + isUploading: () => pendingUploads.current > 0, flush, readMarkdown: () => { flush() diff --git a/apps/web/src/components/block-note/upload-image.ts b/apps/web/src/components/block-note/upload-image.ts new file mode 100644 index 00000000..40f63d1d --- /dev/null +++ b/apps/web/src/components/block-note/upload-image.ts @@ -0,0 +1,68 @@ +import type { RefObject } from 'react' + +import { requestContentImageUploadInputSchema } from '@altstack/shared/schemas/upload' + +import { toast } from '@altstack/ui/components/toast' + +export async function uploadContentImage(file: File, projectId?: string) { + const parsed = requestContentImageUploadInputSchema.safeParse({ + body: { + filename: file.name.slice(0, 100), + contentType: file.type, + size: file.size, + projectId, + }, + }) + if (!parsed.success) { + throw new Error('Choose a PNG, JPEG, WebP or GIF image up to 5 MB.') + } + + const { client } = await import('#/utils/orpc') + const { key, presignedUrl } = await client.contentImageUpload.request( + parsed.data + ) + const response = await fetch(presignedUrl, { + method: 'PUT', + body: file, + headers: { 'Content-Type': parsed.data.body.contentType }, + }) + if (!response.ok) { + throw new Error('Image upload failed. Please try again.') + } + const { url } = await client.contentImageUpload.complete({ + body: { key, projectId }, + }) + return url +} + +export async function uploadEditorImage( + file: File, + projectId: string | undefined, + pendingUploads: RefObject +) { + pendingUploads.current += 1 + try { + return await uploadContentImage(file, projectId) + } catch (error) { + toast.add({ + type: 'error', + title: 'Could not upload image', + description: error instanceof Error ? error.message : 'Please try again.', + }) + throw error + } finally { + pendingUploads.current -= 1 + } +} + +export function canSaveEditorContent( + editor: { isUploading?: () => boolean } | null +) { + if (!editor?.isUploading?.()) return true + toast.add({ + type: 'error', + title: 'Image upload in progress', + description: 'Wait for the image upload to finish before saving.', + }) + return false +} diff --git a/apps/web/src/components/public-content/image.tsx b/apps/web/src/components/public-content/image.tsx new file mode 100644 index 00000000..7bb5406b --- /dev/null +++ b/apps/web/src/components/public-content/image.tsx @@ -0,0 +1,31 @@ +import { useState } from 'react' + +export function ContentImage({ + src, + alt = '', + title, +}: { + src?: string + alt?: string + title?: string +}) { + const [failedSource, setFailedSource] = useState() + if (!src || failedSource === src) { + return ( + + {alt ? `Image unavailable: ${alt}` : 'Image unavailable'} + + ) + } + return ( + {alt} setFailedSource(src)} + /> + ) +} diff --git a/apps/web/src/components/public-content/index.tsx b/apps/web/src/components/public-content/index.tsx index 778c9844..3d8e18b4 100644 --- a/apps/web/src/components/public-content/index.tsx +++ b/apps/web/src/components/public-content/index.tsx @@ -6,6 +6,7 @@ import remarkGfm from 'remark-gfm' import { PublicCodeBlock } from '#/components/public-content/code-block' import { ContentLink } from '#/components/public-content/content-link' import { ContentHeading } from '#/components/public-content/heading' +import { ContentImage } from '#/components/public-content/image' import { remarkArticleHeadings } from '#/features/public-content/headings' import { emptyPublicContent } from '#/features/public-content/model' import type { PreparedPublicContent } from '#/features/public-content/model' @@ -55,6 +56,9 @@ export function PublicContent({ } skipHtml components={{ + img: ({ src, alt, title }) => ( + + ), h1: ({ node, children, ...props }) => ( ) { event.preventDefault() if (busy) return + if (!canSaveEditorContent(editor.current)) return editor.current?.flush() const parsed = blogBodySchema.safeParse({ ...value, diff --git a/apps/web/src/features/admin-collections/editor.tsx b/apps/web/src/features/admin-collections/editor.tsx index a3026b55..1fa1da33 100644 --- a/apps/web/src/features/admin-collections/editor.tsx +++ b/apps/web/src/features/admin-collections/editor.tsx @@ -62,6 +62,7 @@ import { toast } from '@altstack/ui/components/toast' import { ActionConfirmation } from '#/components/action-confirmation' import BlockNoteEditor from '#/components/block-note/editor' import type { BlockNoteEditorHandle } from '#/components/block-note/editor' +import { canSaveEditorContent } from '#/components/block-note/upload-image' import { PublicContent } from '#/components/public-content' import { CollectionProjectSelector } from '#/features/admin-collections/project-selector' import type { SelectedProject } from '#/features/admin-collections/project-selector' @@ -205,6 +206,7 @@ function useCollectionEditor(initial: Initial | undefined) { async function save() { if (busy) return + if (!canSaveEditorContent(editor.current)) return editor.current?.flush() const parsed = collectionBodySchema.safeParse({ ...value, diff --git a/apps/web/src/features/admin-projects/components/create-project-form.tsx b/apps/web/src/features/admin-projects/components/create-project-form.tsx index ce7b9b3a..97e1914e 100644 --- a/apps/web/src/features/admin-projects/components/create-project-form.tsx +++ b/apps/web/src/features/admin-projects/components/create-project-form.tsx @@ -30,6 +30,7 @@ import { toast } from '@altstack/ui/components/toast' import BlockNoteEditor from '#/components/block-note/editor' import type { BlockNoteEditorHandle } from '#/components/block-note/editor' +import { canSaveEditorContent } from '#/components/block-note/upload-image' import { LogoUploader, ScreenshotUploader } from '#/components/image-uploader' import { GithubMetadataPrefill } from '#/features/admin-projects/components/github-metadata-prefill' import { GithubReadmeImport } from '#/features/admin-projects/components/github-readme-import' @@ -182,6 +183,7 @@ export function CreateProjectForm() { className="min-w-0" onSubmit={(e) => { e.preventDefault() + if (!canSaveEditorContent(editorRef.current)) return editorRef.current?.flush() void form.handleSubmit() }} diff --git a/apps/web/src/features/admin-projects/components/edit-project-form.tsx b/apps/web/src/features/admin-projects/components/edit-project-form.tsx index 177aaf0c..c26cf1ed 100644 --- a/apps/web/src/features/admin-projects/components/edit-project-form.tsx +++ b/apps/web/src/features/admin-projects/components/edit-project-form.tsx @@ -30,6 +30,7 @@ import { toast } from '@altstack/ui/components/toast' import type { BlockNoteEditorHandle } from '#/components/block-note/editor' import BlockNoteEditor from '#/components/block-note/editor' +import { canSaveEditorContent } from '#/components/block-note/upload-image' import { LogoUploader, ScreenshotUploader } from '#/components/image-uploader' import { GithubMetadataPrefill } from '#/features/admin-projects/components/github-metadata-prefill' import { GithubReadmeImport } from '#/features/admin-projects/components/github-readme-import' @@ -245,6 +246,7 @@ export function EditProjectForm({ project }: { project: AdminProject }) { className="min-w-0" onSubmit={(e) => { e.preventDefault() + if (!canSaveEditorContent(editorRef.current)) return editorRef.current?.flush() void form.handleSubmit() }} @@ -278,6 +280,7 @@ export function EditProjectForm({ project }: { project: AdminProject }) { disabled={false} pending={isSubmitting} onAction={(status) => { + if (!canSaveEditorContent(editorRef.current)) return form.setFieldValue('status', status) editorRef.current?.flush() void form.handleSubmit() diff --git a/apps/web/src/features/project/components/member-project-editor.tsx b/apps/web/src/features/project/components/member-project-editor.tsx index 7d4bd3ed..129cdbc8 100644 --- a/apps/web/src/features/project/components/member-project-editor.tsx +++ b/apps/web/src/features/project/components/member-project-editor.tsx @@ -16,6 +16,7 @@ import { toast } from '@altstack/ui/components/toast' import BlockNoteEditor from '#/components/block-note/editor' import type { BlockNoteEditorHandle } from '#/components/block-note/editor' +import { canSaveEditorContent } from '#/components/block-note/upload-image' import { CategoryComboboxView } from '#/components/category-combobox-view' import { LogoUploader, ScreenshotUploader } from '#/components/image-uploader' import { memberUploadApi, ownershipKey } from '#/features/project/ownership' @@ -60,6 +61,7 @@ export function MemberProjectEditor({ async function save(event: React.SyntheticEvent) { event.preventDefault() if (saving.current) return + if (!canSaveEditorContent(editor.current)) return const parsed = memberProjectUpdateBodySchema.safeParse({ revision: project.revision, name, @@ -198,6 +200,7 @@ export function MemberProjectEditor({ Project content { editor._tiptapEditor.destroy() } }) + it('round-trips permanent image URLs and captions through saved Markdown', () => { + const editor = CoreEditor.create({ schema: blockNoteSchema }) + const url = 'https://cdn.test/content/images/diagram.png' + editor.replaceBlocks(editor.document, [ + { type: 'image', props: { url, caption: 'Architecture diagram' } }, + ]) + const markdown = editor.blocksToMarkdownLossy(editor.document) + expect(markdown).toContain(url) + const image = editor + .tryParseMarkdownToBlocks(markdown) + .find((block) => block.type === 'image') + expect(image?.props.url).toBe(url) + expect(image?.props.caption).toBe('Architecture diagram') + }) it('empty conversion fails without replacing existing document or form content', () => { const ref = createRef() const change = vi.fn() diff --git a/apps/web/tests/browser/content-images.browser.tsx b/apps/web/tests/browser/content-images.browser.tsx new file mode 100644 index 00000000..42aefa94 --- /dev/null +++ b/apps/web/tests/browser/content-images.browser.tsx @@ -0,0 +1,141 @@ +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from '@testing-library/react' +import { createRef } from 'react' +import { afterEach, beforeEach, expect, it, vi } from 'vite-plus/test' +import { page } from 'vite-plus/test/browser' + +import { ThemeProvider } from '@altstack/ui/components/customs/theme-provider' + +import BlockNoteEditor from '#/components/block-note/editor' +import type { BlockNoteEditorHandle } from '#/components/block-note/editor' +import type { ContentEditor } from '#/components/block-note/schema' +import { canSaveEditorContent } from '#/components/block-note/upload-image' +import { PublicContent } from '#/components/public-content' +import '#/styles.css' + +const { request, complete } = vi.hoisted(() => { + return { + request: vi.fn(), + complete: vi.fn(), + } +}) +vi.mock('#/utils/orpc', () => { + return { + client: { contentImageUpload: { request, complete } }, + } +}) +vi.mock('@altstack/ui/components/toast', () => { + return { toast: { add: vi.fn() } } +}) +vi.mock('#/features/public-content/server-function', () => { + return { + preparePublicContent: vi.fn(), + } +}) +vi.mock('@altstack/env/web', () => { + return { + env: { + VITE_APP_URL: 'https://app.test', + VITE_S3_PUBLIC_URL: 'https://cdn.test', + }, + } +}) +vi.mock('#/components/block-note/project-mention-menu', () => { + return { + ProjectMentionMenus: ({ editor }: { editor: ContentEditor }) => ( + + ), + } +}) + +beforeEach(() => { + vi.resetAllMocks() + request.mockResolvedValue({ + key: 'tmp/content-images/test.png', + presignedUrl: 'https://s3.test/signed', + }) + const originalFetch = globalThis.fetch + vi.spyOn(globalThis, 'fetch').mockImplementation((input, init) => { + if (input === 'https://s3.test/signed') { + return Promise.resolve(new Response(null, { status: 200 })) + } + return originalFetch(input, init) + }) +}) +afterEach(() => { + cleanup() + vi.restoreAllMocks() + document.documentElement.classList.remove('dark') +}) + +it.each([ + [390, 'light'], + [1440, 'dark'], +] as const)( + 'uploads through the real BlockNote file panel and renders saved Markdown at %ipx in %s mode', + async (width, theme) => { + await page.viewport(width, 900) + localStorage.setItem('content-image-theme', theme) + const ref = createRef() + render( + + + + ) + await page.getByRole('button', { name: 'Insert test image block' }).click() + await page.getByText('Add image', { exact: true }).click() + await page.getByRole('tab', { name: 'Upload', exact: true }).click() + const input = + document.querySelector('input[type="file"]')! + expect(input).toBeTruthy() + let finish!: (value: { url: string }) => void + complete.mockImplementation( + () => + new Promise((resolve) => { + finish = resolve + }) + ) + const bytes = Uint8Array.from( + atob( + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAusB9Wl6EJkAAAAASUVORK5CYII=' + ), + (char) => char.charCodeAt(0) + ) + const file = new File([bytes], 'diagram.png', { type: 'image/png' }) + const transfer = new DataTransfer() + transfer.items.add(file) + input.files = transfer.files + fireEvent.change(input) + await waitFor(() => expect(complete).toHaveBeenCalled()) + expect(canSaveEditorContent(ref.current)).toBe(false) + const url = new URL('/og/default.png', window.location.href).href + finish({ url }) + await waitFor(() => + expect( + document.querySelector('.bn-editor img')?.getAttribute('src') + ).toBe(url) + ) + expect(canSaveEditorContent(ref.current)).toBe(true) + const markdown = ref.current!.readMarkdown() + expect(markdown).toContain(url) + expect(markdown).not.toContain('tmp/content-images') + cleanup() + render() + const image = screen.getByRole('img') as HTMLImageElement + await waitFor(() => expect(image.naturalWidth).toBeGreaterThan(0)) + expect(image.getBoundingClientRect().width).toBeLessThanOrEqual(width) + expect(document.documentElement.scrollWidth).toBeLessThanOrEqual(width) + } +) diff --git a/apps/web/tests/content-image-upload.test.ts b/apps/web/tests/content-image-upload.test.ts new file mode 100644 index 00000000..821c70c2 --- /dev/null +++ b/apps/web/tests/content-image-upload.test.ts @@ -0,0 +1,128 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from 'vite-plus/test' + +import { + uploadContentImage, + uploadEditorImage, + canSaveEditorContent, +} from '#/components/block-note/upload-image' + +const { request, complete, toast } = vi.hoisted(() => { + return { + request: vi.fn(), + complete: vi.fn(), + toast: vi.fn(), + } +}) +vi.mock('#/utils/orpc', () => { + return { + client: { contentImageUpload: { request, complete } }, + } +}) +vi.mock('@altstack/ui/components/toast', () => { + return { toast: { add: toast } } +}) + +beforeEach(() => { + vi.resetAllMocks() + request.mockResolvedValue({ + key: 'tmp/content-images/test.png', + presignedUrl: 'https://s3.test/signed', + }) + complete.mockResolvedValue({ + url: 'https://cdn.test/content/images/test.png', + }) + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true })) +}) +afterEach(() => vi.unstubAllGlobals()) + +describe('Markdown image upload client', () => { + it('uploads using the signed PUT then returns only the permanent URL', async () => { + const file = new File(['png'], 'image.png', { type: 'image/png' }) + expect(await uploadContentImage(file)).toBe( + 'https://cdn.test/content/images/test.png' + ) + }) + it('passes project scope and the declared image type to the API and storage', async () => { + const projectId = '550e8400-e29b-41d4-a716-446655440004' + const file = new File(['png'], 'image.png', { type: 'image/png' }) + await uploadContentImage(file, projectId) + expect(request).toHaveBeenCalledWith({ + body: { + filename: 'image.png', + contentType: 'image/png', + size: 3, + projectId, + }, + }) + expect(fetch).toHaveBeenCalledWith('https://s3.test/signed', { + method: 'PUT', + body: file, + headers: { 'Content-Type': 'image/png' }, + }) + expect(complete).toHaveBeenCalledWith({ + body: { key: 'tmp/content-images/test.png', projectId }, + }) + }) + it('does not publish a failed PUT and exposes completion failures', async () => { + const file = new File(['png'], 'image.png', { type: 'image/png' }) + vi.mocked(fetch).mockResolvedValueOnce({ ok: false } as Response) + await expect(uploadContentImage(file)).rejects.toThrow( + 'Image upload failed' + ) + expect(complete).not.toHaveBeenCalled() + complete.mockRejectedValueOnce(new Error('Storage unavailable')) + await expect(uploadContentImage(file)).rejects.toThrow( + 'Storage unavailable' + ) + }) + it.each([ + new File(['svg'], 'image.svg', { type: 'image/svg+xml' }), + new File([], 'empty.png', { type: 'image/png' }), + new File([new Uint8Array(5 * 1024 * 1024 + 1)], 'huge.png', { + type: 'image/png', + }), + ])( + 'validates image type and size before requesting storage access: $name', + async (file) => { + await expect(uploadContentImage(file)).rejects.toThrow('up to 5 MB') + expect(request).not.toHaveBeenCalled() + expect(fetch).not.toHaveBeenCalled() + } + ) + it('prevents saving during an upload and allows saving when it finishes', () => { + expect(canSaveEditorContent({ isUploading: () => true })).toBe(false) + expect(toast).toHaveBeenCalledWith( + expect.objectContaining({ title: 'Image upload in progress' }) + ) + expect(canSaveEditorContent({ isUploading: () => false })).toBe(true) + }) + + it('tracks concurrent uploads and clears pending state after failures', async () => { + const pending = { current: 0 } + let finish!: (value: { url: string }) => void + complete.mockImplementationOnce( + () => + new Promise((resolve) => { + finish = resolve + }) + ) + const file = new File(['png'], 'image.png', { type: 'image/png' }) + const first = uploadEditorImage(file, undefined, pending) + expect(pending.current).toBe(1) + const failed = uploadEditorImage( + new File(['svg'], 'bad.svg', { type: 'image/svg+xml' }), + undefined, + pending + ) + await expect(failed).rejects.toThrow('up to 5 MB') + expect(pending.current).toBe(1) + await vi.waitFor(() => expect(complete).toHaveBeenCalled()) + finish({ url: 'https://cdn.test/image.png' }) + await expect(first).resolves.toBe('https://cdn.test/image.png') + expect(pending.current).toBe(0) + expect(toast).toHaveBeenCalledWith( + expect.objectContaining({ title: 'Could not upload image' }) + ) + }) +}) diff --git a/apps/web/tests/public-content.test.tsx b/apps/web/tests/public-content.test.tsx index d3d6857a..a2decd91 100644 --- a/apps/web/tests/public-content.test.tsx +++ b/apps/web/tests/public-content.test.tsx @@ -91,6 +91,33 @@ async function documentRouter( } describe('public Markdown rendering', () => { + it('renders permanent storage images with alt text, title and responsive lazy loading', async () => { + const router = await documentRouter( + '![Architecture](https://storage.test/content/images/diagram.png "System diagram")' + ) + render() + const image = screen.getByRole('img', { name: 'Architecture' }) + expect(image.getAttribute('src')).toBe( + 'https://storage.test/content/images/diagram.png' + ) + expect(image.getAttribute('title')).toBe('System diagram') + expect(image.getAttribute('loading')).toBe('lazy') + expect(image.getAttribute('decoding')).toBe('async') + expect(image.classList.contains('max-w-full')).toBe(true) + fireEvent.error(image) + expect(screen.getByText('Image unavailable: Architecture')).toBeTruthy() + expect(screen.queryByRole('img', { name: 'Architecture' })).toBeNull() + }) + + it('does not render unsafe image URLs or raw image HTML', async () => { + const router = await documentRouter( + '![Unsafe](javascript:alert%281%29)\n\n' + ) + render() + expect(document.querySelector('article img')).toBeNull() + expect(screen.getByText('Image unavailable: Unsafe')).toBeTruthy() + }) + it('decorates ordinary web links without changing their names or destinations', async () => { const router = await documentRouter( '[internal](/about?ref=docs#team) [same origin](https://altstack.test/blogs) [relative](./guide) [external](https://docs.example.com:8443/guide?ref=docs#setup) [protocol relative](//docs.example.com/other)' diff --git a/docs/development.md b/docs/development.md index 049d0a1b..9b4e3aa6 100644 --- a/docs/development.md +++ b/docs/development.md @@ -113,6 +113,40 @@ web origins and upload methods/headers used by [storage.ts](../packages/api/src/storage.ts). Configure lifecycle cleanup for temporary `tmp/*` uploads; keep database backups in separate private storage. +Markdown editors accept PNG, JPEG, WebP and GIF images up to 5 MB through the +BlockNote image block (`/image`, paste or drag and drop). The authenticated API +issues a ten-minute presigned PUT URL, verifies the stored size and content type, +then copies the image into `content/images/*`. Markdown stores the permanent +public URL, so viewing public content does not need a session or an expiring +signed GET URL. Admins can upload in all content editors; project members need +current editing access to their published project. Upload requests are limited +to 40 per account per ten minutes. Saving is blocked while an upload is pending. + +For R2, enable a public custom domain for the bucket and use it as both +`S3_PUBLIC_URL` and `VITE_S3_PUBLIC_URL`. The S3 API endpoint is for signed uploads, +not public image delivery. For local development, an example bucket CORS policy is: + +```json +[ + { + "AllowedOrigins": ["http://localhost:3010"], + "AllowedMethods": ["PUT"], + "AllowedHeaders": ["Content-Type"], + "MaxAgeSeconds": 3600 + } +] +``` + +Replace the origin with your deployed web origin in production. Keep lifecycle +expiry scoped to `tmp/`; never expire `content/images/`. Completion keeps the +temporary source until lifecycle cleanup so retries use the same permanent URL. +Published images are retained when removed from a document because another +document may reference them. Images uploaded into abandoned drafts also remain; +permanent unreferenced image cleanup is manual. + +References: [BlockNote S3 uploads](https://www.blocknotejs.org/examples/backend/s3) +and [R2 bucket CORS](https://developers.cloudflare.com/r2/buckets/cors/). + **Email:** configure a Resend-verified sender domain and both email credentials. GitHub-only local sign-in can run without them; email requests return a generic retryable error until configured. Disable click tracking for authentication diff --git a/packages/api/src/contracts/index.ts b/packages/api/src/contracts/index.ts index e5466c9b..25b361ea 100644 --- a/packages/api/src/contracts/index.ts +++ b/packages/api/src/contracts/index.ts @@ -13,7 +13,10 @@ import { projectContract } from '@altstack/api/contracts/project' import { reportContract } from '@altstack/api/contracts/report' import { submissionContract } from '@altstack/api/contracts/submission' import { tagContract, adminTagContract } from '@altstack/api/contracts/tag' -import { uploadContract } from '@altstack/api/contracts/upload' +import { + uploadContract, + contentImageUploadContract, +} from '@altstack/api/contracts/upload' import { aboutContract, adminAboutContract } from './about' import { adBillingContract, adminAdBillingContract } from './ad-billing' @@ -23,6 +26,7 @@ import { collectionContract, adminCollectionContract } from './collection' import { ownershipContract } from './ownership' export const contracts = { + contentImageUpload: contentImageUploadContract, about: aboutContract, adBilling: adBillingContract, ads: adsContract, diff --git a/packages/api/src/contracts/upload.ts b/packages/api/src/contracts/upload.ts index 3d0430d9..21d1e757 100644 --- a/packages/api/src/contracts/upload.ts +++ b/packages/api/src/contracts/upload.ts @@ -3,6 +3,10 @@ import { openapi } from '@orpc/openapi' import { baseContract } from '@altstack/api/contracts/base' import { + requestContentImageUploadInputSchema, + requestContentImageUploadOutputSchema, + completeContentImageUploadInputSchema, + completeContentImageUploadOutputSchema, changeLogoUploadInputSchema, changeLogoUploadOutputSchema, changeScreenshotUploadInputSchema, @@ -17,6 +21,37 @@ import { requestScreenshotUploadOutputSchema, } from '@altstack/shared' +export const contentImageUploadContract = { + request: baseContract + .meta( + openapi({ + path: '/uploads/content-image', + method: 'POST', + inputStructure: 'detailed', + outputStructure: 'compact', + summary: 'Request a Markdown image upload URL', + tags: ['Uploads'], + operationId: 'requestContentImageUpload', + }) + ) + .input(requestContentImageUploadInputSchema) + .output(requestContentImageUploadOutputSchema), + complete: baseContract + .meta( + openapi({ + path: '/uploads/content-image/complete', + method: 'POST', + inputStructure: 'detailed', + outputStructure: 'compact', + summary: 'Verify and publish a Markdown image upload', + tags: ['Uploads'], + operationId: 'completeContentImageUpload', + }) + ) + .input(completeContentImageUploadInputSchema) + .output(completeContentImageUploadOutputSchema), +} as const + const requestLogoUploadContract = baseContract .meta( openapi({ diff --git a/packages/api/src/routers/index.ts b/packages/api/src/routers/index.ts index d321f1cb..0d89b920 100644 --- a/packages/api/src/routers/index.ts +++ b/packages/api/src/routers/index.ts @@ -20,7 +20,10 @@ import { projectRouter } from '@altstack/api/routers/project' import { reportRouter } from '@altstack/api/routers/report' import { submissionRouter } from '@altstack/api/routers/submission' import { tagRouter, adminTagRouter } from '@altstack/api/routers/tag' -import { uploadRouter } from '@altstack/api/routers/upload' +import { + uploadRouter, + contentImageUploadRouter, +} from '@altstack/api/routers/upload' import { aboutRouter, adminAboutRouter } from './about' import { adBillingRouter, adminAdBillingRouter } from './ad-billing' @@ -30,6 +33,7 @@ import { collectionRouter, adminCollectionRouter } from './collection' import { ownershipRouter } from './ownership' export const routers = o.router({ + contentImageUpload: contentImageUploadRouter, about: aboutRouter, ads: adsRouter, adBilling: adBillingRouter, diff --git a/packages/api/src/routers/upload.ts b/packages/api/src/routers/upload.ts index f9e81dd5..6a02b5bb 100644 --- a/packages/api/src/routers/upload.ts +++ b/packages/api/src/routers/upload.ts @@ -5,9 +5,20 @@ import { } from '@aws-sdk/client-s3' import { getSignedUrl } from '@aws-sdk/s3-request-presigner' import type { ORPCErrorConstructorMap } from '@orpc/server' +import { createHash, randomUUID } from 'node:crypto' -import { adminProcedure } from '@altstack/api/procedures' -import { s3, S3_BUCKET } from '@altstack/api/s3' +import { enforceClaimQuota } from '@altstack/api/ownership-otp' +import { adminProcedure, protectedProcedure } from '@altstack/api/procedures' +import { requireActiveAccount } from '@altstack/api/queries/account-access' +import { requireProjectMember } from '@altstack/api/queries/ownership' +import { s3, S3_BUCKET, publicUrlForKey } from '@altstack/api/s3' +import { + promoteContentImage, + InvalidTempUploadError, + TempUploadMissingError, +} from '@altstack/api/storage' + +import { env } from '@altstack/env/server' import { TMP_LOGO_PREFIX, @@ -192,3 +203,71 @@ export const uploadRouter = { change: changeScreenshotUploadHandler, }, } + +const contentImageProcedure = protectedProcedure.use( + async ({ context, next, errors }) => { + const origin = context.requestHeaders?.get('origin') + if ( + (origin && !env.CORS_ORIGINS.includes(origin)) || + context.requestHeaders?.get('sec-fetch-site') === 'cross-site' + ) { + throw errors.FORBIDDEN() + } + const account = await requireActiveAccount(context.db, context.auth.user.id) + return next({ context: { account } }) + } +) + +function contentImagePrefix(userId: string, projectId?: string) { + const scope = createHash('sha256') + .update(JSON.stringify([userId, projectId ?? null])) + .digest('hex') + return `tmp/content-images/${scope}/` +} + +export const contentImageUploadRouter = { + request: contentImageProcedure.contentImageUpload.request.handler( + async ({ context: { db, auth, account }, input: { body }, errors }) => { + if (account.role !== 'admin') { + if (!body.projectId) throw errors.FORBIDDEN() + await requireProjectMember(db, body.projectId, auth.user.id) + } + await enforceClaimQuota(db, `content-image:${auth.user.id}`, 40) + const key = `${contentImagePrefix(auth.user.id, body.projectId)}${randomUUID()}.${EXT_BY_MIME[body.contentType]}` + const presignedUrl = await getSignedUrl( + s3, + new PutObjectCommand({ + Bucket: S3_BUCKET, + Key: key, + ContentType: body.contentType, + ContentLength: body.size, + }), + { expiresIn: 600, signableHeaders: new Set(['content-type']) } + ) + return { key, presignedUrl } + } + ), + complete: contentImageProcedure.contentImageUpload.complete.handler( + async ({ context: { db, auth, account }, input: { body }, errors }) => { + if (account.role !== 'admin') { + if (!body.projectId) throw errors.FORBIDDEN() + await requireProjectMember(db, body.projectId, auth.user.id) + } + if ( + !body.key.startsWith(contentImagePrefix(auth.user.id, body.projectId)) + ) { + throw errors.NOT_FOUND() + } + try { + const key = await promoteContentImage(body.key) + return { url: publicUrlForKey(key) } + } catch (error) { + if (error instanceof TempUploadMissingError) { + throw errors.UPLOAD_EXPIRED() + } + if (error instanceof InvalidTempUploadError) throw errors.BAD_REQUEST() + throw errors.INTERNAL_SERVER_ERROR() + } + } + ), +} diff --git a/packages/api/src/s3.ts b/packages/api/src/s3.ts index ae648eb3..96d699fd 100644 --- a/packages/api/src/s3.ts +++ b/packages/api/src/s3.ts @@ -10,6 +10,8 @@ export const s3 = new S3Client({ secretAccessKey: env.S3_SECRET_ACCESS_KEY, }, forcePathStyle: false, + // Presigned PUTs have no body here; the browser supplies it after signing. + requestChecksumCalculation: 'WHEN_REQUIRED', }) export const S3_BUCKET = env.S3_BUCKET diff --git a/packages/api/src/storage.ts b/packages/api/src/storage.ts index 7aa8e312..7728f6d7 100644 --- a/packages/api/src/storage.ts +++ b/packages/api/src/storage.ts @@ -16,6 +16,7 @@ import { } from '@altstack/shared/constants' import { blogCoverKeySchema } from '@altstack/shared/schemas/blog' import { + contentImageUploadKeySchema, projectLogoKeySchema, projectScreenshotKeySchema, screenshotKeySchema, @@ -176,6 +177,20 @@ export async function promoteTempImageToBlogCover({ }) } +export async function promoteContentImage(tmpKey: string) { + if (!contentImageUploadKeySchema.safeParse(tmpKey).success) { + throw new StorageError('Invalid content image upload.') + } + // Keep the same destination on retries, including a lost completion response. + const finalKey = tmpKey.replace('tmp/content-images/', 'content/images/') + return promoteTempImage({ + tmpKey, + finalKey, + kind: 'screenshot', + consumeSource: false, + }) +} + export async function promoteTempImageToAd( tmpKey: string, adId: string, @@ -216,7 +231,7 @@ async function promoteTempImage({ const allowedMime = ALLOWED_MIME_BY_KIND[kind] as ReadonlyArray const storedSize = head.ContentLength const storedType = head.ContentType - const sizeOk = storedSize != null && storedSize <= maxSize + const sizeOk = storedSize != null && storedSize > 0 && storedSize <= maxSize const typeOk = !!storedType && allowedMime.includes(storedType) if (!sizeOk || !typeOk) { await s3 diff --git a/packages/api/tests/content-image-upload.test.ts b/packages/api/tests/content-image-upload.test.ts new file mode 100644 index 00000000..3e51568e --- /dev/null +++ b/packages/api/tests/content-image-upload.test.ts @@ -0,0 +1,219 @@ +import { + CopyObjectCommand, + DeleteObjectCommand, + HeadObjectCommand, + PutObjectCommand, +} from '@aws-sdk/client-s3' +import { getSignedUrl } from '@aws-sdk/s3-request-presigner' +import { createRouterClient, ORPCError } from '@orpc/server' +import { beforeEach, describe, expect, it, vi } from 'vite-plus/test' + +import type { ORPCContext } from '@altstack/api/context' +import { enforceClaimQuota } from '@altstack/api/ownership-otp' +import { requireActiveAccount } from '@altstack/api/queries/account-access' +import { requireProjectMember } from '@altstack/api/queries/ownership' +import { contentImageUploadRouter } from '@altstack/api/routers/upload' + +const { send } = vi.hoisted(() => { + return { send: vi.fn() } +}) +vi.mock('@altstack/api/s3', () => { + return { + s3: { send }, + S3_BUCKET: 'test-bucket', + publicUrlForKey: (key: string) => `https://cdn.test/${key}`, + } +}) +vi.mock('@altstack/env/server', () => { + return { + env: { CORS_ORIGINS: ['https://app.test'] }, + } +}) +vi.mock('@aws-sdk/s3-request-presigner', () => { + return { getSignedUrl: vi.fn() } +}) +vi.mock('@altstack/api/ownership-otp', () => { + return { enforceClaimQuota: vi.fn() } +}) +vi.mock('@altstack/api/queries/account-access', () => { + return { + requireActiveAccount: vi.fn(), + } +}) +vi.mock('@altstack/api/queries/ownership', () => { + return { + requireProjectMember: vi.fn(), + } +}) + +const projectId = '550e8400-e29b-41d4-a716-446655440004' +const body = { + filename: 'diagram.png', + contentType: 'image/png' as const, + size: 1024, +} +const database = {} as ORPCContext['db'] +function client( + role: 'admin' | 'user' | null = 'admin', + id = 'editor', + headers?: Headers +) { + return createRouterClient(contentImageUploadRouter, { + context: { + db: database, + auth: role ? ({ user: { id, role } } as ORPCContext['auth']) : null, + requestHeaders: headers, + }, + }) +} + +beforeEach(() => { + vi.resetAllMocks() + vi.mocked(requireActiveAccount).mockResolvedValue({ + id: 'editor', + role: 'admin', + }) + vi.mocked(getSignedUrl).mockResolvedValue('https://s3.test/signed') + send.mockResolvedValue({ ContentLength: 1024, ContentType: 'image/png' }) +}) + +describe('content image uploads', () => { + it('signs unique temporary keys and publishes a stable public URL after verification', async () => { + const rpc = client() + const upload = await rpc.request({ body }) + const other = await rpc.request({ body }) + expect(upload.key).not.toBe(other.key) + expect(upload.key).toMatch( + /^tmp\/content-images\/[a-f0-9]{64}\/[a-f0-9-]{36}\.png$/ + ) + const command = vi.mocked(getSignedUrl).mock.calls[0]?.[1] + expect(command).toBeInstanceOf(PutObjectCommand) + expect(command?.input).toMatchObject({ + Bucket: 'test-bucket', + Key: upload.key, + ContentLength: 1024, + ContentType: 'image/png', + }) + expect(enforceClaimQuota).toHaveBeenCalledWith( + database, + 'content-image:editor', + 40 + ) + expect(vi.mocked(getSignedUrl).mock.calls[0]?.[2]).toEqual({ + expiresIn: 600, + signableHeaders: new Set(['content-type']), + }) + const result = await rpc.complete({ body: { key: upload.key } }) + expect(result.url).toBe( + `https://cdn.test/${upload.key.replace('tmp/content-images/', 'content/images/')}` + ) + expect(result.url).not.toContain('signed') + expect(send.mock.calls[0]?.[0]).toBeInstanceOf(HeadObjectCommand) + expect(send.mock.calls[1]?.[0]).toBeInstanceOf(CopyObjectCommand) + expect( + send.mock.calls.some(([cmd]) => cmd instanceof DeleteObjectCommand) + ).toBe(false) + expect(await rpc.complete({ body: { key: upload.key } })).toEqual(result) + }) + + it('rejects anonymous callers, non-editors and cross-site requests before signing', async () => { + await expect(client(null).request({ body })).rejects.toMatchObject({ + code: 'UNAUTHORIZED', + }) + vi.mocked(requireActiveAccount).mockResolvedValue({ + id: 'editor', + role: 'user', + }) + await expect(client('user').request({ body })).rejects.toMatchObject({ + code: 'FORBIDDEN', + }) + await expect( + client( + 'user', + 'editor', + new Headers({ origin: 'https://evil.test' }) + ).request({ body: { ...body, projectId } }) + ).rejects.toMatchObject({ code: 'FORBIDDEN' }) + expect(getSignedUrl).not.toHaveBeenCalled() + }) + + it('checks active project membership again on completion', async () => { + vi.mocked(requireActiveAccount).mockResolvedValue({ + id: 'editor', + role: 'user', + }) + const rpc = client('user') + const upload = await rpc.request({ body: { ...body, projectId } }) + expect(requireProjectMember).toHaveBeenCalledWith( + database, + projectId, + 'editor' + ) + vi.mocked(requireProjectMember).mockRejectedValue( + new ORPCError('NOT_FOUND') + ) + await expect( + rpc.complete({ body: { key: upload.key, projectId } }) + ).rejects.toMatchObject({ code: 'NOT_FOUND' }) + expect(send).not.toHaveBeenCalled() + }) + + it('does not complete uploads belonging to another user or project scope', async () => { + const upload = await client().request({ body: { ...body, projectId } }) + await expect( + client('admin', 'another').complete({ + body: { key: upload.key, projectId }, + }) + ).rejects.toMatchObject({ code: 'NOT_FOUND' }) + await expect( + client().complete({ body: { key: upload.key } }) + ).rejects.toMatchObject({ code: 'NOT_FOUND' }) + expect(send).not.toHaveBeenCalled() + }) + + it.each([ + { contentType: 'image/svg+xml' }, + { contentType: 'text/html' }, + { size: 0 }, + { size: 5 * 1024 * 1024 + 1 }, + ])('rejects unsupported uploads before signing: %j', async (patch) => { + await expect( + client().request({ body: { ...body, ...patch } as typeof body }) + ).rejects.toMatchObject({ code: 'BAD_REQUEST' }) + expect(getSignedUrl).not.toHaveBeenCalled() + }) + + it.each([ + { ContentLength: 0, ContentType: 'image/png' }, + { ContentLength: 6 * 1024 * 1024, ContentType: 'image/png' }, + { ContentLength: 1024, ContentType: 'text/html' }, + ])( + 'rejects invalid stored metadata before publishing: %j', + async (metadata) => { + const upload = await client().request({ body }) + send.mockResolvedValueOnce(metadata) + await expect( + client().complete({ body: { key: upload.key } }) + ).rejects.toMatchObject({ code: 'BAD_REQUEST' }) + expect( + send.mock.calls.some(([cmd]) => cmd instanceof CopyObjectCommand) + ).toBe(false) + } + ) + + it('reports an expired upload and blocks signing when quota is exhausted', async () => { + const upload = await client().request({ body }) + send.mockRejectedValueOnce({ $metadata: { httpStatusCode: 404 } }) + await expect( + client().complete({ body: { key: upload.key } }) + ).rejects.toMatchObject({ code: 'UPLOAD_EXPIRED' }) + vi.mocked(enforceClaimQuota).mockRejectedValue( + new ORPCError('TOO_MANY_REQUESTS') + ) + vi.mocked(getSignedUrl).mockClear() + await expect(client().request({ body })).rejects.toMatchObject({ + code: 'TOO_MANY_REQUESTS', + }) + expect(getSignedUrl).not.toHaveBeenCalled() + }) +}) diff --git a/packages/api/tests/s3-presigned-upload.test.ts b/packages/api/tests/s3-presigned-upload.test.ts new file mode 100644 index 00000000..13e23a0e --- /dev/null +++ b/packages/api/tests/s3-presigned-upload.test.ts @@ -0,0 +1,40 @@ +import { PutObjectCommand } from '@aws-sdk/client-s3' +import { getSignedUrl } from '@aws-sdk/s3-request-presigner' +import { expect, it, vi } from 'vite-plus/test' + +import { s3 } from '@altstack/api/s3' + +vi.mock('@altstack/env/server', () => { + return { + env: { + S3_REGION: 'auto', + S3_ENDPOINT: 'https://example.r2.cloudflarestorage.com', + S3_BUCKET: 'test-bucket', + S3_PUBLIC_URL: 'https://cdn.test', + S3_ACCESS_KEY_ID: 'synthetic-test-access', + S3_SECRET_ACCESS_KEY: 'synthetic-test-secret', + }, + } +}) + +it('signs browser PUTs without a checksum for the absent signing-time body', async () => { + const url = new URL( + await getSignedUrl( + s3, + new PutObjectCommand({ + Bucket: 'test-bucket', + Key: 'image.png', + ContentLength: 3, + ContentType: 'image/png', + }), + { expiresIn: 600, signableHeaders: new Set(['content-type']) } + ) + ) + expect(url.searchParams.has('x-amz-checksum-crc32')).toBe(false) + expect(url.searchParams.has('x-amz-sdk-checksum-algorithm')).toBe(false) + expect(url.searchParams.get('X-Amz-SignedHeaders')).toContain( + 'content-length' + ) + expect(url.searchParams.get('X-Amz-SignedHeaders')).toContain('content-type') + expect(url.searchParams.get('X-Amz-Expires')).toBe('600') +}) diff --git a/packages/shared/src/schemas/upload.ts b/packages/shared/src/schemas/upload.ts index 4800a8cb..d39ec3e4 100644 --- a/packages/shared/src/schemas/upload.ts +++ b/packages/shared/src/schemas/upload.ts @@ -119,3 +119,32 @@ export const changeScreenshotUploadInputSchema = z.object({ export const changeScreenshotUploadOutputSchema = z.object({ success: z.literal(true), }) + +export const contentImageUploadKeySchema = z + .string() + .regex( + /^tmp\/content-images\/[a-f0-9]{64}\/[a-f0-9-]{36}\.(png|jpg|webp|gif)$/, + 'Invalid content image upload key' + ) + +export const requestContentImageUploadInputSchema = z.object({ + body: requestScreenshotUploadBodySchema.extend({ + projectId: z.uuid().optional(), + }), +}) + +export const requestContentImageUploadOutputSchema = z.object({ + key: contentImageUploadKeySchema, + presignedUrl: z.url(), +}) + +export const completeContentImageUploadInputSchema = z.object({ + body: z.object({ + key: contentImageUploadKeySchema, + projectId: z.uuid().optional(), + }), +}) + +export const completeContentImageUploadOutputSchema = z.object({ + url: z.url(), +}) From ffcd1bff47eea249a147b8840736c178160847cb Mon Sep 17 00:00:00 2001 From: mrboxs Date: Thu, 12 Nov 2026 03:00:52 +0700 Subject: [PATCH 2/2] fix: preserve published images during upload retries --- docs/development.md | 2 + packages/api/src/storage.ts | 19 ++- .../api/tests/content-image-storage.test.ts | 145 ++++++++++++++++++ .../api/tests/content-image-upload.test.ts | 17 +- 4 files changed, 178 insertions(+), 5 deletions(-) create mode 100644 packages/api/tests/content-image-storage.test.ts diff --git a/docs/development.md b/docs/development.md index 9b4e3aa6..16395b61 100644 --- a/docs/development.md +++ b/docs/development.md @@ -140,6 +140,8 @@ not public image delivery. For local development, an example bucket CORS policy Replace the origin with your deployed web origin in production. Keep lifecycle expiry scoped to `tmp/`; never expire `content/images/`. Completion keeps the temporary source until lifecycle cleanup so retries use the same permanent URL. +Completion reuses an existing published image even after the temporary source +expires. Failed or concurrent completion attempts never delete that shared URL. Published images are retained when removed from a document because another document may reference them. Images uploaded into abandoned drafts also remain; permanent unreferenced image cleanup is manual. diff --git a/packages/api/src/storage.ts b/packages/api/src/storage.ts index 7728f6d7..039db8f8 100644 --- a/packages/api/src/storage.ts +++ b/packages/api/src/storage.ts @@ -188,6 +188,7 @@ export async function promoteContentImage(tmpKey: string) { finalKey, kind: 'screenshot', consumeSource: false, + reuseDestination: true, }) } @@ -213,12 +214,25 @@ async function promoteTempImage({ finalKey, kind, consumeSource, + reuseDestination = false, }: { tmpKey: string finalKey: string kind: ProjectImageKind consumeSource: boolean + reuseDestination?: boolean }): Promise { + if (reuseDestination) { + try { + await s3.send(new HeadObjectCommand({ Bucket: S3_BUCKET, Key: finalKey })) + return finalKey + } catch (error) { + if (!isS3NotFoundError(error)) { + throw new StorageError('Failed to read promoted image') + } + } + } + let contentType: string | undefined try { const head = await s3.send( @@ -270,13 +284,14 @@ async function promoteTempImage({ }) ) } catch { - await deleteFinalKeysBestEffort([finalKey]) + // A concurrent completion may have already published a reusable key. + if (!reuseDestination) await deleteFinalKeysBestEffort([finalKey]) throw new StorageError(`Failed to promote temporary ${kind}`) } try { await s3.send(new HeadObjectCommand({ Bucket: S3_BUCKET, Key: finalKey })) } catch { - await deleteFinalKeysBestEffort([finalKey]) + if (!reuseDestination) await deleteFinalKeysBestEffort([finalKey]) throw new StorageError(`Failed to verify promoted ${kind}`) } if (consumeSource) { diff --git a/packages/api/tests/content-image-storage.test.ts b/packages/api/tests/content-image-storage.test.ts new file mode 100644 index 00000000..b7ed77d2 --- /dev/null +++ b/packages/api/tests/content-image-storage.test.ts @@ -0,0 +1,145 @@ +import { + CopyObjectCommand, + DeleteObjectCommand, + HeadObjectCommand, +} from '@aws-sdk/client-s3' +import { beforeEach, describe, expect, it, vi } from 'vite-plus/test' + +import { promoteContentImage, StorageError } from '@altstack/api/storage' + +type StorageCommand = + | CopyObjectCommand + | DeleteObjectCommand + | HeadObjectCommand +type ImageMetadata = { ContentLength: number; ContentType: string } + +const { send } = vi.hoisted(() => { + return { + send: vi.fn<(command: StorageCommand) => Promise>>(), + } +}) +vi.mock('@altstack/api/s3', () => { + return { s3: { send }, S3_BUCKET: 'test-bucket' } +}) + +const tmpKey = `tmp/content-images/${'a'.repeat(64)}/550e8400-e29b-41d4-a716-446655440004.png` +const finalKey = tmpKey.replace('tmp/content-images/', 'content/images/') +const image = { ContentLength: 1024, ContentType: 'image/png' } +const objects = new Map() +const notFound = () => + Object.assign(new Error('Not found'), { + $metadata: { httpStatusCode: 404 }, + }) + +function sendCommand(command: StorageCommand) { + const key = command.input.Key! + if (command instanceof HeadObjectCommand) { + const metadata = objects.get(key) + if (!metadata) return Promise.reject(notFound()) + return Promise.resolve(metadata) + } + if (command instanceof CopyObjectCommand) { + const source = command.input.CopySource!.slice('test-bucket/'.length) + const metadata = objects.get(source) + if (!metadata) return Promise.reject(notFound()) + objects.set(key, metadata) + return Promise.resolve({}) + } + objects.delete(key) + return Promise.resolve({}) +} + +beforeEach(() => { + objects.clear() + objects.set(tmpKey, image) + send.mockReset().mockImplementation(sendCommand) +}) + +describe('content image completion retries', () => { + it('returns the published key after its temporary source expires without copying again', async () => { + expect(await promoteContentImage(tmpKey)).toBe(finalKey) + objects.delete(tmpKey) + send.mockClear() + expect(await promoteContentImage(tmpKey)).toBe(finalKey) + expect(objects.get(finalKey)).toEqual(image) + expect(send).toHaveBeenCalledExactlyOnceWith(expect.any(HeadObjectCommand)) + }) + + it('does not overwrite a published image when the temporary source changes', async () => { + await promoteContentImage(tmpKey) + objects.set(tmpKey, { ContentLength: 2048, ContentType: 'image/jpeg' }) + send.mockClear() + await promoteContentImage(tmpKey) + expect(objects.get(finalKey)).toEqual(image) + expect( + send.mock.calls.some(([command]) => command instanceof CopyObjectCommand) + ).toBe(false) + }) + + it('preserves a published image when the retry destination lookup fails', async () => { + await promoteContentImage(tmpKey) + send.mockRejectedValueOnce(new Error('Storage unavailable')) + await expect(promoteContentImage(tmpKey)).rejects.toBeInstanceOf( + StorageError + ) + expect(objects.get(finalKey)).toEqual(image) + expect( + send.mock.calls.some( + ([command]) => command instanceof DeleteObjectCommand + ) + ).toBe(false) + expect(await promoteContentImage(tmpKey)).toBe(finalKey) + }) + + it.each(['copy', 'verification'] as const)( + 'preserves a concurrently published image when %s fails after a missing destination lookup', + async (failure) => { + let destinationLookups = 0 + send.mockImplementation(async (command) => { + if ( + command instanceof HeadObjectCommand && + command.input.Key === finalKey + ) { + destinationLookups += 1 + if (destinationLookups === 1) { + // A competing completion publishes after this request saw no object. + objects.set(finalKey, image) + throw notFound() + } + if (failure === 'verification') throw new Error('HEAD failed') + } + if (failure === 'copy' && command instanceof CopyObjectCommand) { + throw new Error('Copy failed') + } + return sendCommand(command) + }) + await expect(promoteContentImage(tmpKey)).rejects.toBeInstanceOf( + StorageError + ) + expect(objects.get(finalKey)).toEqual(image) + expect( + send.mock.calls.some( + ([command]) => command instanceof DeleteObjectCommand + ) + ).toBe(false) + send.mockImplementation(sendCommand) + objects.delete(tmpKey) + expect(await promoteContentImage(tmpKey)).toBe(finalKey) + } + ) + + it('recovers a lost copy response without deleting the copied image', async () => { + send.mockImplementation(async (command) => { + const result = await sendCommand(command) + if (command instanceof CopyObjectCommand) throw new Error('Response lost') + return result + }) + await expect(promoteContentImage(tmpKey)).rejects.toBeInstanceOf( + StorageError + ) + expect(objects.get(finalKey)).toEqual(image) + send.mockImplementation(sendCommand) + objects.delete(tmpKey) + expect(await promoteContentImage(tmpKey)).toBe(finalKey) + }) +}) diff --git a/packages/api/tests/content-image-upload.test.ts b/packages/api/tests/content-image-upload.test.ts index 3e51568e..da32d5ef 100644 --- a/packages/api/tests/content-image-upload.test.ts +++ b/packages/api/tests/content-image-upload.test.ts @@ -103,13 +103,20 @@ describe('content image uploads', () => { expiresIn: 600, signableHeaders: new Set(['content-type']), }) + send.mockRejectedValueOnce({ $metadata: { httpStatusCode: 404 } }) const result = await rpc.complete({ body: { key: upload.key } }) expect(result.url).toBe( `https://cdn.test/${upload.key.replace('tmp/content-images/', 'content/images/')}` ) expect(result.url).not.toContain('signed') expect(send.mock.calls[0]?.[0]).toBeInstanceOf(HeadObjectCommand) - expect(send.mock.calls[1]?.[0]).toBeInstanceOf(CopyObjectCommand) + expect(send.mock.calls[0]?.[0]).toMatchObject({ + input: { + Key: upload.key.replace('tmp/content-images/', 'content/images/'), + }, + }) + expect(send.mock.calls[1]?.[0]).toBeInstanceOf(HeadObjectCommand) + expect(send.mock.calls[2]?.[0]).toBeInstanceOf(CopyObjectCommand) expect( send.mock.calls.some(([cmd]) => cmd instanceof DeleteObjectCommand) ).toBe(false) @@ -191,7 +198,9 @@ describe('content image uploads', () => { 'rejects invalid stored metadata before publishing: %j', async (metadata) => { const upload = await client().request({ body }) - send.mockResolvedValueOnce(metadata) + send + .mockRejectedValueOnce({ $metadata: { httpStatusCode: 404 } }) + .mockResolvedValueOnce(metadata) await expect( client().complete({ body: { key: upload.key } }) ).rejects.toMatchObject({ code: 'BAD_REQUEST' }) @@ -203,7 +212,9 @@ describe('content image uploads', () => { it('reports an expired upload and blocks signing when quota is exhausted', async () => { const upload = await client().request({ body }) - send.mockRejectedValueOnce({ $metadata: { httpStatusCode: 404 } }) + send + .mockRejectedValueOnce({ $metadata: { httpStatusCode: 404 } }) + .mockRejectedValueOnce({ $metadata: { httpStatusCode: 404 } }) await expect( client().complete({ body: { key: upload.key } }) ).rejects.toMatchObject({ code: 'UPLOAD_EXPIRED' })