Buổi 14: Security & Quality — OWASP, Secrets, Code Review 🔒

Thành quả: Security audit report + code review PR workflow hoàn chỉnh

---

🎯 Mục Tiêu

1. Hiểu OWASP Top 10 cho web developers
2. Setup secret scanning (cm-secret-shield)
3. Code review workflow chuyên nghiệp (cm-code-review)
4. Security audit toolchain (cm-security-gate)
5. Viết secure code patterns

---

📖 Phần 1: OWASP Top 10 (2024)

#	Vulnerability	Description	Fix
1	Broken Access Control	User A can access User B's data	Middleware auth + ownership check
2	Cryptographic Failures	Weak hashing, plain text secrets	bcrypt + .env + secret manager
3	Injection	SQL/NoSQL/Command injection	Parameterized queries (Prisma)
4	Insecure Design	Missing security in architecture	Threat modeling in cm-planning
5	Security Misconfiguration	Default passwords, verbose errors	Config audit, production mode
6	Vulnerable Components	Outdated npm packages	npm audit, Snyk, Dependabot
7	Auth Failures	Weak passwords, session issues	JWT rotation, rate limiting
8	Data Integrity	CSRF, unsafe deserialization	CSRF tokens, input validation
9	Logging Failures	No audit trail	Structured logging, monitoring
10	SSRF	Server-side request forgery	URL allowlisting, firewall

Secure Code Patterns

// ❌ Broken Access Control
app.get('/api/users/:id', async (req, res) => {
  const user = await prisma.user.findUnique({ where: { id: req.params.id } });
  res.json(user); // ANY user can see ANY other user's data!
});

// ✅ Fixed: Ownership check
app.get('/api/users/:id', authenticate, async (req, res) => {
  if (req.user.id !== req.params.id && req.user.role !== 'ADMIN') {
    throw new AppError('Forbidden', 403);
  }
  const user = await prisma.user.findUnique({
    where: { id: req.params.id },
    select: { id: true, name: true, email: true }, // Never return password
  });
  res.json(user);
});

// ❌ SQL Injection (raw query)
const users = await prisma.$queryRaw`
  SELECT * FROM users WHERE name = '${req.query.name}'
`;

// ✅ Parameterized
const users = await prisma.$queryRaw`
  SELECT * FROM users WHERE name = ${req.query.name}
`;
// Prisma auto-parameterizes the template literal

// ❌ Verbose error in production
app.use((err, req, res, next) => {
  res.status(500).json({
    error: err.message,
    stack: err.stack,     // 💀 Exposes internal code paths!
    query: err.query,     // 💀 Exposes DB queries!
  });
});

// ✅ Clean error response
app.use((err, req, res, next) => {
  const isDev = process.env.NODE_ENV === 'development';
  
  if (err instanceof AppError) {
    return res.status(err.statusCode).json({
      success: false,
      error: { code: err.code, message: err.message },
    });
  }

  // Log full error internally
  logger.error({ err, req: { method: req.method, url: req.url } });

  // Return generic message to client
  res.status(500).json({
    success: false,
    error: {
      code: 'INTERNAL_ERROR',
      message: 'An unexpected error occurred',
      ...(isDev && { debug: err.message }), // Only in development
    },
  });
});

Rate Limiting

import rateLimit from 'express-rate-limit';

// Global rate limit
app.use(rateLimit({
  windowMs: 15 * 60 * 1000, // 15 minutes
  max: 100,                  // 100 requests per window
  message: { error: { code: 'RATE_LIMITED', message: 'Too many requests' } },
}));

// Strict rate limit for auth endpoints
app.use('/api/v1/auth', rateLimit({
  windowMs: 15 * 60 * 1000,
  max: 5,                    // Only 5 login attempts per 15 min
  message: { error: { code: 'AUTH_RATE_LIMITED', message: 'Too many login attempts' } },
}));

---

📖 Phần 2: cm-secret-shield

Pre-commit Hook Setup

# Install Gitleaks
brew install gitleaks

# Create pre-commit hook
cat > .git/hooks/pre-commit << 'EOF'
#!/bin/bash
gitleaks protect --staged --verbose
if [ $? -ne 0 ]; then
  echo "❌ Secret detected! Fix before committing."
  exit 1
fi
echo "✅ No secrets detected"
EOF
chmod +x .git/hooks/pre-commit

