OCI builder: fixes from review of #2602

Libraries/src/AWS.OCI.Builder/BaseImagePullOptions.cs

@@ -0,0 +1,30 @@
+using System.Net;
+
+namespace AWS.OCI.Builder;
+
+/// <summary>
+/// Options controlling how the base image is pulled by <see cref="OciImageBuilder.FromBaseImageAsync(string, string, BaseImagePullOptions?, CancellationToken)"/>.
+/// </summary>
+/// <remarks>
+/// All properties are optional. The registry client created with these options also downloads the base image's
+/// layers when <see cref="OciImageBuilder.PushToEcrAsync"/> copies them to ECR, so they apply to those downloads too.
+/// </remarks>
+public sealed class BaseImagePullOptions
+{
+    /// <summary>
+    /// The proxy for requests to the base image's registry, or <see langword="null"/> (the default) for
+    /// <see cref="System.Net.Http.HttpClient.DefaultProxy"/>, which reads the <c>HTTPS_PROXY</c>, <c>HTTP_PROXY</c>
+    /// and <c>NO_PROXY</c> environment variables (and, on Windows, the system proxy settings).
+    /// </summary>
+    public IWebProxy? Proxy { get; init; }
+
+    /// <summary>
+    /// The HTTP timeout for each request to the base image's registry. Defaults to
+    /// <see cref="System.Threading.Timeout.InfiniteTimeSpan"/>, so a request is bounded only by the
+    /// <see cref="CancellationToken"/> passed to the pull, or to the push for a base layer download.
+    /// </summary>
+    /// <remarks>
+    /// A finite value must be positive; zero or a negative value is rejected when the pull starts.
+    /// </remarks>
+    public TimeSpan Timeout { get; init; } = System.Threading.Timeout.InfiniteTimeSpan;
+}

Libraries/src/AWS.OCI.Builder/EcrPushOptions.cs

@@ -1,3 +1,5 @@
+using System.Net;
+
 namespace AWS.OCI.Builder;
 
 /// <summary>
@@ -59,10 +61,11 @@ public sealed class EcrPushOptions
     /// so the contract holds even if layers are later uploaded concurrently — see <see cref="LayerPushProgress"/>.
     /// </para>
     /// <para>
-    /// Use <see cref="System.Progress{T}"/> for the common case: it captures the current
-    /// <see cref="System.Threading.SynchronizationContext"/> and raises callbacks on it, which keeps UI updates on
-    /// the UI thread. Any custom implementation must be thread-safe, since a future parallel push may report from
-    /// multiple threads at once.
+    /// In a UI app, use <see cref="System.Progress{T}"/>: it raises callbacks on the
+    /// <see cref="System.Threading.SynchronizationContext"/> it captured, which keeps UI updates on the UI thread.
+    /// Where there is no synchronization context (console apps, services, PowerShell), <see cref="System.Progress{T}"/>
+    /// raises callbacks on thread-pool threads, possibly several at once. To read events on your own thread instead,
+    /// use <see cref="LayerPushProgressQueue"/>. Any custom implementation must be thread-safe.
     /// </para>
     /// </remarks>
     public IProgress<LayerPushProgress>? Progress { get; init; }
@@ -81,4 +84,15 @@ public sealed class EcrPushOptions
     /// (zero or negative) is rejected at push time.
     /// </remarks>
     public TimeSpan UploadTimeout { get; init; } = System.Threading.Timeout.InfiniteTimeSpan;
+
+    /// <summary>
+    /// The proxy for requests to the target ECR registry, or <see langword="null"/> (the default) for
+    /// <see cref="System.Net.Http.HttpClient.DefaultProxy"/>, which reads the <c>HTTPS_PROXY</c>, <c>HTTP_PROXY</c>
+    /// and <c>NO_PROXY</c> environment variables (and, on Windows, the system proxy settings).
+    /// </summary>
+    /// <remarks>
+    /// This proxy is used only for ECR. Base layers are downloaded from their source registry by the client that
+    /// pulled the base image; set <see cref="BaseImagePullOptions.Proxy"/> for those requests.
+    /// </remarks>
+    public IWebProxy? Proxy { get; init; }
 }

Libraries/src/AWS.OCI.Builder/FileManifest.cs

@@ -27,10 +27,14 @@ namespace AWS.OCI.Builder;
 public sealed class FileManifest : IEnumerable<FileManifestEntry>
 {
     private readonly List<FileManifestEntry> _entries = new();
+    private readonly List<string> _emptyDirectories = new();
 
     /// <summary>The entries that make up this manifest.</summary>
     public IReadOnlyList<FileManifestEntry> Entries => _entries;
 
+    /// <summary>The container folders added with <see cref="AddEmptyDirectory"/>, which have no local source.</summary>
+    public IReadOnlyList<string> EmptyDirectories => _emptyDirectories;
+
     /// <summary>
     /// Adds a local file or directory to be placed at <paramref name="containerPath"/> inside the container, with
     /// its Unix mode derived from the source (on Linux/macOS) or defaulted (on Windows).
@@ -61,6 +65,23 @@ public void Add(string localPath, string containerPath, UnixFileMode mode)
     public void AddExecutable(string localPath, string containerPath)
         => _entries.Add(new FileManifestEntry(localPath, containerPath, FileManifestEntry.ExecutableMode));
 
+    /// <summary>
+    /// Adds an empty folder entry (mode <c>0755</c>) at <paramref name="containerPath"/>. Unlike the other
+    /// <c>Add</c> methods, it maps nothing from the local filesystem.
+    /// </summary>
+    /// <remarks>
+    /// The layer has no entries for the parent folders of mapped files; tar extraction creates them implicitly, and
+    /// leaving them out keeps the base image's own folders (for example <c>/tmp</c>'s <c>1777</c>) unchanged. Some
+    /// image consumers require an entry for every folder a layer creates, so add each new folder here. Don't add
+    /// folders the base image already has, or their mode is reset to <c>0755</c>.
+    /// </remarks>
+    /// <param name="containerPath">The absolute folder path inside the container (for example <c>/opt/app/lib</c>).</param>
+    public void AddEmptyDirectory(string containerPath)
+    {
+        ArgumentException.ThrowIfNullOrWhiteSpace(containerPath);
+        _emptyDirectories.Add(containerPath);
+    }
+
     /// <inheritdoc />
     public IEnumerator<FileManifestEntry> GetEnumerator() => _entries.GetEnumerator();
 

Libraries/src/AWS.OCI.Builder/ImageReference.cs

@@ -47,7 +47,10 @@ private ImageReference(string registry, string repository, string reference, boo
     public bool ReferenceIsDigest { get; }
 
     /// <summary>Whether this reference points at an Amazon ECR registry.</summary>
-    public bool IsEcr => EcrHostRegex().IsMatch(Registry);
+    public bool IsEcr => IsEcrHost(Registry);
+
+    /// <summary>Whether <paramref name="registryHost"/> is an Amazon ECR registry host.</summary>
+    internal static bool IsEcrHost(string registryHost) => EcrHostRegex().IsMatch(registryHost);
 
     /// <summary>
     /// The AWS region embedded in an ECR registry host (for example <c>us-west-2</c>), or <see langword="null"/>
@@ -177,6 +180,7 @@ public override string ToString()
 
     // Matches an ECR host and captures the region: <account>.dkr.ecr.<region>.amazonaws.com
     // (also supports the FIPS and China/GovCloud partitions' domains).
-    [GeneratedRegex(@"^\d{12}\.dkr\.ecr(-fips)?\.(?<region>[a-z0-9-]+)\.amazonaws\.com(\.cn)?$", RegexOptions.IgnoreCase)]
+    // Anchored with \z, not $, so a trailing newline can't pass (the host decides which registry is trusted).
+    [GeneratedRegex(@"^\d{12}\.dkr\.ecr(-fips)?\.(?<region>[a-z0-9-]+)\.amazonaws\.com(\.cn)?\z", RegexOptions.IgnoreCase)]
     private static partial Regex EcrHostRegex();
 }

Libraries/src/AWS.OCI.Builder/Images/Layer.cs

@@ -1,6 +1,7 @@
 using System.Formats.Tar;
 using System.IO.Compression;
 using System.Security.Cryptography;
+using System.Text;
 
 namespace AWS.OCI.Builder.Images;
 
@@ -28,8 +29,10 @@ namespace AWS.OCI.Builder.Images;
 ///   </item>
 /// </list>
 /// <para>
-/// Entries are written in a stable sorted order with fixed timestamps and normalized paths. Digests are not
-/// guaranteed to be identical across builds, even for identical input.
+/// Entries are written in a stable sorted order with fixed timestamps, owners and normalized paths, so the same
+/// input produces the same digests on every build with the same .NET major version. Different major versions can
+/// produce different bytes, both compressed (their zlib differs) and uncompressed (.NET 8 writes the ustar uid and
+/// gid fields as NULs, .NET 10 as zeros), so the digest and the diff_id can both change across them.
 /// </para>
 /// </remarks>
 internal sealed class Layer
@@ -117,9 +120,9 @@ public static async Task<Layer> CreateAsync(
         {
             throw new ArgumentException("The destination stream must be writable.", nameof(destination));
         }
-        if (manifest.Entries.Count == 0)
+        if (manifest.Entries.Count == 0 && manifest.EmptyDirectories.Count == 0)
         {
-            throw new ArgumentException("The file manifest is empty; a layer must contain at least one file.", nameof(manifest));
+            throw new ArgumentException("The file manifest is empty; a layer must contain at least one file or folder.", nameof(manifest));
         }
 
         // Expand every manifest entry into concrete file additions (container path -> local file + mode), plus the
@@ -132,6 +135,16 @@ public static async Task<Layer> CreateAsync(
         {
             CollectEntry(entry, files, directories);
         }
+        foreach (string directory in manifest.EmptyDirectories)
+        {
+            string containerPath = NormalizeContainerPath(directory);
+            if (files.ContainsKey(containerPath))
+            {
+                throw new ArgumentException($"The container path '{containerPath}' is mapped as both a file and a folder.");
+            }
+            directories.Add(containerPath);
+        }
+        RejectEntriesUnderFiles(files, directories);
 
         // One streaming pass:  tar -> [hash: diff_id] -> gzip -> [normalize gzip header] -> [hash + count: digest,
         // size] -> destination. Nothing is buffered beyond stream buffers, so layer size is not limited by memory
@@ -157,6 +170,26 @@ public static async Task<Layer> CreateAsync(
         return new Layer(digest, diffId, compressedSink.BytesWritten, mediaType);
     }
 
+    /// <summary>
+    /// Throws when a file or folder would sit beneath a path that is mapped as a file, which no filesystem can
+    /// represent: extracting such a layer fails or silently drops one of the entries.
+    /// </summary>
+    private static void RejectEntriesUnderFiles(SortedDictionary<string, FileSource> files, SortedSet<string> directories)
+    {
+        foreach (string path in files.Keys.Concat(directories))
+        {
+            for (int slash = path.LastIndexOf('/'); slash > 0; slash = path.LastIndexOf('/', slash - 1))
+            {
+                string ancestor = path[..slash];
+                if (files.ContainsKey(ancestor))
+                {
+                    throw new ArgumentException(
+                        $"The container path '{path}' is beneath '{ancestor}', which is mapped as a file.");
+                }
+            }
+        }
+    }
+
     private static string ToDigestString(IncrementalHash hash)
         // Convert.ToHexStringLower exists only on net9.0+, so lower-case explicitly for net8.0 compatibility.
         => "sha256:" + Convert.ToHexString(hash.GetHashAndReset()).ToLowerInvariant();
@@ -296,14 +329,23 @@ private static UnixFileMode ResolveFileMode(string localPath, UnixFileMode? expl
     }
 
     /// <summary>
-    /// Writes the directory and file entries as a PAX tar archive, in a deterministic order, to
+    /// Writes the directory and file entries as a tar archive, in a deterministic order, to
     /// <paramref name="destination"/> (the hashing/compression pipeline). Source files are streamed, not buffered.
     /// </summary>
     /// <remarks>
-    /// PAX is the format used by container tooling (including <c>Microsoft.NET.Build.Containers</c>) and is what
-    /// registries and runtimes expect for image layers. Every entry gets a fixed extended-attribute set (see
-    /// <see cref="ReproducibleExtendedAttributes"/>) and a fixed <see cref="ReproducibleTimestamp"/> rather than
-    /// the current time.
+    /// <para>
+    /// Entries are written as ustar wherever ustar can hold them, so the archive is byte-for-byte reproducible.
+    /// ustar has no atime or ctime fields, and every entry gets the fixed <see cref="ReproducibleTimestamp"/>.
+    /// The alternatives can't be made reproducible on every runtime the library supports. .NET's PAX writer names
+    /// each extended header <c>PaxHeaders.&lt;process id&gt;</c>. .NET 8's GNU writer stamps the current time into
+    /// atime and ctime and rejects anything earlier than the epoch, and the epoch written there makes readers that
+    /// treat those bytes as a ustar prefix (Python's <c>tarfile</c>) misread every name.
+    /// </para>
+    /// <para>
+    /// An entry ustar can't hold falls back to PAX with fixed time attributes: a name too long for ustar's 100-byte
+    /// name and 155-byte prefix fields, or a file of 8 GiB or more. Only layers with such entries vary between
+    /// processes.
+    /// </para>
     /// </remarks>
     private static async Task WriteTarAsync(
         SortedDictionary<string, FileSource> files,
@@ -312,48 +354,97 @@ private static async Task WriteTarAsync(
         CancellationToken cancellationToken)
     {
         // leaveOpen: true — the destination is the hashing/compression pipeline, finalized by the caller.
-        await using (var tar = new TarWriter(destination, TarEntryFormat.Pax, leaveOpen: true))
+        await using (var tar = new TarWriter(destination, TarEntryFormat.Ustar, leaveOpen: true))
         {
             // Directories first, shallowest-to-deepest (the sorted set already yields a stable order), so every
             // file's parent exists before the file entry. Both collections are ordinally sorted for determinism.
             foreach (string dir in directories)
             {
                 cancellationToken.ThrowIfCancellationRequested();
-                var dirEntry = new PaxTarEntry(TarEntryType.Directory, ToTarEntryName(dir, isDirectory: true), ReproducibleExtendedAttributes())
-                {
-                    Mode = DefaultDirectoryMode,
-                    ModificationTime = ReproducibleTimestamp,
-                };
+                TarEntry dirEntry = CreateEntry(TarEntryType.Directory, ToTarEntryName(dir, isDirectory: true), DefaultDirectoryMode, size: 0);
                 await tar.WriteEntryAsync(dirEntry, cancellationToken).ConfigureAwait(false);
             }
 
             foreach ((string containerPath, FileSource file) in files)
             {
                 cancellationToken.ThrowIfCancellationRequested();
-                var fileEntry = new PaxTarEntry(TarEntryType.RegularFile, ToTarEntryName(containerPath, isDirectory: false), ReproducibleExtendedAttributes())
-                {
-                    Mode = file.Mode,
-                    ModificationTime = ReproducibleTimestamp,
-                };
 
                 // Open each source file read-only and shared so concurrent readers (e.g. the build) aren't blocked.
                 await using FileStream source = new(file.LocalPath, FileMode.Open, FileAccess.Read, FileShare.Read);
+                TarEntry fileEntry = CreateEntry(TarEntryType.RegularFile, ToTarEntryName(containerPath, isDirectory: false), file.Mode, source.Length);
                 fileEntry.DataStream = source;
                 await tar.WriteEntryAsync(fileEntry, cancellationToken).ConfigureAwait(false);
             }
         }
     }
 
-    /// <summary>A resolved file to place in the layer: its source path and the Unix mode to stamp on it.</summary>
-    private readonly record struct FileSource(string LocalPath, UnixFileMode Mode);
+    /// <summary>Creates a ustar entry, or a PAX entry when ustar can't hold the name or size (see <see cref="FitsUstar"/>).</summary>
+    internal static TarEntry CreateEntry(TarEntryType type, string name, UnixFileMode mode, long size)
+    {
+        // A directory name longer than ustar's name field loses its trailing '/'. Otherwise the last '/' is the
+        // trailing one and the name field would be left empty, which tar itself never writes; the type flag already
+        // marks the entry as a directory.
+        if (type == TarEntryType.Directory && Encoding.UTF8.GetByteCount(name) > UstarNameFieldLength)
+        {
+            name = name.TrimEnd('/');
+        }
+
+        TarEntry entry = FitsUstar(name, size)
+            ? new UstarTarEntry(type, name)
+            : new PaxTarEntry(type, name, ReproducibleExtendedAttributes());
+        entry.Mode = mode;
+        entry.ModificationTime = ReproducibleTimestamp;
+        return entry;
+    }
+
+    // ustar header limits: a 100-byte name field, a 155-byte prefix field, and an 11-digit octal size field, so the
+    // largest size is 8^11 - 1 (one byte under 8 GiB).
+    private const int UstarNameFieldLength = 100;
+    private const int UstarPrefixFieldLength = 155;
+    private const long UstarMaxSize = (1L << 33) - 1;
 
     /// <summary>
-    /// The PAX extended attributes applied to every entry: fixed <c>mtime</c>, <c>atime</c> and <c>ctime</c>.
+    /// Whether an entry can be written as ustar: its size fits the size field, and its UTF-8 name either fits the
+    /// name field or splits at its last <c>/</c> into a prefix that fits the prefix field and a non-empty name that
+    /// fits the name field. Other names use PAX.
     /// </summary>
     /// <remarks>
-    /// On .NET 8 the PAX writer otherwise fills <c>atime</c> and <c>ctime</c> with the current wall-clock time;
-    /// pinning all three to a fixed epoch keeps them out of the archive.
+    /// .NET's ustar writer chooses the split itself. It always accepts a name that fits the name field, and a name
+    /// whose split at the last <c>/</c> fits, because it tries that split first. Some names that only fit when split at
+    /// an earlier <c>/</c> are rejected by .NET, so this predicate doesn't accept them, and they use PAX. Names up to
+    /// 255 bytes therefore stay ustar in the common case of a short file name under a long folder path; a PAX entry
+    /// would carry .NET's <c>PaxHeaders.&lt;process id&gt;</c> header and make the layer differ across processes.
     /// </remarks>
+    internal static bool FitsUstar(string name, long size)
+    {
+        if (size > UstarMaxSize)
+        {
+            return false;
+        }
+
+        int length = Encoding.UTF8.GetByteCount(name);
+        if (length <= UstarNameFieldLength)
+        {
+            return true;
+        }
+        if (length > UstarPrefixFieldLength + 1 + UstarNameFieldLength)
+        {
+            return false;
+        }
+
+        // The prefix and name fields are joined with a '/' on read, so the '/' itself is in neither field. A split at
+        // index 0 would leave an empty prefix, which readers take as no prefix.
+        byte[] utf8 = Encoding.UTF8.GetBytes(name);
+        int slash = Array.LastIndexOf(utf8, (byte)'/');
+        return slash >= 1
+            && slash <= UstarPrefixFieldLength
+            && length - slash - 1 is > 0 and <= UstarNameFieldLength;
+    }
+
+    /// <summary>
+    /// The PAX extended attributes for an entry that falls back to PAX: fixed <c>mtime</c>, <c>atime</c> and
+    /// <c>ctime</c> (the .NET 8 writer otherwise fills <c>atime</c> and <c>ctime</c> with the current time).
+    /// </summary>
     private static Dictionary<string, string> ReproducibleExtendedAttributes()
     {
         string epoch = ReproducibleTimestamp.ToUnixTimeSeconds().ToString("D", System.Globalization.CultureInfo.InvariantCulture);
@@ -365,6 +456,9 @@ private static Dictionary<string, string> ReproducibleExtendedAttributes()
         };
     }
 
+    /// <summary>A resolved file to place in the layer: its source path and the Unix mode to stamp on it.</summary>
+    private readonly record struct FileSource(string LocalPath, UnixFileMode Mode);
+
     /// <summary>
     /// Normalizes a caller-supplied container path to an absolute, forward-slash path with no trailing slash
     /// and no redundant segments.