leocrm/docs/infrastructure_audit_report.md

# LeoCRM Infrastructure & Deployment Audit Report

**Audit Date:** 2026-07-30
**Auditor:** Runtime DevOps Engineer (parallel worker)
**Repository:** /a0/usr/workdir/leocrm-fix
**Live Endpoint:** https://crm.media-on.de/api/v1/health → `{"status":"healthy","version":"1.0.0"}`

---

## Executive Summary

| Severity | Count |
|----------|-------|
| CRITICAL | 1     |
| HIGH     | 3     |
| MEDIUM   | 5     |
| LOW      | 4     |

The application is live and healthy. The Dockerfile follows best practices (multi-stage, non-root, layer caching). However, there is a **CRITICAL SQL injection** in `prestart.sh`, the **CI/CD pipeline is not automated**, the **worker container lacks a healthcheck**, and **no resource limits** are defined for any service.

---

## 1. Container Health — docker-compose.yml

### Findings

| # | Severity | Finding | Location |
|---|----------|---------|----------|
| 1.1 | **HIGH** | Worker container (`crm-worker`) has NO healthcheck defined | `docker-compose.yml:119-153` |
| 1.2 | **MEDIUM** | No resource limits (memory/CPU) on ANY service | `docker-compose.yml` (entire file) |
| 1.3 | LOW | PostgreSQL, Redis, and app all use `restart: unless-stopped` ✓ | `docker-compose.yml:14,48,73,126` |
| 1.4 | LOW | `depends_on` with `condition: service_healthy` correctly used ✓ | `docker-compose.yml:75-76,130-131` |

### Details

**1.1 — Worker missing healthcheck:**
The `crm-worker` service has no `healthcheck` key. The `healthcheck.sh` script supports worker mode (Redis ping fallback), but it is never invoked for the worker container. Docker/Coolify cannot detect a wedged worker.

**Recommended fix:**
```yaml
crm-worker:
  healthcheck:
    test: ["CMD", "/app/healthcheck.sh"]
    interval: 30s
    timeout: 10s
    retries: 3
    start_period: 15s
```

**1.2 — No resource limits:**
None of the 4 services define `deploy.resources.limits` or `mem_limit`/`cpus`. A memory leak in the app or worker can OOM the host. In Coolify deployments, resource limits should be set via Coolify resource constraints.

---

## 2. Worker Stability — worker.sh, app/core/worker.py

### Findings

| # | Severity | Finding | Location |
|---|----------|---------|----------|
| 2.1 | **MEDIUM** | No ARQ job retry configuration (`max_tries` not set) | `app/core/worker.py:243-244` |
| 2.2 | LOW | `max_jobs = 10`, `job_timeout = 300s` — reasonable defaults | `app/core/worker.py:243-244` |
| 2.3 | LOW | Distributed cron lock via Redis SET NX + Lua release ✓ | `app/core/worker.py:30-52` |
| 2.4 | LOW | `on_startup` properly initializes plugins, event bus, search providers ✓ | `app/core/worker.py:78-130` |
| 2.5 | LOW | `exec arq` in worker.sh makes ARQ PID 1 for signal forwarding ✓ | `worker.sh:20` |

### Details

**2.1 — No job retry:**
ARQ's `WorkerSettings` does not set `max_tries`. ARQ defaults to `max_tries=0` (no retries). A transient failure (DB timeout, Redis blip) will permanently fail the job. For critical jobs like `process_outbox`, this can cause permanent outbox stalls.

**Recommended fix:**
```python
class WorkerSettings:
    max_tries = 3  # Retry failed jobs up to 3 times
```

---

## 3. Redis Connections — app/core/redis.py, app/core/auth.py, app/core/worker.py

### Findings

