Skip to content

Commit de6b7d6

Browse files
advancedresearcharraycursoragent
andcommitted
fix: enforce severity/cvss validation for security advisory writes
Implement validateSeverityOrCVSS that was documented but missing from the prior commit. Create requires exactly one of severity or cvssVectorString; update rejects both together. Also require package name in vulnerability schema per GitHub API. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent a2f8099 commit de6b7d6

5 files changed

Lines changed: 91 additions & 7 deletions

File tree

README.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1362,12 +1362,12 @@ The following sets of tools are available:
13621362
- **Accepted OAuth Scopes**: `repo`, `security_events`
13631363
- `credits`: Users credited for the advisory. (object[], optional)
13641364
- `cveId`: The CVE ID to assign to the advisory. (string, optional)
1365-
- `cvssVectorString`: The CVSS vector string for the advisory. Exactly one of severity or cvssVectorString is required. (string, optional)
1365+
- `cvssVectorString`: The CVSS vector string for the advisory. (string, optional)
13661366
- `cweIds`: Common Weakness Enumeration IDs (for example, ["CWE-79"]). (string[], optional)
13671367
- `description`: A detailed description of the security advisory. (string, required)
13681368
- `owner`: The owner of the repository. (string, required)
13691369
- `repo`: The name of the repository. (string, required)
1370-
- `severity`: The severity of the advisory. Exactly one of severity or cvssVectorString is required. (string, optional)
1370+
- `severity`: The severity of the advisory. (string, optional)
13711371
- `startPrivateFork`: Whether to create a temporary private fork for collaborating on a fix. (boolean, optional)
13721372
- `summary`: A short summary of the security advisory. (string, required)
13731373
- `vulnerabilities`: Affected products and version ranges. (object[], required)
@@ -1421,13 +1421,13 @@ The following sets of tools are available:
14211421
- **Accepted OAuth Scopes**: `repo`, `security_events`
14221422
- `credits`: Users credited for the advisory. (object[], optional)
14231423
- `cveId`: The CVE ID to assign to the advisory. (string, optional)
1424-
- `cvssVectorString`: The CVSS vector string for the advisory. Cannot be set together with severity. (string, optional)
1424+
- `cvssVectorString`: The CVSS vector string for the advisory. (string, optional)
14251425
- `cweIds`: Common Weakness Enumeration IDs (for example, ["CWE-79"]). (string[], optional)
14261426
- `description`: A detailed description of the security advisory. (string, optional)
14271427
- `ghsaId`: GitHub Security Advisory ID (format: GHSA-xxxx-xxxx-xxxx). (string, required)
14281428
- `owner`: The owner of the repository. (string, required)
14291429
- `repo`: The name of the repository. (string, required)
1430-
- `severity`: The severity of the advisory. Cannot be set together with cvssVectorString. (string, optional)
1430+
- `severity`: The severity of the advisory. (string, optional)
14311431
- `state`: The advisory state. Set to "published" to publish the advisory. (string, optional)
14321432
- `summary`: A short summary of the security advisory. (string, optional)
14331433
- `vulnerabilities`: Affected products and version ranges. (object[], optional)

pkg/github/__toolsnaps__/create_repository_security_advisory.snap

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,8 @@
114114
}
115115
},
116116
"required": [
117-
"ecosystem"
117+
"ecosystem",
118+
"name"
118119
],
119120
"type": "object"
120121
},

pkg/github/__toolsnaps__/update_repository_security_advisory.snap

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,8 @@
124124
}
125125
},
126126
"required": [
127-
"ecosystem"
127+
"ecosystem",
128+
"name"
128129
],
129130
"type": "object"
130131
},

pkg/github/security_advisories_write.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ var securityAdvisoryPackageSchema = &jsonschema.Schema{
3030
Description: "The package name.",
3131
},
3232
},
33-
Required: []string{"ecosystem"},
33+
Required: []string{"ecosystem", "name"},
3434
}
3535

