Skip to content

Draft: Add jobs failed alert to security MR widget

What does this MR do?

This is a PoC/draft of the remaining work for #267504 (closed). It's incomplete feature-wise, and will likely become increasingly stale as time goes on due to design churn of &5710 (closed) and &3534. In fact, it's already stale based on the discussions in #267504 (closed) (e.g., see this thread: #267504 (comment 572527697)).

Alert implementation commit

This adds the alert, as originally designed in #12896 (closed), to the security MR widget.

The design is being rethought at the moment, perhaps in large part due to &5710 (closed), which is an effort to redesign all merge request widgets.

This alert implementation is almost certainly not going to be part of the eventual design (as far as I can tell), but the general structure of how to implement it is here (minus tests).

Note that several unrelated GraphQL queries were modified (id fields were added to various entities) due to a manifestation of #326101 (closed).

"Show when target branch has security reports, but source branch doesn't" implementation commit

Specifically, this makes the security MR widget show when the source branch is based on an old commit of the target branch, such that it isn't configured to run any security scans. Meanwhile, the target branch has advanced and is configured to run security scans. This gives an opportunity to prompt the user to rebase the source branch, in order to run security scans.

It should be noted that this is a rare edge case, and I'm not sure how worthwhile it is to actually handle it. A project is only likely to enable security scanning once, and once it has, any new branches created after it are unaffected. Only a few existing MRs, based on an old commit of the target branch, would show this, until they're rebased. To put this another way, the window when this is likely to happen for a given project is brief, and happens probably only once in a given project's lifetime.

This minimal implementation exposes the security_reports_up_to_date? field to the frontend, as per the second example in #275818 (closed).

Unfortunately, more work need to be done, as the MR widget renders in this case with a successful status, and message "Security scanning detected no vulnerabilities" which is not correct - security scanning did not even run!

Exactly what should be rendered instead varies from design to design, so it's not clear what to actually implement right now. As with the previous commit, this may become clearer with &5710 (closed).

Screenshots (strongly suggested)

Job failure alert

As mentioned above, this was the original design, but is now stale (the intended design is TBD).

alert_in_mr_2

Widget shown when target branch has security reports, but source branch doesn't

As described above, the implementation doesn't show the right message/status for this case, but the design is TBD.

no_vulns

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Related to #267504 (closed)

Edited by Mark Florian

Merge request reports