fix: distinct ID input was not escaped

Author: Codex-

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,

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<void> {
   try {
@@ -27,10 +31,12 @@ export async function main(): Promise<void> {
     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,

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,

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,20 +126,19 @@ export function handleActionFail(): void {
 export interface GetRunIdAndUrlOpts {
   startTime: number;
   branch: BranchNameResult;
-  distinctId: string;
+  distinctIdRegex: RegExp;
   workflow: string | number;
   workflowId: number;
   workflowTimeoutMs: number;
 }
 export async function getRunIdAndUrl({
   startTime,
   branch,
-  distinctId,
+  distinctIdRegex,
   workflow,
   workflowId,
   workflowTimeoutMs,
 }: GetRunIdAndUrlOpts): Promise<Result<{ id: number; url: string }>> {
-  const distinctIdRegex = new RegExp(distinctId);
   const retryTimeout = Math.max(
     constants.WORKFLOW_FETCH_TIMEOUT_MS,
     workflowTimeoutMs,

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,13 +23,18 @@ export function mockLoggingFunctions(): MockedLoggingFunctions {
   const coreInfoLogMock: MockInstance<typeof core.info> = vi
     .spyOn(core, "info")
     .mockImplementation(() => undefined);
+  const coreWarningLogMock: MockInstance<typeof core.error> = vi.spyOn(
+    core,
+    "warning",
+  );
   const coreErrorLogMock: MockInstance<typeof core.error> = vi
     .spyOn(core, "error")
     .mockImplementation(() => undefined);
 
   const coreLogMockSet = new Set<MockInstance<(message: string) => void>>([
     coreDebugLogMock,
     coreInfoLogMock,
+    coreWarningLogMock,
     coreErrorLogMock,
   ]);
   const assertOnlyCalled = (
@@ -44,6 +50,7 @@ export function mockLoggingFunctions(): MockedLoggingFunctions {
   return {
     coreDebugLogMock,
     coreInfoLogMock,
+    coreWarningLogMock,
     coreErrorLogMock,
     assertOnlyCalled,
     assertNoneCalled,

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"`,
+      );
     });
   });
 });

src/utils.ts

@@ -64,3 +64,39 @@ export function logInfoForBranchNameResult(
 export function sleep(ms: number): Promise<void> {
   return new Promise<void>((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);
+}