diff --git a/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids-from-module-graph.ts b/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids-from-module-graph.ts index c451fb1ea..200ac657f 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids-from-module-graph.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids-from-module-graph.ts @@ -15,6 +15,7 @@ import { collectMutatedImportedObjectVariables, resolveStaticStringValue, } from './static-string-resolution'; +import { getStaticPropertyKey, hasExpressionValue } from './type-guards'; import { walkModuleGraph } from './walk-module-graph'; const CONNECTION_ID_PROPERTY = 'connectionId'; @@ -43,7 +44,7 @@ export function extractConnectionIdsFromModuleGraph( return getConnectionIds(calls); } -export function getConnectionIds(calls: ActionCatalogCall[]): string[] { +export function getConnectionIds(calls: ReadonlyArray<{ connectionId?: string }>): string[] { const connectionIds = new Set(); for (const { connectionId } of calls) { @@ -174,16 +175,6 @@ function findConnectionIdProperty( return connectionIdProperty; } -function hasExpressionValue(property: Property): property is ConnectionIdProperty { - const { value } = property; - return ( - value.type !== 'ObjectPattern' && - value.type !== 'ArrayPattern' && - value.type !== 'RestElement' && - value.type !== 'AssignmentPattern' - ); -} - // A shorthand `__proto__` is an own property, and a null prototype supplies nothing. function isInertProtoKey(key: string, property: Property): boolean { const { value } = property; @@ -197,16 +188,6 @@ function isConnectionIdKey(property: Property): boolean { return getStaticPropertyKey(property) === CONNECTION_ID_PROPERTY; } -function getStaticPropertyKey(property: Property): string | undefined { - if (property.key.type === 'Identifier') { - return property.key.name; - } - if (property.key.type === 'Literal' && typeof property.key.value === 'string') { - return property.key.value; - } - return undefined; -} - 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/list-action-calls.test.ts b/packages/plugins/apps/src/backend/ast-parsing/list-action-calls.test.ts index ec1475c45..d92b0f9fa 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/list-action-calls.test.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/list-action-calls.test.ts @@ -34,7 +34,7 @@ function extract( ]); const modules = new Map(moduleEntries); const calls = collectActionCatalogCalls(entryId, modules, buildRoot); - return listActionCalls(calls, fqns); + return listActionCalls(calls, fqns, modules); } describe('Backend Functions - listActionCalls', () => { @@ -66,8 +66,16 @@ describe('Backend Functions - listActionCalls', () => { const actions = extract([entry, helper]); expect(actions).toEqual([ - { fqn: 'com.datadoghq.http.request', connectionIds: ['conn-http'] }, - { fqn: 'com.datadoghq.slack.chat.postMessage', connectionIds: ['conn-slack'] }, + { + fqn: 'com.datadoghq.http.request', + connectionIds: ['conn-http'], + calls: [{ connectionId: 'conn-http', inputs: {} }], + }, + { + fqn: 'com.datadoghq.slack.chat.postMessage', + connectionIds: ['conn-slack'], + calls: [{ connectionId: 'conn-slack', inputs: {} }], + }, ]); }); @@ -90,8 +98,20 @@ describe('Backend Functions - listActionCalls', () => { const actions = extract([entry]); expect(actions).toEqual([ - { fqn: 'com.datadoghq.http.request', connectionIds: ['conn-a', 'conn-b'] }, - { fqn: 'com.datadoghq.slack.chat.postMessage', connectionIds: [] }, + { + fqn: 'com.datadoghq.http.request', + connectionIds: ['conn-a', 'conn-b'], + calls: [ + { connectionId: 'conn-b', inputs: {} }, + { connectionId: 'conn-a', inputs: {} }, + { connectionId: 'conn-b', inputs: {} }, + ], + }, + { + fqn: 'com.datadoghq.slack.chat.postMessage', + connectionIds: [], + calls: [{ inputs: {} }], + }, ]); }); @@ -143,4 +163,280 @@ describe('Backend Functions - listActionCalls', () => { const actions = extract([entry], fqns); expect(actions).toBeUndefined(); }); + + const listHttpCallInputs = (code: string, helper?: string) => { + const source = `import { request } from '@datadog/action-catalog/http/http';\n${code}`; + const helpers = helper ? [createRecord(helperId, helper)] : []; + const helperIds = helpers.map(({ id }) => id); + const entry = createRecord(entryId, source, [HTTP_MODULE, ...helperIds]); + const actions = extract([entry, ...helpers]); + return actions?.[0].calls.map((call) => call.inputs); + }; + + const inputCases = [ + { + description: 'string literals', + code: `request({ inputs: { url: 'https://example.com', verb: 'GET' } });`, + expected: { url: 'https://example.com', verb: 'GET' }, + }, + { + description: 'module consts', + code: ` + const BASE_URL = 'https://example.com'; + const VERB = 'POST'; + request({ inputs: { url: BASE_URL, verb: VERB } }); + `, + expected: { url: 'https://example.com', verb: 'POST' }, + }, + { + description: 'a const imported from another module', + code: ` + import { BASE_URL } from './helpers/slack.js'; + request({ inputs: { url: BASE_URL } }); + `, + helper: `export const BASE_URL = 'https://example.com/api';`, + expected: { url: 'https://example.com/api' }, + }, + { + description: 'an imported const object property, left out', + code: ` + import { URLS } from './helpers/slack.js'; + request({ inputs: { url: URLS.API, verb: 'GET' } }); + `, + helper: `export const URLS = { API: 'https://example.com/api' };`, + expected: { verb: 'GET' }, + }, + { + description: 'a const object property that an alias mutates, left out', + code: ` + const URLS = { API: 'https://example.com/api' }; + const alias = URLS; + alias.API = 'https://other.example'; + request({ inputs: { url: URLS.API, verb: 'GET' } }); + `, + expected: { verb: 'GET' }, + }, + { + description: 'a const read from an object a call mutates, left out', + code: ` + const URLS = { API: 'https://example.com/api' }; + Object.assign(URLS, { API: 'https://other.example' }); + const API_URL = URLS.API; + request({ inputs: { url: API_URL, verb: 'GET' } }); + `, + expected: { verb: 'GET' }, + }, + { + description: 'a template literal with no expressions', + code: 'request({ inputs: { url: `https://example.com/api`, verb: "GET" } });', + expected: { url: 'https://example.com/api', verb: 'GET' }, + }, + { + description: 'a template literal with an expression, left out even over a const', + code: ` + const HOST = 'example.com'; + request({ inputs: { url: \`https://\${HOST}/api\`, verb: 'GET' } }); + `, + expected: { verb: 'GET' }, + }, + { + description: 'dynamic values, left out', + code: ` + export function run(target, host) { + return request({ + inputs: { + url: target, + verb: 'GET', + body: \`https://\${host}/api\`, + timeout: 30, + headers: { accept: 'application/json' }, + parse() { return 'x'; }, + }, + }); + } + `, + expected: { verb: 'GET' }, + }, + { + description: 'a spread, which can add a toJSON, leaving out every input', + code: ` + export function run(extra) { + return request({ inputs: { ...extra, url: 'https://a.example' } }); + } + `, + expected: {}, + }, + { + description: 'a computed key, which can add a toJSON, leaving out every input', + code: ` + export function run(key, toJson) { + return request({ inputs: { [key]: toJson, url: 'https://a.example' } }); + } + `, + expected: {}, + }, + { + description: 'a getter, which can rewrite other inputs, leaving out every input', + code: ` + request({ + inputs: { + get verb() { this.url = 'https://other.example'; return 'GET'; }, + url: 'https://a.example', + }, + }); + `, + expected: {}, + }, + { + description: 'a toJSON method, leaving out every input', + code: ` + request({ + inputs: { + url: 'https://a.example', + toJSON() { return { url: 'https://other.example' }; }, + }, + }); + `, + expected: {}, + }, + { + description: 'a then method, which awaiting the inputs runs, leaving out every input', + code: ` + request({ + inputs: { + url: 'https://a.example', + then(resolve) { this.url = 'https://other.example'; resolve({}); }, + }, + }); + `, + expected: {}, + }, + { + description: 'a getter on the call options, leaving out every input', + code: ` + request({ + get extra() { this.inputs.url = 'https://other.example'; return 1; }, + inputs: { url: 'https://a.example' }, + }); + `, + expected: {}, + }, + { + description: 'a setter, leaving out every input', + code: `request({ inputs: { set url(value) {}, verb: 'GET' } });`, + expected: {}, + }, + { + description: 'a numeric key, which only sets its own key', + code: `request({ inputs: { url: 'https://a.example', 0: 'zero', 1: 2 } });`, + expected: { url: 'https://a.example', 0: 'zero' }, + }, + { + description: 'a duplicate key whose last value is dynamic', + code: ` + export function run(target) { + return request({ inputs: { url: 'https://a.example', url: target } }); + } + `, + expected: {}, + }, + { + description: 'a duplicate key whose last value is static', + code: ` + export function run(target) { + return request({ inputs: { url: target, url: 'https://b.example' } }); + } + `, + expected: { url: 'https://b.example' }, + }, + { + description: 'a __proto__ key, which can inherit a toJSON, leaving out every input', + code: ` + const PROTO = { toJSON: () => ({ url: 'https://other.example' }) }; + request({ inputs: { __proto__: PROTO, url: 'https://a.example' } }); + `, + expected: {}, + }, + { + description: 'inputs that are not an inline object', + code: ` + const INPUTS = { url: 'https://example.com' }; + request({ inputs: INPUTS }); + `, + expected: {}, + }, + { + description: 'no inputs', + code: `request({ connectionId: 'conn-http' });`, + expected: {}, + }, + ]; + + test.each(inputCases)( + "Should list each call's static string inputs from $description", + ({ code, helper, expected }) => { + const inputs = listHttpCallInputs(code, helper); + expect(inputs).toEqual([expected]); + }, + ); + + const rejectedOptionCases = [ + { + description: 'a toJSON', + code: ` + request({ + inputs: { url: 'https://a.example' }, + toJSON: () => ({ inputs: { url: 'https://other.example' } }), + }); + `, + }, + { + description: 'a __proto__ key', + code: ` + const PROTO = { toJSON: () => ({ inputs: { url: 'https://other.example' } }) }; + request({ __proto__: PROTO, inputs: { url: 'https://a.example' } }); + `, + }, + ]; + + test.each(rejectedOptionCases)( + 'Should reject a call whose options have $description before listing its inputs', + ({ code }) => { + expect(() => listHttpCallInputs(code)).toThrow('could hide a connectionId'); + }, + ); + + test('Should list each call separately, with its own connectionId', () => { + const entry = createRecord( + entryId, + ` + import { request } from '@datadog/action-catalog/http/http'; + export function run() { + request({ connectionId: 'conn-b', inputs: { url: 'https://b.example', verb: 'POST' } }); + request({ inputs: { url: 'https://a.example', verb: 'GET' } }); + } + `, + [HTTP_MODULE], + ); + + const actions = extract([entry]); + expect(actions?.[0].calls).toEqual([ + { connectionId: 'conn-b', inputs: { url: 'https://b.example', verb: 'POST' } }, + { inputs: { url: 'https://a.example', verb: 'GET' } }, + ]); + }); + + test('Should keep an empty-string connectionId on its call', () => { + const entry = createRecord( + entryId, + ` + import { request } from '@datadog/action-catalog/http/http'; + export function run() { return request({ connectionId: '', inputs: {} }); } + `, + [HTTP_MODULE], + ); + + const actions = extract([entry]); + expect(actions?.[0].calls).toEqual([{ connectionId: '', inputs: {} }]); + }); }); diff --git a/packages/plugins/apps/src/backend/ast-parsing/list-action-calls.ts b/packages/plugins/apps/src/backend/ast-parsing/list-action-calls.ts index 312a12a50..14c81bc2b 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/list-action-calls.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/list-action-calls.ts @@ -2,41 +2,129 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. -import type { Identifier } from 'estree'; +import type { Identifier, ObjectExpression, Property } from 'estree'; -import type { DebugBundleAction } from '../protocol'; +import type { DebugBundleAction, DebugBundleActionCall } from '../protocol'; -import type { ActionCatalogCall } from './extract-connection-ids-from-module-graph'; +import { + type ActionCatalogCall, + getConnectionIds, +} from './extract-connection-ids-from-module-graph'; import type { ParsedModuleRecord } from './module-graph'; import { resolveIdentifier } from './module-scope'; +import { resolveStaticStringConstant } from './static-string-resolution'; +import { getStaticPropertyKey, hasExpressionValue } from './type-guards'; + +const INPUTS_PROPERTY = 'inputs'; /** - * Groups action-catalog calls by action FQN, with the connection IDs bound to each. - * Returns undefined when a call has no known FQN, so a list missing it is never returned. + * Groups action-catalog calls by action FQN, with the connection IDs bound to each and every + * call's static string inputs. Returns undefined when a call has no known FQN, so a list missing + * it is never returned. */ export function listActionCalls( calls: ActionCatalogCall[], exportFqnsByModule: ReadonlyMap>, + modules: ReadonlyMap, ): DebugBundleAction[] | undefined { - const connectionIdsByFqn = new Map>(); + const callsByFqn = new Map(); for (const call of calls) { const fqn = resolveCallFqn(call, exportFqnsByModule); if (!fqn) { return undefined; } - const connectionIds = connectionIdsByFqn.get(fqn) ?? new Set(); - if (call.connectionId !== undefined) { - connectionIds.add(call.connectionId); - } - connectionIdsByFqn.set(fqn, connectionIds); + const fqnCalls = callsByFqn.get(fqn) ?? []; + const describedCall = describeCall(call, modules); + fqnCalls.push(describedCall); + callsByFqn.set(fqn, fqnCalls); } - return [...connectionIdsByFqn] - .map(([fqn, connectionIds]) => ({ fqn, connectionIds: [...connectionIds].sort() })) + return [...callsByFqn] + .map(([fqn, fqnCalls]) => ({ + fqn, + connectionIds: getConnectionIds(fqnCalls), + calls: fqnCalls, + })) .sort((a, b) => (a.fqn < b.fqn ? -1 : 1)); } +function describeCall( + { record, callSite, connectionId }: ActionCatalogCall, + modules: ReadonlyMap, +): DebugBundleActionCall { + const [options] = callSite.arguments; + return { + ...(connectionId !== undefined && { connectionId }), + inputs: + options?.type === 'ObjectExpression' + ? listStaticInputs(options, record.id, modules) + : {}, + }; +} + +// Serializing or awaiting the call can run a getter, toJSON or then on these objects: a spread or +// computed key can add the last two, and __proto__ any of them. +function getPlainDataEntries(object: ObjectExpression): Array<[string, Property]> | undefined { + const entries: Array<[string, Property]> = []; + for (const property of object.properties) { + if (property.type !== 'Property' || property.computed || property.kind !== 'init') { + return undefined; + } + const key = getStaticPropertyKey(property); + if (key === undefined || key === 'toJSON' || key === 'then' || key === '__proto__') { + return undefined; + } + entries.push([key, property]); + } + return entries; +} + +function listStaticInputs( + options: ObjectExpression, + moduleId: string, + modules: ReadonlyMap, +): Record { + const optionEntries = getPlainDataEntries(options); + if (!optionEntries) { + return {}; + } + let inputs: Property['value'] | undefined; + for (const [key, property] of optionEntries) { + if (key === INPUTS_PROPERTY) { + inputs = property.value; + } + } + const inputEntries = + inputs?.type === 'ObjectExpression' ? getPlainDataEntries(inputs) : undefined; + if (!inputEntries) { + return {}; + } + + const resolved = new Map(); + for (const [key, property] of inputEntries) { + const value = resolveInputValue(property, moduleId, modules); + if (value === undefined) { + resolved.delete(key); + } else { + resolved.set(key, value); + } + } + return Object.fromEntries(resolved); +} + +function resolveInputValue( + property: Property, + moduleId: string, + modules: ReadonlyMap, +): string | undefined { + if (!hasExpressionValue(property)) { + return undefined; + } + const result = resolveStaticStringConstant(modules, moduleId, property.value); + return result.kind === 'resolved' ? result.value : undefined; +} + function resolveCallFqn( { record, callSite }: ActionCatalogCall, exportFqnsByModule: ReadonlyMap>, diff --git a/packages/plugins/apps/src/backend/ast-parsing/static-string-resolution.ts b/packages/plugins/apps/src/backend/ast-parsing/static-string-resolution.ts index 4bb565abb..27fb504a5 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/static-string-resolution.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/static-string-resolution.ts @@ -29,6 +29,7 @@ import { resolveStaticDefinitionForIdentifier, type UnsupportedStaticDefinition, } from './static-definition-resolution'; +import { getStaticPropertyKey, hasExpressionValue } from './type-guards'; import { walkAst } from './walk-ast'; export type StaticStringValueResolution = ResolvedStaticStringValue | UnsupportedStaticStringValue; @@ -171,6 +172,7 @@ interface ResolverState { modules: ReadonlyMap; mutatedImportedObjectVariables: ReadonlySet; seenVariables: Set; + followObjectMembers: boolean; } interface ImportedObjectMutationState { @@ -229,6 +231,25 @@ export function resolveStaticStringValue( modules, mutatedImportedObjectVariables, seenVariables: new Set(), + followObjectMembers: true, + }, + moduleId, + expression, + ); +} + +// Leaves out object members, since an alias or a call can mutate a const object unseen. +export function resolveStaticStringConstant( + modules: ReadonlyMap, + moduleId: string, + expression: Expression, +): StaticStringValueResolution { + return resolveExpression( + { + modules, + mutatedImportedObjectVariables: new Set(), + seenVariables: new Set(), + followObjectMembers: false, }, moduleId, expression, @@ -250,7 +271,9 @@ function resolveExpression( case 'Identifier': return resolveIdentifierValue(state, moduleId, expression); case 'MemberExpression': - return resolveMemberExpressionValue(state, moduleId, expression); + return state.followObjectMembers + ? resolveMemberExpressionValue(state, moduleId, expression) + : unsupportedExpression(moduleId, expression.type); default: return unsupportedExpression(moduleId, expression.type); } @@ -468,16 +491,6 @@ function resolveObjectPropertyExpression( }; } -function hasExpressionValue(property: Property): property is Property & { value: Expression } { - const { value } = property; - return ( - value.type !== 'ObjectPattern' && - value.type !== 'ArrayPattern' && - value.type !== 'RestElement' && - value.type !== 'AssignmentPattern' - ); -} - export function collectMutatedImportedObjectVariables( modules: ReadonlyMap, ): Set { @@ -654,16 +667,6 @@ function getAliasVariable( return undefined; } -function getStaticPropertyKey(property: Property): string | undefined { - if (property.key.type === 'Identifier') { - return property.key.name; - } - if (property.key.type === 'Literal' && typeof property.key.value === 'string') { - return property.key.value; - } - return undefined; -} - function unwrapStaticExpression(expression: Expression): Expression { let current = expression; 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 5258b61b0..e7c8faf97 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/type-guards.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/type-guards.ts @@ -4,8 +4,10 @@ import type { BaseNode, + Expression, Node, Program, + Property, SimpleLiteral, TemplateLiteral, VariableDeclarator, @@ -42,6 +44,31 @@ export function isStringLiteral(node: unknown): node is StringLiteral { return hasNodeType(node, 'Literal') && 'value' in node && typeof node.value === 'string'; } +export function getStaticPropertyKey(property: Property): string | undefined { + if (property.key.type === 'Identifier') { + return property.key.name; + } + if (property.key.type === 'Literal' && typeof property.key.value === 'string') { + return property.key.value; + } + if (property.key.type === 'Literal' && typeof property.key.value === 'number') { + return String(property.key.value); + } + return undefined; +} + +export function hasExpressionValue( + property: Property, +): property is Property & { value: Expression } { + const { value } = property; + return ( + value.type !== 'ObjectPattern' && + value.type !== 'ArrayPattern' && + value.type !== 'RestElement' && + value.type !== 'AssignmentPattern' + ); +} + export function isTypeOnly(node: TypeOnlyAwareNode): boolean { return node.importKind === 'type' || node.exportKind === 'type'; } diff --git a/packages/plugins/apps/src/backend/client/dev-client/dev-client.test.ts b/packages/plugins/apps/src/backend/client/dev-client/dev-client.test.ts index bf6f91efe..6fc993ad0 100644 --- a/packages/plugins/apps/src/backend/client/dev-client/dev-client.test.ts +++ b/packages/plugins/apps/src/backend/client/dev-client/dev-client.test.ts @@ -653,7 +653,16 @@ describe('dev client', () => { test('Should forward the action list with the bundle', async () => { const { posted, reply } = installFakeWindow(); - const actions = [{ fqn: 'com.datadoghq.http.request', connectionIds: ['conn-1'] }]; + const actions = [ + { + fqn: 'com.datadoghq.http.request', + connectionIds: ['conn-1'], + calls: [ + { connectionId: 'conn-1', inputs: { url: 'https://example.com', verb: 'GET' } }, + { inputs: {} }, + ], + }, + ]; mockFetchResponse({ ok: true, json: async () => ({ ...BUNDLE, actions }) }); const result = postMessageTransport(QUERY_NAME, ARGS); @@ -666,10 +675,68 @@ describe('dev client', () => { const malformedActionLists = [ { description: 'not a list', actions: { fqn: 'com.datadoghq.http.request' } }, - { description: 'an action without an fqn', actions: [{ connectionIds: [] }] }, + { description: 'an action without an fqn', actions: [{ connectionIds: [], calls: [] }] }, { description: 'non-string connection IDs', - actions: [{ fqn: 'com.datadoghq.http.request', connectionIds: [1] }], + actions: [{ fqn: 'com.datadoghq.http.request', connectionIds: [1], calls: [] }], + }, + { + description: 'an action without calls', + actions: [{ fqn: 'com.datadoghq.http.request', connectionIds: [] }], + }, + { + description: 'a call with a non-string connectionId', + actions: [ + { + fqn: 'com.datadoghq.http.request', + connectionIds: [], + calls: [{ connectionId: 1, inputs: {} }], + }, + ], + }, + { + description: 'a call that is not an object', + actions: [{ fqn: 'com.datadoghq.http.request', connectionIds: [], calls: [null] }], + }, + { + description: 'a call without inputs', + actions: [{ fqn: 'com.datadoghq.http.request', connectionIds: [], calls: [{}] }], + }, + { + description: 'an action whose fields are inherited', + actions: [ + Object.create({ fqn: 'com.datadoghq.http.request', connectionIds: [], calls: [] }), + ], + }, + { + description: 'a call whose inputs are inherited', + actions: [ + { + fqn: 'com.datadoghq.http.request', + connectionIds: [], + calls: [Object.create({ inputs: { url: 'https://example.com' } })], + }, + ], + }, + { + description: 'a call whose inputs are a list', + actions: [ + { + fqn: 'com.datadoghq.http.request', + connectionIds: [], + calls: [{ inputs: ['https://example.com'] }], + }, + ], + }, + { + description: 'a call with a non-string input', + actions: [ + { + fqn: 'com.datadoghq.http.request', + connectionIds: [], + calls: [{ inputs: { url: 'https://example.com', timeout: 30 } }], + }, + ], }, ]; diff --git a/packages/plugins/apps/src/backend/client/guards.ts b/packages/plugins/apps/src/backend/client/guards.ts index 28fa66109..8d7881927 100644 --- a/packages/plugins/apps/src/backend/client/guards.ts +++ b/packages/plugins/apps/src/backend/client/guards.ts @@ -2,7 +2,7 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. -import type { DebugBundleAction, DebugBundleResponse } from '../protocol'; +import type { DebugBundleAction, DebugBundleActionCall, DebugBundleResponse } from '../protocol'; export function isRecord(data: unknown): data is object { return data !== null && typeof data === 'object'; @@ -32,20 +32,58 @@ export function isDebugBundle(data: unknown): data is DebugBundleResponse { ); } +function isStringRecord(value: unknown): value is Record { + return isRecord(value) && Object.values(value).every((item) => typeof item === 'string'); +} + +function isDebugBundleActionCall(value: unknown): value is DebugBundleActionCall { + return ( + isRecord(value) && + (!('connectionId' in value) || typeof value.connectionId === 'string') && + 'inputs' in value && + isStringRecord(value.inputs) + ); +} + function isDebugBundleAction(value: unknown): value is DebugBundleAction { return ( isRecord(value) && 'fqn' in value && typeof value.fqn === 'string' && 'connectionIds' in value && - isStringArray(value.connectionIds) + isStringArray(value.connectionIds) && + 'calls' in value && + Array.isArray(value.calls) && + value.calls.every(isDebugBundleActionCall) ); } +// Copies own enumerable entries into a plain object, so no inherited key or accessor survives. +function copyOwnEntries(record: object): Record { + const entries = Object.keys(record).map((key) => [key, Reflect.get(record, key)]); + return Object.fromEntries(entries); +} + +function getOwn(record: object, key: string): unknown { + return Object.prototype.hasOwnProperty.call(record, key) ? Reflect.get(record, key) : undefined; +} + +function copyCall(call: unknown): object | undefined { + if (!isRecord(call)) { + return undefined; + } + const inputs = getOwn(call, 'inputs'); + const hasOwnConnectionId = Object.prototype.hasOwnProperty.call(call, 'connectionId'); + return { + ...(hasOwnConnectionId && { connectionId: Reflect.get(call, 'connectionId') }), + inputs: isRecord(inputs) && !Array.isArray(inputs) ? copyOwnEntries(inputs) : undefined, + }; +} + // A malformed list is dropped on its own, so the bundle it came with is still sent. export function copyDebugBundleActions(bundle: object): DebugBundleAction[] | undefined { try { - const actions = 'actions' in bundle ? bundle.actions : undefined; + const actions = getOwn(bundle, 'actions'); if (!Array.isArray(actions)) { return undefined; } @@ -54,10 +92,12 @@ export function copyDebugBundleActions(bundle: object): DebugBundleAction[] | un if (!isRecord(action)) { return undefined; } - const ids = 'connectionIds' in action ? action.connectionIds : undefined; + const ids = getOwn(action, 'connectionIds'); + const calls = getOwn(action, 'calls'); return { - fqn: 'fqn' in action ? action.fqn : undefined, + fqn: getOwn(action, 'fqn'), connectionIds: Array.isArray(ids) ? copyByIndex(ids) : undefined, + calls: Array.isArray(calls) ? copyByIndex(calls).map(copyCall) : undefined, }; }); return copy.every(isDebugBundleAction) ? copy : undefined; diff --git a/packages/plugins/apps/src/backend/client/transports/post-message-transport/post-message-transport.test.ts b/packages/plugins/apps/src/backend/client/transports/post-message-transport/post-message-transport.test.ts index f8493e80e..f4ae7de6a 100644 --- a/packages/plugins/apps/src/backend/client/transports/post-message-transport/post-message-transport.test.ts +++ b/packages/plugins/apps/src/backend/client/transports/post-message-transport/post-message-transport.test.ts @@ -109,8 +109,16 @@ describe('postMessageTransport', () => { test("Should post only the checked fields of the dev client's action list", async () => { const { posted, reply } = installFakeWindow(); - const action = { fqn: 'com.datadoghq.http.request', connectionIds: ['conn-1'] }; - const extendedActions = [{ ...action, extra: () => 'not cloneable' }]; + const call = { connectionId: 'conn-1', inputs: { url: 'https://example.com' } }; + const action = { + fqn: 'com.datadoghq.http.request', + connectionIds: ['conn-1'], + calls: [call], + }; + const extendedCall = { ...call, extra: () => 'not cloneable' }; + const extendedActions = [ + { ...action, calls: [extendedCall], extra: () => 'not cloneable' }, + ]; const bundle = { ...BUNDLE, actions: extendedActions }; const prepareCall = jest.fn().mockResolvedValue({ targetOrigin: DATADOG_PARENT, bundle }); Reflect.set(globalThis, DEV_CLIENT_HOOK, prepareCall); @@ -128,8 +136,12 @@ describe('postMessageTransport', () => { test("Should copy the dev client's action list without running that array's own methods", async () => { const { posted, reply } = installFakeWindow(); - const action = { fqn: 'com.datadoghq.http.request', connectionIds: ['conn-1'] }; - const forgedAction = { fqn: 'com.datadoghq.forged', connectionIds: ['conn-forged'] }; + const action = { fqn: 'com.datadoghq.http.request', connectionIds: ['conn-1'], calls: [] }; + const forgedAction = { + fqn: 'com.datadoghq.forged', + connectionIds: ['conn-forged'], + calls: [], + }; class ForgingArray extends Array {} Reflect.defineProperty(ForgingArray, Symbol.species, { get: () => @@ -156,6 +168,92 @@ describe('postMessageTransport', () => { }); }); + test("Should copy a call's inputs as own string entries, without running the calls array's own methods", async () => { + const { posted, reply } = installFakeWindow(); + const forged = { map: () => forged, every: () => true, extra: () => 'not cloneable' }; + const calls = [{ inputs: { url: 'https://example.com' } }]; + Object.defineProperty(calls, 'constructor', { + value: { [Symbol.species]: () => forged }, + }); + const inputs = Object.create({ inherited: 'not copied' }); + inputs.verb = 'GET'; + const actions = [ + { fqn: 'com.datadoghq.http.request', connectionIds: [], calls }, + { fqn: 'com.datadoghq.slack.chat.postMessage', connectionIds: [], calls: [{ inputs }] }, + ]; + const bundle = { ...BUNDLE, actions }; + const prepareCall = jest.fn().mockResolvedValue({ targetOrigin: DATADOG_PARENT, bundle }); + Reflect.set(globalThis, DEV_CLIENT_HOOK, prepareCall); + + const result = postMessageTransport(QUERY_NAME, ARGS); + const message = await posted; + reply(message.requestId, 'ok'); + + await expect(result).resolves.toBe('ok'); + const expectedActions = [ + { + fqn: 'com.datadoghq.http.request', + connectionIds: [], + calls: [{ inputs: { url: 'https://example.com' } }], + }, + { + fqn: 'com.datadoghq.slack.chat.postMessage', + connectionIds: [], + calls: [{ inputs: { verb: 'GET' } }], + }, + ]; + expect(message).toEqual({ + ...MESSAGE_WITHOUT_BUNDLE, + bundle: { ...BUNDLE, actions: expectedActions }, + }); + }); + + test("Should copy a call's connectionId only when the call sets it itself", async () => { + const { posted, reply } = installFakeWindow(); + const call = Object.create({ connectionId: 'conn-inherited' }); + call.inputs = {}; + const actions = [{ fqn: 'com.datadoghq.http.request', connectionIds: [], calls: [call] }]; + const bundle = { ...BUNDLE, actions }; + const prepareCall = jest.fn().mockResolvedValue({ targetOrigin: DATADOG_PARENT, bundle }); + Reflect.set(globalThis, DEV_CLIENT_HOOK, prepareCall); + + const result = postMessageTransport(QUERY_NAME, ARGS); + const message = await posted; + reply(message.requestId, 'ok'); + + await expect(result).resolves.toBe('ok'); + const expectedActions = [ + { fqn: 'com.datadoghq.http.request', connectionIds: [], calls: [{ inputs: {} }] }, + ]; + expect(message).toEqual({ + ...MESSAGE_WITHOUT_BUNDLE, + bundle: { ...BUNDLE, actions: expectedActions }, + }); + }); + + test("Should post the bundle without its action list when reading a call's inputs throws", async () => { + const { posted, reply } = installFakeWindow(); + const inputs = Object.defineProperty({}, 'url', { + enumerable: true, + get(): never { + throw new Error('unreadable'); + }, + }); + const actions = [ + { fqn: 'com.datadoghq.http.request', connectionIds: [], calls: [{ inputs }] }, + ]; + const bundle = { ...BUNDLE, actions }; + const prepareCall = jest.fn().mockResolvedValue({ targetOrigin: DATADOG_PARENT, bundle }); + Reflect.set(globalThis, DEV_CLIENT_HOOK, prepareCall); + + const result = postMessageTransport(QUERY_NAME, ARGS); + const message = await posted; + reply(message.requestId, 'ok'); + + await expect(result).resolves.toBe('ok'); + expect(message).toEqual({ ...MESSAGE_WITHOUT_BUNDLE, bundle: BUNDLE }); + }); + test('Should post the bundle without its action list when reading that list throws', async () => { const { postMessage, posted, reply } = installFakeWindow(); const bundle = Object.defineProperty({ ...BUNDLE }, 'actions', { diff --git a/packages/plugins/apps/src/backend/protocol.ts b/packages/plugins/apps/src/backend/protocol.ts index cdb254fd3..0fc57188a 100644 --- a/packages/plugins/apps/src/backend/protocol.ts +++ b/packages/plugins/apps/src/backend/protocol.ts @@ -29,9 +29,23 @@ export const PONG = 'app-builder:pong'; // The `globalThis` key where the dev client the dev server injects installs its call hook. export const DEV_CLIENT_HOOK = 'DD_APPS_PREPARE_CALL'; +export interface DebugBundleActionCall { + /** Absent when the call sets no `connectionId`. */ + connectionId?: string; + /** + * Inline `inputs` keys set to a string literal or a top-level string const, such as + * http.request's `url` and `verb`. Empty when `inputs` isn't an object literal of plain static + * keys, the options or `inputs` literal has an accessor, or `inputs` has a toJSON, then or + * __proto__ key. A hook inherited from a prototype or added by an executor wrapper isn't detected. + */ + inputs: Record; +} + export interface DebugBundleAction { fqn: string; connectionIds: string[]; + /** One entry per call site. */ + calls: DebugBundleActionCall[]; } export interface DebugBundleResponse { 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 e5d14d50b..fa378d326 100644 --- a/packages/plugins/apps/src/vite/dev-server.integration.test.ts +++ b/packages/plugins/apps/src/vite/dev-server.integration.test.ts @@ -545,8 +545,21 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { const body = JSON.parse(rawBody); expect(body.allowedConnectionIds).toEqual(['conn-1', 'conn-http']); expect(body.actions).toEqual([ - { fqn: 'com.datadoghq.http.request', connectionIds: ['conn-http'] }, - { fqn: 'com.datadoghq.slack.chat.postMessage', connectionIds: ['conn-1'] }, + { + fqn: 'com.datadoghq.http.request', + connectionIds: ['conn-http'], + calls: [ + { + connectionId: 'conn-http', + inputs: { url: 'https://example.com', verb: 'GET' }, + }, + ], + }, + { + fqn: 'com.datadoghq.slack.chat.postMessage', + connectionIds: ['conn-1'], + calls: [{ connectionId: 'conn-1', inputs: { text: 'hi' } }], + }, ]); }, 30000); diff --git a/packages/plugins/apps/src/vite/dev-server.test.ts b/packages/plugins/apps/src/vite/dev-server.test.ts index f5eef3340..97432ba4c 100644 --- a/packages/plugins/apps/src/vite/dev-server.test.ts +++ b/packages/plugins/apps/src/vite/dev-server.test.ts @@ -767,8 +767,16 @@ describe('Dev Server Middleware', () => { code: '// bundled', allowedConnectionIds: ['conn-http'], actions: [ - { fqn: 'com.datadoghq.http.request', connectionIds: ['conn-http'] }, - { fqn: 'com.datadoghq.slack.chat.postMessage', connectionIds: [] }, + { + fqn: 'com.datadoghq.http.request', + connectionIds: ['conn-http'], + calls: [{ connectionId: 'conn-http', inputs: {} }], + }, + { + fqn: 'com.datadoghq.slack.chat.postMessage', + connectionIds: [], + calls: [{ inputs: {} }], + }, ], }); expect(mockLogFn).not.toHaveBeenCalledWith(NO_ACTION_FQNS_MESSAGE, 'warn'); diff --git a/packages/plugins/apps/src/vite/dev-server.ts b/packages/plugins/apps/src/vite/dev-server.ts index 02ac8d273..063322d40 100644 --- a/packages/plugins/apps/src/vite/dev-server.ts +++ b/packages/plugins/apps/src/vite/dev-server.ts @@ -228,7 +228,7 @@ async function bundleBackendFunction( if (calls.length > 0 && !hasPackageExportFqns) { warnNoActionFqns(); } - const actions = listActionCalls(calls, exportFqnsByModule); + const actions = listActionCalls(calls, exportFqnsByModule, moduleRecords); if (!actions) { const missingListMessage = getMissingActionListMessage(displayName); log.debug(missingListMessage);