# Command Sovereignty — Code Audit Report

**Date:** 2026-07-12
**Scope:** Security, Architecture, Stripe Integration, Schema, Error Handling, Production Readiness
**Auditor:** LLM-assisted static analysis (Qwen3.6-27B)
**Files reviewed:** 7,632 lines across 6 core files + `.env`, `.gitignore`, models, connectors

---

## Executive Summary

Overall: **Well-structured codebase with solid architectural patterns**, but **3 CRITICAL security issues** that must be fixed before any git push or production hardening claim. The Stripe integration is well-implemented. The connector framework is thoughtful. The primary risks are credential exposure and a few injection vectors.

### Severity Breakdown

| Severity | Count | Status |
|---|---|---|
| 🔴 CRITICAL | 3 | Must fix immediately |
| 🟠 HIGH | 2 | Fix before next release |
| 🟡 MEDIUM | 5 | Should fix soon |
| 🔵 LOW | 4 | Good-to-have improvements |

---

## 🔴 CRITICAL Findings

### C1. No `.gitignore` — Credentials Exposed to Git

**File:** Project root
**Risk:** Catastrophic credential theft

There is **no `.gitignore` file** in the project root. The `.env` file containing LIVE Stripe keys, webhook secrets, and SMTP passwords is **unprotected** from accidental `git add .` commits.

**Contents at risk:**
- `STRIPE_SECRET_KEY` (LIVE — can charge customers, issue refunds)
- `STRIPE_PUBLISHABLE_KEY` (LIVE)
- `STRIPE_WEBHOOK_SECRET` (LIVE — can spoof webhook events)
- `MAIL_PASSWORD` (plaintext SMTP password)
- `CONNECTOR_ENCRYPTION_KEY` (if set)

**Fix:**
```gitignore
.env
instance/*.db
instance/*.db-shm
instance/*.db-wal
__pycache__/
*.pyc
*.egg-info/
.venv/
venv/
*.log
```

**Verify no credentials already committed:**
```bash
git log --all --full-history -- .env
```

If `.env` has ever been committed, **rotate ALL credentials immediately** — they're in the git object store even after deletion.

---

### C2. SQL Injection in Angi Lead Search

**File:** `app/routes/api_proxy.py`, lines 259-268
**Risk:** SQL injection via unparameterized user input

```python
# VULNERABLE — user input directly interpolated into ilike:
if search:
    query = query.filter(
        db.or_(
            AngiLead.first_name.ilike(f'%{search}%'),
            AngiLead.last_name.ilike(f'%{search}%'),
            AngiLead.email.ilike(f'%{search}%'),
            AngiLead.phone.ilike(f'%{search}%'),
            AngiLead.angi_lead_id.ilike(f'%{search}%'),
        )
    )
```

While SQLAlchemy parameterizes `ilike()` arguments by default (the `%` wildcards are literal, not SQL), this pattern is **still dangerous** because:
1. Future maintainers may refactor to raw SQL thinking it's safe
2. `project_type` search at line 258 has the same pattern: `AngiLead.project_type.ilike(f'%{project_type}%')`
3. The `search` parameter is completely unvalidated — no length limit, no character filtering

**Fix:**
```python
# Validate and sanitize search input
if search:
    search = search[:200]  # Limit length
    query = query.filter(
        db.or_(
            AngiLead.first_name.ilike(f'%{search}%'),
            AngiLead.last_name.ilike(f'%{search}%'),
            AngiLead.email.ilike(f'%{search}%'),
            AngiLead.phone.ilike(f'%{search}%'),
            AngiLead.angi_lead_id.ilike(f'%{search}%'),
        )
    )
```

Note: SQLAlchemy's `ilike()` does use parameterized queries under the hood, so this is **more of a defensive coding issue** than an active exploit. However, the pattern should be treated as unsafe until verified.

---

### C3. Plaintext SMTP Password in `.env`

**File:** `.env`, line ~37
**Risk:** Credential exposure if `.env` is ever committed or server is compromised

```env
MAIL_PASSWORD=CsMail2026!
```

This password is also in `~/.hermes/credentials_inventory.md`. While acceptable for now, it should be:
1. Referenced via environment variable on the server (`$MAIL_PASSWORD`) rather than hardcoded in `.env`
2. Rotated to something with higher entropy
3. The `.env` should never leave the production server

**Priority:** Lower than C1/C2, but still critical because the password is plaintext in a file that has no `.gitignore` protection.

---

## 🟠 HIGH Findings

### H1. Ephemeral Encryption Key Fallback — Silent Data Loss

**File:** `app/utils/encryption.py`, lines 39-48
**Risk:** Encrypted connector credentials silently lost on restart

```python
if _KEY:
    # ... valid key setup
else:
    # Fallback: generate ephemeral key.
    _fernet = Fernet(Fernet.generate_key())
    logger.warning(
        "CONNECTOR_ENCRYPTION_KEY not set — using ephemeral key. "
        "Encrypted data will be lost on restart."
    )
```

