mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-20 11:21:49 +00:00
fix: preserve escaped path through the console proxy for SigV4 (#931)
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.
This commit is contained in:
@@ -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")
|
||||
|
||||
+21
-10
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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 = "/"
|
||||
}
|
||||
|
||||
@@ -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{
|
||||
|
||||
Reference in New Issue
Block a user