security: Fix multi-tenancy and ownership verification (#7, #8)#25
Merged
DevanshuNEU merged 3 commits intoDec 5, 2025
Merged
Conversation
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
Tests for Issue #7 (multi-tenancy) and #8 (hardcoded API key): 1. SupabaseService ownership methods: - list_repositories_for_user filters by user_id - get_repository_with_owner requires both repo_id and user_id - verify_repo_ownership returns bool 2. RepoManager delegation tests: - list_repos_for_user delegates to supabase - verify_ownership delegates to supabase 3. Security helper tests: - get_repo_or_404 raises 404 for unauthorized access - get_repo_or_404 returns repo for owner - verify_repo_access raises 404 for unauthorized 4. Dev API key security tests: - Dev key fails without DEBUG=true - Dev key fails without explicit DEV_API_KEY env var - Dev key works with both DEBUG=true AND DEV_API_KEY - Wrong dev key fails even in DEBUG mode 5. Info leakage prevention tests: - Non-existent and unauthorized repos get identical 404 6. Integration tests (code inspection): - list_repositories uses user-filtered method - All repo endpoints use ownership verification - search endpoint verifies repo ownership - explain endpoint verifies repo ownership All 18 tests passing.
|
@DevanshuNEU is attempting to deploy a commit to the Dev's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Add client_no_auth fixture for testing auth behavior - client fixture now bypasses auth for functional tests - Update test assertions to handle 404 from ownership checks - Simplify mock-based tests to signature verification - All 49 tests passing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Critical security fixes for multi-tenancy isolation and hardcoded credentials.
Security Issues Fixed
Issue #7 - Broken Multi-tenancy (CRITICAL)
Before: Any authenticated user could see ALL repositories in the system.
After: Users can only see and access their own repositories.
Endpoints Fixed:
GET /api/reposuser_idPOST /api/repos/{id}/indexGET /api/repos/{id}/dependenciesPOST /api/repos/{id}/impactGET /api/repos/{id}/insightsGET /api/repos/{id}/style-analysisPOST /api/searchPOST /api/explainWebSocket /ws/index/{id}Issue #8 - Hardcoded API Key (MEDIUM)
Before: Default
dev-secret-keycould work in production if DEBUG not explicitly false.After: Dev key requires BOTH
DEBUG=trueAND explicitDEV_API_KEYenv var.Technical Changes
New Helper Functions (main.py)
New Supabase Methods
list_repositories_for_user(user_id)get_repository_with_owner(repo_id, user_id)verify_repo_ownership(repo_id, user_id)Security Design Decisions
Testing
Closes #7, Closes #8