From 2ced6491e1bff6e2c1bb78c7a482c5bc2780ef83 Mon Sep 17 00:00:00 2001 From: k1LoW Date: Mon, 30 Oct 2023 11:43:12 +0900 Subject: [PATCH] Add the concept of scope. And disallow remote reads and reads of directories above the working directory by default. --- book.go | 11 ++++++++- book_test.go | 30 ++++++++++++++++++++++++ capture/runbook_test.go | 3 +++ cmd/root.go | 4 ++++ flags/flags.go | 2 ++ operator.go | 2 +- option.go | 8 +++++++ path.go | 43 ++++++++++++++++++++++++++++++++-- scope.go | 51 +++++++++++++++++++++++++++++++++++++++++ 9 files changed, 150 insertions(+), 4 deletions(-) create mode 100644 scope.go diff --git a/book.go b/book.go index 50a0750a..c9a156bc 100644 --- a/book.go +++ b/book.go @@ -68,8 +68,10 @@ type book struct { capturers capturers stdout io.Writer stderr io.Writer - // skip some errors for `runn list` + // Skip some errors for `runn list` loadOnly bool + // Actually not used, but used as a temporary value receiver + scopes []string } func LoadBook(path string) (*book, error) { @@ -479,6 +481,13 @@ func (bk *book) parseSSHRunnerWithDetailed(name string, b []byte) (bool, error) } func (bk *book) applyOptions(opts ...Option) error { + tmpBk := newBook() + for _, opt := range opts { + _ = opt(tmpBk) + } + if err := setScopes(tmpBk.scopes...); err != nil { + return err + } opts = setupBuiltinFunctions(opts...) for _, opt := range opts { if err := opt(bk); err != nil { diff --git a/book_test.go b/book_test.go index 5848556b..8170541f 100644 --- a/book_test.go +++ b/book_test.go @@ -1,6 +1,7 @@ package runn import ( + "fmt" "net/http" "net/url" "os" @@ -107,6 +108,35 @@ func TestApplyOptions(t *testing.T) { } } +func TestApplyOptionsWithScope(t *testing.T) { + tests := []struct { + opts []Option + readRemote bool + wantErr bool + }{ + {[]Option{Book("testdata/book/book.yml")}, false, false}, + {[]Option{Book("github://k1LoW/runn/testdata/book/http.yml")}, false, true}, + {[]Option{Book("github://k1LoW/runn/testdata/book/http.yml")}, true, false}, + {[]Option{Scopes(ScopeAllowReadRemote), Book("github://k1LoW/runn/testdata/book/http.yml")}, false, false}, + {[]Option{Book("github://k1LoW/runn/testdata/book/http.yml"), Scopes(ScopeAllowReadRemote)}, false, false}, + } + for i, tt := range tests { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + globalScopes.readRemote = tt.readRemote + bk := newBook() + if err := bk.applyOptions(tt.opts...); err != nil { + if !tt.wantErr { + t.Errorf("got %v", err) + } + return + } + if tt.wantErr { + t.Error("want err") + } + }) + } +} + func TestParseRunnerForHttpRunner(t *testing.T) { secureUrl, _ := url.Parse("https://example.com/") url, _ := url.Parse("http://example.com/") diff --git a/capture/runbook_test.go b/capture/runbook_test.go index ebda1021..706194cc 100644 --- a/capture/runbook_test.go +++ b/capture/runbook_test.go @@ -37,6 +37,7 @@ func TestRunbook(t *testing.T) { runn.GrpcRunner("greq", gs.Conn()), runn.DBRunner("db", db), runn.Capture(Runbook(dir)), + runn.Scopes(runn.ScopeAllowReadParent), } o, err := runn.New(opts...) if err != nil { @@ -84,6 +85,7 @@ func TestRunnable(t *testing.T) { runn.GrpcRunner("greq", gs.Conn()), runn.DBRunner("db", db), runn.Capture(Runbook(dir)), + runn.Scopes(runn.ScopeAllowReadParent), } o, err := runn.New(opts...) if err != nil { @@ -103,6 +105,7 @@ func TestRunnable(t *testing.T) { runn.HTTPRunner("req", hs.URL, hs.Client(), runn.MultipartBoundary(testutil.MultipartBoundary)), runn.GrpcRunner("greq", gs.Conn()), runn.DBRunner("db", db), + runn.Scopes(runn.ScopeAllowReadParent), } o, err := runn.New(opts...) if err != nil { diff --git a/cmd/root.go b/cmd/root.go index e146a64f..62ee807e 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -46,3 +46,7 @@ func Execute() { os.Exit(1) } } + +func init() { + rootCmd.PersistentFlags().StringSliceVarP(&flgs.Scopes, "scopes", "", []string{}, flgs.Usage("Scopes")) // Plural to support comma-separated input +} diff --git a/flags/flags.go b/flags/flags.go index d589006f..ed0f1a3e 100644 --- a/flags/flags.go +++ b/flags/flags.go @@ -59,6 +59,7 @@ type Flags struct { ProfileSort string `usage:"-"` CacheDir string `usage:"specify cache directory for remote runbooks"` RetainCacheDir bool `usage:"retain cache directory for remote runbooks"` + Scopes []string `usage:"additional scopes for runn"` Verbose bool `usage:"verbose"` } @@ -78,6 +79,7 @@ func (f *Flags) ToOpts() ([]runn.Option, error) { runn.GRPCProtos(f.GRPCProtos), runn.GRPCImportPaths(f.GRPCImportPaths), runn.Profile(f.Profile), + runn.Scopes(f.Scopes...), } if f.RunID != "" { opts = append(opts, runn.RunID(f.RunID)) diff --git a/operator.go b/operator.go index 9a553938..688ee2c8 100644 --- a/operator.go +++ b/operator.go @@ -1152,7 +1152,7 @@ type operators struct { func Load(pathp string, opts ...Option) (*operators, error) { bk := newBook() - opts = append([]Option{RunMatch(os.Getenv("RUNN_RUN")), RunID(os.Getenv("RUNN_ID"))}, opts...) + opts = append([]Option{RunMatch(os.Getenv("RUNN_RUN")), RunID(os.Getenv("RUNN_ID")), Scopes(os.Getenv("RUNN_SCOPES"))}, opts...) if err := bk.applyOptions(opts...); err != nil { return nil, err } diff --git a/option.go b/option.go index ee47c305..33d08c43 100644 --- a/option.go +++ b/option.go @@ -849,6 +849,14 @@ func LoadOnly() Option { } } +// Scopes - Set scopes for runn +func Scopes(scopes ...string) Option { + return func(bk *book) error { + bk.scopes = scopes + return nil + } +} + // bookWithStore - Load runbook with store. func bookWithStore(path string, store map[string]any) Option { return func(bk *book) error { diff --git a/path.go b/path.go index e9e364a1..efd7fdfa 100644 --- a/path.go +++ b/path.go @@ -56,6 +56,9 @@ func fetchPaths(pathp string) ([]string, error) { switch { case strings.HasPrefix(base, prefixHttps): // https:// + if !globalScopes.readRemote { + return nil, fmt.Errorf("scope error: remote file not allowed: %s", pp) + } if strings.Contains(pattern, "*") { return nil, fmt.Errorf("https scheme does not support wildcard: %s", pp) } @@ -66,6 +69,9 @@ func fetchPaths(pathp string) ([]string, error) { paths = append(paths, p) case strings.HasPrefix(base, prefixGitHub): // github:// + if !globalScopes.readRemote { + return nil, fmt.Errorf("scope error: remote file not allowed: %s", pp) + } splitted := strings.Split(strings.TrimPrefix(base, prefixGitHub), "/") if len(splitted) < 2 { return nil, fmt.Errorf("invalid path: %s", pp) @@ -141,9 +147,38 @@ func fetchPath(path string) (string, error) { // readFile reads single file from local or cache. // When retrieving a cache file, if the cache file does not exist, re-fetch it. func readFile(p string) ([]byte, error) { - _, err := os.Stat(p) + fi, err := os.Stat(p) if err == nil { - // Read local file or cache + if globalCacheDir != "" && strings.HasPrefix(p, globalCacheDir) { + // Read cache file + if !globalScopes.readRemote { + return nil, fmt.Errorf("scope error: remote file not allowed: %s", p) + } + return os.ReadFile(p) + } + if fi.Mode()&os.ModeSymlink == os.ModeSymlink { + // Read symlink + p, err = os.Readlink(p) + if err != nil { + return nil, err + } + } + abs, err := filepath.Abs(p) + if err != nil { + return nil, err + } + wd, err := os.Getwd() + if err != nil { + return nil, err + } + rel, err := filepath.Rel(wd, abs) + if err != nil { + return nil, err + } + if !globalScopes.readParent && strings.Contains(rel, "..") { + return nil, fmt.Errorf("scope error: parent directory not allowed: %s", p) + } + // Read local file return os.ReadFile(p) } if globalCacheDir == "" || !strings.HasPrefix(p, globalCacheDir) { @@ -151,6 +186,10 @@ func readFile(p string) ([]byte, error) { return nil, err } + if !globalScopes.readRemote { + return nil, fmt.Errorf("scope error: remote file not allowed: %s", p) + } + // Re-fetch remote file and create cache cachePath, err := filepath.Rel(globalCacheDir, p) if err != nil { diff --git a/scope.go b/scope.go new file mode 100644 index 00000000..d46152da --- /dev/null +++ b/scope.go @@ -0,0 +1,51 @@ +package runn + +import ( + "errors" + "strings" + "sync" +) + +type scopes struct { + readParent bool + readRemote bool + mu sync.RWMutex +} + +const ( + ScopeAllowReadParent = "read:parent" + ScopeAllowReadRemote = "read:remote" + ScopeDenyReadParent = "!read:parent" + ScopeDenyReadRemote = "!read:remote" +) + +var ErrInvalidScope = errors.New("invalid scope") + +var globalScopes = &scopes{ + readParent: false, + readRemote: false, +} + +func setScopes(scopes ...string) error { + globalScopes.mu.Lock() + defer globalScopes.mu.Unlock() + for _, s := range scopes { + splitted := strings.Split(strings.TrimSpace(s), ",") + for _, ss := range splitted { + switch ss { + case ScopeAllowReadParent: + globalScopes.readParent = true + case ScopeAllowReadRemote: + globalScopes.readRemote = true + case ScopeDenyReadParent: + globalScopes.readParent = false + case ScopeDenyReadRemote: + globalScopes.readRemote = false + case "": + default: + return ErrInvalidScope + } + } + } + return nil +}