Upload failed SARIF for risk assessments in init-post step#3519
Conversation
github-actions
bot
added
size/L
There was a problem hiding this comment.
Pull request overview
This PR enhances the init-post action to upload failed SARIF files as workflow artifacts when running risk assessments, extending the existing failure diagnostics infrastructure to support a new analysis type.
Changes:
- Refactored failed SARIF preparation logic to support both code scanning (uploaded to GitHub) and risk assessment (uploaded as artifacts)
- Added
isRiskAssessmentEnabledhelper function to check if risk assessment analysis is enabled - Renamed
runfunction touploadFailureInfoininit-action-post-helper.tsto better reflect its purpose - Added comprehensive test coverage for risk assessment artifact upload scenarios
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/init-action-post.ts | Updated function call from run to uploadFailureInfo |
| src/init-action-post-helper.ts | Refactored failed SARIF logic: extracted prepareFailedSarif and generateFailedSarif, added maybeUploadFailedSarifArtifact for risk assessments, improved conditional logic |
| src/init-action-post-helper.test.ts | Added tests for risk assessment artifact uploads (both diagnosticsExport and databaseExportDiagnostics paths) and edge cases |
| src/config-utils.ts | Added isRiskAssessmentEnabled helper function for checking risk assessment analysis kind |
| lib/init-action-post.js | Auto-generated JavaScript from TypeScript source (not reviewed) |
Comments suppressed due to low confidence (1)
src/init-action-post-helper.ts:86
- Typo in the comment: "can contains" should be "can contain" (singular, not plural).
* Tries to prepare a SARIF file that can contains information about a failed analysis.
henrymercer
left a comment
henrymercer
left a comment
There was a problem hiding this comment.
This looks good. I like the way you've broken up the methods and the use of Result. Before merging, I'd like to see an end-to-end test similar to the "Submit SARIF after failure" PR check for risk assessment. CCR also has suggested some typo fixes.
henrymercer
left a comment
henrymercer
left a comment
There was a problem hiding this comment.
Happy to approve now to help avoid merge conflicts, providing we add the PR check as followup.
This modifies the
init-post action to upload a failed SARIF as a workflow artifact for risk assessments. This mirrors what we do for Code Scanning, except there we upload the SARIF to the API.Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Workflow types:
dynamicworkflows (Default Setup, Code Quality, ...).Products:
Environments:
github.comand/or GitHub Enterprise Cloud with Data Residency.How did/will you validate this change?
.test.tsfiles).pr-checks).If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist