FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fetch proxy binaries from `defaults.json` release by mbg · Pull Request #3110 · github/codeql-action · GitHub

/ codeql-action Public

Fetch proxy binaries from defaults.json release - #3110

Merged
mbg merged 3 commits into
mainfrom
mbg/proxy/fetch-from-release
Sep 12, 2025
ConversationCommits (3)ChecksFiles changed
Merged

Fetch proxy binaries from defaults.json release#3110
mbg merged 3 commits into
mainfrom
mbg/proxy/fetch-from-release

Conversation

mbg commented Sep 11, 2025

Copy link
Copy Markdown
Member

Currently, the start-proxy action obtains the platform-specific update-job-proxy binary it needs from a hard-coded CodeQL CLI bundle release on the github/codeql-action repo, if it is not already in the runner's toolcache.

We will soon be including up-to-date versions of the update-job-proxy binaries with every CodeQL CLI bundle release.

This PR modifies the start-proxy action to search the release assets of the release pointed at by defaults.json for an appropriate update-job-proxy asset and downloads it, if it is not already in the runner's toolcache.

If the release pointed at by defaults.json doesn't contain the right asset for whatever reason, we revert to using the hard-coded release instead.

Because update-job-proxy isn't versioned, and we require a version for the toolcache, we use the CodeQL CLI version of the release the update-job-proxy is 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:

  • 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.
  • High risk: Changes are not fully under feature flags, have limited visibility and/or cannot be tested outside of production.

Merge / deployment checklist

  • 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.

Copilot AI review requested due to automatic review settings September 11, 2025 18:03
mbg requested a review from a team as a code owner September 11, 2025 18:03
mbg requested a review from henrymercer September 11, 2025 18:04
Copilot AI reviewed Sep 11, 2025
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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)

Comment thread src/start-proxy.ts Outdated
Comment thread src/start-proxy.ts
for (const asset of cliRelease.data.assets) {
if (asset.name === proxyPackage) {
logger.info(
`Found '${proxyPackage}' in release '${defaults.bundleVersion}' at '${asset.url}'`,

Copilot AI Sep 11, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

[nitpick] Logging the asset URL could potentially expose sensitive information. Consider logging only the asset name and release version instead of the full URL.

Suggested change
`Found '${proxyPackage}' in release '${defaults.bundleVersion}' at '${asset.url}'`,
`Found '${proxyPackage}' in release '${defaults.bundleVersion}'.`,

Copilot uses AI. Check for mistakes.

mbg Sep 12, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Comment thread src/start-proxy-action.ts
let proxyBin = toolcache.find(proxyFileName, proxyInfo.version);
if (!proxyBin) {
const temp = await toolcache.downloadTool(proxyURL);
const temp = await toolcache.downloadTool(proxyInfo.url);

Copilot AI Sep 11, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.
henrymercer reviewed Sep 12, 2025
View reviewed changes

henrymercer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good, couple of minor comments.

Comment thread src/start-proxy.ts Outdated
Comment thread src/start-proxy.ts
Comment on lines +10 to +12
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/";

henrymercer Sep 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Minor: Rename to include _FALLBACK_?

mbg Sep 12, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I am not going to bother, since we can hopefully just remove the fallback logic once the next CodeQL CLI release has happened?

mbg merged commit dc9a47d into main Sep 12, 2025
290 checks passed
mbg deleted the mbg/proxy/fetch-from-release branch September 12, 2025 11:38
This was referenced Sep 17, 2025
Closed
Merged
github-actions Bot mentioned this pull request Sep 25, 2025
Merged
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
henrymercer henrymercer approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL