Address all review feedback: salt injection, code style, localization formatting

Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>

.github/workflows/main.yml

@@ -123,6 +123,8 @@ jobs:
         uses: actions/setup-dotnet@baa11fbfe1d6520db94683bd5c7a3818018e4309 # v5.1.0
 
       - name: Publish app
+        env:
+          ENCRYPTION_SALT: ${{ secrets.ENCRYPTION_SALT }}
         run: >
           dotnet publish ${{ matrix.app }}
           -p:Version=${{ github.ref_type == 'tag' && github.ref_name || format('999.9.9-ci-{0}', github.sha) }}

DiscordChatExporter.Gui/DiscordChatExporter.Gui.csproj

@@ -18,6 +18,31 @@
     <EnableAotAnalyzer>false</EnableAotAnalyzer>
   </PropertyGroup>
 
+  <!-- Set ENCRYPTION_SALT env var in CI to use a build-specific salt.
+       Local/dev builds fall back to a stable default. -->
+  <PropertyGroup>
+    <EncryptionSalt Condition="'$(ENCRYPTION_SALT)' != ''">$(ENCRYPTION_SALT)</EncryptionSalt>
+    <EncryptionSalt Condition="'$(EncryptionSalt)' == ''">DCE-Token-Salt</EncryptionSalt>
+  </PropertyGroup>
+
+  <Target Name="GenerateEncryptionSaltSource" BeforeTargets="CoreCompile">
+    <ItemGroup>
+      <_SaltLines Include="namespace DiscordChatExporter.Gui.Services%3B" />
+      <_SaltLines Include="internal partial class TokenEncryptionConverter" />
+      <_SaltLines Include="{" />
+      <_SaltLines Include="    private const string EncryptionSalt = &quot;$(EncryptionSalt)&quot;%3B" />
+      <_SaltLines Include="}" />
+    </ItemGroup>
+    <WriteLinesToFile
+      File="$(IntermediateOutputPath)TokenEncryptionConverter.EncryptionSalt.g.cs"
+      Lines="@(_SaltLines)"
+      Overwrite="true"
+      Encoding="UTF-8" />
+    <ItemGroup>
+      <Compile Include="$(IntermediateOutputPath)TokenEncryptionConverter.EncryptionSalt.g.cs" />
+    </ItemGroup>
+  </Target>
+
   <ItemGroup>
     <AvaloniaResource Include="..\favicon.ico" Link="favicon.ico" />
   </ItemGroup>

DiscordChatExporter.Gui/Localization/LocalizationManager.English.cs

@@ -53,7 +53,10 @@ public partial class LocalizationManager
             [nameof(AutoUpdateTooltip)] = "Perform automatic updates on every launch",
             [nameof(PersistTokenLabel)] = "Persist token",
             [nameof(PersistTokenTooltip)] =
-                "Save the last used token to a file so that it can be persisted between sessions. The token is stored encrypted, but may still be recovered by an attacker who has access to your file system.",
+                """
+                Save the last used token to a file so that it can be persisted between sessions.
+                **Warning**: although the token is stored encrypted, it may still be recovered by an attacker who has access to your system.
+                """,
             [nameof(RateLimitPreferenceLabel)] = "Rate limit preference",
             [nameof(RateLimitPreferenceTooltip)] =
                 "Whether to respect advisory rate limits. If disabled, only hard rate limits (i.e. 429 responses) will be respected.",

DiscordChatExporter.Gui/Localization/LocalizationManager.French.cs

@@ -55,7 +55,10 @@ public partial class LocalizationManager
         [nameof(AutoUpdateTooltip)] = "Effectuer des mises à jour automatiques à chaque lancement",
         [nameof(PersistTokenLabel)] = "Conserver le token",
         [nameof(PersistTokenTooltip)] =
-            "Enregistrer le dernier token utilisé dans un fichier pour le conserver entre les sessions. Le token est stocké chiffré, mais peut toujours être récupéré par un attaquant ayant accès à votre système de fichiers.",
+            """
+            Enregistrer le dernier token utilisé dans un fichier pour le conserver entre les sessions.
+            **Avertissement** : bien que le token soit stocké chiffré, il peut toujours être récupéré par un attaquant ayant accès à votre système.
+            """,
         [nameof(RateLimitPreferenceLabel)] = "Préférence de limite de débit",
         [nameof(RateLimitPreferenceTooltip)] =
             "Indique s'il faut respecter les limites de débit recommandées. Si désactivé, seules les limites strictes (réponses 429) seront respectées.",

DiscordChatExporter.Gui/Localization/LocalizationManager.German.cs

@@ -55,7 +55,10 @@ public partial class LocalizationManager
         [nameof(AutoUpdateTooltip)] = "Automatische Updates bei jedem Start durchführen",
         [nameof(PersistTokenLabel)] = "Token speichern",
         [nameof(PersistTokenTooltip)] =
