[High] [Lens 13 — Testing] — The fixture builds an owner-less deployment, so the negative test asserts nothing

Under UN-2202 created_by is audit-only; creator access flows through an OWNER membership row. The base class does exactly that for self.workflow (tenant_account_v2/tests.py:98-99), but _api_deployment creates no membership row — and no post_save signal supplies one (tenant_account_v2/signals.py has only post_delete receivers; no ResourceMembership.objects.create exists outside tests and migrations).

So every deployment built here is visible to nobody, including self.owner. Two consequences:

1. test_unshared_deployment_stays_invisible (:95-97) would still pass if Q(pk__in=member_ids) were deleted outright from api_v2/models.py:52. It reads as a negative control but constrains nothing.
2. The likeliest regression from this refactor — the owner can no longer see their own executions — has no test. The PR body claims coverage of "owner, co-owner, direct share, group share and shared_to_org"; only the last two are actually asserted.

Suggested fix: add the OWNER membership row in _api_deployment, then add test_owner_sees_own_executions, a direct-viewer test via the base module's currently-unused _add_viewers (tenant_account_v2/tests.py:61-67), and a co-owner test. Re-check that test_unshared_deployment_stays_invisible still passes once the deployment is genuinely owned — that version is the one that tests something.

Confidence: High.

[Medium] [Lens 13 — Testing] — Two of the three ORed branches in the rewritten for_user are never satisfied

_execution() hardcodes pipeline_id=deployment.id, and all six tests build their execution through it. Against models/execution.py:83:

final_filter = (workflow_filter & Q(pipeline_id__isnull=True)) | deployment_filter

• workflow_filter & Q(pipeline_id__isnull=True) can never be true here, so the swap to Workflow.objects.for_user at models/execution.py:69 is completely unexercised — despite workflow-level executions being a primary flow, and despite being exactly the case in row 3 of the PR's manual test table.
• pipeline_filter (models/execution.py:75) is never satisfied — ETL/TASK pipeline executions have no coverage in either direction.

A regression hiding workflow-level executions from their owner, or leaking pipeline executions org-wide, ships green.

Suggested fix: parameterise _execution(deployment=None) so pipeline_id can be None; add (a) a group-shared self.workflow case asserting the member sees the workflow-level execution and the outsider does not, and (b) a Pipeline-backed twin of the existing grant/deny pair.

Confidence: High.

[High] [Lens 13 — Testing] — No test reaches HTTP; the 403 and the entire export action are unverified

_log_queryset hand-constructs the viewset and calls get_queryset() directly, bypassing as_view(), initialize_request, initial(), check_permissions(), dispatch() and the DRF exception handler. Consequences:

• The PR's headline claim — GET /execution/<id>/logs/ returns 403 — has no automated assertion anywhere. The test proves PermissionDenied is raised, not that the endpoint returns 403; that translation is entirely a dispatch()-level behaviour the test never enters.
• The permission_classes change from [IsAuthenticated, IsOwner] to [IsAuthenticated] is unpinned.
• export has zero coverage. It inherits the gate only incidentally, via self.filter_queryset(self.get_queryset()) at execution_log_view.py:73, and nothing pins that coupling. A later streaming fast path, a .values() rewrite, an async hand-off, or hoisting the row-cap above the queryset build silently reopens bulk CSV/JSON exfiltration of any execution's logs by id — with CI green. This is the higher-impact of the two endpoints and the one with no test.

A future try/except around get_queryset() (a common pagination-hardening change) would downgrade the gate to an empty 200, and every test here still passes.

Suggested fix: route through as_view({"get": "list"}) and as_view({"get": "export"}) with force_authenticate, asserting 403 for the denied user and 200 for the shared user. The base module already establishes this idiom at tenant_account_v2/tests.py:344-348. Keep the existing unit-level tests as fast checks — they are fine, just not sufficient on their own for an authz change.

Confidence: High.

[Medium] [Lens 2 — Architectural fit, Lens 4 — Security] — Dropping IsOwner leaves a ModelViewSet whose write handlers carry only IsAuthenticated

Removing IsOwner is correct — it implements only has_object_permission, which DRF never invokes on list/export, exactly as the commit message says. But the class is still a ModelViewSet (:30), so it retains create / update / partial_update / destroy.

Nothing is exploitable today because only list and export are routed (workflow_manager/execution/urls.py:15-16, workflow_v2/urls/workflow.py:23-24). The issue is the pattern: create never calls get_queryset(), so the new gate does not cover it, and the object-level class that used to sit in permission_classes is gone. "Drop the permission class, gate the queryset instead" is only safe on a read-only viewset, and nothing in the code says so — the next engineer who copies this onto a writable viewset inherits an unguarded write path.

Suggested fix: change the base to viewsets.ReadOnlyModelViewSet. One word, removes the latent write surface, and makes the queryset-gating pattern self-evidently sufficient. ExecutionLog is written only by workers and its fields are editable=False, so nothing legitimate is lost.

Confidence: High.

[Medium] [Lens 16 — Doc accuracy] — Docstring in this block claims org scoping comes from the view; it now comes only from this manager

Line 46 of this same docstring reads:

Service accounts see all executions (org-scoped by view).

That is false, and this diff makes it more dangerous:

• The service-account branch org-scopes itself at :54-58; the view does not do it.
• The only caller, ExecutionViewSet, sets filter_backends = [DjangoFilterBackend, DeterministicOrderingFilter] (workflow_manager/execution/views/execution.py:37), which replaces DEFAULT_FILTER_BACKENDS and therefore drops OrganizationFilterBackend — whose own docstring states "Viewsets MUST NOT override filter_backends" (utils/filters/organization_filter.py:31-33).
• This diff relocates non-admin tenant isolation into the three delegated managers (see the new comment at :66-68), making for_user the sole tenant boundary for /execution/.

A maintainer trusting line 46 could remove the in-manager org filter and open a cross-tenant leak.

Suggested fix: rewrite line 46 to something like "Service accounts and org admins see all executions in the current organization; org scoping is enforced here, not by the view." Optionally, append to rather than replace filter_backends on ExecutionViewSet to restore the defence-in-depth.

For the record, the two comments this diff adds (:66-68 here and execution_log_view.py:42-44) are both accurate — I checked each claim against the code.

Confidence: High.

[Medium] [Lens 13 — Testing] — No cross-organization test, on the change that makes shared_to_org load-bearing for execution visibility

This comment's claim — "Those managers are org-scoped, so no explicit org arg" — is correct today (DefaultOrganizationManagerMixin.get_queryset filters organization=UserContext.get_organization(), utils/models/organization_mixin.py:27-30). But nothing in the suite pins it.

WorkflowExecutionManager extends BaseModelManager (:30) and is not org-scoped. After this change, the only thing stopping Q(shared_to_org=True) inside APIDeployment.objects.for_user (api_v2/models.py:53) from matching another tenant's deployments is that mixin reading a thread-local. The suite runs with exactly one Organization, set once in GroupSharingTestBase.setUp. Dropping the mixin from any of the three sub-managers would pass every test in the new file.

This PR is precisely what makes shared_to_org load-bearing for execution visibility, so it is the right PR to pin it.

Suggested fix: one test — create a second Organization with a shared_to_org=True APIDeployment and an execution, assert self.outsider (org A) neither sees it via for_user nor can read its logs. The two-org fixture pattern already exists at tenant_account_v2/tests.py:207-216.

Confidence: Medium.

[Medium] [Lens 9 — Performance] — for_user now costs three extra uncached admin lookups per call, on a polled path

for_user resolves is_user_organization_admin(user) at :60, then delegates to Workflow.objects.for_user, APIDeployment.objects.for_user and Pipeline.objects.for_user — each of which re-resolves the same predicate as its second statement.

OrganizationMemberService.is_user_organization_admin is uncached (tenant_account_v2/organization_member_service.py:31-45): one OrganizationMember.objects.get() round-trip plus an AuthenticationController() construction per call. Net +3 queries per for_user call for every non-admin user.

Where that lands:

• /execution/ (paginated list).
• /execution/<id>/logs/ — which the Logs modal polls on a refresh interval, and which this PR newly routes through for_user.
• ExecutionViewSet calls for_user twice on retrieve (workflow_manager/execution/views/execution.py:30 and :44), so that path pays it six times.

Separately, each subquery went from a single indexed lookup on resource_membership to a SELECT DISTINCT over the resource table wrapping 2–3 nested INs. The subquery-not-materialised property that UN-2202 (#1797) deliberately tuned for this exact call site is preserved, so plan caching still holds — the added cost is depth, not an N+1.

Suggested fix: memoize the admin predicate. permissions/permission.py:57-67 already has exactly this cache, but keyed on request, which the managers never see — simplest is a per-user memo inside OrganizationMemberService.is_user_organization_admin.

Confidence: High on the mechanism, Medium on impact — an EXPLAIN on a realistic org plus the modal's poll interval would settle whether this is Medium or Low.