diff --git a/.vscode/tests.code-snippets b/.vscode/tests.code-snippets new file mode 100644 index 0000000000..3ee5ceb757 --- /dev/null +++ b/.vscode/tests.code-snippets @@ -0,0 +1,30 @@ +{ + // Place your codeql-action workspace snippets here. Each snippet is defined under a snippet name and has a scope, prefix, body and + // description. Add comma separated ids of the languages where the snippet is applicable in the scope field. If scope + // is left empty or omitted, the snippet gets applied to all languages. The prefix is what is + // used to trigger the snippet and the body will be expanded and inserted. Possible variables are: + // $1, $2 for tab stops, $0 for the final cursor position, and ${1:label}, ${2:another} for placeholders. + // Placeholders with the same ids are connected. + // Example: + // "Print to console": { + // "scope": "javascript,typescript", + // "prefix": "log", + // "body": [ + // "console.log('$1');", + // "$2" + // ], + // "description": "Log output to console" + // } + "Test Macro": { + "scope": "javascript, typescript", + "prefix": "testMacro", + "body": [ + "const ${1:nameMacro} = test.macro({", + " exec: async (t: ExecutionContext) => {},", + "", + " title: (providedTitle = \"\") => `${2:common title} - \\${providedTitle}`,", + "});", + ], + "description": "An Ava test macro", + }, +} diff --git a/lib/start-proxy-action.js b/lib/start-proxy-action.js index 6b88aef405..f88505a997 100644 --- a/lib/start-proxy-action.js +++ b/lib/start-proxy-action.js @@ -121817,7 +121817,10 @@ function getCredentials(logger, registrySecrets, registriesCredentials, language ); } } - if ((!hasUsername(authConfig) || !isDefined2(authConfig.username)) && isUsernamePassword(authConfig) && isDefined2(authConfig.password) && isPAT(authConfig.password) || isToken(authConfig) && isDefined2(authConfig.token) && isPAT(authConfig.token)) { + const noUsername = !hasUsername(authConfig) || !isDefined2(authConfig.username); + const passwordIsPAT = isUsernamePassword(authConfig) && isDefined2(authConfig.password) && isPAT(authConfig.password); + const tokenIsPAT = isToken(authConfig) && isDefined2(authConfig.token) && isPAT(authConfig.token); + if (noUsername && (passwordIsPAT || tokenIsPAT)) { logger.warning( `A ${e.type} private registry is configured for ${e.host || e.url} using a GitHub Personal Access Token (PAT), but no username was provided. This may not work correctly. When configuring a private registry using a PAT, select "Username and password" and enter the username of the user who generated the PAT.` ); diff --git a/src/start-proxy.test.ts b/src/start-proxy.test.ts index 755189c21d..38cdf06347 100644 --- a/src/start-proxy.test.ts +++ b/src/start-proxy.test.ts @@ -14,9 +14,9 @@ import * as startProxyExports from "./start-proxy"; import { parseLanguage } from "./start-proxy"; import * as statusReport from "./status-report"; import { + assertNotLogged, checkExpectedLogMessages, createFeatures, - getRecordingLogger, makeTestToken, RecordingLogger, setupTests, @@ -439,41 +439,156 @@ test("getCredentials accepts OIDC configurations", (t) => { t.assert(credentials.some((c) => startProxyExports.isJFrogConfig(c))); }); -test("getCredentials logs a warning when a PAT is used without a username", async (t) => { - const loggedMessages = []; - const logger = getRecordingLogger(loggedMessages); - const likelyWrongCredentials = toEncodedJSON([ +const testPATWarning = test.macro({ + exec: async ( + t: ExecutionContext, + credentials: startProxyExports.RawCredential[], + checkAccepted: ( + t: ExecutionContext, + logger: RecordingLogger, + results: startProxyExports.Credential[], + ) => void, + ) => { + const logger = new RecordingLogger(); + const likelyWrongCredentials = toEncodedJSON(credentials); + + const results = startProxyExports.getCredentials( + logger, + undefined, + likelyWrongCredentials, + undefined, + ); + + checkAccepted(t, logger, results); + }, + + title: (providedTitle = "") => + `getCredentials logs a warning when a PAT is used - ${providedTitle}`, +}); + +test( + "password without a username", + testPATWarning, + [ { type: "git_server", host: "https://github.com/", password: `ghp_${makeTestToken()}`, }, - ]); - - const results = startProxyExports.getCredentials( - logger, - undefined, - likelyWrongCredentials, - undefined, - ); + ], + (t, logger, results) => { + // The configurations should be accepted, despite the likely problem. + t.assert(results); + t.is(results.length, 1); + t.is(results[0].type, "git_server"); + t.is(results[0].host, "https://github.com/"); + + if (startProxyExports.isUsernamePassword(results[0])) { + t.assert(results[0].password?.startsWith("ghp_")); + } else { + t.fail("Expected a `UsernamePassword`-based credential."); + } + + // A warning should have been logged. + checkExpectedLogMessages(t, logger.messages, [ + "using a GitHub Personal Access Token (PAT), but no username was provided", + ]); + }, +); - // The configuration should be accepted, despite the likely problem. - t.assert(results); - t.is(results.length, 1); - t.is(results[0].type, "git_server"); - t.is(results[0].host, "https://github.com/"); +test( + "password with a username", + testPATWarning, + [ + { + type: "git_server", + host: "https://github.com/", + username: "someone", + password: `ghp_${makeTestToken()}`, + }, + ], + (t, logger, results) => { + // The configurations should be accepted, despite the likely problem. + t.assert(results); + t.is(results.length, 1); + t.is(results[0].type, "git_server"); + t.is(results[0].host, "https://github.com/"); + + if (startProxyExports.isUsernamePassword(results[0])) { + t.assert(results[0].password?.startsWith("ghp_")); + } else { + t.fail("Expected a `UsernamePassword`-based credential."); + } + + assertNotLogged( + t, + logger, + "using a GitHub Personal Access Token (PAT), but no username was provided", + ); + }, +); - if (startProxyExports.isUsernamePassword(results[0])) { - t.assert(results[0].password?.startsWith("ghp_")); - } else { - t.fail("Expected a `UsernamePassword`-based credential."); - } +test( + "token without a username", + testPATWarning, + [ + { + type: "git_server", + host: "https://github.com/", + token: `ghp_${makeTestToken()}`, + }, + ], + (t, logger, results) => { + // The configurations should be accepted, despite the likely problem. + t.assert(results); + t.is(results.length, 1); + t.is(results[0].type, "git_server"); + t.is(results[0].host, "https://github.com/"); + + if (startProxyExports.isToken(results[0])) { + t.assert(results[0].token?.startsWith("ghp_")); + } else { + t.fail("Expected a `Token`-based credential."); + } + + // A warning should have been logged. + checkExpectedLogMessages(t, logger.messages, [ + "using a GitHub Personal Access Token (PAT), but no username was provided", + ]); + }, +); - // A warning should have been logged. - checkExpectedLogMessages(t, loggedMessages, [ - "using a GitHub Personal Access Token (PAT), but no username was provided", - ]); -}); +test( + "token with a username", + testPATWarning, + [ + { + type: "git_server", + host: "https://github.com/", + username: "someone", + token: `ghp_${makeTestToken()}`, + }, + ], + (t, logger, results) => { + // The configurations should be accepted, despite the likely problem. + t.assert(results); + t.is(results.length, 1); + t.is(results[0].type, "git_server"); + t.is(results[0].host, "https://github.com/"); + + if (startProxyExports.isToken(results[0])) { + t.assert(results[0].token?.startsWith("ghp_")); + } else { + t.fail("Expected a `Token`-based credential."); + } + + assertNotLogged( + t, + logger, + "using a GitHub Personal Access Token (PAT), but no username was provided", + ); + }, +); test("getCredentials returns all credentials for Actions when using LANGUAGE_TO_REGISTRY_TYPE", async (t) => { const credentialsInput = toEncodedJSON(mixedCredentials); diff --git a/src/start-proxy.ts b/src/start-proxy.ts index 32762eae33..1a68d99cf3 100644 --- a/src/start-proxy.ts +++ b/src/start-proxy.ts @@ -447,15 +447,18 @@ export function getCredentials( } // If the password or token looks like a GitHub PAT, warn if no username is configured. - if ( - ((!hasUsername(authConfig) || !isDefined(authConfig.username)) && - isUsernamePassword(authConfig) && - isDefined(authConfig.password) && - isPAT(authConfig.password)) || - (isToken(authConfig) && - isDefined(authConfig.token) && - isPAT(authConfig.token)) - ) { + const noUsername = + !hasUsername(authConfig) || !isDefined(authConfig.username); + const passwordIsPAT = + isUsernamePassword(authConfig) && + isDefined(authConfig.password) && + isPAT(authConfig.password); + const tokenIsPAT = + isToken(authConfig) && + isDefined(authConfig.token) && + isPAT(authConfig.token); + + if (noUsername && (passwordIsPAT || tokenIsPAT)) { logger.warning( `A ${e.type} private registry is configured for ${e.host || e.url} using a GitHub Personal Access Token (PAT), but no username was provided. ` + `This may not work correctly. When configuring a private registry using a PAT, select "Username and password" and enter the username of the user ` + diff --git a/src/testing-utils.ts b/src/testing-utils.ts index 8a7cf8e2d9..d6fe86cb09 100644 --- a/src/testing-utils.ts +++ b/src/testing-utils.ts @@ -184,8 +184,8 @@ export interface LoggedMessage { export class RecordingLogger implements Logger { messages: LoggedMessage[] = []; - groups: string[] = []; - unfinishedGroups: Set = new Set(); + readonly groups: string[] = []; + readonly unfinishedGroups: Set = new Set(); private currentGroup: string | undefined = undefined; constructor(private readonly logToConsole: boolean = true) {} @@ -199,6 +199,19 @@ export class RecordingLogger implements Logger { } } + /** + * Checks whether the logged messages contain `messageOrRegExp`. + * + * If `messageOrRegExp` is a string, this function returns true as long as + * `messageOrRegExp` appears as part of one of the `messages`. + * + * If `messageOrRegExp` is a regular expression, this function returns true as long as + * one of the `messages` matches `messageOrRegExp`. + */ + hasMessage(messageOrRegExp: string | RegExp): boolean { + return hasLoggedMessage(this.messages, messageOrRegExp); + } + isDebug() { return true; } @@ -237,41 +250,37 @@ export function getRecordingLogger( messages: LoggedMessage[], { logToConsole }: { logToConsole?: boolean } = { logToConsole: true }, ): Logger { - return { - debug: (message: string) => { - messages.push({ type: "debug", message }); - if (logToConsole) { - // eslint-disable-next-line no-console - console.debug(message); - } - }, - info: (message: string) => { - messages.push({ type: "info", message }); - if (logToConsole) { - // eslint-disable-next-line no-console - console.info(message); - } - }, - warning: (message: string | Error) => { - messages.push({ type: "warning", message }); - if (logToConsole) { - // eslint-disable-next-line no-console - console.warn(message); - } - }, - error: (message: string | Error) => { - messages.push({ type: "error", message }); - if (logToConsole) { - // eslint-disable-next-line no-console - console.error(message); - } - }, - isDebug: () => true, - startGroup: () => undefined, - endGroup: () => undefined, - }; + const logger = new RecordingLogger(logToConsole); + logger.messages = messages; + return logger; } +/** + * Checks whether `messages` contains `messageOrRegExp`. + * + * If `messageOrRegExp` is a string, this function returns true as long as + * `messageOrRegExp` appears as part of one of the `messages`. + * + * If `messageOrRegExp` is a regular expression, this function returns true as long as + * one of the `messages` matches `messageOrRegExp`. + */ +function hasLoggedMessage( + messages: LoggedMessage[], + messageOrRegExp: string | RegExp, +): boolean { + const check = (val: string) => + typeof messageOrRegExp === "string" + ? val.includes(messageOrRegExp) + : messageOrRegExp.test(val); + + return messages.some( + (msg) => typeof msg.message === "string" && check(msg.message), + ); +} + +/** + * Checks that `messages` contains all of `expectedMessages`. + */ export function checkExpectedLogMessages( t: ExecutionContext, messages: LoggedMessage[], @@ -280,13 +289,7 @@ export function checkExpectedLogMessages( const missingMessages: string[] = []; for (const expectedMessage of expectedMessages) { - if ( - !messages.some( - (msg) => - typeof msg.message === "string" && - msg.message.includes(expectedMessage), - ) - ) { + if (!hasLoggedMessage(messages, expectedMessage)) { missingMessages.push(expectedMessage); } } @@ -303,6 +306,20 @@ export function checkExpectedLogMessages( } } +/** + * Asserts that `message` should not have been logged to `logger`. + */ +export function assertNotLogged( + t: ExecutionContext, + logger: RecordingLogger, + message: string | RegExp, +) { + t.false( + logger.hasMessage(message), + `'${message}' should not have been logged, but was.`, + ); +} + /** * Initialises a recording logger and calls `body` with it. *