GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Make `auth` in `GitHubApiDetails` optional by mbg · Pull Request #4230 · github/codeql-action

Conversation

@mbg

@mbg mbg commented Oct 13, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Related to #4198 (comment).

We currently have a few different scenarios for where we fetch CodeQL CLI releases from depending on the environment that the CodeQL Action is running in. The usual case is that the running Action comes from github.com/github/codeql-action and is running in a workflow on github.com. In that case, the token we have can be used with api.github.com.

The CodeQL Action may also be running on GHES or GitHub Enterprise Cloud with Data Residency. In those cases, the token we have is for the respective environment. Our logic for finding a release to download the CodeQL CLI from prioritises the more specific environments, but falls back to fetching a release from github.com/github/codeql-action if the release is not available more locally. In that case, we should not use the token to authenticate any requests made from the host instance to the GitHub.com instance.

Currently, for downloads, this is done with the help of getAuthorizationHeaderFor which tests whether a given url matches the GitHub instance represented by a particular GitHubApiDetails object, which also contains the token. The token is only returned if the url is a match for the GitHub instance.

We don't currently have an equivalent process for API requests from a different GitHub instance to GitHub.com, even though the GitHub.com API will accept unauthenticated requests for public resources (with much tighter rate limits than authenticated requests). This is, in part, because we use getOctokitOptions from @actions/github to construct OctokitOptions for the API client, which requires a (non-empty) token.

This PR tidies up some of the code related to this, but mainly replaces the (virtually unnecessary) call to getOctokitOptions so that we are constructing the OctokitOptions ourselves and allow no token to be provided. This is fine for Octokit. This relaxation of the requirements in our api-client.ts library isn't used here, but it will allow us to make some API requests from other GitHub instances to GitHub.com in the future when necessary.

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.

Which use cases does this change impact?

Workflow types:

  • Advanced setup - Impacts users who have custom CodeQL workflows.
  • Managed - Impacts users with dynamic workflows (Default Setup, Code Quality, ...).

Products:

  • Code Scanning - The changes impact analyses when analysis-kinds: code-scanning.
  • Code Quality - The changes impact analyses when analysis-kinds: code-quality.
  • Other first-party - The changes impact other first-party analyses.
  • Third-party analyses - The changes affect the upload-sarif action.

Environments:

  • Dotcom - Impacts CodeQL workflows on github.com and/or GitHub Enterprise Cloud with Data Residency.
  • GHES - Impacts CodeQL workflows on GitHub Enterprise Server.

How did/will you validate this change?

  • Unit tests - I am depending on unit test coverage (i.e. tests in .test.ts files).
  • End-to-end tests - I am depending on PR checks (i.e. tests in pr-checks).

If something goes wrong after this change is released, what are the mitigation and rollback strategies?

  • Rollback - Change can only be disabled by rolling back the release or releasing a new version with a fix.

How will you know if something goes wrong after this change is released?

  • Telemetry - I rely on existing telemetry or have made changes to the telemetry.
    • Dashboards - I will watch relevant dashboards for issues after the release. Consider whether this requires this change to be released at a particular time rather than as part of a regular release.
    • Alerts - New or existing monitors will trip if something goes wrong with this change.

Are there any special considerations for merging or releasing this change?

  • No special considerations - This change can be merged at any time.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@github-actions github-actions Bot added the size/L May be hard to review label Oct 13, 2026
@mbg
mbg force-pushed the mbg/auth/allow-unauthed branch from 8d9efb0 to 654f5d9 Compare October 13, 2026 16:30
@mbg mbg mentioned this pull request Oct 13, 2026
Use correct GitHubApiDetails instance for getApiClient in getCodeQLAssetDownloadURL #4231
Open
@mbg
mbg added this pull request to stack #4232 October 13, 2026 17:46
@mbg
mbg marked this pull request as ready for review October 14, 2026 09:33
@mbg
mbg requested a review from a team as a code owner October 14, 2026 09:33
Copilot AI balanced review requested due to automatic review settings October 14, 2026 09:33
Copilot started reviewing on behalf of mbg October 14, 2026 09:34
View session
Copilot AI reviewed Oct 14, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

🟡 Changes recommended

Auth-less details can still produce malformed token undefined headers, and the new unauthenticated path lacks direct coverage.

2 open findings
  • Medium severity Avoid emitting Authorization header with undefined token · New
  • Low severity Add tests for unauthenticated client behavior · New