3636
var securityAdvisoryVulnerabilitySchema = &jsonschema.Schema{
@@ -162,6 +162,18 @@ func optionalStringPtr(value string) *string {
162162
return &value
163163
}
164164

165+
func validateSeverityOrCVSS(severity, cvssVectorString string, requireOne bool) error {
166+
hasSeverity := severity != ""
167+
hasCVSS := cvssVectorString != ""
168+
if hasSeverity && hasCVSS {
169+
return fmt.Errorf("severity and cvssVectorString cannot both be set")
170+
}
171+
if requireOne && !hasSeverity && !hasCVSS {
172+
return fmt.Errorf("exactly one of severity or cvssVectorString must be provided")
173+
}
174+
return nil
175+
}
176+
165177
func marshalRepositorySecurityAdvisoryResponse(advisory *github.SecurityAdvisory) (*mcp.CallToolResult, error) {
166178
r, err := json.Marshal(advisory)
167179
if err != nil {
@@ -301,6 +313,9 @@ func CreateRepositorySecurityAdvisory(t translations.TranslationHelperFunc) inve
301313
if err != nil {
302314
return utils.NewToolResultError(err.Error()), nil, nil
303315
}
316+
if err := validateSeverityOrCVSS(severity, cvssVectorString, true); err != nil {
317+
return utils.NewToolResultError(err.Error()), nil, nil
318+
}
304319

305320
requestBody := createRepositorySecurityAdvisoryRequest{
306321
Summary: summary,
@@ -464,6 +479,9 @@ func UpdateRepositorySecurityAdvisory(t translations.TranslationHelperFunc) inve
464479
if err != nil {
465480
return utils.NewToolResultError(err.Error()), nil, nil
466481
}
482+
if err := validateSeverityOrCVSS(severity, cvssVectorString, false); err != nil {
483+
return utils.NewToolResultError(err.Error()), nil, nil
484+
}
467485

468486
requestBody := updateRepositorySecurityAdvisoryRequest{}
469487
if summary != "" {

pkg/github/security_advisories_write_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,38 @@ func Test_CreateRepositorySecurityAdvisory(t *testing.T) {
137137
expectError: true,
138138
expectedErrMsg: "missing required parameter: summary",
139139
},
140+
{
141+
name: "missing severity and cvss",
142+
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
143+
PostReposSecurityAdvisoriesByOwnerByRepo: mockResponse(t, http.StatusCreated, mockAdvisory),
144+
}),
145+
requestArgs: map[string]any{
146+
"owner": "octo",
147+
"repo": "hello-world",
148+
"summary": "Stored XSS in Core",
149+
"description": "A stored XSS vulnerability in Core.",
150+
"vulnerabilities": sampleAdvisoryVulnerabilities(),
151+
},
152+
expectError: true,
153+
expectedErrMsg: "exactly one of severity or cvssVectorString must be provided",
154+
},
155+
{
156+
name: "both severity and cvss set",
157+
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
158+
PostReposSecurityAdvisoriesByOwnerByRepo: mockResponse(t, http.StatusCreated, mockAdvisory),
159+
}),
160+
requestArgs: map[string]any{
161+
"owner": "octo",
162+
"repo": "hello-world",
163+
"summary": "Stored XSS in Core",
164+
"description": "A stored XSS vulnerability in Core.",
165+
"severity": "high",
166+
"cvssVectorString": "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H",
167+
"vulnerabilities": sampleAdvisoryVulnerabilities(),
168+
},
169+
expectError: true,
170+
expectedErrMsg: "severity and cvssVectorString cannot both be set",
171+
},
140172
{
141173
name: "API error handling",
142174
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
@@ -150,6 +182,7 @@ func Test_CreateRepositorySecurityAdvisory(t *testing.T) {
150182
"repo": "hello-world",
151183
"summary": "Stored XSS in Core",
152184
"description": "A stored XSS vulnerability in Core.",
185+
"severity": "high",
153186
"vulnerabilities": sampleAdvisoryVulnerabilities(),
154187
},
155188
expectError: true,
@@ -261,6 +294,21 @@ func Test_UpdateRepositorySecurityAdvisory(t *testing.T) {
261294
expectError: true,
262295
expectedErrMsg: "at least one of summary, description, vulnerabilities, cveId, cweIds, severity, cvssVectorString, credits, or state must be provided for update",
263296
},
297+
{
298+
name: "both severity and cvss set on update",
299+
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
300+
PatchReposSecurityAdvisoriesByOwnerByRepoByGhsaID: mockResponse(t, http.StatusOK, mockAdvisory),
301+
}),
302+
requestArgs: map[string]any{
303+
"owner": "octo",
304+
"repo": "hello-world",
305+
"ghsaId": "GHSA-xxxx-xxxx-xxxx",
306+
"severity": "high",
307+
"cvssVectorString": "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H",
308+
},
309+
expectError: true,
310+
expectedErrMsg: "severity and cvssVectorString cannot both be set",
311+
},
264312
{
265313
name: "API error handling",
266314
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
@@ -448,3 +496,19 @@ func Test_ParseAdvisoryVulnerabilities(t *testing.T) {
448496
assert.Equal(t, "example-package", *vulns[0].Package.Name)
449497
})
450498
}
499+
500+
func Test_validateSeverityOrCVSS(t *testing.T) {
501+
t.Run("create requires exactly one", func(t *testing.T) {
502+
assert.NoError(t, validateSeverityOrCVSS("high", "", true))
503+
assert.NoError(t, validateSeverityOrCVSS("", "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H", true))
504+
assert.Error(t, validateSeverityOrCVSS("", "", true))
505+
assert.Error(t, validateSeverityOrCVSS("high", "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H", true))
506+
})
507+
508+
t.Run("update rejects both but allows neither", func(t *testing.T) {
509+
assert.NoError(t, validateSeverityOrCVSS("", "", false))
510+
assert.NoError(t, validateSeverityOrCVSS("high", "", false))
511+
assert.NoError(t, validateSeverityOrCVSS("", "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H", false))
512+
assert.Error(t, validateSeverityOrCVSS("high", "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H", false))
513+
})
514+
}

0 commit comments

Comments
 (0)