From a0022b245bc5ad7b28b4851248a9bd558fc11a32 Mon Sep 17 00:00:00 2001 From: Luke Kingland Date: Fri, 16 Feb 2024 14:53:33 +0900 Subject: [PATCH] refactor Resolve the link target one level higher such that it can be used when creating the tar header without repetition. --- pkg/oci/builder_test.go | 3 ++- pkg/oci/containerize.go | 37 ++++++++++++++++++------------------ pkg/oci/containerize_test.go | 9 ++++----- 3 files changed, 24 insertions(+), 25 deletions(-) diff --git a/pkg/oci/builder_test.go b/pkg/oci/builder_test.go index c4a8da1b4..2c61732cc 100644 --- a/pkg/oci/builder_test.go +++ b/pkg/oci/builder_test.go @@ -70,9 +70,10 @@ func TestBuilder_Files(t *testing.T) { var linkMode fs.FileMode var linkExecutable bool if runtime.GOOS != "windows" { - // Default: create a symlink + // Default case: use symlinks linkMode = fs.ModeSymlink linkExecutable = true + if err := os.Symlink("a.txt", "a.lnk"); err != nil { t.Fatal(err) } diff --git a/pkg/oci/containerize.go b/pkg/oci/containerize.go index 25edfd08d..4a5f2896c 100644 --- a/pkg/oci/containerize.go +++ b/pkg/oci/containerize.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "io" + "io/fs" "os" slashpath "path" "path/filepath" @@ -152,12 +153,14 @@ func newDataTarball(root, target string, ignored []string, verbose bool) error { } } - // Check for invalid links (absolute, outside of function root, etc) - if err := validateLink(root, path, info); err != nil { - return err + lnk := "" // if link, this will be used as the target + if info.Mode()&fs.ModeSymlink != 0 { + if lnk, err = validateLink(root, path, info); err != nil { + return err + } } - header, err := tar.FileInfoHeader(info, info.Name()) + header, err := tar.FileInfoHeader(info, lnk) if err != nil { return err } @@ -188,26 +191,22 @@ func newDataTarball(root, target string, ignored []string, verbose bool) error { }) } -// validateLink returns an error if the given file is allowed given the -// - Is an absoute link -// - Is a link to something outside of the given function root -// - Errors obtaining this information -func validateLink(root, path string, info os.FileInfo) error { - if info.Mode()&os.ModeSymlink != os.ModeSymlink { - return nil // not a symlink - } - +// validateLink returns the target of a given link and an error if +// that target is either absolute or outside the given project root. +// or referrs to a target outside of the given root. +// For conven +func validateLink(root, path string, info os.FileInfo) (tgt string, err error) { // tgt is the raw target of the link. // This path is either absolute or relative to the link's location. - tgt, err := os.Readlink(path) + tgt, err = os.Readlink(path) if err != nil { - return err + return tgt, fmt.Errorf("cannot read link: %w", err) } // Absolute links will not be correct when copied into the runtime // container, because they are placed into path into '/func', if filepath.IsAbs(tgt) { - return errors.New("project may not contain absolute links") + return tgt, errors.New("project may not contain absolute links") } // Calculate the actual target of the link @@ -218,14 +217,14 @@ func validateLink(root, path string, info os.FileInfo) error { // this actual target location relLnkTgt, err := filepath.Rel(root, lnkTgt) if err != nil { - return err + return tgt, err } // Fail if this path is outside the function's root. if strings.HasPrefix(relLnkTgt, ".."+string(filepath.Separator)) || relLnkTgt == ".." { - return errors.New("links must stay within project root") + return tgt, errors.New("links must stay within project root") } - return nil + return } // newCertLayer creates the shared data layer in the container file hierarchy and diff --git a/pkg/oci/containerize_test.go b/pkg/oci/containerize_test.go index 6817b3274..322cd93f9 100644 --- a/pkg/oci/containerize_test.go +++ b/pkg/oci/containerize_test.go @@ -7,6 +7,8 @@ import ( "testing" ) +// Test_validateLink ensures that the validateLink function disallows +// links which are absolute or refer to targets outside the given root. func Test_validateLink(t *testing.T) { root := "testdata/test-links" @@ -15,11 +17,8 @@ func Test_validateLink(t *testing.T) { valid bool // If it should be considered valid name string // descriptive name of the test }{ - {"a.txt", true, "do not evaluate regular files"}, - {"a.lnk", true, "do not evaluate directories"}, {"absoluteLink", false, "disallow absolute-path links"}, {"a.lnk", true, "links to files within the root are allowed"}, - {"...validName.txt", true, "allow files with dot prefixes"}, {"...validName.lnk", true, "allow links with target of dot prefixed names"}, {"linkToRoot", true, "allow links to the project root"}, {"b/linkToRoot", true, "allow links to the project root from within subdir"}, @@ -35,7 +34,7 @@ func Test_validateLink(t *testing.T) { if err != nil { t.Fatal(err) } - err = validateLink(root, path, info) + _, err = validateLink(root, path, info) if err == nil != tt.valid { t.Fatalf("expected %v, got %v", tt.valid, err) } @@ -49,7 +48,7 @@ func Test_validateLink(t *testing.T) { if err != nil { t.Fatal(err) } - err = validateLink(root, path, info) + _, err = validateLink(root, path, info) if err == nil { t.Fatal("absolute path should be invalid on windows") }