From 707a45e639f79c3207e05e5495c584416db43d60 Mon Sep 17 00:00:00 2001 From: VAIO73 <50487331+Vaso73@users.noreply.github.com> Date: Wed, 28 Oct 2026 09:46:32 +0000 Subject: [PATCH] fix: guard host reboot with native package locks --- AppImage/scripts/host_update_controls.py | 56 +++++++++++++++++++++++- tests/test_host_update_controls.py | 39 ++++++++++++++++- 2 files changed, 91 insertions(+), 4 deletions(-) diff --git a/AppImage/scripts/host_update_controls.py b/AppImage/scripts/host_update_controls.py index 1c4b7520..c2d78179 100644 --- a/AppImage/scripts/host_update_controls.py +++ b/AppImage/scripts/host_update_controls.py @@ -4,18 +4,64 @@ import os from pathlib import Path import socket import stat +import subprocess +import sys +import time from kernel_reboot import reboot_status from host_update_jobs import list_jobs LOCK = '/run/proxmenux-host-update.lock' BOOT_ID = Path('/proc/sys/kernel/random/boot_id') +PROC = Path('/proc') +PACKAGE_LOCKS = ('/var/lib/dpkg/lock-frontend', '/var/lib/dpkg/lock') UNIT = 'proxmenux-action-reboot-after-update' # The unit obtains the SAME lock as CLI/legacy/web updates. Holding it for # the shutdown transition closes the precheck-to-launch race. If an updater # wins that race, flock exits75 and systemctl reboot is never invoked. -COMMAND = ['/usr/bin/flock', '-n', '-E', '75', LOCK, '/bin/sh', '-c', - '/bin/systemctl reboot && /bin/sleep 60'] +COMMAND = ['/usr/bin/flock', '-n', '-E', '75', LOCK, '/usr/bin/python3', + '/usr/local/share/proxmenux/monitor-app/usr/bin/host_update_controls.py', '--execute-reboot'] + + +def package_process_active(): + try: + for path in PROC.iterdir(): + if not path.name.isdigit(): + continue + try: + if (path/'comm').read_text().strip() in ('apt', 'apt-get', 'dpkg', 'unattended-upgr'): + return True + except (FileNotFoundError, ProcessLookupError): + continue + except OSError: + return True + return False + + +def execute_reboot(): + """Unit-only final guard: POSIX dpkg locks also protect direct APT callers. + + Do not use flock for these files: APT/dpkg use POSIX record locks. The + parent retains both locks while systemctl runs and during shutdown. + """ + handles = [] + try: + for path in PACKAGE_LOCKS: + fd = os.open(path, os.O_RDWR | os.O_NOFOLLOW | os.O_NONBLOCK) + handles.append(fd) + if not stat.S_ISREG(os.fstat(fd).st_mode): + return 76 + fcntl.lockf(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + if reboot_status()['state'] != 'required': + return 77 + subprocess.run(['/bin/systemctl', 'reboot'], check=True, timeout=10) + time.sleep(60) + return 0 + except (OSError, subprocess.SubprocessError): + return 75 + finally: + for fd in handles: + os.close(fd) def reboot_context(): @@ -35,6 +81,8 @@ def reboot_context(): return result if any(job.get('state') != 'complete' for job in jobs): return dict(result, reason='busy') + if package_process_active(): + return dict(result, reason='busy') try: fd = os.open(LOCK, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK) except FileNotFoundError: @@ -55,3 +103,7 @@ def reboot_context(): reboot = reboot_status() return dict(result, allowed=reboot['state']=='required', reason='required' if reboot['state']=='required' else 'current' if reboot['state']=='current' else 'unknown') + + +if __name__ == '__main__': + sys.exit(execute_reboot() if sys.argv[1:] == ['--execute-reboot'] else 2) diff --git a/tests/test_host_update_controls.py b/tests/test_host_update_controls.py index 31ec8f03..7a7a1ed2 100644 --- a/tests/test_host_update_controls.py +++ b/tests/test_host_update_controls.py @@ -94,6 +94,8 @@ class RebootGuards(unittest.TestCase): def setUp(self): temporary=tempfile.TemporaryDirectory();self.addCleanup(temporary.cleanup) self.root=Path(temporary.name); self.lock=self.root/'update.lock' + (self.root/'proc').mkdir() + self.proc=patch.object(controls,'PROC',self.root/'proc');self.proc.start();self.addCleanup(self.proc.stop) boot=self.root/'boot_id';boot.write_text('fixture-boot') self.boot=patch.object(controls,'BOOT_ID',boot);self.boot.start();self.addCleanup(self.boot.stop) self.a=patch.object(controls,'LOCK',str(self.lock)); self.a.start(); self.addCleanup(self.a.stop) @@ -121,8 +123,8 @@ class RebootGuards(unittest.TestCase): self.lock.touch() marker=self.root/'inert-action' command=list(controls.COMMAND) - self.assertEqual(command[-1],'/bin/systemctl reboot && /bin/sleep 60') - command[4]=str(self.lock);command[-1]='printf reached > '+shlex.quote(str(marker)) + self.assertEqual(command[5:],['/usr/bin/python3','/usr/local/share/proxmenux/monitor-app/usr/bin/host_update_controls.py','--execute-reboot']) + command[4]=str(self.lock);command[5:]=['/bin/sh','-c','printf reached > '+shlex.quote(str(marker))] with self.lock.open('r') as owner: fcntl.flock(owner,fcntl.LOCK_EX|fcntl.LOCK_NB) self.assertFalse(controls.reboot_context()['allowed']) @@ -137,6 +139,39 @@ class RebootGuards(unittest.TestCase): self.probe.return_value={'state':state} self.assertFalse(controls.reboot_context()['allowed']) + def test_direct_package_process_blocks_before_kernel_probe(self): + path=self.root/'proc/123';path.mkdir();(path/'comm').write_text('apt-get\n') + self.assertFalse(controls.reboot_context()['allowed']) + self.probe.assert_not_called() + + def test_worker_package_locks_missing_or_unknown_kernel_never_reboot(self): + paths=[str(self.root/name) for name in ('frontend','backend')] + with patch.object(controls,'PACKAGE_LOCKS',paths),patch.object(controls.subprocess,'run') as reboot,patch.object(controls.time,'sleep') as delay: + self.assertEqual(controls.execute_reboot(),75) + reboot.assert_not_called();delay.assert_not_called() + for path in paths:Path(path).touch() + self.probe.return_value={'state':'unknown'} + self.assertEqual(controls.execute_reboot(),77) + reboot.assert_not_called() + self.probe.return_value={'state':'required'} + self.assertEqual(controls.execute_reboot(),0) + reboot.assert_called_once_with(['/bin/systemctl','reboot'],check=True,timeout=10) + delay.assert_called_once_with(60) + + def test_actual_posix_package_lock_blocks_worker_before_reboot(self): + paths=[str(self.root/name) for name in ('frontend','backend')] + for path in paths:Path(path).touch() + # A separate process holds the same POSIX lock, just as APT does. + code='import fcntl,sys,time; f=open(sys.argv[1],"r+"); fcntl.lockf(f,fcntl.LOCK_EX); print("locked",flush=True); time.sleep(5)' + child=subprocess.Popen([sys.executable,'-c',code,paths[0]],stdout=subprocess.PIPE,text=True) + try: + self.assertEqual(child.stdout.readline().strip(),'locked') + with patch.object(controls,'PACKAGE_LOCKS',paths),patch.object(controls.subprocess,'run') as reboot: + self.assertEqual(controls.execute_reboot(),75) + reboot.assert_not_called() + finally: + child.terminate();child.wait(timeout=2);child.stdout.close() + class Routes(unittest.TestCase): def setUp(self):