fix(backup): reject cross-repository image pin

internal/engine/backup_image_test.go

@@ -71,6 +71,76 @@ func TestReEnableKeepsTheRecordedPinAndDoesNotReachTheRegistry(t *testing.T) {
 	}
 }
 
+func TestReEnableRejectsPinFromDifferentRepository(t *testing.T) {
+	const stalePin = "postgres@sha256:06cad38a5d9f5d24b4d83d86def30795d5e4b757fedbf5281172b576dedcd941"
+	const managedPin = "ghcr.io/labstack/onebox-postgres@sha256:16cad38a5d9f5d24b4d83d86def30795d5e4b757fedbf5281172b576dedcd942"
+	fake := &transport.Fake{Dynamic: func(cmd string) (transport.Result, bool) {
+		switch {
+		case strings.Contains(cmd, "docker image inspect") && strings.Contains(cmd, stalePin):
+			return transport.Result{Stdout: "present\n"}, true
+		case strings.Contains(cmd, "docker pull"):
+			return transport.Result{}, true
+		case strings.Contains(cmd, "RepoDigests"):
+			return transport.Result{Stdout: managedPin + "\n"}, true
+		}
+		return transport.Result{Stdout: "absent\n"}, true
+	}}
+	e := protectedImageTestEngine(fake)
+	bound, err := e.Spec.WithServiceRuntimeStates(map[string]app.ServiceRuntimeState{
+		"database": {
+			BackupState: "disabled", ServiceImage: stalePin,
+			PublicationVerified: true, DigestAvailable: true,
+		},
+	})
+	if err != nil {
+		t.Fatalf("binding disabled service state: %v", err)
+	}
+	e.Spec = bound
+
+	got, err := e.ResolveProtectedImage(context.Background(), "database", stalePin, "ghcr.io/labstack/onebox-postgres:18")
+	if err != nil {
+		t.Fatalf("resolving an inconsistent recorded pin: %v", err)
+	}
+	if got != managedPin {
+		t.Fatalf("resolved %q, want %q", got, managedPin)
+	}
+}
+
+func TestReEnableDoesNotCreatePinFromDifferentRepository(t *testing.T) {
+	const stalePin = "postgres@sha256:06cad38a5d9f5d24b4d83d86def30795d5e4b757fedbf5281172b576dedcd941"
+	const managedPin = "ghcr.io/labstack/onebox-postgres@sha256:16cad38a5d9f5d24b4d83d86def30795d5e4b757fedbf5281172b576dedcd942"
+	fake := &transport.Fake{Dynamic: func(cmd string) (transport.Result, bool) {
+		switch {
+		case strings.Contains(cmd, "docker image inspect") && strings.Contains(cmd, stalePin):
+			return transport.Result{Stdout: "present\n"}, true
+		case strings.Contains(cmd, "docker pull"):
+			return transport.Result{}, true
+		case strings.Contains(cmd, "RepoDigests"):
+			return transport.Result{Stdout: managedPin + "\n"}, true
+		}
+		return transport.Result{Stdout: "absent\n"}, true
+	}}
+	e := protectedImageTestEngine(fake)
+	bound, err := e.Spec.WithServiceRuntimeStates(map[string]app.ServiceRuntimeState{
+		"database": {
+			BackupState: "enabled", ServiceImage: stalePin,
+			PublicationVerified: true, DigestAvailable: true,
+		},
+	})
+	if err != nil {
+		t.Fatalf("binding enabled service state: %v", err)
+	}
+	e.Spec = bound
+
+	got, err := e.ResolveProtectedImage(context.Background(), "database", stalePin, "postgres:18")
+	if err != nil {
+		t.Fatalf("resolving after a repository migration: %v", err)
+	}
+	if got != managedPin {
+		t.Fatalf("resolved %q, want %q", got, managedPin)
+	}
+}
+
 // The pin is only reusable while the project still declares the reference that
 // produced it. Changing the declared version is how an operator asks for
 // different bytes, and that has to reach the registry.

internal/engine/backup_postgres.go

@@ -15,6 +15,7 @@ import (
 	"strings"
 	"time"
 
+	distref "github.com/distribution/reference"
 	"github.com/labstack/onebox/internal/app"
 )
 
