Newer
Older
cortex-hub / docs / reviews / feature_review_api_security.md

Code Review Report: Feature 4 — API, Routing & Security

This report performs a deep-dive audit of the edge layer and security framework, focusing on app.py, auth.py, and sessions.py through the lens of 12-Factor App Methodology, Pythonic Code Style, and Production Hardening.


🏗️ 12-Factor App Compliance Audit

Factor Status Observation
III. Config 🟡 Warning Scattered Environment Usage: app.py (Lines 313-314) accesses environment variables like CORS_ORIGINS and HUB_PUBLIC_URL using os.getenv directly. These should be moved to app/config.py for centralized validation.
VI. Processes 🔴 Problem Concurrency Drifts: Background tasks (_periodic_mirror_cleanup) are launched per-process using asyncio.create_task. If the Hub is scaled to multiple replicas, these tasks will duplicate effort and potentially create database contention (e.g., competing to purge the same ghost mirrors).
IX. Disposability 🔴 Major Issue Abrupt Shutdown: app.state.grpc_server.stop(0) (Line 99) triggers an immediate termination of all node connections. This can interrupt active long-running agent tasks without providing nodes a chance to checkpoint or finish their current gRPC stream.
XII. Admin Success bootstrap_system_skills and bootstrap_local_admin are correctly handled during the application lifespan.

🔍 File-by-File Diagnostic

1. app/core/services/auth.py

The gateway for user identity and OIDC integration.

[!CAUTION] CRITICAL SECURITY RISK: Unverified Signatures Line 61: jwt.decode(id_token, options={"verify_signature": False}) The application decodes ID tokens without verifying their signature. This means an attacker can provide a forged JWT with a spoofed sub or email claim, and the Hub will blindly trust it and grant system access. Fix: Implement proper signature verification using the OIDC provider's JWKS (JSON Web Key Set).


2. app/app.py

The main FastAPI entry point and lifecycle manager.

Identified Problems:

  • Permissive CORS: allow_methods=["*"] and allow_headers=["*"] (Lines 322-323) provide a wide attack surface. Production environments should strictly enumerate allowed headers and methods.
  • Monolithic Initializer: create_app is becoming a "God Method" (~130 lines), manually instantiating and wiring 15+ services.
  • Fix: Moving towards a dependency injection framework or a more modular ServiceContainer bootstrap would improve testability.

3. app/api/routes/sessions.py

Handles stateful AI interactions and token status.

Identified Problems:

  • Performance Hotspot: As identified in the previous RAG audit, the session routes often perform heavy DB joins (joinedload(models.Session.messages)) on every operation. This leads to slow page loads as chat history grows.
  • Prefix Consistency: The transition to /api/v1 is implemented (Line 311), which is a major win for API versioning and Factor X compliance.

🛠️ Summary Recommendations

  1. Harden Authentication: Immediately patch auth.py to fetch OIDC public keys and enforce signature verification on all incoming ID tokens.
  2. Externalize Background Tasks: Move periodic maintenance (health checks, mirror cleanup) to a dedicated worker process (e.g., Celery or a standalone k8s CronJob) to avoid duplication across web replicas (Factor VI).
  3. Refactor Config Consolidation: Ensure all environment variable access occurs within app/config.py to provide a single, validated source of truth for the application state.
  4. Graceful Termination: Update lifespan to provide a reasonable timeout (e.g., stop(5)) for gRPC and HTTP connections to drain gracefully (Factor IX).

This concludes the initial backend code review. I have identified three CRITICAL security/architectural risks across all features (Shell Injection, Unverified JWTs, and Blocking I/O in Async loops). I am ready to assist with implementing the remediation plans for any of these findings.