Make owner optional in parseRemoteFileAddress

This commit is contained in:
Michael B. Gale
2026-06-23 17:49:21 +01:00
parent e537ff20a4
commit 12821cff0c
5 changed files with 121 additions and 53 deletions

View File

@@ -6,7 +6,7 @@ import {
RepositoryPropertyName,
} from "../feature-flags/properties";
import { Logger } from "../logging";
import { ConfigurationError } from "../util";
import { ConfigurationError, getEnv } from "../util";
import { parseUserConfig, UserConfig } from "./db-config";
import { parseRemoteFileAddress } from "./remote-file";
@@ -55,7 +55,7 @@ export async function getRemoteConfig(
apiDetails: api.GitHubApiCombinedDetails,
validateConfig: boolean,
): Promise<UserConfig> {
const address = parseRemoteFileAddress(configFile);
const address = parseRemoteFileAddress(getEnv(), configFile);
const response = await api
.getApiClientWithExternalAuth(apiDetails)

View File

@@ -1,5 +1,8 @@
import test from "ava";
import sinon from "sinon";
import { ActionsEnvVars } from "../actions-util";
import { getTestEnv } from "../testing-utils";
import { ConfigurationError } from "../util";
import {
@@ -10,7 +13,9 @@ import {
} from "./remote-file";
test("expandConfigFileInput accepts full remote addresses", async (t) => {
t.deepEqual(parseRemoteFileAddress("owner/repo/path@ref"), {
const env = getTestEnv();
t.deepEqual(parseRemoteFileAddress(env, "owner/repo/path@ref"), {
owner: "owner",
repo: "repo",
path: "path",
@@ -18,7 +23,7 @@ test("expandConfigFileInput accepts full remote addresses", async (t) => {
} satisfies RemoteFileAddress);
t.deepEqual(
parseRemoteFileAddress("owner/repo/path/to/codeql.yml@ref/feature"),
parseRemoteFileAddress(env, "owner/repo/path/to/codeql.yml@ref/feature"),
{
owner: "owner",
repo: "repo",
@@ -28,15 +33,50 @@ test("expandConfigFileInput accepts full remote addresses", async (t) => {
);
});
test("expandConfigFileInput 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);
});
test("expandConfigFileInput 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,
});
});
test("expandConfigFileInput accepts remote address without a path", async (t) => {
t.deepEqual(parseRemoteFileAddress("owner/repo@ref"), {
const env = getTestEnv();
t.deepEqual(parseRemoteFileAddress(env, "owner/repo@ref"), {
owner: "owner",
repo: "repo",
path: DEFAULT_CONFIG_FILE_NAME,
ref: "ref",
} satisfies RemoteFileAddress);
t.deepEqual(parseRemoteFileAddress("owner/repo"), {
t.deepEqual(parseRemoteFileAddress(env, "owner/repo"), {
owner: "owner",
repo: "repo",
path: DEFAULT_CONFIG_FILE_NAME,
@@ -45,14 +85,16 @@ test("expandConfigFileInput accepts remote address without a path", async (t) =>
});
test("expandConfigFileInput accepts remote address without a ref", async (t) => {
t.deepEqual(parseRemoteFileAddress("owner/repo/path"), {
const env = getTestEnv();
t.deepEqual(parseRemoteFileAddress(env, "owner/repo/path"), {
owner: "owner",
repo: "repo",
path: "path",
ref: DEFAULT_CONFIG_FILE_REF,
} satisfies RemoteFileAddress);
t.deepEqual(parseRemoteFileAddress("owner/repo/path@"), {
t.deepEqual(parseRemoteFileAddress(env, "owner/repo/path@"), {
owner: "owner",
repo: "repo",
path: "path",
@@ -61,13 +103,17 @@ test("expandConfigFileInput accepts remote address without a ref", async (t) =>
});
test("expandConfigFileInput rejects invalid values", async (t) => {
t.throws(() => parseRemoteFileAddress(" "), {
const env = getTestEnv();
const owner = "owner";
const getRequired = sinon.stub(env, "getRequired");
getRequired
.withArgs(ActionsEnvVars.GITHUB_REPOSITORY)
.returns(`${owner}/current-repo`);
t.throws(() => parseRemoteFileAddress(env, " "), {
instanceOf: ConfigurationError,
});
t.throws(() => parseRemoteFileAddress("repo//absolute"), {
instanceOf: ConfigurationError,
});
t.throws(() => parseRemoteFileAddress("repo:file.yml:unexpected"), {
t.throws(() => parseRemoteFileAddress(env, "repo//absolute"), {
instanceOf: ConfigurationError,
});
});

View File

@@ -1,3 +1,5 @@
import { ActionsEnvVars } from "../actions-util";
import { Env } from "../environment";
import * as errorMessages from "../error-messages";
import { ConfigurationError } from "../util";
@@ -19,30 +21,57 @@ export const DEFAULT_CONFIG_FILE_NAME = ".github/codeql-action.yaml";
/** The default ref to use in configuration file shorthands. */
export const DEFAULT_CONFIG_FILE_REF = "main";
/** Extracts the owner from the `GITHUB_REPOSITORY` environment variable. */
function getDefaultOwner(env: Env): string {
const currentRepoNwo = env.getRequired(ActionsEnvVars.GITHUB_REPOSITORY);
const nwoParts = currentRepoNwo.split("/");
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}'.`,
);
}
return nwoParts[0].trim();
}
/**
* Attempts to parse `configFile` into an array of `RemoteFileAddress` components.
*
* @param env The current environment variables.
* @param configFile The string to try and parse.
* @returns The successful result of executing the regex.
* @throws `ConfigurationError` if the format of `configFile` is not valid.
*/
export function parseRemoteFileAddress(configFile: string): RemoteFileAddress {
export function parseRemoteFileAddress(
env: Env,
configFile: string,
): RemoteFileAddress {
// retrieve the various parts of the config location, and ensure they're present
const format = new RegExp(
"(?<owner>[^/]+)/(?<repo>[^/@]+)(/(?<path>[^@]+))?(@(?<ref>.*))?",
"((?<owner>[^/]+)/)?(?<repo>[^/@]+)(/(?<path>[^@]+))?(@(?<ref>.*))?",
);
const pieces = format.exec(configFile);
// Check that the regular expression matched and that we have at least the required components.
if (!pieces?.groups?.owner || !pieces?.groups?.repo) {
// Check that the regular expression matched and that we have at least the repo name.
if (!pieces?.groups?.repo || pieces.groups.repo.trim().length === 0) {
throw new ConfigurationError(
errorMessages.getConfigFileRepoFormatInvalidMessage(configFile),
);
}
// Ensure that the path is a relative path.
if (pieces.groups.path?.startsWith("/")) {
throw new ConfigurationError(
`The path component of '${configFile}' cannot be an absolute path.`,
);
}
return {
owner: pieces.groups.owner,
repo: pieces.groups.repo,
owner: pieces.groups.owner || getDefaultOwner(env),
repo: pieces.groups.repo.trim(),
path: pieces.groups.path || DEFAULT_CONFIG_FILE_NAME,
ref: pieces.groups.ref || DEFAULT_CONFIG_FILE_REF,
};