diff --git a/src/main.spec.ts b/src/main.spec.ts index 6f5978b..31577ba 100644 --- a/src/main.spec.ts +++ b/src/main.spec.ts @@ -57,6 +57,9 @@ describe("main", () => { let utilsLogInfoForBranchNameResult: MockInstance< typeof utils.logInfoForBranchNameResult >; + let utilsCreateDistinctIdRegexMock: MockInstance< + typeof utils.createDistinctIdRegex + >; // Return Dispatch let returnDispatchGetRunIdAndUrlMock: MockInstance< @@ -93,6 +96,7 @@ describe("main", () => { utils, "logInfoForBranchNameResult", ); + utilsCreateDistinctIdRegexMock = vi.spyOn(utils, "createDistinctIdRegex"); returnDispatchGetRunIdAndUrlMock = vi.spyOn( returnDispatch, @@ -114,6 +118,7 @@ describe("main", () => { }); it("should successfully complete", async () => { + const distinctIdRegex = new RegExp(testCfg.distinctId); const returnDispatchSuccessResult = { success: true, value: { @@ -123,6 +128,7 @@ describe("main", () => { } as const; utilsGetBranchNameMock.mockReturnValue(testBranch); + utilsCreateDistinctIdRegexMock.mockReturnValue(distinctIdRegex); returnDispatchGetWorkflowIdMock.mockResolvedValue(0); returnDispatchGetRunIdAndUrlMock.mockResolvedValue( returnDispatchSuccessResult, @@ -154,13 +160,17 @@ describe("main", () => { testBranch, testCfg.ref, ); + expect(utilsCreateDistinctIdRegexMock).toHaveBeenCalledOnce(); + expect(utilsCreateDistinctIdRegexMock).toHaveBeenCalledWith( + testCfg.distinctId, + ); // Get run ID expect(returnDispatchGetRunIdAndUrlMock).toHaveBeenCalledOnce(); expect(returnDispatchGetRunIdAndUrlMock).toHaveBeenCalledWith({ startTime: Date.now(), branch: testBranch, - distinctId: testCfg.distinctId, + distinctIdRegex: distinctIdRegex, workflow: testCfg.workflow, workflowId: 0, workflowTimeoutMs: testCfg.workflowTimeoutSeconds * 1000, diff --git a/src/main.ts b/src/main.ts index 4197e70..a3400f2 100644 --- a/src/main.ts +++ b/src/main.ts @@ -8,7 +8,11 @@ import { handleActionSuccess, getRunIdAndUrl, } from "./return-dispatch.ts"; -import { getBranchName, logInfoForBranchNameResult } from "./utils.ts"; +import { + createDistinctIdRegex, + getBranchName, + logInfoForBranchNameResult, +} from "./utils.ts"; export async function main(): Promise { try { @@ -27,10 +31,12 @@ export async function main(): Promise { const branch = getBranchName(config.ref); logInfoForBranchNameResult(branch, config.ref); + const distinctIdRegex = createDistinctIdRegex(config.distinctId); + const result = await getRunIdAndUrl({ startTime, branch, - distinctId: config.distinctId, + distinctIdRegex, workflow: config.workflow, workflowId, workflowTimeoutMs: config.workflowTimeoutSeconds * 1000, diff --git a/src/return-dispatch.spec.ts b/src/return-dispatch.spec.ts index 2984d6d..2ee1469 100644 --- a/src/return-dispatch.spec.ts +++ b/src/return-dispatch.spec.ts @@ -29,14 +29,6 @@ import * as utils from "./utils.ts"; vi.mock("@actions/core"); vi.mock("./api.ts"); -// vi.mock(import('./utils.ts'), async (importOriginal) => { -// const mod = await importOriginal() // type is inferred -// return { -// ...mod, -// // replace some exports -// total: vi.fn(mod.sleep), -// } -// }) describe("return-dispatch", () => { const { @@ -473,6 +465,7 @@ describe("return-dispatch", () => { describe("getRunIdAndUrl", () => { const distinctId = crypto.randomUUID(); + const distinctIdRegex = new RegExp(distinctId); const workflow = "workflow.yml"; const workflowId = 123; const branch: utils.BranchNameResult = Object.freeze({ @@ -483,7 +476,7 @@ describe("return-dispatch", () => { const defaultOpts: GetRunIdAndUrlOpts = { startTime: Date.now(), branch: branch, - distinctId: distinctId, + distinctIdRegex: distinctIdRegex, workflow: workflow, workflowId: workflowId, workflowTimeoutMs: 100, diff --git a/src/return-dispatch.ts b/src/return-dispatch.ts index 9e8c8c0..968b871 100644 --- a/src/return-dispatch.ts +++ b/src/return-dispatch.ts @@ -4,7 +4,7 @@ import { ActionOutputs } from "./action.ts"; import * as api from "./api.ts"; import * as constants from "./constants.ts"; import type { Result } from "./types.ts"; -import { sleep, type BranchNameResult } from "./utils.ts"; +import { escapeRegExp, sleep, type BranchNameResult } from "./utils.ts"; export function shouldRetryOrThrow( error: Error, @@ -126,7 +126,7 @@ export function handleActionFail(): void { export interface GetRunIdAndUrlOpts { startTime: number; branch: BranchNameResult; - distinctId: string; + distinctIdRegex: RegExp; workflow: string | number; workflowId: number; workflowTimeoutMs: number; @@ -134,12 +134,11 @@ export interface GetRunIdAndUrlOpts { export async function getRunIdAndUrl({ startTime, branch, - distinctId, + distinctIdRegex, workflow, workflowId, workflowTimeoutMs, }: GetRunIdAndUrlOpts): Promise> { - const distinctIdRegex = new RegExp(distinctId); const retryTimeout = Math.max( constants.WORKFLOW_FETCH_TIMEOUT_MS, workflowTimeoutMs, diff --git a/src/test-utils/logging.mock.ts b/src/test-utils/logging.mock.ts index 5f8067d..26f60bb 100644 --- a/src/test-utils/logging.mock.ts +++ b/src/test-utils/logging.mock.ts @@ -8,6 +8,7 @@ import { type MockInstance, vi, expect } from "vitest"; interface MockedLoggingFunctions { coreDebugLogMock: MockInstance<(message: string) => void>; coreInfoLogMock: MockInstance<(message: string) => void>; + coreWarningLogMock: MockInstance<(message: string) => void>; coreErrorLogMock: MockInstance<(message: string) => void>; assertOnlyCalled: ( ...coreLogMocks: MockInstance<(message: string) => void>[] @@ -22,6 +23,10 @@ export function mockLoggingFunctions(): MockedLoggingFunctions { const coreInfoLogMock: MockInstance = vi .spyOn(core, "info") .mockImplementation(() => undefined); + const coreWarningLogMock: MockInstance = vi.spyOn( + core, + "warning", + ); const coreErrorLogMock: MockInstance = vi .spyOn(core, "error") .mockImplementation(() => undefined); @@ -29,6 +34,7 @@ export function mockLoggingFunctions(): MockedLoggingFunctions { const coreLogMockSet = new Set void>>([ coreDebugLogMock, coreInfoLogMock, + coreWarningLogMock, coreErrorLogMock, ]); const assertOnlyCalled = ( @@ -44,6 +50,7 @@ export function mockLoggingFunctions(): MockedLoggingFunctions { return { coreDebugLogMock, coreInfoLogMock, + coreWarningLogMock, coreErrorLogMock, assertOnlyCalled, assertNoneCalled, diff --git a/src/utils.spec.ts b/src/utils.spec.ts index 3e3cf4c..bbda2c6 100644 --- a/src/utils.spec.ts +++ b/src/utils.spec.ts @@ -9,13 +9,24 @@ import { } from "vitest"; import { mockLoggingFunctions } from "./test-utils/logging.mock.ts"; -import { getBranchName, logInfoForBranchNameResult, sleep } from "./utils.ts"; +import { + createDistinctIdRegex, + escapeRegExp, + getBranchName, + logInfoForBranchNameResult, + sleep, +} from "./utils.ts"; vi.mock("@actions/core"); describe("utils", () => { - const { coreDebugLogMock, coreInfoLogMock, assertOnlyCalled } = - mockLoggingFunctions(); + const { + coreDebugLogMock, + coreInfoLogMock, + coreWarningLogMock, + assertOnlyCalled, + assertNoneCalled, + } = mockLoggingFunctions(); afterEach(() => { vi.resetAllMocks(); @@ -187,6 +198,54 @@ describe("utils", () => { await vi.advanceTimersByTimeAsync(1000); await expect(sleepPromise).resolves.toBeUndefined(); + + assertNoneCalled(); + }); + }); + + describe("escapeRegExp", () => { + const escaped = "\\^\\$\\.\\*\\+\\?\\(\\)\\[\\]\\{\\}\\|\\\\"; + const unescaped = "^$.*+?()[]{}|\\"; + + it("should escape values", () => { + expect(escapeRegExp(unescaped + unescaped)).toBe(escaped + escaped); + assertNoneCalled(); + }); + + it("should handle strings with nothing to escape", () => { + expect(escapeRegExp("abc")).toBe("abc"); + assertNoneCalled(); + }); + + it("should return an empty string for empty values", () => { + expect(escapeRegExp("")).toEqual(""); + assertNoneCalled(); + }); + }); + + describe("createDistinctIdRegex", () => { + it("should return a regex without warning if the input is safe", () => { + expect(createDistinctIdRegex("test-cfg")).toStrictEqual( + new RegExp("test-cfg"), + ); + assertNoneCalled(); + }); + + it("should return a regex with warning if the input is required escaping", () => { + const input = "test$.*+?()[]{}|\\cfg"; + const escapedInput = escapeRegExp(input); + + const distinctId = createDistinctIdRegex(input); + + // Behaviour + expect(distinctId).toStrictEqual(new RegExp(escapedInput)); + + // Logging + assertOnlyCalled(coreWarningLogMock); + expect(coreWarningLogMock).toHaveBeenCalledOnce(); + expect(coreWarningLogMock.mock.calls[0]?.[0]).toMatchInlineSnapshot( + `"Unescaped characters found in distinctId input, using: test\\$\\.\\*\\+\\?\\(\\)\\[\\]\\{\\}\\|\\\\cfg"`, + ); }); }); }); diff --git a/src/utils.ts b/src/utils.ts index c801763..b80813c 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -64,3 +64,39 @@ export function logInfoForBranchNameResult( export function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } + +/** + * Used to match `RegExp` + * [syntax characters](http://ecma-international.org/ecma-262/7.0/#sec-patterns). + * + * https://github.com/lodash/lodash/blob/main/src/escapeRegExp.ts + */ +const reRegExpChar = /[\\^$.*+?()[\]{}|]/g; +const reHasRegExpChar = RegExp(reRegExpChar.source); + +/** + * Escapes the `RegExp` special characters "^", "$", "\", ".", "*", "+", + * "?", "(", ")", "[", "]", "{", "}", and "|" in `string`. + * + * https://github.com/lodash/lodash/blob/main/src/escapeRegExp.ts + */ +export function escapeRegExp(str: string): string { + return reHasRegExpChar.test(str) + ? str.replace(reRegExpChar, "\\$&") + : str || ""; +} + +/** + * If the input distinct ID contains unescaped characters, log the + * escaped distinct ID as a warning. + */ +export function createDistinctIdRegex(distinctId: string): RegExp { + const escapedDistinctId = escapeRegExp(distinctId); + if (distinctId !== escapedDistinctId) { + core.warning( + `Unescaped characters found in distinctId input, using: ${escapedDistinctId}`, + ); + } + + return new RegExp(escapedDistinctId); +}