diff --git a/pkg/oci/builder_test.go b/pkg/oci/builder_test.go index eb4086fc9b..b824a6e691 100644 --- a/pkg/oci/builder_test.go +++ b/pkg/oci/builder_test.go @@ -46,7 +46,69 @@ func TestBuilder_Build(t *testing.T) { last := path(f.Root, fn.RunDataDir, "builds", "last", "oci") - validateOCI(last, t) + validateOCIStructure(last, t) // validate it adheres to the basics of the OCI spec +} + +// TestBuilder_Files ensures that static files are added to the container +// image as expected. This includes template files, regular files and links. +func TestBuilder_Files(t *testing.T) { + root, done := Mktemp(t) + defer done() + + // Create a function with the default template + f, err := fn.New().Init(fn.Function{Root: root, Runtime: "go"}) + if err != nil { + t.Fatal(err) + } + + // Add a regular file + if err := os.WriteFile("a.txt", []byte("file a"), 0644); err != nil { + t.Fatal(err) + } + + // Links + var link struct { + Target string + Mode fs.FileMode + Executable bool + } + if runtime.GOOS != "windows" { + // Default case: use symlinks + link.Target = "a.txt" + link.Mode = fs.ModeSymlink + link.Executable = true + + if err := os.Symlink("a.txt", "a.lnk"); err != nil { + t.Fatal(err) + } + } else { + // Windows: create a copy + if err := os.WriteFile("a.lnk", []byte("file a"), 0644); err != nil { + t.Fatal(err) + } + } + + if err := NewBuilder("", true).Build(context.Background(), f, TestPlatforms); err != nil { + t.Fatal(err) + } + + expected := []fileInfo{ + {Path: "/etc/pki/tls/certs/ca-certificates.crt"}, + {Path: "/etc/ssl/certs/ca-certificates.crt"}, + {Path: "/func", Type: fs.ModeDir}, + {Path: "/func/README.md"}, + {Path: "/func/a.lnk", Linkname: link.Target, Type: link.Mode, Executable: link.Executable}, + {Path: "/func/a.txt"}, + {Path: "/func/f", Executable: true}, + {Path: "/func/func.yaml"}, + {Path: "/func/go.mod"}, + {Path: "/func/handle.go"}, + {Path: "/func/handle_test.go"}, + } + + last := path(f.Root, fn.RunDataDir, "builds", "last", "oci") + + validateOCIFiles(last, expected, t) } // TestBuilder_Concurrency @@ -145,9 +207,9 @@ type ImageIndex struct { } `json:"manifests"` } -// validateOCI performs a cursory check that the given path exists and +// validateOCIStructue performs a cursory check that the given path exists and // has the basics of an OCI compliant structure. -func validateOCI(path string, t *testing.T) { +func validateOCIStructure(path string, t *testing.T) { if _, err := os.Stat(path); err != nil { t.Fatalf("unable to stat output path. %v", path) return @@ -185,9 +247,24 @@ func validateOCI(path string, t *testing.T) { } if len(imageIndex.Manifests) < 1 { - t.Fatal("fewer manifests") + t.Fatal("no manifests") } +} +// validateOCIFiles ensures that the OCI image at path contains files with +// the given attributes. +func validateOCIFiles(path string, expected []fileInfo, t *testing.T) { + // Load the Image Index + bb, err := os.ReadFile(filepath.Join(path, "index.json")) + if err != nil { + t.Fatalf("failed to read index.json: %v", err) + } + var imageIndex ImageIndex + if err = json.Unmarshal(bb, &imageIndex); err != nil { + t.Fatalf("failed to parse index.json: %v", err) + } + + // Load the first manifest digest := strings.TrimPrefix(imageIndex.Manifests[0].Digest, "sha256:") manifestFile := filepath.Join(path, "blobs", "sha256", digest) manifestFileData, err := os.ReadFile(manifestFile) @@ -203,12 +280,6 @@ func validateOCI(path string, t *testing.T) { if err != nil { t.Fatal(err) } - - type fileInfo struct { - Path string - Type fs.FileMode - Executable bool - } var files []fileInfo for _, layer := range mf.Layers { @@ -239,6 +310,7 @@ func validateOCI(path string, t *testing.T) { Path: hdr.Name, Type: hdr.FileInfo().Mode() & fs.ModeType, Executable: (hdr.FileInfo().Mode()&0111 == 0111) && !hdr.FileInfo().IsDir(), + Linkname: hdr.Linkname, }) } }() @@ -247,19 +319,14 @@ func validateOCI(path string, t *testing.T) { return files[i].Path < files[j].Path }) - expectedFiles := []fileInfo{ - {Path: "/etc/pki/tls/certs/ca-certificates.crt"}, - {Path: "/etc/ssl/certs/ca-certificates.crt"}, - {Path: "/func", Type: fs.ModeDir}, - {Path: "/func/README.md"}, - {Path: "/func/f", Executable: true}, - {Path: "/func/func.yaml"}, - {Path: "/func/go.mod"}, - {Path: "/func/handle.go"}, - {Path: "/func/handle_test.go"}, - } - - if diff := cmp.Diff(expectedFiles, files); diff != "" { + if diff := cmp.Diff(expected, files); diff != "" { t.Error("files in oci differ from expectation (-want, +got):", diff) } } + +type fileInfo struct { + Path string + Type fs.FileMode + Executable bool + Linkname string +} diff --git a/pkg/oci/containerize.go b/pkg/oci/containerize.go index 6e5f43c77f..011ecc5e1f 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" @@ -14,6 +15,7 @@ import ( v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/tarball" "github.com/google/go-containerregistry/pkg/v1/types" + "github.com/pkg/errors" ) // languageLayerBuilder builds the layer for the given language whuch may @@ -123,7 +125,7 @@ func newDataLayer(cfg *buildConfig) (desc v1.Descriptor, layer v1.Layer, err err return } -func newDataTarball(source, target string, ignored []string, verbose bool) error { +func newDataTarball(root, target string, ignored []string, verbose bool) error { targetFile, err := os.Create(target) if err != nil { return err @@ -136,11 +138,12 @@ func newDataTarball(source, target string, ignored []string, verbose bool) error tw := tar.NewWriter(gw) defer tw.Close() - return filepath.Walk(source, func(path string, info os.FileInfo, err error) error { + return filepath.Walk(root, func(path string, info os.FileInfo, err error) error { if err != nil { return err } + // Skip files explicitly ignored for _, v := range ignored { if info.Name() == v { if info.IsDir() { @@ -150,27 +153,30 @@ func newDataTarball(source, target string, ignored []string, verbose bool) error } } - header, err := tar.FileInfoHeader(info, info.Name()) + lnk := "" // if link, this will be used as the target + if info.Mode()&fs.ModeSymlink != 0 { + if lnk, err = validatedLinkTarget(root, path); err != nil { + return err + } + } + + header, err := tar.FileInfoHeader(info, lnk) if err != nil { return err } - relPath, err := filepath.Rel(source, path) + relPath, err := filepath.Rel(root, path) if err != nil { return err } - header.Name = slashpath.Join("/func", filepath.ToSlash(relPath)) - // TODO: should we set file timestamps to the build start time of cfg.t? - // header.ModTime = timestampArgument - if err := tw.WriteHeader(header); err != nil { return err } if verbose { fmt.Printf("→ %v \n", header.Name) } - if info.IsDir() { + if !info.Mode().IsRegular() { //nothing more to do for non-regular return nil } @@ -185,6 +191,40 @@ func newDataTarball(source, target string, ignored []string, verbose bool) error }) } +// validatedLinkTarget returns the target of a given link or an error if +// that target is either absolute or outside the given project root. +func validatedLinkTarget(root, path string) (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) + if err != nil { + 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 tgt, errors.New("project may not contain absolute links") + } + + // Calculate the actual target of the link + // (relative to the parent of the symlink) + lnkTgt := filepath.Join(filepath.Dir(path), tgt) + + // Calculate the relative path from the function's root to + // this actual target location + relLnkTgt, err := filepath.Rel(root, lnkTgt) + if err != nil { + return + } + + // Fail if this path is outside the function's root. + if strings.HasPrefix(relLnkTgt, ".."+string(filepath.Separator)) || relLnkTgt == ".." { + return tgt, errors.New("links must stay within project root") + } + return +} + // newCertLayer creates the shared data layer in the container file hierarchy and // returns both its descriptor and layer metadata. func newCertsLayer(cfg *buildConfig) (desc v1.Descriptor, layer v1.Layer, err error) { diff --git a/pkg/oci/containerize_test.go b/pkg/oci/containerize_test.go new file mode 100644 index 0000000000..33acffb461 --- /dev/null +++ b/pkg/oci/containerize_test.go @@ -0,0 +1,55 @@ +package oci + +import ( + "path/filepath" + "runtime" + "testing" +) + +// Test_validatedLinkTaarget ensures that the function disallows +// links which are absolute or refer to targets outside the given root, in +// addition to the basic job of returning the value of reading the link. +func Test_validatedLinkTarget(t *testing.T) { + root := "testdata/test-links" + + // Windows-specific absolute link and link target values: + absoluteLink := "absoluteLink" + linkTarget := "./a.txt" + if runtime.GOOS == "windows" { + absoluteLink = "absoluteLinkWindows" + linkTarget = ".\\a.txt" + } + + tests := []struct { + path string // path of the file within test project root + valid bool // If it should be considered valid + target string // optional test of the returned value (target) + name string // descriptive name of the test + }{ + {absoluteLink, false, "", "disallow absolute-path links on linux"}, + {"a.lnk", true, linkTarget, "spot-check link target"}, + {"a.lnk", true, "", "links to files within the root are allowed"}, + {"...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"}, + {"b/linkToCurrentDir", true, "", "allow links to a subdirectory within the project"}, + {"b/linkToRootsParent", false, "", "disallow links to the project's immediate parent"}, + {"b/linkOutsideRootsParent", false, "", "disallow links outside project root and its parent"}, + {"b/c/linkToParent", true, "", " allow links up, but within project"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := filepath.Join(root, tt.path) + target, err := validatedLinkTarget(root, path) + + if err == nil != tt.valid { + t.Fatalf("expected validity '%v', got '%v'", tt.valid, err) + } + if tt.target != "" && target != tt.target { + t.Fatalf("expected target %q, got %q", tt.target, target) + } + }) + } + +} diff --git a/pkg/oci/testdata/test-links/...validName.lnk b/pkg/oci/testdata/test-links/...validName.lnk new file mode 120000 index 0000000000..444926c99e --- /dev/null +++ b/pkg/oci/testdata/test-links/...validName.lnk @@ -0,0 +1 @@ +...validName.txt \ No newline at end of file diff --git a/pkg/oci/testdata/test-links/...validName.txt b/pkg/oci/testdata/test-links/...validName.txt new file mode 100644 index 0000000000..fa6198b097 --- /dev/null +++ b/pkg/oci/testdata/test-links/...validName.txt @@ -0,0 +1 @@ +validName.txt diff --git a/pkg/oci/testdata/test-links/a.lnk b/pkg/oci/testdata/test-links/a.lnk new file mode 120000 index 0000000000..1891a26c08 --- /dev/null +++ b/pkg/oci/testdata/test-links/a.lnk @@ -0,0 +1 @@ +./a.txt \ No newline at end of file diff --git a/pkg/oci/testdata/test-links/a.txt b/pkg/oci/testdata/test-links/a.txt new file mode 100644 index 0000000000..4ef30bbfe2 --- /dev/null +++ b/pkg/oci/testdata/test-links/a.txt @@ -0,0 +1 @@ +file a diff --git a/pkg/oci/testdata/test-links/absoluteLink b/pkg/oci/testdata/test-links/absoluteLink new file mode 120000 index 0000000000..28434a9033 --- /dev/null +++ b/pkg/oci/testdata/test-links/absoluteLink @@ -0,0 +1 @@ +/var/example/absolute/link \ No newline at end of file diff --git a/pkg/oci/testdata/test-links/absoluteLinkWindows b/pkg/oci/testdata/test-links/absoluteLinkWindows new file mode 120000 index 0000000000..c3aaf3f5c3 --- /dev/null +++ b/pkg/oci/testdata/test-links/absoluteLinkWindows @@ -0,0 +1 @@ +c://some/absolute/path \ No newline at end of file diff --git a/pkg/oci/testdata/test-links/b/c/linkToParent b/pkg/oci/testdata/test-links/b/c/linkToParent new file mode 120000 index 0000000000..a96aa0ea9d --- /dev/null +++ b/pkg/oci/testdata/test-links/b/c/linkToParent @@ -0,0 +1 @@ +.. \ No newline at end of file diff --git a/pkg/oci/testdata/test-links/b/linkOutsideRootsParent b/pkg/oci/testdata/test-links/b/linkOutsideRootsParent new file mode 120000 index 0000000000..a8a4f8c212 --- /dev/null +++ b/pkg/oci/testdata/test-links/b/linkOutsideRootsParent @@ -0,0 +1 @@ +../../.. \ No newline at end of file diff --git a/pkg/oci/testdata/test-links/b/linkToCurrentDir b/pkg/oci/testdata/test-links/b/linkToCurrentDir new file mode 120000 index 0000000000..945c9b46d6 --- /dev/null +++ b/pkg/oci/testdata/test-links/b/linkToCurrentDir @@ -0,0 +1 @@ +. \ No newline at end of file diff --git a/pkg/oci/testdata/test-links/b/linkToRoot b/pkg/oci/testdata/test-links/b/linkToRoot new file mode 120000 index 0000000000..a96aa0ea9d --- /dev/null +++ b/pkg/oci/testdata/test-links/b/linkToRoot @@ -0,0 +1 @@ +.. \ No newline at end of file diff --git a/pkg/oci/testdata/test-links/b/linkToRootsParent b/pkg/oci/testdata/test-links/b/linkToRootsParent new file mode 120000 index 0000000000..c25bddb6dd --- /dev/null +++ b/pkg/oci/testdata/test-links/b/linkToRootsParent @@ -0,0 +1 @@ +../.. \ No newline at end of file diff --git a/pkg/oci/testdata/test-links/linkToRoot b/pkg/oci/testdata/test-links/linkToRoot new file mode 120000 index 0000000000..945c9b46d6 --- /dev/null +++ b/pkg/oci/testdata/test-links/linkToRoot @@ -0,0 +1 @@ +. \ No newline at end of file