fix(launcher): close LA4 review findings

src/AcDream.Launcher.Core/Profiles/LauncherProfileStore.cs

@@ -81,6 +81,8 @@ public sealed class LauncherProfileStore
             return false;
         }
 
+        EnsureExistingCredentialFilePermissions();
+
         LauncherProfileDocument? document;
         using (FileStream stream = File.OpenRead(FilePath))
         {
@@ -110,6 +112,7 @@ public sealed class LauncherProfileStore
                 + $"expected {CurrentVersion}.");
         }
 
+        ValidateAndNormalizeDocument(document);
         Document = document;
         return true;
     }
@@ -152,6 +155,13 @@ public sealed class LauncherProfileStore
                 JsonSerializer.Serialize(stream, Document, SerializerOptions);
             }
 
+            if (OperatingSystem.IsLinux()
+                && File.GetUnixFileMode(tempPath) != OwnerOnlyFileMode)
+            {
+                throw new IOException(
+                    "The launcher credential temp file could not be secured to mode 0600.");
+            }
+
             File.Move(tempPath, FilePath, overwrite: true);
         }
         catch
@@ -160,10 +170,6 @@ public sealed class LauncherProfileStore
             throw;
         }
 
-        if (OperatingSystem.IsLinux())
-        {
-            File.SetUnixFileMode(FilePath, OwnerOnlyFileMode);
-        }
     }
 
     /// <summary>
@@ -248,21 +254,21 @@ public sealed class LauncherProfileStore
                 throw new LauncherProfileException(
                     $"A server named '{newName}' already exists.");
             }
-
-            server.Name = newName;
         }
 
         if (newHost is not null)
         {
             ArgumentException.ThrowIfNullOrWhiteSpace(newHost);
-            server.Host = newHost;
         }
 
         if (newPort is not null)
         {
             RequireValidPort(newPort.Value);
-            server.Port = newPort.Value;
         }
+
+        server.Name = newName ?? server.Name;
+        server.Host = newHost ?? server.Host;
+        server.Port = newPort ?? server.Port;
     }
 
     public void RemoveServer(string name)
@@ -308,10 +314,10 @@ public sealed class LauncherProfileStore
                 throw new LauncherProfileException(
                     $"Account '{newAccount}' already exists on server '{serverName}'.");
             }
-
-            profile.Account = newAccount;
         }
 
+        profile.Account = newAccount ?? profile.Account;
+
         if (newPassword is not null)
         {
             profile.Password = newPassword;
@@ -345,6 +351,9 @@ public sealed class LauncherProfileStore
         IReadOnlyList<string>? loginCommands = null)
     {
         ArgumentException.ThrowIfNullOrWhiteSpace(characterName);
+        RequireValidLaunchMode(launchMode);
+        ValidateStringList(plugins, "plugin", requireUnique: true);
+        ValidateStringList(loginCommands, "login command", requireUnique: false);
         ServerProfile server = FindServerOrThrow(serverName);
         AccountProfile profile = FindAccountOrThrow(server, account);
 
@@ -393,6 +402,8 @@ public sealed class LauncherProfileStore
         AccountProfile profile = FindAccountOrThrow(server, account);
         CharacterProfile character = FindCharacterOrThrow(profile, characterName);
 
+        string? normalizedId = null;
+
         if (newName is not null)
         {
             ArgumentException.ThrowIfNullOrWhiteSpace(newName);
@@ -402,13 +413,11 @@ public sealed class LauncherProfileStore
                 throw new LauncherProfileException(
                     $"Character '{newName}' already exists on account '{account}'.");
             }
-
-            character.Name = newName;
         }
 
         if (newId is not null)
         {
-            string? normalizedId = NormalizeCharacterId(newId);
+            normalizedId = NormalizeCharacterId(newId);
             if (normalizedId is not null
                 && profile.Characters.Any(candidate =>
                     !ReferenceEquals(candidate, character)
@@ -417,7 +426,19 @@ public sealed class LauncherProfileStore
                 throw new LauncherProfileException(
                     $"Character id '{normalizedId}' already exists on account '{account}'.");
             }
+        }
 
+        if (launchMode is not null)
+        {
+            RequireValidLaunchMode(launchMode.Value);
+        }
+
+        ValidateStringList(plugins, "plugin", requireUnique: true);
+        ValidateStringList(loginCommands, "login command", requireUnique: false);
+
+        character.Name = newName ?? character.Name;
+        if (newId is not null)
+        {
             character.Id = normalizedId;
         }
 
@@ -437,6 +458,57 @@ public sealed class LauncherProfileStore
         }
     }
 
