Add expliciit prefix for remote file addresses

Author: mbg

lib/entry-points.js

@@ -41,6 +41,7 @@ import {
   initAllState,
   callee,
   SAMPLE_DOTCOM_API_DETAILS,
+  AssertableTarget,
 } from "./testing-utils";
 import {
   GitHubVariant,
@@ -2605,18 +2606,104 @@ test.serial(
     await withTmpDir(async (tmpDir) => {
       const getRemoteConfig = sinon.stub(file, "getRemoteConfig").resolves({});
 
-      const remoteAddress = "repo:file";
-      await callee(configUtils.loadUserConfig)
-        .withArgs(remoteAddress, tmpDir, SAMPLE_DOTCOM_API_DETAILS, tmpDir)
-        .passes(t.deepEqual, {});
+      // Construct the basic test target.
+      const target = callee(configUtils.loadUserConfig)
+        .withDefaultActionsEnv()
+        .withFeatures([Feature.NewRemoteFileAddresses]);
+
+      // Utility function to assert that `targetWithArgs` has identified
+      // the input as a remote file address.
+      const checkIsRemote =
+        (address: string) =>
+        async <R>(targetWithArgs: AssertableTarget<R>) => {
+          // We have stubbed `getRemoteConfig` to resolve to `{}`, so we
+          // expect that result.
+          await targetWithArgs.passes(t.deepEqual, {});
+
+          // And `getRemoteConfig` should have been called exactly once.
+          t.is(getRemoteConfig.callCount, 1);
+
+          // Get the arguments for the call and check that there were three.
+          // We don't care about the first, but check that the other two
+          // match our expectations. We break it down like this to get
+          // more useful test output.
+          const args = getRemoteConfig.getCalls()[0].args;
+          t.is(args.length, 3);
+          t.deepEqual(args[1], address);
+          t.deepEqual(args[2], SAMPLE_DOTCOM_API_DETAILS);
+        };
 
-      t.true(
-        getRemoteConfig.calledOnceWithExactly(
-          sinon.match.any,
-          remoteAddress,
+      // Utility function to assert that `targetWithArgs` has not identified
+      // the input as a remote file address.
+      const checkIsNotRemote = async <R>(
+        targetWithArgs: AssertableTarget<R>,
+      ) => {
+        // We expect `loadUserConfig` to have thrown if it thinks the path is local,
+        // since the inputs we provide aren't for files that exist.
+        await targetWithArgs.throws(t);
+
+        // Additionally, we expect that `getRemoteConfig` wasn't called.
+        t.is(getRemoteConfig.callCount, 0);
+      };
+
+      // Utility function to add the explicit `REMOTE_PATH_PREFIX` to the input.
+      const withExplicitPrefix = (str: string) =>
+        `${file.REMOTE_PATH_PREFIX}${str}`;
+
+      // Utility to set up a call to `loadUserConfig` with the provided `address`
+      // and pass it to `assertion`.
+      const testTargetWith = async (
+        address: string,
+        assertion: (
+          targetWithArgs: AssertableTarget<Promise<UserConfig>>,
+        ) => Promise<any>,
+      ) => {
+        // Reset the stub's history since we re-use it.
+        getRemoteConfig.resetHistory();
+
+        // Log the input we are testing so that, in the event of a failure,
+        // it is easier to see which input was responsible.
+        t.log(`testTargetWith("${address}")`);
+
+        // Prepare the test call to `loadUserConfig`.
+        const targetWithArgs = target.withArgs(
+          address,
+          tmpDir,
           SAMPLE_DOTCOM_API_DETAILS,
-        ),
+          tmpDir,
+        );
+
+        // Pass it to the provided assertion function.
+        await assertion(targetWithArgs);
+      };
+
+      // Since this input contains an '@' character, it is treated as a remote path
+      // by the old logic even without the explicit prefix.
+      const remoteWithoutPrefix = "repo@main";
+      await testTargetWith(
+        remoteWithoutPrefix,
+        checkIsRemote(remoteWithoutPrefix),
+      );
+      await testTargetWith(
+        withExplicitPrefix(remoteWithoutPrefix),
+        checkIsRemote(remoteWithoutPrefix),
       );
+      // It is only treated as a local path with the corresponding prefix.
+      await testTargetWith(`./${remoteWithoutPrefix}`, checkIsNotRemote);
+
+      // The following test inputs are examples of ambiguous paths. They could refer to
+      // valid local or remote paths. For each, we check that they are treated as remote
+      // paths if the explicit remote file prefix is used and as local paths otherwise.
+      const testInputs = ["repo:file", "input", "../input"];
+
+      for (const testInput of testInputs) {
+        for (const addPrefix of [true, false]) {
+          await testTargetWith(
+            addPrefix ? withExplicitPrefix(testInput) : testInput,
+            addPrefix ? checkIsRemote(testInput) : checkIsNotRemote,
+          );
+        }
+      }
     });
   },
 );

src/config-utils.test.ts

@@ -31,7 +31,11 @@ import {
   parseUserConfig,
   UserConfig,
 } from "./config/db-config";
-import { getRemoteConfig, LOCAL_PATH_PREFIX } from "./config/file";
+import {
+  getRemoteConfig,
+  LOCAL_PATH_PREFIX,
+  REMOTE_PATH_PREFIX,
+} from "./config/file";
 import {
   parseRegistries,
   type RegistryConfigNoCredentials,
@@ -501,6 +505,12 @@ export async function loadUserConfig(
     );
     return getLocalConfig(actionState.logger, configFile, validateConfig);
   } else {
+    // Drop the explicit prefix if it is present. Since `REMOTE_PATH_PREFIX` is chosen
+    // to not conflict with permissable characters in "owner" or "repo" components,
+    // this does not risk removing valid parts of either component by accident.
+    if (isRemotePath(configFile)) {
+      configFile = configFile.substring(REMOTE_PATH_PREFIX.length);
+    }
     return await getRemoteConfig(actionState, configFile, apiDetails);
   }
 }
