[BE] - Add project toggle for continuous vulnerability scans

Proposal

In order to implement [FE] - Add a setting to toggle CVS feature in the Security configuration page we need to do the following:

• Add a project attribute to toggle cvs on an doff
• Add a graphQL mutation to change this attribute
• Expose the enabled/disabled status of this attribute to frontend via a presenter

Implementation plan

• add new attribute to project security settings Add setting for enabling Continuous Vulnerabili... (!131305 - merged)
  • add continuous_vulnerability_scans_enabled attribute to ProjectSecuritySetting
    • add #toggle to flip the value of this attribute
  • add migration to add continuous_vulnerability_scans_enabled column
    • bool type, defaults to false
• update presenter to show value of setting above Add cvs toggle to security settings configurati... (!131315 - merged)
  • add #continuous_vulnerability_scans_enabled method calling the model above
• create graphql mutation to allow toggle of this feature Add a graphql mutation for toggling cvs (!131450 - merged)

  • create mutation under security_configuration mutations

      mutation ToggleContinuousVulnerabilityScans($input: ToggleContinuousVulnerabilityScans!) {
        toggleContinuousVulnerabilityScans(input: $input) {
            continuousVulnerabilityScanningEnabled
            errors
        }
      }

    • with input project_path: and enable: [true|false]

  • add set_continuous_vulnerability_scans to https://gitlab.com/gitlab-org/gitlab/-/tree/e4a855925c08d3eaedf368db80769f4d8d74244d/ee/app/graphql/mutations/security/ci_configuration

    • authorize with toggle_continuous_vulnerability_scans (see authorization below)
    • #resolve
      • calls ProjectSetting#toggle_continuous_vulnerability_scans
      • returns project, user, and value of ProjectSetting#continuous_vulnerability_scans_enabled

  • add toggle_continuous_vulnerability_scans authorization in https://gitlab.com/gitlab-org/gitlab/-/blob/e4a855925c08d3eaedf368db80769f4d8d74244d/ee/app/policies/ee/project_policy.rb

    • condition rule { Feature.enabled?(:dependency_scanning_on_advisory_ingestion) & can?(:develeoper_access) }.enable :enable_continuous_vulnerability_scans

  • mount mutation in https://gitlab.com/gitlab-org/gitlab/-/blob/e4a855925c08d3eaedf368db80769f4d8d74244d/ee/app/graphql/ee/types/mutation_type.rb

marked this issue as related to #423903 (closed)

changed milestone to %16.4

added Category:Software Composition Analysis Deliverable Enterprise Edition GitLab Ultimate SCA:Dependency Scanning backend devopssecure featureaddition groupcomposition analysis sectionsec typefeature workflowrefinement labels

cc @ifrenkel @gonzoyumo I broke up the original issue into FE and a BE issue. This is the BE portion.

@fernando-c is it worth adding a graphql call to check the toggle rather than the presenter? That way we use the same point of access and abstractions for this feature?

@ifrenkel

is it worth adding a graphql call to check the toggle rather than the presenter?

The FE implementation is a re-usable implementation for all the security configs. It would be beneficial to add the state on initial page load via the existing presenter

Otherwise we run into a race condition where the UI renders first without knowing the state, and then a follow up network request happens and then it "pops" into a state.

To be consistent it would be best to add the logic in the presenter. I believe the boolean would be derived from the same ruby logic anyway defined in the graphQL resolver for the field. So I imagined it would be more work for FE and BE to do a graphQL field.

Something like https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/presenters/projects/security/configuration_presenter.rb#L25

      def to_h
        {
          ....
          cvs_enabled: cvs_enabled?
        }
      end


      def cvs_enabled?
          ...logic here
      end

The logic in cvs_enabled? would have been identical for a graphQL resolver, but complicates the FE states since the current implementation assumes the initial toggle state is known at page load.

Makes sense Thanks for explaining this Fernando, I wasn't sure about how this worked.

added to epic &9534 (closed)

mentioned in issue #423903 (closed)

This issue is scheduled for completion in this milestone but doesn't have an assignee. Changing health status to 'needs attention'.

Issue participants are welcome to override this by setting the health status to another value.

changed health status to needs attention

