# Fix Input Validation Issues Plan

## Problems

### Issue #8: No Status Validation
**File:** `src/routes/leads.js`
**Description:** Lead status updates accept any value - can set status to 'banana' or any string.
**Impact:** Database corruption, broken filters, undefined UI behavior.

### Issue #9: DoS via Unbounded Limit
**Files:** Multiple routes
**Description:** Most routes pass `parseInt(limit)` directly to database without max cap.
**Impact:** `?limit=999999999` causes massive queries, memory exhaustion, DoS.

## Severity
- Status Validation: **MEDIUM** - Data integrity issue
- Unbounded Limit: **HIGH** - DoS vulnerability

## Solutions

### Fix #1: Add Status Validation

Create validation constant and check:

```javascript
// Add to src/routes/leads.js or src/utils/validation.js
const VALID_LEAD_STATUSES = ['new', 'engaged', 'converted', 'opted_out', 'archived'];

// In PATCH /api/leads/:id handler (around line 447)
if (updates.status && !VALID_LEAD_STATUSES.includes(updates.status)) {
  return res.status(400).json({
    error: `Invalid status. Must be one of: ${VALID_LEAD_STATUSES.join(', ')}`
  });
}
```

### Fix #2: Add Limit Validation Helper

Add to `src/utils/validation.js`:

```javascript
/**
 * Safely parse and cap a limit parameter
 * @param {string|number} limit - The limit value from query
 * @param {number} defaultLimit - Default if not provided (default: 20)
 * @param {number} maxLimit - Maximum allowed (default: 100)
 * @returns {number} - Validated limit
 */
function parseLimit(limit, defaultLimit = 20, maxLimit = 100) {
  const parsed = parseInt(limit, 10);
  if (isNaN(parsed) || parsed < 1) return defaultLimit;
  return Math.min(parsed, maxLimit);
}

// Export it
module.exports = {
  // ... existing exports
  parseLimit,
};
```

Then replace raw `parseInt(limit)` calls with `parseLimit(limit)`.

## Files to Modify

### Status Validation
| File | Change |
|------|--------|
| `src/routes/leads.js` | Add VALID_LEAD_STATUSES check in PATCH handler |
| `src/utils/validation.js` | Export VALID_LEAD_STATUSES constant |

### Limit Validation (Priority Order)
| File | Lines | Current | Fix |
|------|-------|---------|-----|
| `src/routes/aiAlerts.js` | 45 | `parseInt(limit)` | `parseLimit(limit)` |
| `src/routes/coaching.js` | 75, 429 | `parseInt(limit)` | `parseLimit(limit)` |
| `src/routes/analytics.js` | 126, 134 | `parseInt(limit)` | `parseLimit(limit)` |
| `src/routes/sms.js` | 938, 1090, 1396 | `parseInt(limit)` | `parseLimit(limit)` |
| `src/routes/activation.js` | 54, 423, 1187 | `parseInt(limit)` | `parseLimit(limit)` |
| `src/routes/xp.js` | (check) | `parseInt(limit)` | `parseLimit(limit)` |

**Already Fixed:**
- `src/routes/callList.js:30` - Uses `Math.min(100, ...)`
- `src/routes/battlegroundV2.js` - Uses `|| 10` default

## Implementation Steps
- [ ] Phase 1: Add `parseLimit()` function to `src/utils/validation.js`
- [ ] Phase 2: Add `VALID_LEAD_STATUSES` constant and export
- [ ] Phase 3: Add status validation in `src/routes/leads.js` PATCH handler
- [ ] Phase 4: Replace `parseInt(limit)` with `parseLimit(limit)` in affected files
- [ ] Phase 5: Test status validation rejects invalid values
- [ ] Phase 6: Test limit capping works (request with ?limit=99999, should cap)

## Database Migration
None required.

## API Changes

### Status Validation
- `PATCH /api/leads/:id` with invalid status → 400 error
- Valid statuses: `new`, `engaged`, `converted`, `opted_out`, `archived`

### Limit Capping
- All list endpoints cap at 100 (or route-specific max)
- Example: `?limit=99999` → effectively `?limit=100`

## Success Criteria
1. `PATCH /api/leads/:id` with `status: "banana"` returns 400
2. `GET /api/sms/messages?limit=999999` returns max 100 results
3. No breaking changes for valid requests

## Notes for Builder

### Status Validation Code
Add before the field loop in PATCH handler (~line 447):
```javascript
// Validate status if being updated
const VALID_LEAD_STATUSES = ['new', 'engaged', 'converted', 'opted_out', 'archived'];
if (updates.status && !VALID_LEAD_STATUSES.includes(updates.status)) {
  return res.status(400).json({
    error: `Invalid status. Must be one of: ${VALID_LEAD_STATUSES.join(', ')}`
  });
}
```

### parseLimit Helper
```javascript
function parseLimit(limit, defaultLimit = 20, maxLimit = 100) {
  const parsed = parseInt(limit, 10);
  if (isNaN(parsed) || parsed < 1) return defaultLimit;
  return Math.min(parsed, maxLimit);
}
```

### Search Pattern for Limit Fixes
```bash
grep -rn "parseInt(limit)" src/routes/ --include="*.js"
```

## Notes for QA

### Test Status Validation
```bash
curl -X PATCH /api/leads/:id \
  -H "Authorization: Bearer ..." \
  -d '{"status": "invalid_status"}'
# Should return 400
```

### Test Limit Capping
```bash
curl "/api/sms/messages?limit=999999"
# Should return max 100 results, not crash
```

## Why This Matters

### Status Validation
- Invalid statuses break lead filtering
- UI may crash on unexpected values
- Reports become inaccurate
- Database integrity compromised

### Limit Capping
- Unbounded queries can exhaust memory
- Large result sets slow down the server
- Potential DoS attack vector
- Database connection pool exhaustion