Security Best Practices - jra3/mulm GitHub Wiki

Security Best Practices

Development security guidelines for contributing to the Mulm platform. Follow these practices to maintain the security posture of the application.

Table of Contents

  1. Input Validation
  2. SQL Injection Prevention
  3. Authentication & Passwords
  4. Session Management
  5. File Upload Security
  6. XSS Prevention
  7. CSRF Protection
  8. Secrets Management
  9. Error Handling
  10. Security Checklist

Input Validation

Always Validate User Input

Use Zod schemas for ALL user input:

// ✅ Good - Zod validation
import { z } from 'zod';

const submissionSchema = z.object({
  species_common_name: z.string().min(2).max(200),
  species_latin_name: z.string().min(3).max(200),
  reproduction_date: z.string().datetime(),
  count: z.string().max(50),
});

const result = submissionSchema.safeParse(req.body);
if (!result.success) {
  // Handle validation errors
  return res.render('form', { errors: result.error });
}

// ❌ Bad - No validation
const name = req.body.species_common_name;
await db.run('INSERT INTO submissions (species_common_name) VALUES (?)', [name]);

Validation Rules

String lengths:

z.string().min(2).max(200)  // Prevent empty and oversized input

Email addresses:

z.string().email()  // Built-in email validation

Numeric values:

z.number().int().positive().max(10000)  // Integer, positive, bounded

Dates:

z.string().datetime()  // ISO 8601 format
z.string().refine(val => new Date(val) <= new Date(), {
  message: 'Date cannot be in the future'
})

Enums:

z.enum(['fish', 'plant', 'coral'])  // Only allow specific values

SQL Injection Prevention

ALWAYS Use Prepared Statements

✅ CORRECT - Parameterized queries:

// Using query helper
const members = await query<MemberRecord>(
  'SELECT * FROM members WHERE contact_email = ?',
  [email]  // Parameters in array
);

// Using prepared statements directly
const stmt = await db.prepare('SELECT * FROM members WHERE id = ?');
try {
  const member = await stmt.get(id);
} finally {
  await stmt.finalize();  // Always finalize!
}

// Using insertOne/updateOne helpers
await insertOne('members', {
  contact_email: userInput,
  display_name: userInput
});

await updateOne(
  'members',
  { id: memberId },
  { display_name: userInput }
);

❌ NEVER - String concatenation:

// ❌ DANGEROUS - SQL Injection vulnerability
const query = `SELECT * FROM members WHERE contact_email = '${email}'`;
await db.all(query);

// ❌ DANGEROUS - Template literals
const query = `SELECT * FROM members WHERE id = ${id}`;
await db.all(query);

// ❌ DANGEROUS - String concatenation
const query = 'SELECT * FROM members WHERE name = "' + name + '"';
await db.all(query);

Dynamic Queries

If you must build dynamic SQL (rare):

// ✅ Good - Whitelist approach
const validColumns = ['id', 'display_name', 'contact_email'];
const sortBy = validColumns.includes(req.query.sort)
  ? req.query.sort
  : 'id';

const results = await query(
  `SELECT * FROM members ORDER BY ${sortBy} ASC`  // Safe - whitelisted
);

// ❌ Bad - Direct user input
const sortBy = req.query.sort;  // User can inject: "id; DROP TABLE members--"
const results = await query(
  `SELECT * FROM members ORDER BY ${sortBy} ASC`
);

Authentication & Passwords

Password Hashing

Use scrypt (already implemented):

import { makePasswordEntry, checkPassword } from '@/auth';

// Creating password
const passwordEntry = await makePasswordEntry(clearPassword);
// Returns: { N: 16384, r: 8, p: 1, salt: '...', hash: '...' }

// Store in database
await createOrUpdatePassword(memberId, passwordEntry);

// Verifying password
const storedPassword = await getMemberPassword(memberId);
const isValid = await checkPassword(storedPassword, userEnteredPassword);

Scrypt parameters:

  • N: 16384 - CPU/memory cost (2^14)
  • r: 8 - Block size
  • p: 1 - Parallelization

Why scrypt?

  • ✅ Memory-hard (resistant to GPU attacks)
  • ✅ Tunable difficulty
  • ✅ Better than bcrypt for password hashing
  • ✅ NIST recommended