-            "Den zuletzt verwendeten Token in einer Datei speichern, damit er zwischen Sitzungen erhalten bleibt. Der Token wird verschlüsselt gespeichert, kann aber dennoch von einem Angreifer mit Zugriff auf Ihr Dateisystem wiederhergestellt werden.",
+            """
+            Den zuletzt verwendeten Token in einer Datei speichern, damit er zwischen Sitzungen erhalten bleibt.
+            **Warnung**: Der Token wird verschlüsselt gespeichert, kann aber dennoch von einem Angreifer mit Zugriff auf Ihr System wiederhergestellt werden.
+            """,
         [nameof(RateLimitPreferenceLabel)] = "Ratenlimit-Einstellung",
         [nameof(RateLimitPreferenceTooltip)] =
             "Ob empfohlene Ratenlimits eingehalten werden sollen. Wenn deaktiviert, werden nur harte Ratenlimits (d. h. 429-Antworten) eingehalten.",

DiscordChatExporter.Gui/Localization/LocalizationManager.Spanish.cs

@@ -53,7 +53,10 @@ public partial class LocalizationManager
             [nameof(AutoUpdateTooltip)] = "Realizar actualizaciones automáticas en cada inicio",
             [nameof(PersistTokenLabel)] = "Guardar token",
             [nameof(PersistTokenTooltip)] =
-                "Guardar el último token utilizado en un archivo para conservarlo entre sesiones. El token se almacena cifrado, pero aún puede ser recuperado por un atacante con acceso a tu sistema de archivos.",
+                """
+                Guardar el último token utilizado en un archivo para conservarlo entre sesiones.
+                **Advertencia**: aunque el token se almacena cifrado, aún puede ser recuperado por un atacante con acceso a tu sistema.
+                """,
             [nameof(RateLimitPreferenceLabel)] = "Preferencia de límite de velocidad",
             [nameof(RateLimitPreferenceTooltip)] =
                 "Si se deben respetar los límites de velocidad recomendados. Si está desactivado, solo se respetarán los límites estrictos (respuestas 429).",

DiscordChatExporter.Gui/Localization/LocalizationManager.Ukrainian.cs

@@ -53,7 +53,10 @@ public partial class LocalizationManager
             [nameof(AutoUpdateTooltip)] = "Виконувати автоматичні оновлення при кожному запуску",
             [nameof(PersistTokenLabel)] = "Зберігати токен",
             [nameof(PersistTokenTooltip)] =
-                "Зберігати останній використаний токен у файлі для збереження між сеансами. Токен зберігається у зашифрованому вигляді, але може бути відновлений зловмисником, який має доступ до вашої файлової системи.",
+                """
+                Зберігати останній використаний токен у файлі для збереження між сеансами.
+                **Увага**: хоча токен зберігається у зашифрованому вигляді, він може бути відновлений зловмисником, який має доступ до вашої системи.
+                """,
             [nameof(RateLimitPreferenceLabel)] = "Ліміт запитів",
             [nameof(RateLimitPreferenceTooltip)] =
                 "Чи дотримуватись рекомендованих лімітів запитів. Якщо вимкнено, будуть дотримуватись лише жорсткі ліміти (тобто відповіді 429).",

DiscordChatExporter.Gui/Services/TokenEncryptionConverter.cs

@@ -7,13 +7,13 @@ using System.Text.Json.Serialization;
 
 namespace DiscordChatExporter.Gui.Services;
 
