inou/docs/CODE-REVIEW-2026-02-28.md

inou Code Review — February 28, 2026

Executive Summary

• Files reviewed: 42
• Lines reviewed: ~12,500
• Issues found: 14 (2 critical, 4 high, 5 medium, 3 low)
• Fixes applied: 3

This code review examined the inou health platform, a production medical imaging and health data platform built in Go. The review focused on security, correctness, performance, and code quality across the API layer, library code, portal, and import tools.

---

Critical Findings

CR-001: Authentication Backdoor Code

File: lib/dbcore.go:634 Issue: A hardcoded backdoor code 250365 bypasses authentication for any user.

if code != 250365 && storedCode != fmt.Sprintf("%06d", code) {
    return "", fmt.Errorf("invalid code")
}

Impact: Any attacker who discovers this code can access ANY user account without knowing the real verification code. Complete auth bypass for the entire platform. Fix applied: Removed backdoor code. See commit.

CR-002: Deterministic Nonce in AES-GCM (Theoretical)

File: lib/crypto.go:32-46 Issue: The deriveNonce function creates a deterministic nonce from plaintext via AES encryption:

func deriveNonce(data []byte, nonceSize int) []byte {
    block, _ := aes.NewCipher(masterKey)
    nonce := make([]byte, nonceSize)
    for i := 0; i < len(data); i += 16 {
        // ... XOR encrypted chunks into nonce
    }
    return nonce
}

Impact: AES-GCM requires unique nonces. Identical plaintexts produce identical nonces, enabling known-plaintext attacks. However, in practice:

1. The same data encrypted twice yields identical ciphertext (dedup-safe by design)
2. Different data produces different nonces
3. The system is append-mostly with medical data (low collision risk)

Recommendation: Document this is intentional "convergent encryption" for deduplication. For truly sensitive data needing semantic security, consider random nonces or SIV mode. Status: Documented, not fixed (design decision).

---

High Findings

HI-001: CORS Wildcard on Medical Data APIs

File: portal/api_mobile.go:20, portal/mcp_http.go:179 Issue: CORS headers allow any origin to access the API:

w.Header().Set("Access-Control-Allow-Origin", "*")

Impact: Any malicious website can make authenticated requests if the user is logged in, potentially exfiltrating medical data. Recommendation: Restrict to specific origins (the production domain, mobile app origins) or use dynamic origin validation against an allowlist. Fix applied: Changed to origin allowlist check.

HI-002: LOINC Matching Not Working

File: lib/normalize.go:38-41 Issue: The Normalize function skips entries that already have SearchKey2 set:

if e.SearchKey2 != "" {
    continue // already normalized
}

However, the known issue is that 0 lab entries have data["loinc"] set, meaning buildLabRefData() (if it exists) returns empty. After tracing the code, the root cause is:

1. Lab entries imported from JSON/MyChart do NOT call Normalize() automatically
2. Normalize() is only called manually or by specific import paths
3. Once an entry has SearchKey2 set (even without LOINC), it's skipped forever

Root Cause Chain:

• import_json.go creates lab entries but doesn't call Normalize()
• Entries get SearchKey2 set from other sources (MyChart imports)
• Normalize() checks SearchKey2 != "" and skips
• LOINC codes never populated

Recommendation:

1. Split the skip condition: skip if BOTH SearchKey2 AND data["loinc"] are set
2. Or: Always re-normalize if LOINC is missing Status: Fix applied with improved skip logic.

HI-003: No Rate Limiting on Auth Endpoints

File: portal/main.go (handleSendCode, handleVerify) Issue: No rate limiting on verification code sends or verification attempts. An attacker can:

1. Flood a user with verification emails (harassment)
2. Brute-force the 6-digit code (10^6 attempts needed) Impact: Medium-High. 6-digit codes have 1M possibilities; at 100 req/s, brute-force takes ~3 hours. Recommendation: Add per-IP and per-email rate limiting. Consider time-based lockouts after failed attempts. Status: Not fixed (requires infrastructure changes).

HI-004: Missing Foreign Key Enforcement Check

File: lib/dbcore.go (initialization) Issue: SQLite foreign keys are OFF by default. Code assumes cascade deletes work. Impact: Deleting a study may orphan series/slices. Deleting a dossier may orphan entries. Verification needed: Check if PRAGMA foreign_keys = ON is executed at connection time. Status: Verified - not enforced. Need to add _fk=1 to connection string or execute pragma.

---

Medium Findings

ME-001: Email Logged in Verification Errors

File: lib/dbcore.go:639

return "", fmt.Errorf("unknown email")

Issue: While the email isn't logged here, the error may bubble up to logging middleware. Impact: Low. Standard error handling, but review log sanitization.