.gitleaks.toml

[allowlist]
description = "Allowlist for false positives"
paths = [
  '''\.env\.example$''',
  '''package-lock\.json$''',
]

[[rules]]
id = "generic-api-key"
description = "Generic API Key"
regex = '''(?i)(api[_-]?key|apikey)\s*[:=]\s*['\"]?([0-9a-zA-Z]{20,})'''

Secret Management Checklist

✅ .env in .gitignore
✅ .env.example with dummy values committed
✅ Secrets in GitHub Secrets (for CI/CD)
✅ Pre-commit hook scans for secrets
✅ No hardcoded credentials ANYWHERE
✅ JWT secrets are random 64+ char strings
✅ API keys rotated quarterly

---

📖 Phần 3: Code Review Workflow

PR Template

<!-- .github/pull_request_template.md -->
## Description
<!-- What does this PR do? -->

## Type of Change
- [ ] 🐛 Bug fix
- [ ] ✨ New feature
- [ ] ♻️ Refactoring
- [ ] 📝 Documentation
- [ ] 🔧 Chore

## Testing
- [ ] Unit tests added/updated
- [ ] Integration tests pass
- [ ] Manual testing done

## Security
- [ ] No new secrets/credentials
- [ ] Input validation added
- [ ] Auth checks verified

## Screenshots
<!-- For UI changes -->

Code Review Checklist (Reviewer)

## Review Checklist

### Correctness
- [ ] Logic is correct
- [ ] Edge cases handled
- [ ] Error handling complete

### Security
- [ ] No injection vulnerabilities
- [ ] Auth/authz properly checked
- [ ] No secret exposure
- [ ] Input validated

### Performance
- [ ] No N+1 queries
- [ ] Proper indexing
- [ ] No memory leaks

### Maintainability
- [ ] Clear naming
- [ ] No duplication
- [ ] Tests cover new code
- [ ] Documentation updated

### Style
- [ ] Consistent formatting
- [ ] TypeScript strict
- [ ] No TODO/FIXME without ticket

---

📖 Phần 4: Security Audit Tools

# 1. npm audit (built-in)
npm audit
npm audit fix

# 2. Snyk (advanced scanning)
npx snyk test
npx snyk monitor

# 3. Dependency-check
npx better-npm-audit audit

# 4. SAST (Static Application Security Testing)
npx eslint-plugin-security .

Security Report Template

# Security Audit Report
Date: [date]
Auditor: [name] + cm-security-gate

## Summary
| Severity | Count |
|----------|-------|
| 🔴 Critical | 0 |
| 🟠 High | 2 |
| 🟡 Medium | 5 |
| 🟢 Low | 8 |

## Findings

### Finding 1: Missing Rate Limiting on Auth Endpoints
**Severity:** 🟠 High
**Location:** src/routes/auth.routes.ts
**Description:** Login endpoint has no rate limiting
**Fix:** Add express-rate-limit with max 5 attempts per 15 min
**Status:** Fixed ✅

### Finding 2: Verbose Error Messages in Production
**Severity:** 🟡 Medium
**Location:** src/middleware/error.middleware.ts
**Description:** Stack traces returned in error responses
**Fix:** Only show stack in development mode
**Status:** Fixed ✅

---

🧪 Lab: Security Audit Sprint

Task: Full audit + fix + review (60 min)

Step 1: Security scan (15 min)
→ npm audit
→ npx gitleaks detect
→ Review OWASP checklist on your code

Step 2: Fix findings (20 min)
→ Add rate limiting
→ Fix error messages
→ Verify auth on all protected routes
→ Update dependencies

Step 3: Code Review Practice (15 min)
→ Create PR with security fixes
→ Fill PR template
→ Self-review with checklist

Step 4: Document (10 min)
→ Write security-audit-report.md
→ Evidence for each finding and fix

---

🎓 Tóm Tắt

Area	Tool / Practice
OWASP	Top 10 checklist on every feature
Secrets	Gitleaks pre-commit + .env in .gitignore
Code Review	PR template + reviewer checklist
Audit	npm audit + Snyk + SAST
Rate Limiting	express-rate-limit on auth endpoints

⏭️ Buổi tiếp theo

Buổi 15: Automation & Custom Skills — Build Your Workflow ⚡