diff --git a/lib/entry-points.js b/lib/entry-points.js index 2d781776f..7f11d8a40 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -149234,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.existsSync(localFilePath)) { + 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 */ ); @@ -149720,11 +149742,6 @@ function isLocal(configPath) { return !containsAtRef(configPath); } function getLocalConfig(logger, configFile, validateConfig) { - if (!fs9.existsSync(configFile)) { - throw new ConfigurationError( - getConfigFileDoesNotExistErrorMessage(configFile) - ); - } return parseUserConfig( logger, configFile, diff --git a/src/config-utils.test.ts b/src/config-utils.test.ts index 45d5ecfe0..ff0d7a46d 100644 --- a/src/config-utils.test.ts +++ b/src/config-utils.test.ts @@ -41,6 +41,7 @@ import { initAllState, callee, SAMPLE_DOTCOM_API_DETAILS, + setupBaseActionsVars, } from "./testing-utils"; import { GitHubVariant, @@ -443,6 +444,7 @@ test.serial("load non-existent input", async (t) => { try { const state = initAllState(); + setupBaseActionsVars({}, state.env); await configUtils.initConfig( state, createTestInitConfigInputs({ @@ -2604,16 +2606,51 @@ test.serial( async (t) => { await withTmpDir(async (tmpDir) => { const getRemoteConfig = sinon.stub(file, "getRemoteConfig").resolves({}); + const target = callee(configUtils.loadUserConfig) + .withDefaultActionsEnv() + .withFeatures([Feature.NewRemoteFileAddresses]); - const remoteAddress = "repo:file"; - await callee(configUtils.loadUserConfig) - .withArgs(remoteAddress, tmpDir, SAMPLE_DOTCOM_API_DETAILS, tmpDir) - .passes(t.deepEqual, {}); + const testTargetWith = async (remoteAddress: string) => { + getRemoteConfig.resetHistory(); - t.true( + 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.false( getRemoteConfig.calledOnceWithExactly( sinon.match.any, - remoteAddress, + explicitlyLocalAddress, SAMPLE_DOTCOM_API_DETAILS, ), ); diff --git a/src/config-utils.ts b/src/config-utils.ts index 737bea8bd..30a328a66 100644 --- a/src/config-utils.ts +++ b/src/config-utils.ts @@ -485,17 +485,60 @@ export async function loadUserConfig( apiDetails: api.GitHubApiCombinedDetails, tempDir: string, ): Promise { - 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.existsSync(localFilePath)) { + 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, ); @@ -1321,13 +1364,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,