Skip to content

Commit

Permalink
Add the concept of scope.
Browse files Browse the repository at this point in the history
And disallow remote reads and reads of directories above the working directory by default.
  • Loading branch information
k1LoW committed Oct 30, 2023
1 parent 337b1f0 commit 2ced649
Show file tree
Hide file tree
Showing 9 changed files with 150 additions and 4 deletions.
11 changes: 10 additions & 1 deletion book.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
30 changes: 30 additions & 0 deletions book_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package runn

import (
"fmt"
"net/http"
"net/url"
"os"
Expand Down Expand Up @@ -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/")
Expand Down
3 changes: 3 additions & 0 deletions capture/runbook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
2 changes: 2 additions & 0 deletions flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}

Expand All @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
8 changes: 8 additions & 0 deletions option.go
Original file line number Diff line number Diff line change
Expand Up @@ -849,6 +849,14 @@ func LoadOnly() Option {
}
}

// Scopes - Set scopes for runn

Check failure on line 852 in option.go

View workflow job for this annotation

GitHub Actions / Test

Comment should end in a period (godot)

Check failure on line 852 in option.go

View workflow job for this annotation

GitHub Actions / golangci

[golangci] option.go#L852

Comment should end in a period (godot)
Raw output
option.go:852:32: Comment should end in a period (godot)
// 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 {
Expand Down
43 changes: 41 additions & 2 deletions path.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
Expand Down Expand Up @@ -141,16 +147,49 @@ 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) {
// Not cache file
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 {
Expand Down
51 changes: 51 additions & 0 deletions scope.go
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 2ced649

Please sign in to comment.