feat(hardened): add deployment-time registry prefix replacement map (TC-5107) - #642
Conversation
…TC-5107) Add configurable registry prefix replacement for hardened image recommendations, applied at response serialization time to avoid mutating cached index data. - Accept ordered map via `trustedcontent.recommendation.hardened.registry-map` - Replace `repository_url` qualifier prefix at lookup time - Skip digest-only images (no tag qualifier) from recommendations - Preserve image tags unchanged during replacement - Log configured mappings at startup (INFO level) - 10 new unit tests covering all acceptance criteria Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reviewer's GuideImplements configurable deployment-time registry prefix remapping for hardened image recommendations, including digest-aware lookup behavior, non-mutating application of a registry map at serialization time, configuration surface changes, startup logging, and comprehensive unit tests around mapping and digest handling. Sequence diagram for hardened image lookup with registry prefix mappingsequenceDiagram
actor Client
participant HardenedImageProvider
participant HummingbirdClient
participant HardenedImageResponseHandler
Client->>HardenedImageProvider: lookupBySbomId(sbomId)
HardenedImageProvider->>HardenedImageProvider: buildDockerRef(pkgRef)
alt [digest-only image]
HardenedImageProvider-->>Client: emptyMap
else [tag present or no digest]
HardenedImageProvider->>HummingbirdClient: getHardenedRecommendations(baseImageRef)
HummingbirdClient-->>HardenedImageProvider: List<IndexedRecommendation>
alt [registryMap not empty]
loop for each IndexedRecommendation
HardenedImageProvider->>HardenedImageResponseHandler: applyRegistryMap(rec.packageName, registryMap)
HardenedImageResponseHandler-->>HardenedImageProvider: PackageRef
HardenedImageProvider->>HardenedImageProvider: build new IndexedRecommendation
end
end
HardenedImageProvider-->>Client: Map<PackageRef, List<IndexedRecommendation>>
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- Consider treating
config.registryMap()as potentially null in bothlookupBySbomIdandlogRegistryMap(e.g., normalizing to an empty map when null) to avoid NPEs if the configuration provider ever returns a null map. - In
lookupBySbomId, you may want to avoid callingconfig.registryMap()on every lookup by resolving and caching a normalized (possibly immutable) map once at construction time, which would also ensure a consistent, ordered map forapplyRegistryMap.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider treating `config.registryMap()` as potentially null in both `lookupBySbomId` and `logRegistryMap` (e.g., normalizing to an empty map when null) to avoid NPEs if the configuration provider ever returns a null map.
- In `lookupBySbomId`, you may want to avoid calling `config.registryMap()` on every lookup by resolving and caching a normalized (possibly immutable) map once at construction time, which would also ensure a consistent, ordered map for `applyRegistryMap`.
## Individual Comments
### Comment 1
<location path="src/main/java/io/github/guacsec/trustifyda/integration/providers/trustify/hardened/HardenedImageResponseHandler.java" line_range="114-123" />
<code_context>
+ * Iterates through the map entries in order; the first matching source prefix is replaced with
+ * the target prefix. Returns the original ref unchanged if no entry matches or the map is empty.
+ */
+ static PackageRef applyRegistryMap(PackageRef ref, Map<String, String> registryMap) {
+ if (registryMap == null || registryMap.isEmpty()) {
+ return ref;
+ }
+
+ var qualifiers = ref.purl().getQualifiers();
+ if (qualifiers == null) {
+ return ref;
+ }
+ String repoUrl = qualifiers.get("repository_url");
+ if (repoUrl == null) {
+ return ref;
+ }
+
+ for (var entry : registryMap.entrySet()) {
+ String sourcePrefix = entry.getKey();
+ if (repoUrl.startsWith(sourcePrefix)) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Relying on `Map.entrySet()` order may lead to non-deterministic mapping if the provided map is not ordered.
The Javadoc promises an "in order" scan with the first matching prefix winning, but `Map` does not guarantee iteration order unless an ordered implementation is used (e.g., `LinkedHashMap`). If `registryMap()` ever returns a `HashMap` or similar, the chosen prefix may be non-deterministic. To ensure deterministic priority, either require an ordered map type at the API boundary or copy into a known-ordered structure before iterating.
</issue_to_address>
### Comment 2
<location path="src/test/java/io/github/guacsec/trustifyda/integration/providers/trustify/hardened/HardenedImageProviderTest.java" line_range="613-610" />
<code_context>
+ assertEquals(ref.ref(), result.ref());
+ }
+
+ // Verifies that applyRegistryMap handles an empty map gracefully.
+ @Test
+ void testApplyRegistryMapEmptyMap() {
+ // Given a PackageRef and an empty registry map
+ PackageRef ref = new PackageRef(HARDENED_NGINX_PURL);
+
+ // When applying the empty map
+ PackageRef result = HardenedImageResponseHandler.applyRegistryMap(ref, Collections.emptyMap());
+
+ // Then the original ref is returned unchanged
+ assertEquals(ref.ref(), result.ref());
+ }
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for PackageRefs missing qualifiers or repository_url in applyRegistryMap.
One remaining edge case is when the `PackageRef` qualifiers are `null` or don’t contain `"repository_url"`. Since the method short-circuits in those cases, please add tests where `ref.purl().getQualifiers()` returns `null` and where the qualifiers map exists but lacks `"repository_url"`, asserting that the original `ref` (or at least `ref.ref()`) is returned unchanged.
Suggested implementation:
```java
// Then the original ref is returned unchanged
assertEquals(ref.ref(), result.ref());
}
// Verifies that applyRegistryMap handles null qualifiers gracefully.
@Test
void testApplyRegistryMapNullQualifiers() {
// Given a PackageRef whose PURL has null qualifiers
PackageRef ref = mock(PackageRef.class);
PackageURL purl = mock(PackageURL.class);
when(ref.purl()).thenReturn(purl);
when(ref.ref()).thenReturn(HARDENED_NGINX_PURL);
when(purl.getQualifiers()).thenReturn(null);
Map<String, String> registryMap =
Map.of("quay.io/hummingbird", "registry.access.redhat.com/hi");
// When applying the registry map
PackageRef result = HardenedImageResponseHandler.applyRegistryMap(ref, registryMap);
// Then the original ref is returned unchanged
assertEquals(ref.ref(), result.ref());
}
// Verifies that applyRegistryMap handles missing repository_url qualifier gracefully.
@Test
void testApplyRegistryMapMissingRepositoryUrlQualifier() {
// Given a PackageRef whose PURL qualifiers do not contain repository_url
PackageRef ref = mock(PackageRef.class);
PackageURL purl = mock(PackageURL.class);
when(ref.purl()).thenReturn(purl);
when(ref.ref()).thenReturn(HARDENED_NGINX_PURL);
when(purl.getQualifiers()).thenReturn(Map.of("some_other_key", "some_value"));
Map<String, String> registryMap =
Map.of("quay.io/hummingbird", "registry.access.redhat.com/hi");
// When applying the registry map
PackageRef result = HardenedImageResponseHandler.applyRegistryMap(ref, registryMap);
// Then the original ref is returned unchanged
assertEquals(ref.ref(), result.ref());
}
import static org.junit.jupiter.api.Assertions.assertEquals;
```
1. Ensure the following imports exist at the top of `HardenedImageProviderTest.java` (add them if they are missing):
- `import com.github.packageurl.PackageURL;`
- `import java.util.Map;`
- `import org.junit.jupiter.api.Test;`
- `import static org.mockito.Mockito.mock;`
- `import static org.mockito.Mockito.when;`
2. If `HARDENED_NGINX_PURL` is not already a constant in this test class, replace it in the new tests with an appropriate ref string consistent with the rest of the tests.
</issue_to_address>
### Comment 3
<location path="src/test/java/io/github/guacsec/trustifyda/integration/providers/trustify/hardened/HardenedImageProviderTest.java" line_range="77" />
<code_context>
+ void setUp() {
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests to verify the logging behavior for the registry map configuration.
The acceptance criteria require logging the applied registry map at startup, but this behavior isn’t covered by tests. Please add tests that construct a `HardenedImageProvider` with (1) an empty registry map and (2) a non-empty map, and assert the expected INFO log messages (e.g., via a log appender or log-capturing extension). This will help catch regressions in the registry map observability behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #642 +/- ##
============================================
+ Coverage 56.67% 57.19% +0.52%
- Complexity 836 878 +42
============================================
Files 92 92
Lines 4863 5021 +158
Branches 655 691 +36
============================================
+ Hits 2756 2872 +116
- Misses 1814 1841 +27
- Partials 293 308 +15
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
… time (TC-5107) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Verification Report — TC-5107
DetailsScope Containment — All 5 changed files are within the hardened image provider package, directly implementing the task requirements. Diff Size — ~236 additions / 4 deletions across 5 files (~65% test code). Proportionate for the feature scope. Commit Traceability — All 3 commits reference TC-5107 in their subject lines. Sensitive Patterns — No secrets, credentials, or sensitive data detected. CI Status — All checks passed (Integration Tests, Sourcery review, commitlint). Acceptance Criteria — All 7 criteria satisfied: ConfigMapping interface with Test Quality — Three no-change scenarios correctly parameterized with Test Change Classification — 8 new test methods added to existing file. No existing tests modified or removed. This comment was AI-generated by sdlc-workflow/verify-pr v0.12.3. |
…s (TC-5107) Docker Hub images can appear as bare names (mariadb), docker.io/library/mariadb, or docker.io/mariadb. Normalize all variants to docker.io/<name> at both ingest and lookup time to prevent silent recommendation misses. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… (TC-5107)
The digest-pinned skip logic only matched sha256: prefixes. OCI supports
sha512 and sha384 as well — use contains(":") to catch any algorithm.
Also removes unreferenced test fixture.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Adds configurable deployment-time registry prefix replacement for hardened image recommendations (TC-5107).
trustedcontent.recommendation.hardened.registry-mapconfig propertyIndexedRecommendationobjects are created with mappedPackageRefvaluestagqualifier, version issha256:)Acceptance Criteria Coverage
Test Plan
HardenedImageProviderTestrepository_urlprefix in recommendationsmvn verify)🤖 Generated with Claude Code
Summary by Sourcery
Introduce configurable registry prefix mapping for hardened image recommendations and adjust lookup behavior for digest-pinned images.
New Features:
Enhancements:
Documentation:
Tests: