Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions api/__tests__/image-search-insertion.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { describe, it, expect, vi, beforeEach } from 'vitest'
import { Readable } from 'node:stream'

vi.mock('../_imageSearch.js', () => ({
ImageSearchError: class ImageSearchError extends Error {},
imageSearchErrorBody: vi.fn(() => ({ error: 'Search failed' })),
searchImages: vi.fn(),
embedImage: vi.fn(),
resolveKeys: vi.fn(() => ({})),
}))

import handler from '../image-search.js'
import { searchImages, embedImage } from '../_imageSearch.js'

const images = [
{ id: 'one', url: 'https://images.example/one.jpg', thumbnailUrl: 'https://images.example/one-thumb.jpg', provider: 'openverse' },
{ id: 'two', url: 'https://images.example/two.jpg', thumbnailUrl: 'https://images.example/two-thumb.jpg', provider: 'openverse' },
]

async function post(body) {
const req = Readable.from([JSON.stringify(body)])
req.method = 'POST'
const res = {
statusCode: 200,
setHeader: vi.fn(),
end: vi.fn(),
}
await handler(req, res)
return { status: res.statusCode, body: JSON.parse(res.end.mock.calls[0][0]) }
}

describe('manual image selection', () => {
beforeEach(() => {
vi.resetAllMocks()
searchImages.mockResolvedValue(images)
embedImage.mockResolvedValue('data:image/jpeg;base64,YWJj')
})

it('embeds the image actually clicked, not the first search result', async () => {
const response = await post({ query: 'apple', provider: 'openverse', limit: 20, embedSelectedUrl: images[1].url })
expect(response.status).toBe(200)
expect(response.body).toMatchObject({ embedded: true, candidate: images[1] })
expect(embedImage).toHaveBeenCalledExactlyOnceWith(images[1].url)
expect(searchImages).toHaveBeenCalledWith(expect.objectContaining({ query: 'apple', provider: 'openverse' }))
})

it('rejects an unverified URL without downloading it (SSRF guard)', async () => {
const response = await post({ query: 'apple', provider: 'openverse', embedSelectedUrl: 'http://127.0.0.1/private' })
expect(response.status).toBe(409)
expect(response.body.code).toBe('image_selection_expired')
expect(embedImage).not.toHaveBeenCalled()
})

it('can embed the provider thumbnail if the original is blocked', async () => {
embedImage.mockRejectedValueOnce(new Error('403')).mockResolvedValueOnce('data:image/jpeg;base64,YWJj')
const response = await post({ query: 'apple', provider: 'openverse', embedSelectedUrl: images[0].url })
expect(response.status).toBe(200)
expect(response.body).toMatchObject({ embedded: true, thumbnailFallback: true })
expect(embedImage).toHaveBeenNthCalledWith(2, images[0].thumbnailUrl)
})

it('reports download failures instead of pretending insertion succeeded', async () => {
embedImage.mockRejectedValue(new Error('403'))
const response = await post({ query: 'apple', provider: 'openverse', embedSelectedUrl: images[0].url })
expect(response.status).toBe(422)
expect(response.body.code).toBe('image_download_failed')
expect(response.body).not.toHaveProperty('dataUrl')
})

it('preserves ordinary image search requests', async () => {
const response = await post({ query: 'apple', provider: 'openverse' })
expect(response.status).toBe(200)
expect(response.body.results).toEqual(images)
expect(embedImage).not.toHaveBeenCalled()
})
})
30 changes: 30 additions & 0 deletions api/image-search.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,36 @@ export default async function handler(req, res) {
: { query },
})

// Manual selection: only embed a URL returned by this fresh provider search.
// Never fetch an arbitrary client-supplied URL (server-side request forgery risk).
if (body.embedSelectedUrl) {
const selectedUrl = String(body.embedSelectedUrl)
const selected = results.find((result) => result.url === selectedUrl)
if (!selected) {
return json(res, 409, {
error: 'The selected image is no longer in the search results. Search again and reselect it.',
code: 'image_selection_expired',
})
}
try {
const dataUrl = await embedImage(selected.url)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Validate redirect destinations before embedding images

A fresh provider result is still an untrusted external URL, so it does not establish an SSRF boundary: an indexed attacker-controlled image URL can redirect to a loopback, private-network, or cloud-metadata address. embedImage explicitly uses redirect: 'follow' and performs no hostname or resolved-IP validation, allowing this new selected-image route (and its thumbnail fallback) to make the server follow that redirect. Reject private/reserved destinations after DNS resolution and validate every redirect hop before downloading.

Useful? React with 👍 / 👎.

return json(res, 200, { dataUrl, embedded: true, candidate: selected })
} catch (originalError) {
// Some providers block hotlinking the original but allow their thumbnail.
if (selected.thumbnailUrl && selected.thumbnailUrl !== selected.url) {
try {
const dataUrl = await embedImage(selected.thumbnailUrl)
return json(res, 200, { dataUrl, embedded: true, candidate: selected, thumbnailFallback: true })
} catch { /* Preserve the original failure below. */ }
}
return json(res, 422, {
error: 'This image cannot be downloaded. Choose another search result.',
code: 'image_download_failed',
details: originalError instanceof Error ? originalError.message : 'Image download failed',
})
}
}

