diff --git a/Makefile b/Makefile index 77626d22c..02077b7d1 100644 --- a/Makefile +++ b/Makefile @@ -53,6 +53,6 @@ test: .gofmt .govet .golint .gotest .golint: golint -set_exit_status $(PACKAGES) -UTDIRS = ./validate/... +UTDIRS = ./filepath/... ./validate/... .gotest: go test $(UTDIRS) diff --git a/filepath/abs.go b/filepath/abs.go new file mode 100644 index 000000000..c19bba26a --- /dev/null +++ b/filepath/abs.go @@ -0,0 +1,52 @@ +package filepath + +import ( + "errors" + "regexp" + "strings" +) + +var windowsAbs = regexp.MustCompile(`^[a-zA-Z]:\\.*$`) + +// Abs is a version of path/filepath's Abs with an explicit operating +// system and current working directory. +func Abs(os, path, cwd string) (_ string, err error) { + if os == "windows" { + return "", errors.New("Abs() does not support windows yet") + } + if IsAbs(os, path) { + return Clean(os, path), nil + } + return Clean(os, Join(os, cwd, path)), nil +} + +// IsAbs is a version of path/filepath's IsAbs with an explicit +// operating system. +func IsAbs(os, path string) bool { + if os == "windows" { + // FIXME: copy hideous logic from Go's + // src/path/filepath/path_windows.go into somewhere where we can + // put 3-clause BSD licensed code. + return windowsAbs.MatchString(path) + } + sep := Separator(os) + + // POSIX has [1]: + // + // > If a pathname begins with two successive characters, + // > the first component following the leading characters + // > may be interpreted in an implementation-defined manner, + // > although more than two leading characters shall be + // > treated as a single character. + // + // And Boost treats // as non-absolute [2], but Linux [3,4], Python + // [5] and Go [6] all treat // as absolute. + // + // [1]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13 + // [2]: https://github.com/boostorg/filesystem/blob/boost-1.64.0/test/path_test.cpp#L861 + // [3]: http://man7.org/linux/man-pages/man7/path_resolution.7.html + // [4]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/filesystems/path-lookup.md?h=v4.12#n41 + // [5]: https://github.com/python/cpython/blob/v3.6.1/Lib/posixpath.py#L64-L66 + // [6]: https://go.googlesource.com/go/+/go1.8.3/src/path/path.go#199 + return strings.HasPrefix(path, string(sep)) +} diff --git a/filepath/abs_test.go b/filepath/abs_test.go new file mode 100644 index 000000000..7e48c5fa2 --- /dev/null +++ b/filepath/abs_test.go @@ -0,0 +1,171 @@ +package filepath + +import ( + "fmt" + "testing" +) + +func TestAbs(t *testing.T) { + for _, test := range []struct { + os string + path string + cwd string + expected string + }{ + { + os: "linux", + path: "/", + cwd: "/cwd", + expected: "/", + }, + { + os: "linux", + path: "/a", + cwd: "/cwd", + expected: "/a", + }, + { + os: "linux", + path: "/a/", + cwd: "/cwd", + expected: "/a", + }, + { + os: "linux", + path: "//a", + cwd: "/cwd", + expected: "/a", + }, + { + os: "linux", + path: ".", + cwd: "/cwd", + expected: "/cwd", + }, + { + os: "linux", + path: "./c", + cwd: "/a/b", + expected: "/a/b/c", + }, + { + os: "linux", + path: ".//c", + cwd: "/a/b", + expected: "/a/b/c", + }, + { + os: "linux", + path: "../a", + cwd: "/cwd", + expected: "/a", + }, + { + os: "linux", + path: "../../b", + cwd: "/cwd", + expected: "/b", + }, + } { + t.Run( + fmt.Sprintf("Abs(%q,%q,%q)", test.os, test.path, test.cwd), + func(t *testing.T) { + abs, err := Abs(test.os, test.path, test.cwd) + if err != nil { + t.Error(err) + } else if abs != test.expected { + t.Errorf("unexpected result: %q (expected %q)\n", abs, test.expected) + } + }, + ) + } +} + +func TestIsAbs(t *testing.T) { + for _, test := range []struct { + os string + path string + expected bool + }{ + { + os: "linux", + path: "/", + expected: true, + }, + { + os: "linux", + path: "/a", + expected: true, + }, + { + os: "linux", + path: "//", + expected: true, + }, + { + os: "linux", + path: "//a", + expected: true, + }, + { + os: "linux", + path: ".", + expected: false, + }, + { + os: "linux", + path: "./a", + expected: false, + }, + { + os: "linux", + path: ".//a", + expected: false, + }, + { + os: "linux", + path: "../a", + expected: false, + }, + { + os: "linux", + path: "../../a", + expected: false, + }, + { + os: "windows", + path: "c:\\", + expected: true, + }, + { + os: "windows", + path: "c:\\a", + expected: true, + }, + { + os: "windows", + path: ".", + expected: false, + }, + { + os: "windows", + path: ".\\a", + expected: false, + }, + { + os: "windows", + path: "..\\a", + expected: false, + }, + } { + t.Run( + fmt.Sprintf("IsAbs(%q,%q)", test.os, test.path), + func(t *testing.T) { + abs := IsAbs(test.os, test.path) + if abs != test.expected { + t.Errorf("unexpected result: %t (expected %t)\n", abs, test.expected) + } + }, + ) + } +} diff --git a/filepath/ancestor.go b/filepath/ancestor.go new file mode 100644 index 000000000..896cd8206 --- /dev/null +++ b/filepath/ancestor.go @@ -0,0 +1,32 @@ +package filepath + +import ( + "fmt" + "strings" +) + +// IsAncestor returns true when pathB is an strict ancestor of pathA, +// and false where the paths are equal or pathB is outside of pathA. +// Paths that are not absolute will be made absolute with Abs. +func IsAncestor(os, pathA, pathB, cwd string) (_ bool, err error) { + if pathA == pathB { + return false, nil + } + + pathA, err = Abs(os, pathA, cwd) + if err != nil { + return false, err + } + pathB, err = Abs(os, pathB, cwd) + if err != nil { + return false, err + } + sep := Separator(os) + if !strings.HasSuffix(pathA, string(sep)) { + pathA = fmt.Sprintf("%s%c", pathA, sep) + } + if pathA == pathB { + return false, nil + } + return strings.HasPrefix(pathB, pathA), nil +} diff --git a/filepath/ancestor_test.go b/filepath/ancestor_test.go new file mode 100644 index 000000000..bcbe8345a --- /dev/null +++ b/filepath/ancestor_test.go @@ -0,0 +1,113 @@ +package filepath + +import ( + "fmt" + "testing" +) + +func TestIsAncestor(t *testing.T) { + for _, test := range []struct { + os string + pathA string + pathB string + cwd string + expected bool + }{ + { + os: "linux", + pathA: "/", + pathB: "/a", + cwd: "/cwd", + expected: true, + }, + { + os: "linux", + pathA: "/a", + pathB: "/a", + cwd: "/cwd", + expected: false, + }, + { + os: "linux", + pathA: "/a", + pathB: "/", + cwd: "/cwd", + expected: false, + }, + { + os: "linux", + pathA: "/a", + pathB: "/ab", + cwd: "/cwd", + expected: false, + }, + { + os: "linux", + pathA: "/a/", + pathB: "/a", + cwd: "/cwd", + expected: false, + }, + { + os: "linux", + pathA: "//a", + pathB: "/a", + cwd: "/cwd", + expected: false, + }, + { + os: "linux", + pathA: "//a", + pathB: "/a/b", + cwd: "/cwd", + expected: true, + }, + { + os: "linux", + pathA: "/a", + pathB: "/a/", + cwd: "/cwd", + expected: false, + }, + { + os: "linux", + pathA: "/a", + pathB: ".", + cwd: "/cwd", + expected: false, + }, + { + os: "linux", + pathA: "/a", + pathB: "b", + cwd: "/a", + expected: true, + }, + { + os: "linux", + pathA: "/a", + pathB: "../a", + cwd: "/cwd", + expected: false, + }, + { + os: "linux", + pathA: "/a", + pathB: "../a/b", + cwd: "/cwd", + expected: true, + }, + } { + t.Run( + fmt.Sprintf("IsAncestor(%q,%q,%q,%q)", test.os, test.pathA, test.pathB, test.cwd), + func(t *testing.T) { + ancestor, err := IsAncestor(test.os, test.pathA, test.pathB, test.cwd) + if err != nil { + t.Error(err) + } else if ancestor != test.expected { + t.Errorf("unexpected result: %t", ancestor) + } + }, + ) + } +} diff --git a/filepath/clean.go b/filepath/clean.go new file mode 100644 index 000000000..b70c575f2 --- /dev/null +++ b/filepath/clean.go @@ -0,0 +1,56 @@ +package filepath + +import ( + "fmt" + "strings" +) + +// Clean is an explicit-OS version of path/filepath's Clean. +func Clean(os, path string) string { + abs := IsAbs(os, path) + sep := Separator(os) + elements := strings.Split(path, string(sep)) + + // Replace multiple Separator elements with a single one. + for i := 0; i < len(elements); i++ { + if len(elements[i]) == 0 { + elements = append(elements[:i], elements[i+1:]...) + i-- + } + } + + // Eliminate each . path name element (the current directory). + for i := 0; i < len(elements); i++ { + if elements[i] == "." && len(elements) > 1 { + elements = append(elements[:i], elements[i+1:]...) + i-- + } + } + + // Eliminate each inner .. path name element (the parent directory) + // along with the non-.. element that precedes it. + for i := 1; i < len(elements); i++ { + if i > 0 && elements[i] == ".." { + elements = append(elements[:i-1], elements[i+1:]...) + i -= 2 + } + } + + // Eliminate .. elements that begin a rooted path: + // that is, replace "/.." by "/" at the beginning of a path, + // assuming Separator is '/'. + if abs && len(elements) > 0 { + for elements[0] == ".." { + elements = elements[1:] + } + } + + cleaned := strings.Join(elements, string(sep)) + if abs { + cleaned = fmt.Sprintf("%c%s", sep, cleaned) + } + if cleaned == path { + return path + } + return Clean(os, cleaned) +} diff --git a/filepath/clean_test.go b/filepath/clean_test.go new file mode 100644 index 000000000..b65e7c096 --- /dev/null +++ b/filepath/clean_test.go @@ -0,0 +1,70 @@ +package filepath + +import ( + "fmt" + "testing" +) + +func TestClean(t *testing.T) { + for _, test := range []struct { + os string + path string + expected string + }{ + { + os: "linux", + path: "/", + expected: "/", + }, + { + os: "linux", + path: "//", + expected: "/", + }, + { + os: "linux", + path: "/a", + expected: "/a", + }, + { + os: "linux", + path: "/a/", + expected: "/a", + }, + { + os: "linux", + path: "//a", + expected: "/a", + }, + { + os: "linux", + path: ".", + expected: ".", + }, + { + os: "linux", + path: "./c", + expected: "c", + }, + { + os: "linux", + path: ".././a", + expected: "../a", + }, + { + os: "linux", + path: "a/../b", + expected: "b", + }, + } { + t.Run( + fmt.Sprintf("Clean(%q,%q)", test.os, test.path), + func(t *testing.T) { + clean := Clean(test.os, test.path) + if clean != test.expected { + t.Errorf("unexpected result: %q (expected %q)\n", clean, test.expected) + } + }, + ) + } +} diff --git a/filepath/doc.go b/filepath/doc.go new file mode 100644 index 000000000..7ee085bf4 --- /dev/null +++ b/filepath/doc.go @@ -0,0 +1,6 @@ +// Package filepath implements Go's filepath package with explicit +// operating systems (and for some functions and explicit working +// directory). This allows tools built for one OS to operate on paths +// targeting another OS. For example, a Linux build can determine +// whether a path is absolute on Linux or on Windows. +package filepath diff --git a/filepath/join.go b/filepath/join.go new file mode 100644 index 000000000..b865d237c --- /dev/null +++ b/filepath/join.go @@ -0,0 +1,9 @@ +package filepath + +import "strings" + +// Join is an explicit-OS version of path/filepath's Join. +func Join(os string, elem ...string) string { + sep := Separator(os) + return Clean(os, strings.Join(elem, string(sep))) +} diff --git a/filepath/separator.go b/filepath/separator.go new file mode 100644 index 000000000..2c5e8905a --- /dev/null +++ b/filepath/separator.go @@ -0,0 +1,9 @@ +package filepath + +// Separator is an explicit-OS version of path/filepath's Separator. +func Separator(os string) rune { + if os == "windows" { + return '\\' + } + return '/' +} diff --git a/validate/validate.go b/validate/validate.go index bbad95a8b..1c8eacb68 100644 --- a/validate/validate.go +++ b/validate/validate.go @@ -10,7 +10,6 @@ import ( "os" "path/filepath" "reflect" - "regexp" "runtime" "strings" "syscall" @@ -20,6 +19,7 @@ import ( "github.com/blang/semver" "github.com/hashicorp/go-multierror" rspec "github.com/opencontainers/runtime-spec/specs-go" + osFilepath "github.com/opencontainers/runtime-tools/filepath" "github.com/sirupsen/logrus" "github.com/syndtr/gocapability/capability" @@ -202,18 +202,18 @@ func (v *Validator) CheckHooks() (errs error) { logrus.Debugf("check hooks") if v.spec.Hooks != nil { - errs = multierror.Append(errs, checkEventHooks("pre-start", v.spec.Hooks.Prestart, v.HostSpecific)) - errs = multierror.Append(errs, checkEventHooks("post-start", v.spec.Hooks.Poststart, v.HostSpecific)) - errs = multierror.Append(errs, checkEventHooks("post-stop", v.spec.Hooks.Poststop, v.HostSpecific)) + errs = multierror.Append(errs, v.checkEventHooks("prestart", v.spec.Hooks.Prestart, v.HostSpecific)) + errs = multierror.Append(errs, v.checkEventHooks("poststart", v.spec.Hooks.Poststart, v.HostSpecific)) + errs = multierror.Append(errs, v.checkEventHooks("poststop", v.spec.Hooks.Poststop, v.HostSpecific)) } return } -func checkEventHooks(hookType string, hooks []rspec.Hook, hostSpecific bool) (errs error) { - for _, hook := range hooks { - if !filepath.IsAbs(hook.Path) { - errs = multierror.Append(errs, fmt.Errorf("the %s hook %v: is not absolute path", hookType, hook.Path)) +func (v *Validator) checkEventHooks(hookType string, hooks []rspec.Hook, hostSpecific bool) (errs error) { + for i, hook := range hooks { + if !osFilepath.IsAbs(v.platform, hook.Path) { + errs = multierror.Append(errs, fmt.Errorf("hooks.%s[%d].path %v: is not absolute path", hookType, i, hook.Path)) } if hostSpecific { @@ -245,7 +245,7 @@ func (v *Validator) CheckProcess() (errs error) { } process := v.spec.Process - if !filepath.IsAbs(process.Cwd) { + if !osFilepath.IsAbs(v.platform, process.Cwd) { errs = multierror.Append(errs, fmt.Errorf("cwd %q is not an absolute path", process.Cwd)) } @@ -425,24 +425,15 @@ func (v *Validator) CheckMounts() (errs error) { if supportedTypes != nil && !supportedTypes[mountA.Type] { errs = multierror.Append(errs, fmt.Errorf("unsupported mount type %q", mountA.Type)) } - if v.platform == "windows" { - if err := pathValid(v.platform, mountA.Destination); err != nil { - errs = multierror.Append(errs, err) - } - if err := pathValid(v.platform, mountA.Source); err != nil { - errs = multierror.Append(errs, err) - } - } else { - if err := pathValid(v.platform, mountA.Destination); err != nil { - errs = multierror.Append(errs, err) - } + if !osFilepath.IsAbs(v.platform, mountA.Destination) { + errs = multierror.Append(errs, fmt.Errorf("mounts[%d].destination %q is not absolute", i, mountA.Destination)) } for j, mountB := range v.spec.Mounts { if i == j { continue } // whether B.Desination is nested within A.Destination - nested, err := nestedValid(v.platform, mountA.Destination, mountB.Destination) + nested, err := osFilepath.IsAncestor(v.platform, mountA.Destination, mountB.Destination, ".") if err != nil { errs = multierror.Append(errs, err) continue @@ -502,7 +493,7 @@ func (v *Validator) CheckLinux() (errs error) { for index := 0; index < len(v.spec.Linux.Namespaces); index++ { ns := v.spec.Linux.Namespaces[index] - if !namespaceValid(ns) { + if !v.namespaceValid(ns) { errs = multierror.Append(errs, fmt.Errorf("namespace %v is invalid", ns)) } @@ -834,7 +825,7 @@ func (v *Validator) rlimitValid(rlimit rspec.POSIXRlimit) (errs error) { return } -func namespaceValid(ns rspec.LinuxNamespace) bool { +func (v *Validator) namespaceValid(ns rspec.LinuxNamespace) bool { switch ns.Type { case rspec.PIDNamespace: case rspec.NetworkNamespace: @@ -847,72 +838,13 @@ func namespaceValid(ns rspec.LinuxNamespace) bool { return false } - if ns.Path != "" && !filepath.IsAbs(ns.Path) { + if ns.Path != "" && !osFilepath.IsAbs(v.platform, ns.Path) { return false } return true } -func pathValid(os, path string) error { - if os == "windows" { - matched, err := regexp.MatchString("^[a-zA-Z]:(\\\\[^\\\\/<>|:*?\"]+)+$", path) - if err != nil { - return err - } - if !matched { - return fmt.Errorf("invalid windows path %v", path) - } - return nil - } - if !filepath.IsAbs(path) { - return fmt.Errorf("%v is not an absolute path", path) - } - return nil -} - -// Check whether pathB is nested whithin pathA -func nestedValid(os, pathA, pathB string) (bool, error) { - if pathA == pathB { - return false, nil - } - if pathA == "/" && pathB != "" { - return true, nil - } - - var sep string - if os == "windows" { - sep = "\\" - } else { - sep = "/" - } - - splitedPathA := strings.Split(filepath.Clean(pathA), sep) - splitedPathB := strings.Split(filepath.Clean(pathB), sep) - lenA := len(splitedPathA) - lenB := len(splitedPathB) - - if lenA > lenB { - if (lenA - lenB) == 1 { - // if pathA is longer but not end with separator - if splitedPathA[lenA-1] != "" { - return false, nil - } - splitedPathA = splitedPathA[:lenA-1] - } else { - return false, nil - } - } - - for i, partA := range splitedPathA { - if partA != splitedPathB[i] { - return false, nil - } - } - - return true, nil -} - func deviceValid(d rspec.LinuxDevice) bool { switch d.Type { case "b", "c", "u":