[BUGFIX] Security audit for non-default branches#706
Conversation
|
Regarding the commit message, please use |
|
After thinking about it for a little longer, this would mean that if we want to ignore a dependency on the v2 branch, we need to maintain this in the configuration on main? Maybe we should rather maintain a dedicated configuration file for v2. The content is rather straight forward I think. |
|
@RandomByte that was my initial point, but in that case, we might end up in a situation where we need to maintain a bigger set of configurations that are different per each branch. With the current approach, this is maintained centrally and IMO it could be better handled. Of course, the obvious downside is that the exceptions list in the configuration could be not fully relevant for all of the branches. Just thought of something else.... we could eventually check for a local config and if not, use the one in the default branch. |
Even worse, if we ignore a dependency centrally because we can't upgrade it in one branch (for example because Node.js version support limitations), we would not receive warnings for other issues with that dependencies on branches where an upgrade would be feasible.
That also came to my find. But realistically speaking, we only need this job for the |
|
I'm also interested in @matz3's view on this. Maybe we can put this PR on hold until we come to a good solution and also Matthias was able to add his thoughts? |
I'll add just some notes here. Probably it's a bit more complicated scenario, but some CIs use special branches for that purpose. In those branches is the whole configuration for the CI. I doubt that we have such a complex scenario as you mention that we need this only for the |
| - name: Checkout the default branch (security configuration) | ||
| uses: actions/checkout@v3 | ||
| with: | ||
| path: default |
There was a problem hiding this comment.
What about using the actual branch name main?
| - name: Security check for '${{ matrix.branch }}' branch | ||
| run: | | ||
| cd ${{ matrix.branch }} | ||
| npx audit-ci@^6 --config ../default/audit-ci.jsonc |
There was a problem hiding this comment.
I would expect that the audit-ci.jsonc is stored in the branch that is tested. Otherwise we can't scale this solution to other branches. For example, I don't see a reason why we shouldn't also use this approach for the main branch.
|
Abandoning this pull request as we've agreed to keep the security audit config within the scanned branch. |
…I usage (#706) BREAKING CHANGE: Set default workspaceName to "default" for API usage (SAP/ui5-project#586) JIRA: CPOUI5FOUNDATION-802 Relates to: #701 --------- Co-authored-by: Matthias Oßwald <mat.osswald@sap.com>
…check for UI5 v1.100.0+ Up-port of #706 (cherry picked from commit 10853a5c8f0dac3182813f83b6cdcbfc8ad0e27d)
The configuration of the security audit tool is contained within the main (default) branch, but the security scan should be made on some other branch.
We need to execute the scan on a "random" branch, but take the configuration from the default one.
So, the workflow is: