fix: golang-loader's image extraction (#471)

Fix golang-loader's image extraction logic.

---

We found this bug until today, in some ways, these means no one use
composer-lite for now. 😞

---------

Signed-off-by: wbpcode <wbphub@gmail.com>

Author: wbpcode

extensions/composer/goplugin-loader/imagefetcher/fetcher.go

@@ -37,6 +37,11 @@ type Option struct {
 // maxPluginSize is the upper bound for a plugin's size (512 MB).
 const maxPluginSize int64 = 512 * 1024 * 1024
 
+// errPluginBinaryNotFound is returned when a tar stream contains no .so file.
+// It is a sentinel so callers can distinguish "this layer has no plugin binary"
+// (keep looking in other layers) from a genuine extraction failure.
+var errPluginBinaryNotFound = errors.New(".so file not found in the archive")
+
 // FetchPlugin fetches a Go plugin binary from an OCI registry and returns the
 // local file path. The ref should be a plain OCI reference (e.g.
 // "registry.example.com/plugins/myplugin:v1") without an "oci://" scheme prefix.
@@ -127,8 +132,12 @@ func fetchPluginBinary(img v1.Image, destPath string) error {
 	return extractImageLayer(img, destPath, types.OCILayer)
 }
 
-// extractImageLayer extracts the plugin binary from the last layer of an image,
-// validating that the layer has the expected media type.
+// extractImageLayer extracts the plugin binary from an image. The .so is not
+// necessarily in the last layer: images built with multiple COPY instructions
+// produce one layer per instruction, and the plugin binary often lives in an
+// earlier layer than the packaged manifests. Layers are searched newest-first
+// so a later layer can override an earlier one, mirroring how an image's
+// filesystem is assembled.
 func extractImageLayer(img v1.Image, destPath string, expectedMediaType types.MediaType) error {
 	layers, err := img.Layers()
 	if err != nil {
@@ -138,15 +147,30 @@ func extractImageLayer(img v1.Image, destPath string, expectedMediaType types.Me
 		return errors.New("number of layers must be greater than zero")
 	}
 
-	layer := layers[len(layers)-1]
-	mt, err := layer.MediaType()
-	if err != nil {
-		return fmt.Errorf("failed to get media type: %w", err)
-	}
-	if mt != expectedMediaType {
-		return fmt.Errorf("invalid media type %s (expect %s)", mt, expectedMediaType)
+	for i := len(layers) - 1; i >= 0; i-- {
+		layer := layers[i]
+		mt, err := layer.MediaType()
+		if err != nil {
+			return fmt.Errorf("failed to get media type: %w", err)
+		}
+		if mt != expectedMediaType {
+			continue
+		}
+
+		if err := extractLayerBinary(layer, destPath); err != nil {
+			if errors.Is(err, errPluginBinaryNotFound) {
+				continue
+			}
+			return err
+		}
+		return nil
 	}
+	return errPluginBinaryNotFound
+}
 
+// extractLayerBinary opens a single layer's compressed tar.gz stream and
+// extracts the plugin binary from it.
+func extractLayerBinary(layer v1.Layer, destPath string) error {
 	r, err := layer.Compressed()
 	if err != nil {
 		return fmt.Errorf("failed to get layer content: %w", err)
@@ -217,5 +241,5 @@ func extractPluginBinary(r io.Reader, destPath string) error {
 		}
 		return nil
 	}
-	return fmt.Errorf(".so file not found in the archive")
+	return errPluginBinaryNotFound
 }

extensions/composer/goplugin-loader/imagefetcher/fetcher_test.go

@@ -99,7 +99,126 @@ func TestExtractImageLayer(t *testing.T) {
 				}
 			})
 
-			t.Run("invalid media type", func(t *testing.T) {
+			t.Run("plugin binary in earlier layer", func(t *testing.T) {
+				// Replicates an image built with multiple COPY instructions
+				// (Dockerfile.plugin): the plugin .so lands in an early layer,
+				// followed by manifest/schema layers that contain no .so. The
+				// last layer is therefore NOT the plugin layer.
+				exp := "this is plugin binary"
+				pluginLayer, err := newMockLayer(tt.mediaType, map[string][]byte{
+					"plugin.so": []byte(exp),
+				})
+				if err != nil {
+					t.Fatal(err)
+				}
+				manifestLayer, err := newMockLayer(tt.mediaType, map[string][]byte{
+					"manifest.yaml": []byte("name: example-go"),
+				})
+				if err != nil {
+					t.Fatal(err)
+				}
+				schemaLayer, err := newMockLayer(tt.mediaType, map[string][]byte{
+					"config.schema.json": []byte("{}"),
+				})
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				// Append in build order: plugin first, then the manifests.
+				img := empty.Image
+				for _, l := range []v1.Layer{pluginLayer, manifestLayer, schemaLayer} {
+					img, err = mutate.Append(img, mutate.Addendum{Layer: l})
+					if err != nil {
+						t.Fatal(err)
+					}
+				}
+
+				dest := filepath.Join(t.TempDir(), "plugin.so")
+				if extractImageLayerErr := extractImageLayer(img, dest, tt.mediaType); extractImageLayerErr != nil {
+					t.Fatalf("extractImageLayer failed: %v", extractImageLayerErr)
+				}
+				b, err := os.ReadFile(dest) //nolint:gosec // Test code reads from temp dir.
+				if err != nil {
+					t.Fatalf("failed to read written plugin: %v", err)
+				}
+				if string(b) != exp {
+					t.Fatalf("got %s, but want %s", string(b), exp)
+				}
+			})
+
+			t.Run("no plugin binary in any layer", func(t *testing.T) {
+				manifestLayer, err := newMockLayer(tt.mediaType, map[string][]byte{
+					"manifest.yaml": []byte("name: example-go"),
+				})
+				if err != nil {
+					t.Fatal(err)
+				}
+				schemaLayer, err := newMockLayer(tt.mediaType, map[string][]byte{
+					"config.schema.json": []byte("{}"),
+				})
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				img := empty.Image
+				for _, l := range []v1.Layer{manifestLayer, schemaLayer} {
+					img, err = mutate.Append(img, mutate.Addendum{Layer: l})
+					if err != nil {
+						t.Fatal(err)
+					}
+				}
+
+				dest := filepath.Join(t.TempDir(), "plugin.so")
+				err = extractImageLayer(img, dest, tt.mediaType)
+				if err == nil || !strings.Contains(err.Error(), "not found") {
+					t.Fatalf("extractImageLayer should fail with not found, got: %v", err)
+				}
+			})
+
+			t.Run("skips layer with unexpected media type", func(t *testing.T) {
+				// A layer whose media type does not match (e.g. a config layer)
+				// must be skipped, not treated as fatal, so the plugin in a
+				// matching layer is still found. Build order: plugin first, then
+				// a mismatched layer last (searched first).
+				exp := "this is plugin binary"
+				pluginLayer, err := newMockLayer(tt.mediaType, map[string][]byte{
+					"plugin.so": []byte(exp),
+				})
+				if err != nil {
+					t.Fatal(err)
+				}
+				mismatchedLayer, err := newMockLayer(types.DockerPluginConfig, map[string][]byte{
+					"config.json": []byte("{}"),
+				})
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				img := empty.Image
+				for _, l := range []v1.Layer{pluginLayer, mismatchedLayer} {
+					img, err = mutate.Append(img, mutate.Addendum{Layer: l})
+					if err != nil {
+						t.Fatal(err)
+					}
+				}
+
+				dest := filepath.Join(t.TempDir(), "plugin.so")
+				if extractImageLayerErr := extractImageLayer(img, dest, tt.mediaType); extractImageLayerErr != nil {
+					t.Fatalf("extractImageLayer failed: %v", extractImageLayerErr)
+				}
+				b, err := os.ReadFile(dest) //nolint:gosec // Test code reads from temp dir.
+				if err != nil {
+					t.Fatalf("failed to read written plugin: %v", err)
+				}
+				if string(b) != exp {
+					t.Fatalf("got %s, but want %s", string(b), exp)
+				}
+			})
+
+			t.Run("only unexpected media type layers", func(t *testing.T) {
+				// When no layer has the expected media type, the search finds no
+				// plugin binary and reports not found (rather than aborting on
+				// the first mismatch).
 				l, err := newMockLayer(types.DockerPluginConfig, nil)
 				if err != nil {
 					t.Fatal(err)
@@ -110,8 +229,8 @@ func TestExtractImageLayer(t *testing.T) {
 				}
 				dest := filepath.Join(t.TempDir(), "plugin.so")
 				err = extractImageLayer(img, dest, tt.mediaType)
-				if err == nil || !strings.Contains(err.Error(), "invalid media type") {
-					t.Fatal("extractImageLayer should fail due to invalid media type")
+				if err == nil || !strings.Contains(err.Error(), "not found") {
+					t.Fatalf("extractImageLayer should fail with not found, got: %v", err)
 				}
 			})
 		})