Consider comparing SAST report with branch starting point instead of target head
Description
As originally stated by @connorshea:
Do we compare the merge request to a file based on the most recent master? This MR is 2000 commits behind so I assume that’s why this is happening.
Should we be running SAST on the version of the codebase at the most recent version of the branch (e.g.
feature-123
), or should it be run on a version of the codebase where the MR is merged into master. Right now SAST says that the MR introduces new vulnerabilities, but it’s only seeing vulnerabilities because the Gemfile hasn’t been updated with the 2000 commits to master that have been made since the MR was first opened. (Include problem, use cases, benefits, and/or goals)
The SAST analysis can't presume the result of the merge (it would require that we somehow DO the merge locally and then run the pipeline on it).
Currently the comparison is done between feature branch's head and target branch's head. If the target branch has evolved too much since the feature branch was created, such case can happen and you'll then have some "new vulns" reported because they've actually been fixed in master since you've created that feature branch. Chances are that when you'll merge that branch, these "new vulns" reported won't actually exist.
So with current implementation I only see 2 ways to have an accurate result:
- rebasing the feature branch on up-to-date target branch
- merging back the target branch into the feature branch
This will provide an up to date state of your feature branch, making the comparison with the target branch's SAST analysis more realistic.
Proposal
@markpundsack suggested:
Can’t we compare to the SAST resolves from the beginning of the MR? Meaning, take the SHA of the beginning and see if there’s a pipeline for it, so you’re taking the SAST results from the point-in-time of
master
, not the head of master?
We need to double check this behavior will suits well with the specificities of the different security testing features. E.g. the vulns databases are evolving over time
Links / references
slack discussion: https://gitlab.slack.com/archives/C8S0HHM44/p1520009937000049