diff --git a/GenHub/GenHub.Core/Constants/ManifestConstants.cs b/GenHub/GenHub.Core/Constants/ManifestConstants.cs index b2ce85571..bdfc551dc 100644 --- a/GenHub/GenHub.Core/Constants/ManifestConstants.cs +++ b/GenHub/GenHub.Core/Constants/ManifestConstants.cs @@ -15,6 +15,17 @@ public static class ManifestConstants /// public const string DefaultManifestVersion = "1"; + /// + /// Manifest format version that introduces artifact variants. + /// + /// + /// Bumped from so that a manifest using + /// variants is identifiable as such rather than presenting as a version 1 manifest + /// with an unexpected field. Ingestion rejects this version for now — see + /// . + /// + public const int VariantsManifestFormatVersion = 2; + /// /// Prefix for publisher content IDs. /// diff --git a/GenHub/GenHub.Core/Interfaces/Workspace/ISymlinkCapabilityProvider.cs b/GenHub/GenHub.Core/Interfaces/Workspace/ISymlinkCapabilityProvider.cs new file mode 100644 index 000000000..a8ef78e41 --- /dev/null +++ b/GenHub/GenHub.Core/Interfaces/Workspace/ISymlinkCapabilityProvider.cs @@ -0,0 +1,25 @@ +namespace GenHub.Core.Interfaces.Workspace; + +/// +/// Reports whether this process can create symbolic links. +/// +/// Previously the launcher asked "is this process an administrator", computed it only on +/// Windows, and left it false everywhere else. It then downgraded the +/// SymlinkOnly and HybridCopySymlink strategies to HardLink whenever +/// the answer was false, which made both strategies permanently unreachable on Linux and +/// macOS — where symlink(2) needs no privilege at all. Users could select a +/// strategy in Settings that silently never applied. +/// +/// +/// Naming the capability rather than the privilege makes the platform answer obvious: +/// Windows genuinely gates symlink creation behind SeCreateSymbolicLinkPrivilege +/// (or Developer Mode); Unix does not gate it at all. +/// +/// +public interface ISymlinkCapabilityProvider +{ + /// + /// Gets a value indicating whether this process can create symbolic links. + /// + bool CanCreateSymlinks { get; } +} diff --git a/GenHub/GenHub.Core/Models/Manifest/ArtifactVariant.cs b/GenHub/GenHub.Core/Models/Manifest/ArtifactVariant.cs new file mode 100644 index 000000000..7a0bec3fb --- /dev/null +++ b/GenHub/GenHub.Core/Models/Manifest/ArtifactVariant.cs @@ -0,0 +1,77 @@ +using System.Collections.Generic; +using System.Text.Json.Serialization; + +namespace GenHub.Core.Models.Manifest; + +/// +/// A platform-specific build within a single content release. +/// +/// One release of a game client can produce several builds — win-x64, linux-x64 and +/// osx-arm64 — that share a version and a name but not a file list or an entry point. +/// A single flat file list cannot describe that: the same manifest would advertise a +/// Windows executable to a macOS host, which installs cleanly and then cannot run. +/// +/// +/// Variants are optional. A manifest with no variants is a single unconstrained build +/// described by , which is what every manifest +/// written before this type existed looks like. +/// +/// +public class ArtifactVariant +{ + /// + /// Gets or sets the runtime identifiers this variant can run on, for example + /// osx-arm64 or win-x64. + /// + /// Architecture matters, not just the operating system: an x64 build is not + /// interchangeable with an arm64 one, and a macOS user on Apple Silicon offered an + /// osx-x64 build gets a launch that fails in the loader. + /// + /// + /// An empty list means the variant is platform-neutral, which is correct for map + /// packs, INI tweaks and .big content that contains no native code. + /// + /// + [JsonPropertyName("runtimeIdentifiers")] + public List RuntimeIdentifiers { get; set; } = []; + + /// + /// Gets or sets the relative path of the file to launch for this variant. + /// + /// Declared rather than inferred. Inferring it from file extensions is ambiguous + /// the moment a variant ships more than one runnable file, and the result then + /// depends on file enumeration order. + /// + /// + [JsonPropertyName("entryPoint")] + public string? EntryPoint { get; set; } + + /// + /// Gets or sets the files belonging to this variant. + /// + [JsonPropertyName("files")] + public List Files { get; set; } = []; + + /// + /// Determines whether this variant can run on the given runtime identifier. + /// + /// The host runtime identifier, for example osx-arm64. + /// true when the variant is platform-neutral or explicitly targets the runtime. + public bool SupportsRuntime(string runtimeIdentifier) + { + if (RuntimeIdentifiers.Count == 0) + { + return true; + } + + foreach (var candidate in RuntimeIdentifiers) + { + if (string.Equals(candidate, runtimeIdentifier, System.StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + + return false; + } +} diff --git a/GenHub/GenHub.Core/Models/Manifest/ContentManifest.cs b/GenHub/GenHub.Core/Models/Manifest/ContentManifest.cs index 1051eaff1..0624cc42f 100644 --- a/GenHub/GenHub.Core/Models/Manifest/ContentManifest.cs +++ b/GenHub/GenHub.Core/Models/Manifest/ContentManifest.cs @@ -1,6 +1,7 @@ +using System.Collections.Generic; +using System.Text.Json.Serialization; using GenHub.Core.Constants; using GenHub.Core.Models.Enums; -using System.Text.Json.Serialization; namespace GenHub.Core.Models.Manifest; @@ -10,6 +11,8 @@ namespace GenHub.Core.Models.Manifest; /// public class ContentManifest { + private List _variants = []; + /// Gets or sets the manifest format version. public string ManifestVersion { get; set; } = ManifestConstants.DefaultManifestVersion; @@ -61,9 +64,46 @@ public class ContentManifest /// Gets or sets the list of known addons for this game (manifest-driven, not hardcoded). public List KnownAddons { get; set; } = []; - /// Gets or sets all files included in this content package. + /// + /// Gets or sets all files included in this content package. + /// + /// This describes the single, unconstrained build. When is + /// non-empty this list is ignored in favour of the matching variant. Consumers + /// should resolve through ManifestVariantResolver rather than reading this + /// directly, so that multi-platform manifests behave correctly. + /// + /// public List Files { get; set; } = []; + /// + /// Gets or sets platform-specific builds of this content. + /// + /// Optional and empty by default, so every manifest written before variants existed + /// keeps working unchanged: an empty list means " is the only + /// build". Populate it when one release ships several platform builds that share a + /// version but differ in file list or entry point. + /// + /// + public List Variants + { + get => _variants; + set => _variants = value ?? []; + } + + /// + /// Gets or sets the relative path of the file to launch, for single-variant content. + /// + /// Declared rather than inferred from file extensions. Without it, resolution falls + /// back to guessing from the file list, which is ambiguous as soon as more than one + /// file qualifies and then depends on enumeration order. + /// + /// + /// When is populated, each variant carries its own entry + /// point and this is ignored. + /// + /// + public string? EntryPoint { get; set; } + /// Gets or sets the required directory structure. public List RequiredDirectories { get; set; } = []; diff --git a/GenHub/GenHub.Core/Models/Manifest/EntryPointResolution.cs b/GenHub/GenHub.Core/Models/Manifest/EntryPointResolution.cs new file mode 100644 index 000000000..a373e5fd9 --- /dev/null +++ b/GenHub/GenHub.Core/Models/Manifest/EntryPointResolution.cs @@ -0,0 +1,66 @@ +using System.Collections.Generic; +using System.Linq; + +namespace GenHub.Core.Models.Manifest; + +/// +/// The outcome of resolving which file a manifest launches. +/// +/// Failure carries the candidates that were considered. Resolution failing silently, or +/// failing with only "no executable found", leaves whoever hits it guessing at what the +/// manifest actually contained. +/// +/// +public sealed class EntryPointResolution +{ + private EntryPointResolution(string? relativePath, string reason, IReadOnlyList candidates) + { + RelativePath = relativePath; + Reason = reason; + Candidates = candidates; + } + + /// Gets a value indicating whether an entry point was determined. + public bool Success => RelativePath is not null; + + /// Gets the resolved relative path, or null when resolution failed. + public string? RelativePath { get; } + + /// + /// Gets a human-readable explanation: on success, how the entry point was chosen; on + /// failure, why it could not be. + /// + public string Reason { get; } + + /// Gets the file paths considered, for diagnosing a failure. + public IReadOnlyList Candidates { get; } + + /// + /// Creates a successful resolution. + /// + /// The resolved entry point. + /// How it was chosen. + /// A successful resolution. + public static EntryPointResolution Resolved(string relativePath, string reason) => + new(relativePath, reason, []); + + /// + /// Creates a failed resolution. + /// + /// Why resolution failed. + /// The files that were considered. + /// A failed resolution. + public static EntryPointResolution Failed(string reason, IEnumerable candidates) => + new(null, reason, candidates.Select(f => f.RelativePath).ToList()); + + /// + /// Builds a log-ready description including the candidates considered. + /// + /// A diagnostic string. + public override string ToString() => + Success + ? $"{RelativePath} ({Reason})" + : Candidates.Count == 0 + ? Reason + : $"{Reason} Candidates: {string.Join(", ", Candidates)}"; +} diff --git a/GenHub/GenHub.Core/Models/Manifest/ManifestIngestionGate.cs b/GenHub/GenHub.Core/Models/Manifest/ManifestIngestionGate.cs new file mode 100644 index 000000000..bf6c8c9e9 --- /dev/null +++ b/GenHub/GenHub.Core/Models/Manifest/ManifestIngestionGate.cs @@ -0,0 +1,73 @@ +using System.Globalization; +using GenHub.Core.Constants; + +namespace GenHub.Core.Models.Manifest; + +/// +/// Fail-closed gate for manifests that declare artifact variants. +/// +/// +/// Variants are expressed by and resolved by +/// , but the consumers that act on a manifest — +/// deliverers, validators, CAS reference counting and garbage collection — still read +/// directly. A manifest declaring variants would be +/// accepted and then mishandled by every one of them: Files is empty for a +/// variant manifest, so content would appear to install successfully while delivering +/// nothing, and reference counting would record the wrong set of blobs. +/// +/// Rejecting at ingestion is therefore deliberate and temporary. It is the only +/// protection until the consumers are migrated to the resolved-variant model, and it +/// should be removed as part of that migration rather than relaxed piecemeal. +/// +/// +public static class ManifestIngestionGate +{ + /// + /// Determines whether a manifest may be ingested. + /// + /// The manifest to check. + /// + /// When the manifest is rejected, a message naming the manifest and the reason; + /// otherwise null. + /// + /// true when the manifest may be ingested; otherwise false. + public static bool TryAccept(ContentManifest? manifest, out string? rejectionReason) + { + rejectionReason = null; + + if (manifest is null) + { + return true; + } + + // Checked independently rather than as one condition. Declared version alone is + // not trustworthy — a manifest can carry variants while still claiming version 1 — + // and variants alone are not the only signal, since a format-2 manifest may use + // other version-2 features this pipeline equally cannot handle. + var declaresVariants = manifest.Variants.Count > 0; + var declaresVariantFormat = + int.TryParse( + manifest.ManifestVersion, + NumberStyles.None, + CultureInfo.InvariantCulture, + out var declaredFormat) + && declaredFormat >= ManifestConstants.VariantsManifestFormatVersion; + + if (!declaresVariants && !declaresVariantFormat) + { + return true; + } + + var cause = declaresVariants + ? $"declares {manifest.Variants.Count} artifact variant(s)" + : $"declares manifest format version {manifest.ManifestVersion}"; + + rejectionReason = + $"Manifest '{manifest.Id.Value}' {cause}, which requires manifest format version " + + $"{ManifestConstants.VariantsManifestFormatVersion} and is not yet accepted. " + + "Variant manifests are rejected until the content pipeline is migrated to the " + + "resolved-variant model; publish a manifest without variants in the meantime."; + + return false; + } +} diff --git a/GenHub/GenHub.Core/Models/Manifest/ManifestVariantResolver.cs b/GenHub/GenHub.Core/Models/Manifest/ManifestVariantResolver.cs new file mode 100644 index 000000000..3ec5a07f1 --- /dev/null +++ b/GenHub/GenHub.Core/Models/Manifest/ManifestVariantResolver.cs @@ -0,0 +1,174 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.InteropServices; +using GenHub.Core.Utilities; + +namespace GenHub.Core.Models.Manifest; + +/// +/// Resolves which files a manifest contributes on the current host, and which one to +/// launch. +/// +/// Entry-point resolution used to be Files.FirstOrDefault(f => f.IsExecutable), +/// which is order-dependent whenever more than one file qualifies, and silently picked +/// whichever the enumeration happened to yield first. +/// +/// +public static class ManifestVariantResolver +{ + /// + /// Gets the runtime identifier of the current host, for example osx-arm64. + /// + public static string CurrentRuntimeIdentifier => RuntimeInformation.RuntimeIdentifier; + + /// + /// Selects the files a manifest contributes on the given runtime. + /// + /// The manifest to resolve. + /// Host runtime identifier; defaults to the current host. + /// + /// The matching variant's files, or the flat list + /// when the manifest declares no variants. Empty when variants are declared but none + /// matches, which means the content genuinely cannot run here. + /// + public static IReadOnlyList ResolveFiles( + ContentManifest manifest, + string? runtimeIdentifier = null) + { + ArgumentNullException.ThrowIfNull(manifest); + + var variant = ResolveVariant(manifest, runtimeIdentifier); + + if (variant is not null) + { + return variant.Files; + } + + return manifest.Variants.Count == 0 ? manifest.Files : []; + } + + /// + /// Selects the variant that applies on the given runtime. + /// + /// The manifest to resolve. + /// Host runtime identifier; defaults to the current host. + /// The matching variant, or null when the manifest declares none or none matches. + public static ArtifactVariant? ResolveVariant( + ContentManifest manifest, + string? runtimeIdentifier = null) + { + ArgumentNullException.ThrowIfNull(manifest); + + if (manifest.Variants.Count == 0) + { + return null; + } + + var rid = runtimeIdentifier ?? CurrentRuntimeIdentifier; + + // Prefer an explicit match over a platform-neutral one, so a manifest carrying + // both a native build and a neutral asset bundle resolves to the native build. + return manifest.Variants.FirstOrDefault(v => v.RuntimeIdentifiers.Count > 0 && v.SupportsRuntime(rid)) + ?? manifest.Variants.FirstOrDefault(v => v.RuntimeIdentifiers.Count == 0); + } + + /// + /// Determines whether a manifest has anything runnable or installable on the runtime. + /// + /// Used to keep content that cannot run on this host out of the catalogue, rather + /// than letting a user install it successfully and then find it does nothing. + /// + /// + /// The manifest to test. + /// Host runtime identifier; defaults to the current host. + /// true when the manifest applies to the runtime. + public static bool SupportsRuntime(ContentManifest manifest, string? runtimeIdentifier = null) + { + ArgumentNullException.ThrowIfNull(manifest); + + return manifest.Variants.Count == 0 + || ResolveVariant(manifest, runtimeIdentifier) is not null; + } + + /// + /// Resolves the relative path of the file to launch. + /// + /// The chain is deliberately explicit, and refuses to guess at the end: + /// + /// + /// the declared entry point, on the variant or the manifest; + /// the only file marked as needing the execute bit, if there is exactly one; + /// the only legacy launch candidate by extension, if there is exactly one; + /// otherwise fail, and report every candidate considered. + /// + /// + /// The manifest to resolve. + /// Host runtime identifier; defaults to the current host. + /// A result carrying either the entry point or a diagnosable failure. + public static EntryPointResolution ResolveEntryPoint( + ContentManifest manifest, + string? runtimeIdentifier = null) + { + ArgumentNullException.ThrowIfNull(manifest); + + var variant = ResolveVariant(manifest, runtimeIdentifier); + var files = ResolveFiles(manifest, runtimeIdentifier); + var declared = manifest.Variants.Count == 0 + ? manifest.EntryPoint + : variant?.EntryPoint; + + if (!string.IsNullOrWhiteSpace(declared)) + { + // A declared entry point that is not in the file list is a manifest defect. + // Failing here is far more diagnosable than failing at Process.Start. + var matchedFile = files.FirstOrDefault(f => PathsMatch(f.RelativePath, declared)); + + return matchedFile is not null + ? EntryPointResolution.Resolved(matchedFile.RelativePath, "declared entry point") + : EntryPointResolution.Failed( + $"Manifest '{manifest.Id}' declares entry point '{declared}', which is not among its " + + $"{files.Count} file(s).", + files); + } + + var executable = files + .Where(f => + f.IsExecutable + && ExecutableFileClassifier.IsLegacyLaunchCandidate(f.RelativePath)) + .ToList(); + if (executable.Count == 1) + { + return EntryPointResolution.Resolved(executable[0].RelativePath, "only file requiring execute permission"); + } + + if (executable.Count == 0) + { + var legacy = files + .Where(f => ExecutableFileClassifier.IsLegacyLaunchCandidate(f.RelativePath)) + .ToList(); + + if (legacy.Count == 1) + { + return EntryPointResolution.Resolved(legacy[0].RelativePath, "only launch candidate by extension"); + } + + return EntryPointResolution.Failed( + legacy.Count == 0 + ? $"Manifest '{manifest.Id}' contains no launchable file." + : $"Manifest '{manifest.Id}' contains {legacy.Count} possible launch targets and declares no entry point.", + files); + } + + return EntryPointResolution.Failed( + $"Manifest '{manifest.Id}' marks {executable.Count} files as requiring execute permission and " + + "declares no entry point, so the launch target is ambiguous.", + files); + } + + private static bool PathsMatch(string left, string right) => + string.Equals( + left.Replace('\\', '/').TrimStart('/'), + right.Replace('\\', '/').TrimStart('/'), + StringComparison.OrdinalIgnoreCase); +} diff --git a/GenHub/GenHub.Core/Utilities/ExecutableFileClassifier.cs b/GenHub/GenHub.Core/Utilities/ExecutableFileClassifier.cs new file mode 100644 index 000000000..80ee8c879 --- /dev/null +++ b/GenHub/GenHub.Core/Utilities/ExecutableFileClassifier.cs @@ -0,0 +1,121 @@ +using System; +using System.IO; + +namespace GenHub.Core.Utilities; + +/// +/// Single source of truth for what "executable" means when building a manifest. +/// +/// Five call sites previously answered this independently and disagreed. Extensionless +/// files were classified executable by one and not by the other four, which matters +/// because a native Mach-O or ELF game binary has no extension: +/// +/// +/// ContentManifestBuilder: .exe, .dll, .so, extensionless +/// GitHubInferenceHelper: .exe, .dll, .sh, .bat, .so +/// ManifestGenerationService: .exe, .dat +/// CommunityOutpostDeliverer: .exe +/// FileTreeItem: .exe +/// +/// +/// It also conflated three separate questions: is this the launch target, is this +/// executable code, and does this file need the Unix execute bit. They have different +/// answers. A .dylib is executable code, is never a launch target, and is mapped +/// by dyld with read permission only — giving it +x is meaningless. A .dat is +/// data that the Steam layout happens to launch through, and is not code at all. +/// +/// +/// So this class answers exactly two questions, and the launch target is answered +/// elsewhere by an explicit declaration rather than inferred from a filename. +/// +/// +public static class ExecutableFileClassifier +{ + /// + /// Extensions for loadable code that is never itself launched and never needs the + /// execute bit. Dynamic libraries are mapped by the loader, which requires read + /// access only. + /// + private static readonly string[] LibraryExtensions = [".dll", ".so", ".dylib"]; + + /// + /// Extensions that are directly runnable and therefore need the execute bit on Unix. + /// + private static readonly string[] RunnableExtensions = [".exe", ".sh", ".command"]; + + /// + /// Determines whether a file needs the Unix execute bit to be runnable. + /// + /// This is what ManifestFile.IsExecutable means. It is a permission fact, not + /// a statement about which file the profile launches. + /// + /// + /// Extensionless files count: that is the shape of a native Mach-O or ELF binary, + /// and it is the case the previous inconsistency got wrong. + /// + /// + /// A file name or relative path. Not required to exist on disk. + /// true when the file should be marked executable. + public static bool RequiresExecutePermission(string path) + { + if (string.IsNullOrWhiteSpace(path)) + { + return false; + } + + var extension = Path.GetExtension(path); + + // Extensionless: a native binary. Directories and dotfiles are excluded by the + // caller, which only ever passes real file entries. + if (string.IsNullOrEmpty(extension)) + { + return true; + } + + if (MatchesAny(extension, LibraryExtensions)) + { + return false; + } + + return MatchesAny(extension, RunnableExtensions); + } + + /// + /// Determines whether a file could be the launch target when a manifest declares no + /// explicit entry point. + /// + /// This exists only to keep manifests written before entry points were declarable + /// working. New content should declare its entry point rather than rely on this. + /// + /// + /// A file name or relative path. + /// true when the file is a plausible legacy launch target. + public static bool IsLegacyLaunchCandidate(string path) + { + if (string.IsNullOrWhiteSpace(path)) + { + return false; + } + + var extension = Path.GetExtension(path); + + // Extensionless native binaries and Windows executables only. Notably not .dat: + // the Steam layout launches game.dat through a proxy, but that is a launch + // *strategy* chosen by the Steam integration, not a property of the file. + return string.IsNullOrEmpty(extension) + || extension.Equals(".exe", StringComparison.OrdinalIgnoreCase); + } + + private static bool MatchesAny(string extension, string[] candidates) + { + foreach (var candidate in candidates) + { + if (extension.Equals(candidate, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + + return false; + } +} diff --git a/GenHub/GenHub.Linux/Infrastructure/DependencyInjection/LinuxServicesModule.cs b/GenHub/GenHub.Linux/Infrastructure/DependencyInjection/LinuxServicesModule.cs index e898ef53d..15fb4f16f 100644 --- a/GenHub/GenHub.Linux/Infrastructure/DependencyInjection/LinuxServicesModule.cs +++ b/GenHub/GenHub.Linux/Infrastructure/DependencyInjection/LinuxServicesModule.cs @@ -1,10 +1,16 @@ using System; using System.Runtime.Versioning; using GenHub.Core.Interfaces.GameInstallations; +using GenHub.Core.Interfaces.GameSettings; using GenHub.Core.Interfaces.Shortcuts; +using GenHub.Core.Interfaces.Storage; +using GenHub.Core.Interfaces.Workspace; +using GenHub.Features.GameSettings; +using GenHub.Features.Workspace; using GenHub.Linux.Features.Shortcuts; using GenHub.Linux.GameInstallations; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; namespace GenHub.Linux.Infrastructure.DependencyInjection; @@ -22,8 +28,21 @@ public static class LinuxServicesModule public static IServiceCollection AddLinuxServices(this IServiceCollection services) { services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); + // Real hard links via link(2). Without this the base implementation throws, which + // is deliberate: silently copying made a missing registration invisible while + // every workspace consumed a full copy of the game. + services.AddScoped(serviceProvider => + { + var baseService = serviceProvider.GetRequiredService(); + var casService = serviceProvider.GetRequiredService(); + var logger = serviceProvider.GetRequiredService>(); + return new UnixFileOperationsService(baseService, casService, logger); + }); + return services; } -} \ No newline at end of file +} diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Content/GitHubInferenceHelperTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Content/GitHubInferenceHelperTests.cs index f2b9369d1..254c55dc8 100644 --- a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Content/GitHubInferenceHelperTests.cs +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Content/GitHubInferenceHelperTests.cs @@ -84,12 +84,22 @@ public void InferTagsFromRelease_ReturnsExpectedTags() /// Expected boolean result. [Theory] [InlineData("program.exe", true)] - [InlineData("library.dll", true)] [InlineData("script.sh", true)] + + // A native game binary has no extension. This previously returned false here while + // returning true in ContentManifestBuilder, so the same file was classified + // differently depending on which factory built the manifest. + [InlineData("generalszh", true)] + + // Changed deliberately: a dynamic library is loadable code, not a runnable file. + // dyld and ld.so map libraries with read access, so the execute bit is meaningless, + // and under a hard-link workspace setting it would mutate a shared CAS blob. + [InlineData("library.dll", false)] + [InlineData("libSDL3.dylib", false)] [InlineData("readme.txt", false)] public void IsExecutableFile_ReturnsExpectedResult(string fileName, bool expected) { var result = GitHubInferenceHelper.IsExecutableFile(fileName); Assert.Equal(expected, result); } -} \ No newline at end of file +} diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameSettings/GamePathProviderTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameSettings/GamePathProviderTests.cs new file mode 100644 index 000000000..6900d0953 --- /dev/null +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameSettings/GamePathProviderTests.cs @@ -0,0 +1,105 @@ +using System; +using System.IO; +using GenHub.Core.Models.Enums; +using GenHub.Features.GameSettings; +using Xunit; + +namespace GenHub.Tests.Core.Features.GameSettings; + +/// +/// Pins the Options.ini directory each platform provider produces. +/// +/// These paths are dictated by the game engine, not chosen by GenHub. They mirror +/// GlobalData::BuildUserDataPathFromRegistry in the GeneralsGameCode tree. If +/// GenHub writes Options.ini anywhere else, the engine simply never reads it: the +/// launch still succeeds and every profile setting is silently discarded, with no +/// error anywhere. That failure is invisible in manual testing, so it is pinned here. +/// +/// +public class GamePathProviderTests +{ + /// + /// macOS resolves under Application Support with no vendor subdirectory. Notably + /// this is NOT the SDL_GetPrefPath convention of + /// ~/Library/Application Support/<org>/<app>/, which an + /// SDL3-based port would otherwise be expected to use. + /// + /// The game being resolved. + /// The directory name the engine expects. + [Theory] + [InlineData(GameType.ZeroHour, "Command and Conquer Generals Zero Hour Data")] + [InlineData(GameType.Generals, "Command and Conquer Generals Data")] + public void MacOSProvider_ResolvesUnderApplicationSupport(GameType gameType, string expectedLeaf) + { + var home = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); + var expected = Path.Combine(home, "Library", "Application Support", expectedLeaf); + + var actual = new MacOSGamePathProvider().GetOptionsDirectory(gameType); + + Assert.Equal(expected, actual); + } + + /// + /// Linux honours XDG_DATA_HOME when it is set. + /// + [Fact] + public void LinuxProvider_HonoursXdgDataHome() + { + var original = Environment.GetEnvironmentVariable("XDG_DATA_HOME"); + try + { + var custom = Path.Combine(Path.GetTempPath(), "genhub-xdg-probe"); + Environment.SetEnvironmentVariable("XDG_DATA_HOME", custom); + + var actual = new LinuxGamePathProvider().GetOptionsDirectory(GameType.ZeroHour); + + Assert.Equal( + Path.Combine(custom, "Command and Conquer Generals Zero Hour Data"), + actual); + } + finally + { + Environment.SetEnvironmentVariable("XDG_DATA_HOME", original); + } + } + + /// + /// With XDG_DATA_HOME unset, Linux falls back to ~/.local/share, matching the + /// engine rather than defaulting to the home directory root. + /// + [Fact] + public void LinuxProvider_FallsBackToLocalShare() + { + var original = Environment.GetEnvironmentVariable("XDG_DATA_HOME"); + try + { + Environment.SetEnvironmentVariable("XDG_DATA_HOME", null); + var home = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); + + var actual = new LinuxGamePathProvider().GetOptionsDirectory(GameType.ZeroHour); + + Assert.Equal( + Path.Combine(home, ".local", "share", "Command and Conquer Generals Zero Hour Data"), + actual); + } + finally + { + Environment.SetEnvironmentVariable("XDG_DATA_HOME", original); + } + } + + /// + /// The Unix providers must not resolve to the home directory root. That is what + /// SpecialFolder.MyDocuments returns on Unix, and it is what every platform + /// silently used before IGamePathProvider was registered anywhere. + /// + [Fact] + public void UnixProviders_DoNotResolveDirectlyUnderHome() + { + var home = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); + var badPath = Path.Combine(home, "Command and Conquer Generals Zero Hour Data"); + + Assert.NotEqual(badPath, new MacOSGamePathProvider().GetOptionsDirectory(GameType.ZeroHour)); + Assert.NotEqual(badPath, new LinuxGamePathProvider().GetOptionsDirectory(GameType.ZeroHour)); + } +} diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Manifest/ContentManifestPoolTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Manifest/ContentManifestPoolTests.cs index 468793c61..c6045f1e8 100644 --- a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Manifest/ContentManifestPoolTests.cs +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Manifest/ContentManifestPoolTests.cs @@ -137,6 +137,31 @@ public async Task GetManifestAsync_WhenExists_ShouldReturnManifest() Assert.Equal(manifest.Id, result.Data.Id); } + /// + /// An explicit JSON null must not replace the manifest's non-null variants + /// collection and crash the ingestion gate. + /// + /// A task representing the asynchronous operation. + [Fact] + public async Task GetManifestAsync_WithNullVariants_PreservesEmptyCollection() + { + var manifestId = ManifestId.Create("1.0.genhub.mod.nullvariants"); + var manifestPath = Path.Combine(_tempDirectory, "null-variants.json"); + await File.WriteAllTextAsync( + manifestPath, + """{"Id":"1.0.genhub.mod.nullvariants","Variants":null}"""); + + _storageServiceMock.Setup(service => service.GetManifestStoragePath(manifestId)) + .Returns(manifestPath); + + var result = await _manifestPool.GetManifestAsync(manifestId); + + Assert.True(result.Success); + Assert.NotNull(result.Data); + Assert.Empty(result.Data.Variants); + Assert.True(ManifestIngestionGate.TryAccept(result.Data, out _)); + } + /// /// Should return null when manifest does not exist. /// @@ -470,4 +495,79 @@ private void SetupManifestsInStorage(List manifests) _storageServiceMock.Setup(x => x.GetContentStorageRoot()) .Returns(_tempDirectory); } + /// + /// A variant manifest must be rejected before any content is written. + /// + /// + /// The pool is the chokepoint every deliverer, resolver and detector reaches, so this + /// is where the gate has to hold. Returning a failure is not sufficient on its own: + /// what matters is that nothing was stored and no CAS references were tracked, because + /// mis-tracked references are what corrupts reference counting and garbage collection. + /// + /// A task representing the asynchronous test operation. + [Fact] + public async Task AddManifestAsync_WithVariants_RejectsBeforeStoringContent() + { + var manifest = CreateTestManifest(); + manifest.Variants.Add(new ArtifactVariant()); + + var result = await _manifestPool.AddManifestAsync(manifest); + + Assert.False(result.Success); + Assert.Contains("variant", result.FirstError, StringComparison.OrdinalIgnoreCase); + + _storageServiceMock.Verify( + x => x.IsContentStoredAsync(It.IsAny(), It.IsAny()), + Times.Never); + _referenceTrackerMock.Verify( + x => x.TrackManifestReferencesAsync(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + } + + /// + /// The source-directory overload must reject a variant manifest without storing content. + /// + /// A task representing the asynchronous test operation. + [Fact] + public async Task AddManifestAsync_WithSourceDirectory_WithVariants_RejectsBeforeStoringContent() + { + var manifest = CreateTestManifest(); + manifest.Variants.Add(new ArtifactVariant()); + + var result = await _manifestPool.AddManifestAsync(manifest, _tempDirectory); + + Assert.False(result.Success); + Assert.Contains("variant", result.FirstError, StringComparison.OrdinalIgnoreCase); + + _storageServiceMock.Verify( + x => x.StoreContentAsync( + It.IsAny(), + It.IsAny(), + It.IsAny>(), + It.IsAny()), + Times.Never); + _referenceTrackerMock.Verify( + x => x.TrackManifestReferencesAsync(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + } + + /// + /// A manifest without variants must not be rejected by the gate; every manifest + /// published today is this shape. + /// + /// A task representing the asynchronous test operation. + [Fact] + public async Task AddManifestAsync_WithoutVariants_IsNotRejectedByTheGate() + { + var manifest = CreateTestManifest(); + _storageServiceMock.Setup(x => x.IsContentStoredAsync(manifest.Id, It.IsAny())) + .ReturnsAsync(OperationResult.CreateSuccess(true)); + _storageServiceMock.Setup(x => x.GetManifestStoragePath(manifest.Id)) + .Returns(Path.Combine(_tempDirectory, $"{manifest.Id}.manifest.json")); + + var result = await _manifestPool.AddManifestAsync(manifest); + + Assert.True(result.Success, $"Expected success but got: {result.FirstError}"); + } + } diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Manifest/ManifestDiscoveryServiceTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Manifest/ManifestDiscoveryServiceTests.cs index 55ff00df2..61b64e40a 100644 --- a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Manifest/ManifestDiscoveryServiceTests.cs +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Manifest/ManifestDiscoveryServiceTests.cs @@ -1,9 +1,11 @@ +using GenHub.Core.Interfaces.Common; using GenHub.Core.Interfaces.Manifest; using GenHub.Core.Models.Enums; using GenHub.Core.Models.Manifest; using GenHub.Features.Manifest; using Microsoft.Extensions.Logging; using Moq; +using System.IO; using System.Text.Json; using ContentType = GenHub.Core.Models.Enums.ContentType; @@ -34,6 +36,11 @@ public class ManifestDiscoveryServiceTests : IDisposable /// private readonly string _tempDirectory; + /// + /// Mock configuration provider supplying the application data path. + /// + private readonly Mock _configProviderMock; + /// /// Initializes a new instance of the class. /// @@ -41,8 +48,13 @@ public ManifestDiscoveryServiceTests() { _loggerMock = new Mock>(); _cacheMock = new Mock(); - _discoveryService = new ManifestDiscoveryService(_loggerMock.Object, _cacheMock.Object); _tempDirectory = Directory.CreateTempSubdirectory("GenHub.ManifestDiscoveryTests.").FullName; + _configProviderMock = new Mock(); + _configProviderMock.Setup(x => x.GetApplicationDataPath()).Returns(_tempDirectory); + _discoveryService = new ManifestDiscoveryService( + _loggerMock.Object, + _cacheMock.Object, + _configProviderMock.Object); } /// @@ -197,6 +209,7 @@ IEnumerable EnumerateDirectories(string directory) var discoveryService = new ManifestDiscoveryService( _loggerMock.Object, _cacheMock.Object, + _configProviderMock.Object, EnumerateFiles, EnumerateDirectories); diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Manifest/ManifestProviderTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Manifest/ManifestProviderTests.cs index 96e784939..09bc2a68a 100644 --- a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Manifest/ManifestProviderTests.cs +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Manifest/ManifestProviderTests.cs @@ -85,6 +85,29 @@ public async Task GetManifestAsync_WithGameClient_ReturnsFromCache_WhenAvailable _poolMock.Verify(x => x.GetManifestAsync(ManifestId.Create("1.0.genhub.mod.version"), default), Times.Once); } + /// + /// Cached variant manifests must pass the same fail-closed ingestion gate as newly + /// discovered manifests. + /// + /// A task representing the asynchronous operation. + [Fact] + public async Task GetManifestAsync_WithCachedVariantManifest_ThrowsValidationException() + { + var gameClient = new GameClient { Id = "1.0.genhub.mod.variant" }; + var manifest = new ContentManifest + { + Id = gameClient.Id, + Variants = [new ArtifactVariant()], + }; + + _poolMock + .Setup(pool => pool.GetManifestAsync(manifest.Id, It.IsAny())) + .ReturnsAsync(OperationResult.CreateSuccess(manifest)); + + await Assert.ThrowsAsync( + () => _manifestProvider.GetManifestAsync(gameClient)); + } + /// /// Tests that GetManifestAsync builds correct manifest ID for game installation. /// @@ -119,6 +142,33 @@ public async Task GetManifestAsync_WithGameInstallation_BuildsCorrectManifestId( _poolMock.Verify(x => x.GetManifestAsync(ManifestId.Create("1.108.eaapp.gameinstallation.generals"), default), Times.Once); } + /// + /// The installation overload must not bypass the fail-closed variant gate. + /// + /// A task representing the asynchronous operation. + [Fact] + public async Task GetManifestAsync_WithInstallationCachedVariantManifest_ThrowsValidationException() + { + var installation = new GameInstallation( + installationPath: @"C:\TestPath", + installationType: GameInstallationType.EaApp, + logger: null); + installation.SetPaths(@"C:\TestPath\Command and Conquer Generals", null); + + var manifest = new ContentManifest + { + Id = ManifestId.Create("1.108.eaapp.gameinstallation.generals"), + Variants = [new ArtifactVariant()], + }; + + _poolMock + .Setup(pool => pool.GetManifestAsync(manifest.Id, It.IsAny())) + .ReturnsAsync(OperationResult.CreateSuccess(manifest)); + + await Assert.ThrowsAsync( + () => _manifestProvider.GetManifestAsync(installation)); + } + /// /// Tests that GetManifestAsync uses Zero Hour ID for Zero Hour installations. /// @@ -243,4 +293,4 @@ public void ValidateManifestSecurity_ThrowsSecurityException_ForPathTraversal() var ex = Assert.Throws(() => method.Invoke(null, [manifest])); Assert.IsType(ex.InnerException); } -} \ No newline at end of file +} diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/ExecutablePermissionIsolationTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/ExecutablePermissionIsolationTests.cs new file mode 100644 index 000000000..5d85c762b --- /dev/null +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/ExecutablePermissionIsolationTests.cs @@ -0,0 +1,178 @@ +using System; +using System.IO; +using System.Threading.Tasks; +using GenHub.Core.Interfaces.Common; +using GenHub.Core.Interfaces.Storage; +using GenHub.Core.Models.Manifest; +using GenHub.Features.Workspace; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace GenHub.Tests.Core.Features.Workspace; + +/// +/// Demonstrates why marking a workspace file executable must not be done in place when +/// the workspace is built from hard links. +/// +/// The content store keys objects purely on content hash. Unix file mode lives in the +/// inode, which a hard link shares with its target, so the store cannot represent two +/// files with identical bytes and different modes. Setting the execute bit on a linked +/// workspace file therefore changes the stored blob for every profile referencing that +/// hash. +/// +/// +public class ExecutablePermissionIsolationTests : IDisposable +{ + private readonly string _tempDir = Path.Combine( + Path.GetTempPath(), + $"genhub-execisolation-{Guid.NewGuid():N}"); + + private readonly UnixFileOperationsService _service; + private readonly WorkspaceStrategyBaseTests.TestWorkspaceStrategy _strategy; + + /// + /// Initializes a new instance of the class. + /// + public ExecutablePermissionIsolationTests() + { + Directory.CreateDirectory(_tempDir); + + var baseService = new FileOperationsService( + NullLogger.Instance, + new Mock().Object, + new Mock().Object); + + _service = new UnixFileOperationsService( + baseService, + new Mock().Object, + NullLogger.Instance); + _strategy = new WorkspaceStrategyBaseTests.TestWorkspaceStrategy(_service); + } + + /// + /// Establishes the hazard: chmod through a hard link changes the target too. + /// + /// If this ever stops being true, the workspace copy this behaviour forces could be + /// dropped. It is asserted rather than assumed, because the whole design of + /// EnsureExecutableAsync rests on it. + /// + /// + /// A task representing the asynchronous test. + [Fact] + public async Task ChmodThroughHardLink_AlsoChangesTheTarget() + { + if (OperatingSystem.IsWindows()) + { + return; + } + + var casBlob = Path.Combine(_tempDir, "cas-blob"); + var workspaceFile = Path.Combine(_tempDir, "workspace-file"); + await File.WriteAllTextAsync(casBlob, "engine binary"); + File.SetUnixFileMode(casBlob, UnixFileMode.UserRead | UnixFileMode.UserWrite); + + await _service.CreateHardLinkAsync(workspaceFile, casBlob); + + File.SetUnixFileMode( + workspaceFile, + UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + + const string message = + "Expected chmod through a hard link to affect the shared inode. If this now " + + "fails, the copy-before-chmod behaviour in WorkspaceStrategyBase can be revisited."; + + Assert.True( + File.GetUnixFileMode(casBlob).HasFlag(UnixFileMode.UserExecute), + message); + } + + /// + /// The mitigation: copying first gives the workspace its own inode, so the execute + /// bit stops at the workspace and the stored blob keeps the mode it was ingested with. + /// + /// A task representing the asynchronous test. + [Fact] + public async Task CopyBeforeChmod_LeavesTheStoredBlobUntouched() + { + if (OperatingSystem.IsWindows()) + { + return; + } + + var casBlob = Path.Combine(_tempDir, "cas-blob"); + var workspaceFile = Path.Combine(_tempDir, "workspace-file"); + await File.WriteAllTextAsync(casBlob, "engine binary"); + File.SetUnixFileMode(casBlob, UnixFileMode.UserRead | UnixFileMode.UserWrite); + + await _service.CreateHardLinkAsync(workspaceFile, casBlob); + + var temporaryPath = workspaceFile + ".genhub-exec-tmp"; + await _strategy.TestEnsureExecutableAsync( + new ManifestFile { RelativePath = "workspace-file", IsExecutable = true }, + workspaceFile); + + Assert.True(File.GetUnixFileMode(workspaceFile).HasFlag(UnixFileMode.UserExecute)); + Assert.False( + File.GetUnixFileMode(casBlob).HasFlag(UnixFileMode.UserExecute), + "The stored blob was modified, so every other profile using this hash is affected."); + + Assert.False( + File.Exists(temporaryPath), + "The temporary copy must not survive; workspace validation would report it."); + + // Content must survive the round trip; a broken link is only acceptable if the + // bytes are identical. + Assert.Equal("engine binary", await File.ReadAllTextAsync(workspaceFile)); + } + + /// + /// The destination must never be observable as missing or non-executable. Verification + /// never mutates, so a workspace left with a non-executable entry point stays broken + /// for every later launch — the failure this sequence exists to prevent. + /// + /// A representing the asynchronous test operation. + [Fact] + public async Task ExecutableMaterialization_ReplacesDestinationAtomically() + { + if (OperatingSystem.IsWindows()) + { + return; + } + + var workspaceFile = Path.Combine(_tempDir, "atomic-entry-point"); + await File.WriteAllTextAsync(workspaceFile, "engine binary"); + File.SetUnixFileMode(workspaceFile, UnixFileMode.UserRead | UnixFileMode.UserWrite); + + var temporaryPath = workspaceFile + ".genhub-exec-tmp"; + await _strategy.TestEnsureExecutableAsync( + new ManifestFile { RelativePath = "atomic-entry-point", IsExecutable = true }, + workspaceFile); + + Assert.True(File.Exists(workspaceFile)); + Assert.True( + File.GetUnixFileMode(workspaceFile).HasFlag(UnixFileMode.UserExecute), + "The replacement was already executable before it became the destination."); + Assert.False(File.Exists(temporaryPath)); + Assert.Equal("engine binary", await File.ReadAllTextAsync(workspaceFile)); + } + + /// + /// Releases the temporary directory. + /// + public void Dispose() + { + GC.SuppressFinalize(this); + try + { + if (Directory.Exists(_tempDir)) + { + Directory.Delete(_tempDir, recursive: true); + } + } + catch (IOException) + { + // A leftover temp directory is not worth failing a test over. + } + } +} diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/FileOperationsServiceTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/FileOperationsServiceTests.cs index dc1938060..929d8539b 100644 --- a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/FileOperationsServiceTests.cs +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/FileOperationsServiceTests.cs @@ -95,42 +95,34 @@ public async Task CreateSymlinkAsync_CreatesSymlinkOrCopies() } /// - /// Tests that CreateHardLinkAsync creates a hard link or falls back to copy on unsupported platforms. + /// The base service must refuse to create a hard link, because it cannot. + /// + /// This replaces a test that swallowed five exception types and then asserted only + /// File.Exists and matching content — assertions a plain File.Copy + /// satisfies. The base implementation did exactly that on Unix, so the test passed + /// while every Linux workspace silently full-copied the game instead of linking it. + /// A test that cannot distinguish the bug from the fix is worse than no test. + /// + /// + /// Real link behaviour is covered per platform, where it can actually be asserted: + /// see UnixFileOperationsServiceTests and WindowsFileOperationsServiceTests. + /// /// /// A representing the asynchronous unit test. [Fact] - public async Task CreateHardLinkAsync_CreatesHardLinkOrCopies() + public async Task CreateHardLinkAsync_OnBaseService_RefusesInsteadOfCopying() { var src = Path.Combine(_tempDir, "source.txt"); var link = Path.Combine(_tempDir, "hardlink.txt"); await File.WriteAllTextAsync(src, "test content"); - // Try to create hard link; on unsupported platforms or not implemented, skip test - try - { - await _service.CreateHardLinkAsync(link, src); - } - catch (NotImplementedException) - { - // Not implemented in base service, skip test - return; - } - catch (PlatformNotSupportedException) - { - return; - } - catch (UnauthorizedAccessException) - { - return; - } - catch (NotSupportedException) - { - return; - } + var thrown = await Record.ExceptionAsync(() => _service.CreateHardLinkAsync(link, src)); - Assert.True(File.Exists(link)); - Assert.Equal("test content", await File.ReadAllTextAsync(link)); + Assert.IsType(thrown); + Assert.False( + File.Exists(link), + "The base service produced a file, which means it silently copied rather than refusing."); } /// diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/GameProfileWorkspaceIntegrationTest.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/GameProfileWorkspaceIntegrationTest.cs index 597d9178e..47ed06c01 100644 --- a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/GameProfileWorkspaceIntegrationTest.cs +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/GameProfileWorkspaceIntegrationTest.cs @@ -13,10 +13,12 @@ using GenHub.Core.Models.Storage; using GenHub.Core.Models.Workspace; using GenHub.Features.Storage.Services; +using GenHub.Features.Workspace; using GenHub.Infrastructure.DependencyInjection; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Moq; +using System.Runtime.InteropServices; namespace GenHub.Tests.Core.Features.Workspace; @@ -82,6 +84,19 @@ public GameProfileWorkspaceIntegrationTest() services.AddWorkspaceServices(); + // AddWorkspaceServices registers the base FileOperationsService, which cannot + // create hard links on any platform by design — each host registers a decorator + // that can. Do the same here on Unix so the HardLink strategy is genuinely + // exercised rather than skipped. + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + services.AddScoped(sp => new UnixFileOperationsService( + sp.GetRequiredService(), + sp.GetRequiredService(), + sp.GetRequiredService>())); + services.AddScoped(); + } + _serviceProvider = services.BuildServiceProvider(); _workspaceManager = _serviceProvider.GetRequiredService(); @@ -326,8 +341,9 @@ public async Task PrepareWorkspace_AllStrategies_HandleGameInstallationFiles(Wor return; } - // Skip HardLink strategy on Windows in Core tests - the base FileOperationsService - // doesn't support hard links on Windows, use WindowsFileOperationsService instead + // Skip HardLink on Windows: the decorator that implements it there lives in + // GenHub.Windows and is covered by WindowsFileOperationsServiceTests. On Unix the + // real UnixFileOperationsService is registered above, so HardLink runs for real. if (strategy == WorkspaceStrategy.HardLink && isWindows) { return; diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/UnixFileOperationsServiceTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/UnixFileOperationsServiceTests.cs new file mode 100644 index 000000000..31dbdc172 --- /dev/null +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/UnixFileOperationsServiceTests.cs @@ -0,0 +1,199 @@ +using System; +using System.IO; +using System.Runtime.InteropServices; +using System.Threading.Tasks; +using GenHub.Core.Interfaces.Common; +using GenHub.Core.Interfaces.Storage; +using GenHub.Core.Models.Results; +using GenHub.Features.Workspace; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace GenHub.Tests.Core.Features.Workspace; + +/// +/// Tests for . +/// +/// These assert that a hard link is a hard link. The previous test asserted only +/// File.Exists and matching content, which a plain copy satisfies — which is +/// exactly why nobody noticed that Unix had been copying instead of linking. +/// +/// +public class UnixFileOperationsServiceTests : IDisposable +{ + private readonly string _tempDir = Path.Combine( + Path.GetTempPath(), + $"genhub-unixfileops-{Guid.NewGuid():N}"); + + private readonly Mock _casServiceMock = new(); + private readonly UnixFileOperationsService _service; + + /// + /// Initializes a new instance of the class. + /// + public UnixFileOperationsServiceTests() + { + Directory.CreateDirectory(_tempDir); + + var baseService = new FileOperationsService( + NullLogger.Instance, + new Mock().Object, + new Mock().Object); + + _service = new UnixFileOperationsService( + baseService, + _casServiceMock.Object, + NullLogger.Instance); + } + + private static bool OnUnix => !RuntimeInformation.IsOSPlatform(OSPlatform.Windows); + + /// + /// The link and its target must be the same inode with a link count of two. Content + /// equality is not sufficient evidence: a copy has identical content and a distinct + /// inode, and that is the failure mode this test exists to detect. + /// + /// A task representing the asynchronous test. + [Fact] + public async Task CreateHardLinkAsync_ProducesRealLinkNotCopy() + { + if (!OnUnix) + { + return; + } + + var source = Path.Combine(_tempDir, "source.dat"); + var link = Path.Combine(_tempDir, "link.dat"); + await File.WriteAllTextAsync(source, "payload"); + + await _service.CreateHardLinkAsync(link, source); + + Assert.True(File.Exists(link)); + + var sourceInfo = new FileInfo(source); + var linkInfo = new FileInfo(link); + + // UnixFileMode alone would not distinguish a copy; the identity check is the point. + Assert.Equal(sourceInfo.Length, linkInfo.Length); + + // Mutating through one path must be visible through the other. That is only true + // for a shared inode, so it distinguishes a link from a copy without needing stat. + await File.WriteAllTextAsync(source, "mutated through the source path"); + Assert.Equal("mutated through the source path", await File.ReadAllTextAsync(link)); + + // And the reverse direction, to rule out a coincidence of ordering. + await File.WriteAllTextAsync(link, "mutated through the link path"); + Assert.Equal("mutated through the link path", await File.ReadAllTextAsync(source)); + } + + /// + /// A missing target must raise a diagnosable error naming the file, rather than the + /// bare "errno 2" that an uninterpreted P/Invoke failure would produce. + /// + /// A task representing the asynchronous test. + [Fact] + public async Task CreateHardLinkAsync_MissingTarget_ThrowsFileNotFound() + { + if (!OnUnix) + { + return; + } + + var missing = Path.Combine(_tempDir, "does-not-exist.dat"); + var link = Path.Combine(_tempDir, "link.dat"); + + var thrown = await Record.ExceptionAsync(() => _service.CreateHardLinkAsync(link, missing)); + + Assert.IsType(thrown); + Assert.Contains("does-not-exist.dat", thrown.Message); + } + + /// + /// Linking over an existing file replaces it, matching the Windows implementation. + /// + /// A task representing the asynchronous test. + [Fact] + public async Task CreateHardLinkAsync_ExistingDestination_IsReplaced() + { + if (!OnUnix) + { + return; + } + + var source = Path.Combine(_tempDir, "source.dat"); + var link = Path.Combine(_tempDir, "link.dat"); + await File.WriteAllTextAsync(source, "new content"); + await File.WriteAllTextAsync(link, "stale content"); + + await _service.CreateHardLinkAsync(link, source); + + Assert.Equal("new content", await File.ReadAllTextAsync(link)); + } + + /// + /// Copying from CAS must create an independent inode. Full-copy and hybrid callers + /// are allowed to modify their destination without changing the shared CAS blob. + /// + /// A task representing the asynchronous test. + [Fact] + public async Task CopyFromCasAsync_ProducesIndependentCopy() + { + var casBlob = Path.Combine(_tempDir, "cas-copy-source.dat"); + var copy = Path.Combine(_tempDir, "cas-copy-destination.dat"); + await File.WriteAllTextAsync(casBlob, "shared content"); + + _casServiceMock + .Setup(service => service.GetContentPathAsync("hash", It.IsAny())) + .ReturnsAsync(OperationResult.CreateSuccess(casBlob)); + + Assert.True(await _service.CopyFromCasAsync("hash", copy)); + + await File.WriteAllTextAsync(copy, "workspace content"); + + Assert.Equal("shared content", await File.ReadAllTextAsync(casBlob)); + Assert.Equal("workspace content", await File.ReadAllTextAsync(copy)); + } + + /// + /// The base implementation must refuse rather than quietly copy. Silently copying is + /// what hid the missing Unix registration for as long as it existed. + /// + /// A task representing the asynchronous test. + [Fact] + public async Task BaseService_CreateHardLinkAsync_ThrowsRatherThanCopying() + { + var baseService = new FileOperationsService( + NullLogger.Instance, + new Mock().Object, + new Mock().Object); + + var source = Path.Combine(_tempDir, "base-source.dat"); + var link = Path.Combine(_tempDir, "base-link.dat"); + await File.WriteAllTextAsync(source, "payload"); + + var thrown = await Record.ExceptionAsync(() => baseService.CreateHardLinkAsync(link, source)); + + Assert.IsType(thrown); + Assert.False(File.Exists(link), "The base service copied the file instead of refusing."); + } + + /// + /// Releases the temporary directory. + /// + public void Dispose() + { + GC.SuppressFinalize(this); + try + { + if (Directory.Exists(_tempDir)) + { + Directory.Delete(_tempDir, recursive: true); + } + } + catch (IOException) + { + // A leftover temp directory is not worth failing a test over. + } + } +} diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/WorkspaceStrategyBaseTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/WorkspaceStrategyBaseTests.cs index 6280f87e8..9eb5ff9b4 100644 --- a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/WorkspaceStrategyBaseTests.cs +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/WorkspaceStrategyBaseTests.cs @@ -284,6 +284,19 @@ public void TestUpdateWorkspaceInfo(WorkspaceInfo workspaceInfo, int fileCount, /// The total size in bytes. public long TestCalculateActualTotalSize(WorkspaceConfiguration configuration) => CalculateActualTotalSize(configuration); + /// + /// Exposes executable materialization for production-path regression tests. + /// + /// The manifest file. + /// The materialized workspace path. + /// A cancellation token. + /// A task representing the operation. + public Task TestEnsureExecutableAsync( + ManifestFile file, + string targetPath, + CancellationToken cancellationToken = default) => + EnsureExecutableAsync(file, targetPath, cancellationToken); + /// protected override Task CreateCasLinkAsync(string hash, string targetPath, GenHub.Core.Models.Enums.ContentType? contentType, CancellationToken cancellationToken) { @@ -291,4 +304,4 @@ protected override Task CreateCasLinkAsync(string hash, string targetPath, GenHu return Task.CompletedTask; } } -} \ No newline at end of file +} diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/WorkspaceValidatorTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/WorkspaceValidatorTests.cs index e7473a7ab..d9294666a 100644 --- a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/WorkspaceValidatorTests.cs +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/WorkspaceValidatorTests.cs @@ -1,3 +1,4 @@ +using System.Runtime.InteropServices; using GenHub.Core.Interfaces.Workspace; using GenHub.Core.Models.Enums; using GenHub.Core.Models.GameClients; @@ -14,7 +15,7 @@ namespace GenHub.Tests.Core.Features.Workspace; /// /// Tests for the WorkspaceValidator class. /// -public class WorkspaceValidatorTests : IDisposable +public partial class WorkspaceValidatorTests : IDisposable { private readonly Mock> _mockLogger; private readonly WorkspaceValidator _validator; @@ -251,6 +252,80 @@ public async Task ValidatePrerequisitesAsync_InsufficientDiskSpace_ReturnsWarnin (i.Severity == ValidationSeverity.Warning && i.Message.Contains("disk space"))); } + /// + /// An execute bit for an identity other than the effective process identity must not + /// make a workspace entry point appear executable. + /// + /// A task representing the asynchronous test. + [Fact] + public async Task ValidateWorkspaceAsync_OtherOnlyExecuteBit_ReturnsAccessWarning() + { + // Root bypasses the permission bits entirely: faccessat reports execute access for + // an other-only bit, so the behaviour under test does not exist for uid 0. Checked + // via geteuid rather than the user name, which is wrong under `sudo -E` and for any + // uid-0 account named otherwise. + if (OperatingSystem.IsWindows() || GetEffectiveUserId() == 0) + { + return; + } + + var executablePath = Path.Combine(_workspaceDir, "client"); + await File.WriteAllTextAsync(executablePath, "engine binary"); + File.SetUnixFileMode( + executablePath, + UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.OtherExecute); + + var workspaceInfo = new WorkspaceInfo + { + Id = "test-workspace", + WorkspacePath = _workspaceDir, + ExecutablePath = executablePath, + }; + + var result = await _validator.ValidateWorkspaceAsync(workspaceInfo); + + Assert.True(result.Success); + Assert.NotNull(result.Data); + Assert.Contains( + result.Data.Issues, + issue => issue.IssueType == ValidationIssueType.AccessDenied + && issue.Severity == ValidationSeverity.Warning); + } + + /// + /// A workspace entry point executable by the current identity remains valid. + /// + /// A task representing the asynchronous test. + [Fact] + public async Task ValidateWorkspaceAsync_ExecutableEntryPoint_HasNoAccessError() + { + if (OperatingSystem.IsWindows()) + { + return; + } + + var executablePath = Path.Combine(_workspaceDir, "client"); + await File.WriteAllTextAsync(executablePath, "engine binary"); + File.SetUnixFileMode( + executablePath, + UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + + var workspaceInfo = new WorkspaceInfo + { + Id = "test-workspace", + WorkspacePath = _workspaceDir, + ExecutablePath = executablePath, + }; + + var result = await _validator.ValidateWorkspaceAsync(workspaceInfo); + + Assert.True(result.Success); + Assert.NotNull(result.Data); + Assert.DoesNotContain( + result.Data.Issues, + issue => issue.IssueType == ValidationIssueType.AccessDenied); + } + /// /// Disposes of test resources. /// @@ -290,4 +365,12 @@ private WorkspaceConfiguration CreateValidConfiguration() Strategy = WorkspaceStrategy.FullCopy, }; } + + /// + /// Effective user ID, POSIX geteuid(2). Declared here because the production + /// equivalent is internal to the GenHub assembly. + /// + /// The effective user ID; 0 is root. + [LibraryImport("libc", EntryPoint = "geteuid")] + private static partial uint GetEffectiveUserId(); } diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/ApplicationDataPathConventionTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/ApplicationDataPathConventionTests.cs new file mode 100644 index 000000000..7c5c2afe9 --- /dev/null +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/ApplicationDataPathConventionTests.cs @@ -0,0 +1,100 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using Xunit; + +namespace GenHub.Tests.Core.Infrastructure; + +/// +/// Guards the convention that application data paths are resolved through +/// IConfigurationProviderService rather than read directly from the OS. +/// +/// ConfigurationProviderService.GetApplicationDataPath() honours a user-configured +/// UserSettings.ApplicationDataPath. Five separate runtime call sites bypassed it +/// with a raw Environment.GetFolderPath(SpecialFolder.ApplicationData), so +/// relocating the data directory moved profiles, workspaces and CAS but silently left +/// manifest discovery, provider loading, the Steam patcher and tool workspaces behind in +/// the default tree. +/// +/// +/// The pattern recurred five times independently, which is evidence that code review does +/// not catch it. This test does. If a new legitimate use appears, add it to the allowlist +/// with a comment explaining why it is not a bypass. +/// +/// +public class ApplicationDataPathConventionTests +{ + private const string ForbiddenPattern = "GetFolderPath(Environment.SpecialFolder.ApplicationData)"; + + /// + /// Files permitted to read the OS application-data folder directly, with the reason. + /// + private static readonly Dictionary Allowed = new(StringComparer.OrdinalIgnoreCase) + { + // The implementation of the convention itself has to start somewhere. + ["ConfigurationProviderService.cs"] = "Defines the canonical path.", + ["UserSettingsService.cs"] = "Loads the settings file that stores the override; cannot depend on it.", + + // Displays the built-in default next to the user's override in the UI. + ["SettingsViewModel.cs"] = "Computes the factory-default path to show on reset.", + + // Core-layer fallback, overridden at the composition root by ContentPipelineModule. + ["ProviderDefinitionLoader.cs"] = "Default only; the DI registration supplies an override.", + }; + + /// + /// Scans the shared and core projects for direct application-data lookups. + /// + [Fact] + public void NoUnapprovedDirectApplicationDataLookups() + { + var repoRoot = FindRepositoryRoot(); + var searchRoots = new[] + { + Path.Combine(repoRoot, "GenHub", "GenHub"), + Path.Combine(repoRoot, "GenHub", "GenHub.Core"), + }; + + var offenders = searchRoots + .Where(Directory.Exists) + .SelectMany(root => Directory.EnumerateFiles(root, "*.cs", SearchOption.AllDirectories)) + .Where(path => !path.Contains($"{Path.DirectorySeparatorChar}obj{Path.DirectorySeparatorChar}") + && !path.Contains($"{Path.DirectorySeparatorChar}bin{Path.DirectorySeparatorChar}")) + .Where(path => !Allowed.ContainsKey(Path.GetFileName(path))) + .Where(path => File.ReadAllText(path).Contains(ForbiddenPattern, StringComparison.Ordinal)) + .Select(path => Path.GetRelativePath(repoRoot, path)) + .OrderBy(p => p, StringComparer.Ordinal) + .ToList(); + + var message = + $"These files read the OS application-data folder directly:{Environment.NewLine}" + + string.Join(Environment.NewLine, offenders.Select(o => " " + o)) + + $"{Environment.NewLine}Use IConfigurationProviderService.GetApplicationDataPath() so a user-relocated " + + "data directory is honoured, or add the file to the allowlist in this test with a reason."; + + Assert.True(offenders.Count == 0, message); + } + + /// + /// Walks up from the test assembly to the directory containing the solution. + /// + /// The repository root path. + private static string FindRepositoryRoot() + { + var directory = new DirectoryInfo(AppContext.BaseDirectory); + + while (directory is not null) + { + if (Directory.Exists(Path.Combine(directory.FullName, "GenHub", "GenHub.Core"))) + { + return directory.FullName; + } + + directory = directory.Parent; + } + + throw new InvalidOperationException( + $"Could not locate the repository root above {AppContext.BaseDirectory}."); + } +} diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/DependencyInjection/GameProfileModuleTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/DependencyInjection/GameProfileModuleTests.cs index fb201c65d..a3c873106 100644 --- a/GenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/DependencyInjection/GameProfileModuleTests.cs +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/DependencyInjection/GameProfileModuleTests.cs @@ -2,6 +2,7 @@ using GenHub.Core.Interfaces.Content; using GenHub.Core.Interfaces.GameInstallations; using GenHub.Core.Interfaces.GameProfiles; +using GenHub.Core.Interfaces.GameSettings; using GenHub.Core.Interfaces.Launching; using GenHub.Core.Interfaces.Manifest; using GenHub.Core.Interfaces.Notifications; @@ -39,6 +40,8 @@ public void AddGameProfileServices_ShouldRegisterAllExpectedServices() services.AddLogging(); services.AddSingleton(configProviderMock.Object); services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); // Mock missing dependencies services.AddScoped(provider => new Mock().Object); @@ -75,6 +78,8 @@ public void AddLaunchingServices_ShouldRegisterAllExpectedServices() services.AddLogging(); services.AddSingleton(configProvider_mock.Object); services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); // Mock dependencies required for manifest services services.AddSingleton(new GenHub.Core.Models.Manifest.ManifestIdService()); @@ -118,6 +123,8 @@ public void AddGameProfileServices_GameProfileRepository_ShouldBeSingleton() services.AddLogging(); services.AddSingleton(configProvider_mock.Object); services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); // Mock missing dependencies services.AddScoped(provider => new Mock().Object); @@ -151,6 +158,8 @@ public void AddLaunchingServices_LaunchRegistry_ShouldBeSingleton() services.AddLogging(); services.AddSingleton(configProvider_mock.Object); services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); // Act services.AddLaunchingServices(); @@ -181,6 +190,8 @@ public void AddGameProfileServices_GameProfileManager_ShouldBeScoped() services.AddLogging(); services.AddSingleton(configProvider_mock.Object); services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); services.AddScoped(provider => new Mock().Object); services.AddScoped(provider => new Mock().Object); services.AddScoped(provider => new Mock().Object); @@ -224,6 +235,8 @@ public void AddGameProfileServices_ShouldCreateProfilesDirectory() services.AddLogging(); services.AddSingleton(configProviderMock.Object); services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); try { @@ -266,6 +279,8 @@ public void AddGameProfileServices_ProfileLauncherFacade_ShouldBeSingleton() services.AddLogging(); services.AddSingleton(configProvider_mock.Object); services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); // Mock missing dependencies services.AddSingleton(new Mock().Object); @@ -321,6 +336,8 @@ public void AddGameProfileServices_ProfileEditorFacade_ShouldBeSingleton() services.AddLogging(); services.AddSingleton(configProvider_mock.Object); services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); // Mock missing dependencies services.AddScoped(provider => new Mock().Object); diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/DependencyInjection/SharedViewModelModuleTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/DependencyInjection/SharedViewModelModuleTests.cs index 78f1ca7a9..b1506fb4e 100644 --- a/GenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/DependencyInjection/SharedViewModelModuleTests.cs +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/DependencyInjection/SharedViewModelModuleTests.cs @@ -1,7 +1,9 @@ using GenHub.Common.ViewModels; using GenHub.Core.Interfaces.Common; using GenHub.Core.Interfaces.Content; +using GenHub.Core.Interfaces.GameSettings; using GenHub.Core.Interfaces.Manifest; +using GenHub.Core.Interfaces.Workspace; using GenHub.Core.Models.Common; using GenHub.Core.Models.Enums; using GenHub.Core.Models.Manifest; @@ -33,6 +35,8 @@ public void AllViewModels_Registered() var configProvider = CreateMockConfigProvider(); services.AddSingleton(configProvider); services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); + services.AddSingleton(new Mock().Object); services.AddSingleton(CreateMockUserSettingsService()); services.AddSingleton(CreateMockAppConfiguration()); diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Models/Manifest/ManifestIngestionGateTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Models/Manifest/ManifestIngestionGateTests.cs new file mode 100644 index 000000000..7883532cd --- /dev/null +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Models/Manifest/ManifestIngestionGateTests.cs @@ -0,0 +1,106 @@ +using System.Globalization; +using GenHub.Core.Constants; +using GenHub.Core.Models.Manifest; +using Xunit; + +namespace GenHub.Tests.Core.Models.Manifest; + +/// +/// Tests for , the fail-closed gate that keeps +/// variant manifests out until the content pipeline is migrated. +/// +public class ManifestIngestionGateTests +{ + /// + /// A manifest without variants is the current published shape and must be unaffected. + /// + [Fact] + public void TryAccept_WithoutVariants_Accepts() + { + var manifest = new ContentManifest { Id = new("1.0.genhub.mod.legacy") }; + + Assert.True(ManifestIngestionGate.TryAccept(manifest, out var reason)); + Assert.Null(reason); + } + + /// + /// A manifest declaring variants must be rejected: every consumer still reads + /// Files, which is empty for a variant manifest, so accepting it would deliver + /// nothing while reporting success. + /// + [Fact] + public void TryAccept_WithVariants_RejectsWithActionableReason() + { + var manifest = new ContentManifest { Id = new("1.0.genhub.mod.variant") }; + manifest.Variants.Add(new ArtifactVariant()); + + Assert.False(ManifestIngestionGate.TryAccept(manifest, out var reason)); + Assert.NotNull(reason); + + // The message must name the manifest, the version it requires, and the way out. + Assert.Contains("1.0.genhub.mod.variant", reason); + Assert.Contains("2", reason); + Assert.Contains("without", reason); + } + + /// + /// A null manifest is not the gate's concern; callers already treat null as a failed + /// parse, and reporting it here would attribute a parse failure to variants. + /// + [Fact] + public void TryAccept_WithNull_Accepts() + { + Assert.True(ManifestIngestionGate.TryAccept(null, out var reason)); + Assert.Null(reason); + } + + /// + /// A manifest declaring the variants format version is rejected even with no variants + /// present: that version may carry other features this pipeline cannot handle. + /// + [Fact] + public void TryAccept_WithVariantFormatVersionButNoVariants_Rejects() + { + var manifest = new ContentManifest + { + Id = new("1.0.genhub.mod.futureformat"), + ManifestVersion = ManifestConstants.VariantsManifestFormatVersion.ToString(CultureInfo.InvariantCulture), + }; + + Assert.False(ManifestIngestionGate.TryAccept(manifest, out var reason)); + Assert.Contains("format version", reason); + } + + /// + /// Variants are rejected even when the manifest claims the legacy version, so a + /// mislabelled manifest cannot slip past by understating its format. + /// + [Fact] + public void TryAccept_WithVariantsButLegacyVersion_StillRejects() + { + var manifest = new ContentManifest + { + Id = new("1.0.genhub.mod.mislabelled"), + ManifestVersion = ManifestConstants.DefaultManifestVersion, + }; + manifest.Variants.Add(new ArtifactVariant()); + + Assert.False(ManifestIngestionGate.TryAccept(manifest, out var reason)); + Assert.Contains("variant", reason); + } + + /// + /// The default version must remain acceptable; every manifest published today carries it. + /// + [Fact] + public void TryAccept_WithDefaultVersion_Accepts() + { + var manifest = new ContentManifest + { + Id = new("1.0.genhub.mod.legacy"), + ManifestVersion = ManifestConstants.DefaultManifestVersion, + }; + + Assert.True(ManifestIngestionGate.TryAccept(manifest, out _)); + } +} diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Models/Manifest/ManifestVariantResolverTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Models/Manifest/ManifestVariantResolverTests.cs new file mode 100644 index 000000000..fb9d392f0 --- /dev/null +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Models/Manifest/ManifestVariantResolverTests.cs @@ -0,0 +1,238 @@ +using System.Collections.Generic; +using GenHub.Core.Models.Manifest; +using Xunit; + +namespace GenHub.Tests.Core.Models.Manifest; + +/// +/// Tests for , which replaced an order-dependent +/// FirstOrDefault(f => f.IsExecutable) with an explicit resolution chain. +/// +public class ManifestVariantResolverTests +{ + /// + /// A manifest with no variants keeps behaving exactly as before. Every manifest + /// written before variants existed is this shape, including everything already + /// sitting in a user's content store. + /// + [Fact] + public void NoVariants_UsesFlatFileList() + { + var manifest = new ContentManifest { Files = [File("generals.exe", true), File("data.big")] }; + + Assert.Equal(2, ManifestVariantResolver.ResolveFiles(manifest).Count); + Assert.True(ManifestVariantResolver.SupportsRuntime(manifest, "osx-arm64")); + Assert.Null(ManifestVariantResolver.ResolveVariant(manifest)); + } + + /// + /// With variants declared, the host's runtime identifier selects one. + /// + [Fact] + public void Variants_SelectByRuntimeIdentifier() + { + var manifest = new ContentManifest + { + Variants = + [ + new() { RuntimeIdentifiers = ["win-x64"], EntryPoint = "generalszh.exe", Files = [File("generalszh.exe", true)] }, + new() { RuntimeIdentifiers = ["osx-arm64"], EntryPoint = "generalszh", Files = [File("generalszh", true), File("libSDL3.dylib")] }, + ], + }; + + Assert.Equal("generalszh", ManifestVariantResolver.ResolveEntryPoint(manifest, "osx-arm64").RelativePath); + Assert.Equal("generalszh.exe", ManifestVariantResolver.ResolveEntryPoint(manifest, "win-x64").RelativePath); + Assert.Equal(2, ManifestVariantResolver.ResolveFiles(manifest, "osx-arm64").Count); + } + + /// + /// Content that declares variants but matches none must report that it cannot run + /// here, so the catalogue can hide it rather than let a user install something inert. + /// + [Fact] + public void Variants_UnmatchedRuntime_IsNotSupported() + { + var manifest = new ContentManifest + { + Variants = [new() { RuntimeIdentifiers = ["win-x64"], Files = [File("generals.exe", true)] }], + }; + + Assert.False(ManifestVariantResolver.SupportsRuntime(manifest, "osx-arm64")); + Assert.Empty(ManifestVariantResolver.ResolveFiles(manifest, "osx-arm64")); + } + + /// + /// A platform-neutral variant is a valid fallback, but an explicit match wins. A + /// release carrying both a native build and a neutral asset bundle must resolve to + /// the native build on a platform it supports. + /// + [Fact] + public void ExplicitRuntimeMatch_BeatsNeutralVariant() + { + var manifest = new ContentManifest + { + Variants = + [ + new() { RuntimeIdentifiers = [], EntryPoint = "shared", Files = [File("shared", true)] }, + new() { RuntimeIdentifiers = ["osx-arm64"], EntryPoint = "native", Files = [File("native", true)] }, + ], + }; + + Assert.Equal("native", ManifestVariantResolver.ResolveEntryPoint(manifest, "osx-arm64").RelativePath); + Assert.Equal("shared", ManifestVariantResolver.ResolveEntryPoint(manifest, "linux-x64").RelativePath); + } + + /// + /// This is the case the old code got silently wrong. Several files can legitimately + /// need the execute bit, and picking the first one is picking by enumeration order. + /// + [Fact] + public void MultipleExecutables_WithoutEntryPoint_FailsAndListsCandidates() + { + var manifest = new ContentManifest + { + Files = [File("generalszh", true), File("crashhandler", true), File("updater", true)], + }; + + var resolution = ManifestVariantResolver.ResolveEntryPoint(manifest); + + Assert.False(resolution.Success); + Assert.Contains("ambiguous", resolution.Reason); + Assert.Equal(3, resolution.Candidates.Count); + Assert.Contains("generalszh", resolution.ToString()); + } + + /// + /// A declared entry point removes the ambiguity above. + /// + [Fact] + public void DeclaredEntryPoint_ResolvesAmbiguity() + { + var manifest = new ContentManifest + { + EntryPoint = "generalszh", + Files = [File("crashhandler", true), File("generalszh", true), File("updater", true)], + }; + + var resolution = ManifestVariantResolver.ResolveEntryPoint(manifest); + + Assert.True(resolution.Success); + Assert.Equal("generalszh", resolution.RelativePath); + } + + /// + /// An entry point naming a file the manifest does not contain is a manifest defect. + /// Catching it here is far more diagnosable than a missing-file error at launch. + /// + [Fact] + public void DeclaredEntryPoint_NotInFileList_Fails() + { + var manifest = new ContentManifest + { + EntryPoint = "generalszh", + Files = [File("generals.exe", true)], + }; + + var resolution = ManifestVariantResolver.ResolveEntryPoint(manifest); + + Assert.False(resolution.Success); + Assert.Contains("not among its", resolution.Reason); + } + + /// + /// Path separators and case must not defeat the entry-point match: manifests are + /// authored on Windows and consumed on Unix. + /// + /// The declared entry point. + /// The path as stored in the file list. + [Theory] + [InlineData("Release/generalszh", "Release/generalszh")] + [InlineData("Release\\generalszh", "Release/generalszh")] + [InlineData("release/GENERALSZH", "Release/generalszh")] + public void EntryPointMatching_IgnoresSeparatorAndCase(string entryPoint, string filePath) + { + var manifest = new ContentManifest { EntryPoint = entryPoint, Files = [File(filePath, true)] }; + + var resolution = ManifestVariantResolver.ResolveEntryPoint(manifest); + + Assert.True(resolution.Success); + Assert.Equal(filePath, resolution.RelativePath); + } + + /// + /// A variant must not inherit the flat manifest entry point. The flat file list and + /// its entry point are ignored whenever variants are present. + /// + [Fact] + public void VariantWithoutEntryPoint_DoesNotUseFlatManifestEntryPoint() + { + var manifest = new ContentManifest + { + EntryPoint = "flat.exe", + Files = [File("flat.exe", true)], + Variants = + [ + new() + { + RuntimeIdentifiers = ["linux-x64"], + Files = [File("run.sh", true), File("generalszh", true)], + }, + ], + }; + + var resolution = ManifestVariantResolver.ResolveEntryPoint(manifest, "linux-x64"); + + Assert.True(resolution.Success); + Assert.Equal("generalszh", resolution.RelativePath); + } + + /// + /// Helper scripts need execute permission but are not inferred launch targets. + /// + [Fact] + public void ExecutePermissionHelper_DoesNotMakeNativeClientAmbiguous() + { + var manifest = new ContentManifest + { + Files = [File("run.sh", true), File("generalszh", true)], + }; + + var resolution = ManifestVariantResolver.ResolveEntryPoint(manifest); + + Assert.True(resolution.Success); + Assert.Equal("generalszh", resolution.RelativePath); + } + + /// + /// Legacy manifests that set no execute flags still resolve when exactly one file + /// looks like a launch target by extension. + /// + [Fact] + public void LegacyManifest_WithSingleExe_StillResolves() + { + var manifest = new ContentManifest { Files = [File("generals.exe"), File("data.big"), File("d3d8.dll")] }; + + var resolution = ManifestVariantResolver.ResolveEntryPoint(manifest); + + Assert.True(resolution.Success); + Assert.Equal("generals.exe", resolution.RelativePath); + } + + /// + /// A manifest with nothing runnable reports that plainly rather than resolving to + /// some arbitrary data file. + /// + [Fact] + public void ManifestWithNoLaunchableFile_Fails() + { + var manifest = new ContentManifest { Files = [File("maps.big"), File("Options.ini")] }; + + var resolution = ManifestVariantResolver.ResolveEntryPoint(manifest); + + Assert.False(resolution.Success); + Assert.Contains("no launchable file", resolution.Reason); + } + + private static ManifestFile File(string path, bool isExecutable = false) => + new() { RelativePath = path, IsExecutable = isExecutable }; +} diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Utilities/ExecutableFileClassifierTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Utilities/ExecutableFileClassifierTests.cs new file mode 100644 index 000000000..ec5ce734b --- /dev/null +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Utilities/ExecutableFileClassifierTests.cs @@ -0,0 +1,86 @@ +using GenHub.Core.Utilities; +using Xunit; + +namespace GenHub.Tests.Core.Utilities; + +/// +/// Table-driven tests for , which replaced five +/// disagreeing implementations of "is this executable". +/// +public class ExecutableFileClassifierTests +{ + /// + /// Verifies which files are marked as needing the Unix execute bit. + /// + /// The candidate path. + /// Whether the execute bit is required. + [Theory] + + // Native binaries have no extension. This is the case the old classifiers disagreed + // on, and the one a native macOS or Linux game client depends on. + [InlineData("generalszh", true)] + [InlineData("GeneralsMD/Release/generalszh", true)] + [InlineData("generals.exe", true)] + [InlineData("run.sh", true)] + [InlineData("Launch.command", true)] + + // Loadable code, mapped by the loader with read access. Marking these +x is + // meaningless and, under a hard-link workspace, would mutate a shared CAS blob. + [InlineData("libSDL3.dylib", false)] + [InlineData("libbgfx.so", false)] + [InlineData("d3d8.dll", false)] + + // Data, whatever the Steam layout does with it. + [InlineData("game.dat", false)] + [InlineData("INIZH.big", false)] + [InlineData("Options.ini", false)] + [InlineData("texture.tga", false)] + [InlineData("", false)] + public void RequiresExecutePermission_ClassifiesCorrectly(string path, bool expected) + { + Assert.Equal(expected, ExecutableFileClassifier.RequiresExecutePermission(path)); + } + + /// + /// Verifies which files may serve as a launch target when no entry point is declared. + /// + /// The candidate path. + /// Whether the file is a plausible legacy launch target. + [Theory] + [InlineData("generalszh", true)] + [InlineData("generals.exe", true)] + [InlineData("GeneralsOnlineZH_60.exe", true)] + + // A library is never launched, so it must not win a FirstOrDefault over the real + // entry point simply by appearing earlier in the file list. + [InlineData("libSDL3.dylib", false)] + [InlineData("libbgfx.so", false)] + [InlineData("d3d8.dll", false)] + + // Shell wrappers are runnable but are not what a profile launches; the engine binary is. + [InlineData("run.sh", false)] + [InlineData("game.dat", false)] + [InlineData("", false)] + public void IsLegacyLaunchCandidate_ClassifiesCorrectly(string path, bool expected) + { + Assert.Equal(expected, ExecutableFileClassifier.IsLegacyLaunchCandidate(path)); + } + + /// + /// The two questions must not be assumed equivalent. A dylib needs neither, a shell + /// wrapper needs the execute bit but is not a launch target, and a native binary + /// needs both. Collapsing them into one boolean is what this class exists to undo. + /// + [Fact] + public void TheTwoQuestionsAreIndependent() + { + Assert.True(ExecutableFileClassifier.RequiresExecutePermission("run.sh")); + Assert.False(ExecutableFileClassifier.IsLegacyLaunchCandidate("run.sh")); + + Assert.True(ExecutableFileClassifier.RequiresExecutePermission("generalszh")); + Assert.True(ExecutableFileClassifier.IsLegacyLaunchCandidate("generalszh")); + + Assert.False(ExecutableFileClassifier.RequiresExecutePermission("libSDL3.dylib")); + Assert.False(ExecutableFileClassifier.IsLegacyLaunchCandidate("libSDL3.dylib")); + } +} diff --git a/GenHub/GenHub.Windows/Features/Workspace/WindowsSymlinkCapabilityProvider.cs b/GenHub/GenHub.Windows/Features/Workspace/WindowsSymlinkCapabilityProvider.cs new file mode 100644 index 000000000..185e7efd4 --- /dev/null +++ b/GenHub/GenHub.Windows/Features/Workspace/WindowsSymlinkCapabilityProvider.cs @@ -0,0 +1,58 @@ +using System; +using System.IO; +using System.Runtime.Versioning; +using GenHub.Core.Interfaces.Workspace; + +namespace GenHub.Windows.Features.Workspace; + +/// +/// Symlink capability on Windows. +/// +/// +/// Capability is determined by a real, cached creation probe rather than Administrator +/// membership. Developer Mode can permit unelevated symlink creation, while policy can +/// deny it to an otherwise elevated process. +/// +[SupportedOSPlatform("windows")] +public sealed class WindowsSymlinkCapabilityProvider : ISymlinkCapabilityProvider +{ + private static readonly Lazy _cachedCapability = new(ProbeCapability); + + /// + public bool CanCreateSymlinks => _cachedCapability.Value; + + private static bool ProbeCapability() + { + var probeId = Guid.NewGuid().ToString("N"); + var targetPath = Path.Combine(Path.GetTempPath(), $"genhub-symlink-target-{probeId}.tmp"); + var linkPath = Path.Combine(Path.GetTempPath(), $"genhub-symlink-link-{probeId}.tmp"); + + try + { + File.WriteAllText(targetPath, string.Empty); + File.CreateSymbolicLink(linkPath, targetPath); + return new FileInfo(linkPath).LinkTarget is not null; + } + catch (Exception) + { + return false; + } + finally + { + DeleteIfExists(linkPath); + DeleteIfExists(targetPath); + } + } + + private static void DeleteIfExists(string path) + { + try + { + File.Delete(path); + } + catch (Exception) + { + // Best-effort cleanup of a uniquely named temporary probe. + } + } +} diff --git a/GenHub/GenHub.Windows/Infrastructure/DependencyInjection/WindowsServicesModule.cs b/GenHub/GenHub.Windows/Infrastructure/DependencyInjection/WindowsServicesModule.cs index 6651030eb..bbe90325a 100644 --- a/GenHub/GenHub.Windows/Infrastructure/DependencyInjection/WindowsServicesModule.cs +++ b/GenHub/GenHub.Windows/Infrastructure/DependencyInjection/WindowsServicesModule.cs @@ -1,10 +1,12 @@ using System; using System.Runtime.Versioning; using GenHub.Core.Interfaces.GameInstallations; +using GenHub.Core.Interfaces.GameSettings; using GenHub.Core.Interfaces.GitHub; using GenHub.Core.Interfaces.Shortcuts; using GenHub.Core.Interfaces.Storage; using GenHub.Core.Interfaces.Workspace; +using GenHub.Features.GameSettings; using GenHub.Features.Workspace; using GenHub.Windows.Features.GitHub.Services; using GenHub.Windows.Features.Shortcuts; @@ -29,6 +31,8 @@ public static IServiceCollection AddWindowsServices(this IServiceCollection serv { // Register Windows-specific services services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); diff --git a/GenHub/GenHub/Features/Content/Services/CommunityOutpost/CommunityOutpostDeliverer.cs b/GenHub/GenHub/Features/Content/Services/CommunityOutpost/CommunityOutpostDeliverer.cs index 4122f2b61..2426893d4 100644 --- a/GenHub/GenHub/Features/Content/Services/CommunityOutpost/CommunityOutpostDeliverer.cs +++ b/GenHub/GenHub/Features/Content/Services/CommunityOutpost/CommunityOutpostDeliverer.cs @@ -1,11 +1,3 @@ -using System; -using System.Collections.Generic; -using System.IO; -using System.IO.Compression; -using System.Linq; -using System.Text.Json; -using System.Threading; -using System.Threading.Tasks; using GenHub.Core.Constants; using GenHub.Core.Interfaces.Common; using GenHub.Core.Interfaces.Content; @@ -18,10 +10,19 @@ using GenHub.Core.Models.GameInstallations; using GenHub.Core.Models.Manifest; using GenHub.Core.Models.Results; +using GenHub.Core.Utilities; using Microsoft.Extensions.Logging; using SharpCompress.Archives; using SharpCompress.Archives.SevenZip; using SharpCompress.Common; +using System; +using System.Collections.Generic; +using System.IO; +using System.IO.Compression; +using System.Linq; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; namespace GenHub.Features.Content.Services.CommunityOutpost; @@ -177,7 +178,7 @@ private static async Task> CreateGenericManifestAsync( RelativePath = relativePath, Size = fileInfo.Length, IsRequired = true, - IsExecutable = relativePath.EndsWith(".exe", StringComparison.OrdinalIgnoreCase), + IsExecutable = ExecutableFileClassifier.RequiresExecutePermission(relativePath), SourceType = ContentSourceType.ExtractedPackage, }); } diff --git a/GenHub/GenHub/Features/Content/Services/Helpers/GitHubInferenceHelper.cs b/GenHub/GenHub/Features/Content/Services/Helpers/GitHubInferenceHelper.cs index 7498bfff4..6897335d2 100644 --- a/GenHub/GenHub/Features/Content/Services/Helpers/GitHubInferenceHelper.cs +++ b/GenHub/GenHub/Features/Content/Services/Helpers/GitHubInferenceHelper.cs @@ -5,6 +5,7 @@ using GenHub.Core.Constants; using GenHub.Core.Models.Enums; using GenHub.Core.Models.GitHub; +using GenHub.Core.Utilities; namespace GenHub.Features.Content.Services.Helpers; @@ -190,12 +191,7 @@ public static List InferTagsFromRelease(GitHubRelease release) /// True when the extension matches a known executable type. public static bool IsExecutableFile(string fileName) { - var ext = Path.GetExtension(fileName); - return ext.Equals(".exe", StringComparison.OrdinalIgnoreCase) - || ext.Equals(".dll", StringComparison.OrdinalIgnoreCase) - || ext.Equals(".sh", StringComparison.OrdinalIgnoreCase) - || ext.Equals(".bat", StringComparison.OrdinalIgnoreCase) - || ext.Equals(".so", StringComparison.OrdinalIgnoreCase); + return ExecutableFileClassifier.RequiresExecutePermission(fileName); } /// diff --git a/GenHub/GenHub/Features/GameProfiles/Services/ProfileLauncherFacade.cs b/GenHub/GenHub/Features/GameProfiles/Services/ProfileLauncherFacade.cs index ce7159d6f..68ae0c2d3 100644 --- a/GenHub/GenHub/Features/GameProfiles/Services/ProfileLauncherFacade.cs +++ b/GenHub/GenHub/Features/GameProfiles/Services/ProfileLauncherFacade.cs @@ -52,6 +52,7 @@ public class ProfileLauncherFacade( IPublisherReconcilerRegistry reconcilerRegistry, IConfigurationProviderService configurationProvider, IGameProcessManager gameProcessManager, + ISymlinkCapabilityProvider symlinkCapability, ILogger logger) : IProfileLauncherFacade { /// @@ -156,7 +157,7 @@ public async Task> LaunchProfileAsync(str // Define a base path for the workspace - for tools we can use a temp dir or app data // WorkspaceManager requires a BaseInstallationPath, even if empty for tools - var appDataBase = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), AppConstants.AppName); + var appDataBase = configurationProvider.GetApplicationDataPath(); if (!Directory.Exists(appDataBase)) Directory.CreateDirectory(appDataBase); var baseDetails = appDataBase; @@ -165,22 +166,16 @@ public async Task> LaunchProfileAsync(str var resolutionResult = await dependencyResolver.ResolveDependenciesWithManifestsAsync(profile.EnabledContentIds ?? [], cancellationToken); var allManifests = resolutionResult.Success ? resolutionResult.ResolvedManifests : [toolManifest]; - // Determine effective strategy with admin rights check for symlink strategies - var effectiveToolStrategy = profile.WorkspaceStrategy ?? configurationProvider.GetDefaultWorkspaceStrategy(); - var isToolAdmin = false; - if (OperatingSystem.IsWindows()) - { - using var identity = WindowsIdentity.GetCurrent(); - var principal = new WindowsPrincipal(identity); - isToolAdmin = principal.IsInRole(WindowsBuiltInRole.Administrator); - } + // Determine effective strategy, downgrading symlink strategies only + // where symlinks genuinely cannot be created. See ISymlinkCapabilityProvider. + var requestedToolStrategy = profile.WorkspaceStrategy ?? configurationProvider.GetDefaultWorkspaceStrategy(); + var effectiveToolStrategy = ResolveSupportedWorkspaceStrategy(requestedToolStrategy); - if (!isToolAdmin && (effectiveToolStrategy == WorkspaceStrategy.HybridCopySymlink || effectiveToolStrategy == WorkspaceStrategy.SymlinkOnly)) + if (effectiveToolStrategy != requestedToolStrategy) { logger.LogInformation( - "[Launch] Tool workspace - Switching from {OriginalStrategy} to HardLink strategy due to missing admin rights", - effectiveToolStrategy); - effectiveToolStrategy = WorkspaceStrategy.HardLink; + "[Launch] Tool workspace - Switching from {OriginalStrategy} to HardLink: symlinks are unavailable in this environment", + requestedToolStrategy); } actualWorkspaceId = $"{ProfileConstants.ToolProfileWorkspaceIdPrefix}-{profile.Id}"; @@ -452,45 +447,33 @@ public async Task> LaunchProfileAsync(str logger.LogDebug("[Launch] Step 4: Options.ini will be applied by GameLauncher (delegated)"); var effectiveStrategy = profile.WorkspaceStrategy ?? configurationProvider.GetDefaultWorkspaceStrategy(); - logger.LogDebug("[Launch] Step 5: Checking workspace strategy and admin rights - Strategy: {Strategy}", effectiveStrategy); + logger.LogDebug("[Launch] Step 5: Checking workspace strategy and symlink capability - Strategy: {Strategy}", effectiveStrategy); - // Admin check for symlink strategies. - // If not admin and using a symlink-based strategy, permanently switch the profile to HardLink strategy. - var isProfileAdmin = false; - if (OperatingSystem.IsWindows()) - { - using var identity = WindowsIdentity.GetCurrent(); - var principal = new WindowsPrincipal(identity); - isProfileAdmin = principal.IsInRole(WindowsBuiltInRole.Administrator); - logger.LogInformation( - "Profile {ProfileId} launch - Admin check: IsAdmin={IsAdmin}, User={User}, Strategy={Strategy}", - profileId, - isProfileAdmin, - identity.Name, - effectiveStrategy); - } - else - { - logger.LogInformation( - "Profile {ProfileId} launch - Non-Windows platform, admin check skipped, Strategy={Strategy}", - profileId, - effectiveStrategy); - } + // Downgrade symlink strategies only where symlinks genuinely cannot be created. + // This previously asked whether the process was an administrator and computed + // that only on Windows, leaving it false on Unix — which made SymlinkOnly and + // HybridCopySymlink unreachable there despite symlink(2) needing no privilege. + var canCreateSymlinks = symlinkCapability.CanCreateSymlinks; + logger.LogInformation( + "Profile {ProfileId} launch - Symlink capability: {CanCreateSymlinks}, Strategy={Strategy}", + profileId, + canCreateSymlinks, + effectiveStrategy); - if (!isProfileAdmin && (effectiveStrategy == WorkspaceStrategy.HybridCopySymlink || effectiveStrategy == WorkspaceStrategy.SymlinkOnly)) + if (!canCreateSymlinks && (effectiveStrategy == WorkspaceStrategy.HybridCopySymlink || effectiveStrategy == WorkspaceStrategy.SymlinkOnly)) { - // No admin rights - switch profile to HardLink strategy permanently + // Symlinks unavailable here - switch the profile to HardLink var originalStrategy = effectiveStrategy; effectiveStrategy = WorkspaceStrategy.HardLink; // Force HardLink instead of default which might be symlink-based logger.LogInformation( - "Profile {ProfileId} - Switching from {OriginalStrategy} to HardLink strategy due to missing admin rights", + "Profile {ProfileId} - Switching from {OriginalStrategy} to HardLink because symlinks are unavailable in this environment", profileId, originalStrategy); notificationService.ShowInfo( "Workspace Strategy Changed", - $"'{profile.Name}' requires admin for {originalStrategy}. Switching to HardLink strategy.", + $"'{profile.Name}' cannot use {originalStrategy} here because symlinks are unavailable. Switching to HardLink.", NotificationDurations.Long); // Persist the downgrade only if the profile had an explicit strategy set (not inheriting default) @@ -504,13 +487,18 @@ public async Task> LaunchProfileAsync(str if (strategyUpdateResult.Success) { logger.LogInformation( - "Updated profile {ProfileId} workspace strategy to {Strategy} due to admin rights requirement", + "Updated profile {ProfileId} workspace strategy to {Strategy} because symlinks are unavailable", profileId, effectiveStrategy); } } } + // GameLauncher constructs the actual workspace request from this in-memory + // profile. Persisting an explicit downgrade is not enough: inherited defaults + // are intentionally not persisted, and persistence may fail independently. + profile.WorkspaceStrategy = effectiveStrategy; + // Use dynamic workspace path based on the game installation location var casPoolPath = storageLocationService.GetCasPoolPath(resolvedInstallation); var workspacePath = storageLocationService.GetWorkspacePath(resolvedInstallation); @@ -950,7 +938,8 @@ public async Task> PrepareWorkspaceAsync(s Id = profileId, Manifests = manifests, GameClient = profile.GameClient!, - Strategy = profile.WorkspaceStrategy ?? configurationProvider.GetDefaultWorkspaceStrategy(), + Strategy = ResolveSupportedWorkspaceStrategy( + profile.WorkspaceStrategy ?? configurationProvider.GetDefaultWorkspaceStrategy()), ForceRecreate = false, ValidateAfterPreparation = true, ManifestSourcePaths = manifestSourcePaths, @@ -1732,6 +1721,14 @@ private async Task> VerifyCasContentAvailabilityAsync(IEnu return OperationResult.CreateSuccess(true); } + private WorkspaceStrategy ResolveSupportedWorkspaceStrategy(WorkspaceStrategy strategy) + { + return !symlinkCapability.CanCreateSymlinks + && strategy is WorkspaceStrategy.HybridCopySymlink or WorkspaceStrategy.SymlinkOnly + ? WorkspaceStrategy.HardLink + : strategy; + } + /// /// Detects if a profile is implicitly a tool profile and returns the tool content ID. /// diff --git a/GenHub/GenHub/Features/GameProfiles/ViewModels/FileTreeItem.cs b/GenHub/GenHub/Features/GameProfiles/ViewModels/FileTreeItem.cs index e6ed9918a..dc1dbfc2f 100644 --- a/GenHub/GenHub/Features/GameProfiles/ViewModels/FileTreeItem.cs +++ b/GenHub/GenHub/Features/GameProfiles/ViewModels/FileTreeItem.cs @@ -2,6 +2,7 @@ using System.Collections.ObjectModel; using System.IO; using CommunityToolkit.Mvvm.ComponentModel; +using GenHub.Core.Utilities; namespace GenHub.Features.GameProfiles.ViewModels; @@ -33,7 +34,7 @@ public partial class FileTreeItem : ObservableObject /// /// Gets a value indicating whether this file is an executable (.exe). /// - public bool IsExecutable => IsFile && Path.GetExtension(Name).Equals(".exe", StringComparison.OrdinalIgnoreCase); + public bool IsExecutable => IsFile && ExecutableFileClassifier.IsLegacyLaunchCandidate(Name); /// /// Gets or sets a value indicating whether this item is selected as the executable. diff --git a/GenHub/GenHub/Features/GameSettings/GamePathProviderBase.cs b/GenHub/GenHub/Features/GameSettings/GamePathProviderBase.cs new file mode 100644 index 000000000..fa2c358a2 --- /dev/null +++ b/GenHub/GenHub/Features/GameSettings/GamePathProviderBase.cs @@ -0,0 +1,43 @@ +using System.IO; +using GenHub.Core.Constants; +using GenHub.Core.Interfaces.GameSettings; +using GenHub.Core.Models.Enums; + +namespace GenHub.Features.GameSettings; + +/// +/// Shared behaviour for implementations. +/// +/// Every platform uses the same leaf folder name — "Command and Conquer Generals Data" +/// or "Command and Conquer Generals Zero Hour Data" — and differs only in the base +/// directory those sit under. Subclasses supply the base; this class owns the rest. +/// +/// +/// The paths are dictated by the game engine, not chosen by GenHub. See +/// GlobalData::BuildUserDataPathFromRegistry in the GeneralsGameCode tree: +/// Windows resolves Documents via SHGetKnownFolderPath, macOS uses +/// ~/Library/Application Support, and Linux uses XDG_DATA_HOME with a +/// ~/.local/share fallback. Writing Options.ini anywhere else means the engine +/// silently never reads it, the launch still succeeds, and every profile setting is +/// discarded without an error. +/// +/// +public abstract class GamePathProviderBase : IGamePathProvider +{ + /// + public string GetOptionsDirectory(GameType gameType) + { + var folderName = gameType == GameType.ZeroHour + ? GameSettingsConstants.FolderNames.ZeroHour + : GameSettingsConstants.FolderNames.Generals; + + return Path.Combine(GetUserDataBaseDirectory(), folderName); + } + + /// + /// Gets the platform's base directory for per-user game data, without the + /// game-specific leaf folder. + /// + /// An absolute directory path. + protected abstract string GetUserDataBaseDirectory(); +} diff --git a/GenHub/GenHub/Features/GameSettings/GameSettingsService.cs b/GenHub/GenHub/Features/GameSettings/GameSettingsService.cs index 88ce6a78f..2dc4a45d8 100644 --- a/GenHub/GenHub/Features/GameSettings/GameSettingsService.cs +++ b/GenHub/GenHub/Features/GameSettings/GameSettingsService.cs @@ -18,7 +18,7 @@ namespace GenHub.Features.GameSettings; /// /// Service for managing game settings (Options.ini) for Generals and Zero Hour. /// -public class GameSettingsService(ILogger logger, IGamePathProvider? pathProvider = null) : IGameSettingsService +public class GameSettingsService(ILogger logger, IGamePathProvider pathProvider) : IGameSettingsService { private static readonly JsonSerializerOptions _jsonSerializerOptions = new() { @@ -34,7 +34,13 @@ public class GameSettingsService(ILogger logger, IGamePathP private static readonly SemaphoreSlim _optionsIniWriteSemaphore = new(1, 1); private readonly ILogger _logger = logger ?? throw new ArgumentNullException(nameof(logger)); - private readonly IGamePathProvider _pathProvider = pathProvider ?? new WindowsGamePathProvider(); + + // Required, not optional. This previously defaulted to WindowsGamePathProvider when + // nothing was registered — which was every platform, because no DI module registered + // an IGamePathProvider at all. macOS and Linux therefore wrote Options.ini via + // SpecialFolder.MyDocuments, which .NET maps to $HOME on Unix. An unregistered + // dependency must fail at container build, not silently resolve to the wrong OS. + private readonly IGamePathProvider _pathProvider = pathProvider ?? throw new ArgumentNullException(nameof(pathProvider)); /// public virtual string GetOptionsFilePath(GameType gameType) diff --git a/GenHub/GenHub/Features/GameSettings/LinuxGamePathProvider.cs b/GenHub/GenHub/Features/GameSettings/LinuxGamePathProvider.cs new file mode 100644 index 000000000..8ddf7450e --- /dev/null +++ b/GenHub/GenHub/Features/GameSettings/LinuxGamePathProvider.cs @@ -0,0 +1,36 @@ +using System; +using System.IO; + +namespace GenHub.Features.GameSettings; + +/// +/// Linux implementation of . +/// +/// Resolves to $XDG_DATA_HOME/Command and Conquer Generals Zero Hour Data, +/// falling back to ~/.local/share/... when the variable is unset, matching +/// GlobalData::BuildUserDataPathFromRegistry in the game engine. +/// +/// +/// Until this existed, Linux fell through to WindowsGamePathProvider and +/// resolved Environment.SpecialFolder.MyDocuments, which .NET maps to the home +/// directory on Unix. Options.ini therefore landed directly in $HOME. +/// +/// +public sealed class LinuxGamePathProvider : GamePathProviderBase +{ + /// + protected override string GetUserDataBaseDirectory() + { + var xdgDataHome = Environment.GetEnvironmentVariable("XDG_DATA_HOME"); + if (!string.IsNullOrWhiteSpace(xdgDataHome)) + { + return xdgDataHome; + } + + var home = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); + + return string.IsNullOrEmpty(home) + ? Directory.GetCurrentDirectory() + : Path.Combine(home, ".local", "share"); + } +} diff --git a/GenHub/GenHub/Features/GameSettings/MacOSGamePathProvider.cs b/GenHub/GenHub/Features/GameSettings/MacOSGamePathProvider.cs new file mode 100644 index 000000000..002c753c4 --- /dev/null +++ b/GenHub/GenHub/Features/GameSettings/MacOSGamePathProvider.cs @@ -0,0 +1,32 @@ +using System; +using System.IO; + +namespace GenHub.Features.GameSettings; + +/// +/// macOS implementation of . +/// +/// Resolves to ~/Library/Application Support/Command and Conquer Generals Zero Hour Data, +/// matching GlobalData::BuildUserDataPathFromRegistry in the game engine. +/// +/// +/// Note there is no vendor subdirectory and the leaf name is identical to Windows. +/// This is deliberately not the SDL_GetPrefPath convention +/// (~/Library/Application Support/<org>/<app>/) that an SDL3-based +/// port might be expected to use. +/// +/// +public sealed class MacOSGamePathProvider : GamePathProviderBase +{ + /// + protected override string GetUserDataBaseDirectory() + { + var home = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); + + // Environment.SpecialFolder.ApplicationData maps to ~/.config on macOS, which is + // the Linux convention rather than the Apple one, so it is not used here. + return string.IsNullOrEmpty(home) + ? Directory.GetCurrentDirectory() + : Path.Combine(home, "Library", "Application Support"); + } +} diff --git a/GenHub/GenHub/Features/GameSettings/WindowsGamePathProvider.cs b/GenHub/GenHub/Features/GameSettings/WindowsGamePathProvider.cs index d353148ae..13ad2f45a 100644 --- a/GenHub/GenHub/Features/GameSettings/WindowsGamePathProvider.cs +++ b/GenHub/GenHub/Features/GameSettings/WindowsGamePathProvider.cs @@ -1,23 +1,20 @@ using System; -using System.IO; -using GenHub.Core.Constants; -using GenHub.Core.Interfaces.GameSettings; -using GenHub.Core.Models.Enums; namespace GenHub.Features.GameSettings; /// -/// Windows-specific implementation of game path provider. +/// Windows implementation of . +/// +/// Resolves to Documents/Command and Conquer Generals Zero Hour Data, matching +/// GlobalData::BuildUserDataPathFromRegistry in the game engine, which uses +/// SHGetKnownFolderPath(FOLDERID_Documents) so that OneDrive and Group Policy +/// folder redirection are honoured. SpecialFolder.MyDocuments resolves through +/// the same known-folder mechanism. +/// /// -public class WindowsGamePathProvider : IGamePathProvider +public sealed class WindowsGamePathProvider : GamePathProviderBase { /// - public string GetOptionsDirectory(GameType gameType) - { - var documentsPath = Environment.GetFolderPath(Environment.SpecialFolder.MyDocuments); - var folderName = gameType == GameType.ZeroHour - ? GameSettingsConstants.FolderNames.ZeroHour - : GameSettingsConstants.FolderNames.Generals; - return Path.Combine(documentsPath, folderName); - } -} \ No newline at end of file + protected override string GetUserDataBaseDirectory() => + Environment.GetFolderPath(Environment.SpecialFolder.MyDocuments); +} diff --git a/GenHub/GenHub/Features/Manifest/ContentManifestBuilder.cs b/GenHub/GenHub/Features/Manifest/ContentManifestBuilder.cs index a7bebcbc7..5117bfc4c 100644 --- a/GenHub/GenHub/Features/Manifest/ContentManifestBuilder.cs +++ b/GenHub/GenHub/Features/Manifest/ContentManifestBuilder.cs @@ -1,8 +1,3 @@ -using System; -using System.Collections.Generic; -using System.IO; -using System.Linq; -using System.Threading.Tasks; using GenHub.Core.Constants; using GenHub.Core.Interfaces.Common; using GenHub.Core.Interfaces.Manifest; @@ -10,7 +5,13 @@ using GenHub.Core.Models.Enums; using GenHub.Core.Models.GameInstallations; using GenHub.Core.Models.Manifest; +using GenHub.Core.Utilities; using Microsoft.Extensions.Logging; +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading.Tasks; namespace GenHub.Features.Manifest; @@ -733,8 +734,10 @@ public ContentManifest Build() private static bool IsExecutableFile(string filePath) { - var extension = Path.GetExtension(filePath).ToLowerInvariant(); - return (extension == ".exe" || extension == ".dll" || extension == ".so" || extension == string.Empty) && File.Exists(filePath); + // Delegates to the shared classifier. The previous implementation also required + // File.Exists, which made classification depend on disk state at build time: + // the same manifest could classify differently depending on when it was built. + return ExecutableFileClassifier.RequiresExecutePermission(filePath); } /// The normalized version string. diff --git a/GenHub/GenHub/Features/Manifest/ContentManifestPool.cs b/GenHub/GenHub/Features/Manifest/ContentManifestPool.cs index 763776256..a60211dcf 100644 --- a/GenHub/GenHub/Features/Manifest/ContentManifestPool.cs +++ b/GenHub/GenHub/Features/Manifest/ContentManifestPool.cs @@ -321,6 +321,14 @@ private static OperationResult ValidateManifest(ContentManifest manifest) { var errors = new List(); + // Applied here rather than at each caller: both AddManifestAsync overloads run + // this, and every deliverer, resolver and detector reaches the pool through them. + // Gating the discovery and provider services alone left those paths open. + if (!ManifestIngestionGate.TryAccept(manifest, out var variantRejection)) + { + errors.Add(variantRejection!); + } + if (string.IsNullOrEmpty(manifest.Id.Value)) errors.Add("Manifest ID is required"); diff --git a/GenHub/GenHub/Features/Manifest/ManifestDiscoveryService.cs b/GenHub/GenHub/Features/Manifest/ManifestDiscoveryService.cs index e272862ed..4aa61f124 100644 --- a/GenHub/GenHub/Features/Manifest/ManifestDiscoveryService.cs +++ b/GenHub/GenHub/Features/Manifest/ManifestDiscoveryService.cs @@ -7,6 +7,7 @@ using System.Threading; using System.Threading.Tasks; using GenHub.Core.Constants; +using GenHub.Core.Interfaces.Common; using GenHub.Core.Interfaces.Manifest; using GenHub.Core.Models.Enums; using GenHub.Core.Models.Manifest; @@ -17,32 +18,26 @@ namespace GenHub.Features.Manifest; /// /// Service for discovering and indexing manifests in the GenHub file system, and for populating the manifest cache. /// -public class ManifestDiscoveryService(ILogger logger, IManifestCache manifestCache) +/// +/// The filesystem enumerators are optional constructor parameters rather than a separate +/// test-only constructor. A second constructor chaining into this one has to hardcode the +/// primary constructor's arity, so adding a dependency here breaks that chain without +/// producing a merge conflict — it merges cleanly and fails to compile instead. +/// +public class ManifestDiscoveryService( + ILogger logger, + IManifestCache manifestCache, + IConfigurationProviderService configurationProvider, + Func>? enumerateFiles = null, + Func>? enumerateDirectories = null) { private static readonly JsonSerializerOptions JsonOptions = new() { PropertyNameCaseInsensitive = true }; private readonly Func> _enumerateFiles = - (directory, pattern) => Directory.EnumerateFiles(directory, pattern, SearchOption.TopDirectoryOnly); + enumerateFiles ?? ((directory, pattern) => + Directory.EnumerateFiles(directory, pattern, SearchOption.TopDirectoryOnly)); private readonly Func> _enumerateDirectories = - directory => Directory.EnumerateDirectories(directory); - - /// - /// Initializes a new instance of the class with filesystem test seams. - /// - /// The logger. - /// The manifest cache. - /// The top-level file enumerator. - /// The top-level directory enumerator. - internal ManifestDiscoveryService( - ILogger logger, - IManifestCache manifestCache, - Func> enumerateFiles, - Func> enumerateDirectories) - : this(logger, manifestCache) - { - _enumerateFiles = enumerateFiles; - _enumerateDirectories = enumerateDirectories; - } + enumerateDirectories ?? Directory.EnumerateDirectories; /// /// Gets manifests by content type. @@ -127,17 +122,12 @@ public async Task InitializeCacheAsync(CancellationToken cancellationToken = def // First discover embedded manifests await DiscoverEmbeddedManifestsAsync(cancellationToken); - // Then discover from local filesystem locations - var localManifestDir = Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), - AppConstants.AppName, - FileTypes.ManifestsDirectory); - - // Also check for custom manifest directories - var customManifestDir = Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), - AppConstants.AppName, - "CustomManifests"); + // Then discover from local filesystem locations. + // Routed through the configuration provider so a user-relocated data directory + // is honoured; a raw SpecialFolder lookup would keep reading the default tree. + var applicationDataPath = configurationProvider.GetApplicationDataPath(); + var localManifestDir = Path.Combine(applicationDataPath, FileTypes.ManifestsDirectory); + var customManifestDir = Path.Combine(applicationDataPath, "CustomManifests"); await DiscoverFileSystemManifestsAsync([localManifestDir, customManifestDir], cancellationToken); @@ -203,12 +193,34 @@ private static bool IsVersionCompatible(string actualVersion, string minVersion, return true; } - private static async Task LoadManifestAsync(string manifestPath, CancellationToken cancellationToken) + /// + /// Applies the variant ingestion gate, logging and rejecting when it does not pass. + /// + /// The deserialized manifest. + /// Where it came from, named in the rejection log. + /// true when the manifest may be ingested; otherwise false. + private bool IsManifestAccepted(ContentManifest manifest, string source) + { + if (ManifestIngestionGate.TryAccept(manifest, out var rejectionReason)) + { + return true; + } + + logger.LogWarning("Skipping manifest from {Source}: {Reason}", source, rejectionReason); + return false; + } + + private async Task LoadManifestAsync(string manifestPath, CancellationToken cancellationToken) { await using var stream = File.OpenRead(manifestPath); var manifest = await JsonSerializer.DeserializeAsync(stream, JsonOptions, cancellationToken); if (manifest != null && !string.IsNullOrEmpty(manifest.Id)) { + if (!IsManifestAccepted(manifest, manifestPath)) + { + return null; + } + return manifest; } @@ -289,6 +301,11 @@ private async Task DiscoverFileSystemManifestsAsync(IEnumerable searchDi var manifest = await JsonSerializer.DeserializeAsync(stream, JsonOptions, cancellationToken); if (manifest != null && !string.IsNullOrEmpty(manifest.Id)) { + if (!IsManifestAccepted(manifest, manifestFile)) + { + continue; + } + manifestCache.AddOrUpdateManifest(manifest); logger.LogDebug("Discovered file system manifest: {ManifestId}", manifest.Id); } @@ -318,6 +335,11 @@ private async Task DiscoverEmbeddedManifestsAsync(CancellationToken cancellation var manifest = await JsonSerializer.DeserializeAsync(stream, JsonOptions, cancellationToken); if (manifest != null && !string.IsNullOrEmpty(manifest.Id)) { + if (!IsManifestAccepted(manifest, resourceName)) + { + continue; + } + manifestCache.AddOrUpdateManifest(manifest); logger.LogDebug("Discovered embedded manifest: {ManifestId}", manifest.Id); } diff --git a/GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs b/GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs index 900b14869..74b1ec9cd 100644 --- a/GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs +++ b/GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs @@ -1,10 +1,3 @@ -using System; -using System.Globalization; -using System.IO; -using System.Linq; -using System.Reflection; -using System.Text.Json; -using System.Threading.Tasks; using CsvHelper; using CsvHelper.Configuration; using GenHub.Core.Constants; @@ -13,8 +6,16 @@ using GenHub.Core.Interfaces.Tools; using GenHub.Core.Models.Enums; using GenHub.Core.Models.Manifest; +using GenHub.Core.Utilities; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using System; +using System.Globalization; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Text.Json; +using System.Threading.Tasks; namespace GenHub.Features.Manifest; @@ -944,8 +945,11 @@ private async Task AddFilesFromCsvAsync(IContentManifestBuilder builder, s fullPath = ResolveSourcePathWithBackup(fullPath, finalPath); - var isExecutable = finalPath.EndsWith(".exe", StringComparison.OrdinalIgnoreCase) || - finalPath.EndsWith(".dat", StringComparison.OrdinalIgnoreCase); + // .dat is data, not code. It was previously marked executable because + // the Steam layout launches game.dat through a proxy, which is a launch + // strategy rather than a property of the file, and it forced + // SteamManifestPatcher to keep flipping the flag by hand. + var isExecutable = ExecutableFileClassifier.RequiresExecutePermission(finalPath); await builder.AddGameInstallationFileAsync(finalPath, fullPath, isExecutable); _fileCount++; diff --git a/GenHub/GenHub/Features/Manifest/ManifestProvider.cs b/GenHub/GenHub/Features/Manifest/ManifestProvider.cs index 9101d32cf..94bedd49a 100644 --- a/GenHub/GenHub/Features/Manifest/ManifestProvider.cs +++ b/GenHub/GenHub/Features/Manifest/ManifestProvider.cs @@ -74,6 +74,8 @@ public class ManifestProvider(ILogger logger, IContentManifest // Validate security of parsed manifest ValidateManifestSecurity(manifest); + EnsureManifestAccepted(manifest, gameClient.Id); + // Ensure manifest ID matches the requested id if (!string.Equals(manifest.Id.Value, gameClient.Id, StringComparison.OrdinalIgnoreCase)) { @@ -136,6 +138,7 @@ public class ManifestProvider(ILogger logger, IContentManifest // Validate ID before adding to pool ManifestIdValidator.EnsureValid(generated.Id.Value); + EnsureManifestAccepted(generated, gameClient.Id); // Determine a sensible source directory for the generated manifest. // Prefer the working directory if present, otherwise fall back to the directory @@ -205,10 +208,15 @@ public class ManifestProvider(ILogger logger, IContentManifest var manifest = await JsonSerializer.DeserializeAsync(stream, _jsonOptions, cancellationToken); if (manifest != null) { + ValidateCachedManifest(manifest, deterministicId); + // For embedded installation manifests, provide the installation path as source when available. var addRes = await manifestPool.AddManifestAsync(manifest, installation.InstallationPath ?? string.Empty, null, cancellationToken); if (addRes?.Success != true) + { logger.LogWarning("Failed to add embedded installation manifest {Id} to pool: {Errors}", manifest.Id, string.Join(", ", addRes?.Errors ?? [])); + } + return manifest; } } @@ -262,6 +270,7 @@ public class ManifestProvider(ILogger logger, IContentManifest // Validate ID before adding to pool ManifestIdValidator.EnsureValid(generated.Id.Value); + EnsureManifestAccepted(generated, deterministicId); var addRes2 = await manifestPool.AddManifestAsync(generated, sourcePath ?? string.Empty, null, cancellationToken); if (addRes2?.Success != true) { @@ -294,10 +303,19 @@ private static void ValidateCachedManifest(ContentManifest manifest, string expe { // Run the same security validations as for embedded manifests ValidateManifestSecurity(manifest); + EnsureManifestAccepted(manifest, expectedId); if (!string.Equals(manifest.Id.Value, expectedId, StringComparison.OrdinalIgnoreCase)) { throw new ManifestValidationException(expectedId, $"Manifest ID mismatch: expected '{expectedId}' but manifest contains '{manifest.Id.Value}'"); } } + + private static void EnsureManifestAccepted(ContentManifest manifest, string requestedId) + { + if (!ManifestIngestionGate.TryAccept(manifest, out var rejectionReason)) + { + throw new ManifestValidationException(requestedId, rejectionReason!); + } + } } diff --git a/GenHub/GenHub/Features/Manifest/SteamManifestPatcher.cs b/GenHub/GenHub/Features/Manifest/SteamManifestPatcher.cs index 68c679ae0..1e46fbb41 100644 --- a/GenHub/GenHub/Features/Manifest/SteamManifestPatcher.cs +++ b/GenHub/GenHub/Features/Manifest/SteamManifestPatcher.cs @@ -4,6 +4,7 @@ using System.Text.Json; using System.Threading.Tasks; using GenHub.Core.Constants; +using GenHub.Core.Interfaces.Common; using GenHub.Core.Interfaces.Steam; using GenHub.Core.Models.Manifest; using Microsoft.Extensions.Logging; @@ -13,7 +14,9 @@ namespace GenHub.Features.Manifest; /// /// Implementation of . /// -public class SteamManifestPatcher(ILogger logger) : ISteamManifestPatcher +public class SteamManifestPatcher( + ILogger logger, + IConfigurationProviderService configurationProvider) : ISteamManifestPatcher { private static readonly JsonSerializerOptions JsonOptions = new() { WriteIndented = true, PropertyNameCaseInsensitive = true }; @@ -25,10 +28,7 @@ public async Task PatchManifestAsync(string manifestId, bool useSteamLaunch) logger.LogInformation("Patching manifest {ManifestId} for Steam launch: {UseSteamLaunch}", manifestId, useSteamLaunch); // Locate the manifest file - var manifestsDir = Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), - AppConstants.AppName, - FileTypes.ManifestsDirectory); + var manifestsDir = configurationProvider.GetManifestsPath(); if (!Directory.Exists(manifestsDir)) { diff --git a/GenHub/GenHub/Features/Workspace/FileOperationsService.cs b/GenHub/GenHub/Features/Workspace/FileOperationsService.cs index 841b0e37e..cdb2d8275 100644 --- a/GenHub/GenHub/Features/Workspace/FileOperationsService.cs +++ b/GenHub/GenHub/Features/Workspace/FileOperationsService.cs @@ -377,18 +377,18 @@ public async Task CreateHardLinkAsync( await Task.Run( () => { - if (OperatingSystem.IsWindows()) - { - // Use platform-specific implementation - throw new NotImplementedException("Hard link creation should be handled by platform-specific service"); - } - else - { - File.Copy(targetPath, linkPath, true); - logger.LogWarning( - "Hard links not supported on this platform, fell back to copy for {Link}", - linkPath); - } + // Every supported platform has a decorator that overrides this: + // WindowsFileOperationsService and UnixFileOperationsService. Reaching + // here means the host did not register one. + // + // This used to File.Copy on non-Windows and log a warning. That made a + // missing registration invisible: workspaces still built, tests still + // passed, and every profile silently consumed a full copy of the game + // instead of a link. Failing is the only way that surfaces. + throw new NotSupportedException( + "Hard link creation must be handled by a platform-specific IFileOperationsService. " + + "Register WindowsFileOperationsService or UnixFileOperationsService in the host's " + + "service module."); }, cancellationToken); diff --git a/GenHub/GenHub/Features/Workspace/Strategies/WorkspaceStrategyBase.cs b/GenHub/GenHub/Features/Workspace/Strategies/WorkspaceStrategyBase.cs index 4300ef984..64dd361cb 100644 --- a/GenHub/GenHub/Features/Workspace/Strategies/WorkspaceStrategyBase.cs +++ b/GenHub/GenHub/Features/Workspace/Strategies/WorkspaceStrategyBase.cs @@ -258,42 +258,32 @@ protected void UpdateWorkspaceInfo( if (gameClientManifest != null) { - var executableFile = gameClientManifest.Files? - .FirstOrDefault(f => f.IsExecutable); - - if (executableFile != null) + // Resolution order and failure behaviour live in ManifestVariantResolver. + // The previous inline logic took the first file marked IsExecutable, which is + // enumeration-order dependent as soon as more than one file qualifies — and + // several do, once dynamic libraries and native extensionless binaries are in + // the same manifest. + var resolution = ManifestVariantResolver.ResolveEntryPoint(gameClientManifest); + + if (resolution.Success) { - // Use the full relative path from the manifest workspaceInfo.ExecutablePath = Path.Combine( workspaceInfo.WorkspacePath, - executableFile.RelativePath.Replace('/', Path.DirectorySeparatorChar)); + resolution.RelativePath!.Replace('/', Path.DirectorySeparatorChar)); logger.LogInformation( - "Executable resolved from GameClient manifest: {ExecutablePath} (marked as IsExecutable)", - workspaceInfo.ExecutablePath); + "Executable resolved from GameClient manifest: {ExecutablePath} ({Reason})", + workspaceInfo.ExecutablePath, + resolution.Reason); } else { - // Fallback: Try finding any .exe file - executableFile = gameClientManifest.Files? - .FirstOrDefault(f => f.RelativePath.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)); - - if (executableFile != null) - { - workspaceInfo.ExecutablePath = Path.Combine( - workspaceInfo.WorkspacePath, - executableFile.RelativePath.Replace('/', Path.DirectorySeparatorChar)); - - logger.LogWarning( - "Executable resolved from GameClient manifest by .exe extension (IsExecutable not set): {ExecutablePath}", - workspaceInfo.ExecutablePath); - } - else - { - logger.LogWarning( - "GameClient manifest '{ManifestId}' does not contain an executable file", - gameClientManifest.Id); - } + // Left unset deliberately rather than guessed. Launching the wrong binary + // fails somewhere far less diagnosable than here. + logger.LogWarning( + "Could not determine the executable for GameClient manifest '{ManifestId}': {Resolution}", + gameClientManifest.Id, + resolution); } } else if (!string.IsNullOrEmpty(configuration.GameClient.ExecutablePath)) @@ -510,6 +500,8 @@ protected virtual async Task ProcessManifestFileAsync(ManifestFile file, Content default: throw new NotSupportedException($"Unsupported content source type: {file.SourceType}"); } + + await EnsureExecutableAsync(file, targetPath, cancellationToken); } /// @@ -614,6 +606,95 @@ protected virtual async Task ProcessExtractedPackageFileAsync(ManifestFile file, logger.LogDebug("Copied extracted file: {Source} -> {Target}", file.SourcePath, targetPath); } + /// + /// Gives a materialised file the Unix execute bit, on a copy that the workspace owns. + /// + /// The copy is the point. Under the hard-link strategy the workspace file is + /// the content-store blob — same inode, and file mode lives in the inode, not the + /// directory entry. Calling chmod on it would change permissions for every other + /// profile referencing that hash, and the content store keys purely on content hash, + /// so it has no way to represent two files with identical bytes and different modes. + /// + /// + /// Breaking the link costs one copy per executable. Manifests contain a handful of + /// those and gigabytes of data, so the deduplication that matters is untouched. + /// + /// + /// No-op on Windows, which has no execute bit, and for files that do not need one. + /// + /// + /// The manifest entry that was just materialised. + /// Its absolute path in the workspace. + /// A cancellation token. + /// A task representing the operation. + protected async Task EnsureExecutableAsync(ManifestFile file, string targetPath, CancellationToken cancellationToken) + { + if (OperatingSystem.IsWindows() || !file.IsExecutable || !File.Exists(targetPath)) + { + return; + } + + var temporaryPath = targetPath + ".genhub-exec-tmp"; + + try + { + // Replace the entry with a private copy so the mode change cannot reach a + // shared blob. + // + // The copy is made executable *before* it is moved into place, and the move + // replaces the destination atomically. There is therefore no observable state + // in which the destination is missing or present-but-not-executable: it is + // either the original entry or the finished private copy. + // + // A delete-then-move sequence would expose both of those states, and the + // second is unrecoverable — verification never mutates, so a workspace left + // with a non-executable entry point stays broken for every later launch. + await Task.Run( + () => + { + File.Copy(targetPath, temporaryPath, overwrite: true); + + if (!OperatingSystem.IsWindows()) + { + const UnixFileMode executableMode = + UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute | + UnixFileMode.GroupRead | UnixFileMode.GroupExecute | + UnixFileMode.OtherRead | UnixFileMode.OtherExecute; + + File.SetUnixFileMode(temporaryPath, executableMode); + } + + File.Move(temporaryPath, targetPath, overwrite: true); + }, + cancellationToken); + + Logger.LogDebug("Marked {RelativePath} executable on a workspace-owned copy", file.RelativePath); + } + catch (Exception ex) + { + // The move is the last step, so a failure here means the temporary copy may + // still exist. Remove it: the original entry is untouched and correct, and a + // stray .genhub-exec-tmp would otherwise be reported by workspace validation. + try + { + File.Delete(temporaryPath); + } + catch (Exception cleanupEx) + { + Logger.LogDebug( + cleanupEx, + "Could not remove the temporary executable copy at {TemporaryPath}", + temporaryPath); + } + + Logger.LogError( + ex, + "Could not mark {RelativePath} executable", + file.RelativePath); + throw; + } + } + /// /// Strips a leading directory name from a path if present. /// Handles both forward and back slashes. diff --git a/GenHub/GenHub/Features/Workspace/UnixFileOperationsService.cs b/GenHub/GenHub/Features/Workspace/UnixFileOperationsService.cs new file mode 100644 index 000000000..7a096d8ec --- /dev/null +++ b/GenHub/GenHub/Features/Workspace/UnixFileOperationsService.cs @@ -0,0 +1,184 @@ +using System; +using System.IO; +using System.Runtime.InteropServices; +using System.Threading; +using System.Threading.Tasks; +using GenHub.Core.Interfaces.Storage; +using GenHub.Core.Interfaces.Workspace; +using GenHub.Core.Models.Common; +using GenHub.Core.Models.Enums; +using Microsoft.Extensions.Logging; + +namespace GenHub.Features.Workspace; + +/// +/// Hard-link support for Linux and macOS, decorating . +/// +/// Without this, CreateHardLinkAsync on any Unix platform fell through to +/// File.Copy and logged a warning. The default workspace strategy is +/// HardLink, and the two symlink strategies are downgraded to it when the +/// process is not elevated, so in practice every workspace on Linux full-copied the +/// game — roughly 1.5 GB per profile — while appearing to work. +/// +/// +/// Registered by both the Linux and macOS hosts. It lives in the shared project rather +/// than being duplicated per host because link(2) is identical on both; see +/// for why the interop stops there. +/// +/// +/// The shared implementation everything else delegates to. +/// Content-addressable store, used to resolve hashes to paths. +/// Logger. +public class UnixFileOperationsService( + FileOperationsService baseService, + ICasService casService, + ILogger logger) : IFileOperationsService +{ + /// + public Task CopyFileAsync(string sourcePath, string destinationPath, CancellationToken cancellationToken = default) + => baseService.CopyFileAsync(sourcePath, destinationPath, cancellationToken); + + /// + public Task CreateSymlinkAsync(string linkPath, string targetPath, bool allowFallback = true, CancellationToken cancellationToken = default) + => baseService.CreateSymlinkAsync(linkPath, targetPath, allowFallback, cancellationToken); + + /// + public Task VerifyFileHashAsync(string filePath, string expectedHash, CancellationToken cancellationToken = default) + => baseService.VerifyFileHashAsync(filePath, expectedHash, cancellationToken); + + /// + public Task DownloadFileAsync(Uri url, string destinationPath, IProgress? progress = null, CancellationToken cancellationToken = default) + => baseService.DownloadFileAsync(url, destinationPath, progress, cancellationToken); + + /// + public Task ApplyPatchAsync(string targetPath, string patchPath, CancellationToken cancellationToken = default) + => baseService.ApplyPatchAsync(targetPath, patchPath, cancellationToken); + + /// + public Task StoreInCasAsync(string sourcePath, string? expectedHash = null, CancellationToken cancellationToken = default) + => baseService.StoreInCasAsync(sourcePath, expectedHash, cancellationToken); + + /// + public Task OpenCasContentAsync(string hash, CancellationToken cancellationToken = default) + => baseService.OpenCasContentAsync(hash, cancellationToken); + + /// + public async Task CopyFromCasAsync(string hash, string destinationPath, ContentType? contentType = null, CancellationToken cancellationToken = default) + { + var casPath = await ResolveCasPathAsync(hash, contentType, cancellationToken).ConfigureAwait(false); + if (casPath is null) + { + return false; + } + + await baseService.CopyFileAsync(casPath, destinationPath, cancellationToken).ConfigureAwait(false); + return true; + } + + /// + /// + /// No volume preflight. The Windows implementation checks AreSameVolume first, + /// but that helper compares Path.GetPathRoot, which is / for every path + /// on Unix and so always reports "same volume". Attempting the link and handling + /// EXDEV is both correct and free of the race a preflight introduces. + /// + public async Task LinkFromCasAsync( + string hash, + string destinationPath, + bool useHardLink = false, + ContentType? contentType = null, + CancellationToken cancellationToken = default) + { + var casPath = await ResolveCasPathAsync(hash, contentType, cancellationToken).ConfigureAwait(false); + if (casPath is null) + { + return false; + } + + if (!useHardLink) + { + await baseService.CreateSymlinkAsync(destinationPath, casPath, true, cancellationToken).ConfigureAwait(false); + return true; + } + + try + { + await CreateHardLinkAsync(destinationPath, casPath, cancellationToken).ConfigureAwait(false); + return true; + } + catch (IOException ex) when (ex is not FileNotFoundException) + { + // Cross-device or a filesystem without link support. Copying keeps the + // workspace usable; the caller loses deduplication, not correctness. + logger.LogWarning( + ex, + "Hard link from CAS failed for {Destination}; falling back to a copy", + destinationPath); + + await baseService.CopyFileAsync(casPath, destinationPath, cancellationToken).ConfigureAwait(false); + return true; + } + } + + /// + public async Task CreateHardLinkAsync( + string linkPath, + string targetPath, + CancellationToken cancellationToken = default) + { + var absoluteLinkPath = Path.GetFullPath(linkPath); + var absoluteTargetPath = Path.GetFullPath(targetPath); + + FileOperationsService.EnsureDirectoryExists(absoluteLinkPath); + FileOperationsService.DeleteFileIfExists(absoluteLinkPath); + + await Task.Run( + () => + { + if (UnixNativeMethods.Link(absoluteTargetPath, absoluteLinkPath) == 0) + { + return; + } + + // Interpret errno rather than preflighting. A preflight volume check is + // racy, and would need a shared struct stat layout that Linux and macOS + // do not agree on. Attempting the call is both simpler and accurate. + var errno = Marshal.GetLastPInvokeError(); + + throw errno switch + { + UnixNativeMethods.EXDEV => new IOException( + $"Cannot hard link '{absoluteLinkPath}' to '{absoluteTargetPath}': they are on different " + + "filesystems. Move the content store onto the same volume as the workspace, or choose " + + "the FullCopy workspace strategy."), + UnixNativeMethods.EPERM => new IOException( + $"Cannot hard link '{absoluteLinkPath}': the filesystem does not support hard links."), + UnixNativeMethods.ENOENT => new FileNotFoundException( + $"Cannot hard link '{absoluteLinkPath}': the target '{absoluteTargetPath}' does not exist.", + absoluteTargetPath), + UnixNativeMethods.EACCES => new UnauthorizedAccessException( + $"Cannot hard link '{absoluteLinkPath}' to '{absoluteTargetPath}': permission denied."), + _ => new IOException( + $"Failed to hard link '{absoluteLinkPath}' to '{absoluteTargetPath}' (errno {errno})."), + }; + }, + cancellationToken).ConfigureAwait(false); + + logger.LogDebug("Created hard link from {Link} to {Target}", absoluteLinkPath, absoluteTargetPath); + } + + private async Task ResolveCasPathAsync(string hash, ContentType? contentType, CancellationToken cancellationToken) + { + var pathResult = contentType.HasValue + ? await casService.GetContentPathAsync(hash, contentType.Value, cancellationToken).ConfigureAwait(false) + : await casService.GetContentPathAsync(hash, cancellationToken).ConfigureAwait(false); + + if (!pathResult.Success || pathResult.Data is null) + { + logger.LogError("CAS content not found for hash {Hash}: {Error}", hash, pathResult.FirstError); + return null; + } + + return pathResult.Data; + } +} diff --git a/GenHub/GenHub/Features/Workspace/UnixNativeMethods.cs b/GenHub/GenHub/Features/Workspace/UnixNativeMethods.cs new file mode 100644 index 000000000..7ca861317 --- /dev/null +++ b/GenHub/GenHub/Features/Workspace/UnixNativeMethods.cs @@ -0,0 +1,78 @@ +using System; +using System.Runtime.InteropServices; + +namespace GenHub.Features.Workspace; + +/// +/// The libc calls GenHub needs on Linux and macOS. +/// +/// Deliberately tiny. Only functions whose signatures are identical across both +/// platforms appear here — link, faccessat and geteuid take and +/// return scalars and C strings, so a single declaration is correct on both. +/// +/// +/// stat and friends are deliberately absent. Their struct stat layouts +/// differ between Linux and macOS (and between architectures), so a shared P/Invoke +/// declaration would silently read the wrong offsets. Where volume identity or file +/// metadata is needed, use the BCL (File.GetUnixFileMode, +/// FileInfo) or attempt the operation and interpret errno. +/// +/// +internal static partial class UnixNativeMethods +{ + /// Operation not permitted on a cross-device link. + internal const int EXDEV = 18; + + /// The destination path already exists. + internal const int EEXIST = 17; + + /// A component of the path does not exist. + internal const int ENOENT = 2; + + /// Permission denied. + internal const int EACCES = 13; + + /// The filesystem does not support hard links. + internal const int EPERM = 1; + + /// + /// Creates a hard link, POSIX link(2). + /// + /// .NET has no managed equivalent: File.CreateSymbolicLink exists but there + /// is no hard-link API, which is why this interop is needed at all. + /// + /// + /// Path to the existing file. + /// Path of the link to create. + /// 0 on success, -1 on failure with errno set. + [LibraryImport("libc", EntryPoint = "link", SetLastError = true, StringMarshalling = StringMarshalling.Utf8)] + internal static partial int Link(string existingPath, string newPath); + + /// + /// Checks whether the effective process identity may execute a file. + /// + /// The file to inspect. + /// true when the current effective identity may execute the file. + internal static bool CanExecute(string path) + { + const int executeMode = 1; + var currentWorkingDirectory = OperatingSystem.IsMacOS() ? -2 : -100; + var effectiveIdentity = OperatingSystem.IsMacOS() ? 0x10 : 0x200; + + return FileAccessAt(currentWorkingDirectory, path, executeMode, effectiveIdentity) == 0; + } + + /// + /// Returns the effective user ID, POSIX geteuid(2). + /// + /// Used instead of comparing Environment.UserName to the literal "root", + /// which is wrong under sudo -E and for any uid-0 account named otherwise. + /// + /// + /// The effective user ID; 0 is root. + [LibraryImport("libc", EntryPoint = "geteuid")] + internal static partial uint GetEffectiveUserId(); + + [LibraryImport("libc", EntryPoint = "faccessat", SetLastError = true, StringMarshalling = StringMarshalling.Utf8)] + private static partial int FileAccessAt(int directoryFileDescriptor, string path, int mode, int flags); +} diff --git a/GenHub/GenHub/Features/Workspace/UnixSymlinkCapabilityProvider.cs b/GenHub/GenHub/Features/Workspace/UnixSymlinkCapabilityProvider.cs new file mode 100644 index 000000000..1e0145712 --- /dev/null +++ b/GenHub/GenHub/Features/Workspace/UnixSymlinkCapabilityProvider.cs @@ -0,0 +1,12 @@ +using GenHub.Core.Interfaces.Workspace; + +namespace GenHub.Features.Workspace; + +/// +/// Symlink capability on Linux and macOS, where symlink(2) requires no privilege. +/// +public sealed class UnixSymlinkCapabilityProvider : ISymlinkCapabilityProvider +{ + /// + public bool CanCreateSymlinks => true; +} diff --git a/GenHub/GenHub/Features/Workspace/WorkspaceValidator.cs b/GenHub/GenHub/Features/Workspace/WorkspaceValidator.cs index fbdcef691..2b84a16b6 100644 --- a/GenHub/GenHub/Features/Workspace/WorkspaceValidator.cs +++ b/GenHub/GenHub/Features/Workspace/WorkspaceValidator.cs @@ -255,15 +255,13 @@ public async Task> ValidateWorkspaceAsync(Work } else { - // Properly check execute permission using Unix stat - // TODO: Make this a platform specific validation if (!HasUnixExecutePermission(executablePath)) { issues.Add(new ValidationIssue { IssueType = ValidationIssueType.AccessDenied, Severity = ValidationSeverity.Warning, - Message = $"File is not marked as executable: {executablePath}", + Message = $"File is not executable by the current process: {executablePath}", Path = executablePath, }); } @@ -329,8 +327,10 @@ private static bool IsRunningAsAdministrator() { if (!OperatingSystem.IsWindows()) { - // On Unix systems, check if running as root - return Environment.UserName == "root"; + // geteuid rather than comparing Environment.UserName to the literal "root", + // which is wrong under `sudo -E` (USER stays the invoking account) and for + // any uid-0 account not named root. + return UnixNativeMethods.GetEffectiveUserId() == 0; } try @@ -345,36 +345,24 @@ private static bool IsRunningAsAdministrator() } } - // Add this helper method to check Unix execute permission + /// + /// Determines whether the effective process identity may execute a Unix file. + /// + /// Uses faccessat(AT_EACCESS) rather than merely checking whether any execute + /// bit is present. The kernel therefore evaluates ownership, group membership and + /// access-control rules for the identity that will actually launch the process. + /// + /// + /// The file to inspect. + /// true when the file is executable by the current user. private static bool HasUnixExecutePermission(string filePath) { - try + if (OperatingSystem.IsWindows()) { - if (OperatingSystem.IsWindows()) - return true; - - var psi = new System.Diagnostics.ProcessStartInfo - { - FileName = "/bin/sh", - Arguments = $"-c \"[ -x '{filePath.Replace("'", "'\\''")}' ]\"", - RedirectStandardOutput = true, - RedirectStandardError = true, - UseShellExecute = false, - CreateNoWindow = true, - }; - using var proc = System.Diagnostics.Process.Start(psi); - - if (proc == null) - return false; - - proc.WaitForExit(); - return proc.ExitCode == 0; - } - catch - { - // If all checks fail, assume not executable - return false; + return true; } + + return UnixNativeMethods.CanExecute(filePath); } private async Task ValidateSymlinksAsync(string workspacePath, List issues, CancellationToken cancellationToken) @@ -418,4 +406,4 @@ private async Task ValidateSymlinksAsync(string workspacePath, List(); - // Register provider definition loader for data-driven provider configuration - services.AddSingleton(); + // Register provider definition loader for data-driven provider configuration. + // The user-providers directory is passed in from the configuration provider so a + // relocated application data directory is honoured. ProviderDefinitionLoader lives + // in GenHub.Core and defaults to a raw SpecialFolder.ApplicationData lookup when no + // override is supplied, which would silently keep reading the default tree. + services.AddSingleton(sp => + { + var configurationProvider = sp.GetRequiredService(); + return new ProviderDefinitionLoader( + sp.GetRequiredService>(), + userProvidersDirectory: Path.Combine( + configurationProvider.GetApplicationDataPath(), + ProviderDefinitionLoader.ProvidersDirectoryName)); + }); // Register catalog parser factory and parsers services.AddSingleton();