diff --git a/src/backend/base/langflow/api/v1/deployments.py b/src/backend/base/langflow/api/v1/deployments.py index c6cbd81cb8..18b594e9f8 100644 --- a/src/backend/base/langflow/api/v1/deployments.py +++ b/src/backend/base/langflow/api/v1/deployments.py @@ -1253,9 +1253,30 @@ async def update_snapshot( detail=f"No attachment found for provider_snapshot_id '{snapshot_id}'.", ) + # The downstream provider call is scoped to ONE provider account (the + # one we resolve ``deployment_adapter`` against), but + # ``update_flow_version_by_provider_snapshot_id`` rewrites every owner + # row matching ``(owner_id, snapshot_id)`` regardless of which provider + # account the deployment belongs to. If the same provider_snapshot_id + # exists across multiple provider accounts in the owner's namespace, + # mutating one provider account while rewriting attachments tied to a + # different account would corrupt their pointer state. Refuse the + # ambiguous case so the caller has to disambiguate (typically by + # cleaning up snapshot id collisions). + provider_account_ids = {d.deployment_provider_account_id for d in resolved_deployments} + if len(provider_account_ids) > 1: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=( + f"Provider snapshot '{snapshot_id}' is attached to deployments across " + f"{len(provider_account_ids)} provider accounts. The PATCH route cannot " + "safely update them in one operation." + ), + ) + # Pick any authorized attachment + deployment for the downstream provider # call — they all share the same owner, snapshot, and provider account by - # construction. The mutation operates on the full set via + # the check above. The mutation operates on the full set via # ``update_flow_version_by_provider_snapshot_id``. deployment = resolved_deployments[0] attachment = next(c for c in all_candidates if c.deployment_id == deployment.id) diff --git a/src/backend/tests/unit/api/v1/test_deployment_route_handlers.py b/src/backend/tests/unit/api/v1/test_deployment_route_handlers.py index c0e71b3b28..42438f9eb2 100644 --- a/src/backend/tests/unit/api/v1/test_deployment_route_handlers.py +++ b/src/backend/tests/unit/api/v1/test_deployment_route_handlers.py @@ -60,9 +60,14 @@ def _fake_provider_account( provider_url: str = "https://api.us-south.wxo.cloud.ibm.com/instances/tenant-1", api_key: str = "encrypted-key", provider_tenant_id: str | None = "tenant-1", + user_id=None, ) -> SimpleNamespace: return SimpleNamespace( id=uuid4(), + # ``user_id`` is read by ``list_deployment_configs`` to pick the + # provider namespace for adapter calls; default to a random UUID + # so tests that don't care about the owner still pass. + user_id=user_id if user_id is not None else uuid4(), provider_key=provider_key, provider_url=provider_url, api_key=api_key, @@ -1197,10 +1202,10 @@ class TestUpdateSnapshotRoute: @patch(f"{ROUTES_MODULE}.resolve_deployment_adapter") @patch(f"{ROUTES_MODULE}.get_deployment_mapper") @patch(f"{ROUTES_MODULE}.get_owned_provider_account_or_404", new_callable=AsyncMock) - @patch(f"{ROUTES_MODULE}.get_attachment_by_provider_snapshot_id", new_callable=AsyncMock) + @patch(f"{ROUTES_MODULE}.list_attachments_by_provider_snapshot_id", new_callable=AsyncMock) async def test_updates_all_attachment_rows_for_snapshot( self, - mock_get_attachment, + mock_list_attachments, mock_get_pa, mock_get_mapper, mock_resolve_adapter, @@ -1213,19 +1218,23 @@ class TestUpdateSnapshotRoute: user = _fake_user() flow_id = uuid4() target_flow_version_id = uuid4() + # ``update_snapshot`` filters the candidate set to attachments owned + # by the actor when share-aware fetch is off. Pin the owner to the + # actor's id so the OSS pass-through path keeps the row. + deployment = _fake_deployment_row( + user_id=user.id, + deployment_provider_account_id=uuid4(), + ) attachment = SimpleNamespace( flow_version_id=uuid4(), - deployment_id=uuid4(), + deployment_id=deployment.id, provider_snapshot_id="tool-1", - ) - deployment = _fake_deployment_row( - id=attachment.deployment_id, - deployment_provider_account_id=uuid4(), + user_id=user.id, ) provider_account = _fake_provider_account() provider_account.id = deployment.deployment_provider_account_id - mock_get_attachment.return_value = attachment + mock_list_attachments.return_value = [attachment] mock_get_deployment_row.return_value = deployment mock_get_flow_version.return_value = SimpleNamespace(id=target_flow_version_id, flow_id=flow_id, data={}) mock_get_pa.return_value = provider_account @@ -1249,9 +1258,8 @@ class TestUpdateSnapshotRoute: assert response.flow_version_id == target_flow_version_id assert response.provider_snapshot_id == "tool-1" - mock_get_attachment.assert_awaited_once_with( + mock_list_attachments.assert_awaited_once_with( session, - user_id=user.id, provider_snapshot_id="tool-1", ) mock_update_rows.assert_awaited_once_with( @@ -1270,10 +1278,10 @@ class TestUpdateSnapshotRoute: @patch(f"{ROUTES_MODULE}.resolve_deployment_adapter") @patch(f"{ROUTES_MODULE}.get_deployment_mapper") @patch(f"{ROUTES_MODULE}.get_owned_provider_account_or_404", new_callable=AsyncMock) - @patch(f"{ROUTES_MODULE}.get_attachment_by_provider_snapshot_id", new_callable=AsyncMock) + @patch(f"{ROUTES_MODULE}.list_attachments_by_provider_snapshot_id", new_callable=AsyncMock) async def test_commit_failure_attempts_provider_compensation( self, - mock_get_attachment, + mock_list_attachments, mock_get_pa, mock_get_mapper, mock_resolve_adapter, @@ -1287,14 +1295,15 @@ class TestUpdateSnapshotRoute: flow_id = uuid4() previous_flow_version_id = uuid4() target_flow_version_id = uuid4() + deployment = _fake_deployment_row( + user_id=user.id, + deployment_provider_account_id=uuid4(), + ) attachment = SimpleNamespace( flow_version_id=previous_flow_version_id, - deployment_id=uuid4(), + deployment_id=deployment.id, provider_snapshot_id="tool-1", - ) - deployment = _fake_deployment_row( - id=attachment.deployment_id, - deployment_provider_account_id=uuid4(), + user_id=user.id, ) provider_account = _fake_provider_account() provider_account.id = deployment.deployment_provider_account_id @@ -1302,7 +1311,7 @@ class TestUpdateSnapshotRoute: target_version = SimpleNamespace(id=target_flow_version_id, flow_id=flow_id, data={"nodes": []}) previous_version = SimpleNamespace(id=previous_flow_version_id, flow_id=flow_id, data={"nodes": []}) mock_get_flow_version.side_effect = [target_version, previous_version] - mock_get_attachment.return_value = attachment + mock_list_attachments.return_value = [attachment] mock_get_deployment_row.return_value = deployment mock_get_pa.return_value = provider_account adapter = AsyncMock() @@ -2249,7 +2258,10 @@ class TestGetDeploymentSync: await get_deployment(deployment_id=dep_row.id, session=session, current_user=user) assert exc_info.value.status_code == 404 - mock_delete_row.assert_awaited_once_with(session, user_id=user.id, deployment_id=dep_row.id) + # Stale-row delete now runs in the deployment owner's namespace so + # a non-owner with a share grant can still prune the owner's stale + # row. ``current_user`` is only the actor for audit. + mock_delete_row.assert_awaited_once_with(session, user_id=dep_row.user_id, deployment_id=dep_row.id) session.commit.assert_awaited_once() @pytest.mark.asyncio diff --git a/src/backend/tests/unit/api/v1/test_deployments_telemetry.py b/src/backend/tests/unit/api/v1/test_deployments_telemetry.py index fbc68ae9e7..bf215f4aa0 100644 --- a/src/backend/tests/unit/api/v1/test_deployments_telemetry.py +++ b/src/backend/tests/unit/api/v1/test_deployments_telemetry.py @@ -109,7 +109,9 @@ def mock_db_crud(mock_mapper): _mock_del_dep = stack.enter_context( patch("langflow.api.v1.deployments._delete_local_deployment_row_with_commit_retry") ) - mock_get_att = stack.enter_context(patch("langflow.api.v1.deployments.get_attachment_by_provider_snapshot_id")) + mock_list_att_by_snapshot = stack.enter_context( + patch("langflow.api.v1.deployments.list_attachments_by_provider_snapshot_id") + ) mock_get_dep_row = stack.enter_context( patch("langflow.services.database.models.deployment.crud.get_deployment") ) @@ -149,8 +151,25 @@ def mock_db_crud(mock_mapper): "watsonx-orchestrate", "tenant-test", ) - mock_get_att.return_value = AsyncMock(deployment_id=uuid4(), flow_version_id=uuid4()) - mock_get_dep_row.return_value = AsyncMock(deployment_provider_account_id=uuid4()) + # ``update_snapshot`` now enumerates every attachment with the + # given provider_snapshot_id and authorizes each one's deployment. + # The fake deployment row carries a stable user_id so the candidate + # filter keeps it under the OSS pass-through path. + snapshot_deployment_id = uuid4() + snapshot_owner_id = uuid4() + mock_list_att_by_snapshot.return_value = [ + AsyncMock( + deployment_id=snapshot_deployment_id, + flow_version_id=uuid4(), + user_id=snapshot_owner_id, + provider_snapshot_id="tool-1", + ) + ] + mock_get_dep_row.return_value = AsyncMock( + id=snapshot_deployment_id, + user_id=snapshot_owner_id, + deployment_provider_account_id=uuid4(), + ) mock_get_fv.return_value = AsyncMock(flow_id=uuid4(), data={}) mock_upd_fv.return_value = 1 mock_count_deps.return_value = 0