diff --git a/packages/plugins/apps/package.json b/packages/plugins/apps/package.json index 2a737b1e2..978bf8414 100644 --- a/packages/plugins/apps/package.json +++ b/packages/plugins/apps/package.json @@ -32,12 +32,15 @@ "dependencies": { "@dd/core": "workspace:*", "chalk": "2.3.1", + "eslint-scope": "7.2.2", "glob": "11.1.0", "jszip": "3.10.1", "pretty-bytes": "5.6.0" }, "devDependencies": { + "@types/eslint-scope": "3.7.7", "@types/estree": "1.0.8", + "rollup": "4.45.1", "typescript": "5.4.3", "vite": "6.3.5" } diff --git a/packages/plugins/apps/src/backend/ast-parsing/action-catalog-call-sites.ts b/packages/plugins/apps/src/backend/ast-parsing/action-catalog-call-sites.ts new file mode 100644 index 000000000..46e9ed946 --- /dev/null +++ b/packages/plugins/apps/src/backend/ast-parsing/action-catalog-call-sites.ts @@ -0,0 +1,467 @@ +// 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 eslintScope from 'eslint-scope'; +import type { + AssignmentExpression, + Identifier, + MemberExpression, + Node, + Pattern, + Program, + SimpleCallExpression, + VariableDeclarator, +} from 'estree'; + +import type { ActionCatalogImports } from './action-catalog-imports'; +import { walkAst } from './walk-ast'; + +// Do not trust names alone when deciding whether `request(...)` is an +// action-catalog call. A local parameter or variable can reuse the same name: +// +// import { request } from '@datadog/action-catalog/http/http'; +// request({ connectionId: 'real' }); +// +// function run(request) { +// request({ connectionId: 'local' }); +// } +// +// Both calls use the text `request`, but only the first one refers to the +// imported action-catalog function. eslint-scope tells us which declaration each +// identifier refers to, and ScopeAnalysis keeps the lookup tables we need while +// walking the file. +export interface ScopeAnalysis { + // The full scope model from eslint-scope, used when we need declared + // variables for aliases like `const action = request`. + scopeManager: eslintScope.ScopeManager; + + // Maps each identifier node to the declaration eslint-scope resolved it to. + references: Map; + + // The actual import variables for action-catalog functions and namespaces. + // Call sites must resolve to one of these variables to count. + actionFunctions: Set; + actionNamespaces: Set; +} + +interface ActionCatalogCallState { + scopeAnalysis: ScopeAnalysis; + unsupportedAliases: Set; +} + +type NodeWithRange = Node & { start?: number; end?: number; range?: [number, number] }; + +export function analyzeActionCatalogScopes( + ast: Program, + imports: ActionCatalogImports, +): ScopeAnalysis { + ensureRanges(ast); + const scopeManager = eslintScope.analyze(ast, { + ecmaVersion: 2022, + ignoreEval: true, + sourceType: 'module', + }); + + const references = new Map(); + const actionFunctions = new Set(); + const actionNamespaces = new Set(); + + // Cache every identifier reference so call classification can ask "what + // variable does this exact node resolve to?" without re-walking scopes. + for (const scope of scopeManager.scopes) { + for (const reference of scope.references) { + references.set(reference.identifier, reference); + } + + // Save the actual import declarations that came from action-catalog. + // Later, when we see `request(...)`, we check whether that `request` + // points back to one of these declarations instead of to a local + // parameter or variable with the same name. + for (const variable of scope.variables) { + if (!isImportVariable(variable)) { + continue; + } + if (imports.functions.has(variable.name)) { + actionFunctions.add(variable); + } + if (imports.namespaces.has(variable.name)) { + actionNamespaces.add(variable); + } + } + } + + return { scopeManager, references, actionFunctions, actionNamespaces }; +} + +export function findActionCatalogCallSites( + ast: Program, + scopeAnalysis: ScopeAnalysis, + filePath: string, +): SimpleCallExpression[] { + const callState: ActionCatalogCallState = { + scopeAnalysis, + unsupportedAliases: collectUnsupportedActionCatalogAliases(ast, scopeAnalysis), + }; + const callSites: SimpleCallExpression[] = []; + + walkAst(ast, callState, { + CallExpression(node, { state }) { + if (classifyActionCatalogCall(node, state, filePath)) { + callSites.push(node); + } + }, + }); + + return callSites; +} + +export function resolveIdentifier( + identifier: Identifier, + scopeAnalysis: ScopeAnalysis, +): eslintScope.Variable | undefined { + return scopeAnalysis.references.get(identifier)?.resolved ?? undefined; +} + +export function isImportVariable(variable: eslintScope.Variable): boolean { + return variable.defs.some((definition) => definition.type === 'ImportBinding'); +} + +function classifyActionCatalogCall( + node: SimpleCallExpression, + state: ActionCatalogCallState, + filePath: string, +): boolean { + const callee = node.callee; + + if (callee.type === 'Identifier') { + if (resolvesTo(callee, state.unsupportedAliases, state.scopeAnalysis)) { + throw unsupportedActionCatalogCall(filePath, 'action-catalog call aliases'); + } + if (resolvesTo(callee, state.scopeAnalysis.actionFunctions, state.scopeAnalysis)) { + if (node.optional) { + throw unsupportedActionCatalogCall(filePath, 'optional action-catalog calls'); + } + return true; + } + return false; + } + + if (callee.type !== 'MemberExpression') { + return false; + } + + if (!isNamespaceMember(callee, state.scopeAnalysis)) { + return false; + } + + if (node.optional || hasUnsupportedMemberAccess(callee)) { + throw unsupportedActionCatalogCall( + filePath, + 'optional or computed action-catalog namespace calls', + ); + } + return true; +} + +function collectUnsupportedActionCatalogAliases( + ast: Program, + scopeAnalysis: ScopeAnalysis, +): Set { + const unsupportedAliases = new Set(); + + walkAst(ast, scopeAnalysis, { + VariableDeclarator(node, { state }) { + for (const aliasVariable of getActionCatalogAliasVariables(node, state)) { + unsupportedAliases.add(aliasVariable); + } + }, + AssignmentExpression(node, { state }) { + for (const aliasVariable of getAssignedActionCatalogAliasVariables(node, state)) { + unsupportedAliases.add(aliasVariable); + } + }, + }); + + return unsupportedAliases; +} + +/** + * Finds variables declared as aliases of an action-catalog function. + * + * Examples this catches: + * - `const action = request` + * - `const action = http.request` + * - `const { request: action } = http` + * + * We do not try to follow these aliases. Instead, we mark them as unsupported + * so a later `action(...)` call fails closed instead of silently missing a + * `connectionId`. + */ +function getActionCatalogAliasVariables( + node: VariableDeclarator, + scopeAnalysis: ScopeAnalysis, +): eslintScope.Variable[] { + // `const action = request` + if ( + node.id.type === 'Identifier' && + node.init?.type === 'Identifier' && + resolvesTo(node.init, scopeAnalysis.actionFunctions, scopeAnalysis) + ) { + return getDeclaredVariables(node, scopeAnalysis, [node.id.name]); + } + + // `const action = http.request` + if ( + node.id.type === 'Identifier' && + node.init?.type === 'MemberExpression' && + isNamespaceMember(node.init, scopeAnalysis) + ) { + return getDeclaredVariables(node, scopeAnalysis, [node.id.name]); + } + + // Ignore declarations that are not destructuring an action-catalog namespace, + // then handle `const { request: action } = http` below. + if ( + node.id.type !== 'ObjectPattern' || + node.init?.type !== 'Identifier' || + !resolvesTo(node.init, scopeAnalysis.actionNamespaces, scopeAnalysis) + ) { + return []; + } + + // In a declaration, eslint-scope can give us declared variables from the + // whole `const { request: action } = http` node. We only need identifier + // names to pick the alias variables out of that declaration result. + const aliasNames = node.id.properties + .flatMap((property) => { + if (property.type === 'RestElement' || property.computed) { + return []; + } + return collectPatternIdentifiers(property.value); + }) + .map((identifier) => identifier.name); + return getDeclaredVariables(node, scopeAnalysis, aliasNames); +} + +/** + * Finds existing variables that are assigned an action-catalog function after + * they have already been declared. + * + * Examples this catches: + * - `let action; action = request` + * - `let action; action = http.request` + * - `let action; ({ request: action } = http)` + * + * These are the assignment-expression versions of the declarations handled by + * `getActionCatalogAliasVariables`. + */ +function getAssignedActionCatalogAliasVariables( + node: AssignmentExpression, + scopeAnalysis: ScopeAnalysis, +): eslintScope.Variable[] { + // `let action; action = request` + if ( + node.left.type === 'Identifier' && + node.right.type === 'Identifier' && + resolvesTo(node.right, scopeAnalysis.actionFunctions, scopeAnalysis) + ) { + return getResolvedVariables([node.left], scopeAnalysis); + } + + // `let action; action = http.request` + if ( + node.left.type === 'Identifier' && + node.right.type === 'MemberExpression' && + isNamespaceMember(node.right, scopeAnalysis) + ) { + return getResolvedVariables([node.left], scopeAnalysis); + } + + // Ignore assignments that are not destructuring an action-catalog namespace, + // then handle `let action; ({ request: action } = http)` below. + if ( + node.left.type !== 'ObjectPattern' || + node.right.type !== 'Identifier' || + !resolvesTo(node.right, scopeAnalysis.actionNamespaces, scopeAnalysis) + ) { + return []; + } + + // In an assignment, the variables already exist. Keep the actual identifier + // nodes so eslint-scope can resolve each one back to the existing variable. + const aliasIdentifiers = node.left.properties.flatMap((property) => { + if (property.type === 'RestElement' || property.computed) { + return []; + } + return collectPatternIdentifiers(property.value); + }); + return getResolvedVariables(aliasIdentifiers, scopeAnalysis); +} + +/** + * Returns true when a property access starts from an imported action-catalog + * namespace. + * + * For `http.request(...)`, this checks that `http` is the namespace imported by + * `import * as http from '@datadog/action-catalog/...'`, not a local variable + * that happens to be named `http`. + */ +function isNamespaceMember(node: MemberExpression, scopeAnalysis: ScopeAnalysis): boolean { + const root = getMemberExpressionRoot(node); + return !!root && resolvesTo(root, scopeAnalysis.actionNamespaces, scopeAnalysis); +} + +/** + * Returns the left-most identifier in a property access chain. + * + * Examples: + * - `http.request` -> `http` + * - `catalog.http.request` -> `catalog` + * + * The root name is what scope analysis can resolve back to an import or local + * declaration. + */ +function getMemberExpressionRoot(node: MemberExpression): Identifier | undefined { + if (node.object.type === 'Identifier') { + return node.object; + } + if (node.object.type === 'MemberExpression') { + return getMemberExpressionRoot(node.object); + } + return undefined; +} + +/** + * Detects namespace call shapes we intentionally do not support. + * + * We only support direct, non-optional property access like `http.request(...)`. + * Computed or optional forms such as `http['request'](...)` and + * `http?.request(...)` could hide what action is called, so they fail closed. + */ +function hasUnsupportedMemberAccess(node: MemberExpression): boolean { + if (node.optional || node.computed) { + return true; + } + return node.object.type === 'MemberExpression' && hasUnsupportedMemberAccess(node.object); +} + +/** + * Checks whether an identifier points to one of the exact variables we care + * about. + * + * This is the shadowing-safe comparison. For example, a local function + * parameter named `request` has the same text as an imported `request`, but + * eslint-scope resolves it to a different variable. + * + * @param identifier - The exact identifier node from the AST, such as the + * `request` in `request(...)`. + * @param variables - The set of allowed target variables, such as the imported + * action-catalog function declarations. + * @param scopeAnalysis - The precomputed eslint-scope lookup tables that map + * identifier nodes back to the variables they reference. + */ +function resolvesTo( + identifier: Identifier, + variables: ReadonlySet, + scopeAnalysis: ScopeAnalysis, +): boolean { + // eslint-scope has already resolved this Identifier to the declaration it + // refers to. Comparing Variable identity is what makes shadowing safe. + const reference = scopeAnalysis.references.get(identifier); + return !!reference?.resolved && variables.has(reference.resolved); +} + +/** + * Converts identifier nodes into the variables they refer to. + * + * This is used for assignment aliases because the variable already exists: + * `action = request` does not declare `action`, it only assigns to it. We ask + * eslint-scope which existing variable that `action` identifier points to. + */ +function getResolvedVariables( + identifiers: Identifier[], + scopeAnalysis: ScopeAnalysis, +): eslintScope.Variable[] { + return identifiers.flatMap((identifier) => { + const variable = resolveIdentifier(identifier, scopeAnalysis); + return variable ? [variable] : []; + }); +} + +/** + * Returns variables created by a declaration node, limited to the names we + * extracted from the declaration pattern. + * + * This is used for alias declarations like `const action = request`, where the + * declaration itself creates the `action` variable we need to remember. + */ +function getDeclaredVariables( + node: Node, + scopeAnalysis: ScopeAnalysis, + names: string[], +): eslintScope.Variable[] { + // Alias declarations are tracked as Variables too, so later `action(...)` + // calls can fail closed only when they resolve to the alias we identified. + const wantedNames = new Set(names); + return scopeAnalysis.scopeManager + .getDeclaredVariables(node) + .filter((variable) => wantedNames.has(variable.name)); +} + +/** + * Pulls identifier nodes out of a declaration or assignment pattern. + * + * Patterns are the left side of declarations or assignments, such as: + * - `const action = ...` + * - `const { request: action } = ...` + * - `({ request: action } = ...)` + * + * Declarations use the identifier names to filter variables created by the + * declaration. Assignments use the identifier nodes directly, because + * eslint-scope resolves those nodes to existing variables. + */ +function collectPatternIdentifiers(pattern: Pattern): Identifier[] { + switch (pattern.type) { + case 'Identifier': + return [pattern]; + case 'ObjectPattern': + return pattern.properties.flatMap((property) => { + if (property.type === 'RestElement') { + return collectPatternIdentifiers(property.argument); + } + return collectPatternIdentifiers(property.value); + }); + case 'ArrayPattern': + return pattern.elements.flatMap((element) => + element ? collectPatternIdentifiers(element) : [], + ); + case 'RestElement': + return collectPatternIdentifiers(pattern.argument); + case 'AssignmentPattern': + return collectPatternIdentifiers(pattern.left); + case 'MemberExpression': + return []; + } +} + +function ensureRanges(node: Node): void { + walkAst(node, null, { + _(child) { + const nodeWithRange = child as NodeWithRange; + if ( + !nodeWithRange.range && + typeof nodeWithRange.start === 'number' && + typeof nodeWithRange.end === 'number' + ) { + nodeWithRange.range = [nodeWithRange.start, nodeWithRange.end]; + } + }, + }); +} + +function unsupportedActionCatalogCall(filePath: string, unsupported: string): Error { + return new Error( + `Unsupported action-catalog call in ${filePath}: ${unsupported} could hide a connectionId.`, + ); +} diff --git a/packages/plugins/apps/src/backend/ast-parsing/action-catalog-imports.ts b/packages/plugins/apps/src/backend/ast-parsing/action-catalog-imports.ts new file mode 100644 index 000000000..83f994b39 --- /dev/null +++ b/packages/plugins/apps/src/backend/ast-parsing/action-catalog-imports.ts @@ -0,0 +1,57 @@ +// 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 { BaseNode, Program } from 'estree'; + +const ACTION_CATALOG_PACKAGE = '@datadog/action-catalog'; + +export interface ActionCatalogImports { + functions: Set; + namespaces: Set; +} + +type NodeWithOptionalImportKind = BaseNode & { importKind?: string }; + +export function collectActionCatalogImports(ast: Program): ActionCatalogImports { + const functions = new Set(); + const namespaces = new Set(); + + for (const node of ast.body) { + if (node.type !== 'ImportDeclaration' || !isActionCatalogSource(node.source.value)) { + continue; + } + if (isTypeOnly(node)) { + continue; + } + + for (const specifier of node.specifiers) { + if (isTypeOnly(specifier)) { + continue; + } + + if (specifier.type === 'ImportNamespaceSpecifier') { + namespaces.add(specifier.local.name); + } else { + functions.add(specifier.local.name); + } + } + } + + return { functions, namespaces }; +} + +export function hasActionCatalogImports(imports: ActionCatalogImports): boolean { + return imports.functions.size > 0 || imports.namespaces.size > 0; +} + +function isActionCatalogSource(source: unknown): boolean { + return ( + typeof source === 'string' && + (source === ACTION_CATALOG_PACKAGE || source.startsWith(`${ACTION_CATALOG_PACKAGE}/`)) + ); +} + +function isTypeOnly(node: NodeWithOptionalImportKind): boolean { + return node.importKind === 'type'; +} diff --git a/packages/plugins/apps/src/backend/ast-parsing/connection-id-values.ts b/packages/plugins/apps/src/backend/ast-parsing/connection-id-values.ts new file mode 100644 index 000000000..139a997e4 --- /dev/null +++ b/packages/plugins/apps/src/backend/ast-parsing/connection-id-values.ts @@ -0,0 +1,73 @@ +// 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 { ObjectExpression, Property, SimpleCallExpression } from 'estree'; + +const CONNECTION_ID_PROPERTY = 'connectionId'; + +export function extractConnectionIdFromActionCall( + node: SimpleCallExpression, + filePath: string, +): string | undefined { + const [firstArg] = node.arguments; + if (!firstArg || firstArg.type !== 'ObjectExpression') { + throw unsupportedActionCatalogCall(filePath, 'non-object action-catalog call arguments'); + } + + const connectionIdProperty = findConnectionIdProperty(firstArg, filePath); + if (!connectionIdProperty) { + return undefined; + } + + const { value } = connectionIdProperty; + if (value.type === 'Literal' && typeof value.value === 'string') { + return value.value; + } + + throw unsupportedConnectionId(filePath, value.type); +} + +function findConnectionIdProperty( + objectExpression: ObjectExpression, + filePath: string, +): Property | undefined { + let connectionIdProperty: Property | undefined; + for (const property of objectExpression.properties) { + if (property.type === 'SpreadElement') { + throw unsupportedActionCatalogCall(filePath, 'spread object arguments'); + } + if (property.computed) { + throw unsupportedActionCatalogCall(filePath, 'computed object property keys'); + } + if (isConnectionIdKey(property)) { + if (connectionIdProperty) { + throw unsupportedActionCatalogCall(filePath, 'multiple connectionId properties'); + } + if (property.kind !== 'init') { + throw unsupportedActionCatalogCall(filePath, 'accessor connectionId properties'); + } + connectionIdProperty = property; + } + } + return connectionIdProperty; +} + +function isConnectionIdKey(property: Property): boolean { + if (property.key.type === 'Identifier') { + return property.key.name === CONNECTION_ID_PROPERTY; + } + return property.key.type === 'Literal' && property.key.value === CONNECTION_ID_PROPERTY; +} + +function unsupportedActionCatalogCall(filePath: string, unsupported: string): Error { + return new Error( + `Unsupported action-catalog call in ${filePath}: ${unsupported} could hide a connectionId.`, + ); +} + +function unsupportedConnectionId(filePath: string, type: string): Error { + return new Error( + `Unsupported action-catalog connectionId in ${filePath}: expected an inline string literal, got ${type}.`, + ); +} diff --git a/packages/plugins/apps/src/backend/ast-parsing/extract-backend-functions.ts b/packages/plugins/apps/src/backend/ast-parsing/extract-backend-functions.ts index ac60c5664..f77263f93 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/extract-backend-functions.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/extract-backend-functions.ts @@ -2,8 +2,7 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. -import type { Declaration, Expression, Program } from 'estree'; -import type { AstNode } from 'rollup'; +import type { BaseNode, Declaration, Expression, Program } from 'estree'; import { isProgramNode } from './type-guards'; import type { BackendExport } from './types'; @@ -16,14 +15,14 @@ import type { BackendExport } from './types'; * Throws on invalid exports (e.g. default exports) and unexpected AST shapes. * Returns an empty array when the file has no named exports. * - * @param ast - AstNode from `this.parse()` in unplugin's transform hook + * @param ast - ESTree node from `this.parse()` in unplugin's transform hook * @param filePath - Path to the source file (used in error messages) */ -export function extractExportedFunctions(ast: AstNode, filePath: string): string[] { +export function extractExportedFunctions(ast: BaseNode, filePath: string): string[] { return enumerateBackendExports(ast, filePath).map((backendExport) => backendExport.name); } -export function enumerateBackendExports(ast: AstNode, filePath: string): BackendExport[] { +export function enumerateBackendExports(ast: BaseNode, filePath: string): BackendExport[] { if (!isProgramNode(ast)) { throw new Error( `Expected a Program node from this.parse() for ${filePath}, got ${ast.type}`, diff --git a/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids.test.ts b/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids.test.ts new file mode 100644 index 000000000..d14d92688 --- /dev/null +++ b/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids.test.ts @@ -0,0 +1,358 @@ +// 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 { Program } from 'estree'; +import { parseAst } from 'rollup/parseAst'; + +import { extractConnectionIds } from './extract-connection-ids'; + +const filePath = '/project/src/backend/actions.backend.js'; + +function parse(code: string): Program { + return parseAst(code) as Program; +} + +describe('Backend Functions - extractConnectionIds', () => { + test('Should extract inline string literal connection IDs from named action-catalog imports', () => { + const ast = parse(` + import { request } from '@datadog/action-catalog/http/http'; + + export function run() { + return request({ connectionId: 'conn-b', inputs: {} }); + } + `); + + expect(extractConnectionIds(ast, filePath)).toEqual(['conn-b']); + }); + + test('Should dedupe and sort connection IDs', () => { + const ast = parse(` + import { request } from '@datadog/action-catalog/http/http'; + + export function run() { + request({ connectionId: 'conn-b', inputs: {} }); + request({ connectionId: 'conn-a', inputs: {} }); + request({ connectionId: 'conn-b', inputs: {} }); + } + `); + + expect(extractConnectionIds(ast, filePath)).toEqual(['conn-a', 'conn-b']); + }); + + test('Should include same-file helper action calls', () => { + const ast = parse(` + import { request } from '@datadog/action-catalog/http/http'; + + function helper() { + return request({ connectionId: 'conn-helper', inputs: {} }); + } + + export function run() { + return helper(); + } + `); + + expect(extractConnectionIds(ast, filePath)).toEqual(['conn-helper']); + }); + + test('Should detect default and namespace action-catalog imports', () => { + const ast = parse(` + import request from '@datadog/action-catalog/http/http'; + import * as slack from '@datadog/action-catalog/slack/messages'; + + export function run() { + request({ connectionId: 'conn-default', inputs: {} }); + slack.postMessage({ connectionId: 'conn-namespace', inputs: {} }); + } + `); + + expect(extractConnectionIds(ast, filePath)).toEqual(['conn-default', 'conn-namespace']); + }); + + test('Should ignore non-action-catalog calls with connectionId properties', () => { + const ast = parse(` + import { request } from './local'; + + export function run() { + request({ connectionId: 'ignored', inputs: {} }); + } + `); + + expect(extractConnectionIds(ast, filePath)).toEqual([]); + }); + + test('Should ignore action-catalog object arguments without connectionId', () => { + const ast = parse(` + import { request } from '@datadog/action-catalog/http/http'; + + export function run() { + request({ inputs: {} }); + } + `); + + expect(extractConnectionIds(ast, filePath)).toEqual([]); + }); + + test('Should ignore type-only action-catalog imports', () => { + const ast = parse(` + import { request } from '@datadog/action-catalog/http/http'; + + export function run() { + request({ connectionId: 'ignored', inputs: {} }); + } + `); + const importDeclaration = (ast as unknown as { body: Array<{ importKind?: string }> }) + .body[0]; + // Rollup's parser rejects TypeScript `import type` syntax, so patch the + // ESTree field that a TypeScript-aware parser would add. + importDeclaration.importKind = 'type'; + + expect(extractConnectionIds(ast, filePath)).toEqual([]); + }); + + test('Should ignore type-only action-catalog import specifiers', () => { + const ast = parse(` + import { request } from '@datadog/action-catalog/http/http'; + + export function run() { + request({ connectionId: 'ignored', inputs: {} }); + } + `); + const importSpecifier = ( + ast as unknown as { + body: Array<{ specifiers: Array<{ importKind?: string }> }>; + } + ).body[0].specifiers[0]; + // Rollup's parser rejects TypeScript `import { type ... }` syntax, so + // patch the ESTree field that a TypeScript-aware parser would add. + importSpecifier.importKind = 'type'; + + expect(extractConnectionIds(ast, filePath)).toEqual([]); + }); + + test.each([ + { + description: 'function parameters that shadow named imports', + code: ` + import { request } from '@datadog/action-catalog/http/http'; + + export function run(request) { + return request({ connectionId: 'ignored', inputs: {} }); + } + `, + }, + { + description: 'function parameters that shadow namespace imports', + code: ` + import * as http from '@datadog/action-catalog/http/http'; + + export function run(http) { + return http.request({ connectionId: 'ignored', inputs: {} }); + } + `, + }, + { + description: 'catch parameters that shadow named imports', + code: ` + import { request } from '@datadog/action-catalog/http/http'; + + export function run() { + try { + throw new Error('nope'); + } catch (request) { + request({ connectionId: CONNECTIONS.HTTP, inputs: {} }); + } + } + `, + }, + { + description: 'local aliases of shadowed parameters', + code: ` + import { request } from '@datadog/action-catalog/http/http'; + + export function run(request) { + const action = request; + action({ connectionId: 'ignored', inputs: {} }); + } + `, + }, + { + description: 'for-of bindings that shadow named imports', + code: ` + import { request } from '@datadog/action-catalog/http/http'; + + export function run(handlers) { + for (const request of handlers) { + request({ connectionId: CONNECTIONS.HTTP, inputs: {} }); + } + } + `, + }, + { + description: 'for-statement bindings that shadow named imports', + code: ` + import { request } from '@datadog/action-catalog/http/http'; + + export function run(handlers) { + for (const request = handlers.next; request;) { + request({ connectionId: CONNECTIONS.HTTP, inputs: {} }); + } + } + `, + }, + { + description: 'for-in bindings that shadow namespace imports', + code: ` + import * as http from '@datadog/action-catalog/http/http'; + + export function run(clients) { + for (const http in clients) { + http.request({ connectionId: CONNECTIONS.HTTP, inputs: {} }); + } + } + `, + }, + ])( + 'Should not treat shadowed action-catalog import names as action calls: $description', + ({ code }) => { + expect(extractConnectionIds(parse(code), filePath)).toEqual([]); + }, + ); + + test.each([ + { + description: 'identifier value', + source: 'const ID = "conn"; request({ connectionId: ID, inputs: {} });', + expectedType: 'Identifier', + }, + { + description: 'template literal value', + source: 'request({ connectionId: `conn`, inputs: {} });', + expectedType: 'TemplateLiteral', + }, + { + description: 'member expression value', + source: 'request({ connectionId: CONNECTIONS.HTTP, inputs: {} });', + expectedType: 'MemberExpression', + }, + { + description: 'call expression value', + source: 'request({ connectionId: getConnectionId(), inputs: {} });', + expectedType: 'CallExpression', + }, + { + description: 'binary expression value', + source: "request({ connectionId: 'conn-' + suffix, inputs: {} });", + expectedType: 'BinaryExpression', + }, + ])('Should fail closed for unsupported $description', ({ source, expectedType }) => { + const ast = parse(` + import { request } from '@datadog/action-catalog/http/http'; + + export function run() { + ${source} + } + `); + + expect(() => extractConnectionIds(ast, filePath)).toThrow( + `expected an inline string literal, got ${expectedType}`, + ); + }); + + test.each([ + { + description: 'non-object first arguments', + source: 'request(opts);', + expectedMessage: 'non-object action-catalog call arguments', + }, + { + description: 'spread-composed object arguments', + source: 'request({ ...opts });', + expectedMessage: 'spread object arguments', + }, + { + description: 'computed connectionId keys', + source: "request({ ['connectionId']: 'conn' });", + expectedMessage: 'computed object property keys', + }, + { + description: 'optional action calls', + source: "request?.({ connectionId: 'conn' });", + expectedMessage: 'optional action-catalog calls', + }, + { + description: 'action-catalog import aliases', + source: "const action = request; action({ connectionId: 'conn' });", + expectedMessage: 'action-catalog call aliases', + }, + { + description: 'action-catalog namespace member aliases', + source: "const action = http.request; action({ connectionId: 'conn' });", + expectedMessage: 'action-catalog call aliases', + importStatement: "import * as http from '@datadog/action-catalog/http/http';", + }, + { + description: 'action-catalog namespace destructuring aliases', + source: "const { request: action } = http; action({ connectionId: 'conn' });", + expectedMessage: 'action-catalog call aliases', + importStatement: "import * as http from '@datadog/action-catalog/http/http';", + }, + { + description: 'assigned action-catalog import aliases', + source: "let action; action = request; action({ connectionId: 'conn' });", + expectedMessage: 'action-catalog call aliases', + }, + { + description: 'assigned action-catalog namespace member aliases', + source: "let action; action = http.request; action({ connectionId: 'conn' });", + expectedMessage: 'action-catalog call aliases', + importStatement: "import * as http from '@datadog/action-catalog/http/http';", + }, + { + description: 'assigned action-catalog namespace destructuring aliases', + source: "let action; ({ request: action } = http); action({ connectionId: 'conn' });", + expectedMessage: 'action-catalog call aliases', + importStatement: "import * as http from '@datadog/action-catalog/http/http';", + }, + { + description: 'multiple connectionId properties', + source: "request({ connectionId: 'conn-a', connectionId: 'conn-b' });", + expectedMessage: 'multiple connectionId properties', + }, + { + description: 'accessor connectionId properties', + source: 'request({ get connectionId() { return CONNECTIONS.HTTP; } });', + expectedMessage: 'accessor connectionId properties', + }, + { + description: 'computed namespace calls', + source: "http['request']({ connectionId: 'conn' });", + expectedMessage: 'optional or computed action-catalog namespace calls', + importStatement: "import * as http from '@datadog/action-catalog/http/http';", + }, + ])( + 'Should fail closed for unsupported $description', + ({ source, expectedMessage, importStatement }) => { + const ast = parse(` + ${importStatement ?? "import { request } from '@datadog/action-catalog/http/http';"} + + export function run() { + ${source} + } + `); + + expect(() => extractConnectionIds(ast, filePath)).toThrow(expectedMessage); + }, + ); + + test('Should return an empty allowlist when no connection IDs are present', () => { + const ast = parse(` + export function run() { + return 'ok'; + } + `); + + expect(extractConnectionIds(ast, filePath)).toEqual([]); + }); +}); diff --git a/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids.ts b/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids.ts new file mode 100644 index 000000000..4496aa83e --- /dev/null +++ b/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids.ts @@ -0,0 +1,38 @@ +// 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 { BaseNode } from 'estree'; + +import { + analyzeActionCatalogScopes, + findActionCatalogCallSites, +} from './action-catalog-call-sites'; +import { collectActionCatalogImports, hasActionCatalogImports } from './action-catalog-imports'; +import { extractConnectionIdFromActionCall } from './connection-id-values'; +import { isProgramNode } from './type-guards'; + +export function extractConnectionIds(ast: BaseNode, filePath: string): string[] { + if (!isProgramNode(ast)) { + throw new Error( + `Expected a Program node from this.parse() for ${filePath}, got ${ast.type}`, + ); + } + + const imports = collectActionCatalogImports(ast); + if (!hasActionCatalogImports(imports)) { + return []; + } + + const scopeAnalysis = analyzeActionCatalogScopes(ast, imports); + const connectionIds = new Set(); + + for (const callSite of findActionCatalogCallSites(ast, scopeAnalysis, filePath)) { + const connectionId = extractConnectionIdFromActionCall(callSite, filePath); + if (connectionId) { + connectionIds.add(connectionId); + } + } + + return [...connectionIds].sort(); +} diff --git a/packages/plugins/apps/src/backend/ast-parsing/type-guards.ts b/packages/plugins/apps/src/backend/ast-parsing/type-guards.ts index 7a5d3205c..5f1776c75 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/type-guards.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/type-guards.ts @@ -2,9 +2,8 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. -import type { Program } from 'estree'; -import type { AstNode } from 'rollup'; +import type { BaseNode, Program } from 'estree'; -export function isProgramNode(node: AstNode): node is AstNode & Program { +export function isProgramNode(node: BaseNode): node is Program { return node.type === 'Program'; } diff --git a/packages/plugins/apps/src/backend/discovery.test.ts b/packages/plugins/apps/src/backend/discovery.test.ts index 4698c560c..0ee67a0b6 100644 --- a/packages/plugins/apps/src/backend/discovery.test.ts +++ b/packages/plugins/apps/src/backend/discovery.test.ts @@ -4,13 +4,12 @@ import { extractExportedFunctions } from '@dd/apps-plugin/backend/ast-parsing/extract-backend-functions'; import type { Program } from 'estree'; -import type { AstNode } from 'rollup'; /** - * Helper to build a minimal ESTree Program AstNode for testing. + * Helper to build a minimal ESTree Program for testing. */ -function program(body: Program['body']): AstNode & Program { - return { type: 'Program', sourceType: 'module', body, start: 0, end: 0 }; +function program(body: Program['body']): Program { + return { type: 'Program', sourceType: 'module', body }; } describe('Backend Functions - extractExportedFunctions', () => { diff --git a/packages/plugins/apps/src/index.test.ts b/packages/plugins/apps/src/index.test.ts index 5daa06bdc..4d126a46d 100644 --- a/packages/plugins/apps/src/index.test.ts +++ b/packages/plugins/apps/src/index.test.ts @@ -20,6 +20,7 @@ import { runBundlers } from '@dd/tests/_jest/helpers/runBundlers'; import fsp from 'fs/promises'; import nock from 'nock'; import path from 'path'; +import { parseAst } from 'rollup/parseAst'; import { APPS_API_PATH } from './constants'; @@ -252,21 +253,19 @@ describe('Apps Plugin - getPlugins', () => { }; transform.handler.call( { - parse: () => ({ - type: 'Program', - body: [ - { - type: 'ExportNamedDeclaration', - declaration: { - type: 'FunctionDeclaration', - id: { type: 'Identifier', name: 'greet' }, - }, - specifiers: [], - }, - ], - }), + parse: parseAst, }, - 'export function greet() {}', + ` + import { request } from '@datadog/action-catalog/http/http'; + + export function greet() { + request({ connectionId: 'conn-b', inputs: {} }); + } + + export function salute() { + request({ connectionId: 'conn-a', inputs: {} }); + } + `, '/project/src/backend/greet.backend.js', ); @@ -282,7 +281,10 @@ describe('Apps Plugin - getPlugins', () => { ); expect( Object.keys((manifest as { backend: { functions: object } }).backend.functions), - ).toEqual([expect.stringMatching(/^[a-f0-9]{64}\.greet$/)]); + ).toEqual([ + expect.stringMatching(/^[a-f0-9]{64}\.greet$/), + expect.stringMatching(/^[a-f0-9]{64}\.salute$/), + ]); expect(manifest).toMatchObject({ backend: { functions: expect.any(Object) }, }); @@ -290,7 +292,10 @@ describe('Apps Plugin - getPlugins', () => { Object.values( (manifest as { backend: { functions: Record } }).backend.functions, ), - ).toEqual([{ allowedConnectionIds: [] }]); + ).toEqual([ + { allowedConnectionIds: ['conn-a', 'conn-b'] }, + { allowedConnectionIds: ['conn-a', 'conn-b'] }, + ]); }); test('Should surface upload errors', async () => { diff --git a/packages/plugins/apps/src/index.ts b/packages/plugins/apps/src/index.ts index 1299947de..f23758b2c 100644 --- a/packages/plugins/apps/src/index.ts +++ b/packages/plugins/apps/src/index.ts @@ -14,6 +14,7 @@ import { createArchive } from './archive'; import type { Asset } from './assets'; import { collectAssets } from './assets'; import { extractExportedFunctions } from './backend/ast-parsing/extract-backend-functions'; +import { extractConnectionIds } from './backend/ast-parsing/extract-connection-ids'; import { encodeQueryName } from './backend/encodeQueryName'; import { generateProxyModule } from './backend/proxy-codegen'; import type { BackendFunction } from './backend/types'; @@ -34,6 +35,7 @@ function buildProxyModule( exportNames: string[], id: string, buildRoot: string, + allowedConnectionIds: string[], ): { functions: BackendFunction[]; proxyCode: string } { const relativePath = path.relative(buildRoot, id); const refPath = relativePath.replace(BACKEND_FILE_RE, ''); @@ -46,7 +48,7 @@ function buildProxyModule( relativePath: refPath, name: exportName, absolutePath: id, - allowedConnectionIds: [], + allowedConnectionIds, }; functions.push(func); proxyExports.push({ exportName, queryName: encodeQueryName(func) }); @@ -275,7 +277,8 @@ Either: // them as backend functions, and replace the module with a // frontend proxy that calls executeBackendFunction at runtime. handler(code, id) { - const exportNames = extractExportedFunctions(this.parse(code), id); + const ast = this.parse(code); + const exportNames = extractExportedFunctions(ast, id); if (exportNames.length === 0) { log.warn( `Backend file ${id} has no exported functions. ` + @@ -287,10 +290,12 @@ Either: return { code: '', map: null }; } + const allowedConnectionIds = extractConnectionIds(ast, id); const { functions, proxyCode } = buildProxyModule( exportNames, id, context.buildRoot, + allowedConnectionIds, ); setBackendFunctions(id, functions); log.debug(`Generated proxy for ${id} with ${functions.length} export(s)`); diff --git a/packages/published/esbuild-plugin/package.json b/packages/published/esbuild-plugin/package.json index f2fd247db..fa3b43184 100644 --- a/packages/published/esbuild-plugin/package.json +++ b/packages/published/esbuild-plugin/package.json @@ -53,6 +53,7 @@ "@datadog/js-instrumentation-wasm": "1.0.8", "async-retry": "1.3.3", "chalk": "2.3.1", + "eslint-scope": "7.2.2", "glob": "11.1.0", "json-stream-stringify": "3.1.6", "jszip": "3.10.1", diff --git a/packages/published/rollup-plugin/package.json b/packages/published/rollup-plugin/package.json index 384b08767..609b96254 100644 --- a/packages/published/rollup-plugin/package.json +++ b/packages/published/rollup-plugin/package.json @@ -56,6 +56,7 @@ "@datadog/js-instrumentation-wasm": "1.0.8", "async-retry": "1.3.3", "chalk": "2.3.1", + "eslint-scope": "7.2.2", "glob": "11.1.0", "json-stream-stringify": "3.1.6", "jszip": "3.10.1", diff --git a/packages/published/rspack-plugin/package.json b/packages/published/rspack-plugin/package.json index 861025ca5..c141a25ce 100644 --- a/packages/published/rspack-plugin/package.json +++ b/packages/published/rspack-plugin/package.json @@ -53,6 +53,7 @@ "@datadog/js-instrumentation-wasm": "1.0.8", "async-retry": "1.3.3", "chalk": "2.3.1", + "eslint-scope": "7.2.2", "glob": "11.1.0", "json-stream-stringify": "3.1.6", "jszip": "3.10.1", diff --git a/packages/published/vite-plugin/package.json b/packages/published/vite-plugin/package.json index 5c67e58ee..e4d979c8a 100644 --- a/packages/published/vite-plugin/package.json +++ b/packages/published/vite-plugin/package.json @@ -53,6 +53,7 @@ "@datadog/js-instrumentation-wasm": "1.0.8", "async-retry": "1.3.3", "chalk": "2.3.1", + "eslint-scope": "7.2.2", "glob": "11.1.0", "json-stream-stringify": "3.1.6", "jszip": "3.10.1", diff --git a/packages/published/webpack-plugin/package.json b/packages/published/webpack-plugin/package.json index 6fed55ed7..6d6142390 100644 --- a/packages/published/webpack-plugin/package.json +++ b/packages/published/webpack-plugin/package.json @@ -53,6 +53,7 @@ "@datadog/js-instrumentation-wasm": "1.0.8", "async-retry": "1.3.3", "chalk": "2.3.1", + "eslint-scope": "7.2.2", "glob": "11.1.0", "json-stream-stringify": "3.1.6", "jszip": "3.10.1", diff --git a/yarn.lock b/yarn.lock index 329b10d2e..0df3f371d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1699,6 +1699,7 @@ __metadata: async-retry: "npm:1.3.3" chalk: "npm:2.3.1" esbuild: "npm:0.25.8" + eslint-scope: "npm:7.2.2" glob: "npm:11.1.0" json-stream-stringify: "npm:3.1.6" jszip: "npm:3.10.1" @@ -1757,6 +1758,7 @@ __metadata: async-retry: "npm:1.3.3" chalk: "npm:2.3.1" esbuild: "npm:0.25.8" + eslint-scope: "npm:7.2.2" glob: "npm:11.1.0" json-stream-stringify: "npm:3.1.6" jszip: "npm:3.10.1" @@ -1808,6 +1810,7 @@ __metadata: async-retry: "npm:1.3.3" chalk: "npm:2.3.1" esbuild: "npm:0.25.8" + eslint-scope: "npm:7.2.2" glob: "npm:11.1.0" json-stream-stringify: "npm:3.1.6" jszip: "npm:3.10.1" @@ -1859,6 +1862,7 @@ __metadata: async-retry: "npm:1.3.3" chalk: "npm:2.3.1" esbuild: "npm:0.25.8" + eslint-scope: "npm:7.2.2" glob: "npm:11.1.0" json-stream-stringify: "npm:3.1.6" jszip: "npm:3.10.1" @@ -1910,6 +1914,7 @@ __metadata: async-retry: "npm:1.3.3" chalk: "npm:2.3.1" esbuild: "npm:0.25.8" + eslint-scope: "npm:7.2.2" glob: "npm:11.1.0" json-stream-stringify: "npm:3.1.6" jszip: "npm:3.10.1" @@ -1945,11 +1950,14 @@ __metadata: resolution: "@dd/apps-plugin@workspace:packages/plugins/apps" dependencies: "@dd/core": "workspace:*" + "@types/eslint-scope": "npm:3.7.7" "@types/estree": "npm:1.0.8" chalk: "npm:2.3.1" + eslint-scope: "npm:7.2.2" glob: "npm:11.1.0" jszip: "npm:3.10.1" pretty-bytes: "npm:5.6.0" + rollup: "npm:4.45.1" typescript: "npm:5.4.3" vite: "npm:6.3.5" languageName: unknown @@ -4008,7 +4016,7 @@ __metadata: languageName: node linkType: hard -"@types/eslint-scope@npm:^3.7.7": +"@types/eslint-scope@npm:3.7.7, @types/eslint-scope@npm:^3.7.7": version: 3.7.7 resolution: "@types/eslint-scope@npm:3.7.7" dependencies: @@ -6516,7 +6524,7 @@ __metadata: languageName: node linkType: hard -"eslint-scope@npm:^7.2.2": +"eslint-scope@npm:7.2.2, eslint-scope@npm:^7.2.2": version: 7.2.2 resolution: "eslint-scope@npm:7.2.2" dependencies: