fix: never write a backup through a link

A BIOS folder can sit on a share other people write to. A retrobios-backup that is a link is no longer written through (the backup goes inside the BIOS folder instead), run folders are created for the user only and carry a marker, and --restore only takes a folder holding that marker, walking it without following links. Beginner pages label their links to the installer page as details.

install.ps1

@@ -11,7 +11,7 @@
 
     $ErrorActionPreference = "Stop"
     $defaultInstallUrl = "https://raw.githubusercontent.com/Abdess/retrobios/main/install.py"
-    $defaultInstallSha256 = "8bf9b5bf68dbde80025b1b4e1c18e31f43c016758df91be202613933f1de17d5"
+    $defaultInstallSha256 = "001ddf03177f12051e70d5f57db788316c7971f23366ff2532f91a2b9aca2ffd"
     $maximumInstallerBytes = 2MB
     $installerExit = 0
     $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null }

install.py

@@ -3437,6 +3437,10 @@ def check_local(
 # file of the BIOS folder, BizHawk walks its firmware folder recursively. A
 # kept copy anywhere under that folder could be taken for the BIOS.
 BACKUP_DIR = "retrobios-backup"
+# Written into every run folder the installer creates. --restore only accepts
+# a folder holding it, so a folder someone else placed (a shared BIOS folder
+# on a network share) is never copied over the user's files.
+BACKUP_MARKER = ".retrobios-backup"
 
 
 class _NotKept(Exception):
@@ -3492,13 +3496,18 @@ 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")
                         for n in range(1, 1000):
                             run = holder / (stamp if n == 1 else f"{stamp}-{n}")
                             try:
-                                run.mkdir()
+                                run.mkdir(mode=0o700)
                                 break
                             except FileExistsError:
                                 continue
+                        (run / BACKUP_MARKER).write_text(
+                            "Files the RetroBIOS installer replaced.\n", encoding="utf-8")
                     except OSError as exc:
                         failure = exc
                         continue
@@ -3661,16 +3670,22 @@ def restore_kept(run: Path, dry_run: bool = False) -> int:
     the folder is no backup.
     """
     run = Path(os.path.abspath(os.path.expanduser(str(run))))
-    if not run.is_dir() or run.parent.name != BACKUP_DIR:
+    if (not run.is_dir() or run.is_symlink() or run.parent.name != BACKUP_DIR
+            or run.parent.is_symlink() or not (run / BACKUP_MARKER).is_file()):
         print(f"Error: {run} is not a folder the installer kept files in.", file=sys.stderr)
         print(f"  Name one of the dated folders inside a {BACKUP_DIR} folder, as the "
               "end of the install printed it.", file=sys.stderr)
         return 2
     root = run.parent.parent
+    # Links inside the backup are never followed: os.walk does not descend
+    # into a linked folder and a linked file is skipped.
     files = sorted(
-        path for path in run.rglob("*")
-        if path.is_file() and not path.is_symlink()
-        and not (path.name.startswith(".") and path.name.endswith(".part"))
+        Path(base) / name
+        for base, _dirs, names in os.walk(run, followlinks=False)
+        for name in names
+        if not (Path(base) / name).is_symlink()
+        and not (name.startswith(".") and name.endswith(".part"))
+        and not (Path(base) == run and name == BACKUP_MARKER)
     )
     if not files:
         print(f"{run} holds no file. Nothing to put back.")

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="8bf9b5bf68dbde80025b1b4e1c18e31f43c016758df91be202613933f1de17d5"
+DEFAULT_INSTALL_SHA256="001ddf03177f12051e70d5f57db788316c7971f23366ff2532f91a2b9aca2ffd"
 MAX_INSTALLER_BYTES=2097152
 
 cleanup() {

tests/test_install.py

@@ -5205,6 +5205,41 @@ class AFileYouHadIsKept(unittest.TestCase):
             self.assertEqual(install.restore_kept(self.bios), 2)
         self.assertIn(install.BACKUP_DIR, err.getvalue())
 
+    def test_a_linked_backup_holder_is_not_written_through(self):
+        # Someone with write access to a shared parent folder placed a link
+        # where the holder goes: the user's file must not land at its target.
+        elsewhere = self.home / "elsewhere"
+        elsewhere.mkdir()
+        (self.home / install.BACKUP_DIR).symlink_to(elsewhere)
+        (self.bios / "x.bin").write_bytes(self.OLD)
+        self.assertTrue(self._download("x.bin")[1])
+        self.assertEqual(list(elsewhere.iterdir()), [])
+        inside = list((self.bios / install.BACKUP_DIR).iterdir())
+        self.assertEqual(len(inside), 1)
+        self.assertEqual((inside[0] / "x.bin").read_bytes(), self.OLD)
+
+    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)
+        (planted / "system" / "x.bin").write_bytes(b"not the user's")
+        (self.bios / "x.bin").write_bytes(self.OLD)
+        with contextlib.redirect_stderr(io.StringIO()):
+            self.assertEqual(install.restore_kept(planted), 2)
+        self.assertEqual((self.bios / "x.bin").read_bytes(), self.OLD)
+
+    def test_restore_does_not_follow_links_inside_the_backup(self):
+        (self.bios / "x.bin").write_bytes(self.OLD)
+        self.assertTrue(self._download("x.bin")[1])
+        run = self._run_folder()
+        outside = self.home / "outside"
+        (outside / "deep").mkdir(parents=True)
+        (outside / "deep" / "y.bin").write_bytes(b"foreign")
+        (run / "system" / "linked").symlink_to(outside)
+        with contextlib.redirect_stdout(io.StringIO()):
+            self.assertEqual(install.restore_kept(run), 0)
+        self.assertEqual((self.bios / "x.bin").read_bytes(), self.OLD)
+        self.assertFalse((self.bios / "linked").exists())
+
 
 class KeptFilesEndToEnd(unittest.TestCase, _SyntheticTree):
     """The run says what it kept and where, --dry-run what it would keep

wiki/faq.md

@@ -83,7 +83,7 @@ that folder and the command that puts the files back, the same command with
 `retrobios-backup` does the same. Files that were not there before can be
 deleted from the BIOS folder. The installer changed nothing outside it, apart
 from the backup folder.
-[More on kept files](installer.md#files-you-already-had).
+Details: [kept files](installer.md#files-you-already-had).
 
 ## Can I install onto an SD card or another machine's drive?
 

wiki/first-install.md

@@ -260,5 +260,5 @@ may need a file that nobody has dumped yet.
 To remove what the installer wrote, delete the files from the BIOS folder.
 A file you had that it replaced is in `retrobios-backup`, beside that folder:
 the end of the run printed the `--restore` command that puts it back
-([details](installer.md#files-you-already-had)). Nothing else outside the
+(Details: [kept files](installer.md#files-you-already-had)). Nothing else outside the
 BIOS folder was changed.

wiki/installer.md

@@ -168,6 +168,12 @@ inside it, and the same `--restore` puts the files back. A file that cannot be
 moved aside is not replaced: it stays, and the run reports it as failed with
 `file in place could not be kept aside`.
 
+A BIOS folder can sit on a network share other people write to. So a
+`retrobios-backup` that is a link is never written through (the backup goes
+inside the BIOS folder instead), and `--restore` only takes a folder the
+installer created, marked by a `.retrobios-backup` file, without following
+links found inside it.
+
 On one drive the old file is moved, not copied, so keeping it needs no space
 beyond what it already used. A link sitting at a file's name is not kept: the
 file it points to is never touched. Data trees and `--standalone-copies` keep

wiki/troubleshooting.md

@@ -46,7 +46,7 @@ looks for the name. Common causes:
 
 Running the same command without `--check` replaces each file with a wrong
 hash by a verified copy, and keeps yours in `retrobios-backup` beside the BIOS
-folder ([putting it back](installer.md#files-you-already-had)). Each system page on the site also lists the expected
+folder (Details: [putting it back](installer.md#files-you-already-had)). Each system page on the site also lists the expected
 hashes, to compare against your file's MD5 or SHA1 by hand.
 
 ### Wrong region BIOS