+    private void EnsureExistingCredentialFilePermissions()
+    {
+        if (!OperatingSystem.IsLinux())
+        {
+            return;
+        }
+
+        try
+        {
+            UnixFileMode mode = File.GetUnixFileMode(FilePath);
+            if (mode != OwnerOnlyFileMode)
+            {
+                File.SetUnixFileMode(FilePath, OwnerOnlyFileMode);
+                mode = File.GetUnixFileMode(FilePath);
+            }
+
+            if (mode != OwnerOnlyFileMode)
+            {
+                throw new IOException($"Mode remained {mode} after normalization.");
+            }
+        }
+        catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
+        {
+            throw new LauncherProfileException(
+                $"'{FilePath}' could not be secured to owner-only mode 0600.",
+                ex);
+        }
+    }
+
+    /// <summary>
+    /// Applies one profile mutation and its atomic file replacement as a
+    /// single in-memory/on-disk transaction. Any validation or I/O failure
+    /// restores the exact pre-mutation document, including credentials.
+    /// </summary>
+    public void ExecuteTransaction(Action mutation)
+    {
+        ArgumentNullException.ThrowIfNull(mutation);
+        LauncherProfileDocument before = CloneDocument(Document);
+        try
+        {
+            mutation();
+            ValidateAndNormalizeDocument(Document);
+            Save();
+        }
+        catch
+        {
+            Document = before;
+            throw;
+        }
+    }
+
     public void RemoveCharacter(
         string serverName,
         string account,
@@ -472,38 +544,49 @@ public sealed class LauncherProfileStore
         ServerProfile server = FindServerOrThrow(serverName);
         AccountProfile profile = FindAccountOrThrow(server, account);
 
+        var rosterIds = new HashSet<uint>();
+        var rosterNames = new HashSet<string>(StringComparer.Ordinal);
+        foreach (CharacterRosterEntry entry in roster)
+        {
+            if (entry.Id == 0)
+            {
+                throw new LauncherProfileException("A roster character id cannot be zero.");
+            }
+
+            ArgumentException.ThrowIfNullOrWhiteSpace(entry.Name);
+            if (!rosterIds.Add(entry.Id) || !rosterNames.Add(entry.Name))
+            {
+                throw new LauncherProfileException(
+                    "The reported character roster contains a duplicate id or name.");
+            }
+        }
+
         foreach (CharacterRosterEntry entry in roster)
         {
             string idText = CharacterIdFormat.ToHexString(entry.Id);
 
-            // Normalize BOTH sides through TryParse/ToHexString rather
-            // than a raw string compare (Campaign LA plan §LA3 review
-            // finding F10): a stored id that round-trips to the same
-            // uint (different case, or — before this fix — no "0x"
-            // prefix) must match even though its text isn't byte-
-            // identical to the canonical form this method itself always
-            // writes.
-            CharacterProfile? existing = profile.Characters.Find(
-                character => CharacterIdFormat.TryParse(character.Id, out uint existingId)
-                    && existingId == entry.Id);
-
-            // Defensive fallback for a row whose id is missing OR
-            // unparseable (e.g. a hand-edited id with no "0x" prefix,
-            // which TryParse now rejects outright) — match by name
-            // instead so a later merge self-heals the id into the
-            // canonical form rather than creating a permanent duplicate
-            // row.
-            existing ??= profile.Characters.Find(
-                character => !CharacterIdFormat.TryParse(character.Id, out _)
-                    && string.Equals(
-                        character.Name,
-                        entry.Name,
-                        StringComparison.Ordinal));
+            CharacterProfile[] matches = profile.Characters
+                .Where(character =>
+                    (CharacterIdFormat.TryParse(character.Id, out uint existingId)
+                        && existingId == entry.Id)
+                    || string.Equals(character.Name, entry.Name, StringComparison.Ordinal))
+                .ToArray();
+            CharacterProfile? existing = matches.FirstOrDefault(character =>
+                    CharacterIdFormat.TryParse(character.Id, out uint existingId)
+                    && existingId == entry.Id)
+                ?? matches.FirstOrDefault();
 
             if (existing is not null)
             {
                 existing.Id = idText;
                 existing.Name = entry.Name;
+                foreach (CharacterProfile duplicate in matches)
+                {
+                    if (!ReferenceEquals(duplicate, existing))
+                    {
+                        profile.Characters.Remove(duplicate);
+                    }
+                }
                 continue;
             }
 
@@ -583,6 +666,192 @@ public sealed class LauncherProfileStore
         && CharacterIdFormat.TryParse(right, out uint rightId)
         && leftId == rightId;
 
+    private static LauncherProfileDocument CloneDocument(
+        LauncherProfileDocument source) =>
+        new()
+        {
+            Version = source.Version,
+            Servers = source.Servers.Select(server => new ServerProfile
+            {
+                Name = server.Name,
+                Host = server.Host,
+                Port = server.Port,
+                Accounts = server.Accounts.Select(account => new AccountProfile
+                {
+                    Account = account.Account,
+                    Password = account.Password,
+                    Characters = account.Characters.Select(character => new CharacterProfile
+                    {
+                        Name = character.Name,
+                        Id = character.Id,
+                        LaunchMode = character.LaunchMode,
+                        Plugins = [.. character.Plugins],
+                        LoginCommands = [.. character.LoginCommands],
+                    }).ToList(),
+                }).ToList(),
+            }).ToList(),
+        };
+
+    private static void ValidateAndNormalizeDocument(LauncherProfileDocument document)
+    {
+        if (document.Servers is null)
+        {
+            throw new LauncherProfileException("The servers collection cannot be null.");
+        }
+
+        var serverNames = new HashSet<string>(StringComparer.Ordinal);
+        var normalizedIds = new List<(CharacterProfile Character, uint Id)>();
+        foreach (ServerProfile? server in document.Servers)
+        {
+            if (server is null)
+            {
+                throw new LauncherProfileException("A server entry cannot be null.");
+            }
+
+            RequireLoadedText(server.Name, "server name");
+            RequireLoadedText(server.Host, $"host for server '{server.Name}'");
+            RequireValidPort(server.Port);
+            if (!serverNames.Add(server.Name))
+            {
+                throw new LauncherProfileException(
+                    $"A server named '{server.Name}' appears more than once.");
+            }
+
+            if (server.Accounts is null)
+            {
+                throw new LauncherProfileException(
+                    $"The accounts collection for server '{server.Name}' cannot be null.");
+            }
+
+            var accountNames = new HashSet<string>(StringComparer.Ordinal);
+            foreach (AccountProfile? account in server.Accounts)
+            {
+                if (account is null)
+                {
+                    throw new LauncherProfileException(
+                        $"A null account appears under server '{server.Name}'.");
+                }
+
+                RequireLoadedText(account.Account, "account name");
+                if (account.Password is null)
+                {
+                    throw new LauncherProfileException(
+                        $"Password for account '{account.Account}' cannot be null.");
+                }
+
+                if (!accountNames.Add(account.Account))
+                {
+                    throw new LauncherProfileException(
+                        $"Account '{account.Account}' appears more than once on server '{server.Name}'.");
+                }
+
+                if (account.Characters is null)
+                {
+                    throw new LauncherProfileException(
+                        $"The characters collection for account '{account.Account}' cannot be null.");
+                }
+
+                var characterNames = new HashSet<string>(StringComparer.Ordinal);
+                var characterIds = new HashSet<uint>();
+                foreach (CharacterProfile? character in account.Characters)
+                {
+                    if (character is null)
+                    {
+                        throw new LauncherProfileException(
+                            $"A null character appears under account '{account.Account}'.");
+                    }
+
+                    RequireLoadedText(character.Name, "character name");
+                    if (!characterNames.Add(character.Name))
+                    {
+                        throw new LauncherProfileException(
+                            $"Character '{character.Name}' appears more than once on account '{account.Account}'.");
+                    }
+
+                    RequireValidLaunchMode(character.LaunchMode);
+                    if (character.Id is not null)
+                    {
+                        if (!CharacterIdFormat.TryParse(character.Id, out uint id) || id == 0)
+                        {
+                            throw new LauncherProfileException(
+                                $"Character '{character.Name}' has an invalid id '{character.Id}'.");
+                        }
+
+                        if (!characterIds.Add(id))
+                        {
+                            throw new LauncherProfileException(
+                                $"Character id '{character.Id}' appears more than once on account '{account.Account}'.");
+                        }
+
+                        normalizedIds.Add((character, id));
+                    }
+
+                    if (character.Plugins is null || character.LoginCommands is null)
+                    {
+                        throw new LauncherProfileException(
+                            $"Character '{character.Name}' has a null settings collection.");
+                    }
+
+                    ValidateStringList(character.Plugins, "plugin", requireUnique: true);
+                    ValidateStringList(
+                        character.LoginCommands,
+                        "login command",
+                        requireUnique: false);
+                }
+            }
+        }
+
+        foreach ((CharacterProfile character, uint id) in normalizedIds)
+        {
+            character.Id = CharacterIdFormat.ToHexString(id);
+        }
+    }
+
+    private static void ValidateStringList(
+        IReadOnlyList<string>? values,
+        string valueName,
+        bool requireUnique)
+    {
+        if (values is null)
+        {
+            return;
+        }
+
+        HashSet<string>? seen = requireUnique
+            ? new HashSet<string>(StringComparer.Ordinal)
+            : null;
+        foreach (string? value in values)
+        {
+            if (string.IsNullOrWhiteSpace(value))
+            {
+                throw new LauncherProfileException(
+                    $"A {valueName} cannot be null or whitespace.");
+            }
+
+            if (seen is not null && !seen.Add(value))
+            {
+                throw new LauncherProfileException(
+                    $"The {valueName} '{value}' appears more than once.");
+            }
+        }
+    }
+
+    private static void RequireLoadedText(string? value, string field)
+    {
+        if (string.IsNullOrWhiteSpace(value))
+        {
+            throw new LauncherProfileException($"The {field} cannot be null or whitespace.");
+        }
+    }
+
+    private static void RequireValidLaunchMode(LaunchMode mode)
+    {
+        if (!Enum.IsDefined(mode))
+        {
+            throw new LauncherProfileException($"Launch mode '{mode}' is not supported.");
+        }
+    }
+
     private static void RequireValidPort(int port)
     {
         if (port is < 1 or > 65535)