feat: add OCI spec-compliant descriptor and digest validation#383
feat: add OCI spec-compliant descriptor and digest validation#383sajayantony wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #383 +/- ##
==========================================
+ Coverage 91.92% 92.19% +0.27%
==========================================
Files 67 68 +1
Lines 2886 2922 +36
Branches 374 380 +6
==========================================
+ Hits 2653 2694 +41
+ Misses 139 134 -5
Partials 94 94 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
63d2692 to
5f3ccd9
Compare
There was a problem hiding this comment.
Pull request overview
Adds OCI image-spec v1.1.1-aligned validation utilities for digests and descriptors, enabling callers to validate descriptor fields explicitly while keeping DTO construction permissive.
Changes:
- Strengthen
Content.Digestvalidation to strictly enforce sha256/sha512 encoded requirements while allowing unrecognized algorithms that match the general digest grammar. - Add
Descriptor.TryValidate(out string error)andDescriptor.Validate()to validatemediaType,size, anddigest. - Expand/adjust tests to cover new digest/descriptor validation behavior and introduce
InvalidDescriptorException.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/OrasProject.Oras/Content/Digest.cs | Tightens digest validation and adds algorithm-/error-constants. |
| src/OrasProject.Oras/Oci/Descriptor.cs | Adds explicit descriptor validation APIs and error constants; updates spec link. |
| src/OrasProject.Oras/Exceptions/InvalidDescriptorException.cs | Introduces a dedicated exception for descriptor validation failures. |
| tests/OrasProject.Oras.Tests/Content/ContentTest.cs | Updates digest tests for sha512 compliance and unrecognized-algorithm behavior. |
| tests/OrasProject.Oras.Tests/Serialization/ManifestSerializationTest.Negative.cs | Adds positive/negative coverage for descriptor validation (digest/mediaType/size). |
| tests/OrasProject.Oras.Tests/Exceptions/ExceptionTest.cs | Adds constructor coverage for InvalidDescriptorException. |
Add explicit TryValidate/Validate methods to Descriptor and strengthen Digest validation per OCI image-spec v1.1.1 requirements. - Add algorithm constants (AlgorithmSha256, AlgorithmSha512) - Enforce per-algorithm encoded format for registered algorithms - Allow unrecognized algorithms per OCI spec SHOULD clause - Add Descriptor.TryValidate(out string error) as primary API - Add Descriptor.Validate() throwing InvalidDescriptorException - Validate mediaType (non-empty), size (non-negative), digest format - Error messages follow .NET framework conventions with quoted values - 100% line and branch coverage on Digest.cs and Descriptor.cs Resolves: oras-project#381 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Sajay Antony <sajaya@microsoft.com>
2d2c228 to
29be7ff
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
tests/OrasProject.Oras.Tests/Serialization/ManifestSerializationTest.Negative.cs:273
- The sha512 InlineData literal here makes the line exceed the repo’s 120-column limit. Similar to the other cases in this file, consider using
const stringfields for long digests and referencing those constants from InlineData.
[Theory]
[InlineData("sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a")]
[InlineData("sha512:cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e")]
public void Descriptor_TryValidate_ValidDigest_ReturnsTrue(string validDigest)
| /// <returns>true if the descriptor is valid; otherwise, false.</returns> | ||
| public bool TryValidate(out string error) | ||
| { | ||
| if (string.IsNullOrWhiteSpace(MediaType)) |
There was a problem hiding this comment.
TryValidate's XML doc promises OCI image-spec v1.1.1 descriptor validation, but this currently only checks that mediaType is non-empty. Per the descriptor spec, mediaType MUST comply with RFC 6838, and artifactType has the same requirement when present; repo guidance also calls out optional artifactType non-empty validation. As written, descriptors such as MediaType = "not a media type" or ArtifactType = "" can pass the new validation API.
Can we either reuse/extract the existing media-type validator from Packer and apply it to MediaType plus non-null/non-empty ArtifactType, or narrow the docs to say this is only minimal structural validation rather than full OCI descriptor validation?
| internal static bool IsNullOrInvalid(Descriptor? descriptor) | ||
| { | ||
| return descriptor == null || string.IsNullOrWhiteSpace(descriptor.Digest) || string.IsNullOrWhiteSpace(descriptor.MediaType); | ||
| return descriptor == null || string.IsNullOrWhiteSpace(descriptor.Digest) || string.IsNullOrWhiteSpace(descriptor.MediaType) || descriptor.Size < 0; |
There was a problem hiding this comment.
nit: The repo guidance sets a hard 120-column limit, and the current PR head still has several added lines over that limit despite the earlier resolved thread saying this was fixed. A consolidated list from the current diff:
src/OrasProject.Oras/Content/Digest.cs:30-31— 138/139 columns for the digest error constants.src/OrasProject.Oras/Oci/Descriptor.cs:83— 156 columns for thisIsNullOrInvalidreturn expression.tests/OrasProject.Oras.Tests/Content/ContentTest.cs:40— 155-column sha512InlineData.tests/OrasProject.Oras.Tests/Exceptions/ExceptionTest.cs:73-74— 127/133 columns.tests/OrasProject.Oras.Tests/Serialization/ManifestSerializationTest.Negative.cs:157and:272— long sha512InlineDatavalues.
Summary
Add explicit
TryValidate/Validatemethods toDescriptorand strengthenDigestvalidation per OCI image-spec v1.1.1 requirements.Resolves: #381
What Changed
Product Code
src/OrasProject.Oras/Content/Digest.csAlgorithmSha256,AlgorithmSha512) — no magic stringssha256: encoded portion MUST match/[a-f0-9]{64}/(lowercase only, exactly 64 chars)sha512: encoded portion MUST match/[a-f0-9]{128}/(lowercase only, exactly 128 chars)ToLowerInvariant()instead ofToLower()for culture-safe hex encodingsrc/OrasProject.Oras/Oci/Descriptor.csDescriptor.TryValidate(out string error)— non-throwing, performance-sensitive validationDescriptor.Validate()— convenience wrapper that throwsInvalidDescriptorExceptionmediaType(non-empty),size(non-negative), anddigest(delegates toContent.Digest.TryValidate)Validate()docs recommendTryValidatefor perf-sensitive code pathsDescriptorremains a permissive DTO (no breaking change)src/OrasProject.Oras/Exceptions/InvalidDescriptorException.csDescriptor.Validate()throwsInvalidDescriptorException(notInvalidDigestException)Error Message Format
Messages follow .NET framework conventions (see
FormatException,UriFormatException):Test Changes
tests/.../Content/ContentTest.cstests/.../Serialization/ManifestSerializationTest.Negative.cstests/.../Exceptions/ExceptionTest.csInvalidDescriptorExceptionDesign Decisions
TryValidateas primary APIValidate()throwsInvalidDescriptorExceptionStartsWith+ debuggability with quoted valueCoverage
Digest.cs: 100% line, 100% branchDescriptor.cs: 100% line, 100% branchTest Results
This PR description was generated with AI assistance.