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.
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.