Skip to content

Commit 12821cf

Browse files
committed
Make owner optional in parseRemoteFileAddress
1 parent e537ff2 commit 12821cf

5 files changed

Lines changed: 121 additions & 53 deletions

File tree

lib/entry-points.js

Lines changed: 27 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/config-utils.test.ts

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -424,34 +424,6 @@ test.serial("load input outside of workspace", async (t) => {
424424
});
425425
});
426426

427-
test.serial("load non-local input with invalid repo syntax", async (t) => {
428-
return await withTmpDir(async (tempDir) => {
429-
// no filename given, just a repo
430-
const configFile = "octo-org/codeql-config@main";
431-
432-
try {
433-
await configUtils.initConfig(
434-
createFeatures([]),
435-
createTestInitConfigInputs({
436-
configFile,
437-
tempDir,
438-
workspacePath: tempDir,
439-
}),
440-
);
441-
throw new Error("initConfig did not throw error");
442-
} catch (err) {
443-
t.deepEqual(
444-
err,
445-
new ConfigurationError(
446-
errorMessages.getConfigFileRepoFormatInvalidMessage(
447-
"octo-org/codeql-config@main",
448-
),
449-
),
450-
);
451-
}
452-
});
453-
});
454-
455427
test.serial("load non-existent input", async (t) => {
456428
return await withTmpDir(async (tempDir) => {
457429
const languagesInput = "javascript";

src/config/file.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
RepositoryPropertyName,
77
} from "../feature-flags/properties";
88
import { Logger } from "../logging";
9-
import { ConfigurationError } from "../util";
9+
import { ConfigurationError, getEnv } from "../util";
1010

1111
import { parseUserConfig, UserConfig } from "./db-config";
1212
import { parseRemoteFileAddress } from "./remote-file";
@@ -55,7 +55,7 @@ export async function getRemoteConfig(
5555
apiDetails: api.GitHubApiCombinedDetails,
5656
validateConfig: boolean,
5757
): Promise<UserConfig> {
58-
const address = parseRemoteFileAddress(configFile);
58+
const address = parseRemoteFileAddress(getEnv(), configFile);
5959

6060
const response = await api
6161
.getApiClientWithExternalAuth(apiDetails)

src/config/remote-file.test.ts

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import test from "ava";
2+
import sinon from "sinon";
23

4+
import { ActionsEnvVars } from "../actions-util";
5+
import { getTestEnv } from "../testing-utils";
36
import { ConfigurationError } from "../util";
47

58
import {
@@ -10,15 +13,17 @@ import {
1013
} from "./remote-file";
1114

1215
test("expandConfigFileInput accepts full remote addresses", async (t) => {
13-
t.deepEqual(parseRemoteFileAddress("owner/repo/path@ref"), {
16+
const env = getTestEnv();
17+
18+
t.deepEqual(parseRemoteFileAddress(env, "owner/repo/path@ref"), {
1419
owner: "owner",
1520
repo: "repo",
1621
path: "path",
1722
ref: "ref",
1823
} satisfies RemoteFileAddress);
1924

2025
t.deepEqual(
21-
parseRemoteFileAddress("owner/repo/path/to/codeql.yml@ref/feature"),
26+
parseRemoteFileAddress(env, "owner/repo/path/to/codeql.yml@ref/feature"),
2227
{
2328
owner: "owner",
2429
repo: "repo",
@@ -28,15 +33,50 @@ test("expandConfigFileInput accepts full remote addresses", async (t) => {
2833
);
2934
});
3035

36+
test("expandConfigFileInput accepts remote address without an owner", async (t) => {
37+
const env = getTestEnv();
38+
const owner = "test-owner";
39+
const getRequired = sinon.stub(env, "getRequired");
40+
getRequired
41+
.withArgs(ActionsEnvVars.GITHUB_REPOSITORY)
42+
.returns(`${owner}/current-repo`);
43+
44+
t.deepEqual(parseRemoteFileAddress(env, "repo@ref"), {
45+
owner,
46+
repo: "repo",
47+
path: DEFAULT_CONFIG_FILE_NAME,
48+
ref: "ref",
49+
} satisfies RemoteFileAddress);
50+
51+
t.deepEqual(parseRemoteFileAddress(env, "repo"), {
52+
owner,
53+
repo: "repo",
54+
path: DEFAULT_CONFIG_FILE_NAME,
55+
ref: DEFAULT_CONFIG_FILE_REF,
56+
} satisfies RemoteFileAddress);
57+
});
58+
59+
test("expandConfigFileInput throws for invalid `GITHUB_REPOSITORY`", async (t) => {
60+
const env = getTestEnv();
61+
const getRequired = sinon.stub(env, "getRequired");
62+
getRequired.withArgs(ActionsEnvVars.GITHUB_REPOSITORY).returns(`not-valid`);
63+
64+
t.throws(() => parseRemoteFileAddress(env, "repo@ref"), {
65+
instanceOf: Error,
66+
});
67+
});
68+
3169
test("expandConfigFileInput accepts remote address without a path", async (t) => {
32-
t.deepEqual(parseRemoteFileAddress("owner/repo@ref"), {
70+
const env = getTestEnv();
71+
72+
t.deepEqual(parseRemoteFileAddress(env, "owner/repo@ref"), {
3373
owner: "owner",
3474
repo: "repo",
3575
path: DEFAULT_CONFIG_FILE_NAME,
3676
ref: "ref",
3777
} satisfies RemoteFileAddress);
3878

39-
t.deepEqual(parseRemoteFileAddress("owner/repo"), {
79+
t.deepEqual(parseRemoteFileAddress(env, "owner/repo"), {
4080
owner: "owner",
4181
repo: "repo",
4282
path: DEFAULT_CONFIG_FILE_NAME,
@@ -45,14 +85,16 @@ test("expandConfigFileInput accepts remote address without a path", async (t) =>
4585
});
4686

4787
test("expandConfigFileInput accepts remote address without a ref", async (t) => {
48-
t.deepEqual(parseRemoteFileAddress("owner/repo/path"), {
88+
const env = getTestEnv();
89+
90+
t.deepEqual(parseRemoteFileAddress(env, "owner/repo/path"), {
4991
owner: "owner",
5092
repo: "repo",
5193
path: "path",
5294
ref: DEFAULT_CONFIG_FILE_REF,
5395
} satisfies RemoteFileAddress);
5496

55-
t.deepEqual(parseRemoteFileAddress("owner/repo/path@"), {
97+
t.deepEqual(parseRemoteFileAddress(env, "owner/repo/path@"), {
5698
owner: "owner",
5799
repo: "repo",
58100
path: "path",
@@ -61,13 +103,17 @@ test("expandConfigFileInput accepts remote address without a ref", async (t) =>
61103
});
62104

63105
test("expandConfigFileInput rejects invalid values", async (t) => {
64-
t.throws(() => parseRemoteFileAddress(" "), {
65-
instanceOf: ConfigurationError,
66-
});
67-
t.throws(() => parseRemoteFileAddress("repo//absolute"), {
106+
const env = getTestEnv();
107+
const owner = "owner";
108+
const getRequired = sinon.stub(env, "getRequired");
109+
getRequired
110+
.withArgs(ActionsEnvVars.GITHUB_REPOSITORY)
111+
.returns(`${owner}/current-repo`);
112+
113+
t.throws(() => parseRemoteFileAddress(env, " "), {
68114
instanceOf: ConfigurationError,
69115
});
70-
t.throws(() => parseRemoteFileAddress("repo:file.yml:unexpected"), {
116+
t.throws(() => parseRemoteFileAddress(env, "repo//absolute"), {
71117
instanceOf: ConfigurationError,
72118
});
73119
});

src/config/remote-file.ts

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { ActionsEnvVars } from "../actions-util";
2+
import { Env } from "../environment";
13
import * as errorMessages from "../error-messages";
24
import { ConfigurationError } from "../util";
35

@@ -19,30 +21,57 @@ export const DEFAULT_CONFIG_FILE_NAME = ".github/codeql-action.yaml";
1921
/** The default ref to use in configuration file shorthands. */
2022
export const DEFAULT_CONFIG_FILE_REF = "main";
2123

24+
/** Extracts the owner from the `GITHUB_REPOSITORY` environment variable. */
25+
function getDefaultOwner(env: Env): string {
26+
const currentRepoNwo = env.getRequired(ActionsEnvVars.GITHUB_REPOSITORY);
27+
const nwoParts = currentRepoNwo.split("/");
28+
29+
if (nwoParts.length !== 2 || nwoParts[0].trim().length === 0) {
30+
// This shouldn't happen, so we should throw if `GITHUB_REPOSITORY` doesn't match
31+
// our expectations.
32+
throw new Error(
33+
`Expected ${ActionsEnvVars.GITHUB_REPOSITORY} to contain a name with owner, but got '${currentRepoNwo}'.`,
34+
);
35+
}
36+
37+
return nwoParts[0].trim();
38+
}
39+
2240
/**
2341
* Attempts to parse `configFile` into an array of `RemoteFileAddress` components.
2442
*
43+
* @param env The current environment variables.
2544
* @param configFile The string to try and parse.
2645
* @returns The successful result of executing the regex.
2746
* @throws `ConfigurationError` if the format of `configFile` is not valid.
2847
*/
29-
export function parseRemoteFileAddress(configFile: string): RemoteFileAddress {
48+
export function parseRemoteFileAddress(
49+
env: Env,
50+
configFile: string,
51+
): RemoteFileAddress {
3052
// retrieve the various parts of the config location, and ensure they're present
3153
const format = new RegExp(
32-
"(?<owner>[^/]+)/(?<repo>[^/@]+)(/(?<path>[^@]+))?(@(?<ref>.*))?",
54+
"((?<owner>[^/]+)/)?(?<repo>[^/@]+)(/(?<path>[^@]+))?(@(?<ref>.*))?",
3355
);
3456
const pieces = format.exec(configFile);
3557

36-
// Check that the regular expression matched and that we have at least the required components.
37-
if (!pieces?.groups?.owner || !pieces?.groups?.repo) {
58+
// Check that the regular expression matched and that we have at least the repo name.
59+
if (!pieces?.groups?.repo || pieces.groups.repo.trim().length === 0) {
3860
throw new ConfigurationError(
3961
errorMessages.getConfigFileRepoFormatInvalidMessage(configFile),
4062
);
4163
}
4264

65+
// Ensure that the path is a relative path.
66+
if (pieces.groups.path?.startsWith("/")) {
67+
throw new ConfigurationError(
68+
`The path component of '${configFile}' cannot be an absolute path.`,
69+
);
70+
}
71+
4372
return {
44-
owner: pieces.groups.owner,
45-
repo: pieces.groups.repo,
73+
owner: pieces.groups.owner || getDefaultOwner(env),
74+
repo: pieces.groups.repo.trim(),
4675
path: pieces.groups.path || DEFAULT_CONFIG_FILE_NAME,
4776
ref: pieces.groups.ref || DEFAULT_CONFIG_FILE_REF,
4877
};

0 commit comments

Comments
 (0)