Fetch proxy binaries from defaults.json release
#3110
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the start-proxy action to fetch proxy binaries from the current CodeQL CLI bundle release instead of a hard-coded release. The main purpose is to ensure the proxy binaries are always up-to-date with the CLI bundle version specified in defaults.json.
Key changes:
- Adds dynamic proxy binary discovery from the current CLI bundle release
- Implements fallback mechanism to hard-coded release if dynamic discovery fails
- Uses CLI version for proxy binary versioning in toolcache
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/start-proxy.ts | Adds new functions for dynamic proxy binary URL resolution and platform detection |
| src/start-proxy.test.ts | Adds unit tests for the new proxy binary discovery logic |
| src/start-proxy-action.ts | Updates proxy binary path resolution to use dynamic URL discovery |
| lib/start-proxy-action.js | Generated JavaScript code (no review needed per guidelines) |
| for (const asset of cliRelease.data.assets) { | ||
| if (asset.name === proxyPackage) { | ||
| logger.info( | ||
| `Found '${proxyPackage}' in release '${defaults.bundleVersion}' at '${asset.url}'`, |
There was a problem hiding this comment.
[nitpick] Logging the asset URL could potentially expose sensitive information. Consider logging only the asset name and release version instead of the full URL.
| `Found '${proxyPackage}' in release '${defaults.bundleVersion}' at '${asset.url}'`, | |
| `Found '${proxyPackage}' in release '${defaults.bundleVersion}'.`, |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
We include the asset URL in the log in setup-codeql.ts as well. I don't think there should be any sensitive information, since the releases are public.
| let proxyBin = toolcache.find(proxyFileName, proxyInfo.version); | ||
| if (!proxyBin) { | ||
| const temp = await toolcache.downloadTool(proxyURL); | ||
| const temp = await toolcache.downloadTool(proxyInfo.url); |
There was a problem hiding this comment.
The URL from proxyInfo.url is downloaded without validation. When using the GitHub API asset URL, consider adding verification that the URL is from a trusted GitHub domain to prevent potential security issues.
Copilot uses AI. Check for mistakes.
| export const UPDATEJOB_PROXY_VERSION = "v2.0.20250624110901"; | ||
| export const UPDATEJOB_PROXY_URL_PREFIX = | ||
| "https://github.com/github/codeql-action/releases/download/codeql-bundle-v2.22.0/"; |
There was a problem hiding this comment.
Minor: Rename to include _FALLBACK_?
There was a problem hiding this comment.
I am not going to bother, since we can hopefully just remove the fallback logic once the next CodeQL CLI release has happened?
Currently, the
start-proxyaction obtains the platform-specificupdate-job-proxybinary it needs from a hard-coded CodeQL CLI bundle release on thegithub/codeql-actionrepo, if it is not already in the runner's toolcache.We will soon be including up-to-date versions of the
update-job-proxybinaries with every CodeQL CLI bundle release.This PR modifies the
start-proxyaction to search the release assets of the release pointed at bydefaults.jsonfor an appropriateupdate-job-proxyasset and downloads it, if it is not already in the runner's toolcache.If the release pointed at by
defaults.jsondoesn't contain the right asset for whatever reason, we revert to using the hard-coded release instead.Because
update-job-proxyisn't versioned, and we require a version for the toolcache, we use the CodeQL CLI version of the release theupdate-job-proxyis obtained from as its version.I have added a few unit tests for this logic.
Risk assessment
For internal use only. Please select the risk level of this change:
Merge / deployment checklist