fix: harden security across CSRF, storage, version, and SQL handling

- **CSRF**: Change missing `ZEN_APP_URL` behavior from bypass (return
  `true`) to enforced deny (return `false`) with an error-level log,
  preventing unauthenticated access when the env var is misconfigured

- **Version endpoint**: Require authentication on the `/version` route
  by passing `request` to `handleVersion`; add session/token validation
  inside the handler so version info is no longer publicly accessible

- **Storage handler**: Enforce a minimum path depth of 3 segments for
  public blog file access to prevent unintentional root-prefix exposure;
  strip raw storage error messages (bucket names, keys) from all client
  responses, logging full details server-side only

- **SQL injection hardening**: Wrap the whitelisted `sortColumn`
  identifier in double-quotes in the `handleListUsers` query to enforce
  identifier boundaries and prevent any edge case from being interpreted
  as SQL syntax

- **Misc**: Improve log clarity for orphaned profile picture deletion
  failures; add inline comments explaining security rationale throughout
This commit is contained in:
2026-04-12 17:49:12 -04:00
parent 7fc14fece7
commit 49ddcc02fc
11 changed files with 200 additions and 57 deletions
+4 -4
View File
@@ -39,8 +39,8 @@ function passesCsrfCheck(request) {
const appUrl = process.env.ZEN_APP_URL; const appUrl = process.env.ZEN_APP_URL;
if (!appUrl) { if (!appUrl) {
console.warn('[ZEN CSRF] ZEN_APP_URL is not set — CSRF origin check bypassed.'); console.error('[ZEN CSRF] ZEN_APP_URL is not set — CSRF origin check is ENFORCED DENY. Configure this variable immediately.');
return true; return false;
} }
let expectedOrigin; let expectedOrigin;
@@ -160,9 +160,9 @@ async function routeCoreRequest(request, path, method) {
return await handleHealth(); return await handleHealth();
} }
// Version endpoint // Version endpoint — authentication required; see handlers/version.js
if (path[0] === 'version' && method === 'GET') { if (path[0] === 'version' && method === 'GET') {
return await handleVersion(); return await handleVersion(request);
} }
// Updates endpoint // Updates endpoint
+13 -4
View File
@@ -33,14 +33,21 @@ export async function handleGetFile(request, fileKey) {
const pathParts = rawSegments; 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[0] === 'blog') {
if (pathParts.length < 3) {
return { error: 'Bad Request', message: 'Invalid file path' };
}
const result = await getFile(fileKey); const result = await getFile(fileKey);
if (!result.success) { if (!result.success) {
if (result.error.includes('NoSuchKey') || result.error.includes('not found')) { if (result.error.includes('NoSuchKey') || result.error.includes('not found')) {
return { error: 'Not Found', message: 'File 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 { return {
success: true, success: true,
@@ -112,9 +119,10 @@ export async function handleGetFile(request, fileKey) {
message: 'File not found' message: 'File not found'
}; };
} }
// Never forward raw storage errors (which may contain bucket names/keys) to clients.
return { return {
error: 'Internal Server Error', 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) { } catch (error) {
// Log full error server-side; never surface internal details to the client.
console.error('[ZEN STORAGE] Error serving file:', error); console.error('[ZEN STORAGE] Error serving file:', error);
return { return {
error: 'Internal Server Error', error: 'Internal Server Error',
message: error.message || 'Failed to retrieve file' message: 'Failed to retrieve file'
}; };
} }
} }
+31 -13
View File
@@ -248,9 +248,14 @@ export async function handleListUsers(request) {
// Validate sort order // Validate sort order
const order = sortOrder.toLowerCase() === 'asc' ? 'ASC' : 'DESC'; 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 // Get users from database with dynamic sorting
const result = await query( 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] [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({ const validation = validateUpload({
filename: file.name, filename: file.name,
size: file.size, size: file.size,
allowedTypes: FILE_TYPE_PRESETS.IMAGES, allowedTypes: FILE_TYPE_PRESETS.IMAGES,
maxSize: FILE_SIZE_LIMITS.AVATAR maxSize: FILE_SIZE_LIMITS.AVATAR,
buffer,
}); });
if (!validation.valid) { if (!validation.valid) {
@@ -416,9 +425,6 @@ export async function handleUploadProfilePicture(request) {
// Generate storage path // Generate storage path
const key = generateUserFilePath(session.user.id, 'profile', uniqueFilename); 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 — // Derive the authoritative content-type from the server-side whitelist —
// never trust the client-supplied file.type, which is fully attacker-controlled. // never trust the client-supplied file.type, which is fully attacker-controlled.
const contentType = ALLOWED_IMAGE_MIME_TYPES.has(file.type) 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) // Update user profile with storage key (not URL).
const updatedUser = await updateUser(session.user.id, { // The DB write is committed first. Only if it succeeds do we delete the old
image: key // 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) { if (oldImageKey) {
try { try {
await deleteFile(oldImageKey); await deleteFile(oldImageKey);
console.log(`[ZEN] Deleted old profile picture: ${oldImageKey}`); console.log(`[ZEN] Deleted old profile picture: ${oldImageKey}`);
} catch (deleteError) { } catch (deleteError) {
// Log error but don't fail the upload // Non-fatal: log for operator cleanup; the DB is consistent.
console.error('[ZEN] Failed to delete old profile picture:', deleteError); console.error('[ZEN] Failed to delete old profile picture (orphaned object):', deleteError);
} }
} }
+20 -2
View File
@@ -1,11 +1,29 @@
/** /**
* Version Handler * 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 { 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 { return {
name: 'ZEN API', name: 'ZEN API',
appName: getAppName(), appName: getAppName(),
+12 -10
View File
@@ -24,6 +24,8 @@ export async function GET(request, { params }) {
'Content-Length': response.file.contentLength?.toString() || '', 'Content-Length': response.file.contentLength?.toString() || '',
'Cache-Control': 'private, max-age=3600', 'Cache-Control': 'private, max-age=3600',
'Last-Modified': response.file.lastModified?.toUTCString() || new Date().toUTCString(), 'Last-Modified': response.file.lastModified?.toUTCString() || new Date().toUTCString(),
'X-Content-Type-Options': 'nosniff',
'X-Frame-Options': 'DENY',
}; };
if (response.file.filename) { if (response.file.filename) {
const encoded = encodeURIComponent(response.file.filename); const encoded = encodeURIComponent(response.file.filename);
@@ -43,9 +45,9 @@ export async function GET(request, { params }) {
} catch (error) { } catch (error) {
console.error('API Error:', error); console.error('API Error:', error);
return NextResponse.json( return NextResponse.json(
{ {
error: 'Internal Server Error', error: 'Internal Server Error',
message: error.message message: 'An unexpected error occurred. Please try again later.'
}, },
{ status: 500 } { status: 500 }
); );
@@ -71,9 +73,9 @@ export async function POST(request, { params }) {
} catch (error) { } catch (error) {
console.error('API Error:', error); console.error('API Error:', error);
return NextResponse.json( return NextResponse.json(
{ {
error: 'Internal Server Error', error: 'Internal Server Error',
message: error.message message: 'An unexpected error occurred. Please try again later.'
}, },
{ status: 500 } { status: 500 }
); );
@@ -99,9 +101,9 @@ export async function PUT(request, { params }) {
} catch (error) { } catch (error) {
console.error('API Error:', error); console.error('API Error:', error);
return NextResponse.json( return NextResponse.json(
{ {
error: 'Internal Server Error', error: 'Internal Server Error',
message: error.message message: 'An unexpected error occurred. Please try again later.'
}, },
{ status: 500 } { status: 500 }
); );
@@ -127,9 +129,9 @@ export async function DELETE(request, { params }) {
} catch (error) { } catch (error) {
console.error('API Error:', error); console.error('API Error:', error);
return NextResponse.json( return NextResponse.json(
{ {
error: 'Internal Server Error', error: 'Internal Server Error',
message: error.message message: 'An unexpected error occurred. Please try again later.'
}, },
{ status: 500 } { status: 500 }
); );
@@ -155,9 +157,9 @@ export async function PATCH(request, { params }) {
} catch (error) { } catch (error) {
console.error('API Error:', error); console.error('API Error:', error);
return NextResponse.json( return NextResponse.json(
{ {
error: 'Internal Server Error', error: 'Internal Server Error',
message: error.message message: 'An unexpected error occurred. Please try again later.'
}, },
{ status: 500 } { status: 500 }
); );
+4 -4
View File
@@ -50,8 +50,8 @@ function passesCsrfCheck(request) {
const appUrl = process.env.ZEN_APP_URL; const appUrl = process.env.ZEN_APP_URL;
if (!appUrl) { if (!appUrl) {
console.warn('[ZEN CSRF] ZEN_APP_URL is not set — CSRF origin check bypassed. Set this variable in production.'); console.error('[ZEN CSRF] ZEN_APP_URL is not set — CSRF origin check is ENFORCED DENY. Configure this variable immediately.');
return true; return false;
} }
let expectedOrigin; let expectedOrigin;
@@ -189,9 +189,9 @@ async function routeCoreRequest(request, path, method) {
return await handleHealth(); return await handleHealth();
} }
// Version endpoint // Version endpoint — authentication required; see handlers/version.js
if (path[0] === 'version' && method === 'GET') { if (path[0] === 'version' && method === 'GET') {
return await handleVersion(); return await handleVersion(request);
} }
// Storage endpoint - serve files securely // Storage endpoint - serve files securely
+10
View File
@@ -179,6 +179,11 @@ async function updateById(tableName, id, data, idColumn = 'id') {
* @returns {Promise<Array>} Array of updated records * @returns {Promise<Array>} Array of updated records
*/ */
async function update(tableName, conditions, data) { 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 safeTable = safeIdentifier(tableName);
const dataColumns = Object.keys(data).map(safeIdentifier); const dataColumns = Object.keys(data).map(safeIdentifier);
const dataValues = Object.values(data); const dataValues = Object.values(data);
@@ -222,6 +227,11 @@ async function deleteById(tableName, id, idColumn = 'id') {
* @returns {Promise<number>} Number of deleted records * @returns {Promise<number>} Number of deleted records
*/ */
async function deleteWhere(tableName, conditions) { 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 values = [];
const whereConditions = Object.keys(conditions).map((key, index) => { const whereConditions = Object.keys(conditions).map((key, index) => {
values.push(conditions[key]); values.push(conditions[key]);
+1 -1
View File
@@ -39,7 +39,7 @@ async function createAuthTables() {
image text, image text,
created_at timestamptz DEFAULT CURRENT_TIMESTAMP NOT NULL, created_at timestamptz DEFAULT CURRENT_TIMESTAMP NOT NULL,
updated_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'))
) )
` `
}, },
+28 -4
View File
@@ -217,9 +217,11 @@ export async function registerExternalModules(modules = []) {
registerModule(moduleName, moduleData); 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') { if (typeof moduleConfig.setup === 'function') {
const ctx = await buildModuleContext(); const ctx = await buildModuleContext(moduleConfig.envVars || []);
await moduleConfig.setup(ctx); await moduleConfig.setup(ctx);
} }
@@ -267,9 +269,16 @@ function buildAdminConfig(moduleConfig) {
/** /**
* Build the context object injected into module setup() callbacks. * Build the context object injected into module setup() callbacks.
* All services are lazy-initialized internally — importing them is safe. * 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<Object>} ctx * @returns {Promise<Object>} ctx
*/ */
async function buildModuleContext() { async function buildModuleContext(allowedKeys = []) {
const [db, email, storage, payments] = await Promise.all([ const [db, email, storage, payments] = await Promise.all([
import('../../core/database/index.js'), import('../../core/database/index.js'),
import('../../core/email/index.js'), import('../../core/email/index.js'),
@@ -277,13 +286,28 @@ async function buildModuleContext() {
import('../../core/payments/index.js'), import('../../core/payments/index.js'),
]); ]);
const allowedSet = new Set(allowedKeys);
return { return {
db, db,
email, email,
storage, storage,
payments, payments,
config: { 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];
},
}, },
}; };
} }
+22 -10
View File
@@ -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 {string} email - Customer email
* @param {Object} defaultData - Default data if creating new customer * @param {Object} defaultData - Default data if creating new customer
* @returns {Promise<Object>} Stripe customer * @returns {Promise<Object>} Stripe customer
*/ */
export async function getOrCreateCustomer(email, defaultData = {}) { export async function getOrCreateCustomer(email, defaultData = {}) {
const stripe = await getStripe(); const stripe = await getStripe();
// Search for existing customer // Search for existing customer first.
const existing = await stripe.customers.list({ const existing = await stripe.customers.list({
email, email,
limit: 1, limit: 1,
}); });
if (existing.data.length > 0) { if (existing.data.length > 0) {
return existing.data[0]; return existing.data[0];
} }
// Create new customer // Use a deterministic idempotency key so that concurrent requests for the
return await stripe.customers.create({ // same email address result in a single Stripe customer even if both pass
email, // the list-check above before either create completes.
...defaultData, 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 }
);
} }
/** /**
+55 -5
View File
@@ -243,6 +243,38 @@ export const FILE_SIZE_LIMITS = {
LARGE_FILE: 1024 * 1024 * 1024, // 1 GB 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 * Validate upload file
* @param {Object} options - Validation options * @param {Object} options - Validation options
@@ -250,24 +282,42 @@ export const FILE_SIZE_LIMITS = {
* @param {number} options.size - File size in bytes * @param {number} options.size - File size in bytes
* @param {string[]} options.allowedTypes - Allowed file types * @param {string[]} options.allowedTypes - Allowed file types
* @param {number} options.maxSize - Maximum file size * @param {number} options.maxSize - Maximum file size
* @param {Buffer} [options.buffer] - Optional file buffer for content inspection
* @returns {Object} Validation result * @returns {Object} Validation result
*/ */
export function validateUpload({ filename, size, allowedTypes, maxSize }) { export function validateUpload({ filename, size, allowedTypes, maxSize, buffer }) {
const errors = []; const errors = [];
if (!filename) { if (!filename) {
errors.push('Filename is required'); 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)) { if (allowedTypes && !validateFileType(filename, allowedTypes)) {
const typesList = allowedTypes.join(', '); const typesList = allowedTypes.join(', ');
errors.push(`File type not allowed. Allowed types: ${typesList}`); errors.push(`File type not allowed. Allowed types: ${typesList}`);
} }
if (maxSize && !validateFileSize(size, maxSize)) { if (maxSize && !validateFileSize(size, maxSize)) {
errors.push(`File size exceeds limit of ${formatFileSize(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 { return {
valid: errors.length === 0, valid: errors.length === 0,
errors, errors,