CHANGELOG.md

@@ -5,11 +5,6 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [Unreleased]
-
-### Fixed
-- **Runtime ENV handling** – Empty or invalid environment variables (e.g. `SMTP_PORT=`, `PORT=`, `POOL_SIZE=`, `DATABASE_PORT=`) no longer cause `ArgumentError` at boot. Instead raises clear errors for required vars set but empty (e.g. DATABASE_HOST, PHX_HOST/DOMAIN, SECRET_KEY_BASE).
-
 ## [1.1.1] - 2026-03-16
 
 ### Added

CODE_GUIDELINES.md

@@ -2177,14 +2177,6 @@ mix phx.gen.secret
 mix phx.gen.secret
 ```
 
-**Runtime configuration (config/runtime.exs):**
-
-- Production config is loaded from `config/runtime.exs` at boot (releases and `mix phx.server`). Environment variables are read via helpers so that **empty or invalid values do not cause cryptic crashes** (e.g. `ArgumentError` from `String.to_integer("")`).
-- **Helpers used:** `get_env_or_file` / `get_env_or_file!` (with `_FILE` support); `get_env_required` (required vars: raises if missing or empty after trim); `get_env_non_empty` (optional string: empty treated as unset, returns default); `parse_positive_integer` (PORT, POOL_SIZE, SMTP_PORT: empty or invalid → default).
-- **Required vars** (e.g. DATABASE_HOST, PHX_HOST/DOMAIN, SECRET_KEY_BASE): if set but empty, the app raises at boot with a clear message including “(Variable X is set but empty.)”.
-- **Optional numeric vars** (PORT, POOL_SIZE, SMTP_PORT, DATABASE_PORT): empty or invalid value is treated as “unset” and the documented default is used (e.g. PORT=4000, SMTP_PORT=587).
-- When adding new ENV in `runtime.exs`, use these helpers instead of raw `System.get_env(...)` and `String.to_integer(...)` so that misconfigured or empty variables fail fast with clear errors.
-
 ### 5.6 Security Headers
 
 **Configure Security Headers:**

config/runtime.exs

@@ -32,91 +32,11 @@ get_env_or_file = fn var_name, default ->
   end
 end
 
-# Same as get_env_or_file but raises if the value is not set or empty (after trim).
-# Empty values lead to unclear runtime errors; failing at boot with a clear message is preferred.
+# Same as get_env_or_file but raises if the value is not set
 get_env_or_file! = fn var_name, error_message ->
   case get_env_or_file.(var_name, nil) do
-    nil ->
-      raise error_message
-
-    value when is_binary(value) ->
-      trimmed = String.trim(value)
-
-      if trimmed == "" do
-        raise """
-        #{error_message}
-        (Variable #{var_name} or #{var_name}_FILE is set but the value is empty.)
-        """
-      else
-        trimmed
-      end
-
-    value ->
-      value
-  end
-end
-
-# Returns default when env_value is nil, empty after trim, or not a valid positive integer.
-# Used for PORT, POOL_SIZE, SMTP_PORT to avoid ArgumentError on empty or invalid values.
-parse_positive_integer = fn env_value, default ->
-  case env_value do
-    nil ->
-      default
-
-    v when is_binary(v) ->
-      case String.trim(v) do
-        "" ->
-          default
-
-        trimmed ->
-          case Integer.parse(trimmed) do
-            {n, _} when n > 0 -> n
-            _ -> default
-          end
-      end
-
-    _ ->
-      default
-  end
-end
-
-# Returns default when the key is missing or the value is empty (after trim).
-# Use for optional string ENV vars (e.g. DATABASE_PORT) so empty string is treated as "unset".
-get_env_non_empty = fn key, default ->
-  case System.get_env(key) do
-    nil ->
-      default
-
-    v when is_binary(v) ->
-      trimmed = String.trim(v)
-      if trimmed == "", do: default, else: trimmed
-
-    v ->
-      v
-  end
-end
-
-# Returns the trimmed value when set and non-empty; otherwise raises with error_message.
-# Use for required vars (DATABASE_HOST, etc.) so "set but empty" fails at boot with a clear message.
-get_env_required = fn key, error_message ->
-  case System.get_env(key) do
-    nil ->
-      raise error_message
-
-    v when is_binary(v) ->
-      trimmed = String.trim(v)
-
-      if trimmed == "" do
-        raise """
-        #{error_message}
-        (Variable #{key} is set but empty.)
-        """
-      else
-        trimmed
-      end
-
-    v ->
-      v
+    nil -> raise error_message
+    value -> value
   end
 end
 
@@ -129,14 +49,12 @@ build_database_url = fn ->
     nil ->
       # Build URL from separate components
       host =
-        get_env_required.("DATABASE_HOST", """
-        DATABASE_HOST is required when DATABASE_URL is not set.
-        """)
+        System.get_env("DATABASE_HOST") ||
+          raise "DATABASE_HOST is required when DATABASE_URL is not set"
 
       user =
-        get_env_required.("DATABASE_USER", """
-        DATABASE_USER is required when DATABASE_URL is not set.
-        """)
+        System.get_env("DATABASE_USER") ||
+          raise "DATABASE_USER is required when DATABASE_URL is not set"
 
       password =
         get_env_or_file!.("DATABASE_PASSWORD", """
@@ -144,11 +62,10 @@ build_database_url = fn ->
         """)
 
       database =
-        get_env_required.("DATABASE_NAME", """
-        DATABASE_NAME is required when DATABASE_URL is not set.
-        """)
+        System.get_env("DATABASE_NAME") ||
+          raise "DATABASE_NAME is required when DATABASE_URL is not set"
 
-      port = get_env_non_empty.("DATABASE_PORT", "5432")
+      port = System.get_env("DATABASE_PORT", "5432")
 
       # URL-encode the password to handle special characters
       encoded_password = URI.encode_www_form(password)
@@ -185,7 +102,7 @@ if config_env() == :prod do
   config :mv, Mv.Repo,
     # ssl: true,
     url: database_url,
-    pool_size: parse_positive_integer.(System.get_env("POOL_SIZE"), 10),
+    pool_size: String.to_integer(System.get_env("POOL_SIZE") || "10"),
     socket_options: maybe_ipv6
 
   # The secret key base is used to sign/encrypt cookies and other secrets.
@@ -203,14 +120,11 @@ if config_env() == :prod do
   # PHX_HOST or DOMAIN can be used to set the host for the application.
   # DOMAIN is commonly used in deployment environments (e.g., Portainer templates).
   host =
-    get_env_non_empty.("PHX_HOST", nil) ||
-      get_env_non_empty.("DOMAIN", nil) ||
-      raise """
-      Please define the PHX_HOST or DOMAIN environment variable.
-      (Variable may be set but empty.)
-      """
+    System.get_env("PHX_HOST") ||
+      System.get_env("DOMAIN") ||
+      raise "Please define the PHX_HOST or DOMAIN environment variable."
 
-  port = parse_positive_integer.(System.get_env("PORT"), 4000)
+  port = String.to_integer(System.get_env("PORT") || "4000")
 
   config :mv, :dns_cluster_query, System.get_env("DNS_CLUSTER_QUERY")
 
@@ -316,7 +230,11 @@ if config_env() == :prod do
   smtp_host_env = System.get_env("SMTP_HOST")
 
   if smtp_host_env && String.trim(smtp_host_env) != "" do
-    smtp_port_env = parse_positive_integer.(System.get_env("SMTP_PORT"), 587)
+    smtp_port_env =
+      case System.get_env("SMTP_PORT") do
+        nil -> 587
+        v -> String.to_integer(String.trim(v))
+      end
 
     smtp_password_env =
       case System.get_env("SMTP_PASSWORD") do

docs/smtp-configuration-concept.md

@@ -42,8 +42,6 @@ When an ENV variable is set, the corresponding Settings field is read-only in th
 | Sender name    | `MAIL_FROM_NAME`       | `smtp_from_name`    | Display name in "From" header (default: Mila)|
 | Sender email   | `MAIL_FROM_EMAIL`      | `smtp_from_email`   | Address in "From" header; must match SMTP user on most servers |
 
-**Boot-time ENV handling:** In `config/runtime.exs`, if `SMTP_PORT` is set but empty or invalid, it is treated as unset and default 587 is used. This avoids startup crashes (e.g. `ArgumentError` from `String.to_integer("")`) when variables are misconfigured in deployment.
-
 **Important:** On most SMTP servers (e.g. Postfix with strict relay policies) the sender email (`smtp_from_email`) must be the same address as `smtp_username` or an alias that is owned by that account.
 
 **Settings UI:** The form uses three rows on wide viewports: host, port, TLS/SSL | username, password | sender email, sender name. Content width is limited by the global settings wrapper (see `DESIGN_GUIDELINES.md` §6.4).

lib/mv/mailer.ex

@@ -100,11 +100,7 @@ defmodule Mv.Mailer do
   """
   @spec smtp_config() :: keyword()
   def smtp_config do
-    # In test we use Swoosh.Adapters.Test; do not override with SMTP opts or emails would not land in the test mailbox.
-    adapter = Application.get_env(:mv, __MODULE__, []) |> Keyword.get(:adapter)
-
-    if Mv.Config.smtp_configured?() and not boot_smtp_configured?() and
-         adapter != Swoosh.Adapters.Test do
+    if Mv.Config.smtp_configured?() and not boot_smtp_configured?() do
       verify_mode =
         if Application.get_env(:mv, :smtp_verify_peer, false),
           do: :verify_peer,

test/mv_web/controllers/auth_controller_test.exs

@@ -316,11 +316,9 @@ defmodule MvWeb.AuthControllerTest do
 
         # When OIDC-only is enabled, password sign-in must not succeed (no redirect to sign_in_with_token).
         case result do
-          {:error, {:redirect, opts}} when is_map(opts) ->
-            to_path = Map.get(opts, :to) || Map.get(opts, "to") || ""
-
-            refute to_path =~ "sign_in_with_token",
-                   "Expected password sign-in to be rejected when OIDC-only, got redirect to: #{to_path}"
+          {:error, {:redirect, %{to: to}}} ->
+            refute to =~ "sign_in_with_token",
+                   "Expected password sign-in to be rejected when OIDC-only, got redirect to: #{to}"
 
           _ ->
             # LiveView re-rendered (e.g. with flash error) instead of redirecting to success
@@ -400,24 +398,8 @@ defmodule MvWeb.AuthControllerTest do
 
     test "returns 200 when OIDC-only but OIDC not configured", %{conn: authenticated_conn} do
       {:ok, settings} = Membership.get_settings()
-
-      prev = %{
-        oidc_only: settings.oidc_only,
-        oidc_client_id: settings.oidc_client_id,
-        oidc_base_url: settings.oidc_base_url,
-        oidc_redirect_uri: settings.oidc_redirect_uri,
-        oidc_client_secret: settings.oidc_client_secret
-      }
-
-      # Set OIDC-only but leave OIDC unconfigured so the plug does not redirect.
-      {:ok, _} =
-        Membership.update_settings(settings, %{
-          oidc_only: true,
-          oidc_client_id: nil,
-          oidc_base_url: nil,
-          oidc_redirect_uri: nil,
-          oidc_client_secret: nil
-        })
+      original_oidc_only = Map.get(settings, :oidc_only, false)
+      {:ok, _} = Membership.update_settings(settings, %{oidc_only: true})
 
       try do
         conn = build_unauthenticated_conn(authenticated_conn)
@@ -425,7 +407,7 @@ defmodule MvWeb.AuthControllerTest do
         assert conn.status == 200
       after
         {:ok, s} = Membership.get_settings()
-        Membership.update_settings(s, prev)
+        Membership.update_settings(s, %{oidc_only: original_oidc_only})
       end
     end