| # | Severity | Finding | Location |
|---|----------|---------|----------|
| 3.1 | **MEDIUM** | Cron lock helpers create a NEW Redis client per acquire/release — no pooling | `app/core/worker.py:37,49` |
| 3.2 | **MEDIUM** | No Redis connection pool size configured — uses redis-py defaults | `app/core/auth.py:37-38,62-63` |
| 3.3 | LOW | Session keys use SETEX with TTL (28800s = 8h) ✓ | `app/core/auth.py:145-147` |
| 3.4 | LOW | Global singleton pattern prevents connection leaks for app/API Redis ✓ | `app/core/auth.py:28-38` |

### Details

**3.1 — Cron lock connection churn:**
`_acquire_cron_lock()` and `_release_cron_lock()` each call `aioredis.from_url()` and `aclose()` on every invocation. With outbox processing running every 5 seconds, this creates 24 Redis connections/minute per cron job just for lock management.

**Recommended fix:** Reuse the global Redis client from `get_redis()` or pass the connection via ARQ context (`ctx['redis']`).

**3.2 — No pool size:**
`aioredis.from_url()` is called without `max_connections` parameter. Under high load, the default pool may exhaust. Add:
```python
aioredis.from_url(url, decode_responses=True, max_connections=50)
```

---

## 4. Migration Pipeline — alembic/

### Findings

| # | Severity | Finding | Location |
|---|----------|---------|----------|
| 4.1 | LOW | 83 migrations, single head at 0083 ✓ | `alembic/versions/` |
| 4.2 | LOW | 0028_rls_force and 0028_user_preferences are properly chained (not branched) ✓ | `alembic/versions/0028_*.py` |
| 4.3 | LOW | test_migrations.sh tests upgrade/downgrade/idempotency ✓ | `scripts/test_migrations.sh` |
| 4.4 | LOW | Downgrade failure is non-fatal in test_migrations.sh (acceptable) | `scripts/test_migrations.sh:54` |
| 4.5 | LOW | alembic/env.py uses async engine from config ✓ | `alembic/env.py:35-44` |

### Details

Migration graph is clean — `alembic heads` confirms a single head. The test script (`test_migrations.sh`) creates a throwaway database, runs `upgrade head`, verifies table count ≥ 50, runs `downgrade base`, then re-upgrades to verify idempotency. Solid approach.

---

## 5. CI/CD Pipeline — .github/workflows/, scripts/

### Findings

| # | Severity | Finding | Location |
|---|----------|---------|----------|
| 5.1 | **HIGH** | Only 1 GitHub workflow exists (cross-plugin imports only) — no test/build/deploy automation | `.github/workflows/check-cross-plugin-imports.yml` |
| 5.2 | **HIGH** | `scripts/ci_pipeline.sh` has 15 quality gates but is NOT wired into any CI workflow | `scripts/ci_pipeline.sh` (entire file) |
| 5.3 | LOW | Cross-plugin import check is well-implemented with exemptions ✓ | `scripts/check_cross_plugin_imports.py` |
| 5.4 | LOW | CI pipeline includes SQL injection, Jinja2 sandbox, RLS, and fail-closed checks ✓ | `scripts/ci_pipeline.sh:52-62` |

### Details

**5.1 + 5.2 — CI pipeline not automated:**
The `.github/workflows/` directory contains only `check-cross-plugin-imports.yml` (triggers on `app/plugins/**` changes). The comprehensive `ci_pipeline.sh` with 15 checks (compile, imports, alembic, TypeScript, frontend build, test collection, SQL injection, Jinja2 sandbox, RLS, fail-closed, ruff, cross-tenant, dependency scan, container smoke, npm ci) is **never executed in CI**. It must be run manually.

**Recommended fix:** Create `.github/workflows/ci.yml`:
```yaml
name: CI
on: [push, pull_request]
jobs:
  ci:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-python@v5
        with: { python-version: '3.12' }
      - uses: actions/setup-node@v4
        with: { node-version: '20' }
      - run: pip install -r requirements.txt
      - run: cd frontend && npm ci
      - run: bash scripts/ci_pipeline.sh
```

