diff --git a/Assets/Scripts/Gameplay/ShipModShips.cs b/Assets/Scripts/Gameplay/ShipModShips.cs index aff2f9755..89106a6b7 100644 --- a/Assets/Scripts/Gameplay/ShipModShips.cs +++ b/Assets/Scripts/Gameplay/ShipModShips.cs @@ -66,7 +66,7 @@ public static class ShipModShips catch (Exception error) { visual?.Destroy(); - faults.Add(new ShipModCatalog.Exclusion { Package = catalog.Get(hull.Visual)?.Id ?? hull.Name, Reason = $"{hull.Name}: {error.Message}" }); + faults.Add(new ShipModCatalog.Exclusion { Package = catalog.Get(hull.Visual)?.Id ?? "(all mods)", Reason = $"{hull.Name}: {error.Message}" }); } } if (faults.Count > 0) throw new ImportFault(faults.ToArray()); diff --git a/Assets/Scripts/ServerShared/ShipModCatalog.cs b/Assets/Scripts/ServerShared/ShipModCatalog.cs index 35dbc24c6..ba912904d 100644 --- a/Assets/Scripts/ServerShared/ShipModCatalog.cs +++ b/Assets/Scripts/ServerShared/ShipModCatalog.cs @@ -194,22 +194,48 @@ public static class ShipModCatalog // Returns the destination. Compose and ResolveCatalog never read RemovedMods. public static string RemovePackage(string modsRoot, string package) { - if (string.IsNullOrEmpty(package) || package == "." || package == ".." || package == "(all mods)" || - package != Path.GetFileName(package) || package.IndexOfAny(Path.GetInvalidFileNameChars()) >= 0) - throw new ArgumentException("Not a mod package name.", nameof(package)); - var source = Path.Combine(Path.GetFullPath(modsRoot), package); - if (!Directory.Exists(source)) throw new DirectoryNotFoundException("The package is not a directory under the mods folder."); + var root = Path.GetFullPath(modsRoot).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + // Containment, not a blacklist: the package must resolve to a strict child of the mods folder whose final name is + // the name given. A name the filesystem rewrites ('...', 'a.', 'a ', 'CON', 'a:b') resolves to something else. + if (!IsPackageName(package)) throw new ArgumentException("Not a mod package name.", nameof(package)); + var source = ChildOf(root, package) ?? throw new ArgumentException("Not a mod package name.", nameof(package)); + var info = new DirectoryInfo(source); + if (!info.Exists) throw new DirectoryNotFoundException("The package is not a directory under the mods folder."); + if (info.Attributes.HasFlag(FileAttributes.ReparsePoint)) + throw new ArgumentException("A package that is a link is not removed.", nameof(package)); var removed = RemovedRoot(modsRoot); Directory.CreateDirectory(removed); - var destination = Path.Combine(removed, package); - for (var copy = 2; Directory.Exists(destination) || File.Exists(destination); copy++) - destination = Path.Combine(removed, $"{package} ({copy})"); + var destination = ChildOf(removed, package); + for (var copy = 2; destination != null && (Directory.Exists(destination) || File.Exists(destination)); copy++) + destination = ChildOf(removed, $"{package} ({copy})"); + if (destination == null) throw new ArgumentException("Not a mod package name.", nameof(package)); Directory.Move(source, destination); return destination; } + private static readonly string[] ReservedDeviceNames = + { "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", + "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9" }; + + // A name Windows keeps as written: not a dot name, no separator, stream colon or wildcard, no trailing dot or space, and + // not a device name (with or without an extension). Held on every platform so a mod moves the same way everywhere. + private static bool IsPackageName(string name) => + !string.IsNullOrEmpty(name) && name != "(all mods)" && name.Trim('.') != "" && + name.IndexOfAny(Path.GetInvalidFileNameChars()) < 0 && name.IndexOfAny(new[] { ':', '*', '?', '"', '<', '>', '|', '/', '\\' }) < 0 && + name.All(character => character >= ' ') && !name.EndsWith('.') && !name.EndsWith(' ') && + !ReservedDeviceNames.Contains(name.Split('.')[0].TrimEnd(' '), StringComparer.OrdinalIgnoreCase); + + // The full path of `name` directly under `root`, or null when the filesystem would resolve it anywhere else or under + // another name. The comparison is of resolved path segments, never of a string prefix ('Mods2' is not inside 'Mods'). + private static string ChildOf(string root, string name) + { + var full = Path.GetFullPath(Path.Combine(root, name)); + return string.Equals(Path.GetDirectoryName(full), root, StringComparison.Ordinal) && + string.Equals(Path.GetFileName(full), name, StringComparison.Ordinal) ? full : null; + } + private static string RemovedRoot(string modsRoot) => - Path.Combine(Path.GetDirectoryName(Path.GetFullPath(modsRoot)), "RemovedMods"); + Path.Combine(Path.GetDirectoryName(Path.GetFullPath(modsRoot).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)), "RemovedMods"); private static Package Bind(HullData hull, ShipAuthoring ship, FactionProductData[] products, string directory, string path) { diff --git a/Assets/Scripts/UI/MainMenu.cs b/Assets/Scripts/UI/MainMenu.cs index ea75d836b..2114dd915 100644 --- a/Assets/Scripts/UI/MainMenu.cs +++ b/Assets/Scripts/UI/MainMenu.cs @@ -228,19 +228,23 @@ public class MainMenu : MonoBehaviour // Refuse for faults that name mod packages: each distinct package gets a Remove button, the one RemovePackage. The catalog // stays as this boot opened it, so the dialog says that removal takes effect at the next launch. + private readonly HashSet _removedPackages = new(); + public void RefuseMods(string title, ShipModCatalog.Exclusion[] faults) { + // A package moved aside this session stays named but loses its button: this boot's catalog still holds it. + var open = faults.Where(fault => !_removedPackages.Contains(fault.Package)).ToArray(); var reason = string.Join("\n", faults.Select(fault => fault.ToString())); Debug.LogError($"{title}: {reason}"); Dialog.Clear(); Dialog.Title.text = title; Dialog.AddProperty(() => reason); - foreach (var package in faults.Select(fault => fault.Package).Where(package => package != "(all mods)").Distinct()) - Dialog.AddButton($"Remove {package}", () => Remove(package)); + foreach (var package in open.Select(fault => fault.Package).Where(package => package != "(all mods)").Distinct()) + Dialog.AddButton($"Remove {package}", () => Remove(title, faults, package)); Dialog.Show(onCancel: () => { }, cancelText: "OK"); } - private void Remove(string package) + private void Remove(string title, ShipModCatalog.Exclusion[] faults, string package) { string destination; try { destination = ShipModCatalog.RemovePackage(ActionGameManager.ModsPath, package); } @@ -249,12 +253,13 @@ public class MainMenu : MonoBehaviour Refuse($"Could not remove {package}", $"{error.Message} Remove it by hand from {ActionGameManager.ModsPath}."); return; } + _removedPackages.Add(package); var done = $"{package} moved to {destination}. Mods load at launch: restart Aetheria to play without it, or move it back into Mods to reinstall."; Debug.Log(done); Dialog.Clear(); Dialog.Title.text = $"Removed {package}"; Dialog.AddProperty(() => done); - Dialog.Show(onConfirm: Application.Quit, onCancel: () => { }, confirmText: "Quit", cancelText: "OK"); + Dialog.Show(onConfirm: Application.Quit, onCancel: () => { if (faults.Any(fault => fault.Package != "(all mods)" && !_removedPackages.Contains(fault.Package))) RefuseMods(title, faults); }, confirmText: "Quit", cancelText: "OK"); } private void ShowSettings() diff --git a/tests/Aetheria.Shared.Tests/ShipModCatalogTests.cs b/tests/Aetheria.Shared.Tests/ShipModCatalogTests.cs index 729c3443f..953208163 100644 --- a/tests/Aetheria.Shared.Tests/ShipModCatalogTests.cs +++ b/tests/Aetheria.Shared.Tests/ShipModCatalogTests.cs @@ -612,20 +612,64 @@ public sealed class ShipModCatalogTests : IDisposable Assert.False(File.Exists(Path.Combine(second, "marker.txt"))); } - [Fact] - public void RemovePackageRefusesWhatIsNotAPackageDirectory() + // Hostile ids: the ones a filesystem rewrites or resolves elsewhere. A directory is created for each where the platform + // allows it, so a refusal that merely finds nothing is not enough. Nothing outside the intended package may move. + [Theory] + [InlineData("")] + [InlineData(".")] + [InlineData("..")] + [InlineData("...")] + [InlineData(".. ")] + [InlineData("a.")] + [InlineData("a ")] + [InlineData("CON")] + [InlineData("con.txt")] + [InlineData("NUL")] + [InlineData("a:b")] + [InlineData("(all mods)")] + [InlineData("a/b")] + [InlineData("a\\b")] + [InlineData("../Mods")] + [InlineData("../Mods2")] + [InlineData("missing")] + public void RemovePackageRefusesWhatIsNotAPackageDirectory(string name) { WritePackage("mod.skiff"); File.WriteAllText(Path.Combine(_directory.Path, "outside.txt"), "x"); Directory.CreateDirectory(Path.Combine(_directory.Path, "Mods2")); - foreach (var name in new[] { "", ".", "..", "(all mods)", "a/b", "a\\b", "../Mods", "missing" }) - Assert.ThrowsAny(() => ShipModCatalog.RemovePackage(Mods, name)); + try { Directory.CreateDirectory(Path.Combine(Mods, name)); } catch (Exception) { } + var before = Directory.GetDirectories(Mods).Select(Path.GetFileName).OrderBy(n => n, StringComparer.Ordinal).ToArray(); + + Assert.ThrowsAny(() => ShipModCatalog.RemovePackage(Mods, name)); + Assert.True(Directory.Exists(Mods)); Assert.True(Directory.Exists(Path.Combine(Mods, "mod.skiff"))); Assert.True(Directory.Exists(Path.Combine(_directory.Path, "Mods2"))); Assert.True(File.Exists(Path.Combine(_directory.Path, "outside.txt"))); Assert.False(Directory.Exists(RemovedMods)); - Assert.Equal(new[] { "mod.skiff" }, Directory.GetDirectories(Mods).Select(Path.GetFileName)); + Assert.Equal(before, Directory.GetDirectories(Mods).Select(Path.GetFileName).OrderBy(n => n, StringComparer.Ordinal)); + } + + [Fact] + public void RemovePackageRefusesAnAbsolutePathAndAnInPackageLinkOutsideMods() + { + WritePackage("mod.skiff"); + var outside = Path.Combine(_directory.Path, "Outside"); + Directory.CreateDirectory(outside); + File.WriteAllText(Path.Combine(outside, "keep.txt"), "x"); + Assert.ThrowsAny(() => ShipModCatalog.RemovePackage(Mods, outside)); + Assert.True(File.Exists(Path.Combine(outside, "keep.txt"))); + + var linked = true; + try { Directory.CreateSymbolicLink(Path.Combine(Mods, "link"), outside); } catch (Exception) { linked = false; } + if (linked) + { + Assert.ThrowsAny(() => ShipModCatalog.RemovePackage(Mods, "link")); + Assert.True(File.Exists(Path.Combine(outside, "keep.txt"))); + Assert.True(Directory.Exists(Path.Combine(Mods, "link"))); + } + Assert.False(Directory.Exists(RemovedMods)); + Assert.True(Directory.Exists(Path.Combine(Mods, "mod.skiff"))); } [Fact]