From 6d18a5417dad0acc5dbb48c0f45ee705c3bfb369 Mon Sep 17 00:00:00 2001 From: Kitos Date: Tue, 17 Feb 2026 15:43:05 +0100 Subject: [PATCH] =?UTF-8?q?feat(phase-34):=20resolve=20blocking=20tech=20d?= =?UTF-8?q?ebt=20=E2=80=94=20Redis,=20domain=20exceptions,=20indexes,=20CI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Foundational changes required before any new feature work can begin. - 0.1 Redis infrastructure: add redis:7-alpine to docker-compose dev and prod, REDIS_URL config, singleton client in app/infrastructure/redis_client.py - 0.2 Token blacklist on Redis SEC-001: replace in-memory dict with Redis SETEX keyed by jti, auto-expiring TTL derived from token exp - 0.3 Database indexes SR-006: Alembic migration b019 with 5 composite indexes for scoring, MTTD/MTTR, remediation, and notification queries - 0.4 Domain exceptions TD-003: app/domain/exceptions.py with typed errors, error_handler middleware mapping them to HTTP, services decoupled from FastAPI - 0.5 Fix silenced exceptions TD-007: replace 4 bare except-pass blocks in test_workflow_service with logger.warning with exc_info - 0.6 CI pipeline TD-009: GitHub Actions workflow with Postgres and Redis service containers, ruff lint, pytest; ruff.toml for baseline config --- .github/workflows/ci.yml | 64 ++++++++++++++++ .../versions/b019_add_composite_indexes.py | 67 +++++++++++++++++ backend/app/auth.py | 57 +++++++++------ backend/app/config.py | 3 + backend/app/domain/__init__.py | 0 backend/app/domain/exceptions.py | 67 +++++++++++++++++ backend/app/infrastructure/__init__.py | 0 backend/app/infrastructure/redis_client.py | 34 +++++++++ backend/app/main.py | 5 ++ backend/app/middleware/__init__.py | 0 backend/app/middleware/error_handler.py | 43 +++++++++++ backend/app/routers/auth.py | 3 +- backend/app/services/campaign_service.py | 16 ++-- backend/app/services/test_workflow_service.py | 73 +++++++------------ backend/requirements.txt | 1 + backend/ruff.toml | 13 ++++ backend/tests/test_campaigns_and_snapshots.py | 6 +- backend/tests/test_t106_workflow_service.py | 21 ++++-- backend/tests/test_workflow.py | 72 +++++++++--------- docker-compose.prod.yml | 21 ++++++ docker-compose.yml | 22 ++++++ 21 files changed, 464 insertions(+), 124 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 backend/alembic/versions/b019_add_composite_indexes.py create mode 100644 backend/app/domain/__init__.py create mode 100644 backend/app/domain/exceptions.py create mode 100644 backend/app/infrastructure/__init__.py create mode 100644 backend/app/infrastructure/redis_client.py create mode 100644 backend/app/middleware/__init__.py create mode 100644 backend/app/middleware/error_handler.py create mode 100644 backend/ruff.toml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..0f990ae --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,64 @@ +name: Aegis CI + +on: + push: + branches: [main, develop] + pull_request: + branches: [main] + +jobs: + lint-and-test: + runs-on: ubuntu-latest + + services: + postgres: + image: postgres:15-alpine + env: + POSTGRES_DB: testdb + POSTGRES_USER: test + POSTGRES_PASSWORD: test + ports: + - 5432:5432 + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + redis: + image: redis:7-alpine + ports: + - 6379:6379 + options: >- + --health-cmd "redis-cli ping" + --health-interval 10s + --health-timeout 5s + --health-retries 5 + + defaults: + run: + working-directory: backend + + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: "3.11" + cache: pip + cache-dependency-path: backend/requirements.txt + + - name: Install dependencies + run: | + pip install --upgrade pip + pip install -r requirements.txt + pip install ruff + + - name: Lint + run: ruff check app/ + + - name: Test + env: + DATABASE_URL: postgresql://test:test@localhost:5432/testdb + REDIS_URL: redis://localhost:6379/0 + SECRET_KEY: ci-test-secret-key-not-for-production + run: pytest tests/ -v --tb=short diff --git a/backend/alembic/versions/b019_add_composite_indexes.py b/backend/alembic/versions/b019_add_composite_indexes.py new file mode 100644 index 0000000..aa6a9e4 --- /dev/null +++ b/backend/alembic/versions/b019_add_composite_indexes.py @@ -0,0 +1,67 @@ +"""add_composite_indexes + +Additional composite indexes for scoring, heatmap, metrics, reports, +MTTD/MTTR calculations, and notification queries. + +Revision ID: b019composite +Revises: b018perfidx +Create Date: 2026-02-17 14:00:00.000000 + +""" +from typing import Sequence, Union + +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "b019composite" +down_revision: Union[str, None] = "b018perfidx" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # ── Tests ──────────────────────────────────────────────────────── + # Used by scoring queries that filter by state + validation date + op.create_index( + "ix_tests_state_red_validated_at", + "tests", + ["state", "red_validated_at"], + ) + + # Used by remediation dashboard and metrics + op.create_index( + "ix_tests_remediation_status", + "tests", + ["remediation_status"], + ) + + # ── Audit logs ─────────────────────────────────────────────────── + # Three-column index for MTTD/MTTR queries that filter by entity + action + op.create_index( + "ix_audit_logs_entity_type_entity_id_action", + "audit_logs", + ["entity_type", "entity_id", "action"], + ) + + # Used for per-user audit trail queries + op.create_index( + "ix_audit_logs_user_id", + "audit_logs", + ["user_id"], + ) + + # ── Notifications ──────────────────────────────────────────────── + # Used by "unread notifications" badge and inbox queries + op.create_index( + "ix_notifications_user_id_read", + "notifications", + ["user_id", "read"], + ) + + +def downgrade() -> None: + op.drop_index("ix_notifications_user_id_read", table_name="notifications") + op.drop_index("ix_audit_logs_user_id", table_name="audit_logs") + op.drop_index("ix_audit_logs_entity_type_entity_id_action", table_name="audit_logs") + op.drop_index("ix_tests_remediation_status", table_name="tests") + op.drop_index("ix_tests_state_red_validated_at", table_name="tests") diff --git a/backend/app/auth.py b/backend/app/auth.py index 97a5a8d..92421d2 100644 --- a/backend/app/auth.py +++ b/backend/app/auth.py @@ -4,12 +4,12 @@ Security utilities: password hashing and JWT token management. This module provides pure functions for: - Hashing and verifying passwords using bcrypt via passlib. - Creating JWT access tokens using python-jose. -- Managing an in-memory token blacklist for revocation. +- Managing a Redis-backed token blacklist for revocation. No endpoints are defined here. """ -import threading +import logging import uuid as _uuid from datetime import datetime, timedelta, timezone @@ -18,6 +18,8 @@ from passlib.context import CryptContext from app.config import settings +logger = logging.getLogger(__name__) + # --------------------------------------------------------------------------- # Password hashing # --------------------------------------------------------------------------- @@ -58,36 +60,43 @@ def create_access_token(data: dict) -> str: # --------------------------------------------------------------------------- -# Token blacklist (in-memory) +# Token blacklist (Redis-backed) # --------------------------------------------------------------------------- -# Stores (jti, expiry_timestamp) tuples. Entries are automatically purged -# once they are past their original expiry (the token would be invalid -# anyway at that point). Thread-safe via a simple lock. +# Each revoked token's ``jti`` is stored in Redis with a TTL equal to the +# token's remaining lifetime. This means entries auto-expire exactly when +# the token would have become invalid anyway — no manual cleanup needed. # -# For multi-worker / multi-process deployments, consider replacing this -# with a shared store like Redis. +# Redis survives backend restarts, so blacklisted tokens stay revoked +# across deploys and multi-worker setups. # --------------------------------------------------------------------------- -_blacklist: dict[str, float] = {} # jti → expiry epoch -_blacklist_lock = threading.Lock() +_BLACKLIST_PREFIX = "blacklist:" def blacklist_token(jti: str, exp: float) -> None: - """Add *jti* to the blacklist until it naturally expires at *exp*.""" - with _blacklist_lock: - _blacklist[jti] = exp - _cleanup_blacklist() + """Add *jti* to the Redis blacklist with a TTL derived from *exp*. + + *exp* is the token's ``exp`` claim (epoch timestamp). The TTL is set + to ``exp - now`` so the key vanishes when the token would have expired + naturally. + """ + from app.infrastructure.redis_client import get_redis + + ttl = max(int(exp - datetime.now(timezone.utc).timestamp()), 1) + try: + r = get_redis() + r.setex(f"{_BLACKLIST_PREFIX}{jti}", ttl, "1") + except Exception: + logger.warning("Failed to blacklist token %s in Redis", jti, exc_info=True) def is_token_blacklisted(jti: str) -> bool: - """Return ``True`` if *jti* has been revoked.""" - with _blacklist_lock: - return jti in _blacklist + """Return ``True`` if *jti* has been revoked (exists in Redis).""" + from app.infrastructure.redis_client import get_redis - -def _cleanup_blacklist() -> None: - """Remove entries whose tokens have already expired (caller holds lock).""" - now = datetime.now(timezone.utc).timestamp() - expired = [k for k, exp in _blacklist.items() if exp < now] - for k in expired: - del _blacklist[k] + try: + r = get_redis() + return r.exists(f"{_BLACKLIST_PREFIX}{jti}") > 0 + except Exception: + logger.warning("Failed to check blacklist for %s in Redis", jti, exc_info=True) + return False diff --git a/backend/app/config.py b/backend/app/config.py index 8883fee..6552510 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -24,6 +24,9 @@ class Settings(BaseSettings): ALGORITHM: str = "HS256" ACCESS_TOKEN_EXPIRE_MINUTES: int = 15 # short-lived for security; configurable via env + # ── Redis ───────────────────────────────────────────────────────── + REDIS_URL: str = "redis://redis:6379/0" + # ── CORS ───────────────────────────────────────────────────────── # Comma-separated list of allowed origins, or a JSON array. # In dev this defaults to common local ports; in production set it diff --git a/backend/app/domain/__init__.py b/backend/app/domain/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/backend/app/domain/exceptions.py b/backend/app/domain/exceptions.py new file mode 100644 index 0000000..f37c04a --- /dev/null +++ b/backend/app/domain/exceptions.py @@ -0,0 +1,67 @@ +"""Domain exceptions for Aegis business logic. + +These exceptions are raised by service-layer code and automatically +mapped to HTTP responses by the error-handler middleware registered +in ``app.main``. This keeps the service layer free from any HTTP +or framework coupling. +""" + + +class DomainException(Exception): + """Base for all domain exceptions.""" + + def __init__(self, message: str, code: str = "DOMAIN_ERROR"): + self.message = message + self.code = code + super().__init__(message) + + +class EntityNotFoundError(DomainException): + """Raised when a requested entity does not exist.""" + + def __init__(self, entity: str, identifier: str): + super().__init__(f"{entity} not found: {identifier}", "NOT_FOUND") + self.entity = entity + self.identifier = identifier + + +class DuplicateEntityError(DomainException): + """Raised when creating an entity that already exists.""" + + def __init__(self, entity: str, field: str, value: str): + super().__init__( + f"{entity} with {field}='{value}' already exists", + "DUPLICATE", + ) + + +class InvalidTransitionError(DomainException): + """Raised when a state-machine transition is not allowed.""" + + def __init__( + self, + current_state: str, + target_state: str, + valid_transitions: list[str] | None = None, + ): + msg = f"Cannot transition from '{current_state}' to '{target_state}'" + if valid_transitions: + msg += f". Valid transitions: {valid_transitions}" + super().__init__(msg, "INVALID_TRANSITION") + self.current_state = current_state + self.target_state = target_state + self.valid_transitions = valid_transitions or [] + + +class InvalidOperationError(DomainException): + """Raised when an operation is invalid in the current context.""" + + def __init__(self, message: str): + super().__init__(message, "INVALID_OPERATION") + + +class AuthorizationError(DomainException): + """Raised when the user lacks permissions for an action.""" + + def __init__(self, message: str = "Insufficient permissions"): + super().__init__(message, "FORBIDDEN") diff --git a/backend/app/infrastructure/__init__.py b/backend/app/infrastructure/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/backend/app/infrastructure/redis_client.py b/backend/app/infrastructure/redis_client.py new file mode 100644 index 0000000..cde4e21 --- /dev/null +++ b/backend/app/infrastructure/redis_client.py @@ -0,0 +1,34 @@ +"""Redis client singleton. + +Provides a lazily-initialised Redis connection that is reused across +the application. The connection URL is read from ``settings.REDIS_URL``. + +Usage:: + + from app.infrastructure.redis_client import get_redis + + r = get_redis() + r.set("key", "value", ex=300) +""" + +import logging + +import redis + +from app.config import settings + +logger = logging.getLogger(__name__) + +_redis_client: redis.Redis | None = None + + +def get_redis() -> redis.Redis: + """Return a shared Redis client, creating it on first call.""" + global _redis_client + if _redis_client is None: + _redis_client = redis.from_url( + settings.REDIS_URL, + decode_responses=True, + ) + logger.info("Redis client connected to %s", settings.REDIS_URL) + return _redis_client diff --git a/backend/app/main.py b/backend/app/main.py index fc346db..ea2c659 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -32,6 +32,8 @@ from app.routers import scores as scores_router from app.routers import operational_metrics as operational_metrics_router from app.routers import compliance as compliance_router from app.routers import snapshots as snapshots_router +from app.domain.exceptions import DomainException +from app.middleware.error_handler import domain_exception_handler from app.storage import ensure_bucket_exists from app.jobs.mitre_sync_job import start_scheduler, scheduler @@ -68,6 +70,9 @@ limiter = Limiter(key_func=get_remote_address) app.state.limiter = limiter app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler) +# ── Domain exception → HTTP mapping ────────────────────────────────────── +app.add_exception_handler(DomainException, domain_exception_handler) + # ── CORS ────────────────────────────────────────────────────────────────── from app.config import settings as _settings diff --git a/backend/app/middleware/__init__.py b/backend/app/middleware/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/backend/app/middleware/error_handler.py b/backend/app/middleware/error_handler.py new file mode 100644 index 0000000..ecd62d6 --- /dev/null +++ b/backend/app/middleware/error_handler.py @@ -0,0 +1,43 @@ +"""Domain exception → HTTP response mapping. + +This module provides a single exception handler that converts +domain-layer exceptions into structured JSON responses, keeping +the service layer free from FastAPI's ``HTTPException``. +""" + +from fastapi import Request +from fastapi.responses import JSONResponse + +from app.domain.exceptions import ( + AuthorizationError, + DomainException, + DuplicateEntityError, + EntityNotFoundError, + InvalidOperationError, + InvalidTransitionError, +) + +EXCEPTION_STATUS_MAP: dict[type[DomainException], int] = { + EntityNotFoundError: 404, + DuplicateEntityError: 409, + InvalidTransitionError: 400, + InvalidOperationError: 400, + AuthorizationError: 403, +} + + +async def domain_exception_handler( + request: Request, + exc: DomainException, +) -> JSONResponse: + """Convert a :class:`DomainException` into a JSON error response.""" + status_code = EXCEPTION_STATUS_MAP.get(type(exc), 400) + + content: dict = {"detail": exc.message, "code": exc.code} + + if isinstance(exc, InvalidTransitionError): + content["current_state"] = exc.current_state + content["target_state"] = exc.target_state + content["valid_transitions"] = exc.valid_transitions + + return JSONResponse(status_code=status_code, content=content) diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 1306030..9fddad8 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -98,8 +98,9 @@ def logout( ): """Clear the authentication cookie and revoke the current token. - The token's ``jti`` is added to an in-memory blacklist so it cannot + The token's ``jti`` is added to the Redis blacklist so it cannot be reused even if the cookie has already been copied elsewhere. + The blacklist entry auto-expires when the token's ``exp`` is reached. """ # Attempt to blacklist the token's jti token = aegis_token or request.headers.get("Authorization", "").removeprefix("Bearer ").strip() diff --git a/backend/app/services/campaign_service.py b/backend/app/services/campaign_service.py index 963dec2..cfb4b02 100644 --- a/backend/app/services/campaign_service.py +++ b/backend/app/services/campaign_service.py @@ -8,9 +8,9 @@ import logging import uuid from datetime import datetime -from fastapi import HTTPException from sqlalchemy.orm import Session +from app.domain.exceptions import EntityNotFoundError, InvalidOperationError from app.models.campaign import Campaign, CampaignTest, KILL_CHAIN_PHASES from app.models.test import Test from app.models.test_template import TestTemplate @@ -49,7 +49,7 @@ def validate_no_circular_dependency( ) -> None: """Walk the depends_on chain and verify no cycle is formed. - Raises HTTPException(400) if a circular dependency is detected. + Raises :class:`InvalidOperationError` if a circular dependency is detected. """ if depends_on_id is None: return @@ -59,9 +59,8 @@ def validate_no_circular_dependency( while current is not None: if current in visited or current == test_id: - raise HTTPException( - status_code=400, - detail="Circular dependency detected in campaign test chain", + raise InvalidOperationError( + "Circular dependency detected in campaign test chain" ) visited.add(current) parent = db.query(CampaignTest).filter_by(id=current).first() @@ -119,7 +118,7 @@ def generate_campaign_from_threat_actor( """ actor = db.query(ThreatActor).filter(ThreatActor.id == actor_id).first() if not actor: - raise HTTPException(status_code=404, detail="Threat actor not found") + raise EntityNotFoundError("ThreatActor", str(actor_id)) # Get unvalidated techniques for this actor gap_techniques = ( @@ -132,9 +131,8 @@ def generate_campaign_from_threat_actor( ) if not gap_techniques: - raise HTTPException( - status_code=400, - detail=f"No uncovered techniques found for {actor.name}", + raise InvalidOperationError( + f"No uncovered techniques found for {actor.name}" ) # Create the campaign diff --git a/backend/app/services/test_workflow_service.py b/backend/app/services/test_workflow_service.py index d76d852..ea0b07e 100644 --- a/backend/app/services/test_workflow_service.py +++ b/backend/app/services/test_workflow_service.py @@ -11,18 +11,21 @@ Every public function validates the transition, mutates the test, writes an audit-log entry, and commits the session. """ +import logging from datetime import datetime -from fastapi import HTTPException, status from sqlalchemy.orm import Session from app.config import settings +from app.domain.exceptions import InvalidOperationError, InvalidTransitionError from app.models.enums import TestState from app.models.test import Test from app.models.user import User from app.services.audit_service import log_action from app.services.notification_service import notify_test_state_change, create_notification +logger = logging.getLogger(__name__) + # --------------------------------------------------------------------------- # Valid transition map # --------------------------------------------------------------------------- @@ -59,23 +62,15 @@ def transition_state( ) -> Test: """Validate and perform a state transition, log it, and commit. - Raises :class:`~fastapi.HTTPException` 400 when the transition is invalid. + Raises :class:`InvalidTransitionError` when the transition is invalid. """ if not can_transition(test, target_state): current = test.state if isinstance(test.state, TestState) else TestState(test.state) valid = [s.value for s in VALID_TRANSITIONS.get(current, [])] - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail={ - "message": ( - f"Cannot transition from '{current.value}' to '{target_state.value}'. " - f"Valid transitions: {valid}" - ), - "code": "INVALID_TRANSITION", - "current_state": current.value, - "target_state": target_state.value, - "valid_transitions": valid, - }, + raise InvalidTransitionError( + current_state=current.value, + target_state=target_state.value, + valid_transitions=valid, ) previous_state = test.state.value if isinstance(test.state, TestState) else test.state @@ -103,8 +98,8 @@ def transition_state( # Dispatch in-app notifications for the new state try: notify_test_state_change(db, test, target_state.value) - except Exception: - pass # Notifications are best-effort — don't block the workflow + except Exception as e: + logger.warning("Notification failed for test %s: %s", test.id, e, exc_info=True) return test @@ -169,22 +164,13 @@ def validate_as_red_lead( """ current = test.state.value if isinstance(test.state, TestState) else test.state if test.state not in (TestState.in_review,): - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail={ - "message": f"Cannot validate red side while test is in '{current}' state (must be in_review)", - "code": "INVALID_STATE", - "current_state": current, - }, + raise InvalidOperationError( + f"Cannot validate red side while test is in '{current}' state (must be in_review)" ) if validation_status not in ("approved", "rejected"): - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail={ - "message": "validation_status must be 'approved' or 'rejected'", - "code": "INVALID_VALIDATION_STATUS", - }, + raise InvalidOperationError( + "validation_status must be 'approved' or 'rejected'" ) now = datetime.utcnow() @@ -225,22 +211,13 @@ def validate_as_blue_lead( """ current = test.state.value if isinstance(test.state, TestState) else test.state if test.state not in (TestState.in_review,): - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail={ - "message": f"Cannot validate blue side while test is in '{current}' state (must be in_review)", - "code": "INVALID_STATE", - "current_state": current, - }, + raise InvalidOperationError( + f"Cannot validate blue side while test is in '{current}' state (must be in_review)" ) if validation_status not in ("approved", "rejected"): - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail={ - "message": "validation_status must be 'approved' or 'rejected'", - "code": "INVALID_VALIDATION_STATUS", - }, + raise InvalidOperationError( + "validation_status must be 'approved' or 'rejected'" ) now = datetime.utcnow() @@ -283,8 +260,8 @@ def check_dual_validation(db: Session, test: Test) -> Test: db.commit() try: notify_test_state_change(db, test, "rejected") - except Exception: - pass + except Exception as e: + logger.warning("Notification failed for test %s (rejected): %s", test.id, e, exc_info=True) elif red_status == "approved" and blue_status == "approved": test.state = TestState.validated db.commit() @@ -292,12 +269,12 @@ def check_dual_validation(db: Session, test: Test) -> Test: try: from app.services.score_cache import invalidate invalidate() - except Exception: - pass + except Exception as e: + logger.warning("Score cache invalidation failed: %s", e, exc_info=True) try: notify_test_state_change(db, test, "validated") - except Exception: - pass + except Exception as e: + logger.warning("Notification failed for test %s (validated): %s", test.id, e, exc_info=True) else: # One side hasn't voted yet — stay in_review, just flush db.commit() diff --git a/backend/requirements.txt b/backend/requirements.txt index 23ccb02..590c014 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -17,6 +17,7 @@ python-multipart pydantic-settings slowapi defusedxml +redis>=5.0.0 # Testing pytest diff --git a/backend/ruff.toml b/backend/ruff.toml new file mode 100644 index 0000000..a3bdc3c --- /dev/null +++ b/backend/ruff.toml @@ -0,0 +1,13 @@ +[lint] +# Ignore rules that have widespread pre-existing violations. +# These can be cleaned up incrementally in follow-up PRs. +ignore = [ + "E402", # module-level import not at top of file (app.main, some services) + "E712", # == True comparisons (required by SQLAlchemy filter syntax) + "F401", # unused imports (widespread; clean up incrementally) + "F841", # unused local variables (a few occurrences) +] + +[lint.per-file-ignores] +# Test files may use broad exception catching and unusual import patterns +"tests/**" = ["E", "F"] diff --git a/backend/tests/test_campaigns_and_snapshots.py b/backend/tests/test_campaigns_and_snapshots.py index 72432a8..421f43e 100644 --- a/backend/tests/test_campaigns_and_snapshots.py +++ b/backend/tests/test_campaigns_and_snapshots.py @@ -142,7 +142,7 @@ class TestCampaigns: def test_circular_dependency_prevention(self, db, campaign_with_tests): """Intentar crear dependencia circular en campaign_tests falla.""" - from fastapi import HTTPException + from app.domain.exceptions import InvalidOperationError campaign = campaign_with_tests["campaign"] cts = ( @@ -157,11 +157,11 @@ class TestCampaigns: db.commit() # Try to create B -> A (circular) - with pytest.raises(HTTPException) as exc_info: + with pytest.raises(InvalidOperationError) as exc_info: validate_no_circular_dependency( db, campaign.id, cts[0].id, cts[1].id ) - assert exc_info.value.status_code == 400 + assert exc_info.value.code == "INVALID_OPERATION" def test_campaign_scheduling_next_run(self): """next_run_at se calcula correctamente para weekly/monthly/quarterly.""" diff --git a/backend/tests/test_t106_workflow_service.py b/backend/tests/test_t106_workflow_service.py index 10e9749..a6e9d1c 100644 --- a/backend/tests/test_t106_workflow_service.py +++ b/backend/tests/test_t106_workflow_service.py @@ -43,10 +43,19 @@ class _FakeSettings: SECRET_KEY = "test" ALGORITHM = "HS256" ACCESS_TOKEN_EXPIRE_MINUTES = 60 + REDIS_URL = "redis://localhost:6379/0" MINIO_ENDPOINT = "localhost:9000" MINIO_ACCESS_KEY = "test" MINIO_SECRET_KEY = "test" MINIO_BUCKET = "test" + MINIO_SECURE = False + MAX_RETEST_COUNT = 3 + CORS_ORIGINS = "http://localhost:3000" + SCORING_WEIGHT_TESTS = 40 + SCORING_WEIGHT_DETECTION_RULES = 20 + SCORING_WEIGHT_D3FEND = 15 + SCORING_WEIGHT_FRESHNESS = 15 + SCORING_WEIGHT_PLATFORM_DIVERSITY = 10 config_mod.settings = _FakeSettings() @@ -105,8 +114,8 @@ from app.services.test_workflow_service import ( reopen_test, ) -# We also need HTTPException for assertions -from fastapi import HTTPException +# We need the domain exceptions for assertions +from app.domain.exceptions import InvalidTransitionError # --------------------------------------------------------------------------- # Helpers @@ -175,9 +184,11 @@ def test_draft_to_validated_fails(mock_log): try: transition_state(db, test, TestState.validated, user) - assert False, "Should have raised HTTPException" - except HTTPException as exc: - assert exc.status_code == 400 + assert False, "Should have raised InvalidTransitionError" + except InvalidTransitionError as exc: + assert exc.code == "INVALID_TRANSITION" + assert exc.current_state == "draft" + assert exc.target_state == "validated" print(" [PASS] Transition draft -> validated correctly fails") diff --git a/backend/tests/test_workflow.py b/backend/tests/test_workflow.py index c4d8729..ba3fe64 100644 --- a/backend/tests/test_workflow.py +++ b/backend/tests/test_workflow.py @@ -37,10 +37,13 @@ if "app.config" not in sys.modules: SECRET_KEY = "test" ALGORITHM = "HS256" ACCESS_TOKEN_EXPIRE_MINUTES = 60 + REDIS_URL = "redis://localhost:6379/0" MINIO_ENDPOINT = "localhost:9000" MINIO_ACCESS_KEY = "test" MINIO_SECRET_KEY = "test" MINIO_BUCKET = "test" + MINIO_SECURE = False + MAX_RETEST_COUNT = 3 _cfg.settings = _FakeSettings() sys.modules["app.config"] = _cfg @@ -72,6 +75,7 @@ for _mod in [ # --------------------------------------------------------------------------- from fastapi import HTTPException +from app.domain.exceptions import InvalidOperationError, InvalidTransitionError from app.models.enums import TestState, TestResult from app.services.test_workflow_service import ( VALID_TRANSITIONS, @@ -208,7 +212,7 @@ def test_rejection_and_reopen(mock_log): @patch("app.services.test_workflow_service.log_action") def test_invalid_transitions(mock_log): - """Verify that invalid state transitions raise HTTPException.""" + """Verify that invalid state transitions raise InvalidTransitionError.""" db = _make_db() user = _make_user("admin") @@ -216,41 +220,41 @@ def test_invalid_transitions(mock_log): test = _make_test(TestState.draft) try: transition_state(db, test, TestState.validated, user) - assert False, "Should have raised HTTPException" - except HTTPException as exc: - assert exc.status_code == 400 + assert False, "Should have raised InvalidTransitionError" + except InvalidTransitionError as exc: + assert exc.code == "INVALID_TRANSITION" # draft -> blue_evaluating (should fail) test = _make_test(TestState.draft) try: transition_state(db, test, TestState.blue_evaluating, user) - assert False, "Should have raised HTTPException" - except HTTPException as exc: - assert exc.status_code == 400 + assert False, "Should have raised InvalidTransitionError" + except InvalidTransitionError as exc: + assert exc.code == "INVALID_TRANSITION" # red_executing -> in_review (should fail, must go through blue_evaluating) test = _make_test(TestState.red_executing) try: transition_state(db, test, TestState.in_review, user) - assert False, "Should have raised HTTPException" - except HTTPException as exc: - assert exc.status_code == 400 + assert False, "Should have raised InvalidTransitionError" + except InvalidTransitionError as exc: + assert exc.code == "INVALID_TRANSITION" # validated -> anything (terminal state) test = _make_test(TestState.validated) try: transition_state(db, test, TestState.draft, user) - assert False, "Should have raised HTTPException" - except HTTPException as exc: - assert exc.status_code == 400 + assert False, "Should have raised InvalidTransitionError" + except InvalidTransitionError as exc: + assert exc.code == "INVALID_TRANSITION" # rejected -> red_executing (must go through draft first) test = _make_test(TestState.rejected) try: transition_state(db, test, TestState.red_executing, user) - assert False, "Should have raised HTTPException" - except HTTPException as exc: - assert exc.status_code == 400 + assert False, "Should have raised InvalidTransitionError" + except InvalidTransitionError as exc: + assert exc.code == "INVALID_TRANSITION" # =========================================================================== @@ -268,17 +272,17 @@ def test_red_tech_cannot_access_blue_phase(mock_log): test = _make_test(TestState.red_executing) try: submit_blue_evidence(db, test, red_tech) - assert False, "Should have raised HTTPException" - except HTTPException as exc: - assert exc.status_code == 400 + assert False, "Should have raised InvalidTransitionError" + except InvalidTransitionError as exc: + assert exc.code == "INVALID_TRANSITION" # Red tech cannot validate (test must be in blue_evaluating for submit_blue) test2 = _make_test(TestState.draft) try: submit_blue_evidence(db, test2, red_tech) - assert False, "Should have raised HTTPException" - except HTTPException as exc: - assert exc.status_code == 400 + assert False, "Should have raised InvalidTransitionError" + except InvalidTransitionError as exc: + assert exc.code == "INVALID_TRANSITION" # =========================================================================== @@ -298,17 +302,17 @@ def test_blue_tech_cannot_access_red_phase(mock_log): test = _make_test(TestState.blue_evaluating) try: start_execution(db, test, blue_tech) - assert False, "Should have raised HTTPException" - except HTTPException as exc: - assert exc.status_code == 400 + assert False, "Should have raised InvalidTransitionError" + except InvalidTransitionError as exc: + assert exc.code == "INVALID_TRANSITION" # Blue tech cannot submit red evidence on a draft test test2 = _make_test(TestState.draft) try: submit_red_evidence(db, test2, blue_tech) - assert False, "Should have raised HTTPException" - except HTTPException as exc: - assert exc.status_code == 400 + assert False, "Should have raised InvalidTransitionError" + except InvalidTransitionError as exc: + assert exc.code == "INVALID_TRANSITION" # =========================================================================== @@ -509,15 +513,15 @@ def test_cannot_validate_outside_in_review(mock_log): try: validate_as_red_lead(db, test, red_lead, "approved", "OK") assert False, f"Red Lead should not validate in {state.value}" - except HTTPException as exc: - assert exc.status_code == 400 + except InvalidOperationError as exc: + assert exc.code == "INVALID_OPERATION" test2 = _make_test(state) try: validate_as_blue_lead(db, test2, blue_lead, "approved", "OK") assert False, f"Blue Lead should not validate in {state.value}" - except HTTPException as exc: - assert exc.status_code == 400 + except InvalidOperationError as exc: + assert exc.code == "INVALID_OPERATION" # =========================================================================== @@ -536,8 +540,8 @@ def test_cannot_reopen_non_rejected_test(mock_log): try: reopen_test(db, test, user) assert False, f"Should not reopen from {state.value}" - except HTTPException as exc: - assert exc.status_code == 400 + except InvalidTransitionError as exc: + assert exc.code == "INVALID_TRANSITION" # --------------------------------------------------------------------------- diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 3bde8b1..93584b1 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -47,6 +47,22 @@ services: networks: - aegis-network + # ── Redis ────────────────────────────────────────────────────────────────── + redis: + image: redis:7-alpine + container_name: aegis-redis + command: redis-server --appendonly yes --maxmemory 256mb --maxmemory-policy allkeys-lru + volumes: + - redis_data:/data + healthcheck: + test: ["CMD", "redis-cli", "ping"] + interval: 5s + timeout: 3s + retries: 5 + restart: always + networks: + - aegis-network + # ── FastAPI Backend ──────────────────────────────────────────────────────── backend: build: @@ -63,6 +79,7 @@ services: MINIO_SECRET_KEY: ${MINIO_SECRET_KEY:-minioadmin} MINIO_BUCKET: ${MINIO_BUCKET:-evidence} MINIO_SECURE: ${MINIO_SECURE:-false} + REDIS_URL: redis://redis:6379/0 CORS_ORIGINS: ${CORS_ORIGINS:-} AEGIS_ENV: ${AEGIS_ENV:-production} ADMIN_USERNAME: ${ADMIN_USERNAME:-admin} @@ -70,6 +87,8 @@ services: depends_on: postgres: condition: service_healthy + redis: + condition: service_healthy minio: condition: service_started command: sh /app/entrypoint.prod.sh @@ -108,3 +127,5 @@ volumes: name: aegis_postgres_data_prod minio_data: name: aegis_minio_data_prod + redis_data: + name: aegis_redis_data_prod diff --git a/docker-compose.yml b/docker-compose.yml index a7f7fb3..eae3e04 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -55,6 +55,22 @@ services: retries: 5 restart: unless-stopped + # ── Redis ────────────────────────────────────────────────────────────────── + redis: + image: redis:7-alpine + container_name: aegis-redis + command: redis-server --appendonly yes --maxmemory 256mb --maxmemory-policy allkeys-lru + ports: + - "6379:6379" + volumes: + - redis_data:/data + healthcheck: + test: ["CMD", "redis-cli", "ping"] + interval: 5s + timeout: 3s + retries: 5 + restart: unless-stopped + # ── FastAPI Backend ──────────────────────────────────────────────────────── backend: build: @@ -71,6 +87,8 @@ services: # Set it explicitly if you need persistent sessions across restarts. ALGORITHM: HS256 ACCESS_TOKEN_EXPIRE_MINUTES: 60 + # Redis + REDIS_URL: redis://redis:6379/0 # MinIO MINIO_ENDPOINT: minio:9000 MINIO_ACCESS_KEY: minioadmin @@ -80,6 +98,8 @@ services: depends_on: postgres: condition: service_healthy + redis: + condition: service_healthy minio: condition: service_started volumes: @@ -117,3 +137,5 @@ volumes: name: aegis_postgres_data minio_data: name: aegis_minio_data + redis_data: + name: aegis_redis_data