From a6a72dd5edf729626de1e69ed53ea49ca58b0800 Mon Sep 17 00:00:00 2001 From: Jonathan Nunn Date: Thu, 29 Oct 2026 04:16:02 +0000 Subject: [PATCH 1/6] Exempt only ECR hosts and the configured proxy from the connect guard The connect guard refused every loopback or private address, including the proxy connection, so pushes failed behind any local or corporate proxy, and ECR behind an interface VPC endpoint (which resolves to private addresses) was refused too. - Skip the address check for an Amazon ECR registry host and for the configured proxy: the target of the handler's CONNECT tunnel request, or the proxy that IWebProxy.GetProxy selects (not when it returns the destination itself). Every other host, including caller-supplied base registries, is still checked. - Match the ECR host pattern against the host alone, anchored with \z. - Report transport failures (DNS, connection, TLS, proxy, the guard) as RegistryException with the HttpRequestException as the inner exception. - RegistryClient.Create takes internal proxy and resolveHost parameters so tests can drive a local proxy and simulate private DNS. --- .../src/AWS.OCI.Builder/ImageReference.cs | 8 +- .../Registry/RegistryClient.cs | 133 +++++++++++++-- .../ConnectGuardTests.cs | 155 ++++++++++++++++++ .../test/AWS.OCI.Builder.Tests/TestHttp.cs | 32 ++++ 4 files changed, 312 insertions(+), 16 deletions(-) create mode 100644 Libraries/test/AWS.OCI.Builder.Tests/ConnectGuardTests.cs create mode 100644 Libraries/test/AWS.OCI.Builder.Tests/TestHttp.cs diff --git a/Libraries/src/AWS.OCI.Builder/ImageReference.cs b/Libraries/src/AWS.OCI.Builder/ImageReference.cs index 01b7ca6d2..c0645e42e 100644 --- a/Libraries/src/AWS.OCI.Builder/ImageReference.cs +++ b/Libraries/src/AWS.OCI.Builder/ImageReference.cs @@ -47,7 +47,10 @@ private ImageReference(string registry, string repository, string reference, boo public bool ReferenceIsDigest { get; } /// Whether this reference points at an Amazon ECR registry. - public bool IsEcr => EcrHostRegex().IsMatch(Registry); + public bool IsEcr => IsEcrHost(Registry); + + /// Whether is an Amazon ECR registry host. + internal static bool IsEcrHost(string registryHost) => EcrHostRegex().IsMatch(registryHost); /// /// The AWS region embedded in an ECR registry host (for example us-west-2), or @@ -177,6 +180,7 @@ public override string ToString() // Matches an ECR host and captures the region: .dkr.ecr..amazonaws.com // (also supports the FIPS and China/GovCloud partitions' domains). - [GeneratedRegex(@"^\d{12}\.dkr\.ecr(-fips)?\.(?[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)?\.(?[a-z0-9-]+)\.amazonaws\.com(\.cn)?\z", RegexOptions.IgnoreCase)] private static partial Regex EcrHostRegex(); } diff --git a/Libraries/src/AWS.OCI.Builder/Registry/RegistryClient.cs b/Libraries/src/AWS.OCI.Builder/Registry/RegistryClient.cs index c65cce80f..0c21fbffc 100644 --- a/Libraries/src/AWS.OCI.Builder/Registry/RegistryClient.cs +++ b/Libraries/src/AWS.OCI.Builder/Registry/RegistryClient.cs @@ -59,7 +59,14 @@ private RegistryClient(string registryHost, HttpClient httpClient, bool ownsHttp /// PATCH layer upload is bounded by the caller's cancellation token rather than cancelled by the 100-second /// client default. Must be positive or . /// - public static RegistryClient Create(string registryHost, IRegistryCredentialSource credentialSource, TimeSpan? timeout = null) + /// An explicit proxy, or for the system default proxy. + /// Test-only: replaces DNS resolution in the connect guard. + public static RegistryClient Create( + string registryHost, + IRegistryCredentialSource credentialSource, + TimeSpan? timeout = null, + IWebProxy? proxy = null, + Func>? resolveHost = null) { // Automatic redirects are DISABLED: the same transport also carries the token requests issued by // AuthHandshakeHandler, and an auto-followed redirect from a validated public realm to a private/loopback @@ -72,8 +79,15 @@ public static RegistryClient Create(string registryHost, IRegistryCredentialSour // SSRF defense at the socket layer: resolve the target, reject any blocked (loopback/private/link-local) // address, and connect to the SAME address we validated — so a DNS name that resolves to a private // address is refused, and DNS rebinding cannot swap in a different address between check and connect. - ConnectCallback = GuardedConnectAsync, + // An Amazon ECR registry is trusted even on a private address, which is how ECR resolves through an + // interface VPC endpoint. Any other host, including a caller-supplied base registry, is checked. + ConnectCallback = (context, cancellationToken) => + GuardedConnectAsync(context, TrustedHostOf(registryHost), proxy ?? HttpClient.DefaultProxy, resolveHost, cancellationToken), }; + if (proxy is not null) + { + inner.Proxy = proxy; + } var authHandler = new AuthHandshakeHandler(credentialSource, inner); var http = new HttpClient(authHandler) { @@ -90,14 +104,25 @@ public static RegistryClient Create(string registryHost, IRegistryCredentialSour /// check and the connect use the same resolved address, this is robust against DNS rebinding — unlike a /// separate pre-resolution that the socket could override with a second lookup. /// - private static async ValueTask GuardedConnectAsync(SocketsHttpConnectionContext context, CancellationToken cancellationToken) + private static async ValueTask GuardedConnectAsync( + SocketsHttpConnectionContext context, + string? trustedHost, + IWebProxy? proxy, + Func>? resolveHost, + CancellationToken cancellationToken) { string host = context.DnsEndPoint.Host; int port = context.DnsEndPoint.Port; + // When a proxy is configured the handler connects to the proxy, not to the request's host. The proxy is + // the caller's own configuration and is often on a loopback or private address, and the target's address + // is resolved by the proxy, out of this callback's sight. So the address check applies only to direct + // connections, and not to the trusted ECR registry. + bool checkAddress = IsAddressCheckRequired(context.InitialRequestMessage, host, trustedHost, proxy); + IPAddress[] addresses = IPAddress.TryParse(host, out IPAddress? literal) ? [literal] - : await System.Net.Dns.GetHostAddressesAsync(host, cancellationToken).ConfigureAwait(false); + : await (resolveHost ?? System.Net.Dns.GetHostAddressesAsync)(host, cancellationToken).ConfigureAwait(false); // Try each resolved address in order, skipping blocked ones, connecting to the first that is allowed AND // reachable. A blocked address is never dialed. @@ -105,7 +130,7 @@ private static async ValueTask GuardedConnectAsync(SocketsHttpConnection Exception? lastError = null; foreach (IPAddress address in addresses) { - if (RealmValidator.IsBlockedAddress(address)) + if (checkAddress && RealmValidator.IsBlockedAddress(address)) { lastError = new RegistryException( $"Refusing to connect to '{host}': it resolves to a blocked (loopback/private/link-local) address '{address}'."); @@ -137,6 +162,86 @@ private static async ValueTask GuardedConnectAsync(SocketsHttpConnection return new System.Net.Sockets.NetworkStream(socket, ownsSocket: true); } + /// + /// Whether a connection to must pass the blocked-address check. It is skipped + /// for the trusted ECR registry (, or when the client's + /// registry is not ECR) and for a proxy: the target of the handler's CONNECT tunnel request, or the proxy that + /// selects for . It is required for every other host. + /// + internal static bool IsAddressCheckRequired(HttpRequestMessage request, string connectHost, string? trustedHost, IWebProxy? proxy) + { + string host = NormalizeHost(connectHost); + if (trustedHost is not null && string.Equals(host, trustedHost, StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + Uri? requestUri = request.RequestUri; + + // An https request through a proxy opens its connection with the handler's own CONNECT request, which is + // addressed to the proxy. Only the handler sends CONNECT, and only to a proxy it was configured with. + if (request.Method == HttpMethod.Connect && requestUri is not null + && string.Equals(host, NormalizeHost(requestUri.IdnHost), StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + if (requestUri is not null && proxy is not null && !proxy.IsBypassed(requestUri) + && proxy.GetProxy(requestUri) is { } proxyUri + // Some IWebProxy implementations return the destination itself to mean "connect directly". + && !string.Equals(NormalizeHost(proxyUri.IdnHost), NormalizeHost(requestUri.IdnHost), StringComparison.OrdinalIgnoreCase) + && string.Equals(host, NormalizeHost(proxyUri.IdnHost), StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + return true; + } + + /// Strips IPv6 brackets and a trailing dot so host spellings compare equal. + private static string NormalizeHost(string host) + => TrimTrailingDot(host.Length > 1 && host[0] == '[' && host[^1] == ']' ? host[1..^1] : host); + + /// + /// The host name (no port, no trailing dot) of the registry this client was created for, when it is an Amazon + /// ECR registry; otherwise , so no host is exempt from the address check. + /// + internal static string? TrustedHostOf(string registryHost) + { + // Match the ECR pattern against the host alone, so an explicit port (for example ':443') doesn't hide it. + if (!Uri.TryCreate($"https://{registryHost}", UriKind.Absolute, out Uri? uri)) + { + return null; + } + string host = NormalizeHost(uri.IdnHost); + return ImageReference.IsEcrHost(host) ? host : null; + } + + private static string TrimTrailingDot(string host) + => host.Length > 1 && host[^1] == '.' ? host[..^1] : host; + + /// + /// Sends a request, reporting a failure to send it or to receive the response headers (DNS, connection, TLS, + /// proxy, or the connect guard) as a with the + /// as its inner exception. Not covered: an while a blob body is being read, and the + /// raised when or the caller's + /// token expires. + /// + private async Task SendAsync(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationToken cancellationToken) + { + try + { + return await _http.SendAsync(request, completionOption, cancellationToken).ConfigureAwait(false); + } + catch (HttpRequestException ex) + { + string message = ex.InnerException is RegistryException inner + ? inner.Message + : $"{request.Method} {request.RequestUri?.GetLeftPart(UriPartial.Path)} failed: {ex.Message}"; + throw new RegistryException(message, ex); + } + } + private Uri Base => new($"https://{_registryHost}/v2/"); // ---- Manifests ---------------------------------------------------------------------------- @@ -153,7 +258,7 @@ public async Task GetManifestAsync(string repository, string r using var request = new HttpRequestMessage(HttpMethod.Get, new Uri(Base, $"{repository}/manifests/{reference}")); AddManifestAcceptHeaders(request); - using HttpResponseMessage response = await _http.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false); + using HttpResponseMessage response = await SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false); await EnsureSuccessAsync(response, $"GET manifest {repository}:{reference}", cancellationToken).ConfigureAwait(false); // Read the exact bytes on the wire. Digests must be computed over these, not over a string that has been @@ -183,7 +288,7 @@ public async Task ManifestExistsAsync(string repository, string reference, using var request = new HttpRequestMessage(HttpMethod.Head, new Uri(Base, $"{repository}/manifests/{reference}")); AddManifestAcceptHeaders(request); - using HttpResponseMessage response = await _http.SendAsync(request, cancellationToken).ConfigureAwait(false); + using HttpResponseMessage response = await SendAsync(request, HttpCompletionOption.ResponseContentRead, cancellationToken).ConfigureAwait(false); return response.StatusCode == HttpStatusCode.OK; } @@ -199,7 +304,7 @@ public async Task PutManifestAsync(string repository, string reference, string m }; request.Content.Headers.ContentType = new MediaTypeHeaderValue(mediaType); - using HttpResponseMessage response = await _http.SendAsync(request, cancellationToken).ConfigureAwait(false); + using HttpResponseMessage response = await SendAsync(request, HttpCompletionOption.ResponseContentRead, cancellationToken).ConfigureAwait(false); await EnsureSuccessAsync(response, $"PUT manifest {repository}:{reference}", cancellationToken).ConfigureAwait(false); } @@ -212,7 +317,7 @@ public async Task BlobExistsAsync(string repository, string digest, Cancel ValidateDigest(digest); using var request = new HttpRequestMessage(HttpMethod.Head, new Uri(Base, $"{repository}/blobs/{digest}")); - using HttpResponseMessage response = await _http.SendAsync(request, cancellationToken).ConfigureAwait(false); + using HttpResponseMessage response = await SendAsync(request, HttpCompletionOption.ResponseContentRead, cancellationToken).ConfigureAwait(false); return response.StatusCode == HttpStatusCode.OK; } @@ -242,7 +347,7 @@ public async Task GetBlobAsync(string repository, string digest, Cancell // via a 401 challenge. Presigned storage URLs carry their own authorization. request.Options.Set(AuthHandshakeHandler.SkipAuthentication, true); } - HttpResponseMessage response = await _http.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false); + HttpResponseMessage response = await SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false); if (IsRedirect(response.StatusCode)) { @@ -369,7 +474,7 @@ public async Task TryMountBlobAsync(string repository, string digest, stri var uri = new Uri(Base, $"{repository}/blobs/uploads/?mount={Uri.EscapeDataString(digest)}&from={Uri.EscapeDataString(fromRepository)}"); using var request = new HttpRequestMessage(HttpMethod.Post, uri); - using HttpResponseMessage response = await _http.SendAsync(request, cancellationToken).ConfigureAwait(false); + using HttpResponseMessage response = await SendAsync(request, HttpCompletionOption.ResponseContentRead, cancellationToken).ConfigureAwait(false); if (response.StatusCode == HttpStatusCode.Created) { @@ -457,7 +562,7 @@ public async Task UploadBlobAsync( patch.Content.Headers.ContentLength = length; } - using HttpResponseMessage patchResponse = await _http.SendAsync(patch, cancellationToken).ConfigureAwait(false); + using HttpResponseMessage patchResponse = await SendAsync(patch, HttpCompletionOption.ResponseContentRead, cancellationToken).ConfigureAwait(false); await EnsureSuccessAsync(patchResponse, $"PATCH blob {repository}", cancellationToken).ConfigureAwait(false); // The registry hands back the upload URL to use for the commit. @@ -471,14 +576,14 @@ public async Task UploadBlobAsync( Content = new ByteArrayContent(Array.Empty()), }; put.Content.Headers.ContentLength = 0; - using HttpResponseMessage putResponse = await _http.SendAsync(put, cancellationToken).ConfigureAwait(false); + using HttpResponseMessage putResponse = await SendAsync(put, HttpCompletionOption.ResponseContentRead, cancellationToken).ConfigureAwait(false); await EnsureSuccessAsync(putResponse, $"PUT blob commit {repository} {digest}", cancellationToken).ConfigureAwait(false); } private async Task StartUploadAsync(string repository, CancellationToken cancellationToken) { using var request = new HttpRequestMessage(HttpMethod.Post, new Uri(Base, $"{repository}/blobs/uploads/")); - using HttpResponseMessage response = await _http.SendAsync(request, cancellationToken).ConfigureAwait(false); + using HttpResponseMessage response = await SendAsync(request, HttpCompletionOption.ResponseContentRead, cancellationToken).ConfigureAwait(false); await EnsureSuccessAsync(response, $"POST blob upload start {repository}", cancellationToken).ConfigureAwait(false); return LocationFrom(response) diff --git a/Libraries/test/AWS.OCI.Builder.Tests/ConnectGuardTests.cs b/Libraries/test/AWS.OCI.Builder.Tests/ConnectGuardTests.cs new file mode 100644 index 000000000..ec01af45c --- /dev/null +++ b/Libraries/test/AWS.OCI.Builder.Tests/ConnectGuardTests.cs @@ -0,0 +1,155 @@ +using System.Formats.Tar; +using System.IO.Compression; +using System.Net; +using System.Net.Sockets; +using System.Text; +using AWS.OCI.Builder; +using AWS.OCI.Builder.Images; +using AWS.OCI.Builder.Registry; +using Xunit; + +namespace AWS.OCI.Builder.Tests; + +/// The socket-level address guard: which hosts are checked, proxies, ECR trust, and how failures surface. +public class ConnectGuardTests +{ + private const string EcrHost = "123456789012.dkr.ecr.us-west-2.amazonaws.com"; + private static readonly string AnyDigest = "sha256:" + new string('a', 64); + + [Fact] + public async Task Guard_allows_a_loopback_proxy() + { + using var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + int port = ((IPEndPoint)listener.LocalEndpoint).Port; + + using var client = RegistryClient.Create( + "public.ecr.aws", new AnonymousRegistryCredentialSource(), proxy: new WebProxy($"http://127.0.0.1:{port}")); + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(20)); + Task call = client.GetManifestAsync("docker/library/alpine", "3.20", cts.Token); + + using TcpClient accepted = await listener.AcceptTcpClientAsync(cts.Token); + using var reader = new StreamReader(accepted.GetStream(), Encoding.ASCII, leaveOpen: true); + string? requestLine = await reader.ReadLineAsync(cts.Token); + byte[] reply = Encoding.ASCII.GetBytes("HTTP/1.1 502 Bad Gateway\r\nContent-Length: 0\r\n\r\n"); + await accepted.GetStream().WriteAsync(reply, cts.Token); + + Assert.Equal("CONNECT public.ecr.aws:443 HTTP/1.1", requestLine); + await Assert.ThrowsAnyAsync(() => call); // the fake proxy refuses the tunnel + } + + [Fact] + public async Task Guard_refuses_a_non_ECR_registry_on_a_private_address() + { + using var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + int port = ((IPEndPoint)listener.LocalEndpoint).Port; + + using var client = RegistryClient.Create($"127.0.0.1:{port}", new AnonymousRegistryCredentialSource()); + var ex = await Assert.ThrowsAsync(() => client.GetManifestAsync("repo", "latest")); + Assert.Contains("Refusing to connect to '127.0.0.1'", ex.Message); + Assert.False(listener.Pending()); // the socket was never dialed + } + + [Fact] + public async Task Guard_connects_to_ECR_on_a_private_address_as_through_a_VPC_endpoint() + { + // Simulates ECR private DNS: the ECR host name resolves to a private (here loopback) address. + using var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + int port = ((IPEndPoint)listener.LocalEndpoint).Port; + string ecrHost = $"{EcrHost}:{port}"; + Task Resolve(string host, CancellationToken _) => Task.FromResult(new[] { IPAddress.Loopback }); + + // A realm on another host that also resolves privately must still be refused by the same client. + using var client = RegistryClient.Create(ecrHost, new AnonymousRegistryCredentialSource(), resolveHost: Resolve); + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(20)); + Task call = client.GetManifestAsync("repo", "latest", cts.Token); + using TcpClient accepted = await listener.AcceptTcpClientAsync(cts.Token); // the ECR host was dialed + accepted.Close(); + Exception ex = await Assert.ThrowsAnyAsync(() => call); // TLS then fails against the plain listener + Assert.DoesNotContain("Refusing to connect", ex.ToString()); + + using var other = RegistryClient.Create($"mirror.corp.example:{port}", new AnonymousRegistryCredentialSource(), resolveHost: Resolve); + var refused = await Assert.ThrowsAsync(() => other.GetManifestAsync("repo", "latest")); + Assert.Contains("Refusing to connect to 'mirror.corp.example'", refused.Message); + Assert.False(listener.Pending()); + } + + [Theory] + [InlineData("123456789012.dkr.ecr.us-west-2.amazonaws.com", "123456789012.dkr.ecr.us-west-2.amazonaws.com")] + [InlineData("123456789012.dkr.ecr-fips.us-gov-west-1.amazonaws.com", "123456789012.dkr.ecr-fips.us-gov-west-1.amazonaws.com")] + [InlineData("123456789012.dkr.ecr.cn-north-1.amazonaws.com.cn", "123456789012.dkr.ecr.cn-north-1.amazonaws.com.cn")] + [InlineData("127.0.0.1:5000", null)] + [InlineData("localhost:5000", null)] + [InlineData("public.ecr.aws", null)] + [InlineData("mirror.corp.example", null)] + [InlineData("123456789012.dkr.ecr.us-west-2.amazonaws.com\n", "123456789012.dkr.ecr.us-west-2.amazonaws.com")] // Uri drops the newline; the same host is dialed + [InlineData("123456789012.dkr.ecr.us-west-2.amazonaws.com:443", "123456789012.dkr.ecr.us-west-2.amazonaws.com")] + [InlineData("123456789012.dkr.ecr.us-west-2.amazonaws.com.evil.example", null)] + [InlineData("123456789012.dkr.ecr.us-west-2.amazonaws.com@evil.example", null)] + [InlineData("evil.example/123456789012.dkr.ecr.us-west-2.amazonaws.com", null)] + public void Only_ECR_registries_are_trusted(string registryHost, string? expected) + => Assert.Equal(expected, RegistryClient.TrustedHostOf(registryHost)); + + [Theory] + [InlineData("https://reg.example/v2/", "reg.example", "reg.example", null, false)] // the trusted registry + [InlineData("https://reg.example/v2/", "reg.example", null, null, true)] // no trusted registry + [InlineData("https://reg.example./v2/", "reg.example.", "reg.example", null, false)] // trailing dot + [InlineData("https://auth.example/token", "auth.example", "reg.example", null, true)] // realm on another host + [InlineData("https://[::1]/x", "::1", "reg.example", null, true)] // IPv6 literal, not trusted + [InlineData("https://[::1]:5000/v2/", "::1", "::1", null, false)] // IPv6 registry, trusted + [InlineData("https://auth.example/token", "127.0.0.1", "reg.example", "http://127.0.0.1:3128", false)] // its proxy + [InlineData("https://auth.example/token", "10.0.0.9", "reg.example", "http://127.0.0.1:3128", true)] // not its proxy + public void Guard_decides_which_connections_to_check(string requestUri, string connectHost, string? trustedHost, string? proxy, bool expected) + { + IWebProxy? webProxy = proxy is null ? null : new WebProxy(proxy); + Assert.Equal(expected, RegistryClient.IsAddressCheckRequired(new HttpRequestMessage(HttpMethod.Get, requestUri), connectHost, trustedHost, webProxy)); + } + + /// An IWebProxy that answers "connect directly" by returning the destination itself. + private sealed class DestinationProxy : IWebProxy + { + public ICredentials? Credentials { get; set; } + public Uri GetProxy(Uri destination) => destination; + public bool IsBypassed(Uri host) => false; + } + + [Fact] + public void Guard_checks_a_direct_connection_reported_by_a_destination_returning_proxy() + => Assert.True(RegistryClient.IsAddressCheckRequired(new HttpRequestMessage(HttpMethod.Get, "https://internal.corp/x"), "internal.corp", null, new DestinationProxy())); + + [Fact] + public void Guard_skips_the_handlers_CONNECT_to_its_proxy_but_not_a_CONNECT_elsewhere() + { + var connect = new HttpRequestMessage(HttpMethod.Connect, "http://127.0.0.1:3128"); + Assert.False(RegistryClient.IsAddressCheckRequired(connect, "127.0.0.1", null, null)); + Assert.True(RegistryClient.IsAddressCheckRequired(connect, "10.0.0.9", null, null)); + } + + [Fact] + public void Guard_checks_a_host_the_proxy_bypasses() + { + var proxy = new WebProxy("http://127.0.0.1:3128", BypassOnLocal: false, BypassList: new[] { @"auth\.example" }); + Assert.True(RegistryClient.IsAddressCheckRequired(new HttpRequestMessage(HttpMethod.Get, "https://auth.example/token"), "127.0.0.1", "reg.example", proxy)); + } + + [Fact] + public async Task Guard_failure_surfaces_as_RegistryException() + { + var stub = new StubHttpHandler(_ => throw new HttpRequestException("wrapped", new RegistryException("Refusing to connect to 'x'."))); + var client = new RegistryClient(EcrHost, new HttpClient(stub)); + RegistryException ex = await Assert.ThrowsAsync(() => client.GetManifestAsync("repo", "latest")); + Assert.Equal("Refusing to connect to 'x'.", ex.Message); + } + + [Fact] + public async Task Transport_failures_surface_as_RegistryException() + { + var stub = new StubHttpHandler(_ => throw new HttpRequestException("Connection refused (proxy.corp.example:8080)")); + var client = new RegistryClient(EcrHost, new HttpClient(stub)); + RegistryException ex = await Assert.ThrowsAsync(() => client.BlobExistsAsync("repo", AnyDigest)); + Assert.IsType(ex.InnerException); + Assert.Contains($"HEAD https://{EcrHost}/v2/repo/blobs/{AnyDigest} failed: Connection refused", ex.Message); + } +} diff --git a/Libraries/test/AWS.OCI.Builder.Tests/TestHttp.cs b/Libraries/test/AWS.OCI.Builder.Tests/TestHttp.cs new file mode 100644 index 000000000..96b91f72f --- /dev/null +++ b/Libraries/test/AWS.OCI.Builder.Tests/TestHttp.cs @@ -0,0 +1,32 @@ +using System.Net; +using System.Net.Http.Headers; + +namespace AWS.OCI.Builder.Tests; + +/// An that answers every request from a function and records what it was sent. +internal sealed class StubHttpHandler : HttpMessageHandler +{ + private readonly Func _respond; + + public StubHttpHandler(Func respond) => _respond = respond; + + /// Each request's method, URI and Authorization scheme, in the order they were sent. + public List<(HttpMethod Method, Uri Uri, string? Auth)> Requests { get; } = new(); + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + Requests.Add((request.Method, request.RequestUri!, request.Headers.Authorization?.Scheme)); + return Task.FromResult(_respond(request)); + } +} + +internal static class TestHttp +{ + /// A 401 with a Basic challenge, as Amazon ECR sends it. + public static HttpResponseMessage BasicChallenge(string registryHost) + { + var response = new HttpResponseMessage(HttpStatusCode.Unauthorized); + response.Headers.WwwAuthenticate.Add(new AuthenticationHeaderValue("Basic", $"realm=\"https://{registryHost}/\"")); + return response; + } +} From 04185e10c2dd0ef4197c9ed2c8b2a1d4e11f77ec Mon Sep 17 00:00:00 2001 From: Jonathan Nunn Date: Thu, 29 Oct 2026 04:16:07 +0000 Subject: [PATCH 2/6] Treat only 404 as absent and report registry denials as authentication errors BlobExistsAsync and ManifestExistsAsync returned false for any status other than 200. With an expired ECR token a push treated every base layer as missing and downloaded it before failing on the upload. - A HEAD success or redirect means present, 404 means absent, and anything else throws. - 401, 403 and ECR's 400 DENIED after the handshake become RegistryAuthenticationException, which now carries StatusCode. ECR answers a HEAD with a bare 400, so a GET of /v2/ is used to read the DENIED error. - The message for a client that sends no credentials (base-image pulls) points at a missing repository or tag rather than credentials. --- .../Registry/RegistryClient.cs | 103 ++++++++++++-- .../Registry/RegistryExceptions.cs | 7 + .../RegistryErrorMappingTests.cs | 126 ++++++++++++++++++ 3 files changed, 223 insertions(+), 13 deletions(-) create mode 100644 Libraries/test/AWS.OCI.Builder.Tests/RegistryErrorMappingTests.cs diff --git a/Libraries/src/AWS.OCI.Builder/Registry/RegistryClient.cs b/Libraries/src/AWS.OCI.Builder/Registry/RegistryClient.cs index 0c21fbffc..0c3a422d4 100644 --- a/Libraries/src/AWS.OCI.Builder/Registry/RegistryClient.cs +++ b/Libraries/src/AWS.OCI.Builder/Registry/RegistryClient.cs @@ -25,6 +25,9 @@ internal sealed class RegistryClient : IDisposable private readonly string _registryHost; private readonly bool _ownsHttpClient; + // Whether this client never sends credentials (base-image pulls), which changes what a 401/403 most likely means. + private readonly bool _anonymous; + /// Creates a client for using the supplied authenticating client. /// The registry host, for example registry-1.docker.io or an ECR endpoint. /// @@ -38,11 +41,18 @@ public RegistryClient(string registryHost, HttpClient httpClient) _ownsHttpClient = false; } - private RegistryClient(string registryHost, HttpClient httpClient, bool ownsHttpClient) + /// Test-only: a client over a supplied transport that reports failures as an anonymous client does. + internal RegistryClient(string registryHost, HttpClient httpClient, bool anonymous) + : this(registryHost, httpClient, ownsHttpClient: false, anonymous) + { + } + + private RegistryClient(string registryHost, HttpClient httpClient, bool ownsHttpClient, bool anonymous) { _registryHost = registryHost; _http = httpClient; _ownsHttpClient = ownsHttpClient; + _anonymous = anonymous; } /// The registry host this client targets. @@ -95,7 +105,7 @@ public static RegistryClient Create( // cancel large uploads on slow links. The caller's cancellation token is the intended deadline. Timeout = timeout ?? Timeout.InfiniteTimeSpan, }; - return new RegistryClient(registryHost, http, ownsHttpClient: true); + return new RegistryClient(registryHost, http, ownsHttpClient: true, anonymous: credentialSource is AnonymousRegistryCredentialSource); } /// @@ -289,7 +299,7 @@ public async Task ManifestExistsAsync(string repository, string reference, using var request = new HttpRequestMessage(HttpMethod.Head, new Uri(Base, $"{repository}/manifests/{reference}")); AddManifestAcceptHeaders(request); using HttpResponseMessage response = await SendAsync(request, HttpCompletionOption.ResponseContentRead, cancellationToken).ConfigureAwait(false); - return response.StatusCode == HttpStatusCode.OK; + return await ExistsFromAsync(response, $"HEAD manifest {repository}:{reference}", cancellationToken).ConfigureAwait(false); } /// Uploads a manifest under the given tag or digest reference. @@ -318,7 +328,44 @@ public async Task BlobExistsAsync(string repository, string digest, Cancel using var request = new HttpRequestMessage(HttpMethod.Head, new Uri(Base, $"{repository}/blobs/{digest}")); using HttpResponseMessage response = await SendAsync(request, HttpCompletionOption.ResponseContentRead, cancellationToken).ConfigureAwait(false); - return response.StatusCode == HttpStatusCode.OK; + return await ExistsFromAsync(response, $"HEAD blob {repository} {digest}", cancellationToken).ConfigureAwait(false); + } + + /// + /// Interprets a HEAD response: success or a redirect means present (a registry may redirect a blob HEAD to + /// its storage, as the distribution spec allows) and 404 means absent. Anything else (denied, throttled, + /// server error) is an error, not "absent"; treating it as absent would, for example, make a push with an + /// expired token download every base layer before failing. + /// + private async Task ExistsFromAsync(HttpResponseMessage response, string operation, CancellationToken cancellationToken) + { + if (response.IsSuccessStatusCode || IsRedirect(response.StatusCode)) + { + return true; + } + if (response.StatusCode == HttpStatusCode.NotFound) + { + return false; + } + + // A HEAD response has no body, and ECR reports a rejected token on it as a bare 400; the DENIED code is + // only in the body of a GET. Ask the API root, which needs the same credentials, so the error can be + // reported for what it is. + // Only a denial from the probe is reported; any other probe failure says nothing about the HEAD, which is + // then reported as it was. + if (response.StatusCode == HttpStatusCode.BadRequest) + { + using var probe = new HttpRequestMessage(HttpMethod.Get, Base); + using HttpResponseMessage probeResponse = await SendAsync(probe, HttpCompletionOption.ResponseContentRead, cancellationToken).ConfigureAwait(false); + string probeBody = await ReadErrorBodyAsync(probeResponse, cancellationToken).ConfigureAwait(false); + if (IsDenial(probeResponse.StatusCode, probeBody)) + { + throw CreateFailure(probeResponse, probeBody, operation); + } + } + + await EnsureSuccessAsync(response, operation, cancellationToken).ConfigureAwait(false); + return false; } /// Downloads a blob by digest as a stream. Disposing the returned stream also disposes the underlying HTTP response. @@ -624,29 +671,59 @@ private static Uri AppendQuery(Uri uri, string key, string value) return builder.Uri; } - private static async Task EnsureSuccessAsync(HttpResponseMessage response, string operation, CancellationToken cancellationToken) + private async Task EnsureSuccessAsync(HttpResponseMessage response, string operation, CancellationToken cancellationToken) { if (response.IsSuccessStatusCode) { return; } - string body = string.Empty; + string body = await ReadErrorBodyAsync(response, cancellationToken).ConfigureAwait(false); + throw CreateFailure(response, body, operation); + } + + /// Reads the start of an error body (it is only context for the message), or "" if it can't be read. + private static async Task ReadErrorBodyAsync(HttpResponseMessage response, CancellationToken cancellationToken) + { try { - // Read only the start of the body: it is just context for the exception message, and an unbounded - // read would let a hostile registry exhaust memory with a huge error response. + // Bounded: an unbounded read would let a hostile registry exhaust memory with a huge error response. (byte[] prefix, _) = await BoundedRead.ReadAsync(response.Content, BoundedRead.MaxErrorBodyBytes, cancellationToken).ConfigureAwait(false); - body = System.Text.Encoding.UTF8.GetString(prefix); + return System.Text.Encoding.UTF8.GetString(prefix); } catch { - // Ignore body read failures; the status code is the primary signal. + return string.Empty; // the status code is the primary signal } + } - throw new RegistryRequestException( - (int)response.StatusCode, - $"{operation} failed with {(int)response.StatusCode} {response.ReasonPhrase}. {Truncate(body, 512)}"); + /// + /// Whether a failure is a denial: 401/403 (the handshake already ran and the registry still refused), or ECR's + /// 400 with error code DENIED, which it uses for an invalid or expired token and for missing permissions. + /// + private static bool IsDenial(HttpStatusCode status, string body) + => status is HttpStatusCode.Unauthorized or HttpStatusCode.Forbidden + || (status == HttpStatusCode.BadRequest && body.Contains("\"DENIED\"", StringComparison.Ordinal)); + + private Exception CreateFailure(HttpResponseMessage response, string body, string operation) + { + string status = $"{(int)response.StatusCode} {response.ReasonPhrase}"; + if (!IsDenial(response.StatusCode, body)) + { + return new RegistryRequestException((int)response.StatusCode, $"{operation} failed with {status}. {Truncate(body, 512)}"); + } + + // Without credentials, a refusal usually means the repository or tag doesn't exist (Docker Hub, for + // example, answers a misspelled repository with 401) or the image is private. + string advice = _anonymous + ? "No credentials are sent for base images, so the repository may not exist, the tag may be wrong, or the " + + "image may be private. " + : "Check that the credentials are valid, have not expired, and are allowed to perform this operation on " + + "the repository; for Amazon ECR, an expired token needs a new one from GetAuthorizationToken. "; + return new RegistryAuthenticationException(_registryHost, $"{operation} was denied ({status}). {advice}{Truncate(body, 512)}") + { + StatusCode = (int)response.StatusCode, + }; } private static void ValidateRepository(string repository, string paramName = "repository") diff --git a/Libraries/src/AWS.OCI.Builder/Registry/RegistryExceptions.cs b/Libraries/src/AWS.OCI.Builder/Registry/RegistryExceptions.cs index cddea40dd..274559feb 100644 --- a/Libraries/src/AWS.OCI.Builder/Registry/RegistryExceptions.cs +++ b/Libraries/src/AWS.OCI.Builder/Registry/RegistryExceptions.cs @@ -31,6 +31,13 @@ public sealed class RegistryAuthenticationException : RegistryException /// The registry that denied access. public string RegistryName { get; } + /// + /// The HTTP status the registry returned when it denied a request (401, 403, or 400 for Amazon ECR's + /// DENIED), or when no request was refused, for example when the registry asked + /// for credentials that were not available. + /// + public int? StatusCode { get; init; } + /// Creates the exception. public RegistryAuthenticationException(string registryName, string message) : base($"Unable to authenticate to registry '{registryName}': {message}") diff --git a/Libraries/test/AWS.OCI.Builder.Tests/RegistryErrorMappingTests.cs b/Libraries/test/AWS.OCI.Builder.Tests/RegistryErrorMappingTests.cs new file mode 100644 index 000000000..726bdd4a5 --- /dev/null +++ b/Libraries/test/AWS.OCI.Builder.Tests/RegistryErrorMappingTests.cs @@ -0,0 +1,126 @@ +using System.Formats.Tar; +using System.IO.Compression; +using System.Net; +using System.Net.Sockets; +using System.Text; +using AWS.OCI.Builder; +using AWS.OCI.Builder.Images; +using AWS.OCI.Builder.Registry; +using Xunit; + +namespace AWS.OCI.Builder.Tests; + +/// How registry responses map to existence results and exceptions. +public class RegistryErrorMappingTests +{ + private const string EcrHost = "123456789012.dkr.ecr.us-west-2.amazonaws.com"; + private static readonly string AnyDigest = "sha256:" + new string('a', 64); + + [Theory] + [InlineData(HttpStatusCode.OK, "true")] + [InlineData(HttpStatusCode.NotFound, "false")] + [InlineData(HttpStatusCode.TemporaryRedirect, "true")] // a registry may redirect a blob HEAD to its storage + [InlineData(HttpStatusCode.Unauthorized, nameof(RegistryAuthenticationException))] + [InlineData(HttpStatusCode.Forbidden, nameof(RegistryAuthenticationException))] + [InlineData(HttpStatusCode.TooManyRequests, nameof(RegistryRequestException))] + [InlineData(HttpStatusCode.InternalServerError, nameof(RegistryRequestException))] + public async Task BlobExists_maps_status(HttpStatusCode status, string expected) + { + var client = new RegistryClient(EcrHost, new HttpClient(new StubHttpHandler(_ => new HttpResponseMessage(status)))); + string actual; + try { actual = (await client.BlobExistsAsync("repo", AnyDigest)) ? "true" : "false"; } + catch (RegistryAuthenticationException ex) { actual = ex.GetType().Name; Assert.Equal((int)status, ex.StatusCode); } + catch (RegistryException ex) { actual = ex.GetType().Name; } + Assert.Equal(expected, actual); + } + + [Fact] + public async Task Ecr_DENIED_is_an_authentication_error() + { + var stub = new StubHttpHandler(_ => new HttpResponseMessage(HttpStatusCode.BadRequest) + { + Content = new StringContent("""{"errors":[{"code":"DENIED","message":"Your authorization token has expired. Reauthenticate and try again."}]}"""), + }); + var client = new RegistryClient(EcrHost, new HttpClient(stub)); + var ex = await Assert.ThrowsAsync( + () => client.PutManifestAsync("repo", "latest", "{}", MediaTypes.OciManifestV1)); + Assert.Contains("GetAuthorizationToken", ex.Message); + Assert.Contains("expired", ex.Message); + Assert.Equal(400, ex.StatusCode); + } + + [Fact] + public async Task Ecr_bare_400_on_HEAD_is_reported_as_an_authentication_error() + { + // What ECR does with an invalid token: HEAD -> 400 with no body; GET -> 400 with a DENIED body. + var stub = new StubHttpHandler(r => r.Method == HttpMethod.Head + ? new HttpResponseMessage(HttpStatusCode.BadRequest) + : new HttpResponseMessage(HttpStatusCode.BadRequest) + { + Content = new StringContent("""{"errors":[{"code":"DENIED","message":"Your Authorization Token is invalid."}]}"""), + }); + var client = new RegistryClient(EcrHost, new HttpClient(stub)); + var ex = await Assert.ThrowsAsync(() => client.BlobExistsAsync("repo", AnyDigest)); + Assert.Contains("Authorization Token is invalid", ex.Message); + Assert.Equal(new[] { HttpMethod.Head, HttpMethod.Get }, stub.Requests.Select(r => r.Method)); + } + + [Fact] + public async Task Bare_400_on_HEAD_is_reported_as_is_when_the_probe_fails_for_another_reason() + { + var stub = new StubHttpHandler(r => new HttpResponseMessage(r.Method == HttpMethod.Head ? HttpStatusCode.BadRequest : HttpStatusCode.ServiceUnavailable)); + var client = new RegistryClient(EcrHost, new HttpClient(stub)); + var ex = await Assert.ThrowsAsync(() => client.BlobExistsAsync("repo", AnyDigest)); + Assert.Equal(400, ex.StatusCode); // the HEAD's status, not the probe's 503 + } + + [Fact] + public async Task Anonymous_refusal_suggests_a_missing_repository_not_credentials() + { + var stub = new StubHttpHandler(_ => new HttpResponseMessage(HttpStatusCode.Unauthorized) + { + Content = new StringContent("""{"errors":[{"code":"UNAUTHORIZED","message":"authentication required"}]}"""), + }); + var anonymous = new RegistryClient("registry-1.docker.io", new HttpClient(stub), anonymous: true); + var ex = await Assert.ThrowsAsync(() => anonymous.GetManifestAsync("library/ubuntuu", "22.04")); + Assert.Contains("may not exist", ex.Message); + Assert.DoesNotContain("GetAuthorizationToken", ex.Message); + + var authenticated = new RegistryClient(EcrHost, new HttpClient(stub), anonymous: false); + ex = await Assert.ThrowsAsync(() => authenticated.GetManifestAsync("repo", "latest")); + Assert.Contains("GetAuthorizationToken", ex.Message); + } + + [Fact] + public async Task Bare_400_on_HEAD_stays_a_request_error_when_the_registry_is_reachable() + { + var stub = new StubHttpHandler(r => new HttpResponseMessage(r.Method == HttpMethod.Head ? HttpStatusCode.BadRequest : HttpStatusCode.OK)); + var client = new RegistryClient(EcrHost, new HttpClient(stub)); + var ex = await Assert.ThrowsAsync(() => client.BlobExistsAsync("repo", AnyDigest)); + Assert.Equal(400, ex.StatusCode); + } + + [Fact] + public async Task Push_with_a_bad_token_fails_before_downloading_base_layers() + { + byte[] baseLayer = new byte[1024 * 1024]; + string baseDigest = RegistryClient.ComputeDigest(baseLayer); + var source = new StubHttpHandler(_ => new HttpResponseMessage(HttpStatusCode.OK) { Content = new ByteArrayContent(baseLayer) }); + var target = new StubHttpHandler(_ => TestHttp.BasicChallenge(EcrHost)); // ECR keeps refusing the token + + const string configJson = """{"architecture":"amd64","os":"linux","rootfs":{"type":"layers","diff_ids":["sha256:aaaa"]},"config":{}}"""; + string manifestJson = $$"""{"schemaVersion":2,"mediaType":"application/vnd.oci.image.manifest.v1+json","config":{"mediaType":"application/vnd.oci.image.config.v1+json","size":10,"digest":"{{AnyDigest}}"},"layers":[{"mediaType":"application/vnd.oci.image.layer.v1.tar+gzip","size":{{baseLayer.Length}},"digest":"{{baseDigest}}"}]}"""; + ImageBuilder imageBuilder = ImageBuilder.FromBaseImage(configJson, manifestJson, MediaTypes.OciManifestV1); + var layers = new[] { new Descriptor { MediaType = MediaTypes.OciLayerGzipV1, Size = baseLayer.Length, Digest = baseDigest } }; + + using OciImageBuilder builder = OciImageBuilder.CreateForTest( + ImageReference.Parse("public.ecr.aws/docker/library/alpine:3.20"), + new RegistryClient("public.ecr.aws", new HttpClient(source)), imageBuilder, layers); + builder.UseTargetClientFactoryForTest((host, creds) => new RegistryClient(host, new HttpClient(new AuthHandshakeHandler(creds, target)))); + + string token = Convert.ToBase64String(Encoding.UTF8.GetBytes("AWS:expired")); + var ex = await Assert.ThrowsAsync(() => builder.PushToEcrAsync($"{EcrHost}/repo:tag", token)); + Assert.Equal(EcrHost, ex.RegistryName); + Assert.Empty(source.Requests); + } +} From 821b9a9a1496b7434c32284b59fb5fb52c239be0 Mon Sep 17 00:00:00 2001 From: Jonathan Nunn Date: Thu, 29 Oct 2026 04:16:11 +0000 Subject: [PATCH 3/6] Add FileManifest.AddDirectory for folder entries A layer has no entries for the parent folders of mapped files, and some image consumers reject an image whose new folders have files but no entries of their own. AddDirectory adds a 0755 folder entry without a local source. Building a layer now also rejects a file or folder beneath a path that is mapped as a file. --- Libraries/src/AWS.OCI.Builder/FileManifest.cs | 20 ++++++ Libraries/src/AWS.OCI.Builder/Images/Layer.cs | 34 +++++++++- .../AddDirectoryTests.cs | 65 +++++++++++++++++++ 3 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 Libraries/test/AWS.OCI.Builder.Tests/AddDirectoryTests.cs diff --git a/Libraries/src/AWS.OCI.Builder/FileManifest.cs b/Libraries/src/AWS.OCI.Builder/FileManifest.cs index 62639c5cf..b2ba869ec 100644 --- a/Libraries/src/AWS.OCI.Builder/FileManifest.cs +++ b/Libraries/src/AWS.OCI.Builder/FileManifest.cs @@ -27,10 +27,14 @@ namespace AWS.OCI.Builder; public sealed class FileManifest : IEnumerable { private readonly List _entries = new(); + private readonly List _directories = new(); /// The entries that make up this manifest. public IReadOnlyList Entries => _entries; + /// The container folders added with . + public IReadOnlyList Directories => _directories; + /// /// Adds a local file or directory to be placed at inside the container, with /// its Unix mode derived from the source (on Linux/macOS) or defaulted (on Windows). @@ -61,6 +65,22 @@ public void Add(string localPath, string containerPath, UnixFileMode mode) public void AddExecutable(string localPath, string containerPath) => _entries.Add(new FileManifestEntry(localPath, containerPath, FileManifestEntry.ExecutableMode)); + /// + /// Adds a folder entry (mode 0755) at , with no local source. + /// + /// + /// 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 /tmp's 1777) 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 0755. + /// + /// The absolute folder path inside the container (for example /opt/app/lib). + public void AddDirectory(string containerPath) + { + ArgumentException.ThrowIfNullOrWhiteSpace(containerPath); + _directories.Add(containerPath); + } + /// public IEnumerator GetEnumerator() => _entries.GetEnumerator(); diff --git a/Libraries/src/AWS.OCI.Builder/Images/Layer.cs b/Libraries/src/AWS.OCI.Builder/Images/Layer.cs index 6cc677464..d1a8d4b94 100644 --- a/Libraries/src/AWS.OCI.Builder/Images/Layer.cs +++ b/Libraries/src/AWS.OCI.Builder/Images/Layer.cs @@ -117,9 +117,9 @@ public static async Task CreateAsync( { throw new ArgumentException("The destination stream must be writable.", nameof(destination)); } - if (manifest.Entries.Count == 0) + if (manifest.Entries.Count == 0 && manifest.Directories.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 +132,16 @@ public static async Task CreateAsync( { CollectEntry(entry, files, directories); } + foreach (string directory in manifest.Directories) + { + 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 +167,26 @@ public static async Task CreateAsync( return new Layer(digest, diffId, compressedSink.BytesWritten, mediaType); } + /// + /// 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. + /// + private static void RejectEntriesUnderFiles(SortedDictionary files, SortedSet 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(); diff --git a/Libraries/test/AWS.OCI.Builder.Tests/AddDirectoryTests.cs b/Libraries/test/AWS.OCI.Builder.Tests/AddDirectoryTests.cs new file mode 100644 index 000000000..15222fa9d --- /dev/null +++ b/Libraries/test/AWS.OCI.Builder.Tests/AddDirectoryTests.cs @@ -0,0 +1,65 @@ +using System.Formats.Tar; +using System.IO.Compression; +using System.Net; +using System.Net.Sockets; +using System.Text; +using AWS.OCI.Builder; +using AWS.OCI.Builder.Images; +using AWS.OCI.Builder.Registry; +using Xunit; + +namespace AWS.OCI.Builder.Tests; + +/// Folder entries added with FileManifest.AddDirectory, and paths beneath a mapped file. +public class AddDirectoryTests +{ + [Fact] + public async Task AddDirectory_emits_folder_entries() + { + string dir = Directory.CreateTempSubdirectory("fix-dirs-").FullName; + try + { + string file = Path.Combine(dir, "bootstrap"); + await File.WriteAllTextAsync(file, "x"); + var manifest = new FileManifest(); + manifest.AddExecutable(file, "/opt/app/newdir/sub/bootstrap"); + manifest.AddDirectory("/opt/app/newdir"); + manifest.AddDirectory("/opt/app/newdir/sub/"); + + var entries = await ReadEntriesAsync(manifest); + Assert.Equal( + new[] { "Directory opt/app/newdir/ 755", "Directory opt/app/newdir/sub/ 755", "RegularFile opt/app/newdir/sub/bootstrap 755" }, + entries); + + var clash = new FileManifest(); + clash.Add(file, "/opt/app/x"); + clash.AddDirectory("/opt/app/x"); + await Assert.ThrowsAsync(() => ReadEntriesAsync(clash)); + + var folderUnderFile = new FileManifest(); + folderUnderFile.Add(file, "/a/b"); + folderUnderFile.AddDirectory("/a/b/c"); + await Assert.ThrowsAsync(() => ReadEntriesAsync(folderUnderFile)); + + var fileUnderFile = new FileManifest { { file, "/a/b" }, { file, "/a/b/c" } }; + await Assert.ThrowsAsync(() => ReadEntriesAsync(fileUnderFile)); + + var onlyFolders = new FileManifest(); + onlyFolders.AddDirectory("/opt/empty"); + Assert.Equal(new[] { "Directory opt/empty/ 755" }, await ReadEntriesAsync(onlyFolders)); + } + finally { Directory.Delete(dir, true); } + } + + private static async Task> ReadEntriesAsync(FileManifest manifest) + { + using var blob = new MemoryStream(); + await Layer.CreateAsync(manifest, blob); + blob.Position = 0; + using var gzip = new GZipStream(blob, CompressionMode.Decompress); + using var tar = new TarReader(gzip); + var result = new List(); + while (tar.GetNextEntry() is { } e) result.Add($"{e.EntryType} {e.Name} {Convert.ToString((int)e.Mode, 8)}"); + return result; + } +} From f88fe01babb1e6209d743fa008e84cd5972131e5 Mon Sep 17 00:00:00 2001 From: Jonathan Nunn Date: Thu, 29 Oct 2026 04:16:15 +0000 Subject: [PATCH 4/6] Write layers as ustar so they are reproducible across processes .NET's PAX writer names each extended header PaxHeaders., so the same files built in two processes gave different layer and image digests, and every build uploaded a new layer. Entries are now ustar, which has no atime or ctime fields, with fixed times and owners. An entry ustar cannot hold (a name too long for its fields, or a file of 8 GiB or more) falls back to PAX; whether it fits is decided by asking .NET's ustar writer. Directory names longer than 100 bytes drop their trailing slash so the name field is never left empty. GNU entries were not used: .NET 8 writes the current time into their atime and ctime, and the epoch written there makes Python's tarfile misread names. --- Libraries/src/AWS.OCI.Builder/Images/Layer.cs | 118 ++++++++++--- .../ReproducibleLayerTests.cs | 155 ++++++++++++++++++ 2 files changed, 248 insertions(+), 25 deletions(-) create mode 100644 Libraries/test/AWS.OCI.Builder.Tests/ReproducibleLayerTests.cs diff --git a/Libraries/src/AWS.OCI.Builder/Images/Layer.cs b/Libraries/src/AWS.OCI.Builder/Images/Layer.cs index d1a8d4b94..3c4facf68 100644 --- a/Libraries/src/AWS.OCI.Builder/Images/Layer.cs +++ b/Libraries/src/AWS.OCI.Builder/Images/Layer.cs @@ -28,8 +28,10 @@ namespace AWS.OCI.Builder.Images; /// /// /// -/// 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. /// /// internal sealed class Layer @@ -326,14 +328,23 @@ private static UnixFileMode ResolveFileMode(string localPath, UnixFileMode? expl } /// - /// 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 /// (the hashing/compression pipeline). Source files are streamed, not buffered. /// /// - /// PAX is the format used by container tooling (including Microsoft.NET.Build.Containers) and is what - /// registries and runtimes expect for image layers. Every entry gets a fixed extended-attribute set (see - /// ) and a fixed rather than - /// the current time. + /// + /// 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 . + /// The alternatives can't be made reproducible on every runtime the library supports. .NET's PAX writer names + /// each extended header PaxHeaders.<process id>. .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 tarfile) misread every name. + /// + /// + /// 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. + /// /// private static async Task WriteTarAsync( SortedDictionary files, @@ -342,48 +353,102 @@ 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); } } } - /// A resolved file to place in the layer: its source path and the Unix mode to stamp on it. - private readonly record struct FileSource(string LocalPath, UnixFileMode Mode); + /// Creates a ustar entry, or a PAX entry when ustar can't hold the name or size (see ). + internal static TarEntry CreateEntry(TarEntryType type, string name, UnixFileMode mode, long size) + { + // A directory name longer than ustar's 100-byte name field loses its trailing '/'. Otherwise .NET splits it + // at that slash and leaves the name field empty, which tar itself never writes; the type flag already marks + // the entry as a directory. + if (type == TarEntryType.Directory && System.Text.Encoding.UTF8.GetByteCount(name) > 100) + { + name = name.TrimEnd('/'); + } + + TarEntry entry = FitsUstar(type, name, size) + ? new UstarTarEntry(type, name) + : new PaxTarEntry(type, name, ReproducibleExtendedAttributes()); + entry.Mode = mode; + entry.ModificationTime = ReproducibleTimestamp; + return entry; + } /// - /// The PAX extended attributes applied to every entry: fixed mtime, atime and ctime. + /// Whether a ustar header can hold and . This asks .NET's own ustar + /// writer, against a throwaway stream, rather than re-implementing its rule for splitting a long name into the + /// prefix and name fields (it does not accept every split the format allows). + /// + internal static bool FitsUstar(TarEntryType type, string name, long size) + { + using var writer = new TarWriter(Stream.Null, TarEntryFormat.Ustar, leaveOpen: true); + try + { + var probe = new UstarTarEntry(type, name); + if (size > 0) + { + probe.DataStream = new HeaderProbeStream(size); + } + writer.WriteEntry(probe); + return true; + } + catch (HeaderProbeStream.HeaderWrittenException) + { + return true; // the header was accepted; the writer had started on the data + } + catch (ArgumentException) + { + return false; + } + } + + /// + /// A stream that reports a length but throws as soon as it is read, so can test a header + /// without supplying the data. + /// + private sealed class HeaderProbeStream(long length) : Stream + { + internal sealed class HeaderWrittenException : Exception; + + public override bool CanRead => true; + public override bool CanSeek => true; + public override bool CanWrite => false; + public override long Length => length; + public override long Position { get => 0; set { } } + public override int Read(byte[] buffer, int offset, int count) => throw new HeaderWrittenException(); + public override int Read(Span buffer) => throw new HeaderWrittenException(); + public override long Seek(long offset, SeekOrigin origin) => 0; + public override void Flush() { } + public override void SetLength(long value) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + } + + /// + /// The PAX extended attributes for an entry that falls back to PAX: fixed mtime, atime and + /// ctime (the .NET 8 writer otherwise fills atime and ctime with the current time). /// - /// - /// On .NET 8 the PAX writer otherwise fills atime and ctime with the current wall-clock time; - /// pinning all three to a fixed epoch keeps them out of the archive. - /// private static Dictionary ReproducibleExtendedAttributes() { string epoch = ReproducibleTimestamp.ToUnixTimeSeconds().ToString("D", System.Globalization.CultureInfo.InvariantCulture); @@ -395,6 +460,9 @@ private static Dictionary ReproducibleExtendedAttributes() }; } + /// A resolved file to place in the layer: its source path and the Unix mode to stamp on it. + private readonly record struct FileSource(string LocalPath, UnixFileMode Mode); + /// /// Normalizes a caller-supplied container path to an absolute, forward-slash path with no trailing slash /// and no redundant segments. diff --git a/Libraries/test/AWS.OCI.Builder.Tests/ReproducibleLayerTests.cs b/Libraries/test/AWS.OCI.Builder.Tests/ReproducibleLayerTests.cs new file mode 100644 index 000000000..9a0407eac --- /dev/null +++ b/Libraries/test/AWS.OCI.Builder.Tests/ReproducibleLayerTests.cs @@ -0,0 +1,155 @@ +using System.Formats.Tar; +using System.IO.Compression; +using System.Net; +using System.Net.Sockets; +using System.Text; +using AWS.OCI.Builder; +using AWS.OCI.Builder.Images; +using AWS.OCI.Builder.Registry; +using Xunit; + +namespace AWS.OCI.Builder.Tests; + +/// Layers are byte-for-byte reproducible: ustar entries with fixed metadata, PAX only where ustar cannot hold an entry. +public class ReproducibleLayerTests +{ + [Fact] + public async Task Layer_is_ustar_with_fixed_times_and_no_process_specific_headers() + { + string dir = Directory.CreateTempSubdirectory("fix-repro-").FullName; + try + { + // A long path that still fits ustar's prefix/name split, a non-ASCII name, and one path component + // too long for ustar (forces the PAX fallback for that entry only). + string longDir = Path.Combine(dir, new string('d', 120)); + Directory.CreateDirectory(longDir); + string unicodeName = "ü-ünïcode-" + new string('f', 60) + ".txt"; + await File.WriteAllTextAsync(Path.Combine(longDir, unicodeName), "x"); + string tooLong = new string('t', 150) + ".txt"; + await File.WriteAllTextAsync(Path.Combine(dir, tooLong), "y"); + var manifest = new FileManifest { { dir, "/opt/app" } }; + manifest.AddDirectory("/opt"); + + using var blob = new MemoryStream(); + await Layer.CreateAsync(manifest, blob); + byte[] tarBytes = Gunzip(blob.ToArray()); + + using var tar = new TarReader(new MemoryStream(tarBytes)); + var entries = new List<(TarEntryFormat Format, string Name)>(); + while (tar.GetNextEntry() is { } e) + { + Assert.Equal(DateTimeOffset.UnixEpoch, e.ModificationTime); + Assert.Equal(0, e.Uid); + Assert.Equal(0, e.Gid); + entries.Add((e.Format, e.Name)); + } + Assert.Equal( + new[] + { + (TarEntryFormat.Ustar, "opt/"), + (TarEntryFormat.Ustar, "opt/app/"), + (TarEntryFormat.Pax, $"opt/app/{new string('d', 120)}"), // last component > 100 bytes + (TarEntryFormat.Ustar, $"opt/app/{new string('d', 120)}/{unicodeName}"), + (TarEntryFormat.Pax, $"opt/app/{tooLong}"), + }, + entries); + + // Only the fallback entry has a PAX header; a layer without such names has none at all. + using var plain = new MemoryStream(); + var small = new FileManifest { { Path.Combine(longDir, unicodeName), "/a/b.txt" } }; + await Layer.CreateAsync(small, plain); + Assert.DoesNotContain("PaxHeaders", Encoding.ASCII.GetString(Gunzip(plain.ToArray()))); + } + finally { Directory.Delete(dir, true); } + } + + [Theory] + [InlineData("short/name.txt", 0L, true)] + [InlineData("short/name.txt", (1L << 33) - 1, true)] // largest ustar size + [InlineData("short/name.txt", 1L << 33, false)] // 8 GiB + public void FitsUstar_checks_size(string name, long size, bool expected) + => Assert.Equal(expected, Layer.FitsUstar(TarEntryType.RegularFile, name, size)); + + [Fact] + public async Task Layers_with_many_long_names_build_and_read_back() + { + // Random deep and long paths: each entry must come out as ustar or PAX and read back with its exact name. + var random = new Random(2602); + string[] parts = { "a", "bb", "ü", "dir", new string('x', 40), new string('y', 99), new string('v', 150) }; + string root = Directory.CreateTempSubdirectory("fix-names-").FullName; + try + { + var manifest = new FileManifest(); + var expected = new HashSet(StringComparer.Ordinal); + for (int n = 0; n < 300; n++) + { + string rel = string.Join('/', Enumerable.Range(0, random.Next(1, 5)).Select(_ => parts[random.Next(parts.Length)])) + $"-{n}.txt"; + string local = Path.Combine(root, $"{n}.txt"); + await File.WriteAllTextAsync(local, n.ToString()); + manifest.Add(local, "/" + rel); + expected.Add(rel); + if (n % 3 == 0 && rel.Contains('/')) + { + string folder = rel[..rel.LastIndexOf('/')]; + manifest.AddDirectory("/" + folder); + expected.Add(Encoding.UTF8.GetByteCount(folder) + 1 > 100 ? folder : folder + "/"); + } + } + + using var blob = new MemoryStream(); + await Layer.CreateAsync(manifest, blob); + byte[] tarBytes = Gunzip(blob.ToArray()); + AssertNoEmptyNameFields(tarBytes); + using var tar = new TarReader(new MemoryStream(tarBytes)); + var formats = new Dictionary(); + var names = new HashSet(StringComparer.Ordinal); + while (tar.GetNextEntry() is { } e) + { + names.Add(e.Name); + formats[e.Format] = formats.GetValueOrDefault(e.Format) + 1; + } + Assert.Equal(expected, names); + Assert.True(formats.GetValueOrDefault(TarEntryFormat.Ustar) > 0 && formats.GetValueOrDefault(TarEntryFormat.Pax) > 0, + string.Join(", ", formats.Select(f => $"{f.Key}={f.Value}"))); + } + finally { Directory.Delete(root, true); } + } + + /// Fails if any ustar header leaves the name field empty and puts the whole path in the prefix. + private static void AssertNoEmptyNameFields(byte[] tarBytes) + { + for (int offset = 0; offset + 512 <= tarBytes.Length; offset += 512) + { + ReadOnlySpan header = tarBytes.AsSpan(offset, 512); + if (!header.Slice(257, 5).SequenceEqual("ustar"u8)) + { + continue; + } + bool emptyName = header[0] == 0; + bool hasPrefix = header[345] != 0; + Assert.False(emptyName && hasPrefix, $"header at {offset} has an empty name field and a prefix"); + } + } + + private static byte[] Gunzip(byte[] gz) + { + using var input = new GZipStream(new MemoryStream(gz), CompressionMode.Decompress); + using var output = new MemoryStream(); + input.CopyTo(output); + return output.ToArray(); + } + + [Fact] + public void Layer_falls_back_to_PAX_only_for_entries_ustar_cannot_hold() + { + UnixFileMode mode = FileManifestEntry.ExecutableMode; + Assert.Equal(TarEntryFormat.Ustar, Layer.CreateEntry(TarEntryType.RegularFile, "a/big.bin", mode, (1L << 33) - 1).Format); + Assert.Equal(TarEntryFormat.Pax, Layer.CreateEntry(TarEntryType.RegularFile, "a/big.bin", mode, 1L << 33).Format); + Assert.Equal(TarEntryFormat.Pax, Layer.CreateEntry(TarEntryType.RegularFile, new string('n', 101), mode, 1).Format); + + // A long directory name loses its trailing slash instead of leaving the ustar name field empty. + TarEntry dir = Layer.CreateEntry(TarEntryType.Directory, "opt/" + new string('d', 99) + "/", mode, 0); + Assert.Equal(TarEntryFormat.Ustar, dir.Format); + Assert.Equal("opt/" + new string('d', 99), dir.Name); + } +} From b78ea5a7daef39ea47b4f96cdd5e5c34b5773ef4 Mon Sep 17 00:00:00 2001 From: Jonathan Nunn Date: Thu, 29 Oct 2026 04:16:20 +0000 Subject: [PATCH 5/6] Add LayerPushProgressQueue for polling push progress System.Progress raises callbacks on thread-pool threads when there is no synchronization context, which breaks PowerShell scriptblocks (no runspace) and any sink that is not thread-safe. LayerPushProgressQueue queues events for the caller to drain with TryDequeue or DequeueAll. The EcrPushOptions docs and README now describe Progress accurately and show the queue. --- .../src/AWS.OCI.Builder/EcrPushOptions.cs | 9 +-- .../AWS.OCI.Builder/LayerPushProgressQueue.cs | 48 ++++++++++++++ Libraries/src/AWS.OCI.Builder/README.md | 19 +++++- .../LayerPushProgressQueueTests.cs | 66 +++++++++++++++++++ 4 files changed, 136 insertions(+), 6 deletions(-) create mode 100644 Libraries/src/AWS.OCI.Builder/LayerPushProgressQueue.cs create mode 100644 Libraries/test/AWS.OCI.Builder.Tests/LayerPushProgressQueueTests.cs diff --git a/Libraries/src/AWS.OCI.Builder/EcrPushOptions.cs b/Libraries/src/AWS.OCI.Builder/EcrPushOptions.cs index ec904cd04..afd7795a4 100644 --- a/Libraries/src/AWS.OCI.Builder/EcrPushOptions.cs +++ b/Libraries/src/AWS.OCI.Builder/EcrPushOptions.cs @@ -59,10 +59,11 @@ public sealed class EcrPushOptions /// so the contract holds even if layers are later uploaded concurrently — see . /// /// - /// Use for the common case: it captures the current - /// 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 : it raises callbacks on the + /// it captured, which keeps UI updates on the UI thread. + /// Where there is no synchronization context (console apps, services, PowerShell), + /// raises callbacks on thread-pool threads, possibly several at once. To read events on your own thread instead, + /// use . Any custom implementation must be thread-safe. /// /// public IProgress? Progress { get; init; } diff --git a/Libraries/src/AWS.OCI.Builder/LayerPushProgressQueue.cs b/Libraries/src/AWS.OCI.Builder/LayerPushProgressQueue.cs new file mode 100644 index 000000000..7ec07d3ae --- /dev/null +++ b/Libraries/src/AWS.OCI.Builder/LayerPushProgressQueue.cs @@ -0,0 +1,48 @@ +using System.Collections.Concurrent; + +namespace AWS.OCI.Builder; + +/// +/// A thread-safe sink that queues push progress events for the caller to read, instead +/// of invoking a callback. +/// +/// +/// +/// Use this when callbacks on arbitrary threads are a problem, for example from PowerShell, where a scriptblock +/// invoked on a thread-pool thread has no runspace. Pass it as , then call +/// or from your own thread while the push runs (and once more +/// after it completes) to drain the events in the order they were reported. +/// +/// +/// The queue is unbounded. A push reports one event each time the HTTP stack reads from the layer, plus a few per +/// layer. On .NET 10 those reads are 128 KiB, so a 1 GiB layer queues roughly 8,000 events (about half a +/// megabyte) if they are never read. +/// +/// +public sealed class LayerPushProgressQueue : IProgress +{ + private readonly ConcurrentQueue _events = new(); + + /// The number of events waiting to be read. + public int Count => _events.Count; + + /// Queues an event. Called by the push; safe to call from any thread. + public void Report(LayerPushProgress value) => _events.Enqueue(value); + + /// Removes and returns the oldest queued event, if any. + /// The event, when one was queued. + /// when an event was returned. + public bool TryDequeue(out LayerPushProgress value) => _events.TryDequeue(out value); + + /// Removes and returns every queued event, oldest first (an empty array when none are queued). + /// Convenient from PowerShell: foreach ($p in $queue.DequeueAll()) { ... }. + public LayerPushProgress[] DequeueAll() + { + var drained = new List(_events.Count); + while (_events.TryDequeue(out LayerPushProgress value)) + { + drained.Add(value); + } + return drained.ToArray(); + } +} diff --git a/Libraries/src/AWS.OCI.Builder/README.md b/Libraries/src/AWS.OCI.Builder/README.md index 5387828a2..c392ebbf7 100644 --- a/Libraries/src/AWS.OCI.Builder/README.md +++ b/Libraries/src/AWS.OCI.Builder/README.md @@ -115,8 +115,23 @@ Every event carries the layer's stable `LayerIndex` (1-based), `TotalLayers`, `D (`BaseLayer` vs `BuiltLayer`), all fixed before any upload begins. Key your state by `LayerIndex` or `Digest` rather than the order events arrive: the contract is designed so that if the library later uploads layers in parallel — interleaving their events — consumers keep working unchanged. A custom `IProgress` implementation -must be thread-safe for that reason; `System.Progress` already marshals callbacks to the captured -synchronization context. +must be thread-safe for that reason. + +`System.Progress` raises callbacks on the synchronization context it captured. Where there is none (console +apps, services, PowerShell), it raises them on thread-pool threads, possibly several at once. To read events on +your own thread instead, pass a `LayerPushProgressQueue` and drain it while the push runs: + +```csharp +var queue = new LayerPushProgressQueue(); +Task push = builder.PushToEcrAsync(target, authorizationToken, new EcrPushOptions { Progress = queue }); +while (!push.IsCompleted) +{ + while (queue.TryDequeue(out LayerPushProgress p)) { /* render p */ } + await Task.WhenAny(push, Task.Delay(200)); +} +while (queue.TryDequeue(out LayerPushProgress p)) { /* render p */ } +string digest = await push; +``` ### File permissions diff --git a/Libraries/test/AWS.OCI.Builder.Tests/LayerPushProgressQueueTests.cs b/Libraries/test/AWS.OCI.Builder.Tests/LayerPushProgressQueueTests.cs new file mode 100644 index 000000000..24d562cda --- /dev/null +++ b/Libraries/test/AWS.OCI.Builder.Tests/LayerPushProgressQueueTests.cs @@ -0,0 +1,66 @@ +using System.Formats.Tar; +using System.IO.Compression; +using System.Net; +using System.Net.Sockets; +using System.Text; +using AWS.OCI.Builder; +using AWS.OCI.Builder.Images; +using AWS.OCI.Builder.Registry; +using Xunit; + +namespace AWS.OCI.Builder.Tests; + +/// The polling progress sink and the volume of events a push reports. +public class LayerPushProgressQueueTests +{ + private const string EcrHost = "123456789012.dkr.ecr.us-west-2.amazonaws.com"; + private static readonly string AnyDigest = "sha256:" + new string('a', 64); + + [Fact] + public void ProgressQueue_takes_concurrent_reports_without_loss() + { + var queue = new LayerPushProgressQueue(); + const int threads = 8, perThread = 5000; + Parallel.For(0, threads, new ParallelOptions { MaxDegreeOfParallelism = threads }, t => + { + for (int i = 0; i < perThread; i++) + { + queue.Report(new LayerPushProgress { LayerIndex = t, BytesPushed = i, Digest = AnyDigest }); + } + }); + + Assert.Equal(threads * perThread, queue.Count); + var lastByThread = new long[threads]; + Array.Fill(lastByThread, -1); + int read = 0; + while (queue.TryDequeue(out LayerPushProgress p)) + { + Assert.True(p.BytesPushed > lastByThread[p.LayerIndex]); // each reporter's order is kept + lastByThread[p.LayerIndex] = p.BytesPushed; + read++; + } + Assert.Equal(threads * perThread, read); + } + + [Theory] + [InlineData(10 * 1024 * 1024)] + [InlineData(100 * 1024 * 1024)] + public async Task Upload_progress_volume(int size) + { + // How many Uploading events one layer produces: the queue holds one per read of the layer stream. + var stub = new StubHttpHandler(r => + { + if (r.Method == HttpMethod.Head) return new HttpResponseMessage(HttpStatusCode.NotFound); + if (r.Method == HttpMethod.Patch) r.Content!.ReadAsStream().CopyTo(Stream.Null); + var response = new HttpResponseMessage(r.Method == HttpMethod.Put ? HttpStatusCode.Created : HttpStatusCode.Accepted); + response.Headers.Location = new Uri($"https://{EcrHost}/v2/repo/blobs/uploads/u1"); + return response; + }); + var client = new RegistryClient(EcrHost, new HttpClient(stub)); + byte[] blob = new byte[size]; + int calls = 0; + await client.UploadBlobAsync("repo", RegistryClient.ComputeDigest(blob), new MemoryStream(blob), onBytesSent: _ => calls++, contentLength: size); + // One event per read of the layer stream; the read size is the runtime's (128 KiB measured on .NET 10). + Assert.InRange(size / calls, 16 * 1024, 1024 * 1024); + } +} From b3c326367bb48045f4a1c1eb7c8d60f48d7af881 Mon Sep 17 00:00:00 2001 From: Jonathan Nunn Date: Thu, 29 Oct 2026 04:16:25 +0000 Subject: [PATCH 6/6] Expose the resolved base image digests OciImageBuilder.BaseImageDigest is the digest of the platform manifest the build is based on, and BaseImageIndexDigest the manifest list it was selected from (null for a single-platform reference). An internal FromBaseImageAsync overload takes the source client factory for tests. --- .../src/AWS.OCI.Builder/OciImageBuilder.cs | 53 ++++++++++++--- .../BaseImageDigestTests.cs | 64 +++++++++++++++++++ 2 files changed, 109 insertions(+), 8 deletions(-) create mode 100644 Libraries/test/AWS.OCI.Builder.Tests/BaseImageDigestTests.cs diff --git a/Libraries/src/AWS.OCI.Builder/OciImageBuilder.cs b/Libraries/src/AWS.OCI.Builder/OciImageBuilder.cs index aaa414a52..bdd97bf34 100644 --- a/Libraries/src/AWS.OCI.Builder/OciImageBuilder.cs +++ b/Libraries/src/AWS.OCI.Builder/OciImageBuilder.cs @@ -68,12 +68,16 @@ private OciImageBuilder( ImageReference baseReference, RegistryClient sourceClient, ImageBuilder imageBuilder, - IReadOnlyList baseLayers) + IReadOnlyList baseLayers, + string baseImageDigest, + string? baseImageIndexDigest) { _baseReference = baseReference; _sourceClient = sourceClient; _imageBuilder = imageBuilder; _baseLayers = baseLayers; + BaseImageDigest = baseImageDigest; + BaseImageIndexDigest = baseImageIndexDigest; } /// @@ -85,8 +89,10 @@ internal static OciImageBuilder CreateForTest( ImageReference baseReference, RegistryClient sourceClient, ImageBuilder imageBuilder, - IReadOnlyList baseLayers) - => new(baseReference, sourceClient, imageBuilder, baseLayers); + IReadOnlyList baseLayers, + string baseImageDigest = "", + string? baseImageIndexDigest = null) + => new(baseReference, sourceClient, imageBuilder, baseLayers, baseImageDigest, baseImageIndexDigest); /// Test-only: overrides how the push-target registry client is created. internal void UseTargetClientFactoryForTest(Func factory) @@ -98,6 +104,18 @@ internal void UseTargetClientFactoryForTest(Func internal IReadOnlyList SpilledLayerPathsForTest => _pendingLayers.Select(p => p.FilePath).ToArray(); + /// + /// The digest (sha256:...) of the base image's single-platform manifest that this build is based on, + /// whether the base reference was a tag, a digest, or a multi-platform manifest list. + /// + public string BaseImageDigest { get; } + + /// + /// The digest of the manifest list (image index) the platform manifest was selected from, or + /// when the base reference resolved directly to a single-platform manifest. + /// + public string? BaseImageIndexDigest { get; } + /// The config of the image being built, exposed for customization (entrypoint, env, labels, ...). public ImageConfig Config => _imageBuilder.Config; @@ -116,17 +134,32 @@ internal void UseTargetClientFactoryForTest(FuncA cancellation token. /// The base reference or platform string is malformed. /// The base image or the requested platform could not be fetched. - public static async Task FromBaseImageAsync( + public static Task FromBaseImageAsync( string baseImage, string platform = "linux/amd64", CancellationToken cancellationToken = default) + // Public base images pull anonymously (the handshake still performs a Bearer-token exchange when the + // registry demands one, e.g. Docker Hub; the token is obtained without credentials). + => FromBaseImageAsync( + baseImage, + platform, + registry => RegistryClient.Create(registry, new AnonymousRegistryCredentialSource()), + cancellationToken); + + /// + /// with the source registry client supplied + /// by (given the registry host); tests use it to inject a stubbed transport. + /// + internal static async Task FromBaseImageAsync( + string baseImage, + string platform, + Func createSourceClient, + CancellationToken cancellationToken) { ImageReference baseReference = ImageReference.Parse(baseImage); (string os, string architecture) = ParsePlatform(platform); - // Public base images pull anonymously (the handshake still performs a Bearer-token exchange when the - // registry demands one, e.g. Docker Hub; the token is obtained without credentials). - RegistryClient sourceClient = RegistryClient.Create(baseReference.Registry, new AnonymousRegistryCredentialSource()); + RegistryClient sourceClient = createSourceClient(baseReference.Registry); try { @@ -201,7 +234,11 @@ public static async Task FromBaseImageAsync( // new layer(s). They are a snapshot of the base manifest, distinct from layers added by this build. IReadOnlyList baseLayers = manifest.Layers.ToArray(); - return new OciImageBuilder(baseReference, sourceClient, imageBuilder, baseLayers); + // Record what the build is based on. Both digests are computed over the bytes that were verified above. + string baseImageDigest = RegistryClient.ComputeDigest(platformManifest.RawBytes); + string? baseImageIndexDigest = IsIndexMediaType(top.MediaType) ? RegistryClient.ComputeDigest(top.RawBytes) : null; + + return new OciImageBuilder(baseReference, sourceClient, imageBuilder, baseLayers, baseImageDigest, baseImageIndexDigest); } catch { diff --git a/Libraries/test/AWS.OCI.Builder.Tests/BaseImageDigestTests.cs b/Libraries/test/AWS.OCI.Builder.Tests/BaseImageDigestTests.cs new file mode 100644 index 000000000..4bc1a4656 --- /dev/null +++ b/Libraries/test/AWS.OCI.Builder.Tests/BaseImageDigestTests.cs @@ -0,0 +1,64 @@ +using System.Net; +using System.Net.Http.Headers; +using System.Text; +using AWS.OCI.Builder; +using AWS.OCI.Builder.Registry; +using Xunit; + +namespace AWS.OCI.Builder.Tests; + +/// The base image digests recorded by . +public class BaseImageDigestTests +{ + private const string Registry = "registry.example"; + + private static readonly byte[] Config = Encoding.UTF8.GetBytes( + """{"architecture":"arm64","os":"linux","rootfs":{"type":"layers","diff_ids":[]},"config":{}}"""); + private static readonly string ConfigDigest = RegistryClient.ComputeDigest(Config); + + private static readonly byte[] PlatformManifest = Encoding.UTF8.GetBytes( + $$"""{"schemaVersion":2,"mediaType":"{{MediaTypes.OciManifestV1}}","config":{"mediaType":"{{MediaTypes.OciImageConfigV1}}","size":{{Config.Length}},"digest":"{{ConfigDigest}}"},"layers":[]}"""); + private static readonly string PlatformDigest = RegistryClient.ComputeDigest(PlatformManifest); + + private static readonly byte[] Index = Encoding.UTF8.GetBytes( + $$$"""{"schemaVersion":2,"mediaType":"{{{MediaTypes.OciImageIndexV1}}}","manifests":[{"mediaType":"{{{MediaTypes.OciManifestV1}}}","size":{{{PlatformManifest.Length}}},"digest":"{{{PlatformDigest}}}","platform":{"os":"linux","architecture":"arm64"}}]}"""); + + private static HttpResponseMessage Serve(HttpRequestMessage request) + { + string path = request.RequestUri!.AbsolutePath; + (byte[]? body, string? mediaType) = path switch + { + "/v2/base/manifests/1.0" => (Index, MediaTypes.OciImageIndexV1), + _ when path == $"/v2/base/manifests/{PlatformDigest}" => (PlatformManifest, MediaTypes.OciManifestV1), + _ when path == $"/v2/base/blobs/{ConfigDigest}" => (Config, "application/octet-stream"), + _ => (null, null), + }; + if (body is null) + { + return new HttpResponseMessage(HttpStatusCode.NotFound); + } + var content = new ByteArrayContent(body); + content.Headers.ContentType = new MediaTypeHeaderValue(mediaType!); + return new HttpResponseMessage(HttpStatusCode.OK) { Content = content }; + } + + private static Task PullAsync(string reference) + => OciImageBuilder.FromBaseImageAsync( + reference, "linux/arm64", host => new RegistryClient(host, new HttpClient(new StubHttpHandler(Serve))), CancellationToken.None); + + [Fact] + public async Task A_tag_that_names_an_index_records_both_digests() + { + using OciImageBuilder builder = await PullAsync($"{Registry}/base:1.0"); + Assert.Equal(PlatformDigest, builder.BaseImageDigest); + Assert.Equal(RegistryClient.ComputeDigest(Index), builder.BaseImageIndexDigest); + } + + [Fact] + public async Task A_single_platform_reference_has_no_index_digest() + { + using OciImageBuilder builder = await PullAsync($"{Registry}/base@{PlatformDigest}"); + Assert.Equal(PlatformDigest, builder.BaseImageDigest); + Assert.Null(builder.BaseImageIndexDigest); + } +}