diff --git a/docs/LocalDevelopmentSetup.md b/docs/LocalDevelopmentSetup.md index 8d74398a5..99aeb9ea7 100644 --- a/docs/LocalDevelopmentSetup.md +++ b/docs/LocalDevelopmentSetup.md @@ -408,22 +408,44 @@ The Backend API will start at: cd src/mcp_server ``` - +**Step 2: Copy the template** + +1. Open the `.env.example` file +2. Select all content (CTRL + A) +3. Copy (CTRL + C) +4. Open the new `.env` file +5. Paste (CTRL + V) + +**Step 3: Get Azure values and update `.env`** + +1. Open [Azure Portal](https://portal.azure.com) +2. Navigate to your **Resource Group** +3. Open the **MCP Server Container App** +4. Click **Environment variables** in the left menu +5. Copy each value from Azure and update the corresponding variable in your `.env` file + +**Step 4: Update local development settings** + +In your `.env` file, make these changes: + +- Set `APP_ENV=dev` +- Set `ENABLE_AUTH=false` +- Keep the local host/port defaults: + - `HOST=0.0.0.0` + - `PORT=9000` -### 5.2. Install MCP Server Dependencies +### 5.3. Install MCP Server Dependencies ```bash # Create and activate virtual environment @@ -438,7 +460,7 @@ source .venv/bin/activate # Linux/WSL2 uv sync --python 3.12 ``` -### 5.3. Run the MCP Server +### 5.4. Run the MCP Server ```bash # Run with per-domain routing (recommended) diff --git a/src/backend/api/router.py b/src/backend/api/router.py index dc169fec8..aa5ab1755 100644 --- a/src/backend/api/router.py +++ b/src/backend/api/router.py @@ -893,22 +893,29 @@ async def agent_message_user( status_code=401, detail="Missing or invalid user information" ) - # Attach session_id to span if plan_id is available and capture for events - session_id = None - if agent_message.plan_id: + # Authorization: require plan_id and verify the plan belongs to the + # authenticated user before persisting any agent message for it. This + # prevents cross-user message injection through /api/v4/agent_message. + if not agent_message.plan_id: + raise HTTPException(status_code=400, detail="plan_id is required") + + memory_store = await DatabaseFactory.get_database(user_id=user_id) + plan = await memory_store.get_plan_by_plan_id(plan_id=agent_message.plan_id) + if plan is None: + # Return 404 (not 403) to avoid disclosing whether the plan exists + # under a different user. + raise HTTPException(status_code=404, detail="Plan not found") + + # Attach session_id to span for telemetry (best-effort, non-fatal). + session_id = plan.session_id + if session_id: try: - memory_store = await DatabaseFactory.get_database(user_id=user_id) - plan = await memory_store.get_plan_by_plan_id(plan_id=agent_message.plan_id) - if plan and plan.session_id: - session_id = plan.session_id - span = trace.get_current_span() - if span: - span.set_attribute("session_id", session_id) + span = trace.get_current_span() + if span: + span.set_attribute("session_id", session_id) except Exception: pass # Don't fail request if span attribute fails - # Set the approval in the orchestration config - try: result = await PlanService.handle_agent_messages(agent_message, user_id) diff --git a/src/backend/services/plan_service.py b/src/backend/services/plan_service.py index df71f5d57..0b2f0c197 100644 --- a/src/backend/services/plan_service.py +++ b/src/backend/services/plan_service.py @@ -95,9 +95,11 @@ def build_agent_message_from_agent_message_response( or "" ) - # plan_id / user_id fallback + # plan_id fallback. user_id is ALWAYS taken from the authenticated caller + # to prevent a client from persisting an AgentMessageData row under a + # different user_id via the payload. plan_id_val = getattr(agent_response, "plan_id", "") or "" - user_id_val = getattr(agent_response, "user_id", "") or user_id + user_id_val = user_id return AgentMessageData( plan_id=plan_id_val, diff --git a/src/mcp_server/.env.example b/src/mcp_server/.env.example index 0112a6d9b..9c8dd3ab0 100644 --- a/src/mcp_server/.env.example +++ b/src/mcp_server/.env.example @@ -13,4 +13,7 @@ CLIENT_ID=your-client-id-here JWKS_URI=https://login.microsoftonline.com/your-tenant-id/discovery/v2.0/keys ISSUER=https://sts.windows.net/your-tenant-id/ AUDIENCE=api://your-client-id -DATASET_PATH=./datasets \ No newline at end of file +DATASET_PATH=./datasets +AZURE_OPENAI_ENDPOINT= +AZURE_STORAGE_BLOB_URL= +APP_ENV=dev \ No newline at end of file diff --git a/src/mcp_server/config/settings.py b/src/mcp_server/config/settings.py index 2c50d7998..4fce764b0 100644 --- a/src/mcp_server/config/settings.py +++ b/src/mcp_server/config/settings.py @@ -47,6 +47,8 @@ class MCPServerConfig(BaseSettings): # Backend URL for image proxy (browser loads images via backend instead of direct blob) backend_url: Optional[str] = Field(default=None) + app_env: str = Field(default="prod") + # Global configuration instance config = MCPServerConfig() diff --git a/src/mcp_server/services/image_service.py b/src/mcp_server/services/image_service.py index f2a0ceca0..e5779f8b2 100644 --- a/src/mcp_server/services/image_service.py +++ b/src/mcp_server/services/image_service.py @@ -30,9 +30,9 @@ def _get_credential(): """Return a credential based on environment (dev vs deployed).""" - app_env = os.environ.get("APP_ENV", "prod").lower() + app_env = (config.app_env or os.environ.get("APP_ENV") or "prod").lower() if app_env == "dev": - return DefaultAzureCredential(require_envvar=True) + return DefaultAzureCredential() return ManagedIdentityCredential(client_id=config.azure_client_id) diff --git a/src/tests/backend/api/test_router.py b/src/tests/backend/api/test_router.py index 16b72c3de..f04d5bedb 100644 --- a/src/tests/backend/api/test_router.py +++ b/src/tests/backend/api/test_router.py @@ -451,10 +451,29 @@ def test_success(self, rt): assert resp.json()["status"] == "message recorded" def test_plan_service_error(self, rt): + plan = MagicMock() + plan.session_id = "sess-1" + rt.store.get_plan_by_plan_id.return_value = plan rt.plan_service.handle_agent_messages = AsyncMock(side_effect=Exception("boom")) resp = rt.client.post("/api/v4/agent_message", json=self._payload()) assert resp.status_code == 200 + def test_plan_not_owned_returns_404(self, rt): + # store.get_plan_by_plan_id is user-scoped in the data layer, so a + # plan owned by a different user returns None. The endpoint must + # refuse the write with 404 and must not call handle_agent_messages. + rt.store.get_plan_by_plan_id.return_value = None + resp = rt.client.post("/api/v4/agent_message", json=self._payload()) + assert resp.status_code == 404 + rt.plan_service.handle_agent_messages.assert_not_called() + + def test_missing_plan_id_returns_400(self, rt): + payload = self._payload() + payload["plan_id"] = "" + resp = rt.client.post("/api/v4/agent_message", json=payload) + assert resp.status_code == 400 + rt.plan_service.handle_agent_messages.assert_not_called() + # --------------------------------------------------------------------------- # /upload_team_config diff --git a/src/tests/backend/services/test_plan_service.py b/src/tests/backend/services/test_plan_service.py index d37b7d6e3..782611946 100644 --- a/src/tests/backend/services/test_plan_service.py +++ b/src/tests/backend/services/test_plan_service.py @@ -171,12 +171,26 @@ def test_build_agent_message_from_agent_message_response_basic(self): ) result = build_agent_message_from_agent_message_response(response, "fallback-user") assert result.plan_id == "test-plan-123" - assert result.user_id == "response-user" + # Security: the authenticated user_id must always win, regardless of + # any user_id present on the payload. This prevents cross-user + # message injection via /api/v4/agent_message. + assert result.user_id == "fallback-user" assert result.agent == "TestAgent" assert result.content == "Agent response content" assert result.steps == ["step1", "step2"] assert result.next_steps == ["next1"] + def test_build_agent_message_from_agent_message_response_ignores_payload_user_id(self): + """Regression: payload-supplied user_id must never override the authenticated caller.""" + response = MockAgentMessageResponse( + plan_id="p-1", + user_id="attacker-supplied-victim-id", + agent="TestAgent", + content="malicious", + ) + result = build_agent_message_from_agent_message_response(response, "authenticated-user") + assert result.user_id == "authenticated-user" + def test_build_agent_message_from_agent_message_response_fallbacks(self): response = MockAgentMessageResponse( plan_id="",