-internal class TokenEncryptionConverter : JsonConverter<string?>
+internal partial class TokenEncryptionConverter : JsonConverter<string?>
 {
     private const string Prefix = "enc:";
     private const int MaxPaddingLength = 16;
 
-    // Key is derived from a machine-specific identifier so that a stolen Settings.dat
-    // cannot be decrypted on a different machine.
+    // Key is derived from a machine-specific identifier so that a stolen settings file
+    // cannot be easily decrypted on a different machine.
     private static readonly Lazy<byte[]> Key = new(DeriveKey);
 
     private static byte[] DeriveKey()
@@ -21,7 +21,7 @@ internal class TokenEncryptionConverter : JsonConverter<string?>
         var machineId = GetMachineId();
         return Rfc2898DeriveBytes.Pbkdf2(
             Encoding.UTF8.GetBytes(machineId),
-            "DCE-Token-Salt"u8.ToArray(),
+            Encoding.UTF8.GetBytes(EncryptionSalt),
             iterations: 10_000,
             HashAlgorithmName.SHA256,
             outputLength: 16
@@ -38,7 +38,7 @@ internal class TokenEncryptionConverter : JsonConverter<string?>
                 using var regKey = Microsoft.Win32.Registry.LocalMachine.OpenSubKey(
                     @"SOFTWARE\Microsoft\Cryptography"
                 );
-                if (regKey?.GetValue("MachineGuid") is string guid && guid.Length > 0)
+                if (regKey?.GetValue("MachineGuid") is string guid && !string.IsNullOrWhiteSpace(guid))
                     return guid;
             }
             catch { }
@@ -50,7 +50,7 @@ internal class TokenEncryptionConverter : JsonConverter<string?>
             try
             {
                 var id = File.ReadAllText(path).Trim();
-                if (id.Length > 0)
+                if (!string.IsNullOrWhiteSpace(id))
                     return id;
             }
             catch { }
@@ -68,78 +68,67 @@ internal class TokenEncryptionConverter : JsonConverter<string?>
     {
         var value = reader.GetString();
 
-        // No prefix means the token is stored as plain text (backward compatibility)
+        // No prefix means the token is stored as plain text, which was
+        // the case for older versions of the application.
+        // Load it as is and encrypt it next time we save it.
         if (string.IsNullOrWhiteSpace(value) || !value.StartsWith(Prefix, StringComparison.Ordinal))
             return value;
 
-        byte[] data;
         try
         {
-            data = Convert.FromHexString(value[Prefix.Length..]);
-        }
-        catch (FormatException)
-        {
-            return null;
-        }
+            var data = Convert.FromHexString(value[Prefix.Length..]);
 
-        // Layout: Nonce (12 bytes) | padLen (1 byte) | Tag (16 bytes) | Ciphertext
-        if (data.Length < 29)
-            return null;
+            // Layout: Nonce (12 bytes) | padLen (1 byte) | Tag (16 bytes) | Ciphertext
+            var padLen = data[12];
+            var nonce = data.AsSpan(0, 12);
+            var tag = data.AsSpan(13, 16);
+            var ciphertext = data.AsSpan(29);
 
-        var padLen = data[12];
-        if (padLen < 1 || padLen > MaxPaddingLength)
-            return null;
-
-        var nonce = data.AsSpan(0, 12);
-        var tag = data.AsSpan(13, 16);
-        var ciphertext = data.AsSpan(29);
-
-        var decrypted = new byte[ciphertext.Length];
-        try
-        {
+            var decrypted = new byte[ciphertext.Length];
             using var aes = new AesGcm(Key.Value, 16);
             aes.Decrypt(nonce, ciphertext, tag, decrypted);
+
+            return Encoding.UTF8.GetString(decrypted.AsSpan(padLen));
         }
-        catch (CryptographicException)
+        catch (Exception ex)
+            when (
+                ex
+                is FormatException
+                    or CryptographicException
+                    or ArgumentException
+                    or IndexOutOfRangeException
+            )
         {
             return null;
         }
-
-        if (padLen > decrypted.Length)
-            return null;
-
-        return Encoding.UTF8.GetString(decrypted.AsSpan(padLen));
     }
 
     public override void Write(Utf8JsonWriter writer, string? value, JsonSerializerOptions options)
     {
-        if (value is null)
+        if (string.IsNullOrWhiteSpace(value))
         {
             writer.WriteNullValue();
             return;
         }
 
         var nonce = RandomNumberGenerator.GetBytes(12);
-        var padLen = RandomNumberGenerator.GetInt32(1, MaxPaddingLength + 1);
+        var padding = RandomNumberGenerator.GetBytes(
+            RandomNumberGenerator.GetInt32(1, MaxPaddingLength + 1)
+        );
         var tokenBytes = Encoding.UTF8.GetBytes(value);
 
-        // Random padding bytes vary the output length
-        var padded = new byte[padLen + tokenBytes.Length];
-        RandomNumberGenerator.Fill(padded.AsSpan(0, padLen));
-        tokenBytes.CopyTo(padded, padLen);
-
-        var ciphertext = new byte[padded.Length];
-        var tag = new byte[16];
-
-        using var aes = new AesGcm(Key.Value, 16);
-        aes.Encrypt(nonce, padded, ciphertext, tag);
+        // Assemble plaintext: padding + token
+        var plaintext = new byte[padding.Length + tokenBytes.Length];
+        padding.CopyTo(plaintext.AsSpan());
+        tokenBytes.CopyTo(plaintext.AsSpan(padding.Length));
 
         // Layout: Nonce (12 bytes) | padLen (1 byte) | Tag (16 bytes) | Ciphertext
-        var data = new byte[29 + ciphertext.Length];
+        var data = new byte[29 + plaintext.Length];
         nonce.CopyTo(data.AsSpan(0, 12));
-        data[12] = (byte)padLen;
-        tag.CopyTo(data.AsSpan(13, 16));
-        ciphertext.CopyTo(data.AsSpan(29));
+        data[12] = (byte)padding.Length;
+
+        using var aes = new AesGcm(Key.Value, 16);
+        aes.Encrypt(nonce, plaintext, data.AsSpan(29), data.AsSpan(13, 16));
 
         writer.WriteStringValue(Prefix + Convert.ToHexStringLower(data));
     }