Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: host builder nonregular file support #2156

Merged
merged 12 commits into from
Feb 20, 2024
43 changes: 38 additions & 5 deletions pkg/oci/containerize.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,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
Expand Down Expand Up @@ -141,6 +142,7 @@ func newDataTarball(source, target string, ignored []string, verbose bool) error
return err
}

// Skip files explicitly ignored
for _, v := range ignored {
if info.Name() == v {
if info.IsDir() {
Expand All @@ -150,6 +152,20 @@ func newDataTarball(source, target string, ignored []string, verbose bool) error
}
}

// Error if links exist which reach outside of the function root
if info.Mode()&os.ModeSymlink == os.ModeSymlink {
link, err := os.Readlink(path)
if err != nil {
return err
}
ok, err := isWithin(source, link)
matejvasek marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return errors.Wrap(err, "error checking if link is outside function root")
} else if !ok {
return fmt.Errorf("links outside Function root disallowed. Link %v references %v", path, link)
}
}

header, err := tar.FileInfoHeader(info, info.Name())
matejvasek marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
Expand All @@ -159,18 +175,14 @@ func newDataTarball(source, target string, ignored []string, verbose bool) error
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
}

Expand All @@ -185,6 +197,27 @@ func newDataTarball(source, target string, ignored []string, verbose bool) error
})
}

// isWithin returns true if the parent path has the child path within.
// If parent and child are the same target, this is considered within.
func isWithin(parent, child string) (bool, error) {
var err error
// Convert the paths to clean absolute paths.
if parent, err = filepath.Abs(filepath.Clean(parent)); err != nil {
return false, err
}
if child, err = filepath.Abs(filepath.Clean(child)); err != nil {
return false, err
}
// Calculate the relative path between them
relPath, err := filepath.Rel(parent, child)
if err != nil {
return false, err
}
// Child is not within parent if the relative path from parent to child
// requires ..
return !strings.HasPrefix(relPath, ".."), nil
lkingland marked this conversation as resolved.
Show resolved Hide resolved
}

// 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) {
Expand Down
34 changes: 34 additions & 0 deletions pkg/oci/containerize_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package oci

import "testing"

// Test_isWithin ensures that the isWithin method checks
// that various combinations of parent and child paths.
func Test_isWithin(t *testing.T) {
tests := []struct {
parent string
child string
want bool
name string
}{
{"/", "/b", true, "Base case, a subdir of an absolute path"},
{"/a", "/ab", false, "Ensure simple substring does not match"},
{"./", ".", true, "Ensure links are both made absolute"},
{"/a/b/../c", "/a/c/d/../", true, "Ensure the links are both sanitized"},
{"/a", "/a/b/../../", false, "Ensure escaping the parent is a mismatch"},
{"./", "../", false, "Ensure simple relative mismatch"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := isWithin(tt.parent, tt.child)
if err != nil {
t.Fatal(err)
}
if got != tt.want {
t.Log(tt.name)
t.Errorf("isWithin() = %v, want %v", got, tt.want)
}
})
}

}
Loading