feat(phase-34): resolve blocking tech debt — Redis, domain exceptions, indexes, CI
Some checks failed
Aegis CI / lint-and-test (push) Has been cancelled
Some checks failed
Aegis CI / lint-and-test (push) Has been cancelled
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
This commit is contained in:
67
backend/alembic/versions/b019_add_composite_indexes.py
Normal file
67
backend/alembic/versions/b019_add_composite_indexes.py
Normal file
@@ -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")
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
0
backend/app/domain/__init__.py
Normal file
0
backend/app/domain/__init__.py
Normal file
67
backend/app/domain/exceptions.py
Normal file
67
backend/app/domain/exceptions.py
Normal file
@@ -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")
|
||||
0
backend/app/infrastructure/__init__.py
Normal file
0
backend/app/infrastructure/__init__.py
Normal file
34
backend/app/infrastructure/redis_client.py
Normal file
34
backend/app/infrastructure/redis_client.py
Normal file
@@ -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
|
||||
@@ -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
|
||||
|
||||
|
||||
0
backend/app/middleware/__init__.py
Normal file
0
backend/app/middleware/__init__.py
Normal file
43
backend/app/middleware/error_handler.py
Normal file
43
backend/app/middleware/error_handler.py
Normal file
@@ -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)
|
||||
@@ -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()
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -17,6 +17,7 @@ python-multipart
|
||||
pydantic-settings
|
||||
slowapi
|
||||
defusedxml
|
||||
redis>=5.0.0
|
||||
|
||||
# Testing
|
||||
pytest
|
||||
|
||||
13
backend/ruff.toml
Normal file
13
backend/ruff.toml
Normal file
@@ -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"]
|
||||
@@ -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."""
|
||||
|
||||
@@ -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")
|
||||
|
||||
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user