Skip to content

Commit 80a44a2

Browse files
committed
fix: Production-level improvements to tier system
Security Fixes: - Remove broken RLS UPDATE policy (was allowing any user to update any profile) - Tier updates now ONLY via service role key (payment webhooks) Code Quality: - Add input validation for user_id (empty/null check) - Add Sentry error capture for exceptions - Fail-CLOSED on DB errors during check_repo_count (prevent bypass) - Add LimitCheckError exception class - Add invalidate_tier_cache() method for tier upgrades - Add error_code field to LimitCheckResult - Add tier field to all responses for frontend upgrade prompts - Better Redis error handling (continue on failure) Documentation: - Fix date: 2024 → 2025 - Remove async references (code is sync) - Add fail-safe behavior table - Update error response format to match actual code - Add new error codes: INVALID_USER, SYSTEM_ERROR
1 parent 29b4d61 commit 80a44a2

3 files changed

Lines changed: 197 additions & 74 deletions

File tree

backend/services/user_limits.py

Lines changed: 141 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,12 @@
1212
- #94: Repo size limits
1313
- #95: Repo count limits
1414
"""
15-
from dataclasses import dataclass
15+
from dataclasses import dataclass, field
1616
from typing import Optional, Dict, Any
1717
from enum import Enum
1818

1919
from services.observability import logger, metrics
20+
from services.sentry import capture_exception
2021

2122

2223
class UserTier(str, Enum):
@@ -43,7 +44,7 @@ class TierLimits:
4344
mcp_access: bool = True
4445

4546

46-
# Tier definitions
47+
# Tier definitions - Single source of truth
4748
TIER_LIMITS: Dict[UserTier, TierLimits] = {
4849
UserTier.FREE: TierLimits(
4950
max_repos=3,
@@ -82,20 +83,31 @@ class LimitCheckResult:
8283
current: int
8384
limit: Optional[int]
8485
message: str
86+
tier: str = "free" # Include tier for frontend upgrade prompts
87+
error_code: Optional[str] = None # e.g., "REPO_LIMIT_REACHED"
8588

8689
@property
8790
def limit_display(self) -> str:
8891
"""Display limit as string (handles unlimited)"""
89-
return str(self.limit) if self.limit is not None else ""
92+
return str(self.limit) if self.limit is not None else "unlimited"
9093

9194
def to_dict(self) -> Dict[str, Any]:
92-
return {
95+
result = {
9396
"allowed": self.allowed,
9497
"current": self.current,
9598
"limit": self.limit,
9699
"limit_display": self.limit_display,
97100
"message": self.message,
101+
"tier": self.tier,
98102
}
103+
if self.error_code:
104+
result["error_code"] = self.error_code
105+
return result
106+
107+
108+
class LimitCheckError(Exception):
109+
"""Raised when limit check fails due to system error (not limit exceeded)"""
110+
pass
99111

100112

101113
class UserLimitsService:
@@ -108,19 +120,25 @@ class UserLimitsService:
108120
# Check if user can add another repo
109121
result = limits.check_repo_count(user_id)
110122
if not result.allowed:
111-
raise HTTPException(403, result.message)
123+
raise HTTPException(403, result.to_dict())
112124
113125
# Check if repo size is within limits
114126
result = limits.check_repo_size(user_id, file_count, function_count)
115127
if not result.allowed:
116-
raise HTTPException(400, result.message)
128+
raise HTTPException(400, result.to_dict())
117129
"""
118130

119131
def __init__(self, supabase_client, redis_client=None):
120132
self.supabase = supabase_client
121133
self.redis = redis_client
122134
self._tier_cache_ttl = 300 # Cache tier for 5 minutes
123135

136+
def _validate_user_id(self, user_id: str) -> bool:
137+
"""Validate user_id is not empty"""
138+
if not user_id or not isinstance(user_id, str) or not user_id.strip():
139+
return False
140+
return True
141+
124142
# ===== TIER MANAGEMENT =====
125143

