Skip to content

allow reading uid from ContainerUser to set the Tar entry uid #49770

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
22 changes: 20 additions & 2 deletions src/Containers/Microsoft.NET.Build.Containers/ContainerBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ internal static async Task<int> ContainerizeAsync(
KnownImageFormats.OCI => SchemaTypes.OciManifestV1,
_ => imageBuilder.ManifestMediaType // should be impossible unless we add to the enum
};

Layer newLayer = Layer.FromDirectory(publishDirectory.FullName, workingDir, imageBuilder.IsWindows, imageBuilder.ManifestMediaType);
var userId = imageBuilder.IsWindows ? null : TryParseUserId(containerUser);
Layer newLayer = Layer.FromDirectory(publishDirectory.FullName, workingDir, imageBuilder.IsWindows, imageBuilder.ManifestMediaType, userId);
imageBuilder.AddLayer(newLayer);
imageBuilder.SetWorkingDirectory(workingDir);

Expand Down Expand Up @@ -200,6 +200,24 @@ internal static async Task<int> ContainerizeAsync(
return exitCode;
}

public static int? TryParseUserId(string? containerUser)
{
if (containerUser is null)
{
return null;
}
if (int.TryParse(containerUser, out int userId))
{
return userId;
}
if (containerUser.Equals("root", StringComparison.OrdinalIgnoreCase))
{
return 0; // root user
}
// TODO: on Linux we could _potentially_ try to map the user name to a UID
return null;
}

private static async Task<int> PushToLocalRegistryAsync(ILogger logger, BuiltImage builtImage, SourceImageReference sourceImageReference,
DestinationImageReference destinationImageReference,
CancellationToken cancellationToken)
Expand Down
41 changes: 26 additions & 15 deletions src/Containers/Microsoft.NET.Build.Containers/Layer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ namespace Microsoft.NET.Build.Containers;

internal class Layer
{
// NOTE: The SID string below was created using the following snippet. As the code is Windows only we keep the constant
// NOTE: The SID string below was created using the following snippet. As the code is Windows only we keep the constant,
// so that we can author Windows layers successfully on non-Windows hosts.
//
// private static string CreateUserOwnerAndGroupSID()
// {
// var descriptor = new RawSecurityDescriptor(
Expand Down Expand Up @@ -50,7 +52,7 @@ public static Layer FromDescriptor(Descriptor descriptor)
return new(ContentStore.PathForDescriptor(descriptor), descriptor);
}

