Skip to content

Fix nuget config not being honored #20930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions src/Cli/dotnet/StatInterop.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Runtime.InteropServices;
using Microsoft.Win32.SafeHandles;

namespace Microsoft.DotNet.Cli
{
// https://github.com/dotnet/runtime/blob/main/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Stat.cs

internal static class StatInterop
{
// Even though csc will by default use a sequential layout, a CS0649 warning as error
// is produced for un-assigned fields when no StructLayout is specified.
//
// Explicitly saying Sequential disables that warning/error for consumers which only
// use Stat in debug builds.
[StructLayout(LayoutKind.Sequential)]
internal struct FileStatus
{
internal FileStatusFlags Flags;
internal int Mode;
internal uint Uid;
internal uint Gid;
internal long Size;
internal long ATime;
internal long ATimeNsec;
internal long MTime;
internal long MTimeNsec;
internal long CTime;
internal long CTimeNsec;
internal long BirthTime;
internal long BirthTimeNsec;
internal long Dev;
internal long Ino;
internal uint UserFlags;
}

[Flags]
internal enum Permissions
{
Mask = S_IRWXU | S_IRWXG | S_IRWXO,

S_IRWXU = S_IRUSR | S_IWUSR | S_IXUSR,
S_IRUSR = 0x100,
S_IWUSR = 0x80,
S_IXUSR = 0x40,

S_IRWXG = S_IRGRP | S_IWGRP | S_IXGRP,
S_IRGRP = 0x20,
S_IWGRP = 0x10,
S_IXGRP = 0x8,

S_IRWXO = S_IROTH | S_IWOTH | S_IXOTH,
S_IROTH = 0x4,
S_IWOTH = 0x2,
S_IXOTH = 0x1,

S_IXUGO = S_IXUSR | S_IXGRP | S_IXOTH,
}

[Flags]
internal enum FileStatusFlags
{
None = 0,
HasBirthTime = 1,
}

[DllImport("libSystem.Native", EntryPoint = "SystemNative_LStat", SetLastError = true)]
internal static extern int LStat(string path, out FileStatus output);
}
}
93 changes: 93 additions & 0 deletions src/Cli/dotnet/SudoEnvironmentDirectoryOverride.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
using System;
using System.CommandLine.Parsing;
using System.IO;
using System.Linq;
using Microsoft.DotNet.Cli.Utils;
using Microsoft.DotNet.Tools.Common;
using NuGet.Common;
using NuGet.Configuration;

namespace Microsoft.DotNet.Cli
{
Expand Down Expand Up @@ -32,12 +37,100 @@ public static void OverrideEnvironmentVariableToTmp(ParseResult parseResult)
{
if (!OperatingSystem.IsWindows() && IsRunningUnderSudo() && IsRunningWorkloadCommand(parseResult))
{
if (!TempHomeIsOnlyRootWritable(SudoHomeDirectory))
{
try
{
Directory.Delete(SudoHomeDirectory, recursive: true);
}
catch (DirectoryNotFoundException)
{
// Avoid read after write race condition
}
}

Directory.CreateDirectory(SudoHomeDirectory);

var homeBeforeOverride = Path.Combine(Environment.GetEnvironmentVariable("HOME"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I must have missed this in the first check in of this change, but directly using the HOME env var doesn't match the logic we use for calculating the home dir here: https://github.com/dotnet/sdk/blob/release/6.0.1xx/src/Common/CliFolderPathCalculatorCore.cs#L35, should we be taking those other env vars into account?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine. If the user override the home directory, they are in control of the situation. We just need to make sure the default make sense.

Environment.SetEnvironmentVariable("HOME", SudoHomeDirectory);

CopyUserNuGetConfigToOverriddenHome(homeBeforeOverride);
}
}

/// <summary>
/// To make NuGet honor the user's NuGet config file.
/// Copying instead of using the file directoy to avoid existing file being set higher permission
/// Try to delete the existing NuGet config file in "/tmp/dotnet_sudo_home/"
/// to avoid different user's NuGet config getting mixed.
/// </summary>
private static void CopyUserNuGetConfigToOverriddenHome(string homeBeforeOverride)
{
// https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Common/PathUtil/NuGetEnvironment.cs#L139
// home is cache in NuGet we cannot directly use the call
var userSettingsDir = Path.Combine(homeBeforeOverride, ".nuget", "NuGet");

string userNuGetConfig = Settings.OrderedSettingsFileNames
.Select(fileName => Path.Combine(userSettingsDir, fileName))
.FirstOrDefault(f => File.Exists(f));

var overridenSettingsDir = NuGetEnvironment.GetFolderPath(NuGetFolderPath.UserSettingsDirectory);
var overridenNugetConfig = Path.Combine(overridenSettingsDir, Settings.DefaultSettingsFileName);

if (File.Exists(overridenNugetConfig))
{
try
{
FileAccessRetrier.RetryOnIOException(
() => File.Delete(overridenNugetConfig));
}
catch
{
// best effort to remove
}
}

if (userNuGetConfig != default)
{
try
{
FileAccessRetrier.RetryOnIOException(
() => File.Copy(userNuGetConfig, overridenNugetConfig, overwrite: true));
}
catch
{
// best effort to copy
}
}
}

private static bool IsRunningWorkloadCommand(ParseResult parseResult) =>
parseResult.RootSubCommandResult() == (WorkloadCommandParser.GetCommand().Name);

private static bool TempHomeIsOnlyRootWritable(string path)
{
if (StatInterop.LStat(path, out StatInterop.FileStatus fileStat) != 0)
{
return false;
}

return IsOwnedByRoot(fileStat) && GroupCannotWrite(fileStat) &&
OtherUserCannotWrite(fileStat);
}

private static bool OtherUserCannotWrite(StatInterop.FileStatus fileStat)
{
return (fileStat.Mode & (int) StatInterop.Permissions.S_IWOTH) == 0;
}

private static bool GroupCannotWrite(StatInterop.FileStatus fileStat)
{
return (fileStat.Mode & (int) StatInterop.Permissions.S_IWGRP) == 0;
}

private static bool IsOwnedByRoot(StatInterop.FileStatus fileStat)
{
return fileStat.Uid == 0;
}
}
}