From 298a13c2e4d73f657874b7edd964428a5c3d5fb7 Mon Sep 17 00:00:00 2001
From: Moritz <moritz.m@local-it.org>
Date: Thu, 22 Jan 2026 21:36:09 +0100
Subject: [PATCH] Harden NoActor check with runtime environment guard

Add Mix.env() check to match?/3 for defense in depth.
Document NoActor pattern in CODE_GUIDELINES.md.
---
 CODE_GUIDELINES.md                            | 59 ++++++++++++++++
 lib/mv/authorization/checks/no_actor.ex       |  3 +-
 .../mv/authorization/checks/no_actor_test.exs | 67 +++++++++++++++++++
 3 files changed, 128 insertions(+), 1 deletion(-)
 create mode 100644 test/mv/authorization/checks/no_actor_test.exs

diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md
index 604e2af..778b69a 100644
--- a/CODE_GUIDELINES.md
+++ b/CODE_GUIDELINES.md
@@ -1664,6 +1664,65 @@ case Ash.read(Mv.Membership.Member, actor: actor) do
 end
 ```
 
+### 5.1a NoActor Pattern - Test Environment Only
+
+**IMPORTANT:** The `Mv.Authorization.Checks.NoActor` check is **ONLY for test environment**. It must NEVER be used in production.
+
+**What NoActor Does:**
+
+- Allows CRUD operations without an actor in **test environment only**
+- Denies all operations without an actor in **production/dev** (fail-closed)
+- Uses both compile-time and runtime guards to prevent accidental production use
+
+**Security Guards:**
+
+```elixir
+# Compile-time guard
+@allow_no_actor_bypass Mix.env() == :test
+
+# Runtime guard (double-check)
+def match?(nil, _context, _opts) do
+  if @allow_no_actor_bypass and Mix.env() == :test do
+    true  # Only in test
+  else
+    false  # Production/dev - fail-closed
+  end
+end
+```
+
+**Why This Pattern Exists:**
+
+- Test fixtures often need to create resources without an actor
+- Production operations MUST always have an actor for security
+- The double guard (compile-time + runtime) prevents config drift
+
+**NEVER Use NoActor in Production:**
+
+```elixir
+# ❌ BAD - Don't do this in production code
+Ash.create!(Member, attrs)  # No actor - will fail in prod
+
+# ✅ GOOD - Use admin actor for system operations
+admin_user = get_admin_user()
+Ash.create!(Member, attrs, actor: admin_user)
+```
+
+**Alternative: System Actor Pattern**
+
+For production system operations, use the System Actor Pattern (see Section 3.3) instead of NoActor:
+
+```elixir
+# System operations in production
+system_actor = get_system_actor()
+Ash.create!(Member, attrs, actor: system_actor)
+```
+
+**Testing:**
+
+- NoActor tests verify both compile-time and runtime guards
+- Tests ensure NoActor returns `false` in non-test environments
+- See `test/mv/authorization/checks/no_actor_test.exs`
+
 ### 5.2 Password Security
 
 **Use bcrypt for Password Hashing:**
diff --git a/lib/mv/authorization/checks/no_actor.ex b/lib/mv/authorization/checks/no_actor.ex
index f5eebb7..ffb4a9e 100644
--- a/lib/mv/authorization/checks/no_actor.ex
+++ b/lib/mv/authorization/checks/no_actor.ex
@@ -58,7 +58,8 @@ defmodule Mv.Authorization.Checks.NoActor do
   @impl true
   def match?(nil, _context, _opts) do
     # Actor is nil
-    if @allow_no_actor_bypass do
+    # Double-check: compile-time AND runtime environment
+    if @allow_no_actor_bypass and Mix.env() == :test do
       # Test environment: Allow all operations
       true
     else
diff --git a/test/mv/authorization/checks/no_actor_test.exs b/test/mv/authorization/checks/no_actor_test.exs
new file mode 100644
index 0000000..07efa0a
--- /dev/null
+++ b/test/mv/authorization/checks/no_actor_test.exs
@@ -0,0 +1,67 @@
+defmodule Mv.Authorization.Checks.NoActorTest do
+  @moduledoc """
+  Tests for the NoActor Ash Policy Check.
+
+  This check allows actions without an actor ONLY in test environment.
+  In production/dev, all operations without an actor are denied.
+  """
+  use ExUnit.Case, async: true
+
+  alias Mv.Authorization.Checks.NoActor
+
+  describe "match?/3" do
+    test "returns true when actor is nil in test environment" do
+      # In test environment, NoActor should allow operations
+      result = NoActor.match?(nil, %{}, [])
+      assert result == true
+    end
+
+    test "returns false when actor is present" do
+      actor = %{id: "user-123"}
+      result = NoActor.match?(actor, %{}, [])
+      assert result == false
+    end
+
+    test "has compile-time guard preventing production use" do
+      # The @allow_no_actor_bypass module attribute is set at compile time
+      # In test: true, in prod/dev: false
+      # This test verifies the guard exists (compile-time check)
+      # Runtime check is verified by the fact that match? checks Mix.env()
+      result = NoActor.match?(nil, %{}, [])
+
+      # In test environment, should allow
+      if Mix.env() == :test do
+        assert result == true
+      else
+        # In other environments, should deny
+        assert result == false
+      end
+    end
+
+    test "has runtime guard preventing production use" do
+      # The match? function checks Mix.env() at runtime
+      # This provides defense in depth against config drift
+      result = NoActor.match?(nil, %{}, [])
+
+      # Should match compile-time guard
+      if Mix.env() == :test do
+        assert result == true
+      else
+        assert result == false
+      end
+    end
+  end
+
+  describe "describe/1" do
+    test "returns description based on environment" do
+      description = NoActor.describe([])
+      assert is_binary(description)
+
+      if Mix.env() == :test do
+        assert description =~ "test environment"
+      else
+        assert description =~ "production/dev"
+      end
+    end
+  end
+end