Reviewing Fullstack Code Changes

Bash
# 1. See what changed
git status
git diff

# 2. Syntax-check every modified PHP file
git diff --name-only --diff-filter=ACM -- '*.php' | xargs -I{} php -l {}

# 3. Review diff for logic + security issues (see Workflow below)
git diff > /tmp/review.diff

Then analyze /tmp/review.diff (or the diff output directly) following the four-pass workflow.

Always run the passes in this order. Each pass has a distinct focus — don't mix them or issues get missed.

Progress:
- [ ] Pass 0: Identify scope (git status/diff)
- [ ] Pass 1: Syntax / programming errors
- [ ] Pass 2: Logic errors
- [ ] Pass 3: OWASP security review
- [ ] Pass 4: Summarize findings with severity + fix suggestions

Pass 0 — Identify Scope

• Run git status to list new/modified/staged files.
• Run git diff (unstaged) and git diff --staged (staged) separately — review both.
• Ignore vendor/, node_modules/, generated assets.
• Build a list of changed files grouped by type: PHP, JS, SQL, config (.htaccess, apache confs), templates.

Pass 1 — Programming / Syntax Errors

For each changed PHP file:

• Run php -l <file> (lint). Flag any parse errors immediately — this is a hard blocker.
• Manually scan for:
  • Undefined variables/array keys (especially $_GET/$_POST/$_SESSION used without isset/null checks).
  • Type mismatches (PHP 8.5 is strict about deprecated implicit coercions — check for deprecation notices).
  • Unclosed resources (DB connections, file handles).
  • Missing declare(strict_types=1) where the rest of the codebase uses it.
  • Function/method signature mismatches vs. call sites.
  • Dead code / unreachable branches introduced by the diff.
• For JS changes, check browser console errors mentally: undefined refs, missing await, unhandled promise rejections.

Pass 2 — Logic Errors

• Trace each modified function's data flow: input → transformation → output/DB write.
• For functions touching the database, mentally run or actually run test queries against MariaDB with edge-case inputs:
  • empty string, NULL, 0, negative numbers, very long strings, unicode/emoji, duplicate keys.
• Check boundary conditions: off-by-one in loops/pagination, < vs <=, inclusive/exclusive date ranges.
• Check conditional logic: inverted conditions, missing else, short-circuit evaluation side effects.
• Verify state consistency across request lifecycle (sessions, transactions — is a DB transaction committed/rolled back on all paths?).
• Cross-check business rules against requirements if known (e.g., "price can't go negative", "user can't book past dates").
• If a function changed, check every caller of that function for now-broken assumptions.

Pass 3 — OWASP Security Review

Walk the OWASP Top 10 against the diff explicitly:

Actively try to break it:

• Append ' OR '1'='1 to any input feeding a query — confirm it's neutralized by prepared statements.
• Inject <script>alert(1)</script> into any input that gets rendered — confirm it's escaped.
• Try path traversal (../../etc/passwd) on any file-path input.

Pass 4 — Summarize

Output findings grouped by severity (Critical / High / Medium / Low), each with:

• File + line number
• Category (Syntax / Logic / Security)
• Concrete fix (code snippet), not just a description of the problem.

Example 1: Input:

PHP
$id = $_GET['id'];
$result = $db->query("SELECT * FROM orders WHERE user_id = $id");

Output:

[CRITICAL][Security - SQL Injection] file: orders.php:12
`$id` from $_GET is concatenated directly into SQL.
Fix:
$stmt = $db->prepare("SELECT * FROM orders WHERE user_id = :id");
$stmt->execute(['id' => $id]);
$result = $stmt->fetchAll();
Also validate $id is an expected type (e.g. ctype_digit($id)) before use.

Example 2: Input:

PHP
function getDiscount($total) {
    if ($total > 100) {
        return $total * 0.1;
    }
}

Output:

[MEDIUM][Logic] file: cart.php:45
Function returns NULL (not 0) when $total <= 100, which will cause
"Unsupported operand types" if the caller does arithmetic with the result.
Fix: add `return 0;` as the default/else branch.

[LOW][Programming] Missing type hints — add `function getDiscount(float $total): float`
given the project uses strict_types elsewhere.