From a2b98af88e7fbedb8600826edcbb2f46531c5a2b Mon Sep 17 00:00:00 2001
From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com>
Date: Thu, 22 Oct 2026 14:09:19 +0200
Subject: [PATCH] fix: never reuse a backup folder of another run

When every dated name in retrobios-backup already existed, the loop ended on the last one and wrote into it. It now raises, so the next location is tried and the file stays in place if none works. A holder another user owns is refused on systems with user ids.
---
 install.ps1           |  2 +-
 install.py            | 23 ++++++++++++++++++++---
 install.sh            |  2 +-
 tests/test_install.py | 23 +++++++++++++++++++++++
 4 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/install.ps1 b/install.ps1
index f5a1a8c2..b0c0f1fe 100644
--- a/install.ps1
+++ b/install.ps1
@@ -11,7 +11,7 @@
 
     $ErrorActionPreference = "Stop"
     $defaultInstallUrl = "https://raw.githubusercontent.com/Abdess/retrobios/main/install.py"
-    $defaultInstallSha256 = "001ddf03177f12051e70d5f57db788316c7971f23366ff2532f91a2b9aca2ffd"
+    $defaultInstallSha256 = "c595032e68eec7573ad418cac8b906b0fc84a2253367f02e480d21eef94e1980"
     $maximumInstallerBytes = 2MB
     $installerExit = 0
     $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null }
diff --git a/install.py b/install.py
index 82c4b944..44a0a9dd 100644
--- a/install.py
+++ b/install.py
@@ -3443,6 +3443,21 @@ BACKUP_DIR = "retrobios-backup"
 BACKUP_MARKER = ".retrobios-backup"
 
 
+def _check_holder(holder: Path) -> None:
+    """Refuse a backup holder the user's files must not go into.
+
+    A link would send them wherever it points. On a system with user ids, a
+    folder another user owns was not made by this user's runs, and its owner
+    could read or swap what is moved in.
+    """
+    info = os.lstat(holder)
+    if stat.S_ISLNK(info.st_mode):
+        raise OSError(errno.ELOOP, f"{holder} is a link")
+    getuid = getattr(os, "getuid", None)
+    if getuid is not None and info.st_uid != getuid():
+        raise OSError(errno.EPERM, f"{holder} belongs to another user")
+
+
 class _NotKept(Exception):
     """The file in place differs and could not be set aside: it stays."""
 
@@ -3496,9 +3511,7 @@ class KeptFiles:
                 if run is None:
                     try:
                         holder.mkdir(parents=True, exist_ok=True)
-                        if holder.is_symlink():
-                            # A link would send the user's files wherever it points.
-                            raise OSError(errno.ELOOP, f"{holder} is a link")
+                        _check_holder(holder)
                         for n in range(1, 1000):
                             run = holder / (stamp if n == 1 else f"{stamp}-{n}")
                             try:
@@ -3506,6 +3519,10 @@ class KeptFiles:
                                 break
                             except FileExistsError:
                                 continue
+                        else:
+                            # Every name taken: never reuse a folder this run
+                            # did not create.
+                            raise OSError(errno.EEXIST, f"no free folder name in {holder}")
                         (run / BACKUP_MARKER).write_text(
                             "Files the RetroBIOS installer replaced.\n", encoding="utf-8")
                     except OSError as exc:
diff --git a/install.sh b/install.sh
index 2b2c34c0..269d3f84 100755
--- a/install.sh
+++ b/install.sh
@@ -18,7 +18,7 @@ esac
 TEMP_INSTALLER=""
 TEMP_DIRECTORY=""
 DEFAULT_INSTALL_URL="https://raw.githubusercontent.com/Abdess/retrobios/main/install.py"
-DEFAULT_INSTALL_SHA256="001ddf03177f12051e70d5f57db788316c7971f23366ff2532f91a2b9aca2ffd"
+DEFAULT_INSTALL_SHA256="c595032e68eec7573ad418cac8b906b0fc84a2253367f02e480d21eef94e1980"
 MAX_INSTALLER_BYTES=2097152
 
 cleanup() {
diff --git a/tests/test_install.py b/tests/test_install.py
index 94aaab87..6dc20bef 100644
--- a/tests/test_install.py
+++ b/tests/test_install.py
@@ -5218,6 +5218,29 @@ class AFileYouHadIsKept(unittest.TestCase):
         self.assertEqual(len(inside), 1)
         self.assertEqual((inside[0] / "x.bin").read_bytes(), self.OLD)
 
+    def test_a_holder_with_every_name_taken_is_not_reused(self):
+        # Folders planted under every dated name of this run: none of them
+        # may receive the user's file.
+        holder = self.home / install.BACKUP_DIR
+        stamp = "20260101-000000"
+        for n in range(1, 1000):
+            (holder / (stamp if n == 1 else f"{stamp}-{n}")).mkdir(parents=True)
+        (self.bios / "x.bin").write_bytes(self.OLD)
+        with unittest.mock.patch.object(install.time, "strftime", return_value=stamp):
+            self.assertTrue(self._download("x.bin")[1])
+        self.assertEqual(sum(1 for p in holder.rglob("x.bin")), 0)
+        inside = list((self.bios / install.BACKUP_DIR).iterdir())
+        self.assertEqual(len(inside), 1)
+        self.assertEqual((inside[0] / "x.bin").read_bytes(), self.OLD)
+
+    @unittest.skipUnless(hasattr(os, "getuid"), "user ids are POSIX")
+    def test_a_holder_another_user_owns_is_refused(self):
+        (self.home / install.BACKUP_DIR).mkdir()
+        with self.assertRaises(OSError):
+            with unittest.mock.patch.object(install.os, "getuid", return_value=os.getuid() + 1):
+                install._check_holder(self.home / install.BACKUP_DIR)
+        install._check_holder(self.home / install.BACKUP_DIR)
+
     def test_restore_refuses_a_folder_the_installer_did_not_write(self):
         planted = self.home / install.BACKUP_DIR / "20260101-000000"
         (planted / "system").mkdir(parents=True)