Skip to content

Commit

Permalink
refactor
Browse files Browse the repository at this point in the history
Resolve the link target one level higher such that it can be used
when creating the tar header without repetition.
  • Loading branch information
lkingland committed Feb 16, 2024
1 parent da57b32 commit a0022b2
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 25 deletions.
3 changes: 2 additions & 1 deletion pkg/oci/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
37 changes: 18 additions & 19 deletions pkg/oci/containerize.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/json"
"fmt"
"io"
"io/fs"
"os"
slashpath "path"
"path/filepath"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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.

Check failure on line 196 in pkg/oci/containerize.go

View workflow job for this annotation

GitHub Actions / style / Golang / Lint

[github.com/client9/misspell] reported by reviewdog 🐶 "referrs" is a misspelling of "refers" Raw Output: pkg/oci/containerize.go:196:6: "referrs" is a misspelling of "refers"
// 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
Expand All @@ -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
Expand Down
9 changes: 4 additions & 5 deletions pkg/oci/containerize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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"},
Expand All @@ -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)
}
Expand All @@ -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")
}
Expand Down

0 comments on commit a0022b2

Please sign in to comment.