crm-system/security-review-phase2.md

# LeoCRM Phase 2 — Security & Data Risk Review

**Reviewer:** Security Data Engineer (A0 Orchestrator)  
**Date:** 2026-06-28  
**Project:** leocrm  
**Phase:** Pre-Implementation (Phase 2 to Phase 3)  
**Files reviewed:** architecture.md (1939 lines), task_graph.json (v2.0.0, 13 tasks), requirements.md (2142 lines, 143 features)  
**Scope:** Security architecture, multi-tenant isolation, auth, data persistence, migration, backup/restore, plugin security, dependency risks

---

## VERDICT: APPROVED_WITH_CONCERNS

The architecture is well-structured with strong fundamentals (session-based auth, CSRF protection, CSP headers, RBAC with field-level permissions, audit trail). However, **7 major risks** and **8 minor risks** must be addressed before or during implementation. No critical blocking issues found, but 3 major risks (RLS gap, rate limiting, CORS) should be resolved before Phase 3 start.

| Severity | Count |
|----------|-------|
| Critical | 0 |
| Major | 7 |
| Minor | 8 |

---

## 1. AUTH SECURITY — Session-Based Auth + API Tokens

### Design Summary
- **Session store:** Redis (`session:{id}`, TTL=8h) — primary runtime store
- **Audit trail:** PostgreSQL `sessions` table (immutable, retains all sessions ever created)
- **Cookie:** `leocrm_session=<id>; HttpOnly; Secure; SameSite=Strict; Path=/`
- **Password hashing:** bcrypt cost=12
- **API tokens:** `api_tokens` table (SHA-256 hashed, scoped, expiring) — post-MVP but architecture-ready
- **Password reset:** Token-based, 24h expiry, hashed storage, no user enumeration, session invalidation on reset

### Assessment: GOOD with concerns

**Positive:**
- ADR-05 decision is sound: server-side sessions avoid JWT pitfalls (token leakage, no revocation)
- Immediate session invalidation via Redis key deletion
- Forensic session audit trail in PostgreSQL (session validation flow checks Redis then PG audit then deny)
- No user enumeration on login or password reset
- Cookie flags correctly set (HttpOnly, Secure, SameSite=Strict)

**MAJOR RISK M-01: No brute-force protection on auth endpoints**  
- **Finding:** No account lockout, failed-attempt tracking, or rate limiting on `POST /api/v1/auth/login` or `POST /api/v1/auth/password-reset/request`
- **Impact:** An attacker can perform unlimited password guessing attempts. bcrypt cost=12 slows each attempt (~250ms) but does not prevent distributed attacks.
- **Requirements reference:** F-AUTH-01 has no lockout test scenario. Non-Goals section 18 explicitly excludes rate limiting from v1.
- **Recommendation:** Add a minimal failed-attempt counter in Redis (`login_failures:{email}`, TTL=15min, threshold=10, lockout 15min) before Phase 3. This is distinct from general rate limiting and is scoped to auth only.

**MINOR RISK m-01: LEOCRM_SECRET_KEY purpose and rotation undefined**  
- **Finding:** `LEOCRM_SECRET_KEY=<min-32-chars>` is listed in env vars but its usage is not specified (session ID generation? cookie signing? CSRF token generation?). No rotation policy documented.
- **Recommendation:** Document the secret's purpose in `.env.example` and define a rotation procedure in the admin guide. If used for signing session IDs, rotation invalidates all sessions (acceptable, document it).

**MINOR RISK m-02: No 2FA in v1**  
- **Finding:** Non-Goals section 13 explicitly excludes 2FA. Acceptable for v1 internal CRM, but should be prioritized post-MVP if exposed to internet.
- **Recommendation:** Document as post-MVP roadmap item with priority based on exposure.

---

## 2. MULTI-TENANT ISOLATION — Row-Level Security

### Design Summary
- Every table has `tenant_id` (UUID, NOT NULL on core tables)
- ORM auto-filtering via SQLAlchemy `before_query` event listener (`do_orm_execute`)
- `TenantMixin` base class enforces `tenant_id` column on all models
- Cross-tenant access returns 404 (not 403) to prevent information leakage
- Plugin tables must include `tenant_id` (validator checks)
- Tenant switch via `POST /api/v1/auth/switch-tenant`

### Assessment: MODERATE RISK — needs DB-level enforcement

