diff --git a/pkg/volume/util/atomic_writer.go b/pkg/volume/util/atomic_writer.go index b1345892878c..0613bef6d9f5 100644 --- a/pkg/volume/util/atomic_writer.go +++ b/pkg/volume/util/atomic_writer.go @@ -230,7 +230,7 @@ func validatePayload(payload map[string]FileProjection) (map[string]FileProjecti return nil, err } - cleanPayload[path.Clean(k)] = content + cleanPayload[filepath.Clean(k)] = content } return cleanPayload, nil diff --git a/pkg/volume/util/atomic_writer_test.go b/pkg/volume/util/atomic_writer_test.go index d80e5ca4cbc3..4f8869b1a7c5 100644 --- a/pkg/volume/util/atomic_writer_test.go +++ b/pkg/volume/util/atomic_writer_test.go @@ -811,3 +811,176 @@ func checkVolumeContents(targetDir, tcName string, payload map[string]FileProjec t.Errorf("%v: payload and observed payload do not match.", tcName) } } + +func TestValidatePayload(t *testing.T) { + maxPath := strings.Repeat("a", maxPathLength+1) + + cases := []struct { + name string + payload map[string]FileProjection + expected sets.String + valid bool + }{ + { + name: "valid payload", + payload: map[string]FileProjection{ + "foo": {}, + "bar": {}, + }, + valid: true, + expected: sets.NewString("foo", "bar"), + }, + { + name: "payload with path length > 4096 is invalid", + payload: map[string]FileProjection{ + maxPath: {}, + }, + valid: false, + }, + { + name: "payload with absolute path is invalid", + payload: map[string]FileProjection{ + "/dev/null": {}, + }, + valid: false, + }, + { + name: "payload with reserved path is invalid", + payload: map[string]FileProjection{ + "..sneaky.txt": {}, + }, + valid: false, + }, + { + name: "payload with doubledot path is invalid", + payload: map[string]FileProjection{ + "foo/../etc/password": {}, + }, + valid: false, + }, + { + name: "payload with empty path is invalid", + payload: map[string]FileProjection{ + "": {}, + }, + valid: false, + }, + { + name: "payload with unclean path should be cleaned", + payload: map[string]FileProjection{ + "foo////bar": {}, + }, + valid: true, + expected: sets.NewString("foo/bar"), + }, + } + getPayloadPaths := func(payload map[string]FileProjection) sets.String { + paths := sets.NewString() + for path := range payload { + paths.Insert(path) + } + return paths + } + + for _, tc := range cases { + real, err := validatePayload(tc.payload) + if !tc.valid && err == nil { + t.Errorf("%v: unexpected success", tc.name) + } + + if tc.valid { + if err != nil { + t.Errorf("%v: unexpected failure: %v", tc.name, err) + continue + } + + realPaths := getPayloadPaths(real) + if !realPaths.Equal(tc.expected) { + t.Errorf("%v: unexpected payload paths: %v is not equal to %v", tc.name, realPaths, tc.expected) + } + } + + } +} + +func TestCreateUserVisibleFiles(t *testing.T) { + cases := []struct { + name string + payload map[string]FileProjection + expected map[string]string + }{ + { + name: "simple path", + payload: map[string]FileProjection{ + "foo": {}, + "bar": {}, + }, + expected: map[string]string{ + "foo": "..data/foo", + "bar": "..data/bar", + }, + }, + { + name: "simple nested path", + payload: map[string]FileProjection{ + "foo/bar": {}, + "foo/bar/txt": {}, + "bar/txt": {}, + }, + expected: map[string]string{ + "foo": "..data/foo", + "bar": "..data/bar", + }, + }, + { + name: "unclean nested path", + payload: map[string]FileProjection{ + "./bar": {}, + "foo///bar": {}, + }, + expected: map[string]string{ + "bar": "..data/bar", + "foo": "..data/foo", + }, + }, + } + + for _, tc := range cases { + targetDir, err := utiltesting.MkTmpdir("atomic-write") + if err != nil { + t.Errorf("%v: unexpected error creating tmp dir: %v", tc.name, err) + continue + } + defer os.RemoveAll(targetDir) + + dataDirPath := path.Join(targetDir, dataDirName) + err = os.MkdirAll(dataDirPath, 0755) + if err != nil { + t.Fatalf("%v: unexpected error creating data path: %v", tc.name, err) + } + + writer := &AtomicWriter{targetDir: targetDir, logContext: "-test-"} + payload, err := validatePayload(tc.payload) + if err != nil { + t.Fatalf("%v: unexpected error validating payload: %v", tc.name, err) + } + err = writer.createUserVisibleFiles(payload) + if err != nil { + t.Fatalf("%v: unexpected error creating visible files: %v", tc.name, err) + } + + for subpath, expectedDest := range tc.expected { + visiblePath := path.Join(targetDir, subpath) + destination, err := os.Readlink(visiblePath) + if err != nil && os.IsNotExist(err) { + t.Fatalf("%v: visible symlink does not exist: %v", tc.name, visiblePath) + } else if err != nil { + t.Fatalf("%v: unable to read symlink %v: %v", tc.name, dataDirPath, err) + } + + if expectedDest != destination { + t.Fatalf("%v: symlink destination %q not same with expected data dir %q", tc.name, destination, expectedDest) + } + } + } +}