policy: normalize local policy paths

build/policy_loader.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/docker/buildx/policy"
+	"github.com/docker/buildx/util/osutil"
 	"github.com/docker/buildx/util/sourcemeta"
 	"github.com/moby/buildkit/client/llb"
 	gwclient "github.com/moby/buildkit/frontend/gateway/client"
@@ -152,11 +153,21 @@ func newPolicyPathFS(ctx context.Context, resolver *sourcemeta.Resolver, popt po
 	}
 
 	return func() (fs.StatFS, func() error, error) {
-		return p, p.Close, nil
+		ref := &policyPathFSRef{policyPathFS: p}
+		return ref, ref.Close, nil
 	}
 }
 
-func (p *policyPathFS) Open(name string) (fs.File, error) {
+type policyPathFSRef struct {
+	*policyPathFS
+	mu           sync.Mutex
+	cwdRoot      fs.StatFS
+	cwdClose     func() error
+	contextRoot  fs.StatFS
+	contextClose func() error
+}
+
+func (p *policyPathFSRef) Open(name string) (fs.File, error) {
 	backend, target, err := p.resolve(name)
 	if err != nil {
 		return nil, err
@@ -167,7 +178,7 @@ func (p *policyPathFS) Open(name string) (fs.File, error) {
 	return backend.Open(target)
 }
 
-func (p *policyPathFS) Stat(name string) (fs.FileInfo, error) {
+func (p *policyPathFSRef) Stat(name string) (fs.FileInfo, error) {
 	backend, target, err := p.resolve(name)
 	if err != nil {
 		return nil, err
@@ -178,29 +189,22 @@ func (p *policyPathFS) Stat(name string) (fs.FileInfo, error) {
 	return backend.Stat(target)
 }
 
-func (p *policyPathFS) Close() error {
-	if err := p.cwdFS.close(); err != nil {
-		return err
-	}
-	return p.contextFS.close()
-}
-
-func (p *policyPathFS) resolve(name string) (fs.StatFS, string, error) {
+func (p *policyPathFSRef) resolve(name string) (fs.StatFS, string, error) {
 	if name == "" {
 		return nil, "", errors.New("policy filename is empty")
 	}
 	if v, ok := strings.CutPrefix(name, "cwd://"); ok {
 		if v == "" {
 			return nil, "", errors.Errorf("invalid policy filename %q", name)
 		}
-		cwd, err := p.cwdFS.get()
+		cwd, err := p.getCwdFS()
 		if err != nil {
 			return nil, "", err
 		}
-		return cwd, filepath.Clean(v), nil
+		return cwd, osutil.SanitizePath(v), nil
 	}
 
-	contextFS, err := p.contextFS.get()
+	contextFS, err := p.getContextFS()
 	if err != nil {
 		return nil, "", err
 	}
@@ -214,16 +218,72 @@ func (p *policyPathFS) resolve(name string) (fs.StatFS, string, error) {
 	return contextFS, normalizeLocalPolicyPath(name, p.contextDir), nil
 }
 
+func (p *policyPathFSRef) getCwdFS() (fs.StatFS, error) {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	if p.cwdClose != nil {
+		return p.cwdRoot, nil
+	}
+	cwd, err := p.cwdFS.get()
+	if err != nil {
+		return nil, err
+	}
+	p.cwdRoot = cwd
+	p.cwdClose = p.cwdFS.close
+	return cwd, nil
+}
+
+func (p *policyPathFSRef) getContextFS() (fs.StatFS, error) {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	if p.contextClose != nil {
+		return p.contextRoot, nil
+	}
+	contextFS, err := p.contextFS.get()
+	if err != nil {
+		return nil, err
+	}
+	p.contextRoot = contextFS
+	p.contextClose = p.contextFS.close
+	return contextFS, nil
+}
+
+func (p *policyPathFSRef) Close() error {
+	p.mu.Lock()
+	cwdClose := p.cwdClose
+	contextClose := p.contextClose
+	p.cwdRoot = nil
+	p.contextRoot = nil
+	p.cwdClose = nil
+	p.contextClose = nil
+	p.mu.Unlock()
+
+	var firstErr error
+	if cwdClose != nil {
+		if err := cwdClose(); err != nil {
+			firstErr = err
+		}
+	}
+	if contextClose != nil {
+		if err := contextClose(); err != nil {
+			if firstErr == nil {
+				firstErr = err
+			}
+		}
+	}
+	return firstErr
+}
+
 func normalizeLocalPolicyPath(name, contextDir string) string {
 	if filepath.IsAbs(name) && contextDir != "" {
 		if rel, err := filepath.Rel(contextDir, name); err == nil {
 			rel = filepath.Clean(rel)
 			if rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
-				return rel
+				return osutil.SanitizePath(rel)
 			}
 		}
 	}
-	return filepath.Clean(name)
+	return osutil.SanitizePath(name)
 }
 
 type memoizedPolicyFS struct {
@@ -405,7 +465,7 @@ func (i policyFileInfo) Sys() any {
 	return &types.Stat{Mode: uint32(i.mode), Size: i.size, ModTime: i.tm.UnixNano()}
 }
 
-var _ fs.StatFS = (*policyPathFS)(nil)
+var _ fs.StatFS = (*policyPathFSRef)(nil)
 var _ fs.StatFS = (*remotePolicyFS)(nil)
 var _ fs.File = (*policyReadFile)(nil)
 var _ io.ReaderAt = (*bytes.Reader)(nil)

build/policy_loader_test.go

@@ -1,13 +1,65 @@
 package build
 
 import (
+	"context"
 	"io/fs"
+	"os"
+	"path/filepath"
 	"testing"
 	"testing/fstest"
 
 	"github.com/stretchr/testify/require"
 )
 
+func TestLoadPolicyDataLocalPaths(t *testing.T) {
+	dir := t.TempDir()
+	policyData := []byte("package docker\n")
+	policyRelPath := filepath.Join("policy", "allow.rego")
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "policy"), 0700))
+	require.NoError(t, os.WriteFile(filepath.Join(dir, policyRelPath), policyData, 0600))
+
+	t.Run("context-relative", func(t *testing.T) {
+		provider := newPolicyPathFS(context.Background(), nil, policyOpt{
+			ContextDir: dir,
+		})
+
+		dt, ok, err := loadPolicyData(provider, policyRelPath)
+		require.NoError(t, err)
+		require.True(t, ok)
+		require.Equal(t, policyData, dt)
+	})
+
+	t.Run("context-absolute", func(t *testing.T) {
+		provider := newPolicyPathFS(context.Background(), nil, policyOpt{
+			ContextDir: dir,
+		})
+
+		dt, ok, err := loadPolicyData(provider, filepath.Join(dir, policyRelPath))
+		require.NoError(t, err)
+		require.True(t, ok)
+		require.Equal(t, policyData, dt)
+	})
+
+	t.Run("cwd", func(t *testing.T) {
+		cwd, err := os.Getwd()
+		require.NoError(t, err)
+		require.NoError(t, os.Chdir(dir))
+		t.Cleanup(func() {
+			require.NoError(t, os.Chdir(cwd))
+		})
+
+		provider := newPolicyPathFS(context.Background(), nil, policyOpt{})
+		dt, ok, err := loadPolicyData(provider, "cwd://"+policyRelPath)
+		require.NoError(t, err)
+		require.True(t, ok)
+		require.Equal(t, policyData, dt)
+	})
+}
+
+func TestNormalizeLocalPolicyPath(t *testing.T) {
+	require.Equal(t, "policy/allow.rego", normalizeLocalPolicyPath(filepath.Join("policy", "allow.rego"), ""))
+}
+
 func TestMemoizedPolicyFSRefCountedClose(t *testing.T) {
 	var initCalls int
 	var closeCalls int
@@ -83,3 +135,61 @@ func TestMemoizedPolicyFSReinitializesAfterAllRefsClosed(t *testing.T) {
 	require.NoError(t, m.close())
 	require.Equal(t, 2, closeCalls)
 }
+
+func TestLoadPolicyDataReleasesPolicyDir(t *testing.T) {
+	dir := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "policy.rego"), []byte("package docker\n"), 0600))
+
+	provider := newPolicyPathFS(context.Background(), nil, policyOpt{
+		ContextDir: dir,
+	})
+
+	_, ok, err := loadPolicyData(provider, "policy.rego")
+	require.NoError(t, err)
+	require.True(t, ok)
+	require.NoError(t, os.RemoveAll(dir))
+}
+
+func TestPolicyPathFSRefClose(t *testing.T) {
+	var initCalls int
+	var closeCalls int
+
+	p := &policyPathFS{}
+	p.contextFS.init = func() (fs.StatFS, func() error, error) {
+		initCalls++
+		root := fstest.MapFS{
+			"policy.rego": &fstest.MapFile{Data: []byte("package docker\n")},
+		}
+		return root, func() error {
+			closeCalls++
+			return nil
+		}, nil
+	}
+
+	first := &policyPathFSRef{policyPathFS: p}
+	_, err := first.Stat("policy.rego")
+	require.NoError(t, err)
+	f, err := first.Open("policy.rego")
+	require.NoError(t, err)
+	require.NoError(t, f.Close())
+	require.Equal(t, 1, initCalls)
+
+	require.NoError(t, first.Close())
+	require.Equal(t, 1, closeCalls)
+
+	shared := &policyPathFSRef{policyPathFS: p}
+	_, err = shared.Stat("policy.rego")
+	require.NoError(t, err)
+	require.Equal(t, 2, initCalls)
+
+	second := &policyPathFSRef{policyPathFS: p}
+	_, err = second.Stat("policy.rego")
+	require.NoError(t, err)
+	require.Equal(t, 2, initCalls)
+
+	require.NoError(t, second.Close())
+	require.Equal(t, 1, closeCalls)
+
+	require.NoError(t, shared.Close())
+	require.Equal(t, 2, closeCalls)
+}