Plugins: Fix plugin signature calculation not working on Windows (#66273)

* Plugins: Fix plugin signature calculation not working on Windows

* Plugins: Added test for NTFS path separators in signature verification

* Use filepath.ToSlash and replace its implementation in tests

* Fix typo

pkg/plugins/manager/signature/manifest.go

@@ -12,6 +12,7 @@ import (
 	"net/url"
 	"os"
 	"path"
+	"path/filepath"
 	"runtime"
 	"strings"
 
@@ -55,7 +56,12 @@ N1c5v9v/4h6qeA==
 -----END PGP PUBLIC KEY BLOCK-----
 `
 
-var runningWindows = runtime.GOOS == "windows"
+var (
+	runningWindows = runtime.GOOS == "windows"
+
+	// toSlash is filepath.ToSlash, but can be overwritten in tests path separators cross-platform
+	toSlash = filepath.ToSlash
+)
 
 // PluginManifest holds details for the file manifest
 type PluginManifest struct {
@@ -194,6 +200,9 @@ func Calculate(ctx context.Context, mlog log.Logger, src plugins.PluginSource, p
 	// Track files missing from the manifest
 	var unsignedFiles []string
 	for _, f := range plugin.FS.Files() {
+		// Ensure slashes are used, because MANIFEST.txt always uses slashes regardless of the filesystem
+		f = toSlash(f)
+
 		// Ignoring unsigned Chromium debug.log so it doesn't invalidate the signature for Renderer plugin running on Windows
 		if runningWindows && plugin.JSONData.Type == plugins.Renderer && f == "chrome-win/debug.log" {
 			continue

pkg/plugins/manager/signature/manifest_test.go

@@ -210,6 +210,136 @@ func TestCalculate(t *testing.T) {
 			SigningOrg: "Grafana Labs",
 		}, sig)
 	})
+
+	t.Run("Signature verification should work with any path separator", func(t *testing.T) {
+		var toSlashUnix = newToSlash('/')
+		var toSlashWindows = newToSlash('\\')
+
+		for _, tc := range []struct {
+			name    string
+			sep     string
+			toSlash func(string) string
+		}{
+			{"unix", "/", toSlashUnix},
+			{"windows", "\\", toSlashWindows},
+		} {
+			t.Run(tc.name, func(t *testing.T) {
+				// Replace toSlash for cross-platform testing
+				oldToSlash := toSlash
+				t.Cleanup(func() {
+					toSlash = oldToSlash
+				})
+				toSlash = tc.toSlash
+
+				basePath := "../testdata/app-with-child/dist"
+
+				sig, err := Calculate(context.Background(), log.NewTestLogger(), &fakes.FakePluginSource{
+					PluginClassFunc: func(ctx context.Context) plugins.Class {
+						return plugins.External
+					},
+				}, plugins.FoundPlugin{
+					JSONData: plugins.JSONData{
+						ID:   "myorgid-simple-app",
+						Type: plugins.App,
+						Info: plugins.Info{
+							Version: "%VERSION%",
+						},
+					},
+					FS: newPathSeparatorOverrideFS(tc.sep, map[string]struct{}{
+						filepath.Join(basePath, "MANIFEST.txt"):      {},
+						filepath.Join(basePath, "plugin.json"):       {},
+						filepath.Join(basePath, "child/plugin.json"): {},
+					}, basePath),
+				})
+				require.NoError(t, err)
+				require.Equal(t, plugins.Signature{
+					Status:     plugins.SignatureValid,
+					Type:       plugins.GrafanaSignature,
+					SigningOrg: "Grafana Labs",
+				}, sig)
+			})
+		}
+	})
+}
+
+// newToSlash returns a new function that acts as filepath.ToSlash but for the specified os-separator.
+// This can be used to test filepath.ToSlash-dependant code cross-platform.
+func newToSlash(sep rune) func(string) string {
+	return func(path string) string {
+		if sep == '/' {
+			return path
+		}
+		return strings.ReplaceAll(path, string(sep), "/")
+	}
+}
+
+func TestNewToSlash(t *testing.T) {
+	t.Run("unix", func(t *testing.T) {
+		toSlashUnix := newToSlash('/')
+		require.Equal(t, "folder", toSlashUnix("folder"))
+		require.Equal(t, "/folder", toSlashUnix("/folder"))
+		require.Equal(t, "/folder/file", toSlashUnix("/folder/file"))
+		require.Equal(t, "/folder/other\\file", toSlashUnix("/folder/other\\file"))
+	})
+
+	t.Run("windows", func(t *testing.T) {
+		toSlashWindows := newToSlash('\\')
+		require.Equal(t, "folder", toSlashWindows("folder"))
+		require.Equal(t, "C:/folder", toSlashWindows("C:\\folder"))
+		require.Equal(t, "folder/file.exe", toSlashWindows("folder\\file.exe"))
+	})
+}
+
+// fsPathSeparatorFiles embeds plugins.LocalFS and overrides the Files() behaviour so all the returned elements
+// have the specified path separator. This can be used to test Files() behaviour cross-platform.
+type fsPathSeparatorFiles struct {
+	plugins.LocalFS
+
+	separator string
+}
+
+// newPathSeparatorOverrideFS returns a new fsPathSeparatorFiles. Sep is the separator that will be used ONLY for
+// the elements returned by Files(). Files and basePath MUST use the os-specific path separator (filepath.Separator)
+// if Open() is required to work for the test case.
+func newPathSeparatorOverrideFS(sep string, files map[string]struct{}, basePath string) fsPathSeparatorFiles {
+	return fsPathSeparatorFiles{
+		LocalFS:   plugins.NewLocalFS(files, basePath),
+		separator: sep,
+	}
+}
+
+// Files returns LocalFS.Files(), but all path separators (filepath.Separator) are replaced with f.separator.
+func (f fsPathSeparatorFiles) Files() []string {
+	files := f.LocalFS.Files()
+	const osSepStr = string(filepath.Separator)
+	for i := 0; i < len(files); i++ {
+		files[i] = strings.ReplaceAll(files[i], osSepStr, f.separator)
+	}
+	return files
+}
+
+func TestFSPathSeparatorFiles(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		sep  string
+	}{
+		{"unix", "/"},
+		{"windows", "\\"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			fs := newPathSeparatorOverrideFS("/", map[string]struct{}{
+				"a": {},
+				strings.Join([]string{"a", "b", "c"}, tc.sep): {},
+			}, ".")
+			files := fs.Files()
+			filesMap := make(map[string]struct{}, len(files))
+			// Re-convert to map as the key order is not stable
+			for _, f := range files {
+				filesMap[f] = struct{}{}
+			}
+			require.Equal(t, filesMap, map[string]struct{}{"a": {}, strings.Join([]string{"a", "b", "c"}, tc.sep): {}})
+		})
+	}
 }
 
 func fileList(manifest *PluginManifest) []string {