From 006b1aaf068fa8d1ccf070d9475b0a9f9d508eac Mon Sep 17 00:00:00 2001
From: Moritz <moritz.m@local-it.org>
Date: Wed, 21 Jan 2026 08:02:31 +0100
Subject: [PATCH] Replace Mix.env() with Config.sql_sandbox?() in SystemActor

Use Application config instead of Mix.env() to prevent
runtime crashes in production releases where Mix is not available
---
 lib/mv/helpers/system_actor.ex | 43 +++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/lib/mv/helpers/system_actor.ex b/lib/mv/helpers/system_actor.ex
index 1349c80..7a8ab8b 100644
--- a/lib/mv/helpers/system_actor.ex
+++ b/lib/mv/helpers/system_actor.ex
@@ -39,13 +39,18 @@ defmodule Mv.Helpers.SystemActor do
 
   The system actor should NEVER be used for user-initiated actions. It is
   only for systemic operations that must bypass user permissions.
+
+  The system user is created without a password (`hashed_password = nil`) and
+  without an OIDC ID (`oidc_id = nil`) to prevent login. This ensures the
+  system user cannot be used for authentication, even if credentials are
+  somehow obtained.
   """
 
   use Agent
 
   require Ash.Query
 
-  @system_user_email "system@mila.local"
+  alias Mv.Config
 
   @doc """
   Starts the SystemActor Agent.
@@ -106,8 +111,8 @@ defmodule Mv.Helpers.SystemActor do
   """
   @spec get_system_actor_result() :: {:ok, Mv.Accounts.User.t()} | {:error, term()}
   def get_system_actor_result do
-    # In test environment, always load directly to avoid Agent/Sandbox issues
-    if Mix.env() == :test do
+    # In test environment (SQL sandbox), always load directly to avoid Agent/Sandbox issues
+    if Config.sql_sandbox?() do
       try do
         {:ok, load_system_actor()}
       rescue
@@ -161,7 +166,10 @@ defmodule Mv.Helpers.SystemActor do
   """
   @spec invalidate_cache() :: :ok
   def invalidate_cache do
-    Agent.update(__MODULE__, fn _state -> nil end)
+    case Process.whereis(__MODULE__) do
+      nil -> :ok
+      _pid -> Agent.update(__MODULE__, fn _state -> nil end)
+    end
   end
 
   @doc """
@@ -181,13 +189,20 @@ defmodule Mv.Helpers.SystemActor do
 
   """
   @spec system_user_email() :: String.t()
-  def system_user_email, do: @system_user_email
+  def system_user_email, do: system_user_email_config()
+
+  # Returns the system user email from environment variable or default
+  # This allows configuration via SYSTEM_ACTOR_EMAIL env var
+  @spec system_user_email_config() :: String.t()
+  defp system_user_email_config do
+    System.get_env("SYSTEM_ACTOR_EMAIL") || "system@mila.local"
+  end
 
   # Loads the system actor from the database
   # First tries to find system@mila.local, then falls back to admin user
   @spec load_system_actor() :: Mv.Accounts.User.t() | no_return()
   defp load_system_actor do
-    case find_user_by_email(@system_user_email) do
+    case find_user_by_email(system_user_email_config()) do
       {:ok, user} when not is_nil(user) ->
         load_user_with_role(user)
 
@@ -226,7 +241,7 @@ defmodule Mv.Helpers.SystemActor do
   # Handles fallback error - creates test actor or raises
   @spec handle_fallback_error(String.t()) :: Mv.Accounts.User.t() | no_return()
   defp handle_fallback_error(message) do
-    if Mix.env() == :test do
+    if Config.sql_sandbox?() do
       create_test_system_actor()
     else
       raise "Failed to load system actor: #{message}"
@@ -327,12 +342,15 @@ defmodule Mv.Helpers.SystemActor do
   end
 
   # Creates system user with admin role assigned
+  # SECURITY: System user is created without password (hashed_password = nil) and
+  # without OIDC ID (oidc_id = nil) to prevent login. This user is ONLY for
+  # internal system operations via SystemActor and should never be used for authentication.
   @spec create_system_user_with_role(Mv.Authorization.Role.t()) ::
           Mv.Accounts.User.t() | no_return()
   defp create_system_user_with_role(admin_role) do
     alias Mv.Accounts
 
-    Accounts.create_user!(%{email: @system_user_email},
+    Accounts.create_user!(%{email: system_user_email_config()},
       upsert?: true,
       upsert_identity: :unique_email
     )
@@ -343,11 +361,18 @@ defmodule Mv.Helpers.SystemActor do
   end
 
   # Finds a user by email address
+  # SECURITY: Uses authorize?: false for bootstrap lookup only.
+  # This is necessary because we need to find the system/admin user before
+  # we can load the system actor. If User policies require an actor, this
+  # would create a chicken-and-egg problem. This is safe because:
+  # 1. We only query by email (no sensitive data exposed)
+  # 2. This is only used during system actor initialization (bootstrap phase)
+  # 3. Once system actor is loaded, all subsequent operations use proper authorization
   @spec find_user_by_email(String.t()) :: {:ok, Mv.Accounts.User.t() | nil} | {:error, term()}
   defp find_user_by_email(email) do
     Mv.Accounts.User
     |> Ash.Query.filter(email == ^email)
-    |> Ash.read_one(domain: Mv.Accounts)
+    |> Ash.read_one(domain: Mv.Accounts, authorize?: false)
   end
 
   # Loads a user with their role preloaded (required for authorization)