---

## 6. Prestart Script — prestart.sh

### Findings

| # | Severity | Finding | Location |
|---|----------|---------|----------|
| 6.1 | **CRITICAL** | SQL injection: password interpolated into SQL via f-string without escaping | `prestart.sh:49` |
| 6.2 | LOW | Uses `set -e` for fail-fast ✓ | `prestart.sh:14` |
| 6.3 | LOW | `exec uvicorn` makes it PID 1 for signal forwarding ✓ | `prestart.sh:62` |
| 6.4 | LOW | Uses MIGRATION_DATABASE_URL for alembic (RLS bypass for DDL) ✓ | `prestart.sh:18` |

### Details

**6.1 — SQL injection in prestart.sh:**
Line 49:
```python
await conn.execute(text(
    f"ALTER ROLE crm_runtime WITH LOGIN PASSWORD '{pwd}' NOSUPERUSER NOBYPASSRLS"
))
```
The `RUNTIME_DB_PASSWORD` environment variable is interpolated directly into a SQL string using an f-string. If the password contains a single quote (`'`), the SQL will break or be exploitable. This is a **CRITICAL** injection vulnerability.

**Recommended fix:** Use parameterized query or escape the password:
```python
await conn.execute(text(
    "ALTER ROLE crm_runtime WITH LOGIN PASSWORD :pwd NOSUPERUSER NOBYPASSRLS"
), {"pwd": pwd})
```

---

## 7. Dockerfile

### Findings

| # | Severity | Finding | Location |
|---|----------|---------|----------|
| 7.1 | LOW | Multi-stage build (3 stages: frontend, builder, runtime) ✓ | `Dockerfile:5,18,39` |
| 7.2 | LOW | Non-root user (appuser, UID 1000, GID 1000) ✓ | `Dockerfile:60-61` |
| 7.3 | LOW | Layer caching for npm (`COPY package.json` before `COPY frontend/`) ✓ | `Dockerfile:10-12` |
| 7.4 | LOW | Layer caching for pip (`COPY requirements.txt` before app source) ✓ | `Dockerfile:30-31` |
| 7.5 | LOW | `.dockerignore` excludes secrets, tests, docs, `.git` ✓ | `.dockerignore` |
| 7.6 | LOW | HEALTHCHECK defined in Dockerfile ✓ | `Dockerfile:73-74` |
| 7.7 | LOW | `apt-get` cleanup with `rm -rf /var/lib/apt/lists/*` ✓ | `Dockerfile:23,57` |

**No issues found.** The Dockerfile follows best practices.

---

## 8. Environment Configuration — .env.example, .env.docker.example

### Findings

| # | Severity | Finding | Location |
|---|----------|---------|----------|
| 8.1 | **MEDIUM** | `.env.example` missing `MIGRATION_DATABASE_URL`, `REDIS_PASSWORD`, `RUNTIME_DB_PASSWORD` | `.env.example` |
| 8.2 | LOW | `.env.docker.example` is comprehensive with all required vars ✓ | `.env.docker.example` |
| 8.3 | LOW | Required vars enforced with `:?` in docker-compose.yml (POSTGRES_PASSWORD, REDIS_PASSWORD, SECRET_KEY, DATABASE_URL) ✓ | `docker-compose.yml:17,50,83,84` |
| 8.4 | LOW | Secret generation instructions included ✓ | `.env.docker.example:16-17,24-25` |

### Details

**8.1 — `.env.example` incomplete:**
The `.env.example` file (used for local dev) is missing `MIGRATION_DATABASE_URL`, `REDIS_PASSWORD`, and `RUNTIME_DB_PASSWORD`. Developers following `.env.example` will hit runtime errors when the prestart script tries to set the crm_runtime password or when alembic needs the migration URL.

---

## 9. Health Check — healthcheck.sh, app/routes/health.py

### Findings

