Remove local load restrictions.

pkg/loader/fileloader.go

@@ -45,7 +45,8 @@ import (
 //   These paths refer to resources, patches,
 //   data for ConfigMaps and Secrets, etc.
 //
-//   They must terminate in or below the root.
+//   The loadRestrictor may disallow certain paths
+//   or classes of paths.
 //
 // * bases (other kustomizations)
 //
@@ -75,8 +76,8 @@ import (
 // These restrictions assure that kustomizations
 // are self-contained and relocatable, and impose
 // some safety when relying on remote kustomizations,
-// e.g. a ConfigMap generator specified to read
+// e.g. a remotely loaded ConfigMap generator specified
-// from /etc/passwd will fail.
+// to read from /etc/passwd will fail.
 //
 type fileLoader struct {
 	// Loader that spawned this loader.
@@ -88,6 +89,9 @@ type fileLoader struct {
 	// paths relative to this directory.
 	root fs.ConfirmedDir
 
+	// Restricts behavior of Load function.
+	loadRestrictor LoadRestrictorFunc
+
 	// If this is non-nil, the files were
 	// obtained from the given repository.
 	repoSpec *git.RepoSpec
@@ -105,13 +109,17 @@ type fileLoader struct {
 const CWD = "."
 
 // NewFileLoaderAtCwd returns a loader that loads from ".".
+// A convenience for kustomize edit commands.
 func NewFileLoaderAtCwd(fSys fs.FileSystem) *fileLoader {
-	return newLoaderOrDie(fSys, CWD)
+	return newLoaderOrDie(
+		RestrictionRootOnly, fSys, CWD)
 }
 
 // NewFileLoaderAtRoot returns a loader that loads from "/".
+// A convenience for tests.
 func NewFileLoaderAtRoot(fSys fs.FileSystem) *fileLoader {
-	return newLoaderOrDie(fSys, string(filepath.Separator))
+	return newLoaderOrDie(
+		RestrictionRootOnly, fSys, string(filepath.Separator))
 }
 
 // Root returns the absolute path that is prepended to any
@@ -120,20 +128,23 @@ func (l *fileLoader) Root() string {
 	return l.root.String()
 }
 
-func newLoaderOrDie(fSys fs.FileSystem, path string) *fileLoader {
+func newLoaderOrDie(
+	lr LoadRestrictorFunc, fSys fs.FileSystem, path string) *fileLoader {
 	root, err := demandDirectoryRoot(fSys, path)
 	if err != nil {
 		log.Fatalf("unable to make loader at '%s'; %v", path, err)
 	}
 	return newLoaderAtConfirmedDir(
-		root, fSys, nil, git.ClonerUsingGitExec)
+		lr, root, fSys, nil, git.ClonerUsingGitExec)
 }
 
 // newLoaderAtConfirmedDir returns a new fileLoader with given root.
 func newLoaderAtConfirmedDir(
+	lr LoadRestrictorFunc,
 	root fs.ConfirmedDir, fSys fs.FileSystem,
 	referrer *fileLoader, cloner git.Cloner) *fileLoader {
 	return &fileLoader{
+		loadRestrictor: lr,
 		root:           root,
 		referrer:       referrer,
 		fSys:           fSys,
@@ -190,7 +201,7 @@ func (l *fileLoader) New(path string) (ifc.Loader, error) {
 		return nil, err
 	}
 	return newLoaderAtConfirmedDir(
-		root, l.fSys, l, l.cloner), nil
+		l.loadRestrictor, root, l.fSys, l, l.cloner), nil
 }
 
 // newLoaderAtGitClone returns a new Loader pinned to a temporary
@@ -216,6 +227,8 @@ func newLoaderAtGitClone(
 			repoSpec.AbsPath(), f)
 	}
 	return &fileLoader{
+		// Clones never allowed to escape root.
+		loadRestrictor: RestrictionRootOnly,
 		root:           root,
 		referrer:       referrer,
 		repoSpec:       repoSpec,
@@ -288,29 +301,16 @@ func (l *fileLoader) errIfRepoCycle(newRepoSpec *git.RepoSpec) error {
 
 // Load returns the content of file at the given path,
 // else an error.  Relative paths are taken relative
-// relative to the root.
+// to the root.
 func (l *fileLoader) Load(path string) ([]byte, error) {
-	if filepath.IsAbs(path) {
+	if !filepath.IsAbs(path) {
-		return nil, l.loadOutOfBounds(path)
+		path = l.root.Join(path)
 	}
-	d, f, err := l.fSys.CleanedAbs(l.root.Join(path))
+	path, err := l.loadRestrictor(l.fSys, l.root, path)
 	if err != nil {
 		return nil, err
 	}
-	if f == "" {
+	return l.fSys.ReadFile(path)
-		return nil, fmt.Errorf(
-			"'%s' must be a file (got d='%s')", path, d)
-	}
-	if !d.HasPrefix(l.root) {
-		return nil, l.loadOutOfBounds(path)
-	}
-	return l.fSys.ReadFile(d.Join(f))
-}
-
-func (l *fileLoader) loadOutOfBounds(path string) error {
-	return fmt.Errorf(
-		"security; file '%s' is not in or below '%s'",
-		path, l.root)
 }
 
 // Cleanup runs the cleaner.

pkg/loader/fileloader_test.go

@@ -25,11 +25,10 @@ import (
 	"strings"
 	"testing"
 
-	"sigs.k8s.io/kustomize/pkg/git"
-	"sigs.k8s.io/kustomize/pkg/pgmconfig"
-
 	"sigs.k8s.io/kustomize/pkg/fs"
+	"sigs.k8s.io/kustomize/pkg/git"
 	"sigs.k8s.io/kustomize/pkg/ifc"
+	"sigs.k8s.io/kustomize/pkg/pgmconfig"
 )
 
 type testData struct {
@@ -207,46 +206,47 @@ func TestLoaderMisc(t *testing.T) {
 	}
 }
 
-func TestRestrictedLoadingInRealLoader(t *testing.T) {
+const (
-	// Create a structure like this
+	contentOk           = "hi there, i'm OK data"
-	//
+	contentExteriorData = "i am data from outside the root"
-	//   /tmp/kustomize-test-SZwCZYjySj
+)
-	//   ├── base
+
-	//   │   ├── okayData
+// Create a structure like this
-	//   │   ├── symLinkToOkayData -> okayData
+//
-	//   │   └── symLinkToForbiddenData -> ../forbiddenData
+//   /tmp/kustomize-test-random
-	//   └── forbiddenData
+//   ├── base
-	//
+//   │   ├── okayData
+//   │   ├── symLinkToOkayData -> okayData
+//   │   └── symLinkToExteriorData -> ../exteriorData
+//   └── exteriorData
+//
+func commonSetupForLoaderRestrictionTest() (string, fs.FileSystem, error) {
 	dir, err := ioutil.TempDir("", "kustomize-test-")
 	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
+		return "", nil, err
 	}
-	defer os.RemoveAll(dir)
-
 	fSys := fs.MakeRealFS()
 	fSys.Mkdir(filepath.Join(dir, "base"))
 
-	contentOk := "hi there, i'm OK data"
 	fSys.WriteFile(
 		filepath.Join(dir, "base", "okayData"), []byte(contentOk))
 
-	contentForbidden := "don't be looking at me"
 	fSys.WriteFile(
-		filepath.Join(dir, "forbiddenData"), []byte(contentForbidden))
+		filepath.Join(dir, "exteriorData"), []byte(contentExteriorData))
 
 	os.Symlink(
 		filepath.Join(dir, "base", "okayData"),
 		filepath.Join(dir, "base", "symLinkToOkayData"))
 	os.Symlink(
-		filepath.Join(dir, "forbiddenData"),
+		filepath.Join(dir, "exteriorData"),
-		filepath.Join(dir, "base", "symLinkToForbiddenData"))
+		filepath.Join(dir, "base", "symLinkToExteriorData"))
+	return dir, fSys, nil
+}
 
-	var l ifc.Loader
+// Make sure everything works when loading files
-
+// in or below the loader root.
-	l = newLoaderOrDie(fSys, dir)
+func doSanityChecksAndDropIntoBase(
-
+	t *testing.T, l ifc.Loader) ifc.Loader {
-	// Sanity checks - loading from perspective of "dir".
-	// Everything works, including reading from a subdirectory.
 	data, err := l.Load(path.Join("base", "okayData"))
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -254,11 +254,11 @@ func TestRestrictedLoadingInRealLoader(t *testing.T) {
 	if string(data) != contentOk {
 		t.Fatalf("unexpected content: %v", data)
 	}
-	data, err = l.Load("forbiddenData")
+	data, err = l.Load("exteriorData")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if string(data) != contentForbidden {
+	if string(data) != contentExteriorData {
 		t.Fatalf("unexpected content: %v", data)
 	}
 
@@ -285,9 +285,24 @@ func TestRestrictedLoadingInRealLoader(t *testing.T) {
 	if string(data) != contentOk {
 		t.Fatalf("unexpected content: %v", data)
 	}
+	return l
+}
 
-	// Reading symlink to forbiddenData fails.
+func TestRestrictionRootInRealLoader(t *testing.T) {
-	_, err = l.Load("symLinkToForbiddenData")
+	dir, fSys, err := commonSetupForLoaderRestrictionTest()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	defer os.RemoveAll(dir)
+
+	var l ifc.Loader
+
+	l = newLoaderOrDie(RestrictionRootOnly, fSys, dir)
+
+	l = doSanityChecksAndDropIntoBase(t, l)
+
+	// Reading symlink to exteriorData fails.
+	_, err = l.Load("symLinkToExteriorData")
 	if err == nil {
 		t.Fatalf("expected error")
 	}
@@ -297,7 +312,7 @@ func TestRestrictedLoadingInRealLoader(t *testing.T) {
 
 	// Attempt to read "up" fails, though earlier we were
 	// able to read this file when root was "..".
-	_, err = l.Load("../forbiddenData")
+	_, err = l.Load("../exteriorData")
 	if err == nil {
 		t.Fatalf("expected error")
 	}
@@ -306,6 +321,32 @@ func TestRestrictedLoadingInRealLoader(t *testing.T) {
 	}
 }
 
+func TestRestrictionNoneInRealLoader(t *testing.T) {
+	dir, fSys, err := commonSetupForLoaderRestrictionTest()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	defer os.RemoveAll(dir)
+
+	var l ifc.Loader
+
+	l = newLoaderOrDie(RestrictionNone, fSys, dir)
+
+	l = doSanityChecksAndDropIntoBase(t, l)
+
+	// Reading symlink to exteriorData works.
+	_, err = l.Load("symLinkToExteriorData")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Attempt to read "up" works.
+	_, err = l.Load("../exteriorData")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
 func splitOnNthSlash(v string, n int) (string, string) {
 	left := ""
 	for i := 0; i < n; i++ {
@@ -393,7 +434,7 @@ func TestLoaderDisallowsLocalBaseFromRemoteOverlay(t *testing.T) {
 
 	// Establish that a local overlay can navigate
 	// to the local bases.
-	l1 = newLoaderOrDie(fSys, cloneRoot+"/foo/overlay")
+	l1 = newLoaderOrDie(RestrictionRootOnly, fSys, cloneRoot+"/foo/overlay")
 	if l1.Root() != cloneRoot+"/foo/overlay" {
 		t.Fatalf("unexpected root %s", l1.Root())
 	}
@@ -468,7 +509,7 @@ func TestLocalLoaderReferencingGitBase(t *testing.T) {
 		t.Fatalf("unexpected err:  %v\n", err)
 	}
 	l1 := newLoaderAtConfirmedDir(
-		root, fSys, nil,
+		RestrictionRootOnly, root, fSys, nil,
 		git.DoNothingCloner(fs.ConfirmedDir(cloneRoot)))
 	if l1.Root() != topDir {
 		t.Fatalf("unexpected root %s", l1.Root())

pkg/loader/loader.go

@@ -23,17 +23,24 @@ import (
 	"sigs.k8s.io/kustomize/pkg/ifc"
 )
 
-// NewLoader returns a Loader.
+// NewLoader returns a Loader pointed at the given target.
-func NewLoader(path string, fSys fs.FileSystem) (ifc.Loader, error) {
+// If the target is remote, the loader will be restricted
-	repoSpec, err := git.NewRepoSpecFromUrl(path)
+// to the root and below only.  If the target is local, the
+// loader will have no restrictions.  If the local target
+// attempts to transitively load remote bases, they will all
+// be root-only restricted.
+func NewLoader(
+	target string, fSys fs.FileSystem) (ifc.Loader, error) {
+	repoSpec, err := git.NewRepoSpecFromUrl(target)
 	if err == nil {
+		// The target qualifies as a remote git target.
 		return newLoaderAtGitClone(
 			repoSpec, fSys, nil, git.ClonerUsingGitExec)
 	}
-	root, err := demandDirectoryRoot(fSys, path)
+	root, err := demandDirectoryRoot(fSys, target)
 	if err != nil {
 		return nil, err
 	}
 	return newLoaderAtConfirmedDir(
-		root, fSys, nil, git.ClonerUsingGitExec), nil
+		RestrictionNone, root, fSys, nil, git.ClonerUsingGitExec), nil
 }

pkg/loader/loadrestrictions.go

@@ -0,0 +1,48 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package loader
+
+import (
+	"fmt"
+
+	"sigs.k8s.io/kustomize/pkg/fs"
+)
+
+type LoadRestrictorFunc func(
+	fs.FileSystem, fs.ConfirmedDir, string) (string, error)
+
+func RestrictionRootOnly(
+	fSys fs.FileSystem, root fs.ConfirmedDir, path string) (string, error) {
+	d, f, err := fSys.CleanedAbs(path)
+	if err != nil {
+		return "", err
+	}
+	if f == "" {
+		return "", fmt.Errorf("'%s' must be a file", path)
+	}
+	if !d.HasPrefix(root) {
+		return "", fmt.Errorf(
+			"security; file '%s' is not in or below '%s'",
+			path, root)
+	}
+	return d.Join(f), nil
+}
+
+func RestrictionNone(
+	_ fs.FileSystem, _ fs.ConfirmedDir, path string) (string, error) {
+	return path, nil
+}

pkg/loader/loadrestrictions_test.go

@@ -0,0 +1,74 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package loader
+
+import (
+	"strings"
+	"testing"
+
+	"sigs.k8s.io/kustomize/pkg/fs"
+)
+
+func TestRestrictionNone(t *testing.T) {
+	fSys := fs.MakeFakeFS()
+	root := fs.ConfirmedDir("irrelevant")
+	path := "whatever"
+	p, err := RestrictionNone(fSys, root, path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if p != path {
+		t.Fatalf("expected '%s', got '%s'", path, p)
+	}
+}
+
+func TestRestrictionRootOnly(t *testing.T) {
+	fSys := fs.MakeFakeFS()
+	root := fs.ConfirmedDir("/tmp/foo")
+
+	path := "/tmp/foo/whatever/beans"
+	p, err := RestrictionRootOnly(fSys, root, path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if p != path {
+		t.Fatalf("expected '%s', got '%s'", path, p)
+	}
+
+	// Legal.
+	path = "/tmp/foo/whatever/../../foo/whatever"
+	p, err = RestrictionRootOnly(fSys, root, path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	path = "/tmp/foo/whatever"
+	if p != path {
+		t.Fatalf("expected '%s', got '%s'", path, p)
+	}
+
+	// Illegal.
+	path = "/tmp/illegal"
+	_, err = RestrictionRootOnly(fSys, root, path)
+	if err == nil {
+		t.Fatal("should have an error")
+	}
+	if !strings.Contains(
+		err.Error(),
+		"file '/tmp/illegal' is not in or below '/tmp/foo'") {
+		t.Fatalf("unexpected err: %s", err)
+	}
+}

pkg/target/baseandoverlaysmall_test.go

@@ -17,7 +17,6 @@ limitations under the License.
 package target_test
 
 import (
-	"strings"
 	"testing"
 )
 
@@ -182,7 +181,7 @@ spec:
 `)
 }
 
-func TestSharedPatchDisAllowed(t *testing.T) {
+func TestSharedPatchAllowed(t *testing.T) {
 	th := NewKustTestHarness(t, "/app/overlay")
 	writeSmallBase(th)
 	th.writeK("/app/overlay", `
@@ -201,15 +200,50 @@ metadata:
 spec:
   replicas: 1000
 `)
-	_, err := th.makeKustTarget().MakeCustomizedResMap()
+	m, err := th.makeKustTarget().MakeCustomizedResMap()
-	if err == nil {
+	if err != nil {
-		t.Fatalf("expected error")
+		t.Fatalf("Err: %v", err)
-	}
-	if !strings.Contains(
-		err.Error(),
-		"security; file '../shared/deployment-patch.yaml' is not in or below '/app/overlay'") {
-		t.Fatalf("unexpected error: %s", err)
 	}
+	th.assertActualEqualsExpected(m, `
+apiVersion: v1
+kind: Service
+metadata:
+  labels:
+    app: myApp
+    env: prod
+  name: a-myService
+spec:
+  ports:
+  - port: 7002
+  selector:
+    app: myApp
+    backend: bungie
+    env: prod
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  labels:
+    app: myApp
+    env: prod
+  name: a-myDeployment
+spec:
+  replicas: 1000
+  selector:
+    matchLabels:
+      app: myApp
+      env: prod
+  template:
+    metadata:
+      labels:
+        app: myApp
+        backend: awesome
+        env: prod
+    spec:
+      containers:
+      - image: whatever
+        name: whatever
+`)
 }
 
 func TestSmallOverlayJSONPatch(t *testing.T) {