refactor: dedupe v1 file response + unify embedding patches

convex/httpApiV1/shared.ts

@@ -8,6 +8,47 @@ import { corsHeaders, mergeHeaders } from '../lib/httpHeaders'
 
 export const MAX_RAW_FILE_BYTES = 200 * 1024
 
+const SAFE_TEXT_FILE_CSP =
+  "default-src 'none'; base-uri 'none'; form-action 'none'; frame-ancestors 'none'"
+
+function isSvgLike(contentType: string | undefined, path: string) {
+  return contentType?.toLowerCase().includes('svg') || path.toLowerCase().endsWith('.svg')
+}
+
+export function safeTextFileResponse(params: {
+  textContent: string
+  path: string
+  contentType?: string
+  sha256: string
+  size: number
+  headers?: HeadersInit
+}) {
+  const isSvg = isSvgLike(params.contentType, params.path)
+
+  // For any text response that a browser might try to render, lock it down.
+  // In particular, this prevents SVG <foreignObject> script execution from reading
+  // localStorage tokens on this origin.
+  const headers = mergeHeaders(
+    params.headers,
+    {
+      'Content-Type': params.contentType
+        ? `${params.contentType}; charset=utf-8`
+        : 'text/plain; charset=utf-8',
+      'Cache-Control': 'private, max-age=60',
+      ETag: params.sha256,
+      'X-Content-SHA256': params.sha256,
+      'X-Content-Size': String(params.size),
+      'X-Content-Type-Options': 'nosniff',
+      'X-Frame-Options': 'DENY',
+      'Content-Security-Policy': SAFE_TEXT_FILE_CSP,
+      ...(isSvg ? { 'Content-Disposition': 'attachment' } : {}),
+    },
+    corsHeaders(),
+  )
+
+  return new Response(params.textContent, { status: 200, headers })
+}
+
 export function json(value: unknown, status = 200, headers?: HeadersInit) {
   return new Response(JSON.stringify(value), {
     status,

convex/httpApiV1/skillsV1.ts

@@ -3,7 +3,6 @@ import type { Doc, Id } from '../_generated/dataModel'
 import type { ActionCtx } from '../_generated/server'
 import { getOptionalApiTokenUserId, requireApiTokenUser } from '../lib/apiTokenAuth'
 import { applyRateLimit, parseBearerToken } from '../lib/httpRateLimit'
-import { corsHeaders, mergeHeaders } from '../lib/httpHeaders'
 import { publishVersionForUser } from '../skills'
 import {
   MAX_RAW_FILE_BYTES,
@@ -12,6 +11,7 @@ import {
   parseMultipartPublish,
   parsePublishBody,
   resolveTagsBatch,
+  safeTextFileResponse,
   softDeleteErrorToResponse,
   text,
   toOptionalNumber,
@@ -406,32 +406,14 @@ export async function skillsGetRouterV1Handler(ctx: ActionCtx, request: Request)
     const blob = await ctx.storage.get(file.storageId)
     if (!blob) return text('File missing in storage', 410, rate.headers)
     const textContent = await blob.text()
-
-    const isSvg =
-      file.contentType?.toLowerCase().includes('svg') || file.path.toLowerCase().endsWith('.svg')
-
-    const headers = mergeHeaders(
-      rate.headers,
-      {
-        'Content-Type': file.contentType
-          ? `${file.contentType}; charset=utf-8`
-          : 'text/plain; charset=utf-8',
-        'Cache-Control': 'private, max-age=60',
-        ETag: file.sha256,
-        'X-Content-SHA256': file.sha256,
-        'X-Content-Size': String(file.size),
-        'X-Content-Type-Options': 'nosniff',
-        'X-Frame-Options': 'DENY',
-        // For any text response that a browser might try to render, lock it down.
-        // In particular, this prevents SVG <foreignObject> script execution from
-        // reading localStorage tokens on this origin.
-        'Content-Security-Policy':
-          "default-src 'none'; base-uri 'none'; form-action 'none'; frame-ancestors 'none'",
-        ...(isSvg ? { 'Content-Disposition': 'attachment' } : {}),
-      },
-      corsHeaders(),
-    )
-    return new Response(textContent, { status: 200, headers })
+    return safeTextFileResponse({
+      textContent,
+      path: file.path,
+      contentType: file.contentType ?? undefined,
+      sha256: file.sha256,
+      size: file.size,
+      headers: rate.headers,
+    })
   }
 
   return text('Not found', 404, rate.headers)

convex/httpApiV1/soulsV1.ts

@@ -3,7 +3,6 @@ import type { Doc, Id } from '../_generated/dataModel'
 import type { ActionCtx } from '../_generated/server'
 import { requireApiTokenUser } from '../lib/apiTokenAuth'
 import { applyRateLimit, parseBearerToken } from '../lib/httpRateLimit'
-import { corsHeaders, mergeHeaders } from '../lib/httpHeaders'
 import { publishSoulVersionForUser } from '../souls'
 import {
   MAX_RAW_FILE_BYTES,
@@ -12,6 +11,7 @@ import {
   parseMultipartPublish,
   parsePublishBody,
   resolveSoulTagsBatch,
+  safeTextFileResponse,
   softDeleteErrorToResponse,
   text,
   toOptionalNumber,
@@ -251,29 +251,14 @@ export async function soulsGetRouterV1Handler(ctx: ActionCtx, request: Request)
     const textContent = await blob.text()
 
     void ctx.runMutation(api.soulDownloads.increment, { soulId: soulResult.soul._id })
-
-    const isSvg =
-      file.contentType?.toLowerCase().includes('svg') || file.path.toLowerCase().endsWith('.svg')
-
-    const headers = mergeHeaders(
-      rate.headers,
-      {
-        'Content-Type': file.contentType
-          ? `${file.contentType}; charset=utf-8`
-          : 'text/plain; charset=utf-8',
-        'Cache-Control': 'private, max-age=60',
-        ETag: file.sha256,
-        'X-Content-SHA256': file.sha256,
-        'X-Content-Size': String(file.size),
-        'X-Content-Type-Options': 'nosniff',
-        'X-Frame-Options': 'DENY',
-        'Content-Security-Policy':
-          "default-src 'none'; base-uri 'none'; form-action 'none'; frame-ancestors 'none'",
-        ...(isSvg ? { 'Content-Disposition': 'attachment' } : {}),
-      },
-      corsHeaders(),
-    )
-    return new Response(textContent, { status: 200, headers })
+    return safeTextFileResponse({
+      textContent,
+      path: file.path,
+      contentType: file.contentType ?? undefined,
+      sha256: file.sha256,
+      size: file.size,
+      headers: rate.headers,
+    })
   }
 
   return text('Not found', 404, rate.headers)

convex/skills.ts

@@ -1445,16 +1445,7 @@ export const report = mutation({
     await ctx.db.patch(skill._id, updates)
 
     if (shouldAutoHide) {
-      const embeddings = await ctx.db
-        .query('skillEmbeddings')
-        .withIndex('by_skill', (q) => q.eq('skillId', skill._id))
-        .collect()
-      for (const embedding of embeddings) {
-        await ctx.db.patch(embedding._id, {
-          visibility: 'deleted',
-          updatedAt: now,
-        })
-      }
+      await setSkillEmbeddingsSoftDeleted(ctx, skill._id, true, now)
 
       await ctx.db.insert('auditLogs', {
         actorUserId: userId,
@@ -2125,28 +2116,76 @@ export const setSkillModerationStatusActiveInternal = internalMutation({
   },
 })
 
-async function markSkillEmbeddingsDeleted(ctx: MutationCtx, skillId: Id<'skills'>, now: number) {
-  const embeddings = await ctx.db
+async function listSkillEmbeddingsForSkill(ctx: MutationCtx, skillId: Id<'skills'>) {
+  return ctx.db
     .query('skillEmbeddings')
     .withIndex('by_skill', (q) => q.eq('skillId', skillId))
     .collect()
+}
+
+async function markSkillEmbeddingsDeleted(ctx: MutationCtx, skillId: Id<'skills'>, now: number) {
+  const embeddings = await listSkillEmbeddingsForSkill(ctx, skillId)
   for (const embedding of embeddings) {
     if (embedding.visibility === 'deleted') continue
     await ctx.db.patch(embedding._id, { visibility: 'deleted', updatedAt: now })
   }
 }
 
-async function restoreSkillEmbeddingVisibility(ctx: MutationCtx, skillId: Id<'skills'>, now: number) {
-  const embeddings = await ctx.db
-    .query('skillEmbeddings')
-    .withIndex('by_skill', (q) => q.eq('skillId', skillId))
-    .collect()
+async function restoreSkillEmbeddingsVisibility(ctx: MutationCtx, skillId: Id<'skills'>, now: number) {
+  const embeddings = await listSkillEmbeddingsForSkill(ctx, skillId)
   for (const embedding of embeddings) {
     const visibility = embeddingVisibilityFor(embedding.isLatest, embedding.isApproved)
     await ctx.db.patch(embedding._id, { visibility, updatedAt: now })
   }
 }
 
+async function setSkillEmbeddingsSoftDeleted(
+  ctx: MutationCtx,
+  skillId: Id<'skills'>,
+  deleted: boolean,
+  now: number,
+) {
+  if (deleted) {
+    await markSkillEmbeddingsDeleted(ctx, skillId, now)
+    return
+  }
+
+  await restoreSkillEmbeddingsVisibility(ctx, skillId, now)
+}
+
+async function setSkillEmbeddingsLatestVersion(
+  ctx: MutationCtx,
+  skillId: Id<'skills'>,
+  latestVersionId: Id<'skillVersions'>,
+  now: number,
+) {
+  const embeddings = await listSkillEmbeddingsForSkill(ctx, skillId)
+  for (const embedding of embeddings) {
+    const isLatest = embedding.versionId === latestVersionId
+    await ctx.db.patch(embedding._id, {
+      isLatest,
+      visibility: embeddingVisibilityFor(isLatest, embedding.isApproved),
+      updatedAt: now,
+    })
+  }
+}
+
+async function setSkillEmbeddingsApproved(
+  ctx: MutationCtx,
+  skillId: Id<'skills'>,
+  approved: boolean,
+  now: number,
+) {
+  const embeddings = await listSkillEmbeddingsForSkill(ctx, skillId)
+  for (const embedding of embeddings) {
+    await ctx.db.patch(embedding._id, {
+      isApproved: approved,
+      visibility: embeddingVisibilityFor(embedding.isLatest, approved),
+      updatedAt: now,
+    })
+  }
+}
+
 export const applyBanToOwnedSkillsBatchInternal = internalMutation({
   args: {
     ownerUserId: v.id('users'),
@@ -2180,7 +2219,7 @@ export const applyBanToOwnedSkillsBatchInternal = internalMutation({
       }
 
       await ctx.db.patch(skill._id, patch)
-      await markSkillEmbeddingsDeleted(ctx, skill._id, args.bannedAt)
+      await setSkillEmbeddingsSoftDeleted(ctx, skill._id, true, args.bannedAt)
     }
 
     scheduleNextBatchIfNeeded(
@@ -2229,7 +2268,7 @@ export const restoreOwnedSkillsForUnbanBatchInternal = internalMutation({
         updatedAt: now,
       })
 
-      await restoreSkillEmbeddingVisibility(ctx, skill._id, now)
+      await setSkillEmbeddingsSoftDeleted(ctx, skill._id, false, now)
       restoredCount += 1
     }
 
@@ -2768,25 +2807,15 @@ export const updateTags = mutation({
     }
 
     const latestEntry = args.tags.find((entry) => entry.tag === 'latest')
+    const now = Date.now()
     await ctx.db.patch(skill._id, {
       tags: nextTags,
       latestVersionId: latestEntry ? latestEntry.versionId : skill.latestVersionId,
-      updatedAt: Date.now(),
+      updatedAt: now,
     })
 
     if (latestEntry) {
-      const embeddings = await ctx.db
-        .query('skillEmbeddings')
-        .withIndex('by_skill', (q) => q.eq('skillId', skill._id))
-        .collect()
-      for (const embedding of embeddings) {
-        const isLatest = embedding.versionId === latestEntry.versionId
-        await ctx.db.patch(embedding._id, {
-          isLatest,
-          visibility: embeddingVisibilityFor(isLatest, embedding.isApproved),
-          updatedAt: Date.now(),
-        })
-      }
+      await setSkillEmbeddingsLatestVersion(ctx, skill._id, latestEntry.versionId, now)
     }
   },
 })
@@ -2812,17 +2841,7 @@ export const setRedactionApproved = mutation({
       updatedAt: now,
     })
 
-    const embeddings = await ctx.db
-      .query('skillEmbeddings')
-      .withIndex('by_skill', (q) => q.eq('skillId', skill._id))
-      .collect()
-    for (const embedding of embeddings) {
-      await ctx.db.patch(embedding._id, {
-        isApproved: args.approved,
-        visibility: embeddingVisibilityFor(embedding.isLatest, args.approved),
-        updatedAt: now,
-      })
-    }
+    await setSkillEmbeddingsApproved(ctx, skill._id, args.approved, now)
 
     await ctx.db.insert('auditLogs', {
       actorUserId: user._id,
@@ -2891,18 +2910,7 @@ export const setSoftDeleted = mutation({
       updatedAt: now,
     })
 
-    const embeddings = await ctx.db
-      .query('skillEmbeddings')
-      .withIndex('by_skill', (q) => q.eq('skillId', skill._id))
-      .collect()
-    for (const embedding of embeddings) {
-      await ctx.db.patch(embedding._id, {
-        visibility: args.deleted
-          ? 'deleted'
-          : embeddingVisibilityFor(embedding.isLatest, embedding.isApproved),
-        updatedAt: now,
-      })
-    }
+    await setSkillEmbeddingsSoftDeleted(ctx, skill._id, args.deleted, now)
 
     await ctx.db.insert('auditLogs', {
       actorUserId: user._id,
@@ -2936,10 +2944,7 @@ export const changeOwner = mutation({
       updatedAt: now,
     })
 
-    const embeddings = await ctx.db
-      .query('skillEmbeddings')
-      .withIndex('by_skill', (q) => q.eq('skillId', skill._id))
-      .collect()
+    const embeddings = await listSkillEmbeddingsForSkill(ctx, skill._id)
     for (const embedding of embeddings) {
       await ctx.db.patch(embedding._id, {
         ownerId: args.ownerUserId,
@@ -3575,18 +3580,7 @@ export const setSkillSoftDeletedInternal = internalMutation({
       updatedAt: now,
     })
 
-    const embeddings = await ctx.db
-      .query('skillEmbeddings')
-      .withIndex('by_skill', (q) => q.eq('skillId', skill._id))
-      .collect()
-    for (const embedding of embeddings) {
-      await ctx.db.patch(embedding._id, {
-        visibility: args.deleted
-          ? 'deleted'
-          : embeddingVisibilityFor(embedding.isLatest, embedding.isApproved),
-        updatedAt: now,
-      })
-    }
+    await setSkillEmbeddingsSoftDeleted(ctx, skill._id, args.deleted, now)
 
     await ctx.db.insert('auditLogs', {
       actorUserId: args.userId,