@@ -1287,6 +1297,16 @@ function isRelativePath(configPath: string): boolean {
   return configPath.startsWith(LOCAL_PATH_PREFIX);
 }
 
+/**
+ * Determines if `configPath` starts with the prefix used to explicitly mark a path
+ * as a remote path (`REMOTE_PATH_PREFIX`).
+ *
+ * @param configPath The path to test.
+ */
+function isRemotePath(configPath: string): boolean {
+  return configPath.startsWith(REMOTE_PATH_PREFIX);
+}
+
 /**
  * Determines if `configPath` contains a '@' character.
  *
@@ -1298,8 +1318,6 @@ function containsAtRef(configPath: string): boolean {
 
 /**
  * Determines if `configPath` refers to a local configuration file.
- * This assumes the `OLD_REMOTE_ADDRESS_FORMAT` which must contain a '@'
- * character for remote addresses.
  *
  * @param configPath The path to test.
  * @returns True if it is local, or false otherwise.
@@ -1311,8 +1329,15 @@ function isLocal(configPath: string): boolean {
   if (isRelativePath(configPath)) {
     return true;
   }
+  // If the path starts with `REMOTE_PATH_PREFIX`, it is explicitly remote.
+  // This allows users to resolve ambiguity by specifying `REMOTE_PATH_PREFIX`.
+  if (isRemotePath(configPath)) {
+    return false;
+  }
 
   // Otherwise, the path is also local if it does not contain '@'.
+  // This assumes the `OLD_REMOTE_ADDRESS_FORMAT` which must contain a '@'
+  // character for remote addresses.
   return !containsAtRef(configPath);
 }
 

src/config-utils.ts

@@ -16,6 +16,14 @@ import { parseRemoteFileAddress } from "./remote-file";
  */
 export const LOCAL_PATH_PREFIX = "./";
 
+/**
+ * The prefix that can be specified to indicate that a path should be treated as a remote file address.
+ * The new remote file address format must start with either an owner or repository name. Both
+ * are restricted to ASCII characters, '.', and '-'. The prefix chosen here does not interfere with
+ * those and is _unlikely_ (but not impossible) to appear in a local file path.
+ */
+export const REMOTE_PATH_PREFIX = "::";
+
 /**
  * Gets the value that is configured for the configuration file, if any.
  */