**MAJOR RISK M-02: RLS claimed but only ORM-level filtering implemented**  
- **Finding:** Architecture line 154 states "PostgreSQL 16 with Row-Level Security for tenant isolation" but the implementation (lines 1232-1240) is **exclusively ORM-level filtering** via SQLAlchemy event listener. No `CREATE POLICY`, `ENABLE ROW LEVEL SECURITY`, or `SET app.current_tenant` session variables are defined.
- **Impact:** Any query that bypasses the ORM (raw SQL, `session.execute(text(...))`, stored procedures, Alembic migrations, ARQ worker jobs that don't set tenant context) will NOT be tenant-filtered. A single missed `_tenant_filter_disabled` flag or raw query can leak cross-tenant data.
- **Evidence:** The `before_query` listener checks `if not _tenant_filter_disabled` — this flag must be managed carefully. Any code path that sets it without restoring is a data leak vector.
- **Recommendation:** 
  1. Implement PostgreSQL RLS policies as a **defense-in-depth** layer: `CREATE POLICY tenant_isolation ON <table> USING (tenant_id = current_setting('app.current_tenant')::uuid)`
  2. Set `app.current_tenant` at the beginning of each DB session/transaction from the authenticated session context
  3. Keep ORM filtering as the primary layer; RLS as the safety net
  4. This is a design change — should be approved before Phase 3 implementation

**MAJOR RISK M-03: Tenant context propagation to ARQ workers not defined**  
- **Finding:** Background jobs (exports, mail-sync, reminders, backups) run in a separate worker process. The architecture does not specify how `current_tenant_id()` is set in worker context. If a worker job operates on tenant-scoped data without setting the tenant context, the ORM filter may not apply or may apply incorrectly.
- **Impact:** Cross-tenant data exposure in background job results (e.g., export contains data from all tenants).
- **Recommendation:** Define and document tenant context propagation for ARQ workers: each job must carry `tenant_id` in its job context, and the worker must set `current_tenant_id()` before executing any DB queries.

**MINOR RISK m-03: Tenant switch does not validate user-tenant membership**  
- **Finding:** `POST /api/v1/auth/switch-tenant` updates the session's `tenant_id`. The architecture does not explicitly state that the endpoint validates the user's membership in the target tenant via `user_tenants` table.
- **Recommendation:** Ensure the switch endpoint checks `user_tenants` membership before updating the session. Add a test case: user attempts to switch to a tenant they don't belong to then 403.

---

## 3. CSRF PROTECTION

### Design Summary
- SameSite=Strict cookie (browser-level protection)
- Origin-Header-Validierung middleware (server-side check)
- Only GET/HEAD/OPTIONS exempt from CSRF check
- CSRF token stored per session in Redis and PostgreSQL audit table

### Assessment: GOOD

**Positive:**
- Two-layer CSRF protection (SameSite + Origin validation) is a solid approach
- SameSite=Strict is the strongest browser-level CSRF defense
- Origin validation is server-side and not bypassable by client tweaks
- Test scenarios defined in task_graph.json: "CSRF: POST without Origin header then 403"

**MINOR RISK m-04: CSRF token stored but not validated in requests**  
- **Finding:** A `csrf_token` is generated and stored per session, but the architecture does not describe a mechanism where the frontend sends the token back (e.g., in a `X-CSRF-Token` header) and the backend validates it. The protection relies entirely on SameSite + Origin.
- **Impact:** SameSite=Strict + Origin validation is sufficient for v1. The stored CSRF token appears unused.
- **Recommendation:** Either (a) remove the csrf_token from the session model if SameSite+Origin is the chosen strategy, or (b) implement double-submit cookie pattern for defense-in-depth. Clarify in architecture.

---

## 4. INPUT VALIDATION

### Design Summary
- Pydantic schemas validate all API inputs (F-DATA-03)
- XSS protection: server-side Pydantic validation + frontend DOMPurify/escaped rendering (F-SEC-02)
- CSP headers prevent inline script execution
- HTML in user inputs is escaped, not rendered

### Assessment: GOOD

**Positive:**
- Pydantic on all API inputs is FastAPI best practice
- Server-side + client-side sanitization (defense in depth)
- CSP header is well-configured: `script-src 'self'`, `object-src 'none'`, `base-uri 'self'`
- XSS test scenarios defined in requirements

**MINOR RISK m-05: No SQL injection prevention explicitly documented**  
- **Finding:** While SQLAlchemy ORM with parameterized queries is the default, the architecture does not explicitly state a prohibition on raw SQL or string interpolation in queries.
- **Recommendation:** Add an explicit coding guideline: no raw SQL with string interpolation; all raw queries must use parameterized `text()` with bind parameters.

---

## 5. SECRETS MANAGEMENT (F-ENV-01)

### Design Summary
- `.env.example` documents all environment variables with `<secret>` placeholders
- Secrets never in Git repo (`.gitignore` includes `.env`)
- Missing secret env var then app fails to start with clear error (F-ENV-01 test scenario 3)
- Secrets: POSTGRES_PASSWORD, LEOCRM_SECRET_KEY, SMTP_PASS, MAIL_ENCRYPTION_KEY, S3_SECRET_KEY, AI_API_KEY
- Pydantic Settings for env var loading (config.py)

### Assessment: ADEQUATE for v1 with gaps

**MAJOR RISK M-04: No secret rotation policy**  
- **Finding:** No rotation procedure is defined for any secret (LEOCRM_SECRET_KEY, MAIL_ENCRYPTION_KEY, POSTGRES_PASSWORD, SMTP_PASS). F-ENV-01 only covers initial setup, not lifecycle.
- **Impact:** If a secret is compromised, there is no documented procedure to rotate it. MAIL_ENCRYPTION_KEY rotation is especially critical — changing it without a re-encryption plan would make existing encrypted mail credentials unreadable.
- **Recommendation:** 
  1. Document rotation procedures for each secret in admin guide
  2. For MAIL_ENCRYPTION_KEY: implement key versioning (store key_id with encrypted data, support old + new key during rotation)
  3. For LEOCRM_SECRET_KEY: document that rotation invalidates all sessions (acceptable)
  4. For POSTGRES_PASSWORD: document procedure (change password, update env, restart)

**MINOR RISK m-06: .env file approach for production**  
- **Finding:** Docker Compose uses `env_file: .env` for all services including production on Coolify. This means secrets are stored in a plaintext file on the server.
- **Impact:** If the host filesystem is compromised, all secrets are readable. Docker env vars are also visible via `docker inspect`.
- **Recommendation:** For production on Coolify, use Coolify's secret/environment variable management (injects as container env vars without a file on disk). The `.env` file approach is fine for dev only. Document this split in the deployment guide.

---

## 6. DATA MIGRATION RISK (F-MIG-01)

### Design Summary
- F-MIG-01: CSV import with field mapping, per-row error reporting, auto-company-detection for contact imports
- No legacy system data migration (no ETL from external CRM systems)
- No schema migration risk (greenfield project with Alembic)

### Assessment: LOW RISK — properly scoped

**Positive:**
- CSV import is well-defined with field mapping and error handling
- No complex legacy migration in v1 (correct scope decision)
- Alembic for schema migrations is standard and reliable

**MAJOR RISK M-05: CSV import has no file size limit or row count validation**  
- **Finding:** F-MIG-01 test scenario imports 50 companies. No mention of maximum file size, maximum row count, or memory protection for large CSV files. A 500MB CSV with 1M rows could cause OOM or timeout.
- **Impact:** Denial of service via large CSV upload; potential memory exhaustion.
- **Recommendation:** 
  1. Define max upload size (e.g., 10MB for CSV)
  2. Process CSV in streaming mode (not loading entire file into memory)
  3. Add row count limit (e.g., 50,000 rows per import)
  4. Run import as background job (ARQ) for files >1000 rows

---

## 7. BACKUP/RESTORE (F-INFRA-02)

### Design Summary
- `pg_dump` daily cron job to backup volume or S3
- Storage volume backup (files)
- Restore documented in `docs/admin-guide.md`
- Backup failure triggers alert to admin

### Assessment: MAJOR RISK — inadequate for multi-tenant production

**MAJOR RISK M-06: Backup strategy insufficient for multi-tenant PostgreSQL**  
- **Finding:** The backup design has multiple gaps:
  1. **No backup encryption:** `pg_dump` output is plaintext. Tenant data (companies, contacts, emails) is stored unencrypted in the backup volume/S3.
  2. **No retention policy:** No definition of how many backups to keep (7 days? 30 days?). Unlimited backups cause storage exhaustion; too few cause data loss.
  3. **No tested restore procedure:** F-INFRA-02 acceptance criterion says "Restore-Dokumentation vorhanden" but there is no test scenario that verifies an actual restore works.
  4. **No point-in-time recovery:** Only daily `pg_dump` snapshots. If a tenant accidentally deletes data at 14:00 and notices at 17:00, all data created between 00:00 and 14:00 that day is lost.
  5. **Multi-tenant restore granularity:** `pg_dump` is all-or-nothing. If one tenant needs restore, all tenants are affected. No mention of tenant-level export/restore.
  6. **Redis not backed up:** Session data is in Redis with TTL=8h. Redis is not included in backup strategy. If Redis is lost, all active sessions are invalidated (users must re-login). This is acceptable but should be documented.
- **Recommendation:**
  1. Encrypt pg_dump output (gpg or S3 SSE-KMS)
  2. Define retention: 7 daily + 4 weekly + 12 monthly
  3. Add a restore test to the test suite (backup, restore, verify row count)
  4. Enable PostgreSQL WAL archiving for point-in-time recovery (PITR)
  5. Document that restore is all-tenant; consider tenant-level CSV export as a quick-recovery alternative
  6. Document Redis session loss behavior (acceptable: users re-login)

---

## 8. PLUGIN SECURITY (T03 Plugin Framework)

### Design Summary
- Built-in plugins only (no dynamic external loading in v1) — ADR-03
- Plugin manifest schema (Pydantic)
- Lifecycle hooks: install/activate/deactivate/uninstall
- Plugin DB migration runner with `plugin_migrations` tracking
- Migration validator checks `tenant_id` on all plugin tables
- Service Container DI: plugins receive db, cache, event_bus, storage, notifications
- Event Bus integration: plugins register/unregister event listeners

### Assessment: MODERATE RISK — tenant isolation enforced, but no permission scoping

**MAJOR RISK M-07: No plugin API permission scoping**  
- **Finding:** Plugins receive injected services (db, cache, event_bus, storage, notifications) but there is no permission model restricting what a plugin can do. A plugin with access to the `db` session can query any table within the current tenant context. There is no "plugin A can only read companies, plugin B can only write to its own tables" model.
- **Impact:** A malicious or buggy built-in plugin could access/modify data from other modules within the same tenant. Since all v1 plugins are built-in (shipped with code), this is lower risk, but the architecture should define the permission model for when external plugins are added post-MVP.
- **Recommendation:**
  1. For v1: document that plugins are trusted (built-in only) and have full tenant-scoped access
  2. For post-MVP: define plugin permission scopes in the manifest (e.g., `permissions: ["companies:read", "contacts:write"]`)
  3. Add a test: plugin cannot access data from a different tenant (already covered by tenant_id validator)

**MINOR RISK m-07: Plugin event bus has no namespacing**  
- **Finding:** Plugins register event listeners on a shared event bus. There is no mention of event namespacing to prevent event name collisions between plugins.
- **Recommendation:** Use prefixed event names (e.g., `dms.file.uploaded`, `calendar.event.created`) to avoid collisions.

**MINOR RISK m-08: Plugin uninstall with data removal has no confirmation audit**  
- **Finding:** `DELETE /api/v1/plugins/{name}?remove_data=true` drops plugin tables. The architecture does not mention that this destructive action is logged in the audit log.
- **Recommendation:** Log plugin uninstall with data removal to `audit_log` with actor, timestamp, plugin name, and table list.

---

## 9. DEPENDENCY RISKS

### Design Summary
- **Backend:** FastAPI, SQLAlchemy, Pydantic, ARQ, Redis-py, asyncpg/psycopg
- **Frontend:** React 18, TanStack Query v5, Vite, Tailwind CSS
- **Database:** PostgreSQL 16-alpine
- **Cache/Queue:** Redis 7-alpine
- **Document editing:** OnlyOffice Document Server

### Assessment: LOW-MODERATE RISK

**OnlyOffice `:latest` tag**  
- **Finding:** Docker Compose uses `onlyoffice/documentserver:latest`. This tag is mutable and can introduce breaking changes or security vulnerabilities without notice.
- **Impact:** Unpredictable updates; potential breaking changes; supply chain risk.
- **Recommendation:** Pin to a specific version tag (e.g., `onlyoffice/documentserver:8.2.2`). Update deliberately after testing.

**Other dependency notes:**
- FastAPI, React 18, PostgreSQL 16, Redis 7 are all current stable major versions with active security maintenance
- No known critical CVEs in these major versions as of 2026-06
- **Recommendation:** Pin all dependencies in `requirements.txt` / `package.json` with exact versions or minimum patches. Add `pip-audit` and `npm audit` to CI pipeline.
- **Recommendation:** Use `postgres:16-alpine` and `redis:7-alpine` (already specified — good). Pin minor versions for reproducibility.

---

## 10. RATE LIMITING

### Assessment: MAJOR RISK — explicitly excluded from v1

**Finding:** Non-Goals section 18: "Kein zentrales Rate-Limiting in v1." This means:
- `POST /api/v1/auth/login` — no rate limit (brute-force possible, see M-01)
- `POST /api/v1/auth/password-reset/request` — no rate limit (email bombing possible)
- All API endpoints — no rate limit (DoS via excessive requests)
- API tokens (post-MVP) — no rate limit per token

**Impact:**
- Auth endpoints are brute-force vulnerable (mitigated partially by bcrypt cost=12, but not for distributed attacks)
- Password reset endpoint can be abused to send unlimited emails (SMTP abuse, email bombing)
- General API abuse (data scraping, DoS)

**Recommendation:**
- Implement **auth-scoped rate limiting** (not general rate limiting) before Phase 3:
  - Login: 10 attempts per email per 15 min (Redis counter)
  - Password reset: 3 requests per email per hour
  - This is minimal effort and high security value
- General API rate limiting can remain post-MVP if the app is internal-only, but document the decision

---

## 11. CORS

**MAJOR RISK M-08: CORS configuration not specified**  
- **Finding:** The frontend is served on port 80 (Nginx) and the API on port 8000 (FastAPI). In production, they may share a domain (reverse proxy) or be on separate ports. The architecture does not specify CORS headers.
- **Impact:** If frontend and API are on different origins (e.g., dev environment: `localhost:80` to `localhost:8000`), the browser will block requests without proper CORS headers. If CORS is set to wildcard, credentials (cookies) will not work.
- **Recommendation:**
  - In production: serve frontend + API behind the same reverse proxy (same origin, no CORS needed)
  - In development: configure FastAPI CORS middleware with `allow_origins=["http://localhost:80"]`, `allow_credentials=True`, `allow_methods=["*"]`, `allow_headers=["*"]`
  - Never use `allow_origins=["*"]` with `allow_credentials=True` (browser rejects this)
  - Document CORS configuration in architecture.md

---

## 12. DOCKER/COMPOSE SECURITY

### Findings

| Issue | Severity | Detail |
|-------|----------|--------|
| No non-root user | Minor | All containers run as root by default. Add `user:` directive or use images with non-root users. |
| No `cap_drop: ALL` | Minor | Containers retain all Linux capabilities. Drop all and add only needed ones. |
| No read-only filesystem | Minor | Add `read_only: true` with `tmpfs` for writable paths. |
| Redis without auth | Major | `redis:7-alpine` has no `requirepass` or ACL configured. Any container on the network can access Redis. |
| OnlyOffice `:latest` | Minor | Mutable tag; pin to specific version. |
| All ports exposed | Minor | `ports: ["8000:8000"]`, `["80:80"]`, `["8080:80"]` expose to host. In production, use internal network + reverse proxy only. |
| No health checks on all services | Minor | Only `backend` has a healthcheck. Add for `postgres`, `redis`, `worker`. |
| No resource limits | Minor | No `mem_limit`, `cpus` limits. A runaway process can consume all host resources. |

**Recommendation for Redis auth:**
- Add `REDIS_PASSWORD` env var
- Configure Redis with `requirepass` or use ACL users
- Update `REDIS_URL` to include password: `redis://:<password>@redis:6379/0`

---

## 13. FILE UPLOADS

### Finding
DMS plugin (T05) handles file uploads via `POST /api/v1/dms/files/upload (multipart)`. The architecture does not specify:
- Maximum file size limit
- Allowed file types / MIME type validation
- File content verification (magic bytes, not just extension)
- Malware / virus scanning
- Filename sanitization (path traversal prevention)

### Assessment: MAJOR RISK (deferred to plugin implementation)
- **Impact:** Path traversal via malicious filenames, disk exhaustion via large files, stored XSS via uploaded HTML/SVG files, potential malware storage.
- **Recommendation:** Define upload security in T05 task specification:
  1. Max file size: 50MB (configurable)
  2. Allowed MIME types whitelist (exclude `text/html`, `image/svg+xml`, `application/javascript`)
  3. Filename sanitization: strip path components, use UUID-based storage names
  4. Store files outside web root (already handled by storage service)
  5. Post-MVP: ClamAV integration for malware scanning

---

## 14. LOGGING SENSITIVE DATA

### Assessment: GOOD
- Structured JSON logs (F-INFRA-03): timestamp, level, event, method, path, status, duration, tenant_id, user_id
- No password, token, or secret values in log format
- Log level configurable via `LOG_LEVEL` env var
- **Recommendation:** Add explicit log sanitization in the logging middleware: filter out `password`, `new_password`, `token`, `Authorization` header fields from request body logging if request body is ever logged.

---

## 15. DATA PERSISTENCE AND DATA LOSS RISK

### Assessment: MODERATE RISK
- Soft-delete with `deleted_at` column — good for accidental deletion recovery
- DSGVO hard-delete with `deletion_log` — good for compliance
- `deletion_log` mentioned in architecture but table schema not fully defined in the reviewed sections
- **Risk:** Soft-deleted data is still in the database. If a tenant requests GDPR deletion, the hard-delete must also remove soft-deleted records.
- **Recommendation:** Verify `deletion_log` table schema includes: tenant_id, entity_type, entity_id, deleted_by, deleted_at, data_summary (for audit).

---

## SUMMARY TABLE

| # | Risk | Severity | Domain | Action Before Phase 3? |
|---|------|----------|--------|----------------------|
| M-01 | No brute-force protection on auth | Major | Auth | YES — add Redis-based attempt counter |
| M-02 | RLS claimed but ORM-only filtering | Major | Multi-Tenant | YES — add DB-level RLS as defense-in-depth |
| M-03 | ARQ worker tenant context undefined | Major | Multi-Tenant | YES — define in architecture |
| M-04 | No secret rotation policy | Major | Secrets | NO — document before deployment |
| M-05 | CSV import no size/row limit | Major | Migration | NO — add in T05/T07 implementation |
| M-06 | Backup insufficient for multi-tenant | Major | Backup | NO — resolve before deployment |
| M-07 | No plugin API permission scoping | Major | Plugins | NO — acceptable for v1 (built-in only) |
| M-08 | CORS not specified | Major | Network | YES — configure and document |
| m-01 | LEOCRM_SECRET_KEY purpose/rotation undefined | Minor | Secrets | NO |
| m-02 | No 2FA in v1 | Minor | Auth | NO (post-MVP) |
| m-03 | Tenant switch membership validation | Minor | Multi-Tenant | YES — add test case |
| m-04 | CSRF token stored but unused | Minor | CSRF | NO — clarify architecture |
| m-05 | No SQL injection prevention guideline | Minor | Validation | NO — add coding guideline |
| m-06 | .env file for production | Minor | Secrets | NO — use Coolify env management |
| m-07 | Plugin event bus no namespacing | Minor | Plugins | NO |
| m-08 | Plugin uninstall no audit log | Minor | Plugins | NO |

---

## TOP 3 RISKS

1. **M-02: RLS gap** — Architecture claims PostgreSQL RLS but implements only ORM-level tenant filtering. Raw SQL, worker jobs, or filter bypass bugs can leak cross-tenant data. **Must add DB-level RLS policies as defense-in-depth before implementation.**

2. **M-01: No brute-force protection** — Auth endpoints (login, password reset) have no rate limiting, lockout, or failed-attempt tracking. Combined with M-08 (no CORS config), the attack surface for credential attacks is significant. **Must add minimal Redis-based auth rate limiting before Phase 3.**

3. **M-06: Backup strategy inadequate** — No encryption, no retention policy, no tested restore, no PITR for multi-tenant PostgreSQL. Data loss risk for production tenants. **Must resolve before deployment phase.**

---

## RECOMMENDATION FOR PHASE 3 START

**APPROVED_WITH_CONCERNS — Phase 3 may start after addressing the 3 pre-implementation items:**

1. **M-02:** Add PostgreSQL RLS policy definitions to architecture.md (defense-in-depth alongside ORM filtering)
2. **M-01 + Rate Limiting:** Add auth-scoped rate limiting (login attempt counter + password reset throttle) to T01 task specification
3. **M-08:** Add CORS configuration to architecture.md (same-origin in prod, explicit origins in dev)

Additionally, update T01 task to include:
- ARQ worker tenant context propagation (M-03)
- Tenant switch membership validation test (m-03)
- Redis auth configuration (Docker Compose)

The remaining major risks (M-04, M-05, M-06, M-07) can be addressed during implementation or before deployment.

---

*Review complete. No secrets, credentials, or live values were inspected. All findings based on architecture.md, task_graph.json, and requirements.md content only.*