From 26311baab94beacd6fe1d2816dd8161a96bbe544 Mon Sep 17 00:00:00 2001
From: Ilia Ross <ilia@ross.gdn>
Date: Mon, 22 Jun 2026 20:34:00 +0200
Subject: [PATCH] Fix to use Webmin config locking for ws-link cleanup
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ⓘ Make linked websocket teardown use the same `miniserv.conf.lock` convention as `link.cgi`, release the lock safely on cleanup errors
---
 miniserv.pl  | 81 +++++++++++++++++++++++++++++++++++++++++++++-------
 t/miniserv.t | 24 ++++++++++++++++
 2 files changed, 95 insertions(+), 10 deletions(-)

diff --git a/miniserv.pl b/miniserv.pl
index af100596d..93a62d31a 100755
--- a/miniserv.pl
+++ b/miniserv.pl
@@ -5097,6 +5097,63 @@ close(CONF);
 return %rv;
 }
 
+# lock_config_file(file)
+# Lock a config file using Webmin's .lock sidecar convention. miniserv.pl must
+# stay standalone, but link.cgi also edits miniserv.conf through this lock.
+sub lock_config_file
+{
+my ($file) = @_;
+if ($file =~ /\r|\n|\0/) {
+	die "Lock filename contains invalid characters";
+	}
+my $lockfile = $file.".lock";
+my $tries = 0;
+my $last_lock_err;
+while(1) {
+	my $pid;
+	# Webmin's lock file contains the owner PID. A dead owner means the lock
+	# can be safely reclaimed.
+	if (open(my $locking, "<", $lockfile)) {
+		$pid = int(<$locking>);
+		close($locking);
+		}
+	if (!$pid || !kill(0, $pid) || $pid == $$) {
+		# The sidecar lock is the shared convention with web-lib-funcs.pl;
+		# the non-blocking flock just narrows races between lock claimers.
+		unlink($lockfile);
+		if (open(my $locking, ">", $lockfile)) {
+			my $locked = eval { flock($locking, 2+4) };
+			my $ok;
+			if ($locked) {
+				$ok = print $locking $$,"\n";
+				}
+			my $closed = close($locking);
+			return 1 if ($locked && $ok && $closed);
+			unlink($lockfile);
+			$last_lock_err = "Lock failed : ".($@ || $!);
+			}
+		else {
+			$last_lock_err = "Lock open failed : ".$!;
+			}
+		}
+	elsif ($pid) {
+		$last_lock_err = "Locked by PID $pid";
+		}
+	sleep(1);
+	if ($tries++ > 5*60) {
+		die "Failed to lock config file $file : $last_lock_err";
+		}
+	}
+}
+
+# unlock_config_file(file)
+# Release a config lock taken by lock_config_file.
+sub unlock_config_file
+{
+my ($file) = @_;
+unlink($file.".lock");
+}
+
 # read_any_file(file)
 # Reads any given file and returns its content
 sub read_any_file
@@ -6359,17 +6416,21 @@ close(SOCK);
 if ($ws->{'path'} =~ /\/ws-link-/) {
 	# Linked-server websocket routes are single-use routes registered by
 	# link.cgi, so remove them and refresh the master when the tunnel ends.
-	open(my $config_lock, "<", $config_file) ||
-		die "Failed to open config file $config_file : $!";
-	flock($config_lock, 2);
-	my %miniserv = &read_config_file($config_file);
+	&lock_config_file($config_file);
 	my $deleted;
-	if (delete($miniserv{"websockets_$ws->{'path'}"})) {
-		&write_file($config_file, \%miniserv);
-		$deleted = 1;
-		}
-	flock($config_lock, 8);
-	close($config_lock);
+	# Always release the config lock, even if the read/write path fails.
+	my $cleanup_ok = eval {
+		my %miniserv = &read_config_file($config_file);
+		if (delete($miniserv{"websockets_$ws->{'path'}"})) {
+			&write_file($config_file, \%miniserv);
+			$deleted = 1;
+			}
+		1;
+		};
+	my $cleanup_err = $@;
+	&unlock_config_file($config_file);
+	die $cleanup_err if (!$cleanup_ok);
+	# The master keeps websocket routes parsed in memory.
 	kill('USR1', $miniserv_main_pid || getppid()) if ($deleted);
 	}
 print DEBUG "done websockets loop\n";
diff --git a/t/miniserv.t b/t/miniserv.t
index f65ee2f97..acb35c135 100644
--- a/t/miniserv.t
+++ b/t/miniserv.t
@@ -1245,6 +1245,30 @@ EOF
 	ok(!exists $got{'# this is a comment'}, 'comment lines skipped');
 };
 
+# lock_config_file — matches Webmin's .lock convention without loading web-lib
+subtest 'lock_config_file' => sub {
+	require File::Temp;
+	my ($fh, $path) = File::Temp::tempfile(UNLINK => 1);
+	close($fh);
+
+	ok(miniserv::lock_config_file($path), 'lock succeeds');
+	ok(-e "$path.lock", 'sidecar lock file created');
+
+	open(my $locking, '<', "$path.lock") or die "open lock: $!";
+	chomp(my $pid = <$locking>);
+	close($locking);
+	is($pid, $$, 'lock records the current PID');
+
+	miniserv::unlock_config_file($path);
+	ok(!-e "$path.lock", 'unlock removes sidecar lock file');
+
+	open($locking, '>', "$path.lock") or die "create stale lock: $!";
+	print $locking "999999999\n";
+	close($locking);
+	ok(miniserv::lock_config_file($path), 'stale lock is reclaimed');
+	miniserv::unlock_config_file($path);
+};
+
 # read_any_file — basic file reader; returns undef on open failure
 subtest 'read_any_file' => sub {
 	require File::Temp;