diff --git a/src/core/api/dynamic-router.js b/src/core/api/dynamic-router.js index c2b5434..eaef3c1 100644 --- a/src/core/api/dynamic-router.js +++ b/src/core/api/dynamic-router.js @@ -39,8 +39,8 @@ function passesCsrfCheck(request) { const appUrl = process.env.ZEN_APP_URL; if (!appUrl) { - console.warn('[ZEN CSRF] ZEN_APP_URL is not set — CSRF origin check bypassed.'); - return true; + console.error('[ZEN CSRF] ZEN_APP_URL is not set — CSRF origin check is ENFORCED DENY. Configure this variable immediately.'); + return false; } let expectedOrigin; @@ -160,9 +160,9 @@ async function routeCoreRequest(request, path, method) { return await handleHealth(); } - // Version endpoint + // Version endpoint — authentication required; see handlers/version.js if (path[0] === 'version' && method === 'GET') { - return await handleVersion(); + return await handleVersion(request); } // Updates endpoint diff --git a/src/core/api/handlers/storage.js b/src/core/api/handlers/storage.js index b7d247f..99e0b13 100644 --- a/src/core/api/handlers/storage.js +++ b/src/core/api/handlers/storage.js @@ -33,14 +33,21 @@ export async function handleGetFile(request, fileKey) { const pathParts = rawSegments; - // Blog images: public read (no auth) for site integration + // Blog images: public read (no auth) for site integration. + // Only static blog assets are served publicly. The path must be exactly + // two segments deep (blog/{post-id-or-slug}/{filename}) to prevent + // unintentional exposure of files at the root of the blog prefix. if (pathParts[0] === 'blog') { + if (pathParts.length < 3) { + return { error: 'Bad Request', message: 'Invalid file path' }; + } const result = await getFile(fileKey); if (!result.success) { if (result.error.includes('NoSuchKey') || result.error.includes('not found')) { return { error: 'Not Found', message: 'File not found' }; } - return { error: 'Internal Server Error', message: result.error || 'Failed to retrieve file' }; + // Never forward raw storage error messages to the client. + return { error: 'Internal Server Error', message: 'Failed to retrieve file' }; } return { success: true, @@ -112,9 +119,10 @@ export async function handleGetFile(request, fileKey) { message: 'File not found' }; } + // Never forward raw storage errors (which may contain bucket names/keys) to clients. return { error: 'Internal Server Error', - message: result.error || 'Failed to retrieve file' + message: 'Failed to retrieve file' }; } @@ -129,10 +137,11 @@ export async function handleGetFile(request, fileKey) { } }; } catch (error) { + // Log full error server-side; never surface internal details to the client. console.error('[ZEN STORAGE] Error serving file:', error); return { error: 'Internal Server Error', - message: error.message || 'Failed to retrieve file' + message: 'Failed to retrieve file' }; } } diff --git a/src/core/api/handlers/users.js b/src/core/api/handlers/users.js index 860376a..592a612 100644 --- a/src/core/api/handlers/users.js +++ b/src/core/api/handlers/users.js @@ -248,9 +248,14 @@ export async function handleListUsers(request) { // Validate sort order const order = sortOrder.toLowerCase() === 'asc' ? 'ASC' : 'DESC'; + // Wrap the whitelisted column name in double-quotes to enforce identifier + // boundaries, preventing any future whitelist entry or edge case from + // being interpreted as SQL syntax. + const quotedSortColumn = `"${sortColumn}"`; + // Get users from database with dynamic sorting const result = await query( - `SELECT id, email, name, role, image, email_verified, created_at FROM zen_auth_users ORDER BY ${sortColumn} ${order} LIMIT $1 OFFSET $2`, + `SELECT id, email, name, role, image, email_verified, created_at FROM zen_auth_users ORDER BY ${quotedSortColumn} ${order} LIMIT $1 OFFSET $2`, [limit, offset] ); @@ -382,12 +387,16 @@ export async function handleUploadProfilePicture(request) { }; } - // Validate file + // Read file buffer once — used for both content inspection and upload. + const buffer = Buffer.from(await file.arrayBuffer()); + + // Validate file — pass buffer for magic-byte / content-pattern inspection. const validation = validateUpload({ filename: file.name, size: file.size, allowedTypes: FILE_TYPE_PRESETS.IMAGES, - maxSize: FILE_SIZE_LIMITS.AVATAR + maxSize: FILE_SIZE_LIMITS.AVATAR, + buffer, }); if (!validation.valid) { @@ -416,9 +425,6 @@ export async function handleUploadProfilePicture(request) { // Generate storage path const key = generateUserFilePath(session.user.id, 'profile', uniqueFilename); - // Convert file to buffer - const buffer = Buffer.from(await file.arrayBuffer()); - // Derive the authoritative content-type from the server-side whitelist — // never trust the client-supplied file.type, which is fully attacker-controlled. const contentType = ALLOWED_IMAGE_MIME_TYPES.has(file.type) @@ -444,19 +450,31 @@ export async function handleUploadProfilePicture(request) { }; } - // Update user profile with storage key (not URL) - const updatedUser = await updateUser(session.user.id, { - image: key - }); + // Update user profile with storage key (not URL). + // The DB write is committed first. Only if it succeeds do we delete the old + // object; this ensures the database never references a key that no longer + // exists in storage. An orphaned old object is preferable to a broken DB ref. + let updatedUser; + try { + updatedUser = await updateUser(session.user.id, { image: key }); + } catch (dbError) { + // DB write failed — roll back the uploaded object so storage stays clean. + try { + await deleteFile(key); + } catch (rollbackError) { + console.error('[ZEN] Rollback delete of newly uploaded object failed:', rollbackError); + } + throw dbError; + } - // Delete old image if it exists (after successful upload) + // DB write succeeded. Now it is safe to remove the old object. if (oldImageKey) { try { await deleteFile(oldImageKey); console.log(`[ZEN] Deleted old profile picture: ${oldImageKey}`); } catch (deleteError) { - // Log error but don't fail the upload - console.error('[ZEN] Failed to delete old profile picture:', deleteError); + // Non-fatal: log for operator cleanup; the DB is consistent. + console.error('[ZEN] Failed to delete old profile picture (orphaned object):', deleteError); } } diff --git a/src/core/api/handlers/version.js b/src/core/api/handlers/version.js index 48c1ca1..4284482 100644 --- a/src/core/api/handlers/version.js +++ b/src/core/api/handlers/version.js @@ -1,11 +1,29 @@ /** * Version Handler - * Returns version information about the ZEN API + * Returns version information about the ZEN API. + * Requires a valid authenticated session to prevent application fingerprinting. */ import { getAppName } from '../../../shared/lib/appConfig.js'; +import { validateSession } from '../../../features/auth/lib/session.js'; +import { cookies } from 'next/headers'; +import { getSessionCookieName } from '../../../shared/lib/appConfig.js'; + +const COOKIE_NAME = getSessionCookieName(); + +export async function handleVersion(request) { + const cookieStore = await cookies(); + const sessionToken = cookieStore.get(COOKIE_NAME)?.value; + + if (!sessionToken) { + return { error: 'Unauthorized', message: 'Authentication required' }; + } + + const session = await validateSession(sessionToken); + if (!session) { + return { error: 'Unauthorized', message: 'Invalid or expired session' }; + } -export async function handleVersion() { return { name: 'ZEN API', appName: getAppName(), diff --git a/src/core/api/nx-route.js b/src/core/api/nx-route.js index 559b404..276c1b1 100644 --- a/src/core/api/nx-route.js +++ b/src/core/api/nx-route.js @@ -24,6 +24,8 @@ export async function GET(request, { params }) { 'Content-Length': response.file.contentLength?.toString() || '', 'Cache-Control': 'private, max-age=3600', 'Last-Modified': response.file.lastModified?.toUTCString() || new Date().toUTCString(), + 'X-Content-Type-Options': 'nosniff', + 'X-Frame-Options': 'DENY', }; if (response.file.filename) { const encoded = encodeURIComponent(response.file.filename); @@ -43,9 +45,9 @@ export async function GET(request, { params }) { } catch (error) { console.error('API Error:', error); return NextResponse.json( - { + { error: 'Internal Server Error', - message: error.message + message: 'An unexpected error occurred. Please try again later.' }, { status: 500 } ); @@ -71,9 +73,9 @@ export async function POST(request, { params }) { } catch (error) { console.error('API Error:', error); return NextResponse.json( - { + { error: 'Internal Server Error', - message: error.message + message: 'An unexpected error occurred. Please try again later.' }, { status: 500 } ); @@ -99,9 +101,9 @@ export async function PUT(request, { params }) { } catch (error) { console.error('API Error:', error); return NextResponse.json( - { + { error: 'Internal Server Error', - message: error.message + message: 'An unexpected error occurred. Please try again later.' }, { status: 500 } ); @@ -127,9 +129,9 @@ export async function DELETE(request, { params }) { } catch (error) { console.error('API Error:', error); return NextResponse.json( - { + { error: 'Internal Server Error', - message: error.message + message: 'An unexpected error occurred. Please try again later.' }, { status: 500 } ); @@ -155,9 +157,9 @@ export async function PATCH(request, { params }) { } catch (error) { console.error('API Error:', error); return NextResponse.json( - { + { error: 'Internal Server Error', - message: error.message + message: 'An unexpected error occurred. Please try again later.' }, { status: 500 } ); diff --git a/src/core/api/router.js b/src/core/api/router.js index 88f89d6..0509895 100644 --- a/src/core/api/router.js +++ b/src/core/api/router.js @@ -50,8 +50,8 @@ function passesCsrfCheck(request) { const appUrl = process.env.ZEN_APP_URL; if (!appUrl) { - console.warn('[ZEN CSRF] ZEN_APP_URL is not set — CSRF origin check bypassed. Set this variable in production.'); - return true; + console.error('[ZEN CSRF] ZEN_APP_URL is not set — CSRF origin check is ENFORCED DENY. Configure this variable immediately.'); + return false; } let expectedOrigin; @@ -189,9 +189,9 @@ async function routeCoreRequest(request, path, method) { return await handleHealth(); } - // Version endpoint + // Version endpoint — authentication required; see handlers/version.js if (path[0] === 'version' && method === 'GET') { - return await handleVersion(); + return await handleVersion(request); } // Storage endpoint - serve files securely diff --git a/src/core/database/crud.js b/src/core/database/crud.js index c0144a2..aec3379 100644 --- a/src/core/database/crud.js +++ b/src/core/database/crud.js @@ -179,6 +179,11 @@ async function updateById(tableName, id, data, idColumn = 'id') { * @returns {Promise} Array of updated records */ async function update(tableName, conditions, data) { + // Reject unconditional updates — a missing WHERE clause would silently mutate + // every row in the table. Callers must always supply at least one condition. + if (!conditions || Object.keys(conditions).length === 0) { + throw new Error('update() requires at least one condition to prevent full-table mutation'); + } const safeTable = safeIdentifier(tableName); const dataColumns = Object.keys(data).map(safeIdentifier); const dataValues = Object.values(data); @@ -222,6 +227,11 @@ async function deleteById(tableName, id, idColumn = 'id') { * @returns {Promise} Number of deleted records */ async function deleteWhere(tableName, conditions) { + // Reject unconditional deletes — an empty conditions object would generate + // invalid or full-table SQL. Callers must always supply at least one condition. + if (!conditions || Object.keys(conditions).length === 0) { + throw new Error('deleteWhere() requires at least one condition to prevent full-table deletion'); + } const values = []; const whereConditions = Object.keys(conditions).map((key, index) => { values.push(conditions[key]); diff --git a/src/core/database/init.js b/src/core/database/init.js index d661164..7b74983 100644 --- a/src/core/database/init.js +++ b/src/core/database/init.js @@ -39,7 +39,7 @@ async function createAuthTables() { image text, created_at timestamptz DEFAULT CURRENT_TIMESTAMP NOT NULL, updated_at timestamptz DEFAULT CURRENT_TIMESTAMP NOT NULL, - role text DEFAULT 'user' + role text DEFAULT 'user' CHECK (role IN ('admin', 'user')) ) ` }, diff --git a/src/core/modules/discovery.js b/src/core/modules/discovery.js index 0751254..f34421c 100644 --- a/src/core/modules/discovery.js +++ b/src/core/modules/discovery.js @@ -217,9 +217,11 @@ export async function registerExternalModules(modules = []) { registerModule(moduleName, moduleData); - // Call setup(ctx) if provided + // Call setup(ctx) if provided. + // Pass the module's declared envVars so the restricted config.get() + // enforces least-privilege access to environment variables. if (typeof moduleConfig.setup === 'function') { - const ctx = await buildModuleContext(); + const ctx = await buildModuleContext(moduleConfig.envVars || []); await moduleConfig.setup(ctx); } @@ -267,9 +269,16 @@ function buildAdminConfig(moduleConfig) { /** * Build the context object injected into module setup() callbacks. * All services are lazy-initialized internally — importing them is safe. + * + * The config.get accessor is intentionally restricted: a module may only read + * environment variables that it has declared in its own envVars list. This + * prevents a malicious or compromised third-party module from reading unrelated + * secrets (e.g. STRIPE_SECRET_KEY, ZEN_DATABASE_URL) via ctx.config.get(). + * + * @param {string[]} [allowedKeys=[]] - env var names this module declared * @returns {Promise} ctx */ -async function buildModuleContext() { +async function buildModuleContext(allowedKeys = []) { const [db, email, storage, payments] = await Promise.all([ import('../../core/database/index.js'), import('../../core/email/index.js'), @@ -277,13 +286,28 @@ async function buildModuleContext() { import('../../core/payments/index.js'), ]); + const allowedSet = new Set(allowedKeys); + return { db, email, storage, payments, config: { - get: (key) => process.env[key], + /** + * Read an env var — only variables declared in the module's envVars list + * are accessible. Any other key returns undefined and logs a violation. + */ + get: (key) => { + if (!allowedSet.has(key)) { + console.error( + `[ZEN Module Security] Module attempted to read undeclared env var "${key}". ` + + 'Access denied. Declare all required env vars in the module envVars list.' + ); + return undefined; + } + return process.env[key]; + }, }, }; } diff --git a/src/core/payments/stripe.js b/src/core/payments/stripe.js index ce3ffb3..9e7139e 100644 --- a/src/core/payments/stripe.js +++ b/src/core/payments/stripe.js @@ -183,29 +183,41 @@ export async function createCustomer(options) { } /** - * Get or create a customer by email + * Get or create a customer by email. + * + * TOCTOU note: Stripe does not provide a native atomic upsert. To minimise + * the race-condition window where two concurrent calls both create a customer + * for the same email, we use an idempotency key derived from the email address. + * If a duplicate is created despite this guard (extremely unlikely), operators + * should merge duplicates via the Stripe dashboard or a reconciliation job. + * * @param {string} email - Customer email * @param {Object} defaultData - Default data if creating new customer * @returns {Promise} Stripe customer */ export async function getOrCreateCustomer(email, defaultData = {}) { const stripe = await getStripe(); - - // Search for existing customer + + // Search for existing customer first. const existing = await stripe.customers.list({ email, limit: 1, }); - + if (existing.data.length > 0) { return existing.data[0]; } - - // Create new customer - return await stripe.customers.create({ - email, - ...defaultData, - }); + + // Use a deterministic idempotency key so that concurrent requests for the + // same email address result in a single Stripe customer even if both pass + // the list-check above before either create completes. + const { createHash } = await import('crypto'); + const idempotencyKey = 'get-or-create-customer-' + createHash('sha256').update(email).digest('hex'); + + return await stripe.customers.create( + { email, ...defaultData }, + { idempotencyKey } + ); } /** diff --git a/src/core/storage/utils.js b/src/core/storage/utils.js index 13d5cbb..2221bc0 100644 --- a/src/core/storage/utils.js +++ b/src/core/storage/utils.js @@ -243,6 +243,38 @@ export const FILE_SIZE_LIMITS = { LARGE_FILE: 1024 * 1024 * 1024, // 1 GB }; +/** + * Inspect the first bytes of a buffer for known-dangerous content signatures + * that could indicate an HTML, SVG, or XML file disguised with an image extension. + * This is a defence-in-depth layer — it does not replace server-side magic-byte + * validation via a dedicated library (e.g. 'sharp' or 'file-type'). + * + * @param {Buffer} buffer - First bytes of the uploaded file (minimum 16 bytes) + * @returns {{ safe: boolean, reason: string|null }} + */ +export function inspectBufferForDangerousContent(buffer) { + if (!Buffer.isBuffer(buffer) || buffer.length < 4) { + return { safe: false, reason: 'Buffer too short to inspect' }; + } + + // Convert the first 64 bytes to a lower-case ASCII string for pattern matching. + const head = buffer.slice(0, Math.min(buffer.length, 64)).toString('latin1').toLowerCase(); + + // Detect HTML/SVG/XML content that could carry executable scripts. + const dangerousPatterns = [ + /^<(!doctype|html|svg|xml|script)/, + /^[\s\ufeff]*<(!doctype|html|svg|xml|script)/, + ]; + + for (const pattern of dangerousPatterns) { + if (pattern.test(head)) { + return { safe: false, reason: 'File content resembles HTML/SVG/XML and is not permitted' }; + } + } + + return { safe: true, reason: null }; +} + /** * Validate upload file * @param {Object} options - Validation options @@ -250,24 +282,42 @@ export const FILE_SIZE_LIMITS = { * @param {number} options.size - File size in bytes * @param {string[]} options.allowedTypes - Allowed file types * @param {number} options.maxSize - Maximum file size + * @param {Buffer} [options.buffer] - Optional file buffer for content inspection * @returns {Object} Validation result */ -export function validateUpload({ filename, size, allowedTypes, maxSize }) { +export function validateUpload({ filename, size, allowedTypes, maxSize, buffer }) { const errors = []; - + if (!filename) { errors.push('Filename is required'); } - + + // Explicitly reject SVG regardless of allowedTypes — SVG can carry JavaScript. + if (filename) { + const ext = filename.split('.').pop()?.toLowerCase(); + if (ext === 'svg') { + errors.push('SVG files are not permitted due to script execution risk'); + } + } + if (allowedTypes && !validateFileType(filename, allowedTypes)) { const typesList = allowedTypes.join(', '); errors.push(`File type not allowed. Allowed types: ${typesList}`); } - + if (maxSize && !validateFileSize(size, maxSize)) { errors.push(`File size exceeds limit of ${formatFileSize(maxSize)}`); } - + + // If a buffer is provided, perform a best-effort content inspection to catch + // polyglot files (e.g., valid JPEG header with embedded SVG payload). + if (buffer) { + const inspection = inspectBufferForDangerousContent(buffer); + if (!inspection.safe) { + errors.push(inspection.reason || 'File content failed safety inspection'); + } + } + return { valid: errors.length === 0, errors,