diff --git a/build/policy_loader.go b/build/policy_loader.go index 61c68018160a..ec5406adab1a 100644 --- a/build/policy_loader.go +++ b/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,14 +189,7 @@ 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") } @@ -193,14 +197,14 @@ func (p *policyPathFS) resolve(name string) (fs.StatFS, string, error) { 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) diff --git a/build/policy_loader_test.go b/build/policy_loader_test.go index 584f3150435b..32901027f0b6 100644 --- a/build/policy_loader_test.go +++ b/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) +}