AAP-87798 Skip corrupted Gateway assignments with UUID object_id

ansible_base/rbac/role_sync_utils.py

@@ -5,6 +5,7 @@
 from typing import Any
 
 from django.contrib.contenttypes.models import ContentType
+from django.core.exceptions import ValidationError
 from django.db.models import Q
 
 from ansible_base.lib.utils.apps import is_rbac_installed
@@ -84,8 +85,19 @@ def get_content_object(role_definition, assignment_tuple: AssignmentTuple) -> An
         raise ValueError("get_content_object requires a role_definition with a content_type")
     model = role_definition.content_type.model_class()
     if _is_resource_registered(model):
-        object_resource = Resource.objects.get(ansible_id=assignment_tuple.ansible_id_or_pk)
-        return object_resource.content_object
+        expected_ct = role_definition.content_type
+        try:
+            # ansible_id is a UUIDField; integer PKs ("123") raise ValueError at
+            # evaluation time, so catch that and fall through to the PK lookup.
+            resource = Resource.objects.filter(
+                ansible_id=assignment_tuple.ansible_id_or_pk,
+                content_type__app_label=expected_ct.app_label,
+                content_type__model=expected_ct.model,
+            ).first()
+        except (ValueError, AttributeError, ValidationError):
+            resource = None
+        if resource is not None:
+            return resource.content_object
     return model.objects.get(pk=assignment_tuple.ansible_id_or_pk)
 
 

ansible_base/resource_registry/tasks/sync.py

@@ -4,6 +4,7 @@
 import csv
 import logging
 import time
+import uuid as _uuid_mod
 from collections import defaultdict
 from dataclasses import dataclass, field
 from enum import Enum
@@ -98,6 +99,7 @@ class RemoteAssignmentResult:
 
     assignments: set[AssignmentTuple] = field(default_factory=set)
     is_complete: bool = False
+    protected_pairs: frozenset[tuple[str, str, str]] = field(default_factory=frozenset)
 
 
 class RemoteAssignmentFetcher:
@@ -113,6 +115,7 @@ def __init__(self, api_client: ResourceAPIClient, page_size: int | None = None,
         self.assignments: set[AssignmentTuple] = set()
         self.page_size = page_size if page_size is not None else getattr(settings, 'RESOURCE_SYNC_PAGE_SIZE', DEFAULT_SYNC_PAGE_SIZE)
         self.service_filter = service_filter
+        self._protected_pairs: set[tuple[str, str, str]] = set()
 
     def fetch(self) -> RemoteAssignmentResult:
         """Paginate user then team assignments and return the result.
@@ -126,10 +129,52 @@ def fetch(self) -> RemoteAssignmentResult:
 
         users_ok = self._paginate(self.api_client.list_user_assignments, 'user_ansible_id', 'user')
         if not users_ok:
-            return RemoteAssignmentResult(assignments=self.assignments, is_complete=False)
+            return RemoteAssignmentResult(assignments=self.assignments, is_complete=False, protected_pairs=frozenset(self._protected_pairs))
 
         teams_ok = self._paginate(self.api_client.list_team_assignments, 'team_ansible_id', 'team')
-        return RemoteAssignmentResult(assignments=self.assignments, is_complete=teams_ok)
+        return RemoteAssignmentResult(assignments=self.assignments, is_complete=teams_ok, protected_pairs=frozenset(self._protected_pairs))
+
+    def _process_page(self, results: list, actor_id_key: str, assignment_type: str) -> None:
+        """Add valid assignments from a single response page to ``self.assignments``."""
+        for assignment in results:
+            role_name = assignment['role_definition']
+            if role_name not in self.local_role_names:
+                logger.debug(f"Skipping remote {assignment_type} assignment with unknown local role: {role_name}")
+                continue
+            object_ansible_id = assignment.get('object_ansible_id')
+            ansible_id_or_pk = object_ansible_id or assignment.get('object_id')
+            if not object_ansible_id and ansible_id_or_pk:
+                # ansible_id_or_pk came from object_id.  Non-registered models send their PK
+                # here.  If the value is UUID-format it is almost certainly corrupted Gateway data
+                # (e.g. a RoleDefinition UUID placed in the namespace PK field).  Proceeding
+                # would raise ValueError in get_content_object and silently delete the matching
+                # local assignment.
+                # Note: this guard assumes non-registered RBAC targets use integer PKs.  If a
+                # UUID-PK non-registered model is ever added, this check will need to be made
+                # model-aware (e.g. by inspecting model._meta.pk type for the role's content type).
+                try:
+                    _uuid_mod.UUID(str(ansible_id_or_pk))
+                    actor_id = assignment.get(actor_id_key)
+                    logger.warning(
+                        "Skipping remote %s assignment (actor=%s, role=%s): object_id %r is a UUID "
+                        "but integer PK was expected — possible Gateway data corruption.",
+                        assignment_type,
+                        actor_id,
+                        role_name,
+                        ansible_id_or_pk,
+                    )
+                    self._protected_pairs.add((str(actor_id), role_name, assignment_type))
+                    continue
+                except (ValueError, AttributeError):
+                    pass
+            self.assignments.add(
+                AssignmentTuple(
+                    actor_ansible_id=assignment[actor_id_key],
+                    ansible_id_or_pk=ansible_id_or_pk,
+                    role_definition_name=role_name,
+                    assignment_type=assignment_type,
+                )
+            )
 
     def _paginate(self, list_fn, actor_id_key: str, assignment_type: str) -> bool:
         """Paginate a single assignment endpoint, adding results to ``self.assignments``.
@@ -148,20 +193,7 @@ def _paginate(self, list_fn, actor_id_key: str, assignment_type: str) -> bool:
                     return False
 
                 data = resp.json()
-                for assignment in data.get('results') or []:
-                    role_name = assignment['role_definition']
-                    if role_name not in self.local_role_names:
-                        logger.debug(f"Skipping remote {assignment_type} assignment with unknown local role: {role_name}")
-                        continue
-                    ansible_id_or_pk = assignment.get('object_ansible_id') or assignment.get('object_id')
-                    self.assignments.add(
-                        AssignmentTuple(
-                            actor_ansible_id=assignment[actor_id_key],
-                            ansible_id_or_pk=ansible_id_or_pk,
-                            role_definition_name=role_name,
-                            assignment_type=assignment_type,
-                        )
-                    )
+                self._process_page(data.get('results') or [], actor_id_key, assignment_type)
 
                 if not data.get('next'):
                     return True
@@ -616,15 +648,29 @@ def _sync_assignments(self):
             local_assignments = get_local_assignments(service=self.service_filter)
 
             # Deletions are only safe when the remote fetch was complete.
-            # A partial fetch would cause us to delete assignments that
-            # simply weren't fetched.
-            if remote_result.is_complete:
-                to_delete = local_assignments - remote_result.assignments
-                deleted_count, delete_errors = self._apply_assignment_changes(to_delete, delete_local_assignment, "DELETED")
-            else:
+            # A partial fetch means we never saw some assignments, so we cannot
+            # treat their absence as a revocation.
+            if not remote_result.is_complete:
                 self.write("Skipping assignment deletions — remote fetch was incomplete. Will retry on next sync cycle.")
                 logger.warning("Skipping assignment deletions: remote fetch was incomplete. Deletions deferred to next complete sync.")
                 deleted_count, delete_errors = 0, 0
+            else:
+                to_delete = local_assignments - remote_result.assignments
+                if remote_result.protected_pairs:
+                    # Some remote assignments were corrupted and skipped.  Protect the
+                    # matching local assignments (same actor + role + type) from deletion —
+                    # their apparent absence is due to the corruption, not a real revocation.
+                    # All other deletions (different actor/role combinations) proceed normally.
+                    protected = remote_result.protected_pairs
+                    shielded = {a for a in to_delete if (a.actor_ansible_id, a.role_definition_name, a.assignment_type) in protected}
+                    if shielded:
+                        logger.warning(
+                            "Shielding %d local assignment(s) from deletion — their remote counterpart "
+                            "had a corrupted UUID object_id. Will reconcile on next sync cycle.",
+                            len(shielded),
+                        )
+                    to_delete -= shielded
+                deleted_count, delete_errors = self._apply_assignment_changes(to_delete, delete_local_assignment, "DELETED")
 
             # Creations are safe even on a partial fetch.
             to_create = remote_result.assignments - local_assignments

test_app/tests/rbac/test_role_sync_utils.py

@@ -105,6 +105,32 @@ def test_get_content_object_falls_back_to_pk_lookup():
     assert result == inventory
 
 
+@pytest.mark.django_db
+def test_get_content_object_falls_back_to_pk_for_registered_model(organization):
+    """get_content_object falls back to PK lookup when ansible_id_or_pk is an
+    integer PK for a resource-registered model.
+
+    Resource.ansible_id is a UUIDField; passing "123" raises ValueError at
+    queryset evaluation time. Without the try/except guard, that ValueError
+    would propagate instead of falling through to model.objects.get(pk=...).
+    """
+    from ansible_base.rbac.models import DABContentType, RoleDefinition
+    from test_app.models import Organization
+
+    org_ct = DABContentType.objects.get_for_model(Organization)
+    rd = RoleDefinition.objects.create(name='Org PK Fallback Read', content_type=org_ct, managed=True)
+
+    at = AssignmentTuple(
+        actor_ansible_id='unused',
+        ansible_id_or_pk=str(organization.pk),
+        role_definition_name='Org PK Fallback Read',
+        assignment_type='user',
+    )
+
+    result = get_content_object(rd, at)
+    assert result == organization
+
+
 # ---------------------------------------------------------------------------
 # _is_resource_registered
 # ---------------------------------------------------------------------------

test_app/tests/resource_registry/test_resource_sync.py

@@ -1425,3 +1425,98 @@ def test_cleanup_orphans_continues_after_deletion_error(admin_api_client, static
     assert len(error_lines) == 2
     assert User.objects.filter(username__in=["orphan_one", "orphan_two"]).count() == 2
     assert executor.deleted_count == 0
+
+
+@pytest.mark.django_db
+def test_paginate_skips_corrupted_uuid_object_id():
+    """Assignments whose object_id is a UUID are skipped and flagged as invalid.
+
+    This guards against corrupted Gateway data where a RoleDefinition UUID ends up
+    as object_id for a namespace assignment. The corrupted assignment must be skipped
+    and the actor+role+type recorded in protected_pairs, while a valid assignment on
+    the same page is still applied.
+    """
+    corrupted_object_id = str(uuid4())
+    corrupted_actor = str(uuid4())
+    valid_actor = str(uuid4())
+    role_name = 'galaxy.collection_publisher'
+    RoleDefinition.objects.create(name=role_name, managed=True)
+
+    api_client = mock.Mock(spec=["list_user_assignments", "list_team_assignments"])
+    api_client.list_user_assignments.return_value = _mock_response(
+        body={
+            'results': [
+                # corrupted — object_id is a UUID
+                {
+                    'user_ansible_id': corrupted_actor,
+                    'object_ansible_id': None,
+                    'object_id': corrupted_object_id,
+                    'role_definition': role_name,
+                },
+                # valid — object_id is an integer PK
+                {
+                    'user_ansible_id': valid_actor,
+                    'object_ansible_id': None,
+                    'object_id': '42',
+                    'role_definition': role_name,
+                },
+            ],
+            'next': None,
+        }
+    )
+    api_client.list_team_assignments.return_value = _mock_response()
+
+    result = RemoteAssignmentFetcher(api_client).fetch()
+
+    assert result.is_complete is True
+    # The corrupted actor+role+type is recorded so deletions for that pair are shielded.
+    assert result.protected_pairs == frozenset([(corrupted_actor, role_name, 'user')])
+    assert len(result.assignments) == 1
+    surviving = next(iter(result.assignments))
+    assert surviving.actor_ansible_id == valid_actor
+    assert surviving.ansible_id_or_pk == '42'
+
+
+@pytest.mark.django_db
+def test_sync_assignments_shields_affected_pair_but_allows_other_deletions(stdout):
+    """Only the local assignment matching a corrupted remote pair is shielded from deletion.
+
+    When object_id is a UUID for a given actor+role+type, that specific local
+    assignment is protected.  An unrelated local assignment (different actor) that
+    was genuinely revoked must still be deleted.
+    """
+    corrupted_actor = str(uuid4())
+    unrelated_actor = str(uuid4())
+    role_name = 'galaxy.collection_publisher'
+
+    # Local assignment that matches the corrupted remote pair — must NOT be deleted.
+    shielded_tuple = AssignmentTuple(
+        actor_ansible_id=corrupted_actor,
+        ansible_id_or_pk='42',
+        role_definition_name=role_name,
+        assignment_type='user',
+    )
+    # Local assignment that was genuinely revoked — must be deleted.
+    revoked_tuple = AssignmentTuple(
+        actor_ansible_id=unrelated_actor,
+        ansible_id_or_pk='99',
+        role_definition_name=role_name,
+        assignment_type='user',
+    )
+
+    remote_result = RemoteAssignmentResult(
+        assignments=set(),
+        is_complete=True,
+        protected_pairs=frozenset([(corrupted_actor, role_name, 'user')]),
+    )
+
+    with (
+        mock.patch('ansible_base.resource_registry.tasks.sync.get_remote_assignments', return_value=remote_result),
+        mock.patch('ansible_base.resource_registry.tasks.sync.get_local_assignments', return_value={shielded_tuple, revoked_tuple}),
+        mock.patch('ansible_base.resource_registry.tasks.sync.delete_local_assignment') as mock_delete,
+    ):
+        executor = SyncExecutor(api_client=mock.Mock(), sync_assignments=True)
+        executor._sync_assignments()
+
+    # Only the genuinely revoked assignment should be deleted.
+    mock_delete.assert_called_once_with(revoked_tuple)