oci runtime: preserve file ownership (#7790)

Fixes the issue mentioned here:
https://buildbuddy-corp.slack.com/archives/C05GZM1EGM8/p1729695120908659?thread_ts=1727895825.533649&cid=C05GZM1EGM8

enterprise/server/remote_execution/containers/ociruntime/BUILD

@@ -84,6 +84,7 @@ go_test(
         "//server/testutil/testnetworking",
         "//server/testutil/testregistry",
         "//server/testutil/testshell",
+        "//server/testutil/testtar",
         "//server/util/disk",
         "//server/util/proto",
         "//server/util/status",
@@ -92,8 +93,6 @@ go_test(
         "@com_github_google_go_containerregistry//pkg/crane",
         "@com_github_google_go_containerregistry//pkg/v1:pkg",
         "@com_github_google_go_containerregistry//pkg/v1/mutate",
-        "@com_github_google_go_containerregistry//pkg/v1/partial",
-        "@com_github_google_go_containerregistry//pkg/v1/types",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",
         "@io_bazel_rules_go//go/runfiles:go_default_library",

enterprise/server/remote_execution/containers/ociruntime/ociruntime.go

@@ -1247,7 +1247,7 @@ func downloadLayer(ctx context.Context, layer ctr.Layer, destDir string) error {
 	defer os.RemoveAll(tempUnpackDir)
 
 	// TODO: avoid tar command.
-	cmd := exec.CommandContext(ctx, "tar", "--no-same-owner", "--extract", "--directory", tempUnpackDir)
+	cmd := exec.CommandContext(ctx, "tar", "--extract", "--directory", tempUnpackDir)
 	var stderr bytes.Buffer
 	cmd.Stdin = rc
 	cmd.Stderr = &stderr

enterprise/server/remote_execution/containers/ociruntime/ociruntime_test.go

@@ -1,12 +1,11 @@
 package ociruntime_test
 
 import (
-	"bytes"
+	"archive/tar"
 	"context"
 	"encoding/base64"
 	"encoding/binary"
 	"fmt"
-	"io"
 	"io/fs"
 	"log"
 	"math/rand/v2"
@@ -32,15 +31,14 @@ import (
 	"github.com/buildbuddy-io/buildbuddy/server/testutil/testnetworking"
 	"github.com/buildbuddy-io/buildbuddy/server/testutil/testregistry"
 	"github.com/buildbuddy-io/buildbuddy/server/testutil/testshell"
+	"github.com/buildbuddy-io/buildbuddy/server/testutil/testtar"
 	"github.com/buildbuddy-io/buildbuddy/server/util/disk"
 	"github.com/buildbuddy-io/buildbuddy/server/util/proto"
 	"github.com/buildbuddy-io/buildbuddy/server/util/status"
 	"github.com/buildbuddy-io/buildbuddy/server/util/testing/flags"
 	"github.com/buildbuddy-io/buildbuddy/server/util/uuid"
 	"github.com/google/go-containerregistry/pkg/crane"
 	"github.com/google/go-containerregistry/pkg/v1/mutate"
-	"github.com/google/go-containerregistry/pkg/v1/partial"
-	"github.com/google/go-containerregistry/pkg/v1/types"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -948,30 +946,6 @@ func TestOverlayfsEdgeCases(t *testing.T) {
 	assert.Equal(t, 0, res.ExitCode)
 }
 
-// uncompressedLayer implements partial.UncompressedLayer from raw bytes.
-type uncompressedLayer struct {
-	diffID    containerregistry.Hash
-	mediaType types.MediaType
-	content   []byte
-}
-
-// DiffID implements partial.UncompressedLayer
-func (ul *uncompressedLayer) DiffID() (containerregistry.Hash, error) {
-	return ul.diffID, nil
-}
-
-// Uncompressed implements partial.UncompressedLayer
-func (ul *uncompressedLayer) Uncompressed() (io.ReadCloser, error) {
-	return io.NopCloser(bytes.NewBuffer(ul.content)), nil
-}
-
-// MediaType returns the media type of the layer
-func (ul *uncompressedLayer) MediaType() (types.MediaType, error) {
-	return ul.mediaType, nil
-}
-
-var _ partial.UncompressedLayer = (*uncompressedLayer)(nil)
-
 func TestHighLayerCount(t *testing.T) {
 	// Load busybox oci image
 	busyboxImg := testregistry.ImageFromRlocationpath(t, ociBusyboxRlocationpath)
@@ -1037,6 +1011,7 @@ func TestHighLayerCount(t *testing.T) {
 }
 
 func TestEntrypoint(t *testing.T) {
+	setupNetworking(t)
 	// Load busybox oci image
 	busyboxImg := testregistry.ImageFromRlocationpath(t, ociBusyboxRlocationpath)
 	// Mutate the image with an ENTRYPOINT directive which sets an env var
@@ -1051,7 +1026,6 @@ func TestEntrypoint(t *testing.T) {
 	reg := testregistry.Run(t, testregistry.Opts{})
 	image := reg.Push(t, img, "test-entrypoint:latest")
 	// Set up the container
-	setupNetworking(t)
 	ctx := context.Background()
 	env := testenv.GetTestEnv(t)
 	runtimeRoot := testfs.MakeTempDir(t)
@@ -1078,6 +1052,51 @@ func TestEntrypoint(t *testing.T) {
 	assert.Equal(t, "bar\n", string(res.Stdout))
 }
 
+func TestFileOwnership(t *testing.T) {
+	setupNetworking(t)
+	// Load busybox oci image
+	busyboxImg := testregistry.ImageFromRlocationpath(t, ociBusyboxRlocationpath)
+	// Append a layer with a file that is owned by a non-root user
+	layer := testregistry.NewBytesLayer(t, testtar.EntryBytes(t, &tar.Header{
+		Name:     "/foo.txt",
+		Gid:      1000,
+		Uid:      1000,
+		Mode:     0644,
+		Typeflag: tar.TypeReg,
+	}, nil))
+	img, err := mutate.AppendLayers(busyboxImg, layer)
+	require.NoError(t, err)
+	// Start a test registry and push the mutated busybox image to it
+	reg := testregistry.Run(t, testregistry.Opts{})
+	image := reg.Push(t, img, "test-file-ownership:latest")
+	// Set up the container
+	ctx := context.Background()
+	env := testenv.GetTestEnv(t)
+	runtimeRoot := testfs.MakeTempDir(t)
+	flags.Set(t, "executor.oci.runtime_root", runtimeRoot)
+	buildRoot := testfs.MakeTempDir(t)
+	cacheRoot := testfs.MakeTempDir(t)
+	provider, err := ociruntime.NewProvider(env, buildRoot, cacheRoot)
+	require.NoError(t, err)
+	wd := testfs.MakeDirAll(t, buildRoot, "work")
+	c, err := provider.New(ctx, &container.Init{Props: &platform.Properties{
+		ContainerImage: image,
+	}})
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		err := c.Remove(ctx)
+		require.NoError(t, err)
+	})
+
+	res := c.Run(ctx, &repb.Command{
+		Arguments: []string{"stat", "-c", "%u %g", "/foo.txt"},
+	}, wd, oci.Credentials{})
+
+	require.NoError(t, res.Error)
+	assert.Equal(t, "1000 1000\n", string(res.Stdout))
+	assert.Empty(t, string(res.Stderr))
+}
+
 func TestPersistentWorker(t *testing.T) {
 	setupNetworking(t)
 

server/testutil/testregistry/BUILD

@@ -13,7 +13,9 @@ go_library(
         "@com_github_google_go_containerregistry//pkg/registry",
         "@com_github_google_go_containerregistry//pkg/v1:pkg",
         "@com_github_google_go_containerregistry//pkg/v1/layout",
+        "@com_github_google_go_containerregistry//pkg/v1/partial",
         "@com_github_google_go_containerregistry//pkg/v1/remote",
+        "@com_github_google_go_containerregistry//pkg/v1/types",
         "@com_github_stretchr_testify//require",
         "@io_bazel_rules_go//go/runfiles:go_default_library",
     ],

server/testutil/testregistry/testregistry.go

@@ -2,7 +2,9 @@ package testregistry
 
 import (
 	"bytes"
+	"crypto/sha256"
 	"fmt"
+	"io"
 	"net"
 	"net/http"
 	"testing"
@@ -13,7 +15,9 @@ import (
 	"github.com/google/go-containerregistry/pkg/name"
 	"github.com/google/go-containerregistry/pkg/registry"
 	"github.com/google/go-containerregistry/pkg/v1/layout"
+	"github.com/google/go-containerregistry/pkg/v1/partial"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
+	"github.com/google/go-containerregistry/pkg/v1/types"
 	"github.com/stretchr/testify/require"
 
 	v1 "github.com/google/go-containerregistry/pkg/v1"
@@ -116,3 +120,41 @@ func ImageFromRlocationpath(t *testing.T, rlocationpath string) v1.Image {
 	require.NoError(t, err)
 	return img
 }
+
+// bytesLayer implements partial.UncompressedLayer from raw bytes.
+type bytesLayer struct {
+	content   []byte
+	diffID    v1.Hash
+	mediaType types.MediaType
+}
+
+// NewBytesLayer returns an image layer representing the given bytes.
+//
+// testtar.EntryBytes may be useful for constructing tarball contents.
+func NewBytesLayer(t *testing.T, b []byte) v1.Layer {
+	layer, err := partial.UncompressedToLayer(&bytesLayer{
+		mediaType: types.OCILayer,
+		diffID: v1.Hash{
+			Algorithm: "sha256",
+			Hex:       fmt.Sprintf("%x", sha256.Sum256(b)),
+		},
+		content: b,
+	})
+	require.NoError(t, err)
+	return layer
+}
+
+// DiffID implements partial.UncompressedLayer
+func (ul *bytesLayer) DiffID() (v1.Hash, error) {
+	return ul.diffID, nil
+}
+
+// Uncompressed implements partial.UncompressedLayer
+func (ul *bytesLayer) Uncompressed() (io.ReadCloser, error) {
+	return io.NopCloser(bytes.NewBuffer(ul.content)), nil
+}
+
+// MediaType returns the media type of the layer
+func (ul *bytesLayer) MediaType() (types.MediaType, error) {
+	return ul.mediaType, nil
+}

server/testutil/testtar/BUILD

@@ -0,0 +1,12 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "testtar",
+    testonly = 1,
+    srcs = ["testtar.go"],
+    importpath = "github.com/buildbuddy-io/buildbuddy/server/testutil/testtar",
+    visibility = ["//visibility:public"],
+    deps = [
+        "@com_github_stretchr_testify//require",
+    ],
+)

server/testutil/testtar/testtar.go

@@ -0,0 +1,26 @@
+package testtar
+
+import (
+	"archive/tar"
+	"bytes"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// Entry returns a tar archive entry as bytes.
+//
+// Because tar archives are just a sequence of entries, the returned bytes
+// represent a valid tar file. To create a tar archive with multiple entries,
+// simply concatenate the bytes (e.g. using slices.Concat).
+func EntryBytes(t *testing.T, header *tar.Header, b []byte) []byte {
+	var buf bytes.Buffer
+	w := tar.NewWriter(&buf)
+	err := w.WriteHeader(header)
+	require.NoError(t, err)
+	_, err = w.Write(b)
+	require.NoError(t, err)
+	err = w.Close()
+	require.NoError(t, err)
+	return buf.Bytes()
+}