Password Requirements

Enforce minimum strength:

const passwordSchema = z.string()
  .min(8, 'Password must be at least 8 characters')
  .max(100, 'Password too long');

// Optional: Add complexity requirements
.refine(val => /[A-Z]/.test(val), 'Must contain uppercase letter')
.refine(val => /[a-z]/.test(val), 'Must contain lowercase letter')
.refine(val => /[0-9]/.test(val), 'Must contain number');

Password Reset Tokens

Time-limited, single-use codes:

// Generate secure random code
const code = generateRandomCode(24);  // 24 bytes = 192 bits

// Store with expiration
const authCode: AuthCode = {
  member_id: memberId,
  code: code,
  expires_on: new Date(Date.now() + 60 * 60 * 1000), // 1 hour
  purpose: 'password_reset'
};

await createAuthCode(authCode);

// Validate code
const codeEntry = await getAuthCode(code);
const isValid = codeEntry
  && codeEntry.purpose === 'password_reset'
  && codeEntry.expires_on > new Date();

// Delete code after use
await deleteAuthCode(code);

Security features:

  • Short expiration (1 hour)
  • Single-use (deleted after use)
  • URL-safe encoding (base64url)
  • Cryptographically random

Session Management

Cookie Security

Current implementation:

res.cookie('session_id', sessionId, {
  httpOnly: true,     // Prevent JavaScript access (XSS protection)
  secure: process.env.NODE_ENV === 'production',  // HTTPS only in production
  sameSite: 'lax',    // CSRF protection while allowing navigation
  maxAge: 180 * 86400 * 1000,  // 180 days
});

Security features:

Attribute Purpose Setting
httpOnly Prevent XSS stealing cookies true (always)
secure HTTPS only true (production)
sameSite CSRF protection lax (allows normal navigation)
maxAge Session lifetime 180 days

Session Storage

Database-backed sessions (not in-memory):

// Store in database
CREATE TABLE sessions (
  session_id TEXT PRIMARY KEY,      -- Random 64-byte base64url
  member_id INTEGER NOT NULL,       -- FK to members
  expires_on DATETIME NOT NULL      -- Auto-cleanup after expiry
);

// On each request, validate session
const viewer = await getLoggedInUser(req.cookies.session_id);
if (!viewer) {
  // Session expired or invalid
  return res.redirect('/auth/signin');
}

Why database-backed?

  • ✅ Persists across server restarts
  • ✅ Can invalidate sessions server-side
  • ✅ Easy to implement "logout everywhere"
  • ✅ Audit trail of active sessions

Session Cleanup

Remove expired sessions periodically:

-- Run daily via cron
DELETE FROM sessions WHERE expires_on < datetime('now');

File Upload Security

Magic Byte Validation

NEVER trust MIME types:

// ✅ Good - Validate magic bytes
async function validateImageBuffer(buffer: Buffer): Promise<void> {
  const magicBytes = buffer.subarray(0, 12);

  // JPEG: FF D8 FF
  const isJPEG = magicBytes[0] === 0xFF && magicBytes[1] === 0xD8 && magicBytes[2] === 0xFF;

  // PNG: 89 50 4E 47 0D 0A 1A 0A
  const isPNG = magicBytes[0] === 0x89 && magicBytes[1] === 0x50 &&
                magicBytes[2] === 0x4E && magicBytes[3] === 0x47;

  // WebP: RIFF....WEBP
  const isWebP = magicBytes[0] === 0x52 && magicBytes[1] === 0x49 &&
                 magicBytes[2] === 0x46 && magicBytes[3] === 0x46;

  if (!isJPEG && !isPNG && !isWebP) {
    throw new Error('Invalid image format');
  }
}

// ❌ Bad - Trust MIME type
if (req.file.mimetype === 'image/jpeg') {
  // Attacker can spoof MIME type!
}

File Size Limits

// Multer configuration
const upload = multer({
  storage: multer.memoryStorage(),
  limits: {
    fileSize: 10 * 1024 * 1024,  // 10MB max
    files: 5                      // 5 files max
  }
});

EXIF Data Stripping

Remove metadata for privacy:

