Please test the latest template in the following scenarios:
Auto-DevOps
Scan execution policies
Additional workflows that could be impacted by a template update
Combining MR pipelines with other pipelines supported by other scanners
Note: Please be aware that there is an ongoing effort to determine how to support MR pipeline support in Sec templates. Please leverage DRIs and ongoing testing from that issue to support item 4 above.
I think we need to figure out if we are going to have to mix-and-match by default (for example, SD runs in MR pipelines by default, and everything else stays in branch pipelines) or if we are going to be able to keep it consistent across all the scanners.
I believe the drama in 16.0 was that:
we couldn’t mix and match, due to limitations in policies/vuln management. (Each system would consult only one pipeline, which might be a branch pipeline or MR pipeline—they wouldn't read results from both.)
but we also couldn’t move everything to MR pipelines, because jobs like DAST and CS depend on other jobs, like the jobs to deploy the webapp or build the container image in the first place. Those would presumably be in branch pipelines by default.
Now the VM/policies issue should've been resolved back in an early 16.x release (though I have a suspicion about it not working for SAST + SAST-IaC) so maybe we don’t have to make all the jobs match.
Note: this is a summary and all of the historical details should be in the linked issue.
That's the only one I discovered at the moment. To elaborate a bit more on this workflow (which I'm still working with the customer on), they don't want to workaround the issue by using the default template because they specifically want the behaviour of scanning every commit in the source branch when they open an MR.
So it feels like their options at the moment are:
Continue as they have been, with Pipelines must succeed disabled and wait for a fix from the development team.
Look into pushing a .gitlab-ci.yml file into every branch of every repository so that we don't have to rely on the implicit creation of the pipeline.
Investigate Pipeline Execution Policies as an alternative.
@ahmed.hemdan I've been working with @abellucci to update the description/expectations of this issue based on our 1:1 discussion today. Can you please review this and let us know if you have any additional questions?
@amarpatel As discussed in our 1:1, being a little more specific with the ask here would be quite helpful.
Additional workflows that could be impacted by a template update
Which additional workflows do you have in mind?
Combining MR pipelines with other pipelines supported by other scanners
Note: Please be aware that there is an ongoing effort to determine how to support MR pipeline support in Sec templates. Please leverage DRIs and ongoing testing from that issue to support item 4 above.
There are several types of pipelines, e.g. branch pipelines, tag pipelines, etc. Which type of others pipelines are targeted by this spike? What other scanners do you think should be combined with MR pipelines from the latest template?
I see this was not updated, but the milestone was changed to %17.9. I realize this work is necessary for refining requirements ahead of %18.0 but it would be nice to have this target date pushed back (perhaps close to the end of the milestone).
ADO doesn't support the MR pipeline so it refers to stable SD template. Needless to say, changing the SD template to latest wouldn't make any difference.
MR Security widget considers only the latest commit's branch pipeline results instead of all the commits in the source branch. This leads to showing secret findings in the widget only if the latest commit has any secret committed in it.
This seems to provide the expected result as the SD job ran successfully in this "Merge Request" pipeline and it has scanned the 11 commits from that MR:
This means that by updating the stable template with the content of the latest template, the SD feature in the ADO context will work this way:
if ADO doesn't have MR pipeline enabled, SD will work in "branch mode" and scan the head of the branch
if ADO has MR pipeline enabled, SD will work in MR mode and scan all the commits included in the MR.
Since most of the time MR pipelines won't be enabled in ADO context (it's an explicit change from the user), the "branch" behavior will be used and not all the commits of the MR will be scanned. I assume this is what you meant by We might not provide MR pipeline support for Secret Detection in the projects using Auto DevOps.
I wonder if there is another way for SD to trigger the MR behavior without such a reliance on MR pipeline but I guess you probably already looked at it 😄
Since most of the time MR pipelines won't be enabled in ADO context (it's an explicit change from the user), the "branch" behavior will be used and not all the commits of the MR will be scanned.
Ah, that's why there's a workflow rule in your linked project indicating the pipeline should run for the Merge Request Events. So, I suppose the customers using ADO should add that workflow rule for the MR pipeline support.
I assume this is what you meant by We might not provide MR pipeline support for Secret Detection in the projects using Auto DevOps.
Yes. Basically, GitLab doesn't support MR pipeline out of the box but there's a workaround.
I wonder if there is another way for SD to trigger the MR behavior without such a reliance on MR pipeline but I guess you probably already looked at it 😄
The scenarios you've considered above to trigger Secret Detection are only those even I'm aware of (unless we add Scan Execution Policy into the mix to create a combinations).
Scenario: Migrating from existing stable template to latest template (without SEP/ADO)
Currently, the stable template isn't configured to run the SD analyzer in MR pipelines. Due to the missing MR pipeline, the MR security widget ends up showing the source branch's latest branch pipeline results in the widget. This confused some customers thinking the secret findings detected for the initial commit later "disappeared" after making subsequent secretless commits in the MR widget.
After switching to the latest template:
For a new branch, every commit pushed resulted in triggering a Branch pipeline. If a commit contained a secret, the analyzer included the secret finding in the pipeline's vulnerability report.
When an MR is created for a branch, for every commit pushed to the source branch, an MR pipeline is triggered instead of a Branch pipeline. The analyzer ran the scan on all the commits of the source branch instead of choosing only the latest commit(as it happened for the stable template). This resulted in including all the secret findings detected across the commits in the MR pipeline's vulnerability report and thereby in the MR security widget too.
Conclusion: For an existing customer who is not using Scan Execution Policies or ADO, moving the latest template changes to stable template works as expected ✅
Scenario: Using latest CI/CD template for a new project
Scenarios tested:
Introduce a secret in a commit before creating MR to trigger the Branch pipeline. Verify if the analyzer detects the secret in the vulnerability report ✅
Introduce a secret in a commit in a new branch and then Create MR. Verify if the MR widget shows the existing secret finding that was introduced before creating MR ✅
Introduce a secret in a commit after creating MR to trigger the MR pipeline. Verify if the analyzer detects the secret and the MR widget shows the secret finding ✅
Introduce a secret in the middle of the commit chain in the source branch after creating MR. Verify if the analyzer detects the secret and the MR widget shows the secret finding ✅
Conclusion: For a customer who is starting a new project without Scan Execution Policies or ADO, the latest template change works as expected for the above test scenarios ✅
Note: In case you have any scenario in mind that I missed to consider, feel free to add it as a comment under this post. I'll run the test in that scenario and post the results.
Scenario: When latest CI/CD template chosen for the Scan Execution Policy
When .gitlab-ci.yml file is NOT present in the project (SEP internally creates one but won't be visible in the project)
A branch pipeline is created for every commit to a branch. However, when an MR is created for that branch, neither the MR pipeline nor the Branch pipeline is triggered. This behavior causes MR to be mergeable effectively bypassing SEP. ❌
However, when using the stable SD CI template, a branch pipeline is created for every commit regardless of whether an MR is created for that branch. Note that the MR security widget shows only the latest branch pipeline result due to the same situation present for other scenarios. ❓
When .gitlab-ci.yml file is already present in the project
A branch pipeline is created for every commit to a branch. When an MR is created for that branch, the MR pipeline is triggered.
The vulnerability Report and MR security widget behave as expected for all the test scenarios followed when using latest template without SEP. ✅
Conclusion: When .gitlab-ci.yml file is already present in the project, all the test scenarios worked as expected ✅. However, when .gitlab-ci.yml file is NOT present in the project, the MR pipeline is NOT created when the policy is created with the latest SD template ❌. I didn't dig into this further as it was not in the scope of the spike, I could look at it after completing this spike task.
I could reproduce the same behavior again. I've created an issue (#515224 (closed)) and asked for a confirmation in the Security Policies Slack channel. I'll keep this thread updated about it.
I didn't understand what you meant by "Combining MR pipelines with other pipelines supported by other scanners" in the issue description. Do you mind elaborating on that?
Apart from the ones I covered above, do you have any other workflows on your mind that could be impacted by a template update?
@vbhat161 - Welcome back and thanks for thanks for taking a look at this spike!!!
For combining MR pipelines with other pipelines supported by other scanners I mean, what happens when Secret Detection is using MR pipelines for their template but another scanner, say SAST, is using a branch pipeline. Will this break anything for either scanner?!
I don't have any additional workflows that are top of mind, but I do think it's worth documenting a series of tests that could be automated via end to end tests for when we release new templates.
what happens when Secret Detection is using MR pipelines for their template but another scanner, say SAST, is using a branch pipeline. Will this break anything for either scanner?!
AFAIK the scanner results are processed independently. They may share some common database tables, yet the logic is isolated enough to keep them separated. @ahmed.hemdan Do you have any thoughts on this?
I do think it's worth documenting a series of tests that could be automated via end to end tests for when we release new templates.
I agree. However, similar to what Ahmed mentioned here, we need Quality team's bandwidth and expertise to have automate end-to-end tests in place. In the meantime, we could manually verify them. Either way, documenting a series of test scenarios for Secret Detection would be a good start IMO. WDYT?
@amarpatel I remember you created an issue to note down of all SD analyzer scenarios. I'm not able to find it, do you have that issue handy? We could reuse that here.
All the possible scenarios have been tested and verified using the latest Secret Detection CI/CD template, except for a particular scenario: using SEP latest template + No gitlab-ci.yml present in the project (more details here).
If the above-mentioned scenario is not a bug, then all we have to do to move from latest to stable CI/CD template is to add two CI rules(below). I'll update more about this under "Migration Path" section of the issue description once the blocker is resolved.
Migration changes: Two rules to introduce in secret_detection job.
- if: $CI_PIPELINE_SOURCE == "merge_request_event" - if: $CI_OPEN_MERGE_REQUESTS when: never
There is no new progress made to the spike. I was waiting for a couple of updates from the parent issue to provide an update here. Here's the overall update:
There's been a decision to introduce MR pipeline support with an opt-in based(via CI variable) due to limitations. No deprecation announcement needed since this is an opt-in feature.
As far as the migration to latest template goes, there is already a Merge Request under development that introduces MR pipeline support across all the secure analyzers. So if I understand it right from here, there is no engineering effort required from groupsecret detection to make changes in the Secret Detection CI/CD template.
However, there are two other responsibilities from our side:
An in-depth testing to be done post-migration. We could use the workflows outlined here to move forward, although whether the testing to be done manually or automated fashion is the decision yet to be made.
Add any changes to be made in the documentation specific to Secret Detection analyzer regarding MR pipeline support.
Also, the bug that was raised for a particular scenario using SEP with latest template is planned to be fixed in %17.9
All that being said, the original expectations of this spike seems complete to me:
Requirements to promote the latest template changes for secret detection to stable, with MR pipeline support, by %18.0 is finalized
As mentioned above, there is a group MR under development that includes the migration of the latest template, including an opt-in approach.
Identify all manual testing required (collaborate with Support on this if needed)
All the workflows documented here could be used for manual testing. We could carry on further discussion about the testing in that issue.
@abellucci@amarpatel I believe can close this spike issue and if needed, have a separate issue for post-migration steps, unless there is any expectation not covered in this spike. Let me know your thoughts.
As mentioned above, I have created a dedicated issue #517690 to track plans for Secret Detection analyzer once we release the MR pipeline support feature(MR). That said, I'll go ahead an close this spike issue.
@abelluci Thanks for linking relevant links in the issue 🙇
As per the latest update, particularly the below excerpt:
This means that there is no deprecation announcement to be made in 17.9 as there will be no breaking changes in 18.0.
MR pipeline support doesn't require deprecation announcement and it is not a breaking change since we're going with an opt-in approach (with default being disabled). So, do we have to add this feature in Secret Detection 18.0 deprecations, removals an... (&16508)?
@abellucci we intended on trying to ship this in 17.10 actually. Since this is like releasing any feature addition, there is no need to wait for the 18.0 release.
If this likely creates too much pressure for your team to assess and validate the change in 17.10, please verbalize it in #410880.
Documentation wise, I think it makes sense to have these inlined in the same MR as the template. Due to how documentation deployment process varies from code changes, things can become out of sync. So to me it's simpler to keep things together and ensure they are released synchronously.