🔬 Deep Dive: /dev/null Parsing, bool("false") Trap & CWE-22 Containment

[quantamixsol]

Three Subtle Bugs — One PR

1. /dev/null Diff Parsing

Git unified diffs use --- /dev/null for new files. Our apply_diff() returned success=False for missing files — but never checked if the diff was a new-file creation. The MCP response looked identical to success (valid patches, dry_run=false). Silent data loss.

Fix: Detect /dev/null in diff header lines only (not body), create file atomically via tempfile.NamedTemporaryFile + os.replace().

---

2. bool("false") == True — The MCP Trap

Python's bool() treats any non-empty string as True. MCP frameworks pass boolean params as strings. So dry_run="false" silently became dry_run=True — files were never written.

# BEFORE (broken)
dry_run = bool(args.get("dry_run", True))
# bool("false") → True ❌

# AFTER (safe)
def _coerce_bool(value, default):
    if isinstance(value, str):
        return value.lower() not in ("false", "0", "no", "off", "n")
    return bool(value) if value is not None else default

Rule: Never use bool(string) in MCP argument handling.

---

3. CWE-22 Path Traversal

New file creation from /dev/null diffs could create files outside the project root via ../../ paths. Fixed with Path.resolve() + relative_to() containment check.

---

All three patterns validated across 188 tests ✅

👉 Full diff: PR #72