Skip to content

fix: add rate limiting to download counter to prevent inflation #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
orlyjamie wants to merge 1 commit into
base: main
Choose a base branch
from
+35 −2
Open

fix: add rate limiting to download counter to prevent inflation #40

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 35 additions & 2 deletions convex/downloads.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,28 @@
import { v } from 'convex/values'
import { zipSync } from 'fflate'
import { api } from './_generated/api'
import { api, internal } from './_generated/api'
import { httpAction, mutation } from './_generated/server'
import { applySkillStatDeltas, bumpDailySkillStats } from './lib/skillStats'

// Rate limit: 5 downloads per skill per IP per hour
// NOTE: This is defense-in-depth only. Download counts are fundamentally ungameable
// as a trust metric because:
// 1. Downloads are anonymous (no auth required)
// 2. Attackers can use proxies/VPNs/Tor to bypass IP rate limits
// 3. Even legitimate rate limiting can be circumvented at scale
//
// RECOMMENDATION: De-emphasize download counts in the UI. Stars and installs
// are better trust signals because they require authenticated sessions.
// Consider showing "X users installed" (from CLI telemetry) rather than downloads.
const DOWNLOAD_RATE_LIMIT = 5
const DOWNLOAD_RATE_WINDOW_MS = 60 * 60 * 1000 // 1 hour

// Only trust cf-connecting-ip - other headers like x-forwarded-for are spoofable
function getClientIpSecure(request: Request): string | null {
const cfIp = request.headers.get('cf-connecting-ip')
return cfIp?.trim() || null
}

export const downloadZip = httpAction(async (ctx, request) => {
const url = new URL(request.url)
const slug = url.searchParams.get('slug')?.trim().toLowerCase()
Expand Down Expand Up @@ -53,7 +72,21 @@ export const downloadZip = httpAction(async (ctx, request) => {
const zipArray = Uint8Array.from(zipData)
const zipBlob = new Blob([zipArray], { type: 'application/zip' })

await ctx.runMutation(api.downloads.increment, { skillId: skill._id })
// Only count download if IP passes rate limit check
const clientIp = getClientIpSecure(request)
if (clientIp) {
const rateLimitKey = `download:${skill._id}:${clientIp}`
const rateCheck = await ctx.runMutation(internal.rateLimits.checkRateLimitInternal, {
key: rateLimitKey,
limit: DOWNLOAD_RATE_LIMIT,
windowMs: DOWNLOAD_RATE_WINDOW_MS,
})
if (rateCheck.allowed) {
await ctx.runMutation(api.downloads.increment, { skillId: skill._id })
}
// If rate limited, still serve the file but don't count it
}
// If no IP (shouldn't happen on Cloudflare), don't count the download

return new Response(zipBlob, {
status: 200,
Expand Down