@@ -469,11 +470,9 @@ func (e *Engine) RebindServiceRuntimeStates(states map[string]app.ServiceRuntime
 // ResolveProtectedImage pins the service image by the digest the host actually
 // has, after pulling it.
 //
-// It is the stock PostgreSQL image — wal-g is mounted in beside it rather than
-// baked into a derived one — but it is still pinned, because the reason
-// protected image selection is durable state has nothing to do with which image
-// it is: the bytes running over a live data directory must not change because a
-// tag moved.
+// WAL-G is mounted beside the PostgreSQL runtime image rather than baked into
+// it, but the image is still pinned: the bytes running over a live data
+// directory must not change because a tag moved.
 // recordedPin and recordedReference come from the service's lifecycle record:
 // the digest it was last bound with, and the reference that produced it. When
 // the project still declares that same reference and the host still holds those
@@ -485,8 +484,11 @@ func (e *Engine) RebindServiceRuntimeStates(states map[string]app.ServiceRuntime
 // already had everything it needed, so a rate-limited Docker Hub failed a
 // command that had nothing to fetch.
 //
-// Moving a protected service to a new image is a deliberate act with its own
-// path; it is not a side effect of re-running enable.
+// A declared repository migration is the exception: a digest from the old
+// repository cannot be recorded as if the new reference produced it, so the
+// new declaration is resolved before enablement can write the lifecycle pair.
+// Moving between versions within one repository remains a deliberate act with
+// its own path; it is not a side effect of re-running enable.
 func (e *Engine) ResolveProtectedImage(ctx context.Context, service, recordedPin, recordedReference string) (string, error) {
 	reference, err := e.Spec.ServiceImageForRuntime(service)
 	if err != nil {
@@ -496,8 +498,13 @@ func (e *Engine) ResolveProtectedImage(ctx context.Context, service, recordedPin
 	if err != nil {
 		return "", err
 	}
-	st := e.ui.Step("protected image "+reference.Image, false)
-	if recordedPin != "" && recordedReference == declared && containsDigest(recordedPin) {
+	resolutionReference := reference.Image
+	if !sameImageRepository(resolutionReference, declared) {
+		resolutionReference = declared
+	}
+	st := e.ui.Step("protected image "+resolutionReference, false)
+	if recordedPin != "" && recordedReference == declared && containsDigest(recordedPin) &&
+		sameImageRepository(recordedPin, recordedReference) {
 		held, err := e.imagePresentByDigest(ctx, recordedPin)
 		if err != nil {
 			st(err)
@@ -513,26 +520,26 @@ func (e *Engine) ResolveProtectedImage(ctx context.Context, service, recordedPin
 	// — it only spends registry quota and turns a re-enable into a failure on a
 	// host that is offline or rate-limited while holding exactly what it needs.
 	// A tag still resolves through the registry, because a tag can move.
-	present, err := e.imagePresentByDigest(ctx, reference.Image)
+	present, err := e.imagePresentByDigest(ctx, resolutionReference)
 	if err != nil {
 		st(err)
 		return "", err
 	}
 	if present {
 		st(nil)
-		return reference.Image, nil
+		return resolutionReference, nil
 	}
-	res, err := e.T.Run(ctx, "docker pull "+q(reference.Image))
+	res, err := e.T.Run(ctx, "docker pull "+q(resolutionReference))
 	if err != nil {
 		st(err)
 		return "", err
 	}
 	if res.ExitCode != 0 {
-		err := fmt.Errorf("cannot pull %s: %s", reference.Image, lastLines(res.Stderr, 3))
+		err := fmt.Errorf("cannot pull %s: %s", resolutionReference, lastLines(res.Stderr, 3))
 		st(err)
 		return "", err
 	}
-	res, err = e.T.Run(ctx, "docker image inspect --format '{{index .RepoDigests 0}}' "+q(reference.Image))
+	res, err = e.T.Run(ctx, "docker image inspect --format '{{index .RepoDigests 0}}' "+q(resolutionReference))
 	if err != nil {
 		st(err)
 		return "", err
@@ -541,7 +548,12 @@ func (e *Engine) ResolveProtectedImage(ctx context.Context, service, recordedPin
 	if res.ExitCode != 0 || !containsDigest(pinned) {
 		err := fmt.Errorf(
 			"%s has no registry digest on this host; a protected service runs an image pinned by digest, so it must come from a registry rather than a local build",
-			reference.Image)
+			resolutionReference)
+		st(err)
+		return "", err
+	}
+	if !sameImageRepository(pinned, resolutionReference) {
+		err := fmt.Errorf("resolved digest %s does not belong to image repository %s", pinned, resolutionReference)
 		st(err)
 		return "", err
 	}
@@ -730,3 +742,19 @@ func (e *Engine) imagePresentByDigest(ctx context.Context, reference string) (bo
 // tag. It is the one place that decides, so the pull-skipping guard and the
 // pinning check cannot disagree about what "pinned" means.
 func containsDigest(reference string) bool { return strings.Contains(reference, "@sha256:") }
+
+// sameImageRepository prevents a lifecycle record from pairing an authored
+// reference with a digest from another repository. That mismatch can happen
+// across a managed-image repository migration; retaining it would restart the
+// service on bytes the current declaration can no longer produce.
+func sameImageRepository(left, right string) bool {
+	leftNamed, err := distref.ParseNormalizedNamed(left)
+	if err != nil {
+		return false
+	}
+	rightNamed, err := distref.ParseNormalizedNamed(right)
+	if err != nil {
+		return false
+	}
+	return distref.TrimNamed(leftNamed).Name() == distref.TrimNamed(rightNamed).Name()
+}