From 7f3a30cd04aa884699b7980a9209aade28f1eca2 Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Mon, 12 Sep 2022 23:43:32 +0000 Subject: [PATCH] Merged PR 412962: Prep for .NET 7 API with a new temp folder On Linux, the default /tmp folder is shared across all users and accessible by them. There are some cases in which we put sensitive information in temp and assume it's fine because on Windows, it is. This doesn't actually fix that assumption, since we're currently waiting for a new API that will be introduced in .NET 7 that will make a folder with appropriate permissions. However, this PR changes all the issues Eric Erhardt identified such that they go through a single code path, so to fix the security issue afterwards just requires changing the one place in our code. It did occur to me that we may not be able to use that API, in which case I can just write something to make a folder with a random name under temp then tweak its permissions. --- eng/Versions.props | 2 +- .../Definition/Project_Tests.cs | 16 ++- .../BuildRequestConfiguration_Tests.cs | 2 + .../BackEnd/DebugUtils_tests.cs | 2 +- .../BackEnd/TargetUpToDateChecker_Tests.cs | 2 +- .../Construction/SolutionFile_Tests.cs | 20 ++-- .../SolutionProjectGenerator_Tests.cs | 6 +- .../Evaluation/Expander_Tests.cs | 4 +- .../BackEnd/BuildManager/BuildManager.cs | 8 ++ .../BuildRequestEngine/BuildRequestEngine.cs | 2 +- .../BackEnd/Components/Scheduler/Scheduler.cs | 2 +- src/Framework/NativeMethods.cs | 12 +- src/MSBuild/OutOfProcTaskHostNode.cs | 2 +- src/Shared/CommunicationsUtilities.cs | 2 +- src/Shared/Debugging/DebugUtils.cs | 2 +- src/Shared/ExceptionHandling.cs | 2 +- src/Shared/FileUtilities.cs | 6 +- src/Shared/NamedPipeUtil.cs | 6 + src/Shared/NodeEndpointOutOfProcBase.cs | 12 +- src/Shared/TempFileUtilities.cs | 103 +++++++++++++++--- src/Shared/UnitTests/FileUtilities_Tests.cs | 16 +-- src/Shared/UnitTests/TestEnvironment.cs | 4 +- src/Tasks.UnitTests/Copy_Tests.cs | 4 +- src/Tasks/CodeTaskFactory.cs | 10 +- src/Tasks/GetSDKReferenceFiles.cs | 2 +- .../RoslynCodeTaskFactory.cs | 4 +- src/Tasks/TlbReference.cs | 2 +- src/Tasks/WriteCodeFragment.cs | 2 +- .../TrackedDependencies/FileTracker.cs | 2 +- 29 files changed, 175 insertions(+), 84 deletions(-) diff --git a/eng/Versions.props b/eng/Versions.props index 6f8122214b3..03c1b83c1e3 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -2,7 +2,7 @@ - 17.4.0 + 17.4.1 15.1.0.0 preview true diff --git a/src/Build.OM.UnitTests/Definition/Project_Tests.cs b/src/Build.OM.UnitTests/Definition/Project_Tests.cs index 4d856fb153d..beb78eec2ae 100644 --- a/src/Build.OM.UnitTests/Definition/Project_Tests.cs +++ b/src/Build.OM.UnitTests/Definition/Project_Tests.cs @@ -654,9 +654,7 @@ public void TransformsUseCorrectDirectory_Basic() project.ReevaluateIfNecessary(); project.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe( - NativeMethodsShared.IsWindows - ? Path.Combine(Path.GetTempPath(), @"obj\i386\foo.dll") - : Path.Combine(Path.GetTempPath(), @"obj/i386/foo.dll")); + Path.Combine(FileUtilities.TempFileDirectory, "obj", "i386", "foo.dll")); } finally { @@ -721,8 +719,8 @@ public void TransformsUseCorrectDirectory_DirectoryTransform() Project project = new Project(xml); ProjectInstance projectInstance = new ProjectInstance(xml); - project.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(Path.GetTempPath(), "obj", "i386").Substring(RootPrefixLength) + Path.DirectorySeparatorChar); - projectInstance.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(Path.GetTempPath(), "obj", "i386").Substring(RootPrefixLength) + Path.DirectorySeparatorChar); + project.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(FileUtilities.TempFileDirectory, "obj", "i386").Substring(RootPrefixLength) + Path.DirectorySeparatorChar); + projectInstance.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(FileUtilities.TempFileDirectory, "obj", "i386").Substring(RootPrefixLength) + Path.DirectorySeparatorChar); } finally { @@ -756,8 +754,8 @@ public void TransformsUseCorrectDirectory_DirectoryItemFunction() Project project = new Project(xml); ProjectInstance projectInstance = new ProjectInstance(xml); - project.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(Path.GetTempPath(), "obj", "i386").Substring(RootPrefixLength) + Path.DirectorySeparatorChar); - projectInstance.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(Path.GetTempPath(), "obj", "i386").Substring(RootPrefixLength) + Path.DirectorySeparatorChar); + project.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(FileUtilities.TempFileDirectory, "obj", "i386").Substring(RootPrefixLength) + Path.DirectorySeparatorChar); + projectInstance.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(FileUtilities.TempFileDirectory, "obj", "i386").Substring(RootPrefixLength) + Path.DirectorySeparatorChar); } finally { @@ -794,8 +792,8 @@ public void TransformsUseCorrectDirectory_DirectoryNameItemFunction() ProjectInstance projectInstance = new ProjectInstance(xml); // Should be the full path to the directory - project.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(Path.GetTempPath() /* remove c:\ */, "obj" + Path.DirectorySeparatorChar + "i386")); - projectInstance.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(Path.GetTempPath() /* remove c:\ */, "obj" + Path.DirectorySeparatorChar + "i386")); + project.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(FileUtilities.TempFileDirectory /* remove c:\ */, "obj" + Path.DirectorySeparatorChar + "i386")); + projectInstance.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(FileUtilities.TempFileDirectory /* remove c:\ */, "obj" + Path.DirectorySeparatorChar + "i386")); } finally { diff --git a/src/Build.UnitTests/BackEnd/BuildRequestConfiguration_Tests.cs b/src/Build.UnitTests/BackEnd/BuildRequestConfiguration_Tests.cs index 7d42cfe8206..12fbe5d15d9 100644 --- a/src/Build.UnitTests/BackEnd/BuildRequestConfiguration_Tests.cs +++ b/src/Build.UnitTests/BackEnd/BuildRequestConfiguration_Tests.cs @@ -476,6 +476,7 @@ public void TestCache2() Environment.SetEnvironmentVariable("TEMP", problematicTmpPath); FileUtilities.ClearCacheDirectoryPath(); + FileUtilities.ClearTempFileDirectory(); string cacheFilePath = configuration.GetCacheFile(); Assert.StartsWith(problematicTmpPath, cacheFilePath); } @@ -484,6 +485,7 @@ public void TestCache2() Environment.SetEnvironmentVariable("TMP", originalTmp); Environment.SetEnvironmentVariable("TEMP", originalTemp); FileUtilities.ClearCacheDirectoryPath(); + FileUtilities.ClearTempFileDirectory(); } } diff --git a/src/Build.UnitTests/BackEnd/DebugUtils_tests.cs b/src/Build.UnitTests/BackEnd/DebugUtils_tests.cs index 351d86a0d6a..0419a840a6c 100644 --- a/src/Build.UnitTests/BackEnd/DebugUtils_tests.cs +++ b/src/Build.UnitTests/BackEnd/DebugUtils_tests.cs @@ -24,7 +24,7 @@ public void DumpExceptionToFileShouldWriteInTempPathByDefault() try { ExceptionHandling.DumpExceptionToFile(new Exception("hello world")); - exceptionFiles = Directory.GetFiles(Path.GetTempPath(), "MSBuild_*failure.txt"); + exceptionFiles = Directory.GetFiles(FileUtilities.TempFileDirectory, "MSBuild_*failure.txt"); } finally { diff --git a/src/Build.UnitTests/BackEnd/TargetUpToDateChecker_Tests.cs b/src/Build.UnitTests/BackEnd/TargetUpToDateChecker_Tests.cs index a90afeabc38..f2abb27be9c 100644 --- a/src/Build.UnitTests/BackEnd/TargetUpToDateChecker_Tests.cs +++ b/src/Build.UnitTests/BackEnd/TargetUpToDateChecker_Tests.cs @@ -990,7 +990,7 @@ public void NewSymlinkNewDestinationIsNotUpToDate() _testOutputHelper.WriteLine($"Created input file {inputTarget}"); File.SetLastWriteTime(inputTarget, targetWriteTime); - inputSymlink = FileUtilities.GetTemporaryFile(null, ".linkin", createFile: false); + inputSymlink = FileUtilities.GetTemporaryFile(null, null, ".linkin", createFile: false); if (!CreateSymbolicLink(inputSymlink, inputTarget, 0)) { diff --git a/src/Build.UnitTests/Construction/SolutionFile_Tests.cs b/src/Build.UnitTests/Construction/SolutionFile_Tests.cs index d2ab15a93eb..16c1c196883 100644 --- a/src/Build.UnitTests/Construction/SolutionFile_Tests.cs +++ b/src/Build.UnitTests/Construction/SolutionFile_Tests.cs @@ -141,7 +141,7 @@ public void ParseFirstProjectLine_InvalidProject() [Fact] public void ParseEtpProject() { - string proj1Path = Path.Combine(Path.GetTempPath(), "someproj.etp"); + string proj1Path = Path.Combine(FileUtilities.TempFileDirectory, "someproj.etp"); try { // Create the first .etp project file @@ -192,8 +192,8 @@ public void ParseEtpProject() [Fact] public void CanBeMSBuildFile() { - string proj1Path = Path.Combine(Path.GetTempPath(), "someproj.etp"); - string proj2Path = Path.Combine(Path.GetTempPath(), "someproja.proj"); + string proj1Path = Path.Combine(FileUtilities.TempFileDirectory, "someproj.etp"); + string proj2Path = Path.Combine(FileUtilities.TempFileDirectory, "someproja.proj"); try { // Create the first .etp project file @@ -317,8 +317,8 @@ public void CanBeMSBuildFileRejectsMSBuildLikeFiles() [Fact] public void ParseNestedEtpProjectSingleLevel() { - string proj1Path = Path.Combine(Path.GetTempPath(), "someproj.etp"); - string proj2Path = Path.Combine(Path.GetTempPath(), "someproj2.etp"); + string proj1Path = Path.Combine(FileUtilities.TempFileDirectory, "someproj.etp"); + string proj2Path = Path.Combine(FileUtilities.TempFileDirectory, "someproj2.etp"); try { // Create the first .etp project file @@ -513,9 +513,9 @@ public void TestVSAndSolutionVersionParsing() [Trait("Category", "netcore-linux-failing")] public void ParseNestedEtpProjectMultipleLevel() { - string proj1Path = Path.Combine(Path.GetTempPath(), "someproj.etp"); - string proj2Path = Path.Combine(Path.GetTempPath(), "someproj2.etp"); - string proj3Path = Path.Combine(Path.GetTempPath(), "ETPProjUpgradeTest", "someproj3.etp"); + string proj1Path = Path.Combine(FileUtilities.TempFileDirectory, "someproj.etp"); + string proj2Path = Path.Combine(FileUtilities.TempFileDirectory, "someproj2.etp"); + string proj3Path = Path.Combine(FileUtilities.TempFileDirectory, "ETPProjUpgradeTest", "someproj3.etp"); try { // Create the first .etp project file @@ -567,7 +567,7 @@ public void ParseNestedEtpProjectMultipleLevel() "; // Create the directory for the third project - Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), "ETPProjUpgradeTest")); + Directory.CreateDirectory(Path.Combine(FileUtilities.TempFileDirectory, "ETPProjUpgradeTest")); File.WriteAllText(proj3Path, etpProjContent); // Create the SolutionFile object @@ -602,7 +602,7 @@ public void ParseNestedEtpProjectMultipleLevel() [Fact] public void MalformedEtpProjFile() { - string proj1Path = Path.Combine(Path.GetTempPath(), "someproj.etp"); + string proj1Path = Path.Combine(FileUtilities.TempFileDirectory, "someproj.etp"); try { // Create the .etp project file diff --git a/src/Build.UnitTests/Construction/SolutionProjectGenerator_Tests.cs b/src/Build.UnitTests/Construction/SolutionProjectGenerator_Tests.cs index 780b83ffa0b..6c726a5476d 100644 --- a/src/Build.UnitTests/Construction/SolutionProjectGenerator_Tests.cs +++ b/src/Build.UnitTests/Construction/SolutionProjectGenerator_Tests.cs @@ -850,7 +850,7 @@ public void SolutionConfigurationWithDependencies() Debug|AnyCPU Debug|AnyCPU Debug|AnyCPU -".Replace("`", "\"").Replace("##temp##", Path.GetTempPath()); +".Replace("`", "\"").Replace("##temp##", FileUtilities.TempFileDirectory); Helpers.VerifyAssertLineByLine(expected, solutionConfigurationContents); } @@ -1090,14 +1090,14 @@ public void TestAddPropertyGroupForSolutionConfiguration() msbuildProject.ReevaluateIfNecessary(); string solutionConfigurationContents = msbuildProject.GetPropertyValue("CurrentSolutionConfigurationContents"); - string tempProjectPath = Path.Combine(Path.GetTempPath(), "ClassLibrary1", "ClassLibrary1.csproj"); + string tempProjectPath = Path.Combine(FileUtilities.TempFileDirectory, "ClassLibrary1", "ClassLibrary1.csproj"); Assert.Contains("{6185CC21-BE89-448A-B3C0-D1C27112E595}", solutionConfigurationContents); tempProjectPath = Path.GetFullPath(tempProjectPath); Assert.True(solutionConfigurationContents.IndexOf(tempProjectPath, StringComparison.OrdinalIgnoreCase) > 0); Assert.Contains("CSConfig1|AnyCPU", solutionConfigurationContents); - tempProjectPath = Path.Combine(Path.GetTempPath(), "MainApp", "MainApp.vcxproj"); + tempProjectPath = Path.Combine(FileUtilities.TempFileDirectory, "MainApp", "MainApp.vcxproj"); tempProjectPath = Path.GetFullPath(tempProjectPath); Assert.Contains("{A6F99D27-47B9-4EA4-BFC9-25157CBDC281}", solutionConfigurationContents); Assert.True(solutionConfigurationContents.IndexOf(tempProjectPath, StringComparison.OrdinalIgnoreCase) > 0); diff --git a/src/Build.UnitTests/Evaluation/Expander_Tests.cs b/src/Build.UnitTests/Evaluation/Expander_Tests.cs index e77a47e5ef4..8501b7297f9 100644 --- a/src/Build.UnitTests/Evaluation/Expander_Tests.cs +++ b/src/Build.UnitTests/Evaluation/Expander_Tests.cs @@ -3052,7 +3052,7 @@ public void PropertyFunctionStaticMethodEnumArgument() [Fact] public void PropertyFunctionStaticMethodDirectoryNameOfFileAbove() { - string tempPath = Path.GetTempPath(); + string tempPath = FileUtilities.TempFileDirectory; string tempFile = Path.GetFileName(FileUtilities.GetTemporaryFile()); try @@ -3090,7 +3090,7 @@ public void PropertyFunctionStaticMethodGetPathOfFileAbove() // MockElementLocation mockElementLocation = new MockElementLocation(Path.Combine(ObjectModelHelpers.TempProjectDir, "one", "two", "three", "four", "five", Path.GetRandomFileName())); - string fileToFind = FileUtilities.GetTemporaryFile(ObjectModelHelpers.TempProjectDir, ".tmp"); + string fileToFind = FileUtilities.GetTemporaryFile(ObjectModelHelpers.TempProjectDir, null, ".tmp"); try { diff --git a/src/Build/BackEnd/BuildManager/BuildManager.cs b/src/Build/BackEnd/BuildManager/BuildManager.cs index f683529b74d..a188f318b64 100644 --- a/src/Build/BackEnd/BuildManager/BuildManager.cs +++ b/src/Build/BackEnd/BuildManager/BuildManager.cs @@ -409,6 +409,14 @@ public DeferredBuildMessage(string text, MessageImportance importance) /// Thrown if a build is already in progress. public void BeginBuild(BuildParameters parameters, IEnumerable deferredBuildMessages) { + // TEMP can be modified from the environment. Most of Traits is lasts for the duration of the process (with a manual reset for tests) + // and environment variables we use as properties are stored in a dictionary at the beginning of the build, so they also cannot be + // changed during a build. Some of our older stuff uses live environment variable checks. The TEMP directory previously used a live + // environment variable check, but it now uses a cached value. Nevertheless, we should support changing it between builds, so reset + // it here in case the user is using Visual Studio or the MSBuild server, as those each last for multiple builds without changing + // BuildManager. + FileUtilities.ClearTempFileDirectory(); + // deferredBuildMessages cannot be an optional parameter on a single BeginBuild method because it would break binary compatibility. _deferredBuildMessages = deferredBuildMessages; BeginBuild(parameters); diff --git a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs index 3bb58aa36ab..d635c661da9 100644 --- a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs +++ b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs @@ -127,7 +127,7 @@ internal BuildRequestEngine() if (String.IsNullOrEmpty(_debugDumpPath)) { - _debugDumpPath = Path.GetTempPath(); + _debugDumpPath = FileUtilities.TempFileDirectory; } _status = BuildRequestEngineStatus.Uninitialized; diff --git a/src/Build/BackEnd/Components/Scheduler/Scheduler.cs b/src/Build/BackEnd/Components/Scheduler/Scheduler.cs index 44f487998b8..f166c13d9fa 100644 --- a/src/Build/BackEnd/Components/Scheduler/Scheduler.cs +++ b/src/Build/BackEnd/Components/Scheduler/Scheduler.cs @@ -228,7 +228,7 @@ public Scheduler() if (String.IsNullOrEmpty(_debugDumpPath)) { - _debugDumpPath = Path.GetTempPath(); + _debugDumpPath = FileUtilities.TempFileDirectory; } Reset(); diff --git a/src/Framework/NativeMethods.cs b/src/Framework/NativeMethods.cs index d0c29652824..60d986402ca 100644 --- a/src/Framework/NativeMethods.cs +++ b/src/Framework/NativeMethods.cs @@ -655,6 +655,7 @@ internal static bool IsUnixLike /// /// Gets a flag indicating if we are running under Linux /// + [SupportedOSPlatformGuard("linux")] internal static bool IsLinux { #if CLR2COMPATIBILITY @@ -1470,9 +1471,16 @@ internal static void VerifyThrowWin32Result(int result) } } -#endregion + #endregion + + #region PInvoke + [SupportedOSPlatform("linux")] + [DllImport("libc", SetLastError = true)] + internal static extern int chmod(string pathname, int mode); -#region PInvoke + [SupportedOSPlatform("linux")] + [DllImport("libc", SetLastError = true)] + internal static extern int mkdir(string path, int mode); /// /// Gets the current OEM code page which is used by console apps diff --git a/src/MSBuild/OutOfProcTaskHostNode.cs b/src/MSBuild/OutOfProcTaskHostNode.cs index 9ec5f525074..629d9141a93 100644 --- a/src/MSBuild/OutOfProcTaskHostNode.cs +++ b/src/MSBuild/OutOfProcTaskHostNode.cs @@ -792,7 +792,7 @@ private NodeEngineShutdownReason HandleShutdown() if (_debugCommunications) { - using (StreamWriter writer = File.CreateText(String.Format(CultureInfo.CurrentCulture, Path.Combine(Path.GetTempPath(), @"MSBuild_NodeShutdown_{0}.txt"), Process.GetCurrentProcess().Id))) + using (StreamWriter writer = File.CreateText(String.Format(CultureInfo.CurrentCulture, Path.Combine(FileUtilities.TempFileDirectory, @"MSBuild_NodeShutdown_{0}.txt"), Process.GetCurrentProcess().Id))) { writer.WriteLine("Node shutting down with reason {0}.", _shutdownReason); } diff --git a/src/Shared/CommunicationsUtilities.cs b/src/Shared/CommunicationsUtilities.cs index f8106579fb7..0aa08501488 100644 --- a/src/Shared/CommunicationsUtilities.cs +++ b/src/Shared/CommunicationsUtilities.cs @@ -693,7 +693,7 @@ internal static void Trace(int nodeId, string format, params object[] args) if (String.IsNullOrEmpty(s_debugDumpPath)) { - s_debugDumpPath = Path.GetTempPath(); + s_debugDumpPath = FileUtilities.TempFileDirectory; } else { diff --git a/src/Shared/Debugging/DebugUtils.cs b/src/Shared/Debugging/DebugUtils.cs index 74a3a7b9e79..3ae6cf16891 100644 --- a/src/Shared/Debugging/DebugUtils.cs +++ b/src/Shared/Debugging/DebugUtils.cs @@ -38,7 +38,7 @@ static DebugUtils() } else { - debugDirectory = Path.Combine(Path.GetTempPath(), "MSBuild_Logs"); + debugDirectory = Path.Combine(FileUtilities.TempFileDirectory, "MSBuild_Logs"); } // Out of proc nodes do not know the startup directory so set the environment variable for them. diff --git a/src/Shared/ExceptionHandling.cs b/src/Shared/ExceptionHandling.cs index 2d91a466fbf..3f0b910b267 100644 --- a/src/Shared/ExceptionHandling.cs +++ b/src/Shared/ExceptionHandling.cs @@ -54,7 +54,7 @@ private static string GetDebugDumpPath() return !string.IsNullOrEmpty(debugPath) ? debugPath - : Path.GetTempPath(); + : FileUtilities.TempFileDirectory; } /// diff --git a/src/Shared/FileUtilities.cs b/src/Shared/FileUtilities.cs index da24b87b134..3861a120d79 100644 --- a/src/Shared/FileUtilities.cs +++ b/src/Shared/FileUtilities.cs @@ -45,6 +45,10 @@ internal static partial class FileUtilities /// internal static string cacheDirectory = null; +#if CLR2COMPATIBILITY + internal static string TempFileDirectory => Path.GetTempPath(); +#endif + /// /// FOR UNIT TESTS ONLY /// Clear out the static variable used for the cache directory so that tests that @@ -122,7 +126,7 @@ internal static string GetCacheDirectory() { if (cacheDirectory == null) { - cacheDirectory = Path.Combine(Path.GetTempPath(), String.Format(CultureInfo.CurrentUICulture, "MSBuild{0}-{1}", Process.GetCurrentProcess().Id, AppDomain.CurrentDomain.Id)); + cacheDirectory = Path.Combine(TempFileDirectory, String.Format(CultureInfo.CurrentUICulture, "MSBuild{0}-{1}", Process.GetCurrentProcess().Id, AppDomain.CurrentDomain.Id)); } return cacheDirectory; diff --git a/src/Shared/NamedPipeUtil.cs b/src/Shared/NamedPipeUtil.cs index dfc76317e84..4927b87103d 100644 --- a/src/Shared/NamedPipeUtil.cs +++ b/src/Shared/NamedPipeUtil.cs @@ -30,7 +30,13 @@ internal static string GetPlatformSpecificPipeName(string pipeName) // can be quite long, leaving very little room for the actual pipe name. Fortunately, // '/tmp' is mandated by POSIX to always be a valid temp directory, so we can use that // instead. +#if !CLR2COMPATIBILITY return Path.Combine("/tmp", pipeName); +#else + // We should never get here. This would be a net35 task host running on unix. + ErrorUtilities.ThrowInternalError("Task host used on unix in retrieving the pipe name."); + return string.Empty; +#endif } else { diff --git a/src/Shared/NodeEndpointOutOfProcBase.cs b/src/Shared/NodeEndpointOutOfProcBase.cs index 4c5a3357063..aefd4aaebb2 100644 --- a/src/Shared/NodeEndpointOutOfProcBase.cs +++ b/src/Shared/NodeEndpointOutOfProcBase.cs @@ -232,7 +232,11 @@ internal void InternalConstruct(string pipeName = null) PipeDirection.InOut, 1, // Only allow one connection at a time. PipeTransmissionMode.Byte, - PipeOptions.Asynchronous | PipeOptions.WriteThrough, + PipeOptions.Asynchronous | PipeOptions.WriteThrough +#if FEATURE_PIPEOPTIONS_CURRENTUSERONLY + | PipeOptions.CurrentUserOnly +#endif + , PipeBufferSize, // Default input buffer PipeBufferSize, // Default output buffer security, @@ -248,7 +252,11 @@ internal void InternalConstruct(string pipeName = null) PipeDirection.InOut, 1, // Only allow one connection at a time. PipeTransmissionMode.Byte, - PipeOptions.Asynchronous | PipeOptions.WriteThrough, + PipeOptions.Asynchronous | PipeOptions.WriteThrough +#if FEATURE_PIPEOPTIONS_CURRENTUSERONLY + | PipeOptions.CurrentUserOnly +#endif + , PipeBufferSize, // Default input buffer PipeBufferSize // Default output buffer ); diff --git a/src/Shared/TempFileUtilities.cs b/src/Shared/TempFileUtilities.cs index f3bffb9d425..c0583acb483 100644 --- a/src/Shared/TempFileUtilities.cs +++ b/src/Shared/TempFileUtilities.cs @@ -16,6 +16,57 @@ namespace Microsoft.Build.Shared /// internal static partial class FileUtilities { + // For the current user, these correspond to read, write, and execute permissions. + // Lower order bits correspond to the same for "group" or "other" users. + private const int userRWX = 0x100 | 0x80 | 0x40; + private static string tempFileDirectory = null; + internal static string TempFileDirectory + { + get + { + return tempFileDirectory ??= CreateFolderUnderTemp(); + } + } + + internal static void ClearTempFileDirectory() + { + tempFileDirectory = null; + } + + // For all native calls, directly check their return values to prevent bad actors from getting in between checking if a directory exists and returning it. + private static string CreateFolderUnderTemp() + { + string basePath = Path.Combine(Path.GetTempPath(), $"MSBuildTemp{Environment.UserName}"); + + if (NativeMethodsShared.IsLinux && NativeMethodsShared.mkdir(basePath, userRWX) != 0) + { + if (NativeMethodsShared.chmod(basePath, userRWX) == 0) + { + // Current user owns this file; we can read and write to it. It is reasonable here to assume it was created properly by MSBuild and can be used + // for temporary files. + } + else + { + // Another user created a folder pretending to be us! Find a folder we can actually use. + int extraBits = 0; + string pathToCheck = basePath + extraBits; + while (NativeMethodsShared.mkdir(pathToCheck, userRWX) != 0 && NativeMethodsShared.chmod(pathToCheck, userRWX) != 0) + { + extraBits++; + pathToCheck = basePath + extraBits; + } + + basePath = pathToCheck; + } + } + else + { + Directory.CreateDirectory(basePath); + } + + return FileUtilities.EnsureTrailingSlash(basePath); + } + /// /// Generates a unique directory name in the temporary folder. /// Caller must delete when finished. @@ -24,7 +75,7 @@ internal static partial class FileUtilities /// internal static string GetTemporaryDirectory(bool createDirectory = true, string subfolder = null) { - string temporaryDirectory = Path.Combine(Path.GetTempPath(), "Temporary" + Guid.NewGuid().ToString("N"), subfolder ?? string.Empty); + string temporaryDirectory = Path.Combine(TempFileDirectory, "Temporary" + Guid.NewGuid().ToString("N"), subfolder ?? string.Empty); if (createDirectory) { @@ -43,7 +94,7 @@ internal static string GetTemporaryDirectory(bool createDirectory = true, string /// internal static string GetTemporaryFileName(string extension) { - return GetTemporaryFile(null, extension, false); + return GetTemporaryFile(null, null, extension, false); } /// @@ -57,6 +108,16 @@ internal static string GetTemporaryFile() return GetTemporaryFile(".tmp"); } + /// + /// Generates a unique temporary file name with a given extension in the temporary folder. + /// File is guaranteed to be unique. + /// Caller must delete it when finished. + /// + internal static string GetTemporaryFile(string fileName, string extension, bool createFile) + { + return GetTemporaryFile(null, fileName, extension, createFile); + } + /// /// Generates a unique temporary file name with a given extension in the temporary folder. /// File is guaranteed to be unique. @@ -66,7 +127,7 @@ internal static string GetTemporaryFile() /// internal static string GetTemporaryFile(string extension) { - return GetTemporaryFile(null, extension); + return GetTemporaryFile(null, null, extension); } /// @@ -77,23 +138,33 @@ internal static string GetTemporaryFile(string extension) /// Caller must delete it when finished. /// May throw IOException. /// - internal static string GetTemporaryFile(string directory, string extension, bool createFile = true) + internal static string GetTemporaryFile(string directory, string fileName, string extension, bool createFile = true) { ErrorUtilities.VerifyThrowArgumentLengthIfNotNull(directory, nameof(directory)); - ErrorUtilities.VerifyThrowArgumentLength(extension, nameof(extension)); - - if (extension[0] != '.') - { - extension = '.' + extension; - } try { - directory ??= Path.GetTempPath(); + directory ??= TempFileDirectory; + + // If the extension needs a dot prepended, do so. + if (extension is null) + { + extension = string.Empty; + } + else if (extension.Length > 0 && extension[0] != '.') + { + extension = '.' + extension; + } + + // If the fileName is null, use tmp{Guid}; otherwise use fileName. + if (string.IsNullOrEmpty(fileName)) + { + fileName = $"tmp{Guid.NewGuid():N}"; + } Directory.CreateDirectory(directory); - string file = Path.Combine(directory, $"tmp{Guid.NewGuid():N}{extension}"); + string file = Path.Combine(directory, $"{fileName}{extension}"); ErrorUtilities.VerifyThrow(!FileSystems.Default.FileExists(file), "Guid should be unique"); @@ -131,11 +202,15 @@ public class TempWorkingDirectory : IDisposable { public string Path { get; } - public TempWorkingDirectory(string sourcePath, [CallerMemberName] string name = null) + public TempWorkingDirectory(string sourcePath, +#if !CLR2COMPATIBILITY + [CallerMemberName] +#endif + string name = null) { Path = name == null ? GetTemporaryDirectory() - : System.IO.Path.Combine(System.IO.Path.GetTempPath(), name); + : System.IO.Path.Combine(TempFileDirectory, name); if (FileSystems.Default.DirectoryExists(Path)) { diff --git a/src/Shared/UnitTests/FileUtilities_Tests.cs b/src/Shared/UnitTests/FileUtilities_Tests.cs index 3f9c53f3a68..7e87e2217af 100644 --- a/src/Shared/UnitTests/FileUtilities_Tests.cs +++ b/src/Shared/UnitTests/FileUtilities_Tests.cs @@ -851,7 +851,7 @@ public void GenerateTempFileNameWithDirectoryAndExtension() try { - path = FileUtilities.GetTemporaryFile(directory, ".bat"); + path = FileUtilities.GetTemporaryFile(directory, null, ".bat"); Assert.EndsWith(".bat", path); Assert.True(File.Exists(path)); @@ -902,18 +902,6 @@ public void GenerateTempBatchFileWithBadExtension() ); } /// - /// No extension is given - /// - [Fact] - public void GenerateTempBatchFileWithEmptyExtension() - { - Assert.Throws(() => - { - FileUtilities.GetTemporaryFile(String.Empty); - } - ); - } - /// /// Directory is invalid /// [Fact] @@ -924,7 +912,7 @@ public void GenerateTempBatchFileWithBadDirectory() { Assert.Throws(() => { - FileUtilities.GetTemporaryFile("|", ".tmp"); + FileUtilities.GetTemporaryFile("|", null, ".tmp"); } ); } diff --git a/src/Shared/UnitTests/TestEnvironment.cs b/src/Shared/UnitTests/TestEnvironment.cs index c97b575d88c..51ea0482d88 100644 --- a/src/Shared/UnitTests/TestEnvironment.cs +++ b/src/Shared/UnitTests/TestEnvironment.cs @@ -594,14 +594,14 @@ public TransientTestFile(string extension, bool createFile, bool expectedAsOutpu { _createFile = createFile; _expectedAsOutput = expectedAsOutput; - Path = FileUtilities.GetTemporaryFile(null, extension, createFile); + Path = FileUtilities.GetTemporaryFile(null, null, extension, createFile); } public TransientTestFile(string rootPath, string extension, bool createFile, bool expectedAsOutput) { _createFile = createFile; _expectedAsOutput = expectedAsOutput; - Path = FileUtilities.GetTemporaryFile(rootPath, extension, createFile); + Path = FileUtilities.GetTemporaryFile(rootPath, null, extension, createFile); } public TransientTestFile(string rootPath, string fileName, string contents = null) diff --git a/src/Tasks.UnitTests/Copy_Tests.cs b/src/Tasks.UnitTests/Copy_Tests.cs index 12705101c88..293fec51354 100644 --- a/src/Tasks.UnitTests/Copy_Tests.cs +++ b/src/Tasks.UnitTests/Copy_Tests.cs @@ -537,8 +537,8 @@ public void DoCopyOverCopiedFile(bool skipUnchangedFiles) { using (var env = TestEnvironment.Create()) { - var sourceFile = FileUtilities.GetTemporaryFile(env.DefaultTestDirectory.Path, "src", false); - var destinationFile = FileUtilities.GetTemporaryFile(env.DefaultTestDirectory.Path, "dst", false); + var sourceFile = FileUtilities.GetTemporaryFile(env.DefaultTestDirectory.Path, null, "src", false); + var destinationFile = FileUtilities.GetTemporaryFile(env.DefaultTestDirectory.Path, null, "dst", false); File.WriteAllText(sourceFile, "This is a source temp file."); diff --git a/src/Tasks/CodeTaskFactory.cs b/src/Tasks/CodeTaskFactory.cs index 77c8c929879..e3fd34e9fc6 100644 --- a/src/Tasks/CodeTaskFactory.cs +++ b/src/Tasks/CodeTaskFactory.cs @@ -811,19 +811,13 @@ private Assembly CompileInMemoryAssembly() var fullSpec = new FullTaskSpecification(finalReferencedAssemblies, fullCode); if (!s_compiledTaskCache.TryGetValue(fullSpec, out Assembly existingAssembly)) { - // Invokes compilation. - - // Note: CompileAssemblyFromSource uses Path.GetTempPath() directory, but will not create it. In some cases - // this will throw inside CompileAssemblyFromSource. To work around this, ensure the temp directory exists. - // See: https://github.com/dotnet/msbuild/issues/328 - Directory.CreateDirectory(Path.GetTempPath()); - + // Invokes compilation. CompilerResults compilerResults = provider.CompileAssemblyFromSource(compilerParameters, fullCode); string outputPath = null; if (compilerResults.Errors.Count > 0 || Environment.GetEnvironmentVariable("MSBUILDLOGCODETASKFACTORYOUTPUT") != null) { - string tempDirectory = Path.GetTempPath(); + string tempDirectory = FileUtilities.TempFileDirectory; string fileName = Guid.NewGuid().ToString() + ".txt"; outputPath = Path.Combine(tempDirectory, fileName); File.WriteAllText(outputPath, fullCode); diff --git a/src/Tasks/GetSDKReferenceFiles.cs b/src/Tasks/GetSDKReferenceFiles.cs index 1af05538af3..64afe2300aa 100644 --- a/src/Tasks/GetSDKReferenceFiles.cs +++ b/src/Tasks/GetSDKReferenceFiles.cs @@ -81,7 +81,7 @@ public class GetSDKReferenceFiles : TaskExtension /// /// Folder where the cache files are written to /// - private string _cacheFilePath = Path.GetTempPath(); + private string _cacheFilePath = FileUtilities.TempFileDirectory; #region Properties diff --git a/src/Tasks/RoslynCodeTaskFactory/RoslynCodeTaskFactory.cs b/src/Tasks/RoslynCodeTaskFactory/RoslynCodeTaskFactory.cs index f9fbcadfcae..abeba2b2791 100644 --- a/src/Tasks/RoslynCodeTaskFactory/RoslynCodeTaskFactory.cs +++ b/src/Tasks/RoslynCodeTaskFactory/RoslynCodeTaskFactory.cs @@ -677,8 +677,8 @@ private bool TryCompileInMemoryAssembly(IBuildEngine buildEngine, RoslynCodeTask // The source code cannot actually be compiled "in memory" so instead the source code is written to disk in // the temp folder as well as the assembly. After compilation, the source code and assembly are deleted. - string sourceCodePath = Path.GetTempFileName(); - string assemblyPath = Path.Combine(Path.GetTempPath(), $"{Path.GetRandomFileName()}.dll"); + string sourceCodePath = FileUtilities.GetTemporaryFileName(".tmp"); + string assemblyPath = FileUtilities.GetTemporaryFileName(".dll"); // Delete the code file unless compilation failed or the environment variable MSBUILDLOGCODETASKFACTORYOUTPUT // is set (which allows for debugging problems) diff --git a/src/Tasks/TlbReference.cs b/src/Tasks/TlbReference.cs index 7350cee5080..f08ae5f4d27 100644 --- a/src/Tasks/TlbReference.cs +++ b/src/Tasks/TlbReference.cs @@ -75,7 +75,7 @@ internal class TlbReference : AxTlbBaseReference, ITypeLibImporterNotifySink /// /// directory we should write the wrapper to /// - protected override string OutputDirectory => (HasTemporaryWrapper) ? Path.GetTempPath() : base.OutputDirectory; + protected override string OutputDirectory => (HasTemporaryWrapper) ? FileUtilities.TempFileDirectory : base.OutputDirectory; private readonly bool _noClassMembers; private readonly string _targetProcessorArchitecture; diff --git a/src/Tasks/WriteCodeFragment.cs b/src/Tasks/WriteCodeFragment.cs index 26325de8241..5088a0ff87e 100644 --- a/src/Tasks/WriteCodeFragment.cs +++ b/src/Tasks/WriteCodeFragment.cs @@ -109,7 +109,7 @@ public override bool Execute() OutputFile = new TaskItem(Path.Combine(OutputDirectory.ItemSpec, OutputFile.ItemSpec)); } - OutputFile ??= new TaskItem(FileUtilities.GetTemporaryFile(OutputDirectory.ItemSpec, extension)); + OutputFile ??= new TaskItem(FileUtilities.GetTemporaryFile(OutputDirectory.ItemSpec, null, extension)); File.WriteAllText(OutputFile.ItemSpec, code); // Overwrites file if it already exists (and can be overwritten) } diff --git a/src/Utilities/TrackedDependencies/FileTracker.cs b/src/Utilities/TrackedDependencies/FileTracker.cs index 7eb6306f4a1..a6fa201acc7 100644 --- a/src/Utilities/TrackedDependencies/FileTracker.cs +++ b/src/Utilities/TrackedDependencies/FileTracker.cs @@ -73,7 +73,7 @@ public static class FileTracker #region Static Member Data // The default path to temp, used to create explicitly short and long paths - private static readonly string s_tempPath = Path.GetTempPath(); + private static readonly string s_tempPath = FileUtilities.TempFileDirectory; // The short path to temp private static readonly string s_tempShortPath = FileUtilities.EnsureTrailingSlash(NativeMethodsShared.GetShortFilePath(s_tempPath).ToUpperInvariant());