Home / Skills / Developer Productivity / Security and Hardening
Developer Productivity

Security and Hardening

Addy OsmaniMITSkill language: English
License verifiedNo scriptsLow risk
DemoReal run · claude-sonnet-5-5 · October 2026
Please review this Express handler for security problems and show me the fixed version. It is a login plus avatar-upload endpoint in a small web app (sample code, not a real system). ```js const express = require('express'); const cors = require('cors'); const app = express(); app.use(cors({ origin: '*', credentials: true })); app.use(express.json()); app.post('/login', async (req, res) => { const { email, password } = req.body; const user = await db.query(`SELECT * FROM users WHERE email = '${email}'`); if (user.rows[0] && user.rows[0].password === password) { const token = jwt.sign({ id: user.rows[0].id }, 'secret123'); res.json({ token, user: user.rows[0] }); } else { res.status(401).json({ error: 'bad login', detail: `no user ${email}` }); } }); app.post('/avatar', upload.single('file'), (req, res) => { fs.writeFileSync('/var/www/uploads/' + req.file.originalname, req.file.buffer); res.send('ok'); }); app.get('/profile/:id', async (req, res) => { res.json((await db.query('SELECT * FROM users WHERE id = ' + req.params.id)).rows[0]); }); // the front end stores the token in localStorage ```
Skill: security-and-hardening

Review: security problems found

# Severity Problem
1 Critical SQL injection in /login and /profile/:id, from string-concatenated queries.
2 Critical Path traversal and arbitrary file overwrite in /avatar. originalname is attacker-controlled, so ../../x escapes the folder.
3 Critical Uploads are unauthenticated and unrestricted. There is no size, type, or content check. The files go into /var/www, so an uploaded .html or .svg gives stored XSS, and a script file may get executed.
4 High Plaintext password comparison. Passwords are stored unhashed and compared with ===, which is also not timing-safe.
5 High Hardcoded JWT secret ('secret123'). There is no expiry and the algorithm isn't pinned, so anyone can forge tokens.
6 High Full DB row returned (SELECT *) from /login and /profile. This leaks the password and any other sensitive columns.
7 High /profile/:id has no authentication or authorization (IDOR). Anyone can read any user.
8 High Token in localStorage. Any XSS can steal it. Use an httpOnly cookie instead.
9 Medium cors({ origin: '*', credentials: true }) is a misconfiguration. Use an explicit allowlist.
10 Medium The 401 reply echoes the email (detail). That is reflected input and it enables user enumeration.
11 Medium No rate limiting on login, so brute force and credential stuffing are open.
12 Medium No input validation. email could be an object or array, not just a string.
13 Low No security headers (helmet).
14 Low Async errors are unhandled in Express 4, which gives hangs or stack traces. writeFileSync also blocks the event loop.

Fixed version

const express = require('express');
const cors = require('cors');
const helmet = require('helmet');
const cookieParser = require('cookie-parser');
const rateLimit = require('express-rate-limit');
const bcrypt = require('bcrypt');
const jwt = require('jsonwebtoken');
const multer = require('multer');
const crypto = require('crypto');
const path = require('path');
const fs = require('fs/promises');
const { z } = require('zod');

const JWT_SECRET = process.env.JWT_SECRET;
if (!JWT_SECRET || JWT_SECRET.length < 32) throw new Error('JWT_SECRET missing or too short');

const ALLOWED_ORIGINS = (process.env.CORS_ORIGINS || '').split(',').filter(Boolean);
const UPLOAD_DIR = path.resolve(process.env.UPLOAD_DIR || '/var/app-data/avatars'); // outside the web root
const DUMMY_HASH = bcrypt.hashSync('dummy-password', 12); // equalizes timing for unknown emails

const app = express();
app.use(helmet());
app.use(cors({ origin: ALLOWED_ORIGINS, credentials: true }));
app.use(express.json({ limit: '10kb' }));
app.use(cookieParser());

const wrap = fn => (req, res, next) => fn(req, res, next).catch(next);

// ---- auth middleware ----
function requireAuth(req, res, next) {
  try {
    const payload = jwt.verify(req.cookies.session, JWT_SECRET, { algorithms: ['HS256'] });
    req.userId = payload.id;
    next();
  } catch {
    res.status(401).json({ error: 'unauthorized' });
  }
}

// ---- login ----
const loginLimiter = rateLimit({ windowMs: 15 * 60 * 1000, max: 10 }); // use a shared store (e.g. Redis) if you run >1 instance
const loginSchema = z.object({ email: z.string().email().max(254), password: z.string().min(1).max(200) });

app.post('/login', loginLimiter, wrap(async (req, res) => {
  const parsed = loginSchema.safeParse(req.body);
  if (!parsed.success) return res.status(422).json({ error: 'invalid input' });
  const { email, password } = parsed.data;

  const { rows } = await db.query(
    'SELECT id, email, display_name, password_hash FROM users WHERE email = $1', [email]);
  const user = rows[0];
  const ok = await bcrypt.compare(password, user ? user.password_hash : DUMMY_HASH);
  if (!user || !ok) return res.status(401).json({ error: 'invalid email or password' });

  const token = jwt.sign({ id: user.id }, JWT_SECRET, { algorithm: 'HS256', expiresIn: '1h' });
  res.cookie('session', token, {
    httpOnly: true, secure: true, sameSite: 'strict', maxAge: 60 * 60 * 1000,
  });
  res.json({ user: { id: user.id, email: user.email, displayName: user.display_name } });
}));

