fix: CustomField policies, no system-actor fallback, guidelines

- Tests and UI pass actor for CustomField create/read/destroy; seeds use actor
- Member required-custom-fields validation uses context.actor only (no fallback)
- CODE_GUIDELINES: add rule forbidding system-actor fallbacks

CODE_GUIDELINES.md

@@ -683,6 +683,13 @@ end
 - Falls back to admin user from seeds if system user doesn't exist
 - Should NEVER be used for user-initiated actions (only systemic operations)
 
+**DO NOT use system actor as a fallback:**
+
+- **Never** fall back to `Mv.Helpers.SystemActor.get_system_actor()` when an actor is missing or nil (e.g. in validations, changes, or when reading from context).
+- Fallbacks hide bugs (callers forget to pass actor) and can cause privilege escalation (unauthenticated or low-privilege paths run with system rights).
+- If no actor is available, fail explicitly (validation error, Forbidden, or clear error message). Fix the caller to pass the correct actor instead of adding a fallback.
+- Use system actor only where the operation is **explicitly** a systemic operation (see list above); never as a "safety net" when actor is absent.
+
 **User Mode vs System Mode:**
 
 - **User Mode**: User-initiated actions use the actual user actor, policies are enforced
@@ -1658,6 +1665,30 @@ end
 
 ## 5. Security Guidelines
 
+### 5.0 No system-actor fallbacks (mandatory)
+
+**Do not use the system actor as a fallback when an actor is missing.**
+
+Examples of forbidden patterns:
+
+```elixir
+# ❌ FORBIDDEN - Fallback to system actor when actor is nil
+actor = Map.get(changeset.context, :actor) || Mv.Helpers.SystemActor.get_system_actor()
+
+# ❌ FORBIDDEN - "Safety" fallback in validations, changes, or helpers
+actor = opts[:actor] || Mv.Helpers.SystemActor.get_system_actor()
+
+# ❌ FORBIDDEN - Default actor in function options
+def list_something(opts \\ []) do
+  actor = Keyword.get(opts, :actor) || Mv.Helpers.SystemActor.get_system_actor()
+  # ...
+end
+```
+
+**Why:** Fallbacks hide missing-actor bugs and can lead to privilege escalation (e.g. a request without actor would run with system privileges). Always require the caller to pass the actor for user-facing or context-dependent operations; if none is available, return an error or fail validation instead of using the system actor.
+
+**Allowed:** Use the system actor only where the operation is **by design** a systemic operation (e.g. email sync, seeds, test fixtures, background jobs) and you explicitly call `SystemActor.get_system_actor()` at that call site—never as a fallback when `actor` is nil or absent.
+
 ### 5.1 Authentication & Authorization
 
 **Use AshAuthentication:**

lib/membership/member.ex

@@ -471,11 +471,12 @@ defmodule Mv.Membership.Member do
       end
     end
 
-    # Validate required custom fields
-    validate fn changeset, _ ->
+    # Validate required custom fields (actor from validation context only; no fallback)
+    validate fn changeset, context ->
       provided_values = provided_custom_field_values(changeset)
+      actor = context.actor
 
-      case Mv.Membership.list_required_custom_fields() do
+      case Mv.Membership.list_required_custom_fields(actor: actor) do
         {:ok, required_custom_fields} ->
           missing_fields = missing_required_fields(required_custom_fields, provided_values)
 

lib/membership/membership.ex

@@ -155,23 +155,31 @@ defmodule Mv.Membership do
   Lists only required custom fields.
 
   This is an optimized version that filters at the database level instead of
-  loading all custom fields and filtering in memory.
+  loading all custom fields and filtering in memory. Requires an actor for
+  authorization (CustomField read policy).
+
+  ## Options
+
+    - `:actor` - Required. The actor for authorization (e.g. current user).
+      All roles can read CustomField; actor must have a valid role.
 
   ## Returns
 
     - `{:ok, required_custom_fields}` - List of required custom fields
-    - `{:error, error}` - Error reading custom fields
+    - `{:error, error}` - Error reading custom fields (e.g. Forbidden when no actor)
 
   ## Examples
 
-      iex> {:ok, required_fields} = Mv.Membership.list_required_custom_fields()
+      iex> {:ok, required_fields} = Mv.Membership.list_required_custom_fields(actor: actor)
       iex> Enum.all?(required_fields, & &1.required)
       true
   """
-  def list_required_custom_fields do
+  def list_required_custom_fields(opts \\ []) do
+    actor = Keyword.get(opts, :actor)
+
     Mv.Membership.CustomField
     |> Ash.Query.filter(expr(required == true))
-    |> Ash.read(domain: __MODULE__)
+    |> Ash.read(domain: __MODULE__, actor: actor)
   end
 
   @doc """

