Skip to content

[BUG] Manual image registration identifies images by name and can take over discovered images #284

Open

Description

@LarsLaskowski

Summary

Manual registration on /observed-images decides whether to create or update an observed image by comparing the entered name — untrimmed and case-sensitive — with stored names that are trimmed and not unique. This creates near-duplicate entries, can make registration fail with "Sequence contains more than one element", and lets a manual registration silently overwrite a discovered (own) image, which the next discovery run then overwrites again. The identity of a manually registered image must be its normalized image reference, not its display name.

Area

  • docs/areas/image-catalog.md (added in [Docs] Describe the current behavior by area #263), Observed images → Manual registration; also docs/areas/web-ui.md (Register and scan).
  • Security area: Database and migrations (EF Core query built from user input; no schema change expected).

Current behavior

src/DockerUpdateGuard/Images/ImageRegistrationService.cs:61-91:

var parsedReference = _imageReferenceParser.Parse(request.ImageReference);
var imageVersion = await _imageCatalogRepository.GetOrCreateImageVersionAsync(...);
imageVersion.Source = ImageVersionSource.ObservedImage;

var existingImage = await _dbContext.ObservedImages.SingleOrDefaultAsync(entity => entity.Name == request.Name, cancellationToken)
...
    existingImage = new ObservedImage { Name = request.Name.Trim(), ..., Source = RegistrationSource.Manual };
...
else
{
    existingImage.Description = ...;
    existingImage.CurrentImageVersionId = imageVersion.Id;
    existingImage.UpdatedAtUtc = DateTimeOffset.UtcNow;
    existingImage.IsEnabled = true;
}
  • The lookup uses the raw request.Name (:70), the insert stores request.Name.Trim() (:77), and Name has no unique index (src/DockerUpdateGuard.Data/Configurations/ObservedImageConfiguration.cs:24-26). PostgreSQL compares case-sensitively.
  • The lookup is not restricted to Source == Manual; discovered images are named after their repository (DockerHubAccountImageDiscoveryService.cs:217, :231), so their names are reachable from the form. The update branch keeps Source and changes CurrentImageVersionId.

Failure scenarios:

  1. Near-duplicates: register name web with nginx:1.27 → stored as web. Register web again → the lookup for web finds nothing → a second observed image web with the same reference. Register Web → a third one.
  2. Registration crashes: after scenario 1, register web → SingleOrDefaultAsync finds two rows and throws InvalidOperationException ("Sequence contains more than one element"); the form shows that message and nothing is saved.
  3. Silent takeover of a discovered image: discovery tracks myuser/app (name myuser/app, source discovery). The user registers name myuser/app with reference nginx:1.27. The discovered image's current version becomes nginx:1.27 while it stays an own image on /my-images; the scan opened by the form scans nginx under the own image's name. The next discovery run (DockerHubAccountImageDiscoveryService.cs:231-235) moves it back to the latest myuser/app tag — the manual registration is lost without notice.
  4. Same image twice under different names: registering nginx:1.27 as web and as frontend creates two observed images tracking the same image version (scanned twice, counted twice).

Expected behavior

  • The identity of a manually registered image is its normalized image reference: the canonical registry/repository:tag[@digest] from ImageReferenceParser (canonical Docker Hub form per [BUG] Docker Hub image references are not normalized to one canonical form #275), i.e. the catalog image version it resolves to.
  • Registration looks for an existing observed image with Source == Manual whose current image version is that image version:
  • The name is a trimmed display label, not an identity. A name that is empty after trimming is rejected by the form; duplicate names are allowed (they no longer cause errors because names are not looked up).
  • No takeover of discovered images: registration never changes a discovered image. If the normalized reference is the current image version of a discovered image that is tracked (not ignored, see [BUG] Observed images cannot be disabled or deleted, and discovery re-enables them #282), registration is rejected with "This image is already tracked as own image ''." and nothing is saved; the form links to /my-images/{id}.
  • Registration of a reference that differs only in Docker Hub spelling (nginx:1.27 vs docker.io/library/nginx:1.27) or surrounding whitespace finds the same manual image.

Acceptance criteria

  • Registering the same reference twice (with different names, different case or surrounding whitespace in the name) results in one manual observed image whose name is the trimmed name of the last registration.
  • Registering two different references with the same name creates two observed images and does not throw.
  • Registering a reference that is the current version of a tracked discovered image is rejected with the message above; the discovered image is unchanged (name, description, current version, source, enabled state).
  • Registering a name equal to a discovered image's name with a different reference creates a new manual image and leaves the discovered image unchanged.
  • A name that is blank after trimming is rejected; a trimmed name longer than 200 characters is rejected with a clear message.
  • Unit tests in src/Tests/DockerUpdateGuard.Tests/ImageRegistrationServiceTests.cs, e.g. ImageRegistrationServiceRegisterSameReferenceTwiceUpdatesExistingImage, ImageRegistrationServiceRegisterSameNameDifferentReferenceCreatesSecondImage, ImageRegistrationServiceRegisterTrimsNameAndMatchesByReference, ImageRegistrationServiceRegisterReferenceOfDiscoveredImageIsRejected, ImageRegistrationServiceRegisterDiscoveredImageNameDoesNotTakeOver, ImageRegistrationServiceRegisterDockerHubSpellingVariantsMatchSameImage.
  • The area document docs/areas/image-catalog.md describes the new behavior and its Known gaps entry for this issue is removed; docs/areas/web-ui.md (Register and scan) is updated.

Implementation notes

  • In ImageRegistrationService.RegisterAsync: trim the name first (validate), parse and get-or-create the image version (unchanged), then query ObservedImages.Where(e => e.CurrentImageVersionId == imageVersion.Id) and branch on Source. Use FirstOrDefaultAsync with a deterministic order (oldest first) for the manual match, so pre-existing duplicates from earlier versions do not crash registration.
  • The rejection for discovered images must be raised before SaveChangesAsync; note that GetOrCreateImageVersionAsync already created/returned the catalog entry — that is acceptable (catalog entries are shared and never deleted), but do not set imageVersion.Source before the check.
  • Existing duplicates (same reference, several manual rows) are not merged automatically; they remain and the user can delete the extras once [BUG] Observed images cannot be disabled or deleted, and discovery re-enables them #282 is implemented. Mention this in the area document.
  • No unique index on Name (names are labels). A unique index on (CurrentImageVersionId) for manual images is not added either, because discovered images move their current version and could legitimately collide; enforce the rule in the service.
  • Use an injected TimeProvider for UpdatedAtUtc when touching this code (docs/UNIT_TESTS.md, Time in tests).
  • Depends on the canonical Docker Hub form from [BUG] Docker Hub image references are not normalized to one canonical form #275 for the spelling-variant criterion; the rest can be implemented independently.
  • Do not change: the immediate manual scan after registration and the redirect to the detail page; IImageCatalogRepository as the only catalog writer.
  • No configuration or migration change.

Related


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions