Critical Race Condition in Online GC Leading to Data Loss

[pulkitvats2007-crypto]

ISSUE REPORT: Race Condition in Online Garbage Collection Leading to Data Loss

Description

A critical race condition exists between Harbor's online Garbage Collection (GC) and concurrent artifact push/pull operations. The bug allows GC to delete blobs that were recently "touched" or are in the process of being associated with a new artifact, provided they were previously associated with a project but not an artifact for longer than the GC safety window (default 2 hours).

Root Cause Analysis

1. Safety Window Implementation: GC uses a 2-hour safety window in two places:

   • markOrSweepUntaggedBlobs: Removes project_blob associations for blobs not referenced by any artifact in that project if the association is > 2 hours old.
   • uselessBlobs (via GetBlobsNotRefedByProjectBlob): Identifies blobs for deletion if they have NO records in project_blob AND their update_time in the blob table is > 2 hours old.

2. The Race:

   • A common base layer (e.g., alpine:latest) exists in Project A, associated with an artifact. Its update_time is 5 hours ago.
   • A user pushes a new image to Project B that uses this same alpine layer.
   • Harbor's PutBlobUploadMiddleware sees the blob exists and calls AssociateWithProjectByDigest for Project B. A new project_blob record is created for Project B with creation_time = Now.
   • The blob table's update_time for this digest remains 5 hours old (it is NOT updated during association).
   • GC starts.
   • markOrSweepUntaggedBlobs runs for Project B. It sees the alpine blob in project_blob but no artifact in Project B references it yet. However, the project_blob record is NEW (< 2 hours), so it is kept.
   • Crucially, if the user was re-pushing to Project A where the association was OLD (> 2 hours) and the artifact was just deleted (e.g., overwriting a tag), markOrSweepUntaggedBlobs would DELETE the project_blob association for Project A.
   • Now, if the blob has no other project associations, uselessBlobs checks the blob table. It sees pb.id IS NULL (no associations) and b.update_time <= now() - 2 hours.
   • Even if the user just called HEAD or PUT on this blob (which calls Touch), Touch only updates update_time if the status is NOT StatusNone. If it was already StatusNone, the update_time stays OLD.
   • GC deletes the blob from storage.
   • The user pushes the manifest. It succeeds (DB records exist).
   • Pulling the image fails with 404 for the layer.

Impact

• Data Loss: Persistent blobs are deleted from storage.
• Image Corruption: Images become unpullable.
• High Severity: Affects production registries using online GC and CI/CD pipelines.

Proposed Fix

1. Always update update_time in Touch: Ensure blobController.Touch always updates the update_time and version in the blob table, regardless of the current status, whenever a blob is accessed via HEAD or PUT.
2. Update update_time on Association: When AssociateWithProject is called, the underlying blob record's update_time should also be refreshed to protect it from uselessBlobs if the project association is transiently removed or if it's the only association.

Reproduction Steps

1. Push a blob, wait 3 hours.
2. Delete the only artifact referencing it.
3. Simultaneously start a new push using that blob and run GC.
4. Observe GC deleting the blob before the new manifest is pushed.

[github-actions]

This issue is being marked stale due to a period of inactivity. If this issue is still relevant, please comment or remove the stale label. Otherwise, this issue will close in 30 days.

[github-actions]

This issue is being marked stale due to a period of inactivity. If this issue is still relevant, please comment or remove the stale label. Otherwise, this issue will close in 30 days.