[CryptoRodeo]

pulpcore's CompositeContentGuard ORs guards together: it permits a download if any one of its sub-guards allows it. A distribution can only carry a single guard, so there was no way to require several conditions at once -- e.g. "the org has the entitlement AND the user has the role" (PULP-2349).

This adds an AndGateContentGuard that mirrors CompositeContentGuard but inverts the logic: it permits only when every sub-guard permits, and it fails closed (an AndGate with no guards denies access rather than allowing everyone). Like the patched CompositeContentGuard, it evaluates every sub-guard and surfaces an AuthenticationError (the 401 challenge for a missing identity) ahead of a plain 403, so an unauthenticated client gets a chance to retry with credentials regardless of guard order. The guard lives entirely in pulp-service with the usual model/serializer/viewset/RBAC plumbing, matching FeatureContentGuard.

Covered by unit tests for the permit() logic (AND, short-circuit, fail-closed, exception propagation) and a functional test exercising the real guards M2M against live sub-guards.

Summary by Sourcery

Add an AND-based content guard so distributions can require multiple authorization conditions simultaneously.

New Features:

• Add an AndGateContentGuard that grants access only when all configured content guards permit, with fail-closed behavior for empty configurations.
• Expose AndGateContentGuard through service APIs with serialization, RBAC, role management, and lifecycle support.

Bug Fixes:

• Ensure authentication challenges take precedence over authorization denials when combined guards reject a request.

Tests:

• Add unit and functional coverage for conjunction behavior, fail-closed access, denial handling, and authentication-error precedence.

[sourcery-ai]

Reviewer's Guide

Adds a pulp-service AndGateContentGuard as a fail-closed complement to pulpcore's OR-based CompositeContentGuard, including model/API/RBAC plumbing and tests covering AND evaluation, real sub-guards, and 401-over-403 error precedence.

Sequence diagram for AndGateContentGuard access evaluation

sequenceDiagram
    participant Client
    participant AndGateContentGuard
    participant SubGuardA as SubGuard
    participant SubGuardB as SubGuard

    Client->>AndGateContentGuard: permit(request)
    AndGateContentGuard->>SubGuardA: cast().permit(request)
    SubGuardA-->>AndGateContentGuard: permit or AuthenticationError/PermissionError
    AndGateContentGuard->>SubGuardB: cast().permit(request)
    SubGuardB-->>AndGateContentGuard: permit or AuthenticationError/PermissionError
    alt Any guard raises AuthenticationError
        AndGateContentGuard-->>Client: AuthenticationError (401)
    else Any guard raises PermissionError
        AndGateContentGuard-->>Client: PermissionError (403)
    else All guards permit
        AndGateContentGuard-->>Client: Access permitted
    end

Loading

Entity relationship diagram for AndGateContentGuard sub-guards

erDiagram
    ANDGATE_CONTENT_GUARD {
        int contentguard_ptr PK
    }
    CONTENT_GUARD {
        int id PK
    }
    ANDGATE_CONTENT_GUARD }o--o{ CONTENT_GUARD : guards

Loading

File-Level Changes

Change	Details	Files
Adds a fail-closed logical-AND content guard that evaluates all configured sub-guards and prioritizes authentication challenges over authorization denials.	Introduces the ContentGuard model, guards many-to-many relation, type identifier, and database migration. Implements permit() aggregation with empty-set denial, full evaluation, AuthenticationError precedence, and PermissionError propagation. Adds serializer support for related content guards and wires the model into the service API with endpoint configuration, access policy, RBAC roles, and queryset scoping.	pulp_service/pulp_service/app/migrations/0026_andgatecontentguard.py pulp_service/pulp_service/app/models.py pulp_service/pulp_service/app/serializers.py pulp_service/pulp_service/app/viewsets.py
Documents and validates AndGate behavior through unit and functional coverage.	Tests all-pass, denial cases, fail-closed behavior, exception propagation, and authentication precedence independent of guard order. Exercises real many-to-many relationships and live HeaderContentGuard and FeatureContentGuard permit() implementations. Adds the user-facing feature changelog entry.	pulp_service/pulp_service/tests/unit/test_and_gate_content_guard.py pulp_service/pulp_service/tests/functional/test_and_gate_content_guard.py CHANGES/2348.feature

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 7 issues

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="pulp_service/pulp_service/app/models.py" line_range="95" />
<code_context>
+
+    TYPE = "andgate"
+
+    guards = models.ManyToManyField(ContentGuard, related_name="+", symmetrical=False)
+
+    def permit(self, request):
</code_context>
<issue_to_address>
**Deleted guards weaken access policies**

When a guard referenced by an AndGate is deleted, `_delete_guard_if_unreferenced()` does not check AndGate references, and deleting a guard removes its many-to-many link. The surviving AndGate then evaluates fewer conditions and can grant access under an incomplete policy.

Prevent deletion of guards referenced by an AndGate, including in `_delete_guard_if_unreferenced()`, so required conditions cannot disappear.
</issue_to_address>

### Comment 2
<location path="pulp_service/pulp_service/app/models.py" line_range="110" />
<code_context>
+            AuthenticationError: when a sub-guard needs authentication (takes precedence).
+            PermissionError: when any sub-guard denies, or when no guards are configured.
+        """
+        guards = self.guards.all()
+        if not guards:
+            raise PermissionError(_("Access denied. AndGateContentGuard has no guards configured."))
</code_context>
<issue_to_address>
**New restrictions lag on replicas**

When a replica is configured and a content request arrives after a guard-list update commits on the primary but before that update is replicated, the administrator commits a newly required sub-guard on the primary, then a content request reads the old M2M membership from the replica and evaluates only the previous guards; until replication catches up, the request can download content that the updated policy denies.

Read the guard membership from the primary during authorization, for example by wrapping permit-time reads in `use_primary_database()`.
</issue_to_address>

### Comment 3
<location path="pulp_service/pulp_service/app/models.py" line_range="117" />
<code_context>
+        permission_error = None
+        for guard in guards:
+            try:
+                guard.cast().permit(request)
+            except AuthenticationError as ae:
+                if authentication_error is None:
</code_context>
<issue_to_address>
**Cyclic guards fail content requests**

When an AndGate references itself or is part of a cycle of AndGates, `AndGateContentGuard.permit()` calls `guard.cast().permit(request)` recursively without cycle detection, so Python raises `RecursionError` and the content request returns a server error.

Add cycle detection to AndGate permission evaluation so a self-reference or cycle cannot recurse indefinitely.

Also at `pulp_service/pulp_service/app/serializers.py:77-83`, `pulp_service/pulp_service/app/models.py:95`.
</issue_to_address>

### Comment 4
<location path="pulp_service/pulp_service/app/models.py" line_range="125" />
<code_context>
+                if permission_error is None:
+                    permission_error = pe
+        if authentication_error is not None:
+            raise authentication_error
+        if permission_error is not None:
+            raise permission_error
</code_context>
<issue_to_address>
**Missing identity loses retry challenge**

When an AndGate is used on a PyPI distribution and a request lacks identity while another sub-guard raises `PermissionError`, `AndGateContentGuard.permit()` raises the preferred `AuthenticationError`, but `PulpServiceAccessPolicy._evaluate_content_guard()` catches it and returns `False`; on PyPI endpoints the request is denied with a 403 instead of the promised 401 challenge, so unauthenticated clients do not get the retry flow.

Preserve `AuthenticationError` through the PyPI permission path so the API can return an authentication challenge.
</issue_to_address>

### Comment 5
<location path="pulp_service/pulp_service/app/serializers.py" line_range="83" />
<code_context>
+        help_text=_("List of ContentGuards; access is permitted only if ALL of them permit."),
+        view_name_pattern=r"contentguards(-.*/.*)?-detail",
+        queryset=ContentGuard.objects.all(),
+        allow_null=True,
+    )
+
</code_context>
<issue_to_address>
**Null guard lists cause server errors**

When a create or update request submits `guards: null`, dRF accepts null because `allow_null=True`, then the model serializer passes `None` to the many-to-many manager’s `set()` method, which raises `TypeError` and returns a server error.

Remove `allow_null=True` so requests with `guards: null` are rejected during serializer validation.
</issue_to_address>

### Comment 6
<location path="pulp_service/pulp_service/app/models.py" line_range="95" />
<code_context>
+
+    TYPE = "andgate"
+
+    guards = models.ManyToManyField(ContentGuard, related_name="+", symmetrical=False)
+
+    def permit(self, request):
</code_context>
<issue_to_address>
**Concurrent guard edits combine policies**

When two API updates replace the same AndGate's guard list concurrently, two administrators PATCH the same AndGate from an initial membership of `{old}`; both read that membership, remove `old`, and add their respective new guards. Both requests succeed, but the final membership contains both new guards rather than either requested list, so the AndGate denies clients who satisfy only the intended final policy.

Serialize guard-list replacements by locking the AndGate row for the duration of each update, or use another concurrency-control mechanism that prevents overlapping replacements.

Also at `pulp_service/pulp_service/app/serializers.py:77`.
</issue_to_address>

### Comment 7
<location path="pulp_service/pulp_service/tests/unit/test_and_gate_content_guard.py" line_range="91-92" />
<code_context>
+    and_gate.guards.add(header_guard, feature_guard)
+
+    try:
+        with pytest.raises(AuthenticationError):
+            and_gate.permit(SimpleNamespace(headers={}))
+    finally:
</code_context>
<issue_to_address>
**Skipped guards go unnoticed**

When a `permit()` implementation short-circuits when it encounters an `AuthenticationError`, when a sub-guard raises `AuthenticationError` before another guard, `test_authentication_error_preferred_over_permission_error_both_orders` passes even if `permit()` stops immediately and never evaluates the later guard; its assertions check only the raised exception type.

Retain the sub-guard mocks in each case and assert that every sub-guard's `permit()` was called.
</issue_to_address>

Sourcery assessment

Needs a human reviewer. 7 findings to address first, and if the guard aggregation mistakenly permits when any sub-guard denies, content protected by the other guards could be exposed to unauthorized clients; reverting would stop future access but cannot retract content already retrieved. The empty-guard case denies access, so this does not change existing guard policies by default.

Blocking findings: pulp_service/pulp_service/app/models.py:95, pulp_service/pulp_service/app/models.py:110, pulp_service/pulp_service/app/models.py:117, pulp_service/pulp_service/app/models.py:125, pulp_service/pulp_service/app/serializers.py:83, and 2 more

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook