fix: validate tag format in BuildReferrersTag#384
Draft
sajayantony wants to merge 2 commits into
Draft
Conversation
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>
sajayantony
requested review from
Wwwsylvia,
akashsinghal and
shizhMSFT
as code owners
BuildReferrersTag converts a digest to a tag by replacing ':' with '-'. When unrecognized algorithms containing separators like '+' are used, the resulting tag may not be a valid OCI reference. Add tag validation after the conversion to catch this early with a clear error. - Make Reference.TryValidateTag internal for reuse - Validate resulting tag in BuildReferrersTag, throw InvalidReferenceException - Add test for invalid tag scenario Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Sajay Antony <sajaya@microsoft.com>
sajayantony
force-pushed
the
agent/referrers-tag-validation
branch
from
e232d81 to
a43e434
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #384 +/- ##
==========================================
+ Coverage 91.92% 92.21% +0.28%
==========================================
Files 67 68 +1
Lines 2886 2929 +43
Branches 374 382 +8
==========================================
+ Hits 2653 2701 +48
+ Misses 139 134 -5
Partials 94 94 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens referrers-tag generation by validating that the tag produced from a digest (via : → -) is a valid OCI tag, preventing invalid tags when digests use OCI-grammar-valid but tag-invalid characters (e.g., + in algorithm).
Changes:
- Validate the generated referrers tag in
Referrers.BuildReferrersTagand throwInvalidReferenceExceptionwhen invalid. - Reuse existing tag validation by widening
Reference.TryValidateTagvisibility tointernal. - Add/adjust tests to cover invalid-tag scenarios (note: the diff also contains broader digest/descriptor validation changes due to the stated dependency on #383).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/OrasProject.Oras/Registry/Remote/Referrers.cs | Validates digest-derived referrers tag and throws on invalid tag format. |
| src/OrasProject.Oras/Registry/Reference.cs | Makes TryValidateTag internal for reuse. |
| tests/OrasProject.Oras.Tests/Registry/Remote/ReferrersTest.cs | Adds coverage for invalid-tag generation from OCI-grammar-valid digests. |
| src/OrasProject.Oras/Content/Digest.cs | Updates digest validation behavior (appears tied to dependency #383). |
| src/OrasProject.Oras/Oci/Descriptor.cs | Adds descriptor validation helpers/constants (appears tied to dependency #383). |
| src/OrasProject.Oras/Exceptions/InvalidDescriptorException.cs | Introduces a descriptor-validation exception type (appears tied to dependency #383). |
| tests/OrasProject.Oras.Tests/Content/ContentTest.cs | Updates digest-validation tests for unrecognized algorithms / sha512 vector. |
| tests/OrasProject.Oras.Tests/Serialization/ManifestSerializationTest.Negative.cs | Adds descriptor validation tests (currently placed in serialization test file). |
| tests/OrasProject.Oras.Tests/Exceptions/ExceptionTest.cs | Adds constructor coverage for InvalidDescriptorException. |
Comments suppressed due to low confidence (1)
tests/OrasProject.Oras.Tests/Serialization/ManifestSerializationTest.Negative.cs:273
- This InlineData sha512 digest string is very long and hurts readability. Prefer extracting it to a named constant or using a test data member so each line stays within the usual line-length limit.
[Theory]
[InlineData("sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a")]
[InlineData("sha512:cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e")]
public void Descriptor_TryValidate_ValidDigest_ReturnsTrue(string validDigest)
Comment on lines
+41
to
+45
| /// <summary> | ||
| /// Builds a referrers tag from a descriptor's digest by replacing ':' with '-'. | ||
| /// Validates both the digest format and the resulting tag format. | ||
| /// </summary> | ||
| /// <param name="descriptor">The descriptor whose digest is used to build the tag.</param> |
Comment on lines
+135
to
+148
| [Theory] | ||
| [InlineData("")] | ||
| [InlineData("invalid")] | ||
| [InlineData(":44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a")] | ||
| public void Descriptor_Validate_InvalidDigest_ThrowsInvalidDescriptorException(string invalidDigest) | ||
| { | ||
| var descriptor = new Descriptor | ||
| { | ||
| MediaType = "application/vnd.oci.image.layer.v1.tar+gzip", | ||
| Digest = invalidDigest | ||
| }; | ||
|
|
||
| Assert.Throws<InvalidDescriptorException>(() => descriptor.Validate()); | ||
| } |
Comment on lines
+152
to
+157
| [InlineData("sha256:tooshort")] | ||
| [InlineData("sha256:44136FA355B3678A1146AD16F7E8649E94FB4FC21FE77E8310C060F61CAAFF8A")] // uppercase not allowed | ||
| [InlineData("sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8")] // 63 chars | ||
| [InlineData("sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8aa")] // 65 chars | ||
| [InlineData("sha512:tooshort")] | ||
| [InlineData("sha512:cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3")] // 127 chars |
Comment on lines
38
to
41
| [Theory] | ||
| [InlineData("sha256:6c3c624b58dbbcd3c0dd82b4c53f04194d1247c6eebdaab7c610cf7d66709b3b")] | ||
| [InlineData("sha512:401b09eab3c013d4ca54922bb802bec8fd5318192b0a75f201d8b372742c513925d98f76b340d9e59a4efdc45db9f5c640a21831b3d08be")] | ||
| [InlineData("sha512:cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e")] | ||
| public void Validate_ReturnsDigest_ForRegisteredAlgorithms(string validDigest) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validate the resulting tag format in
BuildReferrersTagto catch invalid OCI references early.Depends on: #383
Problem
BuildReferrersTagconverts a digest to a tag by replacing:with-. After #383 allows unrecognized algorithms per OCI spec, digests with algorithms containing+(e.g.,multihash+base58:Qm...) would pass digest validation but produce an invalid tag (multihash+base58-Qm...) since+is not allowed in OCI tag format ([\w][\w.-]{0,127}).Changes
Reference.cs: ChangeTryValidateTagfromprivatetointernalfor reuseReferrers.cs: Add tag validation after digest-to-tag conversion; throwInvalidReferenceExceptionif invalidReferrersTest.cs: Add test verifyingInvalidReferenceExceptionfor digests that produce invalid tagsTest Results
This PR was generated with AI assistance.