Add support for plugin binary artifacts

Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
2026-05-03 02:03:31 +00:00 · 2026-04-09 00:05:47 +03:00
parent 8be056324a
commit 9a4b93056b
2 changed files with 480 additions and 14 deletions
--- a/internal/plugin/install.go
+++ b/internal/plugin/install.go
@@ -77,21 +77,32 @@ func (inst *Installer) Install(pluginDir string, manifest *PluginManifest, pv *P
 		return fmt.Errorf("checksum verification failed (expected: %s, got: %s)", plat.Checksum, actualChecksum)
 	}

+	// manifest.Bin is the single source of truth for the installed binary
+	// name (e.g. "flux-validate"). On Windows we always append ".exe".
+	// For archives it's also the entry name we look up; for raw binaries
+	// it's the rename target regardless of the URL's filename.
 	binName := manifest.Bin
 	if runtime.GOOS == "windows" {
 		binName += ".exe"
 	}
+	destPath := filepath.Join(pluginDir, binName)

-	destName := pluginPrefix + manifest.Name
-	if runtime.GOOS == "windows" {
-		destName += ".exe"
+	format, err := detectArchiveFormat(tmpFile.Name(), plat.URL)
+	if err != nil {
+		return fmt.Errorf("failed to detect plugin format: %w", err)
 	}
-	destPath := filepath.Join(pluginDir, destName)

-	if strings.HasSuffix(plat.URL, ".zip") {
+	switch format {
+	case formatZip:
 		err = extractFromZip(tmpFile.Name(), binName, destPath)
-	} else {
+	case formatTarGz:
 		err = extractFromTarGz(tmpFile.Name(), binName, destPath)
+	case formatTar:
+		err = extractFromTar(tmpFile.Name(), binName, destPath)
+	case formatBinary:
+		err = copyPluginBinary(tmpFile.Name(), destPath)
+	default:
+		return fmt.Errorf("unexpected plugin format: %v", format)
 	}
 	if err != nil {
 		return err
@@ -162,6 +173,60 @@ func writeReceipt(pluginDir, name string, receipt *Receipt) error {
 	return os.WriteFile(receiptPath(pluginDir, name), data, 0o644)
 }

+// archiveFormat is the detected format of a downloaded plugin artifact.
+type archiveFormat int
+
+const (
+	formatBinary archiveFormat = iota
+	formatZip
+	formatTarGz
+	formatTar
+)
+
+// detectArchiveFormat determines the artifact format by first checking the URL
+// extension, then falling back to magic-byte inspection. Returns formatBinary
+// if neither indicates a known archive, in which case the downloaded file is
+// installed as-is.
+func detectArchiveFormat(path, url string) (archiveFormat, error) {
+	switch lower := strings.ToLower(url); {
+	case strings.HasSuffix(lower, ".zip"):
+		return formatZip, nil
+	case strings.HasSuffix(lower, ".tar.gz"), strings.HasSuffix(lower, ".tgz"):
+		return formatTarGz, nil
+	case strings.HasSuffix(lower, ".tar"):
+		return formatTar, nil
+	}
+
+	f, err := os.Open(path)
+	if err != nil {
+		return formatBinary, err
+	}
+	defer f.Close()
+
+	// Read enough bytes to cover the tar "ustar" magic at offset 257.
+	var hdr [262]byte
+	n, err := io.ReadFull(f, hdr[:])
+	if err != nil && err != io.ErrUnexpectedEOF && err != io.EOF {
+		return formatBinary, err
+	}
+
+	// ZIP: PK\x03\x04 (file), PK\x05\x06 (empty), PK\x07\x08 (spanned).
+	if n >= 4 && hdr[0] == 'P' && hdr[1] == 'K' &&
+		(hdr[2] == 0x03 || hdr[2] == 0x05 || hdr[2] == 0x07) {
+		return formatZip, nil
+	}
+	// gzip: \x1f\x8b
+	if n >= 2 && hdr[0] == 0x1f && hdr[1] == 0x8b {
+		return formatTarGz, nil
+	}
+	// tar: "ustar" at offset 257
+	if n >= 262 && string(hdr[257:262]) == "ustar" {
+		return formatTar, nil
+	}
+
+	return formatBinary, nil
+}
+
 // extractFromTarGz extracts a named file from a tar.gz archive
 // and streams it directly to destPath.
 func extractFromTarGz(archivePath, targetName, destPath string) error {
@@ -177,7 +242,26 @@ func extractFromTarGz(archivePath, targetName, destPath string) error {
 	}
 	defer gr.Close()

-	tr := tar.NewReader(gr)
+	return extractTarStream(gr, targetName, destPath)
+}
+
+// extractFromTar extracts a named file from an uncompressed tar archive
+// and streams it directly to destPath.
+func extractFromTar(archivePath, targetName, destPath string) error {
+	f, err := os.Open(archivePath)
+	if err != nil {
+		return err
+	}
+	defer f.Close()
+
+	return extractTarStream(f, targetName, destPath)
+}
+
+// extractTarStream walks a tar stream and streams the first matching
+// regular file to destPath. Non-regular entries (symlinks, devices,
+// directories) and entries with unsafe paths are skipped.
+func extractTarStream(r io.Reader, targetName, destPath string) error {
+	tr := tar.NewReader(r)
 	for {
 		header, err := tr.Next()
 		if err == io.EOF {
@@ -187,10 +271,13 @@ func extractFromTarGz(archivePath, targetName, destPath string) error {
 			return fmt.Errorf("failed to read tar: %w", err)
 		}

-		if filepath.IsAbs(header.Name) || strings.Contains(header.Name, "..") {
+		if !filepath.IsLocal(header.Name) {
 			continue
 		}
-		if filepath.Base(header.Name) == targetName && header.Typeflag == tar.TypeReg {
+		if !header.FileInfo().Mode().IsRegular() {
+			continue
+		}
+		if filepath.Base(header.Name) == targetName {
 			return writeStreamToFile(tr, destPath)
 		}
 	}
@@ -198,8 +285,21 @@ func extractFromTarGz(archivePath, targetName, destPath string) error {
 	return fmt.Errorf("binary %q not found in archive", targetName)
 }

-// extractFromZip extracts a named file from a zip archive
-// and streams it directly to destPath.
+// copyPluginBinary copies a raw downloaded binary to destPath with 0755 mode.
+// Used when the downloaded artifact is not an archive.
+func copyPluginBinary(srcPath, destPath string) error {
+	src, err := os.Open(srcPath)
+	if err != nil {
+		return fmt.Errorf("failed to open downloaded binary: %w", err)
+	}
+	defer src.Close()
+
+	return writeStreamToFile(src, destPath)
+}
+
+// extractFromZip extracts a named file from a zip archive and streams it
+// directly to destPath. Non-regular entries (symlinks, devices, directories)
+// and entries with unsafe paths are skipped.
 func extractFromZip(archivePath, targetName, destPath string) error {
 	r, err := zip.OpenReader(archivePath)
 	if err != nil {
@@ -208,13 +308,20 @@ func extractFromZip(archivePath, targetName, destPath string) error {
 	defer r.Close()

 	for _, f := range r.File {
-		if filepath.Base(f.Name) == targetName && !f.FileInfo().IsDir() {
+		if !filepath.IsLocal(f.Name) {
+			continue
+		}
+		if !f.FileInfo().Mode().IsRegular() {
+			continue
+		}
+		if filepath.Base(f.Name) == targetName {
 			rc, err := f.Open()
 			if err != nil {
 				return fmt.Errorf("failed to open %q in zip: %w", targetName, err)
 			}
-			defer rc.Close()
-			return writeStreamToFile(rc, destPath)
+			err = writeStreamToFile(rc, destPath)
+			rc.Close()
+			return err
 		}
 	}

--- a/internal/plugin/install_test.go
+++ b/internal/plugin/install_test.go
@@ -18,10 +18,12 @@ package plugin

 import (
 	"archive/tar"
+	"archive/zip"
 	"bytes"
 	"compress/gzip"
 	"crypto/sha256"
 	"fmt"
+	"io/fs"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -55,6 +57,97 @@ func createTestTarGz(name string, content []byte) ([]byte, error) {
 	return buf.Bytes(), nil
 }

+// createTestTar creates an uncompressed tar archive containing a single file.
+func createTestTar(name string, content []byte) ([]byte, error) {
+	var buf bytes.Buffer
+	tw := tar.NewWriter(&buf)
+
+	hdr := &tar.Header{
+		Name: name,
+		Mode: 0o755,
+		Size: int64(len(content)),
+	}
+	if err := tw.WriteHeader(hdr); err != nil {
+		return nil, err
+	}
+	if _, err := tw.Write(content); err != nil {
+		return nil, err
+	}
+
+	tw.Close()
+	return buf.Bytes(), nil
+}
+
+// tarEntry describes a single entry for createTestTarGzMulti.
+type tarEntry struct {
+	header  tar.Header
+	content []byte
+}
+
+// createTestTarGzMulti creates a tar.gz archive with arbitrary entries.
+// Used to test rejection of unsafe or non-regular entries.
+func createTestTarGzMulti(entries []tarEntry) ([]byte, error) {
+	var buf bytes.Buffer
+	gw := gzip.NewWriter(&buf)
+	tw := tar.NewWriter(gw)
+
+	for _, e := range entries {
+		hdr := e.header
+		hdr.Size = int64(len(e.content))
+		if err := tw.WriteHeader(&hdr); err != nil {
+			return nil, err
+		}
+		if len(e.content) > 0 {
+			if _, err := tw.Write(e.content); err != nil {
+				return nil, err
+			}
+		}
+	}
+
+	tw.Close()
+	gw.Close()
+	return buf.Bytes(), nil
+}
+
+// zipEntry describes a single entry for createTestZip.
+type zipEntry struct {
+	name    string
+	mode    fs.FileMode
+	content []byte
+}
+
+// createTestZip creates a zip archive with arbitrary entries. Entries may
+// carry Unix mode bits (e.g. os.ModeSymlink) to exercise non-regular files.
+func createTestZip(entries []zipEntry) ([]byte, error) {
+	var buf bytes.Buffer
+	zw := zip.NewWriter(&buf)
+
+	for _, e := range entries {
+		hdr := &zip.FileHeader{
+			Name:   e.name,
+			Method: zip.Deflate,
+		}
+		mode := e.mode
+		if mode == 0 {
+			mode = 0o755
+		}
+		hdr.SetMode(mode)
+
+		w, err := zw.CreateHeader(hdr)
+		if err != nil {
+			return nil, err
+		}
+		if _, err := w.Write(e.content); err != nil {
+			return nil, err
+		}
+	}
+
+	if err := zw.Close(); err != nil {
+		return nil, err
+	}
+	return buf.Bytes(), nil
+}
+
 func TestInstall(t *testing.T) {
 	binaryContent := []byte("#!/bin/sh\necho hello")
 	archive, err := createTestTarGz("flux-operator", binaryContent)
@@ -291,6 +384,126 @@ platform:
 	})
 }

+func TestInstallRawBinary(t *testing.T) {
+	// Bytes that don't match zip/gzip/tar magic — treated as a raw binary.
+	binaryContent := []byte("#!/bin/sh\necho raw plugin")
+	checksum := fmt.Sprintf("sha256:%x", sha256.Sum256(binaryContent))
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Write(binaryContent)
+	}))
+	defer server.Close()
+
+	pluginDir := t.TempDir()
+
+	manifest := &PluginManifest{
+		Name: "validate",
+		Bin:  "flux-validate",
+	}
+	pv := &PluginVersion{Version: "1.2.3"}
+	plat := &PluginPlatform{
+		OS:   runtime.GOOS,
+		Arch: runtime.GOARCH,
+		// URL filename deliberately differs from manifest.Bin — mimics a
+		// typical GitHub release asset that includes platform/version in
+		// the name. The installer must rename to manifest.Bin.
+		URL:      server.URL + "/download/flux-validate-" + runtime.GOARCH + "-v1.2.3",
+		Checksum: checksum,
+	}
+
+	installer := &Installer{HTTPClient: server.Client()}
+	if err := installer.Install(pluginDir, manifest, pv, plat); err != nil {
+		t.Fatalf("install failed: %v", err)
+	}
+
+	// The installed file must be named exactly manifest.Bin (+ .exe on Windows),
+	// regardless of what the URL path looked like.
+	wantName := "flux-validate"
+	if runtime.GOOS == "windows" {
+		wantName += ".exe"
+	}
+	binPath := filepath.Join(pluginDir, wantName)
+	data, err := os.ReadFile(binPath)
+	if err != nil {
+		t.Fatalf("binary not found at %s: %v", binPath, err)
+	}
+	if !bytes.Equal(data, binaryContent) {
+		t.Errorf("binary content mismatch: got %q, want %q", data, binaryContent)
+	}
+
+	// Nothing should have been written under the URL-derived name.
+	urlDerived := filepath.Join(pluginDir, "flux-validate-"+runtime.GOARCH+"-v1.2.3")
+	if _, err := os.Stat(urlDerived); !os.IsNotExist(err) {
+		t.Errorf("unexpected file at URL-derived path %s", urlDerived)
+	}
+
+	if runtime.GOOS != "windows" {
+		info, err := os.Stat(binPath)
+		if err != nil {
+			t.Fatalf("stat: %v", err)
+		}
+		if info.Mode()&0o111 == 0 {
+			t.Errorf("binary is not executable: mode %v", info.Mode())
+		}
+	}
+
+	// Raw binary install must still produce a receipt.
+	if receipt := ReadReceipt(pluginDir, "validate"); receipt == nil {
+		t.Fatal("receipt not found")
+	}
+}
+
+func TestDetectArchiveFormat(t *testing.T) {
+	tarGz, err := createTestTarGz("bin", []byte("content"))
+	if err != nil {
+		t.Fatalf("createTestTarGz: %v", err)
+	}
+	plainTar, err := createTestTar("bin", []byte("content"))
+	if err != nil {
+		t.Fatalf("createTestTar: %v", err)
+	}
+
+	tests := []struct {
+		name    string
+		url     string
+		content []byte
+		want    archiveFormat
+	}{
+		// Extension-based detection takes precedence over content.
+		{"zip extension", "https://example.com/plugin.zip", []byte("ignored"), formatZip},
+		{"tar.gz extension", "https://example.com/plugin.tar.gz", []byte("ignored"), formatTarGz},
+		{"tgz extension", "https://example.com/plugin.tgz", []byte("ignored"), formatTarGz},
+		{"tar extension", "https://example.com/plugin.tar", []byte("ignored"), formatTar},
+		{"uppercase extension", "https://example.com/PLUGIN.ZIP", []byte("ignored"), formatZip},
+
+		// Magic-byte detection when extension is absent or unrecognized.
+		{"zip magic no extension", "https://example.com/download", []byte{'P', 'K', 0x03, 0x04, 0, 0, 0, 0}, formatZip},
+		{"gzip magic no extension", "https://example.com/download", tarGz, formatTarGz},
+		{"tar magic no extension", "https://example.com/download", plainTar, formatTar},
+
+		// Fallback to raw binary.
+		{"unknown content", "https://example.com/download", []byte("#!/bin/sh\necho hi"), formatBinary},
+		{"short file", "https://example.com/download", []byte("ab"), formatBinary},
+		{"empty file", "https://example.com/download", []byte{}, formatBinary},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			tmp := filepath.Join(t.TempDir(), "artifact")
+			if err := os.WriteFile(tmp, tc.content, 0o644); err != nil {
+				t.Fatal(err)
+			}
+			got, err := detectArchiveFormat(tmp, tc.url)
+			if err != nil {
+				t.Fatalf("detectArchiveFormat: %v", err)
+			}
+			if got != tc.want {
+				t.Errorf("got %v, want %v", got, tc.want)
+			}
+		})
+	}
+}
+
 func TestExtractFromTarGz(t *testing.T) {
 	content := []byte("test binary content")
 	archive, err := createTestTarGz("flux-operator", content)
@@ -314,6 +527,152 @@ func TestExtractFromTarGz(t *testing.T) {
 	}
 }

+func TestExtractFromTarGzRejectsUnsafeEntries(t *testing.T) {
+	content := []byte("legit content")
+
+	// Archive contains, in order:
+	//  1. A symlink whose basename matches the target (must be skipped).
+	//  2. A regular entry with ".." in the path (must be skipped).
+	//  3. An absolute-path entry (must be skipped).
+	//  4. A legitimate regular file that must be extracted.
+	entries := []tarEntry{
+		{
+			header: tar.Header{
+				Name:     "flux-operator",
+				Typeflag: tar.TypeSymlink,
+				Linkname: "/etc/passwd",
+				Mode:     0o777,
+			},
+		},
+		{
+			header: tar.Header{
+				Name:     "../flux-operator",
+				Typeflag: tar.TypeReg,
+				Mode:     0o755,
+			},
+			content: []byte("malicious"),
+		},
+		{
+			header: tar.Header{
+				Name:     "/absolute/flux-operator",
+				Typeflag: tar.TypeReg,
+				Mode:     0o755,
+			},
+			content: []byte("malicious"),
+		},
+		{
+			header: tar.Header{
+				Name:     "bin/flux-operator",
+				Typeflag: tar.TypeReg,
+				Mode:     0o755,
+			},
+			content: content,
+		},
+	}
+
+	archive, err := createTestTarGzMulti(entries)
+	if err != nil {
+		t.Fatalf("createTestTarGzMulti: %v", err)
+	}
+
+	tmpFile := filepath.Join(t.TempDir(), "test.tar.gz")
+	os.WriteFile(tmpFile, archive, 0o644)
+
+	destPath := filepath.Join(t.TempDir(), "flux-operator")
+	if err := extractFromTarGz(tmpFile, "flux-operator", destPath); err != nil {
+		t.Fatalf("extract failed: %v", err)
+	}
+
+	data, err := os.ReadFile(destPath)
+	if err != nil {
+		t.Fatalf("failed to read extracted file: %v", err)
+	}
+	if !bytes.Equal(data, content) {
+		t.Errorf("extracted content mismatch: got %q, want %q", data, content)
+	}
+}
+
+func TestExtractFromZip(t *testing.T) {
+	content := []byte("test binary content")
+	archive, err := createTestZip([]zipEntry{
+		{name: "flux-operator", content: content},
+	})
+	if err != nil {
+		t.Fatalf("createTestZip: %v", err)
+	}
+
+	tmpFile := filepath.Join(t.TempDir(), "test.zip")
+	os.WriteFile(tmpFile, archive, 0o644)
+
+	destPath := filepath.Join(t.TempDir(), "flux-operator")
+	if err := extractFromZip(tmpFile, "flux-operator", destPath); err != nil {
+		t.Fatalf("extract failed: %v", err)
+	}
+
+	data, err := os.ReadFile(destPath)
+	if err != nil {
+		t.Fatalf("failed to read extracted file: %v", err)
+	}
+	if !bytes.Equal(data, content) {
+		t.Errorf("content mismatch: got %q, want %q", data, content)
+	}
+}
+
+func TestExtractFromZipRejectsUnsafeEntries(t *testing.T) {
+	content := []byte("legit content")
+
+	// Archive contains, in order:
+	//  1. A symlink whose basename matches the target (must be skipped).
+	//  2. An entry with ".." in the path (must be skipped).
+	//  3. An absolute-path entry (must be skipped).
+	//  4. A directory entry whose basename matches (must be skipped).
+	//  5. A legitimate regular file that must be extracted.
+	entries := []zipEntry{
+		{
+			name:    "flux-operator",
+			mode:    fs.ModeSymlink | 0o777,
+			content: []byte("/etc/passwd"),
+		},
+		{
+			name:    "../flux-operator",
+			content: []byte("malicious"),
+		},
+		{
+			name:    "/absolute/flux-operator",
+			content: []byte("malicious"),
+		},
+		{
+			name: "flux-operator/",
+			mode: fs.ModeDir | 0o755,
+		},
+		{
+			name:    "bin/flux-operator",
+			content: content,
+		},
+	}
+
+	archive, err := createTestZip(entries)
+	if err != nil {
+		t.Fatalf("createTestZip: %v", err)
+	}
+
+	tmpFile := filepath.Join(t.TempDir(), "test.zip")
+	os.WriteFile(tmpFile, archive, 0o644)
+
+	destPath := filepath.Join(t.TempDir(), "flux-operator")
+	if err := extractFromZip(tmpFile, "flux-operator", destPath); err != nil {
+		t.Fatalf("extract failed: %v", err)
+	}
+
+	data, err := os.ReadFile(destPath)
+	if err != nil {
+		t.Fatalf("failed to read extracted file: %v", err)
+	}
+	if !bytes.Equal(data, content) {
+		t.Errorf("extracted content mismatch: got %q, want %q", data, content)
+	}
+}
+
 func TestExtractFromTarGzNotFound(t *testing.T) {
 	archive, err := createTestTarGz("other-binary", []byte("content"))
 	if err != nil {