public static Layer FromDirectory(string directory, string containerPath, bool isWindowsLayer, string manifestMediaType)
public static Layer FromDirectory(string directory, string containerPath, bool isWindowsLayer, string manifestMediaType, int? userId = null)
{
long fileSize;
Span<byte> hash = stackalloc byte[SHA256.HashSizeInBytes];
Expand Down Expand Up @@ -101,7 +103,7 @@ public static Layer FromDirectory(string directory, string containerPath, bool i
}

// Write an entry for the application directory.
WriteTarEntryForFile(writer, new DirectoryInfo(directory), containerPath, entryAttributes);
WriteTarEntryForFile(writer, new DirectoryInfo(directory), containerPath, entryAttributes, isWindowsLayer ? null : userId);

// Write entries for the application directory contents.
var fileList = new FileSystemEnumerable<(FileSystemInfo file, string containerPath)>(
Expand All @@ -124,7 +126,7 @@ public static Layer FromDirectory(string directory, string containerPath, bool i
});
foreach (var item in fileList)
{
WriteTarEntryForFile(writer, item.file, item.containerPath, entryAttributes);
WriteTarEntryForFile(writer, item.file, item.containerPath, entryAttributes, isWindowsLayer ? null : userId);
}

// Windows layers need a Hives folder, we do not need to create any Registry Hive deltas inside
Expand All @@ -148,27 +150,36 @@ public static Layer FromDirectory(string directory, string containerPath, bool i
Debug.Assert(bW == hash.Length);

// Writes a tar entry corresponding to the file system item.
static void WriteTarEntryForFile(TarWriter writer, FileSystemInfo file, string containerPath, IEnumerable<KeyValuePair<string, string>> entryAttributes)
static void WriteTarEntryForFile(TarWriter writer, FileSystemInfo file, string containerPath, IEnumerable<KeyValuePair<string, string>> entryAttributes, int? userId)
{
UnixFileMode mode = DetermineFileMode(file);
PaxTarEntry entry;

if (file is FileInfo)
{
using var fileStream = File.OpenRead(file.FullName);
PaxTarEntry entry = new(TarEntryType.RegularFile, containerPath, entryAttributes)
var fileStream = File.OpenRead(file.FullName);
entry = new(TarEntryType.RegularFile, containerPath, entryAttributes)
{
Mode = mode,
DataStream = fileStream
DataStream = fileStream,
};
Comment on lines +160 to 164
Copy link
Preview

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

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

The using statement was removed from the FileStream, but the manual disposal at line 182 only happens for entries with DataStream. This creates a resource leak if an exception occurs between opening the stream and writing the entry.

Copilot uses AI. Check for mistakes.

writer.WriteEntry(entry);
}
else
{
PaxTarEntry entry = new(TarEntryType.Directory, containerPath, entryAttributes)
{
Mode = mode
};
writer.WriteEntry(entry);
entry = new(TarEntryType.Directory, containerPath, entryAttributes);
}

entry.Mode = mode;
if (userId is int uid)
{
entry.Uid = uid;
}

writer.WriteEntry(entry);

if (entry.DataStream is not null)
{
// no longer relying on the `using` of the FileStream, so need to do it manually
entry.DataStream.Dispose();
}

static UnixFileMode DetermineFileMode(FileSystemInfo file)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ internal async Task<bool> ExecuteAsync(CancellationToken cancellationToken)
Log.LogErrorWithCodeFromResources(nameof(Strings.InvalidContainerImageFormat), ImageFormat, string.Join(",", Enum.GetValues<KnownImageFormats>()));
}
}

Layer newLayer = Layer.FromDirectory(PublishDirectory, WorkingDirectory, imageBuilder.IsWindows, imageBuilder.ManifestMediaType);
var userId = imageBuilder.IsWindows ? null : ContainerBuilder.TryParseUserId(ContainerUser);
Layer newLayer = Layer.FromDirectory(PublishDirectory, WorkingDirectory, imageBuilder.IsWindows, imageBuilder.ManifestMediaType, userId);
imageBuilder.AddLayer(newLayer);
imageBuilder.SetWorkingDirectory(WorkingDirectory);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,26 @@ public void SingleFileInHiddenFolder()
Assert.True(allEntries.TryGetValue("app/wwwroot/.well-known/TestFile.txt", out var fileEntry) && fileEntry.EntryType == TarEntryType.RegularFile, "Missing app/wwwroot/.well-known/TestFile.txt file entry");
}

[Fact]
public void UserIdIsAppliedToFiles()
{
using TransientTestFolder folder = new();

string testFilePath = Path.Join(folder.Path, "TestFile.txt");
string testString = $"Test content for {nameof(SingleFileInFolder)}";
Copy link
Preview

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

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

The nameof reference is incorrect - it should be {nameof(UserIdIsAppliedToFiles)} since that's the current test method name, not SingleFileInFolder.

Suggested change
string testString = $"Test content for {nameof(SingleFileInFolder)}";
string testString = $"Test content for {nameof(UserIdIsAppliedToFiles)}";

Copilot uses AI. Check for mistakes.

File.WriteAllText(testFilePath, testString);

var userId = 1234;
Layer l = Layer.FromDirectory(directory: folder.Path, containerPath: "/app", false, SchemaTypes.DockerManifestV2, userId: userId);
var allEntries = LoadAllTarEntries(l.BackingFile);
Assert.True(allEntries.TryGetValue("app", out var appEntry) && appEntry.EntryType == TarEntryType.Directory, "Missing app directory entry");
Assert.True(allEntries.TryGetValue("app/TestFile.txt", out var fileEntry) && fileEntry.EntryType == TarEntryType.RegularFile, "Missing TestFile.txt file entry");
Assert.All(allEntries.Values, entry =>
{
Assert.True(entry.Uid == userId, $"Expected UID {userId} for entry {entry.Name}, but got {entry.Uid}");
});
}

private static void VerifyDescriptorInfo(Layer l)
{
Assert.Equal(l.Descriptor.Size, new FileInfo(l.BackingFile).Length);
Expand Down