fix: resolve security lint violations and critical audit findings (#19, #20, #21)

fix: resolve security lint violations and critical audit findings (#19, #20, #21)

Author: acartag7

.env.example

@@ -11,11 +11,15 @@
 # Postgres password (used by both the postgres container and the server)
 POSTGRES_PASSWORD=
 
+# Redis password (used by the redis container and embedded in REDIS_URL)
+# Generate: python -c "import secrets; print(secrets.token_urlsafe(32))"
+REDIS_PASSWORD=
+
 # Database URL (docker-compose sets this automatically; set for standalone)
 # EDICTUM_DATABASE_URL=postgresql+asyncpg://postgres:password@localhost:5432/edictum
 
 # Redis URL (docker-compose sets this automatically; set for standalone)
-# EDICTUM_REDIS_URL=redis://localhost:6379/0
+# EDICTUM_REDIS_URL=redis://:yourpassword@localhost:6379/0
 
 # Secret key for session token signing
 # Generate: python -c "import secrets; print(secrets.token_hex(32))"

.github/workflows/ci.yml

@@ -60,17 +60,14 @@ jobs:
       - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5
         with:
           python-version: "3.12"
-      - run: pip install bandit pip-audit
+      - run: pip install -e ".[dev]" bandit pip-audit
       - run: bandit -r src/ -ll -ii --exclude tests/
       - name: Audit Python dependencies
         run: pip-audit
-        continue-on-error: true
 
   custom-security-lint:
     name: Custom Security Checks
     runs-on: ubuntu-latest
-    # TODO: remove continue-on-error once issues #12-#28 are fixed
-    continue-on-error: true
     steps:
       - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
       - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5

CLAUDE.md

@@ -308,12 +308,13 @@ All prefixed with `EDICTUM_` where possible.
 | Variable | Required | Default | Purpose |
 |----------|----------|---------|---------|
 | `DATABASE_URL` | Yes | — | Postgres connection string |
-| `REDIS_URL` | Yes | — | Redis connection string |
+| `REDIS_URL` | Yes | — | Redis connection string (include password: `redis://:pass@host:6379/0`) |
+| `REDIS_PASSWORD` | Yes | — | Redis `requirepass` (used by docker-compose; embedded in `REDIS_URL`) |
 | `EDICTUM_ADMIN_EMAIL` | First run | — | Bootstrap admin user |
 | `EDICTUM_ADMIN_PASSWORD` | First run | — | Bootstrap admin password |
 | `EDICTUM_AUTH_PROVIDER` | No | `local` | Auth provider (`local`, future: `clerk`, `oidc`) |
 | `EDICTUM_BASE_URL` | No | `http://localhost:8000` | Public URL for webhooks, CORS |
-| `EDICTUM_SECRET_KEY` | Yes | — | Session token signing. No default. |
+| `EDICTUM_SECRET_KEY` | Yes | — | Session HMAC signing. Min 32 chars. No default. |
 
 ## License
 

docker-compose.yml

@@ -15,8 +15,9 @@ services:
 
   redis:
     image: redis:7-alpine
+    command: redis-server --requirepass ${REDIS_PASSWORD:?Set REDIS_PASSWORD in .env}
     healthcheck:
-      test: ["CMD", "redis-cli", "ping"]
+      test: ["CMD", "redis-cli", "-a", "${REDIS_PASSWORD}", "ping"]
       interval: 10s
       timeout: 5s
       retries: 5
@@ -33,7 +34,7 @@ services:
     env_file: .env
     environment:
       EDICTUM_DATABASE_URL: postgresql+asyncpg://postgres:${POSTGRES_PASSWORD:?}@postgres:5432/edictum
-      EDICTUM_REDIS_URL: redis://redis:6379/0
+      EDICTUM_REDIS_URL: redis://:${REDIS_PASSWORD:?}@redis:6379/0
       EDICTUM_SECRET_KEY: ${EDICTUM_SECRET_KEY:?Set EDICTUM_SECRET_KEY in .env}
       EDICTUM_ADMIN_EMAIL: ${EDICTUM_ADMIN_EMAIL:-}
       EDICTUM_ADMIN_PASSWORD: ${EDICTUM_ADMIN_PASSWORD:-}

src/edictum_server/auth/dependencies.py

@@ -30,6 +30,7 @@ class AuthContext:
     agent_id: str | None = None
     email: str | None = None
     is_admin: bool = False
+    api_key_prefix: str | None = None
 
 
 def _extract_bearer(authorization: str) -> str:
@@ -79,6 +80,7 @@ async def require_api_key(
         auth_type="api_key",
         env=api_key.env,
         agent_id=x_edictum_agent_id,
+        api_key_prefix=api_key.key_prefix,
     )
 
 

src/edictum_server/auth/local.py

@@ -3,8 +3,10 @@
 from __future__ import annotations
 
 import hashlib
+import hmac
 import json
 import secrets
+import time
 import uuid
 
 import bcrypt
@@ -15,21 +17,48 @@
 
 _COOKIE_NAME = "edictum_session"
 _SESSION_PREFIX = "session:"
+_MAX_ABSOLUTE_LIFETIME = 7 * 24 * 3600  # 7 days — no session lives beyond this
 
 
 class LocalAuthProvider(AuthProvider):
-    """Bcrypt password auth with Redis-backed sessions."""
+    """Bcrypt password auth with HMAC-signed Redis-backed sessions."""
 
     def __init__(
         self,
         redis: aioredis.Redis,
         session_ttl_hours: int = 24,
         *,
         secure_cookies: bool = False,
+        secret_key: str,
     ) -> None:
+        if not secret_key:
+            raise ValueError("secret_key must not be empty")
         self._redis = redis
         self._session_ttl = session_ttl_hours * 3600
         self._secure_cookies = secure_cookies
+        self._secret_key = secret_key
+
+    def _sign(self, data: str) -> str:
+        """HMAC-SHA256 sign session data, return 'hmac_hex:json' string."""
+        mac = hmac.new(
+            self._secret_key.encode(), data.encode(), hashlib.sha256
+        ).hexdigest()
+        return f"{mac}:{data}"
+
+    def _verify_and_parse(self, raw: str) -> dict[str, object] | None:
+        """Verify HMAC signature and return parsed session data, or None."""
+        if ":" not in raw:
+            return None
+        stored_mac, _, payload = raw.partition(":")
+        expected_mac = hmac.new(
+            self._secret_key.encode(), payload.encode(), hashlib.sha256
+        ).hexdigest()
+        if not hmac.compare_digest(stored_mac, expected_mac):
+            return None
+        try:
+            return json.loads(payload)  # type: ignore[no-any-return]
+        except (json.JSONDecodeError, ValueError):
+            return None
 
     async def authenticate(self, request: Request) -> DashboardAuthContext:
         token = request.cookies.get(_COOKIE_NAME)
@@ -44,14 +73,38 @@ async def authenticate(self, request: Request) -> DashboardAuthContext:
                 status_code=status.HTTP_401_UNAUTHORIZED,
                 detail="Session expired or invalid.",
             )
-        data = json.loads(raw)
+
+        # Verify HMAC signature — reject tampered sessions
+        raw_str = raw if isinstance(raw, str) else raw.decode()
+        data = self._verify_and_parse(raw_str)
+        if data is None:
+            # Tampered or legacy unsigned session — destroy it
+            await self._redis.delete(f"{_SESSION_PREFIX}{token}")
+            raise HTTPException(
+                status_code=status.HTTP_401_UNAUTHORIZED,
+                detail="Session expired or invalid.",
+            )
+
+        # Enforce absolute session lifetime (7 days max)
+        created_at = data.get("created_at")
+        expired = (
+            isinstance(created_at, (int, float))
+            and time.time() - created_at > _MAX_ABSOLUTE_LIFETIME
+        )
+        if expired:
+            await self._redis.delete(f"{_SESSION_PREFIX}{token}")
+            raise HTTPException(
+                status_code=status.HTTP_401_UNAUTHORIZED,
+                detail="Session expired. Please log in again.",
+            )
+
         # Slide the expiration on each successful auth
         await self._redis.expire(f"{_SESSION_PREFIX}{token}", self._session_ttl)
         return DashboardAuthContext(
-            user_id=uuid.UUID(data["user_id"]),
-            tenant_id=uuid.UUID(data["tenant_id"]),
-            email=data["email"],
-            is_admin=data["is_admin"],
+            user_id=uuid.UUID(str(data["user_id"])),
+            tenant_id=uuid.UUID(str(data["tenant_id"])),
+            email=str(data["email"]),
+            is_admin=bool(data["is_admin"]),
         )
 
     async def create_session(
@@ -68,11 +121,14 @@ async def create_session(
                 "tenant_id": str(tenant_id),
                 "email": email,
                 "is_admin": is_admin,
+                "created_at": time.time(),
             }
         )
+        # HMAC-sign session data to prevent forgery via Redis access
+        signed = self._sign(session_data)
         await self._redis.set(
             f"{_SESSION_PREFIX}{token}",
-            session_data,
+            signed,
             ex=self._session_ttl,
         )
         cookie_params: dict[str, str | bool | int] = {

src/edictum_server/config.py

@@ -93,6 +93,11 @@ def validate_required(self) -> None:
                 "EDICTUM_SECRET_KEY is required. "
                 'Generate one with: python -c "import secrets; print(secrets.token_hex(32))"'
             )
+        if len(self.secret_key) < 32:
+            raise SystemExit(
+                f"EDICTUM_SECRET_KEY must be at least 32 characters (got {len(self.secret_key)}). "
+                'Generate one with: python -c "import secrets; print(secrets.token_hex(32))"'
+            )
         if not self.database_url:
             raise SystemExit(
                 "EDICTUM_DATABASE_URL is required. "

src/edictum_server/main.py

@@ -303,6 +303,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]:
         redis=app.state.redis,
         session_ttl_hours=settings.session_ttl_hours,
         secure_cookies=settings.base_url.startswith("https://"),
+        secret_key=settings.secret_key,
     )
 
     # Push manager (SSE)

src/edictum_server/routes/approvals.py

@@ -91,7 +91,11 @@ async def create_approval(
         )
 
     env = auth.env or "production"
-    approval = await approval_service.create_approval(db, auth.tenant_id, body, env=env)
+    # Identity from auth context, not request body (CLAUDE.md rule)
+    agent_id = auth.agent_id or f"key:{auth.api_key_prefix or 'unknown'}"
+    approval = await approval_service.create_approval(
+        db, auth.tenant_id, body, env=env, agent_id=agent_id,
+    )
     await db.commit()
 
     event_data = {
@@ -166,12 +170,14 @@ async def submit_decision(
     push: PushManager = Depends(get_push_manager),
 ) -> ApprovalResponse:
     """Submit a human decision on a pending approval."""
+    # Identity from auth context, not request body (CLAUDE.md rule)
+    decided_by = auth.email or f"user:{auth.user_id}"
     approval = await approval_service.submit_decision(
         db,
         auth.tenant_id,
         approval_id,
         approved=body.approved,
-        decided_by=body.decided_by,
+        decided_by=decided_by,
         reason=body.reason,
         decided_via=body.decided_via or "console",
     )

src/edictum_server/routes/keys.py

@@ -121,7 +121,9 @@ async def revoke_key(
         )
 
     await db.execute(
-        update(ApiKey).where(ApiKey.id == key_id).values(revoked_at=datetime.now(UTC))
+        update(ApiKey)
+        .where(ApiKey.id == key_id, ApiKey.tenant_id == auth.tenant_id)
+        .values(revoked_at=datetime.now(UTC))
     )
     await db.commit()