Make owner optional in parseRemoteFileAddress

Author: mbg

lib/entry-points.js

@@ -424,34 +424,6 @@ test.serial("load input outside of workspace", async (t) => {
   });
 });
 
-test.serial("load non-local input with invalid repo syntax", async (t) => {
-  return await withTmpDir(async (tempDir) => {
-    // no filename given, just a repo
-    const configFile = "octo-org/codeql-config@main";
-
-    try {
-      await configUtils.initConfig(
-        createFeatures([]),
-        createTestInitConfigInputs({
-          configFile,
-          tempDir,
-          workspacePath: tempDir,
-        }),
-      );
-      throw new Error("initConfig did not throw error");
-    } catch (err) {
-      t.deepEqual(
-        err,
-        new ConfigurationError(
-          errorMessages.getConfigFileRepoFormatInvalidMessage(
-            "octo-org/codeql-config@main",
-          ),
-        ),
-      );
-    }
-  });
-});
-
 test.serial("load non-existent input", async (t) => {
   return await withTmpDir(async (tempDir) => {
     const languagesInput = "javascript";

src/config-utils.test.ts

@@ -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)

src/config/file.ts

@@ -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,15 +13,17 @@ 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",
     ref: "ref",
   } 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",
@@ -29,14 +34,16 @@ test("expandConfigFileInput accepts full remote addresses", async (t) => {
 });
 
 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 +52,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 +70,17 @@ test("expandConfigFileInput accepts remote address without a ref", async (t) =>
 });
 
 test("expandConfigFileInput rejects invalid values", async (t) => {
-  t.throws(() => parseRemoteFileAddress("  "), {
-    instanceOf: ConfigurationError,
-  });
-  t.throws(() => parseRemoteFileAddress("repo//absolute"), {
+  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:file.yml:unexpected"), {
+  t.throws(() => parseRemoteFileAddress(env, "repo//absolute"), {
     instanceOf: ConfigurationError,
   });
 });

src/config/remote-file.test.ts

@@ -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,
   };