Validate installer download destination

src/WingetCreateCore/Common/Constants.cs

@@ -31,7 +31,7 @@ public static class Constants
         /// <summary>
         /// App Id for the Winget-Create GitHub App.
         /// </summary>
-        public const int GitHubAppId = 965520;
+        public const int GitHubAppId = 100205;
 
         /// <summary>
         /// Link to the GitHub releases page for the winget-create tool.

src/WingetCreateCore/Common/PackageParser.cs

@@ -178,17 +178,26 @@ public static async Task<string> DownloadFileAsync(string url, bool allowHttp, l
             }
 
             string urlFile = Path.GetFileName(url.Split('?').Last());
-            string contentDispositionFile = response.Content.Headers.ContentDisposition?.FileName?.Trim('"');
+            string contentDispositionFile = Path.GetFileName(response.Content.Headers.ContentDisposition?.FileName?.Trim('"'));
             string requestUrlFileName = Path.GetFileName(response.RequestMessage?.RequestUri?.ToString());
 
             if (!Directory.Exists(InstallerDownloadPath))
             {
                 Directory.CreateDirectory(InstallerDownloadPath);
             }
 
-            // If no relevant filename can be obtained for the installer download, use a temporary filename as last option.
-            string targetFileName = contentDispositionFile.NullIfEmpty() ?? urlFile.NullIfEmpty() ?? requestUrlFileName.NullIfEmpty() ?? Path.GetTempFileName();
+            // If no relevant filename can be obtained for the installer download, use a random filename as last option.
+            string targetFileName = contentDispositionFile.NullIfEmpty() ?? urlFile.NullIfEmpty() ?? requestUrlFileName.NullIfEmpty() ?? Path.GetRandomFileName();
             string targetFile = GetNumericFilename(Path.Combine(InstallerDownloadPath, targetFileName));
+
+            // Defense in depth: ensure the resolved path stays under the installer download directory.
+            string downloadRoot = Path.GetFullPath(InstallerDownloadPath).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
+            string fullTargetFile = Path.GetFullPath(targetFile);
+            if (!fullTargetFile.StartsWith(downloadRoot + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase))
+            {
+                throw new InvalidOperationException("Resolved download path escapes the installer download directory.");
+            }
+
             using var targetFileStream = File.OpenWrite(targetFile);
             var contentStream = await response.Content.ReadAsStreamAsync();
 

src/WingetCreateTests/WingetCreateTests/TestUtils.cs

@@ -97,6 +97,23 @@ public static void SetMockHttpResponseContent(string installerName)
             PackageParser.SetHttpMessageHandler(httpMessageHandler);
         }
 
+        /// <summary>
+        /// Sets the mock http response content along with a server-supplied Content-Disposition filename.
+        /// </summary>
+        /// <param name="installerName">File name of the installer.</param>
+        /// <param name="contentDispositionFileName">The filename value to advertise in the Content-Disposition header.</param>
+        public static void SetMockHttpResponseContent(string installerName, string contentDispositionFileName)
+        {
+            var content = new ByteArrayContent(File.ReadAllBytes(GetTestFile(installerName)));
+            content.Headers.ContentType = new MediaTypeHeaderValue("application/octet-stream");
+            content.Headers.ContentDisposition = new ContentDispositionHeaderValue("attachment")
+            {
+                FileName = contentDispositionFileName,
+            };
+            httpResponseMessage.Content = content;
+            PackageParser.SetHttpMessageHandler(httpMessageHandler);
+        }
+
         /// <summary>
         /// Obtains the relative filepath of the resources test data directory.
         /// </summary>
@@ -170,6 +187,23 @@ public static List<string> CreateResourceCopy(string resource, int numberOfCopie
             return copyPaths;
         }
 
+        /// <summary>
+        /// Deletes existing copies of the specified resource (base and numbered variants) from the test resources directory.
+        /// </summary>
+        /// <param name="resourceName">Name of the resource file whose copies should be deleted.</param>
+        public static void DeleteResourceCopies(string resourceName)
+        {
+            string resourcePath = GetTestFile(resourceName);
+            string directory = Path.GetDirectoryName(resourcePath);
+            string fileName = Path.GetFileNameWithoutExtension(resourcePath);
+            string fileExt = Path.GetExtension(resourcePath);
+
+            foreach (string file in Directory.GetFiles(directory, fileName + "*" + fileExt))
+            {
+                File.Delete(file);
+            }
+        }
+
         /// <summary>
         /// Adds files to an existing test zip archive.
         /// </summary>
@@ -211,9 +245,21 @@ public static void RemoveFilesFromZip(string zipResourceName, List<string> fileN
         /// <param name="testFileNames">Name of the test files to delete.</param>
         public static void DeleteCachedFiles(List<string> testFileNames)
         {
+            string downloadDirectory = PackageParser.InstallerDownloadPath;
+            if (!Directory.Exists(downloadDirectory))
+            {
+                return;
+            }
+
             foreach (string fileName in testFileNames)
             {
-                File.Delete(Path.Combine(PackageParser.InstallerDownloadPath, fileName));
+                string baseName = Path.GetFileNameWithoutExtension(fileName);
+                string fileExt = Path.GetExtension(fileName);
+
+                foreach (string filePath in Directory.GetFiles(downloadDirectory, baseName + "*" + fileExt))
+                {
+                    File.Delete(filePath);
+                }
             }
         }
 

src/WingetCreateTests/WingetCreateTests/UnitTests/PackageParserTests.cs

@@ -66,6 +66,37 @@ public void ParseExeInstallerFile()
             ClassicAssert.AreEqual(InstallerType.Exe, manifests.InstallerManifest.Installers.First().InstallerType);
         }
 
