From 68e24026d9387ad188b8e6dac97d049fb27d25d4 Mon Sep 17 00:00:00 2001
From: djuloori <djuloori@uber.com>
Date: Mon, 23 Feb 2026 18:27:35 +0000
Subject: [PATCH] feat(gateway): add input validation to LandController

Add input validation for LandRequest before processing:
- Validate queue name is not empty
- Validate change source is not empty (handles nil change)
- Validate at least one change ID is provided

Introduce ErrInvalidRequest sentinel error following the existing pattern
from extension/storage. This allows the gRPC layer to map validation
errors to codes.InvalidArgument.

Tests verify error type using IsInvalidRequest() helper, following
CLAUDE.md guidance to avoid asserting on error messages.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 gateway/controller/land.go      | 21 ++++++++
 gateway/controller/land_test.go | 88 +++++++++++++++++++++++++++++++++
 2 files changed, 109 insertions(+)

diff --git a/gateway/controller/land.go b/gateway/controller/land.go
index 83a840c4..cf30fa91 100644
--- a/gateway/controller/land.go
+++ b/gateway/controller/land.go
@@ -2,6 +2,7 @@ package controller
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -13,6 +14,15 @@ import (
 	"go.uber.org/zap"
 )
 
+// ErrInvalidRequest is returned when the request fails validation.
+// This error should be mapped to codes.InvalidArgument at the gRPC layer.
+var ErrInvalidRequest = errors.New("invalid request")
+
+// IsInvalidRequest returns true if any error in the error chain is ErrInvalidRequest.
+func IsInvalidRequest(err error) bool {
+	return errors.Is(err, ErrInvalidRequest)
+}
+
 // LandController handles land business logic for the gateway
 type LandController struct {
 	logger       *zap.Logger
@@ -40,6 +50,17 @@ func (c *LandController) Land(ctx context.Context, req *pb.LandRequest) (*pb.Lan
 
 	c.metricsScope.Counter("land_request_count").Inc(1)
 
+	// Validate required fields.
+	if req.Queue == "" {
+		return nil, fmt.Errorf("LandController requires the request to have a queue name specified: %w", ErrInvalidRequest)
+	}
+	if req.Change == nil || req.Change.Source == "" {
+		return nil, fmt.Errorf("LandController requires the request to have a change source specified: %w", ErrInvalidRequest)
+	}
+	if len(req.Change.GetIds()) == 0 {
+		return nil, fmt.Errorf("LandController requires the request to have at least one change ID specified: %w", ErrInvalidRequest)
+	}
+
 	change := entity.Change{
 		Source: req.Change.GetSource(),
 		IDs:    req.Change.GetIds(),
diff --git a/gateway/controller/land_test.go b/gateway/controller/land_test.go
index 46b78a53..6a598c32 100644
--- a/gateway/controller/land_test.go
+++ b/gateway/controller/land_test.go
@@ -184,3 +184,91 @@ func TestLand_CounterDomainIncludesQueue(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, "request/my-queue", capturedDomain)
 }
+
+func TestLand_ReturnsErrorOnEmptyQueue(t *testing.T) {
+	store := &mockStorage{requestStore: &mockRequestStore{
+		createFunc: func(ctx context.Context, request entity.Request) error {
+			return nil
+		},
+	}}
+	cnt := &mockCounter{nextFunc: func(ctx context.Context, domain string) (int64, error) {
+		return 1, nil
+	}}
+	controller := NewLandController(zap.NewNop(), tally.NoopScope, store, cnt)
+	ctx := context.Background()
+
+	req := &pb.LandRequest{
+		Queue:  "",
+		Change: &pb.Change{Source: "github", Ids: []string{"123"}},
+	}
+	_, err := controller.Land(ctx, req)
+
+	require.Error(t, err)
+	assert.True(t, IsInvalidRequest(err))
+}
+
+func TestLand_ReturnsErrorOnEmptyChangeSource(t *testing.T) {
+	store := &mockStorage{requestStore: &mockRequestStore{
+		createFunc: func(ctx context.Context, request entity.Request) error {
+			return nil
+		},
+	}}
+	cnt := &mockCounter{nextFunc: func(ctx context.Context, domain string) (int64, error) {
+		return 1, nil
+	}}
+	controller := NewLandController(zap.NewNop(), tally.NoopScope, store, cnt)
+	ctx := context.Background()
+
+	req := &pb.LandRequest{
+		Queue:  "test-queue",
+		Change: &pb.Change{Source: "", Ids: []string{"123"}},
+	}
+	_, err := controller.Land(ctx, req)
+
+	require.Error(t, err)
+	assert.True(t, IsInvalidRequest(err))
+}
+
+func TestLand_ReturnsErrorOnNilChange(t *testing.T) {
+	store := &mockStorage{requestStore: &mockRequestStore{
+		createFunc: func(ctx context.Context, request entity.Request) error {
+			return nil
+		},
+	}}
+	cnt := &mockCounter{nextFunc: func(ctx context.Context, domain string) (int64, error) {
+		return 1, nil
+	}}
+	controller := NewLandController(zap.NewNop(), tally.NoopScope, store, cnt)
+	ctx := context.Background()
+
+	req := &pb.LandRequest{
+		Queue:  "test-queue",
+		Change: nil,
+	}
+	_, err := controller.Land(ctx, req)
+
+	require.Error(t, err)
+	assert.True(t, IsInvalidRequest(err))
+}
+
+func TestLand_ReturnsErrorOnEmptyChangeIDs(t *testing.T) {
+	store := &mockStorage{requestStore: &mockRequestStore{
+		createFunc: func(ctx context.Context, request entity.Request) error {
+			return nil
+		},
+	}}
+	cnt := &mockCounter{nextFunc: func(ctx context.Context, domain string) (int64, error) {
+		return 1, nil
+	}}
+	controller := NewLandController(zap.NewNop(), tally.NoopScope, store, cnt)
+	ctx := context.Background()
+
+	req := &pb.LandRequest{
+		Queue:  "test-queue",
+		Change: &pb.Change{Source: "github", Ids: []string{}},
+	}
+	_, err := controller.Land(ctx, req)
+
+	require.Error(t, err)
+	assert.True(t, IsInvalidRequest(err))
+}