What changed in this PR

Enables future unauthenticated GitHub.com API requests from enterprise environments while centralizing GitHub instance details.

Changes:

  • Makes GitHubApiDetails.auth optional and constructs Octokit options directly.
  • Adds shared GitHub instance URL types and helpers.
  • Updates call sites and relocates related tests.
File Description
src/​api-client.ts Supports unauthenticated Octokit clients.
src/​github.ts Adds GitHub instance types and helpers.
src/​github.test.ts Tests the new helpers.
src/​util.ts Removes relocated GitHub URL utilities.
src/​util.test.ts Removes relocated tests.
src/​upload-lib.ts Uses the current-instance helper.
src/​status-report.ts Imports the relocated Dotcom URL.
src/​setup-codeql.ts Uses the relocated Dotcom URL.
src/​setup-codeql-action.ts Uses centralized instance details.
src/​init-action.ts Uses centralized instance details.
src/​database-upload.ts Imports the relocated URL parser.
src/​codeql.test.ts Requires authentication in the relevant test helper.
lib/​entry-points.js Generated bundle file; content excluded from review.
Files excluded by content exclusion policy (1)
  • lib/entry-points.js

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/api-client.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/api-client.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

@mbg
mbg requested a review from henrymercer October 14, 2026 12:41
@mbg
mbg force-pushed the mbg/auth/allow-unauthed branch from 31e38bc to 6315034 Compare October 14, 2026 12:52
mbg added 15 commits October 14, 2026 18:14
@mbg
Improve GitHubApiDetails types and documentation
0735d79
@mbg
Move some definitions out of util.ts into a new github.ts module
102e46e
@mbg
Add functions to get the current and dotcom instance
61bfcff
@mbg
Move GitHubApiDetails to github.ts
e062530
@mbg
Make auth property on GitHubApiDetails optional
a243813
@mbg
Use getCurrentInstance()
96501a3
@mbg
Make externalRepoAuth check more consistent
1b0a617
@mbg
Guard against blank auth value
1d30a84
This shouldn't be possible, but was previously handled by the library
@mbg
Check auth value in getAuthorizationHeaderFor
66c96fd
Also add some tests for it
@mbg
Check token value in uploadBundledDatabase
f9b4185
@mbg
Add very basic getApiClientWithExternalAuth test
bfe264b
@mbg
Allow getApiClient to accept GitHubApiDetails
a08b587
@mbg
Pass apiDetails to getApiClient in uploadBundledDatabase
f6e8dce
@mbg
Add a test for uploadBundledDatabase
047c751
@mbg
Remove explicit authorization header from uploadBundledDatabase
c164184
@mbg
mbg force-pushed the mbg/auth/allow-unauthed branch from 6315034 to c164184 Compare October 14, 2026 19:03
@github-actions github-actions Bot added size/XL May be very hard to review and removed size/L May be hard to review labels Oct 14, 2026
@mbg
mbg requested a balanced review from Copilot October 14, 2026 19:05
Copilot started reviewing on behalf of mbg October 14, 2026 19:05
View session
Copilot AI reviewed Oct 14, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

🟢 Approval recommended

The focused authentication refactor and accompanying regression tests leave no identified blocking issues.

0 open findings

2 resolved since last review
  • Medium severity Avoid emitting Authorization header with undefined token
  • Low severity Add tests for unauthenticated client behavior
Files excluded by content exclusion policy (1)
  • lib/entry-points.js

🧠 Review effort: Balanced

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
henrymercer
henrymercer approved these changes Oct 15, 2026
View reviewed changes

@henrymercer henrymercer left a comment

Copy link
Copy Markdown
Contributor

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.

Thanks for the discussion on Slack and the thorough followup! Just a couple of last suggestions, which I'm happy for you to make in a followup PR.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/database-upload.test.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/database-upload.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/api-client.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

@mbg
mbg added this pull request to the merge queue Oct 16, 2026
Merged via the queue into main with commit 0a458f1 Oct 16, 2026
235 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@mbg
mbg deleted the mbg/auth/allow-unauthed branch October 16, 2026 11:18
This was referenced Oct 21, 2026
Merge main into releases/v4 #4257
Merged
Merge releases/v4 into releases/v3 #4259
Merged
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 henrymercer approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

size/XL May be very hard to review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@mbg @henrymercer
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.