sourcepolicy: normalize parsed source identifiers

Render parsed source identifiers back to their canonical SourceOp form before
source policy evaluation. This lets Git subdir cleanup use the existing source
parser and avoids policy-specific Git parsing.

Add String methods for source identifiers and cover them with unit tests, plus
a client integration regression for canonical Git subdir policy matching.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

client/policy_test.go

@@ -1681,6 +1681,38 @@ func testSourcePolicy(t *testing.T, sb integration.Sandbox) {
 		})
 	}
 
+	t.Run("deny canonical git subdir", func(t *testing.T) {
+		const (
+			src       = "git://github.com/user/repo.git#main:../absolute/path"
+			canonical = "git://github.com/user/repo.git#main:absolute/path"
+		)
+		frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
+			st := llb.NewState(llb.NewSource(src, nil, llb.Constraints{}).Output())
+			def, err := st.Marshal(sb.Context())
+			if err != nil {
+				return nil, err
+			}
+			return c.Solve(ctx, gateway.SolveRequest{
+				Definition: def.ToPB(),
+			})
+		}
+
+		_, err = c.Build(sb.Context(), SolveOpt{
+			SourcePolicy: &sourcepolicypb.Policy{
+				Rules: []*sourcepolicypb.Rule{
+					{
+						Action: sourcepolicypb.PolicyAction_DENY,
+						Selector: &sourcepolicypb.Selector{
+							Identifier: canonical,
+							MatchType:  sourcepolicypb.MatchType_EXACT,
+						},
+					},
+				},
+			},
+		}, "", frontend, nil)
+		require.ErrorContains(t, err, sourcepolicy.ErrSourceDenied.Error())
+	})
+
 	t.Run("Frontend policies", func(t *testing.T) {
 		t.Run("deny http", func(t *testing.T) {
 			denied := "https://raw.githubusercontent.com/moby/buildkit/v0.10.1/README.md"

solver/llbsolver/bridge.go

@@ -163,6 +163,9 @@ func (b *llbBridge) loadResult(ctx context.Context, def *pb.Definition, cacheImp
 }
 
 func (b *llbBridge) policy(engine *sourcepolicy.Engine) SourcePolicyEvaluator {
+	if engine == nil {
+		return nil
+	}
 	return &policyEvaluator{
 		llbBridge: b,
 		engine:    engine,
@@ -279,6 +282,7 @@ func (b *llbBridge) resolveSourceMetadata(ctx context.Context, op *pb.SourceOp,
 	engine := sourcepolicy.NewEngine(opt.SourcePolicies)
 
 	if !withPolicy {
+		op.Identifier = normalizedSourceIdentifier(w, op)
 		if _, err := engine.Evaluate(ctx, op); err != nil {
 			return nil, errors.Wrap(err, "could not resolve image due to policy")
 		}

solver/llbsolver/policy.go

@@ -13,6 +13,7 @@ import (
 	"github.com/moby/buildkit/sourcepolicy"
 	spb "github.com/moby/buildkit/sourcepolicy/pb"
 	"github.com/moby/buildkit/sourcepolicy/policysession"
+	"github.com/moby/buildkit/worker"
 	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 )
@@ -32,6 +33,17 @@ type policyEvaluator struct {
 	engine *sourcepolicy.Engine
 }
 
+func normalizedSourceIdentifier(w worker.Worker, op *pb.SourceOp) string {
+	if w == nil {
+		return op.GetIdentifier()
+	}
+	id, err := w.ParseSource(op, nil)
+	if err != nil {
+		return op.GetIdentifier()
+	}
+	return id.String()
+}
+
 func (p *policyEvaluator) Evaluate(ctx context.Context, op *pb.Op) (bool, error) {
 	return p.evaluate(ctx, op, 10)
 }
@@ -41,6 +53,11 @@ func (p *policyEvaluator) evaluate(ctx context.Context, op *pb.Op, max int) (boo
 	if source == nil {
 		return false, nil
 	}
+	w, err := p.resolveWorker()
+	if err != nil {
+		return false, err
+	}
+	source.Identifier = normalizedSourceIdentifier(w, source)
 	ok, err := p.engine.Evaluate(ctx, source)
 	if err != nil {
 		return false, err

source/containerblob/identifier.go

@@ -42,6 +42,10 @@ func NewImageBlobIdentifier(str string, scheme string) (*ImageBlobIdentifier, er
 
 var _ source.Identifier = (*ImageBlobIdentifier)(nil)
 
+func (id *ImageBlobIdentifier) String() string {
+	return id.Scheme() + "://" + id.Reference.String()
+}
+
 func (id *ImageBlobIdentifier) Scheme() string {
 	if id.SchemeName == "" {
 		return srctypes.DockerImageBlobScheme

source/containerblob/identifier_test.go

@@ -0,0 +1,24 @@
+package containerblob
+
+import (
+	"testing"
+
+	srctypes "github.com/moby/buildkit/source/types"
+	"github.com/stretchr/testify/require"
+)
+
+func TestImageBlobIdentifierString(t *testing.T) {
+	const ref = "docker.io/library/busybox@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+
+	id, err := NewImageBlobIdentifier(ref, srctypes.DockerImageBlobScheme)
+	require.NoError(t, err)
+	require.Equal(t, "docker-image+blob://"+ref, id.String())
+}
+
+func TestOCIBlobIdentifierString(t *testing.T) {
+	const ref = "example.com/repo/layout@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+
+	id, err := NewImageBlobIdentifier(ref, srctypes.OCIBlobScheme)
+	require.NoError(t, err)
+	require.Equal(t, "oci-layout+blob://"+ref, id.String())
+}

source/containerimage/identifier.go

@@ -36,6 +36,10 @@ func NewImageIdentifier(str string) (*ImageIdentifier, error) {
 
 var _ source.Identifier = (*ImageIdentifier)(nil)
 
+func (id *ImageIdentifier) String() string {
+	return srctypes.DockerImageScheme + "://" + id.Reference.String()
+}
+
 func (*ImageIdentifier) Scheme() string {
 	return srctypes.DockerImageScheme
 }
@@ -75,6 +79,10 @@ func NewOCIIdentifier(str string) (*OCIIdentifier, error) {
 
 var _ source.Identifier = (*OCIIdentifier)(nil)
 
+func (id *OCIIdentifier) String() string {
+	return srctypes.OCIScheme + "://" + id.Reference.String()
+}
+
 func (*OCIIdentifier) Scheme() string {
 	return srctypes.OCIScheme
 }

source/containerimage/identifier_test.go

@@ -0,0 +1,19 @@
+package containerimage
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestImageIdentifierString(t *testing.T) {
+	id, err := NewImageIdentifier("docker.io/library/busybox:latest")
+	require.NoError(t, err)
+	require.Equal(t, "docker-image://docker.io/library/busybox:latest", id.String())
+}
+
+func TestOCIIdentifierString(t *testing.T) {
+	id, err := NewOCIIdentifier("example.com/repo/layout:latest")
+	require.NoError(t, err)
+	require.Equal(t, "oci-layout://example.com/repo/layout:latest", id.String())
+}

source/git/identifier.go

@@ -1,6 +1,7 @@
 package git
 
 import (
+	"path"
 	"strings"
 
 	"github.com/containerd/containerd/v2/pkg/reference"
@@ -71,6 +72,22 @@ func NewGitIdentifier(remoteURL string) (*GitIdentifier, error) {
 	return &repo, nil
 }
 
+func (id *GitIdentifier) String() string {
+	remote := id.Remote
+	if u, err := gitutil.ParseURL(remote); err == nil {
+		remote = u.Host + path.Join("/", u.Path)
+	}
+
+	ref := srctypes.GitScheme + "://" + remote
+	if id.Ref != "" || id.Subdir != "" {
+		ref += "#" + id.Ref
+		if id.Subdir != "" {
+			ref += ":" + id.Subdir
+		}
+	}
+	return ref
+}
+
 func (GitIdentifier) Scheme() string {
 	return srctypes.GitScheme
 }

source/git/identifier_test.go

@@ -11,6 +11,7 @@ func TestNewGitIdentifier(t *testing.T) {
 	tests := []struct {
 		url      string
 		expected GitIdentifier
+		str      string
 	}{
 		{
 			url: "ssh://root@subdomain.example.hostname:2222/root/my/really/weird/path/foo.git",
@@ -37,6 +38,7 @@ func TestNewGitIdentifier(t *testing.T) {
 				Remote: "https://github.com/moby/buildkit.git",
 				Ref:    "main",
 			},
+			str: "git://github.com/moby/buildkit.git#main",
 		},
 		{
 			url: "git://github.com/user/repo.git",
@@ -51,6 +53,7 @@ func TestNewGitIdentifier(t *testing.T) {
 				Ref:    "mybranch",
 				Subdir: "mydir/mysubdir",
 			},
+			str: "git://github.com/user/repo.git#mybranch:mydir/mysubdir",
 		},
 		{
 			url: "git://github.com/user/repo.git#:mydir/mysubdir/",
@@ -72,6 +75,7 @@ func TestNewGitIdentifier(t *testing.T) {
 				Ref:    "mybranch",
 				Subdir: "mydir/mysubdir",
 			},
+			str: "git://github.com/user/repo.git#mybranch:mydir/mysubdir",
 		},
 		{
 			url: "git@github.com:user/repo.git",
@@ -132,6 +136,7 @@ func TestNewGitIdentifier(t *testing.T) {
 				Ref:    "main",
 				Subdir: "absolute/path",
 			},
+			str: "git://github.com/user/repo.git#main:absolute/path",
 		},
 		{
 			url: "https://github.com/user/repo.git#main:../",
@@ -171,6 +176,7 @@ func TestNewGitIdentifier(t *testing.T) {
 				Ref:    "main",
 				Subdir: "absolute/path",
 			},
+			str: "git://github.com/user/repo.git#main:absolute/path",
 		},
 	}
 	for _, tt := range tests {
@@ -178,6 +184,9 @@ func TestNewGitIdentifier(t *testing.T) {
 			gi, err := NewGitIdentifier(tt.url)
 			require.NoError(t, err)
 			require.Equal(t, tt.expected, *gi)
+			if tt.str != "" {
+				require.Equal(t, tt.str, gi.String())
+			}
 		})
 	}
 }

source/http/identifier.go

@@ -42,6 +42,10 @@ type HeaderField struct {
 
 var _ source.Identifier = (*HTTPIdentifier)(nil)
 
+func (id *HTTPIdentifier) String() string {
+	return id.URL
+}
+
 func (id *HTTPIdentifier) Scheme() string {
 	if id.TLS {
 		return srctypes.HTTPSScheme

source/http/identifier_test.go

@@ -0,0 +1,19 @@
+package http
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestHTTPIdentifierString(t *testing.T) {
+	id, err := NewHTTPIdentifier("example.com/file.tar.gz", false)
+	require.NoError(t, err)
+	require.Equal(t, "http://example.com/file.tar.gz", id.String())
+}
+
+func TestHTTPSIdentifierString(t *testing.T) {
+	id, err := NewHTTPIdentifier("example.com/file.tar.gz", true)
+	require.NoError(t, err)
+	require.Equal(t, "https://example.com/file.tar.gz", id.String())
+}

source/identifier.go

@@ -11,6 +11,9 @@ var (
 )
 
 type Identifier interface {
+	// String returns the canonical source operation identifier.
+	String() string
+
 	// Scheme returns the scheme of the identifier so that it can be routed back
 	// to an appropriate Source.
 	Scheme() string

source/local/identifier.go

@@ -24,6 +24,10 @@ func NewLocalIdentifier(str string) (*LocalIdentifier, error) {
 	return &LocalIdentifier{Name: str}, nil
 }
 
+func (id *LocalIdentifier) String() string {
+	return srctypes.LocalScheme + "://" + id.Name
+}
+
 func (*LocalIdentifier) Scheme() string {
 	return srctypes.LocalScheme
 }

source/local/identifier_test.go

@@ -0,0 +1,13 @@
+package local
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestLocalIdentifierString(t *testing.T) {
+	id, err := NewLocalIdentifier("context")
+	require.NoError(t, err)
+	require.Equal(t, "local://context", id.String())
+}

worker/base/worker.go

@@ -465,6 +465,10 @@ func (w *Worker) PruneCacheMounts(ctx context.Context, ids map[string]bool) erro
 	return nil
 }
 
+func (w *Worker) ParseSource(op *pb.SourceOp, platform *pb.Platform) (source.Identifier, error) {
+	return w.SourceManager.Identifier(&pb.Op_Source{Source: op}, platform)
+}
+
 func (w *Worker) ResolveSourceMetadata(ctx context.Context, op *pb.SourceOp, opt sourceresolver.Opt, sm *session.Manager, jobCtx solver.JobContext) (*sourceresolver.MetaResponse, error) {
 	if opt.SourcePolicies != nil {
 		return nil, errors.New("source policies can not be set for worker")
@@ -487,7 +491,7 @@ func (w *Worker) ResolveSourceMetadata(ctx context.Context, op *pb.SourceOp, opt
 		}
 	}
 
-	id, err := w.SourceManager.Identifier(&pb.Op_Source{Source: op}, platform)
+	id, err := w.ParseSource(op, platform)
 	if err != nil {
 		return nil, err
 	}

worker/worker.go

@@ -15,6 +15,7 @@ import (
 	"github.com/moby/buildkit/solver"
 	"github.com/moby/buildkit/solver/llbsolver/cdidevices"
 	"github.com/moby/buildkit/solver/pb"
+	"github.com/moby/buildkit/source"
 	"github.com/moby/buildkit/util/leaseutil"
 	"github.com/moby/buildkit/util/network"
 	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
@@ -37,6 +38,7 @@ type Worker interface {
 	LoadRef(ctx context.Context, id string, hidden bool) (cache.ImmutableRef, error)
 	// ResolveOp resolves Vertex.Sys() to Op implementation.
 	ResolveOp(v solver.Vertex, s frontend.FrontendLLBBridge, sm *session.Manager, proxyOpt ProxyOpt) (solver.Op, error)
+	ParseSource(op *pb.SourceOp, platform *pb.Platform) (source.Identifier, error)
 	ResolveSourceMetadata(ctx context.Context, op *pb.SourceOp, opt sourceresolver.Opt, sm *session.Manager, jobCtx solver.JobContext) (*sourceresolver.MetaResponse, error)
 	DiskUsage(ctx context.Context, opt client.DiskUsageInfo) ([]*client.UsageInfo, error)
 	Exporter(name string, sm *session.Manager) (exporter.Exporter, error)