[APPS-2792] Add: harden the in-process local execution path

packages/plugins/apps/src/vite/local-execution.test.ts

@@ -34,6 +34,8 @@ function loadModuleReturning(exports: Record<string, unknown>): LoadModule {
     };
 }
 
+const ORDER_MARKER = '__ddLocalExecutionTestOrder';
+
 describe('local-execution — executeScriptLocally', () => {
     test('Should run a simple function in-process and return its result', async () => {
         const result = await executeScriptLocally(
@@ -77,6 +79,18 @@ describe('local-execution — executeScriptLocally', () => {
         ).rejects.toThrow(`"example" is not a function exported from ${func.absolutePath}`);
     });
 
+    test('Should reject when loadModule itself rejects, same as a native-module load failure would', async () => {
+        // Simulates e.g. a native addon failing to load at require()/import
+        // time, rather than a customer function throwing during its own
+        // logic — the failure happens before the function is ever reached.
+        const loadModule: LoadModule = async () => {
+            throw new Error('cannot find native module');
+        };
+        await expect(
+            executeScriptLocally(func, [], stubExecuteAction, loadModule, mockLogger),
+        ).rejects.toThrow('cannot find native module');
+    });
+
     test('Should resolve a $.Actions.foo.bar(...) call through the injected executeAction, including connectionId', async () => {
         const executeAction = jest.fn().mockResolvedValue({ ok: true });
         const result = await executeScriptLocally(
@@ -156,7 +170,20 @@ describe('local-execution — executeScriptLocally', () => {
         ).rejects.toThrow(/timed out after 50ms/);
     });
 
-    test('Should never expose an auth token via globalThis', async () => {
+    test('Should never expose an auth token to the customer module — only backendFunctionArgs, Actions, and Source are visible on globalThis.$', async () => {
+        const result = await executeScriptLocally(
+            func,
+            [],
+            stubExecuteAction,
+            loadModuleReturning({
+                example: () => Object.keys((globalThis as Record<string, any>).$).sort(),
+            }),
+            mockLogger,
+        );
+        expect(result).toEqual({ data: ['Actions', 'Source', 'backendFunctionArgs'] });
+    });
+
+    test('Should never expose an auth token via globalThis either', async () => {
         const result = await executeScriptLocally(
             func,
             [],
@@ -245,29 +272,180 @@ describe('local-execution — executeScriptLocally', () => {
         });
     });
 
+    describe('non-serializable results', () => {
+        test('Should reject with a clear, attributed error when the result has a circular reference', async () => {
+            await expect(
+                executeScriptLocally(
+                    func,
+                    [],
+                    stubExecuteAction,
+                    loadModuleReturning({
+                        example: () => {
+                            const o: Record<string, unknown> = {};
+                            o.self = o;
+                            return o;
+                        },
+                    }),
+                    mockLogger,
+                ),
+            ).rejects.toThrow(/example.*can't be serialized to JSON/);
+        });
+
+        test('Should reject with a clear, attributed error when the result contains a BigInt', async () => {
+            await expect(
+                executeScriptLocally(
+                    func,
+                    [],
+                    stubExecuteAction,
+                    loadModuleReturning({ example: () => BigInt(10) }),
+                    mockLogger,
+                ),
+            ).rejects.toThrow(/example.*can't be serialized to JSON/);
+        });
+
+        test('Should reject with a clear, attributed error when the result is a bare function (silently dropped by JSON.stringify)', async () => {
+            await expect(
+                executeScriptLocally(
+                    func,
+                    [],
+                    stubExecuteAction,
+                    loadModuleReturning({ example: () => function notSerializable() {} }),
+                    mockLogger,
+                ),
+            ).rejects.toThrow(/example.*JSON.stringify silently drops/);
+        });
+
+        test('Should allow an explicit undefined result through unchanged', async () => {
+            const result = await executeScriptLocally(
+                func,
+                [],
+                stubExecuteAction,
+                loadModuleReturning({ example: () => undefined }),
+                mockLogger,
+            );
+            expect(result).toEqual({ data: undefined });
+        });
+    });
+
     describe('serialization of concurrent executions', () => {
-        function delayedResult<T>(label: T, delayMs: number): () => Promise<T> {
-            return () => new Promise((resolve) => setTimeout(() => resolve(label), delayMs));
+        beforeEach(() => {
+            delete (globalThis as Record<string, unknown>)[ORDER_MARKER];
+        });
+
+        function recordingOrder(label: string, delayMs: number): () => Promise<string> {
+            return async () => {
+                const marker =
+                    ((globalThis as Record<string, unknown>)[ORDER_MARKER] as string[]) ?? [];
+                (globalThis as Record<string, unknown>)[ORDER_MARKER] = marker;
+                marker.push(`start-${label}`);
+                await new Promise((r) => setTimeout(r, delayMs));
+                marker.push(`end-${label}`);
+                return label;
+            };
         }
 
-        test("Should allow two independent calls to run without cross-contaminating each other's result", async () => {
+        test('Should never interleave two concurrent executions — the second never starts until the first fully finishes', async () => {
             const [resultA, resultB] = await Promise.all([
                 executeScriptLocally(
                     func,
                     [],
                     stubExecuteAction,
-                    loadModuleReturning({ example: delayedResult('A', 20) }),
+                    loadModuleReturning({ example: recordingOrder('A', 20) }),
                     mockLogger,
                 ),
                 executeScriptLocally(
                     func,
                     [],
                     stubExecuteAction,
-                    loadModuleReturning({ example: delayedResult('B', 0) }),
+                    loadModuleReturning({ example: recordingOrder('B', 0) }),
                     mockLogger,
                 ),
             ]);
+
             expect([resultA, resultB]).toEqual([{ data: 'A' }, { data: 'B' }]);
+            const order = (globalThis as Record<string, unknown>)[ORDER_MARKER];
+            // Whichever call the queue happened to run first, its start/end
+            // pair must be adjacent — never interrupted by the other call's
+            // start. A real race (no queueing) would produce
+            // ['start-A', 'start-B', 'end-B', 'end-A'] here, since B's 0ms
+            // delay would let it finish first if both started immediately.
+            expect(order).toEqual([
+                expect.stringMatching(/^start-/),
+                expect.stringMatching(/^end-/),
+                expect.stringMatching(/^start-/),
+                expect.stringMatching(/^end-/),
+            ]);
+            expect((order as string[])[0].slice('start-'.length)).toEqual(
+                (order as string[])[1].slice('end-'.length),
+            );
+            expect((order as string[])[2].slice('start-'.length)).toEqual(
+                (order as string[])[3].slice('end-'.length),
+            );
+        });
+
+        function readOwnArgsAfterDelay(delayMs: number): () => Promise<unknown> {
+            return () =>
+                new Promise((resolve) =>
+                    setTimeout(
+                        () => resolve((globalThis as Record<string, any>).$.backendFunctionArgs),
+                        delayMs,
+                    ),
+                );
+        }
+
+        // executeScriptLocally writes to the shared, mutable `globalThis.$` on
+        // every call. Without the `enqueue` queue, two concurrent calls both
+        // write it synchronously before either yields, so the second write
+        // wins for the whole duration of both calls and the first call's
+        // customer code ends up reading the second call's args — see the
+        // `test.skip`'d version of this same test against PR #479's
+        // un-serialized base, which fails with exactly that cross-
+        // contamination. Running it live here, through the real serialized
+        // `executeScriptLocally` entrypoint, proves the queue actually closes
+        // the gap rather than just changing the ordering of interleaved work.
+        test("Should let each concurrent call see its OWN backendFunctionArgs via globalThis.$, not the other call's", async () => {
+            const [resultA, resultB] = await Promise.all([
+                executeScriptLocally(
+                    func,
+                    ['A-arg'],
+                    stubExecuteAction,
+                    loadModuleReturning({ example: readOwnArgsAfterDelay(20) }),
+                    mockLogger,
+                ),
+                executeScriptLocally(
+                    func,
+                    ['B-arg'],
+                    stubExecuteAction,
+                    loadModuleReturning({ example: readOwnArgsAfterDelay(0) }),
+                    mockLogger,
+                ),
+            ]);
+            expect(resultA).toEqual({ data: ['A-arg'] });
+            expect(resultB).toEqual({ data: ['B-arg'] });
+        });
+
+        test('Should still run the next queued execution after an earlier one rejects', async () => {
+            const first = executeScriptLocally(
+                func,
+                [],
+                stubExecuteAction,
+                loadModuleReturning({
+                    example: () => {
+                        throw new Error('first fails');
+                    },
+                }),
+                mockLogger,
+            );
+            const second = executeScriptLocally(
+                func,
+                [],
+                stubExecuteAction,
+                loadModuleReturning({ example: () => 2 }),
+                mockLogger,
+            );
+
+            await expect(first).rejects.toThrow('first fails');
+            await expect(second).resolves.toEqual({ data: 2 });
         });
     });
 });

packages/plugins/apps/src/vite/local-execution.ts

@@ -70,6 +70,33 @@ const LOCAL_DEV_SOURCE = {
     runAsUser: { id: 'local-dev', orgId: 'local-dev-org' },
 };
 
+/**
+ * Local backend-function executions are serialized, never run concurrently.
+ * `@datadog/action-catalog`'s `setExecuteActionImplementation` and
+ * `@datadog/apps-backend`'s `setBackend` both register runtime context via a
+ * shared, module-level setter — safe under production's model (a fresh Deno
+ * subprocess per execution), unsafe under ours (one long-lived Node process
+ * for every local execution). A second concurrent execution's registration
+ * would silently redirect the first's still-in-flight typed-import calls to
+ * the wrong `$.Actions`/user identity, with no error at all. See the RFC's
+ * Decisions and Trade-Offs for the full reasoning.
+ *
+ * Implementation: a simple promise-chain mutex. `queueTail` always resolves
+ * (errors are swallowed via `.catch(() => {})` before being chained) so a
+ * rejected execution never wedges the queue for whatever runs after it; the
+ * real rejection is still preserved and returned to that call's own caller.
+ */
+let queueTail: Promise<unknown> = Promise.resolve();
+
+function enqueue<T>(run: () => Promise<T>): Promise<T> {
+    const result = queueTail.then(run);
+    queueTail = result.then(
+        () => undefined,
+        () => undefined,
+    );
+    return result;
+}
+
 /**
  * Build the $.Actions Proxy. Resolves any nested property path (e.g.
  * $.Actions.slack.chat.postMessage) to a callable that invokes
@@ -164,13 +191,44 @@ async function registerBackendRuntimeIfInstalled(
     setBackend(buildRuntimeFromJsFunctionWithActions($));
 }
 
+/**
+ * Backend functions eventually return through `ExecuteActionResponse`, which
+ * is serialized to JSON over HTTP. Catch a non-JSON-serializable result here,
+ * with a clear, attributed error, rather than let it surface later as an
+ * opaque `JSON.stringify` failure (or silently drop data) further down the
+ * response pipeline. Covers two distinct failure shapes: `JSON.stringify`
+ * throwing outright (a circular reference, a `BigInt`) and `JSON.stringify`
+ * silently returning `undefined` for a value that wasn't actually `undefined`
+ * (a bare function or `Symbol` at the top level).
+ */
+function assertJsonSerializable(result: unknown, func: BackendFunction): unknown {
+    let serialized: string | undefined;
+    try {
+        serialized = JSON.stringify(result);
+    } catch (err) {
+        throw new Error(
+            `Local execution of "${func.name}" returned a value that can't be serialized to JSON: ${
+                err instanceof Error ? err.message : String(err)
+            }`,
+        );
+    }
+    if (serialized === undefined && result !== undefined) {
+        throw new Error(
+            `Local execution of "${func.name}" returned a ${typeof result} value, which JSON.stringify silently drops instead of serializing — return a plain JSON-compatible value instead.`,
+        );
+    }
+    return result;
+}
+
 /**
  * Execute a backend function in-process by importing its real file directly
  * — no bundling, no generated wrapper module. `globalThis.$` and the
  * action-catalog/apps-backend registrations above stand in for what the
  * removed `main($)` wrapper used to do textually; everything else about the
  * call is just invoking the customer's exported function with its own real
  * arguments.
+ *
+ * Serialized via `enqueue` — see its own doc comment for why.
  */
 export async function executeScriptLocally(
     func: BackendFunction,
@@ -179,6 +237,17 @@ export async function executeScriptLocally(
     loadModule: LoadModule,
     log: Logger,
     timeoutMs: number = DEFAULT_TIMEOUT_MS,
+): Promise<BackendOutputs> {
+    return enqueue(() => runScriptLocally(func, args, executeAction, loadModule, log, timeoutMs));
+}
+
+async function runScriptLocally(
+    func: BackendFunction,
+    args: unknown[],
+    executeAction: ExecuteAction,
+    loadModule: LoadModule,
+    log: Logger,
+    timeoutMs: number,
 ): Promise<BackendOutputs> {
     log.debug(`Executing "${func.name}" in-process with args=${JSON.stringify(args)}`);
 
@@ -202,7 +271,7 @@ export async function executeScriptLocally(
         }
 
         const result = await fn(...args);
-        return { data: result };
+        return { data: assertJsonSerializable(result, func) };
     };
 
     let timer: ReturnType<typeof setTimeout> | undefined;