From b9d91791628705bd15efe4ac0b0043ccb0fcfdff Mon Sep 17 00:00:00 2001 From: sicaario Date: Sun, 12 Apr 2026 19:53:00 -0600 Subject: [PATCH 1/6] fix: propagate url.Parse error in buildURL to prevent nil pointer panic in reverse proxy Signed-off-by: sicaario --- pkg/router/handlers.go | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/pkg/router/handlers.go b/pkg/router/handlers.go index c95c221c..23390072 100644 --- a/pkg/router/handlers.go +++ b/pkg/router/handlers.go @@ -17,6 +17,7 @@ limitations under the License. package router import ( + "errors" "fmt" "net/http" "net/http/httputil" @@ -103,27 +104,33 @@ func (s *Server) handleGetSandboxError(c *gin.Context, err error) { c.JSON(http.StatusInternalServerError, gin.H{"error": "internal server error"}) } +// errNoEntryPoint is returned when a sandbox has no entry points configured. +var errNoEntryPoint = errors.New("no entry point found for sandbox") + func determineUpstreamURL(sandbox *types.SandboxInfo, path string) (*url.URL, error) { // prefer matched entrypoint by path for _, ep := range sandbox.EntryPoints { if strings.HasPrefix(path, ep.Path) { - return buildURL(ep.Protocol, ep.Endpoint), nil + return buildURL(ep.Protocol, ep.Endpoint) } } // fallback to first entrypoint if len(sandbox.EntryPoints) == 0 { - return nil, fmt.Errorf("no entry point found for sandbox") + return nil, errNoEntryPoint } ep := sandbox.EntryPoints[0] - return buildURL(ep.Protocol, ep.Endpoint), nil + return buildURL(ep.Protocol, ep.Endpoint) } -func buildURL(protocol, endpoint string) *url.URL { +func buildURL(protocol, endpoint string) (*url.URL, error) { if protocol != "" && !strings.Contains(endpoint, "://") { - endpoint = (strings.ToLower(protocol) + "://" + endpoint) + endpoint = strings.ToLower(protocol) + "://" + endpoint + } + u, err := url.Parse(endpoint) + if err != nil { + return nil, fmt.Errorf("invalid endpoint URL %q: %w", endpoint, err) } - url, _ := url.Parse(endpoint) - return url + return u, nil } // handleAgentInvoke handles agent invocation requests @@ -148,9 +155,11 @@ func (s *Server) forwardToSandbox(c *gin.Context, sandbox *types.SandboxInfo, pa targetURL, err := determineUpstreamURL(sandbox, path) if err != nil { klog.Errorf("Failed to get sandbox access address %s: %v", sandbox.SandboxID, err) - c.JSON(http.StatusNotFound, gin.H{ - "error": err.Error(), - }) + if errors.Is(err, errNoEntryPoint) { + c.JSON(http.StatusNotFound, gin.H{"error": err.Error()}) + } else { + c.JSON(http.StatusInternalServerError, gin.H{"error": "invalid sandbox endpoint configuration"}) + } return } From 08363e48d80292933e7d7a90cf487ed2511aad7e Mon Sep 17 00:00:00 2001 From: sicaario Date: Sun, 12 Apr 2026 20:44:31 -0600 Subject: [PATCH 2/6] fix: compare RSA key components in TestGetPrivateKeyPEM instead of whole struct Signed-off-by: sicaario --- pkg/router/jwt_test.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/router/jwt_test.go b/pkg/router/jwt_test.go index aef69981..5b2f469a 100644 --- a/pkg/router/jwt_test.go +++ b/pkg/router/jwt_test.go @@ -183,7 +183,16 @@ func TestGetPrivateKeyPEM(t *testing.T) { privateKey, err := x509.ParsePKCS1PrivateKey(block.Bytes) assert.NoError(t, err) assert.NotNil(t, privateKey) - assert.Equal(t, manager.privateKey, privateKey) + + // Compare key components instead of whole struct — ParsePKCS1PrivateKey may + // not precompute Dp/Dq/Qinv identically to the original key. + assert.Equal(t, manager.privateKey.PublicKey.N, privateKey.PublicKey.N, "Public key N should match") + assert.Equal(t, manager.privateKey.PublicKey.E, privateKey.PublicKey.E, "Public key E should match") + assert.Equal(t, 0, manager.privateKey.D.Cmp(privateKey.D), "Private exponent D should match") + assert.Equal(t, len(manager.privateKey.Primes), len(privateKey.Primes), "Number of primes should match") + for i := range manager.privateKey.Primes { + assert.Equal(t, 0, manager.privateKey.Primes[i].Cmp(privateKey.Primes[i]), "Prime %d should match", i) + } } func TestLoadPrivateKeyPEM(t *testing.T) { From 02fae1956cde6db71d5992fd880aa31dfb3ddf34 Mon Sep 17 00:00:00 2001 From: sicaario Date: Sun, 12 Apr 2026 22:41:47 -0600 Subject: [PATCH 3/6] fix: include parse error in 500 response and add test for empty entry points returning 404 Signed-off-by: sicaario --- pkg/router/handlers.go | 2 +- pkg/router/handlers_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/pkg/router/handlers.go b/pkg/router/handlers.go index 23390072..4f3426b9 100644 --- a/pkg/router/handlers.go +++ b/pkg/router/handlers.go @@ -158,7 +158,7 @@ func (s *Server) forwardToSandbox(c *gin.Context, sandbox *types.SandboxInfo, pa if errors.Is(err, errNoEntryPoint) { c.JSON(http.StatusNotFound, gin.H{"error": err.Error()}) } else { - c.JSON(http.StatusInternalServerError, gin.H{"error": "invalid sandbox endpoint configuration"}) + c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("invalid sandbox endpoint configuration: %v", err)}) } return } diff --git a/pkg/router/handlers_test.go b/pkg/router/handlers_test.go index 84aa06a8..738f035c 100644 --- a/pkg/router/handlers_test.go +++ b/pkg/router/handlers_test.go @@ -429,6 +429,40 @@ func TestForwardToSandbox_InvalidEndpoint(t *testing.T) { } } +func TestForwardToSandbox_NoEntryPoints(t *testing.T) { + setupEnv() + defer teardownEnv() + + config := &Config{Port: "8080"} + server, err := NewServer(config) + if err != nil { + t.Fatalf("Failed to create server: %v", err) + } + + server.sessionManager = &mockSessionManager{ + sandbox: &types.SandboxInfo{ + SandboxID: "test-sandbox", + SessionID: "test-session", + Name: "test-sandbox", + EntryPoints: []types.SandboxEntryPoint{}, + }, + } + + routerServer := httptest.NewServer(server.engine) + defer routerServer.Close() + + client := &http.Client{Timeout: 5 * time.Second} + resp, err := client.Post(routerServer.URL+"/v1/namespaces/default/agent-runtimes/test-agent/invocations/test", "application/json", nil) + if err != nil { + t.Fatalf("Failed to make request: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusNotFound { + t.Errorf("Expected status code %d, got %d", http.StatusNotFound, resp.StatusCode) + } +} + func TestConcurrencyLimitMiddleware_Overload(t *testing.T) { // Set required environment variables setupEnv() From aa52d24d01e53777d22e5e8f1e99b3ef117cfdbd Mon Sep 17 00:00:00 2001 From: sicaario Date: Mon, 13 Apr 2026 15:26:15 -0600 Subject: [PATCH 4/6] fix: validate scheme and host in buildURL and expand edge case tests Signed-off-by: sicaario --- pkg/router/handlers.go | 6 ++++ pkg/router/handlers_test.go | 69 ++++++++++++++++++++++--------------- 2 files changed, 47 insertions(+), 28 deletions(-) diff --git a/pkg/router/handlers.go b/pkg/router/handlers.go index 4f3426b9..8856d025 100644 --- a/pkg/router/handlers.go +++ b/pkg/router/handlers.go @@ -130,6 +130,12 @@ func buildURL(protocol, endpoint string) (*url.URL, error) { if err != nil { return nil, fmt.Errorf("invalid endpoint URL %q: %w", endpoint, err) } + if u.Scheme == "" { + return nil, fmt.Errorf("invalid endpoint URL %q: missing scheme", endpoint) + } + if u.Host == "" { + return nil, fmt.Errorf("invalid endpoint URL %q: missing host", endpoint) + } return u, nil } diff --git a/pkg/router/handlers_test.go b/pkg/router/handlers_test.go index 738f035c..404bd680 100644 --- a/pkg/router/handlers_test.go +++ b/pkg/router/handlers_test.go @@ -393,39 +393,52 @@ func TestHandleCodeInterpreterInvoke(t *testing.T) { } func TestForwardToSandbox_InvalidEndpoint(t *testing.T) { - setupEnv() - defer teardownEnv() - - config := &Config{Port: "8080"} - server, err := NewServer(config) - if err != nil { - t.Fatalf("Failed to create server: %v", err) + cases := []struct { + name string + endpoint string + }{ + {"malformed scheme", "://invalid-url"}, + {"empty endpoint", ""}, + {"scheme only", "http://"}, + {"no scheme", "localhost:8080"}, } - server.sessionManager = &mockSessionManager{ - sandbox: &types.SandboxInfo{ - SandboxID: "test-sandbox", - SessionID: "test-session", - Name: "test-sandbox", - EntryPoints: []types.SandboxEntryPoint{ - {Endpoint: "://invalid-url", Path: "/test"}, - }, - }, - } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + setupEnv() + defer teardownEnv() - // run via real server to avoid CloseNotifier panic - routerServer := httptest.NewServer(server.engine) - defer routerServer.Close() + config := &Config{Port: "8080"} + server, err := NewServer(config) + if err != nil { + t.Fatalf("Failed to create server: %v", err) + } - client := &http.Client{Timeout: 5 * time.Second} - resp, err := client.Post(routerServer.URL+"/v1/namespaces/default/agent-runtimes/test-agent/invocations/test", "application/json", nil) - if err != nil { - t.Fatalf("Failed to make request: %v", err) - } - defer resp.Body.Close() + server.sessionManager = &mockSessionManager{ + sandbox: &types.SandboxInfo{ + SandboxID: "test-sandbox", + SessionID: "test-session", + Name: "test-sandbox", + EntryPoints: []types.SandboxEntryPoint{ + {Endpoint: tc.endpoint, Path: "/test"}, + }, + }, + } - if resp.StatusCode != http.StatusInternalServerError { - t.Errorf("Expected status code %d, got %d", http.StatusInternalServerError, resp.StatusCode) + routerServer := httptest.NewServer(server.engine) + defer routerServer.Close() + + client := &http.Client{Timeout: 5 * time.Second} + resp, err := client.Post(routerServer.URL+"/v1/namespaces/default/agent-runtimes/test-agent/invocations/test", "application/json", nil) + if err != nil { + t.Fatalf("Failed to make request: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusInternalServerError { + t.Errorf("[%s] expected %d, got %d", tc.name, http.StatusInternalServerError, resp.StatusCode) + } + }) } } From 9dfb8a4952b180cd62e5d334f945380de2ecc766 Mon Sep 17 00:00:00 2001 From: sicaario Date: Mon, 13 Apr 2026 15:29:46 -0600 Subject: [PATCH 5/6] fix: validate proxy scheme whitelist and port range in buildURL Signed-off-by: sicaario --- pkg/router/handlers.go | 16 ++++++++++++++++ pkg/router/handlers_test.go | 5 ++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/pkg/router/handlers.go b/pkg/router/handlers.go index 8856d025..587d2643 100644 --- a/pkg/router/handlers.go +++ b/pkg/router/handlers.go @@ -22,6 +22,7 @@ import ( "net/http" "net/http/httputil" "net/url" + "strconv" "strings" "time" @@ -122,6 +123,12 @@ func determineUpstreamURL(sandbox *types.SandboxInfo, path string) (*url.URL, er return buildURL(ep.Protocol, ep.Endpoint) } +// validProxySchemes are the URL schemes accepted for reverse-proxy targets. +// ws/wss are included to support WebSocket upgrades. +var validProxySchemes = map[string]bool{ + "http": true, "https": true, "ws": true, "wss": true, +} + func buildURL(protocol, endpoint string) (*url.URL, error) { if protocol != "" && !strings.Contains(endpoint, "://") { endpoint = strings.ToLower(protocol) + "://" + endpoint @@ -133,9 +140,18 @@ func buildURL(protocol, endpoint string) (*url.URL, error) { if u.Scheme == "" { return nil, fmt.Errorf("invalid endpoint URL %q: missing scheme", endpoint) } + if !validProxySchemes[u.Scheme] { + return nil, fmt.Errorf("invalid endpoint URL %q: unsupported scheme %q, must be http, https, ws, or wss", endpoint, u.Scheme) + } if u.Host == "" { return nil, fmt.Errorf("invalid endpoint URL %q: missing host", endpoint) } + if portStr := u.Port(); portStr != "" { + port, err := strconv.Atoi(portStr) + if err != nil || port < 1 || port > 65535 { + return nil, fmt.Errorf("invalid endpoint URL %q: port %q out of range (1-65535)", endpoint, portStr) + } + } return u, nil } diff --git a/pkg/router/handlers_test.go b/pkg/router/handlers_test.go index 404bd680..995e855d 100644 --- a/pkg/router/handlers_test.go +++ b/pkg/router/handlers_test.go @@ -399,8 +399,11 @@ func TestForwardToSandbox_InvalidEndpoint(t *testing.T) { }{ {"malformed scheme", "://invalid-url"}, {"empty endpoint", ""}, - {"scheme only", "http://"}, + {"scheme only no host", "http://"}, {"no scheme", "localhost:8080"}, + {"unsupported scheme ftp", "ftp://host:21"}, + {"unsupported scheme file", "file:///etc/passwd"}, + {"port out of range", "http://host:99999"}, } for _, tc := range cases { From ad148fb88989c0327da5ade2c4e2ff8b90c03150 Mon Sep 17 00:00:00 2001 From: sicaario Date: Wed, 15 Apr 2026 16:27:30 -0600 Subject: [PATCH 6/6] fix: use generic 500 message, t.Setenv in tests, and Cmp for big.Int N Signed-off-by: sicaario --- pkg/router/handlers.go | 2 +- pkg/router/handlers_test.go | 10 ++++++---- pkg/router/jwt_test.go | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/pkg/router/handlers.go b/pkg/router/handlers.go index 587d2643..d86506c6 100644 --- a/pkg/router/handlers.go +++ b/pkg/router/handlers.go @@ -180,7 +180,7 @@ func (s *Server) forwardToSandbox(c *gin.Context, sandbox *types.SandboxInfo, pa if errors.Is(err, errNoEntryPoint) { c.JSON(http.StatusNotFound, gin.H{"error": err.Error()}) } else { - c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("invalid sandbox endpoint configuration: %v", err)}) + c.JSON(http.StatusInternalServerError, gin.H{"error": "invalid sandbox endpoint configuration"}) } return } diff --git a/pkg/router/handlers_test.go b/pkg/router/handlers_test.go index 995e855d..0d60087a 100644 --- a/pkg/router/handlers_test.go +++ b/pkg/router/handlers_test.go @@ -408,8 +408,9 @@ func TestForwardToSandbox_InvalidEndpoint(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - setupEnv() - defer teardownEnv() + t.Setenv("REDIS_ADDR", "localhost:6379") + t.Setenv("REDIS_PASSWORD", "test-password") + t.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080") config := &Config{Port: "8080"} server, err := NewServer(config) @@ -446,8 +447,9 @@ func TestForwardToSandbox_InvalidEndpoint(t *testing.T) { } func TestForwardToSandbox_NoEntryPoints(t *testing.T) { - setupEnv() - defer teardownEnv() + t.Setenv("REDIS_ADDR", "localhost:6379") + t.Setenv("REDIS_PASSWORD", "test-password") + t.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080") config := &Config{Port: "8080"} server, err := NewServer(config) diff --git a/pkg/router/jwt_test.go b/pkg/router/jwt_test.go index 5b2f469a..fdbe1f39 100644 --- a/pkg/router/jwt_test.go +++ b/pkg/router/jwt_test.go @@ -186,7 +186,7 @@ func TestGetPrivateKeyPEM(t *testing.T) { // Compare key components instead of whole struct — ParsePKCS1PrivateKey may // not precompute Dp/Dq/Qinv identically to the original key. - assert.Equal(t, manager.privateKey.PublicKey.N, privateKey.PublicKey.N, "Public key N should match") + assert.Equal(t, 0, manager.privateKey.PublicKey.N.Cmp(privateKey.PublicKey.N), "Public key N should match") assert.Equal(t, manager.privateKey.PublicKey.E, privateKey.PublicKey.E, "Public key E should match") assert.Equal(t, 0, manager.privateKey.D.Cmp(privateKey.D), "Private exponent D should match") assert.Equal(t, len(manager.privateKey.Primes), len(privateKey.Primes), "Number of primes should match")