Skip to content
Open
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
10 changes: 6 additions & 4 deletions convex/downloads.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { v } from 'convex/values'
import { zipSync } from 'fflate'
import { api } from './_generated/api'
import { httpAction, mutation } from './_generated/server'
import { CORS_HEADERS } from './lib/cors'
import { insertStatEvent } from './skillStatEvents'

export const downloadZip = httpAction(async (ctx, request) => {
Expand All @@ -11,12 +12,12 @@ export const downloadZip = httpAction(async (ctx, request) => {
const tagParam = url.searchParams.get('tag')?.trim()

if (!slug) {
return new Response('Missing slug', { status: 400 })
return new Response('Missing slug', { status: 400, headers: CORS_HEADERS })
}

const skillResult = await ctx.runQuery(api.skills.getBySlug, { slug })
if (!skillResult?.skill) {
return new Response('Skill not found', { status: 404 })
return new Response('Skill not found', { status: 404, headers: CORS_HEADERS })
}

const skill = skillResult.skill
Expand All @@ -35,10 +36,10 @@ export const downloadZip = httpAction(async (ctx, request) => {
}

if (!version) {
return new Response('Version not found', { status: 404 })
return new Response('Version not found', { status: 404, headers: CORS_HEADERS })
}
if (version.softDeletedAt) {
return new Response('Version not available', { status: 410 })
return new Response('Version not available', { status: 410, headers: CORS_HEADERS })
}

const files: Record<string, Uint8Array> = {}
Expand All @@ -61,6 +62,7 @@ export const downloadZip = httpAction(async (ctx, request) => {
'Content-Type': 'application/zip',
'Content-Disposition': `attachment; filename="${slug}-${version.version}.zip"`,
'Cache-Control': 'private, max-age=60',
...CORS_HEADERS,
},
})
})
Expand Down
12 changes: 12 additions & 0 deletions convex/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ import {
starsPostRouterV1Http,
whoamiV1Http,
} from './httpApiV1'
import { corsPreflightResponse } from './lib/cors'
import { httpAction } from './_generated/server'

// CORS preflight handler for browser clients
const corsPreflightHttp = httpAction(async () => corsPreflightResponse())

const http = httpRouter()

Expand Down Expand Up @@ -191,4 +196,11 @@ http.route({
handler: cliSkillUndeleteHttp,
})

// CORS preflight handler for all API routes
http.route({
pathPrefix: '/api/',
method: 'OPTIONS',
handler: corsPreflightHttp,
})
Comment on lines +199 to +204
Copy link

Choose a reason for hiding this comment

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

[P1] OPTIONS preflight only registered for /api/ prefix

This adds an OPTIONS handler only for pathPrefix: '/api/' (convex/http.ts:199-204). However, this router also exposes LegacyApiRoutes.* (still under /api in many setups) and potentially other non-/api/ HTTP routes (e.g., auth routes). If any of those endpoints are called cross-origin and trigger a preflight, they’ll still fail with a missing/404 OPTIONS route even though the GET/POST responses now include CORS headers.

If the intent is “all API endpoints,” consider also covering legacy routes (and/or the auth routes) with an OPTIONS handler that matches their prefixes.

Prompt To Fix With AI
This is a comment left during a code review.
Path: convex/http.ts
Line: 199:204

Comment:
[P1] OPTIONS preflight only registered for `/api/` prefix

This adds an OPTIONS handler only for `pathPrefix: '/api/'` (`convex/http.ts:199-204`). However, this router also exposes `LegacyApiRoutes.*` (still under `/api` in many setups) and potentially other non-`/api/` HTTP routes (e.g., auth routes). If any of those endpoints are called cross-origin and trigger a preflight, they’ll still fail with a missing/404 OPTIONS route even though the GET/POST responses now include CORS headers.

If the intent is “all API endpoints,” consider also covering legacy routes (and/or the auth routes) with an OPTIONS handler that matches their prefixes.

How can I resolve this? If you propose a fix, please make it concise.


export default http
3 changes: 3 additions & 0 deletions convex/httpApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { Id } from './_generated/dataModel'
import type { ActionCtx } from './_generated/server'
import { httpAction } from './_generated/server'
import { requireApiTokenUser } from './lib/apiTokenAuth'
import { CORS_HEADERS } from './lib/cors'
import { publishVersionForUser } from './skills'

type SearchSkillEntry = {
Expand Down Expand Up @@ -244,6 +245,7 @@ function json(value: unknown, status = 200) {
headers: {
'Content-Type': 'application/json',
'Cache-Control': 'no-store',
...CORS_HEADERS,
},
})
}
Expand All @@ -254,6 +256,7 @@ function text(value: string, status: number) {
headers: {
'Content-Type': 'text/plain; charset=utf-8',
'Cache-Control': 'no-store',
...CORS_HEADERS,
},
})
}
Expand Down
5 changes: 5 additions & 0 deletions convex/httpApiV1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { Doc, Id } from './_generated/dataModel'
import type { ActionCtx } from './_generated/server'
import { httpAction } from './_generated/server'
import { requireApiTokenUser } from './lib/apiTokenAuth'
import { CORS_HEADERS } from './lib/cors'
import { hashToken } from './lib/tokens'
import { publishVersionForUser } from './skills'
import { publishSoulVersionForUser } from './souls'
Expand Down Expand Up @@ -394,6 +395,7 @@ async function skillsGetRouterV1Handler(ctx: ActionCtx, request: Request) {
'Content-Security-Policy':
"default-src 'none'; base-uri 'none'; form-action 'none'; frame-ancestors 'none'",
...(isSvg ? { 'Content-Disposition': 'attachment' } : {}),
...CORS_HEADERS,
})
return new Response(textContent, { status: 200, headers })
}
Expand Down Expand Up @@ -731,6 +733,7 @@ function json(value: unknown, status = 200, headers?: HeadersInit) {
{
'Content-Type': 'application/json',
'Cache-Control': 'no-store',
...CORS_HEADERS,
},
headers,
),
Expand All @@ -744,6 +747,7 @@ function text(value: string, status: number, headers?: HeadersInit) {
{
'Content-Type': 'text/plain; charset=utf-8',
'Cache-Control': 'no-store',
...CORS_HEADERS,
},
headers,
),
Expand Down Expand Up @@ -1014,6 +1018,7 @@ async function soulsGetRouterV1Handler(ctx: ActionCtx, request: Request) {
'Content-Security-Policy':
"default-src 'none'; base-uri 'none'; form-action 'none'; frame-ancestors 'none'",
...(isSvg ? { 'Content-Disposition': 'attachment' } : {}),
...CORS_HEADERS,
})
return new Response(textContent, { status: 200, headers })
}
Expand Down
23 changes: 23 additions & 0 deletions convex/lib/cors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* CORS headers for public API endpoints.
*
* ClawHub is a public skill registry. Browser-based clients (like Cove WebUI)
* need CORS headers to fetch skills directly from the API.
*/

