Stat path to check it is a file

If it does not contain a `@` character.

lib/entry-points.js

@@ -148296,6 +148296,25 @@ function parseOldRemoteFileAddress(input) {
     ref: pieces.groups.ref.trim()
   });
 }
+function parseNewRemoteFileAddress(env, configFile) {
+  const format = new RegExp(
+    "^((?<owner>[^:@/]+)/)?(?<repo>[^:@/]+)(@(?<ref>[^:]+))?(:(?<path>.+))?$"
+  );
+  const pieces = format.exec(configFile.trim());
+  const repo = pieces?.groups?.repo?.trim();
+  if (!pieces?.groups || !repo || repo.length === 0) {
+    return new Failure(void 0);
+  }
+  const owner = pieces.groups.owner?.trim();
+  const path29 = pieces.groups.path?.trim();
+  const ref = pieces.groups.ref?.trim();
+  return new Success({
+    owner: owner || getDefaultOwner(env),
+    repo,
+    path: path29 || DEFAULT_CONFIG_FILE_NAME,
+    ref: ref || DEFAULT_CONFIG_FILE_REF
+  });
+}
 async function parseRemoteFileAddress(actionState, configFile) {
   const oldFormatAddressResult = parseOldRemoteFileAddress(configFile);
   if (oldFormatAddressResult.isSuccess()) {
@@ -148309,30 +148328,22 @@ async function parseRemoteFileAddress(actionState, configFile) {
       getConfigFileRepoOldFormatInvalidMessage(configFile)
     );
   }
-  const format = new RegExp(
-    "^((?<owner>[^:@/]+)/)?(?<repo>[^:@/]+)(@(?<ref>[^:]+))?(:(?<path>.+))?$"
+  const newFormatAddressResult = parseNewRemoteFileAddress(
+    actionState.env,
+    configFile
   );
-  const pieces = format.exec(configFile.trim());
-  const repo = pieces?.groups?.repo?.trim();
-  if (!pieces?.groups || !repo || repo.length === 0) {
+  if (newFormatAddressResult.isFailure()) {
     throw new ConfigurationError(
       getConfigFileRepoFormatInvalidMessage(configFile)
     );
   }
-  const owner = pieces.groups.owner?.trim();
-  const path29 = pieces.groups.path?.trim();
-  const ref = pieces.groups.ref?.trim();
-  if (path29?.startsWith("/")) {
+  const address = newFormatAddressResult.value;
+  if (address.path.startsWith("/")) {
     throw new ConfigurationError(
       `The path component of '${configFile}' cannot be an absolute path.`
     );
   }
-  return {
-    owner: owner || getDefaultOwner(actionState.env),
-    repo,
-    path: path29 || DEFAULT_CONFIG_FILE_NAME,
-    ref: ref || DEFAULT_CONFIG_FILE_REF
-  };
+  return address;
 }
 
 // src/config/file.ts
@@ -149223,15 +149234,37 @@ async function downloadCacheWithTime(codeQL, languages, logger) {
   return { trapCaches, trapCacheDownloadTime };
 }
 async function loadUserConfig(actionState, configFile, workspacePath, apiDetails, tempDir) {
-  if (isLocal(configFile)) {
-    if (configFile !== userConfigFromActionPath(tempDir)) {
-      configFile = path10.resolve(workspacePath, configFile);
-      if (!(configFile + path10.sep).startsWith(workspacePath + path10.sep)) {
-        throw new ConfigurationError(
-          getConfigFileOutsideWorkspaceErrorMessage(configFile)
-        );
+  const generatedConfigFile = userConfigFromActionPath(tempDir);
+  const isWorkspaceRelative = (fp) => (fp + path10.sep).startsWith(workspacePath + path10.sep);
+  let localFile = false;
+  if (configFile === generatedConfigFile) {
+    localFile = true;
+  } else if (isLocal(configFile)) {
+    const localFilePath = path10.resolve(workspacePath, configFile);
+    const workspaceRelative = isWorkspaceRelative(localFilePath);
+    if (workspaceRelative && fs9.statSync(localFilePath, { throwIfNoEntry: false })?.isFile()) {
+      configFile = localFilePath;
+      localFile = true;
+    } else {
+      const allowNewFormat = await actionState.features.getValue(
+        "new_remote_file_addresses" /* NewRemoteFileAddresses */
+      );
+      if (isRelativePath(configFile) || !allowNewFormat) {
+        if (workspaceRelative) {
+          throw new ConfigurationError(
+            getConfigFileDoesNotExistErrorMessage(localFilePath)
+          );
+        } else {
+          throw new ConfigurationError(
+            getConfigFileOutsideWorkspaceErrorMessage(
+              localFilePath
+            )
+          );
+        }
       }
     }
+  }
+  if (localFile) {
     const validateConfig = await actionState.features.getValue(
       "validate_db_config" /* ValidateDbConfig */
     );
@@ -149696,18 +149729,19 @@ async function initConfig(actionState, inputs) {
   await setCppTrapCachingEnvironmentVariables(config, logger);
   return config;
 }
+function isRelativePath(configPath) {
+  return configPath.startsWith("./");
+}
+function containsAtRef(configPath) {
+  return configPath.includes("@");
+}
 function isLocal(configPath) {
-  if (configPath.indexOf("./") === 0) {
+  if (isRelativePath(configPath)) {
     return true;
   }
-  return configPath.indexOf("@") === -1;
+  return !containsAtRef(configPath);
 }
 function getLocalConfig(logger, configFile, validateConfig) {
-  if (!fs9.existsSync(configFile)) {
-    throw new ConfigurationError(
-      getConfigFileDoesNotExistErrorMessage(configFile)
-    );
-  }
   return parseUserConfig(
     logger,
     configFile,

src/config-utils.test.ts

@@ -6,12 +6,14 @@ import test, { ExecutionContext } from "ava";
 import * as yaml from "js-yaml";
 import * as sinon from "sinon";
 
+import { ActionState } from "./action-common";
 import * as actionsUtil from "./actions-util";
 import { AnalysisKind, supportedAnalysisKinds } from "./analyses";
 import * as api from "./api-client";
 import { CachingKind } from "./caching-utils";
 import { createStubCodeQL } from "./codeql";
 import { UserConfig } from "./config/db-config";
+import * as file from "./config/file";
 import * as configUtils from "./config-utils";
 import * as errorMessages from "./error-messages";
 import { Feature } from "./feature-flags";
@@ -38,6 +40,8 @@ import {
   makeMacro,
   initAllState,
   callee,
+  SAMPLE_DOTCOM_API_DETAILS,
+  setupBaseActionsVars,
 } from "./testing-utils";
 import {
   GitHubVariant,
@@ -440,6 +444,7 @@ test.serial("load non-existent input", async (t) => {
 
     try {
       const state = initAllState();
+      setupBaseActionsVars({}, state.env);
       await configUtils.initConfig(
         state,
         createTestInitConfigInputs({
@@ -2529,3 +2534,120 @@ test("determineUserConfig - ignores config file input outside Default Setup if F
       });
   });
 });
+
+test("loadUserConfig - loads local configuration files", async (t) => {
+  await withTmpDir(async (workspaceDir) => {
+    await withTmpDir(async (tmpDir) => {
+      // Construct the test target.
+      const loadUserConfig = (
+        actionState: ActionState<["Logger", "Env", "FeatureFlags"]>,
+        filePath: string,
+      ) =>
+        configUtils.loadUserConfig(
+          actionState,
+          filePath,
+          workspaceDir,
+          SAMPLE_DOTCOM_API_DETAILS,
+          tmpDir,
+        );
+      const target = callee(loadUserConfig);
+
+      // `loadUserConfig` should load local configuration files if they are inside the workspace:
+      const insideOfWorkspace = path.join(workspaceDir, "some-file.yml");
+      fs.writeFileSync(insideOfWorkspace, "test-key: present", "utf8");
+
+      await target
+        .withArgs(insideOfWorkspace)
+        .passes(t.deepEqual, { "test-key": "present" });
+
+      // `loadUserConfig` should normally throw if the path is outside of the workspace:
+      const outsideOfWorkspace = path.join(
+        tmpDir,
+        "not-the-generated-file.yml",
+      );
+      fs.writeFileSync(outsideOfWorkspace, "test-key: present", "utf8");
+
+      await target
+        .withArgs(outsideOfWorkspace)
+        .throws(t, { instanceOf: ConfigurationError });
+
+      // `loadUserConfig` does not throw if the path is the result of `userConfigFromActionPath`:
+      const generatedPath = configUtils.userConfigFromActionPath(tmpDir);
+      fs.writeFileSync(generatedPath, "test-key: present", "utf8");
+
+      await target
+        .withArgs(generatedPath)
+        .passes(t.deepEqual, { "test-key": "present" });
+    });
+  });
+});
+
+test.serial("loadUserConfig - loads remote configuration files", async (t) => {
+  await withTmpDir(async (tmpDir) => {
+    const getRemoteConfig = sinon.stub(file, "getRemoteConfig").resolves({});
+
+    const remoteAddress = "owner/repo/file@ref";
+    await callee(configUtils.loadUserConfig)
+      .withArgs(remoteAddress, tmpDir, SAMPLE_DOTCOM_API_DETAILS, tmpDir)
+      .passes(t.deepEqual, {});
+
+    t.true(
+      getRemoteConfig.calledOnceWithExactly(
+        sinon.match.any,
+        remoteAddress,
+        SAMPLE_DOTCOM_API_DETAILS,
+      ),
+    );
+  });
+});
+
+test.serial(
+  "loadUserConfig - loads remote configuration files (new format, partial)",
+  async (t) => {
+    await withTmpDir(async (tmpDir) => {
+      const getRemoteConfig = sinon.stub(file, "getRemoteConfig").resolves({});
+      const target = callee(configUtils.loadUserConfig)
+        .withDefaultActionsEnv()
+        .withFeatures([Feature.NewRemoteFileAddresses]);
+
+      const testTargetWith = async (remoteAddress: string) => {
+        getRemoteConfig.resetHistory();
+
+        await target
+          .withArgs(remoteAddress, tmpDir, SAMPLE_DOTCOM_API_DETAILS, tmpDir)
+          .passes(t.deepEqual, {});
+
+        t.true(
+          getRemoteConfig.calledOnceWithExactly(
+            sinon.match.any,
+            remoteAddress,
+            SAMPLE_DOTCOM_API_DETAILS,
+          ),
+        );
+      };
+
+      // When the new format FF is enabled, all inputs that don't explicitly refer
+      // to a local file that can be found will be tried as remote addresses.
+      await testTargetWith("repo:file");
+      await testTargetWith("input");
+      await testTargetWith("../input");
+      await testTargetWith("repo@main");
+
+      // An explicitly local path can still override this.
+      const explicitlyLocalAddress = "./repo@main";
+
+      getRemoteConfig.resetHistory();
+
+      await target
+        .withArgs(
+          explicitlyLocalAddress,
+          tmpDir,
+          SAMPLE_DOTCOM_API_DETAILS,
+          tmpDir,
+        )
+        .throws(t);
+
+      t.is(getRemoteConfig.callCount, 0);
+    });
+  },
+);

src/config-utils.ts

@@ -478,24 +478,70 @@ async function downloadCacheWithTime(
  * @param tempDir The temporary directory which may contain a CodeQL Action-generated configuration file.
  * @returns The loaded configuration file, if successful.
  */
-async function loadUserConfig(
+export async function loadUserConfig(
   actionState: ActionState<["Logger", "Env", "FeatureFlags"]>,
   configFile: string,
   workspacePath: string,
   apiDetails: api.GitHubApiCombinedDetails,
   tempDir: string,
 ): Promise<UserConfig> {
-  if (isLocal(configFile)) {
-    if (configFile !== userConfigFromActionPath(tempDir)) {
-      // If the config file is not generated by the Action, it should be relative to the workspace.
-      configFile = path.resolve(workspacePath, configFile);
-      // Error if the config file is now outside of the workspace
-      if (!(configFile + path.sep).startsWith(workspacePath + path.sep)) {
-        throw new ConfigurationError(
-          errorMessages.getConfigFileOutsideWorkspaceErrorMessage(configFile),
-        );
+  // Compute the path of the generated configuration file.
+  const generatedConfigFile = userConfigFromActionPath(tempDir);
+
+  // A helper function to decide if the given path is relative to the workspacePath.
+  const isWorkspaceRelative = (fp: string) =>
+    (fp + path.sep).startsWith(workspacePath + path.sep);
+
+  // We must decide if `configFile` refers to a local or remote file. Using the old remote file
+  // address format, an address referred to a remote file if it contained '@' somewhere. The
+  // new format makes all components, except for the repo name optional, and (in theory) a
+  // repo name could be something like `foo.yml`. That creates ambiguity over whether a
+  // file is local or not.
+  let localFile = false;
+
+  if (configFile === generatedConfigFile) {
+    // The generated file is always local.
+    localFile = true;
+  } else if (isLocal(configFile)) {
+    // If the config file is not generated by the Action, it should be relative to the workspace.
+    const localFilePath = path.resolve(workspacePath, configFile);
+
+    // Determine whether the resulting path is relative to the workspace.
+    const workspaceRelative = isWorkspaceRelative(localFilePath);
+
+    // If the path is relative to the workspace and the file exists, then we use it.
+    if (
+      workspaceRelative &&
+      fs.statSync(localFilePath, { throwIfNoEntry: false })?.isFile()
+    ) {
+      configFile = localFilePath;
+      localFile = true;
+    } else {
+      const allowNewFormat = await actionState.features.getValue(
+        Feature.NewRemoteFileAddresses,
+      );
+
+      // If the FF for the new format is not enabled or the input path is explicitly local,
+      // throw the old errors depending on whether the file is outside of the workspace or not.
+      // Otherwise, we assume the path refers to a remote file.
+      if (isRelativePath(configFile) || !allowNewFormat) {
+        if (workspaceRelative) {
+          throw new ConfigurationError(
+            errorMessages.getConfigFileDoesNotExistErrorMessage(localFilePath),
+          );
+        } else {
+          throw new ConfigurationError(
+            errorMessages.getConfigFileOutsideWorkspaceErrorMessage(
+              localFilePath,
+            ),
+          );
+        }
       }
     }
+  }
+
+  // Try to load the file according to whether it is local or not.
+  if (localFile) {
     const validateConfig = await actionState.features.getValue(
       Feature.ValidateDbConfig,
     );
@@ -1277,13 +1323,43 @@ export async function initConfig(
   return config;
 }
 
+/**
+ * Determines if `configPath` is explicitly local. That is, it starts with "./".
+ * A configuration file path that starts with "./" is always treated as a local path.
+ *
+ * @param configPath The path to test.
+ */
+function isRelativePath(configPath: string): boolean {
+  return configPath.startsWith("./");
+}
+
+/**
+ * Determines if `configPath` contains a '@' character.
+ *
+ * @param configPath The path to test.
+ */
+function containsAtRef(configPath: string): boolean {
+  return configPath.includes("@");
+}
+
+/**
+ * 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.
+ */
 function isLocal(configPath: string): boolean {
-  // If the path starts with ./, look locally
-  if (configPath.indexOf("./") === 0) {
+  // If the path starts with "./", it is explicitly local.
+  // This allows local paths that would otherwise contain '@'
+  // to be used with a "./" prefix.
+  if (isRelativePath(configPath)) {
     return true;
   }
 
-  return configPath.indexOf("@") === -1;
+  // Otherwise, the path is also local if it does not contain '@'.
+  return !containsAtRef(configPath);
 }
 
 export function getLocalConfig(
@@ -1291,13 +1367,6 @@ export function getLocalConfig(
   configFile: string,
   validateConfig: boolean,
 ): UserConfig {
-  // Error if the file does not exist
-  if (!fs.existsSync(configFile)) {
-    throw new ConfigurationError(
-      errorMessages.getConfigFileDoesNotExistErrorMessage(configFile),
-    );
-  }
-
   return parseUserConfig(
     logger,
     configFile,

src/config/remote-file.ts

@@ -1,5 +1,5 @@
 import { ActionState } from "../action-common";
-import { Env, ActionsEnvVars } from "../environment";
+import { ActionsEnvVars, ReadOnlyEnv } from "../environment";
 import * as errorMessages from "../error-messages";
 import { Feature } from "../feature-flags";
 import { ConfigurationError, Failure, Result, Success } from "../util";
@@ -23,7 +23,7 @@ export const DEFAULT_CONFIG_FILE_NAME = ".github/codeql-action.yaml";
 export const DEFAULT_CONFIG_FILE_REF = "main";
 
 /** Extracts the owner from the `GITHUB_REPOSITORY` environment variable. */
-function getDefaultOwner(env: Env): string {
+function getDefaultOwner(env: ReadOnlyEnv): string {
   const currentRepoNwo = env.getRequired(ActionsEnvVars.GITHUB_REPOSITORY);
   const nwoParts = currentRepoNwo.split("/");
 
@@ -70,6 +70,42 @@ function parseOldRemoteFileAddress(
   });
 }
 
+/**
+ * Attempts to parse `input` as a `RemoteFileAddress` using the new format.
+ *
+ * @param env The read-only environment to obtain the owner name from if needed.
+ * @param configFile The input to try and parse.
+ * @returns A `RemoteFileAddress` value if successful or `undefined` otherwise.
+ */
+export function parseNewRemoteFileAddress(
+  env: ReadOnlyEnv,
+  configFile: string,
+): Result<RemoteFileAddress, undefined> {
+  // retrieve the various parts of the config location, and ensure they're present
+  const format = new RegExp(
+    "^((?<owner>[^:@/]+)/)?(?<repo>[^:@/]+)(@(?<ref>[^:]+))?(:(?<path>.+))?$",
+  );
+  const pieces = format.exec(configFile.trim());
+
+  const repo: string | undefined = pieces?.groups?.repo?.trim();
+
+  // Check that the regular expression matched and that we have at least the repo name.
+  if (!pieces?.groups || !repo || repo.length === 0) {
+    return new Failure(undefined);
+  }
+
+  const owner: string | undefined = pieces.groups.owner?.trim();
+  const path: string | undefined = pieces.groups.path?.trim();
+  const ref: string | undefined = pieces.groups.ref?.trim();
+
+  return new Success({
+    owner: owner || getDefaultOwner(env),
+    repo,
+    path: path || DEFAULT_CONFIG_FILE_NAME,
+    ref: ref || DEFAULT_CONFIG_FILE_REF,
+  });
+}
+
 /**
  * Attempts to parse `configFile` into an array of `RemoteFileAddress` components.
  *
@@ -101,15 +137,12 @@ export async function parseRemoteFileAddress(
   }
 
   // retrieve the various parts of the config location, and ensure they're present
-  const format = new RegExp(
-    "^((?<owner>[^:@/]+)/)?(?<repo>[^:@/]+)(@(?<ref>[^:]+))?(:(?<path>.+))?$",
+  const newFormatAddressResult = parseNewRemoteFileAddress(
+    actionState.env,
+    configFile,
   );
-  const pieces = format.exec(configFile.trim());
 
-  const repo: string | undefined = pieces?.groups?.repo?.trim();
-
-  // Check that the regular expression matched and that we have at least the repo name.
-  if (!pieces?.groups || !repo || repo.length === 0) {
+  if (newFormatAddressResult.isFailure()) {
     // Neither the old format nor the new format worked. Throw an error that
     // explains the format we accept. We only mention the new format, since that's
     // what we want to be used going forward.
@@ -118,21 +151,14 @@ export async function parseRemoteFileAddress(
     );
   }
 
-  const owner: string | undefined = pieces.groups.owner?.trim();
-  const path: string | undefined = pieces.groups.path?.trim();
-  const ref: string | undefined = pieces.groups.ref?.trim();
+  const address = newFormatAddressResult.value;
 
   // Ensure that the path is a relative path.
-  if (path?.startsWith("/")) {
+  if (address.path.startsWith("/")) {
     throw new ConfigurationError(
       `The path component of '${configFile}' cannot be an absolute path.`,
     );
   }
 
-  return {
-    owner: owner || getDefaultOwner(actionState.env),
-    repo,
-    path: path || DEFAULT_CONFIG_FILE_NAME,
-    ref: ref || DEFAULT_CONFIG_FILE_REF,
-  };
+  return address;
 }