lib/mv_web/live/custom_field_live/index_component.ex

@@ -176,6 +176,13 @@ defmodule MvWeb.CustomFieldLive.IndexComponent do
     """
   end
 
+  defp stream_custom_fields(actor) do
+    case Ash.read(Mv.Membership.CustomField, actor: actor) do
+      {:ok, custom_fields} -> custom_fields
+      {:error, _} -> []
+    end
+  end
+
   @impl true
   def update(assigns, socket) do
     # Track previous show_form state to detect when form is closed
@@ -207,7 +214,7 @@ defmodule MvWeb.CustomFieldLive.IndexComponent do
      |> assign_new(:show_delete_modal, fn -> false end)
      |> assign_new(:custom_field_to_delete, fn -> nil end)
      |> assign_new(:slug_confirmation, fn -> "" end)
-     |> stream(:custom_fields, Ash.read!(Mv.Membership.CustomField), reset: true)}
+     |> stream(:custom_fields, stream_custom_fields(assigns[:actor]), reset: true)}
   end
 
   @impl true
@@ -226,7 +233,8 @@ defmodule MvWeb.CustomFieldLive.IndexComponent do
 
   @impl true
   def handle_event("edit_custom_field", %{"id" => id}, socket) do
-    custom_field = Ash.get!(Mv.Membership.CustomField, id)
+    actor = socket.assigns[:actor]
+    custom_field = Ash.get!(Mv.Membership.CustomField, id, actor: actor)
 
     # Only send event if form was not already open
     if not socket.assigns[:show_form] do
@@ -242,7 +250,13 @@ defmodule MvWeb.CustomFieldLive.IndexComponent do
 
   @impl true
   def handle_event("prepare_delete", %{"id" => id}, socket) do
-    custom_field = Ash.get!(Mv.Membership.CustomField, id, load: [:assigned_members_count])
+    actor = socket.assigns[:actor]
+
+    custom_field =
+      Ash.get!(Mv.Membership.CustomField, id,
+        load: [:assigned_members_count],
+        actor: actor
+      )
 
     {:noreply,
      socket
@@ -259,9 +273,10 @@ defmodule MvWeb.CustomFieldLive.IndexComponent do
   @impl true
   def handle_event("confirm_delete", _params, socket) do
     custom_field = socket.assigns.custom_field_to_delete
+    actor = socket.assigns[:actor]
 
     if socket.assigns.slug_confirmation == custom_field.slug do
-      case Ash.destroy(custom_field) do
+      case Ash.destroy(custom_field, actor: actor) do
         :ok ->
           send(self(), {:custom_field_deleted, custom_field})
 

lib/mv_web/live/global_settings_live.ex

@@ -130,6 +130,7 @@ defmodule MvWeb.GlobalSettingsLive do
           :if={@active_editing_section != :member_fields}
           module={MvWeb.CustomFieldLive.IndexComponent}
           id="custom-fields-component"
+          actor={@current_user}
         />
       </.form_section>
 

lib/mv_web/live/member_live/form.ex

@@ -226,7 +226,7 @@ defmodule MvWeb.MemberLive.Form do
   def mount(params, _session, socket) do
     # current_user should be set by on_mount hooks (LiveUserAuth + LiveHelpers)
     actor = current_actor(socket)
-    {:ok, custom_fields} = Mv.Membership.list_custom_fields()
+    {:ok, custom_fields} = Mv.Membership.list_custom_fields(actor: actor)
 
     initial_custom_field_values =
       Enum.map(custom_fields, fn cf ->

priv/repo/seeds.exs

@@ -118,10 +118,12 @@ for attrs <- [
         required: false
       }
     ] do
+  # Bootstrap: no admin user yet; CustomField create requires admin, so skip authorization
   Membership.create_custom_field!(
     attrs,
     upsert?: true,
-    upsert_identity: :unique_name
+    upsert_identity: :unique_name,
+    authorize?: false
   )
 end
 
@@ -594,7 +596,7 @@ end)
 
 # Create sample custom field values for some members
 all_members = Ash.read!(Membership.Member, actor: admin_user_with_role)
-all_custom_fields = Ash.read!(Membership.CustomField)
+all_custom_fields = Ash.read!(Membership.CustomField, actor: admin_user_with_role)
 
 # Helper function to find custom field by name
 find_field = fn name -> Enum.find(all_custom_fields, &(&1.name == name)) end

test/mv_web/components/member_filter_component_test.exs

@@ -16,8 +16,10 @@ defmodule MvWeb.Components.MemberFilterComponentTest do
 
   alias Mv.Membership.CustomField
 
-  # Helper to create a boolean custom field
+  # Helper to create a boolean custom field (uses system_actor - only admin can create)
   defp create_boolean_custom_field(attrs \\ %{}) do
+    system_actor = Mv.Helpers.SystemActor.get_system_actor()
+
     default_attrs = %{
       name: "test_boolean_#{System.unique_integer([:positive])}",
       value_type: :boolean
@@ -27,11 +29,13 @@ defmodule MvWeb.Components.MemberFilterComponentTest do
 
     CustomField
     |> Ash.Changeset.for_create(:create, attrs)
-    |> Ash.create!()
+    |> Ash.create!(actor: system_actor)
   end
 
-  # Helper to create a non-boolean custom field
+  # Helper to create a non-boolean custom field (uses system_actor - only admin can create)
   defp create_string_custom_field(attrs \\ %{}) do
+    system_actor = Mv.Helpers.SystemActor.get_system_actor()
+
     default_attrs = %{
       name: "test_string_#{System.unique_integer([:positive])}",
       value_type: :string
@@ -41,7 +45,7 @@ defmodule MvWeb.Components.MemberFilterComponentTest do
 
     CustomField
     |> Ash.Changeset.for_create(:create, attrs)
-    |> Ash.create!()
+    |> Ash.create!(actor: system_actor)
   end
 
   describe "rendering" do

test/mv_web/live/custom_field_live/deletion_test.exs

@@ -20,8 +20,9 @@ defmodule MvWeb.CustomFieldLive.DeletionTest do
 
   setup do
     system_actor = Mv.Helpers.SystemActor.get_system_actor()
+    admin_role = Mv.Fixtures.role_fixture("admin")
 
-    # Create admin user for testing
+    # Create admin user for testing (must have admin role to read/manage CustomField)
     {:ok, user} =
       Mv.Accounts.User
       |> Ash.Changeset.for_create(:register_with_password, %{
@@ -30,8 +31,17 @@ defmodule MvWeb.CustomFieldLive.DeletionTest do
       })
       |> Ash.create(actor: system_actor)
 
-    conn = log_in_user(build_conn(), user)
-    %{conn: conn, user: user}
+    {:ok, user} =
+      user
+      |> Ash.Changeset.for_update(:update, %{})
+      |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
+      |> Ash.update(actor: system_actor)
+
+    user_with_role = Ash.load!(user, :role, domain: Mv.Accounts, actor: system_actor)
+    conn = log_in_user(build_conn(), user_with_role)
+    # Use English locale so "Delete" link text matches in assertions
+    conn = Plug.Test.init_test_session(conn, Map.put(conn.private.plug_session, "locale", "en"))
+    %{conn: conn, user: user_with_role}
   end
 
   describe "delete button and modal" do
@@ -107,9 +117,8 @@ defmodule MvWeb.CustomFieldLive.DeletionTest do
       |> element("#delete-custom-field-modal form")
       |> render_change(%{"slug" => custom_field.slug})
 
-      # Confirm button should be enabled now (no disabled attribute)
-      html = render(view)
-      refute html =~ ~r/disabled(?:=""|(?!\w))/
+      # Confirm button should be enabled now (no disabled attribute on the confirm button)
+      refute has_element?(view, "#delete-custom-field-modal button.btn-error[disabled]")
     end
 
     test "delete button is disabled when slug doesn't match", %{conn: conn} do
@@ -126,9 +135,8 @@ defmodule MvWeb.CustomFieldLive.DeletionTest do
       |> element("#delete-custom-field-modal form")
       |> render_change(%{"slug" => "wrong-slug"})
 
-      # Button should be disabled
-      html = render(view)
-      assert html =~ ~r/disabled(?:=""|(?!\w))/
+      # Confirm button should be disabled
+      assert has_element?(view, "#delete-custom-field-modal button.btn-error[disabled]")
     end
   end
 
@@ -186,10 +194,8 @@ defmodule MvWeb.CustomFieldLive.DeletionTest do
       |> element("#delete-custom-field-modal form")
       |> render_change(%{"slug" => "wrong-slug"})
 
-      # Button should be disabled and we cannot click it
-      # The test verifies that the button is properly disabled in the UI
-      html = render(view)
-      assert html =~ ~r/disabled(?:=""|(?!\w))/
+      # Confirm button should be disabled and we cannot click it
+      assert has_element?(view, "#delete-custom-field-modal button.btn-error[disabled]")
 
       # Custom field should still exist since deletion couldn't proceed
       system_actor = Mv.Helpers.SystemActor.get_system_actor()

test/mv_web/member_live/index_test.exs

@@ -750,8 +750,10 @@ defmodule MvWeb.MemberLive.IndexTest do
   describe "boolean custom field filters" do
     alias Mv.Membership.CustomField
 
-    # Helper to create a boolean custom field
+    # Helper to create a boolean custom field (uses system actor for authorization)
     defp create_boolean_custom_field(attrs \\ %{}) do
+      system_actor = Mv.Helpers.SystemActor.get_system_actor()
+
       default_attrs = %{
         name: "test_boolean_#{System.unique_integer([:positive])}",
         value_type: :boolean
@@ -761,11 +763,13 @@ defmodule MvWeb.MemberLive.IndexTest do
 
       CustomField
       |> Ash.Changeset.for_create(:create, attrs)
-      |> Ash.create!()
+      |> Ash.create!(actor: system_actor)
     end
 
-    # Helper to create a non-boolean custom field
+    # Helper to create a non-boolean custom field (uses system actor for authorization)
     defp create_string_custom_field(attrs \\ %{}) do
+      system_actor = Mv.Helpers.SystemActor.get_system_actor()
+
       default_attrs = %{
         name: "test_string_#{System.unique_integer([:positive])}",
         value_type: :string
@@ -775,7 +779,7 @@ defmodule MvWeb.MemberLive.IndexTest do
 
       CustomField
       |> Ash.Changeset.for_create(:create, attrs)
-      |> Ash.create!()
+      |> Ash.create!(actor: system_actor)
     end
 
     test "mount initializes boolean_custom_field_filters as empty map", %{conn: conn} do
@@ -1016,6 +1020,7 @@ defmodule MvWeb.MemberLive.IndexTest do
 
     test "handle_params removes filter when custom field is deleted", %{conn: conn} do
       conn = conn_with_oidc_user(conn)
+      system_actor = Mv.Helpers.SystemActor.get_system_actor()
       boolean_field = create_boolean_custom_field()
 
       # Set up filter via URL
@@ -1026,8 +1031,8 @@ defmodule MvWeb.MemberLive.IndexTest do
       filters_before = state_before.socket.assigns.boolean_custom_field_filters
       assert filters_before[boolean_field.id] == true
 
-      # Delete the custom field
-      Ash.destroy!(boolean_field)
+      # Delete the custom field (requires actor with destroy permission)
+      Ash.destroy!(boolean_field, actor: system_actor)
 
       # Navigate again - filter should be removed since custom field no longer exists
       {:ok, view2, _html} =
@@ -1328,7 +1333,7 @@ defmodule MvWeb.MemberLive.IndexTest do
 
       members = [member_with_true, member_with_false, member_without_value]
       filters = %{to_string(boolean_field.id) => true}
-      all_custom_fields = Mv.Membership.CustomField |> Ash.read!()
+      all_custom_fields = Mv.Membership.CustomField |> Ash.read!(actor: system_actor)
 
       result =
         MvWeb.MemberLive.Index.apply_boolean_custom_field_filters(
@@ -1378,7 +1383,7 @@ defmodule MvWeb.MemberLive.IndexTest do
 
       members = [member_with_true, member_with_false, member_without_value]
       filters = %{to_string(boolean_field.id) => false}
-      all_custom_fields = Mv.Membership.CustomField |> Ash.read!()
+      all_custom_fields = Mv.Membership.CustomField |> Ash.read!(actor: system_actor)
 
       result =
         MvWeb.MemberLive.Index.apply_boolean_custom_field_filters(
@@ -1417,7 +1422,7 @@ defmodule MvWeb.MemberLive.IndexTest do
 
       members = [member1, member2]
       filters = %{}
-      all_custom_fields = Mv.Membership.CustomField |> Ash.read!()
+      all_custom_fields = Mv.Membership.CustomField |> Ash.read!(actor: system_actor)
 
       result =
         MvWeb.MemberLive.Index.apply_boolean_custom_field_filters(
@@ -1507,7 +1512,7 @@ defmodule MvWeb.MemberLive.IndexTest do
         to_string(boolean_field2.id) => true
       }
 
-      all_custom_fields = Mv.Membership.CustomField |> Ash.read!()
+      all_custom_fields = Mv.Membership.CustomField |> Ash.read!(actor: system_actor)
 
       result =
         MvWeb.MemberLive.Index.apply_boolean_custom_field_filters(
@@ -1538,7 +1543,7 @@ defmodule MvWeb.MemberLive.IndexTest do
 
       members = [member]
       filters = %{fake_id => true}
-      all_custom_fields = Mv.Membership.CustomField |> Ash.read!()
+      all_custom_fields = Mv.Membership.CustomField |> Ash.read!(actor: system_actor)
 
       result =
         MvWeb.MemberLive.Index.apply_boolean_custom_field_filters(