fix(sandbox-router-extproc): reject ports outside [1, 65535]

Port validation only ruled out the 0 sentinel that readHeaders sets on
non-numeric input — and conveniently `r.port < 1` already covered
negatives — but values above 65535 (e.g. "99999") were accepted and
forwarded into x-envoy-original-dst-host as "10.0.0.1:99999". Envoy
rejects that downstream with a less actionable error than our 400.

Tighten the check to `r.port < 1 || r.port > 65535` and clean up the
redundant `|| r.port == 0`. Convert TestHandle_InvalidPortRejected
into a table covering non-numeric, empty, zero, negative, just-over,
big, and max-int32; add TestHandle_PortBoundariesAccepted to lock in
that 1 and 65535 still make it through.

Author: mastersingh24

sandbox-router/internal/extproc/server.go

@@ -211,7 +211,13 @@ func (s *Server) onRequestHeaders(_ context.Context, hdrs *extprocv3.HttpHeaders
 	if !validNamespace(r.namespace) {
 		return immediate(400, `{"detail":"Invalid namespace format."}`)
 	}
-	if r.port < 1 || r.port == 0 {
+	// TCP port range is [1, 65535]. Reject anything outside it (and the
+	// 0 sentinel readHeaders sets on non-numeric input) before we hand
+	// the value to net.JoinHostPort — an out-of-range port would
+	// produce a syntactically valid but semantically junk
+	// x-envoy-original-dst-host (e.g. "10.0.0.1:99999") that Envoy
+	// would reject downstream with a less actionable error.
+	if r.port < 1 || r.port > 65535 {
 		return immediate(400, `{"detail":"Invalid port format."}`)
 	}
 

sandbox-router/internal/extproc/server_test.go

@@ -199,17 +199,71 @@ func TestHandle_InvalidNamespaceRejected(t *testing.T) {
 }
 
 func TestHandle_InvalidPortRejected(t *testing.T) {
+	// Port must fall in [1, 65535]. Anything else gets rejected with a
+	// 400 immediate before we hand the value to net.JoinHostPort, so a
+	// bogus value can't ride along into x-envoy-original-dst-host and
+	// surface as a less actionable Envoy error.
+	cases := []struct {
+		name string
+		port string
+	}{
+		{"non-numeric", "abc"},
+		{"empty-after-trim", " "},
+		{"zero", "0"},
+		{"negative", "-1"},
+		{"way negative", "-99999"},
+		{"just over 65535", "65536"},
+		{"big number", "100000"},
+		{"max int32", "2147483647"},
+	}
 	s, _ := newServer(t)
-	resp := s.handle(context.Background(), reqHeaders(map[string]string{
-		HeaderSandboxID:   "alpha",
-		HeaderSandboxPort: "abc",
-	}))
-	ir := resp.GetImmediateResponse()
-	if ir == nil || ir.Status.Code != 400 {
-		t.Fatalf("expected 400 immediate; got: %+v", resp)
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			resp := s.handle(context.Background(), reqHeaders(map[string]string{
+				HeaderSandboxID:   "alpha",
+				HeaderSandboxPort: tc.port,
+			}))
+			ir := resp.GetImmediateResponse()
+			if ir == nil || ir.Status.Code != 400 {
+				t.Fatalf("port=%q: expected 400 immediate; got: %+v", tc.port, resp)
+			}
+			if !strings.Contains(string(ir.Body), "port") {
+				t.Errorf("port=%q: body should mention port: %s", tc.port, ir.Body)
+			}
+		})
 	}
-	if !strings.Contains(string(ir.Body), "port") {
-		t.Errorf("body should mention port: %s", ir.Body)
+}
+
+func TestHandle_PortBoundariesAccepted(t *testing.T) {
+	// The smallest and largest legal TCP ports both make it through.
+	s, stub := newServer(t)
+	uid := types.UID("boundary-uid")
+	stub[uid] = cache.Entry{PodIP: "10.0.0.1"}
+	for _, port := range []string{"1", "65535"} {
+		t.Run(port, func(t *testing.T) {
+			resp := s.handle(context.Background(), reqHeaders(map[string]string{
+				HeaderSandboxID:   "alpha",
+				HeaderSandboxUID:  string(uid),
+				HeaderSandboxPort: port,
+			}))
+			if ir := resp.GetImmediateResponse(); ir != nil {
+				t.Fatalf("port %s: unexpected immediate %d / %s", port, ir.Status.Code, ir.Body)
+			}
+			rh := resp.GetRequestHeaders()
+			if rh == nil {
+				t.Fatalf("port %s: expected RequestHeaders mutation; got %+v", port, resp)
+			}
+			want := "10.0.0.1:" + port
+			got := ""
+			for _, h := range rh.Response.HeaderMutation.SetHeaders {
+				if h.Header.Key == HeaderOriginalDstHost {
+					got = string(h.Header.RawValue)
+				}
+			}
+			if got != want {
+				t.Fatalf("port %s: dst host got %q want %q", port, got, want)
+			}
+		})
 	}
 }