fix: show unsalted hash in error messages

Author: tyler-french

server/remote_cache/chunking/BUILD

@@ -10,9 +10,11 @@ go_library(
         "//proto:resource_go_proto",
         "//server/interfaces",
         "//server/remote_cache/digest",
+        "//server/util/log",
         "//server/util/proto",
         "//server/util/status",
         "@com_github_buildbuddy_io_fastcdc2020//fastcdc",
+        "@org_golang_google_grpc//status",
         "@org_golang_x_sync//errgroup",
     ],
 )

server/remote_cache/chunking/chunking.go

@@ -14,10 +14,12 @@ import (
 
 	"github.com/buildbuddy-io/buildbuddy/server/interfaces"
 	"github.com/buildbuddy-io/buildbuddy/server/remote_cache/digest"
+	"github.com/buildbuddy-io/buildbuddy/server/util/log"
 	"github.com/buildbuddy-io/buildbuddy/server/util/proto"
 	"github.com/buildbuddy-io/buildbuddy/server/util/status"
 	"github.com/buildbuddy-io/fastcdc2020/fastcdc"
 	"golang.org/x/sync/errgroup"
+	gstatus "google.golang.org/grpc/status"
 
 	repb "github.com/buildbuddy-io/buildbuddy/proto/remote_execution"
 	rspb "github.com/buildbuddy-io/buildbuddy/proto/resource"
@@ -270,7 +272,14 @@ func (cm *Manifest) Store(ctx context.Context, cache interfaces.Cache) error {
 		return err
 	}
 
-	return cache.Set(ctx, acRNProto, arBytes)
+	if err := cache.Set(ctx, acRNProto, arBytes); err != nil {
+		err = sanitizeManifestError(err, acRNProto.GetDigest().GetHash(), cm.BlobDigest.GetHash())
+		if status.IsInternalError(err) {
+			log.CtxInfof(ctx, "Failed to set chunking manifest (blob %s, manifest key %s): %v", cm.BlobDigest.GetHash(), acRNProto.GetDigest().GetHash(), err)
+		}
+		return err
+	}
+	return nil
 }
 
 // LoadManifest retrieves a chunked manifest from the cache. It does NOT validate existence of the chunks.
@@ -282,6 +291,10 @@ func LoadManifest(ctx context.Context, cache interfaces.Cache, blobDigest *repb.
 
 	arBytes, err := cache.Get(ctx, acRNProto)
 	if err != nil {
+		err = sanitizeManifestError(err, acRNProto.GetDigest().GetHash(), blobDigest.GetHash())
+		if status.IsInternalError(err) {
+			log.CtxInfof(ctx, "Failed to get chunking manifest (blob %s, manifest key %s): %v", blobDigest.GetHash(), acRNProto.GetDigest().GetHash(), err)
+		}
 		if status.IsNotFoundError(err) {
 			blobRN := digest.NewCASResourceName(blobDigest, instanceName, digestFunction).ToProto()
 			if exists, existsErr := cache.Contains(ctx, blobRN); existsErr != nil {
@@ -385,6 +398,28 @@ func acResourceName(blobDigest *repb.Digest, instanceName string, digestFunction
 	return acRN.ToProto(), nil
 }
 
+// sanitizeManifestError replaces the salted AC key hash with the original blob hash in
+// the error message, so callers see the blob digest rather than the internal salted key.
+// The gRPC status code is preserved.
+func sanitizeManifestError(err error, saltedHash, blobHash string) error {
+	if err == nil || saltedHash == "" || saltedHash == blobHash {
+		return err
+	}
+	grpcStatus, ok := gstatus.FromError(err)
+	if !ok {
+		return fmt.Errorf("%s", sanitizeMsg(err.Error(), saltedHash, blobHash))
+	}
+	return gstatus.Error(grpcStatus.Code(), sanitizeMsg(grpcStatus.Message(), saltedHash, blobHash))
+}
+
+func sanitizeMsg(msg, saltedHash, blobHash string) string {
+	replacement := fmt.Sprintf("manifestKey(%s)", blobHash)
+	if replaced := strings.ReplaceAll(msg, saltedHash, replacement); replaced != msg {
+		return replaced
+	}
+	return fmt.Sprintf("%s (blob %s)", msg, blobHash)
+}
+
 func DigestsSummary(digests []*repb.Digest) string {
 	const maxShown = 3
 	strs := digestsStrings(digests...)

server/remote_cache/chunking/chunking_test.go

@@ -213,6 +213,29 @@ func TestLoadWithoutManifest_BlobMissing(t *testing.T) {
 	require.True(t, status.IsNotFoundError(err))
 }
 
+func TestLoadWithoutManifest_SaltedHashNotLeaked(t *testing.T) {
+	const salt = "test-salt"
+	flags.Set(t, "cache.chunking.ac_key_salt", salt)
+
+	ctx := context.Background()
+	te := testenv.GetTestEnv(t)
+	ctx, err := prefix.AttachUserPrefixToContext(ctx, te.GetAuthenticator())
+	require.NoError(t, err)
+
+	blobRN, _ := testdigest.RandomCASResourceBuf(t, 500)
+	blobDigest := blobRN.GetDigest()
+
+	saltedDigest, err := digest.Compute(bytes.NewReader([]byte(salt+":"+blobDigest.GetHash())), repb.DigestFunction_SHA256)
+	require.NoError(t, err)
+
+	_, err = chunking.LoadManifest(ctx, te.GetCache(), blobDigest, "", repb.DigestFunction_SHA256)
+
+	require.Error(t, err)
+	require.True(t, status.IsNotFoundError(err))
+	assert.Contains(t, err.Error(), blobDigest.GetHash())
+	assert.NotContains(t, err.Error(), saltedDigest.GetHash())
+}
+
 func TestStore_ErrGroupContextCancellation(t *testing.T) {
 	ctx := context.Background()
 	te := testenv.GetTestEnv(t)