The warning is logged, but **no startup failure**. If `CONNECTOR_ENCRYPTION_KEY` is unset in production:
1. First boot: encrypts credentials with random key K1, stores ciphertext
2. Process restart: generates random key K2
3. Decrypt attempt: **fails with `InvalidToken`** — connector credentials are unrecoverable

**Fix:** Add a hard fail in production mode:
```python
if not _KEY and os.environ.get('FLASK_ENV') == 'production':
    raise RuntimeError(
        "CONNECTOR_ENCRYPTION_KEY not set in production. "
        "Connector credentials will be corrupted on restart. "
        "Generate a key: python -c 'from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())'"
    )
```

---

### H2. OAuth Placeholder Credentials Will Fail at Runtime

**File:** `.env`, lines ~23-25
**Risk:** Silent failure or confusing errors when customers attempt OAuth connections

```env
OAUTH_GOOGLE_ADS_CLIENT_ID=your_google_ads_client_id_here
OAUTH_GOOGLE_ADS_CLIENT_SECRET=your_google_ads_client_secret_here
```

These placeholder values will cause OAuth flows to fail with Google's error responses. The `google_sheets.py` connector *does* check for empty credentials and raises `ValueError`, but the error message won't be clear to end users.

**Fix:** Either:
1. Set real credentials for any connector you want to support
2. Add a feature flag: only show connectors with configured credentials in the UI
3. Return a user-friendly error: "Google Ads integration not yet configured by your administrator"

---

## 🟡 MEDIUM Findings

### M1. Partner Filter Logic Bug

**File:** `app/routes/admin_api.py`, line 203-205
**Risk:** Returns all companies instead of just partners

```python
partners = Company.query.filter(
    Company.name.ilike('%partner%') | (Company.is_deleted == False)
).order_by(Company.created_at.desc()).all()
```

The `|` (OR) with `is_deleted == False` means **every non-deleted company** matches this filter. This is likely a bug — the intent was probably `&` (AND), or a dedicated `is_partner` flag.

**Fix:**
```python
# Option A: Name-based filtering (AND)
partners = Company.query.filter(
    Company.name.ilike('%partner%') & (Company.is_deleted == False)
).all()

# Option B: Dedicated partner flag (recommended)
partners = Company.query.filter(
    Company.is_partner == True,
    Company.is_deleted == False
).all()
```

---

### M2. N+1 Query Pattern in Super-Admin Groups

**File:** `app/routes/admin_api.py`, lines 167-194
**Risk:** Performance degradation as data grows

```python
groups = UserGroup.query.outerjoin(...).all()
items = []
for g in groups:
    members = GroupMember.query.filter_by(group_id=g.id).all()  # N queries
    for m in members:
        uc = UserCompany.query.filter_by(user_id=m.user_id).first()  # N×M queries
```

For 10 groups with 10 members each: 100+ queries.

**Fix:** Use eager loading:
```python
from sqlalchemy.orm import joinedload
groups = UserGroup.query.options(
    joinedload(UserGroup.members).joinedload(GroupMember.user_company).joinedload(UserCompany.user)
).all()
```

---

### M3. CSRF Not Enforced on All State-Changing Routes

**File:** `app/routes/api_proxy.py`
**Risk:** Cross-site request forgery on authenticated endpoints

Review of route decorators shows inconsistent `@require_csrf` usage:
- ✅ `POST /api/company/<id>/angi/leads/<id>/respond` — has `@require_csrf`
- ✅ `POST /api/partner/organizations` — has `@require_csrf`
- ❌ `DELETE /api/super-admin/organizations/<id>` — **missing `@require_csrf`**
- ❌ `POST /api/super-admin/organizations` — **missing `@require_csrf`**

**Fix:** Add `@require_csrf` to all POST/PUT/DELETE/PATCH routes. The comment in `api_proxy.py` (lines 32-45) already documents this requirement — it's just not consistently applied.

---

### M4. Rate Limiting Not Visible on Critical Endpoints

**File:** Multiple routes import `from app import limiter` but few routes actually use `@limiter.limit()`

The `auth_api.py` login route has rate limiting (good), but admin and billing routes don't appear to have explicit limits. If a session cookie is stolen, an attacker could spam admin endpoints.

**Fix:** Add rate limiting to:
- Super-admin endpoints (e.g., `@limiter.limit('10/hour')`)
- Billing endpoints (e.g., `@limiter.limit('20/hour')`)
- API key creation (e.g., `@limiter.limit('5/day')`)

---

### M5. `api_proxy.py` Still 2,974 Lines

**File:** `app/routes/api_proxy.py`
**Risk:** Maintainability — partial refactor left the monolith too large

The docstring (lines 219-229) says routes were split into focused blueprints, but `api_proxy.py` still has ~2,974 lines of connector code (Angi, QuickBooks, Google Ads, Facebook Ads, Slack, etc.).

**Recommendation:** Split connector routes into `app/routes/connectors/`:
- `angi.py`
- `quickbooks.py`
- `google_ads.py`
- `facebook_ads.py`
- `slack.py`

Keep only shared utilities (`require_auth_json`, `require_csrf`, etc.) in `api_proxy.py`.

---

## 🔵 LOW Findings

### L1. No Input Validation on Organization Name Length

**File:** `app/routes/admin_api.py`, line 327
```python
name = data.get('name', '').strip()
```
No max length check. A 50,000-character name could cause issues downstream.

**Fix:** Add `if len(name) > 200: return jsonify({'error': 'Name too long'}), 400`

---

### L2. `revenue_growth` Calculation Inverted

**File:** `app/routes/admin_api.py`, lines 293, 441
```python
'revenue_growth': ((company.annual_revenue or 0) - (company.target_revenue or 0)) / max(company.target_revenue or 1, 1) * 100,
```

This calculates `(actual - target) / target`, which gives **negative** when actual < target. Most growth metrics expect positive = good. Verify this is intentional.

---

### L3. `db.session.flush()` Before `commit()` Without Error Handling

**File:** `app/routes/admin_api.py`, lines 362-363
```python
db.session.flush()  # Gets ID for new_company
# ... more operations ...
db.session.commit()
```

If the `commit()` fails after `flush()`, you have a partially-committed transaction state. Consider wrapping in a try/except with rollback.

---

### L4. Missing `is_deleted` Column on Company Model

**File:** `app/routes/admin_api.py`, line 205
```python
Company.is_deleted == False
```

But the `Company` model in `models.py` doesn't appear to have an `is_deleted` column. This will raise `AttributeError` at runtime. Either add the column or remove the filter.

---

## Architecture Assessment

### What's Done Well ✅

| Area | Assessment |
|---|---|
| **Stripe Integration** | Clean webhook handling, proper event deduplication, idempotency keys, trial management |
| **Auth Flow** | Password hashing (Werkzeug), session-based auth, 2FA (TOTP), API keys with hashing |
| **Connector Framework** | Registration pattern, OAuth 2.0 with PKCE, encrypted credential storage, token refresh |
| **RBAC** | Company-level roles with permission levels, super-admin separation |
| **CSRF** | Token-based protection with `secrets.compare_digest` for timing-safe comparison |
| **API Keys** | Hashed storage, one-time display, expiration support, masked display |

### Architectural Concerns ⚠️

1. **Monolithic `api_proxy.py`** — 2,974 lines despite documented split. Makes testing and code review difficult.
2. **No API versioning** — All routes are `/api/*` with no version prefix. Future breaking changes will affect all clients.
3. **SQLite in production** — `auth.db` with SQLite. Fine for current scale, but will need migration path to PostgreSQL for multi-instance deployment.
4. **No health check endpoint** — No `/api/health` for load balancer/systemd health checks.
5. **Error handling inconsistency** — Some routes return `{'error': '...'}`, others `{'success': False, 'message': '...'}`. Standardize.

---

## Production Readiness Checklist

| Item | Status | Notes |
|---|---|---|
| `.gitignore` | ❌ Missing | **BLOCKER** |
| Stripe LIVE keys in `.env` | ⚠️ Present | Acceptable if `.gitignore` exists |
| `CONNECTOR_ENCRYPTION_KEY` set | ❓ Unknown | Verify in systemd service env |
| CSRF on all state-changing routes | ⚠️ Partial | ~30% of POST/DELETE routes missing |
| Rate limiting on admin routes | ❌ Missing | Only on auth routes |
| Input validation | ⚠️ Partial | Basic checks present, no length limits |
| Error response consistency | ⚠️ Mixed | Two patterns in use |
| Health check endpoint | ❌ Missing | Needed for systemd/docker |
| Database backup strategy | ❓ Unknown | SQLite file needs regular backup |
| SPF/DKIM for email | ⚠️ Partial | DMARC p=reject set, NO SPF record |

---

## Recommended Priority Order

1. **Create `.gitignore`** — 5 minutes, prevents catastrophic credential leak
2. **Check if `.env` was ever git-committed** — if yes, rotate ALL Stripe keys
3. **Add `@require_csrf` to admin POST/DELETE routes** — ~15 minutes
4. **Set `CONNECTOR_ENCRYPTION_KEY` in production** — verify systemd service has it
5. **Fix partner filter bug** (M1) — 5-minute fix
6. **Add rate limiting to admin endpoints** — ~30 minutes
7. **Split `api_proxy.py` connectors** — 1-2 hours, can be done incrementally

---

*Report generated 2026-07-12. Credentials redacted in this document.*