mirror of
https://tangled.org/evan.jarrett.net/at-container-registry
synced 2026-08-28 20:06:02 +00:00
appview: stop a tag delete cascading into another repo's live image
io.atcr.manifest rkeys are the digest alone (digestToRKey), so a single
record backs every repository of a user holding identical content. Both
paths that cascade-delete that record checked for remaining tags scoped to
one repository, which asks the wrong question: a tag in another repo keeps
the shared record alive just as much as a tag in this one.
With me/a:v1 and me/b:v1 at the same digest, deleting me/a:v1 saw no
remaining tags in repo a, deleted the shared PDS record, and purged the
layers on the hold. me/b:v1 was left pointing at content that no longer
exists, and the firehose delete handler then cleared the rows for every
repo (DeleteManifest with an empty repository argument).
The collision predates this, but it was reachable only behind the opt-in
AutoRemoveUntagged profile flag. 1b91768 enabled OCI manifest DELETE and
made TagStore.Untag cascade unconditionally, which turned a latent metadata
collision into blob-level data loss on an ordinary skopeo/crane delete.
- cleanupUntaggedManifest no longer filters candidate tags to
rctx.Repository. The surrounding comment already noted that the tag
collection is account-wide; the filter contradicted it.
- ShouldCascadeDeleteManifest takes the tag question DID-wide via a new
IsManifestTaggedAnyRepo, and drops its now-meaningless repository
parameter.
- IsManifestTagged stays repository-scoped and keeps its caller: the
delete-manifest confirmation prompt is genuinely asking about the one
repo whose tags the user is about to remove.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
adc6394ebc
commit
2580dcdb0f
@@ -101,7 +101,7 @@ func TestShouldCascadeDeleteManifest_LastTagAndNoParent(t *testing.T) {
|
||||
seedCascadeFixture(t, db, did, repo, digest)
|
||||
|
||||
// No tags pointing to this digest, no manifest_references entries.
|
||||
ok, err := ShouldCascadeDeleteManifest(db, did, repo, digest)
|
||||
ok, err := ShouldCascadeDeleteManifest(db, did, digest)
|
||||
if err != nil {
|
||||
t.Fatalf("ShouldCascadeDeleteManifest: %v", err)
|
||||
}
|
||||
@@ -135,7 +135,7 @@ func TestShouldCascadeDeleteManifest_RemainingTagBlocks(t *testing.T) {
|
||||
t.Fatalf("UpsertTag: %v", err)
|
||||
}
|
||||
|
||||
ok, err := ShouldCascadeDeleteManifest(db, did, repo, digest)
|
||||
ok, err := ShouldCascadeDeleteManifest(db, did, digest)
|
||||
if err != nil {
|
||||
t.Fatalf("ShouldCascadeDeleteManifest: %v", err)
|
||||
}
|
||||
@@ -144,6 +144,57 @@ func TestShouldCascadeDeleteManifest_RemainingTagBlocks(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestShouldCascadeDeleteManifest_TagInOtherRepoBlocks: the same digest is
|
||||
// tagged in a *different* repository of the same user. The io.atcr.manifest
|
||||
// record is keyed by digest alone, so one record backs both repos — cascading
|
||||
// here would delete the record out from under the other repo and purge the
|
||||
// shared layers on the hold, breaking an image the user never touched.
|
||||
func TestShouldCascadeDeleteManifest_TagInOtherRepoBlocks(t *testing.T) {
|
||||
db, err := InitDB(":memory:", LibsqlConfig{})
|
||||
if err != nil {
|
||||
t.Fatalf("InitDB: %v", err)
|
||||
}
|
||||
defer db.Close()
|
||||
|
||||
const did = "did:plc:cascade4"
|
||||
const deletingRepo = "myapp"
|
||||
const otherRepo = "myapp-mirror"
|
||||
const digest = "sha256:sharedacrossrepos"
|
||||
|
||||
// Same content pushed to two repositories.
|
||||
seedCascadeFixture(t, db, did, deletingRepo, digest)
|
||||
if _, err := InsertManifest(db, &Manifest{
|
||||
DID: did,
|
||||
Repository: otherRepo,
|
||||
Digest: digest,
|
||||
HoldEndpoint: "did:web:hold.example.com",
|
||||
SchemaVersion: 2,
|
||||
MediaType: "application/vnd.oci.image.manifest.v1+json",
|
||||
CreatedAt: time.Now(),
|
||||
}); err != nil {
|
||||
t.Fatalf("InsertManifest(otherRepo): %v", err)
|
||||
}
|
||||
|
||||
// Only the other repo still carries a tag. The deleting repo has none.
|
||||
if err := UpsertTag(db, &Tag{
|
||||
DID: did,
|
||||
Repository: otherRepo,
|
||||
Tag: "v1",
|
||||
Digest: digest,
|
||||
CreatedAt: time.Now(),
|
||||
}); err != nil {
|
||||
t.Fatalf("UpsertTag: %v", err)
|
||||
}
|
||||
|
||||
ok, err := ShouldCascadeDeleteManifest(db, did, digest)
|
||||
if err != nil {
|
||||
t.Fatalf("ShouldCascadeDeleteManifest: %v", err)
|
||||
}
|
||||
if ok {
|
||||
t.Error("expected cascade=false when the digest is still tagged in another repository")
|
||||
}
|
||||
}
|
||||
|
||||
// TestShouldCascadeDeleteManifest_MultiArchChildBlocks: the digest is a child
|
||||
// of a manifest list (multi-arch parent). Even with no tags, deleting it
|
||||
// would orphan the parent's reference, so we must NOT cascade.
|
||||
@@ -189,7 +240,7 @@ func TestShouldCascadeDeleteManifest_MultiArchChildBlocks(t *testing.T) {
|
||||
t.Fatalf("InsertManifestReference: %v", err)
|
||||
}
|
||||
|
||||
ok, err := ShouldCascadeDeleteManifest(db, did, repo, childDigest)
|
||||
ok, err := ShouldCascadeDeleteManifest(db, did, childDigest)
|
||||
if err != nil {
|
||||
t.Fatalf("ShouldCascadeDeleteManifest: %v", err)
|
||||
}
|
||||
|
||||
@@ -1635,12 +1635,18 @@ func GetTagDigest(db DBTX, did, repository, tag string) (string, error) {
|
||||
}
|
||||
|
||||
// ShouldCascadeDeleteManifest returns true iff a manifest can be safely
|
||||
// deleted after a tag is removed: it has no remaining tags AND is not
|
||||
// referenced by any manifest list (multi-arch parent). Manifest-list
|
||||
// children must be preserved even when untagged, since their parent index
|
||||
// still depends on them.
|
||||
func ShouldCascadeDeleteManifest(db DBTX, did, repository, digest string) (bool, error) {
|
||||
tagged, err := IsManifestTagged(db, did, repository, digest)
|
||||
// deleted after a tag is removed: it has no remaining tags in ANY of the
|
||||
// user's repositories AND is not referenced by any manifest list (multi-arch
|
||||
// parent). Manifest-list children must be preserved even when untagged, since
|
||||
// their parent index still depends on them.
|
||||
//
|
||||
// The tag check is deliberately DID-wide rather than repository-scoped. The
|
||||
// io.atcr.manifest record this gates is keyed by digest alone, so a single
|
||||
// record backs every repository holding identical content; a repo-scoped check
|
||||
// would cascade a delete in one repo into another repo's live image and purge
|
||||
// the shared layers on the hold.
|
||||
func ShouldCascadeDeleteManifest(db DBTX, did, digest string) (bool, error) {
|
||||
tagged, err := IsManifestTaggedAnyRepo(db, did, digest)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
@@ -1654,7 +1660,26 @@ func ShouldCascadeDeleteManifest(db DBTX, did, repository, digest string) (bool,
|
||||
return !referenced, nil
|
||||
}
|
||||
|
||||
// IsManifestTagged checks if a manifest has any tags
|
||||
// IsManifestTaggedAnyRepo reports whether a digest still carries a tag in any
|
||||
// of the user's repositories. Use this to gate deletion of per-digest state
|
||||
// (the io.atcr.manifest record, hold layer records); use IsManifestTagged when
|
||||
// the question is genuinely about one repository, such as a confirmation
|
||||
// prompt listing the tags a user is about to remove.
|
||||
func IsManifestTaggedAnyRepo(db DBTX, did, digest string) (bool, error) {
|
||||
var count int
|
||||
err := db.QueryRow(`
|
||||
SELECT COUNT(*) FROM tags
|
||||
WHERE did = ? AND digest = ?
|
||||
`, did, digest).Scan(&count)
|
||||
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
|
||||
return count > 0, nil
|
||||
}
|
||||
|
||||
// IsManifestTagged checks if a manifest has any tags in the given repository
|
||||
func IsManifestTagged(db DBTX, did, repository, digest string) (bool, error) {
|
||||
var count int
|
||||
err := db.QueryRow(`
|
||||
|
||||
@@ -90,7 +90,7 @@ func (h *DeleteTagHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
// only-tagged image. Failures here are non-fatal: the tag is already
|
||||
// gone, so the worst case is a leftover untagged manifest the user can
|
||||
// clean up via "Delete untagged".
|
||||
shouldCascade, err := db.ShouldCascadeDeleteManifest(h.ReadOnlyDB, user.DID, repo, digest)
|
||||
shouldCascade, err := db.ShouldCascadeDeleteManifest(h.ReadOnlyDB, user.DID, digest)
|
||||
if err != nil {
|
||||
slog.Warn("delete-tag: failed to evaluate cascade", "did", user.DID, "repo", repo, "digest", digest, "error", err)
|
||||
} else if shouldCascade {
|
||||
|
||||
@@ -909,14 +909,20 @@ func cleanupUntaggedManifest(rctx *RegistryContext, oldDigest string) {
|
||||
if json.Unmarshal(rec.Value, &tagRecord) != nil {
|
||||
continue
|
||||
}
|
||||
if tagRecord.Repository != rctx.Repository {
|
||||
continue
|
||||
}
|
||||
// Deliberately not filtered to rctx.Repository. The record this
|
||||
// check protects is keyed by digest alone (digestToRKey), so one
|
||||
// io.atcr.manifest record backs every repository of this DID that
|
||||
// holds identical content. A tag in any of them keeps it alive.
|
||||
// Scoping the check to the current repository would let a delete in
|
||||
// one repo remove the record out from under another and purge the
|
||||
// shared layers on the hold, breaking an image nobody touched.
|
||||
d, dErr := tagRecord.GetManifestDigest()
|
||||
if dErr == nil && d == oldDigest {
|
||||
// Still tagged by another tag — do not delete
|
||||
// Still tagged, here or in another repository — do not delete
|
||||
slog.Debug("Auto-remove: manifest still tagged, skipping",
|
||||
"digest", oldDigest, "tag", tagRecord.Tag)
|
||||
"digest", oldDigest, "tag", tagRecord.Tag,
|
||||
"taggedRepository", tagRecord.Repository,
|
||||
"deletingRepository", rctx.Repository)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user