fix: fix migrate permission not exclude admin and owner (#43311)

api/commands/rbac.py

@@ -463,6 +463,36 @@ def _workspace_member_account_id_batches(tenant_id: str, batch_size: int) -> Ite
         last_join_id = str(rows[-1].id)
 
 
+def _workspace_non_admin_account_id_batches(
+    tenant_id: str,
+    operator_account_id: str,
+    batch_size: int,
+) -> Iterator[list[str]]:
+    """Yield workspace members except accounts bound to RBAC's builtin Owner/Admin roles.
+
+    Resource administrators receive access implicitly through RBAC and their
+    resource access-policy bindings are locked. Resolve their current RBAC
+    bindings in batches instead of relying on the legacy workspace role column.
+    """
+    for batch in _workspace_member_account_id_batches(tenant_id, batch_size):
+        member_roles = RBACService.MemberRoles.batch_get(
+            tenant_id=tenant_id,
+            account_id=operator_account_id,
+            member_account_ids=batch,
+        )
+        protected_account_ids = {
+            member.account_id
+            for member in member_roles
+            if any(
+                role.is_builtin and role.category == "global_system_default" and role.role_tag in {"owner", "admin"}
+                for role in member.roles
+            )
+        }
+        writable_batch = [account_id for account_id in batch if account_id not in protected_account_ids]
+        if writable_batch:
+            yield writable_batch
+
+
 def _replace_resource_whitelist(
     resource_type: str,
     *,
@@ -725,7 +755,7 @@ def migrate_resource_whitelist_scopes_to_automatic_include(
         migrated_count += 1
 
         if scope is RBACResourceWhitelistScope.ALL:
-            for batch in _workspace_member_account_id_batches(workspace_id, member_batch_size):
+            for batch in _workspace_non_admin_account_id_batches(workspace_id, operator_account_id, member_batch_size):
                 _replace_resource_default_access_policies(
                     current_resource_type,
                     tenant_id=workspace_id,
@@ -877,7 +907,7 @@ def migrate_only_me_resource_whitelist_scopes_to_automatic_include(
         )
         migrated_count += 1
 
-        for batch in _workspace_member_account_id_batches(workspace_id, member_batch_size):
+        for batch in _workspace_non_admin_account_id_batches(workspace_id, operator_account_id, member_batch_size):
             _replace_resource_default_access_policies(
                 current_resource_type,
                 tenant_id=workspace_id,
@@ -1187,7 +1217,11 @@ def _report_backing_app_specific_whitelist(options: _AgentAccessBootstrapOptions
 
 
 def _write_agent_access_rows(options: _AgentAccessBootstrapOptions) -> None:
-    for batch in _workspace_member_account_id_batches(options.tenant_id, options.member_batch_size):
+    for batch in _workspace_non_admin_account_id_batches(
+        options.tenant_id,
+        options.operator_account_id,
+        options.member_batch_size,
+    ):
         RBACService.AgentAccess.replace_user_access_policies(
             tenant_id=options.tenant_id,
             account_id=options.operator_account_id,

api/tests/unit_tests/commands/test_legacy_model_type_migration.py

@@ -526,6 +526,7 @@ def test_resource_whitelist_scope_migration_specific_preserves_existing_members(
         "replace_user_access_policies",
         lambda **kwargs: replace_policy_calls.append(kwargs),
     )
+    monkeypatch.setattr(rbac_module.RBACService.MemberRoles, "batch_get", lambda **kwargs: [])
 
     command_module.migrate_resource_whitelist_scopes_to_automatic_include.callback(
         tenant_id=None,
@@ -581,6 +582,7 @@ def test_resource_whitelist_scope_migration_all_syncs_workspace_members(
         "replace_user_access_policies",
         lambda **kwargs: replace_policy_calls.append(kwargs),
     )
+    monkeypatch.setattr(rbac_module.RBACService.MemberRoles, "batch_get", lambda **kwargs: [])
 
     command_module.migrate_resource_whitelist_scopes_to_automatic_include.callback(
         tenant_id=None,
@@ -636,6 +638,7 @@ def test_only_me_resource_whitelist_scope_migration_syncs_workspace_members(
         "replace_user_access_policies",
         lambda **kwargs: replace_policy_calls.append(kwargs),
     )
+    monkeypatch.setattr(rbac_module.RBACService.MemberRoles, "batch_get", lambda **kwargs: [])
 
     command_module.migrate_only_me_resource_whitelist_scopes_to_automatic_include.callback(
         tenant_id=None,

api/tests/unit_tests/commands/test_rbac_agent_migration.py

@@ -11,6 +11,8 @@ from commands.rbac import migrate_agent_permissions_to_rbac
 from services.enterprise.rbac_service import (
     LegacyAgentMigrationReport,
     LegacyAgentRoleMigration,
+    MemberRolesResponse,
+    RBACRole,
     _LegacyResourceWhitelistConfig,
 )
 
@@ -85,6 +87,7 @@ class _AgentPhaseSetup:
     agent_configs: list[_LegacyResourceWhitelistConfig] | None = None
     app_config: _LegacyResourceWhitelistConfig = field(default_factory=_whitelist_config)
     workspace_members: list[str] = field(default_factory=lambda: ["m1", "m2", "m3"])
+    protected_members: list[str] = field(default_factory=list)
     owner_account_id: str = "owner-1"
 
 
@@ -133,6 +136,30 @@ def _run_agent_phase(args: list[str], setup: _AgentPhaseSetup) -> tuple[Result,
             ),
             write_order=MagicMock(),
         )
+
+        def _member_roles(
+            *, tenant_id: str, account_id: str | None, member_account_ids: list[str]
+        ) -> list[MemberRolesResponse]:
+            del tenant_id, account_id
+            return [
+                MemberRolesResponse(
+                    account_id=account_id,
+                    roles=[
+                        RBACRole(
+                            id=f"role-{account_id}",
+                            type="workspace",
+                            category="global_system_default",
+                            name=account_id,
+                            is_builtin=True,
+                            role_tag="owner" if account_id == "owner-1" else "admin",
+                        )
+                    ],
+                )
+                for account_id in member_account_ids
+                if account_id in setup.protected_members
+            ]
+
+        stack.enter_context(patch(f"{MODULE}.RBACService.MemberRoles.batch_get", side_effect=_member_roles))
         mocks.write_order.attach_mock(mocks.replace_user_access_policies, "seed_members")
         mocks.write_order.attach_mock(mocks.sync_creator_bindings, "sync_creator")
         mocks.write_order.attach_mock(mocks.replace_whitelist, "replace_whitelist")
@@ -147,7 +174,11 @@ def _write_order(mocks: _AgentPhaseMocks) -> list[str]:
 def test_agent_bootstrap_apply_writes_whitelist_member_batches_and_creator_sync() -> None:
     result, mocks = _run_agent_phase(
         ["--apply", "--member-batch-size", "2"],
-        _AgentPhaseSetup(agents=[("ag1", "c1", None)], workspace_members=["m1", "m2", "m3"]),
+        _AgentPhaseSetup(
+            agents=[("ag1", "c1", None)],
+            workspace_members=["owner-1", "m1", "admin-1", "m2", "m3"],
+            protected_members=["owner-1", "admin-1"],
+        ),
     )
 
     assert result.exit_code == 0, result.output
@@ -160,9 +191,9 @@ def test_agent_bootstrap_apply_writes_whitelist_member_batches_and_creator_sync(
     assert mocks.replace_whitelist.call_args.kwargs["payload"].automatic_include_workspace_members is True
 
     mocks.member_batches.assert_called_once_with("t1", 2)
-    assert mocks.replace_user_access_policies.call_count == 2
+    assert mocks.replace_user_access_policies.call_count == 3
     calls = mocks.replace_user_access_policies.call_args_list
-    assert [call.kwargs["payload"].account_ids for call in calls] == [["m1", "m2"], ["m3"]]
+    assert [call.kwargs["payload"].account_ids for call in calls] == [["m1"], ["m2"], ["m3"]]
     assert all(call.kwargs["payload"].access_policy_ids == ["default"] for call in calls)
     assert all(call.kwargs["target_account_id"] is None for call in calls)
 
@@ -171,7 +202,13 @@ def test_agent_bootstrap_apply_writes_whitelist_member_batches_and_creator_sync(
     assert mocks.sync_creator_bindings.call_args.kwargs["account_id"] == "c1"
     assert "1 agent(s) changed, 0 already initialised" in result.output
 
-    assert _write_order(mocks) == ["seed_members", "seed_members", "sync_creator", "replace_whitelist"]
+    assert _write_order(mocks) == [
+        "seed_members",
+        "seed_members",
+        "seed_members",
+        "sync_creator",
+        "replace_whitelist",
+    ]
 
 
 def test_agent_bootstrap_is_idempotent_on_a_second_apply() -> None: