batcheval: prevent excising non-user keys

This commit performs some checks in the Excise command evaluation. It
prevents the command to run against non-user keys. Excising non-global
keys is not allowed as it would leave the replica in a bad state. For
example, we don't want to allow excising a range descriptor.

References: #143135

Release note: None

Author: iskettaneh

pkg/kv/kvserver/batcheval/BUILD.bazel

@@ -112,6 +112,7 @@ go_test(
         "cmd_delete_range_gchint_test.go",
         "cmd_delete_range_test.go",
         "cmd_end_transaction_test.go",
+        "cmd_excise_test.go",
         "cmd_export_test.go",
         "cmd_get_test.go",
         "cmd_is_span_empty_test.go",

pkg/kv/kvserver/batcheval/cmd_excise.go

@@ -14,6 +14,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/storage"
+	"github.com/cockroachdb/errors"
 )
 
 func init() {
@@ -22,10 +23,33 @@ func init() {
 
 // EvalExcise evaluates a Excise command.
 func EvalExcise(
-	_ context.Context, _ storage.ReadWriter, cArgs CommandArgs, _ kvpb.Response,
+	_ context.Context, _ storage.ReadWriter, cArgs CommandArgs, resp kvpb.Response,
 ) (result.Result, error) {
 	args := cArgs.Args.(*kvpb.ExciseRequest)
-	start, end := storage.MVCCKey{Key: args.Key}, storage.MVCCKey{Key: args.EndKey}
+	start, end := args.Key, args.EndKey
+
+	// Verify that the start and end keys are for global key space. Excising
+	// non-global keys is not allowed as it would leave the replica in a bad
+	// state. For example, we don't want to allow excising a range descriptor.
+	rStart, err := keys.Addr(start)
+	if err != nil {
+		return result.Result{}, err
+	}
+
+	rEnd, err := keys.Addr(end)
+	if err != nil {
+		return result.Result{}, err
+	}
+
+	if !start.Equal(rStart.AsRawKey()) {
+		return result.Result{},
+			errors.Errorf("excise can only be run against global keys, but found start key: %s", start)
+	}
+
+	if !end.Equal(rEnd.AsRawKey()) {
+		return result.Result{},
+			errors.Errorf("excise can only be run against global keys, but found end key: %s", end)
+	}
 
 	// Since we can't know the exact range stats, mark it as an estimate.
 	cArgs.Stats.ContainsEstimates++
@@ -36,7 +60,7 @@ func EvalExcise(
 	return result.Result{
 		Replicated: kvserverpb.ReplicatedEvalResult{
 			Excise: &kvserverpb.ReplicatedEvalResult_Excise{
-				Span:          roachpb.Span{Key: start.Key, EndKey: end.Key},
+				Span:          roachpb.Span{Key: start, EndKey: end},
 				LockTableSpan: roachpb.Span{Key: ltStart, EndKey: ltEnd},
 			},
 		},

pkg/kv/kvserver/batcheval/cmd_excise_test.go

@@ -0,0 +1,95 @@
+// Copyright 2025 The Cockroach Authors.
+//
+// Use of this software is governed by the CockroachDB Software License
+// included in the /LICENSE file.
+
+package batcheval_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/cockroachdb/cockroach/pkg/keys"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval"
+	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
+	"github.com/cockroachdb/cockroach/pkg/util/hlc"
+	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
+	"github.com/stretchr/testify/require"
+)
+
+// TestExciseEval tests basic Excise evaluation.
+func TestExciseEval(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	ctx := context.Background()
+	clock := hlc.NewClockForTesting(timeutil.NewManualTime(timeutil.Now()))
+	evalCtx := (&batcheval.MockEvalCtx{Clock: clock}).EvalContext()
+
+	testCases := []struct {
+		name      string
+		startKey  roachpb.Key
+		endKey    roachpb.Key
+		expectErr string
+	}{
+		{
+			// Valid range.
+			startKey:  roachpb.Key("a"),
+			endKey:    roachpb.Key("z"),
+			expectErr: "",
+		},
+		{
+			// Only the start key is a non-user key.
+			startKey:  keys.RangeDescriptorKey(roachpb.RKey("a")),
+			endKey:    roachpb.Key("z"),
+			expectErr: "excise can only be run against global keys",
+		},
+		{
+			// Only the end key is a non-user key.
+			startKey:  roachpb.Key("a"),
+			endKey:    keys.RangeDescriptorKey(roachpb.RKey("z")),
+			expectErr: "excise can only be run against global keys",
+		},
+		{
+			// Both keys are non-user keys.
+			startKey:  keys.RangeDescriptorKey(roachpb.RKey("a")),
+			endKey:    keys.RangeDescriptorKey(roachpb.RKey("z")),
+			expectErr: "excise can only be run against global keys",
+		},
+	}
+
+	for _, tc := range testCases {
+		resp := &kvpb.ExciseResponse{}
+		res, err := batcheval.EvalExcise(ctx, nil, batcheval.CommandArgs{
+			EvalCtx: evalCtx,
+			Stats:   &enginepb.MVCCStats{},
+			Args: &kvpb.ExciseRequest{
+				RequestHeader: kvpb.RequestHeader{
+					Key:    tc.startKey,
+					EndKey: tc.endKey,
+				},
+			},
+		}, resp)
+
+		// If there are no errors, we expect the result to be populated.
+		if tc.expectErr == "" {
+			require.NoError(t, err)
+
+			userSpan := roachpb.Span{Key: tc.startKey, EndKey: tc.endKey}
+			ltStart, _ := keys.LockTableSingleKey(tc.startKey, nil)
+			ltEnd, _ := keys.LockTableSingleKey(tc.endKey, nil)
+			LockTableSpan := roachpb.Span{Key: ltStart, EndKey: ltEnd}
+
+			require.NotNil(t, res.Replicated.Excise)
+			require.Equal(t, userSpan, res.Replicated.Excise.Span)
+			require.Equal(t, LockTableSpan, res.Replicated.Excise.LockTableSpan)
+		} else {
+			require.Regexp(t, tc.expectErr, err)
+			require.Nil(t, res.Replicated.Excise)
+		}
+	}
+}