Skip to content

Commit

Permalink
bake: change evaluation of entitlement paths
Browse files Browse the repository at this point in the history
Currently, to compare the local path used by bake against the paths allowed
by entitlements, symlinks were evaluated for path normalization so that the
local path used by build was allowed to not exist while the path allowed by
entitlement needed to exist. If the path used by the build did not exist,
then the deepest existing parent path was used instead. This was concistent
with entitlement rules as that parent path would be the actual path access
is needed.

This raised an issue with `--set` if one provides a non-existing path as
an argument, as these paths are supposed to be allowed automatically. With
the above restrictions set to allowed paths, this meant the build would fail
as it can't grant entitlement to the non-existing paths.

This changes the evaluation logic for allowing paths so that they do not
need to exist. If such a case appears, then the path is evaluated to the
last component that exists, and then the rest of the path is appended as is.

This means that for example, if `output = /tmp/out/foo/` is set in HCL
and `/tmp` is the last component that exists then invoking build with
`--allow fs.write=/tmp/out/foo` will not fail with stat error anymore
but will fail in entitlements validation as build would also need to
write `/tmp/out` that is not inside the allowed `/tmp/out/foo` path. The
same would apply to `--set` as well so that if it points to
a non-existing path, then an additional `--allow` rule is needed
providing access to writing to the last existing component of that path.
This may or may not be unexpected.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
  • Loading branch information
tonistiigi committed Dec 16, 2024
1 parent 3e3242c commit bb4161c
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 13 deletions.
31 changes: 20 additions & 11 deletions bake/entitlements.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/docker/buildx/util/osutil"
"github.com/moby/buildkit/util/entitlements"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

type EntitlementKey string
Expand Down Expand Up @@ -443,10 +444,18 @@ func evaluatePaths(in []string) ([]string, bool, error) {
continue
}
v, err := filepath.Abs(p)
if err != nil {
logrus.Warnf("failed to evaluate entitlement path %q: %v", p, err)
continue
}
v, rest, err := evaluateToExistingPath(v)
if err != nil {
return nil, false, errors.Wrapf(err, "failed to evaluate path %q", p)
}
v, err = filepath.EvalSymlinks(v)
if rest != "" {
v = filepath.Join(v, rest)
}
v, err = osutil.GetLongPathName(v)
if err != nil {
return nil, false, errors.Wrapf(err, "failed to evaluate path %q", p)
}
Expand All @@ -458,7 +467,7 @@ func evaluatePaths(in []string) ([]string, bool, error) {
func evaluateToExistingPaths(in map[string]struct{}) (map[string]struct{}, error) {
m := make(map[string]struct{}, len(in))
for p := range in {
v, err := evaluateToExistingPath(p)
v, _, err := evaluateToExistingPath(p)
if err != nil {
return nil, errors.Wrapf(err, "failed to evaluate path %q", p)
}
Expand All @@ -471,10 +480,10 @@ func evaluateToExistingPaths(in map[string]struct{}) (map[string]struct{}, error
return m, nil
}

func evaluateToExistingPath(in string) (string, error) {
func evaluateToExistingPath(in string) (string, string, error) {
in, err := filepath.Abs(in)
if err != nil {
return "", err
return "", "", err
}

volLen := volumeNameLen(in)
Expand Down Expand Up @@ -529,29 +538,29 @@ func evaluateToExistingPath(in string) (string, error) {
if os.IsNotExist(err) {
for r := len(dest) - 1; r >= volLen; r-- {
if os.IsPathSeparator(dest[r]) {
return dest[:r], nil
return dest[:r], in[start:], nil
}
}
return vol, nil
return vol, in[start:], nil
}
return "", err
return "", "", err
}

if fi.Mode()&fs.ModeSymlink == 0 {
if !fi.Mode().IsDir() && end < len(in) {
return "", syscall.ENOTDIR
return "", "", syscall.ENOTDIR
}
continue
}

linksWalked++
if linksWalked > 255 {
return "", errors.New("too many symlinks")
return "", "", errors.New("too many symlinks")
}

link, err := os.Readlink(dest)
if err != nil {
return "", err
return "", "", err
}

in = link + in[end:]
Expand Down Expand Up @@ -584,7 +593,7 @@ func evaluateToExistingPath(in string) (string, error) {
end = 0
}
}
return filepath.Clean(dest), nil
return filepath.Clean(dest), "", nil
}

func volumeNameLen(s string) int {
Expand Down
4 changes: 2 additions & 2 deletions bake/entitlements_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func TestEvaluateToExistingPath(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result, err := evaluateToExistingPath(tt.input)
result, _, err := evaluateToExistingPath(tt.input)

if tt.expectErr {
require.Error(t, err)
Expand Down Expand Up @@ -341,7 +341,7 @@ func TestValidateEntitlements(t *testing.T) {
return nil
}
// if not, then escapeLink is not allowed
exp, err := evaluateToExistingPath(escapeLink)
exp, _, err := evaluateToExistingPath(escapeLink)
require.NoError(t, err)
exp, err = filepath.EvalSymlinks(exp)
require.NoError(t, err)
Expand Down

0 comments on commit bb4161c

Please sign in to comment.