// ---- avatar upload ----
const upload = multer({
  storage: multer.memoryStorage(),
  limits: { fileSize: 2 * 1024 * 1024, files: 1 },
});

// verify real content via magic bytes; the extension and client MIME type prove nothing
function sniffImage(buf) {
  if (buf.length > 8 && buf.subarray(0, 8).equals(Buffer.from([0x89,0x50,0x4e,0x47,0x0d,0x0a,0x1a,0x0a]))) return 'png';
  if (buf.length > 3 && buf[0] === 0xff && buf[1] === 0xd8 && buf[2] === 0xff) return 'jpg';
  return null; // SVG and HTML deliberately not allowed
}

app.post('/avatar', requireAuth, upload.single('file'), wrap(async (req, res) => {
  if (!req.file) return res.status(422).json({ error: 'file required' });
  const ext = sniffImage(req.file.buffer);
  if (!ext) return res.status(415).json({ error: 'only PNG or JPEG allowed' });

  // server-chosen name: the client filename is never used
  const name = `${req.userId}-${crypto.randomUUID()}.${ext}`;
  const dest = path.join(UPLOAD_DIR, name);
  if (path.dirname(dest) !== UPLOAD_DIR) throw new Error('bad path'); // defense in depth
  await fs.writeFile(dest, req.file.buffer, { flag: 'wx', mode: 0o640 });

  await db.query('UPDATE users SET avatar_file = $1 WHERE id = $2', [name, req.userId]);
  res.json({ ok: true });
}));
// Serve avatars via a static handler or CDN with `X-Content-Type-Options: nosniff`
// and a fixed image Content-Type.

// ---- profile ----
app.get('/profile/:id', requireAuth, wrap(async (req, res) => {
  const id = Number.parseInt(req.params.id, 10);
  if (!Number.isSafeInteger(id)) return res.status(422).json({ error: 'invalid id' });
  if (id !== req.userId) return res.status(403).json({ error: 'forbidden' }); // add an admin check if needed

  const { rows } = await db.query(
    'SELECT id, email, display_name, avatar_file FROM users WHERE id = $1', [id]);
  if (!rows[0]) return res.status(404).json({ error: 'not found' });
  res.json(rows[0]);
}));

// ---- error handler: details go to logs, not to the client ----
app.use((err, req, res, next) => {
  console.error(err);
  res.status(500).json({ error: 'internal error' });
});

Notes on the changes

  • Passwords: your table needs a password_hash column. Migrate existing users by hashing their passwords with bcrypt (cost 12 or higher) or forcing a reset. Never keep the plaintext column.
  • Token storage: moving from localStorage to an httpOnly cookie means the front end must call fetch(..., { credentials: 'include' }) and stop touching the token. sameSite: 'strict' and the CORS allowlist cover CSRF for this setup. If the front end runs on a different site (not just a different subdomain), add CSRF tokens.
  • Secrets: JWT_SECRET comes from the environment. Treat secret123 as compromised and rotate it, since any tokens signed with it can be forged.
  • Rate limiting: the in-memory limiter works for a single process. With multiple instances, back it with Redis or similar, or the effective limit is 10 × instances.
  • Not covered: db, upload, jwt and fs were undefined in your sample, so I assumed pg-style $1 placeholders. I also didn't add account lockout, password policy, or logging of failed logins. Those are worth adding for a real system.

Test run: a deliberately vulnerable sample Express handler (login, upload, profile). The model listed 14 problems by severity and gave a fixed version. The fixed code was not run, and the model noted it assumed a pg-style database client. It did not open the bundled reference files in this run.

What it does

Treats every external input as hostile and every secret as sacred. It starts with a short threat model (map the trust boundaries, name the assets, run STRIDE, write abuse cases next to use cases), then applies a three-tier rule set: always do (validate input, parameterize queries, encode output, hash passwords, secure cookies, run the package manager's audit), ask first (new auth flows, new sensitive data, CORS or upload changes) and never do (commit secrets, log sensitive data, trust client-side validation). It covers injection, XSS, access control (IDOR), sessions, security headers, CORS, file uploads, SSRF, destructive operations on derived paths, rate limiting, secrets, dependency and supply-chain checks, personal data and privacy, and AI features, where model output counts as untrusted input.

Output

A prioritized list of problems with severity, a fixed version of the code, and notes on what to migrate or check next.

Good for

Anyone writing or reviewing code that accepts input, handles accounts, stores data or calls third-party services. The package includes the pattern reference and the shared security checklist the Skill points to.

Notes & risks

Low risk: pure instructions with no scripts. It tells the agent to edit your code and to run your package manager's own audit, but never to apply forced fixes automatically. Test-run once on a deliberately vulnerable sample handler: it found 14 problems and gave a fixed version that was not executed; the model did not open the reference files in that run. It is guidance, not a security audit or a guarantee. The shared checklist was bundled and its paths were fixed so the references resolve. By Addy Osmani, MIT.

SKILL.md original4 files in this package