From 3320c87659cb03228d1bfe4861d621c7f104ecfe Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Wed, 12 Aug 2026 11:08:36 +0000 Subject: [PATCH] fix: Harden instance rename against interleaving and lost unwinds --- provision/instance.go | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/provision/instance.go b/provision/instance.go index 2ba4d30..d4c6d8f 100644 --- a/provision/instance.go +++ b/provision/instance.go @@ -107,34 +107,41 @@ func RenameSlug(name, currentSlug string) (string, error) { // RenameInstance changes an instance's name and re-derives its slug from it. // +// It returns the name and slug the control plane held BEFORE the write, and +// those are the only correct values to unwind with. The caller's own copy of the +// instance may be stale, and admin's copy stores slug with `omitempty`, so an +// unwind driven from there can write an empty slug — which either mis-restores +// the tenant host or trips the unique index against every other slugless row. +// // The count-then-update is racy on its own, and is safe for the same reason // CreateInstanceWithID's loop is: instances.slug carries a unique index, so a // lost race surfaces as a duplicate-key error. Unlike creation there is nothing // to retry with — the caller asked for one specific name — so it becomes // ErrSlugTaken. Do not remove the duplicate-key branch, and do not remove the // index. -func RenameInstance(ctx context.Context, db *mongo.Database, instanceID, name string) (*models.Instance, error) { - var inst models.Instance +func RenameInstance(ctx context.Context, db *mongo.Database, instanceID, name string) (inst *models.Instance, prevName, prevSlug string, err error) { + var cur models.Instance if err := db.Collection("instances").FindOne(ctx, - bson.M{"instance_id": instanceID}).Decode(&inst); err != nil { - return nil, err + bson.M{"instance_id": instanceID}).Decode(&cur); err != nil { + return nil, "", "", err } + prevName, prevSlug = cur.Name, cur.Slug - slug, err := RenameSlug(name, inst.Slug) + slug, err := RenameSlug(name, cur.Slug) if err != nil { - return nil, err + return nil, prevName, prevSlug, err } - if slug != inst.Slug { + if slug != cur.Slug { n, err := db.Collection("instances").CountDocuments(ctx, bson.M{ "slug": slug, "instance_id": bson.M{"$ne": instanceID}, }) if err != nil { - return nil, err + return nil, prevName, prevSlug, err } if n > 0 { - return nil, fmt.Errorf("%w: %s", ErrSlugTaken, slug) + return nil, prevName, prevSlug, fmt.Errorf("%w: %s", ErrSlugTaken, slug) } } @@ -142,14 +149,14 @@ func RenameInstance(ctx context.Context, db *mongo.Database, instanceID, name st bson.M{"instance_id": instanceID}, bson.M{"$set": bson.M{"name": name, "slug": slug}}); err != nil { if mongo.IsDuplicateKeyError(err) { - return nil, fmt.Errorf("%w: %s", ErrSlugTaken, slug) + return nil, prevName, prevSlug, fmt.Errorf("%w: %s", ErrSlugTaken, slug) } - return nil, err + return nil, prevName, prevSlug, err } - inst.Name = name - inst.Slug = slug - return &inst, nil + cur.Name = name + cur.Slug = slug + return &cur, prevName, prevSlug, nil } // RestoreInstanceIdentity writes an exact name and slug back, unwinding a rename