+        /// <summary>
+        /// Verifies that a server-supplied Content-Disposition filename containing directory
+        /// traversal sequences cannot cause the installer to be written outside the intended download
+        /// directory.
+        /// </summary>
+        [Test]
+        public void DownloadFileDispositionStaysInDownloadDirectory()
+        {
+            string downloadedPath = null;
+            try
+            {
+                string url = $"https://fakedomain.com/{TestConstants.TestExeInstaller}";
+                string invalidFileName = @"..\..\..\..\example.exe";
+                TestUtils.SetMockHttpResponseContent(TestConstants.TestExeInstaller, invalidFileName);
+
+                downloadedPath = PackageParser.DownloadFileAsync(url, false).Result;
+
+                string downloadRoot = Path.GetFullPath(PackageParser.InstallerDownloadPath).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
+                string fullDownloadedPath = Path.GetFullPath(downloadedPath);
+                Assert.That(fullDownloadedPath, Does.StartWith(downloadRoot + Path.DirectorySeparatorChar), "Downloaded file escaped the installer download directory.");
+                ClassicAssert.AreEqual("example.exe", Path.GetFileName(downloadedPath));
+            }
+            finally
+            {
+                if (downloadedPath != null && File.Exists(downloadedPath))
+                {
+                    File.Delete(downloadedPath);
+                }
+            }
+        }
+
         /// <summary>
         /// Downloads the MSI installer file from HTTPS localhost and parses the package to create a manifest object.
         /// </summary>

src/WingetCreateTests/WingetCreateTests/UnitTests/UpdateCommandTests.cs

@@ -1381,24 +1381,41 @@ public async Task UpdateMultipleZipInstallers()
         [Test]
         public async Task UpdateZipWithMultipleNestedInstallers()
         {
+            // Ensure a clean slate
+            TestUtils.DeleteResourceCopies(TestConstants.TestPortableInstaller);
+
             // Create copies of test exe installer to be used as portable installers
             List<string> portableFilePaths = TestUtils.CreateResourceCopy(TestConstants.TestExeInstaller, 4, TestConstants.TestPortableInstaller);
 
-            // Add the generated portable installers to the test zip installer
-            TestUtils.AddFilesToZip(TestConstants.TestZipInstaller, portableFilePaths);
+            Manifests updatedManifests;
+            List<string> initialManifestContent;
 
-            // Delete cached zip installer from other test runs so that the modified zip installer is downloaded
-            TestUtils.DeleteCachedFiles(new List<string> { TestConstants.TestZipInstaller });
+            try
+            {
+                // Add the generated portable installers to the test zip installer
+                TestUtils.AddFilesToZip(TestConstants.TestZipInstaller, portableFilePaths);
 
-            TestUtils.InitializeMockDownloads(TestConstants.TestZipInstaller);
-            string installerUrl = $"https://fakedomain.com/{TestConstants.TestZipInstaller}";
-            (UpdateCommand command, var initialManifestContent) = GetUpdateCommandAndManifestData("TestPublisher.ZipMultipleNestedInstallers", null, this.tempPath, new[] { $"{installerUrl}|x64", $"{installerUrl}|x86", $"{installerUrl}|arm", $"{installerUrl}|arm64|user", $"{installerUrl}|arm64|machine" });
+                // Delete cached zip installer from other test runs so that the modified zip installer is downloaded
+                TestUtils.DeleteCachedFiles(new List<string> { TestConstants.TestZipInstaller });
 
-            var updatedManifests = await RunUpdateCommand(command, initialManifestContent);
+                TestUtils.InitializeMockDownloads(TestConstants.TestZipInstaller);
+                string installerUrl = $"https://fakedomain.com/{TestConstants.TestZipInstaller}";
+                (UpdateCommand command, initialManifestContent) = GetUpdateCommandAndManifestData("TestPublisher.ZipMultipleNestedInstallers", null, this.tempPath, new[] { $"{installerUrl}|x64", $"{installerUrl}|x86", $"{installerUrl}|arm", $"{installerUrl}|arm64|user", $"{installerUrl}|arm64|machine" });
 
-            // Perform test clean up before any assertions
-            portableFilePaths.ForEach(File.Delete);
-            TestUtils.RemoveFilesFromZip(TestConstants.TestZipInstaller, portableFilePaths.Select(Path.GetFileName).ToList());
+                updatedManifests = await RunUpdateCommand(command, initialManifestContent);
+            }
+            finally
+            {
+                // Perform test clean up before any assertions
+                portableFilePaths.ForEach(path =>
+                {
+                    if (File.Exists(path))
+                    {
+                        File.Delete(path);
+                    }
+                });
+                TestUtils.RemoveFilesFromZip(TestConstants.TestZipInstaller, portableFilePaths.Select(Path.GetFileName).ToList());
+            }
 
             ClassicAssert.IsNotNull(updatedManifests, "Command should have succeeded");