diff --git a/pkg/loader/fileloader.go b/pkg/loader/fileloader.go index 97da3a117..69e5a3d0d 100644 --- a/pkg/loader/fileloader.go +++ b/pkg/loader/fileloader.go @@ -176,6 +176,9 @@ func (l *fileLoader) New(path string) (ifc.Loader, error) { if err != nil { return nil, err } + if err := l.errIfGitContainmentViolation(root); err != nil { + return nil, err + } if err := l.errIfArgEqualOrHigher(root); err != nil { return nil, err } @@ -215,6 +218,34 @@ func newLoaderAtGitClone( }, nil } +func (l *fileLoader) errIfGitContainmentViolation( + base fs.ConfirmedDir) error { + containingRepo := l.containingRepo() + if containingRepo == nil { + return nil + } + if !base.HasPrefix(containingRepo.CloneDir()) { + return fmt.Errorf( + "security; bases in kustomizations found in "+ + "cloned git repos must be within the repo, "+ + "but base '%s' is outside '%s'", + base, containingRepo.CloneDir()) + } + return nil +} + +// Looks back through referrers for a git repo, returning nil +// if none found. +func (l *fileLoader) containingRepo() *git.RepoSpec { + if l.repoSpec != nil { + return l.repoSpec + } + if l.referrer == nil { + return nil + } + return l.referrer.containingRepo() +} + // errIfArgEqualOrHigher tests whether the argument, // is equal to or above the root of any ancestor. func (l *fileLoader) errIfArgEqualOrHigher( diff --git a/pkg/loader/fileloader_test.go b/pkg/loader/fileloader_test.go index f02552b8f..8b50b5756 100644 --- a/pkg/loader/fileloader_test.go +++ b/pkg/loader/fileloader_test.go @@ -386,3 +386,79 @@ whatever coRoot+"/"+pathInRepo, l2.Root()) } } + +func TestLoaderDisallowsLocalBaseFromRemoteOverlay(t *testing.T) { + // Define an overlay-base structure in the file system. + topDir := "/whatever" + cloneRoot := topDir + "/someClone" + fSys := fs.MakeFakeFS() + fSys.MkdirAll(topDir + "/highBase") + fSys.MkdirAll(cloneRoot + "/foo/base") + fSys.MkdirAll(cloneRoot + "/foo/overlay") + + var l1 ifc.Loader + + // Establish that a local overlay can navigate + // to the local bases. + l1 = newLoaderOrDie(fSys, cloneRoot+"/foo/overlay") + if l1.Root() != cloneRoot+"/foo/overlay" { + t.Fatalf("unexpected root %s", l1.Root()) + } + l2, err := l1.New("../base") + if err != nil { + t.Fatalf("unexpected err: %v\n", err) + } + if l2.Root() != cloneRoot+"/foo/base" { + t.Fatalf("unexpected root %s", l2.Root()) + } + l3, err := l2.New("../../../highBase") + if err != nil { + t.Fatalf("unexpected err: %v\n", err) + } + if l3.Root() != topDir+"/highBase" { + t.Fatalf("unexpected root %s", l3.Root()) + } + + // Establish that a Kustomization found in cloned + // repo can reach (non-remote) bases inside the clone + // but cannot reach a (non-remote) base outside the + // clone but legitimately on the local file system. + // This is to avoid a surprising interaction between + // a remote K and local files. The remote K would be + // non-functional on its own since by definition it + // would refer to a non-remote base file that didn't + // exist in its own repository, so presumably the + // remote K would be deliberately designed to phish + // for local K's. + repoSpec, err := git.NewRepoSpecFromUrl( + "github.com/someOrg/someRepo/foo/overlay") + if err != nil { + t.Fatalf("unexpected err: %v\n", err) + } + l1, err = newLoaderAtGitClone( + repoSpec, fSys, nil, + git.DoNothingCloner(fs.ConfirmedDir(cloneRoot))) + if err != nil { + t.Fatalf("unexpected err: %v\n", err) + } + if l1.Root() != cloneRoot+"/foo/overlay" { + t.Fatalf("unexpected root %s", l1.Root()) + } + // This is okay. + l2, err = l1.New("../base") + if err != nil { + t.Fatalf("unexpected err: %v\n", err) + } + if l2.Root() != cloneRoot+"/foo/base" { + t.Fatalf("unexpected root %s", l2.Root()) + } + // This is not okay. + l3, err = l2.New("../../../highBase") + if err == nil { + t.Fatalf("expected err") + } + if !strings.Contains(err.Error(), + "base '/whatever/highBase' is outside '/whatever/someClone'") { + t.Fatalf("unexpected err: %v", err) + } +}