Code Review Checklist

# ===== 1) Frame the review =====
# Ask: 'What user-controlled data flows through this PR, and where does it end up?'
# If nothing, skip. If something, follow it.

# ===== 2) Identify the SINKS first =====
# Sinks = places where data lands as code/markup.

# JavaScript / browser:
git diff main..HEAD | grep -nE '\.(innerHTML|outerHTML|insertAdjacentHTML)|document\.write|dangerouslySetInnerHTML|v-html|\@html|bypassSecurityTrust'

# Server-side templates:
git diff main..HEAD | grep -nE '\{!!|raw \||\| raw|Markup\(|html_safe|format_html'

# Direct DOM URL writes:
git diff main..HEAD | grep -nE 'window\.location|location\.href|src\s*=\s*'

# Server-side echos:
git diff main..HEAD | grep -nE 'echo $_(GET|POST|REQUEST)'

# For each hit, jot a note on the PR. Even if the value looks 'internal',
# trace it upstream.

# ===== 3) Trace each SINK upstream =====
# For every sink, find: 'where did this value come from?'
# - Direct user input (req body, query, headers)         -> high risk
# - Database (could contain stored user input)            -> high risk
# - Internal config / build-time literal                  -> low risk
# - Another framework template / safe primitive           -> verify

# ===== 4) Verify the encoder matches the CONTEXT =====
# HTML body text:           htmlspecialchars / {{ }} / textContent
# HTML attribute (quoted):  htmlspecialchars / value={...}
# JS string literal:        json_encode / JSON.stringify
# URL parameter:            urlencode / encodeURIComponent
# CSS:                      sparingly; whitelist values
# JSON dropped into HTML:   json_encode with the HEX flags

# Wrong encoder for the wrong context is the most common bug, more common
# than 'no encoder at all'.

# ===== 5) Check the SAFETY NET =====
# CSP header set (script-src self + nonce; object-src none)
# Trusted Types enforced (on Chromium fleets)
# Cookies: HttpOnly + Secure + SameSite=Lax for sessions
# X-Content-Type-Options: nosniff
# Referrer-Policy: strict-origin-when-cross-origin
# X-Frame-Options or frame-ancestors

# Missing CSP is not a 'block this PR' — but flag for follow-up.

# ===== 6) Special cases =====

# a) Sanitised rich-text input (comments, notes)
# - Use DOMPurify / HTMLPurifier; never a hand-rolled regex.
# - Disable scripts + foreign content + javascript: hrefs in the sanitiser config.
# - Render with {!! sanitised !!} or dangerouslySetInnerHTML — leave a comment.

# b) Markdown -> HTML rendering
# - The Markdown library should escape by default.
# - Re-run output through a sanitiser if the library allows raw HTML.

# c) Server-rendered JSON for the client
# - Use json_encode with JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT
#   when dropping into <script>; better: ship as data-* attribute and parse in JS.

# d) URLs from user input
# - Validate scheme: http(s) and mailto only; reject javascript: and data:
# - Use new URL(s) to parse; reject on throw.

# ===== 7) Suggest fixes that the author will accept =====
# - Be specific: 'on line 23, swap innerHTML for textContent here'
# - Provide a one-line patch when possible
# - Explain the CONTEXT, not 'XSS bad'
# - Link to your team's XSS cheat sheet
# - Avoid blocking the PR if the bug is unrelated and minor — file an issue

# ===== 8) Suggest a TEST =====
# Authors absorb the lesson faster when they write a test:
# - 'send <script>alert(1)</script> as the field value; assert the rendered page does not contain a literal <script>'
# - Snapshot the rendered output through the encoder

# ===== 9) Verify the fix re-tests negatively =====
# After the fix, the same payload should land as escaped text:
#   '&lt;script&gt;alert(1)&lt;/script&gt;'
# Verify this in the staged build, not just the unit test.

# ===== 10) Document the review =====
# Leave a single summary comment:
# - 'Reviewed for XSS: looked at N sinks, traced K upstream, found J issues'
# - 'Pending follow-up: CSP not enforced on /admin (issue #234)'
# A short note on every PR builds a culture where the reviewer's standard is visible.

# ===== Common false positives =====
# - Static template strings that look user-controlled but are not
# - Internal admin tools where the threat model is different (still encode!)
# - innerHTML used with literal HTML constants from the codebase
# - Vue v-html in a tightly controlled component
# In all cases: leave a comment with the justification so future reviewers know.

# ===== Common misses =====
# - innerHTML in libraries you depend on (audit deps periodically)
# - URL params reflected into <a href=...> without validation
# - Markdown rendering with unsafe HTML allowed
# - Server-side template fragments cached and reused across users
# - Old browsers shipped to (Trusted Types is Chrome-only)

// Look for:
// - innerHTML / outerHTML / insertAdjacentHTML / document.write
// - dangerouslySetInnerHTML, v-html, [innerHTML]
// - eval, new Function, setTimeout(string, …)
// - href/src built from user input
// - 3rd-party widgets that take HTML