@hacks4oats As discussed on Slack, we'll have to update the AdvisoryScanner you're implementing in Scan projects for newly reported advisories (!129640 - merged) to only create advisories in projects where the feature is enabled. We might not be able to optimize scans for this, and we might have to fetch SBOM occurrences from all projects, including the ones where the feature is disabled. It depends on how easy it is to filter out projects where the feature is disabled. Right now I don't know how that project preference is stored in the DB. I should have a look. cc @ifrenkel Do you know?

@fcatteau we have project security setting https://gitlab.com/gitlab-org/gitlab/-/blob/master/ee/app/models/project_security_setting.rb - would that be usable to optimize the sbom occurrence fetch?

@ifrenkel Thanks! And yes, we could add project_security_settings to the SBOM components finder, JOIN on project_id, and only return SBOM components of projects where the CVS feature is enabled. That should be efficient. Also, we wouldn't need to change the AdvisoryScanner service, which would be great. cc @adamcohen @hacks4oats

EDIT: Also, we need a dedicated issue for this. That's on my TODO list for tomorrow, unless someone wants to create it today (my time).

only return SBOM components of projects where the CVS feature is enabled. we need a dedicated issue for this

Do we need a separate issue for this? @ifrenkel, it looks like you've added this change to the implementation plan of this issue:

update Sbom::PossiblyAffectedOccurrencesFinder to filter out projects which have continuous_vulnerability_scans_enabled set to true

Great! No need to create an issue if it's already part of this one. It's even better.

Do we need a separate issue for this? @ifrenkel, it looks like you've added this change to the implementation plan of this issue:

@adamcohen yes, you're right. It'd be ideal to add another issue, I added that todo item as a placeholder b/c that piece hadn't merged at the time of my refining and only merged when I was finalizing the implementation plan. I'll update this.

assigned to @ifrenkel

changed the description

@adamcohen could you review the mutation point in implementation plan above

Regarding the authorize section do you know if maintainer_access makes sense or should it be looser?

could you review the mutation point in implementation plan above

@ifrenkel the mutation looks good to me

Regarding the authorize section do you know if maintainer_access makes sense or should it be looser?

Maintainer access makes sense to me, unless @gonzoyumo thinks otherwise.

add enable_continous_vulnerability_scans authorization in https://gitlab.com/gitlab-org/gitlab/-/blob/e4a855925c08d3eaedf368db80769f4d8d74244d/ee/app/policies/ee/project_policy.rb

@gonzoyumo do you know if we need to add authorization at the GroupPolicy level as well?

Some other thoughts:

add #continous_vulnerability_scans_enabled method calling the model above

There's a typo in continous - I've changed this to continuous

update Sbom::PossiblyAffectedOccurrencesFinder to filter out projects which have continuous_vulnerability_scans_enabled set to true

Don't you mean "filter out projects which have continuous_vulnerability_scans_enabled set to false"?

Don't you mean "filter out projects which have continuous_vulnerability_scans_enabled set to false"?

That's correct. Thanks.

changed the description

changed the description

changed the description

set weight to 5

added database label

added workflowready for development label and removed workflowrefinement label

mentioned in issue #424629 (closed)

marked this issue as related to #424629 (closed)

changed the description

changed title from [BE] - Add graphQL Query Mutation to toggle CVS and add status to Configuration Presenter to Add project toggle for continuous vulnerability scans

changed the description

mentioned in merge request !131305 (merged)

changed the description

added workflowin dev label and removed workflowready for development label

mentioned in merge request !131315 (merged)

mentioned in issue gitlab-org/database-team/team-tasks#374 (closed)

mentioned in issue gitlab-org/quality/triage-reports#14079 (closed)

changed title from Add project toggle for continuous vulnerability scans to [BE] - Add project toggle for continuous vulnerability scans

added quad-planningcomplete-no-action label

mentioned in merge request !131450 (merged)

added workflowin review label and removed workflowin dev label

changed the description

changed the description

As suggested by @gonzoyumo, we should only show this project setting if the dependency_scanning_on_advisory_ingestion flag is enabled for the given project.

@ifrenkel I don't think this is currently captured by the issue. Is it?

@fcatteau thanks for noting this. Description updated.

mentioned in merge request !129640 (merged)

changed the description

mentioned in merge request !131716 (closed)

mentioned in merge request !131727 (closed)

mentioned in merge request !131768 (merged)

added workflowverification label and removed workflowin review label

closed

added workflowcomplete label and removed workflowverification label

added closedcomplete label

mentioned in merge request !147537 (merged)