fix: reject null credits and cweIds on advisory create/update

Peer review follow-up: null optional fields no longer bypass the update
guard or produce silent no-op PATCH bodies. Adds handler and parser tests
for null vulnerabilities on create and null credits/cweIds on update.

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: Array Fleet

pkg/github/security_advisories_test.go

@@ -768,6 +768,39 @@ func Test_CreateRepositorySecurityAdvisory(t *testing.T) {
 			expectError:    true,
 			expectedErrMsg: "invalid vulnerabilities: at least one vulnerability must be provided",
 		},
+		{
+			name: "reject null vulnerabilities",
+			mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+				PostReposSecurityAdvisoriesByOwnerByRepo: mockResponse(t, http.StatusCreated, mockAdvisory),
+			}),
+			requestArgs: map[string]any{
+				"owner":           "octo",
+				"repo":            "hello-world",
+				"summary":         "Stored XSS in Core",
+				"description":     "A stored XSS vulnerability in Core.",
+				"severity":        "high",
+				"vulnerabilities": nil,
+			},
+			expectError:    true,
+			expectedErrMsg: "invalid vulnerabilities: at least one vulnerability must be provided",
+		},
+		{
+			name: "reject null credits",
+			mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+				PostReposSecurityAdvisoriesByOwnerByRepo: mockResponse(t, http.StatusCreated, mockAdvisory),
+			}),
+			requestArgs: map[string]any{
+				"owner":           "octo",
+				"repo":            "hello-world",
+				"summary":         "Stored XSS in Core",
+				"description":     "A stored XSS vulnerability in Core.",
+				"severity":        "high",
+				"vulnerabilities": sampleAdvisoryVulnerabilities(),
+				"credits":         nil,
+			},
+			expectError:    true,
+			expectedErrMsg: "invalid credits: value must not be null",
+		},
 		{
 			name: "API error handling",
 			mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
@@ -988,6 +1021,34 @@ func Test_UpdateRepositorySecurityAdvisory(t *testing.T) {
 			expectError:    true,
 			expectedErrMsg: "invalid vulnerabilities: at least one vulnerability must be provided",
 		},
+		{
+			name: "reject null credits only",
+			mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+				PatchReposSecurityAdvisoriesByOwnerByRepoByGhsaID: mockResponse(t, http.StatusOK, mockAdvisory),
+			}),
+			requestArgs: map[string]any{
+				"owner":   "octo",
+				"repo":    "hello-world",
+				"ghsaId":  "GHSA-xxxx-xxxx-xxxx",
+				"credits": nil,
+			},
+			expectError:    true,
+			expectedErrMsg: "invalid credits: value must not be null",
+		},
+		{
+			name: "reject null cweIds only",
+			mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+				PatchReposSecurityAdvisoriesByOwnerByRepoByGhsaID: mockResponse(t, http.StatusOK, mockAdvisory),
+			}),
+			requestArgs: map[string]any{
+				"owner":  "octo",
+				"repo":   "hello-world",
+				"ghsaId": "GHSA-xxxx-xxxx-xxxx",
+				"cweIds": nil,
+			},
+			expectError:    true,
+			expectedErrMsg: "invalid cweIds: value must not be null",
+		},
 		{
 			name: "successful update with credits and cweIds",
 			mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
@@ -1374,6 +1435,32 @@ func Test_ParseAdvisoryCredits(t *testing.T) {
 		require.Error(t, err)
 		assert.Contains(t, err.Error(), `credits[0].type "invalid-type" is invalid`)
 	})
+
+	t.Run("null credits rejected", func(t *testing.T) {
+		_, err := parseAdvisoryCredits(map[string]any{
+			"credits": nil,
+		}, "credits")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "invalid credits: value must not be null")
+	})
+}
+
+func Test_ParseAdvisoryCWEIDs(t *testing.T) {
+	t.Run("valid cweIds", func(t *testing.T) {
+		cweIDs, err := parseAdvisoryCWEIDs(map[string]any{
+			"cweIds": []any{"CWE-79"},
+		}, "cweIds")
+		require.NoError(t, err)
+		assert.Equal(t, []string{"CWE-79"}, cweIDs)
+	})
+
+	t.Run("null cweIds rejected", func(t *testing.T) {
+		_, err := parseAdvisoryCWEIDs(map[string]any{
+			"cweIds": nil,
+		}, "cweIds")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "invalid cweIds: value must not be null")
+	})
 }
 
 func Test_validateSeverityOrCVSS(t *testing.T) {

pkg/github/security_advisories_write.go

@@ -190,11 +190,25 @@ func parseAdvisoryVulnerabilities(args map[string]any, param string, required bo
 	return vulns, nil
 }
 
+func parseAdvisoryCWEIDs(args map[string]any, param string) ([]string, error) {
+	raw, ok := args[param]
+	if !ok {
+		return nil, nil
+	}
+	if raw == nil {
+		return nil, fmt.Errorf("invalid %s: value must not be null", param)
+	}
+	return OptionalStringArrayParam(args, param)
+}
+
 func parseAdvisoryCredits(args map[string]any, param string) ([]advisoryCreditRequest, error) {
 	raw, ok := args[param]
-	if !ok || raw == nil {
+	if !ok {
 		return nil, nil
 	}
+	if raw == nil {
+		return nil, fmt.Errorf("invalid %s: value must not be null", param)
+	}
 
 	data, err := json.Marshal(raw)
 	if err != nil {
@@ -389,7 +403,7 @@ func CreateRepositorySecurityAdvisory(t translations.TranslationHelperFunc) inve
 			if err != nil {
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
-			cweIDs, err := OptionalStringArrayParam(args, "cweIds")
+			cweIDs, err := parseAdvisoryCWEIDs(args, "cweIds")
 			if err != nil {
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
@@ -561,7 +575,7 @@ func UpdateRepositorySecurityAdvisory(t translations.TranslationHelperFunc) inve
 			if err != nil {
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
-			cweIDs, err := OptionalStringArrayParam(args, "cweIds")
+			cweIDs, err := parseAdvisoryCWEIDs(args, "cweIds")
 			if err != nil {
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}