github
/
k3s
mirror of https://github.com/k3s-io/k3s
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #80870 from tallclair/automated-cherry-pick-of-#80436-upstream-release-1.14 

Automated cherry pick of #80436: refactors to kubernetes CP command
k3s-v1.14.5
Kubernetes Prow Robot 2019-08-01 16:55:53 -07:00 committed by GitHub
parent 22c2807bce f4f139119b
commit 0e9fcb426b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 108 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
pkg/kubectl/cmd/cp/BUILD
View File

@ -32,6 +32,7 @@ go_test(
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//staging/src/k8s.io/cli-runtime/pkg/genericclioptions:go_default_library",
"//staging/src/k8s.io/client-go/rest/fake:go_default_library",
"//vendor/github.com/stretchr/testify/assert:go_default_library",
"//vendor/github.com/stretchr/testify/require:go_default_library",
],
)

37
pkg/kubectl/cmd/cp/cp.go
View File

@ -441,9 +441,14 @@ func (o *CopyOptions) untarAll(reader io.Reader, destDir, prefix string) error {
// basic file information
mode := header.FileInfo().Mode()
destFileName := path.Join(destDir, header.Name[len(prefix):])
baseName := path.Dir(destFileName)
destFileName := filepath.Join(destDir, header.Name[len(prefix):])
if !isDestRelative(destDir, destFileName) {
fmt.Fprintf(o.IOStreams.ErrOut, "warning: file %q is outside target destination, skipping\n", destFileName)
continue
}
baseName := filepath.Dir(destFileName)
if err := os.MkdirAll(baseName, 0755); err != nil {
return err
}
@ -457,15 +462,14 @@ func (o *CopyOptions) untarAll(reader io.Reader, destDir, prefix string) error {
// We need to ensure that the destination file is always within boundries
// of the destination directory. This prevents any kind of path traversal
// from within tar archive.
dir, file := filepath.Split(destFileName)
evaledPath, err := filepath.EvalSymlinks(dir)
evaledPath, err := filepath.EvalSymlinks(baseName)
if err != nil {
return err
}
// For scrutiny we verify both the actual destination as well as we follow
// all the links that might lead outside of the destination directory.
if !isDestRelative(destDir, destFileName) || !isDestRelative(destDir, filepath.Join(evaledPath, file)) {
fmt.Fprintf(o.IOStreams.ErrOut, "warning: link %q is pointing to %q which is outside target destination, skipping\n", destFileName, header.Linkname)
if !isDestRelative(destDir, filepath.Join(evaledPath, filepath.Base(destFileName))) {
fmt.Fprintf(o.IOStreams.ErrOut, "warning: file %q is outside target destination, skipping\n", destFileName)
continue
}
@ -474,7 +478,11 @@ func (o *CopyOptions) untarAll(reader io.Reader, destDir, prefix string) error {
// We need to ensure that the link destination is always within boundries
// of the destination directory. This prevents any kind of path traversal
// from within tar archive.
if !isDestRelative(destDir, linkJoin(destFileName, linkname)) {
linkTarget := linkname
if !filepath.IsAbs(linkname) {
linkTarget = filepath.Join(evaledPath, linkname)
}
if !isDestRelative(destDir, linkTarget) {
fmt.Fprintf(o.IOStreams.ErrOut, "warning: link %q is pointing to %q which is outside target destination, skipping\n", destFileName, header.Linkname)
continue
}
@ -499,23 +507,10 @@ func (o *CopyOptions) untarAll(reader io.Reader, destDir, prefix string) error {
return nil
}
// linkJoin joins base and link to get the final path to be created.
// It will consider whether link is an absolute path or not when returning result.
func linkJoin(base, link string) string {
if filepath.IsAbs(link) {
return link
}
return filepath.Join(base, link)
}
// isDestRelative returns true if dest is pointing outside the base directory,
// false otherwise.
func isDestRelative(base, dest string) bool {
fullPath := dest
if !filepath.IsAbs(dest) {
fullPath = filepath.Join(base, dest)
}
relative, err := filepath.Rel(base, fullPath)
relative, err := filepath.Rel(base, dest)
if err != nil {
return false
}

159
pkg/kubectl/cmd/cp/cp_test.go
View File

@ -30,6 +30,7 @@ import (
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/api/core/v1"
@ -200,12 +201,12 @@ func TestIsDestRelative(t *testing.T) {
}{
{
base: "/dir",
dest: "../link",
dest: "/dir/../link",
relative: false,
},
{
base: "/dir",
dest: "../../link",
dest: "/dir/../../link",
relative: false,
},
{
@ -213,21 +214,6 @@ func TestIsDestRelative(t *testing.T) {
dest: "/link",
relative: false,
},
{
base: "/dir",
dest: "link",
relative: true,
},
{
base: "/dir",
dest: "int/file/link",
relative: true,
},
{
base: "/dir",
dest: "int/../link",
relative: true,
},
{
base: "/dir",
dest: "/dir/link",
@ -239,8 +225,18 @@ func TestIsDestRelative(t *testing.T) {
relative: true,
},
{
base: "/dir",
dest: "/dir/../../link",
base: "dir",
dest: "dir/link",
relative: true,
},
{
base: "dir",
dest: "dir/int/../link",
relative: true,
},
{
base: "dir",
dest: "dir/../../link",
relative: false,
},
}
@ -744,9 +740,7 @@ func TestUntar(t *testing.T) {
defer os.RemoveAll(testdir)
t.Logf("Test base: %s", testdir)
const (
dest = "base"
)
basedir := filepath.Join(testdir, "base")
type file struct {
path string
@ -755,26 +749,26 @@ func TestUntar(t *testing.T) {
}
files := []file{{
// Absolute file within dest
path: filepath.Join(testdir, dest, "abs"),
expected: filepath.Join(testdir, dest, testdir, dest, "abs"),
path: filepath.Join(basedir, "abs"),
expected: filepath.Join(basedir, basedir, "abs"),
}, { // Absolute file outside dest
path: filepath.Join(testdir, "abs-out"),
expected: filepath.Join(testdir, dest, testdir, "abs-out"),
expected: filepath.Join(basedir, testdir, "abs-out"),
}, { // Absolute nested file within dest
path: filepath.Join(testdir, dest, "nested/nest-abs"),
expected: filepath.Join(testdir, dest, testdir, dest, "nested/nest-abs"),
path: filepath.Join(basedir, "nested/nest-abs"),
expected: filepath.Join(basedir, basedir, "nested/nest-abs"),
}, { // Absolute nested file outside dest
path: filepath.Join(testdir, dest, "nested/../../nest-abs-out"),
expected: filepath.Join(testdir, dest, testdir, "nest-abs-out"),
path: filepath.Join(basedir, "nested/../../nest-abs-out"),
expected: filepath.Join(basedir, testdir, "nest-abs-out"),
}, { // Relative file inside dest
path: "relative",
expected: filepath.Join(testdir, dest, "relative"),
expected: filepath.Join(basedir, "relative"),
}, { // Relative file outside dest
path: "../unrelative",
expected: "",
}, { // Nested relative file inside dest
path: "nested/nest-rel",
expected: filepath.Join(testdir, dest, "nested/nest-rel"),
expected: filepath.Join(basedir, "nested/nest-rel"),
}, { // Nested relative file outside dest
path: "nested/../../nest-unrelative",
expected: "",
@ -786,9 +780,11 @@ func TestUntar(t *testing.T) {
}
return expected + suffix
}
mkBacktickExpectation := func(expected, suffix string) string {
dir, _ := filepath.Split(filepath.Clean(expected))
if len(strings.Split(dir, string(os.PathSeparator))) <= 1 {
mkBacklinkExpectation := func(expected, suffix string) string {
// "resolve" the back link relative to the expectation
targetDir := filepath.Dir(filepath.Dir(expected))
// If the "resolved" target is not nested in basedir, it is escaping.
if !filepath.HasPrefix(targetDir, basedir) {
return ""
}
return expected + suffix
@ -801,17 +797,27 @@ func TestUntar(t *testing.T) {
expected: mkExpectation(f.expected, "-innerlink"),
}, file{
path: f.path + "-innerlink-abs",
linkTarget: filepath.Join(testdir, dest, "link-target"),
linkTarget: filepath.Join(basedir, "link-target"),
expected: mkExpectation(f.expected, "-innerlink-abs"),
}, file{
path: f.path + "-outerlink",
linkTarget: filepath.Join(backtick(f.path), "link-target"),
expected: mkBacktickExpectation(f.expected, "-outerlink"),
path: f.path + "-backlink",
linkTarget: filepath.Join("..", "link-target"),
expected: mkBacklinkExpectation(f.expected, "-backlink"),
}, file{
path: f.path + "-outerlink-abs",
linkTarget: filepath.Join(testdir, "link-target"),
expected: "",
})
if f.expected != "" {
// outerlink is the number of backticks to escape to testdir
outerlink, _ := filepath.Rel(f.expected, testdir)
links = append(links, file{
path: f.path + "outerlink",
linkTarget: filepath.Join(outerlink, "link-target"),
expected: "",
})
}
}
files = append(files, links...)
@ -820,11 +826,11 @@ func TestUntar(t *testing.T) {
file{
path: "nested/again/back-link",
linkTarget: "../../nested",
expected: filepath.Join(testdir, dest, "nested/again/back-link"),
expected: filepath.Join(basedir, "nested/again/back-link"),
},
file{
path: "nested/again/back-link/../../../back-link-file",
expected: filepath.Join(testdir, dest, "back-link-file"),
expected: filepath.Join(basedir, "back-link-file"),
})
// Test chaining back-tick symlinks.
@ -832,20 +838,11 @@ func TestUntar(t *testing.T) {
file{
path: "nested/back-link-first",
linkTarget: "../",
expected: filepath.Join(testdir, dest, "nested/back-link-first"),
expected: filepath.Join(basedir, "nested/back-link-first"),
},
file{
path: "nested/back-link-first/back-link-second",
linkTarget: "../",
expected: filepath.Join(testdir, dest, "back-link-second"),
},
file{
// This case is chaining together symlinks that step back, so that
// if you just look at the target relative to the path it appears
// inside the destination directory, but if you actually follow each
// step of the path you end up outside the destination directory.
path: "nested/back-link-first/back-link-second/back-link-term",
linkTarget: "",
expected: "",
})
@ -885,9 +882,11 @@ func TestUntar(t *testing.T) {
}
tw.Close()
opts := NewCopyOptions(genericclioptions.NewTestIOStreamsDiscard())
// Capture warnings to stderr for debugging.
output := (*testWriter)(t)
opts := NewCopyOptions(genericclioptions.IOStreams{In: &bytes.Buffer{}, Out: output, ErrOut: output})
require.NoError(t, opts.untarAll(buf, filepath.Join(testdir, dest), ""))
require.NoError(t, opts.untarAll(buf, filepath.Join(basedir), ""))
filepath.Walk(testdir, func(path string, info os.FileInfo, err error) error {
if err != nil {
@ -910,10 +909,36 @@ func TestUntar(t *testing.T) {
}
}
// backtick returns a path to one directory up from the target
func backtick(target string) string {
rel, _ := filepath.Rel(filepath.Dir(target), "../")
return rel
func TestUntar_SingleFile(t *testing.T) {
testdir, err := ioutil.TempDir("", "test-untar")
require.NoError(t, err)
defer os.RemoveAll(testdir)
dest := filepath.Join(testdir, "target")
buf := &bytes.Buffer{}
tw := tar.NewWriter(buf)
const (
srcName = "source"
content = "file contents"
)
hdr := &tar.Header{
Name: srcName,
Mode: 0666,
Size: int64(len(content)),
}
require.NoError(t, tw.WriteHeader(hdr))
_, err = tw.Write([]byte(content))
require.NoError(t, err)
tw.Close()
// Capture warnings to stderr for debugging.
output := (*testWriter)(t)
opts := NewCopyOptions(genericclioptions.IOStreams{In: &bytes.Buffer{}, Out: output, ErrOut: output})
require.NoError(t, opts.untarAll(buf, filepath.Join(dest), srcName))
cmpFileData(t, dest, content)
}
func createTmpFile(t *testing.T, filepath, data string) {
@ -931,20 +956,14 @@ func createTmpFile(t *testing.T, filepath, data string) {
}
func cmpFileData(t *testing.T, filePath, data string) {
f, err := os.Open(filePath)
if err != nil {
t.Fatalf("unexpected error: %v", err)
actual, err := ioutil.ReadFile(filePath)
require.NoError(t, err)
assert.EqualValues(t, data, actual)
}
defer f.Close()
buff := &bytes.Buffer{}
if _, err := io.Copy(buff, f); err != nil {
t.Fatal(err)
}
if err := f.Close(); err != nil {
t.Fatal(err)
}
if data != string(buff.Bytes()) {
t.Fatalf("expected: %s, saw: %s", data, string(buff.Bytes()))
}
type testWriter testing.T
func (t *testWriter) Write(p []byte) (n int, err error) {
t.Logf(string(p))
return len(p), nil
}