Fix: only recurse a symbolic link if it is a directory (#35455)

* only recurse a symbolic link if it is a directory

* added test for detecting valid plugins using lib dirs with symbolic links in them (like oracle)

* fix linting errors

* added extra checks as per code-review

pkg/plugins/manager/manager_test.go

@@ -21,6 +21,8 @@ import (
 	"gopkg.in/ini.v1"
 )
 
+const defaultAppURL = "http://localhost:3000/"
+
 func TestPluginManager_Init(t *testing.T) {
 	t.Run("Base case (core + bundled plugins)", func(t *testing.T) {
 		staticRootPath, err := filepath.Abs("../../../public")
@@ -289,7 +291,7 @@ func TestPluginManager_Init(t *testing.T) {
 			setting.AppUrl = origAppURL
 			setting.AppSubUrl = origAppSubURL
 		})
-		setting.AppUrl = "http://localhost:3000/"
+		setting.AppUrl = defaultAppURL
 		setting.AppSubUrl = "/grafana"
 
 		pm := createManager(t, func(pm *PluginManager) {
@@ -316,7 +318,7 @@ func TestPluginManager_Init(t *testing.T) {
 		t.Cleanup(func() {
 			setting.AppUrl = origAppURL
 		})
-		setting.AppUrl = "http://localhost:3000/"
+		setting.AppUrl = defaultAppURL
 
 		pm := createManager(t, func(pm *PluginManager) {
 			pm.Cfg.PluginsPath = "testdata/valid-v2-pvt-signature"
@@ -342,7 +344,7 @@ func TestPluginManager_Init(t *testing.T) {
 		t.Cleanup(func() {
 			setting.AppUrl = origAppURL
 		})
-		setting.AppUrl = "http://localhost:3000/"
+		setting.AppUrl = defaultAppURL
 
 		pm := createManager(t, func(pm *PluginManager) {
 			pm.Cfg.PluginsPath = "testdata/invalid-v2-signature"
@@ -358,7 +360,7 @@ func TestPluginManager_Init(t *testing.T) {
 		t.Cleanup(func() {
 			setting.AppUrl = origAppURL
 		})
-		setting.AppUrl = "http://localhost:3000/"
+		setting.AppUrl = defaultAppURL
 
 		pm := createManager(t, func(pm *PluginManager) {
 			pm.Cfg.PluginsPath = "testdata/invalid-v2-signature-2"
@@ -368,6 +370,33 @@ func TestPluginManager_Init(t *testing.T) {
 		assert.Equal(t, []error{fmt.Errorf(`plugin 'test' has a modified signature`)}, pm.scanningErrors)
 		assert.Nil(t, pm.plugins[("test")])
 	})
+
+	t.Run("With back-end plugin with a lib dir that has symbolic links", func(t *testing.T) {
+		origAppURL := setting.AppUrl
+		t.Cleanup(func() {
+			setting.AppUrl = origAppURL
+		})
+		setting.AppUrl = defaultAppURL
+
+		pm := createManager(t, func(pm *PluginManager) {
+			pm.Cfg.PluginsPath = "testdata/symbolic-file-links"
+		})
+		err := pm.Init()
+		require.NoError(t, err)
+		// This plugin should be properly registered, even though it has a symbolicly linked file in it.
+		require.Empty(t, pm.scanningErrors)
+		const pluginID = "test"
+		assert.NotNil(t, pm.plugins[pluginID])
+		assert.Equal(t, "datasource", pm.plugins[pluginID].Type)
+		assert.Equal(t, "Test", pm.plugins[pluginID].Name)
+		assert.Equal(t, pluginID, pm.plugins[pluginID].Id)
+		assert.Equal(t, "1.0.0", pm.plugins[pluginID].Info.Version)
+		assert.Equal(t, plugins.PluginSignatureValid, pm.plugins[pluginID].Signature)
+		assert.Equal(t, plugins.GrafanaType, pm.plugins[pluginID].SignatureType)
+		assert.Equal(t, "Grafana Labs", pm.plugins[pluginID].SignatureOrg)
+		assert.False(t, pm.plugins[pluginID].IsCorePlugin)
+		assert.NotNil(t, pm.plugins[("test")])
+	})
 }
 
 func TestPluginManager_IsBackendOnlyPlugin(t *testing.T) {

pkg/plugins/manager/testdata/symbolic-file-links/plugin/MANIFEST.txt

@@ -0,0 +1,32 @@
+
+-----BEGIN PGP SIGNED MESSAGE-----
+Hash: SHA512
+
+{
+  "manifestVersion": "2.0.0",
+  "signatureType": "grafana",
+  "signedByOrg": "grafana",
+  "signedByOrgName": "Grafana Labs",
+  "rootUrls": [
+    "http://localhost:3000/"
+  ],
+  "plugin": "test",
+  "version": "1.0.0",
+  "time": 1623327375936,
+  "keyId": "7e4d0c6a708866e7",
+  "files": {
+    "plugin.json": "2bb467c0bfd6c454551419efe475b8bf8573734e73c7bab52b14842adb62886f",
+    "lib/somelib.so.1": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+    "lib/somelib.so": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
+  }
+}
+-----BEGIN PGP SIGNATURE-----
+Version: OpenPGP.js v4.10.1
+Comment: https://openpgpjs.org
+
+wqEEARMKAAYFAmDCApAACgkQfk0ManCIZuc+JAIJAVpL9/0Q1vnQyB+0MaOJ
+oklt6Kw7/8OyL0oOj3lG0eW3qkJSOUfdM7Si2VW/BudruyUQovpU3EXr00/A
+oR1SZtoLAgdNyc9KldvPNV95XAsNz8jjBzn12G2Qer4/Vgjzpl5wvn2WvZ1l
+/NFR8rwHVhvMyGe7c1PJ6Gt6KpbQZ5j20j1NMA==
+=GIHH
+-----END PGP SIGNATURE-----

pkg/plugins/manager/testdata/symbolic-file-links/plugin/lib/somelib.so

@@ -0,0 +1 @@
+somelib.so.1

pkg/plugins/manager/testdata/symbolic-file-links/plugin/plugin.json

@@ -0,0 +1,16 @@
+{
+  "type": "datasource",
+  "name": "Test",
+  "id": "test",
+  "backend": true,
+  "executable": "test",
+  "state": "alpha",
+  "info": {
+    "version": "1.0.0",
+    "description": "Test",
+    "author": {
+      "name": "Will Browne",
+      "url": "https://willbrowne.com"
+    }
+  }
+}

pkg/util/filepath.go

@@ -55,26 +55,28 @@ func walk(path string, info os.FileInfo, resolvedPath string, symlinkPathsFollow
 		}
 		return err
 	}
-	if resolvedPath != "" && info.Mode()&os.ModeSymlink == os.ModeSymlink {
-		path2, err := os.Readlink(resolvedPath)
-		if err != nil {
-			return err
-		}
-		// vout("SymLink Path: %v, links to: %v", resolvedPath, path2)
-		if symlinkPathsFollowed != nil {
-			if _, ok := symlinkPathsFollowed[path2]; ok {
-				errMsg := "potential symLink infinite loop, path: %v, link to: %v"
-				return fmt.Errorf(errMsg, resolvedPath, path2)
-			}
-			symlinkPathsFollowed[path2] = true
-		}
-		info2, err := os.Lstat(path2)
-		if err != nil {
-			return err
-		}
-		return walk(path, info2, path2, symlinkPathsFollowed, walkFn)
-	}
+	// We only want to lstat on directories. If this entry is a symbolic link to a file, no need to recurse.
 	if info.IsDir() {
+		if resolvedPath != "" && info.Mode()&os.ModeSymlink == os.ModeSymlink {
+			path2, err := os.Readlink(resolvedPath)
+			if err != nil {
+				return err
+			}
+			// vout("SymLink Path: %v, links to: %v", resolvedPath, path2)
+			if symlinkPathsFollowed != nil {
+				if _, ok := symlinkPathsFollowed[path2]; ok {
+					errMsg := "potential symLink infinite loop, path: %v, link to: %v"
+					return fmt.Errorf(errMsg, resolvedPath, path2)
+				}
+				symlinkPathsFollowed[path2] = true
+			}
+			info2, err := os.Lstat(path2)
+			if err != nil {
+				return err
+			}
+			return walk(path, info2, path2, symlinkPathsFollowed, walkFn)
+		}
+
 		list, err := ioutil.ReadDir(path)
 		if err != nil {
 			return walkFn(resolvedPath, info, err)