// Using Sharp
const processed = await sharp(buffer)
  .rotate()              // Auto-rotate based on EXIF
  .removeExif()          // Then strip all EXIF data
  .toBuffer();

// EXIF can contain: GPS coordinates, camera info, timestamps, author

Content Type Validation

const ALLOWED_MIME_TYPES = ['image/jpeg', 'image/png', 'image/webp'];

if (!ALLOWED_MIME_TYPES.includes(contentType)) {
  throw new Error('Invalid content type');
}

XSS Prevention

Automatic Escaping (Pug)

Pug automatically escapes output:

//- ✅ Safe - Escaped automatically
p= member.display_name

//- ✅ Safe - Escaped in interpolation
p Welcome, #{member.display_name}!

//- ❌ Dangerous - Unescaped (use only for trusted HTML)
p!= userInput

When you MUST use unescaped:

// Sanitize HTML first
import DOMPurify from 'dompurify';

const sanitized = DOMPurify.sanitize(userInput);
res.render('page', { content: sanitized });
//- Only then use unescaped
div!= content

JSON in Attributes

//- ✅ Safe - Pug escapes
div(data-config=JSON.stringify(config))

//- The attribute will be properly escaped

CSRF Protection

SameSite Cookies

Current implementation uses SameSite=lax:

res.cookie('session_id', sessionId, {
  sameSite: 'lax',  // Protects against CSRF while allowing normal navigation
});

SameSite options:

  • strict - Never sent in cross-site requests (breaks OAuth)
  • lax - Sent in top-level navigation (GET only) ✅ Current choice
  • none - Always sent (requires secure: true)

HTMX and CSRF

HTMX sends requests from same origin (safe):

//- HTMX POST from same domain - protected by SameSite
button(
  hx-post="/admin/submissions/123/approve"
  hx-target="#result"
) Approve

For future API endpoints:

  • Consider CSRF tokens for state-changing operations
  • Or verify Origin header matches expected domain

Secrets Management

NEVER Commit Secrets

Git-ignored files:

# .gitignore
src/config.json
src/config.production.json
.env
*.key
*.pem

Check before committing:

# Search for potential secrets
git diff --cached | grep -i "password\|secret\|key\|token"

# If found, don't commit!
git reset src/config.json

Configuration Files

Development:

// src/config.json (git-ignored)
{
  "databaseFile": "./database/database.db",
  "googleClientSecret": "actual-secret-here",
  "smtpPassword": "actual-password-here",
  "s3Secret": "actual-r2-secret-here"
}

Production:

# Store in /mnt/basny-data/app/config/config.production.json
# Permissions: 600 (owner-only read/write)
# Owner: 1001:65533 (nodejs user in container)

ssh BAP "ls -la /mnt/basny-data/app/config/config.production.json"
# Should show: -rw------- 1001:65533

Environment Variables (Alternative)

// Prefer config file, but env vars work too
const secret = process.env.SECRET || config.secret;

// Never hardcode
const secret = "actual-secret-here"; // ❌ NEVER DO THIS

Error Handling

Don't Leak Information in Errors

✅ Good - Generic error messages to user:

try {
  await processPayment(userId, amount);
} catch (error) {
  logger.error('Payment processing failed', { userId, error });

  // Generic message to user
  res.status(500).json({
    error: 'Payment failed. Please try again.'
  });
}

❌ Bad - Exposing internals:

catch (error) {
  // Exposes database structure, file paths, etc.
  res.status(500).json({
    error: error.message,
    stack: error.stack,
    query: failedQuery
  });
}

Log Sensitive Operations

import { logger } from '@/utils/logger';

// Log authentication attempts
logger.info('Login attempt', {
  email: email,
  success: true,
  ip: req.ip
});

// Log admin actions
logger.info('Submission approved', {
  submissionId: id,
  adminId: viewer.id,
  points: points
});

// Log security events
logger.warn('Password reset requested for non-existent email', {
  email: attemptedEmail,
  ip: req.ip
});

Security Checklist

