-
Notifications
You must be signed in to change notification settings - Fork 438
Add csra analysis kind
#3474
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add csra analysis kind
#3474
Conversation
deac236 to
29847d7
Compare
29847d7 to
40e87b6
Compare
40e87b6 to
c48cd24
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds a new csra analysis kind to the CodeQL Action, intended to largely mirror code-scanning while uploading SARIF to a different endpoint with a different payload shape. The change introduces analysis-kind compatibility enforcement and extends unit + PR-check coverage to include the new kind.
Changes:
- Add
AnalysisKind.CSRAplus a compatibility matrix that prevents unsupported combinations. - Introduce typed upload payload shapes and an
AnalysisConfig.transformPayloadhook to support analysis-specific upload payloads (CSRA addsassessment_id). - Extend unit tests and PR-checks to cover
csraSARIF naming/grouping and matrix validation.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/upload-lib/types.ts | Introduces typed payload interfaces (BasePayload, UploadPayload, AssessmentPayload). |
| src/upload-lib.ts | Uses typed payloads; routes upload payload through transformPayload prior to request. |
| src/upload-lib.test.ts | Updates SARIF grouping tests to account for all analysis kinds; updates upload payload fixtures typing. |
| src/config-utils.ts | Updates “primary analysis config” selection to handle single-kind configs (incl. CSRA) via getAnalysisConfig. |
| src/config-utils.test.ts | Adds tests for getPrimaryAnalysisConfig with single kind and CS+CQ special-case. |
| src/analyze.ts | Simplifies category computation by always using analysis.fixCategory(...). |
| src/analyze.test.ts | Extends SARIF extension tests to include .csra.sarif. |
| src/analyses.ts | Adds csra kind, endpoint, payload transform, compatibility matrix, and SARIF scan order updates. |
| src/analyses.test.ts | Adds compatibility-matrix-driven tests and verifies Code Scanning predicate rejects other extensions. |
| pr-checks/checks/analysis-kinds.yml | Expands PR-check matrix to include csra and sets required env for assessment id; updates artifact handling. |
| .github/workflows/__analysis-kinds.yml | Generated workflow updated to match pr-checks definition (auto-generated). |
| lib/upload-sarif-action.js | Generated JS output (not reviewed). |
| lib/upload-sarif-action-post.js | Generated JS output (not reviewed). |
| lib/upload-lib.js | Generated JS output (not reviewed). |
| lib/start-proxy-action.js | Generated JS output (not reviewed). |
| lib/start-proxy-action-post.js | Generated JS output (not reviewed). |
| lib/setup-codeql-action.js | Generated JS output (not reviewed). |
| lib/resolve-environment-action.js | Generated JS output (not reviewed). |
| lib/init-action.js | Generated JS output (not reviewed). |
| lib/init-action-post.js | Generated JS output (not reviewed). |
| lib/autobuild-action.js | Generated JS output (not reviewed). |
| lib/analyze-action.js | Generated JS output (not reviewed). |
| lib/analyze-action-post.js | Generated JS output (not reviewed). |
| const assessmentId = parseInt( | ||
| getRequiredEnvParam("CODEQL_ACTION_CSRA_ASSESSMENT_ID"), | ||
| 10, | ||
| ); |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addAssessmentId uses parseInt on CODEQL_ACTION_CSRA_ASSESSMENT_ID, but if the env var is non-numeric this will produce NaN and still get uploaded as assessment_id. Consider validating the parsed value (e.g., Number.isFinite) and throwing a clear configuration error when it isn’t a valid integer.
| const assessmentId = parseInt( | |
| getRequiredEnvParam("CODEQL_ACTION_CSRA_ASSESSMENT_ID"), | |
| 10, | |
| ); | |
| const rawAssessmentId = getRequiredEnvParam( | |
| "CODEQL_ACTION_CSRA_ASSESSMENT_ID", | |
| ); | |
| const assessmentId = parseInt(rawAssessmentId, 10); | |
| if (!Number.isFinite(assessmentId)) { | |
| throw new ConfigurationError( | |
| `Environment variable CODEQL_ACTION_CSRA_ASSESSMENT_ID must be a valid integer, but got: ${rawAssessmentId}`, | |
| ); | |
| } |
| target: SARIF_UPLOAD_ENDPOINT.CSRA, | ||
| sarifExtension: ".csra.sarif", | ||
| sarifPredicate: (name) => name.endsWith(CSRA.sarifExtension), | ||
| fixCategory: fixCodeQualityCategory, |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CSRA.fixCategory is set to fixCodeQualityCategory, but that helper is documented as a Code Quality-only workaround for Default Setup category mappings. Applying it to CSRA could unintentionally rewrite categories (and log "Adjusted category for Code Quality...") for a non-Code-Quality upload target. Consider making CSRA’s fixCategory a no-op (like Code Scanning) or introducing a CSRA-specific adjustment if needed.
| fixCategory: fixCodeQualityCategory, | |
| fixCategory: (_, category) => category, |
| const payload = uploadTarget.transformPayload( | ||
| buildPayload( | ||
| await gitUtils.getCommitOid(checkoutPath), | ||
| await gitUtils.getRef(), | ||
| postProcessingResults.analysisKey, | ||
| util.getRequiredEnvParam("GITHUB_WORKFLOW"), | ||
| zippedSarif, | ||
| actionsUtil.getWorkflowRunID(), | ||
| actionsUtil.getWorkflowRunAttempt(), | ||
| checkoutURI, | ||
| postProcessingResults.environment, | ||
| toolNames, | ||
| await gitUtils.determineBaseBranchHeadCommitOid(), | ||
| ), | ||
| ); |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uploadPostProcessedFiles now routes the payload through uploadTarget.transformPayload(...) before calling uploadPayload, and CSRA relies on this to change the request shape. There isn’t a unit test asserting the transformed payload for CSRA (e.g., that assessment_id is included) or even that transformPayload is applied. Adding a focused test around this call site would help prevent regressions as more analysis kinds are added.
First stab at adding this new analysis kind. See the linked internal issues for more details. The new analysis kind largely mirrors
code-scanning.We do not plan this in combination with other analysis kinds, so I added some logic to prevent that.
The SARIF upload for this analysis kind is a bit different than the other two, so I added a
transformPayloadfunction to theAnalysisConfiginterface that can change what's included in the payload. I preferred that over having a conditional block near the upload, since that's harder to see and less reusable.I extended the existing tests for this new analysis kind.
Notes for reviewers
This is ready for an initial review to examine the high-level approach here. There's almost certainly room for polishing this and adding some more tests.
Best reviewed commit-by-commit.
Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
This change should not directly impact any of the below, but it may indirectly as a result of changes to code paths that are also hit by those.
Workflow types:
dynamicworkflows (Default Setup, CCR, ...).Products:
analysis-kinds: code-scanning.analysis-kinds: code-quality.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