fix(audit-g2): async race / state-leak across eight stores
Extracts gallery.js's hand-rolled inflightId pattern into a new useInflightToken composable; adopts in every store that previously had no guard against late-response overwrites or wrong-image URL interpolation. Two operator-impacting bugs the audit (workflow wf_bbe3fdb1-e62) flagged: - modal.removeTag rolled back the chip rail unconditionally even when only the secondary dismiss POST had failed — UI lied until refresh. And all tag-mutation URLs interpolated currentImageId AFTER an await, so a fast prev/next could route DELETE/POST to the wrong image. Both fixed: split try/catch (dismiss failure surfaces a warning, doesn't roll back the delete); imageId captured at call-time and used in URLs throughout. - suggestions.accept dereferenced currentImageId after the awaited POST /api/tags, so the subsequent /suggestions/accept could apply A's chosen tag to image B AND push it to B's allowlist. Fixed by capturing imageId at click-time + inflight guard on load(). Same shape across artist / downloads / artistDirectory / tagDirectory / posts stores: rapid filter/nav changes used to interleave responses (last-writer-wins). Now the late response is discarded and the most-recent request wins. Filter-change-during- search no longer drops the second fetch because the loading flag was still true from the first. gallery.js's inflightId removed in favor of the shared composable so the pattern stays consistent.
This commit is contained in:
@@ -3,6 +3,7 @@ import { toast } from '../utils/toast.js'
|
||||
import { ref, computed } from 'vue'
|
||||
import { useApi } from '../composables/useApi.js'
|
||||
import { useAsyncAction } from '../composables/useAsyncAction.js'
|
||||
import { useInflightToken } from '../composables/useInflightToken.js'
|
||||
|
||||
export const useModalStore = defineStore('modal', () => {
|
||||
const api = useApi()
|
||||
@@ -10,6 +11,11 @@ export const useModalStore = defineStore('modal', () => {
|
||||
const currentImageId = ref(null)
|
||||
const current = ref(null)
|
||||
const { loading, error, run } = useAsyncAction({ errorAs: 'message' })
|
||||
// Tag mutations interpolate the image id into the URL after an
|
||||
// await; without an inflight token, a fast prev/next can route the
|
||||
// DELETE/POST to the wrong image AND the response to the wrong
|
||||
// chip rail. Audit 2026-06-02.
|
||||
const inflight = useInflightToken()
|
||||
|
||||
// Post-scoped cycle. When set, prev/next cycles within this array
|
||||
// (used by PostCard's expanded-mosaic PostImageGrid clicks). When
|
||||
@@ -19,6 +25,9 @@ export const useModalStore = defineStore('modal', () => {
|
||||
const postImageIndex = ref(0)
|
||||
|
||||
async function open (id, opts = {}) {
|
||||
// Cancel any in-flight tag mutation or reloadTags from the
|
||||
// previous image so its late response can't apply to this one.
|
||||
inflight.cancel()
|
||||
currentImageId.value = id
|
||||
current.value = null // cleared upfront so it stays null on error
|
||||
// Update post-scoped state if caller passed it; otherwise clear so
|
||||
@@ -31,12 +40,16 @@ export const useModalStore = defineStore('modal', () => {
|
||||
postImageIds.value = null
|
||||
postImageIndex.value = 0
|
||||
}
|
||||
const t = inflight.claim()
|
||||
await run(async () => {
|
||||
current.value = await api.get(`/api/gallery/image/${id}`)
|
||||
const body = await api.get(`/api/gallery/image/${id}`)
|
||||
if (!t.isCurrent()) return
|
||||
current.value = body
|
||||
})
|
||||
}
|
||||
|
||||
async function close () {
|
||||
inflight.cancel()
|
||||
currentImageId.value = null
|
||||
current.value = null
|
||||
error.value = null
|
||||
@@ -75,37 +88,74 @@ export const useModalStore = defineStore('modal', () => {
|
||||
}
|
||||
|
||||
async function reloadTags () {
|
||||
if (!currentImageId.value) return
|
||||
const tags = await api.get(`/api/images/${currentImageId.value}/tags`)
|
||||
if (current.value) current.value.tags = tags
|
||||
// Capture image id at call-time. After an await, currentImageId
|
||||
// may have advanced via prev/next navigation; without capture, the
|
||||
// GET would target the new image and write its tags onto a chip
|
||||
// rail the user didn't open. Audit 2026-06-02.
|
||||
const imageId = currentImageId.value
|
||||
if (!imageId) return
|
||||
const t = inflight.claim()
|
||||
const tags = await api.get(`/api/images/${imageId}/tags`)
|
||||
if (!t.isCurrent()) return
|
||||
// Only commit if the modal is still showing this image — guards
|
||||
// against close() / nav clearing current between the await and now.
|
||||
if (current.value && currentImageId.value === imageId) {
|
||||
current.value.tags = tags
|
||||
}
|
||||
}
|
||||
|
||||
async function removeTag (tagId) {
|
||||
if (!currentImageId.value) return
|
||||
const imageId = currentImageId.value
|
||||
if (!imageId) return
|
||||
const prev = current.value.tags
|
||||
// Optimistic UI: drop the chip immediately.
|
||||
current.value.tags = current.value.tags.filter(t => t.id !== tagId)
|
||||
// Split the two POSTs so a dismiss failure (secondary side-effect)
|
||||
// doesn't roll back the successful DELETE — previously the catch
|
||||
// unconditionally restored the chip rail even when only the
|
||||
// dismiss had failed, so the UI lied until refresh. Audit 2026-06-02.
|
||||
try {
|
||||
await api.delete(`/api/images/${currentImageId.value}/tags/${tagId}`)
|
||||
await api.post(`/api/images/${currentImageId.value}/suggestions/dismiss`, {
|
||||
await api.delete(`/api/images/${imageId}/tags/${tagId}`)
|
||||
} catch (e) {
|
||||
// Real failure: roll back, surface, rethrow.
|
||||
if (current.value && currentImageId.value === imageId) {
|
||||
current.value.tags = prev
|
||||
}
|
||||
toast({ text: `Failed to remove tag: ${e.message}`, type: 'error' })
|
||||
throw e
|
||||
}
|
||||
// DELETE landed. The dismiss is fire-and-best-effort — log on
|
||||
// failure but DON'T roll back the chip rail; the tag is gone
|
||||
// server-side regardless.
|
||||
try {
|
||||
await api.post(`/api/images/${imageId}/suggestions/dismiss`, {
|
||||
body: { tag_id: tagId },
|
||||
})
|
||||
} catch (e) {
|
||||
current.value.tags = prev
|
||||
toast({ text: `Failed to remove tag: ${e.message}`, type: 'error' })
|
||||
throw e
|
||||
toast({
|
||||
text: `Tag removed, but failed to dismiss suggestion: ${e.message}`,
|
||||
type: 'warning',
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
async function addExistingTag (tagId) {
|
||||
if (!currentImageId.value) return
|
||||
await api.post(`/api/images/${currentImageId.value}/tags`, {
|
||||
const imageId = currentImageId.value
|
||||
if (!imageId) return
|
||||
await api.post(`/api/images/${imageId}/tags`, {
|
||||
body: { tag_id: tagId, source: 'manual' },
|
||||
})
|
||||
await reloadTags()
|
||||
}
|
||||
|
||||
async function createAndAdd ({ name, kind, fandom_id = null }) {
|
||||
// Capture imageId so the post-create reloadTags / addExistingTag
|
||||
// flow stays bound to the image the user clicked on, even if
|
||||
// they navigated during the /api/tags POST.
|
||||
const imageId = currentImageId.value
|
||||
if (!imageId) return
|
||||
const tag = await api.post('/api/tags', { body: { name, kind, fandom_id } })
|
||||
if (currentImageId.value !== imageId) return // navigated away
|
||||
await addExistingTag(tag.id)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user