Skip to content

Commit e7f9d01

Browse files
frankbriaTest User
andauthored
feat(security): add API rate limiting with slowapi (#327)
* feat(security): add API rate limiting with slowapi Add comprehensive rate limiting to protect API endpoints from abuse: - Add slowapi dependency for FastAPI-compatible rate limiting - Create RateLimitConfig with environment variable support: - RATE_LIMIT_ENABLED: Enable/disable rate limiting (default: true) - RATE_LIMIT_AUTH: Auth endpoints limit (default: 10/minute) - RATE_LIMIT_STANDARD: Standard API limit (default: 100/minute) - RATE_LIMIT_AI: AI operations limit (default: 20/minute) - RATE_LIMIT_STORAGE: memory or redis (default: memory) - Create rate limiter middleware with: - Key extraction from user ID (authenticated) or IP (anonymous) - X-Forwarded-For/X-Real-IP header support for proxied requests - Custom 429 response with Retry-After header - Apply rate limits to critical endpoints: - AI rate limits on chat and agent start/resume endpoints - Standard rate limits on CRUD operations - Add rate limit audit logging events - Add 23 TDD tests for rate limiting functionality Rate limit categories: - auth: 10/minute (authentication endpoints) - standard: 100/minute (CRUD operations) - ai: 20/minute (LLM-powered operations) - websocket: 30/minute (connection rate limiting) * chore: remove unused imports in rate limit tests * fix(tests): update UI tests for rate limiting request parameter The rate limiting decorators require a `request: Request` parameter, which changed function signatures. This commit: - Fixes tasks.py to use `body.approved` instead of `request.approved` - Adds mock_request fixture to test files that call endpoints directly - Updates all direct endpoint calls to pass mock_request as first arg - Updates TaskApprovalRequest parameter from `request=` to `body=` - Updates function signature tests to expect `request` parameter This resolves the 30 failing tests caused by the rate limiting changes. * fix: address code review issues in rate limiting Critical fixes: - Remove dead code: unused wrapped() function in decorator - Fix tautological test assertion (always true) with meaningful test High priority fixes: - Use lru_cache for thread-safe config singleton - Add null check for user.id in rate limit key generation - Change Redis import error log from WARNING to ERROR level - Remove unused functools.wraps import * feat: implement audit logging for rate limit exceeded events The PR description claimed audit logging was implemented but it wasn't actually being called. This commit integrates the audit logger into the rate_limit_exceeded_handler to log security-relevant events: - Imports AuditLogger and AuditEventType when database is available - Logs rate limit exceeded events with user_id, IP, endpoint, and metadata - Gracefully handles audit logging failures without affecting response - Consistent with existing audit logging patterns in the codebase * fix: prevent duplicate 'rate limiting disabled' log messages Add module-level _logged_disabled flag to track if we've already logged the disabled message. This prevents duplicate INFO logs on every call to get_rate_limiter() when rate limiting is disabled. Also reset the flag in reset_rate_limiter() for test isolation. * feat(api): add rate limiting to all v2 routers Apply rate limiting decorators to all 16 v2 API routers: - @rate_limit_standard() (100/min) for CRUD operations - @rate_limit_ai() (20/min) for AI-heavy operations (diagnose, discovery, execute) Renamed body request parameters from 'request' to 'body' to avoid conflict with FastAPI's Request object required by rate limiting. * docs: update docstrings to reflect renamed body parameters Update Args sections in v2 router endpoint docstrings: - Add 'request: HTTP request for rate limiting' documentation - Rename 'request' body references to 'body' for consistency * fix(rate-limiter): address security and config review feedback Security fixes: - Add trusted proxy validation to prevent X-Forwarded-For spoofing - Only trust proxy headers when direct connection is from configured proxy - Make "unknown" IP rate limit keys unique to prevent shared bucket DoS - Add warning logs for proxy header spoofing attempts Configuration consolidation: - Add rate limit config fields to GlobalConfig (single source of truth) - Update RateLimitConfig.from_global_config() to delegate to GlobalConfig - Remove duplicate environment variable parsing - Add get_global_config() and reset_global_config() module-level functions Test improvements: - Add reset_caches fixture to all rate limiter test classes - Add TestTrustedProxyValidation with exact IP and CIDR matching tests - Add test_key_for_unknown_ip_is_unique for DoS mitigation - Update tests to use from_global_config() instead of from_environment() * test(rate-limiter): add integration tests and simplify decorator pattern Integration tests: - Add TestRateLimitingIntegration class with 5 tests - Verify rate limiting allows requests within limit - Verify 429 response when limit exceeded - Verify Retry-After header and response body format - Test rate limiting on real v2 endpoint Decorator pattern fix: - Remove redundant config.enabled check (get_rate_limiter already returns None when disabled) - Simplify to: if limiter is None, return func; otherwise apply decorator - This ensures consistent function identity regardless of config state * style: remove unused import in rate limit integration tests --------- Co-authored-by: Test User <test@example.com>
1 parent 6af766f commit e7f9d01

35 files changed

Lines changed: 2054 additions & 242 deletions

.env.example

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,3 +101,31 @@ AUTH_SECRET=CHANGE-ME-IN-PRODUCTION
101101
# Target repository in format "owner/repo"
102102
# Example: frankbria/codeframe
103103
# GITHUB_REPO=owner/repo
104+
105+
# ============================================================================
106+
# Rate Limiting Configuration
107+
# ============================================================================
108+
109+
# Enable/disable rate limiting (default: true)
110+
# Set to false to disable rate limiting entirely
111+
RATE_LIMIT_ENABLED=true
112+
113+
# Rate limit for authentication endpoints (default: 10/minute)
114+
# Format: "N/period" where period is second, minute, hour, or day
115+
RATE_LIMIT_AUTH=10/minute
116+
117+
# Rate limit for standard API endpoints (default: 100/minute)
118+
RATE_LIMIT_STANDARD=100/minute
119+
120+
# Rate limit for AI/expensive operations like chat (default: 20/minute)
121+
RATE_LIMIT_AI=20/minute
122+
123+
# Rate limit for WebSocket connections (default: 30/minute)
124+
RATE_LIMIT_WEBSOCKET=30/minute
125+
126+
# Storage backend for rate limiting (default: memory)
127+
# Options: memory (single instance) or redis (distributed)
128+
RATE_LIMIT_STORAGE=memory
129+
130+
# Redis URL for distributed rate limiting (required if RATE_LIMIT_STORAGE=redis)
131+
# REDIS_URL=redis://localhost:6379/0

codeframe/config/rate_limits.py

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
"""Rate limiting configuration for CodeFRAME API.
2+
3+
This module provides configuration for API rate limiting using slowapi.
4+
It delegates to GlobalConfig in core/config.py as the single source of truth
5+
for environment variable handling.
6+
7+
Environment Variables (via GlobalConfig):
8+
RATE_LIMIT_ENABLED: Enable/disable rate limiting (default: true)
9+
RATE_LIMIT_AUTH: Rate limit for authentication endpoints (default: 10/minute)
10+
RATE_LIMIT_STANDARD: Rate limit for standard API endpoints (default: 100/minute)
11+
RATE_LIMIT_AI: Rate limit for AI/expensive operations (default: 20/minute)
12+
RATE_LIMIT_WEBSOCKET: Rate limit for WebSocket connections (default: 30/minute)
13+
RATE_LIMIT_STORAGE: Storage backend - memory or redis (default: memory)
14+
RATE_LIMIT_TRUSTED_PROXIES: Comma-separated trusted proxy IPs/CIDRs
15+
REDIS_URL: Redis connection URL for distributed rate limiting (optional)
16+
"""
17+
18+
import ipaddress
19+
import logging
20+
from dataclasses import dataclass, field
21+
from functools import lru_cache
22+
from typing import Optional
23+
24+
logger = logging.getLogger(__name__)
25+
26+
27+
@dataclass
28+
class RateLimitConfig:
29+
"""Configuration for API rate limiting.
30+
31+
Attributes:
32+
auth_limit: Rate limit for authentication endpoints
33+
standard_limit: Rate limit for standard API endpoints
34+
ai_limit: Rate limit for AI/expensive operations
35+
websocket_limit: Rate limit for WebSocket connections
36+
enabled: Whether rate limiting is enabled
37+
storage: Storage backend ('memory' or 'redis')
38+
redis_url: Redis connection URL for distributed rate limiting
39+
trusted_proxies: List of trusted proxy IP addresses/networks
40+
"""
41+
42+
auth_limit: str = "10/minute"
43+
standard_limit: str = "100/minute"
44+
ai_limit: str = "20/minute"
45+
websocket_limit: str = "30/minute"
46+
enabled: bool = True
47+
storage: str = "memory"
48+
redis_url: Optional[str] = None
49+
trusted_proxies: list = field(default_factory=list)
50+
51+
def is_trusted_proxy(self, ip: str) -> bool:
52+
"""Check if an IP address is from a trusted proxy.
53+
54+
Args:
55+
ip: IP address to check
56+
57+
Returns:
58+
True if IP is in trusted_proxies list or matches a trusted network
59+
"""
60+
if not self.trusted_proxies:
61+
return False
62+
63+
try:
64+
client_ip = ipaddress.ip_address(ip)
65+
for proxy in self.trusted_proxies:
66+
try:
67+
# Check if it's a network (CIDR notation)
68+
if "/" in proxy:
69+
network = ipaddress.ip_network(proxy, strict=False)
70+
if client_ip in network:
71+
return True
72+
else:
73+
# Check exact IP match
74+
if client_ip == ipaddress.ip_address(proxy):
75+
return True
76+
except ValueError:
77+
# Invalid proxy entry, skip it
78+
continue
79+
return False
80+
except ValueError:
81+
# Invalid IP address
82+
return False
83+
84+
@classmethod
85+
def from_global_config(cls) -> "RateLimitConfig":
86+
"""Create RateLimitConfig from GlobalConfig.
87+
88+
Uses core/config.py as the single source of truth for
89+
environment variable handling.
90+
91+
Returns:
92+
RateLimitConfig instance with values from GlobalConfig
93+
"""
94+
# Import here to avoid circular imports
95+
from codeframe.core.config import get_global_config
96+
97+
global_config = get_global_config()
98+
99+
enabled = global_config.rate_limit_enabled
100+
storage = global_config.rate_limit_storage
101+
redis_url = global_config.redis_url
102+
103+
# Parse trusted proxies from comma-separated string
104+
trusted_proxies_str = global_config.rate_limit_trusted_proxies.strip()
105+
trusted_proxies = []
106+
if trusted_proxies_str:
107+
trusted_proxies = [
108+
p.strip() for p in trusted_proxies_str.split(",") if p.strip()
109+
]
110+
111+
# Validate storage type (already validated by Pydantic, but double-check)
112+
if storage not in ("memory", "redis"):
113+
logger.warning(
114+
f"Invalid RATE_LIMIT_STORAGE: {storage}. "
115+
f"Must be 'memory' or 'redis'. Defaulting to 'memory'."
116+
)
117+
storage = "memory"
118+
119+
# Warn if redis storage is requested but no URL provided
120+
if storage == "redis" and not redis_url:
121+
logger.warning(
122+
"RATE_LIMIT_STORAGE is 'redis' but REDIS_URL is not set. "
123+
"Falling back to in-memory storage."
124+
)
125+
storage = "memory"
126+
127+
return cls(
128+
auth_limit=global_config.rate_limit_auth,
129+
standard_limit=global_config.rate_limit_standard,
130+
ai_limit=global_config.rate_limit_ai,
131+
websocket_limit=global_config.rate_limit_websocket,
132+
enabled=enabled,
133+
storage=storage,
134+
redis_url=redis_url,
135+
trusted_proxies=trusted_proxies,
136+
)
137+
138+
139+
@lru_cache(maxsize=1)
140+
def get_rate_limit_config() -> RateLimitConfig:
141+
"""Get the global rate limit configuration.
142+
143+
Loads from GlobalConfig on first call, cached thereafter.
144+
Thread-safe via lru_cache.
145+
146+
Returns:
147+
RateLimitConfig instance
148+
"""
149+
config = RateLimitConfig.from_global_config()
150+
logger.info(
151+
f"Rate limit config initialized: "
152+
f"enabled={config.enabled}, "
153+
f"storage={config.storage}, "
154+
f"standard={config.standard_limit}, "
155+
f"trusted_proxies={len(config.trusted_proxies)} configured"
156+
)
157+
return config
158+
159+
160+
def _reset_rate_limit_config() -> None:
161+
"""Reset the global rate limit configuration.
162+
163+
Useful for testing to ensure clean state between tests.
164+
"""
165+
get_rate_limit_config.cache_clear()

codeframe/core/config.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,17 @@ class GlobalConfig(BaseSettings):
395395
github_token: Optional[str] = Field(None, alias="GITHUB_TOKEN")
396396
github_repo: Optional[str] = Field(None, alias="GITHUB_REPO") # Format: "owner/repo"
397397

398+
# Rate Limiting Configuration
399+
rate_limit_enabled: bool = Field(True, alias="RATE_LIMIT_ENABLED")
400+
rate_limit_storage: str = Field("memory", alias="RATE_LIMIT_STORAGE")
401+
redis_url: Optional[str] = Field(None, alias="REDIS_URL")
402+
rate_limit_auth: str = Field("10/minute", alias="RATE_LIMIT_AUTH")
403+
rate_limit_standard: str = Field("100/minute", alias="RATE_LIMIT_STANDARD")
404+
rate_limit_ai: str = Field("20/minute", alias="RATE_LIMIT_AI")
405+
rate_limit_websocket: str = Field("30/minute", alias="RATE_LIMIT_WEBSOCKET")
406+
# Comma-separated list of trusted proxy IPs/CIDRs (e.g., "10.0.0.0/8,172.16.0.0/12")
407+
rate_limit_trusted_proxies: str = Field("", alias="RATE_LIMIT_TRUSTED_PROXIES")
408+
398409
model_config = SettingsConfigDict(
399410
env_file=".env", env_file_encoding="utf-8", case_sensitive=False, extra="ignore"
400411
)
@@ -417,6 +428,15 @@ def validate_port(cls, v: int) -> int:
417428
raise ValueError(f"API_PORT must be between 1 and 65535, got: {v}")
418429
return v
419430

431+
@field_validator("rate_limit_storage")
432+
@classmethod
433+
def validate_rate_limit_storage(cls, v: str) -> str:
434+
"""Validate rate limit storage is valid."""
435+
allowed = ["memory", "redis"]
436+
if v not in allowed:
437+
raise ValueError(f"RATE_LIMIT_STORAGE must be one of {allowed}, got: {v}")
438+
return v
439+
420440
def get_cors_origins_list(self) -> list[str]:
421441
"""Parse CORS origins from comma-separated string."""
422442
return [origin.strip() for origin in self.cors_origins.split(",") if origin.strip()]
@@ -580,3 +600,31 @@ def get(self, key: str) -> Any:
580600
current = current[k]
581601

582602
return current
603+
604+
605+
# Module-level singleton for GlobalConfig
606+
_global_config: Optional[GlobalConfig] = None
607+
608+
609+
def get_global_config() -> GlobalConfig:
610+
"""Get the global configuration singleton.
611+
612+
Loads from environment variables on first call, cached thereafter.
613+
This is the recommended way to access GlobalConfig for most use cases.
614+
615+
Returns:
616+
GlobalConfig instance with values from environment
617+
"""
618+
global _global_config
619+
if _global_config is None:
620+
_global_config = GlobalConfig()
621+
return _global_config
622+
623+
624+
def reset_global_config() -> None:
625+
"""Reset the global configuration singleton.
626+
627+
Useful for testing to ensure clean state between tests.
628+
"""
629+
global _global_config
630+
_global_config = None

codeframe/lib/audit_logger.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ class AuditEventType(Enum):
4444
USER_DELETED = "user.deleted"
4545
USER_ROLE_CHANGED = "user.role.changed"
4646

47+
# Rate limiting events
48+
RATE_LIMIT_EXCEEDED = "rate_limit.exceeded"
49+
RATE_LIMIT_WARNING = "rate_limit.warning"
50+
4751

4852
class AuditLogger:
4953
"""Centralized audit logger for security events.
@@ -182,6 +186,38 @@ def log_user_event(
182186
metadata=metadata,
183187
)
184188

189+
def log_rate_limit_event(
190+
self,
191+
event_type: AuditEventType,
192+
user_id: Optional[int] = None,
193+
ip_address: Optional[str] = None,
194+
endpoint: Optional[str] = None,
195+
limit_category: Optional[str] = None,
196+
metadata: Optional[Dict[str, Any]] = None,
197+
) -> None:
198+
"""Log rate limiting event.
199+
200+
Args:
201+
event_type: Type of rate limit event (exceeded or warning)
202+
user_id: User ID (if authenticated)
203+
ip_address: Client IP address
204+
endpoint: API endpoint path
205+
limit_category: Rate limit category (auth, standard, ai, websocket)
206+
metadata: Additional event metadata
207+
"""
208+
self._log_event(
209+
event_type=event_type,
210+
user_id=user_id,
211+
resource_type="rate_limit",
212+
resource_id=None,
213+
ip_address=ip_address,
214+
metadata={
215+
**(metadata or {}),
216+
"endpoint": endpoint,
217+
"limit_category": limit_category,
218+
},
219+
)
220+
185221
def _log_event(
186222
self,
187223
event_type: AuditEventType,

0 commit comments

Comments
 (0)