| # | Severity | Finding | Location |
|---|----------|---------|----------|
| 9.1 | LOW | `/api/v1/health` returns 200 `healthy` on live deployment ✓ | `https://crm.media-on.de/api/v1/health` |
| 9.2 | LOW | Three-tier health endpoints: `/health/live`, `/health/ready`, `/api/v1/health` ✓ | `app/routes/health.py:22,34,57` |
| 9.3 | LOW | healthcheck.sh dual-mode (HTTP + Redis fallback) ✓ | `healthcheck.sh:5-19` |
| 9.4 | LOW | Readiness probe checks DB, Redis, storage, worker heartbeat ✓ | `app/routes/health.py:37-52` |
| 9.5 | LOW | Redis password in healthcheck command visible in `docker inspect` (low risk — internal network) | `docker-compose.yml:59` |

**No critical issues.** Health check implementation is solid.

---

## 10. Backup/Restore — app/services/backup_service.py, scripts/backup.py, scripts/restore.py

### Findings

| # | Severity | Finding | Location |
|---|----------|---------|----------|
| 10.1 | **MEDIUM** | `backup_service.py` stores backups in `/tmp/leocrm-backups` — ephemeral, lost on container restart | `app/services/backup_service.py:13` |
| 10.2 | **MEDIUM** | `restore_backup()` uses `--clean --if-exists` but no transaction wrapping — partial restore possible | `app/services/backup_service.py:118-125` |
| 10.3 | LOW | `scripts/backup.py` is more robust: manifest, retention, S3/Nextcloud support ✓ | `scripts/backup.py` (entire) |
| 10.4 | LOW | `scripts/restore.py` validates manifest.json before restore ✓ | `scripts/restore.py:93-98` |
| 10.5 | LOW | `test_backup_restore.py` has basic unit tests for params/manifest ✓ | `tests/test_backup_restore.py` |
| 10.6 | LOW | No scheduled backup automation — must be triggered manually | (no cron/scheduler for backups) |

### Details

**10.1 — Ephemeral backup storage:**
`backup_service.py` uses `BACKUP_DIR = Path("/tmp/leocrm-backups")`. In a Docker container, `/tmp` is ephemeral. If the container restarts, all backups are lost. The volume mount in docker-compose only covers `/data/storage`, not `/tmp`.

**Recommended fix:** Change to `/data/backups` or use the `storage` volume.

**10.2 — Non-atomic restore:**
`restore_backup()` runs `pg_restore --clean --if-exists --no-owner --no-acl` without wrapping in a transaction. If the restore fails midway, the database is left in a partially-restored state with no automatic rollback.

---

## Summary of Recommendations (Priority Order)

1. **CRITICAL** — Fix SQL injection in `prestart.sh:49` — use parameterized query
2. **HIGH** — Add healthcheck to `crm-worker` in `docker-compose.yml`
3. **HIGH** — Wire `scripts/ci_pipeline.sh` into a GitHub/Forgejo workflow
4. **HIGH** — Expand `.github/workflows/` to include test/build/lint gates
5. **MEDIUM** — Add `max_tries=3` to `WorkerSettings` for job retry
6. **MEDIUM** — Add resource limits to all services in `docker-compose.yml`
7. **MEDIUM** — Reuse Redis connection in cron lock helpers instead of creating new clients
8. **MEDIUM** — Change `backup_service.py` backup dir from `/tmp` to persistent volume
9. **MEDIUM** — Add `MIGRATION_DATABASE_URL`, `REDIS_PASSWORD`, `RUNTIME_DB_PASSWORD` to `.env.example`
10. **LOW** — Configure Redis `max_connections` in `auth.py`
11. **LOW** — Add scheduled backup cron job
12. **LOW** — Wrap `restore_backup()` in a transaction

---

## Live Deployment Status

| Check | Result |
|-------|--------|
| Health endpoint | ✅ `{"status":"healthy","version":"1.0.0"}` |
| HTTPS | ✅ Reachable |
| Response time | < 3s |