diff --git a/packages/core/src/plugins/core-plugins/analytics/routes/api.ts b/packages/core/src/plugins/core-plugins/analytics/routes/api.ts index 69b02d16a..6c73c62d1 100644 --- a/packages/core/src/plugins/core-plugins/analytics/routes/api.ts +++ b/packages/core/src/plugins/core-plugins/analytics/routes/api.ts @@ -1,11 +1,13 @@ import { Hono } from 'hono' import { EventTrackingService } from '../services/event-tracking-service' import type { Bindings, Variables } from '../../../../app' +import { rateLimit } from '../../../../middleware' const apiRoutes = new Hono<{ Bindings: Bindings; Variables: Variables }>() -// POST /api/events - Track a single event or batch of events -apiRoutes.post('/', async (c) => { +// POST /api/events - Track a single event or batch of events. Public + unauthenticated +// ingest, so it is rate-limited per IP to bound anonymous write amplification. +apiRoutes.post('/', rateLimit({ max: 60, windowMs: 60 * 1000, keyPrefix: 'events-ingest' }), async (c) => { const db = c.env.DB const service = new EventTrackingService(db) diff --git a/packages/core/src/routes/api-system.test.ts b/packages/core/src/routes/api-system.test.ts index 961772d29..239ef6ee1 100644 --- a/packages/core/src/routes/api-system.test.ts +++ b/packages/core/src/routes/api-system.test.ts @@ -31,14 +31,20 @@ function createMockEnv(overrides: Partial<{ } } -// Create test app with mock environment -function createTestApp(env: any = createMockEnv()) { +// Create test app with mock environment. By default a signed-in user is set so +// the auth-gated routes (/stats, /env) are reachable; pass `{ user: null }` to +// simulate an anonymous caller. +function createTestApp(env: any = createMockEnv(), opts: { user?: any | null } = {}) { const app = new Hono() + const user = opts.user === undefined + ? { userId: 'test-user', email: 'test@example.com', role: 'admin' } + : opts.user // Add middleware to set env app.use('*', async (c, next) => { c.env = env c.set('appVersion', '2.0.0') + if (user) c.set('user', user) await next() }) @@ -220,6 +226,12 @@ describe('API System Routes', () => { resetCollectionRegistry() }) + it('returns 401 for an anonymous caller', async () => { + const app = createTestApp(createMockEnv(), { user: null }) + const res = await app.request('/api/system/stats') + expect(res.status).toBe(401) + }) + it('should return system statistics', async () => { // Stats counts content from `documents` filtered by the registry's active, // non-internal collection type_ids — register one so the content query runs. @@ -325,6 +337,12 @@ describe('API System Routes', () => { }) describe('GET /api/system/env', () => { + it('returns 401 for an anonymous caller', async () => { + const app = createTestApp(createMockEnv(), { user: null }) + const res = await app.request('/api/system/env') + expect(res.status).toBe(401) + }) + it('should return environment information', async () => { const env = createMockEnv({ ENVIRONMENT: 'production', diff --git a/packages/core/src/routes/api-system.ts b/packages/core/src/routes/api-system.ts index 511af04b5..743bb3ba1 100644 --- a/packages/core/src/routes/api-system.ts +++ b/packages/core/src/routes/api-system.ts @@ -8,6 +8,7 @@ import { Hono } from 'hono' import type { Bindings, Variables } from '../app' import { getCollectionRegistry } from '../services/collection-registry' +import { requireAuth } from '../middleware' export const apiSystemRoutes = new Hono<{ Bindings: Bindings; Variables: Variables }>() @@ -127,8 +128,12 @@ apiSystemRoutes.get('/info', (c) => { /** * System stats * GET /api/system/stats + * + * Requires authentication — exposes aggregate content/media/user counts + * (including the total user count), which is business-intelligence recon for an + * anonymous caller. */ -apiSystemRoutes.get('/stats', async (c) => { +apiSystemRoutes.get('/stats', requireAuth(), async (c) => { try { const db = c.env.DB @@ -211,8 +216,12 @@ apiSystemRoutes.get('/ping', async (c) => { /** * Environment check * GET /api/system/env + * + * Requires authentication — reveals which bindings/integrations are configured + * (DB, cache, R2, email queue, SendGrid, Cloudflare Images), an infrastructure + * fingerprint that should not be exposed to anonymous callers. */ -apiSystemRoutes.get('/env', (c) => { +apiSystemRoutes.get('/env', requireAuth(), (c) => { return c.json({ environment: c.env.ENVIRONMENT || 'production', features: { diff --git a/packages/core/src/routes/public-forms.test.ts b/packages/core/src/routes/public-forms.test.ts index a1e9587db..e22782a83 100644 --- a/packages/core/src/routes/public-forms.test.ts +++ b/packages/core/src/routes/public-forms.test.ts @@ -218,3 +218,59 @@ describe('POST /api/forms/:identifier/submit — XSS sanitization', () => { expect(stored.field3).not.toContain('>') }) }) + +describe('POST /api/forms/:identifier/submit — payload DoS guards', () => { + beforeEach(() => { + vi.clearAllMocks() + capturedSubmissionData = null + }) + + it('rejects a deeply nested payload with 413 (no stack exhaustion)', async () => { + const app = createTestApp(createMockDb()) + let deep: any = 'x' + for (let i = 0; i < 60; i++) deep = { a: deep } // exceeds MAX_SANITIZE_DEPTH (32) + + const res = await app.request('/api/forms/form-1/submit', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ data: deep }), + }) + + expect(res.status).toBe(413) + expect(capturedSubmissionData).toBeNull() // nothing persisted + }) + + it('rejects a payload with too many fields with 413', async () => { + const app = createTestApp(createMockDb()) + const res = await app.request('/api/forms/form-1/submit', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ data: { arr: new Array(10_001).fill('x') } }), // > MAX_SANITIZE_NODES + }) + + expect(res.status).toBe(413) + expect(capturedSubmissionData).toBeNull() + }) + + it('rejects an oversized body by Content-Length with 413', async () => { + const app = createTestApp(createMockDb()) + const res = await app.request('/api/forms/form-1/submit', { + method: 'POST', + headers: { 'Content-Type': 'application/json', 'Content-Length': String(600 * 1024) }, + body: JSON.stringify({ data: { note: 'small body, large declared length' } }), + }) + + expect(res.status).toBe(413) + }) + + it('still accepts a normal small submission', async () => { + const app = createTestApp(createMockDb()) + const res = await app.request('/api/forms/form-1/submit', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ data: { name: 'Jane', message: 'hello' } }), + }) + + expect(res.status).toBe(200) + }) +}) diff --git a/packages/core/src/routes/public-forms.ts b/packages/core/src/routes/public-forms.ts index fefe1870b..d99e473e5 100644 --- a/packages/core/src/routes/public-forms.ts +++ b/packages/core/src/routes/public-forms.ts @@ -1,27 +1,52 @@ import { Hono } from 'hono' import { TurnstileService } from '../plugins/core-plugins/turnstile-plugin/services/turnstile' import { sanitizeInput } from '../utils/sanitize' +import { rateLimit } from '../middleware' + +// Reject oversized submission bodies before parsing — a form submission is small +// JSON, so anything past this ceiling is abuse (memory pressure on JSON.parse). +const MAX_SUBMISSION_BODY_BYTES = 512 * 1024 // Note: form-collection-sync was removed in the drop-db-collections plan (PR 4). // Form submissions are still persisted to `form_submissions`; the legacy dual-write // to the `content` table is gone — content created by submissions will move to the // document model in a follow-up. +// Bounds on the recursive sanitizer below. Public, unauthenticated submissions +// are attacker-controlled, so unbounded recursion is a stack-exhaustion DoS and +// an unbounded node count is a CPU DoS. A real form is shallow and small; these +// ceilings are far above any legitimate submission. +const MAX_SANITIZE_DEPTH = 32 +const MAX_SANITIZE_NODES = 10_000 + +/** Thrown when a submission exceeds the structural limits above. */ +class PayloadTooComplexError extends Error {} + /** * Recursively sanitize all string values in arbitrary JSON data. * HTML-encodes entities (e.g., < becomes <) to prevent stored XSS * when form submission data is rendered in admin templates. + * + * Guarded against hostile payloads: throws PayloadTooComplexError if the data + * nests deeper than MAX_SANITIZE_DEPTH or contains more than MAX_SANITIZE_NODES + * values. */ -function sanitizeDeep(value: unknown): unknown { +function sanitizeDeep(value: unknown, depth = 0, counter = { n: 0 }): unknown { + if (depth > MAX_SANITIZE_DEPTH) { + throw new PayloadTooComplexError('submission nested too deeply') + } + if (++counter.n > MAX_SANITIZE_NODES) { + throw new PayloadTooComplexError('submission has too many fields') + } if (typeof value === 'string') { return sanitizeInput(value) } if (Array.isArray(value)) { - return value.map(sanitizeDeep) + return value.map((v) => sanitizeDeep(v, depth + 1, counter)) } if (value !== null && typeof value === 'object') { const result: Record = {} for (const [k, v] of Object.entries(value)) { - result[k] = sanitizeDeep(v) + result[k] = sanitizeDeep(v, depth + 1, counter) } return result } @@ -513,11 +538,21 @@ publicFormsRoutes.get('/:name', async (c) => { } }) -// Handle form submission (accepts either name or ID) -publicFormsRoutes.post('/:identifier/submit', async (c) => { +// Handle form submission (accepts either name or ID). Public + unauthenticated, +// so it is rate-limited per IP and bounds the request body. +publicFormsRoutes.post( + '/:identifier/submit', + rateLimit({ max: 20, windowMs: 60 * 1000, keyPrefix: 'form-submit' }), + async (c) => { try { const db = c.env.DB const identifier = c.req.param('identifier') + + // Reject oversized bodies before parsing. + const declaredLength = Number(c.req.header('content-length') || 0) + if (declaredLength > MAX_SUBMISSION_BODY_BYTES) { + return c.json({ error: 'Submission too large' }, 413) + } const body = await c.req.json() // Get form by ID or name @@ -572,8 +607,17 @@ publicFormsRoutes.post('/:identifier/submit', async (c) => { } // Sanitize all string values in submission data to prevent stored XSS. - // HTML-encodes entities (e.g., < becomes <) before storage. - const sanitizedData = sanitizeDeep(body.data) as Record + // HTML-encodes entities (e.g., < becomes <) before storage. Bounded to + // reject hostile deeply-nested / oversized payloads. + let sanitizedData: Record + try { + sanitizedData = sanitizeDeep(body.data) as Record + } catch (err) { + if (err instanceof PayloadTooComplexError) { + return c.json({ error: 'Submission payload too large or too deeply nested' }, 413) + } + throw err + } // Create submission const submissionId = crypto.randomUUID() diff --git a/tests/e2e/110-public-surface-hardening.spec.ts b/tests/e2e/110-public-surface-hardening.spec.ts new file mode 100644 index 000000000..7f58604e9 --- /dev/null +++ b/tests/e2e/110-public-surface-hardening.spec.ts @@ -0,0 +1,34 @@ +import { test, expect } from '@playwright/test' + +// A4 — hardening the anonymous public surface: +// - /api/system/stats and /api/system/env now require authentication +// (they leaked user counts and infrastructure configuration) +// - the public form-submit endpoint bounds the request body +// These run unauthenticated against the deployed preview. + +test.describe('public surface hardening @smoke @api', () => { + test('GET /api/system/stats requires auth', async ({ request }) => { + const res = await request.get('/api/system/stats') + expect(res.status()).toBe(401) + }) + + test('GET /api/system/env requires auth', async ({ request }) => { + const res = await request.get('/api/system/env') + expect(res.status()).toBe(401) + }) + + test('GET /api/system/health stays public', async ({ request }) => { + const res = await request.get('/api/system/health') + // Health probe must remain reachable without auth. + expect(res.status()).not.toBe(401) + }) + + test('oversized form submission is rejected with 413', async ({ request }) => { + // The body-size guard runs before the form lookup, so any identifier works. + const res = await request.post('/api/forms/anything/submit', { + headers: { 'content-type': 'application/json' }, + data: { data: { blob: 'x'.repeat(600 * 1024) } }, // > 512KB ceiling + }) + expect(res.status()).toBe(413) + }) +})