diff --git a/internal/plugin/install.go b/internal/plugin/install.go index b8810772..2bf04bb2 100644 --- 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 } } diff --git a/internal/plugin/install_test.go b/internal/plugin/install_test.go index 7b4d4253..e783ca97 100644 --- 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 {