Full context: https://gitlab.com/groups/gitlab-org/-/epics/634 Product discovery (high-level choices): https://gitlab.com/gitlab-org/gitlab-ee/issues/8493 This iteration is about the technical engineering discovery to understand how we can implement first-class vulnerabilities in our current architecture. [Notes from conversation about this topic at Contribute](https://docs.google.com/document/d/1q_k8Y0XYbvpkU99OZFL9ofUE8wLE7D524q56ceXVtr0/edit?usp=sharing). NOTE: much of this issue has refocused on ~UX and Product discovery as many points needed to be revisited. See discussion below for details on initial JTBD, interactions, and direction. --- ### Development log #### Status - [x] [Jobs To Be Done defined](https://gitlab.com/gitlab-org/gitlab-ee/issues/10252#note_203776866) - [x] [Data model defined](https://gitlab.com/gitlab-org/gitlab-ee/issues/10252#note_204147258) - [x] Implementation steps finalized #### Decisions ##### Terminology ###### Finding Something with the potential to be vulnerable. This is a subgroup of the existing `Vulnerability Occurrences` *in a pre-merge state (within Merge Terminology)*. `Findings` are not persisted in the database. ###### Priority Finding Something with the potential to be vulnerable. This is a subgroup of the existing `Vulnerability Occurrences` *in a post-merge state (within default branches)*. `Priority Finding` is persisted in the database and maps to a record in `vulnerability_occurrences` DB table. A `Priority Finding` is an "occurrence" that has not been confirmed (and didn't spawn a `Vulnerability` entity yet) but it has become a part of the production-deployed codebase. It can be in either `confirmed` or `unconfirmed` state. A confirmed `Priority Finding` is the one that has a `Vulnerability` referencing it. ###### Vulnerability A *confirmed* entity that represents a vulnerability within the codebase. This is a result of the "promotion" of a `Priority Finding`. A `Vulnerability` is either in a state of `open` (some unresolved `Findings` exist) or `resolved` (no *more*\*\* unresolved `Findings` exist). \*\*A vulnerability MUST have occurrences. Even externally-sourced weaknesses such as HackerOne reports require triaging, so they will first appear as weakness and require promotion to a `Vulnerability`. ###### Triaging Triaging is the act of promoting a `Finding` to a `Vulnerability`. This behavior will be introduced from a functional perspective within #11205. Triaging is not something that is meant to occur through the MR widget and primarily through dashboard views, hence we say `Priority Finding promotion`, not just `Finding promotion`. This is an important distinction. The existing MR procedures of dismissal are preserved as feedback is directly associated with `vulnerability_occurrences`. When interacting with `Vulnerabilities`, a dismissal would result in creating cascading feedback for all associated `vulnerability_occurrences`. This way we preserve the existing DB relationships, maintain backward compatibility, and allow more granular interactions; i.e. for a `Vulnerability` w/ 2 `Findings` we dismiss 1 `Finding` (within `test` directory) but create an `Issue` to address 2nd `Finding` (within `src` directory). ##### Data Model - Naming consideration: `vulnerability_occurrences` should become `findings` to more accurately capture the _potential_ of a vulnerability, where "occurrences" implies a confirmation - `vulnerabilities` have many `vulnerability_occurrences` - `vulnerabilities` have many `issues` through `vulnerability_feedback` (through `vulnerability_occurrences`) - a `vulnerability` is "resolved" when all `findings` are "resolved" - a `vulnerability` only has 2 states: `open` and `closed`. All possible `closed` sub-states (i.e. "duplicate", "wont fix", "false positive") should be dismissals of the `finding` - `vulnerabilities.severity` and `vulnerabilities.confidence` are aggregates of the associated `findings`, however they can be overridden by the user. - By default, `vulnerability.severity` is the highest-severity of linked occurrences - By default, `vulnerability.confidence` is the lowest-confidence of linked occurrences - `vulnerability_feedback` remains associated with `vulnerability_occurrences`. To dismiss a vulnerability that has 2 findings, dismissal feedback is applied to all associated findings as a cascading transaction - `vulnerability_feedback.feedback_type` uniqueness constraint must be dropped in order to allow a `finding` to have multiple-associated issues - ~~we're not going to add `report_type` attribute to `Vulnerabilities` because the `Vulnerability` has a higher level of abstraction than report and may combine findings from different report types~~ for the [MVC](#13561), we're adding the report_type to the Vulnerabilities (details [here](https://gitlab.com/gitlab-org/gitlab/issues/10252#note_237092657)). ##### Interactions - `Vulnerability` is an epic-like object - `Vulnerability` is a project-scoped object _(not a group-scoped, like Epics)_ - Naming consideration: `findings` are all occurrences. "Priority findings" are present within the default branch. - a 1st Class Vuln (`vulnerability`) can be created _from_ a `finding` - a 1st Class Vuln (`vulnerability`) must be associated with `findings` (a vulnerability cannot be created in isolation as findings contain the specifics of the vulnerability; i.e. location) - Externally-sourced `findings` (i.e. HackerOne reports) will always be created as `findings` to allow triaging and potential promotion to vulnerabilities (`vulnerability_occurrences.scanner.name == "hackerone"`) - a `finding` must be promoted to a `vulnerability` to allow other `findings` to be associated with that vulnerability (bulk-associating findings to a vulnerability on creation will not be supported, but this could be an implementation detail) - a `finding` can be linked to an _existing_ `vulnerability` (and only one) - a `finding` can be disassociated with a `vulnerability` (unless it's the last-associated `finding`) - `vulnerabilities` should be considered "resolved" when all findings are "resolved". - This will not be an automatic action initially as pipeline-completion (when we detect that findings no longer exist) cannot accurately be tied to a production deployment (to which a vulnerability SLA should be associated). - Externally-sourced findings must be manually resolved and will never be auto-closed - a `Vulnerability` is [created confidential](https://gitlab.com/gitlab-org/gitlab/issues/10252#note_188924482) similarly to [confidential Issues](https://docs.gitlab.com/ee/user/project/issues/confidential_issues.html#confidential-issues) #### UX - a [board](https://app.mural.co/t/gitlab2474/m/gitlab2474/1568753675935/383c252c424d3c137c03fc9b84ac23b5da8e422a) with mockups, flows, critical assumptions, and open questions #### Backend MVC Implementation Plan 1. Rename existing `Vulnerabilities API` to `Vulnerability Findings API` 1. Create `vulnerabilities` table (see schema below) 1. Create `Vulnerabilities API` with the following capabilities - `Get a list of project's vulnerabilities` - `Create vulnerability from finding` (the old term is "occurrence") - ~~`Delete vulnerability` (and disassociate from occurrence)~~ [removed](https://gitlab.com/gitlab-org/gitlab/issues/10252#note_227628566) - Update vulnerability status: - `Dismiss vulnerability` (and dismiss all associated occurrences) - `Resolve vulnerability` (and resolve all associated occurrences) 4. [Optional, depending on capacity] `Associate finding to (existing) vulnerability` #### Frontend MVC Implementation Plan - Rename Security Dashboard `Vulnerability List` to `Finding List` - Add secondary tab for `Vulnerabilities` - `GET /api/v4/projects/:id/vulnerabilities` - `GET /api/v4/vulnerabilities/:id` - Add interaction to "promote" a `Finding` to a `Vulnerability` - `POST /api/v4/vulnerabilities { finding_id: <finding_id> }` - ~~Add interaction to delete a `vulnerability`~~ - ~~`DELETE /api/v4/projects/4/vulnerabilities/:primary_identifier_id-:location_fingerprint-:scanner_id`~~ removed because is [considered unnecessary](https://gitlab.com/gitlab-org/gitlab/issues/10252#note_227628566) - Add interaction to mark a `vulnerability` as resolved - `POST /api/v4/vulnerabilities/:id/resolve` - Add interaction to mark a `vulnerability` as dismissed - `POST /api/v4/vulnerabilities/:id/dismiss` - \[Optional, depending on capacity\] Add interaction to associate finding with existing `vulnerability` - `POST /api/v4/vulnerabilities/:id/attach { vulnerability_id: <vulnerability_id> }`: post-MVC, extracted into #34531 ##### Schema `vulnerabilities` ```ruby create_table "vulnerabilities" do |t| # epic-like (or issue-like) attributes t.bigint "milestone_id" t.bigint "epic_id" t.bigint "project_id", null: false t.bigint "author_id", null: false t.bigint "updated_by_id" t.bigint "last_edited_by_id" t.date "start_date" t.date "due_date" t.datetime "last_edited_at" t.datetime "created_at", null: false t.datetime "updated_at", null: false t.string "title", null: false t.string "title_html", null: false t.text "description" t.text "description_html" t.bigint "start_date_sourcing_milestone_id" t.bigint "due_date_sourcing_milestone_id" t.integer "state", limit: 2, default: 1, null: false t.bigint "closed_by_id" t.datetime "closed_at" t.integer "report_type", limit: 2, default: 0, null: false # vulnerability-like attributes t.integer "severity", limit: 2, null: false # auto-calculated as highest-severity finding, but overridable t.boolean "severity_overridden", default: false t.integer "confidence", limit: 2, null: false # auto-calculated as lowest-confidence finding, but overridable t.boolean "confidence_overridden", default: false end add_foreign_key :vulnerabilities, :projects add_foreign_key :vulnerabilities, :milestones add_foreign_key :vulnerabilities, :epics add_foreign_key :vulnerabilities, :users, :author_id add_foreign_key :vulnerabilities, :users, :updated_by_id add_foreign_key :vulnerabilities, :users, :last_edited_by_id add_foreign_key :vulnerabilities, :users, :closed_by_id ``` \* `iid` (project-scoped internal ID) was removed from the initial schema [by the maintainer recommendation](https://gitlab.com/gitlab-org/gitlab/merge_requests/16181/diffs#note_220594759) `vulnerability_occurrences` ```ruby add_reference :vulnerability_occurrences, :vulnerabilities, :bigint, index: true, foreign_key: true ```