From e1a7856e52ec11c66b9f4c01775342e0b9cbfab4 Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Thu, 22 Oct 2026 16:58:55 -0400 Subject: [PATCH] feat(apps): list each action call's static inputs in the debugBundle response Each debugBundle action carries a calls list with one entry per call site: its connectionId, when set, and the inline inputs keys whose value the build resolves to a string from a literal, a template without expressions or a const binding, such as http.request's url and verb. Any other input is left out, so a consent prompt can show it as set when the call runs. That includes an object member, since an alias or a call can mutate a const object without the mutation walk seeing it. Every input is left out when the options or inputs object literal has a getter, setter, spread, computed key, or a toJSON, then or __proto__ key, since serializing or awaiting the call could rewrite any key. A hook inherited from a prototype or added by an executor wrapper isn't detected. The dev client and the frame transport copy only the action list's own fields, so an inherited fqn, connectionId or inputs key never survives, and drop a list whose inputs are an array or hold a non-string value. The static property-key and expression-value helpers live in type-guards.ts, shared by the connection-ID extractor and the string resolver, and read a numeric literal key as its string. Co-Authored-By: Claude Opus 5.5 --- ...xtract-connection-ids-from-module-graph.ts | 23 +- .../ast-parsing/list-action-calls.test.ts | 306 +++++++++++++++++- .../backend/ast-parsing/list-action-calls.ts | 114 ++++++- .../ast-parsing/static-string-resolution.ts | 45 +-- .../src/backend/ast-parsing/type-guards.ts | 27 ++ .../client/dev-client/dev-client.test.ts | 73 ++++- .../plugins/apps/src/backend/client/guards.ts | 50 ++- .../post-message-transport.test.ts | 106 +++++- packages/plugins/apps/src/backend/protocol.ts | 14 + .../src/vite/dev-server.integration.test.ts | 17 +- .../plugins/apps/src/vite/dev-server.test.ts | 12 +- packages/plugins/apps/src/vite/dev-server.ts | 2 +- 12 files changed, 712 insertions(+), 77 deletions(-) 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);