// Optional single-image embed (used by the agent Phase 2)
if (body.embedImage && results.length > 0) {
if (results[0].needsReview) {
Expand Down
94 changes: 70 additions & 24 deletions src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,9 @@ function SetEditor({ set, onBack, onUpdate, onDuplicate, googleImageApiKey, goog
const [imageSearchQuery, setImageSearchQuery] = useState('')
const [imageSearchResults, setImageSearchResults] = useState<ImageCandidate[]>([])
const [imageSearchLoading, setImageSearchLoading] = useState(false)
const [imageSearchApplyLoading, setImageSearchApplyLoading] = useState(false)
const [imageSearchResultsQuery, setImageSearchResultsQuery] = useState('')
const [imageSearchApplyError, setImageSearchApplyError] = useState<string | null>(null)
const initialAgentDraft = loadAgentDraft(set.id)
const [imageAgentOpen, setImageAgentOpen] = useState(false)
const [flashcardAgentMode, setFlashcardAgentMode] = useState<FlashcardAgentMode>(initialAgentDraft.mode)
Expand Down Expand Up @@ -1741,7 +1744,9 @@ function SetEditor({ set, onBack, onUpdate, onDuplicate, googleImageApiKey, goog
backText: card.backText,
})
setImageSearchQuery(intent.query)
setImageSearchResultsQuery(intent.query)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve the originating query for saved candidates

When this dialog opens stored review candidates, this assigns a newly reconstructed default query rather than the query that produced those candidates. The image agent normally searches with styles such as clear photo isolated subject (and can use custom or revision queries), while buildImageSearchIntent here typically reconstructs clear simple illustration; selecting a stored candidate then re-runs that different query, so the API cannot find the candidate URL and returns image_selection_expired. Persist and reuse the candidates' originating query (or a server-issued selection token) so the existing “Review image options” flow remains selectable.

Useful? React with 👍 / 👎.

setImageSearchResults(getStoredImageCandidates(card, side))
setImageSearchApplyError(null)
}
setImageSearchCardId(cardId)
setImageSearchSide(side)
Expand All @@ -1756,6 +1761,8 @@ function SetEditor({ set, onBack, onUpdate, onDuplicate, googleImageApiKey, goog

try {
setImageSearchLoading(true)
setImageSearchApplyError(null)
setImageSearchResults([])
const controller = new AbortController()
const timer = setTimeout(() => controller.abort(), 30_000)
try {
Expand Down Expand Up @@ -1783,6 +1790,7 @@ function SetEditor({ set, onBack, onUpdate, onDuplicate, googleImageApiKey, goog
}
const data = await response.json() as { results?: ImageCandidate[] }
const results = data.results ?? []
setImageSearchResultsQuery(imageSearchQuery.trim())
setImageSearchResults(results)
if (results.length === 0) toast.message('No images found — try a different keyword or configure a provider in Image Search Settings')
} catch (error) {
Expand All @@ -1798,30 +1806,65 @@ function SetEditor({ set, onBack, onUpdate, onDuplicate, googleImageApiKey, goog
}
}

function handleSelectWebImage(candidate: ImageCandidate) {
if (!imageSearchCardId) return
const asset = {
...imageAssetFromCandidate(candidate),
candidates: imageSearchResults,
}
if (imageSearchSide === 'front') {
updateCard(imageSearchCardId, {
frontImageUrl: candidate.url,
frontImage: asset,
frontImageCandidates: undefined,
frontImagePlacement: DEFAULT_IMAGE_PLACEMENT,
})
} else {
updateCard(imageSearchCardId, {
backImageUrl: candidate.url,
backImage: asset,
backImageCandidates: undefined,
backImagePlacement: DEFAULT_IMAGE_PLACEMENT,
async function handleSelectWebImage(candidate: ImageCandidate) {
if (!imageSearchCardId || imageSearchApplyLoading) return
setImageSearchApplyLoading(true)
setImageSearchApplyError(null)
const controller = new AbortController()
const timeout = setTimeout(() => controller.abort(), 45_000)
try {
const response = await fetch('/api/image-search', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
signal: controller.signal,
body: JSON.stringify({
query: imageSearchResultsQuery,
intent: { query: imageSearchResultsQuery, concepts: [imageSearchResultsQuery], style: 'neutral' },
provider: candidate.provider,
braveApiKey: imageBraveKey.trim(),
pixabayApiKey: imagePixabayKey.trim(),
pexelsApiKey: imagePexelsKey.trim(),
googleApiKey: imageGoogleKey.trim() || googleImageApiKey.trim(),
googleCx: imageGoogleCx.trim() || googleImageSearchCx.trim(),
limit: 20,
embedSelectedUrl: candidate.url,
}),
})
const data = await response.json() as { dataUrl?: string; error?: string; thumbnailFallback?: boolean }
if (!response.ok || !data.dataUrl?.startsWith('data:image/')) {
throw new Error(data.error || 'The image could not be downloaded. Choose another result.')
}
// Decode/verify and compress before persisting. Remote URLs alone can be hotlink-blocked.
const imageUrl = await compressImage(data.dataUrl, 850, 0.72)
const asset = { ...imageAssetFromCandidate(candidate, imageUrl), candidates: imageSearchResults }
if (imageSearchSide === 'front') {
updateCard(imageSearchCardId, {
frontImageUrl: imageUrl,
frontImage: asset,
frontImageCandidates: undefined,
frontImagePlacement: DEFAULT_IMAGE_PLACEMENT,
})
} else {
updateCard(imageSearchCardId, {
backImageUrl: imageUrl,
backImage: asset,
backImageCandidates: undefined,
backImagePlacement: DEFAULT_IMAGE_PLACEMENT,
})
}
setImageSearchOpen(false)
setImageSearchResults([])
toast.success(data.thumbnailFallback ? 'Image inserted (thumbnail quality)' : 'Image inserted')
} catch (error) {
const message = controller.signal.aborted
? 'Image download timed out. Choose another result.'
: error instanceof Error ? error.message : 'Could not insert this image.'
setImageSearchApplyError(message)
toast.error(message)
} finally {
clearTimeout(timeout)
setImageSearchApplyLoading(false)
}
setImageSearchOpen(false)
setImageSearchResults([])
toast.success('Image inserted')
}


Expand Down Expand Up @@ -3502,7 +3545,7 @@ function SetEditor({ set, onBack, onUpdate, onDuplicate, googleImageApiKey, goog
</DialogContent>
</Dialog>

<Dialog open={imageSearchOpen} onOpenChange={setImageSearchOpen}>
<Dialog open={imageSearchOpen} onOpenChange={(open) => { if (!imageSearchApplyLoading) setImageSearchOpen(open) }}>
<DialogContent className="sm:max-w-3xl">
<DialogHeader>
<DialogTitle>Search Web Images</DialogTitle>
Expand All @@ -3521,7 +3564,7 @@ function SetEditor({ set, onBack, onUpdate, onDuplicate, googleImageApiKey, goog
}
}}
/>
<Button onClick={runWebImageSearch} disabled={imageSearchLoading} className="sm:w-auto">
<Button onClick={runWebImageSearch} disabled={imageSearchLoading || imageSearchApplyLoading} className="sm:w-auto">
<MagnifyingGlass className="mr-2" weight="bold" />
Search
</Button>
Expand All @@ -3531,13 +3574,16 @@ function SetEditor({ set, onBack, onUpdate, onDuplicate, googleImageApiKey, goog
</Button>
</div>

{imageSearchApplyError && <p role="alert" className="text-sm text-destructive">{imageSearchApplyError}</p>}
{imageSearchApplyLoading && <p role="status" className="text-sm text-muted-foreground">Downloading and saving image…</p>}
{imageSearchResults.length > 0 ? (
<div className="grid grid-cols-2 md:grid-cols-4 gap-3 max-h-[420px] overflow-y-auto pr-1">
{imageSearchResults.map((result) => (
<button
key={result.id}
className="text-left border rounded-lg overflow-hidden hover:border-primary transition-colors"
onClick={() => handleSelectWebImage(result)}
disabled={imageSearchApplyLoading || imageSearchLoading}
type="button"
>
<img
Expand Down
Loading