From 9799ea410cfe6c21d0f6b978ba50c8eeecac95a6 Mon Sep 17 00:00:00 2001
From: Josh Siviour <mrk@mulgadc.com>
Date: Fri, 28 Aug 2026 10:47:40 +1000
Subject: [PATCH] fix: preserve escaped path through the console proxy for
 SigV4 (#931)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The spinifex-ui reverse proxy copied only the decoded URL path and dropped
RawPath, so a percent-encoded path segment (a ':' in a bedrock modelId) was
re-encoded before reaching the gateway. The gateway then canonicalised a
different path than the one the browser SDK signed, and SigV4 rejected the
request with a signature mismatch — surfacing as an unparseable '<?xml'
deserialization error in the console Playground.

Carry RawPath through the rewrite so the escaped path arrives byte-identical.
Also route the auth, cluster-unavailable, and throttle error emitters through a
shared jsonErrorService helper so the bedrock family receives AWS JSON 1.1
errors the SDK can parse, and stamp the signed service onto the request before
verification so a rejection renders in the client's format.
---
 spinifex/gateway/auth.go                      | 16 ++++++++-
 spinifex/gateway/gateway.go                   | 31 +++++++++++------
 spinifex/gateway/gateway_test.go              | 33 +++++++++++++++++++
 spinifex/services/spinifexui/proxy.go         |  7 +++-
 .../services/spinifexui/spinifexui_test.go    | 23 +++++++++++++
 5 files changed, 98 insertions(+), 12 deletions(-)

diff --git a/spinifex/gateway/auth.go b/spinifex/gateway/auth.go
index fabb06922..e46027614 100644
--- a/spinifex/gateway/auth.go
+++ b/spinifex/gateway/auth.go
@@ -85,6 +85,11 @@ func (gw *GatewayConfig) SigV4AuthMiddleware() func(http.Handler) http.Handler {
 			// every rejection below still shares.
 			auditFrom(r.Context()).setAccessKeyID(sig.Credential.AccessKeyID)
 
+			// Carry the signed service now, so a rejection below (a signature
+			// mismatch on a bedrock Converse) renders in the client's format —
+			// GetService reads this to pick JSON over XML for the bedrock family.
+			r = r.WithContext(context.WithValue(r.Context(), ctxService, sig.Credential.Service))
+
 			// Reject unknown services before crypto; otherwise Verify re-signs with
 			// the client-claimed service name and rubber-stamps the scope.
 			if !supportedServices[sig.Credential.Service] {
@@ -362,7 +367,7 @@ func (gw *GatewayConfig) resolveSessionAKID(r *http.Request, accessKeyID, client
 	}, ""
 }
 
-// writeSigV4Error writes an EC2-compatible XML error response for auth failures.
+// writeSigV4Error writes an auth-failure error in the service-appropriate format.
 // Every auth rejection funnels through here, a rate-limit lockout included, so
 // this is the one place that has to record the verdict onto the request.
 func (gw *GatewayConfig) writeSigV4Error(w http.ResponseWriter, r *http.Request, errorCode string) {
@@ -374,6 +379,15 @@ func (gw *GatewayConfig) writeSigV4Error(w http.ResponseWriter, r *http.Request,
 		errorMsg = awserrors.ErrorMessage{HTTPCode: 500, Message: "Internal error"}
 	}
 
+	// AWS JSON 1.1 services (bedrock family, EKS, …) need a JSON error body, or
+	// the SDK chokes deserializing our XML into its shape.
+	if svc, _ := gw.GetService(r); jsonErrorService(svc) {
+		w.Header().Set("Content-Type", eksJSONContentType)
+		w.WriteHeader(errorMsg.HTTPCode)
+		_, _ = w.Write(GenerateEKSErrorResponse(errorCode, errorMsg.Message, requestID))
+		return
+	}
+
 	xmlError := GenerateEC2ErrorResponse(errorCode, errorMsg.Message, requestID)
 
 	w.Header().Set("Content-Type", "application/xml")
diff --git a/spinifex/gateway/gateway.go b/spinifex/gateway/gateway.go
index 60a449a4c..ff5363d6c 100644
--- a/spinifex/gateway/gateway.go
+++ b/spinifex/gateway/gateway.go
@@ -331,6 +331,18 @@ func (gw *GatewayConfig) throttleKeyFuncs() []ratelimit.KeyFunc {
 // eksJSONContentType is the AWS REST-JSON 1.1 content type EKS clients expect.
 const eksJSONContentType = "application/x-amz-json-1.1"
 
+// jsonErrorService reports whether svc returns AWS JSON 1.1 errors rather than
+// XML. One source of truth so every error emitter agrees with ErrorHandler; an
+// XML body to these clients is an unparseable "<?xml…" deserialization error.
+func jsonErrorService(svc string) bool {
+	switch svc {
+	case "eks", "ecr", "acm", "ecs", "tagging",
+		"bedrock", "bedrock-runtime", "bedrock-agent", "bedrock-agent-runtime":
+		return true
+	}
+	return false
+}
+
 // clusterUnavailableMsg is the 503 body when NATS is disconnected. Points
 // operators at /local/status rather than leaving the AWS CLI hanging on timeouts.
 const clusterUnavailableMsg = "cluster unavailable: NATS disconnected — check daemon /local/status"
@@ -341,13 +353,13 @@ const clusterUnavailableMsg = "cluster unavailable: NATS disconnected — check
 func (gw *GatewayConfig) writeClusterUnavailable(w http.ResponseWriter, _ *http.Request, svc string) {
 	requestID := uuid.NewV4().String()
 
-	// EKS and ECS use AWS JSON 1.1.
-	if svc == "eks" || svc == "ecs" {
+	// AWS JSON 1.1 services (EKS/ECS/bedrock family, …) get a JSON body.
+	if jsonErrorService(svc) {
 		body := GenerateEKSErrorResponse(awserrors.ErrorServiceUnavailable, clusterUnavailableMsg, requestID)
 		w.Header().Set("Content-Type", eksJSONContentType)
 		w.WriteHeader(http.StatusServiceUnavailable)
 		if _, err := w.Write(body); err != nil {
-			slog.Error("Failed to write EKS cluster-unavailable response", "err", err)
+			slog.Error("Failed to write JSON cluster-unavailable response", "err", err)
 		}
 		return
 	}
@@ -393,13 +405,13 @@ func (gw *GatewayConfig) writeThrottleError(w http.ResponseWriter, r *http.Reque
 	}
 	errorMsg := awserrors.ErrorLookup[errorCode]
 
-	// EKS and ECS use AWS JSON 1.1.
-	if svc == "eks" || svc == "ecs" {
+	// AWS JSON 1.1 services (EKS/ECS/bedrock family, …) get a JSON body.
+	if jsonErrorService(svc) {
 		body := GenerateEKSErrorResponse(errorCode, errorMsg.Message, requestID)
 		w.Header().Set("Content-Type", eksJSONContentType)
 		w.WriteHeader(errorMsg.HTTPCode)
 		if _, err := w.Write(body); err != nil {
-			slog.Error("Failed to write EKS throttle error response", "err", err)
+			slog.Error("Failed to write JSON throttle error response", "err", err)
 		}
 		return
 	}
@@ -752,10 +764,9 @@ func (gw *GatewayConfig) ErrorHandler(w http.ResponseWriter, r *http.Request, er
 		errorMsg.HTTPCode = 500
 	}
 
-	// EKS, ECR, ACM, ECS, tagging, and the bedrock/bedrock-runtime/bedrock-agent/
-	// bedrock-agent-runtime family use AWS JSON 1.1; query/XML services fall
-	// through.
-	if svc == "eks" || svc == "ecr" || svc == "acm" || svc == "ecs" || svc == "tagging" || svc == "bedrock" || svc == "bedrock-runtime" || svc == "bedrock-agent" || svc == "bedrock-agent-runtime" {
+	// EKS, ECR, ACM, ECS, tagging, and the bedrock family use AWS JSON 1.1;
+	// query/XML services fall through.
+	if jsonErrorService(svc) {
 		body := GenerateEKSErrorResponse(code, errorMsg.Message, requestId)
 		slog.Debug("Generated JSON error response", "service", svc, "error", err, "code", code, "json", string(body), "requestId", requestId)
 		w.Header().Set("Content-Type", eksJSONContentType)
diff --git a/spinifex/gateway/gateway_test.go b/spinifex/gateway/gateway_test.go
index 28baea544..7848cf331 100644
--- a/spinifex/gateway/gateway_test.go
+++ b/spinifex/gateway/gateway_test.go
@@ -1462,6 +1462,39 @@ func TestWriteThrottleError_IAM(t *testing.T) {
 	assert.Contains(t, string(body), "<ErrorResponse>")
 }
 
+func TestWriteThrottleError_Bedrock(t *testing.T) {
+	gw := &GatewayConfig{DisableLogging: true}
+
+	req := httptest.NewRequest(http.MethodPost, "/model/meta.llama3-2-1b-instruct-v1%3A0/converse", nil)
+	req = req.WithContext(context.WithValue(req.Context(), ctxService, "bedrock"))
+	w := httptest.NewRecorder()
+
+	gw.writeThrottleError(w, req)
+
+	resp := w.Result()
+	body, _ := io.ReadAll(resp.Body)
+	assert.Equal(t, eksJSONContentType, resp.Header.Get("Content-Type"), "bedrock must get JSON, not XML")
+	assert.Contains(t, string(body), `"__type"`)
+	assert.NotContains(t, string(body), "<?xml")
+}
+
+func TestWriteSigV4Error_BedrockEmitsJSON(t *testing.T) {
+	gw := &GatewayConfig{DisableLogging: true}
+
+	req := httptest.NewRequest(http.MethodPost, "/model/meta.llama3-2-1b-instruct-v1%3A0/converse", nil)
+	req = req.WithContext(context.WithValue(req.Context(), ctxService, "bedrock"))
+	w := httptest.NewRecorder()
+
+	gw.writeSigV4Error(w, req, awserrors.ErrorSignatureDoesNotMatch)
+
+	resp := w.Result()
+	body, _ := io.ReadAll(resp.Body)
+	assert.Equal(t, 403, resp.StatusCode)
+	assert.Equal(t, eksJSONContentType, resp.Header.Get("Content-Type"), "a bedrock auth failure must be SDK-parseable JSON")
+	assert.Contains(t, string(body), "SignatureDoesNotMatch")
+	assert.NotContains(t, string(body), "<?xml")
+}
+
 func TestThrottleMiddleware_Integration(t *testing.T) {
 	cfg := ratelimit.Config{
 		Enabled: true,
diff --git a/spinifex/services/spinifexui/proxy.go b/spinifex/services/spinifexui/proxy.go
index 0050e72d6..cdfb5b7cf 100644
--- a/spinifex/services/spinifexui/proxy.go
+++ b/spinifex/services/spinifexui/proxy.go
@@ -59,8 +59,13 @@ func newReverseProxy(backendHost, pathPrefix string, transport http.RoundTripper
 			pr.SetURL(target)
 			pr.Out.Host = target.Host
 
-			// Strip the proxy path prefix.
+			// Strip the prefix from both path forms. RawPath must survive: a
+			// client that percent-encodes a segment (a ':' in a bedrock
+			// modelId) signs it, so dropping it re-encodes and breaks SigV4.
 			pr.Out.URL.Path = strings.TrimPrefix(pr.In.URL.Path, pathPrefix)
+			if pr.In.URL.RawPath != "" {
+				pr.Out.URL.RawPath = strings.TrimPrefix(pr.In.URL.RawPath, pathPrefix)
+			}
 			if pr.Out.URL.Path == "" {
 				pr.Out.URL.Path = "/"
 			}
diff --git a/spinifex/services/spinifexui/spinifexui_test.go b/spinifex/services/spinifexui/spinifexui_test.go
index 40e02af53..a7f3f3a23 100644
--- a/spinifex/services/spinifexui/spinifexui_test.go
+++ b/spinifex/services/spinifexui/spinifexui_test.go
@@ -347,6 +347,29 @@ func TestNewReverseProxy_StripsPrefixAndSetsHost(t *testing.T) {
 	}
 }
 
+// TestNewReverseProxy_PreservesEscapedPath guards the SigV4 contract: a
+// percent-encoded path segment (a ':' in a bedrock modelId) must reach the
+// backend byte-identical, or the gateway recanonicalises it and the signature
+// the client computed over the escaped form no longer verifies.
+func TestNewReverseProxy_PreservesEscapedPath(t *testing.T) {
+	var gotEscaped string
+	backend := httptest.NewTLSServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+		gotEscaped = r.URL.EscapedPath()
+	}))
+	defer backend.Close()
+
+	transport, ok := backend.Client().Transport.(*http.Transport)
+	require.True(t, ok, "expected *http.Transport")
+
+	proxy := newReverseProxy(backend.Listener.Addr().String(), "/proxy/awsgw", transport)
+
+	const escaped = "/proxy/awsgw/model/meta.llama3-2-1b-instruct-v1%3A0/converse"
+	proxy.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodPost, escaped, nil))
+
+	assert.Equal(t, "/model/meta.llama3-2-1b-instruct-v1%3A0/converse", gotEscaped,
+		"escaped path must survive the proxy byte-identical")
+}
+
 func TestNewReverseProxy_ErrorHandler(t *testing.T) {
 	// Use a transport that will fail to connect (no backend listening).
 	transport := &http.Transport{