Skip to content

Commit 9c80728

Browse files
committed
security: Fix multi-tenancy and ownership verification (#7, #8)
CRITICAL SECURITY FIXES: 1. Multi-tenancy isolation (Issue #7): - GET /api/repos now only returns user's own repositories - Added verify_repo_ownership() and get_repo_or_404() helpers - All repo-specific endpoints now verify ownership: * POST /api/repos/{id}/index * GET /api/repos/{id}/dependencies * POST /api/repos/{id}/impact * GET /api/repos/{id}/insights * GET /api/repos/{id}/style-analysis * POST /api/search * POST /api/explain * WebSocket /ws/index/{id} - Returns 404 (not 403) to not leak repo existence 2. Hardcoded API key removal (Issue #8): - Removed default 'dev-secret-key' fallback - Dev key now requires BOTH DEBUG=true AND explicit DEV_API_KEY env var - Production environments cannot accidentally use dev keys 3. New Supabase methods: - list_repositories_for_user(user_id) - get_repository_with_owner(repo_id, user_id) - verify_repo_ownership(repo_id, user_id) 4. New RepoManager methods: - list_repos_for_user(user_id) - get_repo_for_user(repo_id, user_id) - verify_ownership(repo_id, user_id) Closes #7, Closes #8
1 parent 0d970d2 commit 9c80728

4 files changed

Lines changed: 85 additions & 29 deletions

File tree

backend/main.py

Lines changed: 52 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,29 @@ async def dispatch(self, request: Request, call_next):
8383
api_key_manager = APIKeyManager(get_supabase_service().client)
8484
cost_controller = CostController(get_supabase_service().client)
8585

86+
87+
# ===== SECURITY HELPERS =====
88+
89+
def get_repo_or_404(repo_id: str, user_id: str) -> dict:
90+
"""
91+
Get repository with ownership verification.
92+
Returns 404 if repo doesn't exist OR if user doesn't own it.
93+
(We return 404 instead of 403 to not leak info about repo existence)
94+
"""
95+
repo = repo_manager.get_repo_for_user(repo_id, user_id)
96+
if not repo:
97+
raise HTTPException(status_code=404, detail="Repository not found")
98+
return repo
99+
100+
101+
def verify_repo_access(repo_id: str, user_id: str) -> None:
102+
"""
103+
Verify user has access to repository.
104+
Raises 404 if no access (not 403, to avoid leaking repo existence).
105+
"""
106+
if not repo_manager.verify_ownership(repo_id, user_id):
107+
raise HTTPException(status_code=404, detail="Repository not found")
108+
86109
# Request/Response Models
87110
class SearchRequest(BaseModel):
88111
query: str
@@ -272,9 +295,11 @@ async def list_repositories(auth: AuthContext = Depends(require_auth)):
272295
"""List all repositories for authenticated user"""
273296
user_id = auth.user_id
274297

275-
# TODO: Filter repos by user_id once we add user_id column to repositories table
276-
# For now, return all repos (will fix in next section)
277-
repos = repo_manager.list_repos()
298+
if not user_id:
299+
raise HTTPException(status_code=401, detail="User ID required")
300+
301+
# Only return repos owned by this user
302+
repos = repo_manager.list_repos_for_user(user_id)
278303
return {"repositories": repos}
279304

280305

@@ -369,16 +394,18 @@ async def websocket_index(websocket: WebSocket, repo_id: str):
369394
if not user:
370395
return
371396

372-
# TODO: Add repo ownership validation once user_id column exists in repos table
373-
# For now, any authenticated user can index any repo they know the ID of
397+
user_id = user.get("user_id")
398+
if not user_id:
399+
await websocket.close(code=4001, reason="User ID required")
400+
return
374401

375-
# Validate repo exists before accepting connection
376-
repo = repo_manager.get_repo(repo_id)
402+
# Verify user owns this repository (return same error to not leak info)
403+
repo = repo_manager.get_repo_for_user(repo_id, user_id)
377404
if not repo:
378405
await websocket.close(code=4004, reason="Repository not found")
379406
return
380407

381-
# Connection authenticated and repo valid - accept
408+
# Connection authenticated and repo ownership verified - accept
382409
await websocket.accept()
383410

384411
try:
@@ -432,9 +459,8 @@ async def index_repository(
432459
start_time = time.time()
433460

434461
try:
435-
repo = repo_manager.get_repo(repo_id)
436-
if not repo:
437-
raise HTTPException(status_code=404, detail="Repository not found")
462+
# Verify ownership - returns 404 if not owned
463+
repo = get_repo_or_404(repo_id, auth.user_id)
438464

439465
# Set status to indexing
440466
repo_manager.update_status(repo_id, "indexing")
@@ -486,6 +512,9 @@ async def search_code(
486512
):
487513
"""Search code semantically with caching and validation"""
488514

515+
# Verify user owns the repository
516+
verify_repo_access(request.repo_id, auth.user_id)
517+
489518
# Validate search query
490519
valid_query, query_error = InputValidator.validate_search_query(request.query)
491520
if not valid_query:
@@ -534,9 +563,8 @@ async def explain_code(
534563
"""Generate code explanation"""
535564

536565
try:
537-
repo = repo_manager.get_repo(request.repo_id)
538-
if not repo:
539-
raise HTTPException(status_code=404, detail="Repository not found")
566+
# Verify ownership
567+
repo = get_repo_or_404(request.repo_id, auth.user_id)
540568

541569
explanation = await indexer.explain_code(
542570
repo_id=request.repo_id,
@@ -545,6 +573,8 @@ async def explain_code(
545573
)
546574

547575
return {"explanation": explanation}
576+
except HTTPException:
577+
raise
548578
except Exception as e:
549579
raise HTTPException(status_code=500, detail=str(e))
550580

@@ -565,9 +595,8 @@ async def get_dependency_graph(
565595
"""Get dependency graph for repository with Supabase caching"""
566596

567597
try:
568-
repo = repo_manager.get_repo(repo_id)
569-
if not repo:
570-
raise HTTPException(status_code=404, detail="Repository not found")
598+
# Verify ownership
599+
repo = get_repo_or_404(repo_id, auth.user_id)
571600

572601
# Try loading from Supabase cache
573602
cached_graph = dependency_analyzer.load_from_cache(repo_id)
@@ -598,9 +627,8 @@ async def analyze_impact(
598627
"""Analyze impact of changing a file with validation and caching"""
599628

600629
try:
601-
repo = repo_manager.get_repo(repo_id)
602-
if not repo:
603-
raise HTTPException(status_code=404, detail="Repository not found")
630+
# Verify ownership
631+
repo = get_repo_or_404(repo_id, auth.user_id)
604632

605633
# Validate file path
606634
valid_path, path_error = InputValidator.validate_file_path(request.file_path, repo["local_path"])
@@ -637,9 +665,8 @@ async def get_repository_insights(
637665
"""Get comprehensive insights about repository with Supabase caching"""
638666

639667
try:
640-
repo = repo_manager.get_repo(repo_id)
641-
if not repo:
642-
raise HTTPException(status_code=404, detail="Repository not found")
668+
# Verify ownership
669+
repo = get_repo_or_404(repo_id, auth.user_id)
643670

644671
# Try loading cached graph from Supabase
645672
graph_data = dependency_analyzer.load_from_cache(repo_id)
@@ -679,9 +706,8 @@ async def get_style_analysis(
679706
"""Analyze code style and team patterns with Supabase caching"""
680707

681708
try:
682-
repo = repo_manager.get_repo(repo_id)
683-
if not repo:
684-
raise HTTPException(status_code=404, detail="Repository not found")
709+
# Verify ownership
710+
repo = get_repo_or_404(repo_id, auth.user_id)
685711

686712
# Try loading from Supabase cache
687713
cached_style = style_analyzer.load_from_cache(repo_id)

backend/middleware/auth.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,12 @@ def _validate_jwt(token: str) -> Optional[AuthContext]:
8282

8383
def _validate_api_key(token: str) -> Optional[AuthContext]:
8484
"""Validate API key (ci_xxx format)"""
85-
# Dev key for local development
86-
dev_key = os.getenv("API_KEY", "dev-secret-key")
87-
if token == dev_key and os.getenv("DEBUG", "false").lower() == "true":
85+
# Dev key ONLY works in explicit DEBUG mode AND must be explicitly set
86+
# This prevents accidental use of dev keys in production
87+
debug_mode = os.getenv("DEBUG", "false").lower() == "true"
88+
dev_key = os.getenv("DEV_API_KEY") # Must be explicitly set, no default
89+
90+
if debug_mode and dev_key and token == dev_key:
8891
return AuthContext(
8992
api_key_name="development",
9093
tier="enterprise"

backend/services/repo_manager.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,22 @@ def list_repos(self) -> List[dict]:
8383
repos = self.db.list_repositories()
8484
return repos
8585

86+
def list_repos_for_user(self, user_id: str) -> List[dict]:
87+
"""List repositories owned by a specific user"""
88+
return self.db.list_repositories_for_user(user_id)
89+
8690
def get_repo(self, repo_id: str) -> Optional[dict]:
8791
"""Get repository by ID from Supabase"""
8892
return self.db.get_repository(repo_id)
8993

94+
def get_repo_for_user(self, repo_id: str, user_id: str) -> Optional[dict]:
95+
"""Get repository only if owned by user"""
96+
return self.db.get_repository_with_owner(repo_id, user_id)
97+
98+
def verify_ownership(self, repo_id: str, user_id: str) -> bool:
99+
"""Verify user owns repository"""
100+
return self.db.verify_repo_ownership(repo_id, user_id)
101+
90102
def add_repo(self, name: str, git_url: str, branch: str = "main", user_id: Optional[str] = None, api_key_hash: Optional[str] = None) -> dict:
91103
"""Add a new repository"""
92104
repo_id = str(uuid.uuid4())

backend/services/supabase_service.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,21 @@ def list_repositories(self) -> List[Dict]:
6767
result = self.client.table("repositories").select("*").order("created_at", desc=True).execute()
6868
return result.data or []
6969

70+
def list_repositories_for_user(self, user_id: str) -> List[Dict]:
71+
"""List repositories owned by a specific user"""
72+
result = self.client.table("repositories").select("*").eq("user_id", user_id).order("created_at", desc=True).execute()
73+
return result.data or []
74+
75+
def get_repository_with_owner(self, repo_id: str, user_id: str) -> Optional[Dict]:
76+
"""Get repository only if owned by user (returns None if not owned)"""
77+
result = self.client.table("repositories").select("*").eq("id", repo_id).eq("user_id", user_id).execute()
78+
return result.data[0] if result.data else None
79+
80+
def verify_repo_ownership(self, repo_id: str, user_id: str) -> bool:
81+
"""Check if user owns repository"""
82+
result = self.client.table("repositories").select("id").eq("id", repo_id).eq("user_id", user_id).execute()
83+
return bool(result.data)
84+
7085
def update_repository(self, repo_id: str, updates: Dict) -> Optional[Dict]:
7186
"""Update repository fields"""
7287
result = self.client.table("repositories").update(updates).eq("id", repo_id).execute()

0 commit comments

Comments
 (0)