Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 35 additions & 15 deletions pkg/connectors/microcks_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unmarshal errors are silently discarded. If ensureValidOperationsList says "valid" but the value isn't a []string (e.g. array of objects), the field is dropped instead of sent — a silent behavior regression vs the old code that forwarded the raw string. At minimum fall back to embedding the raw string, or return an error. I don't think we should swallow the unmarshal error.

Same concern applies to R353 and R358.

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) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nit: ensureValieOAuth2Context has a typo ("Valie"); consider renaming while you're here. Also update wherever applicabel.

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
}
Expand All @@ -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)
}

Expand Down
61 changes: 61 additions & 0 deletions pkg/connectors/microcks_client_test.go
Original file line number Diff line number Diff line change
@@ -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"

Expand Down