Skip to content

Commit

Permalink
fix: Normalize Git store subdirectory config to handle leading ./ c…
Browse files Browse the repository at this point in the history
…orrectly (#1774)

Signed-off-by: Andrew Haines <haines@cerbos.dev>
  • Loading branch information
haines committed Aug 30, 2023
1 parent 9652c90 commit 7bfa52a
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 18 deletions.
22 changes: 13 additions & 9 deletions internal/storage/git/conf.go
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"os"
"path/filepath"
"strings"
"time"

"github.com/go-git/go-git/v5/plumbing/transport"
Expand Down Expand Up @@ -94,33 +95,36 @@ func (conf *Conf) Key() string {
return confKey
}

func (conf *Conf) Validate() error {
var errs []error

func (conf *Conf) Validate() (errs error) {
switch conf.Protocol {
case "ssh", "http", "https", "file":
default:
errs = append(errs, fmt.Errorf("unknown git protocol: %s", conf.Protocol))
errs = multierr.Append(errs, fmt.Errorf("unknown git protocol: %s", conf.Protocol))
}

if conf.URL == "" {
errs = append(errs, errors.New("git URL is required"))
errs = multierr.Append(errs, errors.New("git URL is required"))
}

if conf.CheckoutDir == "" {
cacheDir, err := os.UserCacheDir()
if err != nil {
errs = append(errs, fmt.Errorf("checkoutDir unspecified and failed to determine user cache dir: %w", err))
errs = multierr.Append(errs, fmt.Errorf("checkoutDir unspecified and failed to determine user cache dir: %w", err))
} else {
conf.CheckoutDir = filepath.Join(cacheDir, util.AppName, "git")
}
}

if len(errs) > 0 {
return multierr.Combine(errs...)
subDir := conf.getSubDir()
if filepath.IsAbs(subDir) || strings.HasPrefix(subDir, "../") || subDir == ".." {
errs = multierr.Append(errs, errors.New("subDir must be a relative path within the repository"))
}

return nil
return errs
}

func (conf *Conf) getSubDir() string {
return filepath.ToSlash(filepath.Clean(conf.SubDir))
}

func (conf *Conf) getBranch() string {
Expand Down
14 changes: 5 additions & 9 deletions internal/storage/git/store.go
Expand Up @@ -9,7 +9,6 @@ import (
"fmt"
"io"
"os"
"path/filepath"
"strings"
"time"

Expand Down Expand Up @@ -56,12 +55,14 @@ type Store struct {
repo *git.Repository
sf singleflight.Group
*storage.SubscriptionManager
subDir string
}

func NewStore(ctx context.Context, conf *Conf) (*Store, error) {
s := &Store{
log: zap.S().Named("git.store").With("dir", conf.CheckoutDir),
conf: conf,
subDir: conf.getSubDir(),
SubscriptionManager: storage.NewSubscriptionManager(ctx),
}

Expand Down Expand Up @@ -227,12 +228,7 @@ func (s *Store) cloneRepo(ctx context.Context) error {
}

func (s *Store) loadAll(ctx context.Context) error {
policyDir := "."
if s.conf.SubDir != "" {
policyDir = s.conf.SubDir
}

idx, err := index.Build(ctx, os.DirFS(s.conf.CheckoutDir), index.WithRootDir(policyDir))
idx, err := index.Build(ctx, os.DirFS(s.conf.CheckoutDir), index.WithRootDir(s.subDir))
if err != nil {
return err
}
Expand Down Expand Up @@ -448,8 +444,8 @@ func (s *Store) normalizePath(path string) (string, util.IndexedFileType) {
return path, util.FileTypeNotIndexed
}

if s.conf.SubDir != "" {
relativePath := strings.TrimPrefix(path, strings.TrimSuffix(filepath.ToSlash(s.conf.SubDir), "/")+"/")
if s.subDir != "." {
relativePath := strings.TrimPrefix(path, s.subDir+"/")
if path == relativePath { // not in policies directory
return path, util.FileTypeNotIndexed
}
Expand Down
102 changes: 102 additions & 0 deletions internal/storage/git/store_test.go
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/go-git/go-git/v5"
"github.com/go-git/go-git/v5/plumbing"
"github.com/go-git/go-git/v5/plumbing/object"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"

Expand All @@ -31,6 +32,7 @@ import (
"github.com/cerbos/cerbos/internal/storage/internal"
"github.com/cerbos/cerbos/internal/test"
"github.com/cerbos/cerbos/internal/test/mocks"
"github.com/cerbos/cerbos/internal/util"
)

const (
Expand Down Expand Up @@ -579,6 +581,106 @@ func TestReloadable(t *testing.T) {
internal.TestSuiteReloadable(store, mkInitFn(t, sourceGitDir), mkAddFn(t, sourceGitDir, ps), mkDeleteFn(t, sourceGitDir))(t)
}

func TestNormalizePath(t *testing.T) {
testCases := []struct {
subDir string
path string
wantPath string
wantFileType util.IndexedFileType
}{
{
subDir: ".",
path: "foo.yaml",
wantPath: "foo.yaml",
wantFileType: util.FileTypePolicy,
},
{
subDir: ".",
path: "foo.json",
wantPath: "foo.json",
wantFileType: util.FileTypePolicy,
},
{
subDir: ".",
path: "foo/bar.yaml",
wantPath: "foo/bar.yaml",
wantFileType: util.FileTypePolicy,
},
{
subDir: "foo",
path: "foo/bar.yaml",
wantPath: "bar.yaml",
wantFileType: util.FileTypePolicy,
},
{
subDir: "foo",
path: "foo/bar/baz.yaml",
wantPath: "bar/baz.yaml",
wantFileType: util.FileTypePolicy,
},
{
subDir: ".",
path: "_schemas/foo.json",
wantPath: "foo.json",
wantFileType: util.FileTypeSchema,
},
{
subDir: ".",
path: "_schemas/foo/bar.json",
wantPath: "foo/bar.json",
wantFileType: util.FileTypeSchema,
},
{
subDir: "foo",
path: "foo/_schemas/bar.json",
wantPath: "bar.json",
wantFileType: util.FileTypeSchema,
},
{
subDir: "foo",
path: "foo/_schemas/bar/baz.json",
wantPath: "bar/baz.json",
wantFileType: util.FileTypeSchema,
},
{
subDir: ".",
path: "",
wantPath: "",
wantFileType: util.FileTypeNotIndexed,
},
{
subDir: ".",
path: "foo.txt",
wantPath: "foo.txt",
wantFileType: util.FileTypeNotIndexed,
},
{
subDir: ".",
path: "_schemas/foo.yaml",
wantPath: "_schemas/foo.yaml",
wantFileType: util.FileTypeNotIndexed,
},
{
subDir: "foo",
path: "bar.yaml",
wantPath: "bar.yaml",
wantFileType: util.FileTypeNotIndexed,
},
}

for _, tc := range testCases {
tc := tc
t.Run(fmt.Sprintf("subDir=%s,path=%s", tc.subDir, tc.path), func(t *testing.T) {
store := &Store{subDir: tc.subDir}

havePath, haveFileType := store.normalizePath(tc.path)

assert.Equal(t, tc.wantPath, havePath, "Unexpected path")
assert.Equal(t, tc.wantFileType, haveFileType, "Unexpected file type")
})
}
}

func mkInitFn(t *testing.T, sourceGitDir string) internal.MutateStoreFn {
t.Helper()

Expand Down

0 comments on commit 7bfa52a

Please sign in to comment.