From 514a74cec458d90753d3e40ccec7d2c8abfc13d4 Mon Sep 17 00:00:00 2001 From: CMGS Date: Mon, 11 May 2026 13:47:42 +0800 Subject: [PATCH 1/2] feat(image): validate ref shape against ImageType before auto-pull MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EnsureImage drives pulls based on vmCfg.ImageType, but never checked that vmCfg.Image actually has the shape the chosen backend can handle. A cloudimg backend with a bare OCI ref (`simular/win10:22h2-20260510`) ran all the way down to http.Get(), which returned the cryptic `unsupported protocol scheme ""` — the issue 38 failure mode. Add validateRefShape() ahead of Pull: - cloudimg: must be a URL with scheme (http/https); bare refs or local names are imported images that auto-pull can't materialize anyway - oci: must be parseable by name.ParseReference Malformed refs trigger a clear warn-and-return instead of a wasted HTTP round-trip with a misleading error. This is a defensive guard, not the issue 38 fix proper — vk-cocoon still needs to stop writing bare OCI refs into `cocoonstack.snapshot.baseimage` annotations for cloudimg bases. But the guard makes mis-annotated snapshots fail fast with an actionable signal at the cocoon CLI layer. --- cmd/core/ensure_image_test.go | 26 ++++++++++++++++++++++++++ cmd/core/helpers.go | 27 +++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/cmd/core/ensure_image_test.go b/cmd/core/ensure_image_test.go index 5eb2414e..44d39de6 100644 --- a/cmd/core/ensure_image_test.go +++ b/cmd/core/ensure_image_test.go @@ -98,6 +98,32 @@ func TestEnsureImage_ForceWhenDigestPinned(t *testing.T) { } } +// Issue 38 regression: a cloudimg vmCfg.Image without an http(s) scheme +// must not reach Pull — that would surface as `unsupported protocol scheme` +// from http.Get. The shape guard short-circuits with an actionable warning. +func TestEnsureImage_SkipsBadShape(t *testing.T) { + tests := []struct { + name string + image string + imageType string + }{ + {"cloudimg bare OCI ref", "simular/win10:22h2-20260510", types.ImageTypeCloudImg}, + {"cloudimg local name", "win11", types.ImageTypeCloudImg}, + {"oci malformed ref", "::bad::", types.ImageTypeOCI}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &fakeImageBackend{typ: tt.imageType} + EnsureImage(t.Context(), []imagebackend.Images{f}, &types.VMConfig{ + Config: types.Config{Image: tt.image, ImageType: tt.imageType}, + }) + if len(f.pullRefs) != 0 { + t.Errorf("Pull called %d time(s) with %v, want 0 (shape should have failed)", len(f.pullRefs), f.pullRefs) + } + }) + } +} + func TestEnsureImage_SkipsPullWhenDigestLocal(t *testing.T) { const digest = "sha256:adafd938488daa114be898848eb24b9b0afffc21ac18f8b11f3f0057644b11e1" f := &fakeImageBackend{ diff --git a/cmd/core/helpers.go b/cmd/core/helpers.go index f49f81f5..db431e9d 100644 --- a/cmd/core/helpers.go +++ b/cmd/core/helpers.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "net/url" "os" "strings" "text/tabwriter" @@ -248,6 +249,10 @@ func EnsureImage(ctx context.Context, backends []imagebackend.Images, vmCfg *typ // Pull by digest reference when available — ensures we get the exact // version recorded at snapshot time, not whatever the tag points to now. pullRef := digestPullRef(vmCfg.Image, vmCfg.ImageDigest, vmCfg.ImageType) + if shapeErr := validateRefShape(pullRef, vmCfg.ImageType); shapeErr != nil { + logger.Warnf(ctx, "skipping auto-pull of %s: %v — pre-pull manually if missing", pullRef, shapeErr) + return + } logger.Infof(ctx, "base image not found locally, pulling %s ...", pullRef) // Pinned digest with no local hit: force past cloudimg's URL-keyed cache. needForce := vmCfg.ImageDigest != "" @@ -491,6 +496,28 @@ func IsURL(ref string) bool { return strings.HasPrefix(ref, "http://") || strings.HasPrefix(ref, "https://") } +// validateRefShape catches malformed base-image refs before they hit a backend +// that would return a misleading downstream error. Each backend expects a +// different ref form: cloudimg fetches over HTTP so the ref must be a URL +// with scheme; OCI pulls via container registry protocol so the ref must be +// parseable by name.ParseReference (registry/repo:tag or repo@digest). +// A bare OCI form leaking into the cloudimg path (the issue 38 failure mode) +// would otherwise surface as `unsupported protocol scheme ""` from http.Get. +func validateRefShape(ref, imageType string) error { + switch imageType { + case types.ImageTypeCloudImg: + u, err := url.Parse(ref) + if err != nil || u.Scheme == "" { + return fmt.Errorf("cloudimg ref %q is not an http(s) URL (imported or bare OCI ref?)", ref) + } + case types.ImageTypeOCI: + if _, err := name.ParseReference(ref); err != nil { + return fmt.Errorf("oci ref %q is not a valid OCI reference: %w", ref, err) + } + } + return nil +} + // digestPullRef pins OCI pulls by digest; returns image as-is for others. func digestPullRef(image, digest, imageType string) string { if digest == "" || imageType != types.ImageTypeOCI { From d9297f41d05becc1a8d1559c8110c650b3f4861f Mon Sep 17 00:00:00 2001 From: CMGS Date: Mon, 11 May 2026 14:09:10 +0800 Subject: [PATCH 2/2] review: reuse IsURL, lock acceptance, drop issue refs /code pass on the validateRefShape change: - helpers.go: drop the standalone url.Parse(ref) + Scheme!="" check in favor of the existing IsURL helper. url.Parse accepts non-http schemes (file://, git://) that fail at cloudimg.Pull anyway; IsURL restricts to http(s):// directly, matching the comment and the cloudimg backend's actual contract. Also drops the net/url import. - ensure_image_test.go: add positive-case TestEnsureImage_AcceptsGoodShape to lock in that well-formed cloudimg URLs and tagged OCI refs still reach Pull; add a "cloudimg non-http scheme" (file://) reject case to cover the url.Parse-passes-but-shouldn't trap. - Drop "issue 38" references from godoc and test comment per "don't reference current task/fix" rule; the surrounding behavior stands on its own. --- cmd/core/ensure_image_test.go | 29 ++++++++++++++++++++++++++--- cmd/core/helpers.go | 14 +++++--------- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/cmd/core/ensure_image_test.go b/cmd/core/ensure_image_test.go index 44d39de6..5dde2b4d 100644 --- a/cmd/core/ensure_image_test.go +++ b/cmd/core/ensure_image_test.go @@ -98,9 +98,8 @@ func TestEnsureImage_ForceWhenDigestPinned(t *testing.T) { } } -// Issue 38 regression: a cloudimg vmCfg.Image without an http(s) scheme -// must not reach Pull — that would surface as `unsupported protocol scheme` -// from http.Get. The shape guard short-circuits with an actionable warning. +// A cloudimg ref without an http(s) scheme reaching Pull surfaces as +// `unsupported protocol scheme` from http.Get; the shape guard short-circuits. func TestEnsureImage_SkipsBadShape(t *testing.T) { tests := []struct { name string @@ -109,6 +108,7 @@ func TestEnsureImage_SkipsBadShape(t *testing.T) { }{ {"cloudimg bare OCI ref", "simular/win10:22h2-20260510", types.ImageTypeCloudImg}, {"cloudimg local name", "win11", types.ImageTypeCloudImg}, + {"cloudimg non-http scheme", "file:///foo.img", types.ImageTypeCloudImg}, {"oci malformed ref", "::bad::", types.ImageTypeOCI}, } for _, tt := range tests { @@ -124,6 +124,29 @@ func TestEnsureImage_SkipsBadShape(t *testing.T) { } } +// Acceptance counterpart: well-formed refs must reach Pull. +func TestEnsureImage_AcceptsGoodShape(t *testing.T) { + tests := []struct { + name string + image string + imageType string + }{ + {"cloudimg https url", "https://cloud-images.ubuntu.com/x.img", types.ImageTypeCloudImg}, + {"oci tagged ref", "ghcr.io/cocoonstack/cocoon/ubuntu:24.04", types.ImageTypeOCI}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &fakeImageBackend{typ: tt.imageType} + EnsureImage(t.Context(), []imagebackend.Images{f}, &types.VMConfig{ + Config: types.Config{Image: tt.image, ImageType: tt.imageType}, + }) + if len(f.pullRefs) != 1 || f.pullRefs[0] != tt.image { + t.Errorf("Pull = %v, want one call for %q", f.pullRefs, tt.image) + } + }) + } +} + func TestEnsureImage_SkipsPullWhenDigestLocal(t *testing.T) { const digest = "sha256:adafd938488daa114be898848eb24b9b0afffc21ac18f8b11f3f0057644b11e1" f := &fakeImageBackend{ diff --git a/cmd/core/helpers.go b/cmd/core/helpers.go index db431e9d..b707ad22 100644 --- a/cmd/core/helpers.go +++ b/cmd/core/helpers.go @@ -6,7 +6,6 @@ import ( "encoding/json" "errors" "fmt" - "net/url" "os" "strings" "text/tabwriter" @@ -497,17 +496,14 @@ func IsURL(ref string) bool { } // validateRefShape catches malformed base-image refs before they hit a backend -// that would return a misleading downstream error. Each backend expects a -// different ref form: cloudimg fetches over HTTP so the ref must be a URL -// with scheme; OCI pulls via container registry protocol so the ref must be -// parseable by name.ParseReference (registry/repo:tag or repo@digest). -// A bare OCI form leaking into the cloudimg path (the issue 38 failure mode) -// would otherwise surface as `unsupported protocol scheme ""` from http.Get. +// that would surface a misleading downstream error. cloudimg fetches over HTTP +// (ref must start with http:// or https://); OCI pulls via registry protocol +// (ref must parse via name.ParseReference). A bare OCI ref leaking into the +// cloudimg path would otherwise surface as `unsupported protocol scheme ""`. func validateRefShape(ref, imageType string) error { switch imageType { case types.ImageTypeCloudImg: - u, err := url.Parse(ref) - if err != nil || u.Scheme == "" { + if !IsURL(ref) { return fmt.Errorf("cloudimg ref %q is not an http(s) URL (imported or bare OCI ref?)", ref) } case types.ImageTypeOCI: