fix: prevent STQL injection and reachable panics in service check/discovery

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## Unreleased
+
+- fix: escape the service id used in the StackState snapshot query and build the request body with a JSON encoder, preventing STQL/JSON query injection
+- fix: guard the service check and discovery against missing components, identifiers, short base URLs and unexpected identifier formats instead of panicking, and avoid a possible nil-dereference when a StackState request fails before a response is received
+
 ## v1.0.27
 
 - chore(deps): bump github.com/steadybit/extension-kit

extservice/common.go

@@ -5,6 +5,7 @@ package extservice
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"github.com/go-resty/resty/v2"
 )
@@ -31,17 +32,30 @@ type StackStateHttpClient struct {
 }
 
 func (s *StackStateHttpClient) GetServiceSnapshot(ctx context.Context, serviceId string) (*resty.Response, ViewSnapshotResponseWrapper, error) {
-	return s.executeSnapshotQuery(ctx, fmt.Sprintf("(id = \\\"%s\\\")", serviceId))
+	return s.executeSnapshotQuery(ctx, fmt.Sprintf("(id = %s)", stqlString(serviceId)))
 }
 
 func (s *StackStateHttpClient) GetServiceSnapshots(ctx context.Context) (*resty.Response, ViewSnapshotResponseWrapper, error) {
-	return s.executeSnapshotQuery(ctx, "(type = \\\"service\\\")")
+	return s.executeSnapshotQuery(ctx, `(type = "service")`)
+}
+
+// stqlString renders a value as a quoted, escaped string literal using JSON string escaping,
+// which escapes the quotes and backslashes that could otherwise let the value break out of an
+// STQL string literal and inject into the query.
+func stqlString(value string) string {
+	encoded, _ := json.Marshal(value)
+	return string(encoded)
 }
 
 func (s *StackStateHttpClient) executeSnapshotQuery(ctx context.Context, query string) (*resty.Response, ViewSnapshotResponseWrapper, error) {
+	// Encode the query as a JSON string so it is correctly escaped inside the request body.
+	queryJSON, err := json.Marshal(query)
+	if err != nil {
+		return nil, ViewSnapshotResponseWrapper{}, err
+	}
 	requestBody := fmt.Sprintf(`{
     "_type": "ViewSnapshotRequest",
-    "query": "%v",
+    "query": %s,
     "queryVersion": "0.0.1",
     "metadata": {
         "_type": "QueryMetadata",
@@ -57,7 +71,7 @@ func (s *StackStateHttpClient) executeSnapshotQuery(ctx context.Context, query s
         "neighboringComponents": false,
         "showFullComponent": false
     }
-  }`, query)
+  }`, queryJSON)
 	var stackStateResponse ViewSnapshotResponseWrapper
 	response, err := s.Client.R().
 		SetContext(ctx).

extservice/common_test.go

@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: MIT
+// SPDX-FileCopyrightText: 2024 Steadybit GmbH
+
+package extservice
+
+import (
+	"context"
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/go-resty/resty/v2"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestStqlString(t *testing.T) {
+	assert.Equal(t, `"a\"b\\c"`, stqlString(`a"b\c`))
+}
+
+func TestGetServiceSnapshot_EscapesServiceId(t *testing.T) {
+	var capturedBody []byte
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		capturedBody, _ = io.ReadAll(r.Body)
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{}`))
+	}))
+	defer srv.Close()
+
+	client := &StackStateHttpClient{Client: resty.New().SetBaseURL(srv.URL)}
+	// A service id that tries to break out of the STQL string literal / JSON body.
+	maliciousId := `1") OR (1=1`
+	_, _, err := client.GetServiceSnapshot(context.Background(), maliciousId)
+	require.NoError(t, err)
+
+	// The request body must remain valid JSON despite the embedded quotes...
+	var body struct {
+		Query string `json:"query"`
+	}
+	require.NoError(t, json.Unmarshal(capturedBody, &body), "request body is not valid JSON: %s", capturedBody)
+
+	// ...and the id stays inside a single, escaped STQL string literal (no breakout).
+	assert.Equal(t, `(id = "1\") OR (1=1")`, body.Query)
+}

extservice/service_check.go