For Every Pull Request

  • All user input validated with Zod schemas
  • All SQL queries use prepared statements (no string concatenation)
  • No secrets committed to repository
  • Error messages don't expose system internals
  • File uploads validate magic bytes (if applicable)
  • Sensitive operations are logged
  • Tests include security edge cases
  • No eval() or Function() constructor usage
  • No innerHTML or unescaped HTML (use Pug escaping)
  • Dependencies updated (no known vulnerabilities)

For Database Changes

  • Foreign key constraints defined
  • Appropriate ON DELETE behavior set
  • Indexes added for performance
  • No sensitive data in plain text
  • Migration tested with existing data

For Authentication Changes

  • Passwords hashed with scrypt
  • Session cookies have httpOnly, secure, sameSite
  • Password reset tokens expire (1 hour)
  • Auth codes are single-use
  • No email enumeration vulnerabilities

For File Uploads

  • Magic bytes validated
  • File size limits enforced
  • MIME type whitelist checked
  • EXIF data stripped
  • Files stored outside web root
  • Unique filenames (prevent overwrite)

Common Vulnerabilities to Avoid

SQL Injection

❌ Vulnerable:

const name = req.body.name;
await db.run(`INSERT INTO members (display_name) VALUES ('${name}')`);
// Attacker input: "'); DROP TABLE members; --"

✅ Secure:

await insertOne('members', { display_name: name });

XSS (Cross-Site Scripting)

❌ Vulnerable:

//- Unescaped user input
div!= userBio
//- Attacker bio: "<script>alert('XSS')</script>"

✅ Secure:

//- Escaped automatically
div= userBio
//- Output: &lt;script&gt;alert('XSS')&lt;/script&gt;

Path Traversal

❌ Vulnerable:

const filename = req.params.filename;
res.sendFile(`/uploads/${filename}`);
// Attacker: filename = "../../etc/passwd"

✅ Secure:

const filename = path.basename(req.params.filename);  // Strip path
const safePath = path.join(UPLOAD_DIR, filename);

// Verify path is within allowed directory
if (!safePath.startsWith(UPLOAD_DIR)) {
  return res.status(403).send('Forbidden');
}

res.sendFile(safePath);

Mass Assignment

❌ Vulnerable:

// User can set is_admin=1 by adding field to form
await updateOne('members', { id: memberId }, req.body);

✅ Secure:

// Only update allowed fields
const { display_name, contact_email } = req.body;
await updateOne('members', { id: memberId }, {
  display_name,
  contact_email
});

Production Security

nginx Security Headers

Verify headers are set:

# nginx/conf.d/default.conf
add_header Strict-Transport-Security "max-age=31536000; includeSubDomains; preload";
add_header X-Frame-Options "SAMEORIGIN";
add_header X-Content-Type-Options "nosniff";
add_header X-XSS-Protection "1; mode=block";
add_header Referrer-Policy "strict-origin-when-cross-origin";

Rate Limiting

Already configured in nginx:

limit_req_zone $binary_remote_addr zone=general:10m rate=10r/s;
limit_req_zone $binary_remote_addr zone=api:10m rate=30r/s;
limit_req_zone $binary_remote_addr zone=upload:10m rate=5r/s;

Don't bypass rate limits in code!

File Permissions

Production config must be restrictive:

# Config file: 600 (owner-only)
-rw------- 1001:65533 config.production.json

# Database: 644 (owner write, group/others read)
-rw-r--r-- 1001:65533 database.db

Dependency Security

Regular Updates

# Check for vulnerabilities
npm audit

# Fix automatically (if safe)
npm audit fix

# Review and update manually
npm outdated
npm update

Before Merging

# Ensure no known vulnerabilities
npm audit --production
# Should return: found 0 vulnerabilities

Reporting Security Issues

Found a vulnerability?

DO:

  1. Email directly (not public issue)
  2. Include detailed description
  3. Provide reproduction steps
  4. Suggest fix if possible
  5. Allow time for patch before disclosure

DON'T:

  1. Open public GitHub issue
  2. Disclose publicly before fix
  3. Exploit the vulnerability

We will:

  • Respond within 48 hours
  • Provide fix timeline
  • Credit you in security advisory
  • Keep you updated on progress

Related Documentation


Security is everyone's responsibility. When in doubt, ask!


Last Updated: November 2025

⚠️ **GitHub.com Fallback** ⚠️