ME-002: Nil Pointer Potential in DossierFromEntry

File: lib/dbcore.go:485

func DossierFromEntry(e *Entry) *Dossier {
    d := &Dossier{DossierID: e.DossierID, ...}

Issue: No nil check on e. If called with nil, causes panic. Impact: Low. Internal function, callers should check.

ME-003: Large DICOM Files Fully Loaded in Memory

File: lib/dicom.go (importFromDir) Issue: Each DICOM file is read entirely into memory with os.ReadFile(path). For large studies (100+ MRI slices at 512KB each = 50MB), this is fine. But CT studies can be 500+ slices at 1MB+ each. Impact: Memory pressure during import. Single-threaded processing mitigates OOM risk. Recommendation: Consider streaming for files > 10MB.

ME-004: Unbounded Query in entryReadAccessible

File: lib/dbcore.go:140-158 Issue: When listing all accessible dossiers, no LIMIT is applied to the access table query. Impact: Low for current usage (few grants per user). Could be slow with many grants.

ME-005: Hardcoded Signal Recipient

File: lib/signal.go:9

var signalRecipients = []string{"+17272252475"}

Issue: Johan's phone number hardcoded. Impact: None for production (feature works). Should be configurable.

---

Low / Informational

LO-001: Inconsistent Error Handling

Files: Various Issue: Some functions return errors, some log and continue, some panic. Example: NewID() panics on crypto/rand failure, but DossierLogin returns errors. Recommendation: Establish consistent error handling patterns per layer.

LO-002: TODO in Normalize Skip Logic

File: lib/normalize.go:38 Issue: The skip logic should be more nuanced. Partially addressed in HI-002 fix.

LO-003: Unused Import Variables

File: lib/stubs.go Issue: Legacy stubs exist for migration. Document timeline for removal.

---

Positive Observations

1. Good Security Practices:

   • Uses crypto/rand for all random generation (no math/rand)
   • Parameterized SQL queries throughout (no SQL injection)
   • AES-256-GCM encryption for all sensitive data
   • RBAC enforced at the core EntryRead/EntryWrite level
   • Session tokens are encrypted, not plain UUIDs

2. Well-Structured Data Layer:

   • Single choke points for all data access (dbcore.go)
   • Clear RBAC enforcement before any query
   • Pack/Unpack consistently applied to all string data

3. Medical Data Handling:

   • FIPS 140-3 awareness (crypto/fips140 import)
   • Proper DICOM metadata extraction
   • Reference range lookup with age/sex specificity
   • Audit logging for all data access

4. Code Quality:

   • Clean separation of concerns (lib/, api/, portal/)
   • Consistent naming conventions
   • Reasonable function sizes (most under 100 lines)

---

Recommended Next Steps (prioritized)

1. [DONE] Remove auth backdoor (CR-001)
2. [DONE] Fix CORS to use origin allowlist (HI-001)
3. [DONE] Fix LOINC skip logic (HI-002)
4. [TODO] Add rate limiting to auth endpoints (HI-003)
5. [TODO] Enable SQLite foreign key enforcement (HI-004)
6. [TODO] Document deterministic nonce as intentional (CR-002)
7. [TODO] Consider streaming for large DICOM files (ME-003)
8. [TODO] Make Signal recipient configurable (ME-005)

---

Appendix: LOINC Matching Root Cause Analysis

Current Flow

1. Labs imported from JSON (MyChart, etc.) → import_json.go → creates Entry with Type, Value, Summary
2. Some imports set SearchKey2 to a normalized name from the source
3. Normalize() is called manually or via specific code paths
4. Normalize() checks if e.SearchKey2 != "" { continue } — skips entry
5. LOINC code never assigned

Why buildLabRefData Returns

After searching the codebase, buildLabRefData() doesn't exist. The intended flow is:

1. Normalize() extracts unique test names
2. Calls LLM to get LOINC codes, SI units, etc.
3. Saves to lab_test table via RefExec
4. Updates entries with data["loinc"], SearchKey, SearchKey2

The bug: entries with SearchKey2 already set are skipped before LLM normalization occurs.

Fix Applied

Changed skip condition from:

if e.SearchKey2 != "" {
    continue // already normalized
}

To:

// Skip if fully normalized (has BOTH SearchKey2 AND LOINC)
var data map[string]interface{}
json.Unmarshal([]byte(e.Data), &data)
hasLoinc := data["loinc"] != nil && data["loinc"] != ""
if e.SearchKey2 != "" && hasLoinc {
    continue // fully normalized
}

This allows re-normalization of entries that have SearchKey2 but are missing LOINC codes.