From 5c56e059732252e7ef3d1637d555d288b078317b Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Sat, 29 Aug 2026 14:54:37 -0700 Subject: [PATCH 1/2] fix(backup): reject cross-repository image pin A managed-image reference could retain an official PostgreSQL digest after an upgrade and restart a protected service on incompatible bytes. Re-resolve the declared image when repository identities differ. Closes #139 --- internal/engine/backup_image_test.go | 35 ++++++++++++++++++++++++++++ internal/engine/backup_postgres.go | 20 +++++++++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/internal/engine/backup_image_test.go b/internal/engine/backup_image_test.go index 742879f2..4ee165bc 100644 --- a/internal/engine/backup_image_test.go +++ b/internal/engine/backup_image_test.go @@ -71,6 +71,41 @@ 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) + } +} + // 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. diff --git a/internal/engine/backup_postgres.go b/internal/engine/backup_postgres.go index 688d7b5c..6ccd1b33 100644 --- a/internal/engine/backup_postgres.go +++ b/internal/engine/backup_postgres.go @@ -15,6 +15,7 @@ import ( "strings" "time" + distref "github.com/distribution/reference" "github.com/labstack/onebox/internal/app" ) @@ -497,7 +498,8 @@ func (e *Engine) ResolveProtectedImage(ctx context.Context, service, recordedPin return "", err } st := e.ui.Step("protected image "+reference.Image, false) - if recordedPin != "" && recordedReference == declared && containsDigest(recordedPin) { + if recordedPin != "" && recordedReference == declared && containsDigest(recordedPin) && + sameImageRepository(recordedPin, recordedReference) { held, err := e.imagePresentByDigest(ctx, recordedPin) if err != nil { st(err) @@ -730,3 +732,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() +} From c558488de2e47092d92b73d44759c5a6971678f7 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Sat, 29 Aug 2026 15:01:46 -0700 Subject: [PATCH 2/2] fix(backup): complete repository pin migration --- internal/engine/backup_image_test.go | 35 +++++++++++++++++++++++++ internal/engine/backup_postgres.go | 38 ++++++++++++++++++---------- 2 files changed, 59 insertions(+), 14 deletions(-) diff --git a/internal/engine/backup_image_test.go b/internal/engine/backup_image_test.go index 4ee165bc..577e61dd 100644 --- a/internal/engine/backup_image_test.go +++ b/internal/engine/backup_image_test.go @@ -106,6 +106,41 @@ func TestReEnableRejectsPinFromDifferentRepository(t *testing.T) { } } +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. diff --git a/internal/engine/backup_postgres.go b/internal/engine/backup_postgres.go index 6ccd1b33..7db2ca24 100644 --- a/internal/engine/backup_postgres.go +++ b/internal/engine/backup_postgres.go @@ -470,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 @@ -486,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 { @@ -497,7 +498,11 @@ func (e *Engine) ResolveProtectedImage(ctx context.Context, service, recordedPin if err != nil { return "", err } - st := e.ui.Step("protected image "+reference.Image, false) + 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) @@ -515,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 @@ -543,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 }