Fix: Improve allowedHostnames .env configuration and fail-fast in production (#4482)

* Fix: Improve allowedHostnames .env configuration and fail-fast in production

Addresses GitHub issue #4480: .env app.allowedHostnames does not work as intended

## Problem
- CodeIgniter 4 cannot override array properties from .env
- Setting app.allowedHostnames.0, app.allowedHostnames.1 did NOT populate the array
- Application always fell back to 'localhost' silently in production
- Host header injection protection was effectively disabled

## Solution
1. Support comma-separated .env values: app.allowedHostnames = 'domain1.com,domain2.com'
2. Fail explicitly in production if not configured (throws RuntimeException)
3. Allow localhost fallback in development/testing with ERROR-level logging
4. Update documentation with clear setup instructions

## Changes
- app/Config/App.php: Parse comma-separated .env values, fail in production
- .env.example: Update format documentation
- INSTALL.md: Add prominent security section
- tests/Config/AppTest.php: Comprehensive tests for new behavior

Fixes #4480
Related: GHSA-jchf-7hr6-h4f3
---------

Co-authored-by: Ollama <ollama@steganos.dev>

.env.example

@@ -7,31 +7,20 @@ CI_ENVIRONMENT = production
 #--------------------------------------------------------------------
 # SECURITY: ALLOWED HOSTNAMES
 #--------------------------------------------------------------------
-# IMPORTANT: Whitelist of allowed hostnames to prevent Host Header 
+# CRITICAL: Whitelist of allowed hostnames to prevent Host Header
 # Injection attacks (GHSA-jchf-7hr6-h4f3).
 #
-# If not configured, the application will default to 'localhost',
-# which may break functionality in production.
+# REQUIRED IN PRODUCTION: Application will fail to start if not configured.
+# In development, falls back to 'localhost' with an error log.
 #
-# Configure this with all domains/subdomains that host your application:
-# - Primary domain
-# - WWW subdomain (if used)  
-# - Any alternative domains
+# Configure with comma-separated list of domains/subdomains:
+#   app.allowedHostnames = 'yourdomain.com,www.yourdomain.com'
 #
-# Examples:
-#   Single domain:
-#     app.allowedHostnames.0 = 'example.com'
+# For local development:
+#   app.allowedHostnames = 'localhost'
 #
-#   Multiple domains:
-#     app.allowedHostnames.0 = 'example.com'
-#     app.allowedHostnames.1 = 'www.example.com'
-#     app.allowedHostnames.2 = 'demo.opensourcepos.org'
-#
-# For localhost development:
-#     app.allowedHostnames.0 = 'localhost'
-#
-# Note: Do not include the protocol (http/https) or port number.
-#app.allowedHostnames.0 = ''
+# Note: Do not include protocol (http/https) or port numbers.
+app.allowedHostnames = ''
 
 #--------------------------------------------------------------------
 # DATABASE

INSTALL.md

@@ -8,26 +8,36 @@
 
 ## Security Configuration
 
-### Allowed Hostnames (Required for Production)
+### Allowed Hostnames (REQUIRED for Production)
 
-OpenSourcePOS validates the Host header against a whitelist to prevent Host Header Injection attacks (GHSA-jchf-7hr6-h4f3). **You must configure this for production deployments.**
+⚠️ **CRITICAL**: OpenSourcePOS validates the Host header to prevent Host Header Injection attacks (GHSA-jchf-7hr6-h4f3). **You MUST configure `app.allowedHostnames` for production deployments. If not configured, the application will fail to start.**
 
-Add the following to your `.env` file:
+**Add to your `.env` file:**
 
-```
-app.allowedHostnames.0 = 'yourdomain.com'
-app.allowedHostnames.1 = 'www.yourdomain.com'
+```bash
+# Comma-separated list of allowed hostnames (no protocols or ports)
+app.allowedHostnames = 'yourdomain.com,www.yourdomain.com'
 ```
 
-**For local development**, use:
-```
-app.allowedHostnames.0 = 'localhost'
+**For local development:**
+
+```bash
+app.allowedHostnames = 'localhost'
 ```
 
-If `allowedHostnames` is not configured:
-1. A security warning will be logged
-2. The application will fall back to 'localhost' as the hostname
-3. This means URLs generated by the application (links, redirects, etc.) will point to 'localhost'
+**If you see this error at startup:**
+
+```text
+RuntimeException: Security: allowedHostnames is not configured.
+```
+
+**Solution**: Add `app.allowedHostnames` to your `.env` file with your domain(s).
+
+**Why this matters:**
+- Prevents Host Header Injection attacks (GHSA-jchf-7hr6-h4f3)
+- Ensures URLs are generated with the correct domain
+- Security advisory: https://github.com/opensourcepos/opensourcepos/security/advisories/GHSA-jchf-7hr6-h4f3
+- Fixes issue #4480: .env configuration now works via comma-separated values
 
 ### HTTPS Behind Proxy
 

app/Config/App.php

@@ -62,14 +62,14 @@ class App extends BaseConfig
      * an entry in this list, the request will use the first allowed hostname.
      * 
      * IMPORTANT: This MUST be configured for production deployments.
-     * If empty, the application will fall back to 'localhost'.
+     * If empty in production, the application will fail to start.
+     * In development, it will fall back to 'localhost' with a warning.
      * 
-     * Configure via .env file:
-     *   app.allowedHostnames.0 = 'example.com'
-     *   app.allowedHostnames.1 = 'www.example.com'
+     * Configure via .env file (comma-separated list):
+     *   app.allowedHostnames = 'example.com,www.example.com'
      * 
      * For local development:
-     *   app.allowedHostnames.0 = 'localhost'
+     *   app.allowedHostnames = 'localhost'
      *
      * @var list<string>
      */
@@ -291,6 +291,17 @@ class App extends BaseConfig
     public function __construct()
     {
         parent::__construct();
+        
+        // Solution for CodeIgniter 4 limitation: arrays cannot be set from .env
+        // See: https://github.com/codeigniter4/CodeIgniter4/issues/7311
+        $envAllowedHostnames = getenv('app.allowedHostnames');
+        if ($envAllowedHostnames !== false && trim($envAllowedHostnames) !== '') {
+            $this->allowedHostnames = array_values(array_filter(
+                array_map('trim', explode(',', $envAllowedHostnames)),
+                static fn (string $hostname): bool => $hostname !== ''
+            ));
+        }
+        
         $this->https_on = (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == 'on') || (isset($_ENV['FORCE_HTTPS']) && $_ENV['FORCE_HTTPS'] == 'true');
         
         $host = $this->getValidHost();
@@ -305,19 +316,36 @@ class App extends BaseConfig
      * Security: Prevents Host Header Injection attacks (GHSA-jchf-7hr6-h4f3)
      * by validating the HTTP_HOST against a whitelist of allowed hostnames.
      * 
+     * In production: Fails fast if allowedHostnames is not configured.
+     * In development: Allows localhost fallback with an error log.
+     * 
      * @return string A validated hostname
+     * @throws \RuntimeException If allowedHostnames is not configured in production
      */
     private function getValidHost(): string
     {
         $httpHost = $_SERVER['HTTP_HOST'] ?? 'localhost';
+        
+        // Determine environment
+        // CodeIgniter's test bootstrap sets $_SERVER['CI_ENVIRONMENT'] = 'testing'
+        // Check $_SERVER first, then $_ENV, then fall back to 'production'
+        $environment = $_SERVER['CI_ENVIRONMENT'] ?? $_ENV['CI_ENVIRONMENT'] ?? getenv('CI_ENVIRONMENT') ?: 'production';
 
         if (empty($this->allowedHostnames)) {
-            log_message('warning', 
+            $errorMessage = 
                 'Security: allowedHostnames is not configured. ' .
                 'Host header injection protection is disabled. ' .
-                'Please set app.allowedHostnames in your .env file. ' .
-                'Received Host: ' . $httpHost
-            );
+                'Set app.allowedHostnames in your .env file. ' .
+                'Example: app.allowedHostnames = "example.com,www.example.com" ' .
+                'Received Host: ' . $httpHost;
+            
+            // Production: Fail explicitly to prevent silent security vulnerabilities
+            // Testing and development: Allow localhost fallback
+            if ($environment === 'production') {
+                throw new \RuntimeException($errorMessage);
+            }
+            
+            log_message('error', $errorMessage . ' Using localhost fallback (development only).');
             return 'localhost';
         }
 
@@ -325,6 +353,7 @@ class App extends BaseConfig
             return $httpHost;
         }
 
+        // Host not in whitelist - use first configured hostname as fallback
         log_message('warning', 
             'Security: Rejected HTTP_HOST "' . $httpHost . '" - not in allowedHostnames whitelist. ' .
             'Using fallback: ' . $this->allowedHostnames[0]

tests/Config/AppTest.php

@@ -15,6 +15,10 @@ class AppTest extends CIUnitTestCase
     protected function tearDown(): void
     {
         parent::tearDown();
+        // Clean up environment
+        putenv('CI_ENVIRONMENT');
+        putenv('app.allowedHostnames');
+        unset($_SERVER['HTTP_HOST']);
     }
 
     public function testGetValidHostReturnsHostWhenValid(): void
@@ -59,8 +63,11 @@ class AppTest extends CIUnitTestCase
         $this->assertEquals('example.com', $host);
     }
 
-    public function testGetValidHostReturnsLocalhostWhenNoWhitelist(): void
+    public function testGetValidHostReturnsLocalhostInDevelopmentWhenNoWhitelist(): void
     {
+        // Set development environment
+        putenv('CI_ENVIRONMENT=development');
+
         $app = new class extends App {
             public array $allowedHostnames = [];
             
@@ -80,6 +87,28 @@ class AppTest extends CIUnitTestCase
         $this->assertEquals('localhost', $host);
     }
 
+    public function testGetValidHostThrowsExceptionInProductionWhenNoWhitelist(): void
+    {
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionMessage('allowedHostnames is not configured');
+
+        // Set production environment
+        putenv('CI_ENVIRONMENT=production');
+
+        $app = new class extends App {
+            public array $allowedHostnames = [];
+            
+            public function __construct() {}
+        };
+
+        $reflection = new \ReflectionClass($app);
+        $method = $reflection->getMethod('getValidHost');
+        $method->setAccessible(true);
+
+        $_SERVER['HTTP_HOST'] = 'malicious.com';
+        $method->invoke($app);
+    }
+
     public function testGetValidHostHandlesMissingHttpHost(): void
     {
         $app = new class extends App {
@@ -123,4 +152,133 @@ class AppTest extends CIUnitTestCase
         $this->assertStringContainsString('example.com', $app->baseURL);
         $this->assertStringNotContainsString('malicious.com', $app->baseURL);
     }
+
+    public function testEnvAllowedHostnamesParsedAsCommaSeparated(): void
+    {
+        // Set environment variable
+        putenv('app.allowedHostnames=example.com,www.example.com,demo.example.com');
+
+        $_SERVER['HTTP_HOST'] = 'www.example.com';
+        $_SERVER['SCRIPT_NAME'] = '/index.php';
+        $_SERVER['HTTPS'] = null;
+
+        $app = new App();
+
+        // Constructor should parse comma-separated values
+        $this->assertEquals(['example.com', 'www.example.com', 'demo.example.com'], $app->allowedHostnames);
+        $this->assertStringContainsString('www.example.com', $app->baseURL);
+
+        // Clean up
+        putenv('app.allowedHostnames');
+    }
+
+    public function testEnvAllowedHostnamesTrimmedWhitespace(): void
+    {
+        // Set environment variable with whitespace
+        putenv('app.allowedHostnames= example.com , www.example.com , demo.example.com ');
+
+        $_SERVER['HTTP_HOST'] = 'example.com';
+        $_SERVER['SCRIPT_NAME'] = '/index.php';
+        $_SERVER['HTTPS'] = null;
+
+        $app = new App();
+
+        // Values should be trimmed
+        $this->assertEquals(['example.com', 'www.example.com', 'demo.example.com'], $app->allowedHostnames);
+
+        // Clean up
+        putenv('app.allowedHostnames');
+    }
+
+    public function testEnvAllowedHostnamesSingleValue(): void
+    {
+        // Set environment variable with single value
+        putenv('app.allowedHostnames=localhost');
+
+        $_SERVER['HTTP_HOST'] = 'localhost';
+        $_SERVER['SCRIPT_NAME'] = '/index.php';
+        $_SERVER['HTTPS'] = null;
+
+        $app = new App();
+
+        // Single value should work
+        $this->assertEquals(['localhost'], $app->allowedHostnames);
+        $this->assertStringContainsString('localhost', $app->baseURL);
+
+        // Clean up
+        putenv('app.allowedHostnames');
+    }
+
+    public function testEnvAllowedHostnamesEmptyStringNotConfigured(): void
+    {
+        // Set environment variable to empty string
+        putenv('app.allowedHostnames=');
+
+        // Set development environment
+        putenv('CI_ENVIRONMENT=development');
+
+        $_SERVER['HTTP_HOST'] = 'example.com';
+        $_SERVER['SCRIPT_NAME'] = '/index.php';
+        $_SERVER['HTTPS'] = null;
+
+        $app = new App();
+
+        // Empty string should be treated as not configured
+        $this->assertEquals([], $app->allowedHostnames);
+
+        // In development, should fall back to localhost
+        $this->assertStringContainsString('localhost', $app->baseURL);
+
+        // Clean up
+        putenv('app.allowedHostnames');
+        putenv('CI_ENVIRONMENT');
+    }
+
+    public function testEnvAllowedHostnamesFiltersEmptyEntries(): void
+    {
+        // Trailing comma should not produce empty entry
+        putenv('app.allowedHostnames=example.com,');
+        $_SERVER['HTTP_HOST'] = 'example.com';
+        $_SERVER['SCRIPT_NAME'] = '/index.php';
+        $_SERVER['HTTPS'] = null;
+
+        $app = new App();
+        $this->assertEquals(['example.com'], $app->allowedHostnames);
+
+        // Clean up
+        putenv('app.allowedHostnames');
+
+        // Leading comma should not produce empty entry
+        putenv('app.allowedHostnames=,example.com');
+        $_SERVER['HTTP_HOST'] = 'example.com';
+
+        $app = new App();
+        $this->assertEquals(['example.com'], $app->allowedHostnames);
+
+        // Clean up
+        putenv('app.allowedHostnames');
+
+        // Whitespace-only entry should be filtered
+        putenv('app.allowedHostnames=example.com, ,www.example.com');
+        $_SERVER['HTTP_HOST'] = 'example.com';
+
+        $app = new App();
+        $this->assertEquals(['example.com', 'www.example.com'], $app->allowedHostnames);
+
+        // Clean up
+        putenv('app.allowedHostnames');
+
+        // All-whitespace value should be treated as not configured
+        putenv('CI_ENVIRONMENT=development');
+        putenv('app.allowedHostnames= , , ');
+        $_SERVER['HTTP_HOST'] = 'example.com';
+
+        $app = new App();
+        $this->assertEquals([], $app->allowedHostnames);
+        $this->assertStringContainsString('localhost', $app->baseURL);
+
+        // Clean up
+        putenv('app.allowedHostnames');
+        putenv('CI_ENVIRONMENT');
+    }
 }