From df5783f99e00e20c4058ed26ea4115c76fbe3302 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Tue, 14 Jul 2026 18:07:16 -0400 Subject: [PATCH] Harden NativeAOT safety Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/dotnet-test.yml | 8 + AGENTS.md | 17 ++ .../SettingsTest.cs | 165 +----------------- .../SettingsEngine_Dictionaries.cs | 18 +- .../SettingsEngine_Lists.cs | 11 +- src/UniGetUI.Core.Settings/SettingsJson.cs | 87 +-------- src/UniGetUI.Core.Tools.Tests/MetaTests.cs | 1 + ...aries.WindowsPackageManager.Interop.csproj | 2 - 8 files changed, 58 insertions(+), 251 deletions(-) diff --git a/.github/workflows/dotnet-test.yml b/.github/workflows/dotnet-test.yml index 58cea35a17..c152afb051 100644 --- a/.github/workflows/dotnet-test.yml +++ b/.github/workflows/dotnet-test.yml @@ -101,6 +101,11 @@ jobs: shell: pwsh run: | dotnet build-server shutdown + dotnet restore UniGetUI.Avalonia/UniGetUI.Avalonia.csproj ` + --runtime win-x64 ` + /p:Configuration=Release ` + /p:PublishAot=true ` + /p:SelfContained=true dotnet publish UniGetUI.Avalonia/UniGetUI.Avalonia.csproj ` --no-restore ` --configuration Release ` @@ -113,3 +118,6 @@ jobs: /p:TrimmerSingleWarn=false ` /p:UseSharedCompilation=false ` --verbosity minimal + if (-not (Test-Path "${{ runner.temp }}\unigetui-nativeaot-publish\UniGetUI.exe")) { + throw "NativeAOT publish did not produce UniGetUI.exe." + } diff --git a/AGENTS.md b/AGENTS.md index 70c54919ba..9f832c58b9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,6 +54,23 @@ dotnet publish src/UniGetUI.Avalonia/UniGetUI.Avalonia.csproj /p:Configuration=R - Self-contained, publish-trimmed (partial), Windows App SDK self-contained - Tests use **xUnit** (`[Fact]`, `Assert.*`) +## NativeAOT and Trim Safety + +Release packages are self-contained, fully trimmed NativeAOT binaries on every supported RID. Treat NativeAOT safety as a non-negotiable production requirement: changes must work without runtime-generated code or metadata that the trimmer cannot prove is required. + +- Consider every `IL2xxx` trim warning and `IL3xxx` AOT warning a defect. Do not blanket-suppress warnings or use a broad `RequiresUnreferencedCode`, `RequiresDynamicCode`, `DynamicDependency`, `DynamicallyAccessedMembers`, linker descriptor, or root assembly as a workaround. An exception must be narrowly scoped, explain the concrete runtime requirement, and have a publish-path test that proves it is safe. +- Use `JsonSerializerContext`/`JsonTypeInfo` source-generated metadata for every production JSON serialization or deserialization path, including generic helpers, HTTP payloads, settings, IPC, telemetry, and persisted state. Do not call reflection-based serializer overloads or retain a reflection fallback for unknown application types. Add the closed type to a context and test the persisted or transported path. +- Do not introduce production reflection-driven activation or discovery (`Assembly.Load`, `Type.GetType`, `Activator`, member lookup/invocation), runtime generic construction, expression compilation, runtime code generation, or dynamically loaded plugins. Prefer explicit registrations, direct constructors, generated metadata, and compiled XAML bindings. If a framework-owned dynamic boundary cannot be removed, isolate it and add published-NativeAOT runtime coverage. +- New shipping COM interop must use source-generated interfaces and wrappers (`GeneratedComInterface`, `StrategyBasedComWrappers`) or raw ABI calls; do not add `ComImport`, `CoClass`, RCW activation, or legacy marshal-to-object APIs to a NativeAOT path. Define the ABI precisely, balance COM reference ownership, and test both x64 and arm64 where applicable. +- Prefer `LibraryImport` for new P/Invokes. Existing `DllImport` declarations are acceptable only when their marshalling is supported by NativeAOT; keep platform guards, calling conventions, ownership, callback lifetimes, and x64/arm64 struct layouts explicit and covered. +- Audit all new or changed startup, XAML/view-loading, dependency-injection, serialization, COM/WinRT, P/Invoke, and package-manager code against the publish output, not only a Debug build. Run the relevant NativeAOT publish profile, for example: + + ```shell + dotnet publish src/UniGetUI.Avalonia/UniGetUI.Avalonia.csproj --configuration Release --runtime win-x64 --self-contained true /p:Platform=x64 /p:PublishProfile=Win-x64-NativeAot + ``` + +- When modifying a shared or portable path, retain the same safety guarantees for all checked-in NativeAOT publish profiles: `win-x64`, `win-arm64`, `linux-x64`, `linux-arm64`, `osx-x64`, and `osx-arm64`. Record unsupported runtime-matrix coverage as a gap, never as a pass. + ## Avalonia DevTools (Developer-Only) Use these rules when changing Avalonia diagnostics/devtools behavior: diff --git a/src/UniGetUI.Core.Settings.Tests/SettingsTest.cs b/src/UniGetUI.Core.Settings.Tests/SettingsTest.cs index 6ca45f8ebe..1b65c89e75 100644 --- a/src/UniGetUI.Core.Settings.Tests/SettingsTest.cs +++ b/src/UniGetUI.Core.Settings.Tests/SettingsTest.cs @@ -297,18 +297,7 @@ string[] ls3Array Assert.False(Settings.RemoveFromList(SettingName, ls2[0])); Assert.False(Settings.ListContains(SettingName, ls2[0])); - Settings.SetList(SettingName, ls3); - Assert.Equal(ls3.Count, Settings.GetList(SettingName)?.Count); - Assert.Equal( - ls3[1].sub.sub, - Settings.GetListItem(SettingName, 1)?.sub.sub - ); - Assert.True(Settings.RemoveFromList(SettingName, ls3[0])); - Assert.False(Settings.RemoveFromList(SettingName, ls3[0])); - Assert.Equal( - ls3[1].sub.sub, - Settings.GetListItem(SettingName, 0)?.sub.sub - ); + Assert.Throws(() => Settings.SetList(SettingName, ls3)); Settings.ClearList(SettingName); // Cleanup Assert.Empty( Settings.GetList(SettingName) @@ -383,157 +372,7 @@ string[] strArray Settings.SetDictionaryItem(SettingName, "key", 12); Assert.Equal(12, Settings.GetDictionaryItem(SettingName, "key")); - Settings.SetDictionary(SettingName, test); - Assert.Equal( - JsonSerializer.Serialize(test, Settings.SerializationOptions), - File.ReadAllText( - Path.Join( - CoreData.UniGetUIUserConfigurationDirectory, - $"{Settings.ResolveKey(SettingName)}.json" - ) - ) - ); - Assert.Equal( - test[keyArray[0]]?.sub.count, - Settings - .GetDictionary(SettingName) - ?[keyArray[0]]?.sub.count - ); - Assert.Equal( - test[keyArray[1]]?.sub.count, - Settings - .GetDictionaryItem(SettingName, keyArray[1]) - ?.sub.count - ); - Settings.SetDictionaryItem(SettingName, keyArray[0], test[keyArray[1]]); - Assert.Equal( - test[keyArray[1]]?.sub.count, - Settings - .GetDictionaryItem(SettingName, keyArray[0]) - ?.sub.count - ); - Assert.NotEqual( - test[keyArray[0]]?.sub.count, - Settings - .GetDictionaryItem(SettingName, keyArray[0]) - ?.sub.count - ); - Assert.Equal( - test[keyArray[1]]?.sub.count, - Settings - .GetDictionaryItem(SettingName, keyArray[1]) - ?.sub.count - ); - Assert.Equal( - test[keyArray[1]]?.count, - Settings - .SetDictionaryItem( - SettingName, - keyArray[0], - new SerializableTest( - "this is not test data", - -12000, - new SerializableTestSub("this sub is not test data", -13000) - ) - ) - ?.count - ); - Assert.Equal( - -12000, - Settings - .GetDictionaryItem(SettingName, keyArray[0]) - ?.count - ); - Assert.Equal( - "this is not test data", - Settings - .GetDictionaryItem(SettingName, keyArray[0]) - ?.title - ); - Assert.Equal( - -13000, - Settings - .GetDictionaryItem(SettingName, keyArray[0]) - ?.sub.count - ); - Settings.SetDictionaryItem( - SettingName, - "this is not a test data key", - test[keyArray[0]] - ); - Assert.Equal( - test[keyArray[0]]?.title, - Settings - .GetDictionaryItem( - SettingName, - "this is not a test data key" - ) - ?.title - ); - Assert.Equal( - test[keyArray[0]]?.sub.count, - Settings - .GetDictionaryItem( - SettingName, - "this is not a test data key" - ) - ?.sub.count - ); - Assert.True( - Settings.DictionaryContainsKey( - SettingName, - "this is not a test data key" - ) - ); - Assert.True( - Settings.DictionaryContainsValue( - SettingName, - test[keyArray[0]] - ) - ); - Assert.NotNull( - Settings.RemoveDictionaryKey( - SettingName, - "this is not a test data key" - ) - ); - Assert.Null( - Settings.RemoveDictionaryKey( - SettingName, - "this is not a test data key" - ) - ); - Assert.False( - Settings.DictionaryContainsKey( - SettingName, - "this is not a test data key" - ) - ); - Assert.False( - Settings.DictionaryContainsValue( - SettingName, - test[keyArray[0]] - ) - ); - Assert.True( - Settings.DictionaryContainsValue( - SettingName, - test[keyArray[2]] - ) - ); - - Assert.Equal( - JsonSerializer.Serialize( - Settings.GetDictionary(SettingName), - Settings.SerializationOptions - ), - File.ReadAllText( - Path.Join( - CoreData.UniGetUIUserConfigurationDirectory, - $"{Settings.ResolveKey(SettingName)}.json" - ) - ) - ); + Assert.Throws(() => Settings.SetDictionary(SettingName, test)); Settings.ClearDictionary(SettingName); // Cleanup Assert.Empty( diff --git a/src/UniGetUI.Core.Settings/SettingsEngine_Dictionaries.cs b/src/UniGetUI.Core.Settings/SettingsEngine_Dictionaries.cs index 19701e180f..d85e127f06 100644 --- a/src/UniGetUI.Core.Settings/SettingsEngine_Dictionaries.cs +++ b/src/UniGetUI.Core.Settings/SettingsEngine_Dictionaries.cs @@ -77,6 +77,10 @@ private static readonly ConcurrentDictionary< ); return value; } + catch (InvalidOperationException) + { + throw; + } catch (Exception ex) { Logger.Error($"Could not load dictionary name {setting}"); @@ -99,11 +103,6 @@ Dictionary value where KeyT : notnull { string setting = ResolveKey(settingsKey); - _dictionarySettings[settingsKey] = value.ToDictionary( - kvp => (object)kvp.Key, - kvp => (object?)kvp.Value - ); - var file = Path.Join(CoreData.UniGetUIUserConfigurationDirectory, $"{setting}.json"); try { @@ -111,6 +110,15 @@ Dictionary value File.WriteAllText(file, SettingsJson.SerializeDictionary(value)); else if (File.Exists(file)) File.Delete(file); + + _dictionarySettings[settingsKey] = value.ToDictionary( + kvp => (object)kvp.Key, + kvp => (object?)kvp.Value + ); + } + catch (InvalidOperationException) + { + throw; } catch (Exception e) { diff --git a/src/UniGetUI.Core.Settings/SettingsEngine_Lists.cs b/src/UniGetUI.Core.Settings/SettingsEngine_Lists.cs index 398c320700..02c753dc0e 100644 --- a/src/UniGetUI.Core.Settings/SettingsEngine_Lists.cs +++ b/src/UniGetUI.Core.Settings/SettingsEngine_Lists.cs @@ -62,6 +62,10 @@ public static partial class Settings listSettings[setting] = value.Cast().ToList(); return value; } + catch (InvalidOperationException) + { + throw; + } catch (Exception ex) { Logger.Error($"Could not load list {setting} from settings"); @@ -78,7 +82,6 @@ public static partial class Settings public static void SetList(string setting, List value) { - listSettings[setting] = value.Cast().ToList(); var file = Path.Join(CoreData.UniGetUIUserConfigurationDirectory, $"{setting}.json"); try { @@ -86,6 +89,12 @@ public static void SetList(string setting, List value) File.WriteAllText(file, SettingsJson.SerializeList(value)); else if (File.Exists(file)) File.Delete(file); + + listSettings[setting] = value.Cast().ToList(); + } + catch (InvalidOperationException) + { + throw; } catch (Exception e) { diff --git a/src/UniGetUI.Core.Settings/SettingsJson.cs b/src/UniGetUI.Core.Settings/SettingsJson.cs index 42094f2949..5f74f5ae22 100644 --- a/src/UniGetUI.Core.Settings/SettingsJson.cs +++ b/src/UniGetUI.Core.Settings/SettingsJson.cs @@ -1,4 +1,3 @@ -using System.Diagnostics.CodeAnalysis; using System.Text.Json; using System.Text.Json.Serialization; using System.Text.Json.Serialization.Metadata; @@ -19,38 +18,27 @@ public static string SerializeStringDictionary(Dictionary value) public static string SerializeList(List value) { - JsonTypeInfo>? typeInfo = GetGeneratedTypeInfo>(); - return typeInfo is not null - ? JsonSerializer.Serialize(value, typeInfo) - : SerializeListWithReflection(value); + return JsonSerializer.Serialize(value, GetRequiredTypeInfo>()); } public static List? DeserializeList(string json) { - JsonTypeInfo>? typeInfo = GetGeneratedTypeInfo>(); - return typeInfo is not null - ? JsonSerializer.Deserialize(json, typeInfo) - : DeserializeListWithReflection(json); + return JsonSerializer.Deserialize(json, GetRequiredTypeInfo>()); } public static string SerializeDictionary(Dictionary value) where KeyT : notnull { - JsonTypeInfo>? typeInfo = - GetGeneratedTypeInfo>(); - return typeInfo is not null - ? JsonSerializer.Serialize(value, typeInfo) - : SerializeDictionaryWithReflection(value); + return JsonSerializer.Serialize(value, GetRequiredTypeInfo>()); } public static Dictionary? DeserializeDictionary(string json) where KeyT : notnull { - JsonTypeInfo>? typeInfo = - GetGeneratedTypeInfo>(); - return typeInfo is not null - ? JsonSerializer.Deserialize(json, typeInfo) - : DeserializeDictionaryWithReflection(json); + return JsonSerializer.Deserialize( + json, + GetRequiredTypeInfo>() + ); } private static JsonTypeInfo? GetGeneratedTypeInfo() @@ -66,66 +54,6 @@ private static JsonTypeInfo GetRequiredTypeInfo() ); } - [UnconditionalSuppressMessage( - "Trimming", - "IL2026", - Justification = "Runtime settings use generated metadata for known app types; this fallback preserves generic settings tests and extension scenarios outside trimmed app paths.")] - [UnconditionalSuppressMessage( - "AotCompatibility", - "IL3050", - Justification = "This reflection fallback is only used when generated metadata is unavailable; NativeAOT app paths rely on source-generated metadata for the known settings types.")] - private static string SerializeListWithReflection(List value) - { - return JsonSerializer.Serialize(value, Settings.SerializationOptions); - } - - [UnconditionalSuppressMessage( - "Trimming", - "IL2026", - Justification = "Runtime settings use generated metadata for known app types; this fallback preserves generic settings tests and extension scenarios outside trimmed app paths.")] - [UnconditionalSuppressMessage( - "AotCompatibility", - "IL3050", - Justification = "This reflection fallback is only used when generated metadata is unavailable; NativeAOT app paths rely on source-generated metadata for the known settings types.")] - private static List? DeserializeListWithReflection(string json) - { - return JsonSerializer.Deserialize>(json, Settings.SerializationOptions); - } - - [UnconditionalSuppressMessage( - "Trimming", - "IL2026", - Justification = "Runtime settings use generated metadata for known app types; this fallback preserves generic settings tests and extension scenarios outside trimmed app paths.")] - [UnconditionalSuppressMessage( - "AotCompatibility", - "IL3050", - Justification = "This reflection fallback is only used when generated metadata is unavailable; NativeAOT app paths rely on source-generated metadata for the known settings types.")] - private static string SerializeDictionaryWithReflection( - Dictionary value - ) - where KeyT : notnull - { - return JsonSerializer.Serialize(value, Settings.SerializationOptions); - } - - [UnconditionalSuppressMessage( - "Trimming", - "IL2026", - Justification = "Runtime settings use generated metadata for known app types; this fallback preserves generic settings tests and extension scenarios outside trimmed app paths.")] - [UnconditionalSuppressMessage( - "AotCompatibility", - "IL3050", - Justification = "This reflection fallback is only used when generated metadata is unavailable; NativeAOT app paths rely on source-generated metadata for the known settings types.")] - private static Dictionary? DeserializeDictionaryWithReflection( - string json - ) - where KeyT : notnull - { - return JsonSerializer.Deserialize>( - json, - Settings.SerializationOptions - ); - } } [JsonSourceGenerationOptions(AllowTrailingCommas = true, WriteIndented = true)] @@ -137,5 +65,4 @@ string json [JsonSerializable(typeof(List))] [JsonSerializable(typeof(List))] [JsonSerializable(typeof(List))] -[JsonSerializable(typeof(List))] internal sealed partial class SettingsJsonContext : JsonSerializerContext; diff --git a/src/UniGetUI.Core.Tools.Tests/MetaTests.cs b/src/UniGetUI.Core.Tools.Tests/MetaTests.cs index 694169af0d..ff6bec7309 100644 --- a/src/UniGetUI.Core.Tools.Tests/MetaTests.cs +++ b/src/UniGetUI.Core.Tools.Tests/MetaTests.cs @@ -31,6 +31,7 @@ public void TestJsonSerializationOptions() || x.Contains("JsonSerializerContext") || x.Contains("JsonSourceGenerationOptions") || x.Contains("GetTypeInfo") + || x.Contains("GetRequiredTypeInfo") || x.Contains("typeInfo") ); Assert.True( diff --git a/src/WindowsPackageManager.Interop/ExternalLibraries.WindowsPackageManager.Interop.csproj b/src/WindowsPackageManager.Interop/ExternalLibraries.WindowsPackageManager.Interop.csproj index 8f2f83b591..dfbf549516 100644 --- a/src/WindowsPackageManager.Interop/ExternalLibraries.WindowsPackageManager.Interop.csproj +++ b/src/WindowsPackageManager.Interop/ExternalLibraries.WindowsPackageManager.Interop.csproj @@ -5,8 +5,6 @@ true WindowsPackageManager.Interop - - $(NoWarn);IL2050