From 44667d53377d15c631abb660b7dc7d0e206f119b Mon Sep 17 00:00:00 2001
From: Federico Capoano <f.capoano@openwisp.io>
Date: Tue, 19 May 2026 12:18:38 -0300
Subject: [PATCH] [fix] Decouple OpenVPN config filename from VPN name #572

---
 images/common/utils.sh                   | 42 ++++++++++++++++--
 images/openwisp_openvpn/openvpn.sh       |  3 +-
 images/openwisp_openvpn/supervisord.conf |  2 +-
 tests/runtests.py                        | 56 +++++++++++++++++++++++-
 4 files changed, 95 insertions(+), 8 deletions(-)

diff --git a/images/common/utils.sh b/images/common/utils.sh
index 67e97f04..9385c9e5 100644
--- a/images/common/utils.sh
+++ b/images/common/utils.sh
@@ -236,13 +236,47 @@ function openvpn_config_checksum {
 }
 
 function openvpn_config_download {
+	# Extract in isolation so stale files in / cannot be mistaken for
+	# files from the newly downloaded archive.
+	TMPDIR=$(mktemp -d) || return 1
 	curl --silent --retry 10 --retry-delay 5 --retry-max-time 300\
 		--insecure --output vpn.tar.gz \
-		${API_INTERNAL}/controller/vpn/download-config/$UUID/?key=$KEY
+		"${API_INTERNAL}/controller/vpn/download-config/${UUID}/?key=${KEY}" || {
+		rm -rf -- "$TMPDIR"
+		return 1
+	}
 	curl --silent --insecure --output checksum \
-		${API_INTERNAL}/controller/vpn/checksum/$UUID/?key=$KEY
-	tar xzf vpn.tar.gz
-	chmod 600 *.pem
+		"${API_INTERNAL}/controller/vpn/checksum/${UUID}/?key=${KEY}" || {
+		rm -rf -- "$TMPDIR"
+		return 1
+	}
+	tar xzf vpn.tar.gz -C "$TMPDIR" || {
+		rm -rf -- "$TMPDIR"
+		return 1
+	}
+	chmod 600 -- "$TMPDIR"/*.pem 2>/dev/null || true
+	# Supervisord always starts OpenVPN with openvpn.conf; normalize whatever
+	# config filename the archive contains, including names with whitespace.
+	CONF_FILE="$TMPDIR/openvpn.conf"
+	if [ ! -f "$CONF_FILE" ]; then
+		CONF_FILE=$(find "$TMPDIR" -maxdepth 1 -type f -name '*.conf' -print -quit)
+	fi
+	if [ -z "$CONF_FILE" ]; then
+		echo "ERROR: no OpenVPN config file found after extraction" >&2
+		rm -rf -- "$TMPDIR"
+		return 1
+	fi
+	mv -f -- "$CONF_FILE" openvpn.conf || {
+		rm -rf -- "$TMPDIR"
+		return 1
+	}
+	# Move the remaining extracted files, but leave any extra .conf files behind
+	# so only the normalized openvpn.conf is used at runtime.
+	find "$TMPDIR" -mindepth 1 -maxdepth 1 ! -name '*.conf' -exec mv -f -- {} . \; || {
+		rm -rf -- "$TMPDIR"
+		return 1
+	}
+	rm -rf -- "$TMPDIR"
 }
 
 function crl_download {
diff --git a/images/openwisp_openvpn/openvpn.sh b/images/openwisp_openvpn/openvpn.sh
index a3af89b8..7ea40c6a 100644
--- a/images/openwisp_openvpn/openvpn.sh
+++ b/images/openwisp_openvpn/openvpn.sh
@@ -9,6 +9,5 @@ openvpn_config
 openvpn_config_checksum
 
 if [ "${OFILE}" != "${NFILE}" ]; then
-	openvpn_config_download
-	supervisorctl restart openvpn
+	openvpn_config_download && supervisorctl restart openvpn
 fi
diff --git a/images/openwisp_openvpn/supervisord.conf b/images/openwisp_openvpn/supervisord.conf
index 2c2ce187..73523548 100644
--- a/images/openwisp_openvpn/supervisord.conf
+++ b/images/openwisp_openvpn/supervisord.conf
@@ -18,7 +18,7 @@ pidfile=/supervisord.pid
 [program:openvpn]
 user=root
 directory=/
-command=/usr/sbin/openvpn --config %(ENV_VPN_NAME)s.conf
+command=/usr/sbin/openvpn --config openvpn.conf
 autostart=true
 autorestart=true
 stopsignal=INT
diff --git a/tests/runtests.py b/tests/runtests.py
index 22f99240..f380b88f 100644
--- a/tests/runtests.py
+++ b/tests/runtests.py
@@ -10,7 +10,7 @@
 from selenium.webdriver.common.by import By
 from selenium.webdriver.support import expected_conditions as EC
 from selenium.webdriver.support.ui import WebDriverWait
-from utils import TestUtilities
+from utils import TestConfig, TestUtilities
 
 
 class Pretest(TestUtilities, unittest.TestCase):
@@ -58,6 +58,60 @@ def test_wait_for_services(self):
             self.fail(f"All celery workers are not online: {online_workers}")
 
 
+class OpenVPNContainerTest(TestConfig, unittest.TestCase):
+    def test_openvpn_config_download_with_whitespace_name(self):
+        """Ensure the OpenVPN container normalizes whitespace config names."""
+        script = r"""
+            set -e
+            TMPDIR=$(mktemp -d)
+            mkdir "$TMPDIR/archive" "$TMPDIR/work"
+            printf 'new config' > "$TMPDIR/archive/my vpn.conf"
+            printf 'pem' > "$TMPDIR/archive/client.pem"
+            tar -czf "$TMPDIR/vpn.tar.gz" -C "$TMPDIR/archive" \
+                'my vpn.conf' 'client.pem'
+            printf 'stale config' > "$TMPDIR/work/openvpn.conf"
+            cd "$TMPDIR/work"
+            source /utils.sh
+            curl() {
+                output=''
+                while [ "$#" -gt 0 ]; do
+                    if [ "$1" = '--output' ]; then
+                        shift
+                        output="$1"
+                    fi
+                    shift || true
+                done
+                if [ "$output" = 'vpn.tar.gz' ]; then
+                    cp "$TMPDIR/vpn.tar.gz" "$output"
+                elif [ "$output" = 'checksum' ]; then
+                    printf '%s\n' 'checksum' > "$output"
+                else
+                    return 1
+                fi
+            }
+            API_INTERNAL='https://api.internal'
+            UUID='vpn-uuid'
+            KEY='vpn-key'
+            openvpn_config_download
+            test "$(cat openvpn.conf)" = 'new config'
+            test ! -f 'my vpn.conf'
+            test "$(cat checksum)" = 'checksum'
+            test "$(stat -c '%a' client.pem)" = '600'
+            rm -rf "$TMPDIR"
+        """
+        res = subprocess.run(
+            ["docker", "compose", "exec", "-T", "openvpn", "sh", "-c", script],
+            cwd=self.root_location,
+            capture_output=True,
+            text=True,
+        )
+        self.assertEqual(
+            res.returncode,
+            0,
+            f"stdout:\n{res.stdout}\nstderr:\n{res.stderr}",
+        )
+
+
 class TestServices(TestUtilities, unittest.TestCase):
     custom_static_token = None