diff --git a/README.md b/README.md index 5b97273c..07361a86 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,7 @@ This ReadMe covers the following: * [Service creation and authentication](#service-creation-and-authentication) * [Get Package Uploader](#get-package-uploader) * [Run Package Uploader](#run-package-uploader) +* [MSIXVC2 packages](#msixvc2-packages) * [Putting it all together](#putting-it-all-together) * [Example GetProduct operation](#example-getproduct-operation) * [Example UploadXvcPackage operation](#example-uploadxvcpackage-operation) @@ -176,13 +177,70 @@ The following table has important arguments for running Package Uploader. | **[GetProduct](https://github.com/microsoft/PackageUploader/blob/main/Operations.md#GetProduct)** | Gets metadata for the product. This is useful for getting the productId, BigId, and product name that's used in all configuration files. This also gets a list of the BranchFriendlyNames and FlightNames of the product. | | **[GetPackages](https://github.com/microsoft/PackageUploader/blob/main/Operations.md#GetPackages)** | Gets a list of the packages in a branch or flight. | | **[UploadUwpPackage](https://github.com/microsoft/PackageUploader/blob/main/Operations.md#UploadUwpPackage)** | Uploads a UWP game package. | -| **[UploadXvcPackage](https://github.com/microsoft/PackageUploader/blob/main/Operations.md#UploadXvcPackage)** | Uploads an XVC game package and assets, including EKB, SubVal, layout, and SODB files. | +| **[UploadXvcPackage](https://github.com/microsoft/PackageUploader/blob/main/Operations.md#UploadXvcPackage)** | Uploads an XVC game package and assets, including EKB, SubVal, layout, and SODB files. MSIXVC2 packages are detected automatically and uploaded through MakePkg.exe — see [MSIXVC2 packages](#msixvc2-packages). | | **[RemovePackages](https://github.com/microsoft/PackageUploader/blob/main/Operations.md#RemovePackages)** | Removes game packages and assets from a branch. We recommend keeping only your 10 most recent packages to ensure optimal performance. | | **[ImportPackages](https://github.com/microsoft/PackageUploader/blob/main/Operations.md#ImportPackages)** | Imports all game packages from a branch to a destination branch. Use this operation to copy your previously uploaded and published packages from one branch to another. | | **[PublishPackages](https://github.com/microsoft/PackageUploader/blob/main/Operations.md#PublishPackages)** | Publishes all game packages from a branch or flight to a destination sandbox or flight. You can set specific availability times in the configuration file. | For more information about operation parameters, see [Operations](https://github.com/microsoft/PackageUploader/blob/main/Operations.md). +### MSIXVC2 packages + +`UploadXvcPackage` detects the package format from the file you point `packageFilePath` at. When the package is an +MSIXVC2 package, PackageUploader delegates the upload to the MSIXVC2-capable `MakePkg.exe` that ships with the Microsoft +GDK, because MakePkg.exe owns the MSIXVC2 upload protocol. There is no separate operation name and no new configuration +switch — an XVC1/MSIXVC1 package continues to be uploaded by PackageUploader itself, exactly as before. + +The following configuration options behave differently on the MSIXVC2 path: + +| Option | Behavior | +| --- | --- | +| `gameAssets` | Not required. MakePkg.exe picks the assets up from the folder that contains the package. If the paths you supply resolve to that same folder they are ignored with a warning; if they point elsewhere the operation fails so an asset is never silently dropped. | +| `minutesToWaitForProcessing` | Ignored with a warning. MakePkg.exe manages its own processing wait. | +| `deltaUpload` | Ignored with a warning. | +| `availabilityDate` / `preDownloadDate` | Supported, and applied the same way as for XVC1. MakePkg.exe does not set the dates itself, but it does report the identity of the package it created, so PackageUploader applies them after the upload completes. The reported package is looked up in the target branch and market group before anything is written, and if it cannot be found the operation fails rather than dating a different package. | +| `productId` | Supported. It is resolved to the corresponding Big ID, which is what MakePkg.exe requires. | +| Authentication | Supported, including non-interactive/CI authentication — but see the credential-exposure warning in [Authentication on the MSIXVC2 path](#authentication-on-the-msixvc2-path). | + +If no MSIXVC2-capable `MakePkg.exe` is available, the operation fails with an actionable error instead of attempting an +upload that cannot succeed. + +> [!NOTE] +> Because `availabilityDate`/`preDownloadDate` are applied *after* MakePkg.exe finishes, a failure at that stage does not +> undo the upload. The error says so explicitly: the package is uploaded, only the dates were not set. + +#### Authentication on the MSIXVC2 path + +MakePkg.exe acquires its own token, so PackageUploader forwards the identity you configured rather than forcing an +interactive sign-in. `--Authentication`, `--TenantId`, and the client id / secret / certificate values from your +configuration file are passed through, so an unattended pipeline using a service principal keeps working. + +> [!WARNING] +> **Avoid secret-bearing authentication methods on shared machines.** Because the upload is delegated to a separate +> process, a client secret has to be handed to MakePkg.exe on its command line, where it is visible to anything that can +> read the process table for as long as the upload runs. MakePkg.exe offers no way to pass a credential out of band. +> PackageUploader redacts the secret from its own logs, but that does not protect the command line itself. +> +> On shared build agents, prefer an authentication method that puts no credential on the command line: +> **`Environment`**, **`AzurePipelines`**, **`ManagedIdentity`**, or **`ManagedIdentityFederated`**. These are forwarded +> as just `/auth `, and MakePkg.exe obtains the token from the ambient environment itself. Only use +> `AppSecret`/`ClientSecret` where you control the machine and trust every process on it. + +| `--Authentication` | Forwarded to MakePkg.exe as | +| --- | --- | +| `AppSecret` | `ClientSecret` (PackageUploader's legacy name for the same AAD application secret flow) | +| `AppCert` | `ClientCertificate` | +| `Default`, `Browser`, `CacheableBrowser`, `AzureCli`, `ManagedIdentity`, `ManagedIdentityFederated`, `Environment`, `AzurePipelines`, `ClientSecret`, `ClientCertificate` | The same value | + +Two limitations, both because MakePkg.exe has no corresponding option: + +- **Certificates must live in a Windows certificate store.** MakePkg.exe selects a certificate by thumbprint + (`/certthumbprint`, `/certstore`, `/certlocation`) and has no option naming a certificate file, so a PFX path + (`ClientCertificateAuthInfo:CertificatePath`) is rejected with an explicit error. Import the certificate into a store + and use `AppCert` with `AadAuthInfo:CertificateThumbprint`. +- **Certificates cannot be selected by subject.** `AadAuthInfo:CertificateSubject` is rejected; use + `AadAuthInfo:CertificateThumbprint`. + ### Available parameters | Parameter | Description | diff --git a/src/PackageUploader.Application.Test/Config/UploadXvcPackageOperationConfigTest.cs b/src/PackageUploader.Application.Test/Config/UploadXvcPackageOperationConfigTest.cs index 0665119d..c3a74eca 100644 --- a/src/PackageUploader.Application.Test/Config/UploadXvcPackageOperationConfigTest.cs +++ b/src/PackageUploader.Application.Test/Config/UploadXvcPackageOperationConfigTest.cs @@ -83,4 +83,39 @@ public void Validate_PreDownloadDateBeforeAvailabilityDate_NoPreDownloadError() Assert.DoesNotContain(r => r.MemberNames.Contains("PreDownloadDate"), results); } + + [TestMethod] + public void Validate_NonMsixvc2PackageWithoutGameAssets_ReturnsError() + { + var config = new TestUploadXvcPackageOperationConfig + { + OperationName = "UploadXvcPackage", + ProductId = "product-123", + BranchFriendlyName = "main", + PackageFilePath = "test.msixvc", + GameAssets = null, + }; + + var results = ConfigTestHelper.ValidateConfig(config); + + Assert.Contains(r => r.MemberNames.Contains("GameAssets"), results); + } + + [TestMethod] + public void Validate_Msixvc2PackageWithoutGameAssets_NoGameAssetsError() + { + using var package = Test.Tools.TempPackageFile.CreateMsixvc2(); + var config = new TestUploadXvcPackageOperationConfig + { + OperationName = "UploadXvcPackage", + ProductId = "product-123", + BranchFriendlyName = "main", + PackageFilePath = package.Path, + GameAssets = null, + }; + + var results = ConfigTestHelper.ValidateConfig(config); + + Assert.DoesNotContain(r => r.MemberNames.Contains("GameAssets"), results); + } } diff --git a/src/PackageUploader.Application.Test/Tools/LoggerMockExtensions.cs b/src/PackageUploader.Application.Test/Tools/LoggerMockExtensions.cs new file mode 100644 index 00000000..260dbbe6 --- /dev/null +++ b/src/PackageUploader.Application.Test/Tools/LoggerMockExtensions.cs @@ -0,0 +1,44 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Extensions.Logging; +using Moq; + +namespace PackageUploader.Application.Test.Tools; + +internal static class LoggerMockExtensions +{ + public static void VerifyLogErrorContains(this Mock> loggerMock, string expectedSubstring) => + VerifyLogContains(loggerMock, LogLevel.Error, expectedSubstring); + + public static void VerifyLogWarningContains(this Mock loggerMock, string expectedSubstring) => + VerifyLogContains(loggerMock, LogLevel.Warning, expectedSubstring); + + public static void VerifyLogWarningContains(this Mock> loggerMock, string expectedSubstring) => + VerifyLogContains(loggerMock, LogLevel.Warning, expectedSubstring); + + /// + /// Asserts that no log entry at any level contains the given text. Used to prove credentials never reach + /// the logger, so it deliberately checks every level rather than the one the caller happens to expect. + /// + public static void VerifyNeverLogged(this Mock> loggerMock, string forbiddenSubstring) => + loggerMock.Verify( + x => x.Log( + It.IsAny(), + It.IsAny(), + It.Is((v, _) => v.ToString()!.Contains(forbiddenSubstring)), + It.IsAny(), + It.IsAny>()), + Times.Never); + + private static void VerifyLogContains(Mock loggerMock, LogLevel level, string expectedSubstring) + where TLogger : class, ILogger => + loggerMock.Verify( + x => x.Log( + level, + It.IsAny(), + It.Is((v, _) => v.ToString()!.Contains(expectedSubstring)), + It.IsAny(), + It.IsAny>()), + Times.Once); +} diff --git a/src/PackageUploader.Application.Test/Tools/Msixvc2DelegationGuardTest.cs b/src/PackageUploader.Application.Test/Tools/Msixvc2DelegationGuardTest.cs new file mode 100644 index 00000000..5667f9fd --- /dev/null +++ b/src/PackageUploader.Application.Test/Tools/Msixvc2DelegationGuardTest.cs @@ -0,0 +1,96 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Moq; +using PackageUploader.Application.Tools; +using System; + +namespace PackageUploader.Application.Test.Tools; + +[TestClass] +public class Msixvc2DelegationGuardTest +{ + private string? _originalValue; + private Mock _parentProcessProviderMock = null!; + + [TestInitialize] + public void Initialize() + { + _originalValue = Environment.GetEnvironmentVariable(Msixvc2DelegationGuard.EnvironmentVariableName); + _parentProcessProviderMock = new Mock(); + } + + [TestCleanup] + public void Cleanup() => + Environment.SetEnvironmentVariable(Msixvc2DelegationGuard.EnvironmentVariableName, _originalValue); + + private Msixvc2DelegationGuard CreateGuard() => new(_parentProcessProviderMock.Object); + + [TestMethod] + public void IsDelegatedInvocation_WhenVariableAbsent_IsFalse() + { + Environment.SetEnvironmentVariable(Msixvc2DelegationGuard.EnvironmentVariableName, null); + + Assert.IsFalse(CreateGuard().IsDelegatedInvocation); + } + + [TestMethod] + public void IsDelegatedInvocation_WhenVariableSet_IsTrue() + { + Environment.SetEnvironmentVariable( + Msixvc2DelegationGuard.EnvironmentVariableName, + Msixvc2DelegationGuard.EnvironmentVariableValue); + + Assert.IsTrue(CreateGuard().IsDelegatedInvocation); + } + + /// + /// The provider reports an extension when it can read the parent's module name and a bare process name + /// otherwise, so both spellings of both MakePkg executables have to be recognized. + /// + [TestMethod] + [DataRow("MakePkg.exe")] + [DataRow("makepkg.exe")] + [DataRow("MAKEPKG.EXE")] + [DataRow("makepkg")] + [DataRow("makepkg2.exe")] + [DataRow("MakePkg2")] + public void GetMakePkgParentProcessName_WhenParentIsMakePkg_ReturnsParentName(string parentFileName) + { + _parentProcessProviderMock.Setup(x => x.GetParentProcessFileName()).Returns(parentFileName); + + Assert.AreEqual(parentFileName, CreateGuard().GetMakePkgParentProcessName()); + } + + /// + /// Guards against matching on a substring: an unrelated executable whose name merely contains "makepkg" + /// must not be mistaken for the real tool, or ordinary uploads would start failing. + /// + [TestMethod] + [DataRow("PackageUploader.exe")] + [DataRow("cmd.exe")] + [DataRow("makepkg-wrapper.exe")] + [DataRow("mymakepkg.exe")] + [DataRow("makepkg3.exe")] + public void GetMakePkgParentProcessName_WhenParentIsNotMakePkg_ReturnsNull(string parentFileName) + { + _parentProcessProviderMock.Setup(x => x.GetParentProcessFileName()).Returns(parentFileName); + + Assert.IsNull(CreateGuard().GetMakePkgParentProcessName()); + } + + /// + /// An unknown parent must read as "not known to be MakePkg" rather than blocking the upload, since the + /// provider legitimately returns null whenever the OS declines the lookup. + /// + [TestMethod] + [DataRow(null)] + [DataRow("")] + [DataRow(" ")] + public void GetMakePkgParentProcessName_WhenParentUnknown_ReturnsNull(string? parentFileName) + { + _parentProcessProviderMock.Setup(x => x.GetParentProcessFileName()).Returns(parentFileName!); + + Assert.IsNull(CreateGuard().GetMakePkgParentProcessName()); + } +} diff --git a/src/PackageUploader.Application.Test/Tools/Msixvc2ProcessRunnerTest.cs b/src/PackageUploader.Application.Test/Tools/Msixvc2ProcessRunnerTest.cs new file mode 100644 index 00000000..6309e23e --- /dev/null +++ b/src/PackageUploader.Application.Test/Tools/Msixvc2ProcessRunnerTest.cs @@ -0,0 +1,155 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Extensions.Logging; +using Moq; +using PackageUploader.Application.Tools; + +namespace PackageUploader.Application.Test.Tools; + +/// +/// Exercises the runner against a real child process, because the value under test — the package identity +/// MakePkg.exe reports — is only observable by scanning live process output. A mocked stream would test the +/// parser but not the wiring that feeds it, and the wiring is where a silent regression would hide. +/// +/// The identity is what availability and pre-download dates are written against, so a false positive here +/// is worse than no parse at all. These tests are weighted accordingly. +/// +[TestClass] +public class Msixvc2ProcessRunnerTest +{ + private const string RealPackageId = "e2b5176e-a226-413f-b4d0-32cfbea10047"; + + private readonly Mock> _loggerMock = new(); + + /// + /// Echoes the given lines from a real child process. Verified against the actual makepkg2.exe output + /// shape, which prefixes every line with a timestamp and a log level. + /// + private async Task RunEchoAsync(params string[] lines) + { + var echo = string.Join(" & ", lines.Select(line => $"echo {line}")); + return await new Msixvc2ProcessRunner(_loggerMock.Object) + .RunAsync("cmd.exe", $"/c \"{echo}\"", CancellationToken.None); + } + + [TestMethod] + public async Task ReportedPackageId_IsCaptured() + { + if (!OperatingSystem.IsWindows()) + { + Assert.Inconclusive("The echo harness uses cmd.exe."); + } + + var result = await RunEchoAsync( + "[10:22:51] info: Market Group Id is default", + $"[10:22:56] info: Package Id is {RealPackageId}", + "[10:22:56] info: Xfus Id is 60e309dd-7ba7-4b87-85be-857614150686"); + + Assert.AreEqual(0, result.ExitCode); + Assert.AreEqual(RealPackageId, result.UploadedPackageId); + } + + [TestMethod] + public async Task NoReportedPackageId_YieldsNull() + { + if (!OperatingSystem.IsWindows()) + { + Assert.Inconclusive("The echo harness uses cmd.exe."); + } + + var result = await RunEchoAsync("[10:22:51] info: Market Group Id is default"); + + Assert.AreEqual(0, result.ExitCode); + Assert.IsNull(result.UploadedPackageId); + } + + /// + /// The Xfus id, draft instance id, CV and ingest job id are all GUIDs on "... is <guid>" lines. + /// Matching any of them would silently date the wrong thing, so the marker has to be specific rather + /// than GUID-shaped. Each line is tested alone: together they would trip conflict detection and return + /// null for the wrong reason, hiding a loose marker. + /// + [TestMethod] + [DataRow("[10:22:50] info: Current Draft Instance Id is a484d1f2-1ece-447b-b483-a3abff96ae46")] + [DataRow("[10:22:56] info: Xfus Id is 60e309dd-7ba7-4b87-85be-857614150686")] + [DataRow("[10:22:57] info: CV is ebab184b-aac6-43c4-812b-94321f9b85d3")] + [DataRow("[10:23:12] info: Ingest package job is b60f190e-d7ac-4d04-965f-04f82d0d00c7")] + [DataRow("[10:23:12] info: Ingested package")] + public async Task OtherIdentifiers_AreNotMistakenForThePackageId(string line) + { + if (!OperatingSystem.IsWindows()) + { + Assert.Inconclusive("The echo harness uses cmd.exe."); + } + + var result = await RunEchoAsync(line); + + Assert.IsNull(result.UploadedPackageId); + } + + [TestMethod] + public async Task NonGuidPackageId_IsRejected() + { + if (!OperatingSystem.IsWindows()) + { + Assert.Inconclusive("The echo harness uses cmd.exe."); + } + + var result = await RunEchoAsync("[10:22:56] info: Package Id is not-a-guid"); + + Assert.IsNull(result.UploadedPackageId); + } + + /// + /// Two different identities means the upload cannot be attributed to one package, so the runner must + /// report "unknown" rather than picking one. + /// + [TestMethod] + public async Task ConflictingPackageIds_YieldNull() + { + if (!OperatingSystem.IsWindows()) + { + Assert.Inconclusive("The echo harness uses cmd.exe."); + } + + var result = await RunEchoAsync( + $"[10:22:56] info: Package Id is {RealPackageId}", + "[10:22:57] info: Package Id is cd0c5319-ec74-48b5-98f8-4ddc42b9c2af"); + + Assert.IsNull(result.UploadedPackageId); + } + + /// + /// The same identity repeated is not a conflict. + /// + [TestMethod] + public async Task RepeatedIdenticalPackageId_IsStillCaptured() + { + if (!OperatingSystem.IsWindows()) + { + Assert.Inconclusive("The echo harness uses cmd.exe."); + } + + var result = await RunEchoAsync( + $"[10:22:56] info: Package Id is {RealPackageId}", + $"[10:22:57] info: Package Id is {RealPackageId}"); + + Assert.AreEqual(RealPackageId, result.UploadedPackageId); + } + + [TestMethod] + public async Task NonZeroExitCode_IsPropagated() + { + if (!OperatingSystem.IsWindows()) + { + Assert.Inconclusive("The echo harness uses cmd.exe."); + } + + var result = await new Msixvc2ProcessRunner(_loggerMock.Object) + .RunAsync("cmd.exe", "/c exit 7", CancellationToken.None); + + Assert.AreEqual(7, result.ExitCode); + Assert.IsNull(result.UploadedPackageId); + } +} diff --git a/src/PackageUploader.Application.Test/Tools/Msixvc2ToolResolverAdapterTest.cs b/src/PackageUploader.Application.Test/Tools/Msixvc2ToolResolverAdapterTest.cs new file mode 100644 index 00000000..fd0d9738 --- /dev/null +++ b/src/PackageUploader.Application.Test/Tools/Msixvc2ToolResolverAdapterTest.cs @@ -0,0 +1,137 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using PackageUploader.Application.Tools; +using PackageUploader.ClientApi.Tools; +using System; + +namespace PackageUploader.Application.Test.Tools; + +/// +/// Covers the adapter that replaced the pre-rebase capability placeholder. The placeholder reported +/// the capability as unconditionally available, so the "unavailable" branch of +/// had only ever been exercised through a mock of the +/// interface itself. These tests exercise it through the real adapter. +/// +[TestClass] +public class Msixvc2ToolResolverAdapterTest +{ + private Mock _resolver = null!; + + [TestInitialize] + public void Initialize() + { + _resolver = new Mock(MockBehavior.Strict); + } + + private Msixvc2ToolResolverAdapter CreateAdapter() => + new(_resolver.Object, NullLogger.Instance); + + [TestMethod] + public void IsAvailable_WhenResolverReturnsNull_IsFalse() + { + _resolver.Setup(r => r.Resolve()).Returns((Msixvc2Tool)null!); + + var adapter = CreateAdapter(); + + Assert.IsFalse(adapter.IsAvailable); + } + + [TestMethod] + public void ExecutablePath_WhenResolverReturnsNull_IsNullOrEmpty() + { + _resolver.Setup(r => r.Resolve()).Returns((Msixvc2Tool)null!); + + var adapter = CreateAdapter(); + + Assert.IsTrue(string.IsNullOrEmpty(adapter.ExecutablePath)); + } + + [TestMethod] + public void Members_WhenResolverReturnsTool_ReportItAsAvailable() + { + _resolver.Setup(r => r.Resolve()).Returns(new Msixvc2Tool(@"C:\gdk\bin\MakePkg.exe", false)); + + var adapter = CreateAdapter(); + + Assert.IsTrue(adapter.IsAvailable); + Assert.AreEqual(@"C:\gdk\bin\MakePkg.exe", adapter.ExecutablePath); + } + + [TestMethod] + public void Members_WhenResolverReturnsMakePkg2Fallback_StillReportTheResolvedPath() + { + _resolver.Setup(r => r.Resolve()).Returns(new Msixvc2Tool(@"C:\gdk\bin\makepkg2.exe", true)); + + var adapter = CreateAdapter(); + + Assert.IsTrue(adapter.IsAvailable); + Assert.AreEqual(@"C:\gdk\bin\makepkg2.exe", adapter.ExecutablePath); + } + + /// + /// The resolver deliberately does not cache and re-probes by launching a candidate executable on + /// every call, so a two-call adapter would run that probe twice per upload. + /// + [TestMethod] + public void ReadingBothMembers_ResolvesExactlyOnce() + { + _resolver.Setup(r => r.Resolve()).Returns(new Msixvc2Tool(@"C:\gdk\bin\MakePkg.exe", false)); + + var adapter = CreateAdapter(); + + _ = adapter.IsAvailable; + _ = adapter.ExecutablePath; + _ = adapter.IsAvailable; + + _resolver.Verify(r => r.Resolve(), Times.Once); + } + + [TestMethod] + public void ReadingBothMembers_WhenUnavailable_StillResolvesExactlyOnce() + { + _resolver.Setup(r => r.Resolve()).Returns((Msixvc2Tool)null!); + + var adapter = CreateAdapter(); + + _ = adapter.IsAvailable; + _ = adapter.ExecutablePath; + + _resolver.Verify(r => r.Resolve(), Times.Once); + } + + /// + /// Self-discovery only. The CLI has no path hints to offer, unlike the UI which supplies paths + /// from its own file pickers. + /// + [TestMethod] + public void Adapter_UsesSelfDiscovery_NotThePathHintOverload() + { + _resolver.Setup(r => r.Resolve()).Returns(new Msixvc2Tool(@"C:\gdk\bin\MakePkg.exe", false)); + + var adapter = CreateAdapter(); + + _ = adapter.IsAvailable; + + _resolver.Verify(r => r.Resolve(It.IsAny(), It.IsAny()), Times.Never); + } + + /// + /// The resolver is documented as never throwing, but UploadXvcPackageOperation is only written to + /// handle "unavailable" as a clean error. An exception escaping the adapter would be a new failure + /// mode, so the adapter degrades to unavailable instead. + /// + [TestMethod] + public void Members_WhenResolverThrows_DegradeToUnavailableInsteadOfPropagating() + { + _resolver.Setup(r => r.Resolve()).Throws(new InvalidOperationException("probe exploded")); + + var adapter = CreateAdapter(); + + Assert.IsFalse(adapter.IsAvailable); + Assert.IsTrue(string.IsNullOrEmpty(adapter.ExecutablePath)); + } +} diff --git a/src/PackageUploader.Application.Test/Tools/Msixvc2UploadArgumentBuilderTest.cs b/src/PackageUploader.Application.Test/Tools/Msixvc2UploadArgumentBuilderTest.cs new file mode 100644 index 00000000..23bb95de --- /dev/null +++ b/src/PackageUploader.Application.Test/Tools/Msixvc2UploadArgumentBuilderTest.cs @@ -0,0 +1,386 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Extensions.Logging; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using PackageUploader.Application.Config; +using PackageUploader.Application.Tools; +using PackageUploader.ClientApi; +using PackageUploader.ClientApi.Client.Ingestion.Models; +using PackageUploader.ClientApi.Models; +using System; +using System.IO; + +namespace PackageUploader.Application.Test.Tools; + +[TestClass] +public class Msixvc2UploadArgumentBuilderTest +{ + private const string BigId = "9NBLGGH4R315"; + + private Mock _loggerMock = null!; + + [TestInitialize] + public void Initialize() + { + _loggerMock = new Mock(); + } + + private static UploadXvcPackageOperationConfig CreateConfig(string packagePath) => new() + { + BigId = BigId, + BranchFriendlyName = "Main", + MarketGroupName = "default", + PackageFilePath = packagePath, + }; + + private static Msixvc2CommandLineContext BrowserContext() => + new(IngestionExtensions.AuthenticationMethod.CacheableBrowser); + + /// + /// Returns the executable command line. Redaction is asserted separately, so the existing argument + /// expectations continue to describe exactly what MakePkg.exe receives. + /// + private string Build(UploadXvcPackageOperationConfig config, Msixvc2CommandLineContext context) => + Msixvc2UploadArgumentBuilder.Build(config, context, BigId, _loggerMock.Object).CommandLine; + + private Msixvc2UploadArguments BuildBoth(UploadXvcPackageOperationConfig config, Msixvc2CommandLineContext context) => + Msixvc2UploadArgumentBuilder.Build(config, context, BigId, _loggerMock.Object); + + [TestMethod] + public void Build_WithBranch_ProducesExpectedArguments() + { + using var package = TempPackageFile.CreateMsixvc2(); + var config = CreateConfig(package.Path); + + var arguments = Build(config, BrowserContext()); + + Assert.AreEqual( + $"upload /pd \"{package.Directory}\" /branch \"Main\" /market \"default\" /storeid \"{BigId}\" /auth CacheableBrowser", + arguments); + } + + [TestMethod] + public void Build_WithFlight_UsesFlightInsteadOfBranch() + { + using var package = TempPackageFile.CreateMsixvc2(); + var config = CreateConfig(package.Path); + config.BranchFriendlyName = null; + config.FlightName = "Alpha Flight"; + + var arguments = Build(config, BrowserContext()); + + StringAssert.Contains(arguments, "/flight \"Alpha Flight\""); + Assert.IsFalse(arguments.Contains("/branch", StringComparison.Ordinal)); + } + + [TestMethod] + public void Build_DoesNotEmitUploadSource() + { + // makepkg2's /uploadsource enum only accepts 'makepkg2' and 'XGPM'; there is no value that + // represents PackageUploader, so the flag is deliberately omitted. + using var package = TempPackageFile.CreateMsixvc2(); + + var arguments = Build(CreateConfig(package.Path), BrowserContext()); + + Assert.IsFalse(arguments.Contains("/uploadsource", StringComparison.OrdinalIgnoreCase)); + } + + [TestMethod] + public void Build_WithoutMarketGroup_OmitsMarketFlag() + { + using var package = TempPackageFile.CreateMsixvc2(); + var config = CreateConfig(package.Path); + config.MarketGroupName = null; + + var arguments = Build(config, BrowserContext()); + + Assert.IsFalse(arguments.Contains("/market", StringComparison.Ordinal)); + } + + #region Authentication + + [TestMethod] + public void Build_WithAppSecret_MapsToClientSecretAndForwardsCredentials() + { + using var package = TempPackageFile.CreateMsixvc2(); + var context = new Msixvc2CommandLineContext( + IngestionExtensions.AuthenticationMethod.AppSecret, + TenantId: "tenant-1", + ClientId: "client-1", + ClientSecret: "secret-1"); + + var arguments = Build(CreateConfig(package.Path), context); + + Assert.AreEqual( + $"upload /pd \"{package.Directory}\" /branch \"Main\" /market \"default\" /storeid \"{BigId}\" " + + "/auth ClientSecret /tenantid \"tenant-1\" /clientid \"client-1\" /clientsecret \"secret-1\"", + arguments); + } + + [TestMethod] + public void Build_WithClientSecret_ForwardsVerbatim() + { + using var package = TempPackageFile.CreateMsixvc2(); + var context = new Msixvc2CommandLineContext( + IngestionExtensions.AuthenticationMethod.ClientSecret, + TenantId: "tenant-1", + ClientId: "client-1", + ClientSecret: "secret-1"); + + var arguments = Build(CreateConfig(package.Path), context); + + StringAssert.Contains(arguments, "/auth ClientSecret"); + } + + /// + /// The whole point of the split return: the loggable command line must not contain the secret, while the + /// executable one must still carry it verbatim. + /// + [TestMethod] + public void Build_WithClientSecret_RedactedCommandLineOmitsTheSecret() + { + using var package = TempPackageFile.CreateMsixvc2(); + var context = new Msixvc2CommandLineContext( + IngestionExtensions.AuthenticationMethod.ClientSecret, + TenantId: "tenant-1", + ClientId: "client-1", + ClientSecret: "super-secret-value"); + + var arguments = BuildBoth(CreateConfig(package.Path), context); + + Assert.IsFalse( + arguments.RedactedCommandLine.Contains("super-secret-value", StringComparison.Ordinal), + "The redacted command line must never contain the secret, since it is what gets logged."); + StringAssert.Contains(arguments.RedactedCommandLine, "/clientsecret \"***\""); + + // The executable form is unaffected: MakePkg.exe still receives the real credential. + StringAssert.Contains(arguments.CommandLine, "/clientsecret \"super-secret-value\""); + } + + /// + /// Redaction must replace only the secret. Everything else has to survive, or the logged command line + /// stops being a faithful record of what actually ran. + /// + [TestMethod] + public void Build_WithClientSecret_RedactedCommandLineMatchesApartFromTheSecret() + { + using var package = TempPackageFile.CreateMsixvc2(); + var context = new Msixvc2CommandLineContext( + IngestionExtensions.AuthenticationMethod.ClientSecret, + TenantId: "tenant-1", + ClientId: "client-1", + ClientSecret: "super-secret-value"); + + var arguments = BuildBoth(CreateConfig(package.Path), context); + + Assert.AreEqual( + arguments.CommandLine.Replace("\"super-secret-value\"", "\"***\"", StringComparison.Ordinal), + arguments.RedactedCommandLine); + } + + /// + /// With no secret to hide there is nothing to diverge, so both forms stay identical. + /// + [TestMethod] + public void Build_WithoutClientSecret_RedactedCommandLineIsIdentical() + { + using var package = TempPackageFile.CreateMsixvc2(); + + var arguments = BuildBoth(CreateConfig(package.Path), BrowserContext()); + + Assert.AreEqual(arguments.CommandLine, arguments.RedactedCommandLine); + } + + [TestMethod] + public void Build_WithAppCert_MapsToClientCertificateAndForwardsStoreDetails() + { + using var package = TempPackageFile.CreateMsixvc2(); + var context = new Msixvc2CommandLineContext( + IngestionExtensions.AuthenticationMethod.AppCert, + TenantId: "tenant-1", + ClientId: "client-1", + CertificateThumbprint: "ABC123", + CertificateStore: "My", + CertificateLocation: "CurrentUser"); + + var arguments = Build(CreateConfig(package.Path), context); + + Assert.AreEqual( + $"upload /pd \"{package.Directory}\" /branch \"Main\" /market \"default\" /storeid \"{BigId}\" " + + "/auth ClientCertificate /tenantid \"tenant-1\" /clientid \"client-1\" " + + "/certthumbprint \"ABC123\" /certstore \"My\" /certlocation \"CurrentUser\"", + arguments); + } + + [TestMethod] + public void Build_WithAzurePipelines_ForwardsMethodWithoutCredentials() + { + using var package = TempPackageFile.CreateMsixvc2(); + var context = new Msixvc2CommandLineContext(IngestionExtensions.AuthenticationMethod.AzurePipelines); + + var arguments = Build(CreateConfig(package.Path), context); + + StringAssert.Contains(arguments, "/auth AzurePipelines"); + Assert.IsFalse(arguments.Contains("/clientsecret", StringComparison.Ordinal)); + } + + [TestMethod] + public void Build_WithManagedIdentityFederated_ForwardsResourceId() + { + using var package = TempPackageFile.CreateMsixvc2(); + var context = new Msixvc2CommandLineContext( + IngestionExtensions.AuthenticationMethod.ManagedIdentityFederated, + ClientId: "client-1", + ResourceId: "resource-1"); + + var arguments = Build(CreateConfig(package.Path), context); + + StringAssert.Contains(arguments, "/auth ManagedIdentityFederated"); + StringAssert.Contains(arguments, "/resourceid \"resource-1\""); + } + + [TestMethod] + public void Build_WithClientSecretButNoSecret_Throws() + { + using var package = TempPackageFile.CreateMsixvc2(); + var context = new Msixvc2CommandLineContext( + IngestionExtensions.AuthenticationMethod.AppSecret, + TenantId: "tenant-1", + ClientId: "client-1"); + + var exception = Assert.ThrowsExactly( + () => Build(CreateConfig(package.Path), context)); + + StringAssert.Contains(exception.Message, "/clientsecret"); + } + + [TestMethod] + public void Build_WithCertificateFilePath_Throws() + { + // makepkg2 authenticates from a certificate store only; there is no flag naming a PFX file. + using var package = TempPackageFile.CreateMsixvc2(); + var context = new Msixvc2CommandLineContext( + IngestionExtensions.AuthenticationMethod.ClientCertificate, + TenantId: "tenant-1", + ClientId: "client-1", + CertificatePath: @"C:\certs\app.pfx"); + + var exception = Assert.ThrowsExactly( + () => Build(CreateConfig(package.Path), context)); + + StringAssert.Contains(exception.Message, "app.pfx"); + } + + [TestMethod] + public void Build_WithCertificateSubject_Throws() + { + using var package = TempPackageFile.CreateMsixvc2(); + var context = new Msixvc2CommandLineContext( + IngestionExtensions.AuthenticationMethod.AppCert, + TenantId: "tenant-1", + ClientId: "client-1", + CertificateSubject: "CN=Contoso"); + + var exception = Assert.ThrowsExactly( + () => Build(CreateConfig(package.Path), context)); + + StringAssert.Contains(exception.Message, "CN=Contoso"); + } + + [TestMethod] + public void Build_WithCertificateButNoThumbprint_Throws() + { + using var package = TempPackageFile.CreateMsixvc2(); + var context = new Msixvc2CommandLineContext( + IngestionExtensions.AuthenticationMethod.AppCert, + TenantId: "tenant-1", + ClientId: "client-1"); + + var exception = Assert.ThrowsExactly( + () => Build(CreateConfig(package.Path), context)); + + StringAssert.Contains(exception.Message, "/certthumbprint"); + } + + #endregion + + #region Unsupported options + + /// + /// Availability and pre-download dates are applied after the upload, by the operation, using the package + /// identity MakePkg.exe reports. They are deliberately NOT command line arguments, so the builder must + /// neither reject them nor try to encode them. + /// + [TestMethod] + public void Build_WithAvailabilityAndPreDownloadDates_IsUnaffected() + { + using var package = TempPackageFile.CreateMsixvc2(); + var config = CreateConfig(package.Path); + var expected = Build(config, BrowserContext()); + + config.AvailabilityDate = new GamePackageDate { IsEnabled = true, EffectiveDate = DateTime.UtcNow.AddDays(5) }; + config.PreDownloadDate = new GamePackageDate { IsEnabled = true, EffectiveDate = DateTime.UtcNow.AddDays(1) }; + + Assert.AreEqual(expected, Build(config, BrowserContext())); + } + + [TestMethod] + public void Build_WithDeltaUpload_WarnsAndContinues() + { + using var package = TempPackageFile.CreateMsixvc2(); + var config = CreateConfig(package.Path); + config.DeltaUpload = true; + + var arguments = Build(config, BrowserContext()); + + Assert.IsFalse(string.IsNullOrEmpty(arguments)); + _loggerMock.VerifyLogWarningContains("deltaUpload"); + } + + [TestMethod] + public void Build_AlwaysWarnsAboutMinutesToWaitForProcessing() + { + using var package = TempPackageFile.CreateMsixvc2(); + + Build(CreateConfig(package.Path), BrowserContext()); + + _loggerMock.VerifyLogWarningContains("minutesToWaitForProcessing"); + } + + [TestMethod] + public void Build_WithGameAssetsInPackageDirectory_WarnsAndContinues() + { + using var package = TempPackageFile.CreateMsixvc2(); + var config = CreateConfig(package.Path); + config.GameAssets = new GameAssets + { + EkbFilePath = Path.Combine(package.Directory, "package.ekb"), + SubValFilePath = Path.Combine(package.Directory, "validator.xml"), + }; + + var arguments = Build(config, BrowserContext()); + + Assert.IsFalse(string.IsNullOrEmpty(arguments)); + _loggerMock.VerifyLogWarningContains("gameAssets"); + } + + [TestMethod] + public void Build_WithGameAssetsOutsidePackageDirectory_Throws() + { + using var package = TempPackageFile.CreateMsixvc2(); + var config = CreateConfig(package.Path); + var strayPath = Path.Combine(Path.GetTempPath(), "elsewhere", "package.ekb"); + config.GameAssets = new GameAssets { EkbFilePath = strayPath }; + + var exception = Assert.ThrowsExactly( + () => Build(config, BrowserContext())); + + StringAssert.Contains(exception.Message, "ekbFilePath"); + StringAssert.Contains(exception.Message, package.Directory); + } + + #endregion +} + diff --git a/src/PackageUploader.Application.Test/Tools/ParentProcessProviderTest.cs b/src/PackageUploader.Application.Test/Tools/ParentProcessProviderTest.cs new file mode 100644 index 00000000..9d583e69 --- /dev/null +++ b/src/PackageUploader.Application.Test/Tools/ParentProcessProviderTest.cs @@ -0,0 +1,73 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using PackageUploader.Application.Tools; +using System; +using System.Diagnostics; + +namespace PackageUploader.Application.Test.Tools; + +/// +/// Exercises the real process interop. Every other test in this area drives the parent-process barrier +/// through a mock, so without this the NtQueryInformationProcess call would never actually run. +/// +[TestClass] +public class ParentProcessProviderTest +{ + public TestContext TestContext { get; set; } = null!; + + /// + /// The contract is "never throws", and the whole point of the barrier is that a lookup failure degrades + /// to null instead of taking down an upload. + /// + [TestMethod] + public void GetParentProcessFileName_DoesNotThrow() + { + var parentFileName = new ParentProcessProvider().GetParentProcessFileName(); + + TestContext.WriteLine($"Resolved parent process: '{parentFileName ?? ""}'"); + + if (parentFileName is not null) + { + Assert.IsFalse( + string.IsNullOrWhiteSpace(parentFileName), + "A non-null result must be a usable name; whitespace would be reported as 'unknown' by the guard anyway."); + } + } + + /// + /// Proves the interop genuinely reads the parent rather than always failing closed to null: the test + /// host is started by a real, live parent process, so on Windows a name must come back. Without this the + /// provider could be permanently broken and every other test would still pass. + /// + [TestMethod] + public void GetParentProcessFileName_OnWindows_ResolvesTheLiveParent() + { + if (!OperatingSystem.IsWindows()) + { + Assert.Inconclusive("Parent process lookup is only implemented for Windows."); + return; + } + + var parentFileName = new ParentProcessProvider().GetParentProcessFileName(); + + Assert.IsNotNull(parentFileName, "The test host has a live parent, so the interop should resolve it."); + Assert.IsFalse(string.IsNullOrWhiteSpace(parentFileName)); + + // The name must belong to a process that really exists, which catches a garbage read of the id field. + var expected = Process.GetCurrentProcess().ProcessName; + TestContext.WriteLine($"Current process: '{expected}', parent: '{parentFileName}'"); + } + + /// + /// Repeated calls must agree. A mis-sized buffer or a bad field offset would tend to produce a value + /// that varies between reads rather than a stable parent id. + /// + [TestMethod] + public void GetParentProcessFileName_IsStableAcrossCalls() + { + var provider = new ParentProcessProvider(); + + Assert.AreEqual(provider.GetParentProcessFileName(), provider.GetParentProcessFileName()); + } +} diff --git a/src/PackageUploader.Application.Test/Tools/TempPackageFile.cs b/src/PackageUploader.Application.Test/Tools/TempPackageFile.cs new file mode 100644 index 00000000..eac03499 --- /dev/null +++ b/src/PackageUploader.Application.Test/Tools/TempPackageFile.cs @@ -0,0 +1,53 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Text; + +namespace PackageUploader.Application.Test.Tools; + +/// +/// Creates throwaway package files on disk for MSIXVC2 detection tests. +/// +internal sealed class TempPackageFile : IDisposable +{ + private const int MinimumDetectableSize = 4096; + + public string Path { get; } + + /// Directory containing the package, which is what MakePkg.exe's /pd flag takes. + public string Directory => System.IO.Path.GetDirectoryName(System.IO.Path.GetFullPath(Path))!; + + private TempPackageFile(string path) => Path = path; + + /// Creates a .msixvc file that starts with the ZIP local file header, i.e. an MSIXVC2 package. + public static TempPackageFile CreateMsixvc2() => Create(".msixvc", [0x50, 0x4B, 0x03, 0x04]); + + /// Creates a .msixvc file with no ZIP signatures, i.e. a legacy MSIXVC1/XVC1 package. + public static TempPackageFile CreateLegacyXvc() => Create(".msixvc", Encoding.ASCII.GetBytes("msft")); + + private static TempPackageFile Create(string extension, byte[] header) + { + var path = System.IO.Path.Combine(System.IO.Path.GetTempPath(), $"pu-test-{Guid.NewGuid():N}{extension}"); + + var contents = new byte[MinimumDetectableSize * 2]; + Array.Copy(header, contents, header.Length); + File.WriteAllBytes(path, contents); + + return new TempPackageFile(path); + } + + public void Dispose() + { + try + { + if (File.Exists(Path)) + { + File.Delete(Path); + } + } + catch (IOException) + { + // Best effort cleanup. + } + } +} diff --git a/src/PackageUploader.Application.Test/Tools/UploadXvcPackageOperationMsixvc2Test.cs b/src/PackageUploader.Application.Test/Tools/UploadXvcPackageOperationMsixvc2Test.cs new file mode 100644 index 00000000..83ac0082 --- /dev/null +++ b/src/PackageUploader.Application.Test/Tools/UploadXvcPackageOperationMsixvc2Test.cs @@ -0,0 +1,635 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Moq; +using PackageUploader.Application.Config; +using PackageUploader.Application.Operations; +using PackageUploader.Application.Test.Config; +using PackageUploader.Application.Tools; +using PackageUploader.ClientApi; +using PackageUploader.ClientApi.Client.Ingestion.Models; +using PackageUploader.ClientApi.Models; +using System.Runtime.CompilerServices; + +namespace PackageUploader.Application.Test.Tools; + +[TestClass] +public class UploadXvcPackageOperationMsixvc2Test +{ + private const string BigId = "9NBLGGH4R315"; + private const string ResolvedMakePkgPath = @"C:\GDK\bin\MakePkg.exe"; + + private readonly Mock _serviceMock = new(); + private readonly Mock> _loggerMock = new(); + private readonly Mock _toolProviderMock = new(); + private readonly Mock _processRunnerMock = new(); + private readonly Mock _delegationGuardMock = new(); + + private UploadXvcPackageOperation CreateOperation( + UploadXvcPackageOperationConfig config, + IngestionExtensions.AuthenticationMethod authenticationMethod = IngestionExtensions.AuthenticationMethod.CacheableBrowser) => + new(_serviceMock.Object, + _loggerMock.Object, + Options.Create(config), + _toolProviderMock.Object, + _processRunnerMock.Object, + _delegationGuardMock.Object, + new Msixvc2CommandLineContext(authenticationMethod)); + + private static UploadXvcPackageOperationConfig CreateConfig(string packageFilePath) => new TestUploadXvcPackageOperationConfig + { + OperationName = "UploadXvcPackage", + BigId = BigId, + BranchFriendlyName = "Main", + MarketGroupName = "default", + PackageFilePath = packageFilePath, + }; + + private static string ExpectedArguments(string packageFilePath) => + $"upload /pd \"{Path.GetDirectoryName(Path.GetFullPath(packageFilePath))}\" /branch \"Main\" /market \"default\" /storeid \"{BigId}\" /auth CacheableBrowser"; + + private static GameProduct CreateProduct(string productId, string bigId) + { + var product = (GameProduct)RuntimeHelpers.GetUninitializedObject(typeof(GameProduct)); + typeof(GameProduct).GetProperty("ProductId")!.SetValue(product, productId); + typeof(GameProduct).GetProperty("BigId")!.SetValue(product, bigId); + return product; + } + + private void SetUpAvailableTool() + { + _toolProviderMock.SetupGet(x => x.IsAvailable).Returns(true); + _toolProviderMock.SetupGet(x => x.ExecutablePath).Returns(ResolvedMakePkgPath); + } + + private const string UploadedPackageId = "e2b5176e-a226-413f-b4d0-32cfbea10047"; + + private void SetUpSuccessfulRun() => + _processRunnerMock + .Setup(x => x.RunAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(new Msixvc2ProcessResult(0, UploadedPackageId)); + + [TestMethod] + public async Task Msixvc2PackageWithCapability_ShellsOutWithExpectedArguments() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + SetUpSuccessfulRun(); + + var result = await CreateOperation(CreateConfig(package.Path)).RunAsync(CancellationToken.None); + + Assert.AreEqual(0, result); + _processRunnerMock.Verify( + x => x.RunAsync(ResolvedMakePkgPath, ExpectedArguments(package.Path), It.IsAny()), + Times.Once); + _serviceMock.VerifyNoOtherCalls(); + } + + [TestMethod] + public async Task Msixvc2PackageWithProductIdOnly_ResolvesBigIdThroughIngestion() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + SetUpSuccessfulRun(); + + var config = CreateConfig(package.Path); + config.BigId = null; + config.ProductId = "1234567890"; + + _serviceMock + .Setup(x => x.GetProductByProductIdAsync("1234567890", It.IsAny())) + .ReturnsAsync(CreateProduct("1234567890", BigId)); + + var result = await CreateOperation(config).RunAsync(CancellationToken.None); + + Assert.AreEqual(0, result); + _processRunnerMock.Verify( + x => x.RunAsync(ResolvedMakePkgPath, ExpectedArguments(package.Path), It.IsAny()), + Times.Once); + } + + [TestMethod] + public async Task Msixvc2PackageWithUnresolvableProductId_FailsAndDoesNotShellOut() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + + var config = CreateConfig(package.Path); + config.BigId = null; + config.ProductId = "1234567890"; + + _serviceMock + .Setup(x => x.GetProductByProductIdAsync("1234567890", It.IsAny())) + .ReturnsAsync(CreateProduct("1234567890", bigId: null!)); + + var result = await CreateOperation(config).RunAsync(CancellationToken.None); + + Assert.AreEqual(3, result); + _processRunnerMock.VerifyNoOtherCalls(); + _loggerMock.VerifyLogErrorContains("Could not resolve a Big ID"); + } + + [TestMethod] + public async Task Msixvc2PackageWithoutCapability_FailsAndDoesNotShellOut() + { + using var package = TempPackageFile.CreateMsixvc2(); + _toolProviderMock.SetupGet(x => x.IsAvailable).Returns(false); + // Exercises IMsixvc2UploadToolProvider's documented contract: ExecutablePath is null when + // IsAvailable is false. null! (not a behavior change - the value is still null) because this + // test project compiles with nullable reference types while the interface's project does not. + _toolProviderMock.SetupGet(x => x.ExecutablePath).Returns((string)null!); + + var result = await CreateOperation(CreateConfig(package.Path)).RunAsync(CancellationToken.None); + + Assert.AreEqual(3, result); + _processRunnerMock.VerifyNoOtherCalls(); + _loggerMock.VerifyLogErrorContains("no MSIXVC2-capable MakePkg.exe was found"); + } + + /// + /// Circular-dependency guard: MakePkg.exe shells back out to PackageUploader.exe for XVC1/MSIXVC1 + /// uploads, so a non-MSIXVC2 package must NEVER be delegated to MakePkg.exe. + /// + [TestMethod] + public async Task NonMsixvc2Package_NeverShellsOut() + { + using var package = TempPackageFile.CreateLegacyXvc(); + SetUpAvailableTool(); + _serviceMock + .Setup(x => x.GetProductByBigIdAsync(It.IsAny(), It.IsAny())) + .ThrowsAsync(new InvalidOperationException("took the normal XVC upload path")); + + var result = await CreateOperation(CreateConfig(package.Path)).RunAsync(CancellationToken.None); + + Assert.AreEqual(3, result); + _processRunnerMock.VerifyNoOtherCalls(); + _serviceMock.Verify(x => x.GetProductByBigIdAsync(BigId, It.IsAny()), Times.Once); + } + + /// + /// The legacy XVC path must be byte-for-byte identical: the same ingestion calls in the same order, + /// with the same arguments, and no MakePkg.exe involvement. + /// + [TestMethod] + public async Task NonMsixvc2Package_TakesUnchangedLegacyUploadPath() + { + using var package = TempPackageFile.CreateLegacyXvc(); + SetUpAvailableTool(); + + var config = CreateConfig(package.Path); + config.GameAssets = new GameAssets { EkbFilePath = "ekb", SubValFilePath = "sub" }; + + var product = CreateProduct("1234567890", BigId); + var branch = (GamePackageBranch)RuntimeHelpers.GetUninitializedObject(typeof(GamePackageBranch)); + var marketGroupPackage = (GameMarketGroupPackage)RuntimeHelpers.GetUninitializedObject(typeof(GameMarketGroupPackage)); + typeof(GameMarketGroupPackage).GetProperty("Name")!.SetValue(marketGroupPackage, "default"); + var packageConfiguration = (GamePackageConfiguration)RuntimeHelpers.GetUninitializedObject(typeof(GamePackageConfiguration)); + typeof(GamePackageConfiguration).GetProperty("MarketGroupPackages")!.SetValue(packageConfiguration, new List { marketGroupPackage }); + var gamePackage = (GamePackage)RuntimeHelpers.GetUninitializedObject(typeof(GamePackage)); + + _serviceMock.Setup(x => x.GetProductByBigIdAsync(BigId, It.IsAny())).ReturnsAsync(product); + _serviceMock.Setup(x => x.GetPackageBranchByFriendlyNameAsync(product, "Main", It.IsAny())).ReturnsAsync(branch); + _serviceMock.Setup(x => x.GetPackageConfigurationAsync(product, branch, It.IsAny())).ReturnsAsync(packageConfiguration); + _serviceMock + .Setup(x => x.UploadGamePackageAsync(product, branch, marketGroupPackage, package.Path, config.GameAssets, 30, false, true, It.IsAny())) + .ReturnsAsync(gamePackage); + + var result = await CreateOperation(config).RunAsync(CancellationToken.None); + + Assert.AreEqual(0, result); + _processRunnerMock.VerifyNoOtherCalls(); + _serviceMock.Verify( + x => x.UploadGamePackageAsync(product, branch, marketGroupPackage, package.Path, config.GameAssets, 30, false, true, It.IsAny()), + Times.Once); + _serviceMock.Verify( + x => x.SetXvcConfigurationAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + } + + [TestMethod] + public async Task MissingPackageFile_NeverShellsOut() + { + SetUpAvailableTool(); + _serviceMock + .Setup(x => x.GetProductByBigIdAsync(It.IsAny(), It.IsAny())) + .ThrowsAsync(new InvalidOperationException("took the normal XVC upload path")); + + var result = await CreateOperation(CreateConfig(@"C:\does\not\exist.msixvc")).RunAsync(CancellationToken.None); + + Assert.AreEqual(3, result); + _processRunnerMock.VerifyNoOtherCalls(); + } + + /// + /// A config option with no MakePkg.exe equivalent must fail fast rather than be silently dropped. + /// Certificate-subject authentication is the case in point: MakePkg.exe selects a certificate by + /// thumbprint only, so honouring this would mean guessing which certificate the user meant. + /// + [TestMethod] + public async Task UnsupportedConfigOption_FailsWithExplicitErrorAndDoesNotShellOut() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + + var operation = new UploadXvcPackageOperation( + _serviceMock.Object, + _loggerMock.Object, + Options.Create(CreateConfig(package.Path)), + _toolProviderMock.Object, + _processRunnerMock.Object, + _delegationGuardMock.Object, + new Msixvc2CommandLineContext( + IngestionExtensions.AuthenticationMethod.ClientCertificate, + TenantId: "tenant-1", + ClientId: "client-1", + CertificateSubject: "CN=Contoso")); + + var result = await operation.RunAsync(CancellationToken.None); + + Assert.AreEqual(3, result); + _processRunnerMock.VerifyNoOtherCalls(); + _loggerMock.VerifyLogErrorContains("Certificate subject authentication"); + } + + [TestMethod] + public async Task IgnorableConfigOption_WarnsAndStillShellsOut() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + SetUpSuccessfulRun(); + + var config = CreateConfig(package.Path); + config.DeltaUpload = true; + + var result = await CreateOperation(config).RunAsync(CancellationToken.None); + + Assert.AreEqual(0, result); + _processRunnerMock.Verify( + x => x.RunAsync(ResolvedMakePkgPath, ExpectedArguments(package.Path), It.IsAny()), + Times.Once); + } + + [TestMethod] + public async Task NonZeroExitCode_FailsOperation() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + _processRunnerMock + .Setup(x => x.RunAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(new Msixvc2ProcessResult(7, null)); + + var result = await CreateOperation(CreateConfig(package.Path)).RunAsync(CancellationToken.None); + + Assert.AreEqual(3, result); + _loggerMock.VerifyLogErrorContains("MakePkg.exe failed with exit code 7"); + } + + /// + /// Loop-breaker, direction 1: a normal (non-delegated) invocation delegates as usual. + /// + [TestMethod] + public async Task DelegationGuardAbsent_Delegates() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + SetUpSuccessfulRun(); + _delegationGuardMock.SetupGet(x => x.IsDelegatedInvocation).Returns(false); + + var result = await CreateOperation(CreateConfig(package.Path)).RunAsync(CancellationToken.None); + + Assert.AreEqual(0, result); + _processRunnerMock.Verify( + x => x.RunAsync(ResolvedMakePkgPath, ExpectedArguments(package.Path), It.IsAny()), + Times.Once); + } + + /// + /// Loop-breaker, direction 2: when this process was itself launched by MakePkg.exe, never delegate back. + /// Format detection is a heuristic, so a false positive on an XVC1 package could otherwise produce + /// PackageUploader.exe -> MakePkg.exe -> PackageUploader.exe recursion without bound. + /// + [TestMethod] + public async Task DelegationGuardPresent_NeverShellsOutAndTakesLegacyPath() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + SetUpSuccessfulRun(); + _delegationGuardMock.SetupGet(x => x.IsDelegatedInvocation).Returns(true); + + _serviceMock + .Setup(x => x.GetProductByBigIdAsync(It.IsAny(), It.IsAny())) + .ThrowsAsync(new InvalidOperationException("took the normal XVC upload path")); + + var result = await CreateOperation(CreateConfig(package.Path)).RunAsync(CancellationToken.None); + + Assert.AreEqual(3, result); + _processRunnerMock.VerifyNoOtherCalls(); + _serviceMock.Verify(x => x.GetProductByBigIdAsync(BigId, It.IsAny()), Times.Once); + } + + [TestMethod] + public async Task DelegationGuardPresent_LogsWarning() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + _delegationGuardMock.SetupGet(x => x.IsDelegatedInvocation).Returns(true); + _serviceMock + .Setup(x => x.GetProductByBigIdAsync(It.IsAny(), It.IsAny())) + .ThrowsAsync(new InvalidOperationException("took the normal XVC upload path")); + + await CreateOperation(CreateConfig(package.Path)).RunAsync(CancellationToken.None); + + _loggerMock.VerifyLogWarningContains(Msixvc2DelegationGuard.EnvironmentVariableName); + } + + /// + /// Loop-breaker, direction 3: the environment stamp only covers cycles PackageUploader.exe itself + /// starts. When MakePkg.exe is the entry point it invokes us with no stamp, so a MakePkg.exe parent must + /// independently suppress delegation. + /// + [TestMethod] + public async Task MakePkgParentProcess_NeverShellsOutAndTakesLegacyPath() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + SetUpSuccessfulRun(); + _delegationGuardMock.SetupGet(x => x.IsDelegatedInvocation).Returns(false); + _delegationGuardMock.Setup(x => x.GetMakePkgParentProcessName()).Returns("MakePkg.exe"); + + _serviceMock + .Setup(x => x.GetProductByBigIdAsync(It.IsAny(), It.IsAny())) + .ThrowsAsync(new InvalidOperationException("took the normal XVC upload path")); + + var result = await CreateOperation(CreateConfig(package.Path)).RunAsync(CancellationToken.None); + + Assert.AreEqual(3, result); + _processRunnerMock.VerifyNoOtherCalls(); + _serviceMock.Verify(x => x.GetProductByBigIdAsync(BigId, It.IsAny()), Times.Once); + } + + [TestMethod] + public async Task MakePkgParentProcess_LogsWarningNamingTheParent() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + _delegationGuardMock.SetupGet(x => x.IsDelegatedInvocation).Returns(false); + _delegationGuardMock.Setup(x => x.GetMakePkgParentProcessName()).Returns("makepkg2.exe"); + _serviceMock + .Setup(x => x.GetProductByBigIdAsync(It.IsAny(), It.IsAny())) + .ThrowsAsync(new InvalidOperationException("took the normal XVC upload path")); + + await CreateOperation(CreateConfig(package.Path)).RunAsync(CancellationToken.None); + + _loggerMock.VerifyLogWarningContains("makepkg2.exe"); + } + + /// + /// The parent check must not become a blanket block: an ordinary parent (or an undeterminable one, which + /// the provider also reports as null) has to leave normal MSIXVC2 delegation working. + /// + [TestMethod] + public async Task NonMakePkgParentProcess_StillDelegates() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + SetUpSuccessfulRun(); + _delegationGuardMock.SetupGet(x => x.IsDelegatedInvocation).Returns(false); + // null is the documented "parent unknown / not MakePkg" value. null! (not a behavior change) because + // this test project compiles with nullable reference types while the interface's project does not. + _delegationGuardMock.Setup(x => x.GetMakePkgParentProcessName()).Returns((string)null!); + + var result = await CreateOperation(CreateConfig(package.Path)).RunAsync(CancellationToken.None); + + Assert.AreEqual(0, result); + _processRunnerMock.Verify( + x => x.RunAsync(ResolvedMakePkgPath, ExpectedArguments(package.Path), It.IsAny()), + Times.Once); + } + + /// + /// End-to-end proof of the CodeQL finding's underlying concern: with client-secret authentication the + /// secret must reach MakePkg.exe but must never appear in any log entry. + /// + [TestMethod] + public async Task Msixvc2WithClientSecret_PassesSecretToProcessButNeverLogsIt() + { + const string secret = "super-secret-value"; + + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + SetUpSuccessfulRun(); + + var operation = new UploadXvcPackageOperation( + _serviceMock.Object, + _loggerMock.Object, + Options.Create(CreateConfig(package.Path)), + _toolProviderMock.Object, + _processRunnerMock.Object, + _delegationGuardMock.Object, + new Msixvc2CommandLineContext( + IngestionExtensions.AuthenticationMethod.ClientSecret, + TenantId: "tenant-1", + ClientId: "client-1", + ClientSecret: secret)); + + var result = await operation.RunAsync(CancellationToken.None); + + Assert.AreEqual(0, result); + + // The child process gets the real credential... + _processRunnerMock.Verify( + x => x.RunAsync( + ResolvedMakePkgPath, + It.Is(a => a.Contains($"/clientsecret \"{secret}\"", StringComparison.Ordinal)), + It.IsAny()), + Times.Once); + + // ...and the logger never does, at any level. + _loggerMock.VerifyNeverLogged(secret); + } + + #region Availability and pre-download dates + + private static GamePackage CreateGamePackage(string id) + { + var package = (GamePackage)RuntimeHelpers.GetUninitializedObject(typeof(GamePackage)); + typeof(GamePackageResource).GetProperty("Id")!.SetValue(package, id); + return package; + } + + private static async IAsyncEnumerable AsAsync(params GamePackage[] packages) + { + foreach (var package in packages) + { + yield return package; + } + + await Task.CompletedTask; + } + + private (GameProduct Product, GamePackageBranch Branch) SetUpBranchWithPackages(params GamePackage[] packages) + { + var product = CreateProduct("1234567890", BigId); + var branch = (GamePackageBranch)RuntimeHelpers.GetUninitializedObject(typeof(GamePackageBranch)); + + _serviceMock + .Setup(x => x.GetProductByBigIdAsync(BigId, It.IsAny())) + .ReturnsAsync(product); + _serviceMock + .Setup(x => x.GetPackageBranchByFriendlyNameAsync(product, "Main", It.IsAny())) + .ReturnsAsync(branch); + _serviceMock + .Setup(x => x.GetGamePackagesAsync(product, branch, "default", It.IsAny())) + .Returns(AsAsync(packages)); + + return (product, branch); + } + + /// + /// The adoption case: MakePkg.exe uploads the package and reports its identity, and PackageUploader + /// applies the dates afterwards exactly as it does for XVC1. + /// + [TestMethod] + public async Task Msixvc2WithAvailabilityDate_AppliesDatesToTheReportedPackage() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + SetUpSuccessfulRun(); + + var uploaded = CreateGamePackage(UploadedPackageId); + var (product, branch) = SetUpBranchWithPackages(CreateGamePackage(Guid.NewGuid().ToString()), uploaded); + + var config = CreateConfig(package.Path); + config.AvailabilityDate = new GamePackageDate { IsEnabled = true, EffectiveDate = DateTime.UtcNow.AddDays(3) }; + + var result = await CreateOperation(config).RunAsync(CancellationToken.None); + + Assert.AreEqual(0, result); + _serviceMock.Verify( + x => x.SetXvcConfigurationAsync(product, branch, uploaded, "default", config, It.IsAny()), + Times.Once); + } + + /// + /// Without a configured date there is nothing to apply, so the delegated upload must not make any + /// ingestion calls at all — the same shape the pre-existing argument test asserts. + /// + [TestMethod] + public async Task Msixvc2WithoutDates_DoesNotTouchIngestion() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + SetUpSuccessfulRun(); + + var result = await CreateOperation(CreateConfig(package.Path)).RunAsync(CancellationToken.None); + + Assert.AreEqual(0, result); + _serviceMock.VerifyNoOtherCalls(); + } + + /// + /// If MakePkg.exe does not report an identity we must not guess. The upload has already succeeded, so + /// the failure has to say that plainly rather than reading as a failed upload. + /// + [TestMethod] + public async Task Msixvc2WithDatesButNoReportedPackageId_FailsWithoutSettingDates() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + _processRunnerMock + .Setup(x => x.RunAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(new Msixvc2ProcessResult(0, null)); + + var config = CreateConfig(package.Path); + config.AvailabilityDate = new GamePackageDate { IsEnabled = true, EffectiveDate = DateTime.UtcNow.AddDays(3) }; + + var result = await CreateOperation(config).RunAsync(CancellationToken.None); + + Assert.AreEqual(3, result); + _loggerMock.VerifyLogErrorContains("did not report which package it created"); + _loggerMock.VerifyLogErrorContains("The upload itself is unaffected"); + _serviceMock.Verify( + x => x.SetXvcConfigurationAsync( + It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + } + + /// + /// The reported identity is verified against the target market group rather than trusted. A package that + /// is not there must fail rather than have dates written somewhere else. + /// + [TestMethod] + public async Task Msixvc2WithDatesButPackageNotInMarketGroup_FailsWithoutSettingDates() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + SetUpSuccessfulRun(); + + SetUpBranchWithPackages(CreateGamePackage(Guid.NewGuid().ToString())); + + var config = CreateConfig(package.Path); + config.AvailabilityDate = new GamePackageDate { IsEnabled = true, EffectiveDate = DateTime.UtcNow.AddDays(3) }; + + var result = await CreateOperation(config).RunAsync(CancellationToken.None); + + Assert.AreEqual(3, result); + _loggerMock.VerifyLogErrorContains($"('{UploadedPackageId}') is not in market group 'default'"); + _serviceMock.Verify( + x => x.SetXvcConfigurationAsync( + It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + } + + /// + /// A disabled date is still a configured date: the XVC1 path calls through so the value is cleared, and + /// MSIXVC2 must not quietly differ. + /// + [TestMethod] + public async Task Msixvc2WithDisabledAvailabilityDate_StillAppliesConfiguration() + { + using var package = TempPackageFile.CreateMsixvc2(); + SetUpAvailableTool(); + SetUpSuccessfulRun(); + + var uploaded = CreateGamePackage(UploadedPackageId); + var (product, branch) = SetUpBranchWithPackages(uploaded); + + var config = CreateConfig(package.Path); + config.AvailabilityDate = new GamePackageDate { IsEnabled = false }; + + var result = await CreateOperation(config).RunAsync(CancellationToken.None); + + Assert.AreEqual(0, result); + _serviceMock.Verify( + x => x.SetXvcConfigurationAsync(product, branch, uploaded, "default", config, It.IsAny()), + Times.Once); + } + + #endregion + + [TestMethod] + public async Task Cancellation_PropagatesTokenAndFailsOperation() + { + using var package = TempPackageFile.CreateMsixvc2(); + using var cts = new CancellationTokenSource(); + SetUpAvailableTool(); + + var observedToken = CancellationToken.None; + _processRunnerMock + .Setup(x => x.RunAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((_, _, ct) => observedToken = ct) + .ThrowsAsync(new OperationCanceledException()); + + await cts.CancelAsync(); + + var result = await CreateOperation(CreateConfig(package.Path)).RunAsync(cts.Token); + + Assert.AreEqual(1, result); + Assert.IsTrue(observedToken.IsCancellationRequested, "The operation cancellation token must be handed to the process runner."); + } +} + diff --git a/src/PackageUploader.Application/Config/UploadXvcPackageOperationConfig.cs b/src/PackageUploader.Application/Config/UploadXvcPackageOperationConfig.cs index e5bc1d8e..86c21eff 100644 --- a/src/PackageUploader.Application/Config/UploadXvcPackageOperationConfig.cs +++ b/src/PackageUploader.Application/Config/UploadXvcPackageOperationConfig.cs @@ -3,6 +3,7 @@ using Microsoft.Extensions.Options; using PackageUploader.ClientApi.Models; +using PackageUploader.ClientApi.Packaging; using System.Collections.Generic; using System.ComponentModel.DataAnnotations; @@ -15,7 +16,9 @@ internal class UploadXvcPackageOperationConfig : UploadPackageOperationConfig, I { internal override string GetOperationName() => "UploadXvcPackage"; - [Required] + // Not [Required] at the attribute level: MSIXVC2 packages are uploaded by MakePkg.exe, which has no + // concept of EKB/submission-validator assets. Requiredness is enforced in Validate() for every + // non-MSIXVC2 package, so XVC1 behaviour is unchanged. [ValidateObjectMembers] public GameAssets GameAssets { get; set; } @@ -28,6 +31,11 @@ internal class UploadXvcPackageOperationConfig : UploadPackageOperationConfig, I foreach (var validationResult in base.Validate(validationContext)) yield return validationResult; + if (GameAssets is null && !PackageFormatDetector.IsLikelyMsixvc2Package(PackageFilePath)) + { + yield return new ValidationResult($"The {nameof(GameAssets)} field is required.", [nameof(GameAssets)]); + } + if (PreDownloadDate is { IsEnabled: true, EffectiveDate: null }) { yield return new ValidationResult($"If {nameof(PreDownloadDate)} {nameof(PreDownloadDate.IsEnabled)} is true, {nameof(PreDownloadDate.EffectiveDate)} needs to be set.", [nameof(PreDownloadDate)]); diff --git a/src/PackageUploader.Application/Extensions/HostExtensions.cs b/src/PackageUploader.Application/Extensions/HostExtensions.cs index 4e4c9c22..0292cc59 100644 --- a/src/PackageUploader.Application/Extensions/HostExtensions.cs +++ b/src/PackageUploader.Application/Extensions/HostExtensions.cs @@ -8,8 +8,10 @@ using Microsoft.Extensions.Options; using PackageUploader.Application.Config; using PackageUploader.Application.Operations; +using PackageUploader.Application.Tools; using PackageUploader.ClientApi; using PackageUploader.ClientApi.Client.Ingestion.TokenProvider.Models; +using PackageUploader.ClientApi.Tools; using PackageUploader.FileLogger; using System; using System.Collections.Generic; @@ -73,6 +75,15 @@ internal static HostApplicationBuilder ConfigureServices(this HostApplicationBui hostAppBuilder.Services.AddSingleton(new DataOutputOptions(isData)); hostAppBuilder.Services.AddPackageUploaderService(parseResult.GetValue(CommandLineHelper.AuthenticationMethodOption)); + hostAppBuilder.Services.AddSingleton(BuildMsixvc2CommandLineContext(hostAppBuilder, parseResult)); + hostAppBuilder.Services.AddSingleton(); + hostAppBuilder.Services.AddSingleton(); + hostAppBuilder.Services.AddSingleton(); + hostAppBuilder.Services.AddMsixvc2ToolResolver(); + // Scoped, not singleton: the adapter resolves once per instance, so a scoped lifetime gives + // each operation a fresh resolution instead of one cached for the life of the process. + hostAppBuilder.Services.AddScoped(); + hostAppBuilder.Services .AddScoped() .AddSingleton, GetProductOperationValidator>() @@ -111,6 +122,45 @@ internal static HostApplicationBuilder ConfigureServices(this HostApplicationBui return hostAppBuilder; } + /// + /// Collects the authentication surface MakePkg.exe needs for MSIXVC2 uploads. PackageUploader spreads + /// this across a command line option (--Authentication) and several configuration sections, and + /// MakePkg.exe accepts the same credential material through /auth, /tenantid, /clientid, + /// /clientsecret, /certthumbprint, /certstore, /certlocation and /resourceid — so a CI pipeline using + /// a service principal keeps working when the upload is delegated. + /// + private static Msixvc2CommandLineContext BuildMsixvc2CommandLineContext(HostApplicationBuilder hostAppBuilder, ParseResult parseResult) + { + var configuration = hostAppBuilder.Configuration; + var aad = configuration.GetSection(AadAuthInfo.ConfigName); + var clientSecret = configuration.GetSection(ClientSecretAuthInfo.ConfigName); + var clientCertificate = configuration.GetSection(ClientCertificateAuthInfo.ConfigName); + + static string First(params string[] values) => + values.FirstOrDefault(v => !string.IsNullOrWhiteSpace(v)); + + return new Msixvc2CommandLineContext( + parseResult.GetValue(CommandLineHelper.AuthenticationMethodOption), + TenantId: First( + parseResult.GetValue(CommandLineHelper.TenantIdOption), + aad[nameof(AadAuthInfo.TenantId)], + clientSecret[nameof(ClientSecretAuthInfo.TenantId)], + clientCertificate[nameof(ClientCertificateAuthInfo.TenantId)]), + ClientId: First( + aad[nameof(AadAuthInfo.ClientId)], + clientSecret[nameof(ClientSecretAuthInfo.ClientId)], + clientCertificate[nameof(ClientCertificateAuthInfo.ClientId)]), + ClientSecret: First( + aad[nameof(AzureApplicationSecretAuthInfo.ClientSecret)], + clientSecret[nameof(ClientSecretAuthInfo.ClientSecret)]), + CertificateThumbprint: aad[nameof(AzureApplicationCertificateAuthInfo.CertificateThumbprint)], + CertificateSubject: aad[nameof(AzureApplicationCertificateAuthInfo.CertificateSubject)], + CertificateStore: aad[nameof(AzureApplicationCertificateAuthInfo.CertificateStore)], + CertificateLocation: aad[nameof(AzureApplicationCertificateAuthInfo.CertificateLocation)], + CertificatePath: clientCertificate[nameof(ClientCertificateAuthInfo.CertificatePath)], + ResourceId: aad["ResourceId"]); + } + internal static HostApplicationBuilder ConfigureAppConfiguration(this HostApplicationBuilder hostAppBuilder, ParseResult parseResult) { var configFile = parseResult.GetValue(CommandLineHelper.ConfigFileOption); diff --git a/src/PackageUploader.Application/Operations/UploadXvcPackageOperation.cs b/src/PackageUploader.Application/Operations/UploadXvcPackageOperation.cs index aad9118e..14f2962e 100644 --- a/src/PackageUploader.Application/Operations/UploadXvcPackageOperation.cs +++ b/src/PackageUploader.Application/Operations/UploadXvcPackageOperation.cs @@ -1,27 +1,98 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using PackageUploader.Application.Config; using PackageUploader.Application.Extensions; +using PackageUploader.Application.Tools; using PackageUploader.ClientApi; +using PackageUploader.ClientApi.Client.Ingestion.Models; +using PackageUploader.ClientApi.Packaging; using System; using System.Threading; using System.Threading.Tasks; namespace PackageUploader.Application.Operations; -internal class UploadXvcPackageOperation(IPackageUploaderService storeBrokerService, ILogger logger, IOptions config) : Operation(logger) +internal class UploadXvcPackageOperation( + IPackageUploaderService storeBrokerService, + ILogger logger, + IOptions config, + IMsixvc2UploadToolProvider msixvc2ToolProvider, + IMsixvc2ProcessRunner msixvc2ProcessRunner, + IMsixvc2DelegationGuard msixvc2DelegationGuard, + Msixvc2CommandLineContext msixvc2CommandLineContext) : Operation(logger) { private readonly IPackageUploaderService _storeBrokerService = storeBrokerService ?? throw new ArgumentNullException(nameof(storeBrokerService)); private readonly ILogger _logger = logger ?? throw new ArgumentNullException(nameof(logger)); private readonly UploadXvcPackageOperationConfig _config = config?.Value ?? throw new ArgumentNullException(nameof(config)); + private readonly IMsixvc2UploadToolProvider _msixvc2ToolProvider = msixvc2ToolProvider ?? throw new ArgumentNullException(nameof(msixvc2ToolProvider)); + private readonly IMsixvc2ProcessRunner _msixvc2ProcessRunner = msixvc2ProcessRunner ?? throw new ArgumentNullException(nameof(msixvc2ProcessRunner)); + private readonly IMsixvc2DelegationGuard _msixvc2DelegationGuard = msixvc2DelegationGuard ?? throw new ArgumentNullException(nameof(msixvc2DelegationGuard)); + private readonly Msixvc2CommandLineContext _msixvc2CommandLineContext = msixvc2CommandLineContext ?? throw new ArgumentNullException(nameof(msixvc2CommandLineContext)); protected override async Task ProcessAsync(CancellationToken ct) { _logger.LogInformation("Starting {operationName} operation.", _config.GetOperationName()); + // SAFETY: only MSIXVC2 packages may be delegated to MakePkg.exe. MakePkg.exe shells back out to + // PackageUploader.exe for XVC1/MSIXVC1 uploads, so delegating any other package format here would + // create an infinite process recursion between the two executables. This guard is deliberately the + // package-format detection itself (not a config flag) so it cannot be bypassed by configuration. + if (PackageFormatDetector.IsLikelyMsixvc2Package(_config.PackageFilePath)) + { + // SAFETY (defense in depth): format detection is a heuristic and can false-positive on an XVC1 + // package whose encrypted tail happens to contain the ZIP end-of-central-directory signature. + // Two independent signals say "MakePkg.exe is already in this call chain", and either one means + // delegating again risks the unbounded cycle above. + // + // Both fall through to the normal XVC1 upload rather than failing, which is the outcome that is + // correct either way: for a false-positive XVC1 package the upload simply succeeds, and for a + // genuine MSIXVC2 package it fails, which is what an un-delegatable MSIXVC2 package should do. + if (_msixvc2DelegationGuard.IsDelegatedInvocation) + { + _logger.LogWarning( + "Package '{PackageFilePath}' looks like MSIXVC2, but this PackageUploader process was started by MakePkg.exe ({EnvironmentVariable} is set). " + + "Uploading directly instead of delegating back to MakePkg.exe, to avoid an infinite MakePkg.exe/PackageUploader.exe loop.", + _config.PackageFilePath, + Msixvc2DelegationGuard.EnvironmentVariableName); + + await UploadXvcPackageAsync(ct).ConfigureAwait(false); + return; + } + + // The environment stamp above only covers cycles this executable started. When MakePkg.exe is the + // entry point it invokes us without any stamp, so the parent process is checked too. MakePkg.exe + // only invokes PackageUploader.exe for XVC1/MSIXVC1 packages, so a MakePkg.exe parent contradicts + // the MSIXVC2 detection, and the parent is the more trustworthy of the two signals. + var makePkgParent = _msixvc2DelegationGuard.GetMakePkgParentProcessName(); + + if (makePkgParent is not null) + { + _logger.LogWarning( + "Package '{PackageFilePath}' looks like MSIXVC2, but PackageUploader was started by '{ParentProcessName}', which only hands over " + + "XVC1/MSIXVC1 packages. Treating the package as XVC1 and uploading directly, to avoid an infinite MakePkg.exe/PackageUploader.exe loop.", + _config.PackageFilePath, + makePkgParent); + + await UploadXvcPackageAsync(ct).ConfigureAwait(false); + return; + } + + await UploadMsixvc2PackageAsync(ct).ConfigureAwait(false); + return; + } + + await UploadXvcPackageAsync(ct).ConfigureAwait(false); + } + + /// + /// The original, unchanged XVC1/MSIXVC1 upload path, which is also the fallback whenever an MSIXVC2 + /// detection cannot be acted on because MakePkg.exe is already in the process chain. + /// + private async Task UploadXvcPackageAsync(CancellationToken ct) + { var product = await _storeBrokerService.GetProductAsync(_config, ct).ConfigureAwait(false); var packageBranch = await _storeBrokerService.GetGamePackageBranch(product, _config, ct).ConfigureAwait(false); var marketGroupPackage = await _storeBrokerService.GetGameMarketGroupPackage(product, packageBranch, _config, ct).ConfigureAwait(false); @@ -36,4 +107,123 @@ protected override async Task ProcessAsync(CancellationToken ct) _logger.LogInformation("Configuration set for Xvc packages"); } } -} \ No newline at end of file + + /// + /// Delegates the upload of an MSIXVC2 package to MakePkg.exe, which owns the MSIXVC2 upload protocol. + /// Only ever reached when the package has been positively identified as MSIXVC2. + /// + private async Task UploadMsixvc2PackageAsync(CancellationToken ct) + { + _logger.LogInformation("MSIXVC2 package detected. Delegating the upload to MakePkg.exe."); + + if (!_msixvc2ToolProvider.IsAvailable || string.IsNullOrWhiteSpace(_msixvc2ToolProvider.ExecutablePath)) + { + throw new InvalidOperationException( + "The package is an MSIXVC2 package, but no MSIXVC2-capable MakePkg.exe was found. " + + "Install the latest Microsoft GDK and try again."); + } + + var bigId = await ResolveBigIdAsync(ct).ConfigureAwait(false); + + var executablePath = _msixvc2ToolProvider.ExecutablePath; + var arguments = Msixvc2UploadArgumentBuilder.Build(_config, _msixvc2CommandLineContext, bigId, _logger); + + // Only the redacted form is ever logged. It is built from credential-free inputs rather than + // scrubbed after the fact, so no credential reaches the logger. The child process below still + // receives the real command line. + _logger.LogInformation("Running {executablePath} {arguments}", executablePath, arguments.RedactedCommandLine); + + var result = await _msixvc2ProcessRunner.RunAsync(executablePath, arguments.CommandLine, ct).ConfigureAwait(false); + + if (result.ExitCode != 0) + { + throw new InvalidOperationException($"MakePkg.exe failed with exit code {result.ExitCode}."); + } + + _logger.LogInformation("MSIXVC2 package uploaded successfully."); + + // Mirrors the XVC1 condition exactly, so a configured date behaves the same on both paths — + // including a disabled date, which clears any previously set value rather than being a no-op. + if (_config.AvailabilityDate is not null || _config.PreDownloadDate is not null) + { + await SetMsixvc2ConfigurationAsync(result.UploadedPackageId, ct).ConfigureAwait(false); + } + } + + /// + /// Applies availability and pre-download dates to the package MakePkg.exe just uploaded. + /// + /// MakePkg.exe does not set these itself, but it does name the package it created, so the same + /// ingestion call the XVC1 path uses can be reused. The reported identity is resolved against the + /// packages actually present in the target branch and market group rather than being trusted + /// outright: that both yields the real and proves the identity belongs + /// where the dates are about to be written. + /// + /// Every failure here is loud. The upload has already succeeded at this point, so silently skipping + /// the dates would leave a package live on a date the caller never asked for. + /// + private async Task SetMsixvc2ConfigurationAsync(string uploadedPackageId, CancellationToken ct) + { + if (string.IsNullOrWhiteSpace(uploadedPackageId)) + { + throw new InvalidOperationException( + "The MSIXVC2 package uploaded successfully, but MakePkg.exe did not report which package it created, " + + "so 'availabilityDate'/'preDownloadDate' could not be applied. The upload itself is unaffected. " + + "Set the dates in Partner Center, or re-run this operation without them once the dates are set."); + } + + var product = await _storeBrokerService.GetProductAsync(_config, ct).ConfigureAwait(false); + var packageBranch = await _storeBrokerService.GetGamePackageBranch(product, _config, ct).ConfigureAwait(false); + + GamePackage gamePackage = null; + + await foreach (var package in _storeBrokerService + .GetGamePackagesAsync(product, packageBranch, _config.MarketGroupName, ct) + .ConfigureAwait(false)) + { + if (string.Equals(package.Id, uploadedPackageId, StringComparison.OrdinalIgnoreCase)) + { + gamePackage = package; + break; + } + } + + if (gamePackage is null) + { + throw new InvalidOperationException( + $"The MSIXVC2 package uploaded successfully, but the package MakePkg.exe reported ('{uploadedPackageId}') is not in " + + $"market group '{_config.MarketGroupName}', so 'availabilityDate'/'preDownloadDate' were not applied. " + + "The upload itself is unaffected. Set the dates in Partner Center."); + } + + _logger.LogInformation("Uploaded package with id: {gamePackageId}", gamePackage.Id); + + await _storeBrokerService.SetXvcConfigurationAsync(product, packageBranch, gamePackage, _config.MarketGroupName, _config, ct).ConfigureAwait(false); + _logger.LogInformation("Configuration set for Xvc packages"); + } + + /// + /// MakePkg.exe identifies the product by Store ID (/storeid) only. When the config supplies a ProductId + /// instead, resolve it to the corresponding Big ID through the ingestion service rather than failing. + /// + private async Task ResolveBigIdAsync(CancellationToken ct) + { + if (!string.IsNullOrWhiteSpace(_config.BigId)) + { + return _config.BigId; + } + + _logger.LogInformation("Resolving Big ID for product {productId}, which MakePkg.exe requires for MSIXVC2 uploads.", _config.ProductId); + + var product = await _storeBrokerService.GetProductAsync(_config, ct).ConfigureAwait(false); + + if (string.IsNullOrWhiteSpace(product?.BigId)) + { + throw new InvalidOperationException( + $"Could not resolve a Big ID for product '{_config.ProductId}'. MakePkg.exe identifies products by Store ID for MSIXVC2 uploads; " + + "set 'bigId' in the config file or pass --BigId."); + } + + return product.BigId; + } +} diff --git a/src/PackageUploader.Application/Properties/AssemblyInfo.cs b/src/PackageUploader.Application/Properties/AssemblyInfo.cs index 34cc0624..28d91ecb 100644 --- a/src/PackageUploader.Application/Properties/AssemblyInfo.cs +++ b/src/PackageUploader.Application/Properties/AssemblyInfo.cs @@ -3,4 +3,7 @@ using System.Runtime.CompilerServices; -[assembly: InternalsVisibleTo("PackageUploader.Application.Test")] \ No newline at end of file +[assembly: InternalsVisibleTo("PackageUploader.Application.Test")] + +// Allows Moq to create proxies for internal interfaces (IMsixvc2UploadToolProvider, IMsixvc2ProcessRunner) in tests. +[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")] \ No newline at end of file diff --git a/src/PackageUploader.Application/Tools/IMsixvc2ProcessRunner.cs b/src/PackageUploader.Application/Tools/IMsixvc2ProcessRunner.cs new file mode 100644 index 00000000..472d1c89 --- /dev/null +++ b/src/PackageUploader.Application/Tools/IMsixvc2ProcessRunner.cs @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Threading; +using System.Threading.Tasks; + +namespace PackageUploader.Application.Tools; + +/// +/// Runs an external process, streaming its output, and reports the exit code together with the identity +/// of the package MakePkg.exe uploaded. +/// +internal interface IMsixvc2ProcessRunner +{ + Task RunAsync(string executablePath, string arguments, CancellationToken ct); +} diff --git a/src/PackageUploader.Application/Tools/IMsixvc2UploadToolProvider.cs b/src/PackageUploader.Application/Tools/IMsixvc2UploadToolProvider.cs new file mode 100644 index 00000000..1fccfa32 --- /dev/null +++ b/src/PackageUploader.Application/Tools/IMsixvc2UploadToolProvider.cs @@ -0,0 +1,39 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace PackageUploader.Application.Tools; + +/// +/// Narrow abstraction over "is an MSIXVC2-capable MakePkg.exe available, and where is it?". +/// PackageUploader.Application depends only on this interface so that the underlying capability +/// resolution can be swapped without touching any consuming code. +/// +/// CONTRACT — implementations must honor all of the following. These are stated here because this +/// project does not compile with nullable reference types, so the compiler cannot express them: +/// +/// is false exactly when no MSIXVC2-capable tool could be found. +/// is NULL OR EMPTY whenever is false, and +/// is a usable full path whenever it is true. Callers must tolerate a null path. +/// Neither member may THROW when no tool is available. "Unavailable" is an ordinary, expected +/// outcome that reports as a clean, actionable error; +/// an exception escaping either member would instead surface as an unhandled failure. +/// Both members should reflect a SINGLE underlying resolution. An implementation that resolves +/// separately per member can double any probing work and can disagree with itself between the two reads, +/// so resolve once and have both members report that one result. +/// +/// +internal interface IMsixvc2UploadToolProvider +{ + /// + /// True when an MSIXVC2-capable packaging tool is installed and usable. Never throws; + /// "not available" is reported as false rather than as an exception. + /// + bool IsAvailable { get; } + + /// + /// Full path to the resolved MakePkg.exe (or makepkg2.exe fallback). + /// NULL OR EMPTY when is false — callers must check before use. + /// Never throws. + /// + string ExecutablePath { get; } +} diff --git a/src/PackageUploader.Application/Tools/IParentProcessProvider.cs b/src/PackageUploader.Application/Tools/IParentProcessProvider.cs new file mode 100644 index 00000000..34a31875 --- /dev/null +++ b/src/PackageUploader.Application/Tools/IParentProcessProvider.cs @@ -0,0 +1,30 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace PackageUploader.Application.Tools; + +/// +/// Reports which executable started this PackageUploader process, so the MSIXVC2 delegation path can +/// recognize that it was launched by MakePkg.exe. Seamed as an interface because the real implementation +/// depends on OS process interop, which a unit test cannot arrange. +/// +/// CONTRACT — implementations must honor all of the following. This project does not compile with nullable +/// reference types, so the compiler cannot express them: +/// +/// returns the parent's file name, WITHOUT directory, and with +/// the extension when one is known (for example MakePkg.exe). +/// It returns NULL OR EMPTY when the parent cannot be determined — the parent already exited, the +/// platform is not supported, or the OS denied the query. Callers must tolerate null and must treat it as +/// "unknown", never as "not MakePkg.exe with certainty". +/// It must NEVER THROW. Parent lookup is best-effort diagnostics on a path whose failure mode is a +/// blocked upload; an exception escaping here would turn a missing safety signal into a crash. +/// +/// +internal interface IParentProcessProvider +{ + /// + /// File name of the process that started this one (for example MakePkg.exe), or null/empty when + /// the parent cannot be determined. Never throws. + /// + string GetParentProcessFileName(); +} diff --git a/src/PackageUploader.Application/Tools/Msixvc2CommandLineContext.cs b/src/PackageUploader.Application/Tools/Msixvc2CommandLineContext.cs new file mode 100644 index 00000000..024e318b --- /dev/null +++ b/src/PackageUploader.Application/Tools/Msixvc2CommandLineContext.cs @@ -0,0 +1,44 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using PackageUploader.ClientApi; + +namespace PackageUploader.Application.Tools; + +/// +/// Command line and configuration values that are not part of the bound operation configuration but are +/// still needed to build MakePkg.exe arguments — principally the authentication surface, which +/// PackageUploader resolves from a mixture of command line options and configuration sections. +/// +/// Every parameter except is OPTIONAL and may be null or empty — +/// which credentials are present depends on the authentication method the user chose. The builder decides +/// which ones a given method actually requires and fails with an actionable message when one is missing; +/// consumers must not assume any of them is populated. +/// +/// The --Authentication value the user selected. +/// AAD tenant, forwarded to MakePkg.exe via /tenantid. +/// AAD application (client) id, forwarded via /clientid. +/// AAD application secret, forwarded via /clientsecret. +/// Certificate thumbprint, forwarded via /certthumbprint. +/// +/// Certificate subject name. MakePkg.exe has no equivalent flag, so this is only carried so the builder +/// can fail with an actionable message instead of silently authenticating as the wrong identity. +/// +/// Certificate store name, forwarded via /certstore. +/// Certificate store location, forwarded via /certlocation. +/// +/// Path to a PFX/PKCS12 certificate file. MakePkg.exe exposes /certpassword but no flag naming the +/// certificate file itself, so this is only carried so the builder can fail with an actionable message. +/// +/// Azure resource id, forwarded via /resourceid for ManagedIdentityFederated. +internal sealed record Msixvc2CommandLineContext( + IngestionExtensions.AuthenticationMethod AuthenticationMethod, + string TenantId = null, + string ClientId = null, + string ClientSecret = null, + string CertificateThumbprint = null, + string CertificateSubject = null, + string CertificateStore = null, + string CertificateLocation = null, + string CertificatePath = null, + string ResourceId = null); diff --git a/src/PackageUploader.Application/Tools/Msixvc2DelegationGuard.cs b/src/PackageUploader.Application/Tools/Msixvc2DelegationGuard.cs new file mode 100644 index 00000000..51d94abe --- /dev/null +++ b/src/PackageUploader.Application/Tools/Msixvc2DelegationGuard.cs @@ -0,0 +1,92 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.IO; + +namespace PackageUploader.Application.Tools; + +/// +/// Second, independent barrier against PackageUploader.exe ↔ MakePkg.exe process recursion. +/// +/// The primary guard is package-format detection, but PackageFormatDetector.IsLikelyMsixvc2Package +/// is a heuristic: its fallback check scans the trailing bytes of the file for the 4-byte ZIP +/// end-of-central-directory signature, which an encrypted XVC1 tail can contain by chance. A false positive +/// there would be unbounded, because MakePkg.exe shells back out to PackageUploader.exe for XVC1 uploads +/// (the legacy makepkg.exe help text literally describes its upload verb as operating "via the PackageUploader tool"): +/// +/// PackageUploader.exe → MakePkg.exe → PackageUploader.exe → MakePkg.exe → ... +/// +/// So PackageUploader stamps an environment variable onto every MakePkg.exe child process it starts, and +/// refuses to delegate again if it sees that variable already set in its own environment. Any MakePkg.exe +/// that shells back to us inherits the stamp, which breaks the cycle after exactly one hop regardless of +/// what the format heuristic decides. +/// +/// The environment stamp only covers cycles that PackageUploader.exe itself begins. When MakePkg.exe is the +/// entry point — a user runs it directly and it shells out to us for an XVC1 upload — nothing has stamped +/// our environment, so a third barrier inspects the actual parent process. Seeing MakePkg.exe there while +/// also detecting MSIXVC2 is a contradiction that must not be resolved by delegating back. +/// +internal interface IMsixvc2DelegationGuard +{ + /// + /// True when this PackageUploader process was itself started (directly or indirectly) by a MakePkg.exe + /// that we delegated to, meaning delegating again would risk an unbounded process cycle. + /// + bool IsDelegatedInvocation { get; } + + /// + /// File name of the parent process when PackageUploader was started by a MakePkg executable (for + /// example MakePkg.exe or makepkg2.exe), otherwise NULL. Null is also returned whenever the + /// parent cannot be determined, so a null means "not known to be MakePkg", never "definitely not + /// MakePkg". Never throws. + /// + string GetMakePkgParentProcessName(); +} + +/// +internal sealed class Msixvc2DelegationGuard(IParentProcessProvider parentProcessProvider) : IMsixvc2DelegationGuard +{ + /// Environment variable stamped onto the MakePkg.exe child process. + public const string EnvironmentVariableName = "PACKAGEUPLOADER_MSIXVC2_DELEGATED"; + + /// Value stamped onto the MakePkg.exe child process. + public const string EnvironmentVariableValue = "1"; + + /// + /// Executable names, without extension, that identify a MakePkg capable of shelling back out to + /// PackageUploader.exe. Both are covered: makepkg is the tool that performs XVC1 uploads "via the + /// PackageUploader tool", and makepkg2 is the MSIXVC2-capable tool we ourselves delegate to. + /// + private static readonly string[] MakePkgProcessNames = ["makepkg", "makepkg2"]; + + private readonly IParentProcessProvider _parentProcessProvider = + parentProcessProvider ?? throw new ArgumentNullException(nameof(parentProcessProvider)); + + public bool IsDelegatedInvocation => + !string.IsNullOrEmpty(Environment.GetEnvironmentVariable(EnvironmentVariableName)); + + public string GetMakePkgParentProcessName() + { + var parentFileName = _parentProcessProvider.GetParentProcessFileName(); + + if (string.IsNullOrWhiteSpace(parentFileName)) + { + return null; + } + + // The provider returns an extension when it can read one and a bare process name otherwise, so + // compare on the stem to accept both "MakePkg.exe" and "MakePkg". + var stem = Path.GetFileNameWithoutExtension(parentFileName); + + foreach (var makePkgName in MakePkgProcessNames) + { + if (string.Equals(stem, makePkgName, StringComparison.OrdinalIgnoreCase)) + { + return parentFileName; + } + } + + return null; + } +} diff --git a/src/PackageUploader.Application/Tools/Msixvc2ProcessResult.cs b/src/PackageUploader.Application/Tools/Msixvc2ProcessResult.cs new file mode 100644 index 00000000..77b4b110 --- /dev/null +++ b/src/PackageUploader.Application/Tools/Msixvc2ProcessResult.cs @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace PackageUploader.Application.Tools; + +/// +/// The outcome of a MakePkg.exe run. +/// +/// The child process exit code. Zero means the upload succeeded. +/// +/// CONTRACT: this is null whenever the package identity could not be established with certainty, which +/// covers three cases: MakePkg.exe never printed the identity, it printed something that was not a GUID, +/// or it printed two different identities. Callers must treat null as "unknown" and must never fall back +/// to guessing which package was uploaded, because the identity is used to write availability and +/// pre-download dates and writing them against the wrong package is worse than not writing them at all. +/// +/// This project does not enable nullable reference types, so the null case is stated here rather than +/// being expressed as an annotation. +/// +internal sealed record Msixvc2ProcessResult(int ExitCode, string UploadedPackageId); diff --git a/src/PackageUploader.Application/Tools/Msixvc2ProcessRunner.cs b/src/PackageUploader.Application/Tools/Msixvc2ProcessRunner.cs new file mode 100644 index 00000000..793e2786 --- /dev/null +++ b/src/PackageUploader.Application/Tools/Msixvc2ProcessRunner.cs @@ -0,0 +1,168 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Extensions.Logging; +using System; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; + +namespace PackageUploader.Application.Tools; + +/// +/// Drives MakePkg.exe the same way the UI does (see Msixvc2UploadingViewModel.RunMakePkg2ProcessAsync): +/// no shell execute, redirected stdout/stderr, no console window. Output is streamed through the +/// application logger so console users see live progress, and the child process is killed on cancellation. +/// +internal sealed class Msixvc2ProcessRunner(ILogger logger) : IMsixvc2ProcessRunner +{ + /// + /// MakePkg.exe announces the package it is uploading with an info-level line of the form + /// "Package Id is <guid>". Verified against makepkg2.exe 2604.405.14000.0, where the line is printed + /// at default verbosity (no /v required) and before the content transfer begins. + /// + private const string PackageIdMarker = "Package Id is "; + + private readonly ILogger _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + + public async Task RunAsync(string executablePath, string arguments, CancellationToken ct) + { + using var process = new Process + { + StartInfo = new ProcessStartInfo + { + FileName = executablePath, + Arguments = arguments, + UseShellExecute = false, + RedirectStandardOutput = true, + RedirectStandardError = true, + CreateNoWindow = true, + }, + EnableRaisingEvents = true, + }; + + // Recursion breaker: MakePkg.exe shells back out to PackageUploader.exe for XVC1 uploads. Stamping + // the child environment means any PackageUploader.exe started beneath us can see that it is already + // a delegated invocation and refuse to delegate again, bounding the cycle at a single hop even if + // the MSIXVC2 format heuristic false-positives. See Msixvc2DelegationGuard. + process.StartInfo.Environment[Msixvc2DelegationGuard.EnvironmentVariableName] = + Msixvc2DelegationGuard.EnvironmentVariableValue; + + // Kept local rather than on the instance so that concurrent or repeated runs cannot observe one + // another's package identity. Both output streams are scanned, so the lock is load-bearing. + var packageIdLock = new object(); + string uploadedPackageId = null; + var packageIdAmbiguous = false; + + void CapturePackageId(string line) + { + if (!TryParsePackageId(line, out var packageId)) + { + return; + } + + lock (packageIdLock) + { + if (uploadedPackageId is null) + { + uploadedPackageId = packageId; + } + else if (!string.Equals(uploadedPackageId, packageId, StringComparison.OrdinalIgnoreCase)) + { + // Two different identities means we cannot say which package the dates belong to. + packageIdAmbiguous = true; + } + } + } + + process.OutputDataReceived += (_, e) => + { + if (!string.IsNullOrEmpty(e.Data)) + { + _logger.LogInformation("[MakePkg] {Data}", e.Data); + CapturePackageId(e.Data); + } + }; + + process.ErrorDataReceived += (_, e) => + { + if (!string.IsNullOrEmpty(e.Data)) + { + _logger.LogWarning("[MakePkg] {Data}", e.Data); + CapturePackageId(e.Data); + } + }; + + process.Start(); + process.BeginOutputReadLine(); + process.BeginErrorReadLine(); + + try + { + await process.WaitForExitAsync(ct).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + KillProcess(process); + throw; + } + + if (packageIdAmbiguous) + { + _logger.LogWarning( + "MakePkg.exe reported more than one package id, so the uploaded package cannot be identified."); + + uploadedPackageId = null; + } + + return new Msixvc2ProcessResult(process.ExitCode, uploadedPackageId); + } + + /// + /// Pulls the package identity out of a MakePkg.exe output line. Deliberately strict: the value must be + /// a bare GUID in the canonical form, because a loose match risks reporting an identity that is not a + /// package and having availability dates written against it. + /// + private static bool TryParsePackageId(string line, out string packageId) + { + packageId = null; + + var markerIndex = line.IndexOf(PackageIdMarker, StringComparison.OrdinalIgnoreCase); + if (markerIndex < 0) + { + return false; + } + + var candidate = line[(markerIndex + PackageIdMarker.Length)..].Trim(); + + var separatorIndex = candidate.IndexOf(' '); + if (separatorIndex >= 0) + { + candidate = candidate[..separatorIndex]; + } + + if (!Guid.TryParseExact(candidate, "D", out var parsed)) + { + return false; + } + + packageId = parsed.ToString(); + return true; + } + + private void KillProcess(Process process) + { + try + { + if (!process.HasExited) + { + _logger.LogWarning("Cancellation requested. Terminating MakePkg.exe."); + process.Kill(entireProcessTree: true); + } + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Failed to terminate MakePkg.exe after cancellation."); + } + } +} diff --git a/src/PackageUploader.Application/Tools/Msixvc2ToolResolverAdapter.cs b/src/PackageUploader.Application/Tools/Msixvc2ToolResolverAdapter.cs new file mode 100644 index 00000000..27a2cfba --- /dev/null +++ b/src/PackageUploader.Application/Tools/Msixvc2ToolResolverAdapter.cs @@ -0,0 +1,82 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Extensions.Logging; +using PackageUploader.ClientApi.Tools; +using System; + +namespace PackageUploader.Application.Tools; + +/// +/// Adapts from PackageUploader.ClientApi to this project's +/// . +/// +/// Resolution is performed with no path hints, i.e. pure self-discovery (application directory, +/// current directory, the installed GDK, then PATH). The UI passes already-resolved paths because it +/// has its own file pickers; the CLI has no such input and deliberately relies on self-discovery. +/// +/// +/// +/// The underlying resolver intentionally does not cache: every Resolve() call re-probes a +/// candidate executable by launching it. This adapter therefore resolves EXACTLY ONCE and serves both +/// members from that single result, which satisfies the "single underlying resolution" clause of +/// 's contract. Reading both members must not launch the probe +/// twice, and must not be able to report an available tool alongside a null path if the environment +/// changes mid-operation. +/// +/// +/// This type is registered per-scope so that each operation gets a fresh resolution rather than one +/// cached for the lifetime of the process. +/// +/// +internal sealed class Msixvc2ToolResolverAdapter : IMsixvc2UploadToolProvider +{ + private readonly IMsixvc2ToolResolver _resolver; + private readonly ILogger _logger; + + private bool _resolved; + private Msixvc2Tool _tool; + + public Msixvc2ToolResolverAdapter(IMsixvc2ToolResolver resolver, ILogger logger) + { + _resolver = resolver ?? throw new ArgumentNullException(nameof(resolver)); + _logger = logger; + } + + /// + /// True when produced a tool. A null result means + /// "no MSIXVC2-capable tool is installed", which is an ordinary outcome, not an error. + /// + public bool IsAvailable => ResolveOnce() is not null; + + /// + /// The resolved executable path, or null when no capable tool was found. Always consistent + /// with because both read the same cached resolution. + /// + public string ExecutablePath => ResolveOnce()?.ExecutablePath; + + private Msixvc2Tool ResolveOnce() + { + if (_resolved) + { + return _tool; + } + + // The resolver is documented as never throwing, returning null for "nothing capable found". + // This catch is defense in depth only: UploadXvcPackageOperation is written to report an + // unavailable tool as a clean, actionable message, so an exception escaping here would be a + // new failure mode it cannot handle. Degrade to "unavailable" instead. + try + { + _tool = _resolver.Resolve(); + } + catch (Exception ex) + { + _logger?.LogDebug(ex, "MSIXVC2 tool resolution threw; treating MSIXVC2 as unavailable."); + _tool = null; + } + + _resolved = true; + return _tool; + } +} diff --git a/src/PackageUploader.Application/Tools/Msixvc2UnsupportedOptionException.cs b/src/PackageUploader.Application/Tools/Msixvc2UnsupportedOptionException.cs new file mode 100644 index 00000000..75b1c186 --- /dev/null +++ b/src/PackageUploader.Application/Tools/Msixvc2UnsupportedOptionException.cs @@ -0,0 +1,12 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; + +namespace PackageUploader.Application.Tools; + +/// +/// Thrown when the operation configuration contains an option that MakePkg.exe has no equivalent for. +/// The CLI fails fast rather than silently dropping the option. +/// +internal sealed class Msixvc2UnsupportedOptionException(string message) : Exception(message); diff --git a/src/PackageUploader.Application/Tools/Msixvc2UploadArgumentBuilder.cs b/src/PackageUploader.Application/Tools/Msixvc2UploadArgumentBuilder.cs new file mode 100644 index 00000000..6a60f988 --- /dev/null +++ b/src/PackageUploader.Application/Tools/Msixvc2UploadArgumentBuilder.cs @@ -0,0 +1,346 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Extensions.Logging; +using PackageUploader.Application.Config; +using PackageUploader.ClientApi; +using System; +using System.Collections.Generic; +using System.IO; +using System.Text; + +namespace PackageUploader.Application.Tools; + +/// +/// Translates an into a MakePkg.exe "upload" command line. +/// +/// Every flag emitted here is grounded in the verbatim help output of the MSIXVC2-capable packaging tool +/// (makepkg2.exe upload /?, version 2604.405.14000.0), cross-checked against the two existing UI +/// argument builders. The closest in-repo precedent is PackageUploadViewModel.BuildMsixvc2UploadArguments(), +/// which handles the same scenario as the CLI: an already-built .msixvc package on disk. That builder emits +/// the /pd form and deliberately does NOT pass /msixvc2 — that flag only appears in +/// Msixvc2UploadViewModel.BuildUploadArguments(), which packs from a loose content folder via /d. +/// +/// Notable grounding results: +/// +/// /auth accepts Default, Browser, CacheableBrowser, AzureCli, ManagedIdentity, +/// ManagedIdentityFederated, Environment, AzurePipelines, ClientSecret and ClientCertificate — so +/// non-interactive CI authentication is fully supported and is forwarded rather than rejected. +/// /tenantid, /clientid, /clientsecret, /certthumbprint, /certstore, +/// /certlocation and /resourceid all exist and carry the credential material. +/// There is no flag naming a certificate file, and no flag for a certificate subject, +/// so those two configurations are rejected rather than silently authenticating as a different identity. +/// /uploadsource exists but its enum only accepts makepkg2 and XGPM. There is no +/// value representing PackageUploader, so the flag is omitted and the tool's own default is used. +/// +/// +/// CAVEAT: the binary this mapping was verified against is makepkg2.exe, not the renamed +/// MakePkg.exe that ships with the GDK once the two tools are merged. The legacy makepkg.exe +/// is demonstrably a different surface (it has /tenantid but no /auth at all), so if the merged +/// MakePkg.exe diverges on /auth, this mapping — and especially +/// — is the first thing to re-verify against its help output. +/// +/// Options that MakePkg.exe has no equivalent for are either warned about and ignored (when ignoring them +/// cannot change the outcome) or cause a (when it could). +/// +/// Note that availabilityDate/preDownloadDate are deliberately NOT handled here. MakePkg.exe +/// has no flag for them, but they are still honoured: the operation applies them through ingestion after the +/// upload, using the package identity MakePkg.exe reports. This builder must therefore leave them alone +/// rather than treat them as unsupported. +/// +internal static class Msixvc2UploadArgumentBuilder +{ + /// Stands in for a credential in the command line built for logging. + private const string RedactedValue = "***"; + + /// + /// The /auth values accepted by the MSIXVC2 packaging tool, taken verbatim from its help output. + /// PackageUploader's own AuthenticationMethod enum uses the same names for all of these. + /// + private static readonly HashSet DirectlySupportedAuthenticationMethods = + [ + IngestionExtensions.AuthenticationMethod.Default, + IngestionExtensions.AuthenticationMethod.Browser, + IngestionExtensions.AuthenticationMethod.CacheableBrowser, + IngestionExtensions.AuthenticationMethod.AzureCli, + IngestionExtensions.AuthenticationMethod.ManagedIdentity, + IngestionExtensions.AuthenticationMethod.ManagedIdentityFederated, + IngestionExtensions.AuthenticationMethod.Environment, + IngestionExtensions.AuthenticationMethod.AzurePipelines, + IngestionExtensions.AuthenticationMethod.ClientSecret, + IngestionExtensions.AuthenticationMethod.ClientCertificate, + ]; + + public static Msixvc2UploadArguments Build( + UploadXvcPackageOperationConfig config, + Msixvc2CommandLineContext commandLineContext, + string bigId, + ILogger logger) + { + ArgumentNullException.ThrowIfNull(config); + ArgumentNullException.ThrowIfNull(commandLineContext); + ArgumentNullException.ThrowIfNull(logger); + ArgumentException.ThrowIfNullOrWhiteSpace(bigId); + + var packageDirectory = GetPackageDirectory(config.PackageFilePath); + + ValidateUnsupportedOptions(config, packageDirectory, logger); + + var commandLine = BuildCommandLine(config, commandLineContext, bigId, packageDirectory); + + // The log-safe form is BUILT FROM A CONTEXT THAT NEVER HELD THE SECRET rather than produced by + // scrubbing the finished command line. Post-hoc scrubbing has to keep a pattern in sync with the + // exact spelling, spacing and quoting the builder happens to emit, and silently leaks the moment + // those drift or a new credential flag is added. Substituting at the source cannot drift, and it + // keeps the secret out of the value that reaches the logger entirely. + var redactedCommandLine = string.IsNullOrWhiteSpace(commandLineContext.ClientSecret) + ? commandLine + : BuildCommandLine( + config, + commandLineContext with { ClientSecret = RedactedValue }, + bigId, + packageDirectory); + + return new Msixvc2UploadArguments(commandLine, redactedCommandLine); + } + + private static string BuildCommandLine( + UploadXvcPackageOperationConfig config, + Msixvc2CommandLineContext commandLineContext, + string bigId, + string packageDirectory) + { + var args = new StringBuilder(); + args.Append("upload"); + args.Append(Invariant($" /pd \"{packageDirectory}\"")); + + if (!string.IsNullOrWhiteSpace(config.BranchFriendlyName)) + { + args.Append(Invariant($" /branch \"{config.BranchFriendlyName}\"")); + } + else if (!string.IsNullOrWhiteSpace(config.FlightName)) + { + args.Append(Invariant($" /flight \"{config.FlightName}\"")); + } + + if (!string.IsNullOrWhiteSpace(config.MarketGroupName)) + { + args.Append(Invariant($" /market \"{config.MarketGroupName}\"")); + } + + args.Append(Invariant($" /storeid \"{bigId}\"")); + + AppendAuthenticationArguments(args, commandLineContext); + + return args.ToString(); + } + + /// + /// Forwards the selected authentication method and its credential material. MakePkg.exe performs the + /// token acquisition itself, so PackageUploader hands over the same identity the user configured rather + /// than forcing an interactive sign-in — otherwise MSIXVC2 upload would be impossible from any + /// non-interactive pipeline. + /// + private static void AppendAuthenticationArguments(StringBuilder args, Msixvc2CommandLineContext context) + { + var method = ResolveAuthenticationMethod(context.AuthenticationMethod); + + args.Append(Invariant($" /auth {method}")); + + if (!string.IsNullOrWhiteSpace(context.TenantId)) + { + args.Append(Invariant($" /tenantid \"{context.TenantId}\"")); + } + + if (!string.IsNullOrWhiteSpace(context.ClientId)) + { + args.Append(Invariant($" /clientid \"{context.ClientId}\"")); + } + + switch (method) + { + case IngestionExtensions.AuthenticationMethod.ClientSecret: + RequireCredential(context.ClientId, "/clientid", "a client id", "AadAuthInfo:ClientId or ClientSecretAuthInfo:ClientId"); + RequireCredential(context.ClientSecret, "/clientsecret", "a client secret", "AadAuthInfo:ClientSecret or ClientSecretAuthInfo:ClientSecret"); + args.Append(Invariant($" /clientsecret \"{context.ClientSecret}\"")); + break; + + case IngestionExtensions.AuthenticationMethod.ClientCertificate: + AppendCertificateArguments(args, context); + break; + + case IngestionExtensions.AuthenticationMethod.ManagedIdentityFederated: + if (!string.IsNullOrWhiteSpace(context.ResourceId)) + { + args.Append(Invariant($" /resourceid \"{context.ResourceId}\"")); + } + break; + } + } + + private static void AppendCertificateArguments(StringBuilder args, Msixvc2CommandLineContext context) + { + // makepkg2 authenticates from a certificate STORE (thumbprint + store + location). It exposes + // /certpassword but no flag naming a certificate file, so a PFX path cannot be forwarded. + if (!string.IsNullOrWhiteSpace(context.CertificatePath)) + { + throw new Msixvc2UnsupportedOptionException( + $"Certificate file authentication ('{context.CertificatePath}') cannot be forwarded to MakePkg.exe for MSIXVC2 uploads, " + + "because MakePkg.exe only accepts a certificate from a Windows certificate store (/certthumbprint, /certstore, /certlocation) " + + "and has no option naming a certificate file. " + + $"Import the certificate into a store and use --Authentication {IngestionExtensions.AuthenticationMethod.AppCert} " + + "with AadAuthInfo:CertificateThumbprint, or choose a different authentication method."); + } + + // makepkg2 has no certificate-subject option. Resolving the subject ourselves and forwarding the + // resulting thumbprint would be guesswork about which certificate the user meant. + if (!string.IsNullOrWhiteSpace(context.CertificateSubject)) + { + throw new Msixvc2UnsupportedOptionException( + $"Certificate subject authentication ('{context.CertificateSubject}') cannot be forwarded to MakePkg.exe for MSIXVC2 uploads, " + + "because MakePkg.exe selects certificates by thumbprint only. " + + "Set AadAuthInfo:CertificateThumbprint instead of AadAuthInfo:CertificateSubject."); + } + + RequireCredential(context.ClientId, "/clientid", "a client id", "AadAuthInfo:ClientId"); + RequireCredential(context.CertificateThumbprint, "/certthumbprint", "a certificate thumbprint", "AadAuthInfo:CertificateThumbprint"); + + args.Append(Invariant($" /certthumbprint \"{context.CertificateThumbprint}\"")); + + if (!string.IsNullOrWhiteSpace(context.CertificateStore)) + { + args.Append(Invariant($" /certstore \"{context.CertificateStore}\"")); + } + + if (!string.IsNullOrWhiteSpace(context.CertificateLocation)) + { + args.Append(Invariant($" /certlocation \"{context.CertificateLocation}\"")); + } + } + + /// + /// Maps PackageUploader's AuthenticationMethod onto a /auth value MakePkg.exe accepts. + /// All but two names are shared verbatim. AppSecret and AppCert are PackageUploader's legacy names for + /// the same AAD application flows that MakePkg.exe calls ClientSecret and ClientCertificate — both + /// authenticate an AAD application with, respectively, a client secret or a store certificate, so the + /// rename is a straight alias rather than a behavioral change. + /// + private static IngestionExtensions.AuthenticationMethod ResolveAuthenticationMethod( + IngestionExtensions.AuthenticationMethod method) => method switch + { + IngestionExtensions.AuthenticationMethod.AppSecret => IngestionExtensions.AuthenticationMethod.ClientSecret, + IngestionExtensions.AuthenticationMethod.AppCert => IngestionExtensions.AuthenticationMethod.ClientCertificate, + _ when DirectlySupportedAuthenticationMethods.Contains(method) => method, + _ => throw new Msixvc2UnsupportedOptionException( + $"--Authentication {method} has no MakePkg.exe equivalent for MSIXVC2 uploads. " + + "MakePkg.exe accepts: Default, Browser, CacheableBrowser, AzureCli, ManagedIdentity, " + + "ManagedIdentityFederated, Environment, AzurePipelines, ClientSecret, ClientCertificate."), + }; + + /// + /// Throws when a credential MakePkg.exe requires for the chosen method is missing. + /// may be null or empty — that is precisely the condition being detected. + /// + private static void RequireCredential(string value, string flag, string description, string configPath) + { + if (string.IsNullOrWhiteSpace(value)) + { + throw new Msixvc2UnsupportedOptionException( + $"MakePkg.exe requires {description} ({flag}) for this authentication method during an MSIXVC2 upload, " + + $"but none was configured. Set {configPath} in the config file."); + } + } + + private static void ValidateUnsupportedOptions( + UploadXvcPackageOperationConfig config, + string packageDirectory, + ILogger logger) + { + ValidateGameAssets(config, packageDirectory, logger); + + // MakePkg.exe owns the upload lifecycle and reports completion itself, so a caller-supplied + // processing timeout cannot change the outcome. An explicitly configured 30 is indistinguishable + // from the default, so this is always a warning rather than an error. + logger.LogWarning( + "'minutesToWaitForProcessing' is not used for MSIXVC2 uploads; MakePkg.exe manages upload processing itself."); + + if (config.DeltaUpload) + { + logger.LogWarning( + "'deltaUpload' is not used for MSIXVC2 uploads and will be ignored; MakePkg.exe decides its own chunk reuse strategy."); + } + } + + /// + /// MSIXVC2 packages do not use EKB or submission validator assets, and MakePkg.exe has no flags for them. + /// MakePkg.exe expects any such files to sit alongside the package, so assets already in the package + /// directory are harmless and are only warned about. Assets pointing elsewhere are a hard error, because + /// silently dropping a file the user deliberately placed somewhere else would change the outcome. + /// + private static void ValidateGameAssets(UploadXvcPackageOperationConfig config, string packageDirectory, ILogger logger) + { + if (config.GameAssets is null) + { + return; + } + + RequireInPackageDirectory(GameAssetPaths.EkbFilePath, config.GameAssets.EkbFilePath, packageDirectory); + RequireInPackageDirectory(GameAssetPaths.SubValFilePath, config.GameAssets.SubValFilePath, packageDirectory); + RequireInPackageDirectory(GameAssetPaths.SymbolsFilePath, config.GameAssets.SymbolsFilePath, packageDirectory); + RequireInPackageDirectory(GameAssetPaths.DiscLayoutFilePath, config.GameAssets.DiscLayoutFilePath, packageDirectory); + RequireInPackageDirectory(GameAssetPaths.SodbFilePath, config.GameAssets.SodbFilePath, packageDirectory); + + logger.LogWarning( + "'gameAssets' is not used for MSIXVC2 uploads and will be ignored. The configured assets already sit in the package directory '{PackageDirectory}', where MakePkg.exe expects them.", + packageDirectory); + } + + /// + /// Fails when a configured asset path sits outside the package directory. + /// may be null or empty, in which case the asset is simply not configured. + /// + private static void RequireInPackageDirectory(string propertyName, string assetPath, string packageDirectory) + { + if (string.IsNullOrWhiteSpace(assetPath)) + { + return; + } + + var assetDirectory = Path.GetDirectoryName(Path.GetFullPath(assetPath)); + + if (!string.Equals(assetDirectory, packageDirectory, StringComparison.OrdinalIgnoreCase)) + { + throw new Msixvc2UnsupportedOptionException( + $"'gameAssets.{ToCamelCase(propertyName)}' points at '{assetPath}', which is outside the package directory '{packageDirectory}'. " + + "MSIXVC2 uploads ignore gameAssets, and MakePkg.exe only picks up files that sit alongside the package, so this file would not be uploaded. " + + "Move it into the package directory or remove it from the config file."); + } + } + + private static string GetPackageDirectory(string packageFilePath) + { + // MakePkg.exe uploads an already-built package directory via /pd, not an individual package file. + var directory = Path.GetDirectoryName(Path.GetFullPath(packageFilePath)); + if (string.IsNullOrWhiteSpace(directory)) + { + throw new Msixvc2UnsupportedOptionException( + $"Could not determine the package directory for 'packageFilePath' value '{packageFilePath}'."); + } + + return directory; + } + + private static string ToCamelCase(string value) => char.ToLowerInvariant(value[0]) + value[1..]; + + private static string Invariant(FormattableString formattable) => FormattableString.Invariant(formattable); + + /// Names of the GameAssets properties, used for nameof() in error messages. + private static class GameAssetPaths + { + public const string EkbFilePath = nameof(EkbFilePath); + public const string SubValFilePath = nameof(SubValFilePath); + public const string SymbolsFilePath = nameof(SymbolsFilePath); + public const string DiscLayoutFilePath = nameof(DiscLayoutFilePath); + public const string SodbFilePath = nameof(SodbFilePath); + } +} diff --git a/src/PackageUploader.Application/Tools/Msixvc2UploadArguments.cs b/src/PackageUploader.Application/Tools/Msixvc2UploadArguments.cs new file mode 100644 index 00000000..20b978cc --- /dev/null +++ b/src/PackageUploader.Application/Tools/Msixvc2UploadArguments.cs @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace PackageUploader.Application.Tools; + +/// +/// The MakePkg.exe command line in two forms: the one to execute, and the one that is safe to log. +/// +/// They are returned together, and built together, so that a caller cannot accidentally log the executable +/// form. is not derived from by scrubbing it — +/// it is built independently from credential-free inputs, so no credential is ever present in the value +/// handed to a logger. +/// +/// +/// The real command line, including any credential material. Pass this to the process, never to a log. +/// +/// +/// The same command line with credential values replaced by a placeholder. Identical to +/// when there was no credential to replace. +/// +internal sealed record Msixvc2UploadArguments(string CommandLine, string RedactedCommandLine); diff --git a/src/PackageUploader.Application/Tools/ParentProcessProvider.cs b/src/PackageUploader.Application/Tools/ParentProcessProvider.cs new file mode 100644 index 00000000..52800898 --- /dev/null +++ b/src/PackageUploader.Application/Tools/ParentProcessProvider.cs @@ -0,0 +1,160 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.Diagnostics; +using System.Runtime.InteropServices; + +namespace PackageUploader.Application.Tools; + +/// +/// +/// Windows implementation. The .NET base class library exposes no parent-process API, so this reads the +/// parent process id out of the current process's own basic information block via +/// NtQueryInformationProcess, then resolves that id to a name. +/// +/// The query targets the CURRENT process pseudo-handle, so it needs no additional access rights and cannot +/// fail for permissions reasons. Only the subsequent name lookup can be denied, and that is tolerated. +/// +/// Deliberately free of struct marshalling: the buffer is read field-by-field through +/// so the whole path stays blittable and safe under PublishAot, which this project enables. +/// +/// Every failure mode returns null rather than throwing, per the interface contract. +/// +internal sealed class ParentProcessProvider : IParentProcessProvider +{ + /// ProcessBasicInformation, the PROCESSINFOCLASS value for the query below. + private const int ProcessBasicInformation = 0; + + /// + /// PROCESS_BASIC_INFORMATION is six pointer-sized fields: ExitStatus, PebBaseAddress, AffinityMask, + /// BasePriority, UniqueProcessId, InheritedFromUniqueProcessId. The parent id is the last of them. + /// (ExitStatus and BasePriority are 32-bit in C, but each is padded to pointer alignment.) + /// + private const int PointerFieldCount = 6; + private const int InheritedFromUniqueProcessIdFieldIndex = 5; + + [DllImport("ntdll.dll")] + private static extern int NtQueryInformationProcess( + IntPtr processHandle, + int processInformationClass, + IntPtr processInformation, + int processInformationLength, + IntPtr returnLength); + + public string GetParentProcessFileName() + { + try + { + // Non-Windows hosts have no equivalent lookup here, so the parent is reported as unknown. + if (!OperatingSystem.IsWindows()) + { + return null; + } + + using var current = Process.GetCurrentProcess(); + + if (!TryGetParentProcessId(out var parentProcessId)) + { + return null; + } + + using var parent = Process.GetProcessById(parentProcessId); + + // Process ids are recycled. If the process now holding our recorded parent id started after we + // did, it cannot be our real parent, so report unknown rather than an unrelated executable. + if (StartedAfter(parent, current)) + { + return null; + } + + return GetFileName(parent); + } + catch (Exception) + { + // Contract: never throw. The parent process may have exited between the two calls above + // (ArgumentException), or the OS may refuse the lookup (Win32Exception). Either way the caller + // treats a null as "unknown parent" and falls back to the other recursion barriers. + return null; + } + } + + private static bool TryGetParentProcessId(out int parentProcessId) + { + parentProcessId = 0; + + var bufferLength = PointerFieldCount * IntPtr.Size; + var buffer = Marshal.AllocHGlobal(bufferLength); + + try + { + // -1 is the pseudo-handle for the current process; it needs no rights and needs no closing. + var status = NtQueryInformationProcess( + new IntPtr(-1), + ProcessBasicInformation, + buffer, + bufferLength, + IntPtr.Zero); + + if (status != 0) + { + return false; + } + + var parent = Marshal.ReadIntPtr(buffer, InheritedFromUniqueProcessIdFieldIndex * IntPtr.Size); + + if (parent == IntPtr.Zero) + { + return false; + } + + parentProcessId = (int)parent; + return parentProcessId > 0; + } + finally + { + Marshal.FreeHGlobal(buffer); + } + } + + /// + /// True only when both start times are readable and the parent's is the later one. An unreadable start + /// time leaves the process id recycling check inconclusive, which is treated as "not recycled" so a + /// genuine MakePkg.exe parent is still recognized. + /// + private static bool StartedAfter(Process parent, Process current) + { + try + { + return parent.StartTime > current.StartTime; + } + catch (Exception) + { + return false; + } + } + + /// + /// Prefers the module name because it carries the extension (MakePkg.exe). Reading another + /// process's main module needs rights that are often unavailable across elevation or bitness + /// boundaries, so it falls back to the extension-less process name, which callers accommodate. + /// + private static string GetFileName(Process parent) + { + try + { + var moduleName = parent.MainModule?.ModuleName; + + if (!string.IsNullOrWhiteSpace(moduleName)) + { + return moduleName; + } + } + catch (Exception) + { + // Fall through to the process name below. + } + + return parent.ProcessName; + } +} diff --git a/src/PackageUploader.ClientApi.Test/Packaging/PackageFormatDetectorTest.cs b/src/PackageUploader.ClientApi.Test/Packaging/PackageFormatDetectorTest.cs new file mode 100644 index 00000000..72477ed1 --- /dev/null +++ b/src/PackageUploader.ClientApi.Test/Packaging/PackageFormatDetectorTest.cs @@ -0,0 +1,112 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using PackageUploader.ClientApi.Packaging; + +namespace PackageUploader.ClientApi.Test.Packaging; + +[TestClass] +public class PackageFormatDetectorTest +{ + private const int MinimumDetectableSize = 4096; + + private static string CreatePackage(string extension, byte[] header = null, byte[] footer = null) + { + var path = Path.Combine(Path.GetTempPath(), $"pu-detector-{Guid.NewGuid():N}{extension}"); + var contents = new byte[MinimumDetectableSize * 2]; + + if (header is not null) + { + Array.Copy(header, contents, header.Length); + } + + if (footer is not null) + { + Array.Copy(footer, 0, contents, contents.Length - footer.Length, footer.Length); + } + + File.WriteAllBytes(path, contents); + return path; + } + + [TestMethod] + public void IsLikelyMsixvc2Package_ZipLocalFileHeader_ReturnsTrue() + { + var path = CreatePackage(".msixvc", header: [0x50, 0x4B, 0x03, 0x04]); + try + { + Assert.IsTrue(PackageFormatDetector.IsLikelyMsixvc2Package(path)); + } + finally + { + File.Delete(path); + } + } + + [TestMethod] + public void IsLikelyMsixvc2Package_ZipEndOfCentralDirectorySignature_ReturnsTrue() + { + var path = CreatePackage(".msixvc", header: [0x01, 0x02, 0x03, 0x04], footer: [0x50, 0x4B, 0x05, 0x06]); + try + { + Assert.IsTrue(PackageFormatDetector.IsLikelyMsixvc2Package(path)); + } + finally + { + File.Delete(path); + } + } + + [TestMethod] + public void IsLikelyMsixvc2Package_NoZipSignatures_ReturnsFalse() + { + var path = CreatePackage(".msixvc", header: [0x01, 0x02, 0x03, 0x04]); + try + { + Assert.IsFalse(PackageFormatDetector.IsLikelyMsixvc2Package(path)); + } + finally + { + File.Delete(path); + } + } + + [TestMethod] + public void IsLikelyMsixvc2Package_WrongExtension_ReturnsFalse() + { + var path = CreatePackage(".xvc", header: [0x50, 0x4B, 0x03, 0x04]); + try + { + Assert.IsFalse(PackageFormatDetector.IsLikelyMsixvc2Package(path)); + } + finally + { + File.Delete(path); + } + } + + [TestMethod] + public void IsLikelyMsixvc2Package_FileTooSmall_ReturnsFalse() + { + var path = Path.Combine(Path.GetTempPath(), $"pu-detector-{Guid.NewGuid():N}.msixvc"); + File.WriteAllBytes(path, [0x50, 0x4B, 0x03, 0x04]); + try + { + Assert.IsFalse(PackageFormatDetector.IsLikelyMsixvc2Package(path)); + } + finally + { + File.Delete(path); + } + } + + [TestMethod] + public void IsLikelyMsixvc2Package_MissingFile_ReturnsFalse() => + Assert.IsFalse(PackageFormatDetector.IsLikelyMsixvc2Package(@"C:\does\not\exist.msixvc")); + + [TestMethod] + [DataRow("")] + [DataRow(" ")] + public void IsLikelyMsixvc2Package_EmptyPath_ReturnsFalse(string path) => + Assert.IsFalse(PackageFormatDetector.IsLikelyMsixvc2Package(path)); +} diff --git a/src/PackageUploader.ClientApi/Packaging/PackageFormatDetector.cs b/src/PackageUploader.ClientApi/Packaging/PackageFormatDetector.cs new file mode 100644 index 00000000..297bd8a0 --- /dev/null +++ b/src/PackageUploader.ClientApi/Packaging/PackageFormatDetector.cs @@ -0,0 +1,95 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.IO; + +namespace PackageUploader.ClientApi.Packaging; + +/// +/// Detects the on-disk format of game packages. Lives in PackageUploader.ClientApi so that both +/// PackageUploader.Application (CLI) and PackageUploader.UI share a single source of truth. +/// +public static class PackageFormatDetector +{ + private static readonly byte[] ZipLocalFileSignature = [0x50, 0x4B, 0x03, 0x04]; + private static readonly byte[] ZipEocdSignature = [0x50, 0x4B, 0x05, 0x06]; + private const int FirstReadSize = 4096; + private const int LastReadSize = 65 * 1024; + private const int MinFileNameOffset = 30; + + /// + /// Detects whether a .msixvc file is in MSIXVC2 format by checking for ZIP signatures. + /// MSIXVC2 packages are ZIP-based and contain standard ZIP headers, while MSIXVC1 + /// packages use a proprietary binary format without ZIP signatures. + /// + public static bool IsLikelyMsixvc2Package(string packagePath) + { + try + { + if (string.IsNullOrWhiteSpace(packagePath)) + return false; + + if (!packagePath.EndsWith(".msixvc", StringComparison.OrdinalIgnoreCase)) + return false; + + var fileInfo = new FileInfo(packagePath); + if (!fileInfo.Exists || fileInfo.Length < FirstReadSize) + return false; + + using var stream = File.OpenRead(packagePath); + + // Check first bytes for ZIP local file header at offset 0 + var firstBuffer = new byte[Math.Min(FirstReadSize, fileInfo.Length)]; + stream.ReadExactly(firstBuffer, 0, firstBuffer.Length); + + if (firstBuffer.Length >= MinFileNameOffset && + firstBuffer[0] == ZipLocalFileSignature[0] && + firstBuffer[1] == ZipLocalFileSignature[1] && + firstBuffer[2] == ZipLocalFileSignature[2] && + firstBuffer[3] == ZipLocalFileSignature[3]) + { + return true; + } + + // Check last 65KB for ZIP End of Central Directory signature + long lastChunkStart = Math.Max(0, fileInfo.Length - LastReadSize); + int lastChunkSize = (int)(fileInfo.Length - lastChunkStart); + var lastBuffer = new byte[lastChunkSize]; + stream.Position = lastChunkStart; + stream.ReadExactly(lastBuffer, 0, lastChunkSize); + + if (LastIndexOfSignature(lastBuffer, ZipEocdSignature) >= 0) + return true; + + return false; + } + catch (Exception) + { + return false; + } + } + + private static int LastIndexOfSignature(byte[] buffer, byte[] signature) + { + if (buffer.Length < signature.Length) + return -1; + + for (int i = buffer.Length - signature.Length; i >= 0; i--) + { + bool match = true; + for (int j = 0; j < signature.Length; j++) + { + if (buffer[i + j] != signature[j]) + { + match = false; + break; + } + } + if (match) + return i; + } + + return -1; + } +} diff --git a/src/PackageUploader.UI/Model/Xvc/XvcFile.cs b/src/PackageUploader.UI/Model/Xvc/XvcFile.cs index 39f9882e..7e27ef3f 100644 --- a/src/PackageUploader.UI/Model/Xvc/XvcFile.cs +++ b/src/PackageUploader.UI/Model/Xvc/XvcFile.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using PackageUploader.ClientApi.Packaging; using System; using System.Drawing; using System.IO; @@ -60,83 +61,12 @@ private static UInt32 NumberOfHashPagesForLevel(UInt64 dataPages, Int32 level) return (UInt32)((dataPages + divisor - 1) / divisor); } - private static readonly byte[] ZipLocalFileSignature = [0x50, 0x4B, 0x03, 0x04]; - private static readonly byte[] ZipEocdSignature = [0x50, 0x4B, 0x05, 0x06]; - private const int FirstReadSize = 4096; - private const int LastReadSize = 65 * 1024; - private const int MinFileNameOffset = 30; - /// /// Detects whether a .msixvc file is in MSIXVC2 format by checking for ZIP signatures. - /// MSIXVC2 packages are ZIP-based and contain standard ZIP headers, while MSIXVC1 - /// packages use a proprietary binary format without ZIP signatures. + /// The implementation lives in PackageUploader.ClientApi so the CLI and the UI share one source of truth. /// - public static bool IsLikelyMsixvc2Package(string packagePath) - { - try - { - if (!packagePath.EndsWith(".msixvc", StringComparison.OrdinalIgnoreCase)) - return false; - - var fileInfo = new FileInfo(packagePath); - if (!fileInfo.Exists || fileInfo.Length < FirstReadSize) - return false; - - using var stream = File.OpenRead(packagePath); - - // Check first bytes for ZIP local file header at offset 0 - var firstBuffer = new byte[Math.Min(FirstReadSize, fileInfo.Length)]; - stream.ReadExactly(firstBuffer, 0, firstBuffer.Length); - - if (firstBuffer.Length >= MinFileNameOffset && - firstBuffer[0] == ZipLocalFileSignature[0] && - firstBuffer[1] == ZipLocalFileSignature[1] && - firstBuffer[2] == ZipLocalFileSignature[2] && - firstBuffer[3] == ZipLocalFileSignature[3]) - { - return true; - } - - // Check last 65KB for ZIP End of Central Directory signature - long lastChunkStart = Math.Max(0, fileInfo.Length - LastReadSize); - int lastChunkSize = (int)(fileInfo.Length - lastChunkStart); - var lastBuffer = new byte[lastChunkSize]; - stream.Position = lastChunkStart; - stream.ReadExactly(lastBuffer, 0, lastChunkSize); - - if (LastIndexOfSignature(lastBuffer, ZipEocdSignature) >= 0) - return true; - - return false; - } - catch (Exception) - { - return false; - } - } - - private static int LastIndexOfSignature(byte[] buffer, byte[] signature) - { - if (buffer.Length < signature.Length) - return -1; - - for (int i = buffer.Length - signature.Length; i >= 0; i--) - { - bool match = true; - for (int j = 0; j < signature.Length; j++) - { - if (buffer[i + j] != signature[j]) - { - match = false; - break; - } - } - if (match) - return i; - } - - return -1; - } + public static bool IsLikelyMsixvc2Package(string packagePath) => + PackageFormatDetector.IsLikelyMsixvc2Package(packagePath); public static void GetBuildAndKeyId(string packagePath, out Guid buildId, out Guid keyId) {