diff --git a/packages/plugins/apps/README.md b/packages/plugins/apps/README.md index 8b67669a5..fe8da0295 100644 --- a/packages/plugins/apps/README.md +++ b/packages/plugins/apps/README.md @@ -115,6 +115,8 @@ Local execution doesn't support Yarn Plug'n'Play, whose packages load through ho Backend functions run on the Node backend runtime, and the plugin's build-time and dev-server checks follow its rules: backend functions may import Node built-ins and use Node's network globals such as `fetch` and `WebSocket`, but not `XMLHttpRequest`, `EventSource` or `Deno`, which Node doesn't expose by default. The checks fail only where the missing global would throw: any bare use but `typeof Deno`, or using `globalThis.Deno` as an object, function or class, even after a feature check or a polyfill install, so use an imported polyfill by its name. Feature checks such as `if (globalThis.EventSource)` and destructuring defaults pass. The checks follow a binding the global is declared or destructured into, but not a plain assignment such as `d = globalThis.Deno`, a `let` or parameter alias of `globalThis`, a bulk copy such as `{ ...globalThis }`, or a value that is stored in an object, array or class field, exported, returned or passed on. Local execution runs on your own Node, while the Node backend runtime runs Node 24, so the dev server warns once at start on another major version, and on one before Node 22, which lacks the global `WebSocket`. +The checks also warn, without failing the build, about `process.env` reads: in production, the Node backend runtime doesn't pass your own environment variables or Custom Credentials to backend functions, so those reads don't see your values there. They catch direct reads: `process.env.X`, `process.env[key]`, a destructure of `process.env`, an `env`, default or namespace import from `process` or `node:process`, and a `const` bound to `process.env` or to an `env` destructured from `process`. Like the other checks, they cover only the app's own code and opted-in packages, so an SDK that reads `process.env` (for example, for a default API key) doesn't warn. + ## Package output A production `vite build` writes `datadog-app-assets.zip` beside the Vite output. The ZIP contains `frontend/`, `backend/`, and `manifest.json`. The app's identity is resolved by `@datadog/apps-cli` at deploy time. diff --git a/packages/plugins/apps/src/backend/ast-parsing/ambient-global-access.ts b/packages/plugins/apps/src/backend/ast-parsing/ambient-global-access.ts index f0daab52b..1ef2da782 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/ambient-global-access.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/ambient-global-access.ts @@ -111,7 +111,7 @@ function resolvesToAmbientGlobal( return false; } -function staticMemberName( +export function staticMemberName( node: MemberExpression, scopeAnalysis: ModuleScopeAnalysis, ): string | undefined { @@ -122,11 +122,11 @@ function staticMemberName( } // A defaulted property value (`{ x = fallback }`) is an AssignmentPattern wrapping the real binding — unwrap it so a defaulted alias is treated the same as a bare one. -function unwrapDefaultValue(value: Property['value']): Property['value'] { +export function unwrapDefaultValue(value: Property['value']): Property['value'] { return value.type === 'AssignmentPattern' ? value.left : value; } -function staticPropertyName( +export function staticPropertyName( property: Property, scopeAnalysis: ModuleScopeAnalysis, ): string | undefined { diff --git a/packages/plugins/apps/src/backend/ast-parsing/run-backend-static-checks.test.ts b/packages/plugins/apps/src/backend/ast-parsing/run-backend-static-checks.test.ts index 1eef12c64..033b05afd 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/run-backend-static-checks.test.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/run-backend-static-checks.test.ts @@ -4,7 +4,7 @@ import { analyzeModuleScope } from '@dd/apps-plugin/backend/ast-parsing/module-scope'; import { NODE_REPLACEMENT_HINTS } from '@dd/apps-plugin/backend/ast-parsing/reject-globals-missing-from-node'; -import { runBackendStaticChecks } from '@dd/apps-plugin/backend/ast-parsing/run-backend-static-checks'; +import { createBackendStaticChecks } from '@dd/apps-plugin/backend/ast-parsing/run-backend-static-checks'; import { ensureProgram } from '@dd/apps-plugin/backend/ast-parsing/type-guards'; import { NODE_BACKEND_RUNTIME } from '@dd/apps-plugin/constants'; import { getMockLogger, mockLogFn } from '@dd/tests/_jest/helpers/mocks'; @@ -16,11 +16,11 @@ function check(code: string): void { const ast = parseAst(code); const program = ensureProgram(ast, FILE_PATH); const scopeAnalysis = analyzeModuleScope(program); - const log = getMockLogger(); - runBackendStaticChecks(ast, FILE_PATH, log, scopeAnalysis); + const runStaticChecks = createBackendStaticChecks(getMockLogger()); + runStaticChecks(ast, FILE_PATH, scopeAnalysis); } -describe('Backend Functions - runBackendStaticChecks', () => { +describe('Backend Functions - createBackendStaticChecks', () => { beforeEach(() => { jest.clearAllMocks(); }); @@ -82,4 +82,12 @@ describe('Backend Functions - runBackendStaticChecks', () => { expect(mockLogFn).not.toHaveBeenCalled(); }); + + test('should warn about a process.env read', () => { + check('export function run() { return process.env.STRIPE_KEY; }'); + + const logged = mockLogFn.mock.calls.filter(([, level]) => level === 'warn'); + const messages = logged.map(([message]) => message); + expect(messages).toEqual([expect.stringContaining('process.env.STRIPE_KEY')]); + }); }); diff --git a/packages/plugins/apps/src/backend/ast-parsing/run-backend-static-checks.ts b/packages/plugins/apps/src/backend/ast-parsing/run-backend-static-checks.ts index cc87f2d26..0c95c13d4 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/run-backend-static-checks.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/run-backend-static-checks.ts @@ -7,13 +7,19 @@ import type { BaseNode } from 'estree'; import type { ModuleScopeAnalysis } from './module-scope'; import { rejectGlobalsMissingFromNode } from './reject-globals-missing-from-node'; +import { type WarnedEnvReads, warnAboutEnvReads } from './warn-env-reads'; -/** Runs every static check against a single module; shared by every call site so the checks can't drift. */ -export function runBackendStaticChecks( +export type RunBackendStaticChecks = ( ast: BaseNode, filePath: string, - log: Logger, scopeAnalysis: ModuleScopeAnalysis, -): void { - rejectGlobalsMissingFromNode(ast, filePath, scopeAnalysis); +) => void; + +/** Create one per plugin instance and share it across call sites, so the checks can't drift and each warning shows once. */ +export function createBackendStaticChecks(log: Logger): RunBackendStaticChecks { + const warnedEnvReads: WarnedEnvReads = new Map(); + return (ast, filePath, scopeAnalysis) => { + rejectGlobalsMissingFromNode(ast, filePath, scopeAnalysis); + warnAboutEnvReads(ast, filePath, log, scopeAnalysis, warnedEnvReads); + }; } diff --git a/packages/plugins/apps/src/backend/ast-parsing/warn-env-reads.test.ts b/packages/plugins/apps/src/backend/ast-parsing/warn-env-reads.test.ts new file mode 100644 index 000000000..4c14f45cc --- /dev/null +++ b/packages/plugins/apps/src/backend/ast-parsing/warn-env-reads.test.ts @@ -0,0 +1,369 @@ +// 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 * as ambientGlobalAccess from '@dd/apps-plugin/backend/ast-parsing/ambient-global-access'; +import { analyzeModuleScope } from '@dd/apps-plugin/backend/ast-parsing/module-scope'; +import { ensureProgram } from '@dd/apps-plugin/backend/ast-parsing/type-guards'; +import { + ENV_NOT_PASSED_EXPLANATION, + MAX_TRACKED_FILES, + type WarnedEnvReads, + WHOLE_ENV_USE, + warnAboutEnvReads, +} from '@dd/apps-plugin/backend/ast-parsing/warn-env-reads'; +import { getMockLogger, mockLogFn } from '@dd/tests/_jest/helpers/mocks'; +import { parseAst } from 'rollup/parseAst'; + +const FILE_PATH = '/project/src/billing.backend.ts'; +let warnedEnvReads: WarnedEnvReads; + +function check(code: string, filePath: string = FILE_PATH): void { + const ast = parseAst(code); + const program = ensureProgram(ast, filePath); + const scopeAnalysis = analyzeModuleScope(program); + const log = getMockLogger(); + warnAboutEnvReads(ast, filePath, log, scopeAnalysis, warnedEnvReads); +} + +function warnings(): string[] { + return mockLogFn.mock.calls + .filter(([, level]) => level === 'warn') + .map(([message]) => String(message)); +} + +describe('Backend Functions - warnAboutEnvReads', () => { + beforeEach(() => { + warnedEnvReads = new Map(); + }); + + const mustWarn = [ + { + form: 'process.env.X', + code: 'export function run() { return process.env.STRIPE_KEY; }', + uses: ['process.env.STRIPE_KEY'], + }, + { + form: "process.env['X']", + code: "export function run() { return process.env['STRIPE_KEY']; }", + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'a key that is not an identifier', + code: "export function run() { return process.env['STRIPE-KEY']; }", + uses: ['process.env["STRIPE-KEY"]'], + }, + { + form: 'a non-ASCII identifier key', + code: 'export function run() { return process.env.CAFÉ_KEY; }', + uses: ['process.env.CAFÉ_KEY'], + }, + { + form: 'a dynamic key', + code: 'export function run(name) { return process.env[name]; }', + uses: [WHOLE_ENV_USE], + }, + { + form: 'an optional read', + code: 'export function run() { return process.env?.STRIPE_KEY; }', + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'a key read behind a guard on env', + code: 'export function run() { if (process.env && process.env.STRIPE_KEY) { return 1; } }', + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'a destructure from process.env, with a rename and a default', + code: "export function run() { const { STRIPE_KEY, REGION: region = 'us' } = process.env; return [STRIPE_KEY, region]; }", + uses: ['process.env.STRIPE_KEY', 'process.env.REGION'], + }, + { + form: 'a destructuring assignment from process.env', + code: 'let key; export function run() { ({ STRIPE_KEY: key } = process.env); return key; }', + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'a rest destructure from process.env', + code: 'export function run() { const { ...all } = process.env; return all; }', + uses: [WHOLE_ENV_USE], + }, + { + form: "an env import from 'node:process'", + code: "import { env } from 'node:process';\nexport function run() { return env.STRIPE_KEY; }", + uses: ['process.env.STRIPE_KEY'], + }, + { + form: "a renamed env import from 'process'", + code: "import { env as vars } from 'process';\nexport function run() { const { REGION } = vars; return [REGION, vars.STRIPE_KEY]; }", + uses: ['process.env.REGION', 'process.env.STRIPE_KEY'], + }, + { + form: "a default process import from 'node:process'", + code: "import proc from 'node:process';\nexport function run() { return proc.env.STRIPE_KEY; }", + uses: ['process.env.STRIPE_KEY'], + }, + { + form: "a namespace import from 'node:process'", + code: "import * as proc from 'node:process';\nexport function run() { return proc.env.STRIPE_KEY; }", + uses: ['process.env.STRIPE_KEY'], + }, + { + form: "a default import by name from 'process'", + code: "import { default as proc } from 'process';\nexport function run() { return proc.env.STRIPE_KEY; }", + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'a nested destructure of env from process', + code: 'export function run() { const { env: { STRIPE_KEY } } = process; return STRIPE_KEY; }', + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'a nested destructuring assignment of env from process, with a default', + code: 'let key; export function run() { ({ env: { STRIPE_KEY: key } = {} } = process); return key; }', + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'a destructured parameter defaulting to process.env', + code: 'export function run({ STRIPE_KEY } = process.env) { return STRIPE_KEY; }', + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'a destructure from process.env with a fallback', + code: 'export function run() { const { STRIPE_KEY } = process.env ?? {}; return STRIPE_KEY; }', + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'a const alias of process.env with a fallback', + code: 'const env = process.env || {};\nexport function run() { return env.STRIPE_KEY; }', + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'a const alias of process.env', + code: 'const env = process.env;\nexport function run() { return env.STRIPE_KEY; }', + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'env destructured from process', + code: 'const { env } = process;\nexport function run() { return env.STRIPE_KEY; }', + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'a logical assignment, which reads the value first', + code: "export function run() { process.env.STRIPE_KEY ??= 'x'; }", + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'env destructured from process with a default', + code: 'const { env = {} } = process;\nexport function run() { return env.STRIPE_KEY; }', + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'env destructured from process under another name', + code: 'export function run() { const { env: vars } = process; return vars.STRIPE_KEY; }', + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'a destructure from an optional process?.env', + code: 'export function run() { const { STRIPE_KEY } = process?.env; return STRIPE_KEY; }', + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'a const alias of an optional process?.env', + code: 'const env = process?.env;\nexport function run() { return env.STRIPE_KEY; }', + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'a computed key read inside a destructuring write', + code: 'export function run(o) { ({ [process.env.STRIPE_KEY]: process.env.REGION } = o); }', + uses: ['process.env.STRIPE_KEY'], + }, + { + form: 'a read of a key named like an Object.prototype member', + code: 'export function run() { return process.env.constructor; }', + uses: ['process.env.constructor'], + }, + { + form: 'a destructure of a key named like an Object.prototype member', + code: 'export function run() { const { toString } = process.env; return toString; }', + uses: ['process.env.toString'], + }, + ]; + + test.each(mustWarn)('Should warn on $form', ({ code, uses }) => { + check(code); + + const logged = warnings(); + const useList = uses.join(', '); + expect(logged).toEqual([`${FILE_PATH} uses ${useList}. ${ENV_NOT_PASSED_EXPLANATION}`]); + }); + + const mustNotWarn = [ + { + form: 'a write to process.env', + code: "export function run() { process.env.STRIPE_KEY = 'x'; }", + }, + { + form: 'a delete from process.env', + code: 'export function run() { delete process.env.STRIPE_KEY; }', + }, + { + form: 'an optional delete from process.env', + code: 'export function run() { delete process.env?.STRIPE_KEY; }', + }, + { + form: 'an optional computed delete from process.env', + code: "export function run() { delete process.env?.['STRIPE_KEY']; }", + }, + { + form: 'an optional delete through a const alias of process.env', + code: 'const env = process.env;\nexport function run() { delete env?.STRIPE_KEY; }', + }, + { + form: 'an array destructuring write to process.env', + code: 'export function run(parts) { [process.env.REGION, ...process.env.REST] = parts; }', + }, + { + form: 'an object destructuring write to process.env, with a default', + code: "export function run(o) { ({ a: process.env.REGION = 'us', ...process.env.REST } = o); }", + }, + { + form: 'a nested destructuring write to process.env', + code: 'export function run(o) { ({ a: [process.env.REGION] } = o); }', + }, + { + form: 'a for...of write to process.env', + code: 'export function run(xs) { for (process.env.REGION of xs) {} }', + }, + { + form: 'a for...in write to process.env', + code: 'export function run(o) { for (process.env.REGION in o) {} }', + }, + { + form: 'a for...of destructuring write to process.env', + code: 'export function run(xs) { for ([process.env.REGION] of xs) {} }', + }, + { + form: 'a call of an inherited Object.prototype method', + code: "export function run() { return process.env.hasOwnProperty('STRIPE_KEY'); }", + }, + { + form: 'an optional call of an inherited Object.prototype method', + code: "export function run() { return [process.env?.hasOwnProperty('STRIPE_KEY'), process.env.toString?.()]; }", + }, + { + form: 'a process parameter that shadows the global', + code: 'export function run(process) { return process.env.STRIPE_KEY; }', + }, + { + form: 'a local const named process', + code: 'const process = { env: {} };\nexport function run() { return process.env.STRIPE_KEY; }', + }, + { + form: 'an env import from another module', + code: "import { env } from './config';\nexport function run() { return env.STRIPE_KEY; }", + }, + { + form: 'a destructure from the right side of process.env &&', + code: 'export function run(o) { const { STRIPE_KEY } = process.env && o; return STRIPE_KEY; }', + }, + { + form: 'a destructured parameter defaulting to another object', + code: 'export function run({ STRIPE_KEY } = { STRIPE_KEY: process.argv[2] }) { return STRIPE_KEY; }', + }, + { + form: 'a process member other than env', + code: 'export function run() { return [process.cwd(), process.argv[0]]; }', + }, + { + form: 'a let alias of process.env, not followed', + code: 'let env = process.env;\nexport function run() { return env.STRIPE_KEY; }', + }, + { + form: 'a second-level alias, not followed', + code: 'const env = process.env;\nconst vars = env;\nexport function run() { return vars.STRIPE_KEY; }', + }, + { + form: 'process.env passed on as a whole, not followed', + code: 'export function run(read) { return read(process.env); }', + }, + ]; + + test.each(mustNotWarn)('Should not warn on $form', ({ code }) => { + check(code); + + const logged = warnings(); + expect(logged).toEqual([]); + }); + + test('Should name every key in a file in one warning', () => { + check( + 'export function run() { return [process.env.STRIPE_KEY, process.env.REGION, process.env.STRIPE_KEY]; }', + ); + + const [warning, ...rest] = warnings(); + expect(rest).toEqual([]); + expect(warning).toContain('uses process.env.STRIPE_KEY, process.env.REGION. '); + }); + + test('Should not warn again about a use already reported for the same file', () => { + const code = 'export function run() { return process.env.STRIPE_KEY; }'; + check(code); + check(code); + + const logged = warnings(); + expect(logged).toHaveLength(1); + }); + + test('Should warn only about the uses that are new for a file', () => { + check('export function run() { return process.env.STRIPE_KEY; }'); + check('export function run() { return [process.env.STRIPE_KEY, process.env.REGION]; }'); + + const [, second] = warnings(); + expect(second).toContain('uses process.env.REGION. '); + expect(second).not.toContain('STRIPE_KEY'); + }); + + test('Should warn once per file, without failing, when the check itself throws', () => { + const staticMemberName = jest + .spyOn(ambientGlobalAccess, 'staticMemberName') + .mockImplementation(() => { + throw new Error('unexpected node'); + }); + try { + const code = 'export function run() { return process.env.STRIPE_KEY; }'; + check(code); + check(code); + } finally { + staticMemberName.mockRestore(); + } + + const logged = warnings(); + expect(logged).toEqual([ + `Could not check ${FILE_PATH} for process.env reads: unexpected node`, + ]); + }); + + test('Should warn about the same use in a different file', () => { + const code = 'export function run() { return process.env.STRIPE_KEY; }'; + const otherFilePath = '/project/src/other.backend.ts'; + check(code); + check(code, otherFilePath); + + const [, second] = warnings(); + expect(second).toContain(otherFilePath); + }); + + test('Should warn again about a use once more files with uses than the cap were checked', () => { + const code = 'export function run() { return process.env.STRIPE_KEY; }'; + check(code); + for (let index = 0; index < MAX_TRACKED_FILES; index++) { + check(code, `/project/src/reader${index}.backend.ts`); + } + check(code); + + const logged = warnings(); + const firstFileWarnings = logged.filter((warning) => warning.startsWith(`${FILE_PATH} `)); + expect(firstFileWarnings).toHaveLength(2); + }); +}); diff --git a/packages/plugins/apps/src/backend/ast-parsing/warn-env-reads.ts b/packages/plugins/apps/src/backend/ast-parsing/warn-env-reads.ts new file mode 100644 index 000000000..b049943df --- /dev/null +++ b/packages/plugins/apps/src/backend/ast-parsing/warn-env-reads.ts @@ -0,0 +1,318 @@ +// 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 type { Logger } from '@dd/core/types'; +import type * as eslintScope from 'eslint-scope'; +import type { + BaseNode, + Expression, + ImportSpecifier, + ObjectPattern, + Pattern, + Program, + Property, + Super, + VariableDeclaration, +} from 'estree'; + +import { NODE_BACKEND_RUNTIME } from '../../constants'; + +import { staticMemberName, staticPropertyName, unwrapDefaultValue } from './ambient-global-access'; +import type { ModuleScopeAnalysis } from './module-scope'; +import { ensureProgram } from './type-guards'; +import { walkAst } from './walk-ast'; + +export const WHOLE_ENV_USE = 'process.env'; +// Calling one of these on `process.env` runs the inherited method; any other use reads a variable of that name. +const OBJECT_PROTOTYPE_NAMES = Object.getOwnPropertyNames(Object.prototype); +const OBJECT_PROTOTYPE_KEYS = new Set(OBJECT_PROTOTYPE_NAMES); +const IDENTIFIER_KEY = /^[\p{ID_Start}$_][\p{ID_Continue}$\u200C\u200D]*$/u; +const PROCESS_MODULES = new Set(['process', 'node:process']); + +// Every transform and local run re-checks a file, so each use is reported once per file; cleared wholesale once too large, to cap memory in long dev sessions. +export const MAX_TRACKED_FILES = 500; +export type WarnedEnvReads = Map>; + +export const ENV_NOT_PASSED_EXPLANATION = `In production, the ${NODE_BACKEND_RUNTIME} doesn't pass your own environment variables or Custom Credentials to backend functions, so these reads won't see your values there.`; + +type Role = 'process' | 'env'; +// Recorded beside a file's reported uses, so a failed check warns once per file too. +const CHECK_FAILED = '\0check failed'; + +function isProcessImport(statement: Program['body'][number]): boolean { + return ( + statement.type === 'ImportDeclaration' && + PROCESS_MODULES.has(String(statement.source.value)) + ); +} + +function mentionsProcess(program: Program, scope: ModuleScopeAnalysis): boolean { + for (const [identifier, reference] of scope.referencesByIdentifier) { + if (!reference.resolved && identifier.name === 'process') { + return true; + } + } + return program.body.some(isProcessImport); +} + +function importedName(specifier: ImportSpecifier): string { + const { imported } = specifier; + return imported.type === 'Identifier' ? imported.name : String(imported.value); +} + +function unwrapChain(node: Expression | Super): Expression | Super { + return node.type === 'ChainExpression' ? node.expression : node; +} + +// `process.env ?? {}` and `process.env || {}` evaluate to the real env whenever it exists. +function unwrapFallback(node: Expression | Super): Expression | Super { + const unwrapped = unwrapChain(node); + const isFallback = unwrapped.type === 'LogicalExpression' && unwrapped.operator !== '&&'; + return isFallback ? unwrapFallback(unwrapped.left) : unwrapped; +} + +function envUse(key: string): string { + return IDENTIFIER_KEY.test(key) ? `process.env.${key}` : `process.env[${JSON.stringify(key)}]`; +} + +// Only direct forms: `process` (global or imported), `process.env`, an imported `env`, and a `const` +// bound to `process.env` or to an `env` destructured from `process`. +function createEnvMatcher(program: Program, scope: ModuleScopeAnalysis) { + const roles = new Map(); + const roleOf = (node: Expression | Super): Role | undefined => { + const reference = node.type === 'Identifier' && scope.referencesByIdentifier.get(node); + if (!reference) { + return undefined; + } + if (!reference.resolved) { + return reference.identifier.name === 'process' ? 'process' : undefined; + } + return roles.get(reference.resolved); + }; + const isDirectEnv = (expression: Expression | Super): boolean => { + const node = unwrapFallback(expression); + return ( + node.type === 'MemberExpression' && + staticMemberName(node, scope) === 'env' && + roleOf(node.object) === 'process' + ); + }; + + for (const statement of program.body) { + if (statement.type !== 'ImportDeclaration' || !isProcessImport(statement)) { + continue; + } + for (const specifier of statement.specifiers) { + const [variable] = scope.scopeManager.getDeclaredVariables(specifier); + const name = specifier.type === 'ImportSpecifier' ? importedName(specifier) : undefined; + const isEnvImport = name === 'env'; + const isProcessObjectImport = + specifier.type !== 'ImportSpecifier' || name === 'default'; + if (variable && isProcessObjectImport) { + roles.set(variable, 'process'); + } else if (variable && isEnvImport) { + roles.set(variable, 'env'); + } + } + } + + // A `const` binding's declaration is its only write, which eslint-scope records as a reference. + const markEnvBinding = (binding: Property['value']) => { + const reference = + binding.type === 'Identifier' && scope.referencesByIdentifier.get(binding); + if (reference && reference.resolved) { + roles.set(reference.resolved, 'env'); + } + }; + walkAst(program, null, { + VariableDeclaration(node) { + if (node.kind !== 'const') { + return; + } + for (const { id, init } of node.declarations) { + if (init && isDirectEnv(init)) { + markEnvBinding(id); + } + if (!init || id.type !== 'ObjectPattern' || roleOf(init) !== 'process') { + continue; + } + for (const property of id.properties) { + const key = property.type === 'Property' && staticPropertyName(property, scope); + if (property.type === 'Property' && key === 'env') { + const binding = unwrapDefaultValue(property.value); + markEnvBinding(binding); + } + } + } + }, + }); + + return { + isEnv: (node: Expression | Super): boolean => + isDirectEnv(node) || roleOf(unwrapFallback(node)) === 'env', + isProcess: (node: Expression | Super): boolean => + roleOf(unwrapFallback(node)) === 'process', + }; +} + +// Warns (never rejects) on `process.env` reads, since production doesn't pass the developer's variables or Custom Credentials. +export function warnAboutEnvReads( + ast: BaseNode, + filePath: string, + log: Logger, + scopeAnalysis: ModuleScopeAnalysis, + warnedEnvReads: WarnedEnvReads, +): void { + const program = ensureProgram(ast, filePath); + const uses = new Set(); + const addKey = (key: string | undefined) => { + uses.add(key === undefined ? WHOLE_ENV_USE : envUse(key)); + }; + if (!mentionsProcess(program, scopeAnalysis)) { + return; + } + try { + const { isEnv, isProcess } = createEnvMatcher(program, scopeAnalysis); + const writtenMembers = new Set(); + const calledMembers = new Set(); + const markWriteTargets = (target: Pattern) => { + if (target.type === 'MemberExpression') { + writtenMembers.add(target); + } else if (target.type === 'ArrayPattern') { + for (const element of target.elements) { + if (element) { + markWriteTargets(element); + } + } + } else if (target.type === 'ObjectPattern') { + for (const property of target.properties) { + markWriteTargets(property.type === 'RestElement' ? property : property.value); + } + } else if (target.type === 'AssignmentPattern') { + markWriteTargets(target.left); + } else if (target.type === 'RestElement') { + markWriteTargets(target.argument); + } + }; + const markLoopTarget = (left: Pattern | VariableDeclaration) => { + if (left.type !== 'VariableDeclaration') { + markWriteTargets(left); + } + }; + const addDestructuredKeys = (pattern: ObjectPattern) => { + for (const property of pattern.properties) { + const key = + property.type === 'RestElement' + ? undefined + : staticPropertyName(property, scopeAnalysis); + addKey(key); + } + }; + const checkDestructure = (pattern: ObjectPattern, init: Expression) => { + if (isEnv(init)) { + addDestructuredKeys(pattern); + return; + } + if (!isProcess(init)) { + return; + } + for (const property of pattern.properties) { + const isEnvProperty = + property.type === 'Property' && + staticPropertyName(property, scopeAnalysis) === 'env'; + const value = isEnvProperty && unwrapDefaultValue(property.value); + if (value && value.type === 'ObjectPattern') { + addDestructuredKeys(value); + } + } + }; + walkAst(program, null, { + CallExpression(node) { + calledMembers.add(unwrapChain(node.callee)); + }, + AssignmentExpression(node) { + if (node.operator === '=') { + markWriteTargets(node.left); + } + if (node.left.type === 'ObjectPattern') { + checkDestructure(node.left, node.right); + } + }, + AssignmentPattern(node) { + if (node.left.type === 'ObjectPattern') { + checkDestructure(node.left, node.right); + } + }, + ForInStatement(node) { + markLoopTarget(node.left); + }, + ForOfStatement(node) { + markLoopTarget(node.left); + }, + UnaryExpression(node) { + if (node.operator === 'delete') { + writtenMembers.add(unwrapChain(node.argument)); + } + }, + VariableDeclarator(node) { + if (node.id.type === 'ObjectPattern' && node.init) { + checkDestructure(node.id, node.init); + } + }, + MemberExpression(node) { + if (writtenMembers.has(node) || !isEnv(node.object)) { + return; + } + const key = staticMemberName(node, scopeAnalysis); + const isInheritedMethodCall = + calledMembers.has(node) && key !== undefined && OBJECT_PROTOTYPE_KEYS.has(key); + if (!isInheritedMethodCall) { + addKey(key); + } + }, + }); + } catch (error) { + // A warning-only check must not fail the build. + const warned = warnedUsesFor(warnedEnvReads, filePath); + if (!warned.has(CHECK_FAILED)) { + warned.add(CHECK_FAILED); + const reason = error instanceof Error ? error.message : String(error); + log.warn(`Could not check ${filePath} for process.env reads: ${reason}`); + } + } + reportNewUses(uses, filePath, log, warnedEnvReads); +} + +function warnedUsesFor(warnedEnvReads: WarnedEnvReads, filePath: string): Set { + let warned = warnedEnvReads.get(filePath); + if (!warned) { + if (warnedEnvReads.size >= MAX_TRACKED_FILES) { + warnedEnvReads.clear(); + } + warned = new Set(); + warnedEnvReads.set(filePath, warned); + } + return warned; +} + +function reportNewUses( + uses: ReadonlySet, + filePath: string, + log: Logger, + warnedEnvReads: WarnedEnvReads, +): void { + if (uses.size === 0) { + return; + } + const warned = warnedUsesFor(warnedEnvReads, filePath); + const newUses = [...uses].filter((use) => !warned.has(use)); + if (newUses.length === 0) { + return; + } + for (const use of newUses) { + warned.add(use); + } + const useList = newUses.join(', '); + log.warn(`${filePath} uses ${useList}. ${ENV_NOT_PASSED_EXPLANATION}`); +} diff --git a/packages/plugins/apps/src/vite/backend-static-checks-plugin.test.ts b/packages/plugins/apps/src/vite/backend-static-checks-plugin.test.ts index f63ca503f..ff353d28e 100644 --- a/packages/plugins/apps/src/vite/backend-static-checks-plugin.test.ts +++ b/packages/plugins/apps/src/vite/backend-static-checks-plugin.test.ts @@ -3,6 +3,7 @@ // Copyright 2019-Present Datadog, Inc. import { createParsedModuleRecord } from '@dd/apps-plugin/backend/ast-parsing/module-graph'; +import { createBackendStaticChecks } from '@dd/apps-plugin/backend/ast-parsing/run-backend-static-checks'; import { NODE_BACKEND_RUNTIME } from '@dd/apps-plugin/constants'; import { getMockLogger } from '@dd/tests/_jest/helpers/mocks'; import { parseAst } from 'rollup/parseAst'; @@ -33,7 +34,7 @@ describe('Backend Functions - backend static checks plugin', () => { test('Should accept a module importing a Node built-in and calling fetch', () => { const plugin = createBackendStaticChecksPlugin( '/project', - getMockLogger(), + createBackendStaticChecks(getMockLogger()), () => new Map(), ); @@ -48,7 +49,7 @@ describe('Backend Functions - backend static checks plugin', () => { test('Should reject a module using Deno', () => { const plugin = createBackendStaticChecksPlugin( '/project', - getMockLogger(), + createBackendStaticChecks(getMockLogger()), () => new Map(), ); @@ -63,7 +64,7 @@ describe('Backend Functions - backend static checks plugin', () => { test('Should allow a module with no restricted globals', () => { const plugin = createBackendStaticChecksPlugin( '/project', - getMockLogger(), + createBackendStaticChecks(getMockLogger()), () => new Map(), ); @@ -78,7 +79,7 @@ describe('Backend Functions - backend static checks plugin', () => { test('Should skip modules under node_modules', () => { const plugin = createBackendStaticChecksPlugin( '/project', - getMockLogger(), + createBackendStaticChecks(getMockLogger()), () => new Map(), ); @@ -93,7 +94,7 @@ describe('Backend Functions - backend static checks plugin', () => { test('Should skip virtual modules', () => { const plugin = createBackendStaticChecksPlugin( '/project', - getMockLogger(), + createBackendStaticChecks(getMockLogger()), () => new Map(), ); @@ -114,7 +115,7 @@ describe('Backend Functions - backend static checks plugin', () => { test('Should skip modules with no source (external/synthetic modules)', () => { const plugin = createBackendStaticChecksPlugin( '/project', - getMockLogger(), + createBackendStaticChecks(getMockLogger()), () => new Map(), ); @@ -135,7 +136,7 @@ describe('Backend Functions - backend static checks plugin', () => { } const plugin = createBackendStaticChecksPlugin( '/project', - getMockLogger(), + createBackendStaticChecks(getMockLogger()), () => new Map([[moduleId, record]]), ); const throwIfParsed: typeof parseAst = () => { @@ -154,7 +155,7 @@ describe('Backend Functions - backend static checks plugin', () => { test('Should fall back to parsing when no record exists for the module', () => { const plugin = createBackendStaticChecksPlugin( '/project', - getMockLogger(), + createBackendStaticChecks(getMockLogger()), () => new Map(), ); @@ -169,7 +170,7 @@ describe('Backend Functions - backend static checks plugin', () => { test('Should throw a checks-specific error, not a connection-ID-collector-specific one, when the fallback parse fails', () => { const plugin = createBackendStaticChecksPlugin( '/project', - getMockLogger(), + createBackendStaticChecks(getMockLogger()), () => new Map(), ); diff --git a/packages/plugins/apps/src/vite/backend-static-checks-plugin.ts b/packages/plugins/apps/src/vite/backend-static-checks-plugin.ts index 8e578c743..527c8d6ae 100644 --- a/packages/plugins/apps/src/vite/backend-static-checks-plugin.ts +++ b/packages/plugins/apps/src/vite/backend-static-checks-plugin.ts @@ -2,13 +2,12 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. -import type { Logger } from '@dd/core/types'; import type { BaseNode } from 'estree'; import type { Plugin, Rollup } from 'vite'; import type { ParsedModuleRecord } from '../backend/ast-parsing/module-graph'; import { analyzeModuleScope } from '../backend/ast-parsing/module-scope'; -import { runBackendStaticChecks } from '../backend/ast-parsing/run-backend-static-checks'; +import type { RunBackendStaticChecks } from '../backend/ast-parsing/run-backend-static-checks'; import { ensureProgram } from '../backend/ast-parsing/type-guards'; import { isBackendSourceModule } from '../backend/backend-sources'; @@ -24,7 +23,7 @@ function unsupportedStaticChecksSource(filePath: string, unsupported: string): E // Re-runs the static checks against every backend source module (app code or an opted-in package's code) the nested backend build resolves, not just the `.backend.ts` entry, so an imported helper module is also checked. MUST be registered after the connection-ID collector's plugin so `getModuleRecords` is populated (falls back to a local parse if a module is missing from it). export function createBackendStaticChecksPlugin( buildRoot: string, - log: Logger, + runStaticChecks: RunBackendStaticChecks, getModuleRecords: () => ReadonlyMap, ): Plugin { return { @@ -59,7 +58,7 @@ export function createBackendStaticChecksPlugin( scopeAnalysis = analyzeModuleScope(program); } - runBackendStaticChecks(ast, moduleId, log, scopeAnalysis); + runStaticChecks(ast, moduleId, scopeAnalysis); }, }; } diff --git a/packages/plugins/apps/src/vite/build-backend-functions.test.ts b/packages/plugins/apps/src/vite/build-backend-functions.test.ts index 99b537df7..2cd58452e 100644 --- a/packages/plugins/apps/src/vite/build-backend-functions.test.ts +++ b/packages/plugins/apps/src/vite/build-backend-functions.test.ts @@ -2,6 +2,7 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. +import { createBackendStaticChecks } from '@dd/apps-plugin/backend/ast-parsing/run-backend-static-checks'; import { buildBackendFunctions } from '@dd/apps-plugin/vite/build-backend-functions'; import { existsSync, outputFileSync, readFileSync, rm, rmSync } from '@dd/core/helpers/fs'; import { getTempWorkingDir } from '@dd/tests/_jest/helpers/env'; @@ -55,6 +56,7 @@ describe('buildBackendFunctions', () => { [func], '/project', mockLogger, + createBackendStaticChecks(mockLogger), ), ).rejects.toThrow('static check rejected a reachable helper module'); @@ -78,6 +80,7 @@ describe('buildBackendFunctions', () => { [func], '/project', logger, + createBackendStaticChecks(logger), ), ).rejects.toThrow('static check rejected a reachable helper module'); @@ -102,6 +105,7 @@ describe('buildBackendFunctions', () => { [func], '/project', mockLogger, + createBackendStaticChecks(mockLogger), ), ).rejects.toThrow('static check rejected a reachable helper module'); @@ -139,7 +143,13 @@ describe('buildBackendFunctions', () => { allowedConnectionIds: [], }; - const result = await buildBackendFunctions(build, [namesFunc], workingDir, mockLogger); + const result = await buildBackendFunctions( + build, + [namesFunc], + workingDir, + mockLogger, + createBackendStaticChecks(mockLogger), + ); outDir = result.outDir; const [bundlePath] = [...result.outputs.values()]; const bundleCode = readFileSync(bundlePath); diff --git a/packages/plugins/apps/src/vite/build-backend-functions.ts b/packages/plugins/apps/src/vite/build-backend-functions.ts index 0bf9da2c3..8fd0b04e5 100644 --- a/packages/plugins/apps/src/vite/build-backend-functions.ts +++ b/packages/plugins/apps/src/vite/build-backend-functions.ts @@ -9,6 +9,7 @@ import { tmpdir } from 'os'; import path from 'path'; import type { build } from 'vite'; +import type { RunBackendStaticChecks } from '../backend/ast-parsing/run-backend-static-checks'; import { encodeQueryName } from '../backend/encodeQueryName'; import type { BackendFunction } from '../backend/types'; import { generateVirtualEntryContent } from '../backend/virtual-entry'; @@ -32,6 +33,7 @@ export async function buildBackendFunctions( functions: BackendFunction[], buildRoot: string, log: Logger, + runStaticChecks: RunBackendStaticChecks, ): Promise<{ outDir: string; outputs: Map; functions: BackendFunction[] }> { const tmpRoot = tmpdir(); const outDirPrefix = path.join(tmpRoot, BACKEND_OUT_DIR_PREFIX); @@ -58,7 +60,7 @@ export async function buildBackendFunctions( const staticChecksPlugin = createBackendStaticChecksPlugin( buildRoot, - log, + runStaticChecks, connectionIdCollector.getModuleRecords, ); const baseConfig = getBaseBackendBuildConfig( diff --git a/packages/plugins/apps/src/vite/dev-server-module-graph.test.ts b/packages/plugins/apps/src/vite/dev-server-module-graph.test.ts index 975104397..6c9eb3c21 100644 --- a/packages/plugins/apps/src/vite/dev-server-module-graph.test.ts +++ b/packages/plugins/apps/src/vite/dev-server-module-graph.test.ts @@ -2,7 +2,11 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. -import { getMockLogger } from '@dd/tests/_jest/helpers/mocks'; +import { + createBackendStaticChecks, + type RunBackendStaticChecks, +} from '@dd/apps-plugin/backend/ast-parsing/run-backend-static-checks'; +import { getMockLogger, mockLogFn } from '@dd/tests/_jest/helpers/mocks'; import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import path from 'node:path'; @@ -53,13 +57,13 @@ describe('dev-server-module-graph — collectModuleGraphFromServer', () => { test('Should fail closed, not fall back to the raw specifier, when resolveId fails to resolve a static import', async () => { const server = makeFakeServer(async () => null); - const log = getMockLogger(); + const runStaticChecks = createBackendStaticChecks(getMockLogger()); const collecting = collectModuleGraphFromServer( server, ENTRY_ID, FIXTURE_ROOT, - log, + runStaticChecks, parseAst, ); @@ -72,13 +76,13 @@ describe('dev-server-module-graph — collectModuleGraphFromServer', () => { const resolvedPath = path.join(FIXTURE_ROOT, 'getRuntimeUsers.backend.ts'); const server = makeFakeServer(async () => ({ id: resolvedPath })); - const log = getMockLogger(); + const runStaticChecks = createBackendStaticChecks(getMockLogger()); const records = await collectModuleGraphFromServer( server, ENTRY_ID, FIXTURE_ROOT, - log, + runStaticChecks, parseAst, ); @@ -94,13 +98,13 @@ describe('dev-server-module-graph — collectModuleGraphFromServer', () => { }; const server = makeFakeServer(async () => null, entryNode); - const log = getMockLogger(); + const runStaticChecks = createBackendStaticChecks(getMockLogger()); const collecting = collectModuleGraphFromServer( server, ENTRY_ID, FIXTURE_ROOT, - log, + runStaticChecks, parseAst, ); @@ -129,13 +133,13 @@ describe('dev-server-module-graph — collectModuleGraphFromServer', () => { }; const server = makeFakeServer(async () => null, entryNode); - const log = getMockLogger(); + const runStaticChecks = createBackendStaticChecks(getMockLogger()); const collecting = collectModuleGraphFromServer( server, ENTRY_ID, FIXTURE_ROOT, - log, + runStaticChecks, parseAst, ); @@ -157,13 +161,13 @@ describe('dev-server-module-graph — collectModuleGraphFromServer', () => { }; const server = makeFakeServer(async () => ({ id: rawImportPath }), entryNode); - const log = getMockLogger(); + const runStaticChecks = createBackendStaticChecks(getMockLogger()); const collecting = collectModuleGraphFromServer( server, ENTRY_ID, FIXTURE_ROOT, - log, + runStaticChecks, parseAst, ); @@ -192,13 +196,13 @@ describe('dev-server-module-graph — collectModuleGraphFromServer', () => { }; const server = makeFakeServer(async () => ({ id: sharedFile }), entryNode); - const log = getMockLogger(); + const runStaticChecks = createBackendStaticChecks(getMockLogger()); const collecting = collectModuleGraphFromServer( server, ENTRY_ID, FIXTURE_ROOT, - log, + runStaticChecks, parseAst, ); @@ -218,13 +222,13 @@ describe('dev-server-module-graph — collectModuleGraphFromServer', () => { entryNode.importedModules.add(entryNode); const server = makeFakeServer(async () => ({ id: resolvedPath }), entryNode); - const log = getMockLogger(); + const runStaticChecks = createBackendStaticChecks(getMockLogger()); const records = await collectModuleGraphFromServer( server, ENTRY_ID, FIXTURE_ROOT, - log, + runStaticChecks, parseAst, ); @@ -249,13 +253,13 @@ describe('dev-server-module-graph — collectModuleGraphFromServer', () => { }; const server = makeFakeServer(async () => ({ id: denoHelperPath }), entryNode); - const log = getMockLogger(); + const runStaticChecks = createBackendStaticChecks(getMockLogger()); const collecting = collectModuleGraphFromServer( server, ENTRY_ID, FIXTURE_ROOT, - log, + runStaticChecks, parseAst, ); @@ -263,4 +267,94 @@ describe('dev-server-module-graph — collectModuleGraphFromServer', () => { `"Deno" is not available in the ${NODE_BACKEND_RUNTIME}`, ); }); + + describe('warning on process.env reads', () => { + let tempDir: string; + let envReadingFile: string; + + beforeAll(() => { + const osTempDir = tmpdir(); + const tempDirPrefix = path.join(osTempDir, 'dev-server-module-graph-env-'); + tempDir = mkdtempSync(tempDirPrefix); + envReadingFile = path.join(tempDir, 'charge.ts'); + writeFileSync( + envReadingFile, + 'export function charge(): string | undefined { return process.env.STRIPE_KEY; }', + ); + }); + + afterAll(() => { + rmSync(tempDir, { recursive: true, force: true }); + }); + + let runStaticChecks: RunBackendStaticChecks; + + beforeEach(() => { + runStaticChecks = createBackendStaticChecks(getMockLogger()); + }); + + const collectEnvReadingEntry = () => { + const entryNode: FakeModuleNode = { + id: SUFFIXED_ENTRY_ID, + file: envReadingFile, + importedModules: new Set(), + }; + const server = makeFakeServer(async () => null, entryNode); + return collectModuleGraphFromServer( + server, + ENTRY_ID, + FIXTURE_ROOT, + runStaticChecks, + parseAst, + ); + }; + + const warnLogs = () => + mockLogFn.mock.calls + .filter(([, level]) => level === 'warn') + .map(([message]) => String(message)); + + test('Should warn about a process.env read once', async () => { + await collectEnvReadingEntry(); + + const warnings = warnLogs(); + expect(warnings).toEqual([expect.stringContaining('process.env.STRIPE_KEY')]); + }); + + test('Should not warn about a node_modules dependency that reads process.env', async () => { + const cleanEntryFile = path.join(tempDir, 'entry.ts'); + writeFileSync(cleanEntryFile, 'export function entry(): number { return 1; }'); + const dependencyId = path.join(FIXTURE_ROOT, 'node_modules/stripe-config/index.ts'); + const dependencyNode: FakeModuleNode = { + id: dependencyId, + file: envReadingFile, + importedModules: new Set(), + }; + const entryNode: FakeModuleNode = { + id: SUFFIXED_ENTRY_ID, + file: cleanEntryFile, + importedModules: new Set([dependencyNode]), + }; + const server = makeFakeServer(async () => null, entryNode); + + await collectModuleGraphFromServer( + server, + ENTRY_ID, + FIXTURE_ROOT, + runStaticChecks, + parseAst, + ); + + const warnings = warnLogs(); + expect(warnings).toEqual([]); + }); + + test('Should not warn again when a later local run collects the same module', async () => { + await collectEnvReadingEntry(); + await collectEnvReadingEntry(); + + const warnings = warnLogs(); + expect(warnings).toHaveLength(1); + }); + }); }); diff --git a/packages/plugins/apps/src/vite/dev-server-module-graph.ts b/packages/plugins/apps/src/vite/dev-server-module-graph.ts index 543592085..7a7de82aa 100644 --- a/packages/plugins/apps/src/vite/dev-server-module-graph.ts +++ b/packages/plugins/apps/src/vite/dev-server-module-graph.ts @@ -5,7 +5,6 @@ /* eslint-disable no-await-in-loop */ import { readFile } from '@dd/core/helpers/fs'; -import type { Logger } from '@dd/core/types'; import { transform } from 'esbuild'; import type { ModuleNode, parseAst, ViteDevServer } from 'vite'; @@ -15,7 +14,7 @@ import { resolveStaticModuleSources, unsupportedModuleGraphDependency, } from '../backend/ast-parsing/module-graph'; -import { runBackendStaticChecks } from '../backend/ast-parsing/run-backend-static-checks'; +import type { RunBackendStaticChecks } from '../backend/ast-parsing/run-backend-static-checks'; import { isBackendSourceModule } from '../backend/backend-sources'; import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; @@ -31,11 +30,11 @@ export async function collectModuleGraphFromServer( server: ViteDevServer, bareEntryId: string, buildRoot: string, - log: Logger, + runStaticChecks: RunBackendStaticChecks, parse: typeof parseAst, ): Promise> { const records = new Map(); - await walkModuleGraph(server, bareEntryId, buildRoot, log, parse, records); + await walkModuleGraph(server, bareEntryId, buildRoot, runStaticChecks, parse, records); return records; } @@ -43,7 +42,7 @@ async function walkModuleGraph( server: ViteDevServer, bareEntryId: string, buildRoot: string, - log: Logger, + runStaticChecks: RunBackendStaticChecks, parse: typeof parseAst, records: Map, ): Promise { @@ -139,7 +138,7 @@ async function walkModuleGraph( if (record) { // No build-time moduleParsed hook here (Rollup-only), so this is what catches a // restricted global locally instead of only at publish time. - runBackendStaticChecks(record.ast, record.id, log, record.scopeAnalysis); + runStaticChecks(record.ast, record.id, record.scopeAnalysis); records.set(record.id, record); } 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 9b8dc2d10..96bbdcab6 100644 --- a/packages/plugins/apps/src/vite/dev-server.integration.test.ts +++ b/packages/plugins/apps/src/vite/dev-server.integration.test.ts @@ -10,6 +10,7 @@ */ import { getAuthenticatedRequest } from '@dd/apps-plugin/auth'; +import { createBackendStaticChecks } from '@dd/apps-plugin/backend/ast-parsing/run-backend-static-checks'; import { buildBackendFunctions } from '@dd/apps-plugin/vite/build-backend-functions'; import { collectModuleGraphFromServer } from '@dd/apps-plugin/vite/dev-server-module-graph'; import { createViteHostCheck } from '@dd/apps-plugin/vite/dev-server-request-guard'; @@ -249,6 +250,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { mockLongPolling, FIXTURE_ROOT, getMockLogger(), + createBackendStaticChecks(getMockLogger()), 'development', isHostAllowed, defaultServerConditions, @@ -295,6 +297,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { mockLongPolling, FIXTURE_ROOT, getMockLogger(), + createBackendStaticChecks(getMockLogger()), 'development', isHostAllowed, defaultServerConditions, @@ -335,6 +338,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { mockLongPolling, FIXTURE_ROOT, getMockLogger(), + createBackendStaticChecks(getMockLogger()), 'development', isHostAllowed, defaultServerConditions, @@ -422,12 +426,11 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { // collectModuleGraphFromServer appends LOCAL_EXECUTION_LOAD_SUFFIX internally, so this // closure only handles the bare id — matching vite/index.ts's real wiring. const getAllowedConnectionIds = async (entryId: string) => { - const log = getMockLogger(); const moduleGraph = await collectModuleGraphFromServer( server, entryId, FIXTURE_ROOT, - log, + createBackendStaticChecks(getMockLogger()), parseAst, ); return extractConnectionIdsFromModuleGraph(entryId, moduleGraph, FIXTURE_ROOT); @@ -448,6 +451,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { mockLongPolling, FIXTURE_ROOT, getMockLogger(), + createBackendStaticChecks(getMockLogger()), 'development', isHostAllowed, defaultServerConditions, @@ -474,12 +478,11 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { // must read each module's original source from disk instead of the transformed output. test('Should recognize a connectionId-scoped action-catalog call and allow it, not silently reject it', async () => { const getAllowedConnectionIds = async (entryId: string) => { - const log = getMockLogger(); const moduleGraph = await collectModuleGraphFromServer( server, entryId, FIXTURE_ROOT, - log, + createBackendStaticChecks(getMockLogger()), parseAst, ); return extractConnectionIdsFromModuleGraph(entryId, moduleGraph, FIXTURE_ROOT); @@ -500,6 +503,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { mockLongPolling, FIXTURE_ROOT, getMockLogger(), + createBackendStaticChecks(getMockLogger()), 'development', isHostAllowed, defaultServerConditions, @@ -542,6 +546,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { mockLongPolling, FIXTURE_ROOT, log, + createBackendStaticChecks(log), 'development', createViteHostCheck(server.config), defaultServerConditions, @@ -570,12 +575,11 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { // itself rather than node.importedModules's undocumented ordering for mixed imports. test('Should recognize a connectionId-scoped action-catalog call even when a top-level dynamic import sits between two static imports', async () => { const getAllowedConnectionIds = async (entryId: string) => { - const log = getMockLogger(); const moduleGraph = await collectModuleGraphFromServer( server, entryId, FIXTURE_ROOT, - log, + createBackendStaticChecks(getMockLogger()), parseAst, ); return extractConnectionIdsFromModuleGraph(entryId, moduleGraph, FIXTURE_ROOT); @@ -596,6 +600,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { mockLongPolling, FIXTURE_ROOT, getMockLogger(), + createBackendStaticChecks(getMockLogger()), 'development', isHostAllowed, defaultServerConditions, @@ -642,7 +647,13 @@ describe('Dev Server Middleware — dev:verify bundles', () => { async function buildUploadAndVerify(workingDir: string, func: BackendFunction) { const log = getMockLogger(); process.env.NODE_ENV = 'production'; - const upload = await buildBackendFunctions(build, [func], workingDir, log); + const upload = await buildBackendFunctions( + build, + [func], + workingDir, + log, + createBackendStaticChecks(log), + ); let uploadCode: string; try { const [uploadPath] = [...upload.outputs.values()]; @@ -671,6 +682,7 @@ describe('Dev Server Middleware — dev:verify bundles', () => { mockLongPolling, workingDir, log, + createBackendStaticChecks(log), DEV_VERIFY_MODE, isHostAllowed, defaultServerConditions, diff --git a/packages/plugins/apps/src/vite/dev-server.test.ts b/packages/plugins/apps/src/vite/dev-server.test.ts index 52cf8212f..98cdab93f 100644 --- a/packages/plugins/apps/src/vite/dev-server.test.ts +++ b/packages/plugins/apps/src/vite/dev-server.test.ts @@ -3,6 +3,7 @@ // Copyright 2019-Present Datadog, Inc. import { getAuthenticatedRequest } from '@dd/apps-plugin/auth'; +import { createBackendStaticChecks } from '@dd/apps-plugin/backend/ast-parsing/run-backend-static-checks'; import { buildBackendFunctions } from '@dd/apps-plugin/vite/build-backend-functions'; import { createViteHostCheck } from '@dd/apps-plugin/vite/dev-server-request-guard'; import { @@ -317,10 +318,11 @@ function createTestMiddleware( longPolling?: CreateDevServerMiddlewareArgs[5]; projectRoot?: CreateDevServerMiddlewareArgs[6]; log?: CreateDevServerMiddlewareArgs[7]; - mode?: CreateDevServerMiddlewareArgs[8]; - isHostAllowed?: CreateDevServerMiddlewareArgs[9]; - serverConditions?: CreateDevServerMiddlewareArgs[10]; - executeLocally?: CreateDevServerMiddlewareArgs[11]; + runStaticChecks?: CreateDevServerMiddlewareArgs[8]; + mode?: CreateDevServerMiddlewareArgs[9]; + isHostAllowed?: CreateDevServerMiddlewareArgs[10]; + serverConditions?: CreateDevServerMiddlewareArgs[11]; + executeLocally?: CreateDevServerMiddlewareArgs[12]; } = {}, ): ReturnType { return createDevServerMiddleware( @@ -334,6 +336,7 @@ function createTestMiddleware( overrides.longPolling ?? mockLongPolling, overrides.projectRoot ?? '/project', overrides.log ?? mockLog, + overrides.runStaticChecks ?? createBackendStaticChecks(overrides.log ?? mockLog), overrides.mode ?? 'development', overrides.isHostAllowed ?? localOnlyHostCheck, overrides.serverConditions ?? defaultServerConditions, @@ -824,7 +827,13 @@ describe('Dev Server Middleware', () => { await emitModuleParsed(config, func.absolutePath, source); return { output: [] }; }); - const upload = await buildBackendFunctions(uploadBuild, [func], '/project', mockLog); + const upload = await buildBackendFunctions( + uploadBuild, + [func], + '/project', + mockLog, + createBackendStaticChecks(mockLog), + ); rmSync(upload.outDir); mockBuildWithParsedBackend(); const functionName = encodeQueryName(func); @@ -1698,6 +1707,7 @@ describe('Dev Server Middleware', () => { mockLongPolling, '/project', mockLog, + createBackendStaticChecks(mockLog), 'development', localOnlyHostCheck, defaultServerConditions, diff --git a/packages/plugins/apps/src/vite/dev-server.ts b/packages/plugins/apps/src/vite/dev-server.ts index d328429b8..a9cadb087 100644 --- a/packages/plugins/apps/src/vite/dev-server.ts +++ b/packages/plugins/apps/src/vite/dev-server.ts @@ -12,6 +12,7 @@ import type { build } from 'vite'; import { AUTH_GUIDANCE } from '../auth'; import type { DoAuthenticatedRequest } from '../auth'; +import type { RunBackendStaticChecks } from '../backend/ast-parsing/run-backend-static-checks'; import { encodeQueryName } from '../backend/encodeQueryName'; import { DEBUG_BUNDLE_PATH } from '../backend/protocol'; import type { @@ -146,6 +147,7 @@ async function bundleBackendFunction( func: BackendFunction, projectRoot: string, log: Logger, + runStaticChecks: RunBackendStaticChecks, ): Promise { const displayName = formatRef(func); const virtualId = `${DEV_VIRTUAL_PREFIX}${displayName}`; @@ -163,7 +165,7 @@ async function bundleBackendFunction( const staticChecksPlugin = createBackendStaticChecksPlugin( projectRoot, - log, + runStaticChecks, connectionIdCollector.getModuleRecords, ); const devBaseConfig = getBaseBackendBuildConfig(projectRoot, { [virtualId]: virtualContent }, [ @@ -832,13 +834,14 @@ export function createDevServerMiddleware( longPolling: LongPollingConfig, projectRoot: string, log: Logger, + runStaticChecks: RunBackendStaticChecks, mode: string, isHostAllowed: HostCheck, serverConditions: readonly string[], executeLocally?: LocalExecutor, ): (req: IncomingMessage, res: ServerResponse, next: () => void) => void { const bundle = (func: BackendFunction) => - bundleBackendFunction(viteBuild, serverConditions, func, projectRoot, log); + bundleBackendFunction(viteBuild, serverConditions, func, projectRoot, log, runStaticChecks); const isDevVerifyMode = mode === DEV_VERIFY_MODE; const oversizedFunctionsWarned = new Set(); diff --git a/packages/plugins/apps/src/vite/index.test.ts b/packages/plugins/apps/src/vite/index.test.ts index 053975560..5093e69df 100644 --- a/packages/plugins/apps/src/vite/index.test.ts +++ b/packages/plugins/apps/src/vite/index.test.ts @@ -625,8 +625,10 @@ describe('Backend Functions - getVitePlugin', () => { expect(authSpy).not.toHaveBeenCalled(); }); - const transformBackendFile = (code: string) => { - const plugin = getVitePlugin(defaultOptions); + const transformBackendFile = ( + code: string, + plugin: ReturnType = getVitePlugin(defaultOptions), + ) => { const handler = getTransformHandler(plugin); const resolveMock = jest.fn(async () => null); const loadMock = jest.fn(async () => null); @@ -662,6 +664,41 @@ describe('Backend Functions - getVitePlugin', () => { ).toThrow(`"Deno" is not available in the ${NODE_BACKEND_RUNTIME}`); }); + test('Should warn about a process.env read once per plugin instance', async () => { + const code = 'export function myHandler() { return process.env.STRIPE_KEY; }'; + const firstPlugin = getVitePlugin(defaultOptions); + const secondPlugin = getVitePlugin(defaultOptions); + + await transformBackendFile(code, firstPlugin); + await transformBackendFile(code, firstPlugin); + await transformBackendFile(code, secondPlugin); + + const warnings = mockLogFn.mock.calls.filter( + ([message, level]) => level === 'warn' && String(message).includes('process.env'), + ); + expect(warnings).toHaveLength(2); + }); + + test('Should warn about a process.env read once across the transform and the backend builds', async () => { + const code = + 'export function myHandler() { return process.env.STRIPE_KEY; } export function otherFunc() {}'; + mockViteBuild.mockImplementation(async (config) => { + await emitModuleParsed(config, '/build/src/backend/myHandler.backend.ts', code); + return mockBuildResult(); + }); + const plugin = getVitePlugin(defaultOptions); + + await transformBackendFile(code, plugin); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + await (plugin as any).closeBundle(); + + expect(mockViteBuild).toHaveBeenCalledTimes(2); + const warnings = mockLogFn.mock.calls.filter( + ([message, level]) => level === 'warn' && String(message).includes('process.env'), + ); + expect(warnings).toHaveLength(1); + }); + // Regression test: without the suffix check, ssrLoadModule() would get the proxy stub instead of the real function body. test('Should skip proxy generation for a suffixed local-execution load made from SSR context, returning the real source untouched', async () => { const plugin = getVitePlugin(defaultOptions); diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index 863124a2d..6d91ee21b 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -19,7 +19,7 @@ import { import { extractExportedFunctions } from '../backend/ast-parsing/extract-backend-functions'; import { extractConnectionIdsFromModuleGraph } from '../backend/ast-parsing/extract-connection-ids-from-module-graph'; import { analyzeModuleScope } from '../backend/ast-parsing/module-scope'; -import { runBackendStaticChecks } from '../backend/ast-parsing/run-backend-static-checks'; +import { createBackendStaticChecks } from '../backend/ast-parsing/run-backend-static-checks'; import { ensureProgram } from '../backend/ast-parsing/type-guards'; import { findInstalledBackendFunctionPackages, @@ -234,6 +234,7 @@ export const getVitePlugin = ({ options, }: VitePluginOptions): PluginOptions['vite'] => { const log = context.getLogger(PLUGIN_NAME); + const runStaticChecks = createBackendStaticChecks(log); const { auth } = context; context.inject({ @@ -515,7 +516,7 @@ export const getVitePlugin = ({ const program = ensureProgram(ast, normalizedId); const scopeAnalysis = analyzeModuleScope(program); // Runs even for a file with zero exports, to catch a restricted global as soon as it's written. - runBackendStaticChecks(ast, normalizedId, log, scopeAnalysis); + runStaticChecks(ast, normalizedId, scopeAnalysis); const exportNames = extractExportedFunctions(ast, normalizedId); if (exportNames.length === 0) { // Only a genuinely no-query id can be trusted as a real re-transform of this @@ -634,6 +635,7 @@ export const getVitePlugin = ({ backendFunctions, context.buildRoot, log, + runStaticChecks, ); backendOutDir = result.outDir; backendOutputs = result.outputs; @@ -739,7 +741,7 @@ export const getVitePlugin = ({ server, entryId, context.buildRoot, - log, + runStaticChecks, parseAst, ); return extractConnectionIdsFromModuleGraph(entryId, moduleGraph, context.buildRoot); @@ -754,6 +756,7 @@ export const getVitePlugin = ({ options.longPolling, context.buildRoot, log, + runStaticChecks, server.config.mode, isHostAllowed, bundler.defaultServerConditions,