export const CORS_HEADERS = {
'Access-Control-Allow-Origin': '*',
'Access-Control-Allow-Methods': 'GET, POST, DELETE, OPTIONS',
'Access-Control-Allow-Headers': 'Content-Type, Authorization',
'Access-Control-Max-Age': '86400', // 24 hours
} as const
Comment on lines +8 to +13
Copy link

Choose a reason for hiding this comment

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

[P2] Access-Control-Allow-Headers may be too narrow for real browser clients

CORS_HEADERS hard-codes 'Access-Control-Allow-Headers': 'Content-Type, Authorization' (convex/lib/cors.ts:8-13). In practice, browser requests can include other headers that trigger a preflight (e.g., Accept, If-None-Match, Range, X-Requested-With). If a client sends any additional header, the preflight will be rejected even though the server is otherwise willing to serve the request.

Consider either echoing Access-Control-Request-Headers or broadening this list to the headers your clients actually send.

Prompt To Fix With AI
This is a comment left during a code review.
Path: convex/lib/cors.ts
Line: 8:13

Comment:
[P2] `Access-Control-Allow-Headers` may be too narrow for real browser clients

`CORS_HEADERS` hard-codes `'Access-Control-Allow-Headers': 'Content-Type, Authorization'` (`convex/lib/cors.ts:8-13`). In practice, browser requests can include other headers that trigger a preflight (e.g., `Accept`, `If-None-Match`, `Range`, `X-Requested-With`). If a client sends any additional header, the preflight will be rejected even though the server is otherwise willing to serve the request.

Consider either echoing `Access-Control-Request-Headers` or broadening this list to the headers your clients actually send.

How can I resolve this? If you propose a fix, please make it concise.


/**
* Handle CORS preflight (OPTIONS) requests.
*/
export function corsPreflightResponse(): Response {
return new Response(null, {
status: 204,
headers: CORS_HEADERS,
})
}