From 27738af8899377d2954cf3e39fbe6fbb40643438 Mon Sep 17 00:00:00 2001 From: Krie <112789870+R-Kri@users.noreply.github.com> Date: Mon, 4 May 2026 17:52:15 +0530 Subject: [PATCH] fix: use json.Marshal in CreateResult Signed-off-by: Krie <112789870+R-Kri@users.noreply.github.com> --- pkg/connectors/microcks_client.go | 50 ++++++++++++++------- pkg/connectors/microcks_client_test.go | 61 ++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 15 deletions(-) diff --git a/pkg/connectors/microcks_client.go b/pkg/connectors/microcks_client.go index 00a2d8c..d68e4f4 100644 --- a/pkg/connectors/microcks_client.go +++ b/pkg/connectors/microcks_client.go @@ -67,6 +67,18 @@ type TestResultSummary struct { InProgress bool `json:"inProgress"` } +// createTestResultRequest is the JSON payload sent to the Microcks tests endpoint. +type createTestResultRequest struct { + ServiceID string `json:"serviceId"` + TestEndpoint string `json:"testEndpoint"` + RunnerType string `json:"runnerType"` + Timeout int64 `json:"timeout"` + SecretName string `json:"secretName,omitempty"` + FilteredOperations []string `json:"filteredOperations,omitempty"` + OperationsHeaders map[string][]HeaderDTO `json:"operationsHeaders,omitempty"` + OAuth2Context *OAuth2ClientContext `json:"oAuth2Context,omitempty"` +} + // HeaderDTO represents an operation header passed for Test type HeaderDTO struct { Name string `json:"name"` @@ -323,28 +335,36 @@ func (c *microcksClient) CreateTestResult(serviceID string, testEndpoint string, rel := &url.URL{Path: "tests"} u := c.APIURL.ResolveReference(rel) - // Prepare an input string as body. - var input = "{" - input += ("\"serviceId\": \"" + serviceID + "\", ") - input += ("\"testEndpoint\": \"" + testEndpoint + "\", ") - input += ("\"runnerType\": \"" + runnerType + "\", ") - input += ("\"timeout\": " + strconv.FormatInt(timeout, 10)) - if len(secretName) > 0 { - input += (", \"secretName\": \"" + secretName + "\"") + // Build request payload via a struct so json.Marshal handles escaping. + payload := createTestResultRequest{ + ServiceID: serviceID, + TestEndpoint: testEndpoint, + RunnerType: runnerType, + Timeout: timeout, + SecretName: secretName, } if len(filteredOperations) > 0 && ensureValidOperationsList(filteredOperations) { - input += (", \"filteredOperations\": " + filteredOperations) + var ops []string + _ = json.Unmarshal([]byte(filteredOperations), &ops) + payload.FilteredOperations = ops } if len(operationsHeaders) > 0 && ensureValidOperationsHeaders(operationsHeaders) { - input += (", \"operationsHeaders\": " + operationsHeaders) + var headers map[string][]HeaderDTO + _ = json.Unmarshal([]byte(operationsHeaders), &headers) + payload.OperationsHeaders = headers } if len(oAuth2Context) > 0 && ensureValieOAuth2Context(oAuth2Context) { - input += (", \"oAuth2Context\": " + oAuth2Context) + var oCtx OAuth2ClientContext + _ = json.Unmarshal([]byte(oAuth2Context), &oCtx) + payload.OAuth2Context = &oCtx } - input += "}" + body, err := json.Marshal(payload) + if err != nil { + return "", fmt.Errorf("failed to marshal test result request: %w", err) + } - req, err := http.NewRequest("POST", u.String(), strings.NewReader(input)) + req, err := http.NewRequest("POST", u.String(), bytes.NewReader(body)) if err != nil { return "", err } @@ -365,13 +385,13 @@ func (c *microcksClient) CreateTestResult(serviceID string, testEndpoint string, // Dump response if verbose required. config.DumpResponseIfRequired("Microcks for creating test", resp, true) - body, err := io.ReadAll(resp.Body) + respBody, err := io.ReadAll(resp.Body) if err != nil { panic(err.Error()) } var createTestResp map[string]interface{} - if err := json.Unmarshal(body, &createTestResp); err != nil { + if err := json.Unmarshal(respBody, &createTestResp); err != nil { panic(err) } diff --git a/pkg/connectors/microcks_client_test.go b/pkg/connectors/microcks_client_test.go index 9e9b6c8..0efe174 100644 --- a/pkg/connectors/microcks_client_test.go +++ b/pkg/connectors/microcks_client_test.go @@ -1,12 +1,73 @@ package connectors import ( + "encoding/json" + "io" "net/http" "net/http/httptest" "strings" "testing" ) +func TestCreateTestResultEscapesSpecialChars(t *testing.T) { + // serviceID contains quotes + backslash — old string-concat would have produced broken JSON. + maliciousServiceID := `svc"id\bad` + maliciousEndpoint := `http://evil.com", "injected":"val` + + var gotPayload map[string]interface{} + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + raw, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(raw, &gotPayload) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"id":"test-123"}`)) + })) + defer server.Close() + + client := NewMicrocksClient(server.URL) + id, err := client.CreateTestResult(maliciousServiceID, maliciousEndpoint, "HTTP", "", 5000, "", "", "") + if err != nil { + t.Fatalf("CreateTestResult returned error: %v", err) + } + if id != "test-123" { + t.Fatalf("expected id test-123, got %s", id) + } + + // Values must be preserved exactly — no injection of extra fields. + if got, ok := gotPayload["serviceId"].(string); !ok || got != maliciousServiceID { + t.Fatalf("serviceId not preserved: %v", gotPayload["serviceId"]) + } + if got, ok := gotPayload["testEndpoint"].(string); !ok || got != maliciousEndpoint { + t.Fatalf("testEndpoint not preserved: %v", gotPayload["testEndpoint"]) + } + if _, injected := gotPayload["injected"]; injected { + t.Fatal("JSON injection succeeded — extra field 'injected' found in payload") + } +} + +func TestCreateTestResultOmitsEmptySecretName(t *testing.T) { + var gotPayload map[string]interface{} + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + raw, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(raw, &gotPayload) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"id":"test-456"}`)) + })) + defer server.Close() + + client := NewMicrocksClient(server.URL) + _, err := client.CreateTestResult("svcA", "http://example.com", "HTTP", "", 3000, "", "", "") + if err != nil { + t.Fatalf("CreateTestResult returned error: %v", err) + } + if _, exists := gotPayload["secretName"]; exists { + t.Fatal("secretName should be omitted when empty") + } +} + func TestDownloadArtifactReturnsResponseBody(t *testing.T) { const expectedBody = "artifact downloaded"