Support shorthand forms for `getRemoteConfig` by mbg · Pull Request #3973 · github/codeql-action

mbg · 2026-06-23T17:02:59Z

This is a follow-up to #3963 which modifies getRemoteConfig to accept shorthand addresses for remote files. Concretely, the owner, path, and ref components are now optional and default to the owner of the current repo (as given by GITHUB_REPOSITORY), .github/codeql-action.yaml, and main respectively.

Examples:

owner/repo resolves to owner/repo/.github/codeql-action.yaml@main
owner/repo@dev resolves to owner/repo/.github/codeql-action.yaml@dev
owner/repo/.github/codeql/config.yml resolves to owner/repo/.github/codeql/config.yml@main

Notes for reviewers

We should discuss whether there's a better default we want for the path component
The default ref of main may not always be the default branch for the target repo, but that's OK since it can be manually set to a different ref if needed.
I originally implemented the shorthand forms from my proposal, but then noticed that getRemoteConfig already had a similar (but not equivalent) shorthand format. I then updated that instead so that the components other than the repo are optional. However, this isn't as nice, because / is used as a separator and can also appear in the path component. That means e.g. it is not possible with this format to specify a repo + path since it would be interpreted as owner + repo by the regex instead.

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.

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?

Test repository - This change will be tested on a test repository before merging.
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.

Copilot

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.

Pull request overview

This PR extends remote configuration file support by enhancing getRemoteConfig to accept shorthand remote addresses (optional owner, path, and ref with sensible defaults), and refactors environment access to be more testable via an Env wrapper.

Changes:

Added parseRemoteFileAddress (and tests) to parse shorthand remote config references with defaults for owner/path/ref.
Moved getRemoteConfig into src/config/file.ts and updated callers to use the new parsing logic.
Introduced ActionsEnvVars and an Env abstraction to reduce raw string usage and improve testability.

Show a summary per file

File	Description
src/util.ts	Adds `Env` accessor helpers and refactors env var getters.
src/testing-utils.ts	Adds `getTestEnv()` and tightens typing for default Actions env vars.
src/environment.ts	Introduces `Env` interface abstraction for environment access.
src/config/remote-file.ts	New parser for shorthand remote config references and defaults.
src/config/remote-file.test.ts	Unit tests for remote file address parsing and defaults.
src/config/file.ts	Adds `getRemoteConfig()` using parsed remote address components.
src/config-utils.ts	Switches to the new `getRemoteConfig()` implementation.
src/config-utils.test.ts	Removes a test that no longer matches the supported shorthand syntax.
src/api-client.ts	Replaces raw env var names with `ActionsEnvVars`.
src/actions-util.ts	Adds `ActionsEnvVars` enum and updates env var usages.
lib/entry-points.js	Changed but excluded from review (generated/contents unavailable).

Copilot's findings

Files excluded by content exclusion policy (1)

lib/entry-points.js

Files reviewed: 10/11 changed files
Comments generated: 6

…-file-shorthands

mario-campos

Looks solid! I left a few comments, but minor stuff.

mario-campos · 2026-06-24T16:21:38Z

+  if (nwoParts.length !== 2 || nwoParts[0].trim().length === 0) {
+    // This shouldn't happen, so we should throw if `GITHUB_REPOSITORY` doesn't match
+    // our expectations.
+    throw new Error(
+      `Expected ${ActionsEnvVars.GITHUB_REPOSITORY} to contain a name with owner, but got '${currentRepoNwo}'.`,
+    );
+  }


Nit: if it shouldn't happen, should we check for it?

mario-campos · 2026-06-24T16:34:01Z

+test("parseRemoteFileAddress accepts remote address without an owner", async (t) => {
+  const env = getTestEnv();
+  const owner = "test-owner";
+  const getRequired = sinon.stub(env, "getRequired");
+  getRequired
+    .withArgs(ActionsEnvVars.GITHUB_REPOSITORY)
+    .returns(`${owner}/current-repo`);
+
+  t.deepEqual(parseRemoteFileAddress(env, "repo@ref"), {
+    owner,
+    repo: "repo",
+    path: DEFAULT_CONFIG_FILE_NAME,
+    ref: "ref",
+  } satisfies RemoteFileAddress);
+
+  t.deepEqual(parseRemoteFileAddress(env, "repo"), {
+    owner,
+    repo: "repo",
+    path: DEFAULT_CONFIG_FILE_NAME,
+    ref: DEFAULT_CONFIG_FILE_REF,
+  } satisfies RemoteFileAddress);
+});


What about test cases for repo/path and repo/path@ref?

mario-campos · 2026-06-24T16:35:16Z

+test("parseRemoteFileAddress throws for invalid `GITHUB_REPOSITORY`", async (t) => {
+  const env = getTestEnv();
+  const getRequired = sinon.stub(env, "getRequired");
+  getRequired.withArgs(ActionsEnvVars.GITHUB_REPOSITORY).returns(`not-valid`);
+
+  t.throws(() => parseRemoteFileAddress(env, "repo@ref"), {
+    instanceOf: Error,
+  });
+});


Similarly to my comment above, I think this is overkill, but also know that it's a nitpick.

mario-campos · 2026-06-24T16:39:17Z

+    ref: DEFAULT_CONFIG_FILE_REF,
+  } satisfies RemoteFileAddress);
+
+  t.deepEqual(parseRemoteFileAddress(env, "owner/repo/path@"), {


I think this should be invalid, for the same reason that /repo@ref is invalid.

Edit: Also, I just noticed the expected format below does not seem to indicate that this should be a correct format:

Expected format [<owner>/]<repository>[/<file-path>][@<ref>]

mario-campos · 2026-06-24T16:40:57Z

+  return parseUserConfig(
+    logger,
+    configFile,
+    Buffer.from(fileContents, "base64").toString("binary"),


Suggested change

Buffer.from(fileContents, "base64").toString("binary"),

Buffer.from(fileContents, "base64").toString("utf8"),

UTF-8?

mbg added 10 commits June 23, 2026 16:16

Add ActionsEnvVars enum

c05a5b7

Allow abstracting over process.env

652296e

Add RemoteFileAddress type

9c4ee01

Move getRemoteConfig to config/file.ts

07b6b1e

Refactor parseRemoteFileAddress out of getRemoteConfig

9ef8be7

Return RemoteFileAddress from parseRemoteFileAddress

82e5ca6

Add getRemoteConfig JSDoc

c7a94c9

Add tests for parseRemoteFileAddress

85c8a8c

Make ref optional in parseRemoteFileAddress

598d008

Make path optional in parseRemoteFileAddress

e537ff2

mbg requested a review from mario-campos June 23, 2026 17:03

mbg self-assigned this Jun 23, 2026

github-actions Bot added the size/L May be hard to review label Jun 23, 2026

Make owner optional in parseRemoteFileAddress

12821cf

mbg force-pushed the mbg/repo-props/config-file-shorthands branch from a2218f1 to 12821cf Compare June 23, 2026 17:13

mbg marked this pull request as ready for review June 23, 2026 17:15

mbg requested a review from a team as a code owner June 23, 2026 17:15

Copilot AI review requested due to automatic review settings June 23, 2026 17:15

Copilot started reviewing on behalf of mbg June 23, 2026 17:16 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

Comment thread src/util.ts

Comment thread src/config/remote-file.ts

Comment thread src/config/remote-file.ts Outdated

Comment thread src/config/remote-file.ts

Comment thread src/config/remote-file.test.ts Outdated

Comment thread src/config/remote-file.ts

mbg added 6 commits June 23, 2026 18:31

Fix test names

81ad479

Anchor regex and trim input

8102fa6

Update format in error message

00e5a58

Improve whitespace handling

8d69da9

Fix getEnv not using getOptionalEnvVarFrom

3fe7ef9

Merge remote-tracking branch 'origin/main' into mbg/repo-props/config…

812b882

…-file-shorthands

mario-campos approved these changes Jun 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support shorthand forms for `getRemoteConfig`#3973

Support shorthand forms for `getRemoteConfig`#3973
mbg wants to merge 17 commits into
mainfrom
mbg/repo-props/config-file-shorthands

mbg commented Jun 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mario-campos left a comment

Uh oh!

mario-campos Jun 24, 2026

Uh oh!

mario-campos Jun 24, 2026

Uh oh!

mario-campos Jun 24, 2026

Uh oh!

mario-campos Jun 24, 2026

Uh oh!

mario-campos Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	Buffer.from(fileContents, "base64").toString("binary"),
	Buffer.from(fileContents, "base64").toString("utf8"),

Conversation

mbg commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Risk assessment

Which use cases does this change impact?

How did/will you validate this change?

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

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mario-campos left a comment

Choose a reason for hiding this comment

Uh oh!

mario-campos Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

mario-campos Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

mario-campos Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

mario-campos Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

mario-campos Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mbg commented Jun 23, 2026 •

edited

Loading