126144
def get_user_tier(self, user_id: str) -> UserTier:
@@ -130,23 +148,32 @@ def get_user_tier(self, user_id: str) -> UserTier:
130148
Checks Redis cache first, then Supabase.
131149
Defaults to FREE if not found.
132150
"""
151+
if not self._validate_user_id(user_id):
152+
logger.warning("Invalid user_id provided to get_user_tier", user_id=user_id)
153+
return UserTier.FREE
154+
133155
# Try cache first
134156
if self.redis:
135-
cache_key = f"user:tier:{user_id}"
136-
cached = self.redis.get(cache_key)
137-
if cached:
138-
try:
139-
return UserTier(cached.decode() if isinstance(cached, bytes) else cached)
140-
except ValueError:
141-
pass
157+
try:
158+
cache_key = f"user:tier:{user_id}"
159+
cached = self.redis.get(cache_key)
160+
if cached:
161+
tier_value = cached.decode() if isinstance(cached, bytes) else cached
162+
return UserTier(tier_value)
163+
except Exception as e:
164+
logger.warning("Redis cache read failed", error=str(e))
165+
# Continue to DB lookup
142166

143167
# Query Supabase
144168
tier = self._get_tier_from_db(user_id)
145169

146170
# Cache the result
147171
if self.redis:
148-
cache_key = f"user:tier:{user_id}"
149-
self.redis.setex(cache_key, self._tier_cache_ttl, tier.value)
172+
try:
173+
cache_key = f"user:tier:{user_id}"
174+
self.redis.setex(cache_key, self._tier_cache_ttl, tier.value)
175+
except Exception as e:
176+
logger.warning("Redis cache write failed", error=str(e))
150177

151178
return tier
152179

@@ -160,7 +187,9 @@ def _get_tier_from_db(self, user_id: str) -> UserTier:
160187
return UserTier(tier_value)
161188
except Exception as e:
162189
logger.warning("Failed to get user tier from DB", user_id=user_id, error=str(e))
190+
capture_exception(e)
163191

192+
# Default to FREE - this is safe because FREE has the most restrictive limits
164193
return UserTier.FREE
165194

166195
def get_limits(self, tier: UserTier) -> TierLimits:
@@ -172,15 +201,38 @@ def get_user_limits(self, user_id: str) -> TierLimits:
172201
tier = self.get_user_tier(user_id)
173202
return self.get_limits(tier)
174203

204+
def invalidate_tier_cache(self, user_id: str) -> None:
205+
"""Invalidate cached tier (call after tier upgrade)"""
206+
if self.redis and self._validate_user_id(user_id):
207+
try:
208+
cache_key = f"user:tier:{user_id}"
209+
self.redis.delete(cache_key)
210+
logger.info("Tier cache invalidated", user_id=user_id)
211+
except Exception as e:
212+
logger.warning("Failed to invalidate tier cache", error=str(e))
213+
175214
# ===== REPO COUNT LIMITS (#95) =====
176215

177-
def get_user_repo_count(self, user_id: str) -> int:
178-
"""Get current repo count for user"""
216+
def get_user_repo_count(self, user_id: str, raise_on_error: bool = False) -> int:
217+
"""
218+
Get current repo count for user.
219+
220+
Args:
221+
user_id: The user ID
222+
raise_on_error: If True, raise LimitCheckError on DB failure
223+
If False, return 0 (fail-open for reads, fail-closed for writes)
224+
"""
225+
if not self._validate_user_id(user_id):
226+
return 0
227+
179228
try:
180229
result = self.supabase.table("repositories").select("id", count="exact").eq("user_id", user_id).execute()
181230
return result.count or 0
182231
except Exception as e:
183232
logger.error("Failed to get repo count", user_id=user_id, error=str(e))
233+
capture_exception(e)
234+
if raise_on_error:
235+
raise LimitCheckError(f"Failed to check repository count: {str(e)}")
184236
return 0
185237

186238
def check_repo_count(self, user_id: str) -> LimitCheckResult:
@@ -189,18 +241,43 @@ def check_repo_count(self, user_id: str) -> LimitCheckResult:
189241
190242
Returns:
191243
LimitCheckResult with allowed=True if under limit
244+
245+
Note: Fails CLOSED - if we can't check, we don't allow.
192246
"""
247+
if not self._validate_user_id(user_id):
248+
return LimitCheckResult(
249+
allowed=False,
250+
current=0,
251+
limit=0,
252+
message="Invalid user ID",
253+
tier="unknown",
254+
error_code="INVALID_USER"
255+
)
256+
193257
tier = self.get_user_tier(user_id)
194258
limits = self.get_limits(tier)
195-
current_count = self.get_user_repo_count(user_id)
259+
260+
try:
261+
current_count = self.get_user_repo_count(user_id, raise_on_error=True)
262+
except LimitCheckError as e:
263+
# Fail CLOSED - don't allow if we can't verify
264+
return LimitCheckResult(
265+
allowed=False,
266+
current=0,
267+
limit=limits.max_repos,
268+
message="Unable to verify repository limit. Please try again.",
269+
tier=tier.value,
270+
error_code="SYSTEM_ERROR"
271+
)
196272

197273
# Unlimited repos
198274
if limits.max_repos is None:
199275
return LimitCheckResult(
200276
allowed=True,
201277
current=current_count,
202278
limit=None,
203-
message=f"OK ({current_count}/∞ repos)"
279+
message=f"OK ({current_count} repos)",
280+
tier=tier.value
204281
)
205282

206283
# Check limit
@@ -211,14 +288,17 @@ def check_repo_count(self, user_id: str) -> LimitCheckResult:
211288
allowed=False,
212289
current=current_count,
213290
limit=limits.max_repos,
214-
message=f"Repository limit reached ({current_count}/{limits.max_repos}). Upgrade for more repos."
291+
message=f"Repository limit reached ({current_count}/{limits.max_repos}). Upgrade to add more repositories.",
292+
tier=tier.value,
293+
error_code="REPO_LIMIT_REACHED"
215294
)
216295

217296
return LimitCheckResult(
218297
allowed=True,
219298
current=current_count,
220299
limit=limits.max_repos,
221-
message=f"OK ({current_count}/{limits.max_repos} repos)"
300+
message=f"OK ({current_count}/{limits.max_repos} repos)",
301+
tier=tier.value
222302
)
223303

224304
# ===== REPO SIZE LIMITS (#94) =====
@@ -240,6 +320,16 @@ def check_repo_size(
240320
Returns:
241321
LimitCheckResult with allowed=True if within limits
242322
"""
323+
if not self._validate_user_id(user_id):
324+
return LimitCheckResult(
325+
allowed=False,
326+
current=0,
327+
limit=0,
328+
message="Invalid user ID",
329+
tier="unknown",
330+
error_code="INVALID_USER"
331+
)
332+
243333
tier = self.get_user_tier(user_id)
244334
limits = self.get_limits(tier)
245335

@@ -256,7 +346,9 @@ def check_repo_size(
256346
allowed=False,
257347
current=file_count,
258348
limit=limits.max_files_per_repo,
259-
message=f"Repository too large ({file_count:,} files). {tier.value.title()} tier allows up to {limits.max_files_per_repo:,} files."
349+
message=f"Repository too large ({file_count:,} files). {tier.value.title()} tier allows up to {limits.max_files_per_repo:,} files.",
350+
tier=tier.value,
351+
error_code="REPO_TOO_LARGE"
260352
)
261353

262354
# Check function count
@@ -272,14 +364,17 @@ def check_repo_size(
272364
allowed=False,
273365
current=function_count,
274366
limit=limits.max_functions_per_repo,
275-
message=f"Repository has too many functions ({function_count:,}). {tier.value.title()} tier allows up to {limits.max_functions_per_repo:,} functions."
367+
message=f"Repository has too many functions ({function_count:,}). {tier.value.title()} tier allows up to {limits.max_functions_per_repo:,} functions.",
368+
tier=tier.value,
369+
error_code="REPO_TOO_LARGE"
276370
)
277371

278372
return LimitCheckResult(
279373
allowed=True,
280374
current=file_count,
281375
limit=limits.max_files_per_repo,
282-
message=f"OK ({file_count:,} files, {function_count:,} functions)"
376+
message=f"OK ({file_count:,} files, {function_count:,} functions)",
377+
tier=tier.value
283378
)
284379

285380
# ===== PLAYGROUND RATE LIMITS (#93) =====
@@ -295,6 +390,27 @@ def get_usage_summary(self, user_id: str) -> Dict[str, Any]:
295390
Get complete usage summary for user.
296391
Useful for dashboard display.
297392
"""
393+
if not self._validate_user_id(user_id):
394+
# Return free tier defaults for invalid user
395+
limits = TIER_LIMITS[UserTier.FREE]
396+
return {
397+
"tier": "free",
398+
"repositories": {
399+
"current": 0,
400+
"limit": limits.max_repos,
401+
"display": f"0/{limits.max_repos}"
402+
},
403+
"limits": {
404+
"max_files_per_repo": limits.max_files_per_repo,
405+
"max_functions_per_repo": limits.max_functions_per_repo,
406+
"playground_searches_per_day": limits.playground_searches_per_day,
407+
},
408+
"features": {
409+
"priority_indexing": limits.priority_indexing,
410+
"mcp_access": limits.mcp_access,
411+
}
412+
}
413+
298414
tier = self.get_user_tier(user_id)
299415
limits = self.get_limits(tier)
300416
repo_count = self.get_user_repo_count(user_id)
@@ -304,7 +420,7 @@ def get_usage_summary(self, user_id: str) -> Dict[str, Any]:
304420
"repositories": {
305421
"current": repo_count,
306422
"limit": limits.max_repos,
307-
"display": f"{repo_count}/{limits.max_repos if limits.max_repos else ''}"
423+
"display": f"{repo_count}/{limits.max_repos if limits.max_repos else 'unlimited'}"
308424
},
309425
"limits": {
310426
"max_files_per_repo": limits.max_files_per_repo,

0 commit comments

Comments
 (0)