kitos/Aegis
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

docs: update ARCHITECTURE.md and ARCHITECTURAL_ANALYSIS.md to reflect all low-priority items completed (LP-8)

Browse Source
This commit is contained in:
Kitos
2026-02-20 13:39:55 +01:00
parent d77075272e
commit bbc2dddd86
2 changed files with 115 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files

130
docs/ARCHITECTURAL_ANALYSIS.md
View File

@@ -1,7 +1,7 @@
# Aegis — Deep Architectural Analysis
> **Author:** Automated architecture review
> **Date:** February 11, 2026 (updated February 19, 2026)
> **Date:** February 11, 2026 (updated February 20, 2026)
> **Scope:** Backend (FastAPI/Python), Frontend (React/TypeScript), Infrastructure (Docker)
>
> **Note:** Sections marked with ✅ reflect changes implemented since the initial analysis.
@@ -69,9 +69,9 @@ Aegis follows a **layered monolithic architecture** deployed as two containers (
| Layer | Files | Actual Responsibility |
|-------|-------|----------------------|
| **Routers** | 21 files | ✅ Thin HTTP adapters — auth, param parsing, response formatting. Delegate to services. |
| **Services** | 30+ files | ✅ All business logic, query orchestration, domain validation. Framework-agnostic. |
| **Domain** | 8+ files | ✅ Pure entities, value objects, ports, errors. Zero framework imports. |
| **Routers** | 21 files | ✅ Thin HTTP adapters — auth, param parsing, response formatting. All delegate to services. |
| **Services** | 33+ files | ✅ All business logic, query orchestration, domain validation. Framework-agnostic. |
| **Domain** | 15+ files | ✅ Pure entities (Test, Technique, Campaign, Compliance), value objects, ports (repos + ImportService protocol), errors. Zero framework imports. |
| **Infrastructure** | 5+ files | ✅ Repository implementations, Redis client, mappers. |
| **Models** | 19 files | ORM table definitions — persistence mapping only |
| **Schemas** | 10 files | Pydantic DTOs for request/response |
@@ -91,9 +91,9 @@ def get_threat_actor(actor_id: str, db=Depends(get_db), current_user=Depends(get
return get_actor_detail(db, actor_id)
```
Extracted services: `coverage_report_service`, `metrics_query_service`, `compliance_service`, `detection_rule_service`, `threat_actor_service`, `test_crud_service`, `evidence_service`, `campaign_crud_service`, `scoring_config_service`.
Extracted services: `coverage_report_service`, `metrics_query_service`, `compliance_service`, `detection_rule_service`, `threat_actor_service`, `test_crud_service`, `evidence_service`, `campaign_crud_service`, `scoring_config_service`, `user_service`, `audit_query_service`, `data_source_service`.
**Remaining:** `users.py`, `audit.py`, `data_sources.py`, `heatmap.py` still have direct queries. These are lower priority since they are simpler or already partially extracted.
**Update (Feb 20):** All routers now delegate to services. No routers contain direct ORM queries or business logic.
---
@@ -110,25 +110,25 @@ Schemas NONE NONE LOW — NONE NONE
Database NONE NONE NONE — NONE LOW
```
### 2.2. Router ↔ Model — ✅ LARGELY RESOLVED (was HIGH COUPLING)
### 2.2. Router ↔ Model — ✅ FULLY RESOLVED (was HIGH COUPLING)
**Update (Feb 19):** Most routers no longer import ORM models or execute queries directly. Only **4 out of 21 routers** still have direct DB access:
**Update (Feb 20):** All routers now delegate to services. No router imports ORM models or executes queries directly.
| Router | Status | Detail |
|--------|--------|--------|
| `techniques.py` | ✅ Extracted | Uses `SATechniqueRepository` via dependency injection |
| `reports.py` | ✅ Extracted | Delegates to `coverage_report_service` |
| `metrics.py` | ✅ Extracted | Delegates to `metrics_query_service` |
| `compliance.py` | ✅ Extracted | Delegates to `compliance_service` |
| `detection_rules.py` | ✅ Extracted | Delegates to `detection_rule_service` |
| `threat_actors.py` | ✅ Extracted | Delegates to `threat_actor_service` |
| `tests.py` | ✅ Extracted | Delegates to `test_crud_service` + `test_workflow_service` |
| `evidence.py` | ✅ Extracted | Delegates to `evidence_service` |
| `campaigns.py` | ✅ Extracted | Delegates to `campaign_crud_service` |
| `users.py` | Remaining | Direct queries (simple CRUD) |
| `audit.py` | Remaining | Direct queries (read-only list) |
| `data_sources.py` | Remaining | Direct queries |
| `heatmap.py` | Remaining | Complex queries (partially extracted via `heatmap_service`) |
| Router | Status | Service |
|--------|--------|---------|
| `techniques.py` | ✅ Extracted | `SATechniqueRepository` via dependency injection |
| `reports.py` | ✅ Extracted | `coverage_report_service` |
| `metrics.py` | ✅ Extracted | `metrics_query_service` |
| `compliance.py` | ✅ Extracted | `compliance_service` |
| `detection_rules.py` | ✅ Extracted | `detection_rule_service` |
| `threat_actors.py` | ✅ Extracted | `threat_actor_service` |
| `tests.py` | ✅ Extracted | `test_crud_service` + `test_workflow_service` |
| `evidence.py` | ✅ Extracted | `evidence_service` |
| `campaigns.py` | ✅ Extracted | `campaign_crud_service` |
| `users.py` | ✅ Extracted | `user_service` |
| `audit.py` | ✅ Extracted | `audit_query_service` |
| `data_sources.py` | ✅ Extracted | `data_source_service` |
| `heatmap.py` | ✅ Extracted | `heatmap_service` |
### 2.3. Router ↔ Database — HIGH COUPLING
@@ -197,10 +197,13 @@ Communication is via REST API with aligned but independent types (`types/models.
| **Threat actors** | ✅ SEPARATED | `threat_actor_service.py` handles queries, coverage, and gap analysis (N+1 fixed) |
| **Evidence** | ✅ SEPARATED | `evidence_service.py` handles permission validation and queries with domain exceptions |
| **Campaigns** | ✅ SEPARATED | `campaign_crud_service.py` handles CRUD, lifecycle, and scheduling |
| **Heatmap/visualization** | PARTIAL | `heatmap_service.py` exists but router still has some logic |
| **Data import** | WELL SEPARATED | The 8 import services are correctly isolated |
| **Heatmap/visualization** | ✅ SEPARATED | `heatmap_service.py` contains all layer-building logic; router is a thin adapter |
| **Data import** | ✅ WELL SEPARATED | 8 import services behind `ImportService` protocol + central registry |
| **Data sources** | ✅ SEPARATED | `data_source_service.py` handles CRUD, sync dispatch, and stats |
| **Users** | ✅ SEPARATED | `user_service.py` handles CRUD, validation, and hashing |
| **Audit queries** | ✅ SEPARATED | `audit_query_service.py` handles paginated queries and distinct lookups |
| **Notifications** | WELL SEPARATED | `notification_service.py` encapsulates all logic |
| **Auditing** | WELL SEPARATED | `audit_service.py` is a pure `log_action()` function |
| **Auditing (writes)** | WELL SEPARATED | `audit_service.py` is a pure `log_action()` function |
### 3.2. Anemic Model (Anti-pattern)
@@ -237,36 +240,40 @@ Logic that should be in domain models (business validations, state transitions,
| Component | Compliant? | Detail |
|-----------|-----------|-------|
| `heatmap.py` (router) | PARTIAL | Still has some inline logic; `heatmap_service` exists but not fully extracted |
| `heatmap.py` (router) | ✅ YES | Thin adapter → `heatmap_service` |
| `reports.py` (router) | ✅ YES | Thin adapter → `coverage_report_service` |
| `tests.py` (router) | ✅ YES | Thin adapter → `test_crud_service` + `test_workflow_service` |
| `campaigns.py` (router) | ✅ YES | Thin adapter → `campaign_crud_service` |
| `evidence.py` (router) | ✅ YES | Thin adapter → `evidence_service` |
| `users.py` (router) | ✅ YES | Thin adapter → `user_service` |
| `audit.py` (router) | ✅ YES | Thin adapter → `audit_query_service` |
| `data_sources.py` (router) | ✅ YES | Thin adapter → `data_source_service` |
| `scoring_service.py` | ✅ YES | Reads weights from `scoring_config_service` (DB-backed, not mutable settings) |
| `test_workflow_service.py` | ✅ YES | Single responsibility: test state machine |
| `notification_service.py` | ✅ YES | Single responsibility: notification management |
| `audit_service.py` | ✅ YES | Single responsibility: audit logging |
**Verdict:** All major routers now comply with SRP. Only `heatmap.py` and a few minor routers have remaining inline logic.
**Verdict:** All routers now comply with SRP. Every router is a thin HTTP adapter delegating to a dedicated service.
### 4.2. Open/Closed Principle (OCP) — ✅ PARTIALLY RESOLVED (was VIOLATION)
### 4.2. Open/Closed Principle (OCP) — ✅ MOSTLY RESOLVED (was VIOLATION)
**Update (Feb 19):**
**Update (Feb 20):**
- **Scoring weights:** ✅ Resolved — Weights are now persisted in the `scoring_config` DB table via `scoring_config_service.py`. The `ScoringWeights` value object validates invariants (sum = 100, non-negative). No more mutable global `settings`.
- **Heatmap layers:** Each heatmap type is a separate endpoint with hardcoded logic. Adding a new layer type requires modifying the router.
- **Import services:** Each data source is a separate service without a common interface. Adding a new source requires creating a new service AND modifying `data_sources.py` and `system.py`.
- **Import services:** ✅ Resolved — All import services now satisfy the `ImportService` protocol (`domain/ports/import_service.py`). A central `IMPORT_REGISTRY` maps source names to lazy-loaded handlers. Adding a new import source requires only: (1) creating a new service module, (2) adding one line to `IMPORT_REGISTRY`.
- **Heatmap layers:** Each heatmap type is a separate endpoint with hardcoded logic. Adding a new layer type requires modifying the router. Low priority.
- **Test states:** The state machine is well defined in `VALID_TRANSITIONS`, but adding a new state requires modifying the dictionary AND potentially all services that read `TestState`.
### 4.3. Liskov Substitution Principle (LSP) — N/A (Partial)
There is no significant inheritance or polymorphism in the backend. Services are functions, not classes. There are no interfaces or abstract classes. **Does not directly apply**, but the absence of formal contracts (protocols/ABCs) is a symptom of not being designed for extensibility.
### 4.4. Interface Segregation Principle (ISP) — ✅ PARTIALLY RESOLVED (was VIOLATION)
### 4.4. Interface Segregation Principle (ISP) — ✅ MOSTLY RESOLVED (was VIOLATION)
**Update (Feb 19):**
**Update (Feb 20):**
- ✅ Protocol interfaces exist for `TechniqueRepository` and `TestRepository` in `domain/ports/repositories/`.
- ✅ `ImportService` protocol in `domain/ports/import_service.py` — common contract for all data import services.
- Services expose focused functions per module (e.g., `threat_actor_service` exposes 4 functions, each for one use case).
- The `Settings` object is still monolithic but scoring weights have been extracted to a dedicated DB table with a focused service interface.
@@ -310,7 +317,7 @@ def get_technique_repository(db=Depends(get_db)) -> SATechniqueRepository: ...
| `threat_actors.py` | 312 lines | ~100 lines | `threat_actor_service.py` |
| `evidence.py` | 367 lines | ~200 lines | `evidence_service.py` |
**Remaining:** `heatmap.py` still has inline logic (~528 lines). Lower priority since it's already partially extracted to `heatmap_service`.
**Update (Feb 20):** `heatmap.py` is also now a thin adapter — all logic was already in `heatmap_service`. Additionally, `users.py`, `audit.py`, and `data_sources.py` have been extracted to `user_service`, `audit_query_service`, and `data_source_service` respectively. No remaining fat routers.
### 5.2. ~~CRITICAL RISK: In-Memory Token Blacklist~~ ✅ RESOLVED
@@ -374,7 +381,9 @@ Background jobs create sessions outside the request lifecycle. This is technical
- `domain/value_objects/` — `MitreId`, `ScoringWeights` (immutable, validated).
- ORM models remain anemic by design (persistence mapping only). Business logic lives in domain entities.
**Remaining:** Campaign, ComplianceFramework, ThreatActor still lack domain entity counterparts.
**Update (Feb 20):** `CampaignEntity` (with lifecycle state machine) and `ComplianceFrameworkEntity` / `ComplianceControlEntity` (with coverage calculation logic) have been added.
**Remaining:** ThreatActor still lacks a domain entity counterpart.
### 5.8. ~~MEDIUM RISK: No Explicit Transaction Management~~ ✅ PARTIALLY RESOLVED
@@ -661,14 +670,14 @@ class SQLAlchemyTestRepository(TestRepository):
| Weakness | Original Severity | Current Status |
|----------|----------|--------|
| Fat controllers (routers with business logic) | HIGH | ✅ Resolved — 9 routers extracted to services |
| No repository layer | HIGH | ✅ Resolved (Test, Technique repos + 9 service modules) |
| Fat controllers (routers with business logic) | HIGH | ✅ Resolved — all 21 routers now delegate to services (12 extracted) |
| No repository layer | HIGH | ✅ Resolved (Test, Technique repos + 12 service modules) |
| Services depend on FastAPI | HIGH | ✅ Resolved (domain exceptions + middleware) |
| Anemic models | MEDIUM | ✅ Partially resolved (TestEntity, TechniqueEntity) |
| Anemic models | MEDIUM | ✅ Largely resolved (TestEntity, TechniqueEntity, CampaignEntity, ComplianceFrameworkEntity) |
| In-memory token blacklist | HIGH | ✅ Resolved (Redis-backed) |
| Mutable settings at runtime | MEDIUM | ✅ Resolved (scoring_config DB table) |
| No CI/CD | MEDIUM | ✅ Resolved (GitHub Actions) |
| No dependency inversion | HIGH | ✅ Partially resolved (ports + repos + services) |
| No dependency inversion | HIGH | ✅ Mostly resolved (ports + repos + ImportService protocol + services) |
| No structured logging | LOW | ✅ Resolved (JSON logging for production) |
### Final Classification
@@ -677,34 +686,39 @@ class SQLAlchemyTestRepository(TestRepository):
┌──────────────────────────────────────────────────────────┐
│ Type: Clean Modular Monolith │
│ Maturity: Production-ready │
│ SOLID: 4/5 (SRP ✅, OCP partial, LSP n/a, │
│ ISP partial, DIP ✅ started) │
│ Testability: 7/10 (326 tests, domain unit tests, repo │
│ SOLID: 4.5/5 (SRP ✅, OCP mostly ✅, LSP n/a, │
│ ISP mostly ✅, DIP mostly ✅) │
│ Testability: 8/10 (354 tests, domain unit tests, repo │
│ integration tests, service layer tests) │
│ Coupling: 7/10 (domain decoupled, services agnostic, │
│ most routers are thin adapters) │
│ Cohesion: 8/10 (domain entities own business rules, │
│ services own query logic) │
│ Estimated remaining tech debt: ~1 week │
│ (heatmap extraction, remaining minor routers, │
│ Campaign/ComplianceFramework domain entities) │
│ Coupling: 8/10 (domain decoupled, services agnostic, │
│ all routers are thin adapters) │
│ Cohesion: 9/10 (domain entities own business rules, │
│ services own query logic, clear contracts) │
│ Estimated remaining tech debt: ~2-3 days │
│ (ThreatActor domain entity, heatmap layer extensibility)│
└──────────────────────────────────────────────────────────┘
```
### Recommendation (Updated Feb 19)
### Recommendation (Updated Feb 20)
The architectural refactoring is substantially complete. All critical and high-priority items from the original analysis are resolved:
The architectural refactoring is **complete**. All items from the original analysis — critical, high, medium, and low priority — are resolved:
**Critical / High priority:**
1. ~~Extract domain exceptions~~ ✅ Done
2. ~~Create repositories for Test and Technique~~ ✅ Done
3. ~~Move token blacklist to Redis~~ ✅ Done
4. ~~Set up basic CI/CD~~ ✅ Done
5. ~~Migrate fat routers to services~~ ✅ Done (9 routers extracted)
5. ~~Migrate fat routers to services~~ ✅ Done (12 routers extracted, all 21 now delegate)
6. ~~Persist scoring weights in database~~ ✅ Done
7. ~~Add structured JSON logging~~ ✅ Done
**Remaining low-priority items:**
1. Extract remaining logic from `heatmap.py` to `heatmap_service.py`
2. Create domain entities for Campaign and ComplianceFramework
3. Extract `users.py`, `audit.py`, `data_sources.py` to services (simple CRUD)
4. Add common interface for import services (OCP improvement)
**Low priority (completed Feb 20):**
8. ~~Extract `heatmap.py` logic~~ ✅ Already done (was a thin adapter)
9. ~~Create domain entities for Campaign and ComplianceFramework~~ ✅ Done (with lifecycle validation + coverage calculations)
10. ~~Extract `users.py`, `audit.py`, `data_sources.py` to services~~ ✅ Done
11. ~~Add common interface for import services (OCP)~~ ✅ Done (`ImportService` protocol + registry)
**Remaining nice-to-haves (not blocking):**
- ThreatActor domain entity (currently only a service exists)
- Heatmap layer extensibility (currently hardcoded endpoints)
- Full migration of all services to use Repository pattern (incremental)