Add MSIXVC2 upload support to PackageUploader.exe via MakePkg.exe

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 <method>`, 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 |

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);
+    }
 }

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<T>(this Mock<ILogger<T>> loggerMock, string expectedSubstring) =>
+        VerifyLogContains(loggerMock, LogLevel.Error, expectedSubstring);
+
+    public static void VerifyLogWarningContains(this Mock<ILogger> loggerMock, string expectedSubstring) =>
+        VerifyLogContains(loggerMock, LogLevel.Warning, expectedSubstring);
+
+    public static void VerifyLogWarningContains<T>(this Mock<ILogger<T>> loggerMock, string expectedSubstring) =>
+        VerifyLogContains(loggerMock, LogLevel.Warning, expectedSubstring);
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    public static void VerifyNeverLogged<T>(this Mock<ILogger<T>> loggerMock, string forbiddenSubstring) =>
+        loggerMock.Verify(
+            x => x.Log(
+                It.IsAny<LogLevel>(),
+                It.IsAny<EventId>(),
+                It.Is<It.IsAnyType>((v, _) => v.ToString()!.Contains(forbiddenSubstring)),
+                It.IsAny<Exception>(),
+                It.IsAny<Func<It.IsAnyType, Exception?, string>>()),
+            Times.Never);
+
+    private static void VerifyLogContains<TLogger>(Mock<TLogger> loggerMock, LogLevel level, string expectedSubstring)
+        where TLogger : class, ILogger =>
+        loggerMock.Verify(
+            x => x.Log(
+                level,
+                It.IsAny<EventId>(),
+                It.Is<It.IsAnyType>((v, _) => v.ToString()!.Contains(expectedSubstring)),
+                It.IsAny<Exception>(),
+                It.IsAny<Func<It.IsAnyType, Exception?, string>>()),
+            Times.Once);
+}

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<IParentProcessProvider> _parentProcessProviderMock = null!;
+
+    [TestInitialize]
+    public void Initialize()
+    {
+        _originalValue = Environment.GetEnvironmentVariable(Msixvc2DelegationGuard.EnvironmentVariableName);
+        _parentProcessProviderMock = new Mock<IParentProcessProvider>();
+    }
+
+    [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);
+    }
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    [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());
+    }
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    [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());
+    }
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    [TestMethod]
+    [DataRow(null)]
+    [DataRow("")]
+    [DataRow("   ")]
+    public void GetMakePkgParentProcessName_WhenParentUnknown_ReturnsNull(string? parentFileName)
+    {
+        _parentProcessProviderMock.Setup(x => x.GetParentProcessFileName()).Returns(parentFileName!);
+
+        Assert.IsNull(CreateGuard().GetMakePkgParentProcessName());
+    }
+}