Implement dumping of processed sarif file

[redsun82]

This introduces a CODEQL_ACTION_SARIF_DUMP_DIR environment variable for internal use that causes the processed SARIF file that codeql-action would upload to Code Scanning or Code Quality to be dumped locally. Crucially, this will also happen on analyze with upload: never.

This will allow us to test what is actually sent for upload after the SARIF processing that upload-lib.ts does (validating, merging, filtering, adding fingerprints etc.).

This SARIF file will be dumped in the directory specified by the environment variable, with upload.sarif or upload.quality.sarif as name. There is a risk of naming conflict if an output SARIF file has that name (which depends on the category), but I'm deeming it low risk (and not happening for our testing usage, where the file names are language names).

Risk assessment

For internal use only. Please select the risk level of this change:

• Low risk: Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.

Merge / deployment checklist

• Run analysis with this on a test repository
• Confirm this change is backwards compatible with existing workflows.
• Consider adding a changelog entry for this change.
• Confirm the readme and docs have been updated if necessary.

[mbg]

I have had an initial look over this. It looks like a more involved change than I had hoped -- I was hoping we could just check the environment variable once we have the post-processed SARIF, just before uploading it, and then dump it to a file/folder.

This part of the Action is evolving quite a bit at the moment due to the work on Code Quality and I was planning to make a bunch of changes here in the next few days. They probably wouldn't interact nicely with these changes, though.

[redsun82]

I have had an initial look over this. It looks like a more involved change than I had hoped -- I was hoping we could just check the environment variable once we have the post-processed SARIF, just before uploading it, and then dump it to a file/folder.

This part of the Action is evolving quite a bit at the moment due to the work on Code Quality and I was planning to make a bunch of changes here in the next few days. They probably wouldn't interact nicely with these changes, though.

I had hoped that as well, but:

• we want to do the processing and dump the sarif even if upload: false, so we need a way to arrive at the dump point but skip the uploading
• another unfortunately subtle point, is where to dump this file. At the moment we would do the dumping in the old version of the code we would only have a bunch of input sarif files. By unifying the access to the uploading to only one function with one parameter (so avoiding the uploadFiles/uploadSpecifiedFiles duality), we can decide a bit more clearly where to put the new file (in the directory, if the input is a directory, or as a sibling of the input, if it is a file). I could have turned CODEQL_ACTION_DUMP_SARIF into the desired output path, but decided against that because of the new Code Scanning/Code Quality duality (we would have needed two separate env variables...)

[redsun82]

• but decided against that because of the new Code Scanning/Code Quality duality (we would have needed two separate env variables...)

but in light of the other comments here, it makes sense to make that point to a directory instead

[esbena]

but decided against that because of the new Code Scanning/Code Quality duality (we would have needed two separate env variables...)

We could still have code-scanning/code-quality folders inside our desired output dir. It's still a smaller dependency on internal path choices.

[redsun82]

but decided against that because of the new Code Scanning/Code Quality duality (we would have needed two separate env variables...)

We could still have code-scanning/code-quality folders inside our desired output dir. It's still a smaller dependency on internal path choices.

we already have the .sarif / .quality.sarif discriminant for that. Pointing the env var to a directory is a good choice I think

[redsun82]

@mbg I tried a more surgical approach now (I recommend hiding whitespace in the diff view). I did like the fact that I had only one function in the lib processing the sarifs (as opposed to uploadFiles/uploadSpecifiedFiles), but I can understand it's more pragmatic to limit the changes done in the area, especially as it is a moving target with the Code Quality work.

I still had to change the analyze action as well, because of the requirement to dump the SARIF file if requested even if upload: false.

[Copilot]

Pull Request Overview

This PR implements a new internal testing feature that allows dumping of processed SARIF files to a local directory via the CODEQL_ACTION_SARIF_DUMP_DIR environment variable. The feature works regardless of whether upload is enabled or disabled, making it useful for testing what gets sent to Code Scanning or Code Quality after SARIF processing.

Key changes:

• Added CODEQL_ACTION_SARIF_DUMP_DIR environment variable support for dumping processed SARIF files
• Refactored upload functions to conditionally process and upload based on upload kind and dump directory settings
• Modified analyze action to use the new conditional upload mechanism

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

File	Description
src/upload-lib.ts	Refactored upload functions to support conditional processing and added SARIF dumping functionality
src/environment.ts	Added new environment variable for SARIF dump directory
src/analyze-action.ts	Updated to use conditional upload functions and handle optional upload results

[mbg]

Thanks for simplifying this, it's really appreciated!

I think there are fundamentally two things going on in this PR:

• Dumping the post-processed SARIF based on the value of the environment variable
• Refactoring upload-lib to support this when the upload input is not always

Perhaps it would be sensible to split these two aspects up into separate changes:

• One PR which implements the environment variable controlling where the post-processed SARIF is written to, but which will only do this just before uploading (i.e. the simple case we hoped for). Hopefully that is an easy change that we can extract from this PR and quickly review and merge.
• Then we can think about what the best way to refactor upload-lib to support this in the case where we wouldn't be uploading the SARIF is. I think that's a change that we want to think about more and, possibly, at the same time as changes I should make in the context of the Code Quality work.

[redsun82]

• One PR which implements the environment variable controlling where the post-processed SARIF is written to, but which will only do this just before uploading (i.e. the simple case we hoped for). Hopefully that is an easy change that we can extract from this PR and quickly review and merge.

Fair enough, here's the PR 🙂 :

• Dump soon to be uploaded SARIF on request #3097