fix(coderd/agentapi): reject missing RPC messages (#29150)

Reject `UpdateLifecycle`, `UpdateStartup`, and `BatchUpdateMetadata`
requests with a missing sub-message using `InvalidArgument` instead of
panicking. The whole metadata batch is validated before any value is
trimmed or queued.

Adds tests for each handler, a mixed metadata batch, and one DRPC
round-trip confirming the code propagates and the connection stays
usable.

Refs PLAT-596

Reviewed and updated by Coder Agents on behalf of @dylanhuff-at-coder.

coderd/agentapi/lifecycle.go

@@ -10,7 +10,9 @@ import (
 	"github.com/google/uuid"
 	"golang.org/x/mod/semver"
 	"golang.org/x/xerrors"
+	"google.golang.org/grpc/codes"
 	"google.golang.org/protobuf/types/known/timestamppb"
+	"storj.io/drpc/drpcerr"
 
 	"cdr.dev/slog/v3"
 	agentproto "github.com/coder/coder/v2/agent/proto"
@@ -45,6 +47,10 @@ func (a *LifecycleAPI) now() time.Time {
 }
 
 func (a *LifecycleAPI) UpdateLifecycle(ctx context.Context, req *agentproto.UpdateLifecycleRequest) (*agentproto.Lifecycle, error) {
+	if req.GetLifecycle() == nil {
+		return nil, drpcerr.WithCode(xerrors.New("lifecycle is required"), uint64(codes.InvalidArgument))
+	}
+
 	workspaceAgent, err := a.AgentFn(ctx)
 	if err != nil {
 		return nil, err
@@ -146,6 +152,10 @@ func (a *LifecycleAPI) UpdateLifecycle(ctx context.Context, req *agentproto.Upda
 }
 
 func (a *LifecycleAPI) UpdateStartup(ctx context.Context, req *agentproto.UpdateStartupRequest) (*agentproto.Startup, error) {
+	if req.GetStartup() == nil {
+		return nil, drpcerr.WithCode(xerrors.New("startup is required"), uint64(codes.InvalidArgument))
+	}
+
 	apiVersion, ok := ctx.Value(contextKeyAPIVersion{}).(string)
 	if !ok {
 		return nil, xerrors.Errorf("internal error; api version unspecified")

coderd/agentapi/metadata.go

@@ -8,6 +8,8 @@ import (
 
 	"github.com/google/uuid"
 	"golang.org/x/xerrors"
+	"google.golang.org/grpc/codes"
+	"storj.io/drpc/drpcerr"
 
 	"cdr.dev/slog/v3"
 	agentproto "github.com/coder/coder/v2/agent/proto"
@@ -34,6 +36,16 @@ func (a *MetadataAPI) now() time.Time {
 }
 
 func (a *MetadataAPI) BatchUpdateMetadata(ctx context.Context, req *agentproto.BatchUpdateMetadataRequest) (*agentproto.BatchUpdateMetadataResponse, error) {
+	if req == nil {
+		return nil, drpcerr.WithCode(xerrors.New("metadata request is required"), uint64(codes.InvalidArgument))
+	}
+	// Validate the entire batch before trimming values or discarding excess keys.
+	for i, md := range req.Metadata {
+		if md.GetResult() == nil {
+			return nil, drpcerr.WithCode(xerrors.Errorf("metadata result at index %d is required", i), uint64(codes.InvalidArgument))
+		}
+	}
+
 	const (
 		// maxAllKeysLen is the maximum length of all metadata keys. This is
 		// 6144 to stay below the Postgres NOTIFY limit of 8000 bytes, with some

coderd/agentapi/validation_test.go

@@ -0,0 +1,90 @@
+package agentapi_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc/codes"
+	"storj.io/drpc/drpcerr"
+
+	agentproto "github.com/coder/coder/v2/agent/proto"
+	"github.com/coder/coder/v2/coderd/agentapi"
+	"github.com/coder/coder/v2/codersdk/drpcsdk"
+	"github.com/coder/coder/v2/testutil"
+)
+
+func TestUpdateLifecycleMissingMessage(t *testing.T) {
+	t.Parallel()
+
+	resp, err := (&agentapi.LifecycleAPI{}).UpdateLifecycle(context.Background(), &agentproto.UpdateLifecycleRequest{})
+	require.ErrorContains(t, err, "lifecycle is required")
+	require.EqualValues(t, codes.InvalidArgument, drpcerr.Code(err))
+	require.Nil(t, resp)
+}
+
+func TestUpdateStartupMissingMessage(t *testing.T) {
+	t.Parallel()
+
+	resp, err := (&agentapi.LifecycleAPI{}).UpdateStartup(context.Background(), &agentproto.UpdateStartupRequest{})
+	require.ErrorContains(t, err, "startup is required")
+	require.EqualValues(t, codes.InvalidArgument, drpcerr.Code(err))
+	require.Nil(t, resp)
+}
+
+func TestBatchUpdateMetadataMissingMessage(t *testing.T) {
+	t.Parallel()
+
+	for _, tt := range []struct {
+		name string
+		req  *agentproto.BatchUpdateMetadataRequest
+	}{
+		{name: "NilRequest"},
+		{name: "MixedBatch", req: &agentproto.BatchUpdateMetadataRequest{Metadata: []*agentproto.Metadata{
+			{Key: "valid", Result: &agentproto.WorkspaceAgentMetadata_Result{Value: "value"}},
+			{Key: "invalid"},
+		}}},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			resp, err := (&agentapi.MetadataAPI{}).BatchUpdateMetadata(context.Background(), tt.req)
+			require.Error(t, err)
+			require.EqualValues(t, codes.InvalidArgument, drpcerr.Code(err))
+			require.Nil(t, resp)
+		})
+	}
+}
+
+func TestAgentRPCMissingMessage(t *testing.T) {
+	t.Parallel()
+
+	ctx := testutil.Context(t, testutil.WaitShort)
+	serverCtx, cancel := context.WithCancel(agentapi.WithAPIVersion(ctx, "2.0"))
+	defer cancel()
+	conn, listener := drpcsdk.MemTransportPipe()
+	defer conn.Close()
+	defer listener.Close()
+
+	api := &agentapi.API{
+		LifecycleAPI: &agentapi.LifecycleAPI{},
+		StatsAPI:     &agentapi.StatsAPI{},
+	}
+	server, err := api.Server(serverCtx)
+	require.NoError(t, err)
+	done := make(chan error, 1)
+	go func() {
+		done <- server.Serve(serverCtx, listener)
+	}()
+	client := agentproto.NewDRPCAgentClient(conn)
+
+	_, err = client.UpdateLifecycle(ctx, &agentproto.UpdateLifecycleRequest{})
+	require.ErrorContains(t, err, "lifecycle is required")
+	require.EqualValues(t, codes.InvalidArgument, drpcerr.Code(err))
+	stats, err := client.UpdateStats(ctx, &agentproto.UpdateStatsRequest{})
+	require.NoError(t, err)
+	require.NotNil(t, stats.ReportInterval)
+
+	cancel()
+	require.NoError(t, testutil.RequireReceive(ctx, t, done))
+}