fix: sanitize error responses to prevent internal detail leakage
Add sanitizeError() helper that strips file paths, stack traces, SQLite errors, and system errors from messages before returning them to clients. Applied to all route-level catch blocks in payments, queue, fights, and admin routes. Includes 12 tests for the sanitizer and static analysis test verifying no route files leak raw err.message. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
1b1f9eb2d7
commit
9b0d251d1c
@@ -4,6 +4,7 @@ import { eq, desc, sql, count } from 'drizzle-orm'
|
||||
import { getActiveSSECount } from './fights.js'
|
||||
import { createBackup } from '../engine/backup.js'
|
||||
import { isCreatorPubkey } from '../lib/constants.js'
|
||||
import { sanitizeError } from '../lib/validators.js'
|
||||
|
||||
const startTime = Date.now()
|
||||
|
||||
@@ -115,7 +116,7 @@ adminRouter.get('/backup', (c) => {
|
||||
const path = createBackup()
|
||||
return c.json({ ok: true, path })
|
||||
} catch (err: unknown) {
|
||||
const msg = err instanceof Error ? err.message : 'Backup failed'
|
||||
const msg = sanitizeError(err, 'Backup failed')
|
||||
return c.json({ error: msg }, 500)
|
||||
}
|
||||
})
|
||||
|
||||
@@ -94,4 +94,22 @@ describe('global error handler', () => {
|
||||
expect(content).not.toMatch(/c\.json\([^)]*error\.stack/)
|
||||
}
|
||||
})
|
||||
|
||||
it('route catch blocks use sanitizeError instead of raw err.message', async () => {
|
||||
const fs = await import('fs')
|
||||
const path = await import('path')
|
||||
const routesDir = path.resolve(import.meta.dirname || '.', '.')
|
||||
// Files that handle errors returned to clients (excluding webhook test tools: bots.ts, docs.ts)
|
||||
const checkedFiles = ['payments.ts', 'queue.ts', 'fights.ts', 'admin.ts', 'tournaments.ts']
|
||||
|
||||
for (const file of checkedFiles) {
|
||||
const filePath = path.join(routesDir, file)
|
||||
if (!fs.existsSync(filePath)) continue
|
||||
const content = fs.readFileSync(filePath, 'utf-8')
|
||||
// Should not have the old pattern: err instanceof Error ? err.message : '...'
|
||||
// in the context of returning to c.json (route responses)
|
||||
const unsafePattern = /c\.json\(\{[^}]*err\s*(instanceof\s+Error\s*\?\s*err\.message|\.message)/
|
||||
expect(content, `${file} should use sanitizeError, not raw err.message in responses`).not.toMatch(unsafePattern)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -13,7 +13,7 @@ import { getPendingChallenge, getPendingAnswers, submitHumanResponse } from '../
|
||||
import { getPendingPollChallenge, submitPollResponse, isPollingBot } from '../engine/poll-responses.js'
|
||||
import { authenticateBot } from '../middleware/bot-auth.js'
|
||||
import { checkAnswer } from '../engine/answers.js'
|
||||
import { respondSchema, reactSchema } from '../lib/validators.js'
|
||||
import { respondSchema, reactSchema, sanitizeError } from '../lib/validators.js'
|
||||
|
||||
const isValidId = (id: string) => /^[a-zA-Z0-9_-]{1,64}$/.test(id)
|
||||
|
||||
@@ -250,7 +250,7 @@ fightsRouter.post('/matchmake/:botId', botRateLimit(10_000), async (c) => {
|
||||
try {
|
||||
fightId = await runFightAsync(botId, opponent.id)
|
||||
} catch (err) {
|
||||
const msg = err instanceof Error ? err.message : 'Fight failed to start'
|
||||
const msg = sanitizeError(err, 'Fight failed to start')
|
||||
return c.json({ error: msg }, 400)
|
||||
}
|
||||
|
||||
@@ -304,7 +304,7 @@ fightsRouter.post('/practice/:botId', botRateLimit(10_000), async (c) => {
|
||||
const overrides = isPollingBot(bot.webhookUrl) ? { botAWebhookUrl: 'http://human.local/' } : undefined
|
||||
fightId = await runFightAsync(botId, opponent.id, 'free', overrides)
|
||||
} catch (err) {
|
||||
const msg = err instanceof Error ? err.message : 'Fight failed to start'
|
||||
const msg = sanitizeError(err, 'Fight failed to start')
|
||||
return c.json({ error: msg }, 400)
|
||||
}
|
||||
|
||||
|
||||
@@ -6,7 +6,7 @@ import { eq } from 'drizzle-orm'
|
||||
import { createEntryInvoice, checkPaymentStatus, redeemCashuToken } from '../engine/payments.js'
|
||||
import { encrypt, decrypt } from '../engine/crypto.js'
|
||||
import { rateLimit } from '../middleware/rate-limit.js'
|
||||
import { connectWalletSchema, createInvoiceSchema, submitCashuSchema, disconnectWalletSchema, zapSchema, formatZodError } from '../lib/validators.js'
|
||||
import { connectWalletSchema, createInvoiceSchema, submitCashuSchema, disconnectWalletSchema, zapSchema, formatZodError, sanitizeError } from '../lib/validators.js'
|
||||
|
||||
export const paymentsRouter = new Hono()
|
||||
|
||||
@@ -103,7 +103,7 @@ paymentsRouter.post('/create-invoice', rateLimit(60_000, 10), async (c) => {
|
||||
const result = await createEntryInvoice(botId)
|
||||
return c.json(result)
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : 'Invoice creation failed'
|
||||
const message = sanitizeError(err, 'Invoice creation failed')
|
||||
return c.json({ error: message }, 500)
|
||||
}
|
||||
})
|
||||
@@ -118,7 +118,7 @@ paymentsRouter.get('/check/:paymentId', rateLimit(60_000, 30), async (c) => {
|
||||
const status = await checkPaymentStatus(paymentId)
|
||||
return c.json({ status })
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : 'Status check failed'
|
||||
const message = sanitizeError(err, 'Status check failed')
|
||||
return c.json({ error: message }, 500)
|
||||
}
|
||||
})
|
||||
@@ -194,7 +194,7 @@ paymentsRouter.post('/submit-cashu', async (c) => {
|
||||
status: result.valid ? 'confirmed' : 'failed',
|
||||
})
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : 'Cashu redemption failed'
|
||||
const message = sanitizeError(err, 'Cashu redemption failed')
|
||||
return c.json({ error: message }, 500)
|
||||
}
|
||||
})
|
||||
|
||||
@@ -4,7 +4,7 @@ import { eq } from 'drizzle-orm'
|
||||
import { joinQueue, leaveQueue, getQueueSize, getQueueSnapshot } from '../engine/queue.js'
|
||||
import { joinRankedQueue, getRankedQueueStatus } from '../engine/ranked-queue.js'
|
||||
import { rateLimit } from '../middleware/rate-limit.js'
|
||||
import { joinRankedSchema } from '../lib/validators.js'
|
||||
import { joinRankedSchema, sanitizeError } from '../lib/validators.js'
|
||||
|
||||
export const queueRouter = new Hono()
|
||||
|
||||
@@ -33,8 +33,10 @@ queueRouter.post('/join/:botId', async (c) => {
|
||||
const fightId = await joinQueue(botId)
|
||||
return c.json({ fightId, message: 'Matched! Fight starting.' })
|
||||
} catch (err: any) {
|
||||
const message = err instanceof Error ? err.message : 'Queue error'
|
||||
const status = message.includes('already in a fight') ? 409 : 500
|
||||
const raw = err instanceof Error ? err.message : ''
|
||||
const isConflict = raw.includes('already in a fight')
|
||||
const message = isConflict ? raw : sanitizeError(err, 'Queue error')
|
||||
const status = isConflict ? 409 : 500
|
||||
return c.json({ error: message, fightId: err?.fightId || undefined }, status)
|
||||
}
|
||||
})
|
||||
@@ -76,7 +78,7 @@ queueRouter.post('/join-ranked/:botId', async (c) => {
|
||||
const fightId = await joinRankedQueue(botId, paymentId)
|
||||
return c.json({ fightId, message: 'Ranked match found! Fight starting.' })
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : 'Ranked queue error'
|
||||
const message = sanitizeError(err, 'Ranked queue error')
|
||||
return c.json({ error: message }, 500)
|
||||
}
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user