diff --git a/install.ps1 b/install.ps1 index d375f724..f5a1a8c2 100644 --- a/install.ps1 +++ b/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 } diff --git a/install.py b/install.py index 3a0f3408..82c4b944 100644 --- a/install.py +++ b/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.") diff --git a/install.sh b/install.sh index f018b499..2b2c34c0 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="8bf9b5bf68dbde80025b1b4e1c18e31f43c016758df91be202613933f1de17d5" +DEFAULT_INSTALL_SHA256="001ddf03177f12051e70d5f57db788316c7971f23366ff2532f91a2b9aca2ffd" MAX_INSTALLER_BYTES=2097152 cleanup() { diff --git a/tests/test_install.py b/tests/test_install.py index 73a739d6..94aaab87 100644 --- a/tests/test_install.py +++ b/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 diff --git a/wiki/faq.md b/wiki/faq.md index f49c89e8..076b405a 100644 --- a/wiki/faq.md +++ b/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? diff --git a/wiki/first-install.md b/wiki/first-install.md index 8df22bf1..0dd5a77a 100644 --- a/wiki/first-install.md +++ b/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. diff --git a/wiki/installer.md b/wiki/installer.md index 27939b74..c72a5cc0 100644 --- a/wiki/installer.md +++ b/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 diff --git a/wiki/troubleshooting.md b/wiki/troubleshooting.md index fabf451b..ee524974 100644 --- a/wiki/troubleshooting.md +++ b/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