Skip to content

Commit

Permalink
feat: localize absolute paths
Browse files Browse the repository at this point in the history
Signed-off-by: Claudio Busse <cbusse@redhat.com>
  • Loading branch information
typeid committed Oct 8, 2023
1 parent 779f153 commit 3ec212e
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 10 deletions.
11 changes: 10 additions & 1 deletion api/internal/loader/fileloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,17 @@ func (fl *FileLoader) New(path string) (ifc.Loader, error) {
}

if filepath.IsAbs(path) {
return nil, fmt.Errorf("new root '%s' cannot be absolute", path)
if !strings.HasPrefix(path, fl.root.String()) {
return nil, fmt.Errorf("referenced absolute path '%s' in your Kustomization file is not in the same tree as '%s'", path, fl.Root())
}

// Convert absolute path to relative path for unified handling
path, err = filepath.Rel(fl.root.String(), path)
if err != nil {
return nil, fmt.Errorf("could not convert absolute path '%s' to relative path of root", path)
}
}

root, err := filesys.ConfirmDir(fl.fSys, fl.root.Join(path))
if err != nil {
return nil, errors.WrapPrefixf(err, ErrRtNotDir.Error())
Expand Down
33 changes: 33 additions & 0 deletions api/internal/loader/fileloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,39 @@ func TestLoaderBadRelative(t *testing.T) {
require.Error(err)
}

func TestLoaderAbsolute(t *testing.T) {
require := require.New(t)

l1, err := makeLoader().New("foo/project")
require.NoError(err)
require.Equal("/foo/project", l1.Root())

// It's not okay to escape out of the root directory
_, err = l1.New("/tmp/escapedpath")
require.Error(err)

// It's not okay to escape using relative paths in the absolute path
_, err = l1.New("/foo/project/subdir2/../../otherfile")
require.Error(err)

// It's okay to contain a relative part in the absolute path
l2, err := l1.New("/foo/project/subdir2/../subdir1")
require.NoError(err)
require.Equal("/foo/project/subdir1", l2.Root())

// It's okay to cd into an absolute path
l2, err = l1.New("/foo/project/subdir2")
require.NoError(err)
require.Equal("/foo/project/subdir2", l2.Root())

x := testCases[2]
b, err := l2.Load("fileC.yaml")
require.NoError(err)
if !reflect.DeepEqual([]byte(x.expectedContent), b) {
t.Fatalf("in load expected %s, but got %s", x.expectedContent, b)
}
}

func TestNewEmptyLoader(t *testing.T) {
_, err := makeLoader().New("")
require.Error(t, err)
Expand Down
14 changes: 13 additions & 1 deletion api/internal/localizer/locloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
package localizer

import (
"fmt"
"path/filepath"
"strings"

"sigs.k8s.io/kustomize/api/ifc"
"sigs.k8s.io/kustomize/api/internal/git"
Expand Down Expand Up @@ -89,9 +91,19 @@ func (ll *Loader) Load(path string) ([]byte, error) {
if err != nil {
return nil, errors.WrapPrefixf(err, "invalid file reference")
}

if filepath.IsAbs(path) {
return nil, errors.Errorf("absolute paths not yet supported in alpha: file path %q is absolute", path)
if !strings.HasPrefix(path, ll.Root()) {
return nil, fmt.Errorf("referenced absolute path '%s' in your Kustomization file is not in the same tree as '%s'", path, ll.Root())
}

// Convert absolute path to relative path for unified handling
path, err = filepath.Rel(ll.Root(), path)
if err != nil {
return nil, fmt.Errorf("could not convert absolute path '%s' to relative path of root", path)
}
}

if !loader.IsRemoteFile(path) && ll.local {
cleanPath := cleanFilePath(ll.fSys, filesys.ConfirmedDir(ll.Root()), path)
cleanAbs := filepath.Join(ll.Root(), cleanPath)
Expand Down
34 changes: 29 additions & 5 deletions api/internal/localizer/locloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,11 +276,11 @@ func TestLoadFails(t *testing.T) {
checkNewLoader(req, ldr, &args, "/a", "/a", "/a/newDir", fSys)

cases := map[string]string{
"absolute path": "/a/pod.yaml",
"directory": "b",
"non-existent file": "kubectl.yaml",
"file outside root": "../alpha/beta/gamma/delta/deployment.yaml",
"inside dst": "newDir/pod.yaml",
"absolute path escaping root": "/a/../pod.yaml",
"directory": "b",
"non-existent file": "kubectl.yaml",
"file outside root": "../alpha/beta/gamma/delta/deployment.yaml",
"inside dst": "newDir/pod.yaml",
}
for name, file := range cases {
file := file
Expand All @@ -298,3 +298,27 @@ func TestLoadFails(t *testing.T) {
})
}
}

func TestLoadAbsolute(t *testing.T) {
cases := map[string]string{
"absolute path": "/a/pod.yaml",
"absolute path subdirectory": "/a/test/pod.yaml",
"absolute path upwards traversal": "/a/test/../pod.yaml",
}
for name, file := range cases {
file := file
t.Run(name, func(t *testing.T) {
req := require.New(t)
fSys := makeMemoryFs(t)

ldr, _, err := NewLoader("./a/../a", "/a/../a", "/a/newDir", fSys)
req.NoError(err)

req.NoError(fSys.WriteFile("/a/pod.yaml", []byte(podConfiguration)))
req.NoError(fSys.WriteFile("/a/test/pod.yaml", []byte(podConfiguration)))

_, err = ldr.Load(file)
req.NoError(err)
})
}
}
6 changes: 5 additions & 1 deletion api/internal/localizer/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,11 @@ func hasRef(repoURL string) bool {

// cleanFilePath returns file cleaned, where file is a relative path to root on fSys
func cleanFilePath(fSys filesys.FileSystem, root filesys.ConfirmedDir, file string) string {
abs := root.Join(file)
abs := file
if !filepath.IsAbs(file) {
abs = root.Join(file)
}

dir, f, err := fSys.CleanedAbs(abs)
if err != nil {
log.Fatalf("cannot clean validated file path %q: %s", abs, err)
Expand Down
4 changes: 2 additions & 2 deletions api/krusty/accumulation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ resources:
resource: "file-or-directory",
isAbsolute: true,
errFile: `evalsymlink failure on '%s' .+`,
errDir: `new root '%s' cannot be absolute`,
errDir: `must build at directory: not a valid directory: evalsymlink failure .+`,
},
{
name: "relative file violates restrictions",
Expand All @@ -281,7 +281,7 @@ resources:
errFile: "security; file '%s' is not in or below .+",
// TODO(4348): Over-inclusion of directory error message when we
// know resource is file.
errDir: `new root '%s' cannot be absolute`,
errDir: `referenced absolute path '%s' in your Kustomization file is not in the same tree as .+`,
},
} {
t.Run(test.name, func(t *testing.T) {
Expand Down

0 comments on commit 3ec212e

Please sign in to comment.