feat: extract project/environment helpers for multi-project support#657
feat: extract project/environment helpers for multi-project support#657DevonFulcher wants to merge 4 commits intodf/migrate-requests-to-httpxfrom
Conversation
| response = await client.get( | ||
| f"{dbt_platform_url}/api/v3/accounts/{account_id}/projects/{project_id}/environments/?state=1&offset={offset}&limit={page_size}", | ||
| headers=headers, | ||
| ) |
Check failure
Code scanning / CodeQL
Partial server-side request forgery
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 10 hours ago
In general, to fix partial SSRF in this pattern you constrain user-controlled path components to a safe, expected subset and/or verify that they refer to resources the caller is actually allowed to access. For numeric IDs, that means enforcing integer types and checking that the requested IDs are present in the set of accounts/projects associated with the authenticated user, instead of blindly passing any ID into a backend HTTP call.
Concretely here, the vulnerable pieces are the account_id/project_id values passed into _get_all_environments_for_project from get_deployment_environments and set_selected_project. The best fix without changing intended functionality is to validate that the requested account_id corresponds to one of the accounts returned by get_all_accounts for the current token, and that the project_id corresponds to one of the projects for that account from get_all_projects_for_account. If either does not exist, we return a 4xx-style error (by raising a ValueError as is already done for a missing account in set_selected_project). This both satisfies CodeQL (the URL path still uses user-supplied IDs but now only after authorization/validation) and enforces proper access control.
Specifically:
- In
get_deployment_environments(/environmentshandler):- After building
headers, callget_all_accounts(dbt_platform_url, headers)and ensurerequest.account_idis present. If not, raiseValueError. - Then call
get_all_projects_for_accountfor that account and ensurerequest.project_idexists. If not, raiseValueError. - Only then call
_get_all_environments_for_projectwith those IDs.
- After building
- In
set_selected_project(/selected_projecthandler):- It already validates the account by ID. Add a similar validation step for
selected_project_request.project_idby callingget_all_projects_for_accountwith the resolvedaccountand verifying the project exists before calling_get_all_environments_for_project.
- It already validates the account by ID. Add a similar validation step for
All of these changes occur in src/dbt_mcp/oauth/fastapi_app.py. We do not need to modify _get_all_environments_for_project in environment_resolver.py; it continues to accept IDs but will only be called with IDs that have been vetted against the user’s accessible resources. No new imports are required, as we already import get_all_accounts and get_all_projects_for_account.
| @@ -173,10 +173,26 @@ | ||
| "Accept": "application/json", | ||
| "Authorization": f"Bearer {access_token}", | ||
| } | ||
| # Validate that the requested account and project belong to the authenticated user | ||
| accounts = await get_all_accounts( | ||
| dbt_platform_url=dbt_platform_url, | ||
| headers=headers, | ||
| ) | ||
| account = next((a for a in accounts if a.id == request.account_id), None) | ||
| if account is None: | ||
| raise ValueError(f"Account {request.account_id} not found") | ||
| projects = await get_all_projects_for_account( | ||
| dbt_platform_url=dbt_platform_url, | ||
| account=account, | ||
| headers=headers, | ||
| ) | ||
| project = next((p for p in projects if p.id == request.project_id), None) | ||
| if project is None: | ||
| raise ValueError(f"Project {request.project_id} not found for account {request.account_id}") | ||
| environments = await _get_all_environments_for_project( | ||
| dbt_platform_url=dbt_platform_url, | ||
| account_id=request.account_id, | ||
| project_id=request.project_id, | ||
| account_id=account.id, | ||
| project_id=project.id, | ||
| headers=headers, | ||
| page_size=100, | ||
| ) | ||
| @@ -208,10 +222,23 @@ | ||
| ) | ||
| if account is None: | ||
| raise ValueError(f"Account {selected_project_request.account_id} not found") | ||
| # Validate that the selected project belongs to the selected account | ||
| projects = await get_all_projects_for_account( | ||
| dbt_platform_url=dbt_platform_url, | ||
| account=account, | ||
| headers=headers, | ||
| ) | ||
| project = next( | ||
| (p for p in projects if p.id == selected_project_request.project_id), None | ||
| ) | ||
| if project is None: | ||
| raise ValueError( | ||
| f"Project {selected_project_request.project_id} not found for account {selected_project_request.account_id}" | ||
| ) | ||
| environments = await _get_all_environments_for_project( | ||
| dbt_platform_url=dbt_platform_url, | ||
| account_id=selected_project_request.account_id, | ||
| project_id=selected_project_request.project_id, | ||
| account_id=account.id, | ||
| project_id=project.id, | ||
| headers=headers, | ||
| page_size=100, | ||
| ) |
69ce9db to
9afd83f
Compare
beeaff9 to
4cf34e6
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This will be introduced in the first phase where it has non-empty content. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
3a4bf94 to
70b2a82
Compare
Why
This is Phase 1 of the multi-project feature, extracting shared helper functions that both Server A (env-var mode) and Server B (multi-project mode) will use. This extraction is the blocking dependency for all subsequent phases.
What
New
src/dbt_mcp/project/module with extracted helpers:environment_resolver.py—resolve_environments()(pure logic) andget_environments_for_project()(API + resolution)project_resolver.py—get_all_accounts()andget_all_projects_for_account()Refactored config providers (
config_providers.py):get_config()(existing, unchanged behavior) and a_build_config()static methodget_config_for_project(project_id)from all four providers — env resolution belongs in tool functions, not providers, keeping providers statelessget_environments_for_project()directly at call-time and pass the resolved environment ID to the provider's_build_config()or construct the config override inlineRefactored
fastapi_app.py:resolve_environments()Added
DBT_MCP_MULTI_PROJECT_ENABLEDenv var check inmain.py:true/1/yes, raisesNotImplementedError(stub for Server B, implemented in Phase 7)Updated existing pagination tests to reference new module paths
Notes
task checkpasses (lint, format, mypy)Drafted by Claude Sonnet 4.6 under the direction of @DevonFulcher