Skip to content

Add a security implementation issues validator

Mayra Cabrera requested to merge validate-security-implementation-issues into master

What does this MR do and why?

Implements a class to validate security implementation issues. For starters, the only validation ensures each implementation issue has a valid CVES issue associated and also validates this one has a valid YAML defined.

The validation is run whenever a security issue is linked to the tracking issue. If the implementation security issue is invalid, AppSec is notified.

This is a proactive effort to ensure all the implementation security issues are ready before the blog post is prepopulated.

Related to https://gitlab.com/gitlab-com/gl-infra/delivery/-/issues/19937

Testing

Three fake security implementation issues were created

  1. https://gitlab.com/gitlab-org/security/gitlab/-/issues/1025 -> With no CVES issue associated
  2. https://gitlab.com/gitlab-org/security/gitlab/-/issues/1024 -> With a CVES issue with invalid YAML (the YAML was manually modified to make it invalid)
  3. https://gitlab.com/gitlab-org/security/gitlab/-/issues/1026 -> With a valid CVES issue

Output:

With no CVES issue With invalid YAML
Screenshot_2024-01-03_at_16.47.52 Screenshot_2024-01-03_at_16.48.05
https://gitlab.com/gitlab-org/security/gitlab/-/issues/1025#note_1713245740 https://gitlab.com/gitlab-org/security/gitlab/-/issues/1024#note_1713247328
Script used for testing

To prevent spamming the code was modified as follows:

--- a/lib/release_tools/security/implementation_issue_processor.rb
+++ b/lib/release_tools/security/implementation_issue_processor.rb
@@ -29,7 +29,7 @@ module ReleaseTools
       end

       def execute
-        return unless Feature.enabled?(:implementation_issue_validator)
+      #  return unless Feature.enabled?(:implementation_issue_validator)

         validator.validate

@@ -67,10 +67,14 @@ module ReleaseTools
           .join(', ')
       end

+      User = Struct.new(:username)
+
       def appsec_release_managers
         ReleaseTools::ReleaseManagers::Schedule
           .new
           .active_appsec_release_managers
+
+        [User.new(username: 'mayra-cabrera')]
       end
     end
   end
(END)

Then the security issues were processed locally:

  • With no CVES issue associated:
[1] pry(main)> client = ReleaseTools::GitlabClient
=> ReleaseTools::GitlabClient
[2] pry(main)> raw_issue = client.issue(15642544, 1025)
=> #<Gitlab::ObjectifiedHash:217540 {hash: {"iid"=>1025,
[3] pry(main)> implementation_issue = ReleaseTools::Security::ImplementationIssue.new(raw_issue, [])
=> #<ReleaseTools::Security::ImplementationIssue:0x000000010be400f0
 @iid=1025,
 ... >
[4] pry(main)> ReleaseTools::Security::ImplementationIssueProcessor.new(implementation_issue).execute
=> #<Gitlab::ObjectifiedHash:217560 {hash: {"id"=>1713245740, "type"=>nil, ...}
[5] pry(main)> 2024-01-03 16:36:13.474945 D ReleaseTools::GitlabClient -- [HTTParty] [2024-01-03 16:36:13 -0600] 201 "POST https://gitlab.com/api/v4/projects/15642544/issues/1025/notes" 1349
  • With CVES with invalid YAML:
[6] pry(main)> raw_issue = client.issue(15642544, 1024)
=> #<Gitlab::ObjectifiedHash:217580 {hash: {"id"=>140358417, "iid"=>1024,
[7] pry(main)> implementation_issue = ReleaseTools::Security::ImplementationIssue.new(raw_issue, [])
=> #<ReleaseTools::Security::ImplementationIssue:0x000000010c2cde38
 @iid=1024,
... >
[8] pry(main)> ReleaseTools::Security::ImplementationIssueProcessor.new(implementation_issue).execute
2024-01-03 16:39:40.380087 E ReleaseTools::Security::CvesIssue -- CVE issue contains invalid YAML -- {:issue=>"https://gitlab.com/gitlab-org/cves/-/issues/944", :yaml_str=>"reporter:\n  name: TODO\n  email; TODO\nvulnerability:\n  description: TODO\n  cwe: TODO\n  product:\n    gitlab_path: TODO\n    vendor: TODO\n    name: TODO\n    affected_versions:\n      - TODO\n      - TODO\n    fixed_versions:\n      - TODO\n      - TODO\n  impact: TODO\n  solution: TODO\n  credit: TODO\n  references:\n    - TODO", :error=>"#<Psych::SyntaxError: (<unknown>): could not find expected ':' while scanning a simple key at line 3 column 3>"}
2024-01-03 16:39:41.403922 D ReleaseTools::GitlabClient -- [HTTParty] [2024-01-03 16:39:41 -0600] 200 "GET https://gitlab.com/api/v4/groups/4654006/members" -
=> #<Gitlab::ObjectifiedHash:217600 {hash: {"id"=>1713247328, "type"=>nil, ...}}
[9] pry(main)> 2024-01-03 16:39:42.100586 D ReleaseTools::GitlabClient -- [HTTParty] [2024-01-03 16:39:42 -0600] 201 "POST https://gitlab.com/api/v4/projects/15642544/issues/1024/notes" 1323
  • With a valid CVE issue
[9] pry(main)> raw_issue = client.issue(15642544, 1026)
[10] pry(main)> implementation_issue = ReleaseTools::Security::ImplementationIssue.new(raw_issue, [])
=> #<ReleaseTools::Security::ImplementationIssue:0x000000010c4648f0
 @iid=1026,
[11] pry(main)> ReleaseTools::Security::ImplementationIssueProcessor.new(implementation_issue).execute
=> true
[12] pry(main)> 2024-01-03 16:59:04.330890 I ReleaseTools::Security::ImplementationIssueProcessor -- Valid security implementation issue -- {:issue=>"https://gitlab.com/gitlab-org/security/gitlab/-/issues/1026"}
</details>

Author Check-list

  • [-] Has documentation been updated?
Edited by Mayra Cabrera

Merge request reports