Repository navigation
Conversation
…es without a container runtime
Enable IsTrimmable, EnableTrimAnalyzer, and IsAotCompatible, and remove all reflection-based System.Text.Json usage so the library is trim- and AOT-safe: - Add source-generated OciJsonContext for Manifest, ImageIndex, and TokenResponse; route their serialization/deserialization through compile-time JsonTypeInfo. - Lift TokenResponse to an internal top-level type so the source-gen context can reference it. - Serialize the free-form ImageConfig JsonObject via a Utf8JsonWriter and type each JsonArray.Add argument as JsonNode, dropping the reflection-based DefaultJsonTypeInfoResolver. - Apply UnsafeRelaxedJsonEscaping through the source-gen resolver so manifest bytes (and thus the computed digest) stay identical to other container tooling.
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical vulnerabilities permit SSRF, ECR credential disclosure, and unintended exposure of host files and private image contents.
16 open issues
What changed in this PR
Introduces AWS.OCI.Builder, a pre-1.0 .NET library for AWS tooling to build container images from existing base images and push them to Amazon ECR without a container runtime or AWS SDK dependency.
Changes:
- Adds image assembly, file mappings, Unix permissions, and configurable image metadata.
- Implements registry authentication, digest verification, disk-backed layer uploads, and progress reporting.
- Adds unit and ECR integration tests, documentation, and build/release registration.
| File | Description |
|---|---|
| Libraries/test/AWS.OCI.Builder.Tests/UnixPermissionsTests.cs | Tests octal permission parsing. |
| Libraries/test/AWS.OCI.Builder.Tests/RealmValidatorTests.cs | Tests authentication realm validation. |
| Libraries/test/AWS.OCI.Builder.Tests/OciImageBuilderTests.cs | Tests push orchestration, spill files, and progress. |
| Libraries/test/AWS.OCI.Builder.Tests/LayerTests.cs | Tests layer structure, reproducibility, and permissions. |
| Libraries/test/AWS.OCI.Builder.Tests/ImageReferenceTests.cs | Tests image reference parsing and ECR detection. |
| Libraries/test/AWS.OCI.Builder.Tests/ImageAssemblyTests.cs | Tests configuration, manifests, and image assembly. |
| Libraries/test/AWS.OCI.Builder.Tests/FileManifestTests.cs | Tests file-mapping collection initialization. |
| Libraries/test/AWS.OCI.Builder.Tests/DigestVerificationTests.cs | Tests blob integrity and temporary-file handling. |
| Libraries/test/AWS.OCI.Builder.Tests/AWS.OCI.Builder.Tests.csproj | Defines the unit-test project. |
| Libraries/test/AWS.OCI.Builder.Tests/AuthHandshakeHandlerTests.cs | Tests authentication exchanges and realm rejection. |
| Libraries/test/AWS.OCI.Builder.IntegrationTests/EcrPushIntegrationTests.cs | Exercises real ECR pushes and uploaded permissions. |
| Libraries/test/AWS.OCI.Builder.IntegrationTests/AWS.OCI.Builder.IntegrationTests.csproj | Defines the ECR integration-test project. |
| Libraries/src/AWS.OCI.Builder/UnixPermissions.cs | Converts octal permissions to Unix mode flags. |
| Libraries/src/AWS.OCI.Builder/Registry/TokenResponse.cs | Models authentication tokens and expiration. |
| Libraries/src/AWS.OCI.Builder/Registry/RegistryExceptions.cs | Defines registry and integrity exceptions. |
| Libraries/src/AWS.OCI.Builder/Registry/RegistryClient.cs | Implements manifest and blob transfers with verification. |
| Libraries/src/AWS.OCI.Builder/Registry/RealmValidator.cs | Validates authentication endpoint URLs. |
| Libraries/src/AWS.OCI.Builder/Registry/IRegistryCredentialSource.cs | Defines anonymous and static credential sources. |
| Libraries/src/AWS.OCI.Builder/Registry/AuthHandshakeHandler.cs | Implements Basic and Bearer authentication retries. |
| Libraries/src/AWS.OCI.Builder/README.md | Documents usage, authentication, progress, and permissions. |
| Libraries/src/AWS.OCI.Builder/OciImageBuilder.cs | Coordinates base-image retrieval, layer creation, and ECR pushes. |
| Libraries/src/AWS.OCI.Builder/MediaTypes.cs | Defines OCI and Docker media types. |
| Libraries/src/AWS.OCI.Builder/LayerPushProgress.cs | Defines per-layer progress events. |
| Libraries/src/AWS.OCI.Builder/Images/OciJsonContext.cs | Provides source-generated JSON serialization metadata. |
| Libraries/src/AWS.OCI.Builder/Images/Manifest.cs | Models manifests, indexes, and platform selection. |
| Libraries/src/AWS.OCI.Builder/Images/Layer.cs | Builds reproducible compressed layers and their digests. |
| Libraries/src/AWS.OCI.Builder/Images/ImageConfig.cs | Supports image configuration customization. |
| Libraries/src/AWS.OCI.Builder/Images/ImageBuilder.cs | Assembles final configuration and manifest artifacts. |
| Libraries/src/AWS.OCI.Builder/ImageReference.cs | Parses registry, repository, tag, and digest references. |
| Libraries/src/AWS.OCI.Builder/FileManifest.cs | Defines local-to-container file mappings and permissions. |
| Libraries/src/AWS.OCI.Builder/EcrPushOptions.cs | Configures memory thresholds and progress reporting. |
| Libraries/src/AWS.OCI.Builder/Descriptor.cs | Models content-addressed image descriptors. |
| Libraries/src/AWS.OCI.Builder/AWS.OCI.Builder.csproj | Configures packaging, trimming, and Native AOT compatibility. |
| Libraries/Libraries.sln | Registers the library and test projects. |
| Libraries/AWS.OCI.Builder.slnf | Adds a focused solution filter. |
| buildtools/build.proj | Adds unit tests to the build workflow. |
| .autover/changes/1df94c4d-8841-4630-89ca-9324e3bcaf79.json | Records the initial-release changelog entry. |
| .autover/autover.json | Registers the package for automated versioning. |
Review effort: Balanced
Security and correctness fixes across the library plus regression tests. Security: - Validate the repository name against the OCI grammar so a malformed path cannot override the request authority and leak credentials. - Reject symbolic links / reparse points during layer assembly (a link inside a mapped directory could otherwise copy files from outside it into the image). - Enforce the SSRF guard at the socket layer: disable auto-redirect and add a connect callback that resolves the host, refuses blocked (loopback/private/link-local) addresses, and connects to the validated address - robust against DNS rebinding and covering realm, registry, and blob redirects. Blob-download redirects are followed explicitly and validated. - Create spilled layer/blob temp files with owner-only (0600) permissions. Correctness: - Validate the fetched base config's OS/architecture against the requested platform instead of silently pushing the wrong platform. - Strip an optional tag from a reference that also carries a digest. - Preserve empty descendant directories in recursive directory mappings, and normalize/validate expanded target paths. - Stop emitting synthesized ancestor directory entries so they no longer clobber base-image directory permissions. - Mutate the image builder only after the layer spill succeeds. - Replay a seekable 401-retry body by rewinding instead of buffering the blob. - Make the upload timeout configurable (default: no client timeout). - Use the stream's remaining length for an implicit Content-Length. - Use a synchronous progress sink in the integration test so assertions do not race asynchronous Progress<T> callbacks. Tests: 139 pass on net8.0/net10.0 (+20 new, 2 updated to the corrected directory-entry contract). AOT publish remains warning-free.
|
Thanks for the thorough review. All 16 findings are addressed in 669231d. Summary of how each was handled: High
Medium
Added regression tests for the repository grammar, tag+digest parsing, symlink rejection, empty-directory preservation, the SSRF address/host checks, and the 0600 spill permissions. Test count is 139 (up from 119) passing on net8.0 and net10.0, and the Native AOT publish is still warning-free. One note on #11 (parent-dir permissions): two existing |
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
20 open issues
GetBlobAsyncleaksHttpRequestMessage/HttpResponseMessageinstances on the non-redirect… · Newlocalhosthost detection is currently case-sensitive (StringComparison.Ordinal). Hostnames are… · New
Review effort: Lite
…e-sensitivity - GetBlobAsync now returns a ResponseOwningStream that disposes the underlying HttpResponseMessage and HttpRequestMessage when the stream is disposed, and disposes the per-hop request on redirects and on the error path. Previously the response/request were leaked on the success path. - ImageReference host detection compares the 'localhost' literal with OrdinalIgnoreCase so an uppercase host (e.g. LOCALHOST/app) is recognized as a registry rather than misparsed as a Docker Hub repository. - Fix README typos (based on, non-AWS).
|
Addressed the three new findings from the latest scan in High — Medium — Low — README typos. "change base on" → "based on"; "non AWS context" → "non-AWS context". Verification: 142 unit tests pass on net8.0 and net10.0 (+3 new), the |
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open issues
20 resolved since last review
GetBlobAsyncleaksHttpRequestMessage/HttpResponseMessageinstances on the non-redirect…localhosthost detection is currently case-sensitive (StringComparison.Ordinal). Hostnames are…
Review effort: Lite
- AuthHandshakeHandler.TryParseChallenge now scans all WWW-Authenticate challenges instead of only the first, selecting a usable scheme (preferring Bearer with a realm, falling back to Basic). A registry may return multiple challenges in any order (RFC 7235); the previous first-only logic could fail auth when a usable challenge was not listed first. Added regression tests for Bearer-not-first and Basic-fallback. - Fix Layer class XML doc that claimed synthesized parent directory entries are emitted; they are intentionally not (ancestors are created implicitly on extraction). - Fix OciImageBuilder class XML doc example to call PushToEcrAsync with an auth token instead of the non-existent PushAsync; correct a stale PushAsync reference in an inline comment.
|
Addressed the three new findings from the latest scan in Medium — Medium — Medium — Verification: 144 unit tests pass on net8.0 and net10.0 (+2 new), the |
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open issues
3 resolved since last review
Review effort: Lite
…eck, caller auth - Blob-download redirect hops to a different host/port are marked to skip authentication, and AuthHandshakeHandler honors that: no cached credentials are attached and a 401 challenge is not answered. Previously a registry could redirect a blob GET to a host it controls and harvest credentials via a challenge, since the handler authenticated every request on the pipeline. - Verified blob downloads accept the manifest-declared size and throw a new BlobSizeMismatchException when the downloaded length differs, so a wrong descriptor size fails early with a clear error instead of an opaque Content-Length transport failure during the push. Applied to the base image config and both base-layer paths. - AuthHandshakeHandler respects a caller-supplied Authorization header: the request is sent unchanged and any 401 is returned to the caller.
|
Addressed the three new findings from the latest scan in High — blob redirect could leak credentials through Medium — upload Content-Length used the manifest's declared size. Rather than switching to the file length, I made a declared/actual mismatch fail early. A wrong descriptor size would otherwise fail during the PATCH with an opaque transport error ( Medium — Verification: 153 unit tests pass on net8.0 and net10.0 (+9 new). For each fix I temporarily reverted it and confirmed its new tests fail, then restored. The |
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open issues
\":password\"(empty username) and\"username:\"(empty password), and it… · NewHttpRequestMessage.VersionPolicy(and may therefore change… · NewComputeDigest(Stream)hashes from the stream's current position to EOF, which can produce… · New
3 resolved since last review
Review effort: Lite
…gest docs - Reject an ECR authorization token that decodes to an empty username or password with a clear ArgumentException before any registry interaction, instead of letting it surface later as an opaque authentication failure. - The 401-retry clone now copies VersionPolicy and request Options, so the authenticated retry behaves exactly like the original request. - Document that ComputeDigest(Stream) hashes from the stream's current position (matching SHA256.HashData(Stream)) and pin that behavior with a test.
|
Addressed the three new findings from the latest scan in Medium — ECR token parsing accepted an empty username or password. Medium — the 401-retry clone dropped Medium — Verification: 157 unit tests pass on net8.0 and net10.0 (+4 new). I temporarily reverted the token and clone fixes and confirmed their new tests fail, then restored. The |
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open issues
WWW-Authenticateheaders:… · NewOciImageBuilder.AddLayerAsync… · NewHttpRequestMessagefor the retry but never disposes it. Since… · New
3 resolved since last review
Review effort: Lite
…, options doc - Replace the regex-based Bearer challenge parameter parser with an RFC 7235 auth-param parser: optional whitespace around '=' and ',', quoted values with backslash escapes and embedded commas, unquoted token values, and the full tchar set for names. The old regex dropped realm when whitespace preceded a comma and parsed nothing for unquoted values, failing authentication. - Dispose the 401-retry clone after sending. Its replayed body is wrapped in a non-owning stream, so disposal does not close the caller's upload stream (disposing StreamContent would otherwise close the spilled layer file). - Correct the EcrPushOptions.MaxInMemoryLayerBytes remark: built layers are written to a temp file in AddLayerAsync and uploaded from it; the setting only affects base-layer copying.
|
Addressed the three new findings from the latest scan in Bearer challenge parameter parser was too restrictive. I ran the old regex against a set of real-world formats before changing it. Whitespace after commas (the example in the comment) actually parsed fine, so ECR/Docker Hub-style challenges were unaffected — but two RFC 7235-valid forms failed authentication: whitespace before a comma (
The 401-retry Verification: 169 unit tests pass on net8.0 and net10.0 (+12 new). I reverted each fix individually and confirmed the new tests fail — including the naive |
The test identified its spill files by diffing the shared temp directory before and after AddLayerAsync. A parallel test spilling a file into that window was counted as this test's, and the post-Dispose assertion then failed because only its owner deletes it. Expose the builder's own spill paths via an internal test accessor and assert on exactly those.
Layer.CreateAsync now writes the layer in a single streaming pass (tar -> gzip -> caller-supplied destination stream), computing the diff_id, digest and size on the fly. Built layers were previously materialized in memory several times over and could not exceed ~2 GiB (MemoryStream/byte[] limit). Layer is now metadata only; GzippedTarBytes is removed (breaking, pre-1.0). AddLayerAsync builds directly into the owner-only spill file, removing a second full write. Output is byte-identical to the previous implementation for the same tar. Verified live: a 2.4 GiB layer built with ~111 MiB peak working set and pushed to ECR in a single PATCH. Docs: drop the promise that identical input yields identical digests (PAX extended header names embed the process id, so digests vary across runs). Tests: add streaming pipeline tests; remove the temp directory leak in ImageAssemblyTests; fix the CA1416 and xUnit1031 warnings.
Match the AWS.OCI.Builder project and assembly name. Updates RootNamespace, all namespace/using declarations in the library and tests, and the README sample.
- Bound reads from registries: manifests, configs and token responses are capped at 4 MiB, error and 401 bodies at 4 KiB, and verified blob downloads stop as soon as more bytes arrive than the manifest declared. - Validate digests, tags and repository names (OciNames) before they are used to build request URLs, including digests parsed from base images. - Redact secrets from ToString on BasicRegistryCredentials, TokenResponse and the cached auth entry. - Bind StaticRegistryCredentialSource to a single registry host. The auth handler looks up credentials by host and port, and Bearer token endpoints by their own host, so credentials are never sent to any other host, including across pushes.
Reduce the public surface to the developer-facing types (OciImageBuilder, ImageConfig, FileManifest/FileManifestEntry/UnixPermissions, EcrPushOptions, push progress types) and the registry exceptions. Registry client, auth, credential source, image model and reference types are now internal, as are ImageConfig.Parse/AddLayer/ToJsonBytes/ToJsonString.
jnunn-aws
left a comment
jnunn-aws
left a comment
There was a problem hiding this comment.
I built and pushed images with this from PowerShell on pwsh 7.4 and 7.6, and it works well. Most of what follows came out of that integration. The larger fixes are in #2603 as one commit per finding; the small ones are suggested changes below. In priority order:
- The connect guard blocks proxies and ECR VPC endpoints (
RegistryClient.cs:108). - An expired token is treated as "blob missing" (
RegistryClient.cs:216). - Layers aren't reproducible across processes (
Layer.cs:315). - There's no way to emit folder entries (
Layer.cs:168). Progress<T>breaks from PowerShell (EcrPushOptions.cs:62).- The resolved base image digest isn't exposed (
OciImageBuilder.cs:198). - Smaller items, each with a suggested change:
Cmd/Entrypoint, config getters, http upload locations, assembly version,docker.io, and the reparse-point check.
| Exception? lastError = null; | ||
| foreach (IPAddress address in addresses) | ||
| { | ||
| if (RealmValidator.IsBlockedAddress(address)) |
There was a problem hiding this comment.
The connect guard also checks the connection to a proxy. With HTTPS_PROXY=http://127.0.0.1:3128 set, every request fails with Refusing to connect to '127.0.0.1', and a corporate proxy on a 10.x or 192.168.x address fails the same way. It also refuses ECR through an interface VPC endpoint, because private DNS resolves the ECR host name to private addresses.
Please skip the check for an ECR registry host and for the configured proxy, and keep it for every other host, including caller-supplied base registries (so localhost:5000/... is still refused). For https, the proxy connection is the one the handler opens for its own CONNECT request. Matching on IWebProxy.GetProxy alone doesn't work, because that request is addressed to the proxy, and some proxies return the destination itself to mean "connect directly".
Please also report transport failures as RegistryException with the HttpRequestException inside. A refused connection or a failed proxy tunnel currently surfaces as a bare HttpRequestException, which callers catching RegistryException miss. With both changes, a proxy from HttpClient.DefaultProxy or HTTPS_PROXY works without a new option.
|
|
||
| using var request = new HttpRequestMessage(HttpMethod.Head, new Uri(Base, $"{repository}/blobs/{digest}")); | ||
| using HttpResponseMessage response = await _http.SendAsync(request, cancellationToken).ConfigureAwait(false); | ||
| return response.StatusCode == HttpStatusCode.OK; |
There was a problem hiding this comment.
BlobExistsAsync (and ManifestExistsAsync at line 187) returns false for any status other than 200, including 401, 403, 429 and 500. With an expired ECR token, the push treats every base layer as missing, downloads it from the source registry, and only then fails on POST blob upload start with 400 DENIED.
Please return false only for 404 and throw for anything else. A 401, 403 or DENIED that persists after the handshake should be a RegistryAuthenticationException that covers both an expired token and missing permission, since ECR uses DENIED for both. ECR answers a HEAD with an invalid token with a bare 400 and no body; the DENIED code only comes back on a GET, so a GET /v2/ is needed to report it.
| 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)) |
There was a problem hiding this comment.
.NET's TarWriter names each PAX extended header PaxHeaders.<process id>, and every entry here gets one for its fixed times. The same files built in two processes give different layer digests, so every build uploads a new layer and gets a new image digest even when nothing changed. Pushing the same image from two pwsh processes gave sha256:f89be64f… and then sha256:1acb3f72….
Please write ustar entries, which have no atime or ctime fields, and fall back to PAX only for an entry ustar can't hold: a name too long for its fields, or a file of 8 GiB or more. GNU entries don't work here. .NET 8 writes the current time into their atime and ctime unless they're set and rejects anything before 1970, and the epoch written there makes Python's tarfile read every name as 00000000000/....
A tested implementation is in #2603, commit f88fe01. It asks .NET's ustar writer whether an entry fits, because .NET doesn't accept every name split the format allows. It also drops the trailing / from directory names over 100 bytes, which otherwise leaves the ustar name field empty.
| /// Only EXPLICITLY mapped directories are recorded in <paramref name="directories"/>; the implicit ancestors | ||
| /// of a file are not. Tar extraction creates missing parent directories implicitly, so emitting ancestor | ||
| /// entries is unnecessary — and harmful, because a synthesized 0755 ancestor entry would overlay (and reset) | ||
| /// the base image's own permissions on that directory (for example <c>/tmp</c>'s 1777, or a private 0700 | ||
| /// parent). Leaving ancestors out preserves the base image's directory metadata. | ||
| /// </remarks> |
There was a problem hiding this comment.
Parent folders of mapped files get no tar entry. Mapping one file to /opt/app/newdir/sub/run produces a single RegularFile entry and no Directory entries. Docker accepts this, but at least one image consumer we tested rejects an image whose new folders have files but no entries of their own, and the same image with folder entries works. Today the only way to get an entry is to map an empty local folder.
Please add a way to emit folder entries, such as FileManifest.AddDirectory(containerPath) with no local source. The library doesn't read base layers when it builds a layer, so the caller has to say which folders are new, and the default can stay as it is so /tmp keeps 1777.
A tested implementation is in #2603, commit 821b9a9. It also rejects a file or folder beneath a path mapped as a file, which builds a layer that can't be extracted (that already happens with two files at the base commit).
| /// 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. |
There was a problem hiding this comment.
System.Progress<T> only marshals callbacks when there's a SynchronizationContext. In PowerShell or a console app it runs them on thread-pool threads, possibly several at once. A PowerShell scriptblock run there has no runspace and crashes pwsh, and a PowerShell class used as the sink dropped events or faulted when callbacks overlapped.
Please ship a thread-safe sink that queues events for the caller to poll, with a method that drains them into an array (TryDequeue(out T) alone is awkward from PowerShell, because [ref] to an uninitialized struct fails). Please also correct this doc and the README, which say Progress<T> marshals to the captured context.
| // A registry may return a relative Location; resolve it against the registry base. | ||
| return location.IsAbsoluteUri ? location : new Uri(new Uri($"https://{_registryHost}"), location); |
There was a problem hiding this comment.
LocationFrom accepts an http:// upload location. The auth cache is keyed by Authority, which leaves out the scheme, so the PATCH and the commit PUT would then send the cached Basic ECR credentials in cleartext. It needs the registry itself to return such a location, but GetBlobAsync already rejects non-https redirects, so please apply the same rule here. The next comment covers the handler side.
| // A registry may return a relative Location; resolve it against the registry base. | |
| return location.IsAbsoluteUri ? location : new Uri(new Uri($"https://{_registryHost}"), location); | |
| // A registry may return a relative Location; resolve it against the registry base. | |
| Uri resolved = location.IsAbsoluteUri ? location : new Uri(new Uri($"https://{_registryHost}"), location); | |
| // The upload is sent with the registry's credentials, so never follow it onto plain http (blob download | |
| // redirects have the same rule). | |
| if (!resolved.Scheme.Equals(Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase)) | |
| { | |
| throw new RegistryRequestException( | |
| (int)response.StatusCode, | |
| $"the registry returned a non-https upload location '{OciNames.ForDisplay(resolved.ToString())}'."); | |
| } | |
| return resolved; |
| // such as object storage) are sent as-is: no cached header is attached and a 401 challenge is NOT | ||
| // answered. Otherwise a malicious registry could redirect a blob GET to a host it controls, reply 401 | ||
| // with a challenge, and harvest the credentials this handler would send in response. | ||
| if (request.Options.TryGetValue(SkipAuthentication, out bool skip) && skip) |
There was a problem hiding this comment.
Related to the upload location comment: please never attach cached credentials, or answer a challenge, over plain http.
| if (request.Options.TryGetValue(SkipAuthentication, out bool skip) && skip) | |
| // Credentials are only ever sent over https. The cache below is keyed by Authority, which ignores the | |
| // scheme, so without this an http URL on the same host would pick up the cached header. | |
| if ((request.Options.TryGetValue(SkipAuthentication, out bool skip) && skip) | |
| || !request.RequestUri.Scheme.Equals(Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase)) |
| <IsAotCompatible>true</IsAotCompatible> | ||
|
|
||
| <!-- NuGet package metadata --> | ||
| <Version>0.5.0</Version> |
There was a problem hiding this comment.
buildtools/common.props turns off GenerateAssemblyVersionAttribute and GenerateAssemblyFileVersionAttribute, so the assembly version is 0.0.0.0 and there's no file version; only the informational version (0.5.0+<sha>) is set. Please turn them back on here, as Amazon.Lambda.RuntimeSupport and SnapshotRestore.Registry do.
| <Version>0.5.0</Version> | |
| <Version>0.5.0</Version> | |
| <!-- buildtools/common.props turns these off; generate them from Version, as Amazon.Lambda.RuntimeSupport does. --> | |
| <GenerateAssemblyVersionAttribute>true</GenerateAssemblyVersionAttribute> | |
| <GenerateAssemblyFileVersionAttribute>true</GenerateAssemblyFileVersionAttribute> |
| registry = firstSegment; | ||
| remainder = remainder[(firstSlash + 1)..]; |
There was a problem hiding this comment.
Docker treats docker.io and index.docker.io as Docker Hub and talks to registry-1.docker.io, but this keeps docker.io as the host, so docker.io/library/ubuntu:22.04 goes to a different host than Docker uses. Please normalize those two names.
| registry = firstSegment; | |
| remainder = remainder[(firstSlash + 1)..]; | |
| registry = firstSegment; | |
| remainder = remainder[(firstSlash + 1)..]; | |
| // Docker's names for Docker Hub; its registry API is served from registry-1.docker.io. | |
| if (registry.Equals("docker.io", StringComparison.OrdinalIgnoreCase) | |
| || registry.Equals("index.docker.io", StringComparison.OrdinalIgnoreCase)) | |
| { | |
| registry = DockerHubHost; | |
| } |
| : File.Exists(path) ? new FileInfo(path) | ||
| : null; | ||
|
|
||
| if (info is not null && info.Attributes.HasFlag(FileAttributes.ReparsePoint)) |
There was a problem hiding this comment.
FileAttributes.ReparsePoint is broader than links. On Windows it also marks files that aren't links, such as OneDrive cloud files, deduplicated files and app execution aliases, so mapping a file from a OneDrive-backed folder could be rejected as a symbolic link. Please check LinkTarget, which is set only for symbolic links and junctions.
| if (info is not null && info.Attributes.HasFlag(FileAttributes.ReparsePoint)) | |
| // LinkTarget is set only for symbolic links and junctions. FileAttributes.ReparsePoint is broader: on Windows | |
| // it also marks files that aren't links, such as cloud files (OneDrive), deduplicated files and app | |
| // execution aliases. | |
| if (info?.LinkTarget is not null) |
Draft: Will still reviewing the code myself
Description of changes:
Related to some other tooling we are building we want the ability to create a container without requiring a container runtime installation. Following the inspiration done with the .NET CLI to build containers this library will build containers that just need to add new layers of files and metadata to a base image.
The library is primarily intended for use with other AWS libraries so we plan to keep the version less then 1.0.0 to avoid implying any contract that we won't make API breaking changes. We will make changes based on primarily on the need of our AWS tooling.
The library by design avoids dependencies like the AWS SDK for .NET. This is because we anticipate some of our usage to be for PowerShell where it is easy to get a conflict with already loaded .NET assemblies.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.