public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Plugins: Fs: Add option to access unallowed files in dev mode (#66492)

Browse Source 
* Plugins: Fs: Add option to access unallowed files in dev mode

* Plugins: Fs: allow accessing unallowed files only when in dev mode

* Plugins: Fs: Add ProvideLocalFinder

* Plugins: FS: Pass whole config in NewLocalFinder()

* Plugins: FS: Add AllowListLocalFS

* Plugins: FS: Fix some tests

* Plugins: FS: Update tests

* Plugins: FS: Removed dead code

* Plugins: FS: Add tests for AllowListFS

* Plugins: FS: Update comments

* Plugins: FS: Use variadic arguments for allow list rather than map

* Plugins: FS: Remove unnecessary log

* Plugins: FS: Do not escape plugin root dir

* Fix merge conflict

* Plugins: FS: Update comments

* Plugins: FS: PR review changes

* Fix merge conflict

* Fix tests

* Cleanup

* Fix flaky test

* Changes from PR review

* Lint

* Add comment to LocalFS.Remove

* Fix Windows

* Renamed devMode to production
This commit is contained in:
Giuseppe Guerra
2023-04-27 10:26:15 +02:00
committed by GitHub
parent dfc99cdd19
commit bfe1b68800
14 changed files with 576 additions and 366 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/plugins/manager/signature/manifest.go
+12 -3
View File
@@ -17,6 +17,7 @@ import (
"github.com/ProtonMail/go-crypto/openpgp/clearsign"
"github.com/gobwas/glob"
"github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/plugins/config"
"github.com/grafana/grafana/pkg/plugins/log"
@@ -29,6 +30,9 @@ var (
// toSlash is filepath.ToSlash, but can be overwritten in tests path separators cross-platform
toSlash = filepath.ToSlash
// fromSlash is filepath.FromSlash, but can be overwritten in tests path separators cross-platform
fromSlash = filepath.FromSlash
)
// PluginManifest holds details for the file manifest
@@ -100,8 +104,11 @@ func (s *Signature) Calculate(ctx context.Context, src plugins.PluginSource, plu
if defaultSignature, exists := src.DefaultSignature(ctx); exists {
return defaultSignature, nil
}
if len(plugin.FS.Files()) == 0 {
fsFiles, err := plugin.FS.Files()
if err != nil {
return plugins.Signature{}, fmt.Errorf("files: %w", err)
}
if len(fsFiles) == 0 {
s.mlog.Warn("No plugin file information in directory", "pluginID", plugin.JSONData.ID)
return plugins.Signature{
Status: plugins.SignatureInvalid,
@@ -190,7 +197,7 @@ func (s *Signature) Calculate(ctx context.Context, src plugins.PluginSource, plu
// Track files missing from the manifest
var unsignedFiles []string
for _, f := range plugin.FS.Files() {
for _, f := range fsFiles {
// Ensure slashes are used, because MANIFEST.txt always uses slashes regardless of the filesystem
f = toSlash(f)
@@ -223,6 +230,8 @@ func (s *Signature) Calculate(ctx context.Context, src plugins.PluginSource, plu
}
func verifyHash(mlog log.Logger, plugin plugins.FoundPlugin, path, hash string) error {
path = fromSlash(path)
// nolint:gosec
// We can ignore the gosec G304 warning on this one because `path` is based
// on the path provided in a manifest file for a plugin and not user input.
pkg/plugins/manager/signature/manifest_test.go
+96 -54
View File
@@ -2,6 +2,7 @@ package signature
import (
"context"
"io/fs"
"path/filepath"
"sort"
"strings"
@@ -172,10 +173,7 @@ func TestCalculate(t *testing.T) {
Version: "1.0.0",
},
},
FS: plugins.NewLocalFS(map[string]struct{}{
filepath.Join(basePath, "MANIFEST.txt"): {},
filepath.Join(basePath, "plugin.json"): {},
}, basePath),
FS: mustNewStaticFSForTests(t, basePath),
})
require.NoError(t, err)
require.Equal(t, tc.expectedSignature, sig)
@@ -204,11 +202,7 @@ func TestCalculate(t *testing.T) {
Version: "1.0.0",
},
},
FS: plugins.NewLocalFS(map[string]struct{}{
filepath.Join(basePath, "MANIFEST.txt"): {},
filepath.Join(basePath, "plugin.json"): {},
filepath.Join(basePath, "chrome-win/debug.log"): {},
}, basePath),
FS: mustNewStaticFSForTests(t, basePath),
})
require.NoError(t, err)
require.Equal(t, plugins.Signature{
@@ -219,28 +213,51 @@ func TestCalculate(t *testing.T) {
})
t.Run("Signature verification should work with any path separator", func(t *testing.T) {
var toSlashUnix = newToSlash('/')
var toSlashWindows = newToSlash('\\')
const basePath = "../testdata/app-with-child/dist"
for _, tc := range []struct {
name string
sep string
toSlash func(string) string
platformWindows := fsPlatform{separator: '\\'}
platformUnix := fsPlatform{separator: '/'}
type testCase struct {
name string
platform fsPlatform
fsFactory func() (plugins.FS, error)
}
var testCases []testCase
for _, fsFactory := range []struct {
name string
f func() (plugins.FS, error)
}{
{"unix", "/", toSlashUnix},
{"windows", "\\", toSlashWindows},
{"local fs", func() (plugins.FS, error) {
return plugins.NewLocalFS(basePath), nil
}},
{"static fs", func() (plugins.FS, error) {
return plugins.NewStaticFS(plugins.NewLocalFS(basePath))
}},
} {
testCases = append(testCases, []testCase{
{"unix " + fsFactory.name, platformUnix, fsFactory.f},
{"windows " + fsFactory.name, platformWindows, fsFactory.f},
}...)
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
// Replace toSlash for cross-platform testing
oldToSlash := toSlash
oldFromSlash := fromSlash
t.Cleanup(func() {
toSlash = oldToSlash
fromSlash = oldFromSlash
})
toSlash = tc.toSlash
basePath := "../testdata/app-with-child/dist"
toSlash = tc.platform.toSlashFunc()
fromSlash = tc.platform.fromSlashFunc()
s := ProvideService(&config.Cfg{}, keystore.ProvideService(kvstore.NewFakeKVStore()))
pfs, err := tc.fsFactory()
require.NoError(t, err)
pfs, err = newPathSeparatorOverrideFS(string(tc.platform.separator), pfs)
require.NoError(t, err)
sig, err := s.Calculate(context.Background(), &fakes.FakePluginSource{
PluginClassFunc: func(ctx context.Context) plugins.Class {
return plugins.External
@@ -253,11 +270,7 @@ func TestCalculate(t *testing.T) {
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),
FS: pfs,
})
require.NoError(t, err)
require.Equal(t, plugins.Signature{
@@ -270,20 +283,35 @@ func TestCalculate(t *testing.T) {
})
}
// newToSlash returns a new function that acts as filepath.ToSlash but for the specified os-separator.
type fsPlatform struct {
separator rune
}
// toSlashFunc 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 {
func (p fsPlatform) toSlashFunc() func(string) string {
return func(path string) string {
if sep == '/' {
if p.separator == '/' {
return path
}
return strings.ReplaceAll(path, string(sep), "/")
return strings.ReplaceAll(path, string(p.separator), "/")
}
}
func TestNewToSlash(t *testing.T) {
// fromSlashFunc returns a new function that acts as filepath.FromSlash but for the specified os-separator.
// This can be used to test filepath.FromSlash-dependant code cross-platform.
func (p fsPlatform) fromSlashFunc() func(string) string {
return func(path string) string {
if p.separator == '/' {
return path
}
return strings.ReplaceAll(path, "/", string(p.separator))
}
}
func TestFsPlatform(t *testing.T) {
t.Run("unix", func(t *testing.T) {
toSlashUnix := newToSlash('/')
toSlashUnix := fsPlatform{'/'}.toSlashFunc()
require.Equal(t, "folder", toSlashUnix("folder"))
require.Equal(t, "/folder", toSlashUnix("/folder"))
require.Equal(t, "/folder/file", toSlashUnix("/folder/file"))
@@ -291,39 +319,46 @@ func TestNewToSlash(t *testing.T) {
})
t.Run("windows", func(t *testing.T) {
toSlashWindows := newToSlash('\\')
toSlashWindows := fsPlatform{'\\'}.toSlashFunc()
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
// fsPathSeparatorFiles embeds a plugins.FS 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
plugins.FS
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 {
// the elements returned by Files().
func newPathSeparatorOverrideFS(sep string, ufs plugins.FS) (fsPathSeparatorFiles, error) {
return fsPathSeparatorFiles{
LocalFS: plugins.NewLocalFS(files, basePath),
FS: ufs,
separator: sep,
}
}, nil
}
// Files returns LocalFS.Files(), but all path separators (filepath.Separator) are replaced with f.separator.
func (f fsPathSeparatorFiles) Files() []string {
files := f.LocalFS.Files()
// Files returns LocalFS.Files(), but all path separators for the current platform (filepath.Separator)
// are replaced with f.separator.
func (f fsPathSeparatorFiles) Files() ([]string, error) {
files, err := f.FS.Files()
if err != nil {
return nil, err
}
const osSepStr = string(filepath.Separator)
for i := 0; i < len(files); i++ {
files[i] = strings.ReplaceAll(files[i], osSepStr, f.separator)
}
return files
return files, nil
}
func (f fsPathSeparatorFiles) Open(name string) (fs.File, error) {
return f.FS.Open(strings.ReplaceAll(name, f.separator, string(filepath.Separator)))
}
func TestFSPathSeparatorFiles(t *testing.T) {
@@ -335,17 +370,18 @@ func TestFSPathSeparatorFiles(t *testing.T) {
{"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): {}})
pfs, err := newPathSeparatorOverrideFS(
"/", plugins.NewInMemoryFS(
map[string][]byte{"a": nil, strings.Join([]string{"a", "b", "c"}, tc.sep): nil},
),
)
require.NoError(t, err)
files, err := pfs.Files()
require.NoError(t, err)
exp := []string{"a", strings.Join([]string{"a", "b", "c"}, tc.sep)}
sort.Strings(files)
sort.Strings(exp)
require.Equal(t, exp, files)
})
}
}
@@ -715,3 +751,9 @@ func createV2Manifest(t *testing.T, cbs ...func(*PluginManifest)) *PluginManifes
return m
}
func mustNewStaticFSForTests(t *testing.T, dir string) plugins.FS {
sfs, err := plugins.NewStaticFS(plugins.NewLocalFS(dir))
require.NoError(t, err)
return sfs
}