## Summary The security MR widget and the pipeline security tab open a modal when a user clicks on a finding. The modal is responsible for displaying details of the given security finding. Currently, we load all the data upfront. This means when we load a list of findings, every one of its entries contains the entire dataset needed to render the modal. This approach has the benefit that the modal's content is available instantly when opened. Still, it also fetches more data than needed since it is unlikely that a user inspects the details of every finding within the list. When migrating to GraphQL, moving away from this implementation is more straightforward since we can now select the exact data we request from the backend. Instead of fetching everything at the first request, we can split it up: 1. Fetch the minimum amount of data that is needed to render the reports 2. When a user clicks on a finding, we request the complete dataset for the given finding and then use it to render the modal ## Details ### Diagram ## Phases We will roll out the standalone modal in multiple phases ### Modal component breakdown <details> <summary>Diagram and additional infos</summary>  ## Current implementation --- <details> <summary> ### 1) Vulnerability Details </summary> * Migration is captured here: https://gitlab.com/groups/gitlab-org/-/epics/8910 * TODO: * Refine Generic report: https://gitlab.com/gitlab-org/gitlab/-/issues/382726 </details> --- <details> <summary> ### 2) Solution Card </summary> ## Summary * Display's `vulnerability.solutionHtml` or falls back to `vulnerability.solution` or `vulnerability.remediations\[0\].summary` * If there is not an existing MR for the given finding and a patch can be downloaded then show an extra message with instructions to create an MR or download the patch ### Data dependencies * `vulnerability.solutionHtml` <mark>missing but currently not used for vuex modal version</mark> or `vulnerability.solution` - that means for the MVP this should not be needed * `vulnerability.remediations\[0\]` <mark>missing</mark> * `vulnerability.hasMergeRequest` <mark>missing</mark> ## Endpoints ### Pipeline `GET /api/v4/projects/34/vulnerability_findings` ### MR Widget **Same as Pipeline** ## Notes * There are two versions of the card: * `ee/app/assets/javascripts/vue_shared/security_reports/components/solution_card.vue` * `ee/app/assets/javascripts/vue_shared/security_reports/components/solution_card_vuex.vue` :point_left: used on current modal ## Screenshots <details> <summary>Without MR instructions</summary>  </details> <details> <summary>With MR instructions</summary>  </details> </details> --- <details> <summary> ### 3) Issue Notes </summary> ## Summary * Displays information about issues that were created for the given finding. This includes the creation date, details about the author and a link to the issue. ## Props ### `feedback` ``` { author: { id: '1', name: 'Dave', username: 'davepies', path: 'davpies.me', is_gitlab_employee: true, }, created_at: '2022-11-17T09:33:07.971Z', issue_url: 'http://foo.bar', issue_iid: '111', } ``` ### `project` ``` { url: '/foo/bar', value: 'Foo / Bar', } ``` ### Endpoints * Pipeline: `GET /api/v4/projects/34/vulnerability_findings?pipeline_id=1` -> `vulnerability.issue_feedback` * MR Widget: `GET /http://gdk.test:3000/security/security-reports/-/vulnerability_feedback?category=sast` (fetches for each category) and `GET /security/security-reports/-/merge_requests/5/sast_reports` -> [Vuex adds the data](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/assets/javascripts/vue_shared/security_reports/store/utils.js#L30) to what we get from `vulnerability_findings` from above ## Screenshots <details> <summary>Default</summary>  </details> </details> --- <details> <summary> ### 4) Merge Request Note </summary> ## Summary * Displays information about MRs that were created for the given finding. This includes the creation date, details about the author, and a link to the MR. * Feedback is in `vulnerability.merge_request_feedback` ## Props ### `feedback` ``` { author: { id: '1', name: 'Dave', username: 'davepies', path: 'davpies.me', is_gitlab_employee: true, }, created_at: '2022-11-17T09:33:07.971Z', merge_request_path: '/merge/foo', merge_request_iid: 1, } ``` ### `project` ``` { url: '/foo/bar', value: 'Foo / Bar', } ``` ## Endpoints * Pipeline: `GET /api/v4/projects/34/vulnerability_findings?pipeline_id=1` * MR Widget: `GET /security/security-reports/-/merge_requests/5/sast_reports` (for reach report type) ## Screenshots <details> <summary>Default</summary>  </details> </details> --- <details> <summary> ### 5) Dismissal Note </summary> ## Summary Displays information about a finding's dismissal (if present). This includes comments and information about the author. ## Props ### `feedback` ``` { author: { id: 1, name: 'Author name', username: 'author username', path: 'author/path', is_git_gitlab_employee: true, }, // optional comment_details: { comment_author: { id: '1', name: 'Dave', username: 'davepies', path: 'davpies.me', is_gitlab_employee: true, }, comment_timestamp: '2022-11-17T09:33:07.971Z', comment: 'foo is bar!', }, created_at: '2022-11-17T09:33:07.971Z', project: { url: 'http://foo.bar', value: 'project value', }, // optional pipeline: { id: 1, path: 'pipeline/path', }, }; ``` ### `project` ``` { url: 'http://project/url', value: 'project value' } ``` ## Endpoints * Pipeline: `GET /api/v4/projects/34/vulnerability_findings?pipeline_id=1&scope=dismissed` * MR Widget: e.g: `GET /security/security-reports/-/vulnerability_feedback?category=sast` and `/security/security-reports/-/merge_requests/5/sast_reports` (requests for each report type and data gets combined, like mentioned above) ## Screenshots <details> <summary>Without comment</summary>  </details> <details> <summary>With comment</summary>  </details> </details> --- <details> <summary> ### 6) Dismissal Comment Box </summary> ## Summary * Renders textbox to enter a dismissal comment * Gets displayed when there is a dismissal and no comment yet, or when the existing comment is being edited * Lets a user _add_, _edit_ and _delete_ a comment * Displays any error message that might have been triggered when trying to post the comment to the BE ## Props ### `dismissal-comment` The placeholder text: ``` 'this is a comment' ``` ### `is-active` Set to true if the user is currently commenting ``` true | false ``` ### Error message Any messages from errors that happened when trying to fetch or post data. ``` 'message describing what went wrong' ``` ## Endpoints ### Read * Pipeline: `GET /api/v4/projects/34/vulnerability_findings?pipeline_id=1&scope=dismissed` * MR Widget: `GET /security/security-reports/-/vulnerability_feedback?category=sast` and `/security/security-reports/-/merge_requests/5/sast_reports` (requests for each report type and data gets combined, like mentioned above) ### Write (indirectly) #### Pipeline: `POST /security/security-reports-fresh/-/vulnerability_feedback` <details> <summary>payload</summary> `{ "vulnerability_feedback": { "category": "dependency_scanning", "comment": "test", "feedback_type": "dismissal", "pipeline_id": 1420, "project_fingerprint": "a5b166f93b07f2ead1c16552af374ebfda7fc7d0", "finding_uuid": "8b404791-53fc-5801-81c7-05d0d505a04e", "vulnerability_data": { "id": "client_14", "report_type": "dependency_scanning", "name": "Path Traversal in rubyzip", "severity": "critical", "confidence": "unknown", "scanner": { "external_id": "gemnasium", "name": "Gemnasium", "vendor": "GitLab" }, "identifiers": \[ { "external_type": "gemnasium", "external_id": "99f8ccec-097c-4147-9cd0-2a8cd3a354a4", "name": "Gemnasium-99f8ccec-097c-4147-9cd0-2a8cd3a354a4", "url": "``https://gitlab.com/gitlab-org/security-products/gemnasium-db/-/blob/master/gem/rubyzip/CVE-2018-1000544.yml``" }, { "external_type": "cve", "external_id": "CVE-2018-1000544", "name": "CVE-2018-1000544", "url": "``https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-1000544``" } \], "project_fingerprint": "a5b166f93b07f2ead1c16552af374ebfda7fc7d0", "uuid": "8b404791-53fc-5801-81c7-05d0d505a04e", "create_jira_issue_url": null, "false_positive": false, "create_vulnerability_feedback_issue_path": "/security/security-reports-fresh/-/vulnerability_feedback", "create_vulnerability_feedback_merge_request_path": "/security/security-reports-fresh/-/vulnerability_feedback", "create_vulnerability_feedback_dismissal_path": "/security/security-reports-fresh/-/vulnerability_feedback", "project": { "id": 34, "name": "Security Reports - Fresh", "full_path": "/security/security-reports-fresh", "full_name": "Security / Security Reports - Fresh" }, "dismissal_feedback": null, "issue_feedback": { "id": 169, "created_at": "2022-11-24T13:39:31.705Z", "project_id": 34, "author": { "id": 1, "username": "root", "name": "Administrator", "state": "active", "avatar_url": "``https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon``", "web_url": "``http://gdk.test:3000/root``", "show_status": false, "path": "/root" }, "finding_uuid": "8b404791-53fc-5801-81c7-05d0d505a04e", "issue_iid": 1, "issue_url": "``http://gdk.test:3000/security/security-reports-fresh/-/issues/1``", "category": "dependency_scanning", "feedback_type": "issue", "branch": null, "project_fingerprint": "a5b166f93b07f2ead1c16552af374ebfda7fc7d0", "dismissal_reason": null }, "merge_request_feedback": null, "description": "The gem rubyzip contains a Directory Traversal vulnerability in Zip::File component that can result in write arbitrary files to the filesystem. This attack appear to be exploitable via If a site allows uploading of .zip files, an attacker can upload a malicious file that contains symlinks or files with absolute pathnames .. to write arbitrary files to the filesystem.", "description_html": "` `The gem rubyzip contains a Directory Traversal vulnerability in Zip::File component that can result in write arbitrary files to the filesystem. This attack appear to be exploitable via If a site allows uploading of .zip files, an attacker can upload a malicious file that contains symlinks or files with absolute pathnames .. to write arbitrary files to the filesystem.", "links": \[{ "url": "``https://nvd.nist.gov/vuln/detail/CVE-2018-1000544``" }\], "location": { "file": "dependency-scanning-files/Gemfile.lock", "dependency": { "package": { "name": "rubyzip" }, "version": "1.2.1" } }, "remediations": null, "solution": "Upgrade to version 1.2.2 or above.", "evidence": null, "request": null, "response": null, "evidence_source": null, "supporting_messages": null, "assets": \[\], "details": { "vulnerablePackage": { "type": "text", "name": "Vulnerable Package", "value": "rubyzip:1.2.1" } }, "state": "detected", "scan": { "type": "dependency_scanning", "status": "success", "start_time": "2022-11-22T08:52:43", "end_time": "2022-11-22T08:52:46" }, "blob_path": "/security/security-reports-fresh/-/blob/225fa090f92d057854a68e9f8e071f86523be984/dependency-scanning-files/Gemfile.lock", "hasIssue": true, "hasMergeRequest": false, "isDismissed": false, "category": "dependency_scanning" } } }` </details> #### MR Widget **Same as Pipeline** ### Delete (indirectly) #### Pipeline `PATCH /security-reports-fresh/-/vulnerability_feedback/164` #### MR Widget **Same as Pipeline** ## Screenshots <details> <summary>Without existing comment</summary>  </details> <details> <summary>With existing comment</summary>  </details> </details> --- <details> <summary> ### 7) Error Alert </summary> ## Summary Renders error message that gets displayed when dismissal or comment deletion results in an error. ## Endpoints * Dismissal and comment deletion ## Screenshots <details> <summary>Default</summary>  </details> </details> --- <details> <summary> ### 8) Dismiss / Undo Dismiss </summary> ## Summary ## Endpoints ### Pipeline Undo: `DELETE 0/security/security-reports-fresh/-/vulnerability_feedback/164` ### MR Widget **Same as pipeline** ## Screenshots <details> <summary>Dismiss</summary>  </details> <details> <summary>Undo</summary>  </details> </details> --- <details> <summary> ### 9) Create Issue (GitLab / Jira) </summary> ## Summary If either a path\* for creating a gitlab or jira issue is given, then it renders a "Create Issue" or "Create Jira Issue" button. If both are present then Jira issues take precedence. If there is an existing issue the button also won't render. `\*` `vulnerability.create_vulnerability_feedback_issue_path` / `vulnerability.create_jira_issue_url` ## Endpoints ### Pipeline `POST /security/security-reports-fresh/-/vulnerability_feedback` ### MR Widget **Same as Pipeline** ## Notes Both <mark>data fields currently missing</mark>. When there are more than one action buttons available, they get moved into a split-button (see screenshots below). ## Screenshots <details> <summary>Default</summary> Goes here </details> <details> <summary>Simple</summary>  </details> <details> <summary>Split Button (multiple actions)</summary>  </details> </details> --- <details> <summary> ### 10) Download Patch </summary> ## Summary Uses the remediation's diff's data \* to dynamically create a download link (using: `\~/lib/utils/downloader`) `\*` `vulnerability.remediations\[0\].diff` ## Endpoints ### Pipeline `GET /api/v4/projects/34/vulnerability_findings?pipeline_id=1` ### MR Widget **Same as Pipeline** <details> <summary>Simple</summary>  </details> <details> <summary>Split Button (multile actionns)</summary>  </details> </details> --- <details> <summary> ### 11) Resolve with MR </summary> ## Summary If the given vulnerability is not resolved, doesn't have an existing MR associated and has remedidation data, then the button to create an MR gets shown. ## Endpoints ### Pipeline `POST /security/security-reports/-/vulnerability_feedback` ### MR Widget **Same as Pipeline** <details> <summary>Simple</summary>  </details> <details> <summary>Split Button (multiple actions)</summary>  </details> </details> </details> #### Phase 1 1. New modal component, launched with UUID, Project, and Pipeline params. 2. Modal uses new GraphQL query for findings data (details, 3. Modal uses existing RESTful endpoints for things like dismiss, create issue, create MR. 4. Modal continues to receive and passthrough vulnerability_data from the pipeline and MR pages, passing through to create issue/MR endpoints. #### Phase 2 1. Incrementally replace RESTful endpoints with GraphQL mutations. 2. The mutations will internally retrieve vulnerability_data, allowing us to remove the data passthrough noted above. #### Phase 3 1. Optimizations on the Pipeline and MR Security Widget 1. Reduce data where possible. 2. Replace finding listing with GraphQL where possible. ### GraphQL Work Captured From [Spike](#382913) 1. Issues 1. Add `PipelineSecurityReportFinding.issueLinks` field ([completed](gitlab-org/gitlab#384867)) 2. Add `securityFindingCreateIssue` ([completed](gitlab-org/gitlab#361948)) 2. Merge Requests 1. Add `PipelineSecurityReportFinding.mergeRequest` field ([completed](gitlab-org/gitlab#384509)) 2. Add `PipelineSecurityReportFinding.remediations` field ([completed](gitlab-org/gitlab#384508)) 3. State Change 1. Add `PipelineSecurityFinding.dismissal` field ([in review](gitlab-org/gitlab#387865)) 1. `dismissalReason` 2. `dismissedAt` 3. `dismissedBy` 4. `stateComment` 2. Add `securityFindingRevertToDetected` mutation ([in review](gitlab-org/gitlab#387876)) 1. Similar to `vulnerabilityRevertToDetected` 3. Add ability to change dismissal comment via `securityFindingDismiss` ([refinement](gitlab-org/gitlab#387884)) 1. We would need to change code in the [`Vulnerabilities::FindOrCreateFromSecurityFindingService`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/ee/app/services/vulnerabilities/find_or_create_from_security_finding_service.rb), specifically utilizing the `update_state_for` method. I think [Vulnerabilities::DismissService](https://gitlab.com/gitlab-org/gitlab/-/blob/maaster/ee/app/services/vulnerabilities/dismiss_service.rb#L20) already handles this properly, but it needs to be tested. 4. Merge Request Creation 1. Add `securityFindingCreateMergeRequest` mutation ([ready for dev](gitlab-org/gitlab#387881)) 2. Add `securityFindingDownloadPatch` mutation (may not be needed) 5. External Issues 1. Add `securityFindingCreateExternalIssue` 1. Similar to `vulnerabilityExternalIssueLinkCreate` 6. Solution 1. Add `PipelineSecurityReportFinding.solutionHtml` field ([ready for dev](gitlab-org/gitlab#384510)) ## High-level roadmap ### 15.8 | | Summary | | ------ | ------ | | ~backend | Add the following fields to `securityReportFinding` GraphQL field: <ul><li>issues</li><li> merge requests</li><li>remediations</li></ul> This will enable a chunk of UI work for %"15.9".</li></ul> | | ~frontend | <ul><li>Add the generic-report section (GraphQL-specific version) to the modal and render at least one report type. Having this in place will lay the foundation to add the rest of the report times in %"15.9"</li><li>Add the UI for rendering the error banner, which will be utilised by a number of actions that will be added in future milestones (eg.: dismissal).<li>Add a custom footer to the modal, which will allow us to add buttons for the footer actions in future iterations</li></ul>| ### 15.9 | | Summary | | ------ | ------ | | ~backend | Focus on dismissal fields and mutations: <ul><li>Add `PipelineSecurityFinding.dismissal` field</li><li>Add `securityFindingRevertToDetected` mutation</li><li>Add ability to change dismissal comment via `securityFindingDismiss`</li><li>Once those are completed, work on the Merge Request creation mutation</li> | | ~frontend | Finish with: <ul><li>generic report section</li><li>solution card</li><li>feedback notes</li></ul>| ### 15.10 | FE | BE | Note | | ------ | ------ | ------ | | [Migrate generic report item to GraphQL: "Table"](https://gitlab.com/gitlab-org/gitlab/-/issues/386159) | https://gitlab.com/gitlab-org/gitlab/-/issues/389916 | | | [Render evidence fields within the new GraphQL vulnerability details component](https://gitlab.com/gitlab-org/gitlab/-/issues/382203 ) | ✅ | not finished in "%15.10" | | [Add dismissal notes](https://gitlab.com/gitlab-org/gitlab/-/issues/387653) | ✅ | wip but not finished in "%"15.10" | ### 15.11 #### ~frontend * https://gitlab.com/gitlab-org/gitlab/-/issues/403227+s * https://gitlab.com/gitlab-org/gitlab/-/issues/387653+s * https://gitlab.com/gitlab-org/gitlab/-/issues/387657+s * https://gitlab.com/gitlab-org/gitlab/-/issues/382203+s * https://gitlab.com/gitlab-org/gitlab/-/issues/404756+s * https://gitlab.com/gitlab-org/gitlab/-/issues/393376+s #### ~backend * https://gitlab.com/gitlab-org/gitlab/-/issues/387881+s * https://gitlab.com/gitlab-org/gitlab/-/issues/404894+s * https://gitlab.com/gitlab-org/gitlab/-/issues/389923+s ### 16.0 #### ~frontend * https://gitlab.com/gitlab-org/gitlab/-/issues/393378+s * https://gitlab.com/gitlab-org/gitlab/-/issues/389925+s #### ~backend * https://gitlab.com/gitlab-org/gitlab/-/issues/397768+s * https://gitlab.com/gitlab-org/gitlab/-/issues/404895+s * https://gitlab.com/gitlab-org/gitlab/-/issues/404894+s ### 16.1 #### ~frontend * https://gitlab.com/gitlab-org/gitlab/-/issues/393378+s * https://gitlab.com/gitlab-org/gitlab/-/issues/389925+s * https://gitlab.com/gitlab-org/gitlab/-/issues/413516+s #### ~backend * https://gitlab.com/gitlab-org/gitlab/-/issues/404894+s * https://gitlab.com/gitlab-org/gitlab/-/issues/389923+s * https://gitlab.com/gitlab-org/gitlab/-/issues/397768+s