Gate new remote file format behind FF

lib/entry-points.js

@@ -151056,6 +151056,11 @@ function getInvalidConfigFileMessage(configFile, messages) {
   const andMore = messages.length > 10 ? `, and ${messages.length - 10} more.` : ".";
   return `The configuration file "${configFile}" is invalid: ${messages.slice(0, 10).join(", ")}${andMore}`;
 }
+function getConfigFileRepoOldFormatInvalidMessage(configFile) {
+  let error3 = `The configuration file "${configFile}" is not a supported remote file reference.`;
+  error3 += " Expected format <owner>/<repository>/<file-path>@<ref>";
+  return error3;
+}
 function getConfigFileRepoFormatInvalidMessage(configFile) {
   let error3 = `The configuration file "${configFile}" is not a supported remote file reference.`;
   error3 += " Expected format [<owner>/]<repository>[@<ref>][:<file-path>]";
@@ -151467,11 +151472,19 @@ function parseOldRemoteFileAddress(input) {
     ref: pieces.groups.ref.trim()
   });
 }
-function parseRemoteFileAddress(env, configFile) {
+async function parseRemoteFileAddress(actionState, configFile) {
   const oldFormatAddressResult = parseOldRemoteFileAddress(configFile);
   if (oldFormatAddressResult.isSuccess()) {
     return oldFormatAddressResult.value;
   }
+  const allowNewFormat = await actionState.features.getValue(
+    "new_remote_file_addresses" /* NewRemoteFileAddresses */
+  );
+  if (!allowNewFormat) {
+    throw new ConfigurationError(
+      getConfigFileRepoOldFormatInvalidMessage(configFile)
+    );
+  }
   const format = new RegExp(
     "^((?<owner>[^:@/]+)/)?(?<repo>[^:@/]+)(@(?<ref>[^:]+))?(:(?<path>.+))?$"
   );
@@ -151491,7 +151504,7 @@ function parseRemoteFileAddress(env, configFile) {
     );
   }
   return {
-    owner: owner || getDefaultOwner(env),
+    owner: owner || getDefaultOwner(actionState.env),
     repo,
     path: path29 || DEFAULT_CONFIG_FILE_NAME,
     ref: ref || DEFAULT_CONFIG_FILE_REF
@@ -151515,7 +151528,7 @@ function getConfigFileInput(logger, actions, repositoryProperties) {
   return void 0;
 }
 async function getRemoteConfig(actionState, configFile, apiDetails) {
-  const address = parseRemoteFileAddress(getEnv(), configFile);
+  const address = await parseRemoteFileAddress(actionState, configFile);
   const response = await getApiClientWithExternalAuth(apiDetails).rest.repos.getContent({
     owner: address.owner,
     repo: address.repo,

src/config/file.ts

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

src/config/remote-file.test.ts

@@ -2,7 +2,9 @@ import test from "ava";
 import sinon from "sinon";
 
 import { ActionsEnvVars } from "../actions-util";
-import { getTestEnv } from "../testing-utils";
+import * as errors from "../error-messages";
+import { Feature } from "../feature-flags";
+import { callee, getTestEnv } from "../testing-utils";
 import { ConfigurationError } from "../util";
 
 import {
@@ -12,230 +14,220 @@ import {
   RemoteFileAddress,
 } from "./remote-file";
 
+type ParseRemoteFileAddressTest = {
+  input: string;
+  expected: RemoteFileAddress;
+};
+
 test("parseRemoteFileAddress accepts full remote addresses", async (t) => {
-  const env = getTestEnv();
+  const target = callee(parseRemoteFileAddress);
 
-  // Old format.
+  const expected: RemoteFileAddress = {
-  t.deepEqual(parseRemoteFileAddress(env, "owner/repo/path@ref"), {
     owner: "owner",
     repo: "repo",
     path: "path",
     ref: "ref",
-  } satisfies RemoteFileAddress);
+  };
 
-  t.deepEqual(parseRemoteFileAddress(env, "owner  /repo/path@ref"), {
+  const oldFormatInputs: ParseRemoteFileAddressTest[] = [
-    owner: "owner",
+    { input: "owner/repo/path@ref", expected },
-    repo: "repo",
+    { input: "owner  /repo/path@ref", expected },
-    path: "path",
+    { input: "owner/   repo/path@ref", expected },
-    ref: "ref",
+    { input: "owner/repo   /path@ref", expected },
-  } satisfies RemoteFileAddress);
+    { input: "owner/repo/   path@ref", expected },
-
+    { input: "owner/repo/path   @ref", expected },
-  t.deepEqual(parseRemoteFileAddress(env, "owner/   repo/path@ref"), {
+    { input: "owner/repo/path@   ref", expected },
-    owner: "owner",
-    repo: "repo",
-    path: "path",
-    ref: "ref",
-  } satisfies RemoteFileAddress);
-
-  t.deepEqual(parseRemoteFileAddress(env, "owner/repo   /path@ref"), {
-    owner: "owner",
-    repo: "repo",
-    path: "path",
-    ref: "ref",
-  } satisfies RemoteFileAddress);
-
-  t.deepEqual(parseRemoteFileAddress(env, "owner/repo/   path@ref"), {
-    owner: "owner",
-    repo: "repo",
-    path: "path",
-    ref: "ref",
-  } satisfies RemoteFileAddress);
-
-  t.deepEqual(parseRemoteFileAddress(env, "owner/repo/path   @ref"), {
-    owner: "owner",
-    repo: "repo",
-    path: "path",
-    ref: "ref",
-  } satisfies RemoteFileAddress);
-
-  t.deepEqual(parseRemoteFileAddress(env, "owner/repo/path@   ref"), {
-    owner: "owner",
-    repo: "repo",
-    path: "path",
-    ref: "ref",
-  } satisfies RemoteFileAddress);
-
-  t.deepEqual(
-    parseRemoteFileAddress(env, "owner/repo/path/to/codeql.yml@ref/feature"),
     {
-      owner: "owner",
+      input: "owner/repo/path/to/codeql.yml@ref/feature",
-      repo: "repo",
+      expected: { ...expected, path: "path/to/codeql.yml", ref: "ref/feature" },
-      path: "path/to/codeql.yml",
+    },
-      ref: "ref/feature",
-    } satisfies RemoteFileAddress,
-  );
-
-  t.deepEqual(
-    parseRemoteFileAddress(
-      env,
-      "  owner/repo/path/to/codeql.yml@ref/feature  ",
-    ),
     {
-      owner: "owner",
+      input: "  owner/repo/path/to/codeql.yml@ref/feature  ",
-      repo: "repo",
+      expected: { ...expected, path: "path/to/codeql.yml", ref: "ref/feature" },
-      path: "path/to/codeql.yml",
+    },
-      ref: "ref/feature",
+  ];
-    } satisfies RemoteFileAddress,
+
-  );
+  for (const oldFormatInput of oldFormatInputs) {
+    await target
+      .withArgs(oldFormatInput.input)
+      .passes(async (fn) => t.deepEqual(await fn(), oldFormatInput.expected));
+  }
 
   // New format.
-  t.deepEqual(parseRemoteFileAddress(env, "owner/repo@ref:path"), {
+  const newFormatInputs: ParseRemoteFileAddressTest[] = [
-    owner: "owner",
+    { input: "owner/repo@ref:path", expected },
-    repo: "repo",
+    { input: "owner  /repo@ref:path", expected },
-    path: "path",
+    { input: "owner/   repo@ref:path", expected },
-    ref: "ref",
+    { input: "owner/repo   @ref:path", expected },
-  } satisfies RemoteFileAddress);
+    { input: "owner/repo@   ref:path", expected },
-
+    { input: "owner/repo@ref   :path", expected },
-  t.deepEqual(parseRemoteFileAddress(env, "owner  /repo@ref:path"), {
+    { input: "owner/repo@ref:   path", expected },
-    owner: "owner",
-    repo: "repo",
-    path: "path",
-    ref: "ref",
-  } satisfies RemoteFileAddress);
-
-  t.deepEqual(parseRemoteFileAddress(env, "owner/   repo@ref:path"), {
-    owner: "owner",
-    repo: "repo",
-    path: "path",
-    ref: "ref",
-  } satisfies RemoteFileAddress);
-
-  t.deepEqual(parseRemoteFileAddress(env, "owner/repo   @ref:path"), {
-    owner: "owner",
-    repo: "repo",
-    path: "path",
-    ref: "ref",
-  } satisfies RemoteFileAddress);
-
-  t.deepEqual(parseRemoteFileAddress(env, "owner/repo@   ref:path"), {
-    owner: "owner",
-    repo: "repo",
-    path: "path",
-    ref: "ref",
-  } satisfies RemoteFileAddress);
-
-  t.deepEqual(parseRemoteFileAddress(env, "owner/repo@ref   :path"), {
-    owner: "owner",
-    repo: "repo",
-    path: "path",
-    ref: "ref",
-  } satisfies RemoteFileAddress);
-
-  t.deepEqual(parseRemoteFileAddress(env, "owner/repo@ref:   path"), {
-    owner: "owner",
-    repo: "repo",
-    path: "path",
-    ref: "ref",
-  } satisfies RemoteFileAddress);
-
-  t.deepEqual(
-    parseRemoteFileAddress(env, "owner/repo@ref/feature:path/to/codeql.yml"),
     {
-      owner: "owner",
+      input: "owner/repo@ref/feature:path/to/codeql.yml",
-      repo: "repo",
+      expected: { ...expected, path: "path/to/codeql.yml", ref: "ref/feature" },
-      path: "path/to/codeql.yml",
+    },
-      ref: "ref/feature",
-    } satisfies RemoteFileAddress,
-  );
-
-  t.deepEqual(
-    parseRemoteFileAddress(
-      env,
-      "  owner/repo@ref/feature:path/to/codeql.yml  ",
-    ),
     {
-      owner: "owner",
+      input: "  owner/repo@ref/feature:path/to/codeql.yml  ",
-      repo: "repo",
+      expected: { ...expected, path: "path/to/codeql.yml", ref: "ref/feature" },
-      path: "path/to/codeql.yml",
+    },
-      ref: "ref/feature",
+  ];
-    } satisfies RemoteFileAddress,
+
-  );
+  for (const newFormatInput of newFormatInputs) {
+    const targetWithArgs = target.withArgs(newFormatInput.input);
+
+    // Should fail when the FF is not enabled.
+    await targetWithArgs
+      .withFeatures([])
+      .passes(async (fn) =>
+        t.throwsAsync(fn, { instanceOf: ConfigurationError }),
+      );
+
+    // And pass when the FF is enabled.
+    await targetWithArgs
+      .withFeatures([Feature.NewRemoteFileAddresses])
+      .passes(async (fn) => t.deepEqual(await fn(), newFormatInput.expected));
+  }
 });
 
 test("parseRemoteFileAddress accepts remote address without an owner", async (t) => {
-  const env = getTestEnv();
+  const target = callee(parseRemoteFileAddress);
+
+  const env = target.getState().env;
   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:path.yml"), {
+  const targetWithEnv = target.withEnv(env);
-    owner,
-    repo: "repo",
-    path: "path.yml",
-    ref: "ref",
-  } satisfies RemoteFileAddress);
 
-  t.deepEqual(parseRemoteFileAddress(env, "repo@ref"), {
+  const testCases: ParseRemoteFileAddressTest[] = [
-    owner,
+    {
-    repo: "repo",
+      input: "repo@ref:path.yml",
-    path: DEFAULT_CONFIG_FILE_NAME,
+      expected: {
-    ref: "ref",
+        owner,
-  } satisfies RemoteFileAddress);
+        repo: "repo",
+        path: "path.yml",
+        ref: "ref",
+      },
+    },
+    {
+      input: "repo@ref",
+      expected: {
+        owner,
+        repo: "repo",
+        path: DEFAULT_CONFIG_FILE_NAME,
+        ref: "ref",
+      },
+    },
+    {
+      input: "repo:path.yml",
+      expected: {
+        owner,
+        repo: "repo",
+        path: "path.yml",
+        ref: DEFAULT_CONFIG_FILE_REF,
+      },
+    },
+    {
+      input: "repo",
+      expected: {
+        owner,
+        repo: "repo",
+        path: DEFAULT_CONFIG_FILE_NAME,
+        ref: DEFAULT_CONFIG_FILE_REF,
+      },
+    },
+  ];
 
-  t.deepEqual(parseRemoteFileAddress(env, "repo:path.yml"), {
+  for (const testCase of testCases) {
-    owner,
+    const targetWithArgs = targetWithEnv.withArgs(testCase.input);
-    repo: "repo",
-    path: "path.yml",
-    ref: DEFAULT_CONFIG_FILE_REF,
-  } satisfies RemoteFileAddress);
 
-  t.deepEqual(parseRemoteFileAddress(env, "repo"), {
+    // Should fail when the FF is not enabled.
-    owner,
+    await targetWithArgs
-    repo: "repo",
+      .withFeatures([])
-    path: DEFAULT_CONFIG_FILE_NAME,
+      .passes(async (fn) =>
-    ref: DEFAULT_CONFIG_FILE_REF,
+        t.throwsAsync(fn, { instanceOf: ConfigurationError }),
-  } satisfies RemoteFileAddress);
+      );
+
+    // And pass when the FF is enabled.
+    await targetWithArgs
+      .withFeatures([Feature.NewRemoteFileAddresses])
+      .passes(async (fn) => t.deepEqual(await fn(), testCase.expected));
+  }
 });
 
 test("parseRemoteFileAddress throws for invalid `GITHUB_REPOSITORY`", async (t) => {
-  const env = getTestEnv();
+  const target = callee(parseRemoteFileAddress).withArgs("repo@ref");
+
+  const env = target.getState().env;
   const getRequired = sinon.stub(env, "getRequired");
   getRequired.withArgs(ActionsEnvVars.GITHUB_REPOSITORY).returns(`not-valid`);
 
-  t.throws(() => parseRemoteFileAddress(env, "repo@ref"), {
+  await target
-    instanceOf: Error,
+    .withEnv(env)
-  });
+    .withFeatures([Feature.NewRemoteFileAddresses])
+    .passes(async (fn) => t.throwsAsync(fn, { instanceOf: Error }));
+
+  t.assert(getRequired.calledOnceWith(ActionsEnvVars.GITHUB_REPOSITORY));
 });
 
 test("parseRemoteFileAddress accepts remote address without a path", async (t) => {
-  const env = getTestEnv();
+  const target = callee(parseRemoteFileAddress);
 
-  t.deepEqual(parseRemoteFileAddress(env, "owner/repo@ref"), {
+  const testCases: ParseRemoteFileAddressTest[] = [
-    owner: "owner",
+    {
-    repo: "repo",
+      input: "owner/repo@ref",
-    path: DEFAULT_CONFIG_FILE_NAME,
+      expected: {
-    ref: "ref",
+        owner: "owner",
-  } satisfies RemoteFileAddress);
+        repo: "repo",
+        path: DEFAULT_CONFIG_FILE_NAME,
+        ref: "ref",
+      },
+    },
+    {
+      input: "owner/repo",
+      expected: {
+        owner: "owner",
+        repo: "repo",
+        path: DEFAULT_CONFIG_FILE_NAME,
+        ref: DEFAULT_CONFIG_FILE_REF,
+      },
+    },
+  ];
 
-  t.deepEqual(parseRemoteFileAddress(env, "owner/repo"), {
+  for (const testCase of testCases) {
-    owner: "owner",
+    const targetWithArgs = target.withArgs(testCase.input);
-    repo: "repo",
+
-    path: DEFAULT_CONFIG_FILE_NAME,
+    // Should fail when the FF is not enabled.
-    ref: DEFAULT_CONFIG_FILE_REF,
+    await targetWithArgs
-  } satisfies RemoteFileAddress);
+      .withFeatures([])
+      .passes(async (fn) =>
+        t.throwsAsync(fn, { instanceOf: ConfigurationError }),
+      );
+
+    // And pass when the FF is enabled.
+    await targetWithArgs
+      .withFeatures([Feature.NewRemoteFileAddresses])
+      .passes(async (fn) => t.deepEqual(await fn(), testCase.expected));
+  }
 });
 
 test("parseRemoteFileAddress accepts remote address without a ref", async (t) => {
-  const env = getTestEnv();
+  const target = callee(parseRemoteFileAddress).withArgs("owner/repo:path");
 
-  t.deepEqual(parseRemoteFileAddress(env, "owner/repo:path"), {
+  // Should only accept the input if the FF is enabled.
-    owner: "owner",
+  await target.withFeatures([]).passes(t.throwsAsync);
-    repo: "repo",
+  await target
-    path: "path",
+    .withFeatures([Feature.NewRemoteFileAddresses])
-    ref: DEFAULT_CONFIG_FILE_REF,
+    .passes(async (fn) =>
-  } satisfies RemoteFileAddress);
+      t.deepEqual(await fn(), {
+        owner: "owner",
+        repo: "repo",
+        path: "path",
+        ref: DEFAULT_CONFIG_FILE_REF,
+      } satisfies RemoteFileAddress),
+    );
 });
 
 test("parseRemoteFileAddress rejects invalid values", async (t) => {
@@ -246,49 +238,45 @@ test("parseRemoteFileAddress rejects invalid values", async (t) => {
     .withArgs(ActionsEnvVars.GITHUB_REPOSITORY)
     .returns(`${owner}/current-repo`);
 
-  t.throws(() => parseRemoteFileAddress(env, "  "), {
+  const target = callee(parseRemoteFileAddress).withEnv(env);
-    instanceOf: ConfigurationError,
+
-  });
+  const testInputs = [
-  t.throws(() => parseRemoteFileAddress(env, "repo//absolute"), {
+    "  ",
-    instanceOf: ConfigurationError,
+    "repo//absolute",
-  });
+    "repo:/absolute",
-  t.throws(() => parseRemoteFileAddress(env, "repo:/absolute"), {
+    "/repo@ref",
-    instanceOf: ConfigurationError,
+    "   /repo@ref",
-  });
+    "repo@",
-  t.throws(() => parseRemoteFileAddress(env, "/repo@ref"), {
+    "repo:",
-    instanceOf: ConfigurationError,
+    "repo/",
-  });
+    "/repo",
-  t.throws(() => parseRemoteFileAddress(env, "   /repo@ref"), {
+    ":path",
-    instanceOf: ConfigurationError,
+    "@ref",
-  });
+    "@ref:path",
-  t.throws(() => parseRemoteFileAddress(env, "repo@"), {
+    "owner/@ref:path",
-    instanceOf: ConfigurationError,
+    "owner/@ref",
-  });
+    "owner/:path",
-  t.throws(() => parseRemoteFileAddress(env, "repo:"), {
+  ];
-    instanceOf: ConfigurationError,
+
-  });
+  for (const testInput of testInputs) {
-  t.throws(() => parseRemoteFileAddress(env, "repo/"), {
+    const targetWithArgs = target.withArgs(testInput);
-    instanceOf: ConfigurationError,
+
-  });
+    // Should throw both when the new format is and isn't accepted.
-  t.throws(() => parseRemoteFileAddress(env, "/repo"), {
+    await targetWithArgs.withFeatures([]).passes(async (fn) =>
-    instanceOf: ConfigurationError,
+      t.throwsAsync(fn, {
-  });
+        instanceOf: ConfigurationError,
-  t.throws(() => parseRemoteFileAddress(env, ":path"), {
+        message: errors.getConfigFileRepoOldFormatInvalidMessage(testInput),
-    instanceOf: ConfigurationError,
+      }),
-  });
+    );
-  t.throws(() => parseRemoteFileAddress(env, "@ref"), {
+    await targetWithArgs
-    instanceOf: ConfigurationError,
+      .withFeatures([Feature.NewRemoteFileAddresses])
-  });
+      .passes(async (fn) =>
-  t.throws(() => parseRemoteFileAddress(env, "@ref:path"), {
+        t.throwsAsync(fn, {
-    instanceOf: ConfigurationError,
+          // When the new format is accepted, there are some more specific
-  });
+          // errors in some cases. It is sufficient for us to check that
-  t.throws(() => parseRemoteFileAddress(env, "owner/@ref:path"), {
+          // an exception is thrown.
-    instanceOf: ConfigurationError,
+          instanceOf: ConfigurationError,
-  });
+        }),
-  t.throws(() => parseRemoteFileAddress(env, "owner/@ref"), {
+      );
-    instanceOf: ConfigurationError,
+  }
-  });
-  t.throws(() => parseRemoteFileAddress(env, "owner/:path"), {
-    instanceOf: ConfigurationError,
-  });
 });

src/config/remote-file.ts

@@ -1,6 +1,8 @@
+import { ActionState } from "../action-common";
 import { ActionsEnvVars } from "../actions-util";
 import { Env } from "../environment";
 import * as errorMessages from "../error-messages";
+import { Feature } from "../feature-flags";
 import { ConfigurationError, Failure, Result, Success } from "../util";
 
 /** Represents remote file addresses. */
@@ -72,15 +74,15 @@ function parseOldRemoteFileAddress(
 /**
  * Attempts to parse `configFile` into an array of `RemoteFileAddress` components.
  *
- * @param env The current environment variables.
+ * @param actionState The current Action state.
  * @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(
+export async function parseRemoteFileAddress(
-  env: Env,
+  actionState: ActionState,
   configFile: string,
-): RemoteFileAddress {
+): Promise<RemoteFileAddress> {
   // Try to parse the input using the old format. If successful, return the
   // resulting `RemoteFileAddress`. Otherwise, continue using the new format.
   const oldFormatAddressResult = parseOldRemoteFileAddress(configFile);
@@ -89,6 +91,16 @@ export function parseRemoteFileAddress(
     return oldFormatAddressResult.value;
   }
 
+  // If the FF for the new format is not enabled, throw the old format error.
+  const allowNewFormat = await actionState.features.getValue(
+    Feature.NewRemoteFileAddresses,
+  );
+  if (!allowNewFormat) {
+    throw new ConfigurationError(
+      errorMessages.getConfigFileRepoOldFormatInvalidMessage(configFile),
+    );
+  }
+
   // retrieve the various parts of the config location, and ensure they're present
   const format = new RegExp(
     "^((?<owner>[^:@/]+)/)?(?<repo>[^:@/]+)(@(?<ref>[^:]+))?(:(?<path>.+))?$",
@@ -119,7 +131,7 @@ export function parseRemoteFileAddress(
   }
 
   return {
-    owner: owner || getDefaultOwner(env),
+    owner: owner || getDefaultOwner(actionState.env),
     repo,
     path: path || DEFAULT_CONFIG_FILE_NAME,
     ref: ref || DEFAULT_CONFIG_FILE_REF,

src/error-messages.ts

@@ -30,6 +30,15 @@ export function getInvalidConfigFileMessage(
   return `The configuration file "${configFile}" is invalid: ${messages.slice(0, 10).join(", ")}${andMore}`;
 }
 
+export function getConfigFileRepoOldFormatInvalidMessage(
+  configFile: string,
+): string {
+  let error = `The configuration file "${configFile}" is not a supported remote file reference.`;
+  error += " Expected format <owner>/<repository>/<file-path>@<ref>";
+
+  return error;
+}
+
 export function getConfigFileRepoFormatInvalidMessage(
   configFile: string,
 ): string {