@@ -247,7 +247,7 @@ func MonitorStatusCheckStatus(ctx context.Context, state *ServiceStatusCheckStat
 func loadServiceComponent(ctx context.Context, state *ServiceStatusCheckState, api GetSnapshotApi) (*Component, error) {
 	res, stackStateResponse, err := api.GetServiceSnapshot(ctx, state.ServiceId)
 	if err != nil {
-		return nil, new(extension_kit.ToError(fmt.Sprintf("Failed to retrieve service states from StackState for Service ID %s. Full response: %v", state.ServiceId, res.String()), err))
+		return nil, new(extension_kit.ToError(fmt.Sprintf("Failed to retrieve service states from StackState for Service ID %s.", state.ServiceId), err))
 	}
 	if !res.IsSuccess() {
 		log.Err(err).Msgf("StackState API responded with unexpected status code %d while retrieving service states for Service ID %s. Full response: %v", res.StatusCode(), state.ServiceId, res.String())
@@ -264,6 +264,9 @@ func loadServiceComponent(ctx context.Context, state *ServiceStatusCheckState, a
 			Identifiers: []string{fmt.Sprintf("urn:service:/%s:%s:%s", state.ClusterName, state.ServiceName, state.ServiceId)},
 		}, nil
 	}
+	if len(stackStateResponse.ViewSnapshotResponse.Components) == 0 {
+		return nil, new(extension_kit.ToError(fmt.Sprintf("StackState returned no components for Service ID %s.", state.ServiceId), nil))
+	}
 	return &stackStateResponse.ViewSnapshotResponse.Components[0], nil
 }
 
@@ -282,7 +285,15 @@ func toMetric(service *Component, now time.Time) *action_kit_api.Metric {
 		state = "danger"
 	}
 
-	uiBaseUrl := config.Config.ApiBaseUrl[:(len(config.Config.ApiBaseUrl) - 3)]
+	uiBaseUrl := config.Config.ApiBaseUrl
+	if len(uiBaseUrl) >= 3 {
+		uiBaseUrl = uiBaseUrl[:len(uiBaseUrl)-3]
+	}
+
+	serviceUrl := ""
+	if len(service.Identifiers) > 0 {
+		serviceUrl = fmt.Sprintf("%s/#/components/%s", uiBaseUrl, url.QueryEscape(service.Identifiers[0]))
+	}
 
 	return new(action_kit_api.Metric{
 		Name: new("stackstate_service_status"),
@@ -291,7 +302,7 @@ func toMetric(service *Component, now time.Time) *action_kit_api.Metric {
 			attributeK8ServiceName: service.Name,
 			attributeState:         state,
 			attributeTooltip:       tooltip,
-			attributeUrl:           fmt.Sprintf("%s/#/components/%s", uiBaseUrl, url.QueryEscape(service.Identifiers[0])),
+			attributeUrl:           serviceUrl,
 		},
 		Timestamp: now,
 		Value:     0,

extservice/service_discovery.go

@@ -10,6 +10,8 @@ package extservice
 import (
 	"context"
 	"fmt"
+	"strings"
+
 	"github.com/go-resty/resty/v2"
 	"github.com/rs/zerolog/log"
 	"github.com/steadybit/discovery-kit/go/discovery_kit_api"
@@ -105,7 +107,7 @@ func getAllServices(ctx context.Context, api GetSnapshotsApi) []discovery_kit_ap
 	res, stackStateResponse, err := api.GetServiceSnapshots(ctx)
 
 	if err != nil {
-		log.Err(err).Msgf("Failed to retrieve service states from Stack State. Full response: %v", res.String())
+		log.Err(err).Msgf("Failed to retrieve service states from Stack State.")
 		return result
 	}
 
@@ -127,8 +129,8 @@ func getAllServices(ctx context.Context, api GetSnapshotsApi) []discovery_kit_ap
 }
 
 func toService(service Component) discovery_kit_api.Target {
-	clusterName := service.Properties.ClusterNameIdentifier[len("urn:cluster:/kubernetes:"):]
-	namespace := service.Properties.NamespaceIdentifier[len(fmt.Sprintf("urn:kubernetes:/%v:namespace/", clusterName)):]
+	clusterName := strings.TrimPrefix(service.Properties.ClusterNameIdentifier, "urn:cluster:/kubernetes:")
+	namespace := strings.TrimPrefix(service.Properties.NamespaceIdentifier, fmt.Sprintf("urn:kubernetes:/%v:namespace/", clusterName))
 	return discovery_kit_api.Target{
 		Id:         strconv.